Empty tool calls (name/callId empty) are persisted without validation, then fail the whole session load as corrupt

[lazytiger]

Summary

@deepseek-ai/dsh@0.1.2-rc.1 accepts a degenerate tool call (empty name/id/arguments) from a model response and persists the result chain as-is. On the next load, the strict message-shape validation (@deepseek-ai/dsh-session, same version) rejects the stored tool/result and marks the entire session corrupt — the conversation history cannot be opened and the provider can no longer observe/resume the session. There is no recovery path short of manually editing the zstd frame container.

This happened on multiple sessions across two machines (Windows + Linux), with 15–16 such chains in a single long session.

Environment

• @deepseek-ai/dsh 0.1.2-rc.1 (also @deepseek-ai/dsh-session 0.1.2-rc.1)
• Node 25 (Windows / Linux)
• OpenAI-completions–style adapters (dsh-llm-pi-ai), model deepseek-v4-flash-vision-exp served via local/custom providers

What gets persisted

A model turn ended with a tool call block carrying empty fields:

{"type": "tool-call", "id": "", "name": "", "arguments": "{}"}

The harness then recorded (no validation at the append boundary):

{"type": "tool/call", "seq": 709, "data": {"turn": 2, "step": 3, "callId": "", "name": "", "arguments": "{}"}}

{
  "type": "tool/result",
  "seq": 710,
  "data": {
    "turn": 2, "step": 3,
    "message": {
      "source": {"kind": "tool", "callId": ""},
      "content": [{"type": "tool-result", "toolCallId": "", "isError": true, "content": [{"type": "text", "text": "Error: unknown tool \"\""}]}],
      "role": "user",
      "id": "1121ab3f-4834-4b6e-be7f-a890cc8c0ddd"
    },
    "error": {"name": "ToolNotFoundError", "code": "UNKNOWN_TOOL"}
  },
  "sourceEventSeqs": [709],
  "surfaceOp": "append"
}

What breaks on load

History load (session query) fails with:

stored session "<id>" is corrupt: stored session "<id>" failed validation: Error: session event at seq 710 message must have tool source

because assertMessageEventShape (dsh-session lib/index.js) requires tool/result messages to have source.kind === "tool" and a non-empty source.callId (plus block.toolCallId === source.callId). The event was allowed to be written with callId: "" but can no longer be read back — a write/read validation asymmetry within the same build.

Provider resume/observe then fails from the daemon side:

dsh rpc error (code=gateway/internal): failed to observe session "<id>": ...

Impact

• The affected conversation's full history is unreadable in the UI.
• The agent cannot start or resume — its session is unobservable, so every start attempt fails.
• No CLI/flag lets you drop the offending event; recovery requires manual decompression/patch/re-compression of the .jsonl.zstd frame container.

Suggested fixes

1. Write boundary: when the model output yields a tool call with empty name (or empty id), reject it (raise/fail the step or retry once with a steering message) instead of persisting a degenerate tool/call+tool/result chain. At minimum, never persist a tool/result with an empty callId.
2. Read boundary: quarantine a single malformed message event during load (log a warning, skip just that event/tool-result) rather than failing the whole session — one lost tool result is far less harmful than an unloadable conversation. Keep strict validation for fresh/seed logs if a hardened mode is preferred.
3. Recovery: a documented CLI repair/export path for already-affected sessions (e.g., validate-and-drop or --repair) would save users manual binary surgery.

Repro sketch

1. Use any OpenAI-completions-style provider; cause the model to emit a tool call with empty name (observed with deepseek-v4-flash-vision-exp).
2. Let the step fail with ToolNotFoundError (unknown tool ""); the chain above is persisted.
3. Reload the session (open history / resume) → "corrupt: … message must have tool source".

[argszero]

Thanks — the asymmetry is real, and sharper than "no validation at the append boundary": the validator that rejects your event already exists, it is just never called on the write path.

Where the rule lives vs. where it is enforced

The throw in your error text is packages/core/session/src/index.ts:371-375, inside assertMessageEventShape:

if (sourceRecord['kind'] !== 'tool'
  || typeof sourceRecord['callId'] !== 'string'
  || sourceRecord['callId'] === '') {
  throw new Error(`${subject} message must have tool source`)
}

That function has exactly two call sites: adoptSessionEvent (:169, the import/read boundary) and the seed-envelope validator (:272). Session.append (:703) never calls it. Append runs snapshotJsonValue, validateSessionEventData (packages/core/session/src/surface.ts:140) and surfaceManager.validateNext instead — and the tool/result branch of validateSessionEventData inspects only error → content[0].isError === true, returning early when error is undefined. It never looks at source.callId.

So exactly one rule is missing on the write side, and append's own docstring promises the opposite: "a bad event fails at the append site rather than later during a backend flush."

Your fix 1 can be smaller than "add validation"

validateSessionEventData is already called by both Session.append and the seed path. Moving the non-empty-callId (plus toolCallId === source.callId) requirement into its tool/result branch closes the asymmetry at the single shared seam — no new call site, and seed/import gets it for free. Reusing assertMessageEventShape from append lands in the same place by construction.

Where the degenerate block is born (not in the report)

packages/llm/llm/src/assembler.ts:114-118:

id: partial.toolCallId ?? brandString<ToolCallId>(`call-${index}`),
name: partial.toolCallName ?? '',

The id has a fallback and the name has none — but that fallback never fires for your case either, because :73 assigns partial.toolCallId = chunk.id unconditionally. On the OpenAI-completions path the empty id is a designed sentinel, not an omission: packages/llm/llm-pi-ai/src/stream.ts:184-186 reads partial.id (with '' as its own fallback) and emits brandString<ToolCallId>(known?.id ?? ''). '' is not undefined, so the assembler's ?? never substitutes. Note the same lines omit an empty name entirely — your name: "" is produced later by assembler.ts's ?? '' — while the id is carried through as ''.

Worse, the harness already detects this shape and declines to reject it. packages/llm/llm/src/assistant-stream.ts:138-141:

if (chunk.id.length === 0 || chunk.name === '') {
  this.records.push({ type: 'chunk', time, chunk })
  return timed
}

while the coalesced record it is steered away from has a strict counterpart — tool-call-chunks id must be a non-empty string / name must be a non-empty string (:476, :480). The degenerate chunk is routed into the opaque {type:'chunk'} record precisely so it never meets that validator, which is why it survives persistence and read-back verbatim instead of being dropped at the stream boundary. Two independent sites know the identity is invalid; both degrade instead of rejecting.

An invariant for exactly your suggestion 1 exists — it just is not mounted

packages/core/tools/src/invariant.ts:28-30:

if (exec.name.length === 0 || String(exec.callId).length === 0) {
  fail('tools/result execution must carry non-empty name and callId')
}

That is your fix 1, already written as a testable contract. But @deepseek-ai/dsh-invariants is mounted in exactly one runnable config — packages/bundle/sdk-minimal/cordis.patch.yml:103. web-app, headless, base, acp-app and sdk-app reference it zero times. So it is enforced in dev/SDK-minimal and silently unenforced in every shipped profile. If maintainers want a cheap intermediate step before settling the append-policy question, mounting tools/invariant in the shipped profiles turns silent corruption into a loud, attributable failure.

Two cautions on suggestion 2

The shape rules are load-bearing: the tool/result branch also enforces toolCallId === source.callId and the single-block content shape, and packages/core/session/src/repair.ts correlates crash-recovery closers by call id (:58/:62) — its comment notes providers reject dangling assistant calls. So quarantine the call/result pair, not the single result event: dropping only the tool/result leaves a dangling call the next load must repair. Related, and worth accounting for given your 15–16 chains: pendingCalls is keyed by block.id, so two degenerate calls in one step collapse to one map entry and yield a single synthetic closer.

For anyone who needs a workaround today

llm/stream is a waterfall whose listener returns its own AsyncIterable<StreamChunk> (packages/llm/llm/src/index.ts:72, dispatch at :1116), and tool-call-delta carries index/id/name/argumentsDelta (packages/llm/llm/src/types.ts:394). Substituting a non-empty id there is enough to keep the log loadable — the read rule rejects only an empty source.callId, and an empty name merely yields the UNKNOWN_TOOL result you already saw. I maintain @argszero/cordis-plugin-llm-tool-call-guard on that seam and this is a natural extension. Being explicit about the boundary: that is a mitigation, not the fix — a plugin cannot prevent the append, so the authoritative repair is still closing the write boundary.

One thing I could not confirm from source

The "every start attempt fails" step. ToolNotFoundError is materialized, not thrown, at dispatch (packages/core/tools/src/index.ts:1429 → toolErrorResult), so the degenerate chain is produced by the normal path and the session should still be startable. If you still have the machine, the error/stack from the start path (as opposed to history load) would show whether it is this same assertMessageEventShape throw reached through gateway observe or a different choke — which decides whether fix 1 alone is sufficient.

[argszero]

Follow-up: the producer-side mitigation I described now exists as a published plugin, so anyone hitting this today can recover without waiting for the write-boundary fix.

@argszero/cordis-plugin-llm-tool-call-guard@0.1.4 — https://www.npmjs.com/package/@argszero/cordis-plugin-llm-tool-call-guard (source: https://github.com/argszero/cordis-plugin-llm-tool-call-guard)

npm install @argszero/cordis-plugin-llm-tool-call-guard

Mount it in a dsh profile:

- insert:
    - id: llm-tool-call-guard
      name: '@argszero/cordis-plugin-llm-tool-call-guard'

New in 0.1.4 is an identity guard. Defaults are on, so mounting is enough:

• repair: 'repair' (default) rewrites an empty id to a deterministic synthetic id. It rewrites both the tool-call-delta chunks and the block-end block — repairing only the deltas would be a silent no-op, because the core BlockAssembler treats block-end as authoritative and overwrites the accumulated values (I verified this against the real assembler, not a mock: the built package's regression test pushes the repaired stream through BlockAssembler and asserts the assembled id is non-empty).
• An empty name is deliberately left alone. The harness already turns it into a ToolNotFoundError / UNKNOWN_TOOL result, which is an honest and resumable outcome. Blanking it would make the call vanish; inventing one would fabricate a call the model never requested. So the repaired chain looks like tool/call{callId:"repaired-call-N"} + tool/result{source:{kind:"tool",callId:"repaired-call-N"}, error:{code:"UNKNOWN_TOOL"}} — loadable, and truthful about what happened.
• repair: 'error' cuts the stream with a terminal error finish (TOOL_CALL_EMPTY_IDENTITY) instead, so the degenerate call is never executed at all.
• repair: 'off' preserves the previous behaviour byte-for-byte.
• Every repaired block is reported once at warn level, naming which field was empty — since "nothing was visible" is the heart of the report.

Two honest caveats, so nobody over-reads this:

1. This repairs the stream, not the log. It stops new corrupt sessions from being written; it does not make an already-corrupt log loadable. Those still need the write-boundary fix (or a repair path).
2. Whether an empty name is something the model should be allowed to emit at all is a separate question from whether the resulting chain should be unloadable. The plugin takes the narrow position — make it loadable, and let UNKNOWN_TOOL surface it — precisely because that decision belongs upstream, not to a plugin.

I still owe this thread a retest prompt rather than a claim: if you re-run the same provider and the guard is mounted, the session should survive the reload. If it does not, that is a bug in the plugin and I would want the frame.