Repository navigation
fix(codex): refuse Reserve turns the Desktop authless opt-in cannot serve - #4968
Conversation
…erve With codexDesktopAuthless unset, every Luna Reserve affordance is inert, so a gpt-reserve request was forwarded as an ordinary native model and answered upstream with 'The usage limit has been reached' -- an error naming neither the cause nor the setting that would change it. Add isCodexReserveOptInMissing beside isCodexReserveRequestEligible as its strict complement for the flag reason only, and apply it at the ordinary Responses and compact seams before auth, host-circuit admission or any upstream byte. The refusal is a direct 400 invalid_request_error naming codexDesktopAuthless and the command that sets it; CodexReserveUnavailableError is unsuitable because its CodexAccountCooldownError base renders as a 429 rate_limit_error. Terminal helpers and non-native inbound wires are excluded: enabling the opt-in would not make a helper work, and a gpt-reserve selector on the Chat or Anthropic wire is an operator-authored route. Closes #4940
|
Warning Review limit reachedNext included review available in 42 seconds. View limit detailsLimit details: You’ve used all 10 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (7)
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 |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
리뷰 · 우선순위 76 / 80이 PR은 지금 고치는 방법은 간단합니다. 거절은 두 군데에서 합니다. 범위를 일부러 좁혔습니다. 터미널 vision/search 헬퍼는 빼 두었습니다. 옵트인을 켜도 헬퍼는 기존 types.ts/config.ts 큰 분할 캠페인과 겹치지 않습니다. 닫을 중복 PR도 보이지 않습니다. 라인 - 이게 무슨 문제다 경로 메인테이너의 판단이 필요한 지점
너의 추천 호스티드 CI가 초록이면 머지해도 됩니다. 술어·시임·400 형태·헬퍼/와이어 한정·회귀 행렬이 이 댓글은 grok-bot이 작성했습니다 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0eba5c6517
ℹ️ 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".
| && options.visionDescribeTerminal !== true | ||
| && isCanonicalOpenAiForwardProvider(route.provider) | ||
| && isCodexReserveOptInMissing(options.codexAuthPolicy ?? config, route.modelId, options.admission)) { | ||
| return formatErrorResponse(400, "invalid_request_error", CODEX_RESERVE_OPT_IN_REQUIRED_MESSAGE); |
There was a problem hiding this comment.
When a Responses combo selects canonical gpt-reserve as its first target while codexDesktopAuthless is off, this new 400 is consumed by core-combo.ts, where comboFailureDecision treats invalid_request_error as terminal, so the combo never tries its remaining declared targets; previously the upstream 429 advanced the combo. The compact path has the same regression and can return before its routed combo fallback. Give this local refusal a target-local code that the combo classifier can hop on, and let compact routes with route.combo reach the combo dispatcher, while retaining the direct-request 400.
Useful? React with 👍 / 👎.
|
✅ Deterministic PR hygiene checks passed. |
| // `gpt-reserve` selector reaching us over Chat or Anthropic Messages is an operator-authored | ||
| // route (a `claudeCode.modelMap` entry, say), not a Codex client that was forced onto Reserve by | ||
| // its own usage snapshot, and that route keeps whatever behavior it has today. | ||
| if (inboundWire === "responses" |
There was a problem hiding this comment.
This refusal has no structured code, so comboFailureDecision reads it as a plain invalid_request_error and returns stop (src/combos/failover.ts:703; policy-fallback.ts:89 uses the same decision). A failover combo or policy ladder with the canonical gpt-reserve as a target now ends the chain here, where the upstream 429 this replaces hops to the next target. Same trap as #1524 and #4903. Either skip the refusal when options.comboAttempt is set, or give it its own code plus a hop rule, and pin it with a combo case in the new test.
…shape
The alias provider fixture used "sk-reserve-optin-fixture", whose 24-character
body matches the scanner's sk-[A-Za-z0-9_-]{20,} token-looking pattern, so
bun run privacy:scan failed the gates job and the shard-4 batch that runs it.
Shortened to the same shape the existing reserve fixtures already use. The key
is inert either way; only its length mattered to the scan.
|
Merging with macOS legs outstanding, and recording why rather than leaving it implicit. At this exact head the full Linux suite (test 1/4 through 4/4), This change is platform-neutral, so waiting on a queue that is both saturated and known-unreliable would delay the work without adding information. The evidence that governs the release is not per-PR macOS legs; it is the full-platform Stating the boundary plainly: this is merged on Linux, gates and cross-platform smoke evidence at its exact head, with macOS coverage deferred to the candidate run rather than claimed here. |
2 participants
Summary
Closes #4940.
When Codex Desktop exhausts the ChatGPT allowance it collapses its picker and sends
gpt-reserve.With
codexDesktopAuthlessunset, every Luna Reserve affordance in this proxy is inert:isCodexReserveRequestEligible(src/codex/loopback-target.ts) requires the flag, so the catalogprojection, the
customReserveForwardmain-credential substitution, the reserve authorizationhandshake and the helper-unsupported guard all stay off. The request was therefore forwarded as an
ordinary native model and answered upstream with
429 The usage limit has been reached— an errorthat names neither the real cause nor the setting the operator would have to change. The reporter
saw
429 openai-<acct>/gpt-reserve routeKind=nativeand had no route back to a working model.This refuses that request locally instead of forwarding one the proxy can already prove will fail.
isCodexReserveOptInMissingsits besideisCodexReserveRequestEligibleinsrc/codex/loopback-target.ts, takes the samePick<OcxConfig, …>and admission shapes, and is thestrict complement of it for the flag reason only: exact
gpt-reserve,codexDesktopAuthless !== true,runtimeRole !== "client", and aloopbackadmission source. Callers classify the destination as acanonical OpenAI forward first, the same obligation
isCodexReserveHelperUnsupportedalready carries.Keeping the two functions adjacent is deliberate: flipping the flag always converts a
truehere intoa
truethere, which is what makes it honest for the refusal to name that one setting, and a testwalks the shared input space to prove they can never both hold.
Seam.
src/server/responses/request-prepare.ts, immediately beside the existing Reserve helperrefusal, and
src/server/responses/compact.ts, immediately after route resolution. Both run afteralias and combo resolution but before auth, host-circuit admission, the virtual-model rewrite and any
upstream byte.
input-admission.tswas considered and rejected: despite the name it is acontext-window gate whose provider-canonicality checks at lines ~171 and ~250 compose native context
caps, not admission facts, and it never sees
admission. Compact repeats the check rather thaninheriting it because its native branch dispatches straight to
/responses/compact; only the routedfallback replays through
handleResponses. The compact call composes the same three facts in the sameorder as
customReserveForwarda few dozen lines below it.Error shape. A direct
formatErrorResponse(400, "invalid_request_error", …)at each seam.CodexReserveUnavailableErrorwas rejected on inspection: it extendsCodexAccountCooldownError, andcooldownErrorResponserenders that base as429 rate_limit_error. It suppressesRetry-Afterforthis subclass and
cooldownErrorMessagereturns the subclass message verbatim, so the wording wouldhave survived — but the status and type would restate the exact upstream verdict this refusal exists
to replace, and
shouldMarkAccountNeedsReauthForCodexAuthFailurealready has to special-case it. A400 cannot be mistaken for a rate limit and inherits no retry semantics.
Message. Names
codexDesktopAuthless, givesocx system settings --desktop-authless on, statesplainly that Luna Reserve is not forwarded without the opt-in, and offers choosing another model.
It contains no account identifier, no token and no request body; the test asserts all three absences.
Two narrowings beyond the five conditions, both about not giving advice that does not hold, and
both of which also keep every existing Reserve contract in the suite intact.
would produce the existing
CODEX_RESERVE_HELPER_UNSUPPORTED_MESSAGErefusal instead — so tellingthat caller to enable the flag would be actionable and wrong. The helper guard keeps owning them.
gpt-reserveselector arriving over Chat or AnthropicMessages is an operator-authored route such as a
claudeCode.modelMapentry, not a Codex clientforced onto Reserve by its own usage snapshot, and it keeps the behaviour it has today.
Without those two qualifiers the change would have broken four existing controls that deliberately
pin "opt-in off, Reserve still dispatches": the
still-offandoff-to-on during owned authcases intests/server/reserve-ingress.test.ts(terminal helpers) and both variants intests/server/reserve-claude-policy.test.ts(Anthropic inbound wire). They are unchanged here.Not addressed, deliberately. The Desktop-side picker collapse is client behaviour the proxy does
not control: the app derives reserve mode from its own
backend-api/wham/usagepoll and rewrites theconversation model itself, consulting no catalog we produce. Emitting a bare
gpt-reservecatalog rowoutside authless is #3844, closed NOT PLANNED. An independently credentialed Reserve route is #4869.
This PR only replaces an unhelpful upstream 429 with a local refusal that names the missing opt-in.
Verification
Local verification was not run: this lane forbids running the local suite, typecheck, build, install,
or the
ocxbinary. Hosted CI is the executable verification for this change.Regression coverage extends
tests/codex-integration/reserve-dispatch.test.ts, which already drivesboth
handleResponsesandhandleResponsesCompactagainst a counted fetch fixture, so no new file andno
scripts/test-layout/layout.jsonbookkeeping. It asserts both sides:gpt-reserve, flag missing, loopback admission, canonical forward — refused on both the responsesand compact seams with 400
invalid_request_error, noRetry-After, the opt-in message, noaccount id/token/body in the text, zero upstream sends, zero WHAM reads, and untouched account and
upstream-host health.
gpt-reservewithruntimeRole: "client"— still forwards.gpt-reservewith a non-loopback (dedicated) admission source — still forwards.gpt-reserveon a non-canonical aliased provider route — still forwards.Static review of every other
gpt-reservetest in the tree found no behaviour change: the publiclistener cases carry non-loopback admission, the local listener cases set the flag, the keyed and
combo cases are non-canonical, and the helper and Anthropic cases are excluded by the narrowings above.
Checklist
Docs:
docs-site/src/content/docs/guides/codex-integration.mdgains the behaviour note in theexisting "Routed models during Codex reserve mode" section. Structure:
structure/providers/openai-tiers.mdrecords the predicate and error-shape contract next to theReserve compatibility contract it complements, and
structure/transports/responses.mdrecords the twoseams and their qualifiers. No GUI change, so no screenshot applies.
The refusal runs before authentication and emits no credential material; it strictly reduces what
reaches the upstream and changes no auth, OAuth, workflow or release path.