feat(commands): slash command parity with hermes-agent (batch 1)

[renheqiang]

Summary

• New GET /api/commands exposes hermes-agent's COMMAND_REGISTRY to the webui; static/commands.js loads dynamically at boot — future agent commands appear in the dropdown automatically (single source of truth, no drift)
• New slash commands: /retry, /undo, /stop, /title, /status — behavior parity with gateway/run.py:_handle_*_command
• Behavior fixes:
  • /usage now displays current token usage (was: toggling a settings flag)
  • /compact now toasts "deferred — use CLI for now" (was: silently sending free text to the LLM asking it to compress)
• Critical safety guarantee: unknown/deferred slash commands (/yolo, /reasoning, /fast, /compress, etc.) now toast "Web UI not yet supported" instead of being silently forwarded to the LLM as plain text — eliminates the silent-failure mode where the model could invent fake tool calls
• Full i18n in 5 locales (en, zh, es, de, zh-Hant)

Deferred to a later batch (state lives in the agent process — needs an IPC channel to implement properly): /yolo, /reasoning, /fast, /compress, /voice, /branch, /rollback, /snapshot, /resume, /btw, /background, /queue, /profile, /provider, /insights, /debug, /reload, /reload-mcp, /approve, /deny. These show in the dropdown but toast "not yet supported" rather than running.

Test plan

• 21 new pytest cases — tests/test_commands_endpoint.py (7) + tests/test_session_ops.py (14)
• Full webui suite passes; tests/test_regressions.py::test_skills_slash_command_defined updated for the registry-driven model
• Playwright end-to-end checklist: 29/29 passed — including the no-silent-forward invariant for /yolo, /reasoning, /compact
• Manual checklist added to TESTING.md under "Slash command parity"
• /retry no-double-append guard verified by automated test (test_retry_does_not_double_append)
• Per-session lock guards added to retry_last/undo_last to prevent concurrent read-modify-write races (lock pattern documented inline because Session.save() re-acquires the same non-reentrant LOCK and would deadlock if held)

Notes for reviewer

• 15 commits, all focused on slash command parity (no unrelated drive-by changes)
• Branched off current nesquena/master and rebased clean
• Bug found and fixed during Playwright testing: deferred commands had been put in UNSUPPORTED_IN_WEBUI (which filters them out of the registry entirely), causing /yolo etc. to fall through to send() and get dispatched as plain text. Moved them to the "in-registry-but-no-handler" bucket so they toast properly. See commit 6fd0c14.

🤖 Generated with Claude Code

[nesquena-hermes]

Thanks for this thorough PR — the scope is well-defined, the commit history is clean and readable, and the safety guarantee around deferred commands is genuinely important to fix.

Architecture ✅

The GET /api/commands approach is the right call. A single source of truth from COMMAND_REGISTRY prevents drift and means new agent commands appear in the dropdown automatically. The dynamic load at boot (commands.js fetches on startup) is clean.

Behaviour fixes ✅

• /usage — the old toggle-a-settings-flag behaviour was a footgun. Showing actual usage inline is the right design.
• /compact — the old implementation sent a literal user message to the LLM ("Please compress..."), which is wrong for two reasons: it burns tokens and the model could invent a fake tool call. Deferring with a toast is correct for now.
• Deferred commands no longer fall through to send() — the UNSUPPORTED_IN_WEBUI vs "in-registry-but-no-handler" distinction is important and the commit message (6fd0c14) documents it well.

Specific review questions / items to verify

1. Lock pattern in session_ops.py (critical path)

The fix(session_ops): guard retry_last read-modify-write with LOCK commit notes that LOCK is non-reentrant and get_session() + Session.save() both acquire it internally — so they're placed outside the new critical section. This is correct but fragile: a future refactor that moves get_session() inside the with LOCK: block would self-deadlock silently. A comment in the code calling this out (as the commit message says was done) is important — please confirm it reads clearly in the final diff and isn't just in the commit message.

2. /retry double-append guard

The test test_retry_does_not_double_append is mentioned — make sure this test exercises the concurrent path (two simultaneous calls), not just the sequential idempotence case. If it's sequential-only, the lock guard is tested but the race isn't.

3. GET /api/commands endpoint — what happens if the agent isn't running?

If COMMAND_REGISTRY is unavailable or the endpoint 500s, does commands.js fall back gracefully? I'd expect a fallback to the existing hardcoded list (or at minimum, an error that doesn't break the chat UI entirely). The PR description doesn't mention this case.

4. UNSUPPORTED_IN_WEBUI contents

The commit note says this now contains "truly-irrelevant CLI concepts (voice, paste, image, skin, browser, plan, config, etc.)". One question: /voice — there is an existing TTS/voice feature in the WebUI (#499 is open, and some work exists). Should /voice remain in UNSUPPORTED_IN_WEBUI or should it be in the "deferred but toasts" bucket like /yolo? The current treatment would silently drop it from the dropdown entirely.

5. i18n completeness

The commit message mentions 32 new keys across 5 locales (en, es, de, zh, zh-Hant). The regression test confirms Spanish coverage — but is there a test for de, zh, zh-Hant key completeness, or is it a manual check? Worth noting if it's manual so it doesn't silently regress when batch-2 adds more keys.

6. Playwright checklist

"29/29 passed" is great. Is this checklist automated (part of CI) or manual? If manual, it would be good to note that in TESTING.md so future reviewers know to re-run it before merging batch-2.

Summary

This is a solid, well-scoped PR. The safety fix (deferred commands toast instead of leaking to LLM) alone is worth merging. Items 1–2 are the most important to confirm before merge; items 3–6 are nice-to-have or follow-up candidates.

Happy to approve after the lock comment and /api/commands fallback questions are addressed.

[nesquena-hermes]

Thanks for this — the command parity work is solid in scope and the JS all passes node --check. Three required fixes before merge:

Required:

1. Stale-object TOCTOU in retry_last/undo_last — get_session() acquires+releases LOCK then returns an object reference. On a cache-miss, a second concurrent thread can load a different Python object for the same session. Both threads then enter with LOCK: serially, but each mutates a different s — the second s.save() overwrites the first's state with a stale copy. Fix: re-bind to the canonical object inside the critical section: s = SESSIONS.get(session_id, s). Or explicitly document in the inline comment that this race exists for the disk-cache-miss path.

2. test_retry_does_not_double_append tests sequential calls, not concurrent ones — the PR describes this as a concurrency fix, but the test makes a single _post() call. Please add a concurrent.futures.ThreadPoolExecutor(max_workers=2) test that fires two simultaneous retries against the same session and asserts the transcript stays consistent.

3. /voice in UNSUPPORTED_IN_WEBUI should be in the deferred/toast bucket instead — the WebUI already has voice input (mic button + /api/transcribe). Hiding /voice entirely means a user typing it gets no feedback and the text falls through as plain LLM input. Move it to a HANDLERS.voice = () => showToast(...) with a message like "Use the mic button in the composer for voice input".

Advisory (non-blocking):

• cmdStatus inserts raw session title/model/workspace into markdown template strings — not XSS (renderMd escapes HTML), but a title like **bold** renders as bold. Consider wrapping data values in backticks.
• If /api/commands returns HTTP 200 with {"error": "..."} (no commands field), the fallback path isn't triggered and no console.warn fires — functional but silent.

Happy to re-review once those three are addressed.

[nesquena]

Independent End-to-End Review — PR #618

Independent review picking up the 3 blockers from the prior review. All three pushed in commit d830a1e.

TL;DR

Now merge-ready. All three required fixes from the second review pass are now in the branch. Tests green (1348 passed, 0 failed). One remaining concern (CHANGELOG/version bump) deferred to merge time given the multi-PR coordination already discussed.

Blocker resolution status — d830a1e

1. ✅ Stale-object TOCTOU in retry_last/undo_last (FIXED)

The race the reviewer flagged is real:

Thread A: get_session(sid) → cache miss → loads from disk → S_A
Thread B: get_session(sid) → cache miss → loads from disk → S_B (different instance)
Thread A: SESSIONS[sid] = S_A
Thread B: SESSIONS[sid] = S_B  ← clobbers S_A
Thread A: enters `with LOCK:`, mutates S_A.messages, releases
Thread B: enters `with LOCK:`, mutates S_B.messages, releases
Thread A: S_A.save() → writes truncated state for S_A
Thread B: S_B.save() → writes truncated state for S_B (OVERWRITES Thread A's save)

Fix in api/session_ops.py:

s = get_session(session_id)
with LOCK:
    s = SESSIONS.get(session_id, s)  # re-bind to canonical instance
    history = s.messages or []
    ...

The , s) fallback handles the unlikely case where the cache was evicted between get_session() and the lock acquisition (LRU + concurrent eviction). Same fix applied to both retry_last and undo_last. Comment added to retry_last explaining the rationale; undo_last references back to retry_last to keep the comment DRY.

2. ✅ Concurrent test added

test_retry_concurrent_requests_are_safe in tests/test_session_ops.py:

• Imports session with 4 messages (2 user / 2 assistant pairs)
• Fires 4 concurrent /api/session/retry calls via ThreadPoolExecutor
• Asserts the resulting transcript is one of the valid strict-prefix states [], [(user, msg A), (assistant, reply A)], or [(user, msg A)]
• Critical assertion: there is no phantom duplicate of the resent message

The test is structural (asserts the invariant) rather than racing for a specific failure mode — it would fail reliably if the TOCTOU re-emerged, but doesn't depend on hitting a specific scheduling window.

3. ✅ /voice deferred → toast

/voice removed from UNSUPPORTED_IN_WEBUI set and registered as a real command handler (cmdVoice):

function cmdVoice(){
  // /voice is supported via the mic button (#btnMic) in the composer —
  // the slash command is a discoverability alias that triggers a click
  // when the button is visible (Web Speech API or MediaRecorder is
  // available), or a toast pointing the user to it.
  const mic = document.getElementById('btnMic');
  if(mic && mic.style.display !== 'none' && !mic.disabled){
    try{ mic.click(); return; }catch(_){ /* fall through to toast */ }
  }
  showToast(t('cmd_voice_use_mic'));
}

This is better than the reviewer's minimal "show a toast" suggestion — when the mic button is available (most users), /voice actually triggers it. Falls back to the toast when the browser doesn't support voice input or the button isn't rendered. i18n strings added in all 5 locales (en, es, de, zh-CN, zh-TW).

Security audit ✅

Architecture from the prior review confirmed clean. The new TOCTOU fix is purely defensive — adds a read inside a lock that was already held. No new endpoints, no new auth surface, no new user input paths.

The /voice handler uses getElementById('btnMic') and only calls .click() on a visible+non-disabled button — no script injection, no message-body forwarding to LLM.

Test results ✅

• 1348 passed, 47 skipped, 0 failed (full suite in isolated worktree)
• All 15 tests in test_session_ops.py pass, including the new concurrent test
• node --check passes on static/commands.js and static/i18n.js
• CI green on Python 3.11/3.12/3.13

Remaining items

• CHANGELOG / version bump: not added — left to merge time given the multi-PR version coordination discussed across fix: skill panel auto-open, thinking scroll, nav icon alignment, Safari zoom — closes #643 #638 #636 #630 #647/fix(streaming): strip Gemma 4 thinking tokens — closes #607 #648/fix(config): default model empty string — no more unavailable OpenAI model for non-OpenAI users — closes #646 #649/fix(title): auto-title extraction for tool-heavy first turns #640/feat: /compress with focus topic in Web UI (replace /compact, add transcript-aware UX) #619/feat: custom wallpaper background with brightness adjustment #624/Replace color scheme system with light/dark theme + accent skins #627. Whichever order these merge in, this should claim the next available patch slot.

Summary

Aspect	Status
Tests	✅ 1348 passed, 0 failed
Blocker 1: TOCTOU	✅ Fixed in d830a1e
Blocker 2: concurrent test	✅ Added in d830a1e
Blocker 3: /voice deferred	✅ Improved (auto-clicks mic when available)
Security	✅ Clean
CHANGELOG / version	⏳ At merge time (multi-PR coordination)

All three blockers from the prior review are now actually addressed. Thanks @renheqiang for the substantial command-parity work — happy to push one more round of fixes if the maintainer flags anything else on a final pass.

[nesquena-hermes]

Thank you @renheqiang! Your slash command parity work has been combined with PR #701 (@franksong2702) into PR #711. Both contributors are credited. The combined PR brings /stop, /title, /retry, /undo, /status, /voice commands + skill autocomplete — 862 lines, 27 new tests, all passing.

[nesquena-hermes]

Superseded by combined PR #711 which includes this work plus PR #701. Full credit preserved — Co-authored-by trailer in the commit.