Make every command behave the same from every invocation source

src/renderer/src/app-state/store.test.ts

@@ -77,3 +77,96 @@ describe('useAppStore prompt template migration', () => {
     expect(useAppStore.getState().settings.defaultBuiltInMcpDomains).toEqual([])
   })
 })
+
+// ---------------------------------------------------------------------------
+// Palette sub-mode as store state.
+//
+// This is the invariant that fixes nine commands which did nothing from a
+// keybinding or the native menu — Resume Session (Cmd+Shift+R) and Prompt
+// Template (Alt+P) shipped default chords that were silent no-ops.
+//
+// The mode used to be `useState` on the palette component. A chord does not
+// open the palette; it sets a pending invocation, which mounts the palette
+// INVISIBLY, runs the command, and unmounts it in the same commit. Two
+// independent mechanisms then destroyed the mode: the unmount itself, and the
+// mount-time reset effect, which is passive and therefore runs AFTER the layout
+// effect that dispatched.
+//
+// Testing the STORE rather than the component is deliberate. The old bug was
+// not a rendering bug — it was that this state had a component lifecycle it
+// should never have had. State without that lifecycle cannot be lost the same
+// way, and that property is what these cases pin.
+// ---------------------------------------------------------------------------
+describe('palette sub-mode', () => {
+  beforeEach(() => {
+    vi.resetModules()
+    vi.stubGlobal('localStorage', createStorageMock())
+  })
+
+  afterEach(() => {
+    vi.unstubAllGlobals()
+  })
+
+  it('starts in the command list', async () => {
+    const { useAppStore } = await import('@renderer/app-state/store')
+    expect(useAppStore.getState().paletteMode).toBe('commands')
+  })
+
+  it('makes the palette visible when a mode is entered', async () => {
+    // THE fix. Every mode-entering command wanted the palette up; the ones
+    // invoked by chord had no way to say so. Coupling the two in the action
+    // means a future mode cannot forget it.
+    const { useAppStore } = await import('@renderer/app-state/store')
+    useAppStore.getState().closeCommandPalette()
+    expect(useAppStore.getState().commandPaletteOpen).toBe(false)
+
+    useAppStore.getState().setPaletteMode('prompt-template')
+
+    expect(useAppStore.getState().paletteMode).toBe('prompt-template')
+    expect(useAppStore.getState().commandPaletteOpen).toBe(true)
+  })
+
+  it('resets to the command list on close, so reopening never resumes a sub-flow', async () => {
+    const { useAppStore } = await import('@renderer/app-state/store')
+    useAppStore.getState().setPaletteMode('resume')
+    useAppStore.getState().closeCommandPalette()
+    expect(useAppStore.getState().paletteMode).toBe('commands')
+
+    useAppStore.getState().openCommandPalette()
+    expect(useAppStore.getState().paletteMode).toBe('commands')
+  })
+
+  it('survives the close-after-run rule that used to kill it', async () => {
+    // The chord sequence, in store terms. `closeAfterRun` is captured as
+    // `!commandPaletteOpen` when the invocation is requested, so it is true
+    // here. The palette host now skips the close when the command turned the
+    // palette on — it previously consulted a hardcoded id list holding only
+    // `open-command-palette`, which could never have covered these nine.
+    const { useAppStore } = await import('@renderer/app-state/store')
+    useAppStore.getState().closeCommandPalette()
+    useAppStore.getState().requestCommandInvocation('prompt-template', 'keybinding')
+    const pending = useAppStore.getState().pendingCommandInvocation
+    expect(pending?.closeAfterRun).toBe(true)
+
+    // What the command's `run` does.
+    useAppStore.getState().setPaletteMode('prompt-template')
+    useAppStore.getState().clearCommandInvocation()
+
+    const shouldClose = pending!.closeAfterRun && !useAppStore.getState().commandPaletteOpen
+    expect(shouldClose).toBe(false)
+    expect(useAppStore.getState().paletteMode).toBe('prompt-template')
+  })
+
+  it('still closes after a command that opened nothing', async () => {
+    // The rule must not become "never close" — an ordinary chord still returns
+    // the user to where they were.
+    const { useAppStore } = await import('@renderer/app-state/store')
+    useAppStore.getState().closeCommandPalette()
+    useAppStore.getState().requestCommandInvocation('split-vertical', 'keybinding')
+    const pending = useAppStore.getState().pendingCommandInvocation
+    useAppStore.getState().clearCommandInvocation()
+
+    const shouldClose = pending!.closeAfterRun && !useAppStore.getState().commandPaletteOpen
+    expect(shouldClose).toBe(true)
+  })
+})

src/renderer/src/app-state/types.ts

@@ -1,3 +1,4 @@
+import type { PaletteMode } from '@renderer/features/command-palette/paletteMode'
 import type { Settings } from '@renderer/app-state/settings/types'
 import type {
   DispatchAttachIntent,
@@ -38,7 +39,8 @@ export type UiShellSlice = UiShellState & {
   clearCommandInvocation: () => void
   openCommandPalette: () => void
   closeCommandPalette: () => void
-  toggleCommandPalette: () => void
+  /** Enter a palette sub-mode and make the palette visible. */
+  setPaletteMode: (mode: PaletteMode) => void
   openPathPicker: (defaultValue?: string) => void
   closePathPicker: () => void
   setPathPickerDefault: (value: string) => void

src/renderer/src/app-state/uiShell/slice.ts

@@ -1,3 +1,5 @@
+import { DEFAULT_PALETTE_MODE } from '@renderer/features/command-palette/paletteMode'
+import type { PaletteMode } from '@renderer/features/command-palette/paletteMode'
 import type { StateCreator } from 'zustand'
 
 import type { AppStore, UiShellSlice } from '@renderer/app-state/types'
@@ -10,6 +12,7 @@ export const createUiShellSlice: StateCreator<
   UiShellSlice
 > = set => ({
   commandPaletteOpen: false,
+  paletteMode: DEFAULT_PALETTE_MODE,
   pathPickerOpen: false,
   pathPickerDefault: '',
   tileTabsModalOpen: false,
@@ -68,12 +71,24 @@ export const createUiShellSlice: StateCreator<
   clearCommandInvocation: () =>
     set({ pendingCommandInvocation: null }, false, 'uiShell/clearCommandInvocation'),
 
+  // Opening always lands in the command list. Reopening should never resume a
+  // half-finished sub-flow the user abandoned.
   openCommandPalette: () =>
-    set({ commandPaletteOpen: true }, false, 'uiShell/openCommandPalette'),
+    set({ commandPaletteOpen: true, paletteMode: DEFAULT_PALETTE_MODE }, false, 'uiShell/openCommandPalette'),
   closeCommandPalette: () =>
-    set({ commandPaletteOpen: false }, false, 'uiShell/closeCommandPalette'),
-  toggleCommandPalette: () =>
-    set(state => ({ commandPaletteOpen: !state.commandPaletteOpen }), false, 'uiShell/toggleCommandPalette'),
+    set({ commandPaletteOpen: false, paletteMode: DEFAULT_PALETTE_MODE }, false, 'uiShell/closeCommandPalette'),
+  /**
+   * Enter a sub-mode, MAKING THE PALETTE VISIBLE.
+   *
+   * The open is not a convenience — it is the correctness fix. A sub-mode is
+   * only meaningful rendered, so every caller wanted the palette up; the ones
+   * invoked from a chord simply had no way to say so, because `setMode` was
+   * component-local and the host they were mutating was invisible.
+   *
+   * Coupling the two here means a future mode-entry point cannot forget it.
+   */
+  setPaletteMode: (mode: PaletteMode) =>
+    set({ paletteMode: mode, commandPaletteOpen: true }, false, 'uiShell/setPaletteMode'),
 
   openPathPicker: (defaultValue = '') =>
     set({

src/renderer/src/app-state/uiShell/types.ts

@@ -1,3 +1,4 @@
+import type { PaletteMode } from '@renderer/features/command-palette/paletteMode'
 import type { TabId, SessionId } from '@renderer/workspace/types'
 
 export type DispatchAttachIntent = {
@@ -23,6 +24,24 @@ export type PendingCommandInvocation = {
 
 export type UiShellState = {
   commandPaletteOpen: boolean
+  /**
+   * The palette's active sub-mode (Resume list, Prompt Templates, …).
+   *
+   * WHY this is store state and not `useState` on the palette component, where
+   * it lived until now: a chord never opens the palette. It sets
+   * `pendingCommandInvocation`, which mounts the palette INVISIBLY, runs the
+   * command, and unmounts it in the same commit. A mode set with the local
+   * `setMode` was therefore written to a component already invisible and about
+   * to be destroyed, and a second mechanism finished the job — the mount-time
+   * reset effect runs as a passive effect, i.e. AFTER the layout effect that
+   * dispatched, so it queued 'commands' last and won regardless.
+   *
+   * The result was nine commands that did nothing from any source but the
+   * palette, two of them shipping default chords (Cmd+Shift+R, Alt+P).
+   *
+   * Store state has no component lifecycle, so neither mechanism can reach it.
+   */
+  paletteMode: PaletteMode
   /**
    * A command id waiting to be dispatched, with the source that asked for it.
    *

src/renderer/src/app/App.tsx

@@ -57,7 +57,6 @@ export default function App() {
   const defaultBuiltInMcpDomains = useAppStore(
     state => state.settings.defaultBuiltInMcpDomains,
   )
-  const toggleCommandPalette = useAppStore(state => state.toggleCommandPalette)
 
   useThemeSync()
   useAiWorkspaceOpenRequests()

src/renderer/src/features/agent-status/commands/agentStatusCommands.ts

@@ -1,7 +1,7 @@
 import { DEFAULT_PROVIDER, isAgentProviderKind } from '@shared/types/providerKind'
 import type { CommandContext, CommandDef } from '@renderer/features/command-palette/types'
 import { commandTargetSessionId } from '@renderer/workspace/hook/selectors/commandTargetSessionId'
-import { panel, toggle, value } from '@renderer/features/command-palette/commandState'
+import { toggle } from '@renderer/features/command-palette/commandState'
 
 function focusedAgentSessionId(ctx: CommandContext): string | null {
   const sessionId = commandTargetSessionId(ctx.workspace)

src/renderer/src/features/command-palette/keybindingBaseline.test.ts

@@ -1,5 +1,6 @@
 import { describe, expect, it } from 'vitest'
 
+import { PALETTE_MODE_COMMANDS, SURFACE_OWNER_FLAGS, commandOwnsOpenSurface } from '@renderer/features/command-palette/surfaceOwnership'
 import { builtInCommandCatalog } from '@renderer/features/command-palette/catalog'
 import { resolveEffectiveKeybindings } from '@renderer/features/command-keybindings/resolve'
 
@@ -388,3 +389,127 @@ describe('recorded authority drift (pre-migration history)', () => {
     expect(undisclosed).toEqual(['⌘⇧E', '⌘⇧P', '⌥W', '⌥←', '⌥↑', '⌥→', '⌥↓'])
   })
 })
+
+// ---------------------------------------------------------------------------
+// Surface ownership — the narrow exemption to the interaction-owner gate.
+//
+// `useKeybinds` bails out of the whole key handler while any Radix dialog is
+// mounted, which is correct (⌘W must not close a pane behind a confirmation)
+// but means a dialog-based surface makes its own chord unreachable. That is the
+// entire reason ⌘⇧U could open Usage and never dismiss it, while ⌥R Reader
+// round-tripped fine — Reader renders inline and stamps no owner marker.
+//
+// The exemption lets a chord cross the gate when it maps to the command that
+// owns the surface currently in front of the user. These cases pin the two
+// properties that keep it narrow: the table names real commands, and the
+// predicate only ever answers for those.
+// ---------------------------------------------------------------------------
+describe('surface ownership', () => {
+  const catalogIds = new Set(builtInCommandCatalog.map(command => command.id))
+
+  it('names only commands that exist', () => {
+    for (const commandId of Object.keys(PALETTE_MODE_COMMANDS)) {
+      expect(catalogIds.has(commandId)).toBe(true)
+    }
+    // A stale id here is worse than useless: the chord silently stops being
+    // exempt and the surface becomes undismissable again, with nothing failing.
+    for (const commandId of Object.keys(SURFACE_OWNER_FLAGS)) {
+      expect(catalogIds.has(commandId)).toBe(true)
+    }
+  })
+
+  it('reports ownership only when that command’s own surface is open', () => {
+    expect(commandOwnsOpenSurface('usage.open', { usageModalOpen: true } as never)).toBe(true)
+    expect(commandOwnsOpenSurface('usage.open', { usageModalOpen: false } as never)).toBe(false)
+  })
+
+  it('refuses every command not in the table', () => {
+    // The gate's default is still "bail". Only a listed command may cross it,
+    // so an unrelated chord cannot reach a workspace action while a modal owns
+    // the screen — which is the bug the gate exists to prevent.
+    expect(commandOwnsOpenSurface('close-pane', { usageModalOpen: true } as never)).toBe(false)
+    expect(commandOwnsOpenSurface('split-vertical', { usageModalOpen: true } as never)).toBe(false)
+  })
+
+  it('lets a palette-mode command through whenever the palette is open', () => {
+    // Wider than strict dismissal on purpose: pressing ⌥P while the palette
+    // sits in Resume mode should SWITCH to prompt templates, not be swallowed.
+    // The command compares flags.paletteMode and decides; the table only
+    // decides whether it gets the chance.
+    expect(commandOwnsOpenSurface('prompt-template', { commandPaletteOpen: true } as never)).toBe(true)
+    expect(commandOwnsOpenSurface('prompt-template', { commandPaletteOpen: false } as never)).toBe(false)
+  })
+
+  it('gives every flag-backed command a state badge that reports its surface', () => {
+    // The two halves of the round-trip have to agree. The table lets the chord
+    // CROSS the interaction gate; the command's own state and run are what
+    // dismiss once it does. A command listed here without a `getState` is one
+    // whose second press reaches a `run` that may still only open — the chord
+    // would look like it does nothing, which is exactly the reported bug.
+    // Scoped to SURFACE_OWNER_FLAGS. Palette-mode commands are deliberately
+    // excluded: they dismiss by comparing `flags.paletteMode`, and an
+    // Open/Closed badge on Resume Session would report a property of the
+    // palette rather than of the command. Merging the two tables made this
+    // assertion demand exactly that, which is how the split was found.
+    for (const commandId of Object.keys(SURFACE_OWNER_FLAGS)) {
+      const command = builtInCommandCatalog.find(candidate => candidate.id === commandId)
+      expect(command?.getState, `${commandId} has no getState`).toBeDefined()
+    }
+  })
+
+  it('actually calls the matching close action when its surface is open', () => {
+    // The assertion that was missing, and the one a copy-paste defeats: a
+    // `getState` proving a command CAN see its surface says nothing about the
+    // `run` closing the RIGHT one. Driving each run with its own flag true and
+    // asserting the matching `close*` fired — and that no other one did — is
+    // what pins the pairing.
+    for (const [commandId, flag] of Object.entries(SURFACE_OWNER_FLAGS)) {
+      const command = builtInCommandCatalog.find(candidate => candidate.id === commandId)
+      if (!command) throw new Error(`${commandId} is not in the catalog`)
+
+      const calls: string[] = []
+      const ui = new Proxy({}, {
+        get: (_target, prop: string) => () => calls.push(prop),
+      })
+      command.run({
+        ui,
+        flags: { [flag]: true },
+        workspace: { state: { sessions: {}, tabs: [], activeTabId: '' } },
+      } as never)
+
+      // Two legitimate shapes, and the distinction is real rather than
+      // cosmetic: most of these surfaces have separate open/close actions and
+      // the command must branch, but Remote Panel has a genuine `toggle*` that
+      // is already correct in both directions. Both dismiss when the flag is
+      // true; only "opened anyway" is a defect.
+      const dismissed = calls.filter(name =>
+        (name.startsWith('close') && name !== 'closePalette') || name.startsWith('toggle'),
+      )
+      expect(dismissed, `${commandId} (flag ${flag}) did not dismiss`).toHaveLength(1)
+      expect(
+        calls.some(name => name.startsWith('open')),
+        `${commandId} opened its surface while it was already open`,
+      ).toBe(false)
+    }
+  })
+
+  it('excludes surfaces where a second press must not mean dismiss', () => {
+    // Recorded as a decision, not an omission. Per-target pickers should
+    // RE-TARGET on a second press; Save Debug Logs and Attach Recording Note
+    // each write something on the way in, so a second press legitimately writes
+    // a second one; Settings is a destination, not a peek.
+    for (const excluded of [
+      'view-prompts',
+      'rewind-to-prompt',
+      'set-agent-view-mode',
+      'dispatch.color-flag.set',
+      'save-debug-logs',
+      'attach-recording-note',
+      'open-settings',
+      'open-command-palette',
+    ]) {
+      expect(Object.keys(SURFACE_OWNER_FLAGS)).not.toContain(excluded)
+      expect(Object.keys(PALETTE_MODE_COMMANDS)).not.toContain(excluded)
+    }
+  })
+})

src/renderer/src/features/command-palette/paletteMode.ts

@@ -0,0 +1,41 @@
+// ---------------------------------------------------------------------------
+// The palette's sub-mode.
+//
+// WHY this type lives in its own module rather than beside the component that
+// renders it: the mode is STORE state (see `uiShell.paletteMode`), and
+// `app-state/uiShell/types.ts` needs the type. Importing it from
+// `command-palette/types.ts` would drag the whole `CommandContext` graph —
+// which reaches the workspace store — into the app-state layer that
+// `CommandContext` itself depends on. A five-line module breaks the cycle.
+//
+// WHY the mode is store state at all, when it was `useState` on the palette
+// component for most of this app's life: a chord does not open the palette. It
+// sets a pending invocation, which mounts the palette INVISIBLY, runs the
+// command, and unmounts it in the same commit. Any mode a command set with
+// `setMode` was therefore written to a component that was already invisible and
+// about to be destroyed — nine commands (Resume Session, Prompt Template, the
+// buried pair, the three AI-workspace ones, and two prompt-template siblings)
+// were silent no-ops from every source except the palette itself, two of them
+// with shipped default chords.
+//
+// Store state has no lifecycle tied to the component, so it cannot be lost that
+// way. That is the whole argument; there is no ordering subtlety to get wrong.
+// ---------------------------------------------------------------------------
+
+export type PaletteMode =
+  | 'commands'
+  | 'resume'
+  | 'buried'
+  | 'kill-buried'
+  | 'prompt-template'
+  | 'manage-prompt-template'
+  | 'fill-prompt-template'
+  | 'save-prompt-template'
+  | 'edit-prompt-template'
+  | 'ai-workspace-open'
+  | 'ai-workspace-create'
+  | 'ai-workspace-clear'
+
+/** The mode a freshly-opened palette starts in. Also what closing resets to —
+ *  reopening should never resume a half-finished sub-flow. */
+export const DEFAULT_PALETTE_MODE: PaletteMode = 'commands'