feat(extensions): add session-scoped keyboard modes - #708
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Greptile SummaryThis PR adds extension API v4 session-scoped keyboard modes, including registration, guarded lifecycle management, key routing, host-owned recovery paths, documentation, and a Vim navigation example.
Confidence Score: 4/5The menu-accelerator routing defect should be fixed before merging because an active extension mode can suppress advertised host menu commands. Session modes are inserted between menu fallthrough and app-command dispatch, so a mode that handles an advertised accelerator prevents the host command from executing; the remaining accepted feedback is formatting-related. Files Needing Attention: src/ui/hooks/useAppKeyboardShortcuts.ts, examples/extensions/vim-navigation/index.ts, examples/extensions/vim-navigation/state.ts Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
K[Terminal key] --> M{Host modal or menu navigation claims key?}
M -->|yes| C[Consume or route to focused surface]
M -->|menu accelerator: no| F{Focused input claims key?}
F -->|yes| I[Deliver to input]
F -->|no| V{File-view mode active?}
V -->|handled or exit| C
V -->|pass or inactive| S{Session mode active?}
S -->|handled or exit| C
S -->|pass or inactive| A[Dispatch app command]
A --> X[Execute accelerator and close menu]
Prompt To Fix All With AI### Issue 1
src/ui/hooks/useAppKeyboardShortcuts.ts:555
**Menu accelerators reach session modes**
When a session keyboard mode is active and a dropdown is open, accelerator keys that `handleMenuShortcut` deliberately passes onward reach the session mode before `dispatchAppCommand`. A mode that handles `q`, `r`, `/`, or another advertised accelerator suppresses the host command and leaves the menu open.
### Issue 2
examples/extensions/vim-navigation/index.ts:1-2
**New files violate TypeScript style**
The new Vim example and keyboard-mode implementation use two-space indentation and double-quoted strings throughout, conflicting with the repository requirement for four-space indentation and single quotes and establishing an inconsistent pattern for future extension examples.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "feat(examples): add Vim command-line nav..." | Re-trigger Greptile |
| handleMenuShortcut, | ||
| handleFocusedInputShortcut, | ||
| handleFileViewModeShortcut, | ||
| handleKeyboardModeShortcut, |
There was a problem hiding this comment.
Menu accelerators reach session modes
When a session keyboard mode is active and a dropdown is open, accelerator keys that handleMenuShortcut deliberately passes onward reach the session mode before dispatchAppCommand. A mode that handles q, r, /, or another advertised accelerator suppresses the host command and leaves the menu open.
Knowledge Base Used:
- UI App Shell: AppHost, App, and Controller Hooks
- Extension System: API, Loading, and Host Integration
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/ui/hooks/useAppKeyboardShortcuts.ts
Line: 555
Comment:
**Menu accelerators reach session modes**
When a session keyboard mode is active and a dropdown is open, accelerator keys that `handleMenuShortcut` deliberately passes onward reach the session mode before `dispatchAppCommand`. A mode that handles `q`, `r`, `/`, or another advertised accelerator suppresses the host command and leaves the menu open.
**Knowledge Base Used:**
- [UI App Shell: AppHost, App, and Controller Hooks](https://app.greptile.com/modem/-/custom-context/knowledge-base/modem-dev/hunk/-/docs/ui-app-host.md)
- [Extension System: API, Loading, and Host Integration](https://app.greptile.com/modem/-/custom-context/knowledge-base/modem-dev/hunk/-/docs/extension-system.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Fixed in ec38dedb. When an open menu declines a key, an enabled command accelerator now dispatches before focused inputs, file-view modes, and session keyboard modes can claim it. Added an AppHost regression with both extension mode layers handling ?, verifying that Help still opens and the menu closes.
Responded by Pi using openai-codex/gpt-5.6-sol.
This comment was generated by Pi using gpt-5.6-sol
| import type { HunkExtensionAPI } from "hunkdiff/extension"; | ||
| import { createVimNavigationState, executeVimCommand } from "./state"; |
There was a problem hiding this comment.
New files violate TypeScript style
The new Vim example and keyboard-mode implementation use two-space indentation and double-quoted strings throughout, conflicting with the repository requirement for four-space indentation and single quotes and establishing an inconsistent pattern for future extension examples.
Context Used: guidelines.mdc Cursor rule (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: examples/extensions/vim-navigation/index.ts
Line: 1-2
Comment:
**New files violate TypeScript style**
The new Vim example and keyboard-mode implementation use two-space indentation and double-quoted strings throughout, conflicting with the repository requirement for four-space indentation and single quotes and establishing an inconsistent pattern for future extension examples.
**Context Used:** guidelines.mdc Cursor rule ([source](https://github.com/modem-dev/modem/blob/main/.cursor/rules/guidelines.mdc))
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
No change here. Hunk's repository-local AGENTS.md makes oxfmt authoritative, and this code matches the existing two-space/double-quote format enforced by bun run format:check. The cited style rule belongs to the separate modem repository rather than modem-dev/hunk.
Responded by Pi using openai-codex/gpt-5.6-sol.
This comment was generated by Pi using gpt-5.6-sol
There was a problem hiding this comment.
You're right — I misattributed a style rule from the modem repository to modem-dev/hunk. Since oxfmt is the authoritative formatter here and bun run format:check passes, the two-space / double-quote style is correct and my comment was wrong.
Shall I remember this as a rule: "In modem-dev/hunk, treat oxfmt (enforced via bun run format:check) as the authoritative formatter — do not flag two-space indentation or double-quoted strings as style violations"?
Summary
handled/pass/exitdecisions without renderer or viewport internals:command lineTesting
bun run format:checkbun run typecheckbun run lintbun run check:docsbun run check:packbun run test— 2,047 passed, 9 skippedbun run test:integration— 102 passedbun run test:tty-smoke— 9 passedbun run changeset:statusgit diff --checkThis PR description was generated by Pi using gpt-5.6-sol