Repository navigation
fix(responses): repair legacy dotted call names on replay (#5095) - #5101
Merged
Merged
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: lidge-jun/opencodex/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (9)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe change adds bounded repair for replayed ChangesResponses tool-name repair
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix Sequence Diagram(s)sequenceDiagram
participant Client
participant ResponsesAdapter
participant repairLegacyDottedToolCallNames
participant ToolCatalog
participant ResponsesEndpoint
Client->>ResponsesAdapter: submit replayed request
ResponsesAdapter->>repairLegacyDottedToolCallNames: pass request body
repairLegacyDottedToolCallNames->>ToolCatalog: inspect declared tools
ToolCatalog-->>repairLegacyDottedToolCallNames: return tool declarations
repairLegacyDottedToolCallNames-->>ResponsesAdapter: return repaired or original body
ResponsesAdapter->>ResponsesEndpoint: forward request
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 6 files. (3 skipped: 3 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
|
✅ Deterministic PR hygiene checks passed. |
Base automatically changed from
codex/5095-emit-schema-valid-tool-names
to
dev
September 19, 2026 06:22
lidge-jun
changed the base branch from
codex/5095-emit-schema-valid-tool-names
to
dev
September 19, 2026 06:22
Stopping the emission is only half the report. Codex stores what it received, so a conversation that already contains a default.-prefixed function_call name is refused on EVERY later request that replays it: Invalid 'input[877].name': string does not match pattern '^[a-zA-Z0-9_-]+$'. The task cannot be compacted or continued, and upgrading does not reach it. That is the part of the report that actually hurts: a side chat opened from an 11h43m parent hit it on a plain OpenAI model, from history a routed provider had damaged. repairLegacyDottedToolCallNames repairs the replayed item on the way out, in buildRequest beside backfillWebSearchQueries and again in compact.ts, which forwards the caller's body directly. It runs before the canonical-destination split, because the destination that refused the request was plain OpenAI. What resolves is bounded on purpose: - a dotted spelling the caller's own catalog declares is a real tool identity and is never stripped; rewriting it would change which tool the history says was called; - a suffix claimed by two declared identities is ambiguous and is left alone; - a suffix naming exactly one declared tool resolves to it; - otherwise, only the code-mode helper spellings in CODE_MODE_HELPER_WIRE_NAMES resolve, because a code-mode catalog never declares them -- they exist solely as nested tools.* helpers inside exec, so a recorded call under one of those names can only be a provider echoing the helper. There is deliberately no "strip everything before the first dot" rule. A legitimate tool name may contain a dot in another provider's vocabulary, and a replayed item names a call that already happened, which is the worst possible place to guess. Anything unresolved is left exactly as it is; the upstream still refuses it, which is the pre-existing outcome rather than a new one. Only replayed input items are eligible. The caller's tool catalog is never rewritten, call_id is untouched so a repaired call still pairs with its stored output, and a body with nothing to repair is returned by reference. No local suite was run. Refs #5095
lidge-jun
force-pushed
the
codex/5095-replay-legacy-dotted-name-repair
branch
from
September 19, 2026 06:22
8125f52 to
23fcfc7
Compare
2 tasks done
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
1 participant
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.
Summary
Stacked on #5100, which stops new damage. This is the recovery half, and it is the part of #5095 that actually hurts.
Codex stores what it received. A conversation that already contains a
default.-prefixedfunction_callname is refused on every later request that replays it:So the task cannot be compacted or continued, and upgrading OpenCodex does not reach it. The reporter hit this in a side chat opened from an 11h43m parent task, on a plain OpenAI model (GPT-5.6 Luna), replaying history a routed Command Code Muse turn had damaged.
repairLegacyDottedToolCallNamesrepairs the replayed item on the way out. It is applied inbuildRequestbesidebackfillWebSearchQueries— the existing seam for exactly this class of stored-history repair — and again insrc/server/responses/compact.ts, which forwards the caller's body directly without passing through the adapter. It runs before the canonical-destination split, because the destination that refused the request was plain OpenAI.What resolves is bounded on purpose. Only replayed
inputitems are eligible; the caller's tool catalog is never rewritten.default.view_image,default.apply_patchexeconly)view_image,apply_patchviaCODE_MODE_HELPER_WIRE_NAMESdefault.view_imageview_imageview_image(suffix names exactly one declared tool)default.lookuplookupdeclared inadditional_toolslookupdefault.lookupdefault.view_imageview_imagedefault.view_imagedefaultnamespacedefault.reportingdeclared top-leveldefault.view.imagenamespace: "mcp__x"view image,view_image!,other.view_imageThere is deliberately no "strip everything before the first dot" rule. A legitimate tool name may contain a dot in another provider's vocabulary, and a replayed item names a call that already happened, which is the worst possible place to guess.
default.is one literal prefix: the one invented namespace this proxy is known to have emitted, already the subject of #3402, #4176 and #4412.Anything unresolved is left exactly as it is. The upstream still refuses it, which is the pre-existing outcome rather than a new failure mode.
call_idis untouched, so a repaired call still pairs with its storedfunction_call_output, and a body with nothing to repair is returned by reference.Files:
src/responses/legacy-dotted-tool-name-repair.ts— the repair, reading the request's own catalog throughcollectResponsesToolGroups.src/types/tools.ts,src/types.ts—CODE_MODE_HELPER_WIRE_NAMES, a membership view of the helper tuple that already exists there, so the bounded vocabulary cannot drift from the listnormalizeDeclaredToolNamereads.src/adapters/openai-responses/passthrough.ts,src/server/responses/compact.ts— the two forward seams.structure/transports/responses.md— records the boundary and why it is bounded.Verification
No local suite was run. This lane is static-analysis plus hosted CI only; verification is by source reasoning and the hosted matrix on the exact head.
New regression file
tests/responses/responses-legacy-dotted-tool-name-repair.test.tscovers every row of the table above, plus: the whole helper vocabulary resolving without being declared,call_idpairing surviving the rewrite, a mixed replay where the undamaged items come back by reference and only the damaged one is rewritten, an undamaged history returned by reference, a body with noinputarray, and the catalog staying reference-identical across a repair.Registered in both
scripts/test-layout/layout.jsonandtests/fixtures/test-layout-expected.json. No entry intests/fixtures/file-size-baseline.jsoncovers any file this PR touches, so no cap moves.Checklist
No authorization decision is affected. This rewrites a name in a replayed historical item on the outbound wire; it grants nothing, and the only names it can produce are a declared tool or one of the five code-mode helper spellings the proxy itself owns. The undeclared-tool guard on the response side is untouched.
Refs #5095
Summary by CodeRabbit
Bug Fixes
default.prefix.Tests
Documentation