Harden CodexBar cold-start menu readiness#1150
Conversation
|
@steipete I could not request you through GitHub's reviewer API from this account, so tagging you here for review. |
|
Codex review: needs maintainer review before merge. Reviewed May 25, 2026, 8:48 PM ET / 00:48 UTC. Summary Reproducibility: yes. The PR provides a concrete reboot-based cold-start harness and copied artifact evidence for first-open menu readiness, though I did not rerun it in this read-only Linux review environment. Review metrics: 3 noteworthy metrics.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge Security Review detailsBest possible solution: Land after maintainer acceptance of the single-instance semantics and CI/checks stay green; otherwise narrow duplicate handling to same-bundle/path or split that policy into a follow-up. Do we have a high-confidence way to reproduce the issue? Yes. The PR provides a concrete reboot-based cold-start harness and copied artifact evidence for first-open menu readiness, though I did not rerun it in this read-only Linux review environment. Is this the best way to solve the issue? Unclear. The menu readiness and provider-identity repairs look like the narrow maintainable direction, but the oldest-PID duplicate-launch behavior remains a maintainer compatibility decision. AGENTS.md: found and applied where relevant. Codex review notes: model gpt-5.5, reasoning high; reviewed against c644f6874e9a. Label changesLabel changes:
Label justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
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. How this review workflow works
|
![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: 5ee755640c
ℹ️ 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".
| try register() | ||
| } else { | ||
| try service.unregister() | ||
| guard status() == .enabled else { return } |
There was a problem hiding this comment.
Treating setEnabled(false) as a no-op unless status() == .enabled leaves the login item registered when Service Management reports .requiresApproval. In that state the service is already registered but awaiting user approval, so users who toggle Launch at Login off will not actually remove it and can still see the pending/background item in System Settings on next login. This regression was introduced by the new status guard and should also call unregister() for .requiresApproval (or for any state except .notRegistered).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 31a37207: setEnabled(false) now calls unregister() for any ServiceManagement state except .notRegistered, so pending .requiresApproval login items are removed instead of left registered. Added a focused regression test for the .requiresApproval disable path and verified with swift test --filter LaunchAtLoginManagerTests plus git diff --check.
clawsweeper
Bot
added
proof: sufficient
|
Fixed the hosted submenu provider-identity regression in What changed:
Verification run:
Duplicate-launch note: I left @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
AmrMohamad
force-pushed
the
enhance/UI-performance-issues
branch
from
63f3689 to
351dd19
Compare
clawsweeper
Bot
added
rating: 🐚 platinum hermit
1 participant
Summary
This PR hardens CodexBar’s first-launch menu readiness path after macOS boot. The issue we investigated was an unstable first-open Codex tab where expensive adjunct UI, especially charts and hosted submenu content, could remain absent until the user manually refreshed or switched tabs.
The core fix is not a visual redesign. It keeps the UI shape intact and improves the lifecycle behind it: menu readiness invalidation, hosted submenu hydration, cached token data availability, duplicate-launch protection, and cold-start validation.
Why this changed
CodexBar’s menu is an AppKit
NSStatusItem/NSMenusurface with SwiftUI-hosted rows and lazily hydrated submenus. On first launch after boot, several data sources can become ready after the menu has already been opened:Before this change, the menu invalidation path was too coarse for those adjunct readiness transitions. A parent menu could remain open with stale structure, and a hosted submenu could stay stuck on its placeholder even after the backing data became available.
What changed
Validation
Source and focused behavior checks:
swift test --filter StatusMenuHostedSubmenuRefreshTestsswift test --filter StatusMenuOpenRefreshTestsswift test --filter LaunchAtLoginManagerTestsmake checkgit diff --checkRuntime proof:
.build/package/CodexBar.appcodesign --verify --deep --strict --verbose=2 .build/package/CodexBar.appCold-start artifact:
/Users/amrmohamad/.codex/artifacts/CodexBar/cold-start-20260526-025603Key artifact evidence:
post_boot_first_launch_candidate=trueexisting_codexbar_process_count=0codexbar_login_item_present=falseat runner startuptime_seconds=319completeBuy Credits...,Cost | submenu=true,Usage Dashboard,Status Page, andRefreshCost | submenu=trueandCost submenu item count=1Scripts/prepare_next_boot_cold_start_validation.sh check-resultpassed against the captured artifactNotes
The LaunchAgent’s original summary recorded an older checker failure because the validator used the stdlib PNG decoder fallback instead of Pillow metadata. The artifact itself is valid and immutable; after correcting the checker to accept
image_decoder=stdlib_png, the same first-boot artifact validates successfully.This PR intentionally does not change the visible UI design. The changes are lifecycle, data-readiness, validation, and performance hardening around the existing menu hierarchy.