fix(next palette): the focus overlay goes on the window the palette opens over

Merged
TE
7d ago
+64-7

Fixes #4303.

In cmux NIGHTLY, pressing Cmd+P or Cmd+Shift+P opens (or appears to invoke) the surface picker / command palette, but the palette text field never claims keyboard focus. All subsequent keystrokes go to the underlying focused terminal surface — typing the search query writes directly into the shell instead of filtering the palette.

What was wrong

#24814 fixed the responder handoff inside the panel (onBecomeKey → focusField). The remaining hole is one level up: the window under the panel is not reliably told the palette is open.

The palette panel is a child of the window it opens over (PaletteController.present, which resolves relativeTo: ?? NSApp.keyWindow ?? NSApp.mainWindow, then walks up to the top-level parent). Its focus overlay, though, went on windows.active — a different resolution: the window an input event came from, else the key window's owner, else the last active one.

Those disagree whenever the palette opens over a window that is not the active one:

  • an explicit relativeTo: that is not windows.active;
  • a press in one window while another holds the keys — windows.active is the event's window, present parents to the key window;
  • a key window no controller owns, so owner(of:) is nil and active falls back to lastActive.

When they disagree the overlay lands on a window the palette is not over, and the window under the panel keeps an empty overlay stack. That window's FocusCoordinator therefore goes on treating its terminal as the keyboard target and FocusEffectApplier keeps re-seating the terminal surface as its first responder — the "keystrokes leak to the terminal" shape of this bug. It breaks three stated invariants in plans/cmux-next/input-spec.md: W5 ("panel has the keys but the window's overlay stack is empty"), W6, and F1 ("while an overlay is open it is the keyboard target").

W6's check only counts overlays (palettes.count != 1), so an overlay on the wrong window passed it; only W5 catches it, and only in a verification build, and it reports rather than repairs.

The fix

PaletteController.onVisibilityChange now reports the window the panel actually parented to, and AppServices.observePaletteForFocus opens the overlay on that window's controller instead of guessing from windows.active. parentWindow is assigned before the callback so the reported window and the panel's parent cannot drift. The close path is unchanged — it already closes the overlay on whichever window holds it.

Test

PaletteOverlayWindowTests.theOverlayGoesOnTheWindowThePaletteOpensOver opens two windows, points keyWindowSource at the second so windows.active is the one the palette is not over, presents over the first, and asserts the overlay is on the first and not the second. It fails on the parent commit and passes here. observePaletteForFocus had no test at all before this.

Deliberately out of scope

An independent read of the whole key path also found that ActiveAppKeyPanel.canBecomeKey == isAppActive() means a palette opened while the app is inactive (a CLI/MCP open, a CMUX_NEXT_NO_ACTIVATE=1 run) draws without taking the keys. That is the panel's documented contract — it exists so a non-activating panel never captures what the user is typing into their frontmost app — so changing it is a product decision, not a bug fix, and it is not touched here. The Cmd+P path cannot hit it: a key event reaching the app means the app is active.

No UI change, so no before/after capture applies.

Risk

Low and contained: one callback signature with a single consumer, and one window lookup. The only behaviour change is which window's overlay stack records an open palette, and only in the cases where the two resolutions disagreed.

Agent session: agent session

🤖 Generated with Claude Code

View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need.

Summary by cubic

Fixes the command palette focus leak: when the palette opened over a window that wasn't windows.active, its focus overlay went on the wrong window, leaving the window under the panel with an empty overlay stack so keystrokes went to the terminal instead of the palette.

PaletteController now reports the window the panel actually parents to, and observePaletteForFocus opens the overlay on that window's controller instead of guessing from windows.active. Adds a test (PaletteOverlayWindowTests) that fails on the old behavior and passes here.

No UI change; only the overlay placement logic is affected.

No capture: the change only moves the palette's focus overlay onto the window the palette opens over; nothing drawn changes.