fix(responses): reject undeclared routed tool calls - #1576
Conversation
|
✅ Deterministic PR hygiene checks passed. |
|
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 (7)
📝 WalkthroughWalkthroughThe Responses bridge now receives a request-visible tool-name allowlist. It rejects undeclared provider tool calls in streaming and batch responses. Tool selection builds the allowlist, and regression tests cover catalog construction and failure behavior. ChangesResponses tool validation
Estimated code review effort: 3 (Moderate) | ~20 minutes Mergeability Score: ⚪ Minimal · up to The PR rejects undeclared routed tool calls while preserving declared tool mappings and the existing Code Mode path; no actionable merge-blocking risk remains after normal checks and review. Sequence Diagram(s)sequenceDiagram
participant Client
participant buildToolBridgeMaps
participant ResponsesCore
participant ResponsesBridge
Client->>buildToolBridgeMaps: provide toolChoice
buildToolBridgeMaps->>ResponsesCore: return declaredToolNames
ResponsesCore->>ResponsesBridge: pass declaredToolNames and provider events
ResponsesBridge->>ResponsesBridge: validate emitted tool name
ResponsesBridge-->>Client: emit response.failed or final batch failure
Possibly related PRs
Suggested labels: 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 |
Wibias
left a comment
There was a problem hiding this comment.
No code blocker from my review. The request-visible declared-tool set is derived from the existing toolChoice-filtered catalog, and both streaming and non-streaming paths fail closed before an undeclared tool becomes client-executable. Keeping exec -> tools.apply_patch(...) as the supported Code Mode path instead of guessing a translation is the right boundary for #1544.
Please rebase onto current dev and rerun CI; if that remains green, this is good to merge.
Summary
exec -> tools.apply_patch(...)as the Code Mode path instead of guessing an automatic conversion for top-levelapply_patchCloses #1544
Verification
bun test tests/responses-parser.test.ts tests/responses-stream-tool-events.test.ts tests/bridge.test.ts bun run typecheck bun run privacy:scan git diff --checkFocused result: 87 pass / 0 fail. Typecheck and privacy scan are green.
Checklist
Summary by CodeRabbit