Skip to content

Fix menu bar item identity#1122

Merged
steipete merged 2 commits into
steipete:mainfrom
lederniermagicien:fix/stable-menu-bar-identity
May 23, 2026
Merged

Fix menu bar item identity#1122
steipete merged 2 commits into
steipete:mainfrom
lederniermagicien:fix/stable-menu-bar-identity

Conversation

@lederniermagicien
Copy link
Copy Markdown
Contributor

Problem

CodexBar can be reclassified by menu bar managers as a new item after the user pins it visible. Thaw issue stonerl/Thaw#605 reports CodexBar repeatedly moving back into the hidden section.

The risky CodexBar behavior was in status item visibility recovery: a live status item whose backing window had moved off the current screen could be treated as failed materialization, causing CodexBar to remove and recreate the NSStatusItem. For managers such as Thaw/Ice, that churn can make the pinned item look like a new menu bar item.

Solution

Give CodexBar status items stable manager-facing identity and split recovery into destructive vs non-destructive paths:

  • Real materialization failures still recreate the status item: missing button, missing window, or zero width.
  • Live-but-displaced status items are no longer treated as broken. When a display removal leaves a visible status item attached to a stale/detached screen, CodexBar toggles visibility on the existing NSStatusItem and re-renders it instead of replacing it.
  • Merged and per-provider status items now publish stable autosaveName, accessibility identifier/title, and tooltip values.

Safety details

  • Keeps destructive recreation for the cases that actually indicate a broken AppKit status item.
  • Avoids replacing NSStatusItem instances when a menu bar manager may have intentionally parked or moved the item.
  • Preserves existing split/merged item behavior; per-provider status items still get removed when switching back to merged mode.
  • Tests lock both replacement recovery and non-destructive refresh behavior so the two recovery paths do not collapse into one another again.

Changes

Status item identity

  • Add stable identity metadata for merged and provider status items.
  • Reapply that identity when recovery creates replacement items.

Visibility recovery

  • Distinguish blocked status items from displaced live status items.
  • Use non-destructive visibility refresh for stale/detached-screen status item windows after display removal.
  • Continue destructive recreation for missing button/window or zero-width items.

Tests

  • Cover detached and stale-screen live status items.
  • Cover missing-window recovery.
  • Cover stable status item identity.
  • Cover non-destructive refresh preserving exact NSStatusItem/button instances.

Verification

  • swift test --filter 'MenuBarVisibilityWatcherTests|StatusItemControllerSplitLifecycleTests'
  • make check
  • ./Scripts/compile_and_run.sh
  • SOAK/AppKit probe: title/image/visibility changes preserved the same status item button/window.
  • AX probe against the running packaged app reported AXMenuBarItem with title CodexBar and identifier CodexBar.StatusItem.codex.

Full swift test was also run. It still fails in unrelated existing tests outside this status-item path:

  • ProviderStorageFootprintTests/forced scheduled storage refresh does not restart identical in flight scan
  • CodexManagedOpenAIWebRefreshTests/regular refresh does not await OpenAI web scrape
  • CodexBackgroundRefreshCoalescingTests/rapid regular refreshes coalesce concurrent Codex credits fetches

@clawsweeper
Copy link
Copy Markdown

clawsweeper Bot commented May 23, 2026

Codex review: needs maintainer review before merge.

Latest ClawSweeper review: 2026-05-23 19:39 UTC / May 23, 2026, 3:39 PM ET.

Workflow note: Future ClawSweeper reviews update this same comment in place.

How this review workflow works
  • 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.

Summary
The branch adds stable accessibility identity for merged/provider status items and changes display-change recovery to refresh displaced live items in place while keeping destructive recreation for missing or zero-width items.

Reproducibility: no. high-confidence live reproduction by this review; I did not run Thaw/Ice or a macOS display-removal setup. Source inspection of current main does show display-change recovery can recreate visible detached/off-current-screen status items, matching the reported failure mechanism.

PR rating
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
Patch quality: 🐚 platinum hermit
Summary: The PR has strong runtime proof and a focused implementation, with remaining confidence limited mainly by not reproducing the third-party menu bar manager scenario here.

Rank-up moves:

  • none
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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.

Real behavior proof
Sufficient (live_output): The PR body and follow-up comment provide after-fix live output for focused tests, make check, compile_and_run, an AppKit identity soak, and an AX identity probe.

Mantis proof suggestion
A short native desktop proof with visible AX/item identity diagnostics would add maintainer confidence for this AppKit menu bar behavior. A maintainer can ask Mantis to capture proof by posting a new PR comment that starts with the OpenClaw Mantis account mention, followed by:

visual task: verify CodexBar keeps the same menu bar item identity through display-change recovery with redacted AX/item identity diagnostics.

Risk before merge

  • I did not independently run Thaw/Ice or a real macOS display-removal setup; runtime confidence rests on source inspection plus the contributor's AppKit and AX proof output.
  • The PR body reports full swift test still has unrelated existing failures, although focused tests, make check, and compile_and_run are reported passing.

Maintainer options:

  1. Decide the mitigation before merge
    Merge the non-destructive displaced-item refresh and stable accessibility identity changes if maintainer review accepts the AppKit behavior, while keeping autosaveName persistence out unless separately designed and tested.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge
No repair lane is needed; there is no concrete automated fix target, and the remaining action is normal maintainer review or merge.

Security
Cleared: The diff is limited to AppKit status item behavior, tests, and changelog text; it does not change dependencies, scripts, secrets, permissions, or code-loading paths.

Review details

Best possible solution:

Merge the non-destructive displaced-item refresh and stable accessibility identity changes if maintainer review accepts the AppKit behavior, while keeping autosaveName persistence out unless separately designed and tested.

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

No high-confidence live reproduction by this review; I did not run Thaw/Ice or a macOS display-removal setup. Source inspection of current main does show display-change recovery can recreate visible detached/off-current-screen status items, matching the reported failure mechanism.

Is this the best way to solve the issue?

Yes, based on source review. The patch preserves destructive recreation for missing button/window or zero-width status items, uses in-place refresh only for displaced live items, and adds focused identity/recovery coverage without introducing autosaveName persistence.

Label justifications:

  • P2: This is a focused user-visible menu bar reliability fix with limited blast radius and no emergency data-loss, security, or crash-loop impact.
  • rating: 🐚 platinum hermit: Current PR rating is 🐚 platinum hermit because proof is 🦞 diamond lobster, patch quality is 🐚 platinum hermit, and The PR has strong runtime proof and a focused implementation, with remaining confidence limited mainly by not reproducing the third-party menu bar manager scenario here.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body and follow-up comment provide after-fix live output for focused tests, make check, compile_and_run, an AppKit identity soak, and an AX identity probe.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body and follow-up comment provide after-fix live output for focused tests, make check, compile_and_run, an AppKit identity soak, and an AX identity probe.

What I checked:

Likely related people:

  • steipete: Current main blame for the status-item creation/recovery path points to Peter Steinberger, and the latest PR commit refines the same display-change recovery behavior. (role: current area contributor and PR follow-up author; confidence: high; commits: f61469c3cde6, b7ed1b7c23e8; files: Sources/CodexBar/MenuBarVisibilityWatcher.swift, Sources/CodexBar/StatusItemController.swift)
  • yipjunkai: The changelog credits prior work on waiting for display changes to settle and retrying detached status items, which is adjacent to this recovery path. (role: adjacent display-change recovery contributor; confidence: low; files: CHANGELOG.md)
  • Llldmiao: The changelog credits prior work on recovering visible status items after unplugging the display hosting the menu bar item. (role: adjacent menu-bar recovery contributor; confidence: low; files: CHANGELOG.md)

Codex review notes: model gpt-5.5, reasoning high; reviewed against 94f831a20932.

@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. labels May 23, 2026
@lederniermagicien lederniermagicien force-pushed the fix/stable-menu-bar-identity branch from fe86e1b to c41f618 Compare May 23, 2026 15:06
@lederniermagicien
Copy link
Copy Markdown
Contributor Author

Addressed the ClawSweeper blocker in c41f6184.

  • Removed the new NSStatusItem.autosaveName assignments.
  • Kept the non-destructive recovery path for live displaced status items.
  • Kept stable non-persistent identity via accessibility identifier/title/tooltip.
  • Updated tests to assert we do not introduce CodexBar autosave keys while still preserving the AX identity and same NSStatusItem/button instances through non-destructive refresh.

Verification after the update:

  • swift test --filter 'MenuBarVisibilityWatcherTests|StatusItemControllerSplitLifecycleTests' passed.
  • make check passed.
  • ./Scripts/compile_and_run.sh passed.
  • AX probe against the running packaged app still reports AXMenuBarItem title CodexBar with identifier CodexBar.StatusItem.codex.

@clawsweeper re-review

@clawsweeper
Copy link
Copy Markdown

clawsweeper Bot commented May 23, 2026

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. labels May 23, 2026
@steipete steipete merged commit 22cf7d4 into steipete:main May 23, 2026
4 checks passed
@diazdesandi
Copy link
Copy Markdown
Contributor

Hi, from Thaw here. This fix looks related to an issue we’ve seen in internal testing with CodexBar 0.29.0 on macOS 26, where CodexBar’s menu bar item keeps getting recreated and ends up “stuck” until users reset Control Center’s preferences, which also corrupts its plist files.

We’re working on mitigating this from the Thaw side, but a CodexBar release with this change would solve it properly for users. Is there a rough timeline for when a build including this fix might ship?

cc. @stonerl @nightah

@nk-tedo-001
Copy link
Copy Markdown

@steipete @lederniermagicien
I'm not sure, but it looks like the unmerged icons could still be affected by this issue.
I don't have 100% reproducibility, so I'll keep an eye on it. Could you please confirm or reject this on your end?

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

Labels

P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants