fix(tools): enforce routed tool choice policy - #1382
Conversation
📝 WalkthroughWalkthroughThe change adds ChangesTool-choice predicate and adapter filtering
Bridge authorization
Web-search authorization
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Request
participant toolChoiceToolPredicate
participant BridgePlanning
participant ToolMetadata
Request->>toolChoiceToolPredicate: provide toolChoice
toolChoiceToolPredicate-->>BridgePlanning: authorize image, video, or web-search names
toolChoiceToolPredicate-->>ToolMetadata: authorize advertised tools
BridgePlanning->>BridgePlanning: activate selected bridge
ToolMetadata->>ToolMetadata: build filtered tool maps
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
|
✅ Deterministic PR hygiene checks passed. |
✅ READY
Review readiness checklist
✅ 4/4 boxes ticked. This pull request is already Ready for Review. |
|
@luvs01 Please rebase your other PR's first, if not I will have to be closing the last 2 PR's of yours. Thanks in advance. |
3ef7346 to
96ce1d7
Compare
|
@Wibias All 22 remaining open PRs have now been rebased onto the current #1373 is the sole exception in the lightweight gates: its code checks pass, but it awaits the maintainer-owned |
96ce1d7 to
d1fcc65
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/images/plan.ts`:
- Around line 50-53: Update the image-name handling in
src/images/plan.ts#L50-L53 to retain only names accepted by toolAllowed, adding
the synthetic image name only when authorized; update the video alias handling
in src/images/plan.ts#L114-L115 with the same filtering before returning
toolNames. In tests/videos/plan-video.test.ts#L50-L67, add coverage for
toolChoice selecting VIDEO_GEN_TOOL_NAME while generate_video is detected, and
assert generate_video is excluded from the plan.
🪄 Autofix
❌ Autofix failed (check again to retry)
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: fbdac645-f8ea-493d-bd50-69684d468c86
📒 Files selected for processing (13)
src/adapters/openai-chat.tssrc/adapters/tool-catalog-nudge.tssrc/images/loop.tssrc/images/plan.tssrc/server/responses/collaboration.tssrc/types.tssrc/web-search/index.tssrc/web-search/loop.tstests/images/plan.test.tstests/reasoning-effort.test.tstests/responses-parser.test.tstests/videos/plan-video.test.tstests/web-search.test.ts
|
An unexpected error occurred while generating fixes: Not Found - https://docs.github.com/rest/git/refs#get-a-reference |
|
Tip For best results, initiate chat on the files or code changes.
You are interacting with an AI system. |
|
|
|
|
|
Tip For best results, initiate chat on the files or code changes.
You are interacting with an AI system. |
|
Exact-head CI is blocked only by the macOS |
Wibias
left a comment
There was a problem hiding this comment.
Reviewed the current head. The shared tool-choice predicate is applied consistently across advertisement, restoration, and sidecar planning, including the previously reported media-alias gap. The affected CI is green and I found no remaining blocking issue. This is a useful safety fix because tool_choice now acts as an actual execution boundary, so excluded tools cannot leak into metadata or trigger paid/external sidecar work.
|
Thanks @luvs01 for this fix. 🙏 This is especially useful because it turns |
Summary
Verification
bun run typecheck— passedbun run privacy:scan— passedgit diff --check HEAD^ HEAD— passedChecklist
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
New Features
Bug Fixes
Tests