Repository navigation
fix(fallback): check the first hard-lock profile destination - #7589
Conversation
Apply fallback policy to the ranked profile selection before authentication or dispatch. Keep hard-lock pre-admission unchanged and cover refused and allowed selections with a synthetic caller-owned credential and strict fetch stub.
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes.
|
| Layer / File(s) | Summary |
|---|---|
Guard the selected profile destination src/server/responses/request-prepare.ts, structure/fallback-policy.md, structure/transports/policy-fallback.md, docs-site/src/content/docs/guides/fallback-off.md |
Request preparation checks the profile’s first-ranked destination against the original model under the hard_lock trigger. It returns a refusal response when the hop is denied and commits an allowed hop. The documentation describes how model and provider fallback settings apply and when the admission check occurs. |
Verify refusal and dispatch tests/codex-integration/main-account-hard-lock-profile-dispatch.test.ts, scripts/test-layout/layout.json, tests/fixtures/test-layout-expected.json |
Integration tests verify a 429 refusal with no provider dispatch when model fallback is disabled. With model fallback enabled, they verify a successful dispatch using the caller’s credential and account, with no stored Codex auth file. The test-layout mappings assign the test to codex-integration. |
Priority:
Estimated code review effort: 3 (Moderate) | ~20 minutes
Change: Bug fix
Possibly related issues
- [Design] Minimum safe Desktop composer compatibility across client updates #7587 describes using an alternate route with its own credentials and limits after the main account is exhausted. This PR covers the hard-lock admission decision and tests caller-credential dispatch, but does not establish that it verifies alternate-route limits.
Merge Risk: 
Minimal · up to 2d601
-
No actionable behavior risk was established. The reported conflict with dev still needs to be resolved before merging.
Pre-merge checks: 
4 · 
1
-
Failed checks: 1 warning Passed checks: 4 passedFull details: Docstring Coverage
-
Explanation
Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files. (2 skipped: 2 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
✨ Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 @coderabbitai help to get the list of available commands.
|
✅ Deterministic PR hygiene checks passed. |
Ingwannu
left a comment
There was a problem hiding this comment.
Reviewed the complete diff at 11b5a0ab1a40d2b0d924e82885bb42842736d154. The first ranked profile destination is guarded against the original locked model before authentication/dispatch; admitting a declared provider hop no longer silently admits a same-provider model hop. The positive control uses a synthetic independent caller credential, preventing the stored-main hard lock from masking a missing check. Both test-layout owners and the relevant structure/user contracts are updated.
Independent capped Linux/Bun 1.4.3 with fresh private homes: bun test tests/codex-integration/main-account-hard-lock-profile-dispatch.test.ts — 2 pass / 0 fail / 12 assertions. Current-head hosted CI 39901473620, including the gates/Typecheck step, succeeded. The current review-thread snapshot has no unresolved findings or standing maintainer objection.
The full local suite/typecheck were not repeated on the shared capped host; the completed hosted Ubuntu run supplies broader validation. Approval is bound to this source head and diff, and does not certify a later dev union, native platforms or live provider/client behavior. All external sends in the focused behavior fixtures were stubbed; no real account, runtime configuration, service or OS trust state was changed.
|
Maintainer sweep: squash-merge blocked by conflict with |
|
Integration evidence for exact head ceef0a0 (a dev merge on top of the reviewed change; owner decision D1 approved merging):
Static merge simulation on current dev a564487 (this Mac): dev=a564487c2eb2b72362f01f0cbf801278d2010711 base=47316142fe06171a94de02d63f1727dbed4c0382 pr_files= 7 overlap=[scripts/test-layout/layout.json tests/fixtures/test-layout-expected.json ]; merge clean; install exit=0; gui install exit=0; tsc exit=0; tsc doctor exit=0; structure:check exit=0; privacy:scan exit=0; Merged by the triage manager as maintainer integration (MAINTAINERS.md, dev only) under the owner's static-judgment rule while GitHub Actions is degraded; dev push CI will be checked when Actions recovers. |
Summary
Hard-lock fallback profiles now check their first ranked destination against the model/provider fallback settings before final authentication or dispatch. With provider fallback enabled and model fallback disabled, selecting a different model on the same provider returns
429 fallback_refused; enabling model fallback continues to allow that selection.Declaration-level pre-admission is unchanged. Handler regressions use a synthetic caller-owned credential and a strict fetch stub. The branch was reconciled with
dev@c4d4bb2da2162e23c314befafc5ec104292902a0through a normal merge, preserving the landed combo first-dispatch guard and replay-principal changes. The remaining profile delta is seven files.Verification
2d601aabc5b0021056af39a1d3cd11df97eb93dd; parents are the prior profile head and the dev commit above. The original model-off regression returned 200 before the guard and 429 after it; the model-on control remains allowed.2d601aabc5b0021056af39a1d3cd11df97eb93dd. No live provider request, billing, full local suite or native service operation was tested.Checklist
Summary by CodeRabbit