* fix(tui): respect voice.record_key config instead of hardcoded Ctrl+B
Classic CLI loaded ``voice.record_key`` from config.yaml and bound the
prompt-toolkit handler dynamically (``cli.py`` paths). The new TUI hard-
coded ``Ctrl+B`` everywhere — ``isVoiceToggleKey`` (input handler),
``/voice status`` ("Record key: Ctrl+B"), and ``/voice on`` ("Ctrl+B to
start/stop recording"). A user who set ``voice.record_key: ctrl+o``
(or any other key) saw the documented config silently ignored — only
Ctrl+B worked, the displayed shortcut lied about it.
Wire the configured key end to end through the existing channels:
* **Backend** (``tui_gateway/server.py``): ``voice.toggle`` action=status
AND action=on/off responses now include ``record_key``, sourced from
``config.get('voice', {}).get('record_key', 'ctrl+b')``.
* **Backend types** (``ui-tui/src/gatewayTypes.ts``): ``ConfigFullResponse``
now exposes ``config.voice.record_key`` and ``VoiceToggleResponse``
carries ``record_key`` so the TUI can both bind and display it.
* **Frontend parser/formatter** (``ui-tui/src/lib/platform.ts``):
``parseVoiceRecordKey()`` accepts ``ctrl+b`` / ``alt+r`` / ``cmd+space``
and the common aliases (``option``, ``cmd``, ``win``, …); falls back to
the documented Ctrl+B for empty / multi-character / malformed input so
a typo never silently disables the shortcut. ``formatVoiceRecordKey()``
renders for status text. ``isVoiceToggleKey`` now takes a parsed
``ParsedVoiceRecordKey`` argument; the hardcoded ``ch === 'b'`` is
gone. Default arg keeps existing call sites back-compat.
* **Hydration** (``ui-tui/src/app/useConfigSync.ts``,
``useMainApp.ts``): startup ``config.get full`` already runs; extract
``cfg.voice.record_key`` from it, parse, push into a new
``voiceRecordKey`` state, and forward to the input handler ctx
(``InputHandlerContext.voice.recordKey``). Mtime-poll path also
re-applies the parsed key so a hand-edit of config.yaml takes effect
the next tick — matches existing behaviour for display options.
* **Input handler** (``ui-tui/src/app/useInputHandlers.ts``):
``isVoiceToggleKey(key, ch, voice.recordKey)`` so the configured
binding fires.
* **Slash command** (``ui-tui/src/app/slash/commands/session.ts``):
``/voice status`` and ``/voice on`` use ``formatVoiceRecordKey`` on
the response's ``record_key`` instead of the hardcoded label.
Tests:
* ``parseVoiceRecordKey`` covers ctrl/alt/cmd/super aliases, multi-char
rejection, and empty fallback.
* ``formatVoiceRecordKey`` covers the doc examples (``Ctrl+B``,
``Ctrl+O``, ``Alt+R``, ``Cmd+B``).
* ``isVoiceToggleKey`` regression: ``ctrl+o`` configured → only ``o``
matches, not ``b``; ``alt+r`` matches both alt-bit and meta-bit
encodings (terminal protocol parity); omitted-arg call still binds
Ctrl+B for back-compat.
Full TUI suite (555 tests) passes; ``tsc --noEmit`` clean.
Fixes#18994
Co-authored-by: asheriif <ahmedsherif95@gmail.com>
* fix(tui): support named-key tokens in voice.record_key (space, enter, …)
Reviewer caught that the round-1 parser in #18994 rejected every
multi-character token, so a config value like ``ctrl+space`` (which the
CLI happily binds via prompt_toolkit's ``c-space`` rewrite in
``cli.py``) silently fell back to the documented Ctrl+B default —
re-introducing the same false-shortcut bug the PR was meant to fix,
just at a different surface.
Add explicit named-key support that mirrors what the CLI accepts:
* ``space`` (alias: ``spc``) → matches ``ch === ' '``
* ``enter`` (alias: ``return``, ``ret``) → matches ``key.return``
* ``tab`` → matches ``key.tab``
* ``escape`` (alias: ``esc``) → matches ``key.escape``
* ``backspace`` (alias: ``bs``) → matches ``key.backspace``
* ``delete`` (alias: ``del``) → matches ``key.delete``
``ParsedVoiceRecordKey`` gains an optional ``named`` field; ``ch``
holds either a single char (back-compat) or the canonical named token,
and the runtime matcher dispatches on ``named`` before checking the
modifier shape. Aliases collapse to one canonical name so
``ctrl+esc`` and ``ctrl+escape`` behave identically.
Unrecognised multi-character tokens (e.g. ``ctrl+spcae`` typo, or
unsupported keys like ``ctrl+f5``) still fall back to the Ctrl+B
default rather than silently disabling the binding — keeps the "typo
never silently kills the shortcut" guarantee.
Tests:
* ``parseVoiceRecordKey`` parametrised over every named token + each
alias variant.
* New ``isVoiceToggleKey`` cases for space (ch-based match), enter
(``key.return``), tab, escape, backspace, delete, including
modifier-mismatch negatives.
* ``formatVoiceRecordKey`` renders named keys in title case
(``Ctrl+Space``, ``Ctrl+Enter``).
* Existing fall-back-to-Ctrl+B contract preserved for empty input
AND unrecognised multi-char tokens.
Full TUI suite: 559/559 pass; ``tsc --noEmit`` clean.
Refs #18994 (round-1 review feedback)
Co-authored-by: asheriif <ahmedsherif95@gmail.com>
* test(tui): assert voice.toggle returns configured record_key
Salvage the backend regression from #19339 — asserts ``voice.toggle``
action=on AND action=status responses carry the configured
``voice.record_key`` end-to-end through ``_load_cfg()``. Keeps the
CLI→TUI parity contract visible in the Python test suite alongside
the existing frontend parser/matcher/formatter coverage from #19028.
* fix(tui): address Copilot review on #19835 voice.record_key wiring
Five tightenings on the parser + matcher + hydration surface, all
caught by the Copilot review on the PR — each one turns a silent
false-fire or display/binding skew into a deterministic behaviour.
* **isVoiceToggleKey ctrl branch was too permissive for named keys.**
The doc-default macOS Cmd+B muscle-memory fallback
(``isActionMod(key)`` on top of ``key.ctrl``) fired for every
configured key, so bare Esc — which hermes-ink reports with
``key.meta`` on some macOS terminals — triggered ``ctrl+escape``,
and Alt+Space / Alt+Tab triggered ``ctrl+space`` / ``ctrl+tab``.
Gate the fallback to the literal ``ctrl+b`` binding so any custom
chord requires the real Ctrl bit.
* **Alt branch guarded against Ctrl/Cmd co-press.** Without this,
Ctrl+Alt+<letter> and Cmd+Alt+<letter> also fired ``alt+<letter>``.
* **Dropped the ``meta`` modifier variant and its alias.** In
hermes-ink ``key.meta`` is Alt on xterm-style terminals and Cmd on
legacy macOS ones, so a literal ``meta+b`` config displayed as
``Cmd+B`` while matching Alt+B — exactly the kind of false
shortcut the PR was meant to remove. ``cmd`` / ``command`` now
collapse onto ``super`` (kitty-style ``key.super``, with a macOS
``key.meta`` fallback) and render as ``Cmd+B``. Unknown modifier
tokens fall back to the documented Ctrl+B default rather than
silently coercing to Ctrl.
* **Slash-command display/binding skew.** ``/voice status`` and
``/voice on`` rendered from the fresh gateway ``record_key``
response, but ``useInputHandlers()`` still bound the old key
until the next 5s mtime poll. Thread ``setVoiceRecordKey``
through ``SlashHandlerContext.voice`` and push the parsed spec
into frontend state on every response so text and binding stay
consistent.
* **Test coverage for the two paths Copilot flagged.** Added
vitest coverage for (a) the three-case ``/voice`` slash output
in ``createSlashHandler.test.ts`` and (b) the
``applyDisplay → voice.record_key`` hydration + omit-setter
back-compat paths in ``useConfigSync.test.ts``. Plus regression
cases for every false-fire scenario above.
Suite: 575/575 green, tsc --noEmit clean.
* fix(tui): address Copilot round-2 review on #19835
Three tightenings on the surface introduced in the round-1 fix:
* **``/voice tts`` reset custom bindings to Ctrl+B.** The ``tts`` branch
of ``voice.toggle`` omitted ``record_key`` from its response, so the
frontend's ``r.record_key ?? 'ctrl+b'`` coerced a user's custom
binding back to the default on every TTS toggle. Two-sided fix:
the backend now includes ``record_key`` on the ``tts`` branch (parity
with ``status``/``on``/``off``), and the slash handler only pushes
frontend state when the response actually carries ``record_key`` —
belt-and-suspenders against any future branch forgetting to include
it.
* **``super+b`` / ``win+b`` / ``cmd+b`` displayed "Cmd+B" on Linux and
Windows.** ``formatVoiceRecordKey`` rendered ``mod === 'super'`` as
``Cmd`` universally, which told non-mac users the wrong modifier to
press even though ``isVoiceToggleKey`` matched the right event bits.
Gate the label to ``isMac`` so non-mac renders ``Super+B``.
* **``control+b`` / ``ctrl + b`` lost the macOS Cmd+B fallback.**
``_isDefaultVoiceKey`` keyed off ``parsed.raw`` — so
semantically-equal aliases of the documented default dropped into
the strict branch even though they bind Ctrl+B. Compare on the
parsed spec (mod + ch + named) instead.
Coverage added: Linux ``Super+B`` rendering (and macOS ``Cmd+B``),
``control+b`` / ``ctrl + b`` accepting the Cmd+B fallback on darwin,
``/voice tts`` without ``record_key`` not clobbering cached binding,
and a backend regression asserting every ``voice.toggle`` branch
carries the configured key.
Suite: 579/579 TUI vitest green, 2/2 backend voice tests green,
tsc --noEmit clean.
* fix(tui): address Copilot round-3 review on #19835
Three classes of robustness issue caught on the second pass — all
revolve around malformed YAML tipping ``parseVoiceRecordKey`` or
``_voice_record_key`` into a crash instead of the documented
fallback.
* **Parser crashed on non-string YAML scalars.** ``config.get full``
returns raw ``yaml.safe_load`` output, so ``voice.record_key: 1``
or ``voice.record_key: true`` in a hand-edited config would hit
``.trim()`` on a number/bool and throw, breaking startup and
every mtime re-apply. Accept ``unknown`` at the signature, guard
with ``typeof raw !== 'string'``, and fall back to the default.
* **Backend blew up on non-dict ``voice:``.** Same YAML hazard on
the gateway side: ``voice: true`` / ``voice: cmd+b`` left
``_load_cfg().get("voice")`` as a bool/str, so ``.get("record_key")``
raised AttributeError and took every ``voice.toggle`` branch down
with it. Centralised the lookup in a single
``_voice_record_key()`` helper that ``isinstance``-guards both
``voice`` and ``record_key`` and falls back to ``ctrl+b``.
* **Multi-modifier chords silently dropped extras.** The previous
validator only checked the first modifier token, so ``ctrl+alt+r``
silently parsed as ``ctrl+r`` and ``cmd+ctrl+b`` as ``super+b`` —
a typo bound a different shortcut than the user configured.
Reject multi-modifier spellings outright; the classic CLI only
supports single-modifier bindings via prompt_toolkit's ``c-x`` /
``a-x`` rewrite, so this matches CLI parity.
Coverage added:
* ``parseVoiceRecordKey`` fallback on ``1`` / ``true`` / ``null`` /
``undefined`` / ``{}``.
* ``parseVoiceRecordKey`` fallback on ``ctrl+alt+r`` /
``cmd+ctrl+b`` / ``alt+ctrl+space``.
* ``test_voice_toggle_handles_non_dict_voice_cfg`` exercises
every non-dict ``voice:`` shape (bool, str, None, int, list) and
asserts each falls back to ``record_key: 'ctrl+b'``.
Suite: 581/581 TUI vitest green, 3/3 backend voice tests green,
tsc --noEmit clean.
* fix(tui): address Copilot round-4 review on #19835
Four final corners of the voice.record_key surface:
* **Bare-char configs silently coerced to ``ctrl+<key>``.** A config
like ``voice.record_key: o`` / ``space`` / ``escape`` fell through
to the default ``mod = 'ctrl'`` and silently bound Ctrl+O, while
the classic CLI's prompt_toolkit would bind the raw key (no
rewrite) — so the two runtimes silently disagreed on what "o"
means. Require an explicit modifier; bare-char configs fall back
to the documented Ctrl+B default.
* **Reserved ctrl+<letter> bindings would never fire.**
``useInputHandlers()`` intercepts ``ctrl+c`` (interrupt),
``ctrl+d`` (quit), and ``ctrl+l`` (clear screen) before the voice
check runs, so those configs would be advertised in /voice
status but the advertised shortcut never actually triggers
push-to-talk. Added ``_RESERVED_CTRL_CHARS`` at parse time so
the user gets the documented default instead of a dead shortcut.
(``alt+c``, ``cmd+l``, etc. are not intercepted and stay usable.)
* **``_load_cfg()`` root itself may be a non-dict.**
``_voice_record_key()`` isinstance-guarded the ``voice`` subkey
but not the root — a malformed config.yaml that collapsed to a
scalar/list at the top level (``config.yaml: true`` or ``[]``)
would still raise on ``.get("voice")``. Added the top-level
guard too so every malformed shape falls back to ``ctrl+b``.
* **Stale header comment on ``isVoiceToggleKey``.** The doc-comment
still claimed "On macOS we additionally accept the platform
action modifier (Cmd) for the configured letter" even though the
implementation gates the Cmd fallback to the documented default
only. Rewrote to match.
Coverage added:
* ``parseVoiceRecordKey`` fallback on bare chars (``o``, ``b``,
``space``, ``escape``).
* ``parseVoiceRecordKey`` fallback on ``ctrl+c`` / ``ctrl+d`` /
``ctrl+l``; positive case for ``alt+c`` / ``cmd+l`` still usable.
* Backend ``test_voice_toggle_handles_non_dict_voice_cfg`` now
exercises 5 non-dict shapes at the YAML root too.
Suite: 583/583 TUI vitest green, 3/3 backend voice tests green,
tsc --noEmit clean.
* fix(tui): address Copilot round-5 review on #19835
Three follow-ups on the voice matcher's modifier + shift discipline:
* **``super`` branch falsely fired on Alt+<key> / bare Esc on macOS.**
``isVoiceToggleKey`` accepted ``isMac && key.meta`` as a Cmd
fallback for the ``super`` modifier — but hermes-ink sets
``key.meta`` for plain Alt/Option AND for bare Escape on some
macOS terminals. A ``cmd+b`` config silently fired on Alt+B;
``cmd+space`` on Alt+Space; ``cmd+escape`` on bare Esc. Drop the
fallback and require the literal ``key.super`` bit. Legacy-
terminal users who need Cmd should upgrade to a kitty-protocol
terminal or bind ``alt+X`` explicitly.
* **Shift bit was never checked.** The parser rejects multi-
modifier configs like ``ctrl+shift+tab``, but the runtime
matcher didn't check ``key.shift`` — so ``ctrl+tab`` also fired
on Ctrl+Shift+Tab and ``alt+enter`` on Alt+Shift+Enter.
Early-return on ``key.shift === true`` so the runtime only fires
the exact chord the user configured.
* **Test leaked ``HERMES_VOICE=1`` into later tests.**
``voice.toggle`` action=on writes to ``os.environ`` directly
(CLI parity, runtime-only flag); ``test_voice_toggle_returns_
configured_record_key`` dispatched action=on without letting
monkeypatch take ownership of the var first. Any later test
that read voice mode in the same Python process could inherit a
stale enabled state. Added ``monkeypatch.setenv("HERMES_VOICE",
"0")`` up front so monkeypatch restores the original value at
teardown.
Coverage added:
* ``cmd+b`` / ``cmd+space`` / ``cmd+escape`` do NOT fire on
``key.meta``-only events on darwin.
* ``ctrl+tab`` / ``alt+enter`` / ``ctrl+o`` reject matches when
``key.shift`` is held; sanity cases without Shift still fire.
Suite: 585/585 TUI vitest green, 3/3 backend voice tests green,
tsc --noEmit clean.
* fix(tui): address Copilot round-6 review on #19835
Three classes of modifier-discipline tightening + one config-surface
honesty fix:
* **Default ``ctrl+b`` Cmd fallback leaked Alt+B.** The default's
macOS Cmd+B muscle-memory path used ``isActionMod(key)``, which
returns ``key.meta || key.super`` on darwin. hermes-ink also
reports plain Alt as ``key.meta``, so Alt+B silently fired the
default binding. Replaced with strict ``isMac && key.super ===
true`` — kitty-style Cmd+B still works, Alt+B correctly
rejected. Legacy-terminal mac users (Terminal.app without
CSI-u) now get raw Ctrl+B only; the documented default still
works everywhere.
* **ctrl / super branches accepted extra modifier bits.** The
parser rejects multi-modifier configs like ``ctrl+alt+o``, but
the runtime matcher was permissive — ``ctrl+o`` fired on
Ctrl+Alt+O / Ctrl+Cmd+O, and ``super+b`` fired on Cmd+Alt+B /
Ctrl+Cmd+B. Added strict ``!key.alt && !key.meta && key.super
!== true`` on ctrl, and ``!key.ctrl && !key.alt && !key.meta``
on super, so the runtime only fires the exact chord the parser
would let you configure.
* **Dropped ``cmd`` / ``command`` aliases.** They parsed to
``super`` and rendered as ``Cmd+X``, but legacy macOS terminals
report Cmd as ``key.meta`` (same signal as Alt), so a
``cmd+o`` config was advertised as working but never actually
fired on Terminal.app-without-CSI-u. That recreated the
"displayed shortcut does not work" problem this PR was meant to
remove. Users who want the platform action modifier spell it
``super`` / ``win`` — that matches the unambiguous ``key.super``
bit, and kitty-style macOS terminals render it as ``Cmd+X`` via
platform-aware formatter.
Coverage updated:
* Default ctrl+b no longer fires on Alt+B via ``key.meta`` leak;
raw Ctrl+B and kitty-style Cmd+B still fire.
* ``ctrl+o`` rejects Ctrl+Alt+O / Ctrl+Cmd+O / Ctrl+Meta+O chords.
* ``super+b`` rejects Cmd+Alt+B / Cmd+Meta+B / Ctrl+Cmd+B chords.
* ``cmd+b`` / ``command+b`` / ``meta+b`` all fall back to the
documented default at parse time (joined the ambiguous-mac-mod
rejection class).
* Round-2 expectations that asserted ``cmd+b`` parsed as super
and accepted ``key.meta`` on darwin updated to reflect the new
stricter contract.
Suite: 588/588 TUI vitest green, 3/3 backend voice tests green,
tsc --noEmit clean.
* fix(tui): address Copilot follow-up on wire typing + escape precedence
Two follow-ups from the latest Copilot pass:
* **Config wire typing honesty (`gatewayTypes.ts`)**
`config.get full` forwards raw `yaml.safe_load()` output, so
`voice.record_key` can be any scalar/container when hand-edited.
Typing it as `string` suggests a normalized contract that the
backend does not guarantee and makes unsafe callers more likely.
Change `ConfigVoiceConfig.record_key` to `unknown` with an
explicit comment that callers must normalize at runtime.
* **Escape-based voice bindings were swallowed before voice check**
`useInputHandlers()` handled `key.escape` for queue-edit cancel and
selection clear before `isVoiceToggleKey(...)`, so configured
`ctrl+escape` / `alt+escape` / `super+escape` chords were advertised
but never toggled recording in those UI states.
Add an early escape+voice check before generic Esc handlers so
escape-based voice bindings win when configured, while plain Esc
behavior remains unchanged.
Also updated PR #19835 description text to remove stale cmd/command
alias claims and match the current parser contract.
* fix(tui): pass configured voice shortcut through TextInput layer
Thread the live parsed voiceRecordKey into TextInput so configured voice.record_key chords bubble to useInputHandlers instead of being consumed as editor input. This removes the last hardcoded Ctrl+B pass-through in the composer path while preserving existing global control chord behavior.
* fix(tui): require explicit alt bit for escape-based alt chords
Hermes-ink reports bare Escape as meta=true+escape=true on some terminals, so a configured alt+escape binding was firing on bare Esc. Require an explicit key.alt bit when the configured named key is escape so plain Esc stays plain Esc; kitty-style alt+escape still fires.
* fix(tui): harden voice.record + TextInput paste + super-mod reserved list
Three round-7 Copilot follow-ups on #19835:
- voice.record start handler used _load_cfg().get('voice', {}).get(...) without
shape checks, so malformed YAML (bool/scalar/list) returned 5025 instead of
using VAD defaults. Centralized _voice_cfg_dict() helper and type-guarded
silence_threshold/silence_duration with numeric fallbacks.
- TextInput pass-through check moved above paste/copy handling so configured
voice chords (ctrl+v / alt+v / cmd+v) beat the composer's paste/copy
defaults.
- parser now also rejects super+{c,d,l,v} — on macOS those are
copy/exit/clear/paste and would be advertised in /voice status but never
actually toggle recording.
* Potential fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* fix(tui): round-8 Copilot review — allow ctrl+x, gate super reservations to macOS, preserve voice key on transient RPC failure
Three round-8 Copilot follow-ups on #19835:
- Revert ctrl+x addition to _RESERVED_CTRL_CHARS (landed via Copilot Autofix
commit 731ec86): ctrl+x is only claimed during queue-edit
(queueEditIdx !== null), so voice works the rest of the session and
matches CLI ctrl+<letter> parity.
- Gate super+{c,d,l,v} reservation to isMac. Linux/Windows TUI globals key
off Ctrl, so kitty/CSI-u super+<letter> configs don't collide on non-mac
and should stay usable.
- applyDisplay() now skips setVoiceRecordKey when cfg is null so one
transient quietRpc() failure after a config edit doesn't clobber the
cached binding back to Ctrl+B until the next successful poll.
New coverage:
- parseVoiceRecordKey preserves ctrl+x on linux
- super+{c,d,l,v} rejected on darwin, allowed on linux
- applyDisplay(null, ...) leaves voiceRecordKey untouched
* fix(cli,tui): normalize voice.record_key aliases across CLI + TUI for parity
Round-9 Copilot review on #19835: TUI accepted control+/option+/opt+/super+/win+ aliases but the classic CLI only rewrote literal ctrl+/alt+ before handing to prompt_toolkit, so a TUI-valid config silently bound a different (or no) shortcut in the CLI.
- Added normalize_voice_record_key_for_prompt_toolkit() in hermes_cli/voice.py with a single alias table (ctrl/control/alt/option/opt → c-/a-).
- Wired it into all three cli.py sites (_enable_voice_mode hint, _show_voice_status display, and the prompt_toolkit binding in _register_voice_handler).
- /voice status display now renders control+x as Ctrl+X and option+x as Alt+X (canonical casing) to match TUI formatVoiceRecordKey.
- super/win/windows are intentionally left unchanged: prompt_toolkit has no super modifier, so the CLI will reject them loudly at startup rather than silently binding Ctrl+B. Documented this split at both the TUI _MOD_ALIASES comment and the CLI normalizer docstring.
- Added tests covering ctrl/control/alt/option/opt mapping, case-insensitivity, non-string fallback, empty-string fallback, and super/win pass-through.
* fix(cli): port TUI parser contract into CLI voice.record_key normalizer
Round-10 Copilot review on #19835.
hermes_cli/voice.py's normalize_voice_record_key_for_prompt_toolkit() previously did blind substring replacement with no trim/validate step, so the CLI diverged from the TUI parser on:
- whitespace ('ctrl + b' -> 'c- b' instead of 'c-b')
- typoed named keys ('ctrl+spcae' passed through as 'c-spcae' and prompt_toolkit would reject at startup)
- bare-char configs ('o' should fall back, not pass through as 'o')
- multi-modifier chords ('ctrl+alt+r')
- reserved ctrl chars ('ctrl+c/d/l')
- unknown modifiers ('meta+b' / 'shift+b')
- named-key aliases ('return'/'esc'/'bs'/'del' not collapsed to prompt_toolkit canonicals)
Port the TUI parser contract into Python (_VOICE_MOD_ALIASES, _VOICE_NAMED_KEYS, _VOICE_RESERVED_CTRL_CHARS) so one config value binds the same shortcut in both runtimes.
Also added format_voice_record_key_for_status() shared between the PTT hint and /voice status display. Non-string scalars (voice.record_key: true / 1) now surface as 'Ctrl+B' instead of the raw scalar — /voice status no longer advertises a shortcut that can never bind.
Tests: 29/29 in test_voice_wrapper.py, including 11 new regressions covering whitespace, named-key aliases, typos, bare-char, multi-modifier, reserved ctrl, unknown mods, non-string fallback, and formatter contract.
* fix(cli): shape-safe voice config read + graceful super/win fallback
Round-11 Copilot review on #19835.
Two remaining cross-runtime gaps:
1. load_config().get('voice', {}) still assumed voice was a dict, so a hand-edited voice: true / voice: cmd+b at the top level raised AttributeError before the voice UI could start. Added voice_record_key_from_config(cfg) to hermes_cli/voice.py that isinstance-guards both the root and the voice subkey. All three cli.py read sites (_enable_voice_mode hint, _show_voice_status, PTT binding) now use it.
2. The CLI normalizer previously passed super+/win+/windows+ through unrewritten so prompt_toolkit would reject them loudly at startup — but that crash was a worse UX than a silent fallback. Normalizer now returns c-b for those spellings, and the PTT binding site logs a warning so users see why their TUI-only shortcut isn't binding in the CLI.
Coverage: 34/34 in tests/hermes_cli/test_voice_wrapper.py (5 new cases for voice_record_key_from_config + malformed-root + malformed-voice + extractor/normalizer composition).
* fix(cli): self-audit cleanup — remaining voice-config shape safety + doc drift
Self-review of the voice.record_key change set turned up four remaining items Copilot would very likely flag next round:
1. cli.py _voice_start_continuous still read load_config().get('voice', {}).get('silence_threshold') without an isinstance guard, so a hand-edited voice: true / voice: cmd+b (non-dict) raised AttributeError on VAD recording start. Shape-safe coerce the voice dict and numeric-guard silence_threshold/silence_duration.
2. cli.py _enable_voice_mode's auto_tts check had the same bug — fixed with the same isinstance guard.
3. hermes_cli/voice.py module comment on _VOICE_MOD_ALIASES still said super/win/windows 'pass through unchanged and prompt_toolkit's add() call loudly rejects them at startup'. Round 11 changed the normalizer to silently fall back to c-b with a warning at the binding site; updated the comment to match.
4. ui-tui/src/lib/platform.ts header comment had the same stale 'CLI will loudly reject them at startup' claim; updated to 'falls back to the documented default and logs a warning'.
No behavior change on the code paths already covered by test_voice_wrapper.py; the two cli.py fixes are defensive against malformed YAML that previous rounds already hardened in tui_gateway/server.py but missed in the classic CLI.
* fix(cli,tui): round-12 Copilot review — alt-collide on mac, bool-in-int guards, voice UI hardcodes, mtime-reload test
Five round-12 Copilot review items on #19835:
1. platform.ts: hermes-ink reports Alt as key.meta on many terminals; isActionMod on darwin accepts key.meta as the action modifier. So alt+c/d/l get claimed by isCopyShortcut / isAction('d')/'l') before the voice check. Reject those configs at parse time on macOS only (non-mac keeps them usable).
2. cli.py: four remaining hardcoded 'Ctrl+B' sites in voice-facing UI (_get_voice_status_fragments status bar, _voice_start_recording hints, _get_placeholder composer text) were still lying about non-default configs. Added self._voice_record_key_label() shared helper and wired it into all three sites.
3. server.py + cli.py: bool is a subclass of int, so isinstance(silence_threshold, (int, float)) accepted True/False from malformed YAML and forwarded 1/0 to the VAD engine. Exclude bool explicitly so boolean typos fall back to the documented 200 / 3.0 defaults.
4. useConfigSync.ts: extracted the config.get-full fetch+apply body into a shared hydrateFullConfig() helper. Both the initial hydration and mtime-reload paths now use it, so the polling/RPC wiring is exercised by direct unit tests (4 new cases: fresh apply, reapply on new value, transient RPC failure preserves cache, back-compat without voice setter).
5. Added alt+{c,d,l} rejection regressions on darwin + allow on linux, and bool-leak regressions for both silence_threshold and silence_duration in tests/test_tui_gateway_server.py.
Suite: 602/602 TUI vitest, 38/38 backend voice tests, typecheck + lints clean.
* fix(cli): cache voice record-key label at binding time + status-bar coverage
Round-13 Copilot review on #19835.
_voice_record_key_label() was reading live config on every render, which caused two problems:
1. prompt_toolkit registers the push-to-talk binding once at session start (@kb.add(_voice_key)); the binding does NOT re-read config. Editing voice.record_key mid-session would switch the status-bar / placeholder / recording-hint label to the new shortcut while the actual keybinding stayed on the startup chord — reintroducing the display/binding drift this whole PR is fighting.
2. Hot render path: during recording the UI is invalidated every 150ms, so re-loading + deep-merging config on every call added avoidable UI overhead.
Fix: cache the label at the same site that registers the prompt_toolkit binding via new set_voice_record_key_cache(raw_key). _voice_record_key_label() now just returns the cached value (falls back to 'Ctrl+B' before startup). Status/placeholder/hint are always in sync with the live binding; no config reload per render.
Also added 4 regression cases to tests/cli/test_cli_status_bar.py: configured ctrl+<letter> renders in both wide and compact status bars, configured named key (ctrl+space) renders in the recording hint, pre-startup absent cache falls back to Ctrl+B, and malformed configs (bool True) fall through the formatter to Ctrl+B.
Suite: 60/60 test_cli_status_bar + test_voice_wrapper, typecheck + lints clean.
* fix(cli): route /voice on + /voice status through startup-pinned label; mac alt+cdl parity
Round-14 Copilot review on #19835. All three comments legit:
1. _enable_voice_mode still formatted label from live load_config() — mid-session config edit would make /voice on announce the new shortcut while the prompt_toolkit binding stayed the startup chord. Use self._voice_record_key_label() (cached at binding time, round-13) so /voice on cannot drift from the live binding.
2. _show_voice_status had the same bug — /voice status reported live config instead of the pinned startup binding. Fixed the same way.
3. CLI normalizer accepted alt+c/alt+d/alt+l even though the TUI parser rejects them on macOS (Copilot round-12 — hermes-ink reports Alt as key.meta, isActionMod on darwin accepts it, collides with isCopyShortcut / isAction). Added _VOICE_RESERVED_ALT_CHARS_MAC = {c,d,l} gated to sys.platform == 'darwin' so a shared config like option+c falls back to c-b on both runtimes on macOS; non-mac still binds a-c.
Coverage: 4 new tests in test_voice_wrapper.py covering mac alt+cdl rejection, linux alt+cdl allowed, option/opt alias forms, and mac-specific exclusions for other alt letters. 62/62 in voice wrapper + status bar suites.
---------
Co-authored-by: Tranquil-Flow <tranquil_flow@protonmail.com>
Co-authored-by: asheriif <ahmedsherif95@gmail.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
The Ink TUI (\`hermes --tui\` + dashboard \`/chat\`) had no wiring for the
background self-improvement review. When the review fired and patched
a skill or saved a memory entry, the change landed but the user had
no visual indication it happened — only the CLI had a print surface
for the '💾 Self-improvement review: …' line.
Changes:
- tui_gateway/server.py: in _init_session, attach
agent.background_review_callback to an _emit('review.summary',
sid, {text}) closure. Wrapped in try/except so agents with locked
attribute slots don't break session startup.
- ui-tui/src/app/createGatewayEventHandler.ts: handle 'review.summary'
by routing ev.payload.text through sys(…), matching the existing
'background.complete' pattern. Empty / whitespace payloads are
ignored so the transcript never gets a blank system line.
- ui-tui/src/gatewayTypes.ts: extend the GatewayEvent discriminated
union with { type: 'review.summary', payload?: { text?: string } }.
Gateway platforms (Telegram, Discord, Slack, …) already route the
review summary via background_review_callback → post-delivery queue
in gateway/run.py, so they pick up the new 'Self-improvement review:'
prefix from the companion run_agent change with no platform edits.
Tests:
- tests/tui_gateway/test_review_summary_callback.py (Python, 2 tests):
_init_session attaches a callback that emits the right event; the
callback path survives agents that can't accept the attribute.
- ui-tui/src/__tests__/createGatewayEventHandler.test.ts (vitest, 2
new cases): review.summary events feed sys(...) with the full text;
empty / missing payloads are no-ops.
- TypeScript type-check passes.
- tui_gateway suite: 64/64 pass.
Reset sticky mouse/focus/paste terminal modes before the TUI starts and during graceful shutdown paths so stale tab state from prior crashes cannot poison the next session.
* fix(tui): offload manual compaction RPC
Route TUI session compression through the existing long-handler pool so slow compaction does not block other gateway RPCs.
* fix(tui): show compaction progress immediately
Print a local status line before the compress RPC starts so slow manual compaction does not look like a no-op.
* feat(tui): rich /compress feedback parity with CLI
Show pre-compaction message count and rough token estimate immediately, emit a status update so the bottom bar reflects ongoing compaction, and report a multi-line summary (headline + token delta + optional note) using the shared summarize_manual_compression helper.
* fix(tui): show live compaction estimate in transcript
Mirror compression progress status into the transcript so users see the backend message count and token estimate while /compress is still running.
* fix(tui): single live compaction line with spinner glyph
Drop the redundant local "compressing context..." placeholder and prefix the live backend status line with a braille spinner glyph so /compress reads as a single in-progress row.
* fix(tui): address review nits on /compress feedback
Reuse the precomputed token estimate inside _compress_session_history so the gateway does not redo the O(n) work while holding history_lock, keep the status bar pinned during long manual compactions instead of auto-restoring after 4s, and drop the redundant noop bullet that doubled with the system role glyph.
* fix(tui): release history_lock during compaction LLM call
Move the snapshot/commit pattern into _compress_session_history so the lock is held only across the in-memory bookkeeping, not during agent._compress_context. Also emit a final neutral status update from session.compress so the pinned compressing indicator clears even on errors.
* fix(tui): rebuild prompt cleanly + sync session_key after compress
Pass system_message=None so AIAgent._compress_context rebuilds the system prompt without nesting the cached identity block. Reuse the handler's pre-snapshotted history inside _compress_session_history to avoid a second O(n) copy under the lock. After compaction, when AIAgent._compress_context rotates session_id, sync the gateway session_key, migrate approval notify + yolo state, restart the slash worker, and clear the stale pending title. Mirrors HermesCLI._manual_compress.
* Avoid /compress lock re-entry in slash side effects.
Stop pre-locking history before _compress_session_history in slash command mirroring, keep session-key sync parity with manual compression, and add a regression test that asserts /compress is invoked without holding history_lock.
* fix(tui): word-wrap composer input
Wrap composer input at word boundaries and anchor the good-vibes heart to the full composer row.
* test(tui): cover composer word wrap edge
Add regression coverage for moving the next word instead of splitting it at the composer edge.
Classic CLI exposes ``/reload`` (re-reads ~/.hermes/.env into
``os.environ`` via ``hermes_cli.config.reload_env``) so newly added API
keys take effect without restarting the session. The TUI was missing
the parity command, so users had to Ctrl+C out and ``hermes --tui``
again whenever they added or rotated a credential.
Three small wires:
* New ``reload.env`` JSON-RPC method in ``tui_gateway/server.py`` that
delegates to ``hermes_cli.config.reload_env`` and returns the count
of vars updated.
* New ``/reload`` slash command in ``ui-tui/src/app/slash/commands/ops.ts``
matching the existing ``/reload-mcp`` pattern (native RPC, no slash
worker).
* Drop ``cli_only=True`` from the ``reload`` ``CommandDef`` in
``hermes_cli/commands.py`` so help/menus surface it in the TUI too.
``reload_env`` itself is environment-agnostic.
Same caveat as classic CLI: the *currently constructed* agent's
credential pool / provider routing does not auto-rebuild. Users who
want a brand-new credential resolution should follow with ``/new``.
Tests:
* New ``test_reload_env_rpc_calls_hermes_cli_reload_env`` confirms
RPC delegates and reports the count.
* New ``test_reload_env_rpc_surfaces_errors`` confirms exceptions are
rendered as JSON-RPC errors.
* ``createSlashHandler.test.ts`` slash-parity matrix extended with
``['/reload', 'reload.env', {}]`` so we can't regress the routing.
Validation:
scripts/run_tests.sh tests/test_tui_gateway_server.py — 92/92.
scripts/run_tests.sh tests/hermes_cli/test_commands.py — 128/128.
cd ui-tui && npm run type-check — clean; npm test --run — 390/390.
Fixes from Copilot's two passes on PR #17238:
* Validate parsed URL once: reject missing host, invalid port, and
unsupported scheme up front so malformed inputs (e.g. http://:9222
or http://localhost:abc) don't fall through to a generic 5031.
* Tighten _is_default_local_cdp to require a discovery-style path so
ws://127.0.0.1:9222/devtools/browser/<id> is not collapsed to bare
http://127.0.0.1:9222 (which would lose the path and break the
connect).
* Move browser.manage into _LONG_HANDLERS so the up-to-10s
launch-and-retry loop runs on the RPC pool instead of blocking the
main dispatcher.
* try_launch_chrome_debug uses Windows-appropriate detach kwargs
(creationflags=DETACHED_PROCESS|CREATE_NEW_PROCESS_GROUP) instead
of POSIX-only start_new_session=True.
* manual_chrome_debug_command uses subprocess.list2cmdline on
Windows so the printed instruction is cmd.exe-compatible.
* Mirror host/port validation in cli.py /browser connect so the
classic CLI never persists an invalid BROWSER_CDP_URL.
Split browser.manage into a small dispatcher with named connect/disconnect
helpers, fold _http_ok / _probe_urls / _normalize_cdp_url out of the nested
probe loop, collapse the failure-message scaffolding, and DRY the chrome
candidate path tables. Behaviour and event shape unchanged.
Emit browser.progress JSON-RPC notifications during the connect work and render them in the TUI as system transcript lines, so users see the same step-by-step status the base CLI prints instead of nothing for ~1m followed by a final result.
Return CLI-style browser connect status messages from the gateway and render them in the TUI so local Chrome launch attempts are visible instead of ending in a silent delayed failure.
Share Chrome CDP launch helpers between the classic CLI and TUI so default /browser connect uses loopback consistently, retries local Chrome launch, and reports a copyable manual-start command instead of claiming a dead connection.
Keep the latest prompt sticky while the viewport is in live assistant output beyond history, and clear stale sticky state at the real bottom using fresh scroll height.
* fix(tui): honor documented mouse_tracking config key
The TUI runtime was reading display.tui_mouse while docs and user-facing
examples pointed users at display.mouse_tracking. That made persistent
mouse-disable config look like a no-op for users trying to restore native
terminal selection/copy behavior on Linux/SSH/tmux terminals.
Use display.mouse_tracking as the canonical key, keep display.tui_mouse as
a legacy fallback, and have /mouse write the documented key. Both gateway
config.get and client-side config sync now share the same precedence: the
canonical key wins, then the legacy key, then default on.
* review(copilot): align mouse tracking config coercion
- Load gateway config once before deriving display.mouse_tracking state.
- Use key-presence precedence on the TUI client too, so canonical
mouse_tracking wins over legacy tui_mouse even when the value is null.
- Treat numeric 0 as disabled on both gateway and client, matching the
existing string "0" handling.
- Widen ConfigDisplayConfig mouse fields because config.get full returns raw
YAML, not normalized booleans.
This PR groups the TUI fixes that restore macOS Terminal usability and clean up the theme/composer regressions:
- copy transcript selections on macOS drag-release so Terminal.app users can copy while mouse tracking is enabled
- copy composer selections on macOS drag-release; composer selection is internal to TextInput and does not use the global Ink selection bus
- keep IDE Cmd+C forwarding setup macOS-only, and make keybinding conflict checks respect simple when-clause overlap/negation
- force truecolor before chalk initializes (unless NO_COLOR / FORCE_COLOR / HERMES_TUI_TRUECOLOR opt-outs apply) so the default banner keeps its gold/amber/bronze gradient in Terminal.app
- move TUI surfaces onto semantic theme tokens and preserve skin prompt symbols as bare tokens with renderer-owned spacing
- render focused placeholders as dim hint text in TTY mode instead of inverse/selected-looking synthetic cursor text
* feat(tui): pluggable busy-indicator styles (kaomoji/emoji/unicode/ascii)
The status-bar `FaceTicker` rotated through wide-and-variable kaomoji
glyphs (`(｡•́︿•̀｡)`, `( ͡° ͜ʖ ͡°)`, …) every 2.5s. Real display widths range
from ~5 to ~16 columns, so the rest of the bar (cwd, ctx %, voice,
bg counter) shifted on every cycle. Padding the verb alone (#17116)
helped but didn't address the dominant jitter source — the glyph
itself.
Add four indicator styles, configurable + hot-swappable:
* `kaomoji` (default — preserves the existing vibe; verb is now
pad-stable so the only width churn left is the kaomoji itself).
* `emoji` — single 2-col emoji frame (`⚕ 🌀🤔✨🍵🔮`).
* `unicode` — `unicode-animations` braille spinner (1-col, smooth).
* `ascii` — `| / - \` (1-col, max compat).
Wires:
* `display.tui_status_indicator` in `DEFAULT_CONFIG` (default
`kaomoji`).
* New JSON-RPC `config.set/get indicator` keys, narrow allow-list.
* `applyDisplay` reads the field and patches `UiState.indicatorStyle`,
so the existing `mtime` poll picks up `~/.hermes/config.yaml` edits
within ~5s without a TUI restart.
* `/indicator [style]` slash command (alias `/indicator-style`,
subcommand completion `kaomoji|emoji|unicode|ascii`). Bare form
shows the current style; setter fires `config.set` and
optimistically `patchUiState({ indicatorStyle })` so the live TUI
swaps immediately, matching the `/skin` UX.
* `CommandDef("indicator", ..., subcommands=...)` so classic CLI
autocomplete + TUI `complete.slash` both surface it.
* `FaceTicker` decouples spinner cadence from verb cadence — the
glyph runs at the spinner's authored interval (or `FACE_TICK_MS`
for kaomoji), the verb stays on the original 2.5s cycle, and both
re-arm cleanly when style changes.
Tests:
* `normalizeIndicatorStyle` rejects unknown / non-string input.
* `applyDisplay → tui_status_indicator` covers fan-out + fallback.
* `/indicator <style>` hot-swaps `UiState.indicatorStyle` after a
successful `config.set`.
* `/indicator sparkle` rejects with the usage hint and never hits
the gateway.
* Slash-parity matrix gets `'/indicator'` → `config.get`.
Validation:
cd ui-tui && npm run type-check — clean; npm test --run — 398/398.
scripts/run_tests.sh tests/test_tui_gateway_server.py
tests/hermes_cli/test_commands.py — 220/220.
* chore(tui): drop /indicator-style alias to declutter autocomplete
* fix(tui): drop verb-width pad — /indicator handles glyph jitter directly
* fix(tui): unicode indicator style hides the verb (cleanest option)
* refactor(tui): single source of truth for INDICATOR_STYLES; cleaner error format
Round 1 Copilot review on PR #17150:
- Exported `INDICATOR_STYLES` const tuple from `interfaces.ts`;
`IndicatorStyle` union type is derived from it. `useConfigSync`
builds its validation Set from the tuple, and `session.ts` uses it
for both the usage hint and the runtime allow-list — adding/removing
a style now touches one line.
- Backend `config.set indicator` error message: switched
`sorted(allowed)` list repr to `pick one of ascii|emoji|kaomoji|unicode`
(matches the TUI usage hint), and reports the normalized `raw`
instead of the original `value`. Backend allowed tuple now has a
comment pointing back at `INDICATOR_STYLES` so the two stay aligned.
Note: kept the verb portion unpadded per design intent — fixed-width
padding was the exact UX the `/indicator` command was added to remove.
Stable width comes from the glyph; verbs cycling is part of the kawaii
aesthetic. Reply on the verb thread will explain.
* fix(tui): drop type collapse + gate verb timer + DEFAULT_INDICATOR_STYLE
Round 2 Copilot review on PR #17150:
- `tui_status_indicator?: 'ascii' | ... | string` collapses to `string`
in TS — consumers got no narrowing. Documented as plain `string` with
a comment about runtime validation via `normalizeIndicatorStyle`.
- `FaceTicker` always started a 2.5s verb interval, even for the
`unicode` style which hides the verb entirely. Now gated on
`showVerb` from `renderIndicator` — `unicode` stays calm.
Pre-emptive self-review (avoid round 3):
- Three call sites duplicated the literal `'kaomoji'` default
(uiStore, normalizeIndicatorStyle, slash command). Added
`DEFAULT_INDICATOR_STYLE` to interfaces.ts and threaded it through
so changing the default touches one line.
* fix(tui-gateway): normalize config.get indicator output to match TUI render
Round 4 Copilot review on PR #17150: `config.get` for `indicator`
returned the raw `display.tui_status_indicator` value without
validation, so a hand-edited config.yaml with stray casing or an
unknown style would leave `/indicator` printing one thing while
the TUI rendered the kaomoji default (frontend's
`normalizeIndicatorStyle` does this normalization on receive).
Lifted the allow-list to module scope as `_INDICATOR_STYLES` /
`_INDICATOR_DEFAULT`, reused by both `config.set` and `config.get`.
Comment notes the alignment with `INDICATOR_STYLES` /
`DEFAULT_INDICATOR_STYLE` in interfaces.ts so adding/removing a
style is a one-line change on each end.
Tests cover: known value verbatim, casing/whitespace normalize,
unknown→default, unset→default.
* fix(tui-gateway): preserve falsy-input diagnostics in config.set indicator error
Round 5 Copilot review on PR #17150: `raw = str(value or "").strip().lower()`
collapsed any falsy non-string (`0`, `False`, `[]`) to empty string,
so the error message read `unknown indicator: ` with nothing after —
losing the original input.
Switched to `("" if value is None else str(value)).strip().lower()`
so only `None` (the genuine 'no value' case) becomes blank. Used
`{raw!r}` in the error so the diagnostic is unambiguous (`'0'` vs `0`).
Tests:
- known-value happy path (`'EMOJI'` → `'emoji'`)
- falsy non-string inputs (`0` / `False` / `[]`) surface meaningfully
- `None` keeps the blank-repr error
* feat(tui): expand light-terminal auto-detection (HERMES_TUI_THEME, BG hex)
Modern terminals (Ghostty, Warp, iTerm2) don't set COLORFGBG, so the
auto-light path was effectively COLORFGBG-only and silently broken for
many users. Two pragmatic additions, both opt-in, plus a clearer
priority chain:
1. **`HERMES_TUI_THEME=light|dark`** as a symmetric explicit override.
The existing `HERMES_TUI_LIGHT` is fine but reads as boolean noise;
a named theme env var matches `display.skin` muscle memory.
2. **`HERMES_TUI_BACKGROUND` hex/rgb hint.** Lets advanced users
(or a future OSC11 query helper that caches the answer) state a
ground-truth background colour. Decoded to Rec. 709 luma; ≥ 0.6
counts as light.
Priority order is now fully ordered and explainable:
1. `HERMES_TUI_LIGHT` (1/0/true/false/on/off).
2. `HERMES_TUI_THEME=light|dark`.
3. `HERMES_TUI_BACKGROUND` luminance.
4. `COLORFGBG` last field — light slots 7/15 → light, 0–15 → dark
(authoritative when set, so the new TERM_PROGRAM path can never
stomp on a terminal that already volunteered a dark answer).
5. `TERM_PROGRAM` allow-list — empty by default. The slot is left
in place because folks asked for it but populating it risks
wrongly flipping users on Apple_Terminal / iTerm2 dark profiles
to light. Easy to add per terminal once we have signal.
Tests: 5 new cases in `theme.test.ts` covering theme env, background
hex (3- and 6-char), invalid hex falling through, and COLORFGBG taking
precedence over the future allow-list.
Validation: `npm run type-check` clean, `npm test --run` 392/392.
* review(copilot): tighten theme detection comments + drop unnecessary cast
* review(copilot): strict hex regex so partial garbage doesn't slip into luminance
* test(tui): make TERM_PROGRAM allow-list injectable so precedence is provable
Copilot review on PR #17113: `LIGHT_DEFAULT_TERM_PROGRAMS` is empty
in production, so the prior assertion would have passed even if
`detectLightMode` ignored `COLORFGBG` entirely. That defeats the
test's purpose.
`detectLightMode` now takes the allow-list as an optional second
argument (defaults to the production set). The test injects a set
containing `Apple_Terminal`, asserts the allow-list alone WOULD
return light, then asserts `COLORFGBG: '15;0'` overrides it — the
precedence rule is now exercised, not assumed.
* fix(tui): COLORFGBG empty-trailing-field falls through; isolate DEFAULT_THEME tests
Round 2 Copilot review on PR #17113:
1. `Number(colorfgbg.split(';').at(-1))` returns 0 for an empty trailing
field (e.g. `COLORFGBG='15;'` → bg===0), which would have looked
like an authoritative dark slot and incorrectly blocked the
TERM_PROGRAM allow-list. Added a `/^\d+$/` guard before coercion;
non-numeric trailing fields now fall through.
2. Fixed the misleading '0–6 / 8–15 ranges are dark' comment — the
block returns true for bg===15, so the range is actually 0–6 / 8–14.
3. `DEFAULT_THEME` is computed from `process.env` at module-load.
A developer shell with `HERMES_TUI_THEME=light` (or a bright
`HERMES_TUI_BACKGROUND`) would flip it and break local tests.
The DEFAULT_THEME describe blocks now sterilize the relevant env
vars + dynamically import theme.ts (vi.resetModules pattern from
platform.test.ts). fromSkin tests compare against DARK_THEME
directly to decouple them from ambient env.
* test(tui): isolate ALL env-coupled theme symbols, not just DEFAULT_THEME
Round 3 Copilot review on PR #17113: the static top-level imports of
`fromSkin`, `DARK_THEME`, `LIGHT_THEME` evaluated theme.ts before
`importThemeWithCleanEnv` had a chance to clean the env. Because
`fromSkin` closes over `DEFAULT_THEME`, an ambient `HERMES_TUI_THEME=light`
or bright `HERMES_TUI_BACKGROUND` would still flip the base palette
and cause local-only failures.
Removed the static import entirely. Every test now obtains its theme
symbols via `importThemeWithCleanEnv`, including `detectLightMode`
(for consistency, even though it takes env as a parameter).
`fromSkin` tests assert against the cleaned `DEFAULT_THEME` from the
same dynamic import — preserves the actual contract (skins extend the
ambient base palette) without coupling the test to dev-shell state.
Verified by running with HERMES_TUI_THEME=light + HERMES_TUI_BACKGROUND=#ffffff:
all 20 theme tests still pass.
Self-review (avoid round 4):
- Audited other test files importing DEFAULT_THEME (syntax.test.ts,
streamingMarkdown.test.ts, constants.test.ts) — all just pass it as
a parameter or assert palette property existence (works on both
light + dark), so no env coupling there.
* fix(tui): honor display.busy_input_mode in TUI v2
The TUI v2 frontend hard-coded `composerActions.enqueue(full)` whenever
`ui.busy` was true. The classic CLI and gateway adapters honor the
`display.busy_input_mode` config key (`interrupt` | `queue` | `steer`),
but Ink ignored it — sending a message during a long-running turn always
landed in the queue regardless of config. The config default is already
`interrupt` (hermes_cli/config.py), so users who explicitly opted into
that experience were silently stuck on the legacy queue path.
This wires the value through the existing config-sync surface:
* `applyDisplay` now reads `display.busy_input_mode`, defaults to
`interrupt` (matching `_load_busy_input_mode` in tui_gateway), and
drops it into a new `UiState.busyInputMode` field.
* `dispatchSubmission` and the queue-edit fall-through call a shared
`handleBusyInput` helper that branches on the mode:
* `queue` — legacy behavior, append to the queue.
* `steer` — call `session.steer`; on rejection, fall back to
queue with a sys note.
* `interrupt` — `turnController.interruptTurn(...)` then `send()`,
so the new prompt actually moves.
* Mtime polling in `useConfigSync` already re-applies `config.full`, so
flipping `display.busy_input_mode` in `~/.hermes/config.yaml` takes
effect on the next 5s tick without restarting the TUI.
Tests:
* `applyDisplay → busy_input_mode` covers normalization + UiState fan-out.
* `normalizeBusyInputMode` mirrors the Python side's allow-list.
Validation:
* `npm run type-check` (in `ui-tui/`) — clean.
* `npm test --run` (in `ui-tui/`) — 394/394.
* review(copilot): narrow busy_input_mode type, preserve queue order on steer fallback
* review(copilot): clarify handleBusyInput comment (option, not return value)
* fix(tui): default busy_input_mode to queue in TUI (CLI keeps interrupt)
In a full-screen TUI users typically author the next prompt while the
agent is still streaming, so an unintended interrupt loses in-flight
typing. TUI fallback now defaults to `queue`; CLI / messaging
adapters keep `interrupt` as the framework default.
Override per-config via `display.busy_input_mode: interrupt` (or
`steer`) — the normalize/wire path is unchanged, only the missing-
value branch differs from the Python default.
uiStore initial value also flipped to `queue` so first-frame render
before `config.full` lands matches the eventual normalized value.
`turnController.recordMessageComplete` and `recordMessageDelta` both
prioritised `payload.rendered` over `payload.text`. `payload.rendered`
is the Rich-Console output `tui_gateway` builds for terminals that
can't render markdown themselves; the TUI already renders markdown via
`<Md>`. Two real bugs follow:
1. **Final answer garbled when `display.final_response_markdown: render`
is set** (#16391). Raw ANSI escape sequences pass through into the
React tree and the user sees overlapping coloured text instead of
their answer.
2. **Streaming silently drops content.** Per-delta `rendered` is an
*incremental* Rich fragment. The previous code did
`this.bufRef = rendered ?? this.bufRef + text`, which on every tick
replaced the whole accumulated buffer with the latest mid-sequence
ANSI fragment. Long replies arrived truncated and looked
half-painted — easy to miss as "model is being terse" instead of a
client bug.
Fix:
* `recordMessageComplete` now prefers `payload.text`, falling back to
`payload.rendered` only when the gateway elected not to send any.
* `recordMessageDelta` always accumulates `text`; `rendered` is ignored
on the streaming path entirely (Ink does its own markdown render via
`<Md>` / `streamingMarkdown.tsx`).
Tests:
* `prefers raw text over Rich-rendered ANSI on message.complete` —
the assistant message reflects raw markdown, not ANSI.
* `falls back to payload.rendered when text is missing` — preserves
the legacy "no `text`, only ANSI" path used by some adapters.
* `always accumulates raw text in message.delta and ignores rendered` —
pre-fix code would have made this assertion fail because each delta
overwrote the buffer.
Validation: `npm run type-check` clean, `npm test --run` 392/392 pass.
* feat(tui): opt-in auto-resume of the most recent session
`hermes --tui` always forges a fresh session at startup unless the user
sets `HERMES_TUI_RESUME=<id>`. Disconnects, terminal-window crashes,
and accidental Ctrl+D therefore lose every piece of in-flight context
even though `state.db` still has the full history a `/resume` away.
Add an opt-in path that mirrors classic CLI's `hermes -c` muscle
memory: when `display.tui_auto_resume_recent: true` is set in
`~/.hermes/config.yaml`, the TUI looks up the most recent human-facing
session and resumes it instead of starting fresh. Default off so
existing users aren't surprised; explicit `HERMES_TUI_RESUME` always
wins.
Wires:
* New `session.most_recent` JSON-RPC in `tui_gateway/server.py` that
returns the first non-`tool` row from `list_sessions_rich`, or
`{"session_id": null}` when none. Uses the same deny-list as
`session.list` so sub-agent rows can't sneak in.
* `createGatewayEventHandler.handleReady` re-ordered: explicit
`STARTUP_RESUME_ID` first (unchanged), then conditional auto-resume
via `config.get full → display.tui_auto_resume_recent`, then the
legacy `newSession()` fallback. Failures of either RPC fall back
to `newSession()` so the path is always finite.
* Default `display.tui_auto_resume_recent: False` added to
`DEFAULT_CONFIG` in `hermes_cli/config.py` (no `_config_version`
bump per AGENTS.md — deep-merge handles the additive key).
Tests:
* 4 new vitest cases in `createGatewayEventHandler.test.ts` cover
every gate-and-fallback combination (env wins, config off, config
on with hit, config on with miss).
* 3 new pytest cases for `session.most_recent` (denied row skip,
tool-only → null, db-unavailable → null).
Validation:
scripts/run_tests.sh tests/test_tui_gateway_server.py — 93/93.
cd ui-tui && npm run type-check — clean; npm test --run — 393/393.
* review(copilot): fold session.most_recent errors into null + extend ConfigDisplayConfig
* review(copilot): cover RPC-rejection fallbacks in auto-resume tests
* fix(tui): drop stale stream events after ctrl-c interrupt
Once interruptTurn() flips this.interrupted, only recordMessageDelta
short-circuited. recordReasoningDelta/Available, recordToolStart/
Progress/Complete, and recordInlineDiffToolComplete kept populating
turnState until the python loop reached its next _interrupt_requested
check (~1s on busy turns), making it look like ctrl-c was ignored
while late "thinking" + tool calls kept landing in the UI.
Add the same interrupted guard to every stream-side recorder, and
clear the flag at startMessage() so the next turn isn't suppressed
if the previous turn never delivered message.complete.
* fix(tui): guard recordTodos against post-interrupt mutation; fake-timers in test
Copilot review on PR #16706:
1. `recordToolStart` is interruption-guarded, but `tool.start`
handler also calls `recordTodos(payload.todos)` first — so a
late tool.start carrying todos could still mutate `turnState.todos`
after Ctrl-C, leaving ghost rows in the panel. Adds the same
`if (this.interrupted) return` early-exit to `recordTodos` so
*all* tool.start side-effects are dropped post-interrupt.
2. The interrupt test was leaking a real `setTimeout` (interrupt
cooldown) across test files, which could fire later and mutate
uiStore from the wrong test context. Wraps the test in
`vi.useFakeTimers()` + `vi.runAllTimers()` and restores real
timers in finally.
3. Extends the same test with a todos payload on the post-interrupt
tool.start so we have explicit regression coverage for #1.
* fix(tui): guard pushTrail post-interrupt; harden interrupt-test cleanup
Round 2 Copilot review on PR #16706:
1. `tool.generating` events route through `pushTrail`, which was not
interruption-guarded — late events could still write 'drafting …'
into `turnTrail` after Ctrl-C, leaving a stale shimmer in the UI.
Adds the same `if (this.interrupted) return` early-exit.
2. Test cleanup moved `vi.runAllTimers()` into `finally` (before
`vi.useRealTimers()`) so a mid-test assertion failure can't leak
the interrupt-cooldown setTimeout across other test files.
3. Replaced the misleading 'pre-interrupt todos … expected to be
cleared by the interrupt cycle' comment with an accurate one
reflecting current behaviour (interrupt does NOT clear todos).
4. Added an explicit assertion that a post-interrupt `tool.generating`
event does not extend `turnTrail` — regression coverage for #1.
* fix(tui): append gateway stderr tail to start_timeout activity
`gateway.start_timeout` previously published only `cwd` + `python`,
which made TUI startup failures hard to disambiguate. The user saw
`gateway startup timed out · /path/to/python /repo · /logs to inspect`
with no signal whether the actual cause was a wrong python interpreter,
a missing dependency, or a config parse failure.
Plumb a 20-line stderr tail through the event so the most useful lines
land directly in the TUI activity feed, capped to the last 8 non-empty
lines for readability:
* `gatewayClient.ts` — collect `getLogTail(20)` when the readyTimer
fires and attach it as `payload.stderr_tail`.
* `gatewayTypes.ts` — extend the `gateway.start_timeout` event union
with the new optional field.
* `createGatewayEventHandler.ts` — emit the trimmed lines after the
existing `gateway startup timed out` activity entry, classified
`error`.
Tests: regression test in `createGatewayEventHandler.test.ts` checks
that `ModuleNotFoundError` / `FileNotFoundError` lines from the tail
land in `getTurnState().activity` so they show up in the UI immediately.
Validation: `npm run type-check` clean, `npm test --run` 390/390.
* review(copilot): filter blanks before slice and cap stderr tail at 120 chars
- Rename `removeAt` → `removeAtInPlace` and document the mutation
contract; the old name read like a non-mutating helper.
- Hotkey table + queue header: use `Ctrl+X` / `Esc` to match the
rest of the UI (was `⌃X` / `esc`).
- Render the queued header as a single template literal so JSX
text-node whitespace can't sneak into the rendered line.
- Make `Esc` while editing beat the `terminal.hasSelection` clear:
the header promises 'Esc cancel', so an active selection
shouldn't silently consume the keystroke.
Today there's no way to remove a queued message — ↑ loads it for edit,
ctrl-K dispatches the head, but a draft you no longer want stays put
forever. ctrl-C just clears the composer and exits edit mode without
touching the queue.
Two new bindings, both gated on queueEditIdx !== null so they're
inert when the user isn't pointing at a queue item:
- ctrl-X — delete the queue item being edited, clear composer, exit
edit mode. "cut" matches the mental model and doesn't collide with
any existing binding.
- esc — cancel the edit (composer clears, item stays in queue).
Mirrors ctrl-C's existing behavior so muscle memory has two paths.
Header line now reads `queued (3) · editing 2 · ⌃X delete · esc cancel`
when in edit mode, so the affordance is discoverable without /help.
The /help hotkey table also gets a Ctrl+X entry.
ctrl-C is intentionally unchanged: it should never destroy queued
content. Cancel is non-destructive (esc / ctrl-C); only ctrl-X
removes the item.
Keep the parity test backed by the real Python command registry while avoiding hard failures in Node-only Vitest environments that cannot import hermes_cli.commands.
Use explicit repaint patch semantics for Ctrl/Cmd+L and narrow the hotkey assertion to the actual +L entry so unrelated descriptions do not cause false failures.
Make Ctrl+L non-destructive by redrawing the current screen state instead of starting a new session, and stop auto-appending --global for typed /model commands so session scope remains the default unless explicitly requested.
Route /browser, /reload-mcp, /rollback, /stop, /fast, and /busy through direct TUI RPC handlers so state changes hit the live gateway session instead of slash-worker fallback. Add TUI session finalize/reset parity hooks (memory commit + plugin boundaries) and parity matrix tests to keep mutating commands off fallback.
