Skip to content

Fix Codex quota refresh after manual reset - #2710

Merged
steipete merged 1 commit into
steipete:mainfrom
endless7:agent/fix-codex-manual-reset-refresh
Aug 6, 2026
Merged

Fix Codex quota refresh after manual reset#2710
steipete merged 1 commit into
steipete:mainfrom
endless7:agent/fix-codex-manual-reset-refresh

Conversation

@endless7

@endless7 endless7 commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Summary

  • accept a confirmed early weekly reset when the same Codex reset credit transitions from available to redeemed
  • preserve the existing premature-reset guard when there is no matching redemption evidence
  • add regression coverage for manual reset credit redemption before the previous natural weekly boundary

Root cause

CodexWeeklyResetConfirmation rejected every reset observed more than two minutes before the previously scheduled weekly boundary. A manual reset credit intentionally moves that boundary early, so the app kept preserving the stale pre-reset snapshot even after two matching low-usage samples.

The merged confirmation path treats a provider-reported redemption of the same stable reset credit ID as evidence that the early boundary change is legitimate. All other boundary and two-sample confirmation checks remain in place.

User impact

For providers that retain the consumed credit as redeemed, Codex quota usage refreshes automatically after a user redeems a manual reset credit, without weakening protection against transient or premature zero-usage responses.

Validation for the merged change

  • DEVELOPER_DIR=/Applications/Xcode.app/Contents/Developer xcrun swift test --filter CodexWeeklyResetConfirmationTests (16 tests passed)
  • DEVELOPER_DIR=/Applications/Xcode.app/Contents/Developer make check
  • DEVELOPER_DIR=/Applications/Xcode.app/Contents/Developer make test (821 selections across 69 groups, 0 failures)

No UI changes; screenshots are not applicable.

Post-merge provider finding

This PR was merged at head 288380d52eb75e49c93fdc7754f877a25b0ac2aa before the following provider trace was recovered. The follow-up described here is not part of this merged PR.

Observed on CodexBar 0.47.0 with a signed-in Codex OAuth account. Account identity, tokens, and reset-credit IDs are omitted.

Captured at (UTC) Weekly used Reset boundary (UTC)
2026-08-06 08:59:18 98% 2026-08-10 01:00:26
2026-08-06 09:28:18 99% 2026-08-10 01:00:26
2026-08-06 09:33:18 0% 2026-08-13 09:33:18

The successful reset-credit inventory persisted with the post-redemption observation was:

{"availableCount":0,"credits":[]}

A later read-only live OAuth fetch returned the same successful empty inventory. In this observed provider shape, the consumed credit is omitted instead of retained as a redeemed row, so the merged same-row transition remains conservative and may still preserve the stale snapshot.

Follow-up commit endless7/CodexBar@78035f4f handles both representations, but only when two independent low-usage inventories corroborate consumption and the available count decreased. It has 18 focused tests, a clean make check, and 821 full-suite selections across 69 groups with no failures or retries. The account has no reset credit remaining, so this validation replays the exact redacted timeline through production decision code; it is not presented as a second live redemption.

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal priority bug or improvement with limited blast radius. labels Aug 6, 2026
@endless7
endless7 marked this pull request as ready for review August 6, 2026 14:23
@clawsweeper clawsweeper Bot added the merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. label Aug 6, 2026
@clawsweeper

clawsweeper Bot commented Aug 6, 2026

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed August 6, 2026, 10:42 AM ET / 14:42 UTC.

ClawSweeper review

What this changes

The PR accepts an early Codex weekly quota reset only when the same reset credit changes from available to redeemed, and adds regression coverage for that transition.

Merge readiness

Blocked until real behavior proof from a real setup is added - 3 items remain

The patch is narrowly scoped and source-consistent; it remains open because the previously requested real provider-behavior proof is still missing.

Priority: P2
Reviewed head: 288380d52eb75e49c93fdc7754f877a25b0ac2aa

Review scores

Measure Result What it means
Overall readiness 🦪 silver shellfish (2/6) The patch is focused and source-consistent, but test-only evidence does not establish the provider-facing behavior before merge.
Proof confidence 🦪 silver shellfish (2/6) Needs real behavior proof before merge: The PR reports automated tests only; it has no redacted after-fix runtime evidence of a reset-credit redemption and refreshed quota. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Needs proof Needs real behavior proof before merge: The PR reports automated tests only; it has no redacted after-fix runtime evidence of a reset-credit redemption and refreshed quota. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed 6 items Patch scope and safeguard: The branch retains valid, closely matching reset-boundary checks and only exempts the early-observation timing guard when an available prior credit with the same stable ID is redeemed.
Provider data contract: Reset-credit identifiers are normalized to stable, domain-separated IDs before being stored in snapshots, supporting the proposed same-credit comparison.
Account scope: The production flow verifies matching normalized Codex account emails before confirmation and attaches fresh reset-credit inventory to each Codex result.
Findings None None.
Security None None.

How this fits together

CodexBar collects Codex quota and reset-credit responses, then confirms low-usage weekly snapshots before publishing them. This safeguard feeds the quota state shown in the menu bar and CLI.

flowchart LR
A[Codex usage response] --> B[Usage snapshot]
A --> C[Reset-credit response]
B --> D[Weekly reset confirmation]
C --> D
D --> E{Confirmed early reset?}
E -->|No| F[Keep prior quota]
E -->|Yes| G[Publish refreshed quota]
G --> H[Menu bar and CLI]
Loading

Before merge

  • Add real behavior proof - Needs real behavior proof before merge: The PR reports automated tests only; it has no redacted after-fix runtime evidence of a reset-credit redemption and refreshed quota. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
  • Resolve merge risk (P1) - The change relaxes an existing early-reset safeguard; a real provider trace is needed to show the same-credit condition occurs in production and refreshes the intended quota state.
  • Complete next step (P2) - The remaining action is contributor-supplied real behavior proof, not an automatable repair.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Changed surface 2 files; production +27/-1, tests +67/-3 The patch is concentrated in one quota-confirmation decision and its focused regression coverage.

Merge-risk options

Maintainer options:

  1. Add real reset-flow proof (recommended)
    Provide a redacted runtime log or terminal transcript showing the matching credit transition from available to redeemed and the subsequently published weekly quota.
  2. Accept test-only evidence
    A maintainer may merge on the focused regression coverage while explicitly accepting the absence of a real provider trace.

Technical review

Best possible solution:

Merge the narrow same-credit redemption exception only after a redacted live trace shows the credit transition and refreshed weekly quota while the no-redemption path remains guarded.

Do we have a high-confidence way to reproduce the issue?

Yes at source level: a high-usage snapshot with an available stable credit, followed by matching low-usage snapshots with that credit redeemed, reaches the new branch. A real provider reproduction has not yet been shown.

Is this the best way to solve the issue?

Yes, pending real behavior proof: the same stable credit’s redemption is a narrow condition that preserves the existing timestamp, boundary, and two-sample checks.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 50c5b00221ff.

Labels

Label justifications:

  • P2: A manual reset can leave displayed Codex quota stale, but the evidence does not indicate service unavailability or data loss.
  • merge-risk: 🚨 compatibility: The PR changes when existing users’ quota snapshots are allowed to replace prior weekly state.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR reports automated tests only; it has no redacted after-fix runtime evidence of a reset-credit redemption and refreshed quota. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence

What I checked:

Likely related people:

  • Peter Steinberger: Available feature history shows this person introduced the weekly-reset confirmation implementation and test suite. (role: introduced the current confirmation behavior; confidence: high; commits: 94efcfdf4a4b; files: Sources/CodexBar/Providers/Codex/CodexWeeklyResetConfirmation.swift, Tests/CodexBarTests/CodexWeeklyResetConfirmationTests.swift)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Add a redacted terminal output or runtime log showing one account’s reset credit changing from available to redeemed and the subsequent refreshed weekly quota.
  • Update the PR body with that evidence; it should trigger a fresh review automatically, or a maintainer can request one with @clawsweeper re-review if needed.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (1 earlier review cycle)
  • reviewed 2026-08-06T14:21:45.798Z sha 288380d :: needs real behavior proof before merge. :: none

@steipete
steipete merged commit 5014285 into steipete:main Aug 6, 2026
1 check passed
@steipete

steipete commented Aug 6, 2026

Copy link
Copy Markdown
Owner

Landed. Verified before merge (independent maintainer-agent review): root cause confirmed in CodexWeeklyResetConfirmation.confirmationDecision (the pre-boundary guard rejects legitimately early boundaries after a manual reset); the exemption is narrow — same normalized credit ID transitioning available→redeemed with non-regressing inventory timestamps — and self-limiting (cannot fire twice off one redemption). All other safeguards intact; missing credit inventory falls back to the conservative preserve path. Checked against the #2600 duration-based lane classification: no stale primary=session assumptions. Local: CodexWeeklyResetConfirmationTests 16/16, gatekeeper 38/38, make check clean. CI green at merge. Thanks @endless7!

@endless7

endless7 commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Aug 7, 2026

Copy link
Copy Markdown

🦞🧹
ClawSweeper could not start a re-review for this item.

Reason: re-review requires an open issue or PR.

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

Labels

merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P2 Normal priority bug or improvement with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants