diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 0000000000..3854c8f930 --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,44 @@ +# Dependabot configuration for hermes-agent. +# +# Deliberately scoped to github-actions only. +# +# We do NOT enable Dependabot for pip / npm / any source-dependency ecosystem +# because we pin source dependencies exactly (uv.lock, package-lock.json) as +# part of our supply-chain posture. Automatic version-bump PRs against those +# pins would undermine the strategy — pins are moved deliberately, after +# review, not on a schedule. +# +# github-actions is the exception: action pins (we use full commit SHAs per +# supply-chain policy) must be updated when upstream actions publish +# patches — usually themselves security fixes. Dependabot opens a PR with +# the new SHA and release notes; we review and merge like any other PR. +# +# Security-update PRs for source dependencies (opened ONLY when a CVE is +# published affecting a currently-pinned version) are enabled separately +# via the repo's Dependabot security updates setting +# (Settings → Code security → Dependabot → Dependabot security updates). +# Those are CVE-only, not schedule-driven, and do not conflict with our +# pinning strategy — they fire when a pinned version becomes known-bad, +# which is exactly when we want to move the pin. + +version: 2 +updates: + - package-ecosystem: "github-actions" + directory: "/" + schedule: + interval: "weekly" + day: "monday" + open-pull-requests-limit: 5 + labels: + - "dependencies" + - "github-actions" + commit-message: + prefix: "chore(actions)" + include: "scope" + groups: + # Batch routine action bumps into one PR per week to reduce noise. + # Security updates still open individually and bypass grouping. + actions-minor-patch: + update-types: + - "minor" + - "patch" diff --git a/.github/workflows/osv-scanner.yml b/.github/workflows/osv-scanner.yml new file mode 100644 index 0000000000..db8c3d75ce --- /dev/null +++ b/.github/workflows/osv-scanner.yml @@ -0,0 +1,67 @@ +name: OSV-Scanner + +# Scans lockfiles (uv.lock, package-lock.json) against the OSV vulnerability +# database. Runs on every PR that touches a lockfile and on a weekly schedule +# against main. +# +# This is detection-only — OSV-Scanner does NOT open PRs or modify pins. +# It reports known CVEs in currently-pinned dependency versions so we can +# decide when and how to patch on our own schedule. Our pinning strategy +# (full SHA / exact version) is preserved; only the notification signal +# is added. +# +# Complements the existing supply-chain-audit.yml workflow (which scans +# for malicious code patterns in PR diffs) by covering the orthogonal +# "currently-pinned dep became known-vulnerable" case. +# +# Uses Google's officially-recommended reusable workflow, pinned by SHA. +# Findings land in the repo's Security tab (Code Scanning > OSV-Scanner). +# fail-on-vuln is disabled so the job does not block merges on pre-existing +# vulnerabilities in pinned deps that we may need to patch deliberately. + +on: + pull_request: + branches: [main] + paths: + - 'uv.lock' + - 'pyproject.toml' + - 'package.json' + - 'package-lock.json' + - 'ui-tui/package.json' + - 'ui-tui/package-lock.json' + - 'website/package.json' + - 'website/package-lock.json' + - '.github/workflows/osv-scanner.yml' + push: + branches: [main] + paths: + - 'uv.lock' + - 'pyproject.toml' + - 'package.json' + - 'package-lock.json' + - 'ui-tui/package-lock.json' + - 'website/package-lock.json' + schedule: + # Weekly scan against main — catches CVEs published after merge for + # deps that haven't changed since. + - cron: '0 9 * * 1' + workflow_dispatch: + +permissions: + # Required by the reusable workflow to upload SARIF to the Security tab. + actions: read + contents: read + security-events: write + +jobs: + scan: + name: Scan lockfiles + uses: google/osv-scanner-action/.github/workflows/osv-scanner-reusable.yml@c51854704019a247608d928f370c98740469d4b5 # v2.3.5 + with: + # Scan explicit lockfiles rather than recursing, so we only look at + # the three sources of truth and skip vendored / test / worktree dirs. + scan-args: |- + --lockfile=uv.lock + --lockfile=ui-tui/package-lock.json + --lockfile=website/package-lock.json + fail-on-vuln: false diff --git a/cli.py b/cli.py index 96ecb8ecfd..e5af3c88f2 100644 --- a/cli.py +++ b/cli.py @@ -2579,23 +2579,59 @@ class HermesCLI: return f" {txt} ({elapsed_str})" return f" {txt}" + def _voice_record_key_label(self) -> str: + """Return the configured voice push-to-talk key formatted for UI. + + Shared helper so every voice-facing status line / placeholder / + recording hint advertises the SAME label as the registered + prompt_toolkit binding. + + Cached at startup (see ``set_voice_record_key_cache``) rather + than re-read per render. Two reasons (Copilot round-13 on + #19835): + + * The prompt_toolkit binding is registered once at session + start via ``@kb.add(_voice_key)``; re-reading config per + render meant the status bar could advertise a new shortcut + after a config edit while the actual binding was still the + startup chord — exactly the display/binding drift this PR + is trying to eliminate. + * The label is on the hot render path (status bar + composer + placeholder invalidated every 150ms during recording), so + reading config on every call added avoidable UI overhead. + """ + return getattr(self, "_voice_record_key_display_cache", None) or "Ctrl+B" + + def set_voice_record_key_cache(self, raw_key: object) -> None: + """Populate the voice label cache from a raw ``voice.record_key``. + + Called at CLI startup after the prompt_toolkit binding is + registered so the cached label always matches the live binding. + """ + try: + from hermes_cli.voice import format_voice_record_key_for_status + self._voice_record_key_display_cache = format_voice_record_key_for_status(raw_key) + except Exception: + self._voice_record_key_display_cache = "Ctrl+B" + def _get_voice_status_fragments(self, width: Optional[int] = None): """Return the voice status bar fragments for the interactive TUI.""" width = width or self._get_tui_terminal_width() compact = self._use_minimal_tui_chrome(width=width) + label = self._voice_record_key_label() if self._voice_recording: if compact: return [("class:voice-status-recording", " ● REC ")] - return [("class:voice-status-recording", " ● REC Ctrl+B to stop ")] + return [("class:voice-status-recording", f" ● REC {label} to stop ")] if self._voice_processing: if compact: return [("class:voice-status", " ◉ STT ")] return [("class:voice-status", " ◉ Transcribing... ")] if compact: - return [("class:voice-status", " 🎤 Ctrl+B ")] + return [("class:voice-status", f" 🎤 {label} ")] tts = " | TTS on" if self._voice_tts else "" cont = " | Continuous" if self._voice_continuous else "" - return [("class:voice-status", f" 🎤 Voice mode{tts}{cont} — Ctrl+B to record ")] + return [("class:voice-status", f" 🎤 Voice mode{tts}{cont} — {label} to record ")] def _build_status_bar_text(self, width: Optional[int] = None) -> str: """Return a compact one-line session status string for the TUI footer.""" @@ -8270,20 +8306,38 @@ class HermesCLI: return self._voice_recording = True - # Load silence detection params from config - voice_cfg = {} + # Load silence detection params from config. Shape-safe: a + # hand-edited ``voice: true`` / ``voice: cmd+b`` leaves + # ``load_config()['voice']`` as a non-dict; coerce to {} so + # continuous recording falls back to the documented defaults + # instead of crashing on ``.get()``. + voice_cfg: dict = {} try: from hermes_cli.config import load_config - voice_cfg = load_config().get("voice", {}) + _cfg = load_config().get("voice") + voice_cfg = _cfg if isinstance(_cfg, dict) else {} except Exception: pass if self._voice_recorder is None: self._voice_recorder = create_audio_recorder() - # Apply config-driven silence params - self._voice_recorder._silence_threshold = voice_cfg.get("silence_threshold", 200) - self._voice_recorder._silence_duration = voice_cfg.get("silence_duration", 3.0) + # Apply config-driven silence params (numeric-guarded so YAML + # scalar corruption doesn't break recording start-up). + # + # ``bool`` is explicitly excluded from the numeric check — in + # Python bool is a subclass of int, so a hand-edited + # ``silence_threshold: true`` would otherwise be forwarded as + # ``1`` instead of falling back to the 200 default (Copilot + # round-12 on #19835). + _threshold = voice_cfg.get("silence_threshold") + _duration = voice_cfg.get("silence_duration") + self._voice_recorder._silence_threshold = ( + _threshold if isinstance(_threshold, (int, float)) and not isinstance(_threshold, bool) else 200 + ) + self._voice_recorder._silence_duration = ( + _duration if isinstance(_duration, (int, float)) and not isinstance(_duration, bool) else 3.0 + ) def _on_silence(): """Called by AudioRecorder when silence is detected after speech.""" @@ -8309,12 +8363,13 @@ class HermesCLI: with self._voice_lock: self._voice_recording = False raise + _label = self._voice_record_key_label() if getattr(self._voice_recorder, "supports_silence_autostop", True): - _recording_hint = "auto-stops on silence | Ctrl+B to stop & exit continuous" + _recording_hint = f"auto-stops on silence | {_label} to stop & exit continuous" elif _is_termux_environment(): - _recording_hint = "Termux:API capture | Ctrl+B to stop" + _recording_hint = f"Termux:API capture | {_label} to stop" else: - _recording_hint = "Ctrl+B to stop" + _recording_hint = f"{_label} to stop" _cprint(f"\n{_ACCENT}● Recording...{_RST} {_DIM}({_recording_hint}){_RST}") # Periodically refresh prompt to update audio level indicator @@ -8559,10 +8614,12 @@ class HermesCLI: with self._voice_lock: self._voice_mode = True - # Check config for auto_tts + # Check config for auto_tts (shape-safe — malformed ``voice:`` YAML + # leaves ``voice_config`` as a non-dict, so guard before .get()). try: from hermes_cli.config import load_config - voice_config = load_config().get("voice", {}) + _raw_voice = load_config().get("voice") + voice_config = _raw_voice if isinstance(_raw_voice, dict) else {} if voice_config.get("auto_tts", False): with self._voice_lock: self._voice_tts = True @@ -8574,13 +8631,11 @@ class HermesCLI: # _voice_message_prefix property and its usage in _process_message(). tts_status = " (TTS enabled)" if self._voice_tts else "" - try: - from hermes_cli.config import load_config - _raw_ptt = load_config().get("voice", {}).get("record_key", "ctrl+b") - _ptt_key = _raw_ptt.lower().replace("ctrl+", "c-").replace("alt+", "a-") - except Exception: - _ptt_key = "c-b" - _ptt_display = _ptt_key.replace("c-", "Ctrl+").upper() + # Use the startup-pinned cache so the advertised shortcut always + # matches the live prompt_toolkit binding — reading live config + # here would drift after a mid-session config edit (Copilot + # round-14 on #19835, same class as round-13). + _ptt_display = self._voice_record_key_label() _cprint(f"\n{_ACCENT}Voice mode enabled{tts_status}{_RST}") _cprint(f" {_DIM}{_ptt_display} to start/stop recording{_RST}") _cprint(f" {_DIM}/voice tts to toggle speech output{_RST}") @@ -8637,7 +8692,6 @@ class HermesCLI: def _show_voice_status(self): """Show current voice mode status.""" - from hermes_cli.config import load_config from tools.voice_mode import check_voice_requirements reqs = check_voice_requirements() @@ -8646,9 +8700,11 @@ class HermesCLI: _cprint(f" Mode: {'ON' if self._voice_mode else 'OFF'}") _cprint(f" TTS: {'ON' if self._voice_tts else 'OFF'}") _cprint(f" Recording: {'YES' if self._voice_recording else 'no'}") - _raw_key = load_config().get("voice", {}).get("record_key", "ctrl+b") - _display_key = _raw_key.replace("ctrl+", "Ctrl+").upper() if "ctrl+" in _raw_key.lower() else _raw_key - _cprint(f" Record key: {_display_key}") + # Display the startup-pinned label so /voice status always + # matches the live prompt_toolkit binding (Copilot round-14 on + # #19835, same class as round-13). Reading live config here + # would drift after a mid-session config edit. + _cprint(f" Record key: {self._voice_record_key_label()}") _cprint(f"\n {_BOLD}Requirements:{_RST}") for line in reqs["details"].split("\n"): _cprint(f" {line}") @@ -10484,20 +10540,14 @@ class HermesCLI: self._should_exit = True event.app.exit() - @kb.add('c-S-c') # Ctrl+Shift+C - def handle_ctrl_shift_c(event): - """Copy text to clipboard (terminal-native). - - This is a no-op at the application level. Terminal emulators - handle the actual copy operation when Ctrl+Shift+C is pressed. - This binding prevents Hermes from intercepting the keystroke - as an interrupt signal. - - On macOS the standard copy shortcut is Cmd+C (no Hermes binding - needed). On Linux/Windows Ctrl+Shift+C is the conventional - terminal copy shortcut. - """ - return # No-op — let the terminal perform native copy + # Ctrl+Shift+C: no binding needed. Terminal emulators (GNOME Terminal, + # iTerm2, kitty, Windows Terminal, etc.) intercept Ctrl+Shift+C before + # the keystroke reaches the application's stdin — prompt_toolkit never + # sees it, and prompt_toolkit's key spec parser doesn't even recognise + # 'c-S-c' anyway (the Shift modifier is meaningless on control-sequence + # keys). #19884 added a handler for this; #19895 patched the resulting + # startup crash with try/except. Both were based on a misreading of how + # terminal key events propagate. Deleting the dead handler outright. @kb.add('c-q') # Ctrl+Q def handle_ctrl_q(event): @@ -10628,15 +10678,44 @@ class HermesCLI: run_in_terminal(_suspend) # Voice push-to-talk key: configurable via config.yaml (voice.record_key) - # Default: Ctrl+B (avoids conflict with Ctrl+R readline reverse-search) - # Config uses "ctrl+b" format; prompt_toolkit expects "c-b" format. + # Default: Ctrl+B (avoids conflict with Ctrl+R readline reverse-search). + # Config spellings (ctrl/control/alt/option/opt) are normalized to + # prompt_toolkit's c-x / a-x format via ``normalize_voice_record_key_for_prompt_toolkit`` + # so the same config value binds identically in the TUI and CLI + # (Copilot round-9 review on #19835). ``super``/``win``/``windows`` + # configs silently fall back to the default here since prompt_toolkit + # has no super modifier — log a warning so users notice the + # TUI/CLI split instead of a silent mismatch (round-11). + _raw_key: object = "ctrl+b" try: from hermes_cli.config import load_config - _raw_key = load_config().get("voice", {}).get("record_key", "ctrl+b") - _voice_key = _raw_key.lower().replace("ctrl+", "c-").replace("alt+", "a-") + from hermes_cli.voice import ( + normalize_voice_record_key_for_prompt_toolkit, + voice_record_key_from_config, + ) + _raw_key = voice_record_key_from_config(load_config()) + _voice_key = normalize_voice_record_key_for_prompt_toolkit(_raw_key) + if ( + isinstance(_raw_key, str) + and _raw_key.strip().lower().split("+", 1)[0].strip() in {"super", "win", "windows"} + and _voice_key == "c-b" + ): + logger.warning( + "voice.record_key %r uses a TUI-only modifier (super/win); " + "CLI fell back to Ctrl+B. Use ctrl+ or alt+ for " + "cross-runtime parity.", + _raw_key, + ) except Exception: _voice_key = "c-b" + # Cache the UI label here — same ``_raw_key`` that drives the + # prompt_toolkit binding below. Every status / placeholder / + # recording-hint render reads this cached value so display can + # never drift from the live keybinding even if the user edits + # voice.record_key mid-session (Copilot round-13 on #19835). + self.set_voice_record_key_cache(_raw_key) + @kb.add(_voice_key) def handle_voice_record(event): """Toggle voice recording when voice mode is active. @@ -10939,7 +11018,8 @@ class HermesCLI: def _get_placeholder(): if cli_ref._voice_recording: - return "recording... Ctrl+B to stop, Ctrl+C to cancel" + _label = cli_ref._voice_record_key_label() + return f"recording... {_label} to stop, Ctrl+C to cancel" if cli_ref._voice_processing: return "transcribing..." if cli_ref._sudo_state: @@ -10959,7 +11039,8 @@ class HermesCLI: if cli_ref._agent_running: return "msg=interrupt · /queue · /bg · /steer · Ctrl+C cancel" if cli_ref._voice_mode: - return "type or Ctrl+B to record" + _label = cli_ref._voice_record_key_label() + return f"type or {_label} to record" return "" input_area.control.input_processors.append(_PlaceholderProcessor(_get_placeholder)) diff --git a/gateway/run.py b/gateway/run.py index 2b085d9915..ebfd2731fe 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -15003,15 +15003,14 @@ async def start_gateway(config: Optional[GatewayConfig] = None, replace: bool = runner = GatewayRunner(config) - # Track whether a signal initiated the shutdown (vs. internal request). - # When an unexpected SIGTERM kills the gateway, we exit non-zero so - # systemd's Restart=on-failure revives the process. systemctl stop - # is safe: systemd tracks stop-requested state independently of exit - # code, so Restart= never fires for a deliberate stop. + # Track whether an unexpected signal initiated the shutdown. When an + # unexpected SIGTERM kills the gateway, we exit non-zero so service + # managers can revive the process. Planned stop paths write a marker + # before signalling us so they can exit cleanly instead. _signal_initiated_shutdown = False # Set up signal handlers - def shutdown_signal_handler(): + def shutdown_signal_handler(received_signal=None): nonlocal _signal_initiated_shutdown # Planned --replace takeover check: when a sibling gateway is # taking over via --replace, it wrote a marker naming this PID @@ -15027,10 +15026,28 @@ async def start_gateway(config: Optional[GatewayConfig] = None, replace: bool = except Exception as e: logger.debug("Takeover marker check failed: %s", e) + # Planned stop check: service managers and `hermes gateway stop` + # also send SIGTERM, which is indistinguishable from an unexpected + # external kill unless the CLI marks it first. SIGINT comes from an + # interactive Ctrl+C and is likewise an intentional foreground stop. + planned_stop = False + if received_signal == signal.SIGINT: + planned_stop = True + elif not planned_takeover: + try: + from gateway.status import consume_planned_stop_marker_for_self + planned_stop = consume_planned_stop_marker_for_self() + except Exception as e: + logger.debug("Planned stop marker check failed: %s", e) + if planned_takeover: logger.info( "Received SIGTERM as a planned --replace takeover — exiting cleanly" ) + elif planned_stop: + logger.info( + "Received SIGTERM/SIGINT as a planned gateway stop — exiting cleanly" + ) else: _signal_initiated_shutdown = True logger.info("Received SIGTERM/SIGINT — initiating shutdown") @@ -15066,7 +15083,7 @@ async def start_gateway(config: Optional[GatewayConfig] = None, replace: bool = if threading.current_thread() is threading.main_thread(): for sig in (signal.SIGINT, signal.SIGTERM): try: - loop.add_signal_handler(sig, shutdown_signal_handler) + loop.add_signal_handler(sig, shutdown_signal_handler, sig) except NotImplementedError: pass if hasattr(signal, "SIGUSR1"): @@ -15164,14 +15181,14 @@ async def start_gateway(config: Optional[GatewayConfig] = None, replace: bool = if runner.exit_code is not None: raise SystemExit(runner.exit_code) - # When a signal (SIGTERM/SIGINT) caused the shutdown and it wasn't a - # planned restart (/restart, /update, SIGUSR1), exit non-zero so - # systemd's Restart=on-failure revives the process. This covers: + # When an unexpected SIGTERM caused the shutdown and it wasn't a planned + # restart (/restart, /update, SIGUSR1), exit non-zero so systemd's + # Restart=on-failure revives the process. This covers: # - hermes update killing the gateway mid-work # - External kill commands # - WSL2/container runtime sending unexpected signals - # systemctl stop is safe: systemd tracks "stop requested" state - # independently of exit code, so Restart= never fires for it. + # `hermes gateway stop` and interactive Ctrl+C are handled above as + # planned stops and should not trigger service-manager revival. if _signal_initiated_shutdown and not runner._restart_requested: logger.info( "Exiting with code 1 (signal-initiated shutdown without restart " diff --git a/gateway/status.py b/gateway/status.py index f329b25f08..bdff9aa988 100644 --- a/gateway/status.py +++ b/gateway/status.py @@ -637,6 +637,8 @@ def release_all_scoped_locks( _TAKEOVER_MARKER_FILENAME = ".gateway-takeover.json" _TAKEOVER_MARKER_TTL_S = 60 # Marker older than this is treated as stale +_PLANNED_STOP_MARKER_FILENAME = ".gateway-planned-stop.json" +_PLANNED_STOP_MARKER_TTL_S = 60 def _get_takeover_marker_path() -> Path: @@ -645,6 +647,67 @@ def _get_takeover_marker_path() -> Path: return home / _TAKEOVER_MARKER_FILENAME +def _get_planned_stop_marker_path() -> Path: + """Return the path to the intentional gateway stop marker file.""" + home = get_hermes_home() + return home / _PLANNED_STOP_MARKER_FILENAME + + +def _marker_is_stale(written_at: str, ttl_s: int) -> bool: + try: + written_dt = datetime.fromisoformat(written_at) + age = (datetime.now(timezone.utc) - written_dt).total_seconds() + return age > ttl_s + except (TypeError, ValueError): + return True + + +def _consume_pid_marker_for_self( + path: Path, + *, + pid_field: str, + start_time_field: str, + ttl_s: int, +) -> bool: + record = _read_json_file(path) + if not record: + return False + + try: + target_pid = int(record[pid_field]) + target_start_time = record.get(start_time_field) + written_at = record.get("written_at") or "" + except (KeyError, TypeError, ValueError): + try: + path.unlink(missing_ok=True) + except OSError: + pass + return False + + if _marker_is_stale(written_at, ttl_s): + try: + path.unlink(missing_ok=True) + except OSError: + pass + return False + + our_pid = os.getpid() + our_start_time = _get_process_start_time(our_pid) + matches = ( + target_pid == our_pid + and target_start_time is not None + and our_start_time is not None + and target_start_time == our_start_time + ) + + try: + path.unlink(missing_ok=True) + except OSError: + pass + + return matches + + def write_takeover_marker(target_pid: int) -> bool: """Record that ``target_pid`` is being replaced by the current process. @@ -681,59 +744,13 @@ def consume_takeover_marker_for_self() -> bool: Always unlinks the marker on match (and on detected staleness) so subsequent unrelated signals don't re-trigger. """ - path = _get_takeover_marker_path() - record = _read_json_file(path) - if not record: - return False - - # Any malformed or stale marker → drop it and return False - try: - target_pid = int(record["target_pid"]) - target_start_time = record.get("target_start_time") - written_at = record.get("written_at") or "" - except (KeyError, TypeError, ValueError): - try: - path.unlink(missing_ok=True) - except OSError: - pass - return False - - # TTL guard: a stale marker older than _TAKEOVER_MARKER_TTL_S is ignored. - stale = False - try: - written_dt = datetime.fromisoformat(written_at) - age = (datetime.now(timezone.utc) - written_dt).total_seconds() - if age > _TAKEOVER_MARKER_TTL_S: - stale = True - except (TypeError, ValueError): - stale = True # Unparseable timestamp — treat as stale - - if stale: - try: - path.unlink(missing_ok=True) - except OSError: - pass - return False - - # Does the marker name THIS process? - our_pid = os.getpid() - our_start_time = _get_process_start_time(our_pid) - matches = ( - target_pid == our_pid - and target_start_time is not None - and our_start_time is not None - and target_start_time == our_start_time + return _consume_pid_marker_for_self( + _get_takeover_marker_path(), + pid_field="target_pid", + start_time_field="target_start_time", + ttl_s=_TAKEOVER_MARKER_TTL_S, ) - # Consume the marker whether it matched or not — a marker that doesn't - # match our identity is stale-for-us anyway. - try: - path.unlink(missing_ok=True) - except OSError: - pass - - return matches - def clear_takeover_marker() -> None: """Remove the takeover marker unconditionally. Safe to call repeatedly.""" @@ -743,6 +760,45 @@ def clear_takeover_marker() -> None: pass +def write_planned_stop_marker(target_pid: int) -> bool: + """Record that ``target_pid`` is being stopped intentionally. + + The gateway exits non-zero for unexpected SIGTERM so service managers can + revive it. Service stop commands send the same SIGTERM, so the CLI writes + this short-lived marker first to let the target process exit cleanly. + """ + try: + target_start_time = _get_process_start_time(target_pid) + record = { + "target_pid": target_pid, + "target_start_time": target_start_time, + "stopper_pid": os.getpid(), + "written_at": _utc_now_iso(), + } + _write_json_file(_get_planned_stop_marker_path(), record) + return True + except (OSError, PermissionError): + return False + + +def consume_planned_stop_marker_for_self() -> bool: + """Return True when the current process is being intentionally stopped.""" + return _consume_pid_marker_for_self( + _get_planned_stop_marker_path(), + pid_field="target_pid", + start_time_field="target_start_time", + ttl_s=_PLANNED_STOP_MARKER_TTL_S, + ) + + +def clear_planned_stop_marker() -> None: + """Remove the planned-stop marker unconditionally.""" + try: + _get_planned_stop_marker_path().unlink(missing_ok=True) + except OSError: + pass + + def get_running_pid( pid_path: Optional[Path] = None, *, diff --git a/hermes_cli/gateway.py b/hermes_cli/gateway.py index c1804f9c7f..846736a2cc 100644 --- a/hermes_cli/gateway.py +++ b/hermes_cli/gateway.py @@ -785,6 +785,12 @@ def stop_profile_gateway() -> bool: if pid is None: return False + try: + from gateway.status import write_planned_stop_marker + write_planned_stop_marker(pid) + except Exception: + pass + try: os.kill(pid, signal.SIGTERM) except ProcessLookupError: @@ -2043,6 +2049,13 @@ def systemd_stop(system: bool = False): if system: _require_root_for_system_service("stop") _require_service_installed("stop", system=system) + try: + from gateway.status import get_running_pid, write_planned_stop_marker + pid = get_running_pid(cleanup_stale=False) + if pid is not None: + write_planned_stop_marker(pid) + except Exception: + pass _run_systemctl(["stop", get_service_name()], system=system, check=True, timeout=90) print(f"✓ {_service_scope_label(system).capitalize()} service stopped") @@ -2404,6 +2417,13 @@ def launchd_start(): def launchd_stop(): label = get_launchd_label() target = f"{_launchd_domain()}/{label}" + try: + from gateway.status import get_running_pid, write_planned_stop_marker + pid = get_running_pid(cleanup_stale=False) + if pid is not None: + write_planned_stop_marker(pid) + except Exception: + pass # bootout unloads the service definition so KeepAlive doesn't respawn # the process. A plain `kill SIGTERM` only signals the process — launchd # immediately restarts it because KeepAlive.SuccessfulExit = false. diff --git a/hermes_cli/voice.py b/hermes_cli/voice.py index 0a355ce4fa..f85f30c7bf 100644 --- a/hermes_cli/voice.py +++ b/hermes_cli/voice.py @@ -27,6 +27,192 @@ import sys import threading from typing import Any, Callable, Optional +# Modifier aliases mirrored from the TUI parser (``ui-tui/src/lib/platform.ts``) +# ``_MOD_ALIASES`` table — the contract that removes the cross-runtime +# mismatch Copilot flagged in round-9 on #19835. +# +# ``super``/``win``/``windows`` are intentionally absent: prompt_toolkit +# has no super/meta modifier for the Cmd key, so those spellings are +# TUI-only. The normalizer below returns the documented default +# (``c-b``) for them — a silent fallback was preferred to a hard +# startup crash (Copilot round-11). The CLI binding site +# (``_register_voice_handler`` in cli.py) logs a warning when that +# fallback fires so users see why their TUI-only shortcut isn't +# bound in the classic CLI. +_VOICE_MOD_ALIASES = { + "ctrl": "c-", + "control": "c-", + "alt": "a-", + "option": "a-", + "opt": "a-", +} + +# Named keys prompt_toolkit accepts in ``c-`` / ``a-`` form. +# Aliases collapse to prompt_toolkit's canonical spelling so the same +# config value binds identically in both runtimes (Copilot round-10 on +# #19835). +_VOICE_NAMED_KEYS = { + "space": "space", + "spc": "space", + "enter": "enter", + "return": "enter", + "ret": "enter", + "tab": "tab", + "escape": "escape", + "esc": "escape", + "backspace": "backspace", + "bs": "backspace", + "delete": "delete", + "del": "delete", +} + +# ``useInputHandlers()`` intercepts these before the voice check runs, +# so a binding like ``ctrl+c`` (interrupt), ``ctrl+d`` (quit), or +# ``ctrl+l`` (clear screen) would be advertised in /voice status but +# never fire push-to-talk — the same blocklist the TUI parser uses. +_VOICE_RESERVED_CTRL_CHARS = frozenset({"c", "d", "l"}) + +# On macOS the classic CLI's prompt_toolkit bindings for copy / exit / +# clear also claim ``a-c`` / ``a-d`` / ``a-l`` via the action-modifier +# lookup, and hermes-ink reports Alt as ``key.meta`` on many terminals. +# Mirror the TUI parser's darwin-only reservation so ``option+c`` etc. +# don't bind Alt+C in the CLI while the TUI silently falls back to +# Ctrl+B (Copilot round-14 on #19835). +_VOICE_RESERVED_ALT_CHARS_MAC = frozenset({"c", "d", "l"}) + +_DEFAULT_PT_KEY = "c-b" + + +def voice_record_key_from_config(cfg: Any) -> Any: + """Shape-safe ``cfg.voice.record_key`` lookup. + + ``load_config()`` deep-merges raw YAML and preserves scalar + overrides, so a hand-edited ``voice: true`` / ``voice: cmd+b`` + leaves ``cfg["voice"]`` as a bool/str instead of a dict, and the + naive ``.get("voice", {}).get("record_key")`` chain raises + AttributeError before voice can even start (Copilot round-11 on + #19835). Return ``None`` for malformed shapes so call sites can + feed the result straight into the normalizer/formatter and get + the documented default. + """ + if not isinstance(cfg, dict): + return None + + voice = cfg.get("voice") + if not isinstance(voice, dict): + return None + + return voice.get("record_key") + + +def normalize_voice_record_key_for_prompt_toolkit(raw: Any) -> str: + """Coerce ``voice.record_key`` into prompt_toolkit's ``c-x`` / ``a-x`` format. + + Mirrors the TUI parser contract (``ui-tui/src/lib/platform.ts``) + so one config value binds the same shortcut in both runtimes: + + * non-string / empty / typo'd / bare-char / multi-modifier / reserved + ``ctrl+c|d|l`` → documented default ``c-b`` + * single-char keys: ``ctrl+o`` → ``c-o`` + * named keys: ``ctrl+space`` → ``c-space`` (aliases collapse: + ``ctrl+return`` → ``c-enter``) + * ``super`` / ``win`` / ``windows`` → ``c-b`` (TUI-only modifiers — + prompt_toolkit has no super mod; the CLI binding site is + expected to warn when this fallback fires so users see the + cross-runtime split, Copilot round-11 on #19835) + """ + if not isinstance(raw, str): + return _DEFAULT_PT_KEY + + lowered = raw.strip().lower() + if not lowered: + return _DEFAULT_PT_KEY + + parts = [p.strip() for p in lowered.split("+") if p.strip()] + if not parts: + return _DEFAULT_PT_KEY + + # Multi-modifier chords like ``ctrl+alt+r`` bind different shortcuts + # in prompt_toolkit (a-c-r form) and hermes-ink rejects them; collapse + # to the documented default instead of silently diverging. + if len(parts) > 2: + return _DEFAULT_PT_KEY + + # Bare char / bare named key (no explicit modifier) — the CLI's + # prompt_toolkit binds the raw key without a modifier, which the TUI + # parser refuses; reject here too so both runtimes agree. + if len(parts) == 1: + return _DEFAULT_PT_KEY + + modifier_token, key_token = parts + + # ``super`` / ``win`` / ``windows`` are TUI-only (prompt_toolkit has + # no super modifier, so ``@kb.add(super+b)`` crashes the CLI at + # startup). Fall back to the documented default here; the CLI + # binding site is expected to log a warning when the configured + # value is one of these spellings so users know the TUI+CLI + # runtimes diverge on that shortcut (Copilot round-11 on #19835). + if modifier_token in {"super", "win", "windows"}: + return _DEFAULT_PT_KEY + + normalized_mod = _VOICE_MOD_ALIASES.get(modifier_token) + if not normalized_mod: + return _DEFAULT_PT_KEY + + # Single-char key: reject reserved-ctrl chords that the TUI would + # also block at parse time, plus the mac-only alt reservation. + if len(key_token) == 1: + if normalized_mod == "c-" and key_token in _VOICE_RESERVED_CTRL_CHARS: + return _DEFAULT_PT_KEY + if ( + normalized_mod == "a-" + and sys.platform == "darwin" + and key_token in _VOICE_RESERVED_ALT_CHARS_MAC + ): + return _DEFAULT_PT_KEY + return f"{normalized_mod}{key_token}" + + # Multi-char key token must be a known named key; typos like + # ``ctrl+spcae`` fall back to the default rather than being passed + # through as ``c-spcae`` (which prompt_toolkit would reject). + named = _VOICE_NAMED_KEYS.get(key_token) + if not named: + return _DEFAULT_PT_KEY + + return f"{normalized_mod}{named}" + + +def format_voice_record_key_for_status(raw: Any) -> str: + """Render ``voice.record_key`` for ``/voice status`` in CLI-friendly form. + + Mirrors the TUI's ``formatVoiceRecordKey``: returns ``Ctrl+B`` / + ``Alt+Space`` / ``Ctrl+Enter``. Malformed configs surface as the + documented default so status never advertises a shortcut that + won't bind (Copilot round-10 on #19835). + """ + normalized = normalize_voice_record_key_for_prompt_toolkit(raw) + + if normalized.startswith("c-"): + prefix, key = "Ctrl+", normalized[2:] + elif normalized.startswith("a-"): + prefix, key = "Alt+", normalized[2:] + elif "+" in normalized: + # ``super+`` / ``win+`` — CLI won't bind them, but + # render in title case so status output is still readable. + mod, key = normalized.split("+", 1) + prefix = mod[0].upper() + mod[1:] + "+" + else: + return "Ctrl+B" + + if not key: + return prefix.rstrip("+") + + if len(key) == 1: + return prefix + key.upper() + + return prefix + key[0].upper() + key[1:] + + from tools.voice_mode import ( create_audio_recorder, is_whisper_hallucination, diff --git a/plugins/kanban/dashboard/dist/index.js b/plugins/kanban/dashboard/dist/index.js index d60bc19289..ee1a25f065 100644 --- a/plugins/kanban/dashboard/dist/index.js +++ b/plugins/kanban/dashboard/dist/index.js @@ -1996,7 +1996,10 @@ return h("div", { className: "hermes-kanban-actions" }, b("→ triage", { status: "triage" }, t.status !== "triage"), b("→ ready", { status: "ready" }, t.status !== "ready"), - b("→ running", { status: "running" }, t.status !== "running"), + // No direct → running button: /tasks/:id PATCH rejects status=running + // with 400 (issue #19535). Tasks enter running only through the + // dispatcher's claim_task path, which atomically creates the run row, + // claim lock, and worker process metadata. b("Block", { status: "blocked" }, t.status === "running" || t.status === "ready", DESTRUCTIVE_TRANSITIONS.blocked), diff --git a/plugins/kanban/dashboard/dist/style.css b/plugins/kanban/dashboard/dist/style.css index 34fc714d11..9de372cbe2 100644 --- a/plugins/kanban/dashboard/dist/style.css +++ b/plugins/kanban/dashboard/dist/style.css @@ -364,11 +364,15 @@ letter-spacing: 0.02em; } .hermes-kanban-home-sub--on { - /* Subtly indicate the subscribed state without a hard color change so - * dashboard themes stay coherent. Border + tinted background. */ - border-color: color-mix(in srgb, var(--color-ring) 55%, var(--color-border)); - background: color-mix(in srgb, var(--color-ring) 14%, transparent); + /* Subscribed toggle — use a strong ring-colored accent so the on/off + * distinction reads at a glance, not just from the ✓ prefix. Border + + * filled background + bolder weight keep the state obvious across + * themes (tested on default teal and NERV orange). */ + border-color: var(--color-ring); + background: color-mix(in srgb, var(--color-ring) 32%, transparent); color: var(--color-foreground); + font-weight: 600; + box-shadow: inset 0 0 0 1px color-mix(in srgb, var(--color-ring) 40%, transparent); } .hermes-kanban-section { diff --git a/plugins/platforms/teams/adapter.py b/plugins/platforms/teams/adapter.py index d0a2b7adbc..cdec7e3f1e 100644 --- a/plugins/platforms/teams/adapter.py +++ b/plugins/platforms/teams/adapter.py @@ -509,7 +509,20 @@ class TeamsAdapter(BasePlatformAdapter): for chunk in chunks: try: - result = await self._app.send(chat_id, chunk) + if reply_to and reply_to.isdigit() and reply_to != "0": + try: + result = await self._app.reply(chat_id, reply_to, chunk) + except Exception as reply_err: + # Group chats 400 on threaded sends; the Teams SDK + # doesn't expose typed HTTP errors, so fall back on + # any exception and log for diagnostics. + logger.debug( + "Teams reply() failed, falling back to flat send: %s", + reply_err, + ) + result = await self._app.send(chat_id, chunk) + else: + result = await self._app.send(chat_id, chunk) last_message_id = getattr(result, "id", None) except Exception as e: return SendResult(success=False, error=str(e), retryable=True) diff --git a/tests/cli/test_cli_status_bar.py b/tests/cli/test_cli_status_bar.py index 4a65c6e467..f5c18bfc4d 100644 --- a/tests/cli/test_cli_status_bar.py +++ b/tests/cli/test_cli_status_bar.py @@ -266,6 +266,68 @@ class TestCLIStatusBar: assert fragments == [("class:voice-status-recording", " ● REC ")] + # Round-13 Copilot review regressions on #19835. The label in voice + # status bar / recording hint / placeholder must render the + # configured ``voice.record_key`` — not hardcoded Ctrl+B. Pinning + # the cache (``set_voice_record_key_cache``) keeps display in sync + # with the prompt_toolkit binding without re-reading config on + # every render. + def test_voice_status_bar_renders_configured_ctrl_letter(self): + cli_obj = _make_cli() + cli_obj._voice_mode = True + cli_obj._voice_recording = False + cli_obj._voice_processing = False + cli_obj._voice_tts = False + cli_obj._voice_continuous = False + cli_obj.set_voice_record_key_cache("ctrl+o") + + wide = cli_obj._get_voice_status_fragments(width=120) + assert any("Ctrl+O to record" in text for _cls, text in wide) + + compact = cli_obj._get_voice_status_fragments(width=50) + assert compact == [("class:voice-status", " 🎤 Ctrl+O ")] + + def test_voice_recording_status_bar_renders_configured_named_key(self): + cli_obj = _make_cli() + cli_obj._voice_mode = True + cli_obj._voice_recording = True + cli_obj._voice_processing = False + cli_obj.set_voice_record_key_cache("ctrl+space") + + fragments = cli_obj._get_voice_status_fragments(width=120) + + assert fragments == [("class:voice-status-recording", " ● REC Ctrl+Space to stop ")] + + def test_voice_status_bar_falls_back_to_ctrl_b_without_cache(self): + cli_obj = _make_cli() + cli_obj._voice_mode = True + cli_obj._voice_recording = False + cli_obj._voice_processing = False + cli_obj._voice_tts = False + cli_obj._voice_continuous = False + # No cache set — mirrors pre-startup state; fall back to + # documented Ctrl+B default (Copilot round-13 review). + + compact = cli_obj._get_voice_status_fragments(width=50) + + assert compact == [("class:voice-status", " 🎤 Ctrl+B ")] + + def test_voice_status_bar_renders_malformed_config_as_default(self): + cli_obj = _make_cli() + cli_obj._voice_mode = True + cli_obj._voice_recording = False + cli_obj._voice_processing = False + cli_obj._voice_tts = False + cli_obj._voice_continuous = False + # Non-string / typoed configs fall through the formatter to the + # documented default so the status bar never advertises an + # invalid shortcut. + cli_obj.set_voice_record_key_cache(True) + + compact = cli_obj._get_voice_status_fragments(width=50) + + assert compact == [("class:voice-status", " 🎤 Ctrl+B ")] + class TestCLIUsageReport: def test_show_usage_includes_estimated_cost(self, capsys): diff --git a/tests/gateway/test_status.py b/tests/gateway/test_status.py index 7138b6514e..e7cd0dc060 100644 --- a/tests/gateway/test_status.py +++ b/tests/gateway/test_status.py @@ -702,3 +702,88 @@ class TestTakeoverMarker: # We are not the target — must NOT consume as planned assert result is False + + +class TestPlannedStopMarker: + """Tests for intentional service/manual gateway stop markers.""" + + def test_write_marker_records_target_identity(self, tmp_path, monkeypatch): + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + monkeypatch.setattr(status, "_get_process_start_time", lambda pid: 42) + + ok = status.write_planned_stop_marker(target_pid=12345) + + assert ok is True + marker = tmp_path / ".gateway-planned-stop.json" + assert marker.exists() + payload = json.loads(marker.read_text()) + assert payload["target_pid"] == 12345 + assert payload["target_start_time"] == 42 + assert payload["stopper_pid"] == os.getpid() + assert "written_at" in payload + + def test_consume_returns_true_when_marker_names_self(self, tmp_path, monkeypatch): + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + monkeypatch.setattr(status, "_get_process_start_time", lambda pid: 100) + ok = status.write_planned_stop_marker(target_pid=os.getpid()) + assert ok is True + + result = status.consume_planned_stop_marker_for_self() + + assert result is True + assert not (tmp_path / ".gateway-planned-stop.json").exists() + + def test_consume_returns_false_for_different_pid(self, tmp_path, monkeypatch): + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + monkeypatch.setattr(status, "_get_process_start_time", lambda pid: 100) + ok = status.write_planned_stop_marker(target_pid=os.getpid() + 9999) + assert ok is True + + result = status.consume_planned_stop_marker_for_self() + + assert result is False + assert not (tmp_path / ".gateway-planned-stop.json").exists() + + def test_consume_returns_false_for_stale_marker(self, tmp_path, monkeypatch): + from datetime import datetime, timezone, timedelta + + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + marker_path = tmp_path / ".gateway-planned-stop.json" + stale_time = (datetime.now(timezone.utc) - timedelta(minutes=2)).isoformat() + marker_path.write_text(json.dumps({ + "target_pid": os.getpid(), + "target_start_time": 123, + "stopper_pid": 99999, + "written_at": stale_time, + })) + monkeypatch.setattr(status, "_get_process_start_time", lambda pid: 123) + + result = status.consume_planned_stop_marker_for_self() + + assert result is False + assert not marker_path.exists() + + def test_clear_planned_stop_marker_is_idempotent(self, tmp_path, monkeypatch): + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + monkeypatch.setattr(status, "_get_process_start_time", lambda pid: 100) + + status.clear_planned_stop_marker() + status.write_planned_stop_marker(target_pid=12345) + assert (tmp_path / ".gateway-planned-stop.json").exists() + + status.clear_planned_stop_marker() + + assert not (tmp_path / ".gateway-planned-stop.json").exists() + status.clear_planned_stop_marker() + + def test_write_marker_returns_false_on_write_failure(self, tmp_path, monkeypatch): + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + + def raise_oserror(*args, **kwargs): + raise OSError("simulated write failure") + + monkeypatch.setattr(status, "_write_json_file", raise_oserror) + + ok = status.write_planned_stop_marker(target_pid=12345) + + assert ok is False diff --git a/tests/gateway/test_teams.py b/tests/gateway/test_teams.py index 2befceec94..0e1e05bd1b 100644 --- a/tests/gateway/test_teams.py +++ b/tests/gateway/test_teams.py @@ -32,6 +32,9 @@ def _ensure_teams_mock(): microsoft_teams_api_activities_invoke_adaptive_card = types.ModuleType( "microsoft_teams.api.activities.invoke.adaptive_card" ) + microsoft_teams_common = types.ModuleType("microsoft_teams.common") + microsoft_teams_common_http = types.ModuleType("microsoft_teams.common.http") + microsoft_teams_common_http_client = types.ModuleType("microsoft_teams.common.http.client") microsoft_teams_api_models = types.ModuleType("microsoft_teams.api.models") microsoft_teams_api_models_adaptive_card = types.ModuleType("microsoft_teams.api.models.adaptive_card") microsoft_teams_api_models_invoke_response = types.ModuleType("microsoft_teams.api.models.invoke_response") @@ -76,6 +79,7 @@ def _ensure_teams_mock(): microsoft_teams_apps.App = MockApp microsoft_teams_apps.ActivityContext = MagicMock + microsoft_teams_common_http_client.ClientOptions = MagicMock # MessageActivity mock microsoft_teams_api.MessageActivity = MagicMock @@ -143,6 +147,9 @@ def _ensure_teams_mock(): "microsoft_teams.api.activities.typing": microsoft_teams_api_activities_typing, "microsoft_teams.api.activities.invoke": microsoft_teams_api_activities_invoke, "microsoft_teams.api.activities.invoke.adaptive_card": microsoft_teams_api_activities_invoke_adaptive_card, + "microsoft_teams.common": microsoft_teams_common, + "microsoft_teams.common.http": microsoft_teams_common_http, + "microsoft_teams.common.http.client": microsoft_teams_common_http_client, "microsoft_teams.api.models": microsoft_teams_api_models, "microsoft_teams.api.models.adaptive_card": microsoft_teams_api_models_adaptive_card, "microsoft_teams.api.models.invoke_response": microsoft_teams_api_models_invoke_response, @@ -162,6 +169,13 @@ _teams_mod = load_plugin_adapter("teams") _teams_mod.TEAMS_SDK_AVAILABLE = True _teams_mod.AIOHTTP_AVAILABLE = True +# Ensure SDK symbols that were None (import failed on Python <3.12) are +# replaced with the mocked versions so runtime calls don't silently no-op. +import sys as _sys +_mt = _sys.modules.get("microsoft_teams.api.activities.typing") +if _mt and _teams_mod.TypingActivityInput is None: + _teams_mod.TypingActivityInput = _mt.TypingActivityInput + TeamsAdapter = _teams_mod.TeamsAdapter check_requirements = _teams_mod.check_requirements check_teams_requirements = _teams_mod.check_teams_requirements diff --git a/tests/hermes_cli/test_env_loader.py b/tests/hermes_cli/test_env_loader.py index f94649a634..f309dfd4c6 100644 --- a/tests/hermes_cli/test_env_loader.py +++ b/tests/hermes_cli/test_env_loader.py @@ -37,7 +37,7 @@ def test_project_env_is_sanitized_before_loading(tmp_path, monkeypatch): home = tmp_path / "hermes" project_env = tmp_path / ".env" project_env.write_text( - "TELEGRAM_BOT_TOKEN=8356550917:AAGGEkzg06Hrc3Hjb3Sa1jkGVDOdU_lYy2Q" + "TELEGRAM_BOT_TOKEN=0123456789:test" "ANTHROPIC_API_KEY=sk-ant-test123\n", encoding="utf-8", ) @@ -48,7 +48,7 @@ def test_project_env_is_sanitized_before_loading(tmp_path, monkeypatch): loaded = load_hermes_dotenv(hermes_home=home, project_env=project_env) assert loaded == [project_env] - assert os.getenv("TELEGRAM_BOT_TOKEN") == "8356550917:AAGGEkzg06Hrc3Hjb3Sa1jkGVDOdU_lYy2Q" + assert os.getenv("TELEGRAM_BOT_TOKEN") == "0123456789:test" assert os.getenv("ANTHROPIC_API_KEY") == "sk-ant-test123" diff --git a/tests/hermes_cli/test_env_sanitize_on_load.py b/tests/hermes_cli/test_env_sanitize_on_load.py index 6ac7c2cef3..f23eadd2a5 100644 --- a/tests/hermes_cli/test_env_sanitize_on_load.py +++ b/tests/hermes_cli/test_env_sanitize_on_load.py @@ -14,7 +14,7 @@ def test_load_env_sanitizes_concatenated_lines(): """ from hermes_cli.config import load_env - token = "8356550917:AAGGEkzg06Hrc3Hjb3Sa1jkGVDOdU_lYy2Q" + token = "0123456789:test" # Simulate concatenated line: TOKEN=xxx followed immediately by another key corrupted = f"TELEGRAM_BOT_TOKEN={token}ANTHROPIC_API_KEY=sk-ant-test123\n" @@ -67,7 +67,7 @@ def test_env_loader_sanitizes_before_dotenv(): """Verify env_loader._sanitize_env_file_if_needed fixes corrupted files.""" from hermes_cli.env_loader import _sanitize_env_file_if_needed - token = "8356550917:AAGGEkzg06Hrc3Hjb3Sa1jkGVDOdU_lYy2Q" + token = "0123456789:test" corrupted = f"TELEGRAM_BOT_TOKEN={token}ANTHROPIC_API_KEY=sk-ant-test\n" with tempfile.NamedTemporaryFile( diff --git a/tests/hermes_cli/test_gateway_service.py b/tests/hermes_cli/test_gateway_service.py index 3e9a4d3720..994e8d0284 100644 --- a/tests/hermes_cli/test_gateway_service.py +++ b/tests/hermes_cli/test_gateway_service.py @@ -8,6 +8,7 @@ from types import SimpleNamespace import pytest import hermes_cli.gateway as gateway_cli +from gateway import status from gateway.restart import ( DEFAULT_GATEWAY_RESTART_DRAIN_TIMEOUT, GATEWAY_SERVICE_RESTART_EXIT_CODE, @@ -106,6 +107,30 @@ class TestSystemdServiceRefresh: ["systemctl", "--user", "reload-or-restart", gateway_cli.get_service_name()], ] + def test_systemd_stop_marks_running_gateway_as_planned_stop(self, monkeypatch): + calls = [] + markers = [] + + monkeypatch.setattr(gateway_cli, "_select_systemd_scope", lambda system=False: False) + monkeypatch.setattr(gateway_cli, "_require_service_installed", lambda action, system=False: None) + monkeypatch.setattr(status, "get_running_pid", lambda cleanup_stale=True: 321) + monkeypatch.setattr( + status, + "write_planned_stop_marker", + lambda pid: markers.append(pid) or True, + ) + + def fake_run_systemctl(args, **kwargs): + calls.append(args) + return SimpleNamespace(returncode=0, stdout="", stderr="") + + monkeypatch.setattr(gateway_cli, "_run_systemctl", fake_run_systemctl) + + gateway_cli.systemd_stop() + + assert markers == [321] + assert calls == [["stop", gateway_cli.get_service_name()]] + def test_run_gateway_refreshes_outdated_unit_on_boot(self, tmp_path, monkeypatch): """run_gateway() should refresh the systemd unit on boot so that @@ -127,11 +152,8 @@ class TestSystemdServiceRefresh: monkeypatch.setattr(gateway_cli.subprocess, "run", fake_run) # Prevent run_gateway from actually starting the gateway - def fake_start_gateway(**kwargs): - import asyncio - f = asyncio.Future() - f.set_result(True) - return f + async def fake_start_gateway(**kwargs): + return True monkeypatch.setattr("gateway.run.start_gateway", fake_start_gateway) @@ -163,7 +185,16 @@ class TestRequireServiceInstalled: class TestGeneratedSystemdUnits: - def test_user_unit_avoids_recursive_execstop_and_uses_extended_stop_timeout(self): + def _expected_timeout_stop_sec(self) -> str: + timeout = int(max(60, DEFAULT_GATEWAY_RESTART_DRAIN_TIMEOUT) + 30) + return f"TimeoutStopSec={timeout}" + + def test_user_unit_avoids_recursive_execstop_and_uses_extended_stop_timeout(self, monkeypatch): + monkeypatch.setattr( + gateway_cli, + "_get_restart_drain_timeout", + lambda: DEFAULT_GATEWAY_RESTART_DRAIN_TIMEOUT, + ) unit = gateway_cli.generate_systemd_unit(system=False) assert "ExecStart=" in unit @@ -173,7 +204,7 @@ class TestGeneratedSystemdUnits: # TimeoutStopSec must exceed the default drain_timeout (60s) so # systemd doesn't SIGKILL the cgroup before post-interrupt cleanup # (tool subprocess kill, adapter disconnect) runs — issue #8202. - assert "TimeoutStopSec=90" in unit + assert self._expected_timeout_stop_sec() in unit def test_user_unit_includes_resolved_node_directory_in_path(self, monkeypatch): monkeypatch.setattr(gateway_cli.shutil, "which", lambda cmd: "/home/test/.nvm/versions/node/v24.14.0/bin/node" if cmd == "node" else None) @@ -219,7 +250,12 @@ class TestGeneratedSystemdUnits: assert "/mnt/c/WINDOWS/system32" in unit - def test_system_unit_avoids_recursive_execstop_and_uses_extended_stop_timeout(self): + def test_system_unit_avoids_recursive_execstop_and_uses_extended_stop_timeout(self, monkeypatch): + monkeypatch.setattr( + gateway_cli, + "_get_restart_drain_timeout", + lambda: DEFAULT_GATEWAY_RESTART_DRAIN_TIMEOUT, + ) unit = gateway_cli.generate_systemd_unit(system=True) assert "ExecStart=" in unit @@ -229,7 +265,7 @@ class TestGeneratedSystemdUnits: # TimeoutStopSec must exceed the default drain_timeout (60s) so # systemd doesn't SIGKILL the cgroup before post-interrupt cleanup # (tool subprocess kill, adapter disconnect) runs — issue #8202. - assert "TimeoutStopSec=90" in unit + assert self._expected_timeout_stop_sec() in unit assert "WantedBy=multi-user.target" in unit diff --git a/tests/hermes_cli/test_voice_wrapper.py b/tests/hermes_cli/test_voice_wrapper.py index a372c1194f..3caacf4313 100644 --- a/tests/hermes_cli/test_voice_wrapper.py +++ b/tests/hermes_cli/test_voice_wrapper.py @@ -31,6 +31,243 @@ class TestPublicAPI: assert callable(speak_text) +class TestNormalizeVoiceRecordKeyForPromptToolkit: + """Round-9 Copilot review regression on #19835. + + Classic CLI only normalized ``ctrl+`` / ``alt+``, so TUI-valid + aliases like ``control+``, ``option+``, ``opt+`` silently bound a + different (or no) shortcut in the CLI. Normalizer now maps the + same set of aliases the TUI parser accepts, so one config value + binds identically in both runtimes. + """ + + def test_ctrl_and_alt_map_to_prompt_toolkit_form(self): + from hermes_cli.voice import normalize_voice_record_key_for_prompt_toolkit + + assert normalize_voice_record_key_for_prompt_toolkit("ctrl+b") == "c-b" + assert normalize_voice_record_key_for_prompt_toolkit("alt+r") == "a-r" + + def test_control_option_opt_aliases_match_tui_parser(self): + from hermes_cli.voice import normalize_voice_record_key_for_prompt_toolkit + + assert normalize_voice_record_key_for_prompt_toolkit("control+o") == "c-o" + assert normalize_voice_record_key_for_prompt_toolkit("option+space") == "a-space" + assert normalize_voice_record_key_for_prompt_toolkit("opt+enter") == "a-enter" + + def test_case_insensitive(self): + from hermes_cli.voice import normalize_voice_record_key_for_prompt_toolkit + + assert normalize_voice_record_key_for_prompt_toolkit("Ctrl+B") == "c-b" + assert normalize_voice_record_key_for_prompt_toolkit("CONTROL+O") == "c-o" + + def test_non_string_falls_back_to_default(self): + from hermes_cli.voice import normalize_voice_record_key_for_prompt_toolkit + + assert normalize_voice_record_key_for_prompt_toolkit(None) == "c-b" + assert normalize_voice_record_key_for_prompt_toolkit(1) == "c-b" + assert normalize_voice_record_key_for_prompt_toolkit(True) == "c-b" + assert normalize_voice_record_key_for_prompt_toolkit({}) == "c-b" + + def test_empty_string_falls_back(self): + from hermes_cli.voice import normalize_voice_record_key_for_prompt_toolkit + + assert normalize_voice_record_key_for_prompt_toolkit("") == "c-b" + + def test_super_win_fall_back_to_default_in_cli(self): + """prompt_toolkit has no super modifier, so ``super+b`` / ``win+o`` + would crash the classic CLI at startup if passed through. Fall + back to the documented default; the CLI binding site is + expected to warn so users know the shortcut is TUI-only + (Copilot round-11 on #19835).""" + from hermes_cli.voice import normalize_voice_record_key_for_prompt_toolkit + + assert normalize_voice_record_key_for_prompt_toolkit("super+b") == "c-b" + assert normalize_voice_record_key_for_prompt_toolkit("win+o") == "c-b" + assert normalize_voice_record_key_for_prompt_toolkit("windows+o") == "c-b" + + # Round-10 Copilot review regressions on #19835. + def test_strips_whitespace_within_and_around(self): + """``ctrl + b`` / `` option + space `` are accepted by the TUI + parser; the CLI normalizer must mirror that or the same config + binds different shortcuts across runtimes.""" + from hermes_cli.voice import normalize_voice_record_key_for_prompt_toolkit + + assert normalize_voice_record_key_for_prompt_toolkit("ctrl + b") == "c-b" + assert normalize_voice_record_key_for_prompt_toolkit(" option + space ") == "a-space" + + def test_named_key_aliases_collapse_to_prompt_toolkit_canonical(self): + """TUI accepts ``return`` / ``esc`` / ``bs`` / ``del`` etc.; + CLI must collapse to prompt_toolkit's canonical spelling + (``enter`` / ``escape`` / ``backspace`` / ``delete``).""" + from hermes_cli.voice import normalize_voice_record_key_for_prompt_toolkit + + assert normalize_voice_record_key_for_prompt_toolkit("ctrl+return") == "c-enter" + assert normalize_voice_record_key_for_prompt_toolkit("ctrl+esc") == "c-escape" + assert normalize_voice_record_key_for_prompt_toolkit("ctrl+bs") == "c-backspace" + assert normalize_voice_record_key_for_prompt_toolkit("alt+del") == "a-delete" + + def test_typoed_named_keys_fall_back_to_default(self): + """``ctrl+spcae`` would otherwise pass through as ``c-spcae`` and + prompt_toolkit would reject it at startup — fall back instead.""" + from hermes_cli.voice import normalize_voice_record_key_for_prompt_toolkit + + assert normalize_voice_record_key_for_prompt_toolkit("ctrl+spcae") == "c-b" + assert normalize_voice_record_key_for_prompt_toolkit("ctrl+f5") == "c-b" + + def test_bare_char_and_multi_modifier_fall_back(self): + """TUI parser rejects bare-char (``o``) and multi-modifier + (``ctrl+alt+r``) configs; the CLI normalizer must match.""" + from hermes_cli.voice import normalize_voice_record_key_for_prompt_toolkit + + assert normalize_voice_record_key_for_prompt_toolkit("o") == "c-b" + assert normalize_voice_record_key_for_prompt_toolkit("b") == "c-b" + assert normalize_voice_record_key_for_prompt_toolkit("ctrl+alt+r") == "c-b" + + def test_reserved_ctrl_chars_fall_back(self): + """``ctrl+c`` / ``ctrl+d`` / ``ctrl+l`` are always claimed by + the CLI's prompt_toolkit input layer or terminal driver; match + the TUI parser's rejection to keep /voice status honest.""" + from hermes_cli.voice import normalize_voice_record_key_for_prompt_toolkit + + assert normalize_voice_record_key_for_prompt_toolkit("ctrl+c") == "c-b" + assert normalize_voice_record_key_for_prompt_toolkit("ctrl+d") == "c-b" + assert normalize_voice_record_key_for_prompt_toolkit("ctrl+l") == "c-b" + + def test_unknown_modifier_falls_back(self): + """``meta+b`` is ambiguous on the wire (Alt on xterm, Cmd on + legacy macOS), same class as the TUI parser's rejection.""" + from hermes_cli.voice import normalize_voice_record_key_for_prompt_toolkit + + assert normalize_voice_record_key_for_prompt_toolkit("meta+b") == "c-b" + assert normalize_voice_record_key_for_prompt_toolkit("shift+b") == "c-b" + + # Round-14 Copilot review regression on #19835. On macOS the TUI + # parser rejects alt+c/d/l because hermes-ink reports Alt as + # ``key.meta`` and isActionMod(darwin) accepts it. The CLI + # normalizer must mirror that platform-gated rejection so shared + # configs like ``option+c`` don't bind Alt+C in the CLI while the + # TUI falls back to Ctrl+B. + def test_alt_cdl_rejected_on_macos(self, monkeypatch): + monkeypatch.setattr("sys.platform", "darwin") + + from hermes_cli.voice import normalize_voice_record_key_for_prompt_toolkit + + assert normalize_voice_record_key_for_prompt_toolkit("alt+c") == "c-b" + assert normalize_voice_record_key_for_prompt_toolkit("alt+d") == "c-b" + assert normalize_voice_record_key_for_prompt_toolkit("alt+l") == "c-b" + assert normalize_voice_record_key_for_prompt_toolkit("option+c") == "c-b" + assert normalize_voice_record_key_for_prompt_toolkit("opt+d") == "c-b" + # Other alt letters still bind on darwin. + assert normalize_voice_record_key_for_prompt_toolkit("alt+r") == "a-r" + assert normalize_voice_record_key_for_prompt_toolkit("alt+space") == "a-space" + + def test_alt_cdl_allowed_on_non_macos(self, monkeypatch): + monkeypatch.setattr("sys.platform", "linux") + + from hermes_cli.voice import normalize_voice_record_key_for_prompt_toolkit + + assert normalize_voice_record_key_for_prompt_toolkit("alt+c") == "a-c" + assert normalize_voice_record_key_for_prompt_toolkit("alt+d") == "a-d" + assert normalize_voice_record_key_for_prompt_toolkit("alt+l") == "a-l" + + +class TestVoiceRecordKeyFromConfig: + """Round-11 Copilot review regression on #19835. + + ``load_config()`` preserves YAML scalar overrides, so a hand-edited + ``voice: true`` or ``voice: cmd+b`` made the naive + ``cfg.get('voice', {}).get('record_key')`` chain raise + AttributeError before voice could run. The shape-safe extractor + returns None for every malformed shape so the call-site fallback + (``normalize_…`` / ``format_…``) surfaces the documented default. + """ + + def test_dict_voice_with_string_record_key(self): + from hermes_cli.voice import voice_record_key_from_config + + assert voice_record_key_from_config({"voice": {"record_key": "ctrl+o"}}) == "ctrl+o" + + def test_non_dict_config_root(self): + from hermes_cli.voice import voice_record_key_from_config + + for bad_root in (None, True, 1, "ctrl+b", [], ["ctrl+b"]): + assert voice_record_key_from_config(bad_root) is None, bad_root + + def test_non_dict_voice_entry(self): + from hermes_cli.voice import voice_record_key_from_config + + for bad_voice in (None, True, "cmd+b", 42, ["ctrl+b"]): + assert voice_record_key_from_config({"voice": bad_voice}) is None, bad_voice + + def test_missing_record_key_returns_none(self): + from hermes_cli.voice import voice_record_key_from_config + + assert voice_record_key_from_config({"voice": {"beep_enabled": True}}) is None + assert voice_record_key_from_config({}) is None + + def test_normalizer_accepts_extractor_output_directly(self): + """voice_record_key_from_config + normalize_… must compose — + None / non-string scalars all fall back to c-b.""" + from hermes_cli.voice import ( + normalize_voice_record_key_for_prompt_toolkit, + voice_record_key_from_config, + ) + + for raw in (None, True, 1, "cmd+b", ["ctrl+b"]): + extracted = voice_record_key_from_config({"voice": raw}) + assert normalize_voice_record_key_for_prompt_toolkit(extracted) == "c-b" + + +class TestFormatVoiceRecordKeyForStatus: + """Round-10 Copilot review regression on #19835. + + ``/voice status`` used to print the raw scalar (``True`` / ``1``) + for non-string configs even though the actual binding falls back + to Ctrl+B. The formatter routes through the same normalizer so + status always matches what the CLI actually binds. + """ + + def test_ctrl_and_alt_letter_keys_render_canonically(self): + from hermes_cli.voice import format_voice_record_key_for_status + + assert format_voice_record_key_for_status("ctrl+b") == "Ctrl+B" + assert format_voice_record_key_for_status("ctrl+o") == "Ctrl+O" + assert format_voice_record_key_for_status("alt+r") == "Alt+R" + + def test_named_keys_render_in_title_case(self): + from hermes_cli.voice import format_voice_record_key_for_status + + assert format_voice_record_key_for_status("ctrl+space") == "Ctrl+Space" + assert format_voice_record_key_for_status("alt+enter") == "Alt+Enter" + assert format_voice_record_key_for_status("ctrl+esc") == "Ctrl+Escape" + + def test_aliases_render_via_normalized_form(self): + from hermes_cli.voice import format_voice_record_key_for_status + + assert format_voice_record_key_for_status("control+o") == "Ctrl+O" + assert format_voice_record_key_for_status("option+space") == "Alt+Space" + assert format_voice_record_key_for_status("opt+enter") == "Alt+Enter" + + def test_non_string_scalar_falls_back_to_ctrl_b_label(self): + from hermes_cli.voice import format_voice_record_key_for_status + + # Copilot round-10 regression: previously /voice status printed + # the raw scalar ("True" / "1") even though the actual binding + # fell back to Ctrl+B. + assert format_voice_record_key_for_status(True) == "Ctrl+B" + assert format_voice_record_key_for_status(1) == "Ctrl+B" + assert format_voice_record_key_for_status(None) == "Ctrl+B" + assert format_voice_record_key_for_status({}) == "Ctrl+B" + + def test_malformed_configs_fall_back_to_ctrl_b(self): + from hermes_cli.voice import format_voice_record_key_for_status + + assert format_voice_record_key_for_status("ctrl+spcae") == "Ctrl+B" + assert format_voice_record_key_for_status("ctrl+alt+r") == "Ctrl+B" + assert format_voice_record_key_for_status("") == "Ctrl+B" + assert format_voice_record_key_for_status(" ") == "Ctrl+B" + + class TestStopWithoutStart: def test_returns_none_when_no_recording_active(self, monkeypatch): """Idempotent no-op: stop before start must not raise or touch state.""" diff --git a/tests/plugins/test_kanban_dashboard_plugin.py b/tests/plugins/test_kanban_dashboard_plugin.py index 23589ce690..32e40ceb4e 100644 --- a/tests/plugins/test_kanban_dashboard_plugin.py +++ b/tests/plugins/test_kanban_dashboard_plugin.py @@ -505,9 +505,11 @@ def test_ws_events_rejects_when_token_required(tmp_path, monkeypatch): kb.init_db() # Stub web_server so _check_ws_token has a token to compare against. + import hermes_cli import types stub = types.SimpleNamespace(_SESSION_TOKEN="secret-xyz") monkeypatch.setitem(sys.modules, "hermes_cli.web_server", stub) + monkeypatch.setattr(hermes_cli, "web_server", stub, raising=False) app = FastAPI() app.include_router(_load_plugin_router(), prefix="/api/plugins/kanban") diff --git a/tests/test_tui_gateway_server.py b/tests/test_tui_gateway_server.py index 5a66a86dab..6d39505391 100644 --- a/tests/test_tui_gateway_server.py +++ b/tests/test_tui_gateway_server.py @@ -81,6 +81,173 @@ def test_dispatch_rejects_non_object_params(): } +def test_voice_toggle_returns_configured_record_key(monkeypatch): + monkeypatch.setattr( + server, + "_load_cfg", + lambda: {"voice": {"record_key": "ctrl+o"}}, + ) + monkeypatch.setitem( + sys.modules, + "tools.voice_mode", + types.SimpleNamespace( + check_voice_requirements=lambda: {"available": True, "details": ""} + ), + ) + # ``voice.toggle`` action=on mutates ``os.environ["HERMES_VOICE"]`` + # directly (CLI parity, runtime-only flag). Take monkeypatch + # ownership of the var so the change is reverted at teardown and + # later tests don't inherit a stale ON state (Copilot round-5 + # review on #19835). + monkeypatch.setenv("HERMES_VOICE", "0") + + on_resp = server.dispatch( + {"id": "voice-on", "method": "voice.toggle", "params": {"action": "on"}} + ) + status_resp = server.dispatch( + {"id": "voice-status", "method": "voice.toggle", "params": {"action": "status"}} + ) + + assert on_resp["result"]["record_key"] == "ctrl+o" + assert status_resp["result"]["record_key"] == "ctrl+o" + + +def test_voice_toggle_handles_non_dict_voice_cfg(monkeypatch): + """Round-3 Copilot review regression on #19835. + + ``_load_cfg()`` is raw ``yaml.safe_load()`` output — a hand-edited + ``voice: true`` / ``voice: cmd+b`` / ``voice: null`` leaves ``voice`` + as a bool/str/None, not a dict. Previously ``.get("record_key")`` + on a non-dict broke every ``voice.toggle`` branch. Now it falls + back to the documented default. + """ + monkeypatch.setitem( + sys.modules, + "tools.voice_mode", + types.SimpleNamespace( + check_voice_requirements=lambda: {"available": True, "details": ""} + ), + ) + + for bad in (True, "cmd+b", None, 42, ["ctrl+b"]): + monkeypatch.setattr(server, "_load_cfg", lambda b=bad: {"voice": b}) + + status_resp = server.dispatch( + {"id": "voice-status", "method": "voice.toggle", "params": {"action": "status"}} + ) + + assert status_resp["result"]["record_key"] == "ctrl+b", ( + f"voice.record_key fell back to default for voice={bad!r}" + ) + + # Round-4 follow-up: the YAML root itself may be a non-dict. A + # hand-edit that collapses config.yaml to a scalar / list would + # otherwise crash ``.get("voice")`` before the inner isinstance + # guard gets a chance to run. + for bad_root in (True, None, [], "ctrl+b", 42): + monkeypatch.setattr(server, "_load_cfg", lambda r=bad_root: r) + + status_resp = server.dispatch( + {"id": "voice-status-root", "method": "voice.toggle", "params": {"action": "status"}} + ) + + assert status_resp["result"]["record_key"] == "ctrl+b", ( + f"voice.record_key fell back to default for root={bad_root!r}" + ) + + +def test_voice_record_start_handles_non_dict_voice_cfg(monkeypatch): + """Round-7 Copilot review regression on #19835. + + The ``voice.record`` start path previously read + ``_load_cfg().get("voice", {}).get(...)`` without any shape checks. + When ``voice`` is a non-dict (bool/scalar/list) ``get`` raises + AttributeError and the handler returns 5025 instead of falling + back to the VAD defaults. Now it uses ``_voice_cfg_dict()`` and + non-numeric silence values are coerced to the documented defaults. + """ + captured: dict = {} + + def fake_start_continuous(**kwargs): + captured.update(kwargs) + + monkeypatch.setitem( + sys.modules, + "hermes_cli.voice", + types.SimpleNamespace(start_continuous=fake_start_continuous, stop_continuous=lambda: None), + ) + monkeypatch.setenv("HERMES_VOICE", "1") + + for bad in (True, "cmd+b", None, 42, ["ctrl+b"], {"silence_threshold": "loud"}): + captured.clear() + monkeypatch.setattr(server, "_load_cfg", lambda b=bad: {"voice": b}) + + resp = server.dispatch( + {"id": "voice-record", "method": "voice.record", "params": {"action": "start"}} + ) + + assert "result" in resp, f"voice.record raised for voice={bad!r}: {resp.get('error')}" + assert resp["result"]["status"] == "recording" + assert captured["silence_threshold"] == 200 + assert captured["silence_duration"] == 3.0 + + # Round-12 Copilot review regression on #19835: ``bool`` is a subclass + # of ``int``, so the naive ``isinstance(threshold, (int, float))`` + # guard would forward ``silence_threshold: true`` as ``1`` instead + # of falling back to the documented 200 default. + for bad_bool_cfg in ( + {"silence_threshold": True, "silence_duration": False}, + {"silence_threshold": False}, + {"silence_duration": True}, + ): + captured.clear() + monkeypatch.setattr(server, "_load_cfg", lambda c=bad_bool_cfg: {"voice": c}) + + resp = server.dispatch( + {"id": "voice-record-bool", "method": "voice.record", "params": {"action": "start"}} + ) + + assert "result" in resp, f"voice.record raised for bool cfg={bad_bool_cfg!r}" + assert captured["silence_threshold"] == 200, ( + f"bool silence_threshold leaked through for {bad_bool_cfg!r}" + ) + assert captured["silence_duration"] == 3.0, ( + f"bool silence_duration leaked through for {bad_bool_cfg!r}" + ) + + +def test_voice_toggle_tts_branch_also_carries_record_key(monkeypatch): + """Round-2 Copilot review regression on #19835. + + The ``tts`` branch used to omit ``record_key`` from its response, so a + TUI client would parse ``r.record_key ?? 'ctrl+b'`` and reset a + custom binding to the default on every TTS toggle. Every branch of + ``voice.toggle`` now carries the configured key so frontend state + stays authoritative. + """ + monkeypatch.setattr( + server, + "_load_cfg", + lambda: {"voice": {"record_key": "ctrl+space"}}, + ) + monkeypatch.setitem( + sys.modules, + "tools.voice_mode", + types.SimpleNamespace( + check_voice_requirements=lambda: {"available": True, "details": ""} + ), + ) + monkeypatch.setenv("HERMES_VOICE", "1") + monkeypatch.delenv("HERMES_VOICE_TTS", raising=False) + + tts_resp = server.dispatch( + {"id": "voice-tts", "method": "voice.toggle", "params": {"action": "tts"}} + ) + + assert tts_resp["result"]["record_key"] == "ctrl+space" + assert tts_resp["result"]["tts"] is True + + def test_load_enabled_toolsets_prefers_tui_env(monkeypatch): monkeypatch.setenv("HERMES_TUI_TOOLSETS", "web, terminal, ,memory") diff --git a/tests/tools/test_local_env_cwd_recovery.py b/tests/tools/test_local_env_cwd_recovery.py new file mode 100644 index 0000000000..59aa8f1067 --- /dev/null +++ b/tests/tools/test_local_env_cwd_recovery.py @@ -0,0 +1,187 @@ +"""Tests for LocalEnvironment recovery when ``self.cwd`` is deleted. + +When a tool call inside the persistent terminal session ``rm -rf``'s its own +working directory, the next ``subprocess.Popen(..., cwd=self.cwd)`` would +otherwise raise ``FileNotFoundError`` before bash starts, wedging every +subsequent terminal/file-tool call until the gateway restarts. + +Regression coverage for https://github.com/NousResearch/hermes-agent/issues/17558. +""" + +import os +import shutil +import tempfile +import threading +from unittest.mock import MagicMock, patch + +from tools.environments.local import ( + LocalEnvironment, + _resolve_safe_cwd, +) + + +class TestResolveSafeCwd: + """Pure-function unit tests for the recovery helper.""" + + def test_returns_cwd_when_directory_exists(self, tmp_path): + path = str(tmp_path) + assert _resolve_safe_cwd(path) == path + + def test_walks_up_to_first_existing_ancestor(self, tmp_path): + nested = tmp_path / "child" / "grandchild" + nested.mkdir(parents=True) + deleted = str(nested) + shutil.rmtree(tmp_path / "child") + + # The deepest existing ancestor on the path is tmp_path itself. + assert _resolve_safe_cwd(deleted) == str(tmp_path) + + def test_falls_back_when_path_is_empty(self): + assert _resolve_safe_cwd("") == tempfile.gettempdir() + + def test_returns_tempdir_when_nothing_on_path_exists(self, monkeypatch): + monkeypatch.setattr(os.path, "isdir", lambda p: False) + assert _resolve_safe_cwd("/no/such/dir") == tempfile.gettempdir() + + def test_returns_root_when_only_root_exists(self, monkeypatch): + """If every ancestor except the filesystem root is gone, the root + itself is still a valid recovery target — don't skip it just because + ``os.path.dirname('/') == '/'`` is the loop's exit condition.""" + sep = os.path.sep + monkeypatch.setattr(os.path, "isdir", lambda p: p == sep) + assert _resolve_safe_cwd("/no/such/deep/dir") == sep + + +def _fake_interrupt(): + return threading.Event() + + +def _make_fake_popen(captured: dict, fds: list): + """Build a fake ``Popen`` whose ``stdout`` exposes a real OS file + descriptor so ``BaseEnvironment._wait_for_process`` can call + ``select.select([fd], ...)`` and ``os.read(fd, ...)`` against it without + tripping ``TypeError: fileno() returned a non-integer`` from a MagicMock + ``fileno()`` (or worse, accidentally reading from the test runner's own + stdout). + + The pipe's write end is closed immediately so the drain loop sees EOF on + the first iteration. Every fd handed out is appended to ``fds`` so the + caller can clean up after the test. + """ + def fake_popen(cmd, **kwargs): + captured["cwd"] = kwargs.get("cwd") + captured["env"] = kwargs.get("env", {}) + read_fd, write_fd = os.pipe() + os.close(write_fd) + stdout = os.fdopen(read_fd, "rb", buffering=0) + fds.append(stdout) + proc = MagicMock() + proc.poll.return_value = 0 + proc.returncode = 0 + proc.stdout = stdout + proc.stdin = MagicMock() + return proc + return fake_popen + + +def _close_fds(fds): + for f in fds: + try: + f.close() + except Exception: + pass + + +class TestRunBashCwdRecovery: + """End-to-end recovery: deleted ``self.cwd`` must not crash Popen.""" + + def test_recovers_when_cwd_deleted_after_init(self, tmp_path, caplog): + """Reproduces the wedge from #17558: cwd was valid when the + snapshot was taken, but a subsequent command deleted it before the + next ``Popen``.""" + wedged = tmp_path / "wedge-repro" + wedged.mkdir() + + with patch.object(LocalEnvironment, "init_session", autospec=True, return_value=None): + env = LocalEnvironment(cwd=str(wedged), timeout=10) + + # The previous tool call deleted the working directory. + shutil.rmtree(wedged) + assert env.cwd == str(wedged) and not os.path.isdir(env.cwd) + + captured = {} + fds: list = [] + try: + with patch("tools.environments.local._find_bash", return_value="/bin/bash"), \ + patch("subprocess.Popen", side_effect=_make_fake_popen(captured, fds)), \ + patch("tools.terminal_tool._interrupt_event", _fake_interrupt()), \ + caplog.at_level("WARNING", logger="tools.environments.local"): + env.execute("echo hello") + finally: + _close_fds(fds) + + # Popen must have been handed a real, existing directory. + assert captured["cwd"] == str(tmp_path) + assert os.path.isdir(captured["cwd"]) + + # ``self.cwd`` is updated so the next call doesn't re-warn. + assert env.cwd == str(tmp_path) + + # The warning surfaces the wedge so it isn't silently masked. + assert any("missing on disk" in rec.message for rec in caplog.records) + + def test_no_warning_when_cwd_still_exists(self, tmp_path, caplog): + with patch.object(LocalEnvironment, "init_session", autospec=True, return_value=None): + env = LocalEnvironment(cwd=str(tmp_path), timeout=10) + + captured = {} + fds: list = [] + try: + with patch("tools.environments.local._find_bash", return_value="/bin/bash"), \ + patch("subprocess.Popen", side_effect=_make_fake_popen(captured, fds)), \ + patch("tools.terminal_tool._interrupt_event", _fake_interrupt()), \ + caplog.at_level("WARNING", logger="tools.environments.local"): + env.execute("echo hello") + finally: + _close_fds(fds) + + assert captured["cwd"] == str(tmp_path) + assert env.cwd == str(tmp_path) + assert not any("missing on disk" in rec.message for rec in caplog.records) + + +class TestUpdateCwdRejectsMissingPaths: + """``_update_cwd`` must not propagate a deleted path back into ``self.cwd``.""" + + def test_skips_assignment_when_marker_path_missing(self, tmp_path): + original = tmp_path / "starting" + original.mkdir() + + with patch.object(LocalEnvironment, "init_session", autospec=True, return_value=None): + env = LocalEnvironment(cwd=str(original), timeout=10) + + # Simulate the stale-marker case: the prior command's ``pwd -P`` left + # a path in the cwd file, but that path has since been deleted. + deleted = tmp_path / "wedge-repro" + with open(env._cwd_file, "w") as f: + f.write(str(deleted)) + + env._update_cwd({"output": "", "returncode": 0}) + + assert env.cwd == str(original) + + def test_accepts_assignment_when_marker_path_exists(self, tmp_path): + original = tmp_path / "starting" + original.mkdir() + new_dir = tmp_path / "next" + new_dir.mkdir() + + with patch.object(LocalEnvironment, "init_session", autospec=True, return_value=None): + env = LocalEnvironment(cwd=str(original), timeout=10) + + with open(env._cwd_file, "w") as f: + f.write(str(new_dir)) + + env._update_cwd({"output": "", "returncode": 0}) + + assert env.cwd == str(new_dir) diff --git a/tools/environments/local.py b/tools/environments/local.py index 3200e63e60..72d4f04d9c 100644 --- a/tools/environments/local.py +++ b/tools/environments/local.py @@ -1,5 +1,6 @@ """Local execution environment — spawn-per-call with session snapshot.""" +import logging import os import platform import shutil @@ -12,6 +13,35 @@ from tools.environments.base import BaseEnvironment, _pipe_stdin _IS_WINDOWS = platform.system() == "Windows" +logger = logging.getLogger(__name__) + + +def _resolve_safe_cwd(cwd: str) -> str: + """Return ``cwd`` if it exists as a directory, else the nearest existing + ancestor. Falls back to ``tempfile.gettempdir()`` only if walking up the + path can't find any existing directory (effectively never on a healthy + filesystem, but cheap belt-and-braces). + + Used by ``_run_bash`` to recover when the configured cwd is gone — most + commonly because a previous tool call deleted its own working directory + (issue #17558). Without this guard, ``subprocess.Popen(..., cwd=...)`` + raises ``FileNotFoundError`` before bash starts, wedging every subsequent + terminal call until the gateway restarts. + """ + if cwd and os.path.isdir(cwd): + return cwd + parent = os.path.dirname(cwd) if cwd else "" + while parent: + if os.path.isdir(parent): + return parent + next_parent = os.path.dirname(parent) + if next_parent == parent: + # Reached the filesystem root and it doesn't exist either — + # genuinely nothing to fall back to except the temp dir. + break + parent = next_parent + return tempfile.gettempdir() + # Hermes-internal env vars that should NOT leak into terminal subprocesses. _HERMES_PROVIDER_ENV_FORCE_PREFIX = "_HERMES_FORCE_" @@ -358,6 +388,21 @@ class LocalEnvironment(BaseEnvironment): args = [bash, "-l", "-c", cmd_string] if login else [bash, "-c", cmd_string] run_env = _make_run_env(self.env) + # Recover when the cwd has been deleted out from under us — usually by + # a previous tool call that ran ``rm -rf`` on its own working dir + # (issue #17558). Popen would otherwise raise FileNotFoundError on + # the cwd before bash starts, wedging every subsequent call until the + # gateway restarts. + safe_cwd = _resolve_safe_cwd(self.cwd) + if safe_cwd != self.cwd: + logger.warning( + "LocalEnvironment cwd %r is missing on disk; " + "falling back to %r so terminal commands keep working.", + self.cwd, + safe_cwd, + ) + self.cwd = safe_cwd + proc = subprocess.Popen( args, text=True, @@ -452,11 +497,17 @@ class LocalEnvironment(BaseEnvironment): pass def _update_cwd(self, result: dict): - """Read CWD from temp file (local-only, no round-trip needed).""" + """Read CWD from temp file (local-only, no round-trip needed). + + Skip the assignment when the path no longer exists as a directory — + ``pwd -P`` on a deleted cwd can leave a stale value in the marker + file, and propagating it would re-wedge the next ``Popen``. The + ``_run_bash`` recovery path will resolve a safe fallback if needed. + """ try: with open(self._cwd_file) as f: cwd_path = f.read().strip() - if cwd_path: + if cwd_path and os.path.isdir(cwd_path): self.cwd = cwd_path except (OSError, FileNotFoundError): pass diff --git a/tools/process_registry.py b/tools/process_registry.py index da5c8d224b..0fc312185d 100644 --- a/tools/process_registry.py +++ b/tools/process_registry.py @@ -41,7 +41,7 @@ import time import uuid _IS_WINDOWS = platform.system() == "Windows" -from tools.environments.local import _find_shell, _sanitize_subprocess_env +from tools.environments.local import _find_shell, _resolve_safe_cwd, _sanitize_subprocess_env from dataclasses import dataclass, field from typing import Any, Dict, List, Optional @@ -480,7 +480,7 @@ class ProcessRegistry: command=command, task_id=task_id, session_key=session_key, - cwd=cwd or os.getcwd(), + cwd=_resolve_safe_cwd(cwd or os.getcwd()), started_at=time.time(), ) diff --git a/tui_gateway/server.py b/tui_gateway/server.py index 3edef48c49..b9a144652e 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -5799,6 +5799,30 @@ def _voice_tts_enabled() -> bool: return os.environ.get("HERMES_VOICE_TTS", "").strip() == "1" +def _voice_cfg_dict() -> dict: + """Shape-safe accessor for the ``voice:`` block in config.yaml. + + ``_load_cfg()`` returns raw ``yaml.safe_load()`` output, so both the + root AND ``voice`` may be any YAML scalar / list / None. A hand-edit + like ``voice: true`` or a malformed top-level config that parses to + a scalar would otherwise break ``.get("…")`` and take every + ``voice.*`` branch down with it (Copilot round-3..7 review on + #19835). Coerce through ``isinstance`` at every level so malformed + config falls back to an empty dict instead of crashing /voice. + """ + cfg = _load_cfg() + voice_cfg = cfg.get("voice") if isinstance(cfg, dict) else None + + return voice_cfg if isinstance(voice_cfg, dict) else {} + + +def _voice_record_key() -> str: + """Current ``voice.record_key`` value, documented default on error.""" + record_key = _voice_cfg_dict().get("record_key") + + return str(record_key) if isinstance(record_key, str) and record_key else "ctrl+b" + + @method("voice.toggle") def _(rid, params: dict) -> dict: """CLI parity for the ``/voice`` slash command. @@ -5819,8 +5843,13 @@ def _(rid, params: dict) -> dict: # Mirror CLI's _show_voice_status: include STT/TTS provider # availability so the user can tell at a glance *why* voice mode # isn't working ("STT provider: MISSING ..." is the common case). + # ``record_key`` mirrors the configured ``voice.record_key`` so the + # TUI can both bind it (frontend ``isVoiceToggleKey``) and display + # it in /voice status — previously the TUI hardcoded Ctrl+B and + # ignored the config (#18994). payload: dict = { "enabled": _voice_mode_enabled(), + "record_key": _voice_record_key(), "tts": _voice_tts_enabled(), } try: @@ -5857,7 +5886,14 @@ def _(rid, params: dict) -> dict: except Exception as e: logger.warning("voice: stop_continuous failed during toggle off: %s", e) - return _ok(rid, {"enabled": enabled, "tts": _voice_tts_enabled()}) + return _ok( + rid, + { + "enabled": enabled, + "record_key": _voice_record_key(), + "tts": _voice_tts_enabled(), + }, + ) if action == "tts": if not _voice_mode_enabled(): @@ -5865,7 +5901,18 @@ def _(rid, params: dict) -> dict: new_value = not _voice_tts_enabled() # Runtime-only flag (CLI parity) — see voice.toggle on/off above. os.environ["HERMES_VOICE_TTS"] = "1" if new_value else "0" - return _ok(rid, {"enabled": True, "tts": new_value}) + # Include ``record_key`` on every branch so a /voice tts toggle + # doesn't reset the TUI's cached shortcut to the default when a + # user has a custom binding configured (Copilot review, round 2 + # on #19835). Keeps parity with the status/on/off branches above. + return _ok( + rid, + { + "enabled": True, + "record_key": _voice_record_key(), + "tts": new_value, + }, + ) return _err(rid, 4013, f"unknown voice action: {action}") @@ -5897,15 +5944,26 @@ def _(rid, params: dict) -> dict: from hermes_cli.voice import start_continuous - voice_cfg = _load_cfg().get("voice", {}) + # Shape-safe lookups: malformed ``voice:`` YAML (bool/scalar/list) + # must not crash /voice with a 5025 — fall back to VAD defaults. + # + # Exclude ``bool`` from the numeric check since Python's bool is + # a subclass of int — a hand-edit like ``silence_threshold: true`` + # would otherwise forward as ``1`` instead of falling back to + # the documented 200 / 3.0 defaults (Copilot round-12 on #19835). + voice_cfg = _voice_cfg_dict() + threshold = voice_cfg.get("silence_threshold") + duration = voice_cfg.get("silence_duration") + safe_threshold = threshold if isinstance(threshold, (int, float)) and not isinstance(threshold, bool) else 200 + safe_duration = duration if isinstance(duration, (int, float)) and not isinstance(duration, bool) else 3.0 start_continuous( on_transcript=lambda t: _voice_emit("voice.transcript", {"text": t}), on_status=lambda s: _voice_emit("voice.status", {"state": s}), on_silent_limit=lambda: _voice_emit( "voice.transcript", {"no_speech_limit": True} ), - silence_threshold=voice_cfg.get("silence_threshold", 200), - silence_duration=voice_cfg.get("silence_duration", 3.0), + silence_threshold=safe_threshold, + silence_duration=safe_duration, ) return _ok(rid, {"status": "recording"}) diff --git a/ui-tui/src/__tests__/createSlashHandler.test.ts b/ui-tui/src/__tests__/createSlashHandler.test.ts index e8c50c05d2..c9447f16d8 100644 --- a/ui-tui/src/__tests__/createSlashHandler.test.ts +++ b/ui-tui/src/__tests__/createSlashHandler.test.ts @@ -173,6 +173,64 @@ describe('createSlashHandler', () => { expect(ctx.transcript.sys).toHaveBeenCalledWith(expect.stringContaining('usage: /skills')) }) + // Regressions from Copilot review on #19835: /voice output + frontend + // binding state must both track the gateway's fresh ``record_key`` on + // every response, or a config edit shows the new shortcut in text + // while push-to-talk still fires the old one until the next mtime + // poll (~5s). + it('/voice status renders the gateway record_key and pushes it into frontend state', async () => { + const rpc = vi.fn(() => Promise.resolve({ enabled: true, record_key: 'ctrl+space', tts: false })) + const ctx = buildCtx({ gateway: { ...buildGateway(), rpc } }) + + expect(createSlashHandler(ctx)('/voice status')).toBe(true) + await vi.waitFor(() => { + expect(ctx.transcript.sys).toHaveBeenCalledWith(' Record key: Ctrl+Space') + }) + expect(ctx.voice.setVoiceRecordKey).toHaveBeenCalledWith( + expect.objectContaining({ ch: 'space', mod: 'ctrl', named: 'space' }) + ) + }) + + it('/voice on renders the configured binding for the start/stop hint', async () => { + const rpc = vi.fn(() => Promise.resolve({ enabled: true, record_key: 'alt+r', tts: false })) + const ctx = buildCtx({ gateway: { ...buildGateway(), rpc } }) + + expect(createSlashHandler(ctx)('/voice on')).toBe(true) + await vi.waitFor(() => { + expect(ctx.transcript.sys).toHaveBeenCalledWith('Voice mode enabled') + expect(ctx.transcript.sys).toHaveBeenCalledWith(' Alt+R to start/stop recording') + }) + expect(ctx.voice.setVoiceRecordKey).toHaveBeenCalledWith( + expect.objectContaining({ ch: 'r', mod: 'alt' }) + ) + }) + + it('/voice falls back to Ctrl+B when the gateway response omits record_key', async () => { + const rpc = vi.fn(() => Promise.resolve({ enabled: false, tts: false })) + const ctx = buildCtx({ gateway: { ...buildGateway(), rpc } }) + + expect(createSlashHandler(ctx)('/voice status')).toBe(true) + await vi.waitFor(() => { + expect(ctx.transcript.sys).toHaveBeenCalledWith(' Record key: Ctrl+B') + }) + }) + + // Round-2 Copilot review on #19835: a response missing ``record_key`` + // (e.g. the old tts branch, or any future branch that forgets to + // include it) MUST NOT clobber the user's cached binding back to + // Ctrl+B. The label still renders the default for display; the + // frontend state keeps whatever was last authoritatively set. + it('/voice tts without record_key does not clobber cached frontend binding', async () => { + const rpc = vi.fn(() => Promise.resolve({ enabled: true, tts: true })) + const ctx = buildCtx({ gateway: { ...buildGateway(), rpc } }) + + expect(createSlashHandler(ctx)('/voice tts')).toBe(true) + await vi.waitFor(() => { + expect(ctx.transcript.sys).toHaveBeenCalledWith('Voice TTS enabled.') + }) + expect(ctx.voice.setVoiceRecordKey).not.toHaveBeenCalled() + }) + it('cycles details mode and persists it', async () => { const ctx = buildCtx() @@ -648,7 +706,8 @@ const buildTranscript = () => ({ }) const buildVoice = () => ({ - setVoiceEnabled: vi.fn() + setVoiceEnabled: vi.fn(), + setVoiceRecordKey: vi.fn() }) interface Ctx { diff --git a/ui-tui/src/__tests__/platform.test.ts b/ui-tui/src/__tests__/platform.test.ts index 4166f0b71f..77f1347a3a 100644 --- a/ui-tui/src/__tests__/platform.test.ts +++ b/ui-tui/src/__tests__/platform.test.ts @@ -67,11 +67,15 @@ describe('isVoiceToggleKey', () => { expect(isVoiceToggleKey({ ctrl: true, meta: false, super: false }, 'B')).toBe(true) }) - it('matches Cmd+B on macOS (preserve platform muscle memory)', async () => { + it('matches kitty-style Cmd+B on macOS via key.super', async () => { const { isVoiceToggleKey } = await importPlatform('darwin') - expect(isVoiceToggleKey({ ctrl: false, meta: true, super: false }, 'b')).toBe(true) expect(isVoiceToggleKey({ ctrl: false, meta: false, super: true }, 'b')).toBe(true) + // ``key.meta`` is NOT accepted as Cmd — hermes-ink uses meta for + // Alt too, so accepting it leaked Alt+B into the default binding + // (Copilot round-6 review on #19835). Legacy-terminal mac users + // get strict Ctrl+B. + expect(isVoiceToggleKey({ ctrl: false, meta: true, super: false }, 'b')).toBe(false) }) it('matches Ctrl+B on non-macOS platforms', async () => { @@ -89,6 +93,449 @@ describe('isVoiceToggleKey', () => { }) }) +describe('parseVoiceRecordKey (#18994)', () => { + it('falls back to Ctrl+B for empty input', async () => { + const { DEFAULT_VOICE_RECORD_KEY, parseVoiceRecordKey } = await importPlatform('linux') + + expect(parseVoiceRecordKey('')).toEqual(DEFAULT_VOICE_RECORD_KEY) + }) + + it('parses ctrl+ bindings', async () => { + const { parseVoiceRecordKey } = await importPlatform('linux') + + expect(parseVoiceRecordKey('ctrl+o')).toEqual({ ch: 'o', mod: 'ctrl', raw: 'ctrl+o' }) + expect(parseVoiceRecordKey('Ctrl+R')).toEqual({ ch: 'r', mod: 'ctrl', raw: 'ctrl+r' }) + }) + + it('parses alt/super aliases', async () => { + const { parseVoiceRecordKey } = await importPlatform('linux') + + expect(parseVoiceRecordKey('alt+b').mod).toBe('alt') + expect(parseVoiceRecordKey('option+b').mod).toBe('alt') + expect(parseVoiceRecordKey('super+b').mod).toBe('super') + expect(parseVoiceRecordKey('win+b').mod).toBe('super') + }) + + it('treats ambiguous mac modifiers (meta / cmd / command) as unrecognised', async () => { + const { DEFAULT_VOICE_RECORD_KEY, parseVoiceRecordKey } = await importPlatform('linux') + + // ``meta`` / ``cmd`` / ``command`` are ambiguous on the wire: + // hermes-ink sets ``key.meta`` for plain Alt on every platform AND + // for Cmd on legacy macOS terminals. Accepting any of them would + // produce a display/binding mismatch (Copilot round-6 review on + // #19835). Users on modern kitty-style terminals spell the + // platform action modifier ``super`` / ``win``. + expect(parseVoiceRecordKey('meta+b')).toEqual(DEFAULT_VOICE_RECORD_KEY) + expect(parseVoiceRecordKey('cmd+b')).toEqual(DEFAULT_VOICE_RECORD_KEY) + expect(parseVoiceRecordKey('command+b')).toEqual(DEFAULT_VOICE_RECORD_KEY) + }) + + it('parses named keys (space, enter, tab, escape, backspace, delete)', async () => { + const { parseVoiceRecordKey } = await importPlatform('linux') + + // Every named token from the CLI's prompt_toolkit ``c-`` set is + // accepted with both the canonical name and its common alias. + expect(parseVoiceRecordKey('ctrl+space')).toEqual({ + ch: 'space', + mod: 'ctrl', + named: 'space', + raw: 'ctrl+space' + }) + expect(parseVoiceRecordKey('alt+enter').named).toBe('enter') + expect(parseVoiceRecordKey('alt+return').named).toBe('enter') // ``return`` ↔ ``enter`` + expect(parseVoiceRecordKey('ctrl+tab').named).toBe('tab') + expect(parseVoiceRecordKey('ctrl+escape').named).toBe('escape') + expect(parseVoiceRecordKey('ctrl+esc').named).toBe('escape') // ``esc`` alias + expect(parseVoiceRecordKey('ctrl+backspace').named).toBe('backspace') + expect(parseVoiceRecordKey('ctrl+delete').named).toBe('delete') + expect(parseVoiceRecordKey('ctrl+del').named).toBe('delete') // ``del`` alias + }) + + it('falls back to Ctrl+B for unrecognised multi-character tokens', async () => { + const { DEFAULT_VOICE_RECORD_KEY, parseVoiceRecordKey } = await importPlatform('linux') + + // Typos / unsupported names (``ctrl+spcae``, ``ctrl+f5``, …) fall back + // to the documented Ctrl+B default rather than silently disabling the + // binding. + expect(parseVoiceRecordKey('ctrl+spcae')).toEqual(DEFAULT_VOICE_RECORD_KEY) + expect(parseVoiceRecordKey('ctrl+f5')).toEqual(DEFAULT_VOICE_RECORD_KEY) + }) + + // Round-3 Copilot review regressions on #19835. + it('does not throw on non-string YAML scalars — falls back instead', async () => { + const { DEFAULT_VOICE_RECORD_KEY, parseVoiceRecordKey } = await importPlatform('linux') + + // ``config.get full`` surfaces raw YAML values; ``voice.record_key: 1`` + // or ``voice.record_key: true`` would otherwise crash ``.trim()``. + expect(parseVoiceRecordKey(1 as unknown as string)).toEqual(DEFAULT_VOICE_RECORD_KEY) + expect(parseVoiceRecordKey(true as unknown as string)).toEqual(DEFAULT_VOICE_RECORD_KEY) + expect(parseVoiceRecordKey(null as unknown as string)).toEqual(DEFAULT_VOICE_RECORD_KEY) + expect(parseVoiceRecordKey(undefined as unknown as string)).toEqual(DEFAULT_VOICE_RECORD_KEY) + expect(parseVoiceRecordKey({} as unknown as string)).toEqual(DEFAULT_VOICE_RECORD_KEY) + }) + + it('rejects multi-modifier chords rather than silently dropping extras', async () => { + const { DEFAULT_VOICE_RECORD_KEY, parseVoiceRecordKey } = await importPlatform('linux') + + // Previously ``ctrl+alt+r`` parsed as ``ctrl+r`` and ``cmd+ctrl+b`` as + // ``super+b`` — a typo silently bound a different shortcut. Now a + // multi-modifier spelling falls back to the documented default. + expect(parseVoiceRecordKey('ctrl+alt+r')).toEqual(DEFAULT_VOICE_RECORD_KEY) + expect(parseVoiceRecordKey('cmd+ctrl+b')).toEqual(DEFAULT_VOICE_RECORD_KEY) + expect(parseVoiceRecordKey('alt+ctrl+space')).toEqual(DEFAULT_VOICE_RECORD_KEY) + }) + + // Round-4 Copilot review regressions on #19835. + it('rejects bare-char configs without an explicit modifier', async () => { + const { DEFAULT_VOICE_RECORD_KEY, parseVoiceRecordKey } = await importPlatform('linux') + + // The classic CLI's prompt_toolkit binds raw-char configs to the key + // itself (``c-o`` requires an explicit modifier); rewriting ``o`` + // → ``ctrl+o`` would silently diverge the two runtimes. Refuse. + expect(parseVoiceRecordKey('o')).toEqual(DEFAULT_VOICE_RECORD_KEY) + expect(parseVoiceRecordKey('b')).toEqual(DEFAULT_VOICE_RECORD_KEY) + expect(parseVoiceRecordKey('space')).toEqual(DEFAULT_VOICE_RECORD_KEY) + expect(parseVoiceRecordKey('escape')).toEqual(DEFAULT_VOICE_RECORD_KEY) + }) + + it('rejects ctrl+c / ctrl+d / ctrl+l — reserved by the TUI input handler', async () => { + const { DEFAULT_VOICE_RECORD_KEY, parseVoiceRecordKey } = await importPlatform('linux') + + // ``useInputHandlers()`` intercepts these before the voice check, + // so a binding like ``ctrl+c`` would be advertised but never fire. + // Fall back to the documented default instead of lying to the user. + expect(parseVoiceRecordKey('ctrl+c')).toEqual(DEFAULT_VOICE_RECORD_KEY) + expect(parseVoiceRecordKey('ctrl+d')).toEqual(DEFAULT_VOICE_RECORD_KEY) + expect(parseVoiceRecordKey('ctrl+l')).toEqual(DEFAULT_VOICE_RECORD_KEY) + // Alt-modifier versions of those letters are NOT intercepted, so + // they remain usable. + expect(parseVoiceRecordKey('alt+c').mod).toBe('alt') + // ``ctrl+x`` is intentionally allowed — only intercepted during + // queue-edit (``queueEditIdx !== null``), so the voice binding + // works for most of the session (Copilot round-8 review). + expect(parseVoiceRecordKey('ctrl+x').mod).toBe('ctrl') + expect(parseVoiceRecordKey('ctrl+x').ch).toBe('x') + }) + + it('rejects super+{c,d,l,v} on macOS — action-mod chords are claimed before voice', async () => { + const { DEFAULT_VOICE_RECORD_KEY, parseVoiceRecordKey } = await importPlatform('darwin') + + // On macOS super+c/d/l/v are copy / exit / clear / paste. Reject at + // parse time so /voice status doesn't advertise dead bindings. + expect(parseVoiceRecordKey('super+c')).toEqual(DEFAULT_VOICE_RECORD_KEY) + expect(parseVoiceRecordKey('super+d')).toEqual(DEFAULT_VOICE_RECORD_KEY) + expect(parseVoiceRecordKey('super+l')).toEqual(DEFAULT_VOICE_RECORD_KEY) + expect(parseVoiceRecordKey('super+v')).toEqual(DEFAULT_VOICE_RECORD_KEY) + // Other super letters still work (no global chord claims them). + expect(parseVoiceRecordKey('super+b').mod).toBe('super') + expect(parseVoiceRecordKey('super+o').mod).toBe('super') + }) + + it('allows super+{c,d,l,v} on Linux/Windows — those globals key off Ctrl, not Super', async () => { + const { parseVoiceRecordKey } = await importPlatform('linux') + + // Kitty/CSI-u users on non-mac report Cmd/Super as ``key.super``, + // but the TUI's global shortcuts (copy/exit/clear/paste) key off + // Ctrl there, so ``super+`` doesn't collide. Reject would + // silently coerce valid configs to Ctrl+B (Copilot round-8 review). + expect(parseVoiceRecordKey('super+c').mod).toBe('super') + expect(parseVoiceRecordKey('super+d').mod).toBe('super') + expect(parseVoiceRecordKey('super+l').mod).toBe('super') + expect(parseVoiceRecordKey('super+v').mod).toBe('super') + }) + + it('rejects alt+{c,d,l} on macOS — meta-as-alt collides with isAction', async () => { + const { DEFAULT_VOICE_RECORD_KEY, parseVoiceRecordKey } = await importPlatform('darwin') + + // hermes-ink reports Alt as ``key.meta`` on many terminals, and + // ``isActionMod`` on darwin accepts ``key.meta`` as the action + // modifier. So ``alt+c`` / ``alt+d`` / ``alt+l`` get claimed by + // isCopyShortcut / isAction('d') / isAction('l') before voice + // runs (Copilot round-12 on #19835). + expect(parseVoiceRecordKey('alt+c')).toEqual(DEFAULT_VOICE_RECORD_KEY) + expect(parseVoiceRecordKey('alt+d')).toEqual(DEFAULT_VOICE_RECORD_KEY) + expect(parseVoiceRecordKey('alt+l')).toEqual(DEFAULT_VOICE_RECORD_KEY) + // Other alt letters stay usable on darwin. + expect(parseVoiceRecordKey('alt+r').mod).toBe('alt') + expect(parseVoiceRecordKey('alt+space').mod).toBe('alt') + }) + + it('allows alt+{c,d,l} on Linux/Windows — non-mac isAction keys off Ctrl', async () => { + const { parseVoiceRecordKey } = await importPlatform('linux') + + // On Linux/Windows ``isActionMod`` ignores key.meta, so alt+ + // doesn't collide with copy/exit/clear. Those configs stay usable. + expect(parseVoiceRecordKey('alt+c').mod).toBe('alt') + expect(parseVoiceRecordKey('alt+d').mod).toBe('alt') + expect(parseVoiceRecordKey('alt+l').mod).toBe('alt') + }) + + // Round-5 Copilot review regressions on #19835. + it('super+ does NOT fire on key.meta-only events (Alt+X false-fire guard)', async () => { + const { isVoiceToggleKey, parseVoiceRecordKey } = await importPlatform('darwin') + + // hermes-ink sets ``key.meta`` for Alt/Option AND for bare Esc on + // some macOS terminals. The super branch used to accept + // ``isMac && key.meta`` as a Cmd fallback, which made super+ + // bindings silently fire on Alt+ / bare Esc. + const superB = parseVoiceRecordKey('super+b') + const superSpace = parseVoiceRecordKey('super+space') + const superEscape = parseVoiceRecordKey('super+escape') + + expect(isVoiceToggleKey({ ctrl: false, meta: true, super: false }, 'b', superB)).toBe(false) + expect(isVoiceToggleKey({ ctrl: false, meta: true, super: false }, ' ', superSpace)).toBe(false) + expect(isVoiceToggleKey({ ctrl: false, escape: true, meta: true, super: false }, '', superEscape)).toBe(false) + }) + + // Round-6 Copilot review regressions on #19835. + it('default ctrl+b does NOT fire on Alt+B via isActionMod meta leak', async () => { + const { DEFAULT_VOICE_RECORD_KEY, isVoiceToggleKey } = await importPlatform('darwin') + + // ``isActionMod(key)`` on darwin was accepting ``key.meta`` as the + // action modifier, so Alt+B (key.meta=true) fired the default + // ctrl+b binding. Now the Cmd-fallback path requires literal + // ``key.super`` on macOS and rejects ``key.meta``. + expect(isVoiceToggleKey({ ctrl: false, meta: true, super: false }, 'b', DEFAULT_VOICE_RECORD_KEY)).toBe(false) + // Literal Ctrl+B and Cmd+B (kitty-style) still work on darwin. + expect(isVoiceToggleKey({ ctrl: true, meta: false, super: false }, 'b', DEFAULT_VOICE_RECORD_KEY)).toBe(true) + expect(isVoiceToggleKey({ ctrl: false, meta: false, super: true }, 'b', DEFAULT_VOICE_RECORD_KEY)).toBe(true) + }) + + it('ctrl+ rejects chords with extra alt / meta / super bits', async () => { + const { isVoiceToggleKey, parseVoiceRecordKey } = await importPlatform('linux') + const ctrlO = parseVoiceRecordKey('ctrl+o') + + // ``ctrl+o`` must fire ONLY on literal Ctrl+O, not on + // Ctrl+Alt+O / Ctrl+Cmd+O / Ctrl+Meta+O — otherwise the runtime + // matches a different chord than the parser would let you + // configure. + expect(isVoiceToggleKey({ alt: true, ctrl: true, meta: false, super: false }, 'o', ctrlO)).toBe(false) + expect(isVoiceToggleKey({ ctrl: true, meta: true, super: false }, 'o', ctrlO)).toBe(false) + expect(isVoiceToggleKey({ ctrl: true, meta: false, super: true }, 'o', ctrlO)).toBe(false) + // Sanity: plain Ctrl+O still fires. + expect(isVoiceToggleKey({ ctrl: true, meta: false, super: false }, 'o', ctrlO)).toBe(true) + }) + + it('super+ rejects chords with extra ctrl / alt / meta bits', async () => { + const { isVoiceToggleKey, parseVoiceRecordKey } = await importPlatform('linux') + const superB = parseVoiceRecordKey('super+b') + + expect(isVoiceToggleKey({ alt: true, ctrl: false, meta: false, super: true }, 'b', superB)).toBe(false) + expect(isVoiceToggleKey({ ctrl: false, meta: true, super: true }, 'b', superB)).toBe(false) + expect(isVoiceToggleKey({ ctrl: true, meta: false, super: true }, 'b', superB)).toBe(false) + // Sanity: plain Super+B still fires. + expect(isVoiceToggleKey({ ctrl: false, meta: false, super: true }, 'b', superB)).toBe(true) + }) + + it('alt+escape does not fire on bare Esc meta-shape', async () => { + const { isVoiceToggleKey, parseVoiceRecordKey } = await importPlatform('darwin') + const altEscape = parseVoiceRecordKey('alt+escape') + + // Some terminals surface bare Esc as meta=true + escape=true. + expect(isVoiceToggleKey({ ctrl: false, escape: true, meta: true, super: false }, '', altEscape)).toBe(false) + // Explicit alt bit (kitty-style) still fires the configured chord. + expect(isVoiceToggleKey({ alt: true, ctrl: false, escape: true, meta: false, super: false }, '', altEscape)).toBe(true) + }) + + it('rejects matches when Shift is held (different chord than configured)', async () => { + const { isVoiceToggleKey, parseVoiceRecordKey } = await importPlatform('linux') + + // Parser rejects multi-modifier configs like ``ctrl+shift+tab``, + // so the runtime matcher must also reject Shift-held events — + // otherwise ``ctrl+tab`` would fire on Ctrl+Shift+Tab. + const ctrlTab = parseVoiceRecordKey('ctrl+tab') + const altEnter = parseVoiceRecordKey('alt+enter') + const ctrlO = parseVoiceRecordKey('ctrl+o') + + expect(isVoiceToggleKey({ ctrl: true, meta: false, shift: true, super: false, tab: true }, '', ctrlTab)).toBe(false) + expect(isVoiceToggleKey({ alt: true, ctrl: false, meta: false, return: true, shift: true, super: false }, '', altEnter)).toBe(false) + expect(isVoiceToggleKey({ ctrl: true, meta: false, shift: true, super: false }, 'o', ctrlO)).toBe(false) + + // Sanity: same events without Shift still fire. + expect(isVoiceToggleKey({ ctrl: true, meta: false, shift: false, super: false, tab: true }, '', ctrlTab)).toBe(true) + expect(isVoiceToggleKey({ ctrl: true, meta: false, shift: false, super: false }, 'o', ctrlO)).toBe(true) + }) +}) + +describe('formatVoiceRecordKey (#18994)', () => { + it('renders as the user expects in /voice status', async () => { + const { formatVoiceRecordKey, parseVoiceRecordKey } = await importPlatform('linux') + + expect(formatVoiceRecordKey(parseVoiceRecordKey('ctrl+b'))).toBe('Ctrl+B') + expect(formatVoiceRecordKey(parseVoiceRecordKey('ctrl+o'))).toBe('Ctrl+O') + expect(formatVoiceRecordKey(parseVoiceRecordKey('alt+r'))).toBe('Alt+R') + // ``super``/``win`` render as ``Super`` on non-mac so the hint + // doesn't tell Linux/Windows users to press a Cmd key they don't + // have. + expect(formatVoiceRecordKey(parseVoiceRecordKey('super+b'))).toBe('Super+B') + }) + + it('renders named keys in title case (Ctrl+Space, Ctrl+Enter)', async () => { + const { formatVoiceRecordKey, parseVoiceRecordKey } = await importPlatform('linux') + + expect(formatVoiceRecordKey(parseVoiceRecordKey('ctrl+space'))).toBe('Ctrl+Space') + expect(formatVoiceRecordKey(parseVoiceRecordKey('alt+enter'))).toBe('Alt+Enter') + expect(formatVoiceRecordKey(parseVoiceRecordKey('ctrl+esc'))).toBe('Ctrl+Escape') + expect(formatVoiceRecordKey(parseVoiceRecordKey('super+space'))).toBe('Super+Space') + }) +}) + +describe('isVoiceToggleKey honours configured record key (#18994)', () => { + it('binds the configured letter, not hardcoded b', async () => { + const { isVoiceToggleKey, parseVoiceRecordKey } = await importPlatform('linux') + const ctrlO = parseVoiceRecordKey('ctrl+o') + + expect(isVoiceToggleKey({ ctrl: true, meta: false, super: false }, 'o', ctrlO)).toBe(true) + // The old hardcoded 'b' must NOT match when the user configured 'o'. + expect(isVoiceToggleKey({ ctrl: true, meta: false, super: false }, 'b', ctrlO)).toBe(false) + }) + + it('alt+ binding matches alt OR meta (terminal-protocol parity)', async () => { + const { isVoiceToggleKey, parseVoiceRecordKey } = await importPlatform('linux') + const altR = parseVoiceRecordKey('alt+r') + + expect(isVoiceToggleKey({ alt: true, ctrl: false, meta: false, super: false }, 'r', altR)).toBe(true) + expect(isVoiceToggleKey({ ctrl: false, meta: true, super: false }, 'r', altR)).toBe(true) + expect(isVoiceToggleKey({ ctrl: false, meta: false, super: false }, 'r', altR)).toBe(false) + }) + + it('binds named keys via ink event flags (space → ch === " ", enter → key.return, …)', async () => { + const { isVoiceToggleKey, parseVoiceRecordKey } = await importPlatform('linux') + + const ctrlSpace = parseVoiceRecordKey('ctrl+space') + expect(isVoiceToggleKey({ ctrl: true, meta: false, super: false }, ' ', ctrlSpace)).toBe(true) + // Single-char ``b`` must NOT match a ``space``-configured binding. + expect(isVoiceToggleKey({ ctrl: true, meta: false, super: false }, 'b', ctrlSpace)).toBe(false) + // Space without the configured modifier must not fire either. + expect(isVoiceToggleKey({ ctrl: false, meta: false, super: false }, ' ', ctrlSpace)).toBe(false) + + const ctrlEnter = parseVoiceRecordKey('ctrl+enter') + expect(isVoiceToggleKey({ ctrl: true, meta: false, return: true, super: false }, '', ctrlEnter)).toBe(true) + expect(isVoiceToggleKey({ ctrl: true, meta: false, return: false, super: false }, '', ctrlEnter)).toBe(false) + + const altTab = parseVoiceRecordKey('alt+tab') + expect(isVoiceToggleKey({ alt: true, ctrl: false, meta: false, super: false, tab: true }, '', altTab)).toBe(true) + expect(isVoiceToggleKey({ alt: false, ctrl: false, meta: false, super: false, tab: true }, '', altTab)).toBe(false) + + const ctrlEscape = parseVoiceRecordKey('ctrl+escape') + expect(isVoiceToggleKey({ ctrl: true, escape: true, meta: false, super: false }, '', ctrlEscape)).toBe(true) + expect(isVoiceToggleKey({ ctrl: true, escape: false, meta: false, super: false }, '', ctrlEscape)).toBe(false) + + const ctrlBackspace = parseVoiceRecordKey('ctrl+backspace') + expect(isVoiceToggleKey({ backspace: true, ctrl: true, meta: false, super: false }, '', ctrlBackspace)).toBe(true) + + const ctrlDelete = parseVoiceRecordKey('ctrl+delete') + expect(isVoiceToggleKey({ ctrl: true, delete: true, meta: false, super: false }, '', ctrlDelete)).toBe(true) + }) + + it('omitted configured key falls back to ctrl+b (back-compat)', async () => { + const { isVoiceToggleKey } = await importPlatform('linux') + + // No third arg → DEFAULT_VOICE_RECORD_KEY → Ctrl+B behaviour. + expect(isVoiceToggleKey({ ctrl: true, meta: false, super: false }, 'b')).toBe(true) + expect(isVoiceToggleKey({ ctrl: true, meta: false, super: false }, 'o')).toBe(false) + }) + + // Regressions from Copilot review on #19835: the previous implementation + // accepted ``isActionMod(key)`` in the ``ctrl`` branch for every + // configured key, so bare Esc (which hermes-ink reports with + // ``key.meta`` on some macOS terminals) fired ``ctrl+escape``, and + // Alt+Space / Alt+Tab fired ``ctrl+space`` / ``ctrl+tab``. The fallback + // is now gated to the documented default (``ctrl+b``) only. + it('ctrl+escape does NOT fire on bare Esc via key.meta on macOS', async () => { + const { isVoiceToggleKey, parseVoiceRecordKey } = await importPlatform('darwin') + const ctrlEscape = parseVoiceRecordKey('ctrl+escape') + + // Bare Esc on a legacy macOS terminal: ``key.meta: true``, ``key.escape: true``, no ctrl. + expect(isVoiceToggleKey({ ctrl: false, escape: true, meta: true, super: false }, '', ctrlEscape)).toBe(false) + // Real Ctrl+Esc still fires. + expect(isVoiceToggleKey({ ctrl: true, escape: true, meta: false, super: false }, '', ctrlEscape)).toBe(true) + }) + + it('ctrl+space does NOT fire on Alt+Space on macOS', async () => { + const { isVoiceToggleKey, parseVoiceRecordKey } = await importPlatform('darwin') + const ctrlSpace = parseVoiceRecordKey('ctrl+space') + + // Alt+Space surfaces as ``key.meta: true`` with space char. + expect(isVoiceToggleKey({ ctrl: false, meta: true, super: false }, ' ', ctrlSpace)).toBe(false) + // Real Ctrl+Space still fires. + expect(isVoiceToggleKey({ ctrl: true, meta: false, super: false }, ' ', ctrlSpace)).toBe(true) + }) + + it('default ctrl+b accepts raw Ctrl+B and kitty-style Cmd+B on macOS', async () => { + const { DEFAULT_VOICE_RECORD_KEY, isVoiceToggleKey } = await importPlatform('darwin') + + // Raw Ctrl+B: always works. + expect(isVoiceToggleKey({ ctrl: true, meta: false, super: false }, 'b', DEFAULT_VOICE_RECORD_KEY)).toBe(true) + // Cmd+B via kitty-style ``key.super``: still works. + expect(isVoiceToggleKey({ ctrl: false, meta: false, super: true }, 'b', DEFAULT_VOICE_RECORD_KEY)).toBe(true) + // Cmd+B via legacy ``key.meta`` NO LONGER works — ``key.meta`` is + // hermes-ink's Alt signal, so accepting it leaked Alt+B into the + // default binding (Copilot round-6 review on #19835). + expect(isVoiceToggleKey({ ctrl: false, meta: true, super: false }, 'b', DEFAULT_VOICE_RECORD_KEY)).toBe(false) + }) + + it('custom ctrl+ does NOT accept Cmd fallback on macOS', async () => { + const { isVoiceToggleKey, parseVoiceRecordKey } = await importPlatform('darwin') + const ctrlO = parseVoiceRecordKey('ctrl+o') + + // Only ``ctrl+b`` gets the action-modifier fallback; ``ctrl+o`` must + // be a literal Ctrl bit — otherwise Cmd+O would steal the shortcut. + expect(isVoiceToggleKey({ ctrl: false, meta: true, super: false }, 'o', ctrlO)).toBe(false) + expect(isVoiceToggleKey({ ctrl: false, meta: false, super: true }, 'o', ctrlO)).toBe(false) + expect(isVoiceToggleKey({ ctrl: true, meta: false, super: false }, 'o', ctrlO)).toBe(true) + }) + + it('super+b renders "Cmd+B" on darwin and requires the literal key.super bit', async () => { + const { formatVoiceRecordKey, isVoiceToggleKey, parseVoiceRecordKey } = await importPlatform('darwin') + const superB = parseVoiceRecordKey('super+b') + + expect(formatVoiceRecordKey(superB)).toBe('Cmd+B') + // Kitty-style: key.super fires the binding. + expect(isVoiceToggleKey({ ctrl: false, meta: false, super: true }, 'b', superB)).toBe(true) + // ``key.meta`` is NOT accepted — hermes-ink uses meta for Alt too, + // so accepting it here would make super+b silently fire on Alt+B + // (Copilot round-5 review on #19835). + expect(isVoiceToggleKey({ ctrl: false, meta: true, super: false }, 'b', superB)).toBe(false) + // Ctrl held at the same time → reject (different chord). + expect(isVoiceToggleKey({ ctrl: true, meta: false, super: true }, 'b', superB)).toBe(false) + }) + + // Round-2 Copilot review regressions on #19835. + it('super+b renders "Super+B" on Linux (not "Cmd+B")', async () => { + const { formatVoiceRecordKey, parseVoiceRecordKey } = await importPlatform('linux') + + expect(formatVoiceRecordKey(parseVoiceRecordKey('super+b'))).toBe('Super+B') + expect(formatVoiceRecordKey(parseVoiceRecordKey('win+b'))).toBe('Super+B') + }) + + it('super+b still renders "Cmd+B" on macOS', async () => { + const { formatVoiceRecordKey, parseVoiceRecordKey } = await importPlatform('darwin') + + expect(formatVoiceRecordKey(parseVoiceRecordKey('super+b'))).toBe('Cmd+B') + expect(formatVoiceRecordKey(parseVoiceRecordKey('win+b'))).toBe('Cmd+B') + }) + + it('ctrl+b aliases (control+b, "ctrl + b") still accept Cmd+B fallback on macOS', async () => { + const { isVoiceToggleKey, parseVoiceRecordKey } = await importPlatform('darwin') + const controlB = parseVoiceRecordKey('control+b') + const spacedB = parseVoiceRecordKey('ctrl + b') + + // Both parse to the documented default semantically; both must keep + // the macOS Cmd+B muscle-memory fallback via kitty-style key.super. + // ``key.meta`` is NOT accepted — that's hermes-ink's Alt signal + // (round-6 review), so legacy-terminal users get strict Ctrl+B. + expect(isVoiceToggleKey({ ctrl: false, meta: true, super: false }, 'b', controlB)).toBe(false) + expect(isVoiceToggleKey({ ctrl: false, meta: true, super: false }, 'b', spacedB)).toBe(false) + expect(isVoiceToggleKey({ ctrl: false, meta: false, super: true }, 'b', controlB)).toBe(true) + expect(isVoiceToggleKey({ ctrl: false, meta: false, super: true }, 'b', spacedB)).toBe(true) + // Literal Ctrl+B still fires. + expect(isVoiceToggleKey({ ctrl: true, meta: false, super: false }, 'b', controlB)).toBe(true) + // And still reject a ctrl bit on a different letter. + expect(isVoiceToggleKey({ ctrl: true, meta: false, super: false }, 'o', controlB)).toBe(false) + }) +}) + describe('isMacActionFallback', () => { it('routes raw Ctrl+K and Ctrl+W to readline kill-to-end / delete-word on macOS', async () => { const { isMacActionFallback } = await importPlatform('darwin') diff --git a/ui-tui/src/__tests__/textInputPassThrough.test.ts b/ui-tui/src/__tests__/textInputPassThrough.test.ts new file mode 100644 index 0000000000..5988580f9b --- /dev/null +++ b/ui-tui/src/__tests__/textInputPassThrough.test.ts @@ -0,0 +1,43 @@ +import { describe, expect, it } from 'vitest' + +import { shouldPassThroughToGlobalHandler } from '../components/textInput.js' +import { DEFAULT_VOICE_RECORD_KEY, parseVoiceRecordKey } from '../lib/platform.js' + +const key = (overrides: Record = {}) => + ({ ctrl: false, meta: false, ...overrides }) as any + +describe('shouldPassThroughToGlobalHandler', () => { + it('passes through the configured voice shortcut while composer is focused', () => { + expect( + shouldPassThroughToGlobalHandler('o', key({ ctrl: true }), parseVoiceRecordKey('ctrl+o')) + ).toBe(true) + expect( + shouldPassThroughToGlobalHandler('r', key({ meta: true }), parseVoiceRecordKey('alt+r')) + ).toBe(true) + expect( + shouldPassThroughToGlobalHandler(' ', key({ ctrl: true }), parseVoiceRecordKey('ctrl+space')) + ).toBe(true) + expect( + shouldPassThroughToGlobalHandler('', key({ ctrl: true, return: true }), parseVoiceRecordKey('ctrl+enter')) + ).toBe(true) + }) + + it('keeps the legacy default pass-through when no custom key is provided', () => { + expect(shouldPassThroughToGlobalHandler('b', key({ ctrl: true }), DEFAULT_VOICE_RECORD_KEY)).toBe(true) + expect(shouldPassThroughToGlobalHandler('b', key({ ctrl: true }))).toBe(true) + }) + + it('does not swallow ordinary typing keys', () => { + expect(shouldPassThroughToGlobalHandler('h', key(), parseVoiceRecordKey('ctrl+o'))).toBe(false) + expect(shouldPassThroughToGlobalHandler('o', key(), parseVoiceRecordKey('ctrl+o'))).toBe(false) + }) + + it('always passes through non-voice global control keys', () => { + expect(shouldPassThroughToGlobalHandler('c', key({ ctrl: true }))).toBe(true) + expect(shouldPassThroughToGlobalHandler('x', key({ ctrl: true }))).toBe(true) + expect(shouldPassThroughToGlobalHandler('', key({ escape: true }))).toBe(true) + expect(shouldPassThroughToGlobalHandler('', key({ tab: true }))).toBe(true) + expect(shouldPassThroughToGlobalHandler('', key({ pageUp: true }))).toBe(true) + expect(shouldPassThroughToGlobalHandler('', key({ pageDown: true }))).toBe(true) + }) +}) diff --git a/ui-tui/src/__tests__/useCompletion.test.ts b/ui-tui/src/__tests__/useCompletion.test.ts new file mode 100644 index 0000000000..67a9fcfea8 --- /dev/null +++ b/ui-tui/src/__tests__/useCompletion.test.ts @@ -0,0 +1,35 @@ +import { describe, expect, it } from 'vitest' + +import { completionRequestForInput } from '../hooks/useCompletion.js' + +describe('completionRequestForInput', () => { + it('routes real slash commands to slash completion', () => { + expect(completionRequestForInput('/help')).toMatchObject({ + method: 'complete.slash', + params: { text: '/help' }, + replaceFrom: 1 + }) + }) + + it('does not route absolute paths through slash completion', () => { + expect( + completionRequestForInput('/home/d/Desktop/agenda/CrimsonRed/.hermes/plans/2026-05-04-HANDOFF-NEXT.md') + ).toMatchObject({ + method: 'complete.path', + params: { word: '/home/d/Desktop/agenda/CrimsonRed/.hermes/plans/2026-05-04-HANDOFF-NEXT.md' }, + replaceFrom: 0 + }) + }) + + it('keeps path completion for trailing absolute path tokens', () => { + expect(completionRequestForInput('read /home/d/Desktop/file.md')).toMatchObject({ + method: 'complete.path', + params: { word: '/home/d/Desktop/file.md' }, + replaceFrom: 5 + }) + }) + + it('leaves plain text alone', () => { + expect(completionRequestForInput('hello there')).toBeNull() + }) +}) diff --git a/ui-tui/src/__tests__/useConfigSync.test.ts b/ui-tui/src/__tests__/useConfigSync.test.ts index fc2dad19f1..39020d2763 100644 --- a/ui-tui/src/__tests__/useConfigSync.test.ts +++ b/ui-tui/src/__tests__/useConfigSync.test.ts @@ -1,13 +1,15 @@ -import { beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { $uiState, resetUiState } from '../app/uiStore.js' import { applyDisplay, + hydrateFullConfig, normalizeBusyInputMode, normalizeIndicatorStyle, normalizeMouseTracking, normalizeStatusBar } from '../app/useConfigSync.js' +import type { ParsedVoiceRecordKey } from '../lib/platform.js' describe('applyDisplay', () => { beforeEach(() => { @@ -292,3 +294,139 @@ describe('applyDisplay → tui_status_indicator', () => { expect($uiState.get().indicatorStyle).toBe('kaomoji') }) }) + +// Regressions from Copilot review on #19835: the config-hydration path +// for voice.record_key was untested, so a future regression in the +// hydration or mtime-reapply wiring would slip past the suite. +describe('applyDisplay → voice.record_key (#18994)', () => { + beforeEach(() => { + resetUiState() + }) + + it('parses voice.record_key and pushes it through the setter', () => { + const setBell = vi.fn() + const setVoiceRecordKey = vi.fn() + + applyDisplay( + { config: { display: {}, voice: { record_key: 'ctrl+space' } } }, + setBell, + setVoiceRecordKey + ) + + expect(setVoiceRecordKey).toHaveBeenCalledWith( + expect.objectContaining({ ch: 'space', mod: 'ctrl', named: 'space', raw: 'ctrl+space' }) + ) + }) + + it('falls back to the documented default when voice.record_key is missing', () => { + const setBell = vi.fn() + const setVoiceRecordKey = vi.fn() + + applyDisplay({ config: { display: {} } }, setBell, setVoiceRecordKey) + + expect(setVoiceRecordKey).toHaveBeenCalledWith( + expect.objectContaining({ ch: 'b', mod: 'ctrl', raw: 'ctrl+b' }) + ) + }) + + it('is a no-op when the voice setter is not passed (back-compat)', () => { + const setBell = vi.fn() + + // applyDisplay is used in the setVoiceEnabled-less init path too; + // omitting the third arg must not throw. + expect(() => + applyDisplay({ config: { display: {}, voice: { record_key: 'alt+r' } } }, setBell) + ).not.toThrow() + }) + + it('does not reset voiceRecordKey when cfg is null (transient RPC failure)', () => { + const setBell = vi.fn() + const setVoiceRecordKey = vi.fn() + + // quietRpc() collapses request failures to null. Resetting the + // cached shortcut on every null would clobber a custom binding + // after one transient error until the next successful poll + // (Copilot round-8 review on #19835). + applyDisplay(null, setBell, setVoiceRecordKey) + + expect(setVoiceRecordKey).not.toHaveBeenCalled() + // bell is still applied (defaults to false on null), so the setter + // runs — we specifically only skip voiceRecordKey. + expect(setBell).toHaveBeenCalledWith(false) + }) +}) + +// Round-12 Copilot review regression on #19835: the live mtime-reload +// path was previously untested, so a regression in the polling/RPC +// wiring to applyDisplay would only be visible at runtime. The fetch +// + apply body is now shared as ``hydrateFullConfig()``, exercised +// directly from both the initial hydration and the poll-tick body. +describe('hydrateFullConfig', () => { + beforeEach(() => { + resetUiState() + }) + + const makeFakeGw = (payload: unknown) => + ({ + request: vi.fn(() => Promise.resolve(payload)), + on: vi.fn(), + off: vi.fn() + }) as any + + it('re-applies voice.record_key from a fresh config.get full response', async () => { + const gw = makeFakeGw({ config: { display: {}, voice: { record_key: 'ctrl+o' } } }) + const setBell = vi.fn() + const setVoiceRecordKey = vi.fn() + + await hydrateFullConfig(gw, setBell, setVoiceRecordKey) + + expect(gw.request).toHaveBeenCalledWith('config.get', { key: 'full' }) + expect(setVoiceRecordKey).toHaveBeenCalledWith( + expect.objectContaining({ ch: 'o', mod: 'ctrl', raw: 'ctrl+o' }) + ) + expect(setBell).toHaveBeenCalledWith(false) + }) + + it('reapplies the latest value on each invocation (mtime-reload semantics)', async () => { + const gw = makeFakeGw({ config: { display: {}, voice: { record_key: 'ctrl+b' } } }) + const setBell = vi.fn() + const setVoiceRecordKey = vi.fn() + + await hydrateFullConfig(gw, setBell, setVoiceRecordKey) + expect(setVoiceRecordKey).toHaveBeenLastCalledWith(expect.objectContaining({ ch: 'b' })) + + // Simulate a config edit: gw now returns a new shortcut. + gw.request = vi.fn(() => Promise.resolve({ config: { display: {}, voice: { record_key: 'alt+space' } } })) + + await hydrateFullConfig(gw, setBell, setVoiceRecordKey) + expect(setVoiceRecordKey).toHaveBeenLastCalledWith( + expect.objectContaining({ ch: 'space', mod: 'alt', named: 'space' }) + ) + }) + + it('leaves cached voiceRecordKey untouched when the RPC fails', async () => { + const gw = { request: vi.fn(() => Promise.reject(new Error('boom'))), on: vi.fn(), off: vi.fn() } as any + const setBell = vi.fn() + const setVoiceRecordKey = vi.fn() + + const result = await hydrateFullConfig(gw, setBell, setVoiceRecordKey) + + // quietRpc() swallows the error and returns null; applyDisplay + // sees cfg=null and skips the voice setter (Copilot round-8). + expect(result).toBeNull() + expect(setVoiceRecordKey).not.toHaveBeenCalled() + // bell setter still fires — applyDisplay's null-cfg path applies + // the documented bell default (false). + expect(setBell).toHaveBeenCalledWith(false) + }) + + it('threads through without a voice setter (back-compat call sites)', async () => { + const gw = makeFakeGw({ config: { display: { bell_on_complete: true } } }) + const setBell = vi.fn() + + // No third arg — applyDisplay must not throw and must still apply + // display flags (round-2 / round-8 invariant). + await expect(hydrateFullConfig(gw, setBell)).resolves.toBeTruthy() + expect(setBell).toHaveBeenCalledWith(true) + }) +}) diff --git a/ui-tui/src/app/interfaces.ts b/ui-tui/src/app/interfaces.ts index baf637aa25..dfe88fc040 100644 --- a/ui-tui/src/app/interfaces.ts +++ b/ui-tui/src/app/interfaces.ts @@ -4,6 +4,7 @@ import type { MutableRefObject, ReactNode, RefObject, SetStateAction } from 'rea import type { PasteEvent } from '../components/textInput.js' import type { GatewayClient } from '../gatewayClient.js' import type { ImageAttachResponse } from '../gatewayTypes.js' +import type { ParsedVoiceRecordKey } from '../lib/platform.js' import type { RpcResult } from '../lib/rpc.js' import type { Theme } from '../theme.js' import type { @@ -210,6 +211,7 @@ export interface InputHandlerContext { } voice: { enabled: boolean + recordKey: ParsedVoiceRecordKey recording: boolean setProcessing: StateSetter setRecording: StateSetter @@ -291,6 +293,7 @@ export interface SlashHandlerContext { } voice: { setVoiceEnabled: StateSetter + setVoiceRecordKey: (v: ParsedVoiceRecordKey) => void } } @@ -318,6 +321,7 @@ export interface AppLayoutComposerProps { queuedDisplay: string[] submit: (value: string) => void updateInput: StateSetter + voiceRecordKey: ParsedVoiceRecordKey } export interface AppLayoutProgressProps { diff --git a/ui-tui/src/app/slash/commands/session.ts b/ui-tui/src/app/slash/commands/session.ts index 0a5324ef55..a75419c3b0 100644 --- a/ui-tui/src/app/slash/commands/session.ts +++ b/ui-tui/src/app/slash/commands/session.ts @@ -10,6 +10,7 @@ import type { SessionUsageResponse, VoiceToggleResponse } from '../../../gatewayTypes.js' +import { formatVoiceRecordKey, parseVoiceRecordKey } from '../../../lib/platform.js' import { fmtK } from '../../../lib/text.js' import type { PanelSection } from '../../../types.js' import { DEFAULT_INDICATOR_STYLE, INDICATOR_STYLES, type IndicatorStyle } from '../../interfaces.js' @@ -221,6 +222,30 @@ export const sessionCommands: SlashCommand[] = [ ctx.guarded(r => { ctx.voice.setVoiceEnabled(!!r.enabled) + // Render the configured record key (config.yaml ``voice.record_key``) + // instead of hardcoded "Ctrl+B" — the gateway response carries the + // current value so /voice status and /voice on stay in sync with + // both the CLI and the TUI's actual binding (#18994). + // + // Copilot review on #19835 caught that rendering from the fresh + // backend response WITHOUT updating the frontend ``voice.recordKey`` + // state would skew display and binding between config-edit and + // the next ``mtime`` poll (~5s). Parse once, push into state so + // ``useInputHandlers()`` picks up the new binding immediately. + // + // Round-2 follow-up: only push state when the response actually + // carries ``record_key`` — otherwise an older gateway (or a future + // branch that forgets to include it) would clobber a custom user + // binding back to the default on every /voice invocation. The + // label still falls back to the documented default for display. + const parsed = r.record_key ? parseVoiceRecordKey(r.record_key) : undefined + + if (parsed) { + ctx.voice.setVoiceRecordKey(parsed) + } + + const recordKeyLabel = formatVoiceRecordKey(parsed ?? parseVoiceRecordKey('ctrl+b')) + // Match CLI's _show_voice_status / _enable_voice_mode / // _toggle_voice_tts output shape so users don't have to learn // two vocabularies. @@ -230,11 +255,11 @@ export const sessionCommands: SlashCommand[] = [ ctx.transcript.sys('Voice Mode Status') ctx.transcript.sys(` Mode: ${mode}`) ctx.transcript.sys(` TTS: ${tts}`) - ctx.transcript.sys(' Record key: Ctrl+B') + ctx.transcript.sys(` Record key: ${recordKeyLabel}`) // CLI's "Requirements:" block — surfaces STT/audio setup issues // so the user sees "STT provider: MISSING ..." instead of - // silently failing on every Ctrl+B press. + // silently failing on every record-key press. if (r.details) { ctx.transcript.sys('') ctx.transcript.sys(' Requirements:') @@ -259,7 +284,7 @@ export const sessionCommands: SlashCommand[] = [ if (r.enabled) { const tts = r.tts ? ' (TTS enabled)' : '' ctx.transcript.sys(`Voice mode enabled${tts}`) - ctx.transcript.sys(' Ctrl+B to start/stop recording') + ctx.transcript.sys(` ${recordKeyLabel} to start/stop recording`) ctx.transcript.sys(' /voice tts to toggle speech output') ctx.transcript.sys(' /voice off to disable voice mode') } else { diff --git a/ui-tui/src/app/useConfigSync.ts b/ui-tui/src/app/useConfigSync.ts index ad8f52f148..b0e590ee2c 100644 --- a/ui-tui/src/app/useConfigSync.ts +++ b/ui-tui/src/app/useConfigSync.ts @@ -7,6 +7,11 @@ import type { ConfigMtimeResponse, ReloadMcpResponse } from '../gatewayTypes.js' +import { + DEFAULT_VOICE_RECORD_KEY, + parseVoiceRecordKey, + type ParsedVoiceRecordKey +} from '../lib/platform.js' import { asRpcResult } from '../lib/rpc.js' import { @@ -89,10 +94,47 @@ const quietRpc = async = Record>( } } -export const applyDisplay = (cfg: ConfigFullResponse | null, setBell: (v: boolean) => void) => { +const _voiceRecordKeyFromConfig = (cfg: ConfigFullResponse | null): ParsedVoiceRecordKey => { + const raw = cfg?.config?.voice?.record_key + + return raw ? parseVoiceRecordKey(raw) : DEFAULT_VOICE_RECORD_KEY +} + +/** Fetch ``config.get full`` and fan the result through ``applyDisplay``. + * + * Extracted so the mtime-reload path can be exercised by the test + * suite without a React runtime (Copilot round-12 review on #19835). + * Both the initial hydration and the mtime poller use this shared + * helper, so a regression in the fetch/apply plumbing now fails the + * useConfigSync tests instead of only being visible at runtime. */ +export async function hydrateFullConfig( + gw: GatewayClient, + setBell: (v: boolean) => void, + setVoiceRecordKey?: (v: ParsedVoiceRecordKey) => void +): Promise { + const cfg = await quietRpc(gw, 'config.get', { key: 'full' }) + applyDisplay(cfg, setBell, setVoiceRecordKey) + return cfg +} + +export const applyDisplay = ( + cfg: ConfigFullResponse | null, + setBell: (v: boolean) => void, + setVoiceRecordKey?: (v: ParsedVoiceRecordKey) => void +) => { const d = cfg?.config?.display ?? {} setBell(!!d.bell_on_complete) + // Only push the voice record key when the RPC actually returned a + // config payload. ``quietRpc()`` collapses failures to ``null``; if we + // reset the cached shortcut on every null we would clobber a custom + // binding after one transient RPC error until the next config edit + // (Copilot round-8 review on #19835). The mtime-poll loop advances + // ``mtimeRef`` before this call, so staying silent on null preserves + // the last-good state and lets the next successful poll refresh it. + if (setVoiceRecordKey && cfg) { + setVoiceRecordKey(_voiceRecordKeyFromConfig(cfg)) + } patchUiState({ busyInputMode: normalizeBusyInputMode(d.busy_input_mode), compact: !!d.tui_compact, @@ -109,7 +151,13 @@ export const applyDisplay = (cfg: ConfigFullResponse | null, setBell: (v: boolea }) } -export function useConfigSync({ gw, setBellOnComplete, setVoiceEnabled, sid }: UseConfigSyncOptions) { +export function useConfigSync({ + gw, + setBellOnComplete, + setVoiceEnabled, + setVoiceRecordKey, + sid +}: UseConfigSyncOptions) { const mtimeRef = useRef(0) useEffect(() => { @@ -125,8 +173,8 @@ export function useConfigSync({ gw, setBellOnComplete, setVoiceEnabled, sid }: U quietRpc(gw, 'config.get', { key: 'mtime' }).then(r => { mtimeRef.current = Number(r?.mtime ?? 0) }) - quietRpc(gw, 'config.get', { key: 'full' }).then(r => applyDisplay(r, setBellOnComplete)) - }, [gw, setBellOnComplete, setVoiceEnabled, sid]) + void hydrateFullConfig(gw, setBellOnComplete, setVoiceRecordKey) + }, [gw, setBellOnComplete, setVoiceEnabled, setVoiceRecordKey, sid]) useEffect(() => { if (!sid) { @@ -154,17 +202,18 @@ export function useConfigSync({ gw, setBellOnComplete, setVoiceEnabled, sid }: U quietRpc(gw, 'reload.mcp', { session_id: sid, confirm: true }).then( r => r && turnController.pushActivity('MCP reloaded after config change') ) - quietRpc(gw, 'config.get', { key: 'full' }).then(r => applyDisplay(r, setBellOnComplete)) + void hydrateFullConfig(gw, setBellOnComplete, setVoiceRecordKey) }) }, MTIME_POLL_MS) return () => clearInterval(id) - }, [gw, setBellOnComplete, sid]) + }, [gw, setBellOnComplete, setVoiceRecordKey, sid]) } export interface UseConfigSyncOptions { gw: GatewayClient setBellOnComplete: (v: boolean) => void setVoiceEnabled: (v: boolean) => void + setVoiceRecordKey?: (v: ParsedVoiceRecordKey) => void sid: null | string } diff --git a/ui-tui/src/app/useInputHandlers.ts b/ui-tui/src/app/useInputHandlers.ts index a74c9e8431..20e9b087a4 100644 --- a/ui-tui/src/app/useInputHandlers.ts +++ b/ui-tui/src/app/useInputHandlers.ts @@ -348,9 +348,17 @@ export function useInputHandlers(ctx: InputHandlerContext): InputHandlerResult { return scrollTranscript(key.pageUp ? -step : step) } - // Queue-edit cancel beats selection-clear: the queue header explicitly - // promises "Esc cancel", so honoring it takes priority over the implicit - // selection-dismissal convention. Without an active edit, fall through. + // Escape-based voice bindings (ctrl/alt/super+escape) must win before the + // generic Esc handlers below; otherwise queue-edit cancel / selection-clear + // would swallow the chord and /voice would advertise a shortcut that never + // actually toggles recording in those UI states. + if (key.escape && isVoiceToggleKey(key, ch, voice.recordKey)) { + return voiceRecordToggle() + } + + // Queue-edit cancel beats selection-clear for plain Esc: the queue header + // explicitly promises "Esc cancel", so honoring it takes priority over the + // implicit selection-dismissal convention. Without an active edit, fall through. if (key.escape && cState.queueEditIdx !== null) { return cActions.clearIn() } @@ -439,7 +447,7 @@ export function useInputHandlers(ctx: InputHandlerContext): InputHandlerResult { return } - if (isVoiceToggleKey(key, ch)) { + if (isVoiceToggleKey(key, ch, voice.recordKey)) { return voiceRecordToggle() } diff --git a/ui-tui/src/app/useMainApp.ts b/ui-tui/src/app/useMainApp.ts index 218654f531..b39cc29a32 100644 --- a/ui-tui/src/app/useMainApp.ts +++ b/ui-tui/src/app/useMainApp.ts @@ -18,7 +18,7 @@ import { useGitBranch } from '../hooks/useGitBranch.js' import { useVirtualHistory } from '../hooks/useVirtualHistory.js' import { appendTranscriptMessage } from '../lib/messages.js' import { composerPromptWidth } from '../lib/inputMetrics.js' -import { isMac } from '../lib/platform.js' +import { DEFAULT_VOICE_RECORD_KEY, isMac, type ParsedVoiceRecordKey } from '../lib/platform.js' import { asRpcResult, rpcErrorMessage } from '../lib/rpc.js' import { terminalParityHints } from '../lib/terminalParity.js' import { buildToolTrailLine, sameToolTrailGroup, toolTrailLabel } from '../lib/text.js' @@ -104,6 +104,7 @@ export function useMainApp(gw: GatewayClient) { const [voiceEnabled, setVoiceEnabled] = useState(false) const [voiceRecording, setVoiceRecording] = useState(false) const [voiceProcessing, setVoiceProcessing] = useState(false) + const [voiceRecordKey, setVoiceRecordKey] = useState(DEFAULT_VOICE_RECORD_KEY) const [sessionStartedAt, setSessionStartedAt] = useState(() => Date.now()) const [turnStartedAt, setTurnStartedAt] = useState(null) const [goodVibesTick, setGoodVibesTick] = useState(0) @@ -394,7 +395,7 @@ export function useMainApp(gw: GatewayClient) { } }, [ui.busy]) - useConfigSync({ gw, setBellOnComplete, setVoiceEnabled, sid: ui.sid }) + useConfigSync({ gw, setBellOnComplete, setVoiceEnabled, setVoiceRecordKey, sid: ui.sid }) // Tab title: `⚠` waiting on approval/sudo/secret/clarify, `⏳` busy, `✓` idle. const model = ui.info?.model?.replace(/^.*\//, '') ?? '' @@ -539,6 +540,7 @@ export function useMainApp(gw: GatewayClient) { terminal: { hasSelection, scrollRef, scrollWithSelection, selection, stdout }, voice: { enabled: voiceEnabled, + recordKey: voiceRecordKey, recording: voiceRecording, setProcessing: setVoiceProcessing, setRecording: setVoiceRecording, @@ -642,7 +644,7 @@ export function useMainApp(gw: GatewayClient) { }, slashFlightRef, transcript: { page, panel, send, setHistoryItems, sys, trimLastExchange: session.trimLastExchange }, - voice: { setVoiceEnabled } + voice: { setVoiceEnabled, setVoiceRecordKey } }), [ catalog, @@ -782,9 +784,10 @@ export function useMainApp(gw: GatewayClient) { queueEditIdx: composerState.queueEditIdx, queuedDisplay: composerState.queuedDisplay, submit, - updateInput: composerActions.setInput + updateInput: composerActions.setInput, + voiceRecordKey }), - [cols, composerActions, composerState, empty, pagerPageSize, submit] + [cols, composerActions, composerState, empty, pagerPageSize, submit, voiceRecordKey] ) // Pass current progress through unfrozen — streaming update throttling diff --git a/ui-tui/src/components/appLayout.tsx b/ui-tui/src/components/appLayout.tsx index 8c2d210ca1..ec60726ed3 100644 --- a/ui-tui/src/components/appLayout.tsx +++ b/ui-tui/src/components/appLayout.tsx @@ -288,6 +288,7 @@ const ComposerPane = memo(function ComposerPane({ onSubmit={composer.submit} placeholder={composer.empty ? PLACEHOLDER : ui.busy ? 'Ctrl+C to interrupt…' : ''} value={composer.input} + voiceRecordKey={composer.voiceRecordKey} /> diff --git a/ui-tui/src/components/textInput.tsx b/ui-tui/src/components/textInput.tsx index 3008f0baf4..d8151e72b7 100644 --- a/ui-tui/src/components/textInput.tsx +++ b/ui-tui/src/components/textInput.tsx @@ -5,7 +5,14 @@ import { type MutableRefObject, useEffect, useMemo, useRef, useState } from 'rea import { setInputSelection } from '../app/inputSelectionStore.js' import { readClipboardText, writeClipboardText } from '../lib/clipboard.js' import { cursorLayout, offsetFromPosition } from '../lib/inputMetrics.js' -import { isActionMod, isMac, isMacActionFallback } from '../lib/platform.js' +import { + DEFAULT_VOICE_RECORD_KEY, + isActionMod, + isMac, + isMacActionFallback, + isVoiceToggleKey, + type ParsedVoiceRecordKey +} from '../lib/platform.js' type InkExt = typeof Ink & { stringWidth: (s: string) => number @@ -239,6 +246,7 @@ export function TextInput({ onSubmit, mask, mouseApiRef, + voiceRecordKey = DEFAULT_VOICE_RECORD_KEY, placeholder = '', focus = true }: TextInputProps) { @@ -699,6 +707,15 @@ export function TextInput({ (inp: string, k: Key, event: InputEvent) => { const eventRaw = event.keypress.raw + // Configured voice shortcut wins over composer-level defaults like + // paste/copy so users who bind voice to ctrl+v / alt+v / cmd+v + // actually get voice toggled instead of a paste (Copilot round-7 + // follow-up on #19835). The pass-through predicate is a no-op for + // ordinary typing and plain paste when voice is unbound to 'v'. + if (shouldPassThroughToGlobalHandler(inp, k, voiceRecordKey)) { + return + } + if ( eventRaw === '\x1bv' || eventRaw === '\x1bV' || @@ -744,22 +761,6 @@ export function TextInput({ return } - // Ctrl chords claimed by useInputHandlers — pass through instead of - // letting them fall into readline-style nav or a literal char insert. - // Ctrl+B = voice toggle, Ctrl+X = delete queued message while editing. - if ( - (k.ctrl && inp === 'c') || - (k.ctrl && inp === 'b') || - (k.ctrl && inp === 'x') || - k.tab || - (k.shift && k.tab) || - k.pageUp || - k.pageDown || - k.escape - ) { - return - } - if (k.return) { if (k.shift || k.ctrl || (isMac ? isActionMod(k) : k.meta)) { flushParentChange() @@ -1041,8 +1042,23 @@ interface TextInputProps { onSubmit?: (v: string) => void placeholder?: string value: string + voiceRecordKey?: ParsedVoiceRecordKey } +export const shouldPassThroughToGlobalHandler = ( + input: string, + key: Key, + voiceRecordKey: ParsedVoiceRecordKey = DEFAULT_VOICE_RECORD_KEY +): boolean => + (key.ctrl && input === 'c') || + (key.ctrl && input === 'x') || + key.tab || + (key.shift && key.tab) || + key.pageUp || + key.pageDown || + key.escape || + isVoiceToggleKey(key, input, voiceRecordKey) + export interface TextInputMouseApi { dragAt: (row: number, col: number) => void end: () => void diff --git a/ui-tui/src/gatewayTypes.ts b/ui-tui/src/gatewayTypes.ts index a1513d2a6e..7fca2837fa 100644 --- a/ui-tui/src/gatewayTypes.ts +++ b/ui-tui/src/gatewayTypes.ts @@ -75,8 +75,14 @@ export interface ConfigDisplayConfig { tui_statusbar?: 'bottom' | 'off' | 'on' | 'top' | boolean } +export interface ConfigVoiceConfig { + // Raw `yaml.safe_load()` value from config; may be non-string if hand-edited. + // Callers must normalize/validate at runtime (parseVoiceRecordKey()). + record_key?: unknown +} + export interface ConfigFullResponse { - config?: { display?: ConfigDisplayConfig } + config?: { display?: ConfigDisplayConfig; voice?: ConfigVoiceConfig } } export interface ConfigMtimeResponse { @@ -279,6 +285,7 @@ export interface VoiceToggleResponse { available?: boolean details?: string enabled?: boolean + record_key?: string stt_available?: boolean tts?: boolean } diff --git a/ui-tui/src/hooks/useCompletion.ts b/ui-tui/src/hooks/useCompletion.ts index 08bd4945d7..6bafc35843 100644 --- a/ui-tui/src/hooks/useCompletion.ts +++ b/ui-tui/src/hooks/useCompletion.ts @@ -1,12 +1,43 @@ import { useEffect, useRef, useState } from 'react' import type { CompletionItem } from '../app/interfaces.js' +import { looksLikeSlashCommand } from '../domain/slash.js' import type { GatewayClient } from '../gatewayClient.js' import type { CompletionResponse } from '../gatewayTypes.js' import { asRpcResult } from '../lib/rpc.js' const TAB_PATH_RE = /((?:["']?(?:[A-Za-z]:[\\/]|\.{1,2}\/|~\/|\/|@|[^"'`\s]+\/))[^\s]*)$/ +export function completionRequestForInput( + input: string +): + | { method: 'complete.path'; params: { word: string }; replaceFrom: number } + | { method: 'complete.slash'; params: { text: string }; replaceFrom: number } + | null { + const isSlashCommand = looksLikeSlashCommand(input) + const pathWord = isSlashCommand ? null : (input.match(TAB_PATH_RE)?.[1] ?? null) + + if (!isSlashCommand && !pathWord) { + return null + } + + // `/model` / `/provider` use the two-step ModelPicker (real curated IDs). + // Slash completion here only showed short aliases + vendor/family meta. + if (isSlashCommand && /^\/(?:model|provider)(?:\s|$)/.test(input)) { + return null + } + + if (isSlashCommand) { + return { method: 'complete.slash', params: { text: input }, replaceFrom: 1 } + } + + return { + method: 'complete.path', + params: { word: pathWord! }, + replaceFrom: input.length - pathWord!.length + } +} + export function useCompletion(input: string, blocked: boolean, gw: GatewayClient) { const [completions, setCompletions] = useState([]) const [compIdx, setCompIdx] = useState(0) @@ -33,35 +64,19 @@ export function useCompletion(input: string, blocked: boolean, gw: GatewayClient ref.current = input - const isSlash = input.startsWith('/') - const pathWord = isSlash ? null : (input.match(TAB_PATH_RE)?.[1] ?? null) - - if (!isSlash && !pathWord) { + const request = completionRequestForInput(input) + if (!request) { clear() return } - // `/model` / `/provider` use the two-step ModelPicker (real curated IDs). - // Slash completion here only showed short aliases + vendor/family meta. - if (isSlash && /^\/(?:model|provider)(?:\s|$)/.test(input)) { - clear() - - return - } - - const pathReplace = input.length - (pathWord?.length ?? 0) - const t = setTimeout(() => { if (ref.current !== input) { return } - const req = isSlash - ? gw.request('complete.slash', { text: input }) - : gw.request('complete.path', { word: pathWord }) - - req + gw.request(request.method, request.params) .then(raw => { if (ref.current !== input) { return @@ -71,7 +86,7 @@ export function useCompletion(input: string, blocked: boolean, gw: GatewayClient setCompletions(r?.items ?? []) setCompIdx(0) - setCompReplace(isSlash ? (r?.replace_from ?? 1) : pathReplace) + setCompReplace(request.method === 'complete.slash' ? (r?.replace_from ?? 1) : request.replaceFrom) }) .catch((e: unknown) => { if (ref.current !== input) { @@ -86,7 +101,7 @@ export function useCompletion(input: string, blocked: boolean, gw: GatewayClient } ]) setCompIdx(0) - setCompReplace(isSlash ? 1 : pathReplace) + setCompReplace(request.replaceFrom) }) }, 60) diff --git a/ui-tui/src/lib/platform.ts b/ui-tui/src/lib/platform.ts index 343d8f8683..d7d2cc1ff0 100644 --- a/ui-tui/src/lib/platform.ts +++ b/ui-tui/src/lib/platform.ts @@ -51,13 +51,359 @@ export const isCopyShortcut = ( (isMac && key.ctrl && (key.meta || key.super === true))) /** - * Voice recording toggle key (Ctrl+B). + * Voice recording toggle key — configurable via ``voice.record_key`` in + * ``config.yaml`` (default ``ctrl+b``). * - * Documented as "Ctrl+B" everywhere: tips.py, config.yaml's voice.record_key - * default, and the Python CLI prompt_toolkit handler. We accept raw Ctrl+B on - * every platform so the TUI matches those docs. On macOS we additionally - * accept Cmd+B (the platform action modifier) so existing macOS muscle memory - * keeps working. + * Documented in tips.py, the Python CLI prompt_toolkit handler, and the + * config.yaml default. The TUI honours the same config knob (#18994); + * when ``voice.record_key`` is e.g. ``ctrl+o`` the TUI binds Ctrl+O. + * + * Only the documented default (``ctrl+b``) additionally accepts the + * macOS action modifier (Cmd+B) — custom bindings like ``ctrl+o`` + * require the literal Ctrl bit so Cmd+O can't steal the shortcut. */ -export const isVoiceToggleKey = (key: { ctrl: boolean; meta: boolean; super?: boolean }, ch: string): boolean => - (key.ctrl || isActionMod(key)) && ch.toLowerCase() === 'b' +export type VoiceRecordKeyMod = 'alt' | 'ctrl' | 'super' + +/** Named (multi-character) keys we support, matching the CLI's + * prompt_toolkit binding shape (``c-space``, ``c-enter``, etc.) so a + * config value like ``ctrl+space`` binds in both runtimes. */ +export type VoiceRecordKeyNamed = 'backspace' | 'delete' | 'enter' | 'escape' | 'space' | 'tab' + +export interface ParsedVoiceRecordKey { + /** Single character (``'b'``, ``'o'``) when ``named`` is undefined, + * otherwise the named-key token (``'space'``, ``'enter'``…). Kept as + * one field for back-compat with the v1 ``{ ch, mod, raw }`` shape. */ + ch: string + mod: VoiceRecordKeyMod + named?: VoiceRecordKeyNamed + raw: string +} + +export const DEFAULT_VOICE_RECORD_KEY: ParsedVoiceRecordKey = { + ch: 'b', + mod: 'ctrl', + raw: 'ctrl+b' +} + +/** Modifier aliases. + * + * ``meta`` / ``cmd`` / ``command`` are intentionally absent. + * hermes-ink sets ``key.meta`` for plain Alt/Option on every platform + * AND for Cmd on some legacy macOS terminals (Terminal.app without + * kitty-protocol passthrough). Accepting any of those as a literal + * modifier would produce a display/binding mismatch — a config like + * ``cmd+b`` would render as ``Cmd+B`` but silently fire on Alt+B, or + * never fire at all on legacy terminals even though the UI advertises + * it (Copilot round-6 review on #19835). Users on modern kitty-style + * terminals (iTerm2 CSI-u, Ghostty, Kitty, WezTerm, Alacritty) spell + * the platform action modifier ``super`` / ``win``, which match the + * unambiguous ``key.super`` bit. macOS users on Terminal.app stick + * with the documented ``ctrl+b``. + * + * Cross-runtime parity: the ``ctrl`` / ``control`` / ``alt`` / ``option`` / + * ``opt`` spellings are normalized identically in the classic CLI + * (``hermes_cli/voice.py::normalize_voice_record_key_for_prompt_toolkit``) + * so one ``voice.record_key`` value binds the same shortcut in both + * runtimes (Copilot round-9 review on #19835). The ``super`` / + * ``win`` / ``windows`` spellings are TUI-only — prompt_toolkit has no + * super modifier, so the CLI falls back to the documented default and + * logs a warning at startup (Copilot round-11 review on #19835). */ +const _MOD_ALIASES: Record = { + alt: 'alt', + control: 'ctrl', + ctrl: 'ctrl', + option: 'alt', + opt: 'alt', + super: 'super', + win: 'super', + windows: 'super' +} + +/** Map config-string named tokens to the canonical name used at match time. + * + * Aliases mirror what prompt_toolkit accepts (``return`` ↔ ``enter``, + * ``esc`` ↔ ``escape``) so a config that round-trips through the CLI also + * binds in the TUI. */ +const _NAMED_KEY_ALIASES: Record = { + backspace: 'backspace', + bs: 'backspace', + del: 'delete', + delete: 'delete', + enter: 'enter', + esc: 'escape', + escape: 'escape', + ret: 'enter', + return: 'enter', + space: 'space', + spc: 'space', + tab: 'tab' +} + +/** ``useInputHandlers()`` intercepts these unconditionally before the + * voice check runs, so a binding like ``ctrl+c`` (interrupt), + * ``ctrl+d`` (quit), or ``ctrl+l`` (clear screen) would be advertised + * in /voice status but never fire push-to-talk. Reject at parse time + * so the user gets the documented Ctrl+B instead of a dead shortcut + * (Copilot round-4 review on #19835). + * + * ``ctrl+x`` is intentionally NOT here — it's only claimed during + * queue-edit (``queueEditIdx !== null``), so the voice binding works + * for most of the session and matches CLI parity for ``ctrl+`` + * bindings (Copilot round-8 review on #19835). */ +const _RESERVED_CTRL_CHARS = new Set(['c', 'd', 'l']) + +/** On macOS the action-modifier intercepts these editor chords via + * ``isCopyShortcut`` / ``isAction`` in ``useInputHandlers()``: + * - super+c → copy + * - super+d → exit + * - super+l → clear screen + * - super+v → paste (also claimed at the TextInput layer) + * On Linux/Windows those globals key off Ctrl instead of Super, so + * super+ bindings don't collide. Gate the rejection to darwin + * at parse time so kitty/CSI-u ``super+`` configs still work for + * non-mac users (Copilot round-8 review on #19835). */ +const _RESERVED_SUPER_CHARS = new Set(['c', 'd', 'l', 'v']) + +/** On macOS ``isActionMod`` accepts ``key.meta`` as the action + * modifier — but hermes-ink reports Alt as ``key.meta`` on many + * terminals. So on darwin a configured ``alt+c`` / ``alt+d`` / ``alt+l`` + * gets swallowed by ``isCopyShortcut`` / ``isAction`` before the voice + * check runs. Block at parse time so /voice status doesn't advertise + * a shortcut that actually copies / quits / clears (Copilot round-12 + * review on #19835). */ +const _RESERVED_ALT_CHARS_MAC = new Set(['c', 'd', 'l']) + +interface RuntimeKeyEvent { + alt?: boolean + backspace?: boolean + ctrl: boolean + delete?: boolean + escape?: boolean + meta: boolean + return?: boolean + shift?: boolean + super?: boolean + tab?: boolean +} + +/** Match an ink ``key`` event against a parsed named key. The ink runtime + * sets one boolean per named key; ``space`` is a printable char so it + * arrives as ``ch === ' '`` rather than a dedicated ``key.space`` flag. */ +const _matchesNamedKey = ( + named: VoiceRecordKeyNamed, + key: RuntimeKeyEvent, + ch: string +): boolean => { + switch (named) { + case 'backspace': + return key.backspace === true + case 'delete': + return key.delete === true + case 'enter': + return key.return === true + case 'escape': + return key.escape === true + case 'space': + return ch === ' ' + case 'tab': + return key.tab === true + } +} + +/** + * Parse a config-string voice record key like ``ctrl+b`` / ``alt+r`` / + * ``ctrl+space`` into ``{mod, ch, named?}``. Accepts single characters + * AND the named tokens declared in ``_NAMED_KEY_ALIASES`` (``space``, + * ``enter``/``return``, ``tab``, ``escape``/``esc``, ``backspace``, + * ``delete``) — matching the keys prompt_toolkit accepts on the CLI + * side via the ``c-`` rewrite in ``cli.py``. + * + * Accepts ``unknown`` because the source is raw YAML via + * ``config.get full`` — a hand-edited ``voice.record_key: 1`` or + * ``voice.record_key: true`` would otherwise crash ``.trim()`` on a + * non-string scalar (Copilot round-3 review on #19835). Non-string / + * empty / unrecognised values fall back to the documented Ctrl+B + * default so a typo never silently disables the shortcut. + */ +export const parseVoiceRecordKey = (raw: unknown): ParsedVoiceRecordKey => { + if (typeof raw !== 'string') { + return DEFAULT_VOICE_RECORD_KEY + } + + const lower = raw.trim().toLowerCase() + + if (!lower) { + return DEFAULT_VOICE_RECORD_KEY + } + + const parts = lower.split('+').map(p => p.trim()).filter(Boolean) + + if (!parts.length) { + return DEFAULT_VOICE_RECORD_KEY + } + + const last = parts[parts.length - 1] + const modCandidates = parts.slice(0, -1) + + // Reject multi-modifier chords (``ctrl+alt+r``, ``cmd+ctrl+b``) rather + // than silently dropping the extra modifier — the previous + // single-token validator made a typo bind a different shortcut than + // the user configured (Copilot round-3 review on #19835). The classic + // CLI only supports single-modifier bindings via prompt_toolkit's + // ``c-x`` / ``a-x`` rewrite in ``cli.py``, so this matches CLI parity. + if (modCandidates.length > 1) { + return DEFAULT_VOICE_RECORD_KEY + } + + // Require an explicit modifier. A bare ``o`` / ``space`` / ``escape`` + // has no sensible mapping: the CLI's prompt_toolkit binds the raw + // key (no rewrite) so bare-char configs would silently diverge + // between the two runtimes (Copilot round-4 review on #19835). + // Fall back to the documented default. + if (modCandidates.length === 0) { + return DEFAULT_VOICE_RECORD_KEY + } + + const norm = _MOD_ALIASES[modCandidates[0]] + + // Unknown modifier token (e.g. bare ``meta+b`` which is ambiguous on + // the wire) falls back to the documented default rather than + // silently coercing to Ctrl and producing a misleading bind. + if (!norm) { + return DEFAULT_VOICE_RECORD_KEY + } + + const mod = norm + + // Block bindings the TUI input handler intercepts before the voice + // check — ``ctrl+c`` / ``ctrl+d`` / ``ctrl+l`` would never actually + // fire push-to-talk, so advertising them in /voice status is a lie. + if (mod === 'ctrl' && last.length === 1 && _RESERVED_CTRL_CHARS.has(last)) { + return DEFAULT_VOICE_RECORD_KEY + } + + // Same for ``super+c`` / ``super+d`` / ``super+l`` / ``super+v`` on + // macOS only — those are copy / exit / clear / paste and get claimed + // by ``isCopyShortcut`` / ``isAction`` / the TextInput paste layer + // before voice has a chance to toggle. On Linux/Windows the TUI + // globals key off Ctrl (not Super), so kitty/CSI-u ``super+`` + // bindings stay usable for non-mac users. + if (isMac && mod === 'super' && last.length === 1 && _RESERVED_SUPER_CHARS.has(last)) { + return DEFAULT_VOICE_RECORD_KEY + } + + // On macOS hermes-ink reports Alt as ``key.meta``, which ``isActionMod`` + // accepts as the mac action modifier. So ``alt+c`` / ``alt+d`` / ``alt+l`` + // collide with copy / exit / clear in ``useInputHandlers()`` before the + // voice check. Reject at parse time on darwin only — non-mac ``alt+`` + // bindings are still usable (Copilot round-12 review on #19835). + if (isMac && mod === 'alt' && last.length === 1 && _RESERVED_ALT_CHARS_MAC.has(last)) { + return DEFAULT_VOICE_RECORD_KEY + } + + if (last.length === 1) { + return { ch: last, mod, raw: lower } + } + + const named = _NAMED_KEY_ALIASES[last] + + if (named) { + return { ch: named, mod, named, raw: lower } + } + + // Unknown multi-character token (e.g. typo'd ``ctrl+spcae``) — fall back + // to the doc default rather than silently disabling the binding. + return DEFAULT_VOICE_RECORD_KEY +} + +/** Render a parsed key back as ``Ctrl+B`` / ``Ctrl+Space`` for status text. + * + * Platform-aware for the ``super`` modifier: renders ``Cmd`` on macOS and + * ``Super`` elsewhere. Previously rendered ``Cmd`` universally, which told + * Linux/Windows users the wrong modifier to press (Copilot review, round + * 2 on #19835). */ +export const formatVoiceRecordKey = (parsed: ParsedVoiceRecordKey): string => { + const modLabel = + parsed.mod === 'super' ? (isMac ? 'Cmd' : 'Super') : parsed.mod[0].toUpperCase() + parsed.mod.slice(1) + // Named tokens render in title case (Ctrl+Space, Ctrl+Enter); single + // chars render upper-case to match the existing Ctrl+B convention. + const keyLabel = parsed.named + ? parsed.named[0].toUpperCase() + parsed.named.slice(1) + : parsed.ch.toUpperCase() + + return `${modLabel}+${keyLabel}` +} + +/** Whether the parsed binding is the documented default (ctrl+b). + * + * Compare on the parsed spec rather than ``raw`` so semantically-equal + * aliases (``control+b``, ``ctrl + b``) still get the macOS Cmd+B + * muscle-memory fallback (Copilot review, round 2 on #19835). */ +const _isDefaultVoiceKey = (parsed: ParsedVoiceRecordKey): boolean => + parsed.mod === DEFAULT_VOICE_RECORD_KEY.mod && + parsed.ch === DEFAULT_VOICE_RECORD_KEY.ch && + parsed.named === DEFAULT_VOICE_RECORD_KEY.named + +export const isVoiceToggleKey = ( + key: RuntimeKeyEvent, + ch: string, + configured: ParsedVoiceRecordKey = DEFAULT_VOICE_RECORD_KEY +): boolean => { + // Match the configured key first (single-char compare or named-key + // event-property check). Bail out before evaluating modifier shape + // so the wrong key never reaches the modifier guard. + if (configured.named) { + if (!_matchesNamedKey(configured.named, key, ch)) { + return false + } + } else if (ch.toLowerCase() !== configured.ch) { + return false + } + + // The parser rejects multi-modifier configs (``ctrl+shift+b`` etc.), + // so at match time Shift must always be clear — otherwise + // ``ctrl+tab`` would also fire on Ctrl+Shift+Tab and ``alt+enter`` + // on Alt+Shift+Enter, triggering a different chord than configured + // (Copilot round-5 review on #19835). + if (key.shift === true) { + return false + } + + switch (configured.mod) { + case 'alt': + // Most terminals surface Alt as either ``alt`` or ``meta``; accept + // both so the binding works across xterm-style and kitty-style + // protocols. Guard against ctrl/super bits so a chord like + // Ctrl+Alt+ or Cmd+Alt+ doesn't spuriously fire the + // alt binding. + // + // Bare Escape on hermes-ink can arrive as ``key.meta=true`` on some + // terminals, so a configured ``alt+escape`` must not match that shape; + // require an explicit alt bit for escape chords (Copilot round-7 + // follow-up on #19835). + return (key.alt === true || (key.meta && key.escape !== true)) && !key.ctrl && key.super !== true + case 'ctrl': + // Require the Ctrl bit AND a clear Alt/Super so a chord like + // Ctrl+Alt+ / Ctrl+Cmd+ doesn't spuriously match + // ``ctrl+`` (Copilot round-6 review on #19835). + // + // The documented default (``ctrl+b``) additionally accepts the + // explicit ``key.super`` bit on macOS for Cmd+B muscle memory — + // but ONLY ``key.super`` (kitty-style), never ``key.meta``, since + // ``key.meta`` is hermes-ink's Alt signal and accepting it would + // fire the binding on Alt+B. + if (key.ctrl) { + return !key.alt && !key.meta && key.super !== true + } + + return _isDefaultVoiceKey(configured) && isMac && key.super === true && !key.alt && !key.meta + case 'super': + // Require the explicit ``key.super`` bit (kitty-style protocol) + // AND clear Ctrl/Alt/Meta so Ctrl+Cmd+X or Alt+Cmd+X don't + // spuriously fire the super binding (Copilot round-6 review on + // #19835). Legacy-terminal users whose Cmd arrives as + // ``key.meta`` need a kitty-protocol terminal — see the + // _MOD_ALIASES doc-comment for the rationale. + return key.super === true && !key.ctrl && !key.alt && !key.meta + } +} diff --git a/website/docs/getting-started/quickstart.md b/website/docs/getting-started/quickstart.md index c9af170420..01c5239a11 100644 --- a/website/docs/getting-started/quickstart.md +++ b/website/docs/getting-started/quickstart.md @@ -204,7 +204,7 @@ Only after the base chat works. Pick what you need: hermes gateway setup # Interactive platform configuration ``` -Connect [Telegram](/docs/user-guide/messaging/telegram), [Discord](/docs/user-guide/messaging/discord), [Slack](/docs/user-guide/messaging/slack), [WhatsApp](/docs/user-guide/messaging/whatsapp), [Signal](/docs/user-guide/messaging/signal), [Email](/docs/user-guide/messaging/email), or [Home Assistant](/docs/user-guide/messaging/homeassistant). +Connect [Telegram](/docs/user-guide/messaging/telegram), [Discord](/docs/user-guide/messaging/discord), [Slack](/docs/user-guide/messaging/slack), [WhatsApp](/docs/user-guide/messaging/whatsapp), [Signal](/docs/user-guide/messaging/signal), [Email](/docs/user-guide/messaging/email), or [Home Assistant](/docs/user-guide/messaging/homeassistant), or [Microsoft Teams](/docs/user-guide/messaging/teams). ### Automation and tools @@ -307,7 +307,7 @@ That sequence gets you from "broken vibes" back to a known state fast. - **[CLI Guide](../user-guide/cli.md)** — Master the terminal interface - **[Configuration](../user-guide/configuration.md)** — Customize your setup -- **[Messaging Gateway](../user-guide/messaging/index.md)** — Connect Telegram, Discord, Slack, WhatsApp, Signal, Email, or Home Assistant +- **[Messaging Gateway](../user-guide/messaging/index.md)** — Connect Telegram, Discord, Slack, WhatsApp, Signal, Email, Home Assistant, Teams, and more - **[Tools & Toolsets](../user-guide/features/tools.md)** — Explore available capabilities - **[AI Providers](../integrations/providers.md)** — Full provider list and setup details - **[Skills System](../user-guide/features/skills.md)** — Reusable workflows and knowledge diff --git a/website/docs/getting-started/updating.md b/website/docs/getting-started/updating.md index 8bd84ba157..c39363a9e0 100644 --- a/website/docs/getting-started/updating.md +++ b/website/docs/getting-started/updating.md @@ -107,7 +107,7 @@ Compare against the latest release at the [GitHub releases page](https://github. ### Updating from Messaging Platforms -You can also update directly from Telegram, Discord, Slack, or WhatsApp by sending: +You can also update directly from Telegram, Discord, Slack, WhatsApp, or Teams by sending: ``` /update diff --git a/website/docs/index.md b/website/docs/index.md index 17a2ac8cc2..db7106d955 100644 --- a/website/docs/index.md +++ b/website/docs/index.md @@ -28,7 +28,7 @@ It's not a coding copilot tethered to an IDE or a chatbot wrapper around a singl | 📖 **[Quickstart Tutorial](/docs/getting-started/quickstart)** | Your first conversation and key features to try | | 🗺️ **[Learning Path](/docs/getting-started/learning-path)** | Find the right docs for your experience level | | ⚙️ **[Configuration](/docs/user-guide/configuration)** | Config file, providers, models, and options | -| 💬 **[Messaging Gateway](/docs/user-guide/messaging)** | Set up Telegram, Discord, Slack, or WhatsApp | +| 💬 **[Messaging Gateway](/docs/user-guide/messaging)** | Set up Telegram, Discord, Slack, WhatsApp, Teams, or more | | 🔧 **[Tools & Toolsets](/docs/user-guide/features/tools)** | 68 built-in tools and how to configure them | | 🧠 **[Memory System](/docs/user-guide/features/memory)** | Persistent memory that grows across sessions | | 📚 **[Skills System](/docs/user-guide/features/skills)** | Procedural memory the agent creates and reuses | @@ -47,7 +47,7 @@ It's not a coding copilot tethered to an IDE or a chatbot wrapper around a singl - **A closed learning loop** — Agent-curated memory with periodic nudges, autonomous skill creation, skill self-improvement during use, FTS5 cross-session recall with LLM summarization, and [Honcho](https://github.com/plastic-labs/honcho) dialectic user modeling - **Runs anywhere, not just your laptop** — 6 terminal backends: local, Docker, SSH, Daytona, Singularity, Modal. Daytona and Modal offer serverless persistence — your environment hibernates when idle, costing nearly nothing -- **Lives where you do** — CLI, Telegram, Discord, Slack, WhatsApp, Signal, Matrix, Mattermost, Email, SMS, DingTalk, Feishu, WeCom, BlueBubbles, Home Assistant — 15+ platforms from one gateway +- **Lives where you do** — CLI, Telegram, Discord, Slack, WhatsApp, Signal, Matrix, Mattermost, Email, SMS, DingTalk, Feishu, WeCom, BlueBubbles, Home Assistant, Microsoft Teams — 15+ platforms from one gateway - **Built by model trainers** — Created by [Nous Research](https://nousresearch.com), the lab behind Hermes, Nomos, and Psyche. Works with [Nous Portal](https://portal.nousresearch.com), [OpenRouter](https://openrouter.ai), OpenAI, or any endpoint - **Scheduled automations** — Built-in cron with delivery to any platform - **Delegates & parallelizes** — Spawn isolated subagents for parallel workstreams. Programmatic Tool Calling via `execute_code` collapses multi-step pipelines into single inference calls diff --git a/website/docs/integrations/index.md b/website/docs/integrations/index.md index f511e03bf5..20b86565d8 100644 --- a/website/docs/integrations/index.md +++ b/website/docs/integrations/index.md @@ -82,7 +82,7 @@ Speech-to-text supports six providers: local faster-whisper (free, runs on-devic Hermes runs as a gateway bot on 15+ messaging platforms, all configured through the same `gateway` subsystem: -- **[Telegram](/docs/user-guide/messaging/telegram)**, **[Discord](/docs/user-guide/messaging/discord)**, **[Slack](/docs/user-guide/messaging/slack)**, **[WhatsApp](/docs/user-guide/messaging/whatsapp)**, **[Signal](/docs/user-guide/messaging/signal)**, **[Matrix](/docs/user-guide/messaging/matrix)**, **[Mattermost](/docs/user-guide/messaging/mattermost)**, **[Email](/docs/user-guide/messaging/email)**, **[SMS](/docs/user-guide/messaging/sms)**, **[DingTalk](/docs/user-guide/messaging/dingtalk)**, **[Feishu/Lark](/docs/user-guide/messaging/feishu)**, **[WeCom](/docs/user-guide/messaging/wecom)**, **[WeCom Callback](/docs/user-guide/messaging/wecom-callback)**, **[Weixin](/docs/user-guide/messaging/weixin)**, **[BlueBubbles](/docs/user-guide/messaging/bluebubbles)**, **[QQ Bot](/docs/user-guide/messaging/qqbot)**, **[Home Assistant](/docs/user-guide/messaging/homeassistant)**, **[Webhooks](/docs/user-guide/messaging/webhooks)** +- **[Telegram](/docs/user-guide/messaging/telegram)**, **[Discord](/docs/user-guide/messaging/discord)**, **[Slack](/docs/user-guide/messaging/slack)**, **[WhatsApp](/docs/user-guide/messaging/whatsapp)**, **[Signal](/docs/user-guide/messaging/signal)**, **[Matrix](/docs/user-guide/messaging/matrix)**, **[Mattermost](/docs/user-guide/messaging/mattermost)**, **[Email](/docs/user-guide/messaging/email)**, **[SMS](/docs/user-guide/messaging/sms)**, **[DingTalk](/docs/user-guide/messaging/dingtalk)**, **[Feishu/Lark](/docs/user-guide/messaging/feishu)**, **[WeCom](/docs/user-guide/messaging/wecom)**, **[WeCom Callback](/docs/user-guide/messaging/wecom-callback)**, **[Weixin](/docs/user-guide/messaging/weixin)**, **[BlueBubbles](/docs/user-guide/messaging/bluebubbles)**, **[QQ Bot](/docs/user-guide/messaging/qqbot)**, **[Home Assistant](/docs/user-guide/messaging/homeassistant)**, **[Microsoft Teams](/docs/user-guide/messaging/teams)**, **[Webhooks](/docs/user-guide/messaging/webhooks)** See the [Messaging Gateway overview](/docs/user-guide/messaging) for the platform comparison table and setup guide. diff --git a/website/docs/reference/slash-commands.md b/website/docs/reference/slash-commands.md index 75158d6a0d..1348062af1 100644 --- a/website/docs/reference/slash-commands.md +++ b/website/docs/reference/slash-commands.md @@ -130,7 +130,7 @@ Commands support prefix matching: typing `/h` resolves to `/help`, `/mod` resolv ## Messaging slash commands -The messaging gateway supports the following built-in commands inside Telegram, Discord, Slack, WhatsApp, Signal, Email, and Home Assistant chats: +The messaging gateway supports the following built-in commands inside Telegram, Discord, Slack, WhatsApp, Signal, Email, Home Assistant, and Teams chats: | Command | Description | |---------|-------------| diff --git a/website/docs/user-guide/configuration.md b/website/docs/user-guide/configuration.md index 517cb2e988..4c5e51860e 100644 --- a/website/docs/user-guide/configuration.md +++ b/website/docs/user-guide/configuration.md @@ -103,7 +103,7 @@ For cloud sandboxes such as Modal, Daytona, and Vercel Sandbox, `container_persi | Backend | Where commands run | Isolation | Best for | |---------|-------------------|-----------|----------| | **local** | Your machine directly | None | Development, personal use | -| **docker** | Docker container | Full (namespaces, cap-drop) | Safe sandboxing, CI/CD | +| **docker** | Single persistent Docker container (shared across session, `/new`, subagents) | Full (namespaces, cap-drop) | Safe sandboxing, CI/CD | | **ssh** | Remote server via SSH | Network boundary | Remote dev, powerful hardware | | **modal** | Modal cloud sandbox | Full (cloud VM) | Ephemeral cloud compute, evals | | **daytona** | Daytona workspace | Full (cloud container) | Managed cloud dev environments | @@ -127,6 +127,8 @@ The agent has the same filesystem access as your user account. Use `hermes tools Runs commands inside a Docker container with security hardening (all capabilities dropped, no privilege escalation, PID limits). +**Single persistent container, not per-command.** Hermes starts ONE long-lived container on first use and routes every terminal, file, and `execute_code` call through `docker exec` into that same container — across sessions, `/new`, `/reset`, and `delegate_task` subagents — for the lifetime of the Hermes process. Working-directory changes, installed packages, and files in `/workspace` carry over from one tool call to the next, just like a local shell. The container is stopped and removed on shutdown. See **Container lifecycle** below for details. + ```yaml terminal: backend: docker diff --git a/website/docs/user-guide/docker.md b/website/docs/user-guide/docker.md index 2a13fe6662..e99459cc1d 100644 --- a/website/docs/user-guide/docker.md +++ b/website/docs/user-guide/docker.md @@ -9,7 +9,7 @@ description: "Running Hermes Agent in Docker and using Docker as a terminal back There are two distinct ways Docker intersects with Hermes Agent: 1. **Running Hermes IN Docker** — the agent itself runs inside a container (this page's primary focus) -2. **Docker as a terminal backend** — the agent runs on your host but executes commands inside a Docker sandbox (see [Configuration → terminal.backend](./configuration.md)) +2. **Docker as a terminal backend** — the agent runs on your host but executes every command inside a single, persistent Docker sandbox container that survives across tool calls, `/new`, and subagents for the life of the Hermes process (see [Configuration → Docker Backend](./configuration.md#docker-backend)) This page covers option 1. The container stores all user data (config, API keys, sessions, skills, memories) in a single directory mounted from the host at `/opt/data`. The image itself is stateless and can be upgraded by pulling a new version without losing any configuration. @@ -279,7 +279,7 @@ docker compose up -d ## Skills and credential files -When using Docker as the execution environment (not the methods above, but when the agent runs commands inside a Docker sandbox), Hermes automatically bind-mounts the skills directory (`~/.hermes/skills/`) and any credential files declared by skills into the container as read-only volumes. This means skill scripts, templates, and references are available inside the sandbox without manual configuration. +When using Docker as the execution environment (not the methods above, but when the agent runs commands inside a Docker sandbox — see [Configuration → Docker Backend](./configuration.md#docker-backend)), Hermes reuses a single long-lived container for all tool calls and automatically bind-mounts the skills directory (`~/.hermes/skills/`) and any credential files declared by skills into that container as read-only volumes. Skill scripts, templates, and references are available inside the sandbox without manual configuration, and because the container persists for the life of the Hermes process, any dependencies you install or files you write stay around for the next tool call. The same syncing happens for SSH and Modal backends — skills and credential files are uploaded via rsync or the Modal mount API before each command. diff --git a/website/docs/user-guide/features/hooks.md b/website/docs/user-guide/features/hooks.md index e3893c0a23..92e9bfefc1 100644 --- a/website/docs/user-guide/features/hooks.md +++ b/website/docs/user-guide/features/hooks.md @@ -18,7 +18,7 @@ All three systems are non-blocking — errors in any hook are caught and logged, ## Gateway Event Hooks -Gateway hooks fire automatically during gateway operation (Telegram, Discord, Slack, WhatsApp) without blocking the main agent pipeline. +Gateway hooks fire automatically during gateway operation (Telegram, Discord, Slack, WhatsApp, Teams) without blocking the main agent pipeline. ### Creating a Hook @@ -346,7 +346,7 @@ An earlier version of Hermes shipped this as a built-in hook and silently spawne 5. Errors in any handler are caught and logged — a broken hook never crashes the agent :::info -Gateway hooks only fire in the **gateway** (Telegram, Discord, Slack, WhatsApp). The CLI does not load gateway hooks. For hooks that work everywhere, use [plugin hooks](#plugin-hooks). +Gateway hooks only fire in the **gateway** (Telegram, Discord, Slack, WhatsApp, Teams). The CLI does not load gateway hooks. For hooks that work everywhere, use [plugin hooks](#plugin-hooks). ::: ## Plugin Hooks diff --git a/website/docs/user-guide/features/tools.md b/website/docs/user-guide/features/tools.md index 80e1800812..9f9eddbb51 100644 --- a/website/docs/user-guide/features/tools.md +++ b/website/docs/user-guide/features/tools.md @@ -84,6 +84,10 @@ terminal: docker_image: python:3.11-slim ``` +**One persistent container, shared across the whole process.** Hermes starts a single long-lived container on first use (`docker run -d ... sleep 2h`) and routes every terminal, file, and `execute_code` call through `docker exec` into that same container. Working-directory changes, installed packages, environment tweaks, and files written to `/workspace` all carry over from one tool call to the next, across `/new`, `/reset`, and `delegate_task` subagents, for the lifetime of the Hermes process. The container is stopped and removed on shutdown. + +This means the Docker backend behaves like a persistent sandbox VM, not a fresh container per command. If you `pip install foo` once, it's there for the rest of the session. If you `cd /workspace/project`, subsequent `ls` calls see that directory. See [Configuration → Docker Backend](../configuration.md#docker-backend) for the full lifecycle details and the `container_persistent` flag that controls whether `/workspace` and `/root` survive across Hermes restarts. + ### SSH Backend Recommended for security — agent can't modify its own code: diff --git a/website/docs/user-guide/messaging/index.md b/website/docs/user-guide/messaging/index.md index 578a826b64..25e8e4598f 100644 --- a/website/docs/user-guide/messaging/index.md +++ b/website/docs/user-guide/messaging/index.md @@ -1,12 +1,12 @@ --- sidebar_position: 1 title: "Messaging Gateway" -description: "Chat with Hermes from Telegram, Discord, Slack, WhatsApp, Signal, SMS, Email, Home Assistant, Mattermost, Matrix, DingTalk, Yuanbao, Webhooks, or any OpenAI-compatible frontend via the API server — architecture and setup overview" +description: "Chat with Hermes from Telegram, Discord, Slack, WhatsApp, Signal, SMS, Email, Home Assistant, Mattermost, Matrix, DingTalk, Yuanbao, Microsoft Teams, Webhooks, or any OpenAI-compatible frontend via the API server — architecture and setup overview" --- # Messaging Gateway -Chat with Hermes from Telegram, Discord, Slack, WhatsApp, Signal, SMS, Email, Home Assistant, Mattermost, Matrix, DingTalk, Feishu/Lark, WeCom, Weixin, BlueBubbles (iMessage), QQ, Yuanbao, or your browser. The gateway is a single background process that connects to all your configured platforms, handles sessions, runs cron jobs, and delivers voice messages. +Chat with Hermes from Telegram, Discord, Slack, WhatsApp, Signal, SMS, Email, Home Assistant, Mattermost, Matrix, DingTalk, Feishu/Lark, WeCom, Weixin, BlueBubbles (iMessage), QQ, Yuanbao, Microsoft Teams, or your browser. The gateway is a single background process that connects to all your configured platforms, handles sessions, runs cron jobs, and delivers voice messages. For the full voice feature set — including CLI microphone mode, spoken replies in messaging, and Discord voice-channel conversations — see [Voice Mode](/docs/user-guide/features/voice-mode) and [Use Voice Mode with Hermes](/docs/guides/use-voice-mode-with-hermes). @@ -32,6 +32,7 @@ For the full voice feature set — including CLI microphone mode, spoken replies | BlueBubbles | — | ✅ | ✅ | — | ✅ | ✅ | — | | QQ | ✅ | ✅ | ✅ | — | — | ✅ | — | | Yuanbao | ✅ | ✅ | ✅ | — | — | ✅ | ✅ | +| Microsoft Teams | — | ✅ | — | ✅ | — | ✅ | — | **Voice** = TTS audio replies and/or voice message transcription. **Images** = send/receive images. **Files** = send/receive file attachments. **Threads** = threaded conversations. **Reactions** = emoji reactions on messages. **Typing** = typing indicator while processing. **Streaming** = progressive message updates via editing. @@ -59,8 +60,9 @@ flowchart TB bb[BlueBubbles] qq[QQ] yb[Yuanbao] - api["API Server
(OpenAI-compatible)"] - wh[Webhooks] + ms[Microsoft Teams] + api["API Server
(OpenAI-compatible)"] + wh[Webhooks] end store["Session store
per chat"] @@ -86,6 +88,7 @@ flowchart TB bb --> store qq --> store yb --> store + ms --> store api --> store wh --> store store --> agent @@ -189,6 +192,7 @@ DINGTALK_ALLOWED_USERS=user-id-1 FEISHU_ALLOWED_USERS=ou_xxxxxxxx,ou_yyyyyyyy WECOM_ALLOWED_USERS=user-id-1,user-id-2 WECOM_CALLBACK_ALLOWED_USERS=user-id-1,user-id-2 +TEAMS_ALLOWED_USERS=aad-object-id-1,aad-object-id-2 # Or allow GATEWAY_ALLOWED_USERS=123456789,987654321 @@ -393,6 +397,7 @@ Each platform has its own toolset: | BlueBubbles | `hermes-bluebubbles` | Full tools including terminal | | QQBot | `hermes-qqbot` | Full tools including terminal | | Yuanbao | `hermes-yuanbao` | Full tools including terminal | +| Microsoft Teams | `hermes-teams` | Full tools including terminal | | API Server | `hermes` (default) | Full tools including terminal | | Webhooks | `hermes-webhook` | Full tools including terminal | @@ -416,5 +421,6 @@ Each platform has its own toolset: - [BlueBubbles Setup (iMessage)](bluebubbles.md) - [QQBot Setup](qqbot.md) - [Yuanbao Setup](yuanbao.md) +- [Microsoft Teams Setup](teams.md) - [Open WebUI + API Server](open-webui.md) - [Webhooks](webhooks.md) \ No newline at end of file diff --git a/website/docs/user-guide/sessions.md b/website/docs/user-guide/sessions.md index ec101f6b45..5094edf64c 100644 --- a/website/docs/user-guide/sessions.md +++ b/website/docs/user-guide/sessions.md @@ -10,7 +10,7 @@ Hermes Agent automatically saves every conversation as a session. Sessions enabl ## How Sessions Work -Every conversation — whether from the CLI, Telegram, Discord, Slack, WhatsApp, Signal, Matrix, or any other messaging platform — is stored as a session with full message history. Sessions are tracked in two complementary systems: +Every conversation — whether from the CLI, Telegram, Discord, Slack, WhatsApp, Signal, Matrix, Teams, or any other messaging platform — is stored as a session with full message history. Sessions are tracked in two complementary systems: 1. **SQLite database** (`~/.hermes/state.db`) — structured session metadata with FTS5 full-text search 2. **JSONL transcripts** (`~/.hermes/sessions/`) — raw conversation transcripts including tool calls (gateway) diff --git a/website/docs/user-guide/skills/bundled/autonomous-ai-agents/autonomous-ai-agents-hermes-agent.md b/website/docs/user-guide/skills/bundled/autonomous-ai-agents/autonomous-ai-agents-hermes-agent.md index 1159c64398..c1c501932c 100644 --- a/website/docs/user-guide/skills/bundled/autonomous-ai-agents/autonomous-ai-agents-hermes-agent.md +++ b/website/docs/user-guide/skills/bundled/autonomous-ai-agents/autonomous-ai-agents-hermes-agent.md @@ -165,7 +165,7 @@ hermes gateway status Check status hermes gateway setup Configure platforms ``` -Supported platforms: Telegram, Discord, Slack, WhatsApp, Signal, Email, SMS, Matrix, Mattermost, Home Assistant, DingTalk, Feishu, WeCom, BlueBubbles (iMessage), Weixin (WeChat), API Server, Webhooks. Open WebUI connects via the API Server adapter. +Supported platforms: Telegram, Discord, Slack, WhatsApp, Signal, Email, SMS, Matrix, Mattermost, Home Assistant, DingTalk, Feishu, WeCom, BlueBubbles (iMessage), Weixin (WeChat), Microsoft Teams, API Server, Webhooks. Open WebUI connects via the API Server adapter. Platform docs: https://hermes-agent.nousresearch.com/docs/user-guide/messaging/ diff --git a/website/sidebars.ts b/website/sidebars.ts index c30fec6c52..a24366474e 100644 --- a/website/sidebars.ts +++ b/website/sidebars.ts @@ -133,6 +133,7 @@ const sidebars: SidebarsConfig = { 'user-guide/messaging/bluebubbles', 'user-guide/messaging/qqbot', 'user-guide/messaging/yuanbao', + 'user-guide/messaging/teams', 'user-guide/messaging/open-webui', 'user-guide/messaging/webhooks', ],