Harden menu bar status item placement#1216
Conversation
|
Codex review: needs real behavior proof before merge. Reviewed May 30, 2026, 12:12 PM ET / 16:12 UTC. Summary Reproducibility: no. The PR body describes field debugging on crowded macOS 26 menu bars, but the available evidence does not give a high-confidence current-main reproduction path in a real menu-bar setup. Review metrics: 2 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:
Proof guidance:
Mantis proof suggestion Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land the maintainer-fixup version after adding real macOS after-fix proof for fresh placement and upgrade preservation, or have maintainers explicitly accept the remaining AppKit placement migration risk. Do we have a high-confidence way to reproduce the issue? No. The PR body describes field debugging on crowded macOS 26 menu bars, but the available evidence does not give a high-confidence current-main reproduction path in a real menu-bar setup. Is this the best way to solve the issue? Mostly yes. Stable autosave names with a legacy-preservation guard and focused migration tests look like a maintainable fix, but merge readiness depends on real runtime proof or explicit maintainer acceptance of the remaining AppKit behavior risk. AGENTS.md: found and applied where relevant. Codex review notes: model gpt-5.5, reasoning high; reviewed against 5ce38b605cca. 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: a6f4282e4e
ℹ️ 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".
| static func shouldSetPreferredPosition(_ value: Any?) -> Bool { | ||
| guard let value else { return true } | ||
| guard let number = value as? NSNumber else { return true } | ||
| return number.doubleValue > self.suspiciousPreferredPositionThreshold |
There was a problem hiding this comment.
When a user already has a real NSStatusItem Preferred Position value above 100 (for example after dragging CodexBar farther left among other menu extras or on a wide display), this treats that saved order as corrupt and prepare resets it to 0 every time the status item is created. That erases the user’s menu bar ordering and can also give all split provider items the same preferred position; the repair should be limited to values that are actually offscreen/corrupt or done as a narrower one-time migration.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
rating: 🧂 unranked krab
Set stable autosave names for CodexBar status items and preflight their preferred position before creation so crowded macOS menu bars do not inherit parked Item-N placement. Add CGWindowList-based diagnostics for status item window bounds, keep cached icon renders self-healing, and make merged fallback provider selection follow enabled provider order. Cover placement preflight, window probing, stable identities, cached title repair, and provider-order fallback with tests.
steipete
force-pushed
the
codex/statusitem-placement-preflight
branch
from
a6f4282 to
38f6841
Compare
|
Pushed maintainer fixups on top of this branch. Main changes: stable placement keys now preserve reasonable legacy Proof is in the PR body. Thanks @pdurlej. |
clawsweeper
Bot
added
rating: 🦪 silver shellfish
2 participants
Summary
Harden CodexBar status item placement on crowded macOS 26 menu bars.
This PR adds stable
NSStatusItem.autosaveNamevalues, writes a low preferred-position key before status item creation when the saved value is missing or suspiciously high, and adds CGWindowList-based diagnostics for real menu-bar window bounds. It also keeps cached brand-percent renders self-healing by reapplying the button title, and makes merged fallback provider selection follow the configured enabled provider order.Maintainer follow-up: the placement preflight now preserves reasonable legacy AppKit
Item-Nplacement only for the matching new stable identity, so upgrades keep valid existing placement while fresh or suspicious provider identities still get seeded.Why
Field debugging on macOS 26 showed cases where
NSStatusBarButton.titleand frame were correct, but the host menu bar placed the status item under/over the clock or off-screen. AX title checks were misleading, so the diagnostics now report CGWindow bounds keyed by the stable autosave/window name.This does not try to guarantee visibility under active menu-bar managers; it reduces fragility for standalone CodexBar and gives better evidence when the host menu bar is the failing layer.
Changes
codexbar-mergedcodexbar-<provider>NSStatusItem Preferred Position <autosaveName>to0before item creation when missing or greater than100.Item-Nplacement for the matching stable identity during upgrade.MenuBarStatusItemWindowProbeusingCGWindowListCopyWindowInfofor bounds/on-screen diagnostics.Bundle.modulefor provider icon resources outside app-bundle execution so package tests exercise the brand-percent path.Validation
swift test --filter StatusItemControllerSplitLifecycleTests- 19 tests passed.swift test --filter MenuBarVisibilityWatcherTests --filter StatusItemAnimationSignatureTests --filter StatusMenuTests- 73 tests passed.make check- SwiftFormat and SwiftLint clean.swift test- 3157 tests in 378 suites passed./Users/steipete/Projects/agent-scripts/skills/autoreview/scripts/autoreview --mode local- clean, no accepted/actionable findings.No full app launch in validation: repo policy avoids live provider/keychain prompts for this path; AppKit proof is via status-item/controller tests and CGWindow-style visibility tests.