fix: dispatch unknown slash commands to the agent bridge (/plan, /spike, /goal, …)#983
Open
paulocavallari wants to merge 2 commits into
Open
fix: dispatch unknown slash commands to the agent bridge (/plan, /spike, /goal, …)#983paulocavallari wants to merge 2 commits into
paulocavallari wants to merge 2 commits into
Conversation
The Web UI was eating any `/something` input that wasn't one of the nine
hard-coded "session commands" (`/usage`, `/clear`, `/title`, etc.).
Skill commands like `/plan`, `/spike`, `/tdd`, plus persistent commands
like `/goal`, never reached the Python bridge — they were rewritten to
`{name: 'status', rawName: 'plan'}` and the gateway answered with
"Unknown bridge command: /plan". The CLI and gateway adapters resolve
these the right way; the Web UI just never grew the same hook.
This PR adds the missing dispatch on both layers:
1. `session-command.ts::parseSessionCommand` returns `null` for unknown
aliases instead of fabricating a fake command. Unknown slash inputs
now fall through to `handleBridgeRun` like any normal user message.
2. `hermes_bridge.py::_run_chat` resolves `/skill-name` (and
`/bundle-name`) inputs **after** prepersisting the literal slash
command to the session DB and **before** calling
`AIAgent.run_conversation()`. Resolution uses the upstream helpers
(`agent.skill_commands.build_skill_invocation_message`,
`agent.skill_bundles.build_bundle_invocation_message`) so the Web
UI shares the canonical injection logic with the CLI and gateway —
no duplicated SKILL.md parsing, no drift if those helpers change.
Behavior:
- `/plan criar plano de aula` → DB sees the literal `/plan ...` text;
the model receives the SKILL.md content + user instruction, exactly
like in CLI / Telegram / Discord.
- Unknown `/foobar` input → flows through as a plain user message
instead of erroring out, matching CLI behavior.
- Existing bridge commands (`/usage`, `/clear`, `/abort`, `/title`,
`/compress`, `/steer`, `/queue`, `/destroy`) keep working unchanged.
The Python hook is pure-additive and wrapped in defensive try/except so
import failures or missing skills silently fall back to the original
message — never breaks a run.
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Routes unknown slash commands (e.g., /plan, /spike, /goal, /tdd) from the TS bridge layer down to the Python bridge, where they are resolved against skill bundles and individual skill commands before being sent to the agent.
Changes:
parseSessionCommandno longer coerces unknown slash inputs into astatuscommand; it returnsnullso they fall through tohandleBridgeRun.hermes_bridge.pyadds post-prepersist slash resolution that consultsagent.skill_bundlesthenagent.skill_commands, replacingmessagewith the resolved invocation text on success and silently falling back on failure.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| packages/server/src/services/hermes/run-chat/session-command.ts | Return null for unknown slash commands so they pass through to the bridge. |
| packages/server/src/services/hermes/agent-bridge/hermes_bridge.py | Adds skill-bundle/skill-command resolution for /name [args] messages with defensive try/excepts and logging. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1322
to
+1355
| if _slash_text.startswith("/") and len(_slash_text) > 1: | ||
| _slash_match = re.match(r"^/([a-zA-Z][\w-]*)(?:\s+([\s\S]*))?$", _slash_text) | ||
| if _slash_match: | ||
| _cmd_raw = _slash_match.group(1) | ||
| _cmd_args = (_slash_match.group(2) or "").strip() | ||
| _resolved_msg: Optional[str] = None | ||
|
|
||
| # 1) Skill bundles first (mirrors gateway/run.py order) | ||
| try: | ||
| from agent.skill_bundles import ( | ||
| build_bundle_invocation_message, | ||
| resolve_bundle_command_key, | ||
| ) | ||
| _bundle_key = resolve_bundle_command_key(_cmd_raw) | ||
| if _bundle_key is not None: | ||
| _bundle_result = build_bundle_invocation_message( | ||
| _bundle_key, _cmd_args, task_id=session.session_id | ||
| ) | ||
| if _bundle_result: | ||
| _bundle_msg, _loaded_bundle, _missing_bundle = _bundle_result | ||
| if _bundle_msg: | ||
| _resolved_msg = _bundle_msg | ||
| if _missing_bundle: | ||
| logger.info( | ||
| "Bundle /%s skipped missing skills: %s", | ||
| _cmd_raw, ", ".join(_missing_bundle), | ||
| ) | ||
| except Exception as _bundle_exc: | ||
| logger.debug( | ||
| "Skill bundle dispatch failed (non-fatal) for /%s: %s", | ||
| _cmd_raw, _bundle_exc, | ||
| ) | ||
|
|
||
| # 2) Individual skill command |
Comment on lines
+1330
to
+1334
| try: | ||
| from agent.skill_bundles import ( | ||
| build_bundle_invocation_message, | ||
| resolve_bundle_command_key, | ||
| ) |
Comment on lines
+1349
to
+1353
| except Exception as _bundle_exc: | ||
| logger.debug( | ||
| "Skill bundle dispatch failed (non-fatal) for /%s: %s", | ||
| _cmd_raw, _bundle_exc, | ||
| ) |
Comment on lines
+1319
to
+1381
| try: | ||
| if isinstance(message, str) and message.startswith("/"): | ||
| _slash_text = message.strip() | ||
| if _slash_text.startswith("/") and len(_slash_text) > 1: | ||
| _slash_match = re.match(r"^/([a-zA-Z][\w-]*)(?:\s+([\s\S]*))?$", _slash_text) | ||
| if _slash_match: | ||
| _cmd_raw = _slash_match.group(1) | ||
| _cmd_args = (_slash_match.group(2) or "").strip() | ||
| _resolved_msg: Optional[str] = None | ||
|
|
||
| # 1) Skill bundles first (mirrors gateway/run.py order) | ||
| try: | ||
| from agent.skill_bundles import ( | ||
| build_bundle_invocation_message, | ||
| resolve_bundle_command_key, | ||
| ) | ||
| _bundle_key = resolve_bundle_command_key(_cmd_raw) | ||
| if _bundle_key is not None: | ||
| _bundle_result = build_bundle_invocation_message( | ||
| _bundle_key, _cmd_args, task_id=session.session_id | ||
| ) | ||
| if _bundle_result: | ||
| _bundle_msg, _loaded_bundle, _missing_bundle = _bundle_result | ||
| if _bundle_msg: | ||
| _resolved_msg = _bundle_msg | ||
| if _missing_bundle: | ||
| logger.info( | ||
| "Bundle /%s skipped missing skills: %s", | ||
| _cmd_raw, ", ".join(_missing_bundle), | ||
| ) | ||
| except Exception as _bundle_exc: | ||
| logger.debug( | ||
| "Skill bundle dispatch failed (non-fatal) for /%s: %s", | ||
| _cmd_raw, _bundle_exc, | ||
| ) | ||
|
|
||
| # 2) Individual skill command | ||
| if _resolved_msg is None: | ||
| try: | ||
| from agent.skill_commands import ( | ||
| build_skill_invocation_message, | ||
| resolve_skill_command_key, | ||
| ) | ||
| _cmd_key = resolve_skill_command_key(_cmd_raw) | ||
| if _cmd_key is not None: | ||
| _skill_msg = build_skill_invocation_message( | ||
| _cmd_key, _cmd_args, task_id=session.session_id | ||
| ) | ||
| if _skill_msg: | ||
| _resolved_msg = _skill_msg | ||
| except Exception as _skill_exc: | ||
| logger.debug( | ||
| "Skill command dispatch failed (non-fatal) for /%s: %s", | ||
| _cmd_raw, _skill_exc, | ||
| ) | ||
|
|
||
| if _resolved_msg: | ||
| logger.info( | ||
| "Resolved /%s slash command for session %s (%d chars)", | ||
| _cmd_raw, session.session_id, len(_resolved_msg), | ||
| ) | ||
| message = _resolved_msg | ||
| except Exception as _slash_exc: |
Comment on lines
+1319
to
+1385
| try: | ||
| if isinstance(message, str) and message.startswith("/"): | ||
| _slash_text = message.strip() | ||
| if _slash_text.startswith("/") and len(_slash_text) > 1: | ||
| _slash_match = re.match(r"^/([a-zA-Z][\w-]*)(?:\s+([\s\S]*))?$", _slash_text) | ||
| if _slash_match: | ||
| _cmd_raw = _slash_match.group(1) | ||
| _cmd_args = (_slash_match.group(2) or "").strip() | ||
| _resolved_msg: Optional[str] = None | ||
|
|
||
| # 1) Skill bundles first (mirrors gateway/run.py order) | ||
| try: | ||
| from agent.skill_bundles import ( | ||
| build_bundle_invocation_message, | ||
| resolve_bundle_command_key, | ||
| ) | ||
| _bundle_key = resolve_bundle_command_key(_cmd_raw) | ||
| if _bundle_key is not None: | ||
| _bundle_result = build_bundle_invocation_message( | ||
| _bundle_key, _cmd_args, task_id=session.session_id | ||
| ) | ||
| if _bundle_result: | ||
| _bundle_msg, _loaded_bundle, _missing_bundle = _bundle_result | ||
| if _bundle_msg: | ||
| _resolved_msg = _bundle_msg | ||
| if _missing_bundle: | ||
| logger.info( | ||
| "Bundle /%s skipped missing skills: %s", | ||
| _cmd_raw, ", ".join(_missing_bundle), | ||
| ) | ||
| except Exception as _bundle_exc: | ||
| logger.debug( | ||
| "Skill bundle dispatch failed (non-fatal) for /%s: %s", | ||
| _cmd_raw, _bundle_exc, | ||
| ) | ||
|
|
||
| # 2) Individual skill command | ||
| if _resolved_msg is None: | ||
| try: | ||
| from agent.skill_commands import ( | ||
| build_skill_invocation_message, | ||
| resolve_skill_command_key, | ||
| ) | ||
| _cmd_key = resolve_skill_command_key(_cmd_raw) | ||
| if _cmd_key is not None: | ||
| _skill_msg = build_skill_invocation_message( | ||
| _cmd_key, _cmd_args, task_id=session.session_id | ||
| ) | ||
| if _skill_msg: | ||
| _resolved_msg = _skill_msg | ||
| except Exception as _skill_exc: | ||
| logger.debug( | ||
| "Skill command dispatch failed (non-fatal) for /%s: %s", | ||
| _cmd_raw, _skill_exc, | ||
| ) | ||
|
|
||
| if _resolved_msg: | ||
| logger.info( | ||
| "Resolved /%s slash command for session %s (%d chars)", | ||
| _cmd_raw, session.session_id, len(_resolved_msg), | ||
| ) | ||
| message = _resolved_msg | ||
| except Exception as _slash_exc: | ||
| # Defensive: any unexpected failure must not break the run. | ||
| logger.warning( | ||
| "Slash command resolution failed (non-fatal): %s", _slash_exc, | ||
| ) |
Apply the five suggestions from the automated Copilot review on PR EKKOLearnAI#983: 1. **Hoist skill / bundle imports to module scope.** Both `agent.skill_commands` and `agent.skill_bundles` are now imported once at module load, with a single try/except guarding availability. ImportError now surfaces as a startup WARNING ("Skill command/bundle support unavailable; /<name> commands will fall back to plain user input: ...") instead of being silently swallowed at DEBUG on every request. Per-request lookups reuse the resolved callables — no more import cost on the chat hot path. 2. **Compile the slash regex once.** New `_SLASH_COMMAND_RE` constant at module scope replaces the per-request `re.match(...)` call. 3. **Collapse the redundant guard.** The previous nested `message.startswith("/")` + `_slash_text.startswith("/") and len > 1` pair is gone — the regex handles both checks in one shot. 4. **Narrow the per-request excepts.** Inner blocks now catch `(AttributeError, KeyError, TypeError, ValueError)` (the realistic failure modes from the helper return shapes) with `exc_info=True` so tracebacks land in DEBUG logs when something does go wrong. Future refactors that introduce a real bug will surface it through the bridge logger instead of being silently downgraded. 5. **Remove the outer catch-all.** With the inner excepts narrowed and the imports hoisted, the outer `try/except Exception` was redundant — it would only fire on the regex / isinstance / logger calls, which shouldn't realistically raise. Removed. The contract is unchanged: `/plan`, `/spike`, `/goal`, `/tdd` and all other skill commands still get resolved before `run_conversation()`, the literal `/...` text still gets persisted to session history first, and resolution failures still fall back gracefully to the original message. Verified end-to-end on a live deployment after rebuild.
Contributor
Author
|
Thanks for the review! Pushed
Verified end-to-end on a live deployment after rebuild — |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The Web UI was eating any
/somethinginput that wasn't one of the nine hard-coded "session commands" (/usage,/clear,/title,/abort,/queue,/compress,/steer,/destroy,/status). Skill commands like/plan,/spike,/tdd, plus persistent commands like/goal, never reached the Python bridge — they were rewritten to{name: 'status', rawName: 'plan'}and the gateway answered withUnknown bridge command: /plan.The CLI (
cli.py::process_command) and gateway adapters (gateway/run.py::_handle_message) resolve these the right way, using the canonical helpers inhermes-agent. The Web UI just never grew the same hook. This PR adds the missing dispatch.Repro (before)
In Web UI:
→ Server logs:
Unknown bridge command: /plan. Skill never loaded.Fix (two layers)
1.
session-command.ts::parseSessionCommandreturnsnullfor unknown aliases instead of fabricating{name: 'status', rawName: 'plan'}. Unknown slash inputs now fall through tohandleBridgeRunlike any normal user message.2.
hermes_bridge.py::_run_chatresolves/skill-name(and/bundle-name) inputs after prepersisting the literal slash command to the session DB and before callingAIAgent.run_conversation(). Resolution uses upstream helpers (agent.skill_commands.build_skill_invocation_message,agent.skill_bundles.build_bundle_invocation_message) so the Web UI shares the canonical injection logic with the CLI and gateway — no duplicated SKILL.md parsing, no drift if those helpers change.The Python hook is pure-additive and wrapped in defensive try/except so import failures or missing skills silently fall back to the original message — never breaks a run.
Behavior after
/plan criar plano de aulaUnknown bridge command: /plan/plan ...; model receives full SKILL.md content + instruction (matches CLI/Telegram/Discord)/spike//tdd//goal/ any of the 88+ skill commands/foobar(unknown, not a skill)/usage,/clear,/abort,/title,/compress,/steer,/queue,/destroyVerification
Tested live on a self-hosted Hermes Web UI v0.6.0 deployment with the kiro-go profile (88 skill commands, including
/plan):/plan verifique se a correção dos comandos está funcionando→ SKILL.md was injected as user message with the standard[IMPORTANT: The user has invoked the "plan" skill...]activation header, plus the trailing user instruction. The model executed plan-mode behavior identical to CLI.Resolved /plan slash command for session <sid> (2519 chars)./usage,/clear) keep working unchanged.Risk
Low. The TS change tightens behavior (less false-positive command parsing) without removing any working code path. The Python change is additive — a try/except wrapper around two helper calls — and falls back to the original message on any failure.
Notes for reviewers
agent.skill_commands,agent.skill_bundles) are part ofhermes-agentproper and are already imported elsewhere inhermes_bridge.py's runtime path. They're stable upstream APIs (used bycli.pyandgateway/run.py)._run_chathook runs after_prepersist_user_messageso the user's literal/plantext is what gets stored in session history (matching CLI behavior where the typed command is preserved in transcripts), while the resolved skill content is what the model sees.