fix(shortcuts): keep board keys behind an open modal and tighten the shell key guards

[Chris0Jeky]

Summary

With the shell keyboard-help dialog open over a Legacy board, a plain f or n still reached the
board. AppShell.vue returned without stopping propagation when a keyboard-owning surface was
active and no app-shell binding matched, so the keydown bubbled to BoardView's
useKeyboardShortcuts window listener: f toggled the filter panel behind the modal and n
clicked the column's [data-action="toggle-add-card"] button and pulled focus out of the open
dialog.

This takes the shell-side fix rather than widening boardShortcutsEnabled. The board gate would
have to learn about the shell's help state through the useShellKeyboardHelp seam and would still
leave every other page-level window listener exposed to the same leak; the shell already computes
the surface state on the keydown path, so one guard there covers the whole class. It also stays out
of PaperBoardView, which the board-side gate would have had to reach.

The same handler carried four keyboard-guard LOWs and a MEDIUM recorded on #1968 on 2026-08-29, and
they are fixed here because they live in the lines this change already rewrites.

Closes #2621
Refs #1968, #2636

Changes

test(shortcuts): pin the shell keyboard guard defects red - the failing specs first, in
AppShell.spec.ts. A probe stands in for the routed board: it installs the real
useKeyboardShortcuts with BoardView's own f and n bindings and the same add-card DOM contract
createCardInSelectedColumn drives, so the routing under test is the shipped mechanism. Keys are
dispatched from document.body with bubbles: true, because a window-dispatched event is
AT_TARGET, where capture and bubble listeners both run whatever propagation says.

fix(shortcuts): keep board keys behind an open modal and tighten the shell guards

• AppShell stops a keydown that no app-shell binding consumed while a keyboard-owning surface is
  active. Two carve-outs: Escape is never stopped, because board dialogs the escape stack does not
  carry are closed by BoardView.closeOpenUi on the bubble and stopping it would strand them open;
  and a target inside the surface is left alone, because this listener runs in the capture phase
  ahead of every handler the surface owns.
• Each non-navigate action now says which surface owns its key, marked with data-shell-surface on
  the two help twins and the two command palettes. ? toggles the help dialog it owns and does
  nothing over another modal, mod+k still closes the palette but no longer opens over one, and
  quick capture never stacks a second modal. navigate and quick-capture name no surface, so an
  active surface always wins over them.
• strokeMatches moves into the shared ledger module and gains two rules. An undeclared shift
  now requires Shift to be up over a letter, so Shift+H stops navigating Home. A layout-produced
  character ignores Alt, and ignores Ctrl while Alt is also down, so ? stays reachable on layouts
  that need AltGr without loosening any ordinary Ctrl or Alt combination.
• The surface scan is lazy per event, so a keystroke typed into a field no longer pays for the
  querySelectorAll plus getComputedStyle sweep.

test(shortcuts): assert the advertised strokes instead of the AppShell source text and
test(shortcuts): pin the surface scan out of the typing path - replace the
toContain('APP_SHELL_SHORTCUT_BINDINGS.find') scan, and assert the sweep is not reached for an
ordinary keystroke in a field while mod+k, the one binding allowed inside text entry, still gets
it.

fix(shortcuts): require modality before a surface owns the keyboard (round 2)

• The ownership selector now requires modality. It listed a bare [role="dialog"], and
  CardModal keeps that role in both presentations while setting aria-modal only outside the
  inspector. The Paper desktop card inspector is a sticky side panel that traps nothing, so it
  counted as a keyboard-owning surface: with the new gate, ?, mod+k and mod+shift+c went dead
  for as long as a card was open for reading, and every non-Escape key pressed outside the 420 px
  panel was stopped. The selector is now
  dialog[open], [role="alertdialog"], [aria-modal="true"]. This is the 2026-08-29 [Frontend][UX] Documented bare-letter shortcuts (H/T/B/I/R, G T, board C/R) have no handler, and Settings has no Keyboard page #1968 MEDIUM on
  the same selector. Narrowing also un-gates navigate over a non-modal dialog, which is the same
  correction; PaperBoardView guards dirty navigation with onBeforeRouteLeave, so an unsaved
  inspector edit still raises its discard dialog rather than being lost.
• Ownership no longer needs every surface to be the action's own. Two open shell surfaces left
  neither ? nor mod+k able to close its own surface. It now asks that this action's surface is
  present and that no surface outside the shell is. Stack order is deliberately not used: a
  <Teleport> places its anchor when the shell mounts, not when the surface opens, so document
  order is AppShell's template order whatever the user opened first.
• Both matcher loops were tautologies - they built the event from the stroke's own fields, so
  they were true for any stroke. The util spec now drives the matcher from the DESCRIPTOR, the
  string the help surfaces print, and asserts descriptor and sequence agree. The overlay spec
  couples each rendered chip to formatShortcut of its ledger row. AppShell.spec.ts adds a
  behavioural table over APP_SHELL_SHORTCUT_BINDINGS that presses each advertised stroke through
  the mounted shell and asserts the ledger action runs.
• The escape-stack spec built its stack with keys the gate now refuses and passed only because
  an unattached mount hides every surface from document.querySelectorAll (Vue Test Utils only
  puts the component in the document when attachTo is given). It now stacks by mouse and mounts
  attached; the two other specs that press keys over an open surface attach too, with the reason
  named on mountShell.

The keystroke ledger is untouched: no row added or removed, and no change to the review keymap,
BoardToolbar or PaperReviewView.

#1968 items from the 2026-08-29 comments

Item	Disposition
MEDIUM: modal-ownership selector matches any [role=dialog]	Closed (round 2)
? and the command palette fire over modals	Closed
AltGr ? unreachable	Closed
Per-keystroke DOM scan	Closed for the typing path; a non-text-entry keystroke still scans once
Unconstrained Shift on bare letters	Closed
Source-text assertion in PaperShortcutsOverlay.spec.ts	Closed

Left open: the notation guard still misses Ctrl + (spaced) and Control+. HARDCODED_MODIFIER in
shortcutNotation.spec.ts is still ⌘|Ctrl\/Cmd|Ctrl\+. Different file, different defect, not
touched here.

Test plan

Verified, from frontend/taskdeck-web:

• Round 1 red first: npx vitest --run --maxWorkers=2 src/tests/components/AppShell.spec.ts gave
  6 failed, 41 passed (47) - expected 1 to be +0 for the filter toggle running behind the open
  dialog; the command palette and quick capture opening over an active modal; ? toggling over
  another modal; expected "vi.fn()" to not be called at all, but actually been called 2 times
  (/workspace/home, /workspace/today) for Shift+H and Shift+T; and ? with altKey never
  opening the map. The scan pin was proved separately against origin/main's AppShell.vue:
  expected [ …(2) ] to have a length of +0 but got 2.
• Round 2 red first, the same spec file run against the previously pushed head
  (07ca649502bbae0f61c5cbdb817b1a47bfbc8052): 2 failed, 57 passed (59).
  keeps every shell key live while a non-modal card inspector is open failed with
  AssertionError: expected null not to be null - ? was dead with the inspector open.
  lets each surface of a shell stack close itself with its own key failed with
  AssertionError: expected true to be false - mod+k could not close the palette.
• After the fix, the named suite plus every spec that mounts AppShell, drives a palette, or mounts
  the card inspector: npx vitest --run --maxWorkers=2 over AppShell.spec.ts,
  keyboardShortcuts.spec.ts, PaperShortcutsOverlay.spec.ts, ShellKeyboardHelp.spec.ts,
  shortcutLedgerTruth.spec.ts, shortcutNotation.spec.ts, BoardView.spec.ts,
  BoardView.keyboardRouting.spec.ts, BoardView.coverage.spec.ts,
  useKeyboardShortcuts.spec.ts, AppShell.paperVariant.spec.ts, ShellCommandPalette.spec.ts,
  PaperCommandPalette.spec.ts, useShellKeyboardHelp.spec.ts, paper/PaperBoardView.spec.ts,
  CardModal.spec.ts, useCardModal.spec.ts and paper/PaperBoardManagement.spec.ts gave
  18 files passed, 391 tests passed.
• Whole unit suite, because the change sits on the global keydown path:
  npx vitest --run --maxWorkers=2 gave 371 files passed, 5753 passed and 3 skipped (5756),
  exit 0.
• npm run typecheck clean. npm run build succeeded. npx eslint clean on every changed file.
  git diff --check clean.

NOT verified:

• Playwright. tests/e2e/keyboard-navigation.spec.ts needs a running stack and was not run, so no
  real-browser evidence that a physical AltGr ? or Shift+H behaves as the matcher now says. The
  AltGr and Shift rules are proved only against synthesized KeyboardEvent flags.
• The card inspector is modelled by a role="dialog" probe without aria-modal, matching
  CardModal.vue's attributes, not by mounting PaperBoardView at desktop width inside AppShell.
• The AC1 spec uses the composable-level board probe, not a mounted BoardView inside AppShell. The
  probe installs the real useKeyboardShortcuts with BoardView's own bindings, but it omits the
  enabled predicates BoardView passes. Both predicates are true in the state exercised here.
• No manual browser pass on either skin.

Boundaries and risks

• Known residual, tracked as [Frontend][Shortcuts] Keys pressed with focus inside a shell surface still reach page-level listeners #2636. The guard runs in the capture phase, so it deliberately does
  not touch an event whose target is already inside the active surface. Concretely: open quick
  capture with mod+shift+c, Tab to a button inside the CaptureModal, press n - the key bubbles
  past the shell to the board's window listener and clicks the add-card button on the column behind
  the modal. Both real paths into the help dialog leave focus outside it, and neither help twin
  traps or moves focus, so this needs a deliberate Tab. Closing it would need a second listener on
  document in the bubble phase, which would also silence PaperHomeView and PaperInboxView
  window listeners under any modal - wider than this issue authorizes.
• Latent, not a defect today. isLayoutProducedCharacter treats any single non-alphanumeric
  character as layout-produced, which includes Space. A Space stroke would therefore ignore Alt and
  the AltGr Ctrl. No binding in the ledger uses Space, so nothing exercises it; a future Space
  binding should declare its modifiers or the predicate should exclude Space.
• Escape is exempt by design. BoardView.closeOpenUi keeps closing the label manager, board
  settings, filter panel and column form.
• Stopping propagation for a target outside the surface also keeps the event from reaching that
  background element's own handlers. Text-entry targets return before the guard, so typing into a
  background field is untouched.
• data-shell-surface is now load-bearing: a new shell surface that toggles from a shortcut must
  carry it, or its key will be blocked while the surface is open. A surface WITHOUT the marker is
  treated as foreign and takes every shell key, which is the intended default.
• The AltGr rule applies only to a stroke whose key is a single non-alphanumeric character with no
  declared mod or alt. ? is the only such stroke in the ledger today. Ctrl-only and
  Command-only chords over ? are still refused, and every letter stroke keeps exact Alt and mod
  matching.
• No gitignored files need to survive the worktree; git status --porcelain --ignored shows only
  frontend/taskdeck-web/node_modules/ and dist/ from the build, both reproducible with
  npm ci and npm run build.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

[Chris0Jeky]

Review record (alpha product-trust lane, review-and-ship round 1 at head 07ca649).

Reviewer: one fresh-context independent reviewer subagent (read-only), input = merge-base..head diff plus the worktree at the head. Verdict: FIX-FIRST, one HIGH.

• HIGH, fix-now (round 2): KEYBOARD_OWNING_SURFACE_SELECTOR lists [role="dialog"] on its own, so the Paper desktop card inspector (role="dialog" without aria-modal, a sticky non-focus-trapping side panel on the default skin) counts as a keyboard-owning surface; with the new gate ?, mod+k and mod+shift+c go dead whenever a card is open in the inspector (only navigate was gated at the merge base), and the page-shortcut guard stops every non-Escape key outside the panel while a card is merely being read. This is the 2026-08-29 [Frontend][UX] Documented bare-letter shortcuts (H/T/B/I/R, G T, board C/R) have no handler, and Settings has no Keyboard page #1968 MEDIUM about the selector matching any role=dialog. Round 2 narrows the selector to open native dialogs, alertdialogs and ARIA dialogs explicitly marked modal, with a red-first spec mounting a non-modal role=dialog probe.
• MEDIUM, taken in round 2: the two "matcher accepts the advertised stroke" loops are tautologies (the event is built from the stroke's own fields, so no ledger row can redden them) and they replaced a coupling assertion; replaced with assertions that can fail.
• MEDIUM, taken in round 2: the "closes only the top-most escape surface first" spec expects the palette to open over the help dialog, which the gate now forbids, and passes only because the mount is detached (no attachTo, so the surface scan finds nothing); the same detachment defangs the gate in three other specs. The stacked state is driven by mouse with an attached mount.
• MEDIUM, tracked as [Frontend][Shortcuts] Keys pressed with focus inside a shell surface still reach page-level listeners #2636 (pre-existing class, incomplete fix acknowledged): with focus inside a modal (Tab off the CaptureModal textarea onto a button), n or f bubble past the guard to the board keymap and pull focus out of the dialog.
• LOW, taken in round 2: surfacesOwnAction uses every, so two open shell surfaces make both their keys unable to close their own surface; topmost wins.
• LOW, noted: Space is classified as a layout-produced character (latent, no binding uses it).
• LOW: the STATUS line is the coordinator's (thirteenth block).

Confirmed clean by the reviewer: per-event memoisation is a per-call let with ??=, no cross-event staleness; the chord rewrite is behaviour-preserving; Escape is exempted before anything else so useEscapeStack and the board's close paths are untouched; text-entry targets outside a surface are never stopped and stopPropagation cancels no default action (Tab, native button activation, browser chords survive); every data-shell-surface marker sits on the element that carries the dialog role; the Shift rule covers exactly the bare-letter set and leaves quick capture's declared shift and the US-layout Shift+/ for ? intact; the AltGr rule cannot widen letter bindings (Ctrl+Alt+H does not navigate) and mod+k still requires exactly one of Ctrl or Meta; the diff is exactly the nine claimed files.

Merge gate: round-2 push, one verification pass scoped to the fix diff (the selector and the spec harness change the risk boundary), ci-required green at the round-2 head, aged three minutes, then merge commit.

[Chris0Jeky]

Round-2 verification record (scoped to the fix diff 07ca649..8eb4010; read-only pass). Base updated afterwards onto 9eb6f40 (#2631 touched AppShell.vue): the merged head 019f932 was re-proved locally (14 files / 348 across the shell, shortcut, board, card and capture-store specs) and hosted CI runs at that head.

Verdict: SHIP for the fix diff, no CRITICAL, HIGH or MEDIUM. Confirmed: the final selector is dialog[open], [role="alertdialog"], [aria-modal="true"]; every role="dialog" in src was enumerated with its aria-modal state and only the Paper desktop card inspector (aria-modal absent when presentation is inspector, passed only by PaperBoardView on desktop widths) falls outside it; tablet and phone Paper and every Legacy CardModal use stay guarded; no other non-modal dialog and no native dialog element exist; the newly un-gated navigation keys over a dirty inspector hit PaperBoardView's route-leave guard, so no unsaved-work path opens; single-surface semantics are byte-identical to round 1; with help and palette both open each key closes its own surface, a foreign modal over help blocks ? and Escape is untouched; the replaced matcher assertions are falsifiable (rendered chip text versus formatShortcut of the ledger descriptor; descriptor-derived strokes versus the declared sequence) and the ledger-to-shell coupling survives a removed row; every spec that opens a surface and then presses a key mounts attached, teardown is the file's explicit unmount, and the escape-stack ordering spec still proves its original property; the diff is four files at the stated seams and keyboardShortcuts.ts is untouched.

Three LOWs recorded, no fix cycle: dialog[open] would also match a non-modal native dialog shown with show() (none exists in src; dialog:modal is the precise form if one ever does); the ownership rule is order-independent rather than topmost-wins, so with the palette clicked open over the help dialog ? closes the background help (cosmetic; DOM order cannot express stack order because the Teleport anchors sit in template order, and useEscapeStack would be the readable source if topmost is ever wanted); the new ledger walk has no default arm for a future action type (mitigated by the truth guard requiring a case arm per row).

Merge gate unchanged: ci-required green at 019f932, aged, merge commit.