feat(agent-routing): model-only agent routes (set the verifier or any agent's model on the current provider) - #1617
Conversation
|
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 (1)
📜 Recent review details⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🧰 Additional context used📓 Path-based instructions (6)**/*.{ts,tsx,js,jsx,py}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*⚙️ CodeRabbit configuration file
Files:
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}⚙️ CodeRabbit configuration file
Files:
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}⚙️ CodeRabbit configuration file
Files:
**⚙️ CodeRabbit configuration file
Files:
🔇 Additional comments (1)
📝 WalkthroughWalkthroughAgent routing is refactored to support a new "model-only" route shape ( ChangesAgent Model-Only Routing
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/services/api/agentRouting.test.ts`:
- Around line 172-200: The tests in the 'model-only routes' describe block
intentionally trigger misconfiguration warnings (console.error) when
resolveAgentProvider is called with incomplete agentModels entries like 'bare'
(missing model) and 'half-entry' (missing api_key). Stub console.error using
jest.spyOn at the start of the describe block and restore it in an afterEach
hook. For each test that triggers a warning (bare entry defaults the model and
partial entry skipped), add an assertion using
expect(console.error).toHaveBeenCalled() with the expected warning message to
verify the warning was issued. This silences the console noise in CI logs while
still verifying that the expected warnings are being triggered.
🪄 Autofix (Beta)
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: fad587e2-9c30-463b-914c-4fadd3a5caa8
📒 Files selected for processing (6)
README.mdsrc/services/api/agentRouting.test.tssrc/services/api/agentRouting.tssrc/tools/AgentTool/runAgent.tssrc/utils/settings/agentModelsSchema.test.tssrc/utils/settings/types.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (8)
**/*
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
README.mdsrc/tools/AgentTool/runAgent.tssrc/services/api/agentRouting.test.tssrc/utils/settings/types.tssrc/utils/settings/agentModelsSchema.test.tssrc/services/api/agentRouting.ts
{README.md,CONTRIBUTING.md,docs/**,.github/pull_request_template.md}
⚙️ CodeRabbit configuration file
{README.md,CONTRIBUTING.md,docs/**,.github/pull_request_template.md}: Review docs for accuracy against current code behavior. Flag security or provider claims that overpromise, stale install commands, missing setup caveats, and instructions that could push users toward unsafe credential handling. Keep purely wording-level suggestions non-blocking.
Files:
README.md
**
⚙️ CodeRabbit configuration file
**: # Contributing to OpenClaudeThanks for contributing.
OpenClaude is a fast-moving open-source coding-agent CLI with support for multiple providers, local backends, MCP, and a terminal-first workflow. The best contributions here are focused, well-tested, and easy to review.
Before You Start
- Search existing issues and discussions before opening a new thread.
- Check open pull requests for work that overlaps with your contribution. If a PR already exists that addresses the same change, open an issue or discussion first to align on direction — duplicate PRs may be closed without review.
- Use issues for confirmed bugs and actionable feature work.
- Use discussions for setup help, ideas, and general community conversation.
- For larger changes, open an issue first so the scope is clear before implementation.
- For security reports, follow SECURITY.md.
Pull Requests
Every PR needs a reason. Your PR description must include:
- what changed and why
- the user or developer impact
- the exact checks you ran
- a linked issue when one exists, using
Fixes#123, `Closes `#123, or another clear link- screenshots when the PR touches UI, terminal presentation, or the VS Code extension
- which provider path was tested when the PR changes provider behavior
The PR author is responsible for ensuring their PR is merge-ready. PRs with merge conflicts will not be reviewed or approved until the conflicts are resolved.
Issues are the recommended starting point for anything non-trivial — opening one first helps avoid wasted effort if the change is out of scope or already being worked on. Small fixes, doc corrections, and obvious improvements can stand on their own without a linked issue, as long as the PR description explains the intent.
What Gets Closed Without Review
PRs may be closed without review...
Files:
README.mdsrc/tools/AgentTool/runAgent.tssrc/services/api/agentRouting.test.tssrc/utils/settings/types.tssrc/utils/settings/agentModelsSchema.test.tssrc/services/api/agentRouting.ts
**/*.{ts,tsx,js,jsx,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.{ts,tsx,js,jsx,py}: Follow the existing code style in the touched files
Keep comments useful and concise
Files:
src/tools/AgentTool/runAgent.tssrc/services/api/agentRouting.test.tssrc/utils/settings/types.tssrc/utils/settings/agentModelsSchema.test.tssrc/services/api/agentRouting.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Typecheck TypeScript code before submitting (use
bun run typecheck)
Files:
src/tools/AgentTool/runAgent.tssrc/services/api/agentRouting.test.tssrc/utils/settings/types.tssrc/utils/settings/agentModelsSchema.test.tssrc/services/api/agentRouting.ts
src/{components/permissions,utils/permissions,hooks/toolPermission,tools,entrypoints/sdk}/**
⚙️ CodeRabbit configuration file
src/{components/permissions,utils/permissions,hooks/toolPermission,tools,entrypoints/sdk}/**: Review permission prompts, auto-allow logic, sandbox behavior, SDK permission schemas, shell/PowerShell execution, and background execution paths as security-sensitive. Block on bypasses, unclear trust boundaries, unsafe path handling, missing user visibility, or changes that broaden allowed behavior without an explicit maintainer decision.
Files:
src/tools/AgentTool/runAgent.ts
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}
⚙️ CodeRabbit configuration file
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}: Review provider routing, model selection, env precedence, auth/token handling, OpenAI-compatible shims, retries, proxy behavior, and outbound HTTP behavior with high scrutiny. Block on silent default changes, hidden fallback expansion, credential reuse mistakes, hardcoded provider assumptions, or new network reach that is not intentional and documented.
Files:
src/services/api/agentRouting.test.tssrc/services/api/agentRouting.ts
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}
⚙️ CodeRabbit configuration file
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.
Files:
src/services/api/agentRouting.test.tssrc/utils/settings/agentModelsSchema.test.ts
🪛 GitHub Actions: PR Checks / 2_smoke-and-tests.txt
src/services/api/agentRouting.test.ts
[warning] 1-1: [agentRouting] Warning: agentModels entry "zai" has only one of base_url/api_key; both are required for cross-provider routing. Skipping this route.
[warning] 1-1: [agentRouting] Warning: agentModels entry "half-entry" has only one of base_url/api_key; both are required for cross-provider routing. Skipping this route.
🪛 LanguageTool
README.md
[style] ~256-~256: Consider using “who” when you are referring to a person instead of an object.
Context: ...: verification (the read-only auditor that runs before completion), Explore, and...
(THAT_WHO)
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
I found one test-cleanliness issue that should be addressed before this is ready.
Findings
- [P3] Complete CodeRabbit's request to assert the expected routing warning
src/services/api/agentRouting.test.ts:197
CodeRabbit's review item is still valid for the intentional partialagentModelscase: this test callsresolveAgentProvider()with onlybase_url, which emits the expectedconsole.errorwarning during CI. Please complete that request by stubbing the warning and asserting the expected message in the test, so intentional log noise does not mask new routing warnings or failures in future runs.
Silence the intentional console.error noise from partial agentModels entries in CI logs by stubbing console.error per describe block and asserting the expected warning message fires, so future routing warnings or failures are not masked.
|
Addressed in 3882ec4. The partial-entry tests now stub |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/services/api/agentRouting.test.ts (1)
580-580: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winMove import to top with other imports.
The
shouldEnforceModelAllowlistimport belongs at the top of the file (line 1–11 block) alongside the other imports from./agentRouting.js. This placement breaks the existing file organization pattern.📦 Proposed fix
Move line 580 to the import block at the top:
import { applyAgentProviderOverrideToEnv, isProviderOverride, resolveAgentModelProvider, resolveAgentProvider, resolveAgentRunModelRouting, resolveOutOfProcessTeammateProvider, resolveOutOfProcessTeammateProviderFromCliArgs, + shouldEnforceModelAllowlist, } from './agentRouting.js'Then remove the duplicate import at line 580.
🤖 Prompt for 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. In `@src/services/api/agentRouting.test.ts` at line 580, The import of shouldEnforceModelAllowlist from './agentRouting.js' at line 580 violates the file organization pattern. Move this import to the top of the file (lines 1-11 block) and consolidate it with the existing imports from './agentRouting.js' that are already declared there. Remove the duplicate import statement that currently appears at line 580.
🤖 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.
Outside diff comments:
In `@src/services/api/agentRouting.test.ts`:
- Line 580: The import of shouldEnforceModelAllowlist from './agentRouting.js'
at line 580 violates the file organization pattern. Move this import to the top
of the file (lines 1-11 block) and consolidate it with the existing imports from
'./agentRouting.js' that are already declared there. Remove the duplicate import
statement that currently appears at line 580.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 371d6b78-0174-46c0-a2d2-3d8a9b295da3
📒 Files selected for processing (1)
src/services/api/agentRouting.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: typecheck
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx,js,jsx,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.{ts,tsx,js,jsx,py}: Follow the existing code style in the touched files
Keep comments useful and concise
Files:
src/services/api/agentRouting.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Typecheck TypeScript code before submitting (use
bun run typecheck)
Files:
src/services/api/agentRouting.test.ts
**/*
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
src/services/api/agentRouting.test.ts
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}
⚙️ CodeRabbit configuration file
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}: Review provider routing, model selection, env precedence, auth/token handling, OpenAI-compatible shims, retries, proxy behavior, and outbound HTTP behavior with high scrutiny. Block on silent default changes, hidden fallback expansion, credential reuse mistakes, hardcoded provider assumptions, or new network reach that is not intentional and documented.
Files:
src/services/api/agentRouting.test.ts
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}
⚙️ CodeRabbit configuration file
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.
Files:
src/services/api/agentRouting.test.ts
**
⚙️ CodeRabbit configuration file
**: # Contributing to OpenClaudeThanks for contributing.
OpenClaude is a fast-moving open-source coding-agent CLI with support for multiple providers, local backends, MCP, and a terminal-first workflow. The best contributions here are focused, well-tested, and easy to review.
Before You Start
- Search existing issues and discussions before opening a new thread.
- Check open pull requests for work that overlaps with your contribution. If a PR already exists that addresses the same change, open an issue or discussion first to align on direction — duplicate PRs may be closed without review.
- Use issues for confirmed bugs and actionable feature work.
- Use discussions for setup help, ideas, and general community conversation.
- For larger changes, open an issue first so the scope is clear before implementation.
- For security reports, follow SECURITY.md.
Pull Requests
Every PR needs a reason. Your PR description must include:
- what changed and why
- the user or developer impact
- the exact checks you ran
- a linked issue when one exists, using
Fixes#123, `Closes `#123, or another clear link- screenshots when the PR touches UI, terminal presentation, or the VS Code extension
- which provider path was tested when the PR changes provider behavior
The PR author is responsible for ensuring their PR is merge-ready. PRs with merge conflicts will not be reviewed or approved until the conflicts are resolved.
Issues are the recommended starting point for anything non-trivial — opening one first helps avoid wasted effort if the change is out of scope or already being worked on. Small fixes, doc corrections, and obvious improvements can stand on their own without a linked issue, as long as the PR description explains the intent.
What Gets Closed Without Review
PRs may be closed without review...
Files:
src/services/api/agentRouting.test.ts
🔇 Additional comments (5)
src/services/api/agentRouting.test.ts (5)
1-1: LGTM!
27-33: LGTM!
176-178: LGTM!
183-242: LGTM!
582-592: LGTM!
Move the import to the top import block instead of mid-file.
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
Thanks for the update. I rechecked the previously discussed paths and do not see any remaining actionable issues from my side.
@kevincodex1 LGTM
… agent's model on the current provider) (Twigpine#1617) * feat(agent-routing): allow model-only agentModels entries in settings schema * feat(agent-routing): resolve model-only agent routes that reuse the current provider * feat(agent-routing): enforce org model allowlist for model-only agent routes * docs(agent-routing): document model-only routes and built-in agent keys * test(agent-routing): stub and assert partial-entry routing warnings Silence the intentional console.error noise from partial agentModels entries in CI logs by stubbing console.error per describe block and asserting the expected warning message fires, so future routing warnings or failures are not masked. * test(agent-routing): consolidate shouldEnforceModelAllowlist import Move the import to the top import block instead of mid-file.
Cherry-picked from upstream 9e902db. Adds model-only agent route support: instead of 'agentName:provider' you can now use 'agentName:model' to route a sub-agent to a different model on the SAME provider (e.g. Explore=sonnet, Plan=opus). - New resolveAgentRunModelRouting helper - Extended AgentModelConfig type to allow { model } without provider override - agentModelsSchema zod validation - README update skipped (OC maintains own README) Includes 46 new tests in agentRouting.test.ts + agentModelsSchema.test.ts.
3 participants
Summary
agentModelsentries to omitbase_url/api_key. Such "model-only" entries reuse the current provider's connection and only change the model.verificationauditor — can run on a different model without duplicating endpoint/key. Cross-provider routing is unchanged.Impact
agentRouting: { "verification": "<key>" }now works with a credential-less entry such as{ "mini": { "model": "gpt-5-mini" } }to run the verifier (or any agent) on a different model on your current provider. Full cross-provider entries behave exactly as before. The org model allowlist now also applies to model-only routes.resolveAgentProvider/resolveAgentModelProvidernow returnAgentRoute(ProviderOverride | { model }); added anisProviderOverrideguard and ashouldEnforceModelAllowlisthelper. Out-of-process (pane/window) teammate routing stays cross-provider only.Testing
bun run buildbun run smokebun run check— green except 4 failures inexport/lsp/QueryGuardthat are pre-existing full-suite pollution: they pass when run in isolation and none reference the routing code touched here.bun test src/services/api/agentRouting.test.ts src/utils/settings/agentModelsSchema.test.ts src/tools/AgentTool/→ 87 pass / 0 fail;bun run typecheckclean.Notes
Summary by CodeRabbit
agentModels, including clearer cross-provider versus model-only configuration examples.