diff --git a/gateway/run.py b/gateway/run.py index c19303e61..3f295ee96 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -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. diff --git a/tests/gateway/test_voice_command.py b/tests/gateway/test_voice_command.py index ed36b976e..959592659 100644 --- a/tests/gateway/test_voice_command.py +++ b/tests/gateway/test_voice_command.py @@ -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") diff --git a/tests/gateway/test_voice_mode_platform_isolation.py b/tests/gateway/test_voice_mode_platform_isolation.py index 444c2d578..6cdff6094 100644 --- a/tests/gateway/test_voice_mode_platform_isolation.py +++ b/tests/gateway/test_voice_mode_platform_isolation.py @@ -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 : 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 {: '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