update: rename duplicate tool-call ids on ingest so results land on the right card - #9
Merged
Merged
Conversation
…he right card Some OpenAI-compatible gateways mint a deterministic id per (tool, index) — literally "bash:0" for every bash call on every turn — instead of a unique ToolCallId. One agent turn holds several assistant messages in the live tail (submit_message appends a fresh placeholder per sub-turn), so turn 2's "bash:0" landed beside turn 1's, already Done, and every lookup (`with_live_tool`) matched the FIRST one: the result was stamped onto the dead card, the real call stayed Pending, and its card hung until the 330 s step timeout — on every response with a tool call. Field report: harness unusable in that environment. Fix: - `uniquify` scans the live tail (including never-finals from rows the test freeze already skipped); a duplicate import is renamed to `id#N` (first free N) and the original is stashed on `ToolUse::wire_id`. - `find_streaming_tool` on later delta/end/result events falls back to matching `wire_id`, choosing the NEWEST carrier (sequential collisions produce bash:0, bash:0#2, bash:0#3 — wire addresses all with bash:0). - `with_live_tool` additionally prefers the FIRST NON-TERMINAL match for worried callers that still address the raw id. - The rewritten id goes back out on the wire in BOTH the assistant tool_call and its paired role:"tool" result (all four transports), so the request stays self-consistent — and unlike the original, unambiguous. Field verification: zero occurrences across three parallel sessions after the fix (previously every response with tool calls). Tests: tests/dup_tool_call_id_test.cpp (89/89 pass, incl. the 12 sanitizer entries).
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8a3bb64f8f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+928
to
+929
| for (std::size_t i = m.ui.frozen_through; | ||
| i < m.d.current.messages.size(); ++i) |
There was a problem hiding this comment.
When a gateway reuses an id after the previous turn has already been frozen, this scan starts at frozen_through, so the old tool call is invisible and the new call keeps the same raw id. That leaves duplicate ids in Thread; on the very next tool-continuation request, wire::superseded_read_ids keys by id, so repeated read calls such as read:0 for the same path mark the newest read as superseded too and the model receives only the “earlier read” pointer instead of the result it just requested. Please detect collisions against all current messages, not only the live tail.
Useful? React with 👍 / 👎.
1ay1
added a commit
that referenced
this pull request
Aug 14, 2026
…edundant id-renaming PR #9 fixed the reused-tool-call-id bug (some OpenAI-compatible gateways mint a deterministic id per (tool, index) — "bash:0" every sub-turn — so turn 2's call collided with turn 1's already-Done card and the result was stamped onto the dead card, wedging the live call until the 330s timeout) with TWO independent mechanisms in one commit: A) with_live_tool prefers the first NON-terminal call carrying the id; B) uniquify + ToolUse::wire_id renamed the newcomer at ingest and find_streaming_tool fell back to wire_id. For the failure mode that actually occurs — a duplicate id across sub-turns — (A) alone is sufficient and correct: result / progress / timeout / permission routing all go through with_live_tool, which now skips the terminal turn-1 card. And streaming assembly needs no help either: find_streaming_tool is scoped to messages.back(), the current sub-turn's assistant message, so an earlier sub-turn's identically-id'd call is out of range and cannot shadow the live one. The id scheme is deterministic per (tool, INDEX), so two calls in one message get different ids — the parallel same-message collision (B) was guarding against cannot arise from it. That makes (B) pure redundancy carrying its own edge cases (the #N-suffix collision surface, the silent 1000-collision giveup that reintroduced the bug). Remove it: - drop ToolUse::wire_id - drop the uniquify lambda and the StreamToolUseStart rename branch - drop the wire_id fallback in find_streaming_tool Rewrite dup_tool_call_id_test to prove the remaining mechanism: result routing across sub-turns lands on the live call (with turn 1 untouched), streaming stays isolated to the current sub-turn (turn 1's args are not mutated by turn 2's delta), with_live_tool prefers the non-terminal carrier, a hanging first call doesn't swallow a distinct later result, and unique ids route untouched. 16/16 checks green; full suite 88/89 (the one failure, scrollback_oracle_test, is a pre-existing maya rendering flake unrelated to this change). Net −41 lines of runtime code, one routing mechanism instead of two.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Behavior change
Some OpenAI-compatible gateways mint a deterministic id per (tool, index) — literally `"bash:0"` for every bash call on every turn — instead of a unique `ToolCallId`. One agent turn holds several assistant messages in the live tail (`submit_message` appends a fresh placeholder per sub-turn), so turn 2's `"bash:0"` landed beside turn 1's, already Done, and every lookup (`with_live_tool`) matched the FIRST one:
In my environment (OpenAI-compatible gateway) this reproduced on effectively 100% of responses containing tool calls, which made the harness unusable.
Fix
Verification