Skip to content

[codex] Fix command palette query focus#287

Merged
onevcat merged 3 commits into
mainfrom
codex/fix-command-palette-focus
May 14, 2026
Merged

[codex] Fix command palette query focus#287
onevcat merged 3 commits into
mainfrom
codex/fix-command-palette-focus

Conversation

@onevcat
Copy link
Copy Markdown
Owner

@onevcat onevcat commented May 14, 2026

Summary

  • Replaces the fragile SwiftUI @FocusState query focus handoff with a thin AppKit-backed query field, following the existing terminal search overlay pattern.
  • Reasserts query focus shortly after the palette appears so the macOS 26.5/Tahoe Cmd+P menu-command path cannot leave the terminal as first responder.
  • Keeps the existing hidden shortcut buttons for Up/Down and Ctrl-P/Ctrl-N as a fallback path, while the AppKit field also handles Escape, Return, Up/Down, and Ctrl-P/Ctrl-N when focused.
  • Restores focus to the selected terminal when the palette is dismissed without switching terminals, including Escape, repeated Cmd+P, and passive command actions such as check-for-updates/refresh/debug toast.
  • In Canvas mode, restores focus to the last Canvas-focused terminal card via terminalClient.canvasFocusedWorktreeID().

Root Cause Evidence

The reproduced behavior on macOS 26.5 is specific:

  • Cmd+P opens the palette.
  • The original palette still received navigation keys: Up/Down moved the selection.
  • The query field had no insertion cursor, and typed text continued going to the terminal behind it.
  • Clicking the query field made the original implementation work.
  • Cmd+Shift+P worked in the original implementation.

That points away from shortcut delivery or overlay presentation, and toward the query field failing to become first responder after the Cmd+P open path. Earlier attempts also ruled out Ghostty keybind routing and upstream menu identity alignment as sufficient fixes.

One intermediate AppKit field version made things worse because it replaced the existing keyboard shortcut fallback and closed the palette on text-field focus churn. This version intentionally avoids that: it preserves the fallback shortcut buttons and does not use blur as a dismiss signal.

Regression Check

  • Filtering, selection updates, item activation, row clicks, and recency ordering still use the existing Command Palette reducer and view code paths.
  • Up/Down and Ctrl-P/Ctrl-N hidden shortcut buttons are preserved as fallback keyboard routing.
  • The query changed surface is limited to query focus and query text input plumbing.
  • Terminal focus restoration is routed through TerminalClient.Command.focusSelectedTab.
  • Normal/Shelf restore uses selectedTerminalWorktree; Canvas restore uses canvasFocusedWorktreeID; worktree selection explicitly does not restore the previous terminal focus.

Validation

  • xcodebuild test -project supacode.xcodeproj -scheme supacode -destination "platform=macOS" -only-testing:supacodeTests/AppShortcutsTests -only-testing:supacodeTests/CommandPaletteFeatureTests -only-testing:supacodeTests/AppFeatureCommandPaletteTests CODE_SIGNING_ALLOWED=NO CODE_SIGNING_REQUIRED=NO CODE_SIGN_IDENTITY="" -skipMacroValidation 2>&1 | xcsift -f toon
  • make build-app 2>&1 | xcsift -f toon

Manual Verification

  • On macOS 26.5, focus a terminal, press Cmd+P, type a query, and verify the text appears in the Command Palette field rather than the terminal.
  • Verify Cmd+P then Up/Down changes palette selection.
  • Verify Cmd+P then clicking the query field still allows typing.
  • Verify Cmd+Shift+P opens the palette and accepts typing.
  • Verify Escape dismisses the palette and returns typing focus to the previous terminal in Normal, Shelf, and Canvas modes.
  • Verify repeated Cmd+P dismisses the palette and returns typing focus to the previous terminal in Normal, Shelf, and Canvas modes.
  • Verify choosing a passive command, such as debug toast, dismisses the palette and returns typing focus to the previous terminal in Normal, Shelf, and Canvas modes.
  • Verify choosing a worktree does not restore focus to the previous terminal.

@onevcat onevcat marked this pull request as ready for review May 14, 2026 00:58
@onevcat onevcat marked this pull request as draft May 14, 2026 01:03
@onevcat onevcat force-pushed the codex/fix-command-palette-focus branch from 40941b3 to 4a7b675 Compare May 14, 2026 01:29
@onevcat onevcat changed the title [codex] Fix command palette query focus [codex] Route command palette shortcut through Ghostty May 14, 2026
@onevcat onevcat force-pushed the codex/fix-command-palette-focus branch from 4a7b675 to 51affc7 Compare May 14, 2026 01:50
@onevcat onevcat changed the title [codex] Route command palette shortcut through Ghostty [codex] Stabilize command shortcut menu identity May 14, 2026
@onevcat onevcat force-pushed the codex/fix-command-palette-focus branch from 51affc7 to 53fcd1a Compare May 14, 2026 06:02
@onevcat onevcat changed the title [codex] Stabilize command shortcut menu identity [codex] Use AppKit field for command palette query May 14, 2026
@onevcat onevcat force-pushed the codex/fix-command-palette-focus branch from 53fcd1a to 7962f72 Compare May 14, 2026 06:24
@onevcat onevcat changed the title [codex] Use AppKit field for command palette query [codex] Fix command palette query focus May 14, 2026
@onevcat onevcat force-pushed the codex/fix-command-palette-focus branch from 7962f72 to 623392d Compare May 14, 2026 07:32
@onevcat onevcat marked this pull request as ready for review May 14, 2026 09:10
@onevcat onevcat merged commit 8c12deb into main May 14, 2026
1 check passed
@onevcat onevcat deleted the codex/fix-command-palette-focus branch May 14, 2026 11:52
shanegao pushed a commit to shanegao/Prowl that referenced this pull request May 19, 2026
Adds three worktree navigation commands and reverses the focus-restore
default so future delegate handlers inherit it for free.

Navigation actions (gated on selectedWorktreeID):

- Reveal in Finder — dispatches openWorktree(.finder), so it always
  opens Finder regardless of the user's configured default editor.
  No hotkey hint: ⌘O binds the configurable openWorktree action,
  which may resolve to a different tool.
- Copy Path — writes selectedTerminalWorktree.workingDirectory.path
  to NSPasteboard inline. No hotkey hint: the underlying ⌘⇧C binding
  is dead (onevcat#295 tracks removing it).
- Reveal in Sidebar — sends .showLeftSidebar followed by
  .revealSelectedWorktreeInSidebar.

Focus restore refactor:

The original fix (onevcat#287) patched five sites to merge
restoreCommandPaletteTerminalFocusEffect into their effects. Every
delegate handler added since silently dropped the behavior (Toggle
Sidebar / Active Agents / Shelf, Show Diff, and the new navigation
actions all lost focus restoration).

Instead of per-case opt-in, this commit adds a dedicated Reduce after
`core` that fires the restore for every CommandPalette delegate action
except those in commandPaletteDelegateChangesActiveSelection
(selectWorktree, jumpToLatestUnread, viewArchivedWorktrees,
newWorktree, toggleCanvas). The three explicit .merge(restore...)
calls in checkForUpdates / refreshWorktrees / debugTestToast are
removed since they're now redundant.

Future commands get focus restoration by default; opting out means
adding the case to the deny list.

Implementation notes:

- navigationDelegateAction helper in CommandPaletteFeature mirrors the
  existing pullRequestDelegateAction pattern, keeping appDelegateAction
  under the cyclomatic-complexity threshold.
- ghosttyCommand test updated to expect both performBindingAction and
  the new focusSelectedTab effect (order-independent assertions).
- New tests cover the whitelist default (toggleLeftSidebar) and the
  deny list (toggleCanvas), plus delegate routing for all three new
  navigation actions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant