[Bug] Agent loop: dangling tool/call without paired tool/result when a parallel tool call fails mid-dispatch — deriveMessages() builds a provider-invalid transcript

[wangjianyuweg]

简述（中文）：并行工具调用部分失败时，runGroup() 的 catch 块直接重抛，不为已启动未提交的调用合成 tool/result——日志里留下无配对结果的 tool/call，deriveMessages() 投影出畸形 transcript 发给 provider 会被拒，且重载持久化也无法修复。这与文件 docstring「scheduler failure 不伪造结果」的既有设计意图冲突，想请维护者裁决方向。

Scenario

Found while reviewing the agent loop (packages/core/agent-loop/src/tool-calls.ts, the runGroup() catch block, lines ~228–234 at dsh-v0.1.0-rc.7):

The catch block in question:

1. One step's assistant/message carries parallel tool calls A and B. startCall appends tool/call for both (started = 2).
2. B's dispatch(...) rejects (any tool execution error, including an AbortError on abort). The rejection handler sets schedulerFailure and does not set slots[B].
3. throwSchedulerFailure() throws; the catch block drains in-flight promises but never calls commitReady() again — so neither A's nor B's tool/result is appended — then rethrows. The turn ends with turn/end { kind: 'error' }.
4. The log now holds assistant/message (with A + B tool-call blocks), two tool/call events, and zero tool/result events. tool/call is not a surface event, so deriveMessages() projects an assistant turn with tool-call blocks and no tool-result blocks.
5. The next turn's buildRequest() sends that malformed transcript to the provider (Anthropic/OpenAI reject it or misread it).
6. The only repair path, interruptedTurnClosers (packages/session/session-persistence/src/coordinator.ts), returns [] for a closed turn/end{error} turn — so reloading persistence does not recover it.

Design-intent conflict

The file docstring says "A terminal scheduler failure preserves already-recorded tool/call events without fabricating results" — so the current behavior is deliberate. But this post argues that intent conflicts with the derived-surface validity requirement: not fabricating a result produces an invalid transcript on the very next request, which seems strictly worse than a synthetic error result.

Proposed direction

Parallel to what the abort path already does for unstarted calls via appendSkippedToolCall — in the catch block, after Promise.allSettled(inFlight.values()), synthesize an error tool/result for every started-but-uncommitted call:

for (let i = committed; i < started; i++) {
  appendToolResult(session, turn, step, group[i]!.block, {
    content: [{ type: 'text', text: 'Error: tool call failed during dispatch' }],
    isError: true,
    error: { message: 'tool call failed during dispatch', info: { name: 'SchedulerError', code: ... } },
  }, callSeqs[i]!)
}
throw schedulerFailure.error

callSeqs[i] already holds the tool/call seq, so the pair stays linked.

Would maintainers prefer repairing at append time (above), at replay time (interruptedTurnClosers), or is the current shape considered acceptable? Happy to send a patch via a fork branch either way.

[zoahdev]

这个根因和 #3524 完全一致——runGroup() 的 catch 块在 tool-calls.ts:228-234 直接重抛，不为已提交的 tool/call 补写 tool/result，导致会话残留未配对 tool_calls，之后每轮被上游 400 拒绝。\n\n我们在 #3524 回复里详述了六步链（#3234 家族）：assembler.ts / tool-calls.ts / serialize.ts / apiproxy 裸 parse / load 不愈合，以及 #1697 双副本 Symbol 不匹配的触发条件。修复方向：startCall 里 appendToolCall 之后 try/catch 兜底补写一条 content=错误描述的 tool/result，或回滚整条 tool_calls 声明。\n\n你提到的 deriveMessages() 投影出畸形 transcript 是另一个影响面：即使上游不拒绝，畸形 transcript 也会让工具调用输出不可读。建议同步修复 load 时对未配对 tool_calls 的确定性修复。

[wangjianyuweg]

简述（中文）：按您在 #3524 定下的落地顺序，我把社区两个补丁（@DON738110198 的调度器失败配对、您的 Symbol.for 协议守卫）与缺的两块——load 时确定性修复（deriveMessages() 投影层的 repairDerivedTranscript）和 #3234 的线上 safeArguments 回退——组成了一个分支，在干净的 0.1.1-rc.2 上红/绿验证通过，附复现与修复前后对比截图。

Thanks @zoahdev — following the landing order from your reply (and #3524), I've assembled a branch that combines the community patches with the two missing pieces, verified red/green on clean master (dsh-v0.1.1-rc.2 @ b150a55):

Branch: fix/unpaired-tool-call-recovery · diff vs master — 8 commits:

1. New-failure prevention — cherry-picked @DON738110198's fix/scheduler-failure-tool-results unchanged (startCall/catch-block pairing, TOOL_SCHEDULER_FAILED_BEFORE_DISPATCH vs TOOL_SCHEDULER_OUTCOME_UNKNOWN).
2. The Bug: installing any plugin that depends on @deepseek-ai/dsh-tools breaks every tool call (Cannot read properties of undefined (reading 'prepare')) #1697 trigger — ported your fix/tool-runtime-scheduler-symbol-for (Symbol.for + protocol guard); the port also wraps the code-mode.ts scheduler read site, which the original branch predates.
3. Load-time deterministic repair (the piece your reply asked for) — a new pure repairDerivedTranscript(): Session.deriveMessages() now closes every assistant tool-call that lost its result to a closed error turn, with the same two-state guidance as the crash-tail repair (outcome-unknown vs not-started). Nothing is written to the log — the repair is a deterministic projection of the same events, so every rebuild reproduces the identical transcript and the request invariant (options.messages === deriveMessages()) stays intact.
4. The [Bug Report] Streamed tool-call arguments with truncated JSON are persisted, then every subsequent turn fails with `400 INVALID_REQUEST: Unterminated string ...` in the same session forever #3234 half — serializeAssistant degrades unparseable tool-call arguments to {} on the wire (mirroring dsh-llm-pi-ai's tolerance), so history poisoned before any fix stops 400ing immediately. Current master's schema gate already refuses to execute truncated-argument calls (ToolArgsError / INVALID_ARGS) — I pinned that with a regression test; the residual poison was only the verbatim resend.

Reproduction → fix (same session log, two builds):

Red/green on clean master — packages/core/session/tests/derived-repair.spec.ts (public session API only) fails on unpatched master; with the fix, the session (292), agent-loop (336, including the scheduler-failure pairing tests), tools, llm-deepseek and persistence suites pass (1866-test sweep), and the full workspace typecheck exits 0:

One design note: the repair had to live in the derivation rather than at load — a tail-appended tool/result lands at the transcript end (the surface fold projects events in log order), never next to its assistant message, and mid-log insertion would break the append-only log and seq contiguity. Alternatives (load-time append, request-build drop, persistence-only repair) are recorded in the Agent Notes in the branch.

Also confirmed while assembling this: the #3234 apiproxy noise (viewFor / backscanArgs bare parse) is already soft-falled on current master.

[wangjianyuweg]

简述（中文）：上面的回复发出后，分支又经两轮独立代码评审，从 8 个 commit 演进到 24 个：配对升级为 per-occurrence（同一 call id 在不同闭合轮次悬空时各自独立应答）、agent-loop 闭环按失败来源分类且自身异常安全、修复合成计入 token 账本（repairTokens）、compaction 不再被终态悬空调用永久阻塞，并新增两个 keyless 快照端到端锁定。附新的前后对比：同一份毒化日志、真实 DeepSeek API——修复前 400（与本帖报告的报错逐字一致），修复后 200 且模型按合成指引判断 echo 只读幂等、决定安全重试。

Update since the comment above: the branch has been through two more rounds of independent code review and grew from 8 to 24 commits. Nothing above is withdrawn — the mechanisms were tightened and the evidence upgraded.

What changed under review:

1. Per-occurrence pairing. The repair no longer keys on call ids — each occurrence of a dangling call is matched to its nearest preceding assistant tool-call, so an id that dangles in two different closed turns is answered twice, each in its own turn. The tool-pairing balance that guards compaction cuts got the same semantics, orphaned late results fold neutrally, and a spec pins that the incremental (live) derivation stays identical to a cold rebuild.
2. Failure-origin closure. The scheduler-failure cherry-pick above is no longer untouched: every unpaired call is closed by failure origin — TOOL_SCHEDULER_FAILED_BEFORE_DISPATCH vs TOOL_SCHEDULER_OUTCOME_UNKNOWN vs TOOL_STEP_* (a call whose tool genuinely ran and failed keeps its exact result) — the closing pass is itself exception-safe, and skipped calls on the abort path are recorded durably too.
3. Honest token accounting. Repair synthetics are priced into the token ledger (repairTokens), on both the anchored and unanchored paths.
4. Compaction unblocked. A terminal unanswered call no longer pins the cut window forever — it retires, and a late real result answering a retired call folds neutrally.
5. Two keyless end-to-end snapshots (scheduler-failure-closure, derived-repair) drive the real headless composition: the scheduler fails mid-dispatch → the log closes → the session resumes from persistence → the next request carries the synthetic result verbatim.

Live before/after — same poisoned log, same key, both requests built by the product's own DeepSeek serializer and sent to api.deepseek.com:

Left: master's rebuild path (no repair) — HTTP 400, verbatim the error this thread reported. Right: Session.deriveMessages() — HTTP 200, and the model's answer shows the guidance working: it notes the call's outcome is unknown, reasons that echo derived-repair-probe is read-only and idempotent, and decides a retry is safe. The only variable between the columns is the derivation path.

Fresh numbers on the 24-commit head: session 310, agent-loop 345 (including the per-occurrence and failure-origin specs — derived-repair.spec.ts alone grew 4→14 tests), tools 394, llm-deepseek 342, session-persistence 503 (sweep: 1894 passed | 2 skipped); both snapshots above pass; workspace typecheck exits 0; translation pairing 1011 pairs. The branch/diff links in the comment above now show the full 24 commits.