Repository navigation
fix(codex): enforce stored-account credit policy through dispatch - #6606
Conversation
Enforce credits-after-limit for exact stored selectors, credential acquisition, materialization, forced refresh copies and physical Responses sends. Map late policy refusals without credential quarantine and release unused recovery resources. Preserve spend-control parsing and independent main/Reserve policy. Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configuration
📒 Files selected for processing (31)
✨ 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. |
Ingwannu
left a comment
There was a problem hiding this comment.
Readiness triage for exact head 1ee8b0f9e243b07a8e64b8e9cd38ea65ce69cc8f against dev e53c8077e40b6f540bf1e5d5791a55282466bfac.
I verified that Cross-platform CI 37248988452 and React Doctor 37248988393 completed successfully on this head. The four Linux shards, gates and selected packaging/keyring smokes passed. Full Windows/macOS and other unselected native jobs were skipped; this is not universal native-platform certification.
Please keep the PR draft until the required maintainer authentication/credential and spending-consent boundary review is recorded and the final-head readiness conditions are satisfied. @lidge-jun: review ownership/acceptance is still needed. The local independent-agent review in Verification is author-supplied evidence, not a recorded project-maintainer approval. The sponsorship gate's push-permission exemption and green hygiene do not waive the review required by MAINTAINERS.md.
Retain the local aggregate failure result and its limited base comparison: the two-file control does not establish that all 651 failures were pre-existing. The description already distinguishes that correctly. Direct vision/search dispatch and installed native-client behavior remain explicitly unverified; settle the required coverage or scope exception with the reviewing maintainer. No live billed-provider run is requested, and fixture success must not be described as proof of actual charging behavior.
This is readiness/coordination triage only, not technical or security approval. I have not reviewed the technical patch, executed local tests/new scans, changed your branch, or authorized a workflow run.
|
The previously unverified direct vision/search dispatch paths are now covered and corrected in The resolved stored-account credit policy is rechecked immediately before physical vision/search sends, reset and 429 retries, both search loops, passthrough search and The description records the initial four red dispatch cases, two red abort-race cases and final regression evidence. All 19 new cases passed in the final five-file run; its only failure was a missing tracked inventory fixture in the sparse checkout, and that exact case passed after restoring the fixture. Earlier 173-pass coverage is identified as preceding the final abort correction, not a new final-head full-suite pass. New-head CI is queued. Draft, required project-maintainer authentication/spending-consent review and installed native-client acceptance remain outstanding. No live paid-provider request was made; the historical aggregate failures remain disclosed. |
Two policy refusals could leave a held pool probe behind: a credit hold that lands after header materialization in resolveResponsesCodexAuth (authCtx was scoped inside the try, so the catch could not release it), and a hold thrown by applyCodexAuthContextToProvider in prepareResponsesRequest, which had no mapper and escaped as a rejection instead of the actionable 429. Hoist the context and release it in the catch; wrap provider materialization, release, map to 429 (499 when cancelled), and rethrow unknown errors. Deterministic regressions in responses-pool-401-refresh cover both boundaries for quota and transient probes, cancellation, unknown errors, and the success path that keeps the probe for dispatch.
After a successful pool 401 refresh, a credit hold raised by the refreshed materialization was mapped to a 429 even when the client had already aborted. Check the request and effective signals first in both the Responses and compact refresh catches and return 499 without quarantine. Adds deterministic cancellation cases for request and effective signals on /v1/responses and the compact path; the non-aborted case now also asserts Retry-After.
Independent security review — head
|
Independent security review — head
|
|
Maintainer integration (lidge-jun, dev only) — 2026-10-05 bug triage round 2, lane B4.
|
Summary
Account-qualified Codex model selectors could choose an exhausted stored account directly, bypassing the default credits-after-limit selection hold. A request could consequently reach the upstream with credits available despite the account's spending opt-out.
This change enforces the existing policy before and after credential acquisition, during provider/header materialization, and at physical main Responses HTTP/WebSocket dispatch, including portable compaction, after pacing or retry waits. Direct vision/search helper dispatches and the standalone search relay now retain that live check through each physical send and retry. Resolver contexts retain private live policy references; forced-refresh context copies explicitly retain those bindings. Policy refusals return actionable reset-bound 429 responses, release unused probes, preserve valid credentials, and do not announce a replay send when refused before transport contact.
Explicit opt-in still requires fresh spendable-credit evidence. The spend-control parser and cache-revocation rules are unchanged. Native-main credit policy, its independent hard lock, caller-owned Direct authentication, and Reserve authorization keep their existing boundaries. A reset-less exhausted short window reports its freshness deadline.
Maintainer follow-up on this branch: two late refusal boundaries now release their pool probe lease and map to the actionable 429 (499 when cancelled) instead of leaking the probe or escaping as a rejection: a credit hold after header materialization in
resolveResponsesCodexAuth, and a hold raised byapplyCodexAuthContextToProviderinprepareResponsesRequest. Cancellation also takes precedence over a credit refusal raised after a successful pool 401 refresh (Responses and compact).Verification
Current head (
abb99186899443366e937d6a6088da6e28abf50f)Branch refreshed by merging
devat 7e3e37a, which includes #6616 (no conflicts; #6616's two-argumentsanitizeNonReplayableUpstreamErrorcall and both structure-doc edits are kept). No local tests were run, per owner instruction; behavioral evidence is hosted CI at this exact head.ci: success) and React Doctor 37344040077: success; PR hygiene, target, and labeler checks success.bun run typecheck,bun scripts/file-size-ratchet.ts,bun run structure:check,bun run privacy:scan,git diff --check.structure/runtime.md,structure/transports/responses.mdandresponses-failover.mdstay at 600 lines.tests/responses/responses-pool-401-refresh.test.ts(existing file, no layout change): late materialization hold (429/499, quota probe released), provider-materialization hold for quota and transient probes (429/499, host circuit disabled), unknown materialization error propagates after release, success path keeps the probe, and post-refresh refusal yields to cancellation for request and effective signals on/v1/responsesand compact.Earlier author evidence
Current upstream merge (
fef183a0)Published head:
fef183a05c7d169a5a289f79cc4cfb406ad98dcf, treea8862f31f0abd8534816f84ddfe421e3575fba81. Ordinary merge commits retain the complete preceding headd9c8e561dbe6c0797bcddac2d6aeffc5f18d7c3band upstreamdevat publication,4a7d96e165ba3b5f36e038d314fa5f9e5d75186fwithout rewriting history. The only conflict was adjacent imports incore-auth.ts; both the live credit-policy binding and upstream native-session alias normalization are retained. All vision/search follow-up guards and existing spend-control protections remain intact. Live ancestry was 4 ahead / 0 behind at publication; GitHub reports the conflict resolved and the PR mergeable.Validation used Linux and Bun 1.4.0 with synthetic account homes and mocked inference. Proxy environment variables were removed only for the test processes; no account, permission, credential or system-directory ownership settings were changed.
bun test --isolate --timeout 30000 tests/codex-integration/codex-pool-credit-policy.test.ts tests/codex-integration/codex-sidecar-credit-policy.test.ts tests/codex-integration/codex-credits-after-limit.test.ts tests/codex-integration/codex-credits-after-limit-main.test.ts tests/codex-integration/codex-quota-parser-parity.test.ts tests/codex-integration/reserve-dispatch.test.ts tests/responses/reserve-dispatch-ws.test.ts tests/responses/responses-pool-401-refresh.test.ts tests/responses/responses-fetch-helpers-boundary.test.ts tests/responses/responses-compaction-override.test.ts tests/responses/responses-compaction.test.ts tests/vision/sidecar-abort.test.ts tests/vision/vision-cache.test.ts tests/web-search/web-search-sidecar-429.test.ts tests/web-search/web-search-run-turn-loop.test.ts tests/web-search/web-search-passthrough-bridge.test.ts tests/claude-integration/claude-native-affinity.test.ts tests/server/caller-session-identity.test.ts tests/responses/chat-conversation-affinity.test.ts tests/test-layout.test.ts tests/test-layout-tooling.test.ts tests/lab/core-lab-boundary.test.ts tests/ci-workflows/structure-ssot.test.ts— 582 pass / 0 fail / 3,200 assertions across 23 files on the first merge574e1249d65a6d09ed0c6df277df4c611b544b66. All 30 PR source/test/doc paths remain byte-for-byte unchanged at the final published head. The intervening upstream-only GUI commit was checked separately below. This is not an assertion that the command was rerun after that unrelated merge.bun run typecheck,bun run privacy:scan,bun scripts/file-size-ratchet.ts, andgit diff 4a7d96e165ba3b5f36e038d314fa5f9e5d75186f HEAD --check— pass. The incoming upstream-only GUI delta also passedcd gui && bun test tests/consequence-dialog.test.tsx tests/integrations-surfaces.test.tsx(64 pass / 0 fail / 280 assertions) andbun run build.574e1249:bun run structure:check,bun run skill:surface:check, the GUI lint gate using actual PR paths (skipped, no GUI changes relative to dev), andcd docs-site && bun install --frozen-lockfile && ASTRO_TELEMETRY_DISABLED=1 bun run build— pass, 561 pages / 78,098 checked internal links. Those source surfaces are unchanged by the later upstream-only GUI commit. No new whole-suite local pass is claimed.bun scripts/test.ts --changed=upstream/dev --parallel=2 --timeout 30000, with comparison merge base6af4ef4c0a9c878bdaf4a68e0cca16c6eef8654eon574e1249, hit the repository's 900-second limit (exit 124). The incomplete captured log contains 608 failure entries and 360 occurrences of the existing/tmpcoordinator ownership refusal. No final test summary was produced; these are incomplete log counts, not final suite totals. This run is not passing evidence, and not every failure is attributed to the environment. The historical local failure totals and limited baseline comparison below remain disclosed.bdfd603e6ecdb123694ff5a79f60aa871bf98890151ef91d32e2564c3586408a). This is author-supplied technical evidence, not project-maintainer acceptance.fef183a05c7d169a5a289f79cc4cfb406ad98dcfat 2026-10-05 06:50 UTC. Cross-platform CI reports 17 successful jobs / 8 skipped, including all four Linux test shards and the aggregatecijob. The shard logs report 39,936 pass / 133 skip / 0 fail, identify this exact head, and include the stored-account, sidecar, refresh, session-affinity, WebSocket and search-loop regressions. Gates, structure, docs, API/storage, Docker and selected Windows/Linux packaging/keyring smoke jobs passed. Unselected native jobs remain skipped. Earlier-head receipts below do not validate this new head.Final ancestry rechecked after CI against live
dev62fa24d70at 2026-10-05 06:50 UTC: 4 ahead / 2 behind, with GitHub still reporting the PR mergeable. The two later CLI/documentation commits have no overlap with the 30 PR paths; they were not added to this tested head. Recheck ancestry and any required integration coverage before readiness.Draft, required project-maintainer authentication/credential and spending-consent review, and installed native-client coverage/acceptance remain outstanding. No live billed-provider test or installed native-client acceptance is claimed.
Earlier sidecar dispatch follow-up (
d9c8e56)Published head:
d9c8e561dbe6c0797bcddac2d6aeffc5f18d7c3b, tree7932b433cea20c43dc2cf96fbdba16d4823cdc6f; parent1ee8b0f9e243b07a8e64b8e9cd38ea65ce69cc8f. The integration base remainse53c8077e40b6f540bf1e5d5791a55282466bfac.The resolver now carries a live stored-account credit-policy check into every physical vision/search helper send, including reset and 429 retries, both search loops, passthrough search and the standalone
/v1/alpha/searchrelay. A late hold prevents the attempt, returns unused probes and is not misclassified as a connection/authentication failure. Caller-owned Direct and the independent native-main policy are unchanged.Validation used Windows with Bun 1.4.2, isolated synthetic account homes and mocked provider transport:
connect_errorrecords before the final correction.bun test --isolate tests/codex-integration/codex-sidecar-credit-policy.test.ts tests/codex-integration/codex-pool-credit-policy.test.ts tests/vision/sidecar-abort.test.ts tests/web-search/web-search-sidecar-429.test.ts tests/web-search/web-search-run-turn-loop.test.ts tests/vision/vision-cache.test.ts tests/web-search/web-search-passthrough-bridge.test.ts: 173 pass / 0 fail, 633 assertions.bun test --isolate tests/codex-integration/codex-sidecar-credit-policy.test.ts tests/vision/sidecar-abort.test.ts tests/test-layout.test.ts tests/test-layout-tooling.test.ts tests/lab/core-lab-boundary.test.ts: 76 pass / 1 fail, 776 assertions. All 19 new behavioral cases passed in that run; they were not a separate 19-test execution. The only failure was a source-as-data inventory check withENOENTfor a tracked file omitted by the sparse checkout.devlog/_fin/260905_test_modularization_and_windows/001_test_inventory.mdand verifying its HEAD blob44defb0acd36149161be3a9355f6ab2e35aefbef,bun test --isolate tests/test-layout-tooling.test.ts -t 'fixture histogram': 1 pass / 0 fail / 15 filtered, 3 assertions. The entire five-file group was not rerun.bun run typecheck,bun run privacy:scan,bun run structure:check, andgit diff --check: pass. The unmodified file-size evaluator passed for the 18 changed paths; no whole-repository ratchet pass is claimed for the sparse checkout. The official hygiene evaluator reports no failures for the follow-up patches. No guard, limit or shared test lock was bypassed.d9c8e561dbe6c0797bcddac2d6aeffc5f18d7c3bat 2026-10-05 04:31 UTC: 17 jobs passed / 8 skipped, including all four Linux test shards and the aggregatecijob. Gates (including typecheck, GUI tests and privacy), structure, docs build, API/storage, Docker and Windows/Linux keyring/npm-global smokes passed. Full Windows/macOS suites, desktop shell/widget and the other unselected native jobs were skipped; the separately scoped privacy job was skipped while privacy in gates passed. React Doctor 37255666227 also passed on this head. These hosted results do not replace the recorded failing local aggregate or prove installed/native-client acceptance. Required project-maintainer boundary review and native coverage disposition remain outstanding; Draft is retained.dev829a18ba94941aa7997d34cbd88f97390844d79b: the published head is 2 commits ahead / 1 behind, with merge basee53c8077e40b6f540bf1e5d5791a55282466bfac. GitHub reports it mergeable. No rebase or source push was performed just to chase the newer base; the CI receipt above belongs only to the published head.Earlier verification (
1ee8b0f9)Base: upstream
devate53c8077e40b6f540bf1e5d5791a55282466bfac.Published head:
1ee8b0f9e243b07a8e64b8e9cd38ea65ce69cc8f(treeb6cee1d3f6aeda196e656b86723878c1a388bf6f).bun test --isolate tests/codex-integration/codex-pool-credit-policy.test.ts tests/codex-integration/codex-credits-after-limit.test.ts tests/codex-integration/codex-credits-after-limit-main.test.ts tests/codex-integration/codex-quota-parser-parity.test.ts tests/codex-integration/reserve-dispatch.test.ts tests/responses/reserve-dispatch-ws.test.ts tests/responses/responses-pool-401-refresh.test.ts tests/responses/responses-fetch-helpers-boundary.test.ts tests/responses/responses-compaction-override.test.ts tests/responses/responses-compaction.test.ts— 289 pass, 0 fail, 1,456 assertions. Covers direct/default-Direct selectors, automatic selection, refresh/replay, context copies, spending-control revocation, probe release, policy errors, pacing, late same-account model recovery, portable compaction, legitimate opt-in, main policies, and synthetic WebSocket refusal/cleanup without HTTP fallback.bun test --isolate tests/test-layout.test.ts tests/test-layout-tooling.test.ts tests/lab/core-lab-boundary.test.ts tests/ci-workflows/structure-ssot.test.ts— 94 pass, 0 fail.bun run typecheck,bun run privacy:scan,bun run structure:check,bun scripts/file-size-ratchet.ts, andgit diff --check— pass.bun run lint:gui:if-changedwith the actual changed-file list — skipped as designed; no GUI changes. This repository has no separate backend lint script.cd docs-site && bun install --frozen-lockfile && ASTRO_TELEMETRY_DISABLED=1 bun run build— pass, 561 pages and 78,088 checked internal links.bun scripts/test.ts --changed=upstream-credit-guard-base-20261005was interrupted for review corrections and is not passing evidence; it exposed widespread failures before interruption. The final candidate's owning-domain runbun scripts/test.ts --parallel=2 ./tests/codex-integration/ ./tests/responses/ --timeout 30000completed with 10,607 pass / 46 skip / 651 fail / 1 error across 510 files (exit 1); it is not passing evidence. A separate detached checkout of the unchanged base ranOCX_TEST_NO_QUEUE=1 bun scripts/test.ts --parallel=1 ./tests/codex-integration/bearer-admission-routed-provider.test.ts ./tests/codex-integration/codex-auth-context.test.tsand reproduced 90 pass / 56 fail, including the same local-server 403 admission failures. That establishes pre-existing failures in this environment for those tests, not that every broader failure is pre-existing. The aggregate also encounteredCodexUserIdentityRefusal: this container's/tmpis owned by UID 65534, while the existing POSIX coordinator requires root ownership. Permissions and ownership were not changed.1ee8b0f9e243b07a8e64b8e9cd38ea65ce69cc8f. All four Linux test shards passed (reported 39,802 pass / 133 skip / 0 fail); logs include the new policy, refresh, WebSocket, and portable-compaction regressions. Gates, structure, docs, API/storage, Docker smoke, and selected Windows/Linux packaging and keyring smoke jobs passed. Unselected platform/native jobs were skipped. No older-head result is counted.Validation used synthetic credentials, mocked provider inference and a synthetic WebSocket. No live billed provider requests or installed native-OS tests were performed. The full local suite was not run; focused behavioral and explicit source-as-data checks provide local coverage, with wider coverage provided by the selected exact-head hosted CI jobs. A WebSocket handshake already dispatched remains an attempt even if a later hold prevents its create frame. Draft publication does not establish provider charging behavior or native-client validation.
Required maintainer review and remaining validation
The readiness review at the preceding published head requires this PR to remain Draft until a project maintainer records the authentication/credential and spending-consent boundary review and the final-head readiness conditions are satisfied. Review ownership/acceptance is pending; the local independent-agent review above is author-supplied evidence, not project-maintainer approval.
Direct vision/search dispatch now has the dynamic regression coverage recorded above. Installed native-client behavior remains unverified; the required remaining coverage or an explicit scope exception must be settled with the reviewing maintainer. No live billed-provider test is requested or claimed. The local aggregate failure result and limited two-file baseline comparison remain unchanged; that comparison does not establish that all 651 failures predate this patch.
Checklist
Review readiness checklist
dev62fa24d70(4 ahead / 2 behind; mergeable). Recheck again if either branch changes before readiness.Keep this PR in Draft. No merge or release is requested.
Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>