fix(gateway): block provider routing on dev plans always - #2622
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
💤 Files with no reviewable changes (2)
WalkthroughThe PR separates dev-plan coding-model checks from provider-routing checks in the chat gateway, rejects direct and custom provider routing for dev-plan orgs, updates the error text, and adds a test for provider-prefixed model names. ChangesDev-plan provider routing restriction
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5ea1689baa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (isDevPlan) { | ||
| if ( | ||
| requestedProvider && | ||
| requestedProvider !== "llmgateway" && | ||
| requestedProvider !== "custom" | ||
| ) { |
There was a problem hiding this comment.
Block provider prefixes before clearing deactivated providers
This guard checks requestedProvider after resolveModelInfo() has already normalized it, and that helper clears a specifically requested provider when that provider mapping is deactivated. For example, a dev-plan org with devPlanAllowAllModels can request a deactivated mapping such as aws-bedrock/claude-3-7-sonnet; the provider prefix is parsed, then cleared, so this new isDevPlan block does not reject it and the request falls back to another provider. That leaves a provider-targeting model string accepted in the exact policy this change is trying to enforce; use the original parsed provider to decide whether a prefix was supplied.
Useful? React with 👍 / 👎.
5ea1689 to
c82a43d
Compare
c82a43d to
441e55b
Compare
Direct/custom provider-targeting model strings (e.g. `deepseek/deepseek-v4-pro`) were only rejected on dev plans when devPlanAllowAllModels was off. Enabling allow-all-models cleared isDevPlanRestricted, which also gated the routing-format check, so the `provider/model` format leaked through. Split the routing-format restriction from the model-level coding/cached-input restriction: provider/custom routing is now rejected for any dev plan regardless of allow-all-models. That flag only relaxes the model-level restrictions; only canonical root model ids are ever accepted on dev plans. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
441e55b to
1fd42d7
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Summary
This confirms and fixes the behavior raised: targeting a specific provider for DeepSeek (e.g.
deepseek/deepseek-v4-pro) worked on a dev pass once the "allow all models" feature was enabled. It shouldn't — only canonical model ids (e.g.deepseek-v4-pro) are allowed for dev pass, regardless of allow-all-models.Root cause
The direct- and custom-provider routing rejections lived inside the
if (isDevPlanRestricted)block.isDevPlanRestrictedisfalsewhendevPlanAllowAllModelsis on, so enabling allow-all-models silently also unlocked theprovider/modelrouting format.Fix
Split the two concerns:
provider/model,custom/model) now applies to any dev plan via the existingisDevPlanflag — independent of allow-all-models.isDevPlanRestricted, i.e. relaxed by allow-all-models.Removed the now-misleading "enable access to all models" hint from the routing-format error messages, since that flag no longer affects them.
Test
Added a gateway test asserting
deepseek/deepseek-v4-prois rejected with 403 ("Direct provider routing is not available on coding plans") even withallowAllModels: true. Existing image-output dev-plan tests still pass.🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
403immediately, even when all models are allowed.provider/model) and remove outdated dashboard guidance.Tests
POST /v1/chat/completionstest verifying provider-targeting model strings are rejected with403and the expected error text.