Skip to content

fix(agent-runtime): keep tool-call ids unique in every request - #780

Merged
vastsa merged 1 commit into
vastsa:mainfrom
zhangqingkun976:fix/duplicate-tool-call-ids
Sep 21, 2026
Merged

vastsa merged 1 commit into
vastsa:mainfrom
zhangqingkun976:fix/duplicate-tool-call-ids

Conversation

@zhangqingkun976

Copy link
Copy Markdown
Contributor

Successor of #760, which is closed: the branch was rebased onto main through the API, GitHub reads that as a force-push, and it refuses to reopen a closed PR whose branch was force-pushed (state cannot be changed. The … branch was force-pushed or recreated.). Same commits, same content, rebased head — nothing was rewritten except the base.

Toward #718. This does not claim to find the writer that duplicated the call — it makes the request valid when one exists, and it makes the next report name the writer.

Why this shape

@muzimu217's data-side check narrowed the 400 to the assembly path and listed three candidates (snapshot merging, the compaction boundary, retry replay). I verified what I could against the source and ruled two of them out:

  • A tool row's id is its call id (apps/desktop/electron/main/runtime/event-persistence.ts:327, :334), and the host read is keep-last by id (crates/host-core/src/sessions.rs:752 dedupe_records, applied on both session.get paths). So the "same call, same row id" case is already collapsed — the surviving shape is one call under two different row ids, which nothing collapses.
  • The compaction boundary does not reintroduce tool-call pairs: the DeepSeek-specific reasoning replay drops them (packages/agent-runtime/src/reasoning-replay.ts:109, "Tool-call blocks are dropped on purpose"), and the retained tail keeps user messages only.

I could not pin the writer without a DeepSeek key or the reporter's transcript, so the change is built to be correct either way:

A request can never carry a duplicate tool_use id. The guard runs in convertToLlm, the last view before the wire, which is the one place that sees both a rebuilt history and one that grew during this process:

  • the first occurrence of an id keeps its call, and its result;
  • a later call for that id, and a later result for it, are dropped — so the call/result pair the provider validates stays well-formed (dropping only the call would leave an orphan result, which is a different 400);
  • a request with no duplicates is returned unchanged (the same array, not a copy), so the common path pays nothing;
  • a drop writes one line to the agent log channel: [agent-runtime] dropped N duplicate tool call entries before the request (session=… ids=…).

That log line is the point of the exercise: the next session that hits the provider's sentence turns it into the answer instead of another hypothesis about which of the three candidates it was.

What it does not do

  • It does not fix the writer. If a duplicate is a symptom of something re-persisting a row, that bug is still there — this only stops it from bricking a session, and now says when it happens.
  • It does not rename ids (renaming would break the result pairing) and it rewrites nothing on disk.
  • No compaction, retention, budget or transcript rule changes.

Validation

packages/agent-runtime/src/runtime.test.ts:

  • duplicated transcript — a history carrying the same call under two row ids: the request view keeps one toolCall (call-1) and exactly one toolResult with the matching toolCallId, and logs one line naming the session and the id.
  • unique transcript — the same helper returns the identical array and logs nothing.

Full suite: pnpm --filter @pi-desktop/agent-runtime test → 46/47 files; the one failure is the pre-existing native-pi-session case that fails identically on pristine main. pnpm --filter @pi-desktop/agent-runtime build 0; pnpm lint clean; node scripts/check-architecture.mjs passed; pnpm docs:check (78 EN/zh pairs, 495 pages) exit 0.

E2E gate: NOT RUN. The scenario is written (E2E-RUNTIME-unique-tool-call-ids-per-request, both locales, with traceability rows) but no desktop E2E suite was executed: it needs a deterministic provider fixture that captures the outgoing request, which this repository does not have yet. Alternative validation: the runtime-level tests above build a real runtime from a duplicated history and assert on the same view the request is built from. Remaining risk: the live-appended half of the guard (a duplicate that appears mid-session) is covered by the shared code path but not by a test that provokes it.

Docs

03-runtime/02-agent-runtime.md §5 (the restored-history bullets), 08-meta/decisions-log.md D608, and the E2E scenario above — all mirrored to docs/zh-CN/….

D608 is numbered after the D604–D607 proposed in my open #721, #748 and #756; I will renumber if those land first.

Anthropic-family endpoints, DeepSeek's included, reject the whole turn with
`tool_use ids must be unique` when one call id appears twice, and the session
cannot continue while that lasts (issue vastsa#718).

The transcript is an append-only snapshot stream that tolerates a retried
append, so the same call can reach the assembled context twice: under one row id
(which the host's keep-last read already collapses) or under two (which it
cannot). The last view before the wire now keeps the first occurrence of each
`toolCall` id and drops a later call or a later result for that id, so the
call/result pair the provider validates stays well-formed. A request with no
duplicates is returned unchanged, and a drop is reported once on the `agent` log
channel with the session and the ids.

The guard sits at the request boundary (`convertToLlm`) rather than at the
history rebuild, so it also covers a duplicate that appears while the session
runs. Nothing is rewritten on disk; no compaction, retention or budget rule
changes.

C:\Users\10470\.pi-desktop\scratch\cb9e2d41-8a55-4a18-899d-92ca2791ab51\commit-718-r2.txt
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.

2 participants