fix(tui): allow transcript scroll + Esc during approval/clarify/confirm prompts (#26414)
When an approval / clarify / confirm overlay was active, the global input
handler in useInputHandlers returned for every key that wasn't Ctrl+C, which
silently disabled transcript scrolling. On long threads the context the
prompt was asking about often lived above the visible viewport, and being
unable to scroll while answering felt like the prompt had locked the UI.
ApprovalPrompt also had no Esc handler at all, so the one obvious 'abort'
key did nothing during a permission prompt and the user had to memorize
Ctrl+C or hunt for the deny number.
Fixes:
- Extract shouldFallThroughForScroll(key) (pure, exported) covering wheel
scrolls, PageUp/PageDown, and Shift+ArrowUp/Down. When a prompt overlay
is up and the pressed key is a scroll input, skip the early return so it
reaches the existing wheel/PageUp/Shift+arrow handlers below. Plain
arrows still drive in-prompt selection — they don't fall through.
- ApprovalPrompt now maps Esc to onChoice('deny'), parity with the global
Ctrl+C cancellation path that already invokes cancelOverlayFromCtrlC()
for approvals. The bottom-of-prompt hint now advertises 'Esc/Ctrl+C deny'.
- Extract approvalAction(ch, key, sel) — pure key-dispatch helper for the
approval prompt, exported so the regression matrix (Esc, numbers, Enter,
arrows, edge clamping, precedence) is testable without mounting Ink.
Tests:
- useInputHandlers.test.ts: 6 cases covering shouldFallThroughForScroll
positives (wheel/PageUp/PageDown/Shift+arrows) and negatives (plain
arrows, bare shift, no scroll key).
- approvalAction.test.ts: 8 cases covering Esc→deny, numeric mapping,
Enter, ↑↓ within bounds, edge clamping, Esc-beats-others precedence,
unrelated keystrokes.
review(tui): route cursorLayout through @hermes/ink wrapAnsi shim (Bun runtime parity)
Copilot caught an important runtime parity gap on PR #27489: the fix
imported the npm wrap-ansi package directly, but Ink's `<Text
wrap="wrap">` uses a runtime-selecting shim
(ui-tui/packages/hermes-ink/src/ink/wrapAnsi.ts) that prefers
Bun.wrapAnsi when running under Bun and falls back to the npm package
elsewhere. So under Bun, Ink would render via Bun.wrapAnsi while
cursorLayout would compute breaks via the npm package — any
disagreement reintroduces the exact cursor-drift symptom the PR is
meant to eliminate.
Fix:
- Export wrapAnsi from @hermes/ink (packages/hermes-ink/src/entry-exports.ts
and packages/hermes-ink/index.d.ts) so the shim is the public surface.
- Switch ui-tui/src/lib/inputMetrics.ts from `import wrapAnsi from
'wrap-ansi' to import { wrapAnsi } from '@hermes/ink'`. Both
renderer (Ink) and cursor layout now traverse the same shim, so
they share the runtime-selected implementation by construction.
- Same swap in textInputWrap.test.ts and cursorDriftRegression.test.ts
— tests now assert parity through the shim, which means under Bun
they actually exercise Bun's implementation instead of asserting a
tautology against the npm package.
- Drop the direct "wrap-ansi": "^9.0.0" from ui-tui/package.json.
@hermes/ink (which IS a declared dep) pulls wrap-ansi in
transitively — that's not a phantom dep because the import path
goes through @hermes/ink's public exports, not through a
hoisting accident.
Verified: 791/791 vitest tests pass. @hermes/ink rebuilt
(dist/entry-exports.js includes wrapAnsi export). TUI bundle
rebuilt clean.
feat(tui): support attaching to an existing gateway (#21978)
* feat(tui): support attaching to an existing gateway
Allow the TUI gateway client to connect via HERMES_TUI_GATEWAY_URL while preserving spawned gateway fallback, and mirror event frames to sidecar feeds so dashboard tool activity remains visible.
* review(copilot): redact attach URLs and gate stale transport exits
Strip query strings (and any user info) from gateway / sidecar URLs before logging or surfacing them in gateway.start_timeout, so attach tokens never leak into the TUI log tail or activity feed. Also gate the spawned-proc and websocket close handlers on transport identity so a stale child or socket cannot clear a freshly-started ready timer or reject newly-issued pending requests during reconnect.
* review(copilot): tighten transport restart and shutdown lifecycle
Reject any in-flight RPCs in resetStartupState so callers do not hang on promises issued to the previous transport when start() swaps a child or socket. Have kill() explicitly reject pending so attach-mode promises drain after an intentional shutdown, and reattach when HERMES_TUI_GATEWAY_URL rotates between requests instead of silently keeping the old session. Fold the spawned child error path through handleTransportExit so a failed spawn clears the startup timer and emits a single exit event. Also null the websocket reference before calling close so the identity guard correctly tags stale close events on real WebSocket timing. Locks the new behaviors in with regression tests for kill, URL rotation, and stale-pending cleanup.
* review(copilot): swallow stray ws connect rejection and isolate test env
Attach a no-op catch handler on the websocket connect promise so an unobserved connect-error / early-close rejection cannot surface as an unhandled promise rejection in Node when no request is currently racing the open. Snapshot HERMES_TUI_GATEWAY_URL / HERMES_TUI_SIDECAR_URL in beforeEach and restore them in afterEach so vitest runs that set those env vars beforehand do not get permanently cleared.
* Potential fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* review(copilot): hoist wire decoder and harden redact fallback
Reuse a single module-level TextDecoder for binary websocket frames so high-frequency attach-mode traffic does not allocate one per message. Strengthen the redactUrl fallback so embedded user:pass@ credentials are also masked when the WHATWG URL parser rejects the input, and pin the new behavior with a regression test that drives a malformed bearer URL through the gateway-stderr publish path.
* Potential fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* review(copilot): force redact fallback path with deterministic fixture
Replace the "%zz" user-info fixture, which WHATWG URL actually accepts in recent Node and silently routed the test back through the structured-URL branch, with a port-99999 fixture that the parser rejects across Node versions. Add a pre-flight expect(() => new URL(fixture)).toThrow() assertion so a future URL-parser change can never silently bypass redactUrl()'s fallback again.
* review(copilot): sanitize websocket constructor failures
Avoid logging raw WebSocket constructor error messages because some implementations include the full input URL, including token-bearing query strings. Log the redacted gateway or sidecar URL with the error class instead, and add regression coverage for constructor-throw paths on both attach and sidecar sockets.
* review(self): restart transport on attach-mode transition
Route runtime HERMES_TUI_GATEWAY_URL changes through start() so switching from spawned-gateway mode to attach mode also tears down the previously spawned Python child instead of leaving it alive. Keep the existing fast-fail behavior for pending RPCs. Also make constructor-failure logging fully generic after the redacted URL, avoiding even implementation-specific error class text in the log tail.
* review(copilot): use websocket wording for attach close errors
When the attached websocket closes, reject pending RPCs with an explicit websocket-closed reason instead of the spawned-process oriented gateway exited wording. Add coverage to ensure close code 1011 surfaces as gateway websocket closed (1011).
---------
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
fix(tui): respect voice.record_key config (supersedes #19028, #19339) (#19835)
* 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.deleteParsedVoiceRecordKey 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>
fix(tui): steady transcript scrollbar (#20917)
* fix(tui): steady transcript scrollbar
Keep the visible scrollbar tied to committed viewport position while virtual history can still prefetch against pending scroll targets, and preserve drag grab offset synchronously for native-feeling scrollbar drags.
* fix(tui): smooth precision wheel scroll
Replace the opt-scroll throttle with frame-sized coalescing so modifier wheel gestures stay line-precise without stepping.
fix(tui): anchor splitReasoning unclosed-tag regex to start of input (#29426)
splitReasoning() strips paired <think>…</think> blocks first, then runs
an unclosed-trailing regex to catch reasoning that hasn't yet streamed its
closer. That second regex was unanchored and greedy:
new RegExp(<${tag}>([\\s\\S]*)$, 'i')
So any literal <think> somewhere in prose — a model quoting the tag, a
code example, or a stream-mid-tag before the closer arrives — consumed
every paragraph after it to EOF. User-visible symptom: "TUI eats last
paragraph of output," both during streaming and on settled turns.
Real reasoning streams always lead the message (that's the only place an
unclosed opener can legitimately appear during streaming). Anchor the
regex to ^\s* so mid-prose mentions of the tag are preserved.
Empirical repro before the fix:
splitReasoning('final answer paragraph one.\n\n<think>internal note\n\nfinal answer paragraph two.')
→ text: 'final answer paragraph one.' ← paragraph two GONE
After:
→ text: 'final answer paragraph one.\n\n<think>internal note\n\nfinal answer paragraph two.'
Updated the existing trailing-unclosed test to lead with <think> (the
real-world shape) and added a regression test pinning the mid-text case.
ui-tui type-check clean, 808/808 vitest pass.
fix(tui): handle timeout/error subagent statuses in /agents (#26687)
Accept delegation timeout/error statuses in the TUI subagent model, normalize unknown status strings defensively, and harden /agents overlay rendering/sorting so unknown statuses cannot crash glyph/color lookup. Add regression tests for live event normalization and disk snapshot replay.
feat(tui): segment turns with rule above non-first user msgs; trim ticker dead space (#21846)
Multi-turn transcripts ran together visually because every user message
got the same vertical rhythm regardless of position. Adds a short ─── in
the border colour above every user message after the first, so each turn
reads as its own block. Height estimator gains a withSeparator flag so
virtual scrolling pre-allocates the extra two rows (rule + top margin)
and avoids a jump on first measurement.
While in the area: the busy-indicator duration was padded with
padStart(7), leaving five visible spaces between · and the digits
(⠋ · 2s) — especially loud under the verb-less unicode style.
Drop the padding entirely (⠋ · 2s); the model label now shifts a few
columns as the duration grows, which is the right trade-off for the
minimal indicator styles. The verb-padding test stays; the
duration-padding test is removed alongside the function it covered.
fix(tui): restore macOS copy behavior and theme polish (#17131)
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
fix(tui): restore macOS copy behavior and theme polish (#17131)
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
fix(tui): keep Ink displayCursor in sync with fast-echo writes so cursor stops drifting (#26717)
* fix(tui): keep Ink displayCursor in sync with fast-echo writes so cursor stops drifting
TextInput's fast-echo bypass writes characters directly to stdout to
avoid waiting on a React re-render for each keystroke. The hardware
cursor advances by text.length cells, but Ink's cached displayCursor
(the basis for the next frame's relative cursor-move preamble in
log-update) stayed unchanged. When ANY unrelated component re-rendered
between the fast-echo write and the deferred composer setCur/setParent
flush — status bar timer, streaming reasoning, etc. — the next frame's
preamble emitted a relative cursor move from a stale parked position
and the hardware cursor parked N cells offset from the actual caret.
Visible symptom: extra whitespace between the just-typed character and
the cursor block, intermittent, worse on long sessions during streaming.
Alt-screen was immune because frames begin with absolute CSI H.
This adds a small API in @hermes/ink:
- Ink.noteExternalCursorAdvance(dx, dy?) — bumps displayCursor if
set, otherwise seeds from frontFrame.cursor so the next preamble's
relative move correctly cancels the external advance. No-op on
alt-screen.
- CursorAdvanceContext + useCursorAdvance() hook to expose it.
TextInput then calls noteCursorAdvance(text.length) after the
fast-echo stdout.write(text) append, and noteCursorAdvance(-1)
after the fast-backspace \b \b sequence.
Tests: 4 new vitest cases pin the API contract (bumps when set, seeds
from frontFrame.cursor when null, alt-screen no-op, zero-delta no-op).
All 751 ui-tui tests pass; tests/test_tui_gateway_server.py (177) pass.
* fix(tui): also advance cursorDeclaration so fast-echo survives deferred React state
Copilot review on PR #26717 flagged a gap in the original fix:
TextInput's fast-echo path defers the React cur state update by
16ms (perf optimization that batches re-renders during heavy typing).
Inside that window, useDeclaredCursor still publishes a target
computed from the PRE-keystroke cur — `cursorLayout(display, cur,
columns). Advancing only displayCursor` would let any unrelated
re-render in that 16ms window run onRender's cursor-park branch with
the stale declaration and visually undo the fast-echo's advance.
The fix is symmetric: noteExternalCursorAdvance now bumps BOTH
displayCursor (the log-update relative-move basis) AND, if non-null,
cursorDeclaration.relativeX/Y (the target the cursor parks at after
every frame). When React finally flushes setCur, useDeclaredCursor
publishes a fresh declaration that supersedes our bumped one — exactly
what we want.
Adds two new vitest cases covering both halves:
- active declaration advances in lock-step with displayCursor
- null declaration stays null (no spurious bump)
All 753 ui-tui tests pass; tests/test_tui_gateway_server.py (177) pass.
Closes review threads:
PRRT_kwDOPRF1G86ChKtD (textInput.tsx:1016 fast-echo append)
PRRT_kwDOPRF1G86ChKtF (textInput.tsx:924 fast-backspace)
PRRT_kwDOPRF1G86ChKtG (ink-cursor-advance.test.ts:57 missing coverage)
* fix(tui): make fast-echo survive TextInput rerenders + alt-screen (Copilot round 2)
Round 2 of PR #26717 review. Three real holes Copilot flagged after the
initial cursorDeclaration bump:
1. alt-screen early-return skipped BOTH halves of the notifier. But the
default TUI wraps the composer in <AlternateScreen> — that IS the
production path. CSI H resets log-update's relative-move basis, but
the alt-screen park branch uses absolute CUP =
rect.x + decl.relativeX, so a stale declaration there still parks
the cursor at the pre-keystroke caret. Fix: skip ONLY the
displayCursor half on alt-screen; still bump cursorDeclaration.
2. TextInput's own rerender could clobber the Ink-level bump. The fast-
echo path defers setCur by 16ms; if a parent state change rerenders
TextInput in that window, the layout effect inside useDeclaredCursor
reads the stale React cur state and re-publishes a declaration at
the OLD column. Fix:
cursorLayout(display, curRef.current, columns) — read the always-
up-to-date ref, not the deferred state. useMemo dropped (compute is
cheap, single-line wrap-text in the common case).
3. Tests bypassed the production wiring. Added two structural tests:
- still advances cursorDeclaration on alt-screen in the Ink-level
suite, asserting displayCursor stays put but the declaration
advances by the delta.
- textInputCursorSourceOfTruth.test.ts pins three structural
invariants: layout reads curRef.current, never the bare cur
state, and the fast-echo stdout.write calls remain paired with
noteCursorAdvance(±N). Source-grep invariants > flaky Ink mount
tests for this kind of regression.
757/757 ui-tui tests pass (+3 over round 1). type-check clean. lint
introduces zero new errors on touched files. tests/test_tui_gateway_server.py
(177) pass.
Closes review threads:
PRRT_kwDOPRF1G86ChOG2 (ink.tsx alt-screen guard)
PRRT_kwDOPRF1G86ChOG9 (textInput.tsx fast-backspace rerender window)
PRRT_kwDOPRF1G86ChOHC (textInput.tsx fast-append rerender window)
PRRT_kwDOPRF1G86ChOHJ (alt-screen test asserts wrong invariant)
PRRT_kwDOPRF1G86ChOHP (missing integration-style coverage)
* fix(tui): reject fast-backspace at soft-wrap boundary (Copilot round 3)
PR #26717 round 3. Copilot caught two real things:
1. \b \b cannot move the terminal cursor onto the previous visual
row across a soft-wrap boundary. When the caret sits at visual
column 0 of a wrapped row (e.g. value 'hello ' at width 6 →
cursorLayout produces (line 1, col 0)), backspace would leave the
physical cursor in place while the logical caret moves up to the
end of the previous visual line. noteCursorAdvance(-1) would then
feed Ink a wrong delta. Fix: canFastBackspaceShape now takes the
composer width and rejects when cursorLayout(value, cursor, columns).column === 0.
The fast path falls through to the normal Ink render, which
correctly lays out the new caret position. The PR-description
inconsistency about alt-screen is fixed in a separate gh pr edit.
Adds 4 new tests in textInputFastEcho.test.ts pinning the rejection at
exact-multiple wrap boundaries plus a positive control inside a
wrapped line and a back-compat case where columns is omitted.
761/761 ui-tui tests pass. type-check / lint clean. 177/177 Python
tests/test_tui_gateway_server.py pass.
Closes review threads:
PRRT_kwDOPRF1G86ChxE5 (textInput.tsx:933 wrap-boundary regression)
* fix(tui): polish doc + tests after Copilot round 4
Three polish points Copilot raised:
1. canFastBackspaceShape doc comment overstated the legacy contract —
said it conservatively rejects potential wrap boundaries when
columns is omitted, but the implementation actually skips the
wrap-boundary check entirely. Reworded to make the legacy behavior
explicit and warn callers not to rely on protection they don't get.
2. ink-cursor-advance.test.ts rationale comment for the
'advances cursorDeclaration in lock-step' case still referenced
the pre-fix cursorLayout(display, cur, columns) expression. Now
accurately describes the current source of truth — curRef.current
in textInput.tsx — and explains the window the bump is bridging.
3. Removed the three __get*ForTest accessors from Ink. The test
file already cast the instance to inspect private state in the
couple of tests that needed declaration mutation; the rest now use
a small peek(ink) helper that does the same cast for reads. No
test-only API surface ships in production.
761/761 ui-tui tests pass. type-check clean. lint introduces zero new
errors on touched files. 177/177 tests/test_tui_gateway_server.py pass.
Closes review threads:
PRRT_kwDOPRF1G86Ch23W (canFastBackspaceShape doc accuracy)
PRRT_kwDOPRF1G86Ch23f (stale test rationale)
PRRT_kwDOPRF1G86Ch23p (test-only API surface in production)
* fix(tui): tighten doc + add dy test coverage (Copilot round 5)
Two polish points from round 5:
1. canFastBackspaceShape doc had two paragraphs that conflicted —
the main 'Additionally rejects when the physical cursor sits at
visual column 0' was stated unconditionally, then the columns-param
paragraph qualified that it only happens when columns is passed.
Reworked into clear 'When supplied / When omitted' branches with a
concrete example value ('hello ' returns true without columns even
though it would be unsafe at width 6). No more inconsistency.
2. Added a test asserting cursorDeclaration.relativeY advances when dy
is non-zero. Existing tests exercised dy on displayCursor only.
Newlines in fast-echoed text don't currently hit the bypass
(canFastAppendShape rejects '\n'), but dy is part of the public
notifier contract and must propagate symmetrically with dx so
future callers get a fully-implemented contract.
762/762 ui-tui tests pass (+1). type-check / lint / build clean.
Closes review threads:
PRRT_kwDOPRF1G86Ch6Sz (doc inconsistency)
PRRT_kwDOPRF1G86Ch6TE (missing dy coverage on declaration)
* fix(tui): doc polish (Copilot round 6)
Four small but valid points:
1. textInputCursorSourceOfTruth.test.ts used bare 'fs'/'path'/'url'
imports; the rest of ui-tui consistently uses the 'node:' prefix
(see src/__tests__/useSessionLifecycle.test.ts, src/lib/editor.test.ts).
Switched to node:fs / node:path / node:url to match convention.
2. CursorAdvanceContext.ts type-level doc described only displayCursor.
The notifier intentionally also mutates the active cursorDeclaration
and that's the only part that matters on alt-screen. Reworked the
doc into a two-part 'updates both' summary with the alt-screen
asymmetry called out explicitly.
3. use-cursor-advance.ts hook doc had the same problem. Same fix —
document both pieces of state, both screen modes.
4. App.tsx onCursorAdvance prop comment was incomplete. Same fix —
describe both state updates and the screen-mode asymmetry.
No behavior change. 762/762 ui-tui tests pass. type-check / lint /
build clean.
Closes review threads (auto-resolved on PR but valid critiques):
PRRT_kwDOPRF1G86Ch926 (node: prefix on built-in imports)
PRRT_kwDOPRF1G86Ch92_ (use-cursor-advance.ts doc)
PRRT_kwDOPRF1G86Ch93H (CursorAdvanceContext.ts type doc)
PRRT_kwDOPRF1G86Ch93J (App.tsx prop comment)
fix(tui): respect voice.record_key config (supersedes #19028, #19339) (#19835)
* 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.deleteParsedVoiceRecordKey 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>
fix(tui): right-click copies selection, only pastes when no selection
Sub-issue 5 of #22034.
Right-click on the composer always pasted from the clipboard, even when
the user had highlighted text — diverging from terminal-native behavior
(xterm/iTerm/gnome-terminal) where right-click copies an active selection
and only pastes when nothing is selected.
Extract a small pure helper, decideRightClickAction(value, range), and
route the existing onMouseDown right-click branch through it. Selection
present and non-empty -> writeClipboardText(slice). Otherwise fall back
to the existing emitPaste path.
review(tui): route cursorLayout through @hermes/ink wrapAnsi shim (Bun runtime parity)
Copilot caught an important runtime parity gap on PR #27489: the fix
imported the npm wrap-ansi package directly, but Ink's `<Text
wrap="wrap">` uses a runtime-selecting shim
(ui-tui/packages/hermes-ink/src/ink/wrapAnsi.ts) that prefers
Bun.wrapAnsi when running under Bun and falls back to the npm package
elsewhere. So under Bun, Ink would render via Bun.wrapAnsi while
cursorLayout would compute breaks via the npm package — any
disagreement reintroduces the exact cursor-drift symptom the PR is
meant to eliminate.
Fix:
- Export wrapAnsi from @hermes/ink (packages/hermes-ink/src/entry-exports.ts
and packages/hermes-ink/index.d.ts) so the shim is the public surface.
- Switch ui-tui/src/lib/inputMetrics.ts from `import wrapAnsi from
'wrap-ansi' to import { wrapAnsi } from '@hermes/ink'`. Both
renderer (Ink) and cursor layout now traverse the same shim, so
they share the runtime-selected implementation by construction.
- Same swap in textInputWrap.test.ts and cursorDriftRegression.test.ts
— tests now assert parity through the shim, which means under Bun
they actually exercise Bun's implementation instead of asserting a
tautology against the npm package.
- Drop the direct "wrap-ansi": "^9.0.0" from ui-tui/package.json.
@hermes/ink (which IS a declared dep) pulls wrap-ansi in
transitively — that's not a phantom dep because the import path
goes through @hermes/ink's public exports, not through a
hoisting accident.
Verified: 791/791 vitest tests pass. @hermes/ink rebuilt
(dist/entry-exports.js includes wrapAnsi export). TUI bundle
rebuilt clean.
fix(tui): respect voice.record_key config (supersedes #19028, #19339) (#19835)
* 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.deleteParsedVoiceRecordKey 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>
fix(tui): allow transcript scroll + Esc during approval/clarify/confirm prompts (#26414)
When an approval / clarify / confirm overlay was active, the global input
handler in useInputHandlers returned for every key that wasn't Ctrl+C, which
silently disabled transcript scrolling. On long threads the context the
prompt was asking about often lived above the visible viewport, and being
unable to scroll while answering felt like the prompt had locked the UI.
ApprovalPrompt also had no Esc handler at all, so the one obvious 'abort'
key did nothing during a permission prompt and the user had to memorize
Ctrl+C or hunt for the deny number.
Fixes:
- Extract shouldFallThroughForScroll(key) (pure, exported) covering wheel
scrolls, PageUp/PageDown, and Shift+ArrowUp/Down. When a prompt overlay
is up and the pressed key is a scroll input, skip the early return so it
reaches the existing wheel/PageUp/Shift+arrow handlers below. Plain
arrows still drive in-prompt selection — they don't fall through.
- ApprovalPrompt now maps Esc to onChoice('deny'), parity with the global
Ctrl+C cancellation path that already invokes cancelOverlayFromCtrlC()
for approvals. The bottom-of-prompt hint now advertises 'Esc/Ctrl+C deny'.
- Extract approvalAction(ch, key, sel) — pure key-dispatch helper for the
approval prompt, exported so the regression matrix (Esc, numbers, Enter,
arrows, edge clamping, precedence) is testable without mounting Ink.
Tests:
- useInputHandlers.test.ts: 6 cases covering shouldFallThroughForScroll
positives (wheel/PageUp/PageDown/Shift+arrows) and negatives (plain
arrows, bare shift, no scroll key).
- approvalAction.test.ts: 8 cases covering Esc→deny, numeric mapping,
Enter, ↑↓ within bounds, edge clamping, Esc-beats-others precedence,
unrelated keystrokes.
fix(tui): copilot review on #16707 — naming, label consistency, esc priority
- 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.
fix(tui): steady transcript scrollbar (#20917)
* fix(tui): steady transcript scrollbar
Keep the visible scrollbar tied to committed viewport position while virtual history can still prefetch against pending scroll targets, and preserve drag grab offset synchronously for native-feeling scrollbar drags.
* fix(tui): smooth precision wheel scroll
Replace the opt-scroll throttle with frame-sized coalescing so modifier wheel gestures stay line-precise without stepping.