Skip to content

fix(responses): keep tool call/output pairs adjacent on the wire (#766) - #768

Merged
ranxianglei merged 2 commits into
masterfrom
2026-09-14_fix-responses-orphan-tool-output
Sep 14, 2026
Merged

ranxianglei merged 2 commits into
masterfrom
2026-09-14_fix-responses-orphan-tool-output

Conversation

@ranxianglei

Copy link
Copy Markdown
Owner

Problem

Strict Responses-API backends (DeepSeek) reject a request whose function_call_output does not directly follow its call group. A single non-tool item (a message/reasoning) wedged between a function_call and its matching output yields:

400 {"error":{"message":"No tool output found for tool call call_00_Jix34GFEsxl1dwSXmPPF0629",...}}

even though both items are present — pairing is by position/adjacency, not just by call_id. The client records the malformed [function_call, message, function_call_output] sequence into its session history, so every subsequent forward fails identically and the session is permanently wedged until the client history (codex jsonl) is dropped.

Root cause

surfaceReadonlyJson (src/compress-loop-reponses.ts) appended the acp_status visibility marker as an assistant message after a real function_call in the same response. The client then recorded [function_call, assistant_marker, function_call_output], which DeepSeek rejects.

Fix (two parts)

1. Wire-boundary guard — heals already-poisoned sessions without dropping the jsonl.
New src/tool-pair-order.ts::hoistTrappedToolItems() hoists any non-tool item trapped between a call and its matching output to just before the earliest trapping call, restoring adjacency. It is a pure no-op (same array reference) on healthy input, so well-formed sessions stay byte-stable for the prefix cache. Enforced at every Responses input-build site:

  • unified streaming buildRequest (src/loop/adapter-responses.ts) — the live path serving codex/deepseek;
  • non-streaming prepareResponses (src/server.ts);
  • legacy JSON replay (src/compress-loop-responses.ts).

Because it runs on the outbound request, an already-corrupted session self-heals on the very next forward — no manual jsonl drop required.

2. Root cause. surfaceReadonlyJson now inserts the visibility marker before the first tool call instead of appending after it, so the marker can no longer wedge a pair at the source.

Verification

  • npm run typecheck — clean
  • npm test — 1469 pass / 0 fail / 2 skipped (e2e-gated)
  • tests/tool-pair-order.test.ts — 10 new cases incl. the exact [BUG] No tool output found for tool call call_xxxx #766 pattern, parallel calls, reasoning trap, custom_tool_call variant, healthy-input no-op (strictEqual same ref)
  • npm run build — ok (new module bundled inline)
  • E2E preflight (E2E_CHECK=1) passes (codex 0.147.0 + dist); full 4-phase run needs a live upstream not present in this sandbox (CI e2e is manual-dispatch with secrets)

Fixes #766

Strict Responses backends (DeepSeek) reject a request whose function_call_output does not directly follow its call group: a message/reasoning item wedged between a call and its output yields 'No tool output found for tool call <id>' even though both items exist, permanently wedging the session until the client history is dropped.

Root cause: surfaceReadonlyJson appended the acp_status visibility marker as an assistant message AFTER a real function_call in the same response, so the client recorded [function_call, message, function_call_output].

- New src/tool-pair-order.ts: hoistTrappedToolItems() hoists any non-tool item trapped between a call and its matching output to just before the earliest trapping call. No-op (same array ref) on healthy input, so well-formed sessions stay byte-stable for the prefix cache.
- Enforce at every Responses input-build site: unified streaming buildRequest (adapter-responses.ts), non-streaming prepareResponses (server.ts), and the legacy JSON replay (compress-loop-responses.ts). This also heals already-poisoned sessions without dropping the jsonl.
- Fix the root cause in surfaceReadonlyJson: insert the marker before the first tool call instead of appending after.

typecheck clean, 1469 pass / 0 fail / 2 skipped (e2e-gated), build ok.
@github-actions

github-actions Bot commented Sep 14, 2026 •

Copy link
Copy Markdown
Contributor

📦 Built Package Artifact

Branch: 2026-09-14_fix-responses-orphan-tool-output (dec6c5b)

Option A — Install from npm PR tag (recommended)

npm install -g billion-context@pr-768

Each push to this PR publishes a new version under the pr-768 npm tag.

Option B — Download artifact

  1. Download the artifact from the Actions run
  2. Extract the tarball and install:
tar xzf billion-context-pr768.tgz
npm install -g package

This comment is automatically updated on each push.

…e path (#766)

prepareResponsesCompact returns its forged array as codex's new recorded
session history; a non-tool item wedged between a call and its output there
would re-wedge the session on the next forward. Apply the same
hoistTrappedToolItems guard as the other Responses input-build sites (no-op on
healthy input, so prefix-cache byte-stable).
@ranxianglei

Copy link
Copy Markdown
Owner Author

🤖 Powered by ework · qwen3.8-27b

[bot] 🏷 Reviewed PR #768 (2026-09-14_fix-responses-orphan-tool-output) — fixes the DeepSeek wedge (#766). Verdict below; I pushed one follow-up commit to the branch (dec6c5b).

Diff cleanliness (first gate): clean. One logical change, 5 files (+204). No unrelated churn, no lockfile/version drift, no whitespace noise; every line serves the stated purpose. ✅

Bug is real and correctly layered. Strict Responses backends pair function_call_output to its call by adjacency, not just call_id, so a message/reasoning item wedged between them yields No tool output found for tool call <id> even though both items exist. Because the client records the malformed sequence into its jsonl history, every later forward fails identically until the session file is dropped — that "permanently wedged" behavior is exactly what makes an outbound self-heal worthwhile. You split it right: (a) a wire-boundary guard that heals already-poisoned sessions, (b) the source fix in surfaceReadonlyJson (insert the visibility marker before the first tool call instead of appending after it). Both hit the core — not a symptom-only patch.

Re-verified locally (fresh npm ci against the PR lock):

  • typecheck — clean
  • npm test — 1469 pass / 0 fail / 2 skipped (matches your claim; the 2 are e2e-gated, expected without a live upstream)
  • build — ok, hoistTrappedToolItems bundled inline
  • Confirmed injectResponsesDeveloperMessage inserts at index 0 (after any leading additional_tools) and the nudge .push() appends at the end — so both run after the hoist can't reintroduce a trap. Placing the hoist before them is safe.
  • Branch is master + 1 commit, merges cleanly (no divergence/conflicts).
  • Probed the guard directly: the exact [BUG] No tool output found for tool call call_xxxx #766 shape [function_call, message, function_call_output] → [message, function_call, function_call_output] with 0 remaining traps; healthy input returns the same array ref (prefix-cache byte-stable). ✅

Gap I fixed directly (commit dec6c5b): the codex /responses/compact endpoint forge path — prepareResponsesCompact in src/server.ts (now ~line 2737) — builds its forged array via patchResponsesInput and returns it as {"output": [...]}, which becomes codex's new recorded session history. That's the exact poison vector from #766, yet it was the one Responses input-build site without the guard. Added output = hoistTrappedToolItems(output) there too (no-op on healthy input). Now all four build sites are covered, matching the "enforced at every Responses input-build site" intent. Re-ran typecheck + full suite + build after the change: still green.

Minor, non-blocking (left to you): a single pass does not fully de-trap doubly-interleaved input where two call/output spans overlap. Repro: [fc(D), fc(A), fo(D), msg(M), fo(A)] → after one pass [fc(D), M, fc(A), fo(D), fo(A)], where M is still trapped between D and its output; a second pass clears it. In practice the original bug only ever produced a single wedge (one marker appended after all calls in a turn), so this is unreachable today — I kept the diff minimal rather than touch the core algorithm. If you want belt-and-suspenders, wrap the body in a bounded fixpoint loop (≤ n iterations; it still returns the same ref on healthy input, so the prefix-cache no-op property holds) and add that as a regression test. Happy to fold it in if you want it in this PR.

Net: correct, well-scoped, well-tested fix for a nasty wedge class; with the forge-path commit landed it's complete across all Responses input-build sites. ✅

中文摘要:修复了严格 Responses 后端(DeepSeek)因 function_call_output 与对应调用不连续而导致的会话永久卡死(#766)——核心两处(出向自愈守卫 + surfaceReadonlyJson 根因)都打到了病因。我在 review 中补上唯一遗漏的守卫点:codex /responses/compact 锻造路径(其输出会成为客户端记录的历史,是同一中毒向量),已直接提交到 PR 分支(dec6c5b)并复跑 typecheck、全量测试(1469 通过)、build 均绿;另有一个"双重交错输入单遍无法完全去困"的理论残留,实际不可达,留给你决定是否加 fixpoint 加固。可以合并。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] No tool output found for tool call call_xxxx

1 participant