fix(command-code): pair tool results and synthesize missing-result errors - #1411
Merged
Wibias merged 2 commits intoAug 10, 2026
Merged
Conversation
…rors The /alpha/generate wire requires every assistant tool-call to be closed by a matching tool-result. Codex history can leave a call unpaired (interrupted turns, compaction, multi-step rounds), and the upstream rejects that with 'Tool result is missing for tool call <id>', surfacing as a generic 502 (#1383). - Track declared assistant tool-call ids and pair tool results to them. - Close every declared call that never received a result with an explicit error-text tool-result before the next non-tool message. - Degrade orphan tool results (no declared call) to a user text carrier instead of a 400-prone standalone tool message. - Classify the upstream missing-tool-result error distinctly (errorType upstream_error, code missing_tool_result).
Contributor
|
✅ Deterministic PR hygiene checks passed. |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough
ChangesCommand Code tool-result handling
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant CommandCodeAdapter
participant wireMessages
participant CommandCodeProvider
Client->>CommandCodeAdapter: submit conversation
CommandCodeAdapter->>wireMessages: serialize tool calls and results
wireMessages-->>CommandCodeAdapter: paired or synthesized tool results
CommandCodeAdapter->>CommandCodeProvider: send continuation
CommandCodeProvider-->>CommandCodeAdapter: stream response or missing-tool-result error
CommandCodeAdapter-->>Client: response or classified stream error
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
3 tasks
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/adapters/command-code.ts`:
- Around line 99-112: Update the image-carrier handling in
src/adapters/command-code.ts around lines 99-112 to buffer user image carriers
while pendingCalls is non-empty, flushing them only after every
assistant-declared call has a native or synthesized tool result so tool results
remain contiguous. Add the two-call image-first scenario in
tests/command-code-provider.test.ts around lines 165-183 and assert both
tool-result messages precede the user image carrier.
In `@tests/command-code-provider.test.ts`:
- Around line 360-372: Extend the missing-tool-result coverage in the test using
createCommandCodeAdapter and isMissingToolResultError by adding an upstream
error message containing “tool_result is missing” instead of the spaced variant.
Assert that parsing produces the same errorType “upstream_error” and code
“missing_tool_result” while preserving the existing response behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 8e72932b-8a05-4368-980c-e969d2ff3856
📒 Files selected for processing (2)
src/adapters/command-code.tstests/command-code-provider.test.ts
CodeRabbit flagged that a paired tool result carrying an image emitted its user image carrier immediately, breaking the adjacency of remaining pending tool results on the /alpha/generate wire (assistant -> tool(call_1) -> user image -> tool(call_2)). Buffer image carriers while any declared call is still pending and flush them only after every call has its native or synthesized result. Also add the underscored tool_result is missing classification test.
Wibias
deleted the
ocx-120-provider-compatibility-command-code-returns-502
branch
August 10, 2026 08:02
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
/alpha/generate) rejects any assistant tool call that has no matching tool result on the continuation, surfacing as a generic 502 (Tool result is missing for tool call <id>). The adapter now keeps that pairing invariant before sending: every declared call is closed by a result, and orphan results degrade to text carriers.tool-resultwire shape (unchanged); unpaired calls get an explicit error-text result so the upstream never fails the whole turn.errorType: upstream_error,code: missing_tool_result) so the dashboard/logs distinguish provider-side tool-result validation from generic stream failures.Validation
bun test tests/command-code-provider.test.ts— pass (26 tests, incl. new pairing, orphan-carrier, and classification coverage)bun run typecheck— passbun run privacy:scan— passpre-open-gate.mjs) —ready; bug + security lenses and required probes verifiedbun run test— not fully green on this machine; failures reproduced identically on the base tip (2036ed734) in unrelated subsystems (catalog writer, codex-auth WHAM, restore/journal) and are pre-existing Windows/Bun environment issues, not caused by this changeReview notes
Fixes #1383
Summary by CodeRabbit