feat(web): support slash commands in gateway web chat input#7223
feat(web): support slash commands in gateway web chat input#7223NiuBlibing wants to merge 6 commits into
Conversation
Web chat previously sent every submission, including "/clear" etc., as a raw prompt to the model. Parse a leading "/" as a runtime command instead. Adds a small command registry (slashCommands.ts) and a dispatcher in AgentChat: /help lists commands, /clear and /new reset the session via the existing clearAllMessages path, /model shows or switches the active model via the existing switchModel path. Unknown commands surface an inline notice and are never forwarded to the model. A /-triggered autocomplete popover lists matching commands. New i18n keys back the command output. Fixes zeroclaw-labs#7137
… translate cmd_* Three follow-ups to the slash-command feature uncovered by an adversarial review: - `addLocalMessage` now sets `local: true` on the message it creates, and `uiMessagesToPersisted` skips local messages. Slash-command output (/help, switching banner, unknown-command notice) no longer pollutes localStorage or reappears on reload as a fake assistant reply. - `handleSend` now dispatches slash commands BEFORE the !connected early-return so /help works during transient disconnects. Network- dependent commands (/clear, /model) still self-recover via their existing reconnect paths. - The 13 `agent.cmd_*` keys are now defined for all locales, matching the rest of the i18n table's coverage. Plus two NITs: /model during modelLoading shows a busy banner instead of silently no-op'ing, and /model failures surface as a local message.
WareWolf-MoonWall
left a comment
There was a problem hiding this comment.
Review of PR #7223
Reviewed commit range: all commits in PR as of 2026-06-04T17:48:08Z
Reviewer: @WareWolf-MoonWall
Review type: First review
🟢 What looks good — Architecture and state management
The single-source-of-truth discipline is solid here. COMMANDS in slashCommands.ts is the canonical registry, and both the /help output and the autocomplete popover derive from it — no duplication. The local: true flag on ChatMessage correctly prevents command output from bleeding into localStorage persistence, which was the root cause the author called out in the PR description. This is exactly the pattern AGENTS.md prescribes: tag ephemeral state at the source, filter it at the persistence boundary, and don't cache it anywhere in between.
The reordering of handleSend so command dispatch runs before the !connected gate is the right call — /help working during transient disconnects is a user-facing win and doesn't introduce new failure modes.
🟢 What looks good — i18n completeness
I spot-checked the diff and confirmed the author added all 15 cmd_* keys across the full locale set in web/src/lib/i18n.ts. The keys cover:
cmd_unknown,cmd_help_header,cmd_help_help,cmd_help_clear,cmd_help_new,cmd_help_modelcmd_cleared,cmd_model_current,cmd_model_none,cmd_model_available,cmd_model_unknowncmd_model_switching,cmd_model_busy,cmd_model_failed,cmd_hint_title
All 30+ locales (zh-CN, ja-JP, ar, tr, bn, cs, da, de, el, es, fi, fr, he, hi, hu, id, it, ko, nb, nl, pl, pt, ro, ru, sv, th, tl, uk, ur, vi) received translations. This is consistent with AGENTS.md localization rules (all user-facing output must use fl!()/t() — never bare string literals). No English hardcodes detected in the command handlers.
🔵 Suggestion — Command message role semantics
addLocalMessage emits messages with role: 'agent' (line 637 in AgentContext.tsx). This is semantically off — the output is locally generated by the web client, not by the agent runtime. It works because the UI only cares about user vs. non-user for styling, but it's a future landmine if we ever surface role-based filtering or analytics.
Suggestion: Consider introducing a third role value 'system' or 'info' for locally-generated feedback, or document in a comment why 'agent' is the intentional choice here. If you leave it as-is, add a comment explaining the decision so future maintainers don't "fix" it into a bug.
🔵 Suggestion — Inline style mutation in event handlers
The autocomplete popover uses inline onMouseEnter / onMouseLeave handlers that mutate e.currentTarget.style.background directly (lines 525-526 in AgentChat.tsx). This works but bypasses React's declarative model and makes the hover state invisible to the component tree.
Suggestion: Track hover state in a local useState<string | null> (keyed by command name) and apply the background conditionally via inline style or className. This keeps the style logic declarative and makes it testable if tests ever land.
Not blocking — the current implementation is correct and this is a minor stylistic preference.
🟢 What looks good — Validation rigor
The author ran the full battery:
cargo fmt --all -- --check→ exit 0cargo clippy --all-targets -- -D warnings→ exit 0cargo test --workspace --all-targets→ 7823 passed, 0 failed(cd web && npx tsc -b --force)→ exit 0
Manual verification across /help, /clear, /status, /model, locale switching (zh-CN, ja-JP, ar), and disconnected-gateway behavior. The author explicitly called out NOT testing the legacy Tauri desktop app, which is correct scoping — this is a web-only PR.
No web unit tests exist in the repository (confirmed via find_path — zero .test.* or .spec.* files under web/), so the lack of test additions here is not a regression. The command parsing logic in slashCommands.ts is pure and simple enough that manual validation is acceptable for now.
🟢 What looks good — Diff hygiene and commit structure
Two commits:
- Initial feature implementation (command registry, parsing, autocomplete, i18n)
- Fixes from initial review (reorder
handleSend,local: truepersistence exclusion, additional i18n keys)
The commit structure tells a coherent story. The second commit addressed the two bugs the author called out in the PR body (command output persisting across reloads, /help failing when gateway offline). The fact that those fixes landed in commit 2 suggests the author caught them in self-review or during manual testing, which is the right discipline.
No unrelated changes, no formatting-only noise, no stray debug code. Clean.
🔵 Suggestion — Escape hatch documentation
The PR body mentions "the documented escape is to prefix with a space if needed" for users who previously sent messages starting with / to the agent. The /help output in slashCommands.ts should surface this escape hatch so users can discover it without reading GitHub.
Suggestion: Add a line to helpText() like:
**Tip:** To send a message starting with `/` to the agent (not as a command), prefix it with a space or type `//`.
The code already handles // as an escape (line 51 in slashCommands.ts: !trimmed.startsWith('//')) but doesn't document it.
✅ Verdict
Approve. This is a well-scoped, cleanly implemented feature that addresses the gap described in #7137. The architecture respects the single-source-of-truth rule, the i18n coverage is complete, and the validation evidence is thorough. The two 🔵 suggestions are minor polish items that don't block merge — the author can address them in a follow-up if desired.
Next steps:
- If the author wants to act on the suggestions, they can push a fixup commit or open a follow-up issue.
- If not, this is ready to land as-is.
Foundations referenced:
- FND-005 (Contribution Culture) — feedback taxonomy, review voice
- AGENTS.md — single source of truth rule, localization rules, risk tiers, validation discipline
theonlyhennygod
left a comment
There was a problem hiding this comment.
Review of PR #7223
Reviewed commit: bd13cf0 (current head)
Reviewer: @theonlyhennygod
Review type: Second independent review (following @WareWolf-MoonWall's approval)
I read the full diff, cross-checked the persistence and dispatch logic against the local source in web/src/contexts/AgentContext.tsx, web/src/lib/chatHistoryStorage.ts, and web/src/pages/AgentChat.tsx, and checked the CI rollup on the current head. I agree with @WareWolf-MoonWall that the feature itself is clean and well-scoped — the registry-driven design and the persistence handling are genuinely good. My review adds two things their pass didn't cover: the current red CI, and one correctness gap in the documented escape hatch.
🔴 Blocking (for merge) — CI is red on an unrelated upstream compile error
The required gate is failing on the current head. The failing jobs (Lint, Build for all three targets, Check for all feature sets, Test, Benchmarks Compile) are all failing for the same reason, and it is not in this PR's diff:
error[E0308]: mismatched types
--> crates/zeroclaw-providers/src/ollama.rs:1058:17
|
| temperature,
| ^^^^^^^^^^^ expected `Option<f64>`, found `f64`
This is the exact breakage fixed by commit 125b6aa ("fix(ollama): thread Option temperature through chat()"). That fix is not on master HEAD (a1b641e) yet — it was committed at 22:33Z, after this PR's CI ran at 17:58Z. In other words, the master your branch is built on is currently broken at the Rust layer, and your frontend-only change is inheriting that breakage through no fault of its own.
I'm flagging this as blocking-for-merge only because the required gate genuinely won't go green until the ollama fix lands on master — it is not a request to change anything in your diff. Concretely:
- This is not something a rebase onto current master HEAD will fix today, because master HEAD itself still lacks 125b6aa.
- Once that ollama fix is on master, updating this branch and re-running CI should clear all of these failures.
The PR body's validation evidence (cargo clippy → exit 0, cargo test → 7823 passed) was honest and correct at the time you ran it locally — it just predates the upstream regression. No action needed from you beyond a branch update once master is green; I'm noting it so this doesn't get merged red.
🟡 Warning — the documented "escape hatch" doesn't actually deliver a leading slash to the agent
The PR body and the /help follow-up both describe an escape for sending a message that starts with / to the agent: "prefix it with a space" or "type //". Cross-checking against handleSend and isSlashCommand, neither path produces the intended result:
- Space prefix:
handleSendrunsconst trimmed = input.trim();before the command check, so a leading space is stripped." /foo"becomes"/foo", whichisSlashCommandthen treats as a command. The space escape is a no-op. //prefix:isSlashCommand('//foo')returnsfalse(correct — it's not a command), so it falls through tosendMessage(trimmed)— buttrimmedis still"//foo". The agent receives the literal doubled slash, not/foo.
The net effect is that after this PR there is no way to send the agent a message whose first character the agent sees as a single /. Before this PR, every such message went straight through. That's a small but real capability regression for a niche case.
This isn't blocking, but I'd like it addressed one of two ways before merge: either make // strip to a single leading slash in the non-command branch (so //foo is sent as /foo), or drop the "prefix with a space" claim from the PR body / /help and document only the mechanism that actually works. Whichever you pick, the user-facing docs and the code should agree.
🔵 Suggestion — seconding the role: 'agent' semantics note
I agree with @WareWolf-MoonWall's point on addLocalMessage emitting role: 'agent' (AgentContext.tsx). Locally-generated command output isn't from the agent runtime, and the local: true flag already gives you a clean discriminator at the persistence boundary. If you'd rather not introduce a new role value now, a one-line comment on the role: 'agent' field explaining it's a deliberate styling choice would stop a future maintainer from "fixing" it. Not blocking.
🔵 Suggestion — no keyboard path through the autocomplete popover
The command popover is mouse-only — selection happens on onMouseDown, and handleKeyDown only handles Escape (to dismiss) and Enter (which sends, not selects). A user typing /mo sends /mo as an unknown command rather than completing to /model. Arrow-key navigation + Enter-to-select would match what people expect from a slash-command menu, and it's the affordance that makes the popover useful for keyboard-first users. Optional, and a reasonable follow-up rather than a blocker.
🟢 What looks good — persistence exclusion is implemented at the right boundary
I verified the local: true path end to end: the field is declared on ChatMessage, set in addLocalMessage, and filtered in uiMessagesToPersisted via .filter((m) => !m.local) before the map. That's exactly where the filter belongs — at the persistence boundary, not at render time — so command output shows in the live transcript but never re-hydrates as a fake assistant reply on reload. The inline comments pointing back to #7137 make the intent obvious. This is the cleanest possible fix for the root cause described in the issue.
🟢 What looks good — single source of truth for the command set
COMMANDS in slashCommands.ts is the one registry, and helpText(), matchCommands(), and the popover all derive from it — adding a command is a one-line change in one place with no risk of the help text and the autocomplete drifting apart. The pure parsing helpers (isSlashCommand, parseCommand, matchCommands) are side-effect-free and trivially testable if web tests ever land. Good separation between the command vocabulary (lib) and the dispatch/UI (page).
🟢 What looks good — dispatch-before-connectivity ordering and i18n coverage
Reordering handleSend so command dispatch runs before the !connected gate is the right call — /help working during a transient disconnect is a real UX win and the network-dependent commands still self-recover through their own context paths. And the i18n work is thorough: all 15 cmd_* keys across the full non-English locale set, including the two async-failure keys (cmd_model_busy, cmd_model_failed) for the /model switch path. No bare English literals in the handlers.
Verdict
Comment. The code is approve-quality and I have nothing in the diff I'd personally block on. I'm withholding an approval purely because the required CI gate is red (upstream ollama breakage, not your code) and because the escape-hatch behavior should be reconciled with its documentation before this lands. Once the master-side ollama fix is in and the branch is updated to a green base — and the escape-hatch gap is either fixed or the docs corrected — this is ready to merge.
Next steps:
- Maintainer side: land the ollama
Option<f64>fix (125b6aa) on master, then this branch can update onto a green base and re-run CI. - Author side: reconcile the
/// space escape behavior with the PR body and/helptext. The two 🔵 items are optional polish.
Foundations referenced:
- FND-005 (Contribution Culture) — feedback taxonomy, review voice, not blocking contributors on things outside their control
- AGENTS.md — single source of truth rule, localization rules, validation discipline
|
@singlerider @theonlyhennygod — milestone alignment needed: this PR does not clearly fit within the scope boundary of any open milestone. It's a gateway web-chat UX feature (client-side slash commands), not a net-new channel/provider/tool (v0.8.1), nor config/schema work (v0.8.0), nor auth/security hardening (v0.9.0). Please advise on placement or deferral. |
singlerider
left a comment
There was a problem hiding this comment.
Reviewed at head ce9e516 against current master. The implementation is genuinely clean — both prior reviewers are right about that — and the CI blocker @theonlyhennygod cited has cleared. The one substantive gap is the escape-hatch doc/code mismatch they flagged, which I confirmed in source.
🟢 The architecture is the right shape
COMMANDS in slashCommands.ts is the single registry; helpText(), matchCommands(), and the autocomplete popover all derive from it, so adding a command is one line in one place with no drift. The local: true flag is declared on ChatMessage, set in addLocalMessage, and filtered in uiMessagesToPersisted via !m.local at the persistence boundary — so command output shows in the live transcript but never re-hydrates as a fake assistant reply on reload. Dispatch-before-connectivity ordering (so /help works during a transient disconnect) is a real UX win with no new failure mode, and the i18n coverage is complete (all 15 cmd_* keys across the full locale set, including the async-failure keys). The pure parsers (isSlashCommand, parseCommand, matchCommands) are side-effect-free.
✅ CI is now green
@theonlyhennygod's red-CI blocker was an unrelated upstream ollama.rs Option<f64> mismatch on master. That fix has landed; the full check rollup on this head is green and the branch is MERGEABLE / CLEAN.
🔴 Blocking — the documented escape hatch does not match the code
The PR body says: "the documented escape is to prefix with a space if needed (called out in /help)." Two problems, both confirmed in source:
- The space escape does not work.
isSlashCommanddoesinput.trimStart()before the/check, so" /help"is still treated as a command. A user following the documented advice to send a literal/messageby leading with a space will still trigger command handling. - The real escape is
//, and/helpnever mentions it. The actual escape implemented is double-slash (!trimmed.startsWith('//')), buthelpText()only lists the commands — it carries no escape-hatch line at all, in any locale.
So a user who legitimately needs to send a message starting with / has no working, documented path. Reconcile this: make the body and /help describe the real // escape (and add a cmd_* hint key for it across locales), or implement the space escape the body promises. The code behavior itself (//) is sensible; the docs just have to match it.
🔵 Minor (optional, per prior reviews)
Keyboard navigation through the autocomplete popover (it's mouse-only today; /mo sends /mo rather than completing to /model), the role: 'agent' semantics on locally-generated messages, and the inline-style hover mutation are all reasonable follow-ups, not blockers.
Reconcile the escape hatch (doc + /help) with the // behavior and this is ready — the rest is approve-quality.
| * mode. A bare `/` or `//...` (escaped slash) is NOT a command. | ||
| */ | ||
| export function isSlashCommand(input: string): boolean { | ||
| const trimmed = input.trimStart(); |
There was a problem hiding this comment.
🔴 trimStart() strips a leading space, so the PR body's documented escape ('prefix with a space') doesn't work — " /help" still parses as a command. The real escape is // (the !trimmed.startsWith('//') check), but helpText() never mentions it. Reconcile: document the // escape in the body and /help (with a localized hint key), or implement the space escape the body promises.
`//foo` was forwarded verbatim instead of stripping to `/foo`, and `/help` never documented the escape at all — so there was no working, documented way to send the agent a message starting with a literal `/`. Strip one leading `/` when sending a `//`-prefixed message, and surface the escape in `/help` (new `agent.cmd_help_escape` key, all locales). Addresses review feedback on zeroclaw-labs#7223.
|
Pushed a fix for the escape-hatch doc/code mismatch flagged by @theonlyhennygod and @singlerider:
|
Summary
masterslashCommands.tsprovides a small registry (/help,/clear,/model,/status, plus aliases) that handles input locally — without sending the text to the agent as a chat message.local: true, sochatHistoryStorage.uiMessagesToPersistedexcludes them from the persisted history. Previously,/helpoutput bled into the conversation transcript and persisted across reloads.handleSendso the slash-command dispatch runs before the!connectedgate. Previously, when the gateway was offline, typing/helpproduced the network-error banner instead of opening the local help text.cmd_*keys across all 30 non-English locales so command output is localized end-to-end. Two new keysagent.cmd_model_busyandagent.cmd_model_failedfor/modelasync failure paths./api/agents/{alias}/modelendpoint. No agent-side slash-command interpretation — the agent never sees these inputs./first at the start of a message now get local dispatch instead of forwarding to the agent. Documented in/helpoutput.enhancement,risk: low,size: LValidation Evidence (required)
cargo fmt --all -- --check→ exit 0cargo clippy --all-targets -- -D warnings→ exit 0cargo test --workspace --all-targets→ exit 0, 7823 passed, 0 failed (no Rust code changed; battery run for completeness).(cd web && npx tsc -b --force)→ exit 0/help,/clear,/status,/model gpt-4oagainst a live gateway — each dispatches locally and the output does not survive a page reload. Confirmed/helpworks while the gateway WS is disconnected. Confirmed locale switching renders command output in zh-CN, ja-JP, and one RTL locale (ar). Did NOT verify behaviour in the legacy desktop Tauri app — out of scope for this web-only PR.Security & Privacy Impact (required)
/modelreuses the existing per-agent settings endpoint.Compatibility (required)
/see no behaviour change. Users who previously sent messages starting with/to the agent will now get local handling; the documented escape is to prefix with a space if needed (called out in/help).Rollback (required for medium/high)
git revert <sha>— pure frontend, no state migration. Rollback restores the prior "send everything to the agent" behaviour.