Extract app keyboard shortcuts, fix reverse paging, and deflake CI

[benvinegar]

Why

• the shortcut logic in src/ui/App.tsx had grown into a large useKeyboard(...) block that was hard to change safely
• this keeps scope precedence explicit in one dedicated hook while rolling in the real fixes and test stabilization that fell out of the refactor

Summary

• extract scoped keyboard handling into src/ui/hooks/useAppKeyboardShortcuts.ts
• add shared key alias matchers in src/ui/lib/keyboard.ts
• keep precedence explicit for pager, help, open menu, filter focus, and normal app shortcuts
• restore less-style Shift+Space reverse paging by making it reachable again
• keep pinned file headers in sync when hunk navigation jumps across files after the main rebase
• deflake shortcut interaction coverage and the MCP e2e port-conflict test

Testing

• bun run typecheck
• bun test
• bun run test:integration
• bun run test:tty-smoke

[greptile-apps]

Greptile Summary

This PR extracts the large inline useKeyboard(...) block from App.tsx into a dedicated useAppKeyboardShortcuts hook, and moves the repeated key-alias predicates into a shared src/ui/lib/keyboard.ts utility module backed by new unit tests. The refactoring is clean and behaviorally equivalent to the original — mode precedence (pager → help → menu → filter → default) is faithfully preserved, the new callbacks are well-scoped, and the public utility functions are well-documented.

Key observations:

• isShiftSpacePageUpKey is unreachable in both handlePagerShortcut and handleAppShortcut: isPageDownKey includes the bare isSpaceKey check (no shift guard) and is always evaluated first, so Shift+Space is always consumed as page-down. This was present in the original code but is more visible now that both helpers are named exports with JSDoc.
• Test coverage for isPageDownKey omits the name: "f" alias (only sequence: "f" is exercised).

Confidence Score: 5/5

Safe to merge — the refactoring is behaviourally equivalent to the original and all remaining findings are P2 style/cleanup items.

No new logic errors are introduced. The only flagged issue (Shift+Space being shadowed by isPageDownKey) is a pre-existing condition faithfully preserved by the refactor. All other observations are test coverage gaps or naming clarity suggestions, none of which affect runtime behaviour.

No files require special attention; the unreachable isShiftSpacePageUpKey path in useAppKeyboardShortcuts.ts is worth a follow-up but does not block merge.

Important Files Changed

Filename	Overview
src/ui/hooks/useAppKeyboardShortcuts.ts	New hook containing all extracted keyboard handling. Logic is structurally sound and scope ordering is correct, but isShiftSpacePageUpKey is shadowed by the earlier isPageDownKey check in both handlePagerShortcut and handleAppShortcut.
src/ui/lib/keyboard.ts	Clean utility module for key-alias predicates; all functions are well-documented. isSpaceKey is intentionally unexported (internal helper).
src/ui/App.tsx	Keyboard handling cleanly removed and delegated to useAppKeyboardShortcuts; new memoized callbacks correctly extracted and added to buildAppMenus dep arrays.
test/ui-lib.test.ts	New createKeyEvent factory and keyboard-alias tests cover positive and negative cases for all exported helpers; name: "f" alias for isPageDownKey is not explicitly exercised.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    K([KeyEvent]) --> MT{f10 pressed?}
    MT -- yes, consumed --> DONE([return])
    MT -- no --> PM{pagerMode?}
    PM -- yes --> PS["handlePagerShortcut\nq/esc → quit\nspace/f/pgdn → scrollDown\nb/pgup → scrollUp\nd → halfDown\nu → halfUp\nj/down → stepDown\nk/up → stepUp\nhome/end → scrollContent\nw → lineWrap"]
    PS --> DONE
    PM -- no --> HS{"showHelp and esc?"}
    HS -- yes --> CH[closeHelp] --> DONE
    HS -- no --> MS{"activeMenuId set?"}
    MS -- handled --> DONE
    MS -- not handled --> FS{"focusArea = filter?"}
    FS -- handled --> DONE
    FS -- no --> AS["handleAppShortcut\nq/esc → quit\n? → toggleHelp\ntab → toggleFocus\n/ → focusFilter\nscroll keys\n0/1/2 → layout\ns/r/t/a/l/w/m → toggles\nhunk navigation"]
    AS --> DONE

Loading

_{Reviews (1): Last reviewed commit: "refactor: extract app keyboard shortcuts..." | Re-trigger Greptile}

[greptile-apps]

isShiftSpacePageUpKey is unreachable

isPageDownKey includes the unguarded isSpaceKey helper, which matches any Space key event regardless of the shift modifier. Because isPageDownKey is checked first (line 111), a Shift+Space key press is always consumed as a page-down, and the isShiftSpacePageUpKey branch (line 116) is never reached. The same ordering issue applies to handleAppShortcut at lines 254–261.

This was present in the original code, but naming isShiftSpacePageUpKey as a documented, exported function with a JSDoc comment makes the dead-code more visible.

Two reasonable resolutions:

• If the less-style Shift+Space reverse-page behaviour is intended to work, isPageDownKey should guard its space check with !key.shift so that shifted Space falls through to the page-up branch.
• If the feature is not intended, the isShiftSpacePageUpKey export and its call sites can be removed to avoid the misleading impression that it is active.

[benvinegar]

Good catch. I updated isPageDownKey to ignore shifted Space so Shift+Space now reaches the reverse-page path again, and added regression coverage for both the shared key matcher and the app interaction path.

_{Responded by pi-coding-agent using unknown-model.}

[greptile-apps]

Missing alias coverage in isPageDownKey / isPageUpKey tests

The test exercises { sequence: "f" } for isPageDownKey but never { name: "f" }, and { sequence: "b" } for isPageUpKey but never { name: "b" }. Since the implementations treat key.name and key.sequence as separate branches, the name-based paths go untested. Consider adding:

expect(isPageDownKey(createKeyEvent({ name: "f" }))).toBe(true);
expect(isPageUpKey(createKeyEvent({ name: "b" }))).toBe(true);

[benvinegar]

Added the missing key.name === "f" and key.name === "b" assertions while expanding the keyboard alias coverage.

_{Responded by pi-coding-agent using unknown-model.}