Add selectable menu bar reset windows - #3481
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
clawsweeper
Bot
added
P2
|
Codex review: needs real behavior proof before merge. Reviewed September 8, 2026, 8:38 PM ET / September 9, 2026, 00:38 UTC (Revision 4). ClawSweeper reviewWhat this changesAdds selectable session and weekly reset countdowns and clocks to menu-bar layouts and conditional branches, with refresh scheduling, versioned settings storage, documentation, and tests. Merge readiness⛔ Blocked before merge - 6 items remain The feature remains absent from main and v0.57.0. The earlier V2 persistence defect is addressed, but two actionable defects remain, alongside outstanding feature/storage approval and normal-app proof. Priority: P2 Review scores
Verification
How this fits togetherCodexBar combines provider quota snapshots with saved layout choices to render its macOS menu-bar display. Reset tokens select a quota window, format its reset time, and schedule display updates. flowchart LR
A[Provider quota snapshots] --> C[Select quota window]
B[Saved layout and conditional rules] --> C
B --> F[Versioned settings storage]
C --> D[Format reset time]
C --> E[Schedule next display update]
D --> G[Menu-bar text and accessibility]
E --> G
Decision needed
Why: VISION.md requires approval for new features and storage changes, and the discussion explicitly leaves that approval outstanding. Before merge
Findings
Agent review detailsSecurityNone. Review metrics
Root-cause clusterRelationship: Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: With feature/storage sign-off, retain additive window choices and legacy automatic semantics, preserve saved layouts across upgrades, and refresh countdowns only when their displayed text changes. Do we have a high-confidence way to reproduce the issue? Yes for the patch defects by source inspection: a selected multi-day countdown reaches the minute scheduler, and its accessibility prefix already contains 'in'. No runtime reproduction was executed; the underlying feature request itself is not a bug reproduction. Is this the best way to solve the issue? Unclear pending sign-off: reusing existing quota-window resolution and preserving automatic token spelling is appropriate, but countdown scheduling and accessibility composition need repair. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against 928166f89947. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (3 earlier review cycles)
|
|
Addressed the P1 persistence finding in 79c3756. The original released-V2 startup failure is reproduced; full selections now live in V3, with V2/V1 projections that released readers can decode. Added downgrade/re-upgrade tests, per-provider preservation across older edits/deletions, and conditional-library preservation with explicit clear-all/ID-replacement behavior. Validation: make check is clean; all 87 UTC test groups passed without retries or timeouts. Added native editor captures and scoped runtime receipts for fresh/V2-upgrade settings. These are isolated test-host checks, not ordinary app restart or live timer-delivery proof; the PR body states that limitation. Normal-app proof and maintainer feature sign-off remain outstanding, so this stays a draft. The CI aggregate is incomplete because required macOS tests are deferred for drafts. |
clawsweeper
Bot
added
proof: 📸 screenshot
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. |
![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: 79c375612b
ℹ️ 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".
| if tokens.contains(.resetAbsolute) { | ||
| absoluteResetDates.append(contentsOf: resetDates) | ||
| } | ||
| countdownResetDates += self.menuBarLayoutResetDates(for: provider, now: now, absolute: false) |
There was a problem hiding this comment.
When a user selects “Weekly: Resets in” and the reset is more than a day away, this feeds the weekly date into menuBarCountdownRefreshDelay, which always wakes at the next minute boundary. However, UsageFormatter.resetCountdownDescription omits minutes while days are present, so the displayed text normally changes only once per hour; each wake calls updateIcons() and schedules another minute wake for potentially most of a week. Calculate the next boundary from the formatter's current granularity so long weekly countdowns do not continuously perform invisible menu-bar work.
Useful? React with 👍 / 👎.
| return self.optionalTextToken( | ||
| item.resetIsAbsolute ? text.absolute : text.countdown, | ||
| unavailableLabel: L("%@ unavailable", label), | ||
| accessibilityPrefix: label, |
There was a problem hiding this comment.
For a selected countdown token, label is already something like “Weekly: Resets in”, while the rendered countdown value is “in 2d”; passing the full label as accessibilityPrefix therefore makes VoiceOver announce “Weekly: Resets in in 2d”. Use a prefix that does not contain the countdown preposition, or provide a separately composed accessibility string.
Useful? React with 👍 / 👎.
1 participant
Adds explicit Session and Weekly variants of Resets in and Reset at to the menu-bar layout palette and conditional branches. Existing unqualified tokens retain automatic-window behavior. Missing selected windows display unavailable rather than another quota's reset.
Draft for feature sign-off under VISION.md; implements the proposed capability in #3356. Provider fetching and authentication are unchanged.
Persistence review addressed
The V2 compatibility finding was valid. New enum cases made released v0.56.8 reject a complete layout or override dictionary and overwrite V2 with the oldest-format fallback. A standalone reproduction of that startup contract failed both unrelated-layout preservation assertions.
Full selections now use V3 keys. Every save also writes a released-V2-readable projection preserving supported conditional placements, direct lanes, and provider overrides, plus the oldest-format projection. Re-upgrading restores V3 when the older projections agree. Older edits/deletions are authoritative for each affected provider; untouched providers retain their new reset selections. Nonempty older library edits preserve invisible V3-only rules, older same-ID replacements win, and an older clear-all remains authoritative.
Regression tests exercise fresh defaults, V2 upgrade before editor use, save → released startup → re-upgrade, multiple providers, older edits/deletions, library edits/ID collisions/clearing, and existing predicate directions/metrics. The released decoder fixture rejects the new reset discriminators, rather than relying only on today's decoder.
Validation
912eac223.make check: passed; zero format/lint violations.TZ=UTC CODEXBAR_ALLOW_TEST_KEYCHAIN_ACCESS=0 make test: all 87 groups passed on the first attempt; zero failures, retries, or timeouts.The earlier default Australia/Sydney full run stopped on two untouched OpenCode Go tests whose UTC fixture timestamps cross local midnight. That suite passed all 15 tests in UTC; full-suite runs use UTC and leave those unrelated files unchanged.
The CI aggregate reports incomplete while the PR remains a draft: the repository workflow intentionally defers required macOS tests. This is separate from the local full-suite result and is not claimed green.
Native editor evidence and remaining proof
The screenshots below capture the actual production
MenuBarLayoutEditorin an isolated native XCTest window with synthetic quota data. The test invokes the editor's production activation/persistence entry point, reconstructsSettingsStorefrom the same isolated defaults, and verifies the selected layout survives. Both fresh and V2-upgrade scenarios pass. A real 2.05-second countdown boundary changes the formatted value fromin 7mtoin 6musing the production scheduling calculation.This is supplemental native test-host evidence. It does not demonstrate ordinary application startup, pointer-driven selection, an app restart, or automatic timer delivery in a running menu-bar app. The requested normal-app/live behavior proof and maintainer feature approval remain outstanding. No live account, browser-cookie, or Keychain probes were performed.
Run the proof explicitly:
CODEXBAR_RESET_NATIVE_PROOF_DIR=/tmp/codexbar-reset-native-proof \ CODEXBAR_ALLOW_TEST_KEYCHAIN_ACCESS=0 CODEXBAR_SUPPRESS_TEST_KEYCHAIN_ACCESS=1 \ CODEXBAR_TEST_CODEX_FILE_ISOLATION=1 CODEXBAR_TEST_SESSION_FILE_ISOLATION=1 \ swift test --filter MenuBarResetWindowNativeProofTestsIsolated native editor captures and receipt
V2 upgrade before selecting new tokens:
V2 upgrade after selection and SettingsStore reconstruction:
Fresh settings after selection and SettingsStore reconstruction:
Scoped runtime receipt