fix: derive slash command routing from session capabilities#7
Conversation
…f hardcoded list Replace hardcoded NATIVE_COMMANDS set with getBackendCommands() that reads capabilities/slash_commands from SessionState using three-tier fallback (capabilities.commands → slash_commands → empty set). Emulatable commands always win over native forwarding.
…guments Addresses review findings: test empty capabilities.commands array fallback to slash_commands, and verify commandName() extracts the first token correctly when arguments are present.
Extract repeated command.split(/\s+/)[0] into commandName() helper. Simplify emulator functions to use array literals instead of push-then-join. Inline PTY runner callback to remove redundant async wrapper.
Summary of ChangesHello @teng-lin, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly refactors how slash commands are routed and handled, moving from a static, hardcoded list to a dynamic system that leverages session capabilities. This change improves the flexibility and responsiveness of command execution by prioritizing local emulation for certain commands and providing more accurate information about available commands based on the connected backend's reported capabilities. The update enhances the user experience by making command discovery more intelligent and adaptable to different session states. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request refactors the slash command handling to derive command routing dynamically from session capabilities, removing the hardcoded NATIVE_COMMANDS set. The changes are well-implemented and thoroughly tested. The logic for command discovery with its three-tier fallback is sound. I've added a couple of suggestions to improve the user experience of the /help command by sorting the command list alphabetically for consistency.
| const allNames = [ | ||
| ...NATIVE_COMMANDS, | ||
| ...Object.keys(EMULATABLE_COMMANDS).filter((name) => name !== "/help"), | ||
| ...backendNames, | ||
| ...Object.keys(EMULATABLE_COMMANDS).filter( | ||
| (name) => name !== "/help" && !backendNames.has(name), | ||
| ), | ||
| ]; |
There was a problem hiding this comment.
To ensure a consistent and predictable order for the user, it's a good idea to sort the list of command names alphabetically.
| const allNames = [ | |
| ...NATIVE_COMMANDS, | |
| ...Object.keys(EMULATABLE_COMMANDS).filter((name) => name !== "/help"), | |
| ...backendNames, | |
| ...Object.keys(EMULATABLE_COMMANDS).filter( | |
| (name) => name !== "/help" && !backendNames.has(name), | |
| ), | |
| ]; | |
| const allNames = [ | |
| ...backendNames, | |
| ...Object.keys(EMULATABLE_COMMANDS).filter( | |
| (name) => name !== "/help" && !backendNames.has(name), | |
| ), | |
| ].sort(); |
There was a problem hiding this comment.
Good catch — applied .sort() in 927f9ca on main. The /help fallback now lists commands alphabetically.
Address Gemini Code Assist review comment on PR #7 — sort the merged backend + emulatable command names for consistent ordering.
Fix architecture principle violations #1, #3, #4, #7: - Fix #1: Remove dual lifecycle mutation by deleting applyLifecycleFromBackendMessage() which was redundant with reduceLifecycle() - Fix #3: Make transitionLifecycle() private. Add SESSION_CLOSING system signal and route coordinator's direct calls through process() - Fix #7: Route ConsumerGateway's backend:relaunch_needed emission through the runtime as BACKEND_RELAUNCH_NEEDED system signal instead of emitting directly - Fix #4: Route BackendConnector's inline broadcasts and event emissions through the runtime via routeSystemSignal callback: - Part A: Connect/disconnect lifecycle (cli_connected, backend:connected, etc.) - Part B: Slash passthrough results/errors (slash_command_result, slash_command_error) This enforces the core design principle that all state mutations flow through SessionRuntime.process(), which dispatches to the pure reducer and executes effects. All 2902 tests pass, typecheck clean. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Summary
NATIVE_COMMANDSset withgetBackendCommands()that readscapabilities.commands/slash_commandsfromSessionStateusing three-tier fallback/help,/model,/status,/config,/cost,/context) always win over native forwarding — instant local results, no round-trip/helpnow shows rich capability descriptions when available, falls back to command names fromslash_commands, and shows only emulatable commands before CLI connectsTest plan
npm test— all 1308 existing + 31 executor tests passnpx tsc --noEmit— type check clean/help— shows capabilities descriptions