Typed slash-command registry - #626
Conversation
WalkthroughThe change introduces a typed slash-command registry with centralized parsing, validation, help, access, busy behavior, and dispatch. Channels use structured command messages. Agent command prompts are registered centrally. The design defines adapter integrations and Portal command APIs. ChangesSlash command centralization
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
All slash commands now live in one static table in src/commands. Parsing, dispatch, and /help derive from it, replacing the three dispatch sites in channel.rs (inline match, /agent-id special case, prompt rewrites) and the hand-maintained help array that had drifted from the real command set. - CommandDef with ArgSpec (None/Optional/Required/Choice), Control vs Agent handlers, and category grouping - shared parser: case-insensitive names+aliases, telegram /cmd@botname strip, iOS smart-dash normalization in args, file-path rejection, trailing text on no-arg commands stays conversational - MessageContent::Command wire variant, renders as "/name args" so the text path dispatches it identically - commands now work on every adapter, including portal (previously a silent no-op in the web UI) - /quiet gains /observe as an alias; /help gains /commands
b0731ae to
bca84f9
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
src/commands/registry.rs (1)
120-247: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse full variable names in command code.
The new code uses abbreviated local names such as
defs,def,opt, andcmd. Rename them to explicit names such ascommand_definitions,command_definition,option, andparsed_command.
src/commands/registry.rs#L120-L247: Rename abbreviated registry lookup and help-rendering variables.src/commands/registry.rs#L361-L524: Rename abbreviated test helper and command-definition variables.src/agent/channel.rs#L1184-L1256: Rename thedefcontrol-handler parameter.src/agent/channel.rs#L2048-L2121: Rename the parsed-command binding.As per coding guidelines, “Don't abbreviate variable names. Use
queuenotq,messagenotmsg,channelnotch.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/commands/registry.rs` around lines 120 - 247, Rename abbreviated variables throughout the command code to explicit names without changing behavior: in src/commands/registry.rs lines 120-247, use names such as command_definitions, command_definition, and option; apply the same naming cleanup to test helpers and command-definition variables in lines 361-524. Rename the control-handler def parameter in src/agent/channel.rs lines 1184-1256 and the parsed-command binding in lines 2048-2121 to descriptive full names.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/design-docs/slash-commands.md`:
- Line 337: Add an explicit fence language, such as text, to the fenced code
blocks at the documented examples around lines 337 and 363 in the slash-commands
documentation, preserving their contents while satisfying Markdown linting.
In `@src/agent/channel.rs`:
- Around line 2048-2061: Move control-command parsing and dispatch out of the
serial channel actor path, routing recognized control commands through a
control-plane entry point before enqueueing the InboundMessage. Use the
originating inbound message to bind the reply, return without flushing any
pending coalesce buffer, and leave agent-command rewriting and unrecognized
command text on the existing path; update the relevant channel entry point and
handle_control_command flow rather than the in-actor match alone.
In `@src/commands/mod.rs`:
- Around line 5-10: Move the `commands` module root from `src/commands/mod.rs`
to `src/commands.rs`, preserving the `registry` child-module declaration and all
existing public re-exports (`AgentAction`, `ArgSpec`, `COMMANDS`,
`CommandCategory`, `CommandDef`, `CommandHandler`, `CommandRegistry`,
`ControlAction`, `ParseResult`, `ParsedCommand`, and `REGISTRY`).
In `@src/commands/registry.rs`:
- Around line 515-521: Update names_and_aliases_are_globally_unique to normalize
each command name and alias to a consistent ASCII case before inserting into
seen, matching CommandRegistry::resolve behavior. Keep duplicate assertions
intact while reporting the original values.
- Around line 259-276: Move TASKS_PROMPT, TODAY_PROMPT, and DIGEST_PROMPT out of
src/commands/registry.rs into separate Markdown files under prompts/. Update the
command prompt construction or lookup flow to load/render those files through
the existing prompt system, preserving each prompt’s exact output requirements
and behavior.
---
Nitpick comments:
In `@src/commands/registry.rs`:
- Around line 120-247: Rename abbreviated variables throughout the command code
to explicit names without changing behavior: in src/commands/registry.rs lines
120-247, use names such as command_definitions, command_definition, and option;
apply the same naming cleanup to test helpers and command-definition variables
in lines 361-524. Rename the control-handler def parameter in
src/agent/channel.rs lines 1184-1256 and the parsed-command binding in lines
2048-2121 to descriptive full names.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5003a6a3-80a4-4480-b2d5-77ac16bb6e4b
📒 Files selected for processing (5)
docs/design-docs/slash-commands.mdsrc/agent/channel.rssrc/commands/mod.rssrc/commands/registry.rssrc/lib.rs
| // Slash-command dispatch. Control commands execute deterministically | ||
| // on the spot and never consume an agent turn; agent commands are | ||
| // rewritten into their instruction below. System messages are never | ||
| // commands, and unrecognized "/words" flow to the model as text. | ||
| let parsed_command = if message.source == "system" { | ||
| crate::commands::ParseResult::NotACommand | ||
| } else { | ||
| crate::commands::REGISTRY.parse(&raw_text) | ||
| }; | ||
| match &parsed_command { | ||
| crate::commands::ParseResult::Command(cmd) => { | ||
| if let crate::commands::CommandHandler::Control(action) = cmd.def.handler { | ||
| self.handle_control_command(cmd.def, action).await; | ||
| return Ok(()); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Route control commands outside the channel actor queue.
Lines 2048-2061 execute control commands only after handle_message starts. While run_agent_turn awaits at Lines 2220-2228, the serial actor cannot dequeue a /status or mode command. A pending coalesce buffer also flushes before this path runs.
Parse and route recognized control commands through a control-plane entry point before queueing an InboundMessage. Bind the reply to the originating inbound message. Do not flush the buffered conversation for a control command.
As per coding guidelines, “Don't block the channel. The channel never waits on branches, workers, or compaction.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/agent/channel.rs` around lines 2048 - 2061, Move control-command parsing
and dispatch out of the serial channel actor path, routing recognized control
commands through a control-plane entry point before enqueueing the
InboundMessage. Use the originating inbound message to bind the reply, return
without flushing any pending coalesce buffer, and leave agent-command rewriting
and unrecognized command text on the existing path; update the relevant channel
entry point and handle_control_command flow rather than the in-actor match
alone.
Source: Coding guidelines
There was a problem hiding this comment.
Correct observation, deliberately out of scope for this PR. This is phase 2 of the design committed on this branch (docs/design-docs/slash-commands.md): control commands move to the control plane (ChannelControlHandle / ProcessControl / settings store) and stop entering the channel queue entirely, which also covers the coalesce-flush ordering. Phase 1 is the behavior-preserving port — commands dispatch from the same position in handle_message as the code they replace.
There was a problem hiding this comment.
@jamiepine, acknowledged. The finding is valid, but it does not apply to the Phase 1 scope.
Phase 1 preserves the existing Channel::handle_message dispatch position. Phase 2 will move control commands to the control plane and remove their dependency on the channel actor queue. That change will also prevent coalesce-buffer flushing for control commands.
Would you like me to create a GitHub follow-up issue for Phase 2?
You are interacting with an AI system.
- module root moves to src/commands.rs (repo bans mod.rs files) - the three agent-turn instructions move from Rust constants to prompts/en/commands/*.md.j2, rendered through the prompt engine with raw-text fallback; a new test renders every PromptTemplate key through the real engine so an unregistered key fails at test time - uniqueness test normalizes case to match resolve() semantics - doc code fences get languages
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@prompts/en/commands/today.md.j2`:
- Around line 1-7: Update the prompt’s non-empty output format to require
literal headers for both sections, using “in progress:” and “up next (ready):”
before their task lines. Keep the existing task-line format and empty-section
strings unchanged, ensuring both populated and empty sections remain clearly
distinguishable.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: aebe3f72-e962-4f1c-8b27-b54b1858b7d9
📒 Files selected for processing (9)
docs/design-docs/slash-commands.mdprompts/en/commands/digest.md.j2prompts/en/commands/tasks.md.j2prompts/en/commands/today.md.j2src/agent/channel.rssrc/commands.rssrc/commands/registry.rssrc/prompts/engine.rssrc/prompts/text.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- src/agent/channel.rs
- docs/design-docs/slash-commands.md
| use channel tools to build a local tasks snapshot and reply exactly in this format: | ||
| - first line: today (local tasks snapshot): | ||
| - section 1: in-progress tasks (up to 5), each line: #<task_number> [<priority>] <title> | ||
| - section 2: up next ready tasks (up to 5), each line: #<task_number> [<priority>] <title> | ||
| if a section is empty use: | ||
| - in progress: none | ||
| - up next (ready): none |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Define literal headers for non-empty sections.
The prompt requires an exact format, but it does not require in progress: or up next (ready): when a section contains tasks. The response can contain two indistinguishable groups of task lines. Add literal section headers to the non-empty format and keep the empty format consistent.
Proposed prompt wording
- - section 1: in-progress tasks (up to 5), each line: #<task_number> [<priority>] <title>
- - section 2: up next ready tasks (up to 5), each line: #<task_number> [<priority>] <title>
+ - second line: in progress:
+ - next up to 5 lines: #<task_number> [<priority>] <title>
+ - next section header: up next (ready):
+ - next up to 5 lines: #<task_number> [<priority>] <title>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| use channel tools to build a local tasks snapshot and reply exactly in this format: | |
| - first line: today (local tasks snapshot): | |
| - section 1: in-progress tasks (up to 5), each line: #<task_number> [<priority>] <title> | |
| - section 2: up next ready tasks (up to 5), each line: #<task_number> [<priority>] <title> | |
| if a section is empty use: | |
| - in progress: none | |
| - up next (ready): none | |
| use channel tools to build a local tasks snapshot and reply exactly in this format: | |
| - first line: today (local tasks snapshot): | |
| - second line: in progress: | |
| - next up to 5 lines: #<task_number> [<priority>] <title> | |
| - next section header: up next (ready): | |
| - next up to 5 lines: #<task_number> [<priority>] <title> | |
| if a section is empty use: | |
| - in progress: none | |
| - up next (ready): none |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@prompts/en/commands/today.md.j2` around lines 1 - 7, Update the prompt’s
non-empty output format to require literal headers for both sections, using “in
progress:” and “up next (ready):” before their task lines. Keep the existing
task-line format and empty-section strings unchanged, ensuring both populated
and empty sections remain clearly distinguishable.
1 participant
Phase 1 of the command system design (
docs/design-docs/slash-commands.md, committed on this branch). Stacked on #625.Slash commands were spread across three dispatch sites in
channel.rs: an inline full-stringmatch, a special-cased/agent-idcheck before it, and three raw-text prompt rewrites — plus a hand-maintained/helparray that had already drifted from the real command set. Commands silently did nothing in the portal because it wasn't in the supported-source list.All of that is now one static table in
src/commands. Parsing, dispatch, and/helpderive from it:Controlcommands execute deterministically against channel state (no agent turn);Agentcommands rewrite into their instruction and run as a normal turn.ArgSpecvalidates centrally —Requiredwithout args gets a usage reply,Choicevalidates against the set, and trailing text on a no-arg command (/status update: we shipped) stays conversational and reaches the model untouched.The parser handles the field bugs each adapter would otherwise rediscover: Telegram's
/cmd@botnamegroup addressing, iOS smart-dash mangling (--→—) in args, and pasted file paths (/usr/bin/env) never parsing as commands.Also adds the
MessageContent::Command { name, args }wire variant for surfaces that parse commands client-side. It renders as/name args, so the text path dispatches it identically with no second entry path.Behavior-preserving port: same replies, same ordering guarantees (mode commands still work in observe mode so
/activecan rescue a quieted channel). Changes on top: commands now work on every adapter including portal,/quietgains/observeas an alias,/helpgains/commands.Phases 2–4 (binding-level authority, busy-policy handling, native platform registration, new command set) stack on this.
Testing
15 registry unit tests: parse basics, alias/case-insensitive resolution,
@botnamestripping, path rejection, trailing-text semantics,ArgSpecvalidation and usage replies, smart-dash normalization, help/table drift guards (every command listed exactly once, names and aliases globally unique).