Repository navigation
fix(codex): preserve credit recovery retry hints and scoped cooldowns - #7622
Conversation
|
Caution Review failedThe pull request is closed. Finishing Touches
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 |
|
⏳ DRAFT
What to do
Review readiness checklist
1/4 boxes ticked. Automatic draft conversion failed. Please convert this pull request to a draft manually until every box above is ticked. |
|
@lidge-jun @Ingwannu — could you please review and approve the fork workflow runs for this draft at head The separate PR hygiene and enforce-target jobs both failed with I audited the author checklist and checked only branch currency: current dev is 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. |
|
Triage note (2026-10-24, against 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 ( Exact-head CI at triage time: Please refresh onto current |
|
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 ( Not required for 2.93.0. After the release: refresh #7621 first, then this branch, and complete readiness; both will be reviewed together. |
|
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 ( |
# Conflicts: # tests/fixtures/test-layout-expected.json # tests/responses/direct-credit-renewal-ingress.test.ts
|
Maintainer dev integration record (v2.93.0 release-prep lane L-D2, release-blocking), under the MAINTAINERS.md maintainer-integration exception for
Landing as a squash merge. |
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. |
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.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.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 head9995b64a2ffaa68d9a04fc3ee2f865a9e559eff0; 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
devas needed and revalidate the actual diff before readiness. A changed parent also requires integration revalidation. This PR includes the durableRetry-After: 300assertion in the existing Responses-handler test.Verification
Bun 1.4.3 on macOS; tested base
c4d4bb2da2162e23c314befafc5ec104292902a0. Published tree8fd9ee1bad8d4c84691a3d0595de690f73c383e0is 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.response.completedproves 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 buildThe changed-suite selection overlaps the focused run; counts are not additive.
test:changedis import-connected coverage, not the full suite.Known validation failures (not waived)
The final combined
bun run test:changedrun 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:tests/codex-integration/catalog-remote-pull.test.tscatalog-remote-pull/transport-evaltests/cli/cli-route-policy-write.test.tscli/route-policy-write-editortests/codex-integration/codex-shim-path-readiness.test.tscodex-shim-path-readiness/evalAll 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) andtests/helpers/compaction-routing-fixtures.ts:63(missing requiredOcxConfig.port, TS2352). Both files are unchanged. Source-only typechecking passes but excludes tests. Neither broad validation nor test typing is all green.Checklist
Author checklist audit (2026-10-23)
devisc4d4bb2da2162e23c314befafc5ec104292902a0; 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.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.