diff --git a/docs/SPEC.md b/docs/SPEC.md index d4cd6239..b8c203df 100644 --- a/docs/SPEC.md +++ b/docs/SPEC.md @@ -2729,9 +2729,9 @@ findings) and the build rationale are in Wails window, can't share App.tsx's). This ⌘K palette's own still-open gap — the *inline per-workflow hotkey/armed-state detail* the Acceptance sentence's "showed them the hotkey" half - names — and a ⌘?/⌘/ alias (blocked on the 1:1 command↔binding - registry shape) both stay recorded in the goal file, not silently - dropped. **Update (2026-08-12, session 2)**: the COMMAND-shortcut + names — stays recorded in the goal file, not silently dropped; the + ⌘?/⌘/ alias (previously blocked on the 1:1 command↔binding registry + shape) shipped later, see the Update below. **Update (2026-08-12, session 2)**: the COMMAND-shortcut half of that inline-hotkey-hint gap (as opposed to the still-open per-workflow-TRIGGER half above) is now built and shared, not per-surface: `app/HotkeyHint.tsx` (`resolveHotkeyLabel`/ @@ -2787,7 +2787,18 @@ findings) and the build rationale are in ever called); a clash against another OVERRIDDEN command or a workflow's trigger hotkey (`TriggerService.ClaimedCombos`, reusing `trigger.CheckConflict` verbatim) is caught server-side, naming the - conflicting command or workflow. + conflicting command or workflow. **Update (2026-08-13, BACKLOG.md + Standing #6)**: `Command` gained an optional `extraBindings: + KeyCombo[]` alongside `defaultBinding` — always-on additional + bindings for the same command, deliberately NOT run through the + override/rebind machinery above (Settings still only rebinds the + primary; extras render as read-only secondary `KeyComboChip`s next to + it). `palette.open` carries ⌘/ and ⌘? as its two extras (the ⌘K + palette's own "owner reinforcement" request from goal 0015, deferred + at the time as a cross-cutting registry change) — both land on the + physical `/` key (`keyFromEventCode`, `shared/keybinding.ts`, gained + `Slash` support), distinguished by the Shift mod exactly the way + every other key here already is shift-independent. - **Per-view hotkeys** — Cmd+1 through Cmd+4 jump to a top-level view (Composition/Configure/Activity/Spec, matching the sidebar order, down from an original five once Runs stopped being a top-level view — diff --git a/docs/goals/BACKLOG.md b/docs/goals/BACKLOG.md index 2d415561..e7c634e3 100644 --- a/docs/goals/BACKLOG.md +++ b/docs/goals/BACKLOG.md @@ -200,7 +200,7 @@ live-review material, interleaved during owner reviews, not a lane.** 3. [x] [0031 — AI node family](archive/0031-ai-node-family.md) — DELIVERED 2026-08-12, the flagship capability, shipped as two sequential PRs for reviewability. PR1: `internal/domain/aiprovider` (the `AIProvider` Configure entity, mirrors MCPServer's recipe) + `internal/adapters/aiclient` (`openaicompat` covers Ollama's own `/v1` shim + LM Studio/vLLM/any BYO endpoint, `anthropic` speaks the native Messages API not its OpenAI-compat shim — both httptest-proven) + `process-ai-completion`, plus a pre-flight audit of every registered NodeType's ConfigFields against the Configure-vs-workflow split (verdict: already fully consistent — codified in `.claude/rules/architecture.md`). PR2: `process-ai-extract-structured` (own typed output-field editor, `AIExtractFieldsEditor.tsx`) + `process-ai-classify` (node-local category list, fail-safe on an out-of-enum response) + the "Example: AI classify -> branch" seed — THE decisioning composition (AI writes a category Attribute, Branch routes on it). Effect: static `ClassExternal` on all three, `EffectForNode` downgrades to `ClassLocal` for a loopback AIProvider BaseURL (owner-ratified 2026-08-12: "you're the boss") — remote asks by default, local Ollama frictionless. Conforms to node-standard.md from birth; `docs/SPEC.md` §3.3 flipped OPEN → LOCKED. 4. [x] Copy-management migration ×4 (`app/` → `composition/` → `configure/` → `views/`) then the `eslint-plugin-i18next` revisit — DELIVERED 2026-08-12, see item 10's own writeup above ([0032](archive/0032-copy-management.md)). 5. [x] Workflow pins/favorites (tech debt, split from goal 0015's remainder 2026-08-12) — DELIVERED 2026-08-13: `pinnedWorkflowIds: string[]` + `togglePinnedWorkflow` on `shared/store.ts`'s existing zustand `persist` (same localStorage tier as `activeWorkTabKey`, goal 0033's precedent — no new Go surface). `app/workflowFrecency.ts`'s new `sortWorkflowsByPinnedAndFrecency` partitions pinned (in pin-order) above the existing frecency-sorted unpinned tail, reusing `sortWorkflowsByFrecency` rather than a second algorithm. A Primer `PinIcon` `IconButton` trailing-visual pin toggle on both the Quick Panel's and ⌘K palette's workflow rows (muted outline unpinned, accent-colored "filled" once pinned) — found and fixed a real Primer interaction bug along the way: `ActionList.Item`'s own `TrailingVisual` wraps children in a `VisualWrap` span with `pointer-events: none` (trailing visuals are decorative-only by the library's own convention), which silently ate every click on the toggle until `pointer-events: auto` was added back on the button itself. Vitest covers the pinned-above-frecency/pin-order/unpinned-id-dropped/no-mutation cases; `quick-panel.spec.ts` gained a full pin→sort→unpin→revert→reload-persists e2e case. -6. [ ] ⌘?/⌘/ multi-binding keybinding alias (tech debt, split from goal 0015's remainder 2026-08-12) — schema DECIDED at prioritization (orchestrator, 2026-08-12): `Command` grows an optional `extraBindings: KeyCombo[]` alongside the existing 1:1 `defaultBinding` (backward-compatible; Settings' rebinding UI edits the primary, extras render as secondary chips; dispatch checks all). +6. [x] ⌘?/⌘/ multi-binding keybinding alias (tech debt, split from goal 0015's remainder 2026-08-12) — DELIVERED 2026-08-13: `Command` grew an optional `extraBindings: KeyCombo[]` alongside `defaultBinding` (`shared/commands.ts`, backward-compatible); `shared/keybinding.ts`'s `keyFromEventCode` gained `/` support (shift-independent, same as every other key — the Shift mod is what distinguishes ⌘/ from ⌘?, both on the physical Slash key). `palette.open` carries both as `extraBindings`, checked against the full registry + `RESERVED_COMBOS` first (no collision — nothing else uses `/`). `dispatchCommandForEvent` checks a command's effective (override-aware) primary plus its extras every dispatch; extras themselves are deliberately NOT override-checked this pass (Settings' recorder-based rebinding UI still edits only the primary). `views/KeyboardShortcutsSection.tsx` renders extras as read-only secondary `KeyComboChip`s next to the primary's click-to-rebind button. Vitest covers dispatch-matches-either-binding + override-doesn't-disable-extras + no-extraBindings-backward-compat; `keymap.spec.ts` gained both a live ⌘//⌘⇧+/ → palette-opens case and a Settings-renders-the-two-read-only-chips case. 7. [ ] [0021 — MCP dogfood gap closure](0021-mcp-dogfood-gap-closure.md) Phase 2: orchestrator-driven live MCP probing against the locked-down-enterprise use cases (the mandate names the orchestrator as the prober — self-driveable, exploratory; produces the next ranked gap list). Phase 3 judgments that need the owner surface as they're found. 8. [ ] Dev-loop instance guards (tech debt, owner-hit 2026-08-12 evening: THREE concurrent `mill.dev.app` instances in the dock, real crash risk on the 16GB machine) — two confirmed root causes, both get mechanical fixes: (a) `wails3 dev`'s Go-rebuild cycle relaunches the app WITHOUT killing the previous instance, so orphans accumulate one per rebuild during heavy agent waves — extend goal 0029's start-sweep into a per-rebuild reap (kill any existing `bin/mill.dev.app` process before the new launch; find the right hook in the Taskfile's dev target or wails3 dev's own lifecycle); (b) `task dev` ran twice concurrently (two backgrounded watchers, each with its own app+vite) — the dev target refuses to start when an instance is already running (vite-port 9245 check + wails3-dev process check, clear message naming the existing PID). This makes the standing never-two-data-sharing-instances rule ENFORCED instead of remembered. 9. [ ] Dock-bounce on parked approvals (small, unlocked by wails beta.6's Flash() gaining macOS support via NSApp requestUserAttention — PR #44's changelog finding; Mill calls Flash nowhere today) — the attention stack (goal 0023/ADR-0032's away-user layer) gains a one-shot dock bounce when an approval parks while the user is away; kernel attention-layer surface per ADR-0035 (same class as the dock badge), NOT a new composition path. Tiny: one call site in the existing NotifyPendingApproval flow + manual-only registry entry (real dock behavior isn't CI-testable). diff --git a/frontend/e2e/keymap.spec.ts b/frontend/e2e/keymap.spec.ts index 0711e9dd..4af77e37 100644 --- a/frontend/e2e/keymap.spec.ts +++ b/frontend/e2e/keymap.spec.ts @@ -201,3 +201,44 @@ test('Settings: rebinding a command persists, the new combo works, and a conflic await expect(saveRowAgain.getByTestId('keymap-row-reset')).toHaveCount(0) await expect(saveRowAgain.getByTestId('keymap-row-combo')).toHaveText('⌘S') }) + +// docs/goals/BACKLOG.md Standing #6 -- ⌘?/⌘/ multi-binding aliases on +// palette.open. commands.test.ts already covers dispatchCommandForEvent +// matching an extraBinding in isolation; this proves the live wiring +// (App.tsx's real keydown listener, the real Dialog toggling) for both +// aliases, not just Cmd+K. +test('Cmd+/ and Cmd+Shift+/ (the extra palette.open bindings) both open the command palette, same as Cmd+K', async ({ page }) => { + await page.goto('/') + const paletteDialog = page.getByRole('dialog', { name: 'Command palette' }) + + await page.keyboard.press('Meta+/') + await expect(paletteDialog).toBeVisible() + await page.keyboard.press('Escape') + await expect(paletteDialog).toHaveCount(0) + + await page.keyboard.press('Meta+Shift+/') + await expect(paletteDialog).toBeVisible() + await page.keyboard.press('Escape') + await expect(paletteDialog).toHaveCount(0) +}) + +// Settings' Keyboard Shortcuts list renders the extras as read-only +// secondary chips (views/KeyboardShortcutsSection.tsx) -- distinct from +// the primary combo button above it, which stays the only +// click-to-rebind target. +test('Settings shows palette.open\'s extra bindings as read-only secondary chips', async ({ page }) => { + await page.goto('/') + await page.getByRole('link', { name: 'Settings' }).click() + + const paletteRow = page.locator('[data-testid="keymap-row"][data-command-id="palette.open"]') + await expect(paletteRow.getByTestId('keymap-row-combo')).toHaveText('⌘K') + + const extraChips = paletteRow.getByTestId('keymap-row-extra-binding') + await expect(extraChips).toHaveCount(2) + await expect(extraChips.nth(0)).toHaveText('⌘/') + await expect(extraChips.nth(1)).toHaveText('⌘⇧/') + + // A command with no extraBindings (e.g. workflow.save) renders none. + const saveRow = page.locator('[data-testid="keymap-row"][data-command-id="workflow.save"]') + await expect(saveRow.getByTestId('keymap-row-extra-binding')).toHaveCount(0) +}) diff --git a/frontend/src/app/CommandPalette.tsx b/frontend/src/app/CommandPalette.tsx index b78108bb..d16bec86 100644 --- a/frontend/src/app/CommandPalette.tsx +++ b/frontend/src/app/CommandPalette.tsx @@ -40,20 +40,15 @@ import styles from './CommandPalette.module.css' // onClose('escape') and preventDefault()s, so Escape-to-close needs no // extra wiring here. // -// ⌘? / ⌘/ aliases (the goal's "owner reinforcement" note) are -// deliberately NOT built: shared/commands.ts's Command shape is one -// binding per command (`defaultBinding: KeyCombo | null`), and both -// dispatchCommandForEvent and the Settings "Keyboard Shortcuts" rebind -// UI (KeyboardShortcutsSection.tsx) key off that 1:1 assumption, as -// does the Go side's persisted-override map (settingsservice_keymap.go, -// one KeyCombo per command id). Adding a second real `palette.open` -// command row instead would show two identically-labelled, independently -// rebindable rows in Settings -- not a clean alias. A real alias needs -// either `defaultBinding: KeyCombo[]` threaded through all of the above, -// or teaching shared/keybinding.ts's keyFromEventCode a bare '/'/'?' key -// outside the registry entirely -- both cross-cutting enough that this -// stays ⌘K-only per this goal's own "don't restructure the registry, -// ship ⌘K only and note the deferral" instruction. +// ⌘? / ⌘/ aliases (the goal's "owner reinforcement" note, deferred at +// the time as a cross-cutting registry change) landed later as +// docs/goals/BACKLOG.md Standing #6: shared/commands.ts's Command grew +// an optional `extraBindings: KeyCombo[]` alongside `defaultBinding` +// (backward-compatible), and shared/keybinding.ts's keyFromEventCode +// now recognizes the '/' key. dispatchCommandForEvent checks every +// command's extras too; this Dialog itself needs no changes -- it +// still just renders off the store's paletteOpen flag regardless of +// which bound combo flipped it. type PaletteGroupId = 'commands' | 'workflows' | 'tabs' diff --git a/frontend/src/locales/en/views.json b/frontend/src/locales/en/views.json index a242e645..e328d823 100644 --- a/frontend/src/locales/en/views.json +++ b/frontend/src/locales/en/views.json @@ -60,7 +60,9 @@ "pressCombo": "Press a combo… (Esc to cancel)", "clickToChange": "Click to change", "reset": "Reset", - "unbound": "Unbound" + "unbound": "Unbound", + "alsoLabel": "Also:", + "extraBindingTitle": "Additional shortcut (not editable here)" }, "placeholderView": { "notBuiltYet": "Not built yet", diff --git a/frontend/src/shared/commands.test.ts b/frontend/src/shared/commands.test.ts new file mode 100644 index 00000000..45f7d858 --- /dev/null +++ b/frontend/src/shared/commands.test.ts @@ -0,0 +1,56 @@ +import { describe, expect, it } from 'vitest' +import { dispatchCommandForEvent, findCommand } from './commands' + +// docs/goals/BACKLOG.md Standing #6 (⌘?/⌘/ palette aliases): a +// Command's optional extraBindings (shared/commands.ts) must dispatch +// the SAME command as its primary defaultBinding, and an override on +// the primary must never affect extras (they're deliberately not +// override-checked -- see Command.extraBindings' own doc comment). +describe('dispatchCommandForEvent with extraBindings', () => { + const event = (init: Partial) => init as KeyboardEvent + + it('palette.open has the two documented extra bindings registered', () => { + const command = findCommand('palette.open') + expect(command?.extraBindings).toEqual([ + { mods: ['cmd'], key: '/' }, + { mods: ['cmd', 'shift'], key: '/' }, + ]) + }) + + it('Cmd+K (the primary default) still opens the palette', () => { + const ran = dispatchCommandForEvent(event({ code: 'KeyK', metaKey: true }), {}) + expect(ran).toBe(true) + }) + + it('Cmd+/ (an extra binding) also dispatches palette.open', () => { + const ran = dispatchCommandForEvent(event({ code: 'Slash', metaKey: true }), {}) + expect(ran).toBe(true) + }) + + it('Cmd+Shift+/ (the ⌘? glyph, the second extra binding) also dispatches palette.open', () => { + const ran = dispatchCommandForEvent(event({ code: 'Slash', metaKey: true, shiftKey: true }), {}) + expect(ran).toBe(true) + }) + + it('Ctrl+/ is not bound to anything -- extras match on their exact mods, not just the "/" key', () => { + const ran = dispatchCommandForEvent(event({ code: 'Slash', ctrlKey: true }), {}) + expect(ran).toBe(false) + }) + + it('an override on the primary binding does not disable the extras', () => { + // palette.open rebound to Cmd+P in Settings -- Cmd+K itself no + // longer runs it, but the two extras (never override-checked, per + // Command.extraBindings' own doc comment) still do. + const overrides = { 'palette.open': { mods: ['cmd'], key: 'P' } } + expect(dispatchCommandForEvent(event({ code: 'KeyK', metaKey: true }), overrides)).toBe(false) + expect(dispatchCommandForEvent(event({ code: 'Slash', metaKey: true }), overrides)).toBe(true) + expect(dispatchCommandForEvent(event({ code: 'KeyP', metaKey: true }), overrides)).toBe(true) + }) + + it('a command with no extraBindings is unaffected (backward-compatible)', () => { + // tab.close has no extras -- only its own Cmd+W default dispatches + // it, same behavior as before this feature existed. + expect(dispatchCommandForEvent(event({ code: 'KeyW', metaKey: true }), {})).toBe(true) + expect(dispatchCommandForEvent(event({ code: 'Slash', metaKey: true, ctrlKey: true }), {})).toBe(false) + }) +}) diff --git a/frontend/src/shared/commands.ts b/frontend/src/shared/commands.ts index ae3bb66f..7c58037d 100644 --- a/frontend/src/shared/commands.ts +++ b/frontend/src/shared/commands.ts @@ -31,6 +31,19 @@ export interface Command { // palette.open, whose binding is reserved ahead of goal 0015 actually // building the palette. defaultBinding: KeyCombo | null + // Additional, always-on bindings for the SAME command (docs/goals/ + // BACKLOG.md Standing #6) -- backward-compatible (every existing + // command simply omits it). Deliberately NOT user-rebindable this + // pass: Settings' recorder-based rebinding UI + // (views/KeyboardShortcutsSection.tsx) edits `defaultBinding` only + // (via keybindingOverrides, same as before); extras render as + // read-only secondary KeyComboChips there and are never looked up in + // keybindingOverrides by dispatchCommandForEvent below -- a real + // "edit an alias" feature (its own override storage keyed by + // command+index, its own Go-side persistence) is more than this + // item's scope covers, named as a future extension rather than half- + // built here. + extraBindings?: KeyCombo[] run: () => void } @@ -177,6 +190,19 @@ export const COMMANDS: Command[] = [ // shared signal the same way workflow.save/workflow.run already // do via canvasCommandRequest. defaultBinding: { mods: ['cmd'], key: 'K' }, + // ⌘? / ⌘/ aliases (docs/goals/BACKLOG.md Standing #6, the "owner + // reinforcement" note CommandPalette.tsx used to document as + // deliberately not built): both land on the same physical '/' key + // (keyFromEventCode is shift-independent, shared/keybinding.ts), + // distinguished by the Shift mod -- ⌘/ is the bare combo, ⌘? adds + // Shift (what actually produces the '?' glyph). Checked against + // every other command's defaultBinding above and RESERVED_COMBOS + // (shared/keybinding.ts): neither uses the '/' key on macOS, no + // collision. + extraBindings: [ + { mods: ['cmd'], key: '/' }, + { mods: ['cmd', 'shift'], key: '/' }, + ], run: () => useAppStore.getState().togglePalette(), }, { @@ -245,21 +271,24 @@ export function effectiveBinding(command: Command, overrides: Record): boolean { const pressed = comboFromEvent(e) if (!pressed) return false const want = comboKey(pressed.mods, pressed.key) for (const command of COMMANDS) { const binding = effectiveBinding(command, overrides) - if (!binding) continue - if (comboKey(binding.mods, binding.key) === want) { + const bindings = binding ? [binding, ...(command.extraBindings ?? [])] : (command.extraBindings ?? []) + if (bindings.some((b) => comboKey(b.mods, b.key) === want)) { command.run() return true } diff --git a/frontend/src/shared/keybinding.test.ts b/frontend/src/shared/keybinding.test.ts index f0a7ede3..91e504a6 100644 --- a/frontend/src/shared/keybinding.test.ts +++ b/frontend/src/shared/keybinding.test.ts @@ -40,6 +40,14 @@ describe('keyFromEventCode', () => { expect(keyFromEventCode('Enter')).toBe('Enter') expect(keyFromEventCode('NumpadEnter')).toBe('Enter') }) + + // docs/goals/BACKLOG.md Standing #6 (⌘?/⌘/ palette aliases): the '/' + // physical key, shift-independent like every other key here -- Shift + // held or not is captured separately as a mod (modsFromEvent), not a + // second key value. + it('maps the Slash code to a literal forward slash', () => { + expect(keyFromEventCode('Slash')).toBe('/') + }) }) describe('comboFromEvent', () => { @@ -61,6 +69,15 @@ describe('comboFromEvent', () => { it('captures Ctrl+Tab, the keymap default for tab.next', () => { expect(comboFromEvent(event({ code: 'Tab', ctrlKey: true }))).toEqual({ mods: ['ctrl'], key: 'Tab' }) }) + + // docs/goals/BACKLOG.md Standing #6: ⌘/ and ⌘? (Shift+/) are distinct + // KeyCombos distinguished by the shift mod, both on the same + // physical '/' key -- palette.open's two extraBindings + // (shared/commands.ts) rely on this distinction. + it('distinguishes Cmd+/ from Cmd+Shift+/ (the ⌘? glyph) via the shift mod, same physical key', () => { + expect(comboFromEvent(event({ code: 'Slash', metaKey: true }))).toEqual({ mods: ['cmd'], key: '/' }) + expect(comboFromEvent(event({ code: 'Slash', metaKey: true, shiftKey: true }))).toEqual({ mods: ['cmd', 'shift'], key: '/' }) + }) }) describe('comboKey', () => { diff --git a/frontend/src/shared/keybinding.ts b/frontend/src/shared/keybinding.ts index 54410c32..df8a6079 100644 --- a/frontend/src/shared/keybinding.ts +++ b/frontend/src/shared/keybinding.ts @@ -17,6 +17,13 @@ export function keyFromEventCode(code: string): string | null { if (code === 'Tab') return 'Tab' if (code === 'Comma') return ',' if (code === 'Enter' || code === 'NumpadEnter') return 'Enter' + // docs/goals/BACKLOG.md Standing #6 (⌘?/⌘/ palette aliases): the + // physical key is the same whether Shift is held or not (Shift+/ + // producing '?' is exactly the shift-independence this function's + // own header comment already documents for every other key) -- the + // Shift MOD is what distinguishes ⌘/ from ⌘? at the KeyCombo level + // (modsFromEvent below), not a second key value here. + if (code === 'Slash') return '/' return null } diff --git a/frontend/src/views/KeyboardShortcutsSection.tsx b/frontend/src/views/KeyboardShortcutsSection.tsx index c505c4ae..d2f13a2e 100644 --- a/frontend/src/views/KeyboardShortcutsSection.tsx +++ b/frontend/src/views/KeyboardShortcutsSection.tsx @@ -60,6 +60,7 @@ export default function KeyboardShortcutsSection() { label={command.label} binding={effectiveBinding(command, keybindingOverrides)} isOverridden={command.id in keybindingOverrides} + extraBindings={command.extraBindings} /> ))} @@ -69,11 +70,12 @@ export default function KeyboardShortcutsSection() { ) } -function KeymapRow({ commandId, label, binding, isOverridden }: { +function KeymapRow({ commandId, label, binding, isOverridden, extraBindings }: { commandId: string label: string binding: KeyCombo | null isOverridden: boolean + extraBindings?: KeyCombo[] }) { const { t } = useTranslation('views') const hk = useCommandKeybindingCapture(commandId) @@ -112,6 +114,23 @@ function KeymapRow({ commandId, label, binding, isOverridden }: { {t('keyboardShortcutsSection.reset')} )} + {/* Multi-binding aliases (docs/goals/BACKLOG.md Standing #6): + read-only this pass -- Command.extraBindings' own doc + comment (shared/commands.ts) covers why these aren't + wired into the press-to-capture recorder above. */} + {extraBindings && extraBindings.length > 0 && !hk.recording && ( + + {t('keyboardShortcutsSection.alsoLabel')} + {extraBindings.map((extra) => ( + + ))} + + )} {hk.error && {hk.error}}