Skip to content

fix: confirm Codex weekly reset with zero available credits - #2897

Merged
steipete merged 1 commit into
steipete:mainfrom
luantu:fix/codex-weekly-reset-zero-credit-confirmation
Aug 13, 2026
Merged

fix: confirm Codex weekly reset with zero available credits#2897
steipete merged 1 commit into
steipete:mainfrom
luantu:fix/codex-weekly-reset-zero-credit-confirmation

Conversation

@luantu

@luantu luantu commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Summary

CodexBar can get stuck showing a stale weekly usage percentage forever when a Codex weekly quota reset is observed before the previous reset boundary and the account has zero available reset credits (availableCount == 0).

CodexWeeklyResetConfirmation.confirmationDecision requires proof that a manual reset credit was consumed (confirmsManualResetCreditConsumption). That proof can never hold when the previous snapshot explicitly reported an empty reset-credit inventory, so every refresh re-evaluates to .preservePrevious and the fresh value is never published.

This changes confirmsManualResetCreditConsumption so an explicitly observed zero-credit inventory (previousCredits.availableCount == 0) trusts the two consistent observations (initial + confirmation) that are the only signal a server-side early reset has. A nil/unknown previous inventory stays conservative and still demands consumption proof, so the existing false-positive protection (transient low readings, missing/expired credits) is unchanged.

Changes

  • Sources/CodexBar/Providers/Codex/CodexWeeklyResetConfirmation.swift: when the previous snapshot has an explicitly known-zero credit inventory, return true from confirmsManualResetCreditConsumption instead of deadlocking.
  • Tests/CodexBarTests/CodexWeeklyResetConfirmationTests.swift: new test zero available reset credits confirm a server-side early weekly reset by observation reproducing the live failure (98% → 1%, boundary 8/18 → 8/20, zero credits).

Repro

  • Live account at 98% used / reset 2026-08-18, no reset credits available (codexResetCredits.availableCount == 0, credits == []).
  • Weekly limit reset server-side to 1% used / reset 2026-08-20 (observed well before the old boundary).
  • CodexBar refreshed every minute, fetched the new value, but confirmationDecision returned .preservePrevious indefinitely → status-bar icon stuck at the old value.

Verification

  • CodexWeeklyResetConfirmationTests: 20/20 pass (including the new case; existing conservative cases like "one missing reset credit inventory does not confirm" and "expired omitted reset credit does not confirm" still pass).
  • swift build --target CodexBarCore passes.
  • make check (swiftformat + swiftlint --strict): 0 violations.

Related

@clawsweeper

clawsweeper Bot commented Aug 13, 2026

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

@clawsweeper clawsweeper Bot added 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. labels Aug 13, 2026
@clawsweeper

clawsweeper Bot commented Aug 13, 2026

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed August 13, 2026, 2:01 AM ET / 06:01 UTC.

ClawSweeper review

What this changes

The PR permits two matching early Codex weekly-reset observations when the prior reset-credit inventory explicitly contained zero available credits, with a focused regression test.

Regression provenance

Possible regression — probable (reproduction; reviewed change). No predecessor PR is attributed.

Merge readiness

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

Keep open: the reported zero-credit path is a concrete current-main defect, but the proposed bypass also accepts later inventories with available credits and needs a narrow safeguard plus after-fix real behavior proof.

Priority: P2
Reviewed head: dd6eefd964db3479d58bc1072f74631daaea4677

Review scores

Measure Result What it means
Overall readiness 🦪 silver shellfish (2/6) The reported defect is credible, but a P1 guard regression and mock-only validation prevent merge readiness.
Proof confidence 🦪 silver shellfish (2/6) Needs real behavior proof before merge: The PR reports focused tests, build, and lint, but does not include redacted after-fix live output showing the early weekly value is published. 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 🦪 silver shellfish (2/6) 1 actionable review finding remain.

Verification

Check Result Evidence
Real behavior Needs proof Needs real behavior proof before merge: The PR reports focused tests, build, and lint, but does not include redacted after-fix live output showing the early weekly value is published. 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 5 items Current guard: Current main rejects an early reset when the previous inventory has no available credit to prove consumption.
Branch bypass: The proposed branch returns true from the consumption check based only on the previous inventory's aggregate zero count.
Existing safeguard contract: The shipped omitted-credit fix documents that both follow-up samples must corroborate consumption, which the new branch can bypass when only the prior inventory is empty.
Findings 1 actionable finding [P1] Require later inventories to remain empty before bypassing proof
Security None None.

How this fits together

CodexBar fetches Codex quota snapshots and confirms suspicious early weekly resets before updating the menu-bar usage. The confirmation guard compares prior, initial, and follow-up quota and reset-credit observations before publishing or retaining the current display.

flowchart LR
A[Prior Codex snapshot] --> D[Reset confirmation guard]
B[Initial quota refresh] --> D
C[Follow-up quota refresh] --> D
D --> E{Reset evidence accepted?}
E -->|yes| F[Publish fresh menu-bar usage]
E -->|no| G[Keep prior usage]
Loading

Before merge

  • Add real behavior proof - Needs real behavior proof before merge: The PR reports focused tests, build, and lint, but does not include redacted after-fix live output showing the early weekly value is published. 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.
  • Require later inventories to remain empty before bypassing proof (P1) - previousCredits.availableCount == 0 ignores initialCredits and confirmationCredits. If either later fetch reports an available credit, this publishes the early low reading without a consumed-credit transition, weakening the shipped two-sample safeguard. Restrict the exception to the all-zero case and add that counterexample.
  • Resolve merge risk (P1) - The new bypass publishes an early low-usage observation when only the prior inventory is empty, even if either later inventory exposes an available reset credit and provides no consumption evidence.
  • Resolve merge risk (P1) - The PR has focused test/build/lint claims but no redacted after-fix provider trace showing the affected state is published in a real account.
  • Complete next step (P2) - A mechanical guard correction and regression test can address the branch defect; contributor-provided real behavior proof remains required before merge.
  • Improve patch quality - Constrain the exception to explicitly unavailable inventories across the full confirmation sequence and add the preserving counterexample.
  • Improve patch quality - Attach a redacted terminal trace or live output from the changed path; redact account details, tokens, endpoints, and other private data.

Findings

  • [P1] Require later inventories to remain empty before bypassing proof — Sources/CodexBar/Providers/Codex/CodexWeeklyResetConfirmation.swift:185-186
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Change footprint production +6, tests +34 across 2 files The narrow patch is easy to validate, but its single production condition controls the published quota state.

Root-cause cluster

Relationship: canonical
Canonical: #2897
Summary: This PR is the current canonical follow-up for zero-credit early server resets; the prior merged reset-credit fixes overlap in the guard but address distinct provider states.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge-risk options

Maintainer options:

  1. Preserve three-snapshot evidence (recommended)
    Require the initial and confirmation inventories to remain explicitly unavailable before bypassing consumption proof, and add a preserving regression case for a later available credit.

Technical review

Best possible solution:

Accept the server-side zero-credit case only when every confirmation inventory explicitly remains unavailable, cover a later-available-credit counterexample, and provide a redacted after-fix refresh trace.

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

Yes—current source deterministically preserves the supplied all-zero sequence because an empty prior available inventory returns false; the submitted focused test encodes the same path, though this read-only review did not execute it.

Is this the best way to solve the issue?

No—the intended narrow exception is reasonable, but it must also constrain the initial and confirmation inventories so an available later credit cannot bypass the existing consumption safeguard.

Full review comments:

  • [P1] Require later inventories to remain empty before bypassing proof — Sources/CodexBar/Providers/Codex/CodexWeeklyResetConfirmation.swift:185-186
    previousCredits.availableCount == 0 ignores initialCredits and confirmationCredits. If either later fetch reports an available credit, this publishes the early low reading without a consumed-credit transition, weakening the shipped two-sample safeguard. Restrict the exception to the all-zero case and add that counterexample.
    Confidence: 0.96

Overall correctness: patch is incorrect
Overall confidence: 0.94

AGENTS.md: found and applied where relevant.

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

Labels

Label changes:

  • add P2: This is a bounded Codex quota-display defect affecting accounts that receive an early weekly reset with no available reset credit.
  • add merge-risk: 🚨 compatibility: The changed confirmation default can replace existing users’ retained quota with an unverified early low-usage reading.
  • add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦪 silver shellfish.
  • add 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 focused tests, build, and lint, but does not include redacted after-fix live output showing the early weekly value is published. 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.

Label justifications:

  • P2: This is a bounded Codex quota-display defect affecting accounts that receive an early weekly reset with no available reset credit.
  • merge-risk: 🚨 compatibility: The changed confirmation default can replace existing users’ retained quota with an unverified early low-usage reading.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦪 silver shellfish.
  • 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 focused tests, build, and lint, but does not include redacted after-fix live output showing the early weekly value is published. 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

Acceptance criteria:

  • [P1] swift test --filter CodexWeeklyResetConfirmationTests.
  • [P1] make check.
  • [P1] make test.

What I checked:

Likely related people:

  • endless7: The merged reset-refresh history and release notes attribute the manual-reset and omitted-credit confirmation paths to this contributor. (role: introduced manual-reset confirmation behavior; confidence: high; commits: 5014285fa781, 6d19a437b292; files: Sources/CodexBar/Providers/Codex/CodexWeeklyResetConfirmation.swift, Tests/CodexBarTests/CodexWeeklyResetConfirmationTests.swift)
  • Peter Steinberger: History identifies this contributor on the earlier premature-weekly-reset guard and the current main branch around this subsystem. (role: recent area contributor; confidence: medium; commits: 6f84c4e9425b, 208016687098; files: Sources/CodexBar/Providers/Codex/CodexWeeklyResetConfirmation.swift, CHANGELOG.md)

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.

@steipete
steipete merged commit d14c324 into steipete:main Aug 13, 2026
9 checks passed
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