Skip to content

fix(session): stop creating phantom "unknown" tool parts on re-emitted deltas - #44535

Open
internetisalie wants to merge 1 commit into
anomalyco:devfrom
internetisalie:fix/phantom-unknown-tool-parts
Open

fix(session): stop creating phantom "unknown" tool parts on re-emitted deltas#44535
internetisalie wants to merge 1 commit into
anomalyco:devfrom
internetisalie:fix/phantom-unknown-tool-parts

Conversation

@internetisalie

@internetisalieinternetisalie commented Aug 23, 2026

Copy link
Copy Markdown

Issue for this PR

Closes#33618

Type of change

  • Bug fix
  • New feature
  • Refactor / code improvement
  • Documentation

What does this PR do?

The phantom unknown tool calls in #33618 are created by opencode, not emitted by the model. laradji's measurement in that issue (~21% of tool-part events, 0 orphans, every unknown paired with a completed part on the same callID) is exactly what this code path produces.

The sequence:

  1. A tool call completes normally. tool-result arrives, the processor marks the part completed, and settleToolCall drops the entry from ctx.toolcalls. In the AI SDK adapter, tool-result also does delete state.toolNames[toolCallId].
  2. Qwen via OpenRouter then re-sends tool_calls[i].function.arguments deltas for that same call id, without repeating .function.name. The AI SDK turns those into tool-input-delta / tool-input-end.
  3. The adapter can no longer resolve a name for that id — it just deleted it — so it falls back to the literal string "unknown" (llm/ai-sdk.ts, state.toolNames[event.id] ?? "unknown").
  4. Both tool-input-delta and tool-input-end call ensureToolCall. It finds no live call for the id and creates a brand new pending part named unknown.
  5. At stream end, cleanup sweeps every still-pending call into status: "error", error: "Tool execution aborted", metadata.interrupted: true.

That is the whole reported symptom, including input: {} and raw: "".

The TUI-side escalation is a second-order effect. Both parts are replayed by toModelMessagesEffect, so the next request carries two tool_use blocks with the same toolCallId, one of them named unknown. That is what providers reject, and it is a plausible explanation for the reported loops where the model itself starts calling a tool named unknown — it is copying a tool name it can see in its own previous turn. (The duplicate id is verified; the copying is inference.)

Nothing here is specific to a model version, which matches reports against 3.7 Plus, 3.7 Max and 3.8 Max.

The changes:

  • session/processor.ts — the actual defect. ProcessorContext gains a settled set of call ids that reached a terminal state; ensureToolCall refuses to create a fresh part for one. Cleared next to ctx.toolcalls in cleanup, so it has the same lifetime as the map it guards.
  • session/llm/ai-sdk.ts — stop deleting toolNames on tool-result / tool-error. Late chunks then resolve to the real name instead of "unknown". The map is bounded by the number of tool calls in one stream.
  • session/message-v2.ts — skip a tool part whose callID was already emitted for that assistant message. This is the only part of the change that helps sessions already recorded with the duplicate; the two fixes above cannot clean up rows that are already on disk.

The first change alone stops new phantoms. The third is what makes an affected session usable again without editing the database.

How did you verify your code works?

Live against the real provider.opencode run --format json on openrouter/qwen/qwen3.8-max, same prompt each time (a small TDD exercise: write two source files plus tests, run bun test, read the files back, add a third module and rerun). Three runs on the shipped v1.18.21 binary, three on this branch, then grouped the tool_use parts by callID:

before (v1.18.21)

runtool partsreal callsphantom unknownorphans
1151050
2161060
3171160
total483117 (35%)0

after (this branch)

runtool partsreal callsphantom unknownorphans
1121200
2101000
3101000
total32320 (0%)0

Zero orphans on both sides: every phantom was paired with a completed part on the same callID, which is what laradji measured. The rate here (35%) runs a bit higher than the ~21% reported, likely because this prompt is tool-dense.

A sanitized pair from one before-run, same callID, emitted 0.3s apart:

{"type":"tool_use","part":{"type":"tool","tool":"write","callID":"call_8f053c86…",
"state":{"status":"completed","input":{"filePath":"…/src/greet.test.ts","content":""},
"output":"Wrote file successfully."}}}
{"type":"tool_use","part":{"type":"tool","tool":"unknown","callID":"call_8f053c86…",
"state":{"status":"error","input":{},"raw":"","error":"Tool execution aborted",
"metadata":{"interrupted":true}}}}

One before-run also produced a phantom on top of a call whose real part was the invalid sentinel, so the two paths stack rather than being alternatives.

Regression tests. Two, both mutation-checked — I reverted each fix on its own and confirmed the matching test goes red, so neither is a test that cannot fail:

  • test/session/processor-effect.test.ts — stubs an LLM.Service stream that completes call-1, then replays tool-input-delta / tool-input-end for call-1 with no name. Before the fix the message ends up with [{tool: "lookup", status: "completed"}, {tool: "unknown", status: "error"}]; after it, only the first.
  • test/session/message-v2.test.ts — an assistant message holding both parts for one callID now serializes to a single tool-call / tool-result pair instead of two tool_use blocks sharing an id.

Suites on this branch.

  • bun test test/session — 413 pass, 0 fail
  • bun test test/tool test/server test/cli — 1002 pass, 1 fail: tool.write > sets file permissions when writing sensitive data. That one fails identically on a clean checkout of dev on my machine (umask-related), so it is pre-existing and unrelated to this change.
  • tsgo --noEmit clean; oxlint reports 0 errors on the three changed source files.

Screenshots / recordings

n/a — no UI change.

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

…d deltas
When a provider re-sends `tool_calls[i].function.arguments` deltas for a call
that already produced its result, the AI SDK emits tool-input-delta /
tool-input-end for a call id the processor has already settled. Two things then
go wrong:
- the adapter deleted `state.toolNames[callID]` on tool-result, so the late
chunks resolve to the literal name "unknown"
- `ensureToolCall` finds no live call and creates a second pending part for the
same call id, which `cleanup` then sweeps into
`error: "Tool execution aborted"`, `metadata.interrupted: true`
The result is a phantom part paired 1:1 with a real completed one. Both
serialize into the next request, so the assistant turn carries two tool_use
blocks sharing a toolCallId, one of them named "unknown".
Track settled call ids on the processor context and refuse to mint a new part
for them; keep tool names for the life of the stream so late chunks still
resolve; and dedupe tool parts by call id when building model messages, which
is what heals sessions already recorded with the duplicate.
@github-actions

Copy link
Copy Markdown
Contributor

The following comment was made by an LLM, it may be inaccurate:

Based on my search results, I found one potentially related PR:

Related PR:

However, this related PR (#33622) appears to be a different approach to the same underlying problem. Your PR (#44535) is the current PR being analyzed, and the search confirms there are no other open PRs that are duplicates of this fix. The related PR might be a previous or alternative attempt at the same issue.

No duplicate PRs found

@Enough1122

Copy link
Copy Markdown

AI code review — automated review for reference, author can ignore or act on any point.

Overall: nice root-cause fix with layered defenses — keeping tool names for the stream lifetime stops the immediate "unknown" fallback, the processor's settled guard prevents re-creating parts for finished calls, and the replay-side dedupe repairs sessions already poisoned by older builds. Both regression tests are precisely targeted. One design question and two nits:

  1. packages/opencode/src/session/message-v2.ts:281-285 — the dedupe keeps whichever part appears first for a call ID. That matches the phantom pattern described (real part precedes the phantom), but it's an ordering assumption about legacy data. If any stored session has the phantom/error part first (e.g. an aborted attempt later re-completed under the same call ID), first-wins persists the broken one. Consider selecting by quality instead of position — prefer the part whose tool !== "unknown", then completed-over-errored status — or add a brief comment stating why positional order is trusted.

  2. packages/opencode/src/session/processor.ts:229-231ensureToolCall silently returns undefined for settled calls. Assuming all call sites already tolerate undefined (they appear to, since late events are pure noise), this is correct; a debug-level log or counter of dropped late events would help users diagnose chatty providers like the Qwen case in the test fixture, but is optional.

  3. packages/opencode/src/session/llm/ai-sdk.ts:239-241 — retaining names grows toolNames for the whole stream instead of per-call. Bounded by tool calls per response, so negligible — just noting the intentional tradeoff the new comment already documents.

The settled.clear() alongside toolcalls = {} at message completion (processor.ts:606) keeps state from leaking across turns. Good to merge once point 1 is decided.

Sign up for freeto 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.

Qwen 3.7 Plus/Max (via OpenRouter) unknown/invalid tool calls

2 participants

@internetisalie@Enough1122