Skip to content

fix(codex): enforce stored-account credit policy through dispatch - #6606

Merged
lidge-jun merged 9 commits into
lidge-jun:devfrom
luvs01:codex/upstream-stored-credit-guard-20261005
Oct 5, 2026
Merged

lidge-jun merged 9 commits into
lidge-jun:devfrom
luvs01:codex/upstream-stored-credit-guard-20261005

Conversation

@luvs01

@luvs01 luvs01 commented Oct 5, 2026 •

Copy link
Copy Markdown
Collaborator

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 by applyCodexAuthContextToProvider in prepareResponsesRequest. 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 dev at 7e3e37a, which includes #6616 (no conflicts; #6616's two-argument sanitizeNonReplayableUpstreamError call and both structure-doc edits are kept). No local tests were run, per owner instruction; behavioral evidence is hosted CI at this exact head.

  • Hosted CI at this head: Cross-platform CI 37344040163 (all four Linux test shards, gates, aggregate ci: success) and React Doctor 37344040077: success; PR hygiene, target, and labeler checks success.
  • Static checks at this head (exit 0): 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.md and responses-failover.md stay at 600 lines.
  • New regressions in 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/responses and compact.
  • Independent security review (gpt-6.1-sol, static, credential and spending-consent boundary) at this head: APPROVE-WITH-NOTES: fix(codex): enforce stored-account credit policy through dispatch #6606 (comment)

Earlier author evidence

Current upstream merge (fef183a0)

Published head: fef183a05c7d169a5a289f79cc4cfb406ad98dcf, tree a8862f31f0abd8534816f84ddfe421e3575fba81. Ordinary merge commits retain the complete preceding head d9c8e561dbe6c0797bcddac2d6aeffc5f18d7c3b and upstream dev at publication, 4a7d96e165ba3b5f36e038d314fa5f9e5d75186f without rewriting history. The only conflict was adjacent imports in core-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.

  • Security/session regression command: 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 merge 574e1249d65a6d09ed0c6df277df4c611b544b66. 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.
  • The preceding default-timeout execution returned 578 pass / 4 timeout failures / 4 subsequent asynchronous errors. The new upstream affinity cases exceeded Bun's five-second default; the separate affinity rerun with the 30-second CI timeout passed all 13 cases, and the complete 23-file rerun above passed. The default-timeout run is not passing evidence.
  • On final head: bun run typecheck, bun run privacy:scan, bun scripts/file-size-ratchet.ts, and git diff 4a7d96e165ba3b5f36e038d314fa5f9e5d75186f HEAD --check — pass. The incoming upstream-only GUI delta also passed cd gui && bun test tests/consequence-dialog.test.tsx tests/integrations-surfaces.test.tsx (64 pass / 0 fail / 280 assertions) and bun run build.
  • On 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), and cd 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.
  • Merge aggregate: bun scripts/test.ts --changed=upstream/dev --parallel=2 --timeout 30000, with comparison merge base 6af4ef4c0a9c878bdaf4a68e0cca16c6eef8654e on 574e1249, hit the repository's 900-second limit (exit 124). The incomplete captured log contains 608 failure entries and 360 occurrences of the existing /tmp coordinator 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.
  • Independent read-only final-diff review: approve, no remaining concrete blockers. The reviewer verified both merge trees and all external guards; all 30 PR paths are unchanged and the final patch against the latest dev is identical to the reviewed artifact (SHA256 bdfd603e6ecdb123694ff5a79f60aa871bf98890151ef91d32e2564c3586408a). This is author-supplied technical evidence, not project-maintainer acceptance.
  • Exact-head hosted CI: pass — Cross-platform CI 37273481666 and React Doctor 37273481671 completed successfully for fef183a05c7d169a5a289f79cc4cfb406ad98dcf at 2026-10-05 06:50 UTC. Cross-platform CI reports 17 successful jobs / 8 skipped, including all four Linux test shards and the aggregate ci job. 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 dev 62fa24d70 at 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, tree 7932b433cea20c43dc2cf96fbdba16d4823cdc6f; parent 1ee8b0f9e243b07a8e64b8e9cd38ea65ce69cc8f. The integration base remains e53c8077e40b6f540bf1e5d5791a55282466bfac.

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/search relay. 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:

  • Red/green: on the preceding source, late quota/consent cases produced 2 pass / 4 fail because all four refusal cases sent one mocked request. The same cases passed after the guard wiring. Independent review then identified an abort/refusal race; its two regression cases failed with erroneous connect_error records before the final correction.
  • Before that final two-condition 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.
  • On final source, 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 with ENOENT for a tracked file omitted by the sparse checkout.
  • After adding only devlog/_fin/260905_test_modularization_and_windows/001_test_inventory.md and verifying its HEAD blob 44defb0acd36149161be3a9355f6ab2e35aefbef, 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.
  • Final bun run typecheck, bun run privacy:scan, bun run structure:check, and git 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.
  • Dynamic coverage includes resolver-to-executor wiring, quota/consent changes, reset/429 rechecks, real fetch/runTurn search-loop wiring, caller-abort races, Direct controls, probe return, and reset-bound relay refusal with zero provider sends.
  • The full local suite and docs build were not repeated for this follow-up while concurrent worktrees share test resources. Earlier source-bound validation below remains historical. Exact-head hosted Cross-platform CI 37255666248 completed successfully on d9c8e561dbe6c0797bcddac2d6aeffc5f18d7c3b at 2026-10-05 04:31 UTC: 17 jobs passed / 8 skipped, including all four Linux test shards and the aggregate ci job. 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.
  • Ancestry rechecked against live dev 829a18ba94941aa7997d34cbd88f97390844d79b: the published head is 2 commits ahead / 1 behind, with merge base e53c8077e40b6f540bf1e5d5791a55282466bfac. 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 dev at e53c8077e40b6f540bf1e5d5791a55282466bfac.
Published head: 1ee8b0f9e243b07a8e64b8e9cd38ea65ce69cc8f (tree b6cee1d3f6aeda196e656b86723878c1a388bf6f).

  • Isolated before/after reproduction: the unmodified base admitted and materialized an exhausted opted-out exact account; the changed source refused it. The below-limit control remained usable. Both executions recorded zero fetch calls.
  • New physical-dispatch regressions failed before the dispatch guard and pass after it. Queued quota/consent changes prevent sends; an unchanged opt-in remains usable.
  • Focused command: 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, and git diff --check — pass.
  • GUI lint gate: bun run lint:gui:if-changed with 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.
  • Aggregate limitation: bun scripts/test.ts --changed=upstream-credit-guard-base-20261005 was interrupted for review corrections and is not passing evidence; it exposed widespread failures before interruption. The final candidate's owning-domain run bun scripts/test.ts --parallel=2 ./tests/codex-integration/ ./tests/responses/ --timeout 30000 completed 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 ran OCX_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.ts and 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 encountered CodexUserIdentityRefusal: this container's /tmp is owned by UID 65534, while the existing POSIX coordinator requires root ownership. Permissions and ownership were not changed.
  • Independent read-only final-diff review: approve, no remaining concrete blockers after confirming and correcting late retry materialization/error cleanup and queued portable-compaction dispatch. Separate dynamic harnesses confirmed a reset-bound 429 with zero portable sends, and cancellation of the prior response on a late retry refusal. Physical-dispatch verification covered main Responses and portable compaction; direct vision/search helper dispatch was not dynamically verified.
  • Exact-head hosted CI: pass — Cross-platform CI run 37248988452 and React Doctor run 37248988393 both completed successfully for head 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

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed locally for secrets, auth, and unsafe defaults. Required project-maintainer authentication/credential and spending-consent boundary review is pending, as recorded above.

Review readiness checklist

  • Selected exact-head automated repository validation passed; commands, hosted results, skipped coverage and the historical local aggregate limitation are recorded above.
  • Branch ancestry rechecked against dev 62fa24d70 (4 ahead / 2 behind; mergeable). Recheck again if either branch changes before readiness.
  • All correct review findings are resolved.
  • Ready for review.

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>

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>
@coderabbitai

coderabbitai Bot commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

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
  • Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: d12f1822-b6e4-4c29-ba39-59d9fc13caa0
📥 Commits

Reviewing files that changed from the base of the PR and between 7e3e37a and abb9918.

📒 Files selected for processing (31)
  • docs-site/src/content/docs/guides/codex-integration.md
  • docs-site/src/content/docs/ko/guides/codex-integration.md
  • scripts/test-layout/layout.json
  • src/codex/account-credit-use.ts
  • src/codex/auth-context.ts
  • src/codex/pool-credit-policy.ts
  • src/providers/openai-sidecar-credit.ts
  • src/providers/openai-sidecar.ts
  • src/server/responses/adapter-dispatch.ts
  • src/server/responses/compact.ts
  • src/server/responses/core-auth.ts
  • src/server/responses/core-codex-account.ts
  • src/server/responses/passthrough-dispatch.ts
  • src/server/responses/request-prepare.ts
  • src/server/responses/sidecar-execution.ts
  • src/server/search.ts
  • src/vision/describe.ts
  • src/vision/index.ts
  • src/web-search/executor.ts
  • src/web-search/loop.ts
  • src/web-search/passthrough-bridge.ts
  • src/web-search/run-turn-loop.ts
  • structure/codex-account-controls.md
  • structure/providers-and-adapters.md
  • structure/runtime.md
  • structure/transports/responses.md
  • tests/codex-integration/codex-pool-credit-policy.test.ts
  • tests/codex-integration/codex-sidecar-credit-policy.test.ts
  • tests/fixtures/test-layout-expected.json
  • tests/responses/reserve-dispatch-ws.test.ts
  • tests/responses/responses-pool-401-refresh.test.ts
 ___________________________________________________________
< Ultimately, we're all just debugging someone else's code. >
 -----------------------------------------------------------
  \
   \   (\__/)
       (•ㅅ•)
       /   づ
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Oct 5, 2026

@Ingwannu Ingwannu left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@luvs01

luvs01 commented Oct 5, 2026

Copy link
Copy Markdown
Collaborator Author

The previously unverified direct vision/search dispatch paths are now covered and corrected in d9c8e561dbe6c0797bcddac2d6aeffc5f18d7c3b.

The resolved stored-account credit policy is rechecked immediately before physical vision/search sends, reset and 429 retries, both search loops, passthrough search and /v1/alpha/search. A local policy refusal releases an unused probe without misclassifying valid credentials as connection/authentication failures, including a caller-abort race. Caller-owned Direct and native-main policy remain unchanged.

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.

luvs01 and others added 6 commits October 5, 2026 06:31
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.
@lidge-jun

Copy link
Copy Markdown
Owner

Independent security review — head 4fa86eca9ee8ccbcfb8e5cee16177daecd32e61f

Verdict: APPROVE-WITH-NOTES (independent gpt-6.1-sol reviewer, static source review of all 31 changed files, bound to this exact head).

  • No stored-account spending-consent bypass on the canonical dispatch paths reviewed: passthrough and adapter sends (initial, rebuilt, retry), alternate-account recovery, native compact, WebSocket dial and frames, vision/search helpers, and the /v1/alpha/search relay. Each runs the credit-policy dispatch guard after pacing and before upstream contact.
  • Policy refusals keep credentials (no quarantine or clearing), release probe leases, and return the reset-bound 429 with Retry-After; cancellation now takes precedence (499) on the late materialization, provider materialization, and post-refresh boundaries.
  • Explicit opt-in with fresh spendable-credit evidence, caller-owned Direct auth, native-main policy, and Reserve authorization keep their existing boundaries. No new sensitive logging.
  • Note (Low, test robustness): the pacing-window regression in tests/responses/responses-pool-401-refresh.test.ts relies on a 200 ms window with polling; a controlled pacing clock would make it timing-independent. It passes hosted CI at this head.

Earlier rounds: 9ea18a2 approved with one Medium (cancellation vs post-refresh refusal), fixed in 4fa86ec.

No local tests were run during review (owner instruction). Hosted CI at this head: Cross-platform CI 37341360745 and React Doctor 37341360493, both success.

@lidge-jun

Copy link
Copy Markdown
Owner

Independent security review — head abb99186899443366e937d6a6088da6e28abf50f

Verdict: APPROVE-WITH-NOTES (independent gpt-6.1-sol reviewer, static review of all 31 changed files and the merge with dev at 7e3e37a, which now contains #6616). Supersedes the review at 4fa86ec.

  • The merge preserves both responsibilities. Credit-policy dispatch guards stay on initial and rebuilt sends. fix(responses): redact terminal reset-replacement errors #6616's terminal non-replayable projection keeps its two-argument call and finally cleanup in adapter-dispatch.ts and passthrough-delivery.ts.
  • No spending-consent bypass or credential-disclosure regression. Ordinary policy refusals return 429 with Retry-After, release unused probes, avoid quarantine, and yield to cancellation (499). Explicit opt-in, native-main, caller-owned Direct, and Reserve stay separate. No new sensitive logging.
  • Note (Low, diagnostics only): if a credit hold lands after an ambiguous-reset replacement has already been claimed, the guard still prevents the send, but the client sees the replay refusal instead of the credit message. A follow-up could qualify the docs-site sentence and cover the combined case.

No local tests were run during review (owner instruction). Hosted CI at this head: Cross-platform CI 37344040163 and React Doctor 37344040077, both success.

@lidge-jun
lidge-jun marked this pull request as ready for review October 5, 2026 17:06
@lidge-jun
lidge-jun self-requested a review as a code owner October 5, 2026 17:06
@lidge-jun

Copy link
Copy Markdown
Owner

Maintainer integration (lidge-jun, dev only) — 2026-10-05 bug triage round 2, lane B4.

  • Head: abb9918
  • CI at this head: PR CI 37344040163, 37344040077 (refreshed after fix(responses): redact terminal reset-replacement errors #6616); every executed job passed.
  • Review: stored-account credit policy enforced through dispatch (sidecar, search, vision, pool paths); late credit refusals release probe leases; cancellation wins over late refusal; security review APPROVE-WITH-NOTES at this head; marked ready by maintainer.
  • No local tests were run, per owner instruction; hosted CI is the evidence.
  • scripts/ci/assert-mergeable-review.sh --maintainer-integration: OK.

@lidge-jun
lidge-jun merged commit d00ec9f into lidge-jun:dev Oct 5, 2026
37 of 38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants