fix(commands): exact cross-authority parity for announced slash commands (#6951)

static/commands.js

@@ -186,13 +186,22 @@ function executeCommand(text){
 // submitted. The autocomplete must not advertise anything outside this set:
 // a non-dispatchable registry command (e.g. /agents) would otherwise fall
 // through send() to the ordinary chat path and trigger an unintended
-// model/API request. Keep this in sync with _AGENT_COMMANDS_RUN_ON_WEBUI in
-// messages.js -- the announced list is a subset of the dispatched list
-// (fail-closed), never a superset. Plugin-category commands are always
-// dispatchable via the plugin exec transport. #6951.
+// model/API request.
+//
+// This set holds canonical registry names only. getMatchingCommands() matches
+// cmd.name (the canonical metadata name), so alias/underscore forms such as
+// /reload_mcp are covered implicitly: typing one resolves through
+// getAgentCommandMetadata() at dispatch time and the canonical name is what
+// gets tested against the dispatcher allowlist (#6951).
+//
+// Keep this in sync with _AGENT_COMMANDS_RUN_ON_WEBUI in messages.js and
+// _ALLOWED_AGENT_COMMANDS in api/commands.py -- the announced list is a
+// subset of the dispatched list (fail-closed), never a superset.
+// Plugin-category commands are always dispatchable via the plugin exec
+// transport. #6951.
 const _WEBUI_DISPATCHABLE_AGENT_COMMANDS=new Set([
   // Backend-executed via /api/commands/exec (see _ALLOWED_AGENT_COMMANDS in api/commands.py).
-  'reload-mcp','reload_mcp','reload-skills','reload_skills','codex-runtime','codex_runtime','credits',
+  'reload-mcp','reload-skills','codex-runtime','credits',
   // Native WebUI behaviors handled inside send() without an agent round-trip.
   'moa','sessions','resume',
   // Desktop Companion extension command handled by handlePetSlashCommand().
@@ -244,6 +253,8 @@ function getMatchingCommands(prefix){
     matches.push({
       name,
       desc:String(cmd&&cmd.description||'').trim()||'Agent command',
+      // Surface the registry's argument hint on the autocomplete row (#6951).
+      arg:String(cmd&&cmd.args_hint||'').trim()||undefined,
       source:cmd.category==='Plugin'?'plugin':'agent',
     });
     seen.add(name);

tests/test_cli_only_slash_commands.py

@@ -2,12 +2,15 @@
 
 import json
 from pathlib import Path
+import re
 import subprocess
 import tempfile
 import textwrap
 from types import SimpleNamespace
 
-from api.commands import list_commands
+from api.commands import list_commands, _ALLOWED_AGENT_COMMANDS, _AGENT_COMMAND_ALIASES
+
+import pytest
 
 
 REPO_ROOT = Path(__file__).resolve().parents[1]
@@ -15,6 +18,36 @@ COMMANDS_JS = (REPO_ROOT / "static" / "commands.js").read_text(encoding="utf-8")
 MESSAGES_JS = (REPO_ROOT / "static" / "messages.js").read_text(encoding="utf-8")
 
 
+def _extract_js_set_members(source: str, varname: str) -> set[str]:
+    """Return the string members of a `const NAME=new Set([...])` literal."""
+    m = re.search(rf"const\s+{re.escape(varname)}\s*=\s*new Set\(\[(.*?)\]\)", source, re.S)
+    assert m, f"could not find const {varname}=new Set([...]) in source"
+    return set(re.findall(r"'([^']*)'", m.group(1)))
+
+
+def _canonical_agent_names(names: set[str]) -> set[str]:
+    """Map underscore alias forms to canonical names via api/commands.py's map."""
+    return {_AGENT_COMMAND_ALIASES.get(n, n) for n in names}
+
+
+def _extract_busy_intercept_block() -> str:
+    """Extract the busy-path slash intercept `if` block verbatim from send()."""
+    marker = "Busy-control slash commands must be intercepted"
+    marker_idx = MESSAGES_JS.find(marker)
+    assert marker_idx != -1
+    start = MESSAGES_JS.find("if(text.startsWith('/')&&!literalSlash){", marker_idx)
+    assert start != -1
+    depth = 0
+    for i in range(start, len(MESSAGES_JS)):
+        if MESSAGES_JS[i] == "{":
+            depth += 1
+        elif MESSAGES_JS[i] == "}":
+            depth -= 1
+            if depth == 0:
+                return MESSAGES_JS[start : i + 1]
+    raise AssertionError("unbalanced braces while extracting busy intercept block")
+
+
 def test_api_commands_exposes_cli_only_metadata_for_webui_intercept():
     """CLI-only commands must remain visible so the frontend can explain them."""
     registry = [
@@ -732,21 +765,37 @@ def test_dispatchable_agent_commands_stay_in_autocomplete():
     assert result["plugin_names"] == ["plugin-review"]
 
 
-def test_autocomplete_allowlist_covers_backend_exec_allowlist():
-    """#6951: every canonical backend-exec name from messages.js must be in the
-    commands.js dispatchable allowlist (announced list is a subset of the
-    dispatched list -- fail-closed)."""
-    assert "const _WEBUI_DISPATCHABLE_AGENT_COMMANDS=new Set([" in COMMANDS_JS
-    for slug in (
-        "'reload-mcp'",
-        "'reload_mcp'",
-        "'reload-skills'",
-        "'reload_skills'",
-        "'codex-runtime'",
-        "'codex_runtime'",
-        "'credits'",
-    ):
-        assert slug in COMMANDS_JS
+def test_autocomplete_allowlist_is_exact_parity_with_dispatchers():
+    """#6951 (re-gate): the announced allowlist must be in exact agreement with
+    BOTH real dispatch authorities -- api/commands.py's _ALLOWED_AGENT_COMMANDS
+    (the /api/commands/exec allowlist) and messages.js's
+    _AGENT_COMMANDS_RUN_ON_WEBUI (the send() dispatch set). This compares the
+    actual parsed sets, so drift in ANY authority fails the test: a command
+    added to one allowlist but not the others, a renamed alias, or a removed
+    entry all break parity here."""
+    announced = _extract_js_set_members(COMMANDS_JS, "_WEBUI_DISPATCHABLE_AGENT_COMMANDS")
+    dispatched = _extract_js_set_members(MESSAGES_JS, "_AGENT_COMMANDS_RUN_ON_WEBUI")
+
+    # The backend-exec family must agree exactly across all three authorities
+    # (canonical names after alias normalization).
+    backend_allowed = set(_ALLOWED_AGENT_COMMANDS)
+    assert _canonical_agent_names(dispatched) == backend_allowed
+    assert _canonical_agent_names(announced) & backend_allowed == backend_allowed
+
+    # Announced backend-exec commands are exactly the backend-exec allowlist
+    # (no announced exec command outside /api/commands/exec, none missing).
+    announced_backend = _canonical_agent_names(announced) & backend_allowed
+    assert announced_backend == backend_allowed
+
+    # Every remaining announced command must be a native WebUI behavior that
+    # send() handles without an agent round-trip (moa/sessions/resume/pet).
+    native = announced - announced_backend
+    assert native == {"moa", "sessions", "resume", "pet"}
+
+    # The plugin transport is parity by rule, not by list: the filter accepts
+    # category==='Plugin' and the dispatcher routes the exact same value.
+    assert "category==='Plugin'" in COMMANDS_JS
+    assert "_agentCmd.category==='Plugin'" in MESSAGES_JS
 
 
 def test_busy_path_intercepts_stop_before_mode_routing():
@@ -759,3 +808,195 @@ def test_busy_path_intercepts_stop_before_mode_routing():
     busy_block = MESSAGES_JS[busy_idx:mode_idx]
     assert "['steer','interrupt','queue','terminal','goal','yolo','stop']" in busy_block
     assert "cmdStop" in busy_block or "COMMANDS.find(c=>c.name===_pc.name)" in busy_block
+
+
+def _run_production_autocomplete_js(commands_payload: list[dict], script_body: str) -> dict:
+    """Run the REAL getMatchingCommands/autocomplete pipeline from commands.js
+    against a /api/commands payload (defaults to the real hermes_cli registry
+    serialized exactly as list_commands() produces it)."""
+    script = textwrap.dedent(
+        f"""
+        const vm = require('vm');
+        const ctx = {{
+          console,
+          localStorage: {{ getItem(){{return null;}}, setItem(){{}}, removeItem(){{}} }},
+          t: (key) => key,
+          api: async (path) => {{
+            if (path === '/api/commands') return {{ commands: {json.dumps(commands_payload)} }};
+            if (path === '/api/commands/bundles') return {{ bundles: [] }};
+            if (path === '/api/skills') return {{ skills: [] }};
+            throw new Error('unexpected api path: ' + path);
+          }}
+        }};
+        vm.createContext(ctx);
+        vm.runInContext({json.dumps(COMMANDS_JS)}, ctx);
+        (async () => {{
+          const result = await vm.runInContext(`(async () => {{
+            {script_body}
+          }})()`, ctx);
+          process.stdout.write(JSON.stringify(result));
+        }})().catch(err => {{
+          console.error(err && err.stack || err);
+          process.exit(1);
+        }});
+        """
+    )
+    with tempfile.NamedTemporaryFile("w", suffix=".js", encoding="utf-8", delete=False) as handle:
+        handle.write(script)
+        script_path = Path(handle.name)
+    try:
+        proc = subprocess.run(["node", str(script_path)], check=True, capture_output=True, text=True)
+    finally:
+        script_path.unlink(missing_ok=True)
+    return json.loads(proc.stdout)
+
+
+def _real_registry_payload() -> list[dict] | None:
+    """Serialize the real hermes_cli COMMAND_REGISTRY via the production
+    list_commands() path. Returns None when hermes_cli is unavailable."""
+    try:
+        return list_commands()
+    except Exception:
+        return None
+
+
+def test_real_registry_announced_commands_are_all_dispatchable():
+    """#6951 (re-gate): with the REAL hermes_cli registry as /api/commands
+    payload, every command the production autocomplete announces must be
+    dispatchable -- backend-exec via /api/commands/exec, a native WebUI
+    behavior, or a plugin command. /agents (in the registry, NOT dispatched)
+    must not be announced. This is the announced <= dispatched invariant
+    exercised against the real production data path."""
+    payload = _real_registry_payload()
+    if payload is None:
+        pytest.skip("hermes_cli registry unavailable")
+    announced_by_prefix = {}
+    for cmd in payload:
+        name = str(cmd.get("name") or "")
+        if not name:
+            continue
+        result = _run_production_autocomplete_js(
+            payload,
+            f"""
+            await loadAgentCommandMetadata(true);
+            const matches = await getSlashAutocompleteMatches('/{name}');
+            return matches.map(item => ({{ name: item.name, source: item.source }}));
+            """,
+        )
+        announced_by_prefix[name] = result
+
+    backend_allowed = set(_ALLOWED_AGENT_COMMANDS)
+    for name, rows in announced_by_prefix.items():
+        for row in rows:
+            if row["source"] not in ("agent", "plugin"):
+                continue  # builtin/subarg/skill rows are out of scope
+            assert row["name"] in backend_allowed or row["name"] in (
+                "moa",
+                "sessions",
+                "resume",
+                "pet",
+            ), f"{row['name']} announced but not dispatchable"
+
+    # /agents is in the real registry but must not be announced (it is neither
+    # backend-exec nor a native WebUI behavior nor a plugin).
+    assert announced_by_prefix.get("agents", []) == [], announced_by_prefix.get("agents")
+
+
+def test_alias_typed_text_reaches_exec_dispatch():
+    """#6951 (re-gate): typing an underscore alias (/reload_skills) must
+    resolve through getAgentCommandMetadata to the canonical name, which is
+    what the dispatcher allowlist tests -- so alias forms dispatch without
+    needing their own autocomplete row."""
+    result = _run_commands_js(
+        """
+        const byAlias = await getAgentCommandMetadata('reload_skills');
+        const canonical = byAlias && byAlias.name;
+        return { canonical, dispatched: canonical === 'reload-skills' };
+        """
+    )
+    assert result["canonical"] == "reload-skills"
+    assert result["dispatched"] is True
+
+
+def _run_busy_intercept_js(script_body: str) -> dict:
+    """Execute the REAL busy-path slash intercept block from messages.js with
+    the REAL COMMANDS table + parseCommand from commands.js. This exercises the
+    production routing branch -- not just its source text.
+
+    The harness defines `async function busyIntercept(text, literalSlash)` in
+    the vm context (so it closes over the real COMMANDS/parseCommand) and the
+    script_body then calls it. The block's inner `return;` exits run() early
+    when the command is intercepted, so `intercepted: true` is returned."""
+    block = _extract_busy_intercept_block()
+    script = textwrap.dedent(
+        f"""
+        const vm = require('vm');
+        const calls = [];
+        const ctx = {{
+          console,
+          localStorage: {{ getItem(){{return null;}}, setItem(){{}}, removeItem(){{}} }},
+          t: (key) => key,
+          S: {{
+            busy: true,
+            activeStreamId: 'stream-1',
+            session: {{ session_id: 'sess-1' }},
+            pendingFiles: []
+          }},
+          cancelStream: async (reason) => {{ calls.push('cancelStream:' + reason); return true; }},
+          showToast: () => {{}},
+          $: () => ({{ value: '' }}),
+          autoResize: () => {{}},
+          api: async () => ({{}}),
+          _trySteer: async () => {{ calls.push('steer'); }},
+          queueSessionMessage: () => {{ calls.push('queue'); }}
+        }};
+        vm.createContext(ctx);
+        vm.runInContext({json.dumps(COMMANDS_JS)}, ctx);
+        vm.runInContext(`async function busyIntercept(text, literalSlash){{
+          let reachedModeRouting = false;
+          const run = async () => {{
+            {block}
+            reachedModeRouting = true;
+          }};
+          await run();
+          return {{ intercepted: !reachedModeRouting }};
+        }}`, ctx);
+        (async () => {{
+          const result = await vm.runInContext(`(async () => {{
+            {script_body}
+          }})()`, ctx);
+          process.stdout.write(JSON.stringify({{ result, calls }}));
+        }})().catch(err => {{
+          console.error(err && err.stack || err);
+          process.exit(1);
+        }});
+        """
+    )
+    with tempfile.NamedTemporaryFile("w", suffix=".js", encoding="utf-8", delete=False) as handle:
+        handle.write(script)
+        script_path = Path(handle.name)
+    try:
+        proc = subprocess.run(["node", str(script_path)], check=True, capture_output=True, text=True)
+    finally:
+        script_path.unlink(missing_ok=True)
+    return json.loads(proc.stdout)
+
+
+def test_busy_stop_executes_real_cancel_branch():
+    """#6951 (re-gate): executing the real busy-intercept branch from messages.js
+    with the real COMMANDS table must route /stop to cmdStop -> cancelStream,
+    and must NOT fall through to steer/queue routing."""
+    out = _run_busy_intercept_js(
+        """
+        const r1 = await busyIntercept('/stop', false);
+        const r2 = await busyIntercept('/agents', false);
+        return { stop: r1, agents: r2 };
+        """
+    )
+    # /stop is intercepted by the busy branch (cmdStop -> cancelStream ran,
+    # mode routing never reached), /agents is NOT a busy-control command so it
+    # falls through to mode routing (documented: it is no longer announced).
+    assert out["result"]["stop"] == {"intercepted": True}
+    assert out["result"]["agents"] == {"intercepted": False}
+    assert any(c.startswith("cancelStream:slash-stop") for c in out["calls"]), out["calls"]
+    assert not any(c.startswith("steer") for c in out["calls"]), out["calls"]