diff --git a/litellm/proxy/client/cli/commands/autoroute/commands.py b/litellm/proxy/client/cli/commands/autoroute/commands.py index 75e0bac070a..428daeefa60 100644 --- a/litellm/proxy/client/cli/commands/autoroute/commands.py +++ b/litellm/proxy/client/cli/commands/autoroute/commands.py @@ -9,7 +9,7 @@ import click import yaml from pydantic import JsonValue, TypeAdapter, ValidationError -from ..up import CLAUDE_SETTINGS_PATH, load_json_or_empty, restore_claude_settings, write_backup +from ..up import CLAUDE_SETTINGS_PATH, UpError, load_json_or_empty, restore_claude_settings, write_backup from ..up import BackupRecord as ClaudeBackupRecord from .process import ( AUTOROUTE_DIR, @@ -81,7 +81,10 @@ def up() -> None: if not CONFIG_PATH.exists(): raise click.ClickException("No config found. Run `lite autoroute configure` first.") - existing_pid = read_pid_record() + try: + existing_pid = read_pid_record() + except UpError as e: + raise click.ClickException(str(e)) if existing_pid is not None and is_running(existing_pid.pid): raise click.ClickException( "An ephemeral proxy is already running (lite autoroute up looks already active). " @@ -107,16 +110,21 @@ def up() -> None: clear_pid_record() raise click.ClickException(str(e)) - original_existed = CLAUDE_SETTINGS_PATH.exists() - original_settings = load_json_or_empty(CLAUDE_SETTINGS_PATH) - write_backup( - ClaudeBackupRecord(existed=original_existed, content=original_settings if original_existed else None), - AUTOROUTE_BACKUP_PATH, - ) - merged = merge_claude_settings_static_token(original_settings, base_url, master_key) - CLAUDE_SETTINGS_PATH.parent.mkdir(parents=True, exist_ok=True) - with secure_create(CLAUDE_SETTINGS_PATH) as f: - json.dump(merged, f, indent=2) + try: + original_existed = CLAUDE_SETTINGS_PATH.exists() + original_settings = load_json_or_empty(CLAUDE_SETTINGS_PATH) + write_backup( + ClaudeBackupRecord(existed=original_existed, content=original_settings if original_existed else None), + AUTOROUTE_BACKUP_PATH, + ) + merged = merge_claude_settings_static_token(original_settings, base_url, master_key) + CLAUDE_SETTINGS_PATH.parent.mkdir(parents=True, exist_ok=True) + with secure_create(CLAUDE_SETTINGS_PATH) as f: + json.dump(merged, f, indent=2) + except UpError as e: + terminate(process.pid) + clear_pid_record() + raise click.ClickException(str(e)) click.echo(f"litellm: ephemeral auto-router proxy up at {base_url} (pid {process.pid})") click.echo("Claude Code sessions started now will route through it. Press Ctrl-C to stop and restore.") @@ -129,7 +137,14 @@ def up() -> None: return terminate(process.pid) clear_pid_record() - restore_claude_settings(CLAUDE_SETTINGS_PATH, AUTOROUTE_BACKUP_PATH) + try: + restore_claude_settings(CLAUDE_SETTINGS_PATH, AUTOROUTE_BACKUP_PATH) + except UpError as e: + # Runs from atexit/a signal handler too, outside Click's own exception + # handling -- raising here would only produce an unhandled-exception + # warning on stderr, not a clean message. + click.echo(str(e), err=True) + return click.echo("\nStopped ephemeral proxy and restored Claude Code settings.") click.echo( f"Restart any Claude Code session still open from this session, or another local account could " @@ -154,13 +169,22 @@ def up() -> None: @autoroute_group.command("down") def down() -> None: """Restore Claude Code settings and stop a leftover ephemeral proxy, if any""" - record: PidRecord | None = read_pid_record() + try: + record: PidRecord | None = read_pid_record() + except UpError as e: + # down is the crash-recovery path -- a corrupt pid record must not block it; clear the + # unusable record and keep going rather than leaving the user with no way to clean up. + click.echo(f"{e} Clearing it and continuing cleanup.", err=True) + record = None if record is not None and is_running(record.pid): terminate(record.pid) click.echo(f"Stopped leftover ephemeral proxy (pid {record.pid}).") clear_pid_record() - restored = restore_claude_settings(CLAUDE_SETTINGS_PATH, AUTOROUTE_BACKUP_PATH) + try: + restored = restore_claude_settings(CLAUDE_SETTINGS_PATH, AUTOROUTE_BACKUP_PATH) + except UpError as e: + raise click.ClickException(str(e)) if restored is None: click.echo("Nothing to restore.") elif restored.existed: diff --git a/litellm/proxy/client/cli/commands/autoroute/process.py b/litellm/proxy/client/cli/commands/autoroute/process.py index e1cd38531d5..86fa584a223 100644 --- a/litellm/proxy/client/cli/commands/autoroute/process.py +++ b/litellm/proxy/client/cli/commands/autoroute/process.py @@ -12,9 +12,9 @@ from pathlib import Path import click import requests -from pydantic import TypeAdapter +from pydantic import TypeAdapter, ValidationError -from ..up import secure_create +from ..up import UpError, secure_create AUTOROUTE_DIR = Path.home() / ".litellm" / "autorouter" CONFIG_PATH = AUTOROUTE_DIR / "config.yaml" @@ -103,7 +103,11 @@ def read_pid_record(path: Path | None = None) -> PidRecord | None: if not resolved_path.exists(): return None with open(resolved_path, "r") as f: - return _PID_RECORD_ADAPTER.validate_json(f.read()) + content = f.read() + try: + return _PID_RECORD_ADAPTER.validate_json(content) + except ValidationError: + raise UpError(f"{resolved_path} contains invalid or unexpected JSON; cannot proceed safely.") def clear_pid_record(path: Path | None = None) -> None: diff --git a/tests/test_litellm/proxy/client/cli/autoroute/test_commands.py b/tests/test_litellm/proxy/client/cli/autoroute/test_commands.py index c32cb1e6dce..e0b5d3a71af 100644 --- a/tests/test_litellm/proxy/client/cli/autoroute/test_commands.py +++ b/tests/test_litellm/proxy/client/cli/autoroute/test_commands.py @@ -148,6 +148,34 @@ class TestUpCommand: assert written_config["general_settings"]["master_key"] == "fixed-master-key" assert stat.S_IMODE(config_path.stat().st_mode) == 0o600 + def test_teardown_reports_clean_error_when_backup_is_corrupt(self, monkeypatch, tmp_path): + """A corrupt backup at teardown time (e.g. a concurrent process wrote garbage to it) must + not crash the whole command -- _restore_once in up.py handles the identical case in + lite up the same way, echoing the error instead of propagating it.""" + config_path, _log_path, claude_settings_path, backup_path, pid_record_path = _patch_paths(monkeypatch, tmp_path) + config_path.write_text(yaml.safe_dump({"model_list": []})) + claude_settings_path.write_text(json.dumps({"theme": "dark"})) + _silence_signal_handling(monkeypatch) + + fake_process = FakeProcess(pid=11111) + monkeypatch.setattr(commands_module, "launch_proxy", lambda *a, **k: fake_process) + monkeypatch.setattr(commands_module, "poll_liveliness", lambda *a, **k: None) + monkeypatch.setattr(commands_module, "allocate_free_port", lambda: 65432) + monkeypatch.setattr(commands_module, "terminate", lambda pid, **k: None) + monkeypatch.setattr(commands_module.secrets, "token_urlsafe", lambda n: "fixed-master-key") + + def fake_wait(self, timeout=None): + backup_path.write_text("not json at all {{{") + return True + + monkeypatch.setattr("threading.Event.wait", fake_wait) + + result = self.runner.invoke(up) + + assert result.exit_code == 0, result.output + assert "invalid or unexpected JSON" in result.output + assert not pid_record_path.exists() + def test_surfaces_clean_error_and_cleans_up_when_health_check_fails(self, monkeypatch, tmp_path): config_path, _log_path, claude_settings_path, backup_path, pid_record_path = _patch_paths(monkeypatch, tmp_path) config_path.write_text(yaml.safe_dump({"model_list": []})) @@ -175,6 +203,30 @@ class TestUpCommand: assert not backup_path.exists() assert json.loads(claude_settings_path.read_text()) == original_settings + def test_terminates_ephemeral_proxy_when_claude_settings_is_corrupt(self, monkeypatch, tmp_path): + """The health check can pass and the proxy can come up fine, but if + ~/.claude/settings.json turns out to be corrupt, the just-started proxy must not be left + running with no pid record -- exactly the leak `lite autoroute down` exists to clean up.""" + config_path, _log_path, claude_settings_path, backup_path, pid_record_path = _patch_paths(monkeypatch, tmp_path) + config_path.write_text(yaml.safe_dump({"model_list": []})) + claude_settings_path.write_text("not json at all {{{") + + fake_process = FakeProcess(pid=777) + terminate_calls = [] + monkeypatch.setattr(commands_module, "launch_proxy", lambda *a, **k: fake_process) + monkeypatch.setattr(commands_module, "poll_liveliness", lambda *a, **k: None) + monkeypatch.setattr(commands_module, "allocate_free_port", lambda: 23456) + monkeypatch.setattr(commands_module, "terminate", lambda pid, **k: terminate_calls.append(pid)) + monkeypatch.setattr(commands_module.secrets, "token_urlsafe", lambda n: "fixed-master-key") + + result = self.runner.invoke(up) + + assert result.exit_code != 0 + assert "invalid JSON" in result.output + assert terminate_calls == [777] + assert not pid_record_path.exists() + assert not backup_path.exists() + class TestDownCommand: def setup_method(self): @@ -213,3 +265,36 @@ class TestDownCommand: assert result.exit_code == 0, result.output assert "Nothing to restore." in result.output assert not claude_settings_path.exists() + + def test_clears_a_corrupt_pid_record_and_still_restores_settings(self, monkeypatch, tmp_path): + """down is specifically the crash-recovery path -- a pid file truncated by a mid-write + crash must not block it from clearing the record and restoring Claude settings anyway.""" + _config_path, _log_path, claude_settings_path, backup_path, pid_record_path = _patch_paths( + monkeypatch, tmp_path + ) + pid_record_path.parent.mkdir(parents=True, exist_ok=True) + pid_record_path.write_text("not json at all {{{") + original_settings = {"theme": "dark"} + write_backup(ClaudeBackupRecord(existed=True, content=original_settings), backup_path) + claude_settings_path.write_text(json.dumps({"env": {"ANTHROPIC_AUTH_TOKEN": "fixed-master-key"}})) + + result = self.runner.invoke(down) + + assert result.exit_code == 0, result.output + assert "invalid or unexpected JSON" in result.output + assert "Restored" in result.output + assert not pid_record_path.exists() + assert not backup_path.exists() + assert json.loads(claude_settings_path.read_text()) == original_settings + + def test_surfaces_clean_error_when_backup_is_corrupt(self, monkeypatch, tmp_path): + _config_path, _log_path, _claude_settings_path, backup_path, _pid_record_path = _patch_paths( + monkeypatch, tmp_path + ) + backup_path.parent.mkdir(parents=True, exist_ok=True) + backup_path.write_text("not json at all {{{") + + result = self.runner.invoke(down) + + assert result.exit_code != 0 + assert "invalid or unexpected JSON" in result.output diff --git a/tests/test_litellm/proxy/client/cli/autoroute/test_process.py b/tests/test_litellm/proxy/client/cli/autoroute/test_process.py index d182ebfffc2..8e7f355adc1 100644 --- a/tests/test_litellm/proxy/client/cli/autoroute/test_process.py +++ b/tests/test_litellm/proxy/client/cli/autoroute/test_process.py @@ -9,6 +9,7 @@ from litellm.proxy.client.cli.commands.autoroute import process as process_modul from litellm.proxy.client.cli.commands.autoroute.process import ( PidRecord, ProcessLaunchError, + UpError, allocate_free_port, clear_pid_record, is_running, @@ -66,6 +67,13 @@ class TestPidRecordRoundTrip: def test_read_missing_file_returns_none(self, tmp_path): assert read_pid_record(tmp_path / "missing.json") is None + def test_read_raises_clean_error_on_corrupt_content(self, tmp_path): + path = tmp_path / "pid.json" + path.write_text("not json at all {{{") + + with pytest.raises(UpError, match="invalid or unexpected JSON"): + read_pid_record(path) + def test_clear_removes_an_existing_record(self, tmp_path): path = tmp_path / "pid.json" write_pid_record(PidRecord(pid=1, port=1, config_path="a", log_path="b"), path)