fix(macos): restore shell clicks after reopening

[aryanorastar]

What changed and why

Fixes #12249.

The macOS shell reuses the same NSWindow when it is dismissed and summoned again. While the window was hidden, mouse-interception reconciliation correctly switched ignoresMouseEvents on, but the synchronizer then stopped polling because the window was no longer visible. Ordering that same window back on screen did not trigger another reconciliation, so the visible shell could remain click-through until an unrelated pointer event repaired it. That made visible controls intermittently ignore the first click—or every click if no local event could reach the window.

This change observes AppKit's authoritative NSWindow.isVisible transition and immediately reruns the existing interception policy whenever visibility changes. It also releases that observation with the synchronizer's other lifecycle resources.

The regression test mounts a real AppKit window and exercises the production sequence directly: visible → ordered out → reconciliation enables pass-through → ordered front. It proves the shell restores mouse interception before the first user click, without depending on pointer movement or test-only state.

Product invariants affected

none

How it was verified

• Reproduced test-first: the new AppKit lifecycle test failed against the previous implementation because the reopened window still ignored mouse events.
• Passed the exact regression after the fix.
• Passed 52 focused shell/window tests across ShellClickThroughPolicyTests, GlassPanelHitRegionTests, ShellSummonTests, and ShellWindowChromeTests.
• Repeated the visibility-transition regression 30 times: 30/30 passed.
• Passed the cross-surface agent contract smoke harness.
• Built and launched an isolated signed-in bundle (com.omi.omi-12249-clicks), dismissed and summoned the production shell, then used a real native pointer click on Brain; the selected route changed from Chat to Brain on the first click.
• Ran the complete macOS component suite: all launcher checks, 214 backend tests, and all shell suites passed. Three unrelated Swift cases failed or timed out under concurrent machine load; each exact case then passed serially (ChatTranscriptGestureHarnessTests, MemoryAtlasPerformanceHarnessTests, and RewindArtifactGauntletTests).

Tests

xcrun swift test --jobs 2 --package-path Desktop --filter ShellClickThroughPolicyTests/testOrderingShellBackOnScreenRestoresMouseInterception
xcrun swift test --jobs 2 --package-path Desktop --filter 'ShellClickThroughPolicyTests|GlassPanelHitRegionTests|ShellSummonTests|ShellWindowChromeTests'
./scripts/agent-logic-harness.sh --cross-surface-smoke
./test.sh

Failure class (fixes)

Failure-Class: none

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[Git-on-my-level]

Thanks @aryanorastar — this is a textbook small fix: correct root-cause diagnosis, a five-line production change, and a regression test that exercises the real AppKit lifecycle instead of a mock.

Traced through all three files at head:

• desktop/macos/Desktop/Sources/MainWindow/ShellClickThrough.swift — the diagnosis checks out against the pre-existing code: sync() calls setPollingActive(window.isVisible), and FloatingBarMouseInterceptionPolicy.shouldIgnoreMouseEvents returns true for a hidden window, so a dismissed shell ends up with ignoresMouseEvents == true and no polling timer. The only remaining reconciliation triggers were the mouse-movement monitors, which need pointer motion — so a re-summoned shell could sit there click-through until the mouse moved. Observing \.isVisible closes exactly that gap, and since the handler just calls the existing idempotent sync() (guarded ignoresMouseEvents assignment), both directions of the transition are handled with no new state. [weak self] avoids a retain cycle on the observation token, MainActor.assumeIsolated is the right tool for NSWindow KVO (always delivered on the main thread), and releasing visibilityObservation in detach() alongside pollingCancellable keeps teardown symmetric, with deinit auto-invalidation as the backstop.
• desktop/macos/Desktop/Tests/ShellClickThroughPolicyTests.swift — the new test pins the actual failure sequence: visible → orderOut + sync() (asserting the hidden shell really is in pass-through mode, which validates the test's precondition rather than assuming it) → orderFront → asserts recovery without any pointer movement. Placing the window at mouse.x + 5_000 keeps the pointer outside the frame so the ignoresMouseEvents assertions are deterministic, and defer { sync.detach(); window.orderOut(nil) } tears down the 30 Hz polling timer the KVO path restarts. Adding @MainActor to the class is required and correct given the sync is main-actor-isolated.
• desktop/macos/changelog/unreleased/20260826-shell-click-recovery.json — matches the existing unreleased-entry convention (dated slug, single user-facing change line) and the wording tracks the fixed behavior. One honest-scoping note in your favor: the issue says "intermittently", and this mechanism (dismiss → summon with no pointer movement) explains that intermittency; if a repro path ever shows up that doesn't involve a dismiss/summon cycle, that would be a separate root cause — nothing here overclaims coverage.

The desktop CI lanes are green on this head, including the Swift build & tests with the new lifecycle test. No blockers from my side — leaving the formal approval and merge sign-off to a maintainer.

---

by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with need human response.

[kodjima33]

Small, well-scoped desktop bug fix (fixes #12249) with regression test; holding at approve-only because Desktop Swift Static & Test Contracts is currently red (RuntimeOwnerIdentityTests failure, unrelated to this diff's ShellClickThrough.swift change) — hard floor blocks merge on a failing required check.

[aryanorastar]

No @kodjima33 @Git-on-my-level @undivisible — the implementation is complete and this PR is ready from the author side.

The current red state is an unrelated CI flake in RuntimeOwnerIdentityTests.testReplacementOwnerWaitsForNonCooperativePhysicalEffectToDrain; this PR does not modify that test or its production path. Evidence:

• The same code at c6048400bc already passed the complete Desktop Swift CI run: https://github.com/BasedHardware/omi/actions/runs/32991584530
• The shell-click regression itself remains green.
• I ran the exact failing RuntimeOwner test 30 times on this PR base and another 30 times after refreshing against current main: 60/60 passed.
• The downstream Desktop Swift Build & Tests check only failed because it mirrors the failed static/test-contract result; release compile passed.
• The PR is conflict-free and has maintainer approval.

I attempted to rerun the failed GitHub job directly, but GitHub restricts reruns to repository admins. A maintainer rerun of https://github.com/BasedHardware/omi/actions/runs/33064363595 is the remaining action; no product-code change is warranted for this failure.

[Git-on-my-level]

Follow-up on the current red CI, since the flake analysis above asked for a maintainer-side check — I verified it independently against the Actions history:

• This head (18e8b7f) is code-identical to c604840: the only commit between them is the empty chore(ci): retrigger commit, and the full desktop CI suite passed on that exact tree in run 32991584530.
• The failing suite, RuntimeOwnerIdentityTests.testReplacementOwnerWaitsForNonCooperativePhysicalEffectToDrain, asserts an async drain ordering (effect_cancelled_and_finished vs effect_finished) in the runtime-owner handoff path — nothing this diff touches. The change observes NSWindow.isVisible and re-runs the idempotent sync() in ShellClickThrough.swift; it has no interaction with owner transitions, and ShellClickThroughPolicyTests (including the new testOrderingShellBackOnScreenRestoresMouseInterception) ran green inside the same red run.
• The desktop test lane is flaking repo-wide today: several main commits (e.g. c48562c, a62b15e) failed the same two jobs within the same hours — one on an entirely different suite (AgentPillLifecycleTests) — while other main commits went green.
• Desktop Swift Build & Tests is the aggregation gate mirroring the static/test-contract result (VERIFY_RESULT: failure); the release-compile lane passed.

So no PR-side change is warranted for the red state. The one remaining step is operational: an admin rerun of run 33064363595 so the required checks can re-evaluate this unchanged tree. With the approval already on this head and the diff itself previously reviewed clean, nothing else is pending from my side.

---

by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with need human response.