Prevent premature Codex weekly reset confetti

[gmkbenjamin]

Summary

• Require the previous Codex weekly reset boundary to be due before two matching low samples can replace the public snapshot.
• Apply the same due-boundary guard to reset celebration detection while preserving its prior boundary and baseline.
• Add confirmation, publication, and confetti regressions using the timestamps observed in Codex weekly reset confetti can fire before the prior reset boundary #2481, and make genuine-reset fixtures occur after their prior boundary.

Fixes #2481

Testing

• make check ✅
• swift test --filter CodexWeeklyResetConfirmationTests ✅ (14 tests)
• swift test --filter CodexWeeklyResetPublicationTests ✅ (12 tests)
• swift test --filter UsageStorePlanUtilizationCelebrationTests ✅ (30 tests)
• make test ⚠️ the changed and first 11 shards pass, but the existing Claude credential-storage shard fails with eight .notFound/ownership fixture issues plus one isolated MCP-only expectation; the exact failing shard reproduces independently and none of its files are touched here.

Integration runtime proof

• Packaged and launched the combined integration bundle containing this exact PR head (025dd6d1); the app stayed running.
• With Disable Keychain access enabled, the packaged live provider matrix completed successfully for every enabled provider.
• A fresh app run emitted no Weekly limit reset confetti event, and unified logs contained no matches for either reported provider error signature during the verification window.
• The live account was not forced through an artificial rollover. The premature-low-sample and genuine-post-boundary paths remain covered by the focused deterministic tests above.
• GitHub Actions is green on this exact head.

[clawsweeper]

Codex review: needs real behavior proof before merge. Reviewed July 28, 2026, 11:18 AM ET / 15:18 UTC.

ClawSweeper review

What this changes

This PR prevents premature low Codex weekly-usage samples from replacing the public snapshot or triggering reset confetti until the previously advertised weekly reset boundary is due.

Merge readiness

⛔ Blocked until stronger real behavior proof is added - 3 items remain

Keep this PR open for normal review: its narrow guard and focused regressions directly address the reported premature Codex weekly reset path, but the added packaged-app evidence observes an ordinary no-confetti run rather than exercising the reported pre-boundary rollover. A redacted end-to-end refresh trace is still needed before merge.

Priority: P2
Reviewed head: 025dd6d129f50c20dc308b41081d5e720f85f211

Review scores

Measure	Result	What it means
Overall readiness	🦪 silver shellfish (2/6)	The implementation and focused regressions look well-scoped, but the submitted runtime proof does not directly demonstrate the changed behavior.
Proof confidence	🦪 silver shellfish (2/6)	Needs stronger real behavior proof before merge: The contributor reports an exact-head packaged run and unified logs, but the observed fresh run does not exercise the premature low-sample rollover path that this patch changes. Please add redacted after-fix runtime evidence for that decision path; terminal or unified-log output is suitable, and updating the PR body should trigger a fresh review. 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 stronger real behavior proof before merge: The contributor reports an exact-head packaged run and unified logs, but the observed fresh run does not exercise the premature low-sample rollover path that this patch changes. Please add redacted after-fix runtime evidence for that decision path; terminal or unified-log output is suitable, and updating the PR body should trigger a fresh review. 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	Confirmation guard: The PR requires the confirmation sample to occur no earlier than two minutes before the prior weekly boundary, preventing two low samples with a rolling future boundary from publishing prematurely. Celebration guard: The PR also requires the previous Codex reset boundary to be due before an advanced boundary can permit a reset celebration, while retaining the previous boundary on guarded samples. Focused regression coverage: The branch adds a confetti-boundary regression and extends confirmation, publication, and celebration tests with the timestamps from the linked report, including genuine post-boundary cases.
Findings	None	None.
Security	None	None.

How this fits together

CodexBar polls provider usage and keeps per-account snapshots for its menu, widgets, history, and celebration behavior. Codex weekly samples pass through confirmation and publication checks before visible usage state changes or a reset confetti event is emitted.

flowchart LR
A[Codex usage samples] --> B[Weekly reset confirmation]
B --> C[Public usage snapshot]
C --> D[Menu widgets and history]
B --> E[Reset celebration guard]
E --> F[Confetti event]

Loading

Before merge

• Add real behavior proof - Needs stronger real behavior proof before merge: The contributor reports an exact-head packaged run and unified logs, but the observed fresh run does not exercise the premature low-sample rollover path that this patch changes. Please add redacted after-fix runtime evidence for that decision path; terminal or unified-log output is suitable, and updating the PR body should trigger a fresh review. 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 reported packaged run confirms that an ordinary app launch did not emit confetti, but it does not show the changed pre-boundary low-sample path or prove that a genuine reset remains visible through the real refresh pipeline.
• Complete next step (P2) - The remaining merge gate is contributor-provided real behavior proof rather than a concrete code repair that an automated fix lane can perform.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Patch surface	6 files affected; 198 additions, 17 deletions	The implementation is concentrated in the Codex weekly confirmation and celebration guards, with focused regression coverage.
Focused test coverage	3 named focused suites; 56 reported tests	The PR reports coverage for confirmation, publication, and celebration behavior around the boundary.

Merge-risk options

Maintainer options:

1. Decide the mitigation before merge
   Retain the prior-boundary guard, then add redacted end-to-end evidence from a refresh that exposes the previous boundary, the incoming low sample, the decision to preserve the displayed snapshot, and the absence of a reset event; keep the existing post-boundary regression coverage.
2. Pause or close
   Do not merge this PR until maintainers decide whether the risk is worth taking.

Technical review

Best possible solution:

Retain the prior-boundary guard, then add redacted end-to-end evidence from a refresh that exposes the previous boundary, the incoming low sample, the decision to preserve the displayed snapshot, and the absence of a reset event; keep the existing post-boundary regression coverage.

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

Yes, from the supplied source diff and linked report: a prior 100% weekly sample followed by matching 0% samples with a future rolling boundary reaches the confirmation and celebration decisions before the prior boundary is due. Full current-main execution could not be independently verified because local read-only commands failed before execution.

Is this the best way to solve the issue?

Yes, provisionally: requiring the prior reset boundary to be due in both publication and celebration decisions is the narrowest shared fix, and the branch updates genuine post-boundary fixtures to preserve the intended reset path.

AGENTS.md: found and applied where relevant.

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

Labels

Label justifications:

• P2: This fixes an incorrect Codex weekly usage publication and celebration path with limited blast radius.
• 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 stronger real behavior proof before merge: The contributor reports an exact-head packaged run and unified logs, but the observed fresh run does not exercise the premature low-sample rollover path that this patch changes. Please add redacted after-fix runtime evidence for that decision path; terminal or unified-log output is suitable, and updating the PR body should trigger a fresh review. 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:

• Confirmation guard: The PR requires the confirmation sample to occur no earlier than two minutes before the prior weekly boundary, preventing two low samples with a rolling future boundary from publishing prematurely. (Sources/CodexBar/Providers/Codex/CodexWeeklyResetConfirmation.swift:147, 025dd6d129f5)
• Celebration guard: The PR also requires the previous Codex reset boundary to be due before an advanced boundary can permit a reset celebration, while retaining the previous boundary on guarded samples. (Sources/CodexBar/UsageStore+LimitResetCelebration.swift:125, 025dd6d129f5)
• Focused regression coverage: The branch adds a confetti-boundary regression and extends confirmation, publication, and celebration tests with the timestamps from the linked report, including genuine post-boundary cases. (Tests/CodexBarTests/CodexWeeklyResetConfettiBoundaryTests.swift:1, 025dd6d129f5)
• Linked canonical report: The PR explicitly targets the open report describing a 100%-to-0% Codex weekly reading whose prior August 2 reset boundary had not yet elapsed. (025dd6d129f5)
• Inspection limitation: Read-only local inspection commands could not execute because the sandbox failed during loopback-network initialization, so git-history ownership and full-tree source verification could not be completed.

Likely related people:

• steipete: The repository belongs to this account; local git history could not be inspected after the read-only shell failed before command execution. (role: repository owner fallback; confidence: low; files: Sources/CodexBar/Providers/Codex/CodexWeeklyResetConfirmation.swift, Sources/CodexBar/UsageStore+LimitResetCelebration.swift)

Rank-up moves

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

• Add redacted end-to-end refresh logs or a recording showing a pre-boundary low sample remains unpublished and does not emit confetti.
• Include the corresponding genuine post-boundary refresh outcome if the same diagnostic surface can show it.

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-07-28T11:16:07.574Z sha 025dd6d :: needs real behavior proof before merge. :: none

[gmkbenjamin]

@clawsweeper re-review — added packaged integration-runtime evidence while keeping the temporal rollover claim bounded to deterministic regression tests.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Command router queued. I will update this comment with the next step.