feature(commands) - Refactor Slash Command + Vision For the Future #3175
Conversation
There was a problem hiding this comment.
Summary of Changes
Hello @abhipatel12, 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 initiates a major architectural overhaul of the CLI's slash command system, moving towards a more modular, extensible, and testable design. It lays the groundwork for future features like user-defined commands and a more powerful automation platform, while ensuring backward compatibility for existing commands during the transition.
Highlights
- Architectural Refactor: The core slash command system has been fundamentally refactored. The monolithic
useSlashCommandProcessoris being replaced by a newCommandServicethat manages a scalable, extensible, and testable nested command tree. - New Command System Components: Introduced a new
CommandService(packages/cli/src/services/CommandService.ts) to centralize command management, aSlashCommandinterface (packages/cli/src/ui/commands/types.ts) supporting nested commands, and aCommandContext(packages/cli/src/ui/commands/types.ts) for dependency injection into command actions. - Command Migration: The
/clear,/help, and the entire/memorycommand suite (including itsshow,add, andrefreshsubcommands) have been migrated to the new modular, tree-based structure. - Legacy Compatibility: A crucial legacy adapter has been implemented within
useSlashCommandProcessor(packages/cli/src/ui/hooks/slashCommandProcessor.ts). It prioritizes execution of commands from the newCommandServicebut falls back to the old system for unmigrated commands, ensuring a smooth, incremental transition. - Enhanced Autocompletion: The
useCompletionhook (packages/cli/src/ui/hooks/useCompletion.ts) andInputPromptcomponent (packages/cli/src/ui/components/InputPrompt.tsx) have been significantly refactored to provide intelligent, context-aware autocompletion for nested commands and their arguments. - Improved Testability: New unit tests have been added for the
CommandServiceand all migrated commands. AcreateMockCommandContextutility (packages/cli/src/test-utils/mockCommandContext.ts) was introduced to simplify testing of command actions.
Using Gemini Code Assist
The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and 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 to provide feedback.
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
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-20.x' artifact from the main CI run. |
|
This sounds great and I'm looking forward to reading through the code when I'm back at a computer. One idea while I was reading your description is that you might want to enable tools to define their own commands from the tool definition. You mentioned /memory in your writeup and I wonder if it would be even cleaner to allow the memory tool to define its commands in the tool definition? |
Ohh I like that! I'll have to take a look when I'm back at my computer |
NTaylorMullen
left a comment
There was a problem hiding this comment.
Super exciting changes. There's so much here that's going to make our lives significantly easier with new slash command pieces coming up. Left a good amount of comments around API state. Let me know what you think!
| altName: '?', | ||
| description: 'for help on gemini-cli', | ||
| action: (context, _args) => { | ||
| context.utils.onDebugMessage('Opening help.'); |
There was a problem hiding this comment.
I forget, can't we replace all the onDebugMessage bits with console.debug(...)?
There was a problem hiding this comment.
Yes we def can!
When I was testing this, the difference is how it appears for the user. If the CLI is started in debug mode, the onDebugMessage surfaces in the footer while the console.debug(...) appears within the console.
I think it depends on which UX we prefer. I added a log statement and then ran /clear. The resulting UI is:
Do you have a UX preference here? I can make the switch pretty easily.
There was a problem hiding this comment.
Hmm, I actually think the onDebugMessage is a bit misleading and not worth. I'd argue we should do console.debug only.
f4606da to
392f41a
Compare
|
@NTaylorMullen Thanks for all the great feedback on that last round! I made some pretty significant shifts here so let me know what you think! |
@allenhutchison I wanted to follow up on this idea! It enables The architecture proposed in this PR supports this idea and I want to propose pursuing this in a fast-follow. An example implementation would be as follows: Possible Impl Plan1. Enhance the Tool Interface:We can introduce an optional method, export interface Tool {
// ... existing properties
getSlashCommands?(context: CommandContext): SlashCommand[];
}2.
|
NTaylorMullen
left a comment
There was a problem hiding this comment.
Love the refactor and where you went with it!
| altName: '?', | ||
| description: 'for help on gemini-cli', | ||
| action: (context, _args) => { | ||
| context.utils.onDebugMessage('Opening help.'); |
There was a problem hiding this comment.
Hmm, I actually think the onDebugMessage is a bit misleading and not worth. I'd argue we should do console.debug only.
| type: MessageType.INFO, | ||
| text: 'Refreshing memory from source files...', | ||
| }, | ||
| Date.now(), |
There was a problem hiding this comment.
Do we ever not do Date.now()? Wondering if this can be automatic in addItem
There was a problem hiding this comment.
Originally we had two of these addItems, and we consolidated to a single one. Data.now() is being used as the id here. I agree that it's a bit repetitive, but I believe the best change may be to make this id field optional and have a default id that uses Date.now(). But i think that's best addressed in a follow-up PR if that sounds good to you!
| export interface CommandContext { | ||
| // Core services and configuration | ||
| services: { | ||
| config: Config | null; |
There was a problem hiding this comment.
What scenario is this null in?
There was a problem hiding this comment.
I don't believe it should ever be. This was the type passed in defensively when I made the context in the event that it could be possible, but I agree it shouldn't really be possible.
Since the solution involves modifying the app's startup logic in gemini.tsx to disable input until the config is loaded, it's a non-trivial architectural change if we want to handle this safely. I added a TODO and to have a fast follow to remove this extra check.
| * The return type for a command action that results in a simple message | ||
| * being displayed to the user. | ||
| */ | ||
| export interface MessageActionReturn { |
There was a problem hiding this comment.
Ah is this in place of say ui.addItem?
There was a problem hiding this comment.
Sort of. For multi-step actions, the command takes action through the context.ui.AddItem function. So for example, in /memory we use the context.ui.AddItem in order to provide updates saying "refresh memory", etc.
But when a slash command's job is to produce a single, final message it uses MessageActionReturn.
I added MessageActionReturn for a couple of reasons.
- It simplifies certain commands. This is especially true for error handling. We have a common error handling path currently, and making every command implement its own HistoryItem would be more work.
- It makes the contract stronger. Now that the return is a union, the slash Command Processor knows exactly what each command is doing and it doesn't have to rely on optional checks.
- tool --> needs to schedule a tool call
- message --> needs to display the basic message for the user (maybe it's an error)
- void --> command handled everything, so don't worry about doing anything.
| description?: string; | ||
|
|
||
| // The action to run. Optional for parent commands that only group sub-commands. | ||
| action?: ( |
There was a problem hiding this comment.
What commands don't have an action? Wondering if this can be required / not nullable
There was a problem hiding this comment.
Great question!
The action property is optional by design to support parent commands. In our nested tree structure (/command subcommand ...args), a command can be a "leaf" that performs an action, or it can be a "branch" that only groups other commands.
Example:
The top-level /memory command only exists to contain its subCommands (show, add, refresh). It doesn't have an action of its own. If a user types just /memory, the system should recognize that the action is undefined and displays a help message listing the available subcommands.
If we made the action property required, it would force us to add empty, placeholder actions to all parent commands, which would be more boilerplate.
|
My 2cents on exposing slash commands on tools: We could BUT I'm not sure how valuable it'd be. I could see a world in which we automatically translate some tools to slash commands maybe. Memory might be an exception to the common case here 😄 |
This commit introduces a new command registry architecture to decouple slash command logic from the `useSlashCommandProcessor` hook. The key changes are: - **Command Registry:** A new directory `packages/cli/src/ui/commands` now houses individual command modules and a central registry (`index.ts`). - **`CommandContext`:** A new `CommandContext` object is introduced to provide dependencies to commands via a single, structured object, breaking the tight coupling with the hooks props and state. - **Initial Migration:** The `/help` and `/clear` commands have been migrated to this new architecture as proof of concept. - **Coexistence Model:** The `useSlashCommandProcessor` hook has been adapted to support both the new command registry and the legacy, inline command definitions, allowing for a safe, incremental migration. - **Testing Strategy:** New, isolated tests have been added for the migrated commands, and the testing strategy for the command processor has been updated to mock the command registry. This change lays the foundation for migrating all remaining slash commands, which will improve scalability, testability, and maintainability.
This commit introduces the initial proof-of-concept for the CommandService, which is responsible for loading slash commands from various sources. Key changes: - Created the class in . - Implemented file-based discovery of custom commands from . - Added for parsing command definitions. - Integrated the into the hook to dynamically load and register custom commands. - Added a sample custom command for testing purposes. This lays the foundation for a fully extensible command system
This commit refactors the slash command infrastructure to simplify the CommandContext object and create a more explicit contract for command actions. This addresses PR feedback aimed at improving code clarity and making commands more self-contained.
Key changes include:
- **Simplified CommandContext**: The `actions` and `utils` properties have been removed. UI-related functions like `addItem`, `setDebugMessage`, and `clear` are now consolidated directly under the `ui` object. This reduces indirection and makes the context easier to mock for testing.
- **Explicit Command Action Contract**: Slash command actions no longer return a boolean or an implicit object. They now return a discriminated union:
- `{ type: 'tool', ... }` to schedule a tool call.
- `{ type: 'message', ... }` to display a simple message to the user.
The `useSlashCommandProcessor` is updated to handle this new, clearer contract.
- **Refactored Commands**:
- `/clear` and `/help` are updated to use the new `ui.setDebugMessage`.
- `/memory` subcommands are refactored to be more self-contained. For instance, `/memory refresh` now orchestrates its own async operation and UI updates for success or failure, calling the new `config.refreshMemory()` method.
- **Core Memory Refresh Logic**: A `refreshMemory()` method was added to the core `Config` class, centralizing the logic for reloading memory content from `GEMINI.md` files.
…or `CommandContext`
…. Provide guidelines for future migrations.
… final leaf nodes
f231eae to
ac1d4e8
Compare
Good point, something to consider if we want go forward with that plan! |
|
👏 |
TLDR
This PR implements a fundamental architectural refactor of the Gemini CLI's slash command system. It replaces the monolithic
useSlashCommandProcessorwith a scalable, extensible, and testableCommandServicethat manages a nested command tree.This is the foundational first step in a multi-phase effort to enable user-defined commands and a more powerful automation platform, in a more broad, community driven vision. This PR delivers the core service, the new nested command structure, and migrates the first set of commands (
/memory,/help,/clear) to the new system. It also includes a crucial legacy adapter to ensure all non-migrated commands remain fully functional during the transition.Dive Deeper
The Problem
The previous command system, centered in
useSlashCommandProcessor, was a single, massive hook that was difficult to maintain, test, and extend. Commands with sub-options (e.g.,/memory show) were handled with brittleswitchstatements, and there was no path to allow users to define their own commands—a key strategic feature.The Solution: A Service-Oriented, Nested Architecture
This refactor introduces a clean, modern architecture composed of three key pillars:
The Nested
SlashCommandInterface (types.ts): I've adopted a recursive data structure (inspired by @allenhutchison on Feature: Nested Slash Command Completion and Execution #2622 !!) where a command can containsubCommands. This allows us to model command hierarchies naturally (e.g.,/memoryis a parent toshow,add, andrefresh).The
CommandService(CommandService.ts): This new, central service is now the single source of truth for all commands.memoryCommand.ts). In the future, it will be extended to load commands from user-defined YAML files or any other format we choose to pursue!This service is the central hub responsible for the entire command lifecycle, abstracting the UI from the command source.
Architectural Flow Goals:
This architecture is the key enabler for the phased rollout plan.
The
CommandContextObject (types.ts): I've established a single context object that is passed to every command'saction. This acts as a dependency injection (DI) container, providing access to all necessary services (config,logger), UI actions (addItem,clear), and session data (stats) in a clean, explicit, and highly testable way. This eliminates the need for prop-drilling and makes individual command logic much easier to unit test in isolation.The Legacy Adapter: Ensuring a Smooth Transition
To avoid a massive, all-or-nothing PR, I've implemented a legacy adapter pattern within
useSlashCommandProcessor.handleSlashCommandfunction now first attempts to find and execute a command from the newCommandServicetree using a tree-traversal algorithm.legacyCommandsarray, ensuring that commands not yet migrated (like/chat,/stats,/docs) continue to function perfectly.Scope of This PR
CommandServiceand theCommandContextDI container.SlashCommandinterface intypes.ts./clear,/help, and the entire/memorycommand suite to the new modular, tree-based structure.useSlashCommandProcessorto consume commands from theCommandServiceand include the legacy adapter.useCompletionhook,InputPromptcomponent, and<Help/>component to be fully aware of the nested command structure, providing intelligent, context-aware autocompletion and help display for sub-commands and their arguments.CommandServiceand all migrated commands, leveraging a newcreateMockCommandContexttest utility for easy and consistent test setup.Future Work (To Be Addressed in Follow-up PRs)
This PR lays the groundwork for the next phases! This are for discussion and I would love to get community feedback towards the direction we want to drive this. Some of these just encompass some ideas.
High-Priority Follow-up
legacyCommandsarray (/chat,/stats,/docs,/theme,/auth, etc.) into their own[commandName]Command.tsfiles using the newSlashCommandinterface.legacyCommandsarray and the fallback logic will be removed entirely, simplifyinguseSlashCommandProcessor.Medium-Priority: Custom Commands
CommandServiceto discover and parse user-defined.gemini.ymlfiles from project and user directories.Medium-Priority: MCP Prompts
CommandServicewill be extended to query connected MCP servers for available prompts using theprompts/listprotocol endpoint.create-ticketprompt from ajiraserver becomes the/jira:create-ticketcommand).actionfor these commands will transparently handle theprompts/getrequest/response cycle, making the remote command feel completely native to the user.Low-Priority: UX Polish
Reviewer Test Plan
To validate these changes, please pull down the branch and run the Gemini CLI.
Test Migrated Commands:
/clear. The screen should clear as expected./help. The help dialog should appear, and you should see the nested structure for/memory./memory show,/memory add "test fact", and/memory refresh. All three sub-commands should execute correctly.Test Nested Autocompletion:
/memand pressTab. It should complete to/memory./memoryin the input, pressTabagain. It should cycle through the sub-commands (show,add,refresh)./memory shand pressTab. It should complete to/memory show.Test Legacy Command Fallback:
/statsor/docs.Review the Code:
packages/cli/src/services/CommandService.tsandpackages/cli/src/ui/commands/types.tsas they define the core of the new architecture.packages/cli/src/ui/hooks/slashCommandProcessor.tsto see how the new service is consumed and how the legacy adapter is implemented.packages/cli/src/ui/commands/memoryCommand.tsas a reference for how new, modular, nested commands are defined.Testing Matrix
Linked issues / bugs