Skip to content

fix(codex): preserve credit recovery retry hints and scoped cooldowns - #7622

Merged
lidge-jun merged 12 commits into
lidge-jun:devfrom
Caledvwlch:codex/credit-retry-after-stacked
Oct 26, 2026
Merged

lidge-jun merged 12 commits into
lidge-jun:devfrom
Caledvwlch:codex/credit-retry-after-stacked

Conversation

@Caledvwlch

@Caledvwlch Caledvwlch commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Summary

A local stored-account refusal caused by stale or unavailable credit evidence can tell clients to wait for the included-quota reset, potentially days away. For opted-in accounts, advise retry at the earlier of that reset and the existing 300-second metadata cadence, preserving any later applicable upstream cooldown and the original resetAt.

  • Carry the selected quota scope through resolution, post-token checks, materialization/dispatch, provider conversion and sidecar guards. Account-wide holds still apply; unrelated model-group holds do not.
  • Treat unlimited credits as overriding a finite zero balance, never an explicit spending veto or the five-minute freshness requirement.
  • Distinguish parser-proven unavailable evidence from known denial with creditObservationUnavailable, paired only with null. Preserve the pair through partial writes and persistence, and clear it on replacement. Legacy unmarked null, opt-out and explicit spending/overage/no-credit vetoes retain conservative advice.
  • Missing information still cannot authorize spending, select a different account or trigger a refresh. Main/sub identities and consent remain separate.

Dependency and merge order

Depends on #7621. Merge #7621 first; do not merge this draft independently.

Both drafts target upstream dev, because the parent branch exists in the contributor fork. This PR therefore currently has a cumulative diff including all of #7621, rather than an independent Retry-After-only diff. Its first commit is the exact parent PR head 9995b64a2ffaa68d9a04fc3ee2f865a9e559eff0; the second commit is this follow-up.

For the incremental review, compare the parent head with this follow-up. After the parent lands, update/rebase this branch onto the resulting dev as needed and revalidate the actual diff before readiness. A changed parent also requires integration revalidation. This PR includes the durable Retry-After: 300 assertion in the existing Responses-handler test.

Verification

Bun 1.4.3 on macOS; tested base c4d4bb2da2162e23c314befafc5ec104292902a0. Published tree 8fd9ee1bad8d4c84691a3d0595de690f73c383e0 is byte-identical to the final local integration tree, verified by fetching the remote commits and comparing files. Remote commit metadata differs from the local review history.

  • 356 focused tests passed, 0 failed, across 16 files. Covers parser → store → auth refusal; unlimited/zero; fresh-process disk restoration; main identity and stale writers; 900-second selected-scope, 3600-second account-wide and unrelated Reserve holds; seven refusal boundaries; post-token checks; renewal revocation and unchanged account separation.
  • The original defects had failing pre-fix regressions; the additional main persistence cases are coverage additions, not claimed as historical pre-fix RED runs.
  • Source typecheck, structure, privacy and size-ratchet checks pass; docs build passes with 593 pages. Four local Sub Astra reviewers split implementation cross-review, credit semantics and final integration/evidence review; this does not replace maintainer security review.
  • Tests use isolated fixtures and mocked HTTP. The handler test advances virtual time by 360001ms and invokes renewal explicitly; its fabricated response.completed proves neither real timer execution nor live-provider completion/billing.
Focused and adjacent validation commands
bun run test \
  tests/codex-integration/codex-direct-credit-renewal.test.ts \
  tests/codex-integration/codex-direct-renewal-revocation.test.ts \
  tests/codex-integration/codex-credit-retry-after.test.ts \
  tests/codex-integration/codex-credit-scoped-retry.test.ts \
  tests/responses/direct-credit-renewal-ingress.test.ts \
  tests/codex-integration/codex-pool-credit-policy.test.ts \
  tests/codex-integration/codex-sidecar-credit-policy.test.ts \
  tests/codex-integration/codex-quota-parser-parity.test.ts \
  tests/codex-integration/codex-credits-after-limit.test.ts \
  tests/codex-integration/codex-credits-after-limit-main.test.ts \
  tests/codex-integration/main-quota-provenance.test.ts \
  tests/codex-integration/codex-quota-auto-refresh-generation.test.ts \
  tests/codex-integration/codex-account-store-quota-isolation.test.ts \
  tests/codex-integration/codex-quota-prime.test.ts \
  tests/test-layout.test.ts tests/test-layout-tooling.test.ts
bun run test:changed
bun node_modules/typescript/bin/tsc --noEmit
bun run structure:check
bun run privacy:scan
bun scripts/file-size-ratchet.ts
bun run --cwd docs-site build

The changed-suite selection overlaps the focused run; counts are not additive. test:changed is import-connected coverage, not the full suite.

Known validation failures (not waived)

The final combined bun run test:changed run on Bun 1.4.3/macOS finished with 42,583 passed, 109 skipped, 3 failed across 1,961 files (exit 1). The three failures occur in setup with [cold-spawn-warmup] ... scanned no repository module to warm:

Test Warmup graph
tests/codex-integration/catalog-remote-pull.test.ts catalog-remote-pull/transport-eval
tests/cli/cli-route-policy-write.test.ts cli/route-policy-write-editor
tests/codex-integration/codex-shim-path-readiness.test.ts codex-shim-path-readiness/eval

All three were rerun against an unmodified archive of base c4d4bb2da2162e23c314befafc5ec104292902a0, sharing the installed dependencies: 103 passed / the same 3 setup failures. This is a targeted base control, not a clean dependency installation or a full baseline run. The third file entered the broader import-connected selection after the quota parser/storage change.

Explicit test-dependency typing also exits 1 with the same two base diagnostics: scripts/test-temp-lock.ts:81 (SyncSubprocess.signal, TS2339) and tests/helpers/compaction-routing-fixtures.ts:63 (missing required OcxConfig.port, TS2352). Both files are unchanged. Source-only typechecking passes but excludes tests. Neither broad validation nor test typing is all green.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults. Independent local review and privacy checks are complete; explicit maintainer security review is still required.

Author checklist audit (2026-10-23)

  • Branch currency confirmed: current upstream dev is c4d4bb2da2162e23c314befafc5ec104292902a0; this head is 0 commits behind it (2 commits ahead). The published tree still matches the recorded locally tested tree. Recheck after any head/base change.
  • Local validation remains incomplete, not waived: the documented focused checks pass, but the combined changed suite exits 1 with three setup failures reproduced on the unmodified base, and explicit test-dependency typing retains two base diagnostics. Full-suite and other-platform coverage remain outstanding. The AGENTS.md resource exception does not permit ignoring failures; no blanket passed attestation is made.
  • Local independent review is recorded above. Hosted CodeRabbit review has not run because this is a draft; absence of findings is not treated as completed review. The findings-resolution box remains open pending that review and resolution of any valid findings.
  • Maintainer security review and exact-head CI approval/execution are requested separately. Final readiness remains unchecked; this PR stays a draft. After fix(codex): renew credit evidence for namespace-bound Direct accounts #7621 lands, revalidate the actual follow-up diff against the resulting dev before readiness.

No production installation, configuration, account bindings, daemon restart, merge or deployment is part of this submission. Full-suite validation, other platforms, exact-head hosted CI and maintainer review remain outstanding. No all-passed attestation is made.

Review readiness checklist

This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:

  • Required local validation passed; commands, results, and any full-suite exception are documented.

  • I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).

  • I resolved all correct Codex and CodeRabbit findings.

  • My PR is ready for review.

@coderabbitai

coderabbitai Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Caution

Review failed

The pull request is closed.

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.

❤️ Share

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

@github-actions github-actions Bot added the intake: hygiene-blocked Deterministic PR hygiene checks failed label Oct 23, 2026
@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Deterministic hygiene checks failed.

  • unsponsored_surface — This changes an authentication, workflow, release-automation, or dependency surface. MAINTAINERS.md requires security review for these; ask a maintainer to apply maintainer-sponsored once they have reviewed it. Paths: src/codex/auth-context.ts.

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

github-actions Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

⏳ DRAFT

  • review readiness checklist open (1/4 boxes ticked).

What to do

  • Tick all four boxes in the PR description once you're done (currently 1/4).

Review readiness checklist

  • ⬜ Required local validation passed; commands, results, and any full-suite exception are documented.
  • ✅ I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).
  • ⬜ I resolved all correct Codex and CodeRabbit findings.
  • ⬜ My PR is ready for review.

1/4 boxes ticked.

Automatic draft conversion failed. Please convert this pull request to a draft manually until every box above is ticked.

Copy link
Copy Markdown
Contributor Author

@lidge-jun @Ingwannu — could you please review and approve the fork workflow runs for this draft at head a74dbb3f65f21edab44252ceb032729bc08790e1? Cross-platform CI, React Doctor and Catalog async contracts are awaiting maintainer approval; for example: https://github.com/lidge-jun/opencodex/actions/runs/39946437519 .

The separate PR hygiene and enforce-target jobs both failed with unsponsored_surface for src/codex/auth-context.ts. Please perform the explicit security review required by MAINTAINERS.md and, if acceptable after review, apply maintainer-sponsored. Please check quota-scope propagation and account-wide cooldown preservation across refusal/dispatch boundaries, the persisted unknown-credit marker, explicit spending vetoes and evidence freshness. Missing evidence must never authorize spending or switch accounts. The cumulative diff also includes the parent renewal-authority changes in #7621.

I audited the author checklist and checked only branch currency: current dev is c4d4bb2da2162e23c314befafc5ec104292902a0, with this head 0 commits behind. The published tree matches the recorded integration tree. The 356 focused tests pass; the changed suite remains 42,583 pass / 109 skip / 3 fail, with those three setup failures reproduced on the unmodified base. Two test-dependency typing diagnostics also reproduce on the base. The PR body contains the commands, failures and limitations. These failures are not waived. CodeRabbit has not reviewed the draft yet, so findings resolution and final readiness remain open.

Depends on #7621; merge the parent first. This is currently a cumulative diff, and the resulting follow-up diff must be rebased/updated and revalidated after the parent lands. This request is for CI execution and security review, not merge or deployment approval. No production deployment or account/configuration changes have been made.

@lidge-jun

Copy link
Copy Markdown
Owner

Triage note (2026-10-24, against dev at 2494848).

Needs fixes before this can land: land/reconcile #7621 first, revalidate the incremental diff, resolve the documented changed-suite/test-typing failures, and complete readiness. Maintainer sponsorship/security review and passing CI are also required.

Still needed on dev: partial. Main-account recovery advice exists, but stored-pool credit refusals still use the reset-bound constructor (src/codex/auth-context.ts:548, :568); the Direct renewal gate remains Pool-only (src/codex/auth-api/pool-mode-gate.ts:319).

Exact-head CI at triage time: a74dbb3 fail — hygiene, enforce-target: unsponsored_surface; Cross-platform CI, Catalog async contracts, and React Doctor await approval. lane=all on exact head: n.a.

Please refresh onto current dev when addressing these; heads older than dev need a fresh exact-head CI run before review.

@lidge-jun

Copy link
Copy Markdown
Owner

Release decision (v2.93.0 dev-merge, maintainer triage lane): DEFERRED_POST_RELEASE. The PR stays open; this is not a rejection.

This changes Codex credit-recovery and pool gating code on the credential surface (src/codex/auth-context.ts, src/codex/auth-api/pool-mode-gate.ts, src/codex/auth-api/pool-quota-probe.ts), which needs maintainer sponsorship and an explicit security review under MAINTAINERS.md (unsponsored_surface). It is stacked on #7621, which is deferred for the same reason, and the earlier triage asks (land/reconcile #7621 first, fix the reported changed-suite and test-typing failures, complete readiness) are still open. It is also a draft, 67 commits behind dev, and currently conflicts with it.

Not required for 2.93.0. After the release: refresh #7621 first, then this branch, and complete readiness; both will be reviewed together.

@lidge-jun lidge-jun added the maintainer-sponsored Maintainer sponsors this change to an auth, workflow, release, or dependency surface label Oct 26, 2026
@lidge-jun

Copy link
Copy Markdown
Owner

Maintainer sponsorship (v2.93.0 release-prep lane L-D2): the repository owner has classified this PR as release-blocking and granted admin sponsorship for its restricted surface (src/codex/auth-context.ts, src/codex/auth-api/pool-mode-gate.ts, src/codex/auth-api/pool-quota-probe.ts). I'm applying maintainer-sponsored as the sponsoring maintainer. Sponsorship does not replace the security review MAINTAINERS.md requires: an independent security review of the refreshed head runs before merge, and the merge record will note its result.

@github-actions github-actions Bot removed the intake: hygiene-blocked Deterministic PR hygiene checks failed label Oct 26, 2026
@lidge-jun

Copy link
Copy Markdown
Owner

Maintainer dev integration record (v2.93.0 release-prep lane L-D2, release-blocking), under the MAINTAINERS.md maintainer-integration exception for dev. This is maintainer integration, not an approving review.

  • Exact head: 4bd96fe3ab779d1fde0e1f655ea5e8a8a8e7b05f. No change requests; 0 unresolved review threads. The maintainer-sponsored label was applied under the owner's admin sponsorship (comment above). The parent, fix(codex): renew credit evidence for namespace-bound Direct accounts #7621, landed first as a3e22148a.
  • Maintainer-lane work:
    • d5fdae7bd merges the fix(codex): renew credit evidence for namespace-bound Direct accounts #7621 branch and current dev. It also fixes a scoping defect in src/codex/auth-api/pool-credit-policy.ts:93 and src/codex/auth-context.ts:553: a 600-second account-wide hold could mask a longer 900-second selected-scope hold. The 14 regressions in tests/codex-integration/codex-credit-scoped-retry.test.ts fail before the fix and pass after. The existing 24-hour Retry-After cap is verified.
    • 4bd96fe3a merges dev after fix(codex): renew credit evidence for namespace-bound Direct accounts #7621 landed (no rebase). Its tree is identical to the security-reviewed d5fdae7bd; the only conflicts were the add/add test file and the layout shard, where this branch's superset was kept.
  • Required CI on the exact head: Cross-platform CI 40374335382 (success); Catalog async contracts 40374335354 (success); Enforce PR target branch 40374331320 (success); PR hygiene 40374331175 (success).
  • Windows-sensitive (tests/helpers/** and Windows ACL test markers): Cross-platform CI lane=all 40374343662 on this exact SHA (branch ci/pr-7622-laneall-4bd96fe3) succeeded on attempt 2. On attempt 1, Windows 5/9 failed in tests/codex-integration/codex-cli-update-plan.test.ts ("a real npm call under the isolated argv…", spawnSync cmd.exe ETIMEDOUT after 30 s). This PR does not touch that file, and fix(codex): renew credit evidence for namespace-bound Direct accounts #7621's lane=all run passed it. Only the failed job was rerun.
  • Independent security review (separate agent) of d5fdae7bd, the same tree: PASS, 0 findings. It found no credential-boundary violation, admission bypass, cross-account or cross-scope leakage, secret serialization, or unbounded retry duration.
  • Maintainer recheck on this head merged onto origin/dev 9a61bd96b (after fix(reasoning): separate support, thinking toggles and effort controls #7531 and fix(codex): fence uncertain catalog entry identity #7610): bun test --isolate on codex-credit-retry-after, codex-credit-scoped-retry, main-quota-provenance, direct-credit-renewal-ingress, both layout guards and the file-size ratchet: 172 pass / 0 fail (pinned Bun 1.4.4; run with OCX_TEST_NO_QUEUE=1 because another lane's full suite held the shared queue). Typecheck exit 0, structure:check pass.
  • NOT RUN: live authenticated upstream credit/billing; full local suite (covered by lane=all).

Landing as a squash merge.

@lidge-jun
lidge-jun marked this pull request as ready for review October 26, 2026 20:46
@lidge-jun
lidge-jun merged commit 0f642c4 into lidge-jun:dev Oct 26, 2026
96 of 98 checks passed
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 26, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ⚠️ Failed 2026-10-26T20:48:15.869203Z
We couldn’t complete this review. Comment @codex review to try again.
4bd96fe Draft marked ready
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working maintainer-sponsored Maintainer sponsors this change to an auth, workflow, release, or dependency surface

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants