feat(codex): coordinate reset-credit recovery attempts - #1410
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds ChangesReset-credit recovery
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant RecoveryCoordinator
participant RecoveryDependencies
participant ConsumeAPI
Caller->>RecoveryCoordinator: recover(turn, generation, authorization)
RecoveryCoordinator->>RecoveryDependencies: revalidate generation and credit
RecoveryDependencies-->>RecoveryCoordinator: validated recovery state
RecoveryCoordinator->>ConsumeAPI: dispatch operation-ID-bound consume request
ConsumeAPI-->>RecoveryCoordinator: consume outcome
RecoveryCoordinator-->>Caller: normalized recovery result
Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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. |
✅ READY
Review readiness checklist
✅ 4/4 boxes ticked. This pull request is already Ready for Review. |
d6da400 to
9ce9b26
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/codex/reset-credit-recovery.ts`:
- Around line 439-447: Gate or make internal the resetProcessStateForTests
helper and its process registries so production code cannot clear
terminalByAccount; update src/codex/reset-credit-recovery.ts lines 439-447
accordingly. In tests/codex-reset-credit-recovery.test.ts lines 880-916, split
the post-reset confirmed scenario into a separate test and perform the reset in
beforeEach instead of mid-test.
- Around line 403-421: Update the static active-flight coordination around
CodexResetCreditRecoveryCoordinator so flights are not shared across
coordinators with different dependency sets or limits; include a
per-dependency-set identity in the flight key, or validate identity and limits
when joining and return a dedicated not-dispatched reason on mismatch. In
tests/codex-reset-credit-recovery.test.ts lines 315-359, give each coordinator
distinct consume implementations and assert which one executes to verify the
selected contract.
- Around line 320-322: Update hasFlightCapacity to emit an operational signal
whenever capacity is rejected, such as a log line or counter containing only the
current saturation count. Ensure the signal is triggered on the false path for
the MAX_TRACKED_RECOVERY_ACCOUNTS limit and never includes accountId. Preserve
the existing fail-closed not-dispatched/recovery-state-capacity behavior.
- Around line 145-148: Import CodexResetEligibleExhaustionCode from
quota-rejection.ts, type RESET_ELIGIBLE_CODES as
ReadonlySet<CodexResetEligibleExhaustionCode>, and cast value.semanticCode to
that type in authorizedResetRejection when calling has. Preserve the existing
eligible code values while making changes to the shared union produce
compile-time drift errors.
- Around line 546-572: Add a deadline timer field to RecoveryFlight, assign the
setTimeout handle from runFlightWithDeadline to that field, and clear it during
resetProcessStateForTests alongside the aborts and registry cleanup. Preserve
the existing finally cleanup and ensure the reset handles flights with pending
timers.
In `@tests/codex-reset-credit-recovery.test.ts`:
- Around line 405-410: Export MAX_TRACKED_RECOVERY_ACCOUNTS and
MAX_TRACKED_RECOVERY_FLIGHTS from the reset-credit recovery module, then update
both affected tests to derive loop bounds, expected consume-call counts,
terminal-generation counts, flight counts, and exhaustionGeneration from the
appropriate exported constants instead of literal 128 values. Preserve the
overflow assertions by using the constants consistently for capacity and the
subsequent over-capacity case.
- Around line 315-359: Update the concurrent-flight test around makeCoordinator
and consume so firstCoordinator and secondCoordinator use distinguishable
consume adapters that record which coordinator executed. Keep shared revalidate
behavior, then assert the recorded executedBy value explicitly alongside the
existing single-flight assertions, documenting the implementation’s intended
adapter ownership contract.
- Around line 749-752: Replace the single Promise.resolve microtask drain in the
late transport rejection test with the coordinator’s idle synchronization, such
as waitForIdleForTests, so execution has reached the retry decision before
asserting consumeCalls is 1. Ensure the idle signal actually tracks the captured
flight or otherwise resolves only after the consume loop exits, since the flight
may be removed from allFlights by its finally handler.
- Around line 774-784: Update the exact consume-outcome test.each block so each
case uses a unique suffixed accountId, following the pattern used by the other
multi-case tests, and pass that accountId when creating the logical turn or
coordinator input as appropriate. Confirm the generated identifiers satisfy
CODEX_ACCOUNT_ID_RE, while preserving the existing expected outcome assertions.
- Around line 176-201: Add focused negative recovery cases near the existing
`generic` and `wrongStatus` cases: one must preserve an otherwise authorized
rejection while setting `resetCreditEligible: false`, and another must preserve
valid kind/status and eligibility while using an unrecognized `semanticCode`.
Assert both results equal `generic` alongside the existing recovery assertions,
isolating each gate in `authorizedResetRejection`.
- Around line 133-146: Update the test to import and use MAIN_CODEX_ACCOUNT_ID
instead of hardcoding "__main__" in the accepted account-id case, ensuring it
follows the production contract. Extend the rejected account-id cases with
"__PROTO__" to verify case-insensitive handling of reserved identifiers while
retaining the existing canonical-format rejection coverage.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 4e8adcb2-4dd6-48d5-a1ef-4cd319e0a8e5
📒 Files selected for processing (2)
src/codex/reset-credit-recovery.tstests/codex-reset-credit-recovery.test.ts
|
Tip For best results, initiate chat on the files or code changes.
You are interacting with an AI system. |
|
|
![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: 9ce9b2685d
ℹ️ 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".
9ce9b26 to
bf189c5
Compare
|
|
|
Tip For best results, initiate chat on the files or code changes.
You are interacting with an AI system. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '=== PR diff summary ==='
git diff --stat HEAD^ HEAD || true
printf '%s\n' '=== Candidate files ==='
fd -t f '^(reset-credit-recovery\.ts|codex-reset-credit-recovery\.test\.ts)$' . || true
printf '%s\n' '=== Source outline ==='
ast-grep outline src/codex/reset-credit-recovery.ts --items all --view expanded 2>/dev/null || true
printf '%s\n' '=== Test outline ==='
ast-grep outline tests/codex-reset-credit-recovery.test.ts --items all --view expanded 2>/dev/null || true
printf '%s\n' '=== Changed source ==='
git diff --no-ext-diff -U80 HEAD^ HEAD -- src/codex/reset-credit-recovery.ts || true
printf '%s\n' '=== Changed tests ==='
git diff --no-ext-diff -U25 HEAD^ HEAD -- tests/codex-reset-credit-recovery.test.ts || trueLength of output: 83522 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '=== Coordinator state-management sections ==='
nl -ba src/codex/reset-credit-recovery.ts | sed -n '360,870p'
printf '%s\n' '=== Imported account and rejection contracts ==='
ast-grep outline src/codex/account-id.ts --items all --view expanded 2>/dev/null || true
ast-grep outline src/codex/quota-rejection.ts --items all --view expanded 2>/dev/null || true
nl -ba src/codex/account-id.ts | sed -n '1,220p'
nl -ba src/codex/quota-rejection.ts | sed -n '1,300p'
printf '%s\n' '=== Module and coordination scope references ==='
rg -n -C 4 --glob '!src/codex/reset-credit-recovery.ts' \
'(reset-credit-recovery|CodexResetCreditRecoveryCoordinator|coordinationScope|resetCodexResetCreditRecoveryProcessStateForTests)' \
. || true
printf '%s\n' '=== Relevant terminal-state test references ==='
rg -n -C 8 \
'(terminal|coordination-mismatch|scope|process-wide|same generation)' \
tests/codex-reset-credit-recovery.test.tsLength of output: 1364 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '=== Coordinator state-management sections ==='
cat -n src/codex/reset-credit-recovery.ts | sed -n '360,870p'
printf '%s\n' '=== Imported account and rejection contracts ==='
ast-grep outline src/codex/account-id.ts --items all --view expanded 2>/dev/null || true
ast-grep outline src/codex/quota-rejection.ts --items all --view expanded 2>/dev/null || true
cat -n src/codex/account-id.ts | sed -n '1,220p'
cat -n src/codex/quota-rejection.ts | sed -n '1,300p'
printf '%s\n' '=== Module and coordination scope references ==='
rg -n -C 4 --glob '!src/codex/reset-credit-recovery.ts' \
'(reset-credit-recovery|CodexResetCreditRecoveryCoordinator|coordinationScope|resetCodexResetCreditRecoveryProcessStateForTests)' \
. || true
printf '%s\n' '=== Relevant terminal-state test references ==='
rg -n -C 8 \
'(terminal|coordination-mismatch|scope|process-wide|same generation)' \
tests/codex-reset-credit-recovery.test.tsLength of output: 52056
As a result, coordinator A can complete a recovery for Include You are interacting with an AI system. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bf189c5bec
ℹ️ 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".
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/codex/reset-credit-recovery.ts`:
- Around line 552-574: Define and document an explicit retention policy for
terminalByAccount before the runtime adapter is introduced. Update
hasFlightCapacity and the terminal-outcome recording path to evict fences based
on their recorded credentialGeneration and exhaustionGeneration after the chosen
replay-protection lifetime, while preserving active-flight capacity checks and
allowing capacity for accounts whose fences have expired.
In `@tests/codex-reset-credit-recovery.test.ts`:
- Around line 46-73: Add a test using two coordinators with distinct
coordinationScope objects while keeping revalidate, consume, lifecycleSignal,
operationTimeoutMs, and maxConsumeAttempts identical, and assert they do not
share the irreversible consume path. Use makeCoordinator overrides and the
existing recovery test helpers to isolate and verify the coordinationScope
comparison in recoveryContractsMatch.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 97e446f1-dbe0-4b78-b578-011008e2bb5c
📒 Files selected for processing (2)
src/codex/reset-credit-recovery.tstests/codex-reset-credit-recovery.test.ts
bf189c5 to
1996e79
Compare
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '=== Current commit ==='
git rev-parse HEAD
git status --short
printf '%s\n' '=== Candidate files ==='
fd -t f '^(reset-credit-recovery\.ts|codex-reset-credit-recovery\.test\.ts)$' .
printf '%s\n' '=== Source structure ==='
ast-grep outline src/codex/reset-credit-recovery.ts --items all --view expanded
printf '%s\n' '=== Terminal-state and coordination references ==='
rg -n -C 8 \
'(terminalByAccount|terminal.*Key|coordinationScope|coordination-mismatch|recoveryContract|contract)' \
src/codex/reset-credit-recovery.ts
printf '%s\n' '=== Relevant source region ==='
nl -ba src/codex/reset-credit-recovery.ts | sed -n '330,560p'
printf '%s\n' '=== Regression-test references ==='
rg -n -C 10 \
'(different scope|coordinationScope|terminal.*scope|terminal.*generation|coordination-mismatch)' \
tests/codex-reset-credit-recovery.test.tsLength of output: 20476 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '=== Terminal-state contract regression tests ==='
rg -n -i -C 14 \
'(coordination-mismatch|coordination scope|different scope|terminal.*contract|terminal.*generation|same generation)' \
tests/codex-reset-credit-recovery.test.ts || true
printf '%s\n' '=== Test structure ==='
ast-grep outline tests/codex-reset-credit-recovery.test.ts --items all --view expanded
printf '%s\n' '=== Test file: relevant numbered excerpts ==='
cat -n tests/codex-reset-credit-recovery.test.ts | sed -n '1,420p'Length of output: 30434
In The comparison includes
You are interacting with an AI system. |
Wibias
left a comment
Wibias
left a comment
There was a problem hiding this comment.
Requesting changes on exact head 63c9feb1a4b4faa9ee7339c8c5ec5eb439e8e35e.
Two fail-closed invariants are still not proven for this irreversible-operation coordinator:
-
High: malformed per-request
options.signalcan reject the caller while leaving an authorizing waiter behind.lifecycleSignalis runtime-validated, butoptions.signalis not. A non-AbortSignalvalue can pass the initialsignal.abortedcheck, create/register a flight and waiter, then throw duringsignal.addEventListener(...)injoinFlight. That rejection does not roll backactiveWaitersor the output-guard registration. The scheduled flight can then revalidate and dispatchconsumein the background even though the caller already received a rejection. Validate the request signal before flight creation and make listener registration rollback-safe. Add a regression proving malformed/listener-throwing signals cause no revalidation, no consume, no terminal fence, and no retained flight. -
Major: a fully cancelled flight can still invoke
revalidate(). The flight starts on the next microtask. If the last waiter cancels afterrecover()returns but before that microtask runs,preDispatchAbortis already aborted andactiveWaiters === 0, butrunFlight()still calls the revalidation adapter before checking those conditions. This violates the pre-dispatch cancellation contract and can start network work or retain an abort-ignoring orphan for a request with no live waiter. Check cancellation/waiter count at the very start ofrunFlight()before invoking any adapter, and add the immediate-post-recover()cancellation regression.
The rest of the coordinator has substantially improved and the previously resolved idempotency, single-flight, re-entrancy, capacity, output-guard, and deadline findings appear addressed. These two issues should block wiring or merge because this PR is specifically the safety foundation for a scarce, irreversible reset-credit consume.
63c9feb to
201e5d3
Compare
|
I rechecked the current head I am not approving this head yet. It is 33 commits behind current The direction remains potentially valuable, but this risk surface needs current-base validation rather than an approval based only on the focused 68-test suite. |
3642536 to
f53ff47
Compare
f53ff47 to
9693832
Compare
Wibias
left a comment
Wibias
left a comment
There was a problem hiding this comment.
Re-review on exact head 9693832602504a5cfc773864cc33fca604e341a9 against current dev@3b8f9487676fe258d76295e49e7db75aca26a4cb.
My two previous safety blockers are fixed on this head:
options.signalis validated before flight creation, and abort-listener registration is rollback-safe: a registration failure releases the pre-dispatch waiter/guard and aborts the flight when it was the last waiter.runFlight()now checkspreDispatchAbortandactiveWaiters === 0before invokingrevalidate(), so an immediately cancelled flight does not start adapter work.
I re-read the surrounding irreversible-operation invariants as well: frozen dependency contracts, generation/terminal fencing, output-guard re-entrancy, absolute deadline checks, stable operation IDs across bounded consume retries, waiter caps, and orphan/capacity accounting. I do not see a remaining code-level blocker in the coordinator on this head.
I am not approving yet because the exact-head Cross-platform CI run 31498469913 is red: the macOS Test job failed. React Doctor and the gates job are green. The branch is also 5 commits behind current dev; those commits do not touch this PR's two files, so I do not see a new overlap concern, but required exact-head CI still needs to be green and the PR is still draft.
Once the required matrix is green and the PR is marked ready, this looks like a strong merge candidate.
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
Tip For best results, initiate chat on the files or code changes.
You are interacting with an AI system. |
|
|
|
Tip For best results, initiate chat on the files or code changes.
You are interacting with an AI system. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/codex-reset-credit-recovery.test.ts`:
- Around line 1227-1232: Update the rejection assertion around recover to assert
that it throws a TypeError rather than matching the Bun-specific
“EventTarget.addEventListener” message. Preserve the existing rollback state
assertions following the recover call.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 00ac16bc-964e-4549-8e56-e65a78af1d82
📒 Files selected for processing (2)
src/codex/reset-credit-recovery.tstests/codex-reset-credit-recovery.test.ts
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '=== Exact HEAD and merge base ==='
git rev-parse HEAD
base="$(git merge-base HEAD origin/dev 2>/dev/null || git merge-base HEAD HEAD^)"
printf 'merge-base: %s\n' "$base"
git status --short
printf '%s\n' '=== Diff summary ==='
git diff --stat "$base" HEAD
git diff --name-status "$base" HEAD
printf '%s\n' '=== Source structure ==='
ast-grep outline src/codex/reset-credit-recovery.ts --items all --view expanded
printf '%s\n' '=== Coordinator source ==='
cat -n src/codex/reset-credit-recovery.ts
printf '%s\n' '=== Focused test names and relevant excerpts ==='
rg -n '^\s*(test|it)\s*\(' tests/codex-reset-credit-recovery.test.ts || true
rg -n -i -C 10 '(terminal|contract|scope|timeout|deadline|cancel|abort|waiter|capacity|reentrant|malformed|generation)' \
tests/codex-reset-credit-recovery.test.ts || true
printf '%s\n' '=== Relevant contracts ==='
fd -t f '^(account-id|quota-rejection)\.ts$' src/codex -x sh -c 'echo "=== $1 ==="; cat -n "$1" | sed -n "1,300p"' sh {}
printf '%s\n' '=== PR metadata and checks ==='
gh pr view 1410 --repo lidge-jun/opencodex \
--json headRefOid,baseRefOid,isDraft,mergeStateStatus,reviewDecision,statusCheckRollup,labels \
2>&1 || trueLength of output: 154083 🏁 Script executed: #!/bin/bash
set -euo pipefail
previous_head="9693832602504a5cfc773864cc33fca604e341a9"
printf '%s\n' '=== Exact follow-up diff ==='
git diff --no-ext-diff -U20 "$previous_head" HEAD -- tests/codex-reset-credit-recovery.test.ts
printf '%s\n' '=== Surrounding test block ==='
changed_line="$(git diff --unified=0 "$previous_head" HEAD -- tests/codex-reset-credit-recovery.test.ts \
| awk -F'[,+ ]' '/^\+\+\+|^---/{next} /^@@/{print $3; exit}' | cut -d, -f1)"
if [ -n "${changed_line:-}" ]; then
start=$(( changed_line > 35 ? changed_line - 35 : 1 ))
end=$(( changed_line + 35 ))
cat -n tests/codex-reset-credit-recovery.test.ts | sed -n "${start},${end}p"
fi
printf '%s\n' '=== Current PR state ==='
gh pr view 1410 --repo lidge-jun/opencodex \
--json headRefOid,isDraft,mergeStateStatus,reviewDecision,labels,statusCheckRollup \
2>&1 || trueLength of output: 9579
The change in The readiness status remains invalid for the current head:
Do not mark this PR ready until exact-head CI completes successfully and the PR state is updated. You are interacting with an AI system. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="lidge-jun/opencodex"
pr="1410"
printf '%s\n' '=== Pull request metadata ==='
gh pr view "$pr" --repo "$repo" \
--json number,headRefOid,baseRefOid,isDraft,mergeStateStatus,reviewDecision,labels,statusCheckRollup
printf '%s\n' '=== Check runs for current PR head ==='
head_sha="$(gh pr view "$pr" --repo "$repo" --json headRefOid --jq '.headRefOid')"
gh api "repos/${repo}/commits/${head_sha}/check-runs?per_page=100" \
--jq '.check_runs[] | [.name, .status, (.conclusion // "pending"), .details_url] | `@tsv`' || true
printf '%s\n' '=== Latest workflow runs for current PR head ==='
gh run list --repo "$repo" --commit "$head_sha" --limit 30 \
--json databaseId,name,status,conclusion,event,url \
--jq '.[] | [.databaseId, .name, .status, (.conclusion // "pending"), .event, .url] | `@tsv`' || trueLength of output: 17145
The gate applies to the current head, The following checks remain in progress:
GitHub also reports Do not report all quality gates as passed until the exact-head Cross-platform CI workflow completes successfully and the remaining requested changes are resolved. You are interacting with an AI system. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="lidge-jun/opencodex"
pr="1410"
printf '%s\n' '=== PR metadata ==='
gh pr view "$pr" --repo "$repo" \
--json number,headRefOid,baseRefOid,isDraft,mergeStateStatus,reviewDecision,labels,statusCheckRollup
printf '%s\n' '=== Checks for the reported head ==='
gh pr checks "$pr" --repo "$repo" --required 2>&1 || true
printf '%s\n' '=== Recent workflow runs for the reported head ==='
gh run list --repo "$repo" \
--commit "8b0e4ecafbff53116c5a8d4118add33c8e514662" \
--limit 30 \
--json databaseId,name,status,conclusion,headSha,event,workflowName,urlLength of output: 15209
PR
Do not report “all PR quality gates passed” or final review readiness until the exact-head You are interacting with an AI system. |
|
Thanks @luvs01 for pushing this through the review cycle and hardening the edge cases. This is a useful foundation because reset-credit recovery is a scarce, irreversible operation: the coordinator gives us one stable operation identity per logical turn, process-shared single-flight behavior, exact generation fencing, cancellation/output-exposure guards, bounded retries, and fail-closed handling for stale or ambiguous states. That substantially reduces the risk of duplicate consumes, stale-generation races, or recovery continuing after output has already been committed, while keeping the future runtime wiring behind an explicit safety boundary. Great work. |
3 participants
Summary
This PR intentionally does not wire automatic redemption or request replay into Responses, change the manual consume route, add account selection, or expose configuration/UI. It is a review-ready foundation for independently proving the irreversible-operation invariants requested in #657.
The coordinator is process-local. A future runtime adapter must persist and reuse the same operation identity across uncertain transport outcomes, echo that identity from consume results, bind it to the account generation, advance the main-account identity epoch, and keep the output-exposure guard monotonic.
Refs #657.
Verification
bun test --isolate tests/codex-reset-credit-recovery.test.ts— 68 passed, 0 failed, 351 assertionsbun test --isolate tests/codex-reset-credit-recovery.test.ts— 68 passed, 0 failed, 351 assertionsbun x --package typescript@7.0.2 tsc --noEmit— passedbun x --package typescript@7.0.2 tsc --noEmit— passedbun run privacy:scan— passedgit diff HEAD^ --check— passedChecklist
Review readiness checklist
This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:
All CI tests are green on my local testing.
I pushed my PR to the latest dev commit.
I resolved all correct Codex and CodeRabbit findings.
My PR is ready for review.
Summary by CodeRabbit
New Features
Bug Fixes