Skip to content

fix(responses): enforce tool choice in passthrough repair - #1667

Merged
Wibias merged 1 commit into
lidge-jun:devfrom
luvs01:agent/enforce-passthrough-tool-choice
Aug 14, 2026
Merged

Wibias merged 1 commit into
lidge-jun:devfrom
luvs01:agent/enforce-passthrough-tool-choice

Conversation

@luvs01

@luvs01 luvs01 commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Restore a routed Responses custom call only when the key-auth adapter actually lowered that name after request normalization and the caller's tool_choice still authorizes it.
  • Preserve native apply_patch, forward-auth passthrough, and custom image tools replaced by hosted-provider policy instead of reinterpreting their upstream function calls.
  • Add JSON and SSE regressions for none, allowed_tools, named choices, misleading raw metadata, hosted-tool replacement, and the positive routed-custom path.

Verification

  • Base: dev at a1e5192b75edbf6dcacae51a30912fab93906f87; exact head: 09086ed3a4c5819ab10a955563ac069104b74b5b.
  • Bun 1.3.14: tests/responses-custom-tool-repair.test.ts — 19 pass; tests/openai-responses-passthrough.test.ts — 68 pass.
  • Bun 1.4.0-canary.1: the same focused files — 19 + 68 pass.
  • bun x tsc --noEmit passed on both runtimes.
  • bun run privacy:scan and git diff --check passed.
  • The Bun 1.3.14 full local suite reached tests/api-storage-policy-put-race.test.ts without assertion failures, then the runtime exited with an internal assertion crash; that exact test passed 1/1 in a fresh isolated process.
  • Independent security/correctness review found no remaining actionable P0-P2 issue.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed. The transport decision log now records the post-normalization and tool_choice restoration boundary.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults. Independent review found no remaining actionable P0-P2 issue; maintainer security review remains required for the client-executable tool policy boundary.

Review readiness checklist

This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:

  • All CI tests are green on my local testing.

  • I pushed my PR to the latest dev commit.

  • I resolved all correct Codex and CodeRabbit findings.

  • My PR is ready for review.

Summary by CodeRabbit

  • Bug Fixes
    • Improved Responses passthrough handling for routed custom tools.
    • Custom tool calls are restored only when request settings allow them and conversion occurred.
    • Native function calls, including apply_patch, remain unchanged.
    • Prevented restoration when provider-specific tool replacements or hosted-tool preferences take precedence.
    • Behavior is now consistent for both streaming and non-streaming responses.

@coderabbitai

coderabbitai Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 118eebb0-9945-4f1b-a98c-d933875dc5ff

📥 Commits

Reviewing files that changed from the base of the PR and between a1e5192 and 09086ed.

📒 Files selected for processing (5)
  • src/adapters/base.ts
  • src/adapters/openai-responses.ts
  • src/server/responses/core.ts
  • structure/04_transports-and-sidecars.md
  • tests/responses-custom-tool-repair.test.ts

📝 Walkthrough

Walkthrough

The change tracks routed custom tools converted by the Responses adapter. Passthrough restoration now uses that conversion metadata and tool policy, preserving native apply_patch and provider-replaced tools as upstream function calls.

Changes

Routed custom-tool restoration

Layer / File(s) Summary
Track adapter conversions
src/adapters/base.ts, src/adapters/openai-responses.ts
AdapterRequest exposes converted routed custom-tool names. buildRequest returns the rewritten body and conversion set from rewriteRoutedCustomToolsForUpstream.
Restore only eligible calls
src/server/responses/core.ts
Passthrough tracking starts empty. Non-forward requests add adapter-converted names only when they match declared freeform tools.
Validate restoration policy
tests/responses-custom-tool-repair.test.ts, structure/04_transports-and-sidecars.md
Tests cover streaming and non-streaming exclusions, native apply_patch, and hosted image_generation replacement. Documentation describes the restoration conditions.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 09086

The change is localized to Responses tool-choice passthrough repair with focused regression coverage, and no actionable merge-blocking risk remains after normal checks and review.

Sequence Diagram(s)

sequenceDiagram
  participant buildRequest
  participant rewriteRoutedCustomToolsForUpstream
  participant ResponsesPassthrough
  participant restoreRoutedCustomCallsInJson
  buildRequest->>rewriteRoutedCustomToolsForUpstream: Rewrite routed custom tools
  rewriteRoutedCustomToolsForUpstream-->>buildRequest: Return rewritten body and converted names
  buildRequest-->>ResponsesPassthrough: Provide request metadata
  ResponsesPassthrough->>restoreRoutedCustomCallsInJson: Restore eligible converted calls
Loading

Possibly related PRs

Suggested reviewers: lidge-jun, wibias

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: enforcing tool choice during Responses passthrough repair.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added bug Something isn't working review-ready labels Aug 14, 2026
@github-actions

github-actions Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

✅ READY

  • all PR quality gates passed; the review readiness checklist is complete.

Review readiness checklist

  • ✅ All CI tests are green on my local testing.
  • ✅ I pushed my PR to the latest dev commit.
  • ✅ I resolved all correct Codex and CodeRabbit findings.
  • ✅ My PR is ready for review.

4/4 boxes ticked.

This pull request is already Ready for Review.
The review-ready label marks this PR as ready; review automation runs independently.
Maintainers: @lidge-jun @Ingwannu @Wibias

@github-actions
github-actions Bot marked this pull request as ready for review August 14, 2026 05:00

Wibias commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Thanks for this — especially useful because it tightens the Responses passthrough boundary around client-executable custom tools. Restoring only tools the adapter actually lowered, while still respecting tool_choice, avoids accidentally reinterpreting native/hosted function calls and keeps JSON + streaming behavior consistent.

@Wibias
Wibias merged commit 6737963 into lidge-jun:dev Aug 14, 2026
27 checks passed
agentHits pushed a commit to agentHits/opencodex that referenced this pull request Sep 17, 2026
…gh-tool-choice

fix(responses): enforce tool choice in passthrough repair
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working review-ready

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants