From 058092c1a1e97a53925df21335972999dd26e777 Mon Sep 17 00:00:00 2001 From: ayvee <113040046+asianviking@users.noreply.github.com> Date: Mon, 2 Mar 2026 15:09:29 +0700 Subject: [PATCH] fix: make telegram config optional for external transports (#177) Co-authored-by: Claude Opus 4.5 Co-authored-by: banteg <4562643+banteg@users.noreply.github.com> --- src/takopi/cli/doctor.py | 28 ++++++++++++--------- src/takopi/cli/onboarding_cmd.py | 3 ++- src/takopi/cli/run.py | 2 ++ src/takopi/settings.py | 11 ++++++--- src/takopi/telegram/onboarding.py | 24 +++++++++++------- tests/test_cli_auto_router.py | 35 ++++++++++++++++++++++++++ tests/test_cli_chat_id.py | 26 ++++++++++++++++++++ tests/test_cli_doctor.py | 13 ++++++++++ tests/test_cli_helpers.py | 20 +++++++++------ tests/test_onboarding.py | 41 +++++++++++++++++++++++++++++++ tests/test_settings.py | 35 ++++++++++++++++++++++++++ 11 files changed, 207 insertions(+), 31 deletions(-) diff --git a/src/takopi/cli/doctor.py b/src/takopi/cli/doctor.py index 4f97d2a..5b424c0 100644 --- a/src/takopi/cli/doctor.py +++ b/src/takopi/cli/doctor.py @@ -14,7 +14,7 @@ from ..config import ConfigError from ..engines import list_backend_ids from ..ids import RESERVED_CHAT_COMMANDS from ..runtime_loader import resolve_plugins_allowlist -from ..settings import TakopiSettings, TelegramTopicsSettings +from ..settings import TakopiSettings, TelegramTopicsSettings, TelegramTransportSettings from ..telegram.client import TelegramClient from ..telegram.topics import _validate_topics_setup_for @@ -33,8 +33,8 @@ class DoctorCheck: return f"- {self.label}: {self.status}" -def _doctor_file_checks(settings: TakopiSettings) -> list[DoctorCheck]: - files = settings.transports.telegram.files +def _doctor_file_checks(settings: TelegramTransportSettings) -> list[DoctorCheck]: + files = settings.files if not files.enabled: return [DoctorCheck("file transfer", "ok", "disabled")] if files.allowed_user_ids: @@ -44,10 +44,10 @@ def _doctor_file_checks(settings: TakopiSettings) -> list[DoctorCheck]: return [DoctorCheck("file transfer", "warning", "enabled for all users")] -def _doctor_voice_checks(settings: TakopiSettings) -> list[DoctorCheck]: - if not settings.transports.telegram.voice_transcription: +def _doctor_voice_checks(settings: TelegramTransportSettings) -> list[DoctorCheck]: + if not settings.voice_transcription: return [DoctorCheck("voice transcription", "ok", "disabled")] - api_key = settings.transports.telegram.voice_transcription_api_key + api_key = settings.voice_transcription_api_key if api_key: return [ DoctorCheck("voice transcription", "ok", "voice_transcription_api_key set") @@ -115,8 +115,8 @@ def run_doctor( [str, int, TelegramTopicsSettings, tuple[int, ...]], Awaitable[list[DoctorCheck]], ], - file_checks: Callable[[TakopiSettings], list[DoctorCheck]], - voice_checks: Callable[[TakopiSettings], list[DoctorCheck]], + file_checks: Callable[[TelegramTransportSettings], list[DoctorCheck]], + voice_checks: Callable[[TelegramTransportSettings], list[DoctorCheck]], ) -> None: try: settings, config_path = load_settings_fn() @@ -130,6 +130,13 @@ def run_doctor( err=True, ) raise typer.Exit(code=1) + tg = settings.transports.telegram + if tg is None: + typer.echo( + f"error: Missing [transports.telegram] in {config_path}.", + err=True, + ) + raise typer.Exit(code=1) allowlist = resolve_plugins_allowlist(settings) engine_ids = list_backend_ids(allowlist=allowlist) @@ -143,7 +150,6 @@ def run_doctor( typer.echo(f"error: {exc}", err=True) raise typer.Exit(code=1) from exc - tg = settings.transports.telegram project_chat_ids = projects_cfg.project_chat_ids() telegram_checks_result = anyio.run( telegram_checks, @@ -156,8 +162,8 @@ def run_doctor( telegram_checks_result = [] checks = [ *telegram_checks_result, - *file_checks(settings), - *voice_checks(settings), + *file_checks(tg), + *voice_checks(tg), ] typer.echo("takopi doctor") for check in checks: diff --git a/src/takopi/cli/onboarding_cmd.py b/src/takopi/cli/onboarding_cmd.py index a9b3980..5b2e38b 100644 --- a/src/takopi/cli/onboarding_cmd.py +++ b/src/takopi/cli/onboarding_cmd.py @@ -65,7 +65,8 @@ def chat_id( settings, _ = load_settings_optional_fn() if settings is not None: tg = settings.transports.telegram - token = tg.bot_token or None + if tg is not None: + token = tg.bot_token or None chat = anyio.run(partial(onboarding_mod.capture_chat_id, token=token)) if chat is None: raise typer.Exit(code=1) diff --git a/src/takopi/cli/run.py b/src/takopi/cli/run.py index 6e2ac71..e49e4c9 100644 --- a/src/takopi/cli/run.py +++ b/src/takopi/cli/run.py @@ -289,6 +289,8 @@ def _run_auto_router( ) if settings.transport == "telegram": transport_config = settings.transports.telegram + if transport_config is None: + raise ConfigError(f"Missing [transports.telegram] in {config_path}.") else: transport_config = settings.transport_config( settings.transport, config_path=config_path diff --git a/src/takopi/settings.py b/src/takopi/settings.py index 13a939a..2328199 100644 --- a/src/takopi/settings.py +++ b/src/takopi/settings.py @@ -110,7 +110,7 @@ class TelegramTransportSettings(BaseModel): class TransportsSettings(BaseModel): - telegram: TelegramTransportSettings + telegram: TelegramTransportSettings | None = None model_config = ConfigDict(extra="allow") @@ -191,6 +191,8 @@ class TakopiSettings(BaseSettings): self, transport_id: str, *, config_path: Path ) -> dict[str, Any]: if transport_id == "telegram": + if self.transports.telegram is None: + raise ConfigError(f"Missing [transports.telegram] in {config_path}.") return self.transports.telegram.model_dump() extra = self.transports.model_extra or {} raw = extra.get(transport_id) @@ -211,7 +213,8 @@ class TakopiSettings(BaseSettings): reserved: Iterable[str] = ("cancel",), ) -> ProjectsConfig: default_project = self.default_project - default_chat_id = self.transports.telegram.chat_id + tg = self.transports.telegram + default_chat_id = tg.chat_id if tg is not None else None reserved_lower = {value.lower() for value in reserved} engine_map = {engine.lower(): engine for engine in engine_ids} @@ -248,7 +251,7 @@ class TakopiSettings(BaseSettings): chat_id = entry.chat_id if chat_id is not None: - if chat_id == default_chat_id: + if default_chat_id is not None and chat_id == default_chat_id: raise ConfigError( f"Invalid `projects.{alias}.chat_id` in {config_path}; " "must not match transports.telegram.chat_id." @@ -323,6 +326,8 @@ def require_telegram(settings: TakopiSettings, config_path: Path) -> tuple[str, "(telegram only for now)." ) tg = settings.transports.telegram + if tg is None: + raise ConfigError(f"Missing [transports.telegram] in {config_path}.") return tg.bot_token, tg.chat_id diff --git a/src/takopi/telegram/onboarding.py b/src/takopi/telegram/onboarding.py index 8827d76..4e116c9 100644 --- a/src/takopi/telegram/onboarding.py +++ b/src/takopi/telegram/onboarding.py @@ -207,17 +207,23 @@ def check_setup( settings, config_path = load_settings() if transport_override: settings = settings.model_copy(update={"transport": transport_override}) - try: - require_telegram(settings, config_path) - except ConfigError: - issues.append(config_issue(config_path, title=_CONFIGURE_TELEGRAM_TITLE)) + if settings.transport == "telegram": + try: + require_telegram(settings, config_path) + except ConfigError: + issues.append( + config_issue(config_path, title=_CONFIGURE_TELEGRAM_TITLE) + ) except ConfigError: issues.extend(backend_issues) - title = ( - _CONFIGURE_TELEGRAM_TITLE - if config_path.exists() and config_path.is_file() - else _CREATE_CONFIG_TITLE - ) + if transport_override and transport_override != "telegram": + title = _CREATE_CONFIG_TITLE + else: + title = ( + _CONFIGURE_TELEGRAM_TITLE + if config_path.exists() and config_path.is_file() + else _CREATE_CONFIG_TITLE + ) issues.append(config_issue(config_path, title=title)) return SetupResult(issues=issues, config_path=config_path) diff --git a/tests/test_cli_auto_router.py b/tests/test_cli_auto_router.py index 9edcf64..d0c5f80 100644 --- a/tests/test_cli_auto_router.py +++ b/tests/test_cli_auto_router.py @@ -176,3 +176,38 @@ def test_run_auto_router_missing_config_noninteractive( assert exc.value.exit_code == 1 assert not transport.build_calls + + +def test_run_auto_router_rejects_missing_telegram_config( + monkeypatch, tmp_path: Path +) -> None: + setup = SetupResult(issues=[], config_path=tmp_path / "takopi.toml") + transport = _FakeTransport(setup) + config_path = tmp_path / "takopi.toml" + settings = TakopiSettings.model_validate( + {"transport": "telegram", "transports": {}} + ) + + monkeypatch.setattr( + cli, + "_resolve_setup_engine", + lambda _override: (None, None, None, "codex", _engine_backend()), + ) + monkeypatch.setattr(cli, "_resolve_transport_id", lambda _override: "telegram") + monkeypatch.setattr(cli, "get_transport", lambda _id, allowlist=None: transport) + monkeypatch.setattr(cli, "load_settings", lambda: (settings, config_path)) + monkeypatch.setattr(cli, "setup_logging", lambda **_kwargs: None) + monkeypatch.setattr(cli, "build_runtime_spec", lambda **_kwargs: object()) + + with pytest.raises(cli.typer.Exit) as exc: + cli._run_auto_router( + default_engine_override=None, + transport_override=None, + final_notify=True, + debug=False, + onboard=False, + ) + + assert exc.value.exit_code == 1 + assert not transport.lock_calls + assert not transport.build_calls diff --git a/tests/test_cli_chat_id.py b/tests/test_cli_chat_id.py index 11c9281..68cd9bd 100644 --- a/tests/test_cli_chat_id.py +++ b/tests/test_cli_chat_id.py @@ -68,3 +68,29 @@ def test_chat_id_command_uses_config_token(monkeypatch) -> None: assert result.exit_code == 0 assert "chat_id = 321" in result.output + + +def test_chat_id_command_without_telegram_config(monkeypatch) -> None: + settings = TakopiSettings.model_validate( + {"transport": "my-transport", "transports": {}} + ) + monkeypatch.setattr(cli, "_load_settings_optional", lambda: (settings, Path("x"))) + + async def _capture(*, token: str | None = None): + assert token is None + return onboarding.ChatInfo( + chat_id=321, + username=None, + title="takopi", + first_name=None, + last_name=None, + chat_type="supergroup", + ) + + monkeypatch.setattr(cli.onboarding, "capture_chat_id", _capture) + + runner = CliRunner() + result = runner.invoke(cli.create_app(), ["chat-id"]) + + assert result.exit_code == 0 + assert "chat_id = 321" in result.output diff --git a/tests/test_cli_doctor.py b/tests/test_cli_doctor.py index 8317f0a..65210d2 100644 --- a/tests/test_cli_doctor.py +++ b/tests/test_cli_doctor.py @@ -56,6 +56,19 @@ def test_doctor_errors_exit_nonzero(monkeypatch) -> None: assert "telegram token: error" in result.output +def test_doctor_missing_telegram_config_exits(monkeypatch) -> None: + settings = TakopiSettings.model_validate( + {"transport": "telegram", "transports": {}} + ) + monkeypatch.setattr(cli, "load_settings", lambda: (settings, Path("x"))) + + runner = CliRunner() + result = runner.invoke(cli.create_app(), ["doctor"]) + + assert result.exit_code == 1 + assert "Missing [transports.telegram]" in result.output + + class _FakeBot: def __init__(self, me: User | None, chat: Chat | None) -> None: self._me = me diff --git a/tests/test_cli_helpers.py b/tests/test_cli_helpers.py index b2454f6..dee26d8 100644 --- a/tests/test_cli_helpers.py +++ b/tests/test_cli_helpers.py @@ -5,7 +5,7 @@ import pytest from takopi import cli from takopi.config import ConfigError from takopi.lockfile import LockError -from takopi.settings import TakopiSettings +from takopi.settings import TakopiSettings, TelegramTransportSettings def _settings(overrides: dict | None = None) -> TakopiSettings: @@ -18,6 +18,12 @@ def _settings(overrides: dict | None = None) -> TakopiSettings: return TakopiSettings.model_validate(payload) +def _telegram_settings(settings: TakopiSettings) -> TelegramTransportSettings: + tg = settings.transports.telegram + assert tg is not None + return tg + + def test_parse_key_path_valid() -> None: assert cli._parse_key_path("transports.telegram.chat_id") == [ "transports", @@ -98,7 +104,7 @@ def test_resolve_transport_id_override(monkeypatch) -> None: def test_doctor_file_checks() -> None: settings = _settings() - checks = cli._doctor_file_checks(settings) + checks = cli._doctor_file_checks(_telegram_settings(settings)) assert checks[0].detail == "disabled" settings = _settings( @@ -112,13 +118,13 @@ def test_doctor_file_checks() -> None: } } ) - checks = cli._doctor_file_checks(settings) + checks = cli._doctor_file_checks(_telegram_settings(settings)) assert checks[0].status == "warning" def test_doctor_voice_checks(monkeypatch) -> None: settings = _settings() - checks = cli._doctor_voice_checks(settings) + checks = cli._doctor_voice_checks(_telegram_settings(settings)) assert checks[0].detail == "disabled" settings = _settings( @@ -133,7 +139,7 @@ def test_doctor_voice_checks(monkeypatch) -> None: } ) monkeypatch.delenv("OPENAI_API_KEY", raising=False) - checks = cli._doctor_voice_checks(settings) + checks = cli._doctor_voice_checks(_telegram_settings(settings)) assert checks[0].status == "error" assert checks[0].detail == "API key not set" @@ -149,12 +155,12 @@ def test_doctor_voice_checks(monkeypatch) -> None: } } ) - checks = cli._doctor_voice_checks(settings_with_key) + checks = cli._doctor_voice_checks(_telegram_settings(settings_with_key)) assert checks[0].status == "ok" assert checks[0].detail == "voice_transcription_api_key set" monkeypatch.setenv("OPENAI_API_KEY", "key") - checks = cli._doctor_voice_checks(settings) + checks = cli._doctor_voice_checks(_telegram_settings(settings)) assert checks[0].status == "ok" diff --git a/tests/test_onboarding.py b/tests/test_onboarding.py index f924d1c..b593669 100644 --- a/tests/test_onboarding.py +++ b/tests/test_onboarding.py @@ -75,3 +75,44 @@ def test_check_setup_marks_invalid_bot_token(monkeypatch, tmp_path: Path) -> Non titles = {issue.title for issue in result.issues} assert "configure telegram" in titles + + +def test_check_setup_skips_telegram_validation_for_external_transport( + monkeypatch, tmp_path: Path +) -> None: + backend = engines.get_backend("codex") + monkeypatch.setattr(onboarding.shutil, "which", lambda _name: "/usr/bin/codex") + monkeypatch.setattr( + onboarding, + "load_settings", + lambda: ( + TakopiSettings.model_validate( + {"transport": "my-transport", "transports": {}} + ), + tmp_path / "takopi.toml", + ), + ) + + result = onboarding.check_setup(backend, transport_override="my-transport") + + assert result.ok is True + assert len(result.issues) == 0 + + +def test_check_setup_external_transport_missing_config( + monkeypatch, tmp_path: Path +) -> None: + backend = engines.get_backend("codex") + monkeypatch.setattr(onboarding.shutil, "which", lambda _name: "/usr/bin/codex") + monkeypatch.setattr(onboarding, "HOME_CONFIG_PATH", tmp_path / "takopi.toml") + + def _raise() -> None: + raise onboarding.ConfigError("Missing config file") + + monkeypatch.setattr(onboarding, "load_settings", _raise) + + result = onboarding.check_setup(backend, transport_override="my-transport") + + titles = {issue.title for issue in result.issues} + assert "create a config" in titles + assert "configure telegram" not in titles diff --git a/tests/test_settings.py b/tests/test_settings.py index 1590efe..e81aa62 100644 --- a/tests/test_settings.py +++ b/tests/test_settings.py @@ -176,6 +176,15 @@ def test_transport_config_telegram_and_extra(tmp_path: Path) -> None: settings.transport_config("discord", config_path=config_path) +def test_transport_config_telegram_missing(tmp_path: Path) -> None: + config_path = tmp_path / "takopi.toml" + settings = TakopiSettings.model_validate( + {"transport": "discord", "transports": {"discord": {"token": "abc"}}} + ) + with pytest.raises(ConfigError, match=r"Missing \[transports\.telegram\]"): + settings.transport_config("telegram", config_path=config_path) + + def test_bot_token_none_rejected(tmp_path: Path) -> None: config_path = tmp_path / "takopi.toml" data = { @@ -198,6 +207,15 @@ def test_require_telegram_rejects_non_telegram_transport(tmp_path: Path) -> None require_telegram(settings, config_path) +def test_require_telegram_rejects_missing_telegram_config(tmp_path: Path) -> None: + config_path = tmp_path / "takopi.toml" + settings = TakopiSettings.model_validate( + {"transport": "telegram", "transports": {}} + ) + with pytest.raises(ConfigError, match=r"Missing \[transports\.telegram\]"): + require_telegram(settings, config_path) + + def test_load_settings_if_exists_missing(tmp_path: Path) -> None: config_path = tmp_path / "missing.toml" assert load_settings_if_exists(config_path) is None @@ -235,3 +253,20 @@ def test_load_settings_rejects_non_file(tmp_path: Path) -> None: config_path.mkdir() with pytest.raises(ConfigError, match="exists but is not a file"): load_settings(config_path) + + +def test_load_settings_without_telegram(tmp_path: Path) -> None: + config_path = tmp_path / "takopi.toml" + config_path.write_text( + 'transport = "my-transport"\n\n[transports.my-transport]\nsome_key = "value"\n', + encoding="utf-8", + ) + + settings, loaded_path = load_settings(config_path) + + assert loaded_path == config_path + assert settings.transport == "my-transport" + assert settings.transports.telegram is None + assert settings.transport_config("my-transport", config_path=config_path) == { + "some_key": "value" + }