diff --git a/static/commands.js b/static/commands.js index 685cbf5c..fa787e24 100644 --- a/static/commands.js +++ b/static/commands.js @@ -1,3 +1,7 @@ +const _WEBUI_DISPATCHABLE_AGENT_COMMANDS = new Set([ + 'reload-mcp','reload-skills','codex-runtime','credits', + 'moa','sessions','resume','pet' +]); // ── Slash commands ────────────────────────────────────────────────────────── // Built-in commands intercepted before send(). Each command runs locally // (no round-trip to the agent) and shows feedback via toast or local message. @@ -182,6 +186,31 @@ function executeCommand(text){ return {noEcho:!!cmd.noEcho}; } +// Agent-registry slash commands that the WebUI actually dispatches when +// 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. +// +// 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. + +function _isWebuiDispatchableAgentCommand(cmd){ + const name=String(cmd&&cmd.name||'').trim().toLowerCase(); + if(_WEBUI_DISPATCHABLE_AGENT_COMMANDS.has(name))return true; + // Plugin-registered commands execute via the /api/commands/exec plugin transport. + return String(cmd&&cmd.category||'').trim()==='Plugin'; +} + function getMatchingCommands(prefix){ const q=prefix.toLowerCase(); const matches=COMMANDS.filter(c=>c.name.startsWith(q)).map(c=>({...c,source:'builtin'})); @@ -214,9 +243,14 @@ function getMatchingCommands(prefix){ const name=String(cmd&&cmd.name||'').toLowerCase(); if(!name.startsWith(q)||seen.has(name))continue; if(cmd.cli_only&&name!=='pet')continue; + // #6951: only announce commands send() actually dispatches -- anything + // else would silently fall through to the normal chat path as plain text. + if(!_isWebuiDispatchableAgentCommand(cmd))continue; 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); diff --git a/static/messages.js b/static/messages.js index 92a10712..d1dbac12 100644 --- a/static/messages.js +++ b/static/messages.js @@ -1,3 +1,13 @@ +const _AGENT_COMMAND_ALIASES = { + 'reload_mcp': 'reload-mcp', + 'reload_skills': 'reload-skills', + 'codex_runtime': 'codex-runtime', + 'credits': 'credits' +}; +const _AGENT_COMMANDS_RUN_ON_WEBUI = new Set([ + 'reload-mcp','reload-skills','codex-runtime','credits', + 'reload_mcp','reload_skills','codex_runtime','credits' +]); function _markSessionViewed(sid, messageCount) { if(typeof _setSessionViewedCount!=='function' || !sid) return; const next = Number.isFinite(messageCount) ? Number(messageCount) : 0; @@ -1184,7 +1194,6 @@ const _sessionTitleProvisionalBySid = new Map(); // their canonical command is registered on the backend (for example // /reload-mcp). Keep this intentionally narrow and include underscore variants // observed by users so typing either form still routes through executeAgentCommand. -const _AGENT_COMMANDS_RUN_ON_WEBUI = new Set(['reload-mcp', 'reload_mcp', 'reload-skills', 'reload_skills', 'codex-runtime', 'codex_runtime', 'credits']); function _clearStaleBusyStateBeforeSend({compressionRunning=false}={}){ if(!S||!S.busy||compressionRunning) return false; @@ -1409,14 +1418,16 @@ async function send(){ if(!S.session){await newSession();await renderSessionList();} // Busy-control slash commands must be intercepted HERE, before the // defaultMessageMode routing block, so the user can always type /steer, /interrupt, - // /queue, /terminal, /goal, or /yolo while the agent is running and have + // /queue, /terminal, /goal, /yolo, or /stop while the agent is running and have // them execute immediately. // Without this intercept they fall through to the queue and execute after // the current turn ends — by which point there is no active stream and // cmdSteer / cmdInterrupt say "No active task to stop." + // /stop must cancel the active run immediately instead of being steered + // or queued as the literal text "/stop" (#6951). if(text.startsWith('/')&&!literalSlash){ const _pc=typeof parseCommand==='function'&&parseCommand(text); - if(_pc&&['steer','interrupt','queue','terminal','goal','yolo'].includes(_pc.name)){ + if(_pc&&['steer','interrupt','queue','terminal','goal','yolo','stop'].includes(_pc.name)){ const _bc=COMMANDS.find(c=>c.name===_pc.name); if(_bc){ $('msg').value='';autoResize(); diff --git a/tests/test_cli_only_slash_commands.py b/tests/test_cli_only_slash_commands.py index 9ac5c2d6..aca56d7a 100644 --- a/tests/test_cli_only_slash_commands.py +++ b/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 = [ @@ -163,6 +196,30 @@ def _run_commands_js(script_body: str) -> dict: aliases: ['plugin_review'], cli_only: false, gateway_only: false + }}, + {{ + name: 'agents', + description: 'Manage background agents', + category: 'Session', + aliases: ['tasks'], + cli_only: false, + gateway_only: false + }}, + {{ + name: 'sessions', + description: 'List sessions', + category: 'Session', + aliases: [], + cli_only: false, + gateway_only: false + }}, + {{ + name: 'resume', + description: 'Resume a session', + category: 'Session', + aliases: [], + cli_only: false, + gateway_only: false }} ] }}; @@ -356,8 +413,8 @@ def test_cli_only_slugs_reserve_skill_autocomplete_namespace(): assert result["delegate_names"] == [] assert result["incident_names"] == ["incident-review"] assert result["incident_sources"] == ["bundle"] - assert result["triage_names"] == ["triage-review"] - assert result["triage_sources"] == ["agent"] + assert result["triage_names"] == [] + assert result["triage_sources"] == [] assert result["plugin_names"] == ["plugin-review"] assert result["plugin_sources"] == ["plugin"] assert "skills" in result["skills_names"] @@ -659,3 +716,374 @@ def test_builtin_command_opt_outs_do_not_hit_agent_metadata_lookup(): assert optout_idx != -1 assert metadata_idx != -1 assert "if(_parsedCmd&&!_cmd)" in intercept[optout_idx:metadata_idx + 120] + + +# ── #6951: autocomplete must only announce commands the WebUI dispatches ───── + + +def test_non_dispatchable_agent_command_hidden_from_autocomplete(): + """#6951: registry commands the WebUI does not dispatch (e.g. /agents) must + not be advertised, since submitting them would fall through to plain chat.""" + result = _run_commands_js( + """ + await loadAgentCommandMetadata(true); + const agents = await getSlashAutocompleteMatches('/agents'); + const ag = await getSlashAutocompleteMatches('/ag'); + return { + agents_names: agents.map(item => item.name), + ag_names: ag.map(item => item.name) + }; + """ + ) + assert result["agents_names"] == [] + assert result["ag_names"] == [] + + +def test_dispatchable_agent_commands_stay_in_autocomplete(): + """#6951: commands the WebUI does dispatch -- backend exec allowlist and + native WebUI behaviors -- must keep appearing in autocomplete.""" + result = _run_commands_js( + """ + await loadAgentCommandMetadata(true); + const reload = await getSlashAutocompleteMatches('/reload'); + const sessions = await getSlashAutocompleteMatches('/sessions'); + const resume = await getSlashAutocompleteMatches('/resume'); + const plugin = await getSlashAutocompleteMatches('/plugin'); + return { + reload_names: reload.map(item => item.name), + reload_sources: reload.map(item => item.source), + sessions_names: sessions.map(item => item.name), + resume_names: resume.map(item => item.name), + plugin_names: plugin.map(item => item.name) + }; + """ + ) + assert result["reload_names"] == ["reload-skills"] + assert result["reload_sources"] == ["agent"] + assert result["sessions_names"] == ["sessions"] + assert result["resume_names"] == ["resume"] + assert result["plugin_names"] == ["plugin-review"] + + +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 _run_shared_classic_realm_js() -> dict: + """Evaluate commands.js then messages.js in ONE vm realm -- exactly how + static/index.html loads them (consecutive non-module `defer` classic + scripts share a single global lexical realm). A duplicate top-level + `const` across the two files throws during the second script's + declaration instantiation and aborts it before any statement runs, so the + second script's dispatch consts would never bind. Browser-only globals + referenced at load time are stubbed; runtime (call-time) references are + out of scope for this guard. + """ + script = textwrap.dedent( + f""" + const vm = require('vm'); + const ctx = vm.createContext({{ + console, + localStorage: {{ getItem(){{return null;}}, setItem(){{}}, removeItem(){{}} }}, + t: (key) => key, + document: {{ addEventListener(){{}}, getElementById(){{ return null; }}, hidden: false }}, + window: {{ addEventListener(){{}}, speechSynthesis: {{ speaking: false, paused: false }} }}, + location: {{ href: 'http://localhost/', protocol: 'http:', host: 'localhost' }}, + navigator: {{ userAgent: 'test' }}, + setTimeout: () => 0, clearTimeout: () => 0, + setInterval: () => 0, clearInterval: () => 0, + requestAnimationFrame: () => 0, cancelAnimationFrame: () => 0, + fetch: async () => ({{}}), + EventSource: function(){{}}, + MutationObserver: function(){{ this.observe=()=>{{}}; }}, + Blob: function(){{}}, FileReader: function(){{ this.readAsText=()=>{{}}; }}, + URL: function(){{ this.href=''; }}, FormData: function(){{}}, + WebSocket: function(){{}}, + crypto: {{ getRandomValues: (arr) => arr }}, + CustomEvent: function(){{}} + }}); + vm.runInContext({json.dumps(COMMANDS_JS)}, ctx); + let secondError = null; + try {{ + vm.runInContext({json.dumps(MESSAGES_JS)}, ctx); + }} catch (e) {{ + secondError = String(e && e.message || e); + }} + const bound = vm.runInContext(`({{ + runOnWebui: typeof _AGENT_COMMANDS_RUN_ON_WEBUI, + dispatchable: typeof _WEBUI_DISPATCHABLE_AGENT_COMMANDS, + backendConst: typeof _BACKEND_EXECUTED_AGENT_COMMANDS + }})`, ctx); + process.stdout.write(JSON.stringify({{ secondError, ...bound }})); + """ + ) + 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_commands_and_messages_evaluate_in_one_shared_classic_script_realm(): + """#6962 (re-gate): commands.js and messages.js must coexist as classic + scripts in the same global realm (static/index.html loads them back to + back). A duplicate top-level `const _BACKEND_EXECUTED_AGENT_COMMANDS` + across both files previously threw + `SyntaxError: Identifier ... has already been declared` on messages.js, + aborting its evaluation entirely. Both files must evaluate cleanly in one + realm, the dispatch consts of BOTH files must be bound, and the removed + duplicate authority mirror must not exist in the shared namespace.""" + out = _run_shared_classic_realm_js() + assert out["secondError"] is None, ( + f"messages.js failed to evaluate after commands.js in the shared realm: " + f"{out['secondError']}" + ) + assert out["runOnWebui"] == "object", ( + "messages.js dispatch const _AGENT_COMMANDS_RUN_ON_WEBUI not bound in " + "the shared realm -- declaration instantiation was aborted" + ) + assert out["dispatchable"] == "object", ( + "commands.js dispatch const _WEBUI_DISPATCHABLE_AGENT_COMMANDS not " + "bound in the shared realm" + ) + assert out["backendConst"] == "undefined", ( + "duplicate/unused _BACKEND_EXECUTED_AGENT_COMMANDS mirror must not be " + "declared in the shared realm (declared once -> it would collide; " + "declared in one file -> it is dead authority drift)" + ) + + +def test_busy_path_intercepts_stop_before_mode_routing(): + """#6951: while busy, /stop must cancel the active run immediately instead + of being steered/queued as the literal text '/stop'.""" + busy_idx = MESSAGES_JS.find("Busy-control slash commands must be intercepted") + assert busy_idx != -1 + mode_idx = MESSAGES_JS.find("const defaultMessageMode=", busy_idx) + assert mode_idx != -1 + 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"]