mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-04-25 00:51:20 +00:00
fix(gateway): preserve legacy /voice off suppression across #12542 migration
Legacy unprefixed voice-mode rows were dropped during the #12542 platform-namespacing migration, causing one unwanted auto-TTS reply on Telegram chats that had /voice off before the upgrade. Preserve legacy "off" rows on load and apply them as a cross-platform fallback; explicit platform-prefixed rows take precedence so a later /voice on/tts cannot be silently re-suppressed after a restart. Fixes #14025
This commit is contained in:
Season
parent
432772dbdf
commit
807fd2a69f
3 changed files with 166 additions and 20 deletions
|
|
@ -808,15 +808,28 @@ class GatewayRunner:
|
|||
if mode not in valid_modes:
|
||||
continue
|
||||
key = str(chat_id)
|
||||
# Skip legacy unprefixed keys (warn and skip)
|
||||
if ":" not in key:
|
||||
logger.warning(
|
||||
"Skipping legacy unprefixed voice mode key %r during migration. "
|
||||
"Re-enable voice mode on that chat to rebuild the prefixed key.",
|
||||
if ":" in key:
|
||||
result[key] = mode
|
||||
continue
|
||||
# Legacy pre-#12542 key. Keep "off" so auto-TTS suppression
|
||||
# survives the migration (#14025); drop voice_only/all to
|
||||
# avoid firing TTS on a platform the user never configured.
|
||||
if mode == "off":
|
||||
logger.info(
|
||||
"Preserving legacy unprefixed voice mode key %r (mode=off) "
|
||||
"for cross-adapter auto-TTS suppression. Run any /voice "
|
||||
"command on the affected chat to write a prefixed entry "
|
||||
"that supersedes this fallback on that platform.",
|
||||
key,
|
||||
)
|
||||
continue
|
||||
result[key] = mode
|
||||
result[key] = mode
|
||||
else:
|
||||
logger.warning(
|
||||
"Skipping legacy unprefixed voice mode key %r (mode=%s) "
|
||||
"during migration. Re-enable voice mode on that chat to "
|
||||
"rebuild the prefixed key.",
|
||||
key, mode,
|
||||
)
|
||||
return result
|
||||
|
||||
def _save_voice_modes(self) -> None:
|
||||
|
|
@ -839,7 +852,12 @@ class GatewayRunner:
|
|||
disabled_chats.discard(chat_id)
|
||||
|
||||
def _sync_voice_mode_state_to_adapter(self, adapter) -> None:
|
||||
"""Restore persisted /voice off state into a live platform adapter."""
|
||||
"""Restore persisted /voice off state into a live platform adapter.
|
||||
|
||||
Explicit platform-prefixed entries win; legacy pre-#12542 unprefixed
|
||||
"off" rows act as a cross-platform fallback for chats that have no
|
||||
prefixed entry yet (#14025).
|
||||
"""
|
||||
disabled_chats = getattr(adapter, "_auto_tts_disabled_chats", None)
|
||||
if not isinstance(disabled_chats, set):
|
||||
return
|
||||
|
|
@ -848,10 +866,18 @@ class GatewayRunner:
|
|||
return
|
||||
disabled_chats.clear()
|
||||
prefix = f"{platform.value}:"
|
||||
disabled_chats.update(
|
||||
key[len(prefix):] for key, mode in self._voice_mode.items()
|
||||
if mode == "off" and key.startswith(prefix)
|
||||
)
|
||||
|
||||
explicit_ids: set = set()
|
||||
for key, mode in self._voice_mode.items():
|
||||
if key.startswith(prefix):
|
||||
chat_id = key[len(prefix):]
|
||||
explicit_ids.add(chat_id)
|
||||
if mode == "off":
|
||||
disabled_chats.add(chat_id)
|
||||
|
||||
for key, mode in self._voice_mode.items():
|
||||
if mode == "off" and ":" not in key and key not in explicit_ids:
|
||||
disabled_chats.add(key)
|
||||
|
||||
async def _safe_adapter_disconnect(self, adapter, platform) -> None:
|
||||
"""Call adapter.disconnect() defensively, swallowing any error.
|
||||
|
|
|
|||
|
|
@ -193,6 +193,35 @@ class TestHandleVoiceCommand:
|
|||
assert restored_runner._voice_mode["telegram:123"] == "off"
|
||||
assert adapter._auto_tts_disabled_chats == {"123"}
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_voice_on_after_legacy_off_overrides_fallback_after_restart(self, tmp_path):
|
||||
"""Full lifecycle: a pre-#12542 legacy {chat_id: 'off'} row is
|
||||
preserved across save/reload, but an explicit /voice on writes a
|
||||
prefixed entry that overrides the legacy fallback on subsequent
|
||||
restarts — no split-brain where /voice status reports 'on' while
|
||||
auto-TTS stays suppressed (#14025)."""
|
||||
from gateway.config import Platform
|
||||
|
||||
runner = _make_runner(tmp_path)
|
||||
runner._VOICE_MODE_PATH.write_text(json.dumps({"123": "off"}))
|
||||
runner._voice_mode = runner._load_voice_modes()
|
||||
assert runner._voice_mode == {"123": "off"}
|
||||
|
||||
event = _make_event("/voice on", chat_id="123")
|
||||
await runner._handle_voice_command(event)
|
||||
|
||||
restored_runner = _make_runner(tmp_path)
|
||||
restored_runner._voice_mode = restored_runner._load_voice_modes()
|
||||
adapter = SimpleNamespace(
|
||||
_auto_tts_disabled_chats=set(),
|
||||
platform=Platform.TELEGRAM,
|
||||
)
|
||||
restored_runner._sync_voice_mode_state_to_adapter(adapter)
|
||||
|
||||
assert restored_runner._voice_mode["123"] == "off"
|
||||
assert restored_runner._voice_mode["telegram:123"] == "voice_only"
|
||||
assert "123" not in adapter._auto_tts_disabled_chats
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_per_chat_isolation(self, runner):
|
||||
e1 = _make_event("/voice on", chat_id="aaa")
|
||||
|
|
|
|||
|
|
@ -65,15 +65,18 @@ class TestVoiceModePlatformIsolation:
|
|||
class TestLegacyKeyMigration:
|
||||
"""Test migration of legacy unprefixed keys in _load_voice_modes."""
|
||||
|
||||
def test_load_voice_modes_skips_legacy_keys(self):
|
||||
"""_load_voice_modes skips keys without ':' prefix and logs a warning."""
|
||||
def test_load_voice_modes_skips_legacy_nonoff_keys(self):
|
||||
"""Legacy unprefixed non-'off' keys are dropped with a warning.
|
||||
|
||||
Only 'off' legacy rows are preserved (see #14025); 'all' and
|
||||
'voice_only' legacy rows would broadcast TTS to the wrong
|
||||
platform if migrated without knowing the owner.
|
||||
"""
|
||||
runner = _make_runner()
|
||||
|
||||
# Simulate legacy persisted data with unprefixed keys
|
||||
legacy_data = {
|
||||
"123": "all",
|
||||
"456": "voice_only",
|
||||
# Also includes a properly prefixed key (from after the fix)
|
||||
"telegram:789": "off",
|
||||
}
|
||||
|
||||
|
|
@ -85,16 +88,37 @@ class TestLegacyKeyMigration:
|
|||
with patch("gateway.run.logger") as mock_logger:
|
||||
result = runner._load_voice_modes()
|
||||
|
||||
# Legacy keys without ':' should be skipped
|
||||
assert "123" not in result
|
||||
assert "456" not in result
|
||||
# Prefixed key should be preserved
|
||||
assert result.get("telegram:789") == "off"
|
||||
# Warning should be logged for each legacy key
|
||||
assert mock_logger.warning.called
|
||||
warning_calls = [str(call) for call in mock_logger.warning.call_args_list]
|
||||
assert any("Skipping legacy unprefixed voice mode key" in str(c) for c in warning_calls)
|
||||
|
||||
def test_load_voice_modes_preserves_legacy_off_keys(self):
|
||||
"""Legacy unprefixed keys with mode='off' are preserved so the
|
||||
auto-TTS suppression survives the #12542 migration (#14025)."""
|
||||
runner = _make_runner()
|
||||
|
||||
legacy_data = {
|
||||
"123": "off",
|
||||
"456": "all",
|
||||
"789": "voice_only",
|
||||
"telegram:999": "off",
|
||||
}
|
||||
|
||||
with tempfile.TemporaryDirectory() as tmpdir:
|
||||
voice_path = Path(tmpdir) / "gateway_voice_mode.json"
|
||||
voice_path.write_text(json.dumps(legacy_data))
|
||||
|
||||
with patch.object(runner, "_VOICE_MODE_PATH", voice_path):
|
||||
result = runner._load_voice_modes()
|
||||
|
||||
assert result.get("123") == "off"
|
||||
assert "456" not in result
|
||||
assert "789" not in result
|
||||
assert result.get("telegram:999") == "off"
|
||||
|
||||
def test_load_voice_modes_preserves_prefixed_keys(self):
|
||||
"""_load_voice_modes correctly loads platform-prefixed keys."""
|
||||
runner = _make_runner()
|
||||
|
|
@ -204,9 +228,69 @@ class TestSyncVoiceModeStateToAdapter:
|
|||
# Should not raise
|
||||
runner._sync_voice_mode_state_to_adapter(mock_adapter)
|
||||
|
||||
def test_sync_legacy_off_applies_to_all_adapters(self):
|
||||
"""Legacy unprefixed 'off' rows suppress auto-TTS on every adapter
|
||||
that has no explicit prefixed entry for the same chat id (#14025)."""
|
||||
runner = _make_runner()
|
||||
runner._voice_mode = {
|
||||
"123": "off",
|
||||
"telegram:456": "off",
|
||||
"slack:789": "off",
|
||||
}
|
||||
|
||||
telegram_adapter = _make_adapter(Platform.TELEGRAM)
|
||||
runner._sync_voice_mode_state_to_adapter(telegram_adapter)
|
||||
assert telegram_adapter._auto_tts_disabled_chats == {"123", "456"}
|
||||
|
||||
slack_adapter = _make_adapter(Platform.SLACK)
|
||||
runner._sync_voice_mode_state_to_adapter(slack_adapter)
|
||||
assert slack_adapter._auto_tts_disabled_chats == {"123", "789"}
|
||||
|
||||
def test_explicit_prefixed_state_overrides_legacy_off_on_same_platform(self):
|
||||
"""An explicit <platform>:<chat_id> row wins over a legacy unprefixed
|
||||
off row on the same chat id on that platform, regardless of the
|
||||
explicit mode (#14025). Covers voice_only, all (the mode written by
|
||||
the Discord /voice channel join path), and off."""
|
||||
runner = _make_runner()
|
||||
runner._voice_mode = {
|
||||
"123": "off",
|
||||
"456": "off",
|
||||
"789": "off",
|
||||
"telegram:123": "voice_only",
|
||||
"telegram:456": "all",
|
||||
"telegram:789": "off",
|
||||
"slack:123": "off",
|
||||
}
|
||||
|
||||
telegram_adapter = _make_adapter(Platform.TELEGRAM)
|
||||
runner._sync_voice_mode_state_to_adapter(telegram_adapter)
|
||||
assert telegram_adapter._auto_tts_disabled_chats == {"789"}
|
||||
|
||||
slack_adapter = _make_adapter(Platform.SLACK)
|
||||
runner._sync_voice_mode_state_to_adapter(slack_adapter)
|
||||
assert slack_adapter._auto_tts_disabled_chats == {"123", "456", "789"}
|
||||
|
||||
def test_legacy_off_survives_upgrade_restart_for_telegram(self):
|
||||
"""End-to-end: a pre-#12542 {<chat_id>: 'off'} entry still suppresses
|
||||
auto-TTS on the Telegram adapter after an upgrade and restart without
|
||||
requiring the user to re-run /voice off (#14025)."""
|
||||
runner = _make_runner()
|
||||
legacy_data = {"987654321": "off"}
|
||||
|
||||
with tempfile.TemporaryDirectory() as tmpdir:
|
||||
voice_path = Path(tmpdir) / "gateway_voice_mode.json"
|
||||
voice_path.write_text(json.dumps(legacy_data))
|
||||
with patch.object(runner, "_VOICE_MODE_PATH", voice_path):
|
||||
runner._voice_mode = runner._load_voice_modes()
|
||||
|
||||
telegram_adapter = _make_adapter(Platform.TELEGRAM)
|
||||
runner._sync_voice_mode_state_to_adapter(telegram_adapter)
|
||||
|
||||
assert "987654321" in telegram_adapter._auto_tts_disabled_chats
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Helper
|
||||
# Helpers
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def _make_runner() -> GatewayRunner:
|
||||
|
|
@ -216,3 +300,10 @@ def _make_runner() -> GatewayRunner:
|
|||
runner._voice_mode = {}
|
||||
runner.adapters = {}
|
||||
return runner
|
||||
|
||||
|
||||
def _make_adapter(platform: Platform) -> MagicMock:
|
||||
adapter = MagicMock()
|
||||
adapter.platform = platform
|
||||
adapter._auto_tts_disabled_chats = set()
|
||||
return adapter
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue