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.
This commit is contained in:
+19
-9
@@ -5278,6 +5278,22 @@ def _voice_tts_enabled() -> bool:
|
||||
return os.environ.get("HERMES_VOICE_TTS", "").strip() == "1"
|
||||
|
||||
|
||||
def _voice_record_key() -> str:
|
||||
"""Current ``voice.record_key`` value, documented default on error.
|
||||
|
||||
``_load_cfg()`` returns raw ``yaml.safe_load()`` output, so ``voice``
|
||||
may be any scalar — a hand-edited ``voice: true`` or ``voice: cmd+b``
|
||||
(string where a dict is expected) would break ``.get("record_key")``
|
||||
and take every ``voice.toggle`` branch down with it (Copilot round-3
|
||||
review on #19835). Coerce through ``isinstance`` so malformed config
|
||||
falls back to the documented default instead of crashing /voice.
|
||||
"""
|
||||
voice_cfg = _load_cfg().get("voice")
|
||||
record_key = voice_cfg.get("record_key") if isinstance(voice_cfg, dict) else None
|
||||
|
||||
return str(record_key) if isinstance(record_key, str) and record_key else "ctrl+b"
|
||||
|
||||
|
||||
@method("voice.toggle")
|
||||
def _(rid, params: dict) -> dict:
|
||||
"""CLI parity for the ``/voice`` slash command.
|
||||
@@ -5304,9 +5320,7 @@ def _(rid, params: dict) -> dict:
|
||||
# ignored the config (#18994).
|
||||
payload: dict = {
|
||||
"enabled": _voice_mode_enabled(),
|
||||
"record_key": str(
|
||||
(_load_cfg().get("voice") or {}).get("record_key") or "ctrl+b"
|
||||
),
|
||||
"record_key": _voice_record_key(),
|
||||
"tts": _voice_tts_enabled(),
|
||||
}
|
||||
try:
|
||||
@@ -5347,9 +5361,7 @@ def _(rid, params: dict) -> dict:
|
||||
rid,
|
||||
{
|
||||
"enabled": enabled,
|
||||
"record_key": str(
|
||||
(_load_cfg().get("voice") or {}).get("record_key") or "ctrl+b"
|
||||
),
|
||||
"record_key": _voice_record_key(),
|
||||
"tts": _voice_tts_enabled(),
|
||||
},
|
||||
)
|
||||
@@ -5368,9 +5380,7 @@ def _(rid, params: dict) -> dict:
|
||||
rid,
|
||||
{
|
||||
"enabled": True,
|
||||
"record_key": str(
|
||||
(_load_cfg().get("voice") or {}).get("record_key") or "ctrl+b"
|
||||
),
|
||||
"record_key": _voice_record_key(),
|
||||
"tts": new_value,
|
||||
},
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user