From 3e3dd1c15045e99fffd91ce0a46254df76e98c28 Mon Sep 17 00:00:00 2001 From: dschlueter Date: Fri, 10 Jul 2026 16:25:07 +0200 Subject: [PATCH] fix(cli): stop --print-effective-config from starting a container MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit argparse requires exactly one action, so the documented diagnostic form `--print-effective-config --config … --start` never took the early-return branch in main.run(): it printed the resolved config and then executed a real do_start(), silently replacing a running container with the [default] model. README, installation guide and manual all recommended that form. Make it an action in the mutually exclusive group. It can no longer be combined with --start/--check/--stop/--change/--chat (argparse error, exit 2), and it skips the docker_available() check, so it now really is the offline config check the docs promise. --dry-run remains a modifier and still requires a reachable daemon. Co-Authored-By: Claude Opus 4.8 --- README.md | 11 ++++--- docs/BEDIENUNGSANLEITUNG.md | 2 +- docs/INSTALL_FROM_ARCHIVE.md | 2 +- docs/SECURITY_AND_OPERATIONS.md | 27 ++++++++++------ src/llamacppctl/cli.py | 13 +++++--- src/llamacppctl/main.py | 11 +++---- tests/test_cli.py | 11 +++++-- tests/test_main.py | 57 +++++++++++++++++++++++++++++++++ 8 files changed, 104 insertions(+), 30 deletions(-) create mode 100644 tests/test_main.py diff --git a/README.md b/README.md index a780db4..197c931 100644 --- a/README.md +++ b/README.md @@ -44,9 +44,10 @@ Voraussetzungen auf dem Zielsystem: - Docker (CLI + laufender Daemon), für `--start`/--check`/--stop`/--change` - Netzwerkzugriff auf den Container-Host-Port für `--chat`/--check` -`--print-effective-config` und `--dry-run` benötigen keinen laufenden Docker-Daemon -für die reine Konfigurationsprüfung; `--start`/--check`/--stop`/--change` erfordern -einen erreichbaren Docker-Daemon. +`--print-effective-config` ist eine eigenständige, nebenwirkungsfreie Aktion und +benötigt keinen laufenden Docker-Daemon. `--start`/--check`/--stop`/--change` +erfordern einen erreichbaren Docker-Daemon — auch zusammen mit `--dry-run`, weil +die Verfügbarkeit geprüft wird, bevor der Trockenlauf greift. ## Konfiguration @@ -93,8 +94,8 @@ llamacppctl --start --config llama.cpp.config # Nur den geplanten docker-run-Befehl anzeigen, nichts ausführen llamacppctl --start --config llama.cpp.config --dry-run -# Effektive Konfiguration als JSON ausgeben -llamacppctl --print-effective-config --config llama.cpp.config --start +# Effektive Konfiguration als JSON ausgeben (startet nichts, braucht kein Docker) +llamacppctl --print-effective-config --config llama.cpp.config # Status prüfen llamacppctl --check --config llama.cpp.config diff --git a/docs/BEDIENUNGSANLEITUNG.md b/docs/BEDIENUNGSANLEITUNG.md index 87b2605..7b8a91c 100644 --- a/docs/BEDIENUNGSANLEITUNG.md +++ b/docs/BEDIENUNGSANLEITUNG.md @@ -49,7 +49,7 @@ Siehe [`INSTALL_FROM_ARCHIVE.md`](INSTALL_FROM_ARCHIVE.md). ```bash llamacppctl --help -llamacppctl --print-effective-config --config llama.cpp.config --start --dry-run +llamacppctl --print-effective-config --config llama.cpp.config ``` --- diff --git a/docs/INSTALL_FROM_ARCHIVE.md b/docs/INSTALL_FROM_ARCHIVE.md index 2c7c2ef..ab0e641 100644 --- a/docs/INSTALL_FROM_ARCHIVE.md +++ b/docs/INSTALL_FROM_ARCHIVE.md @@ -64,7 +64,7 @@ cp llama.cpp.config.example llama.cpp.config Auflösung prüfen, ohne etwas zu starten: ```bash -llamacppctl --print-effective-config --config llama.cpp.config --start --dry-run +llamacppctl --print-effective-config --config llama.cpp.config ``` Das zeigt die vollständig aufgelöste Konfiguration als JSON **und** das diff --git a/docs/SECURITY_AND_OPERATIONS.md b/docs/SECURITY_AND_OPERATIONS.md index 4eaa144..254d7d6 100644 --- a/docs/SECURITY_AND_OPERATIONS.md +++ b/docs/SECURITY_AND_OPERATIONS.md @@ -258,22 +258,29 @@ keinen vermeidbaren Ausfall verursacht. ## 6. `--dry-run` und `--print-effective-config` als Sicherheitswerkzeuge -- `--print-effective-config` gibt die vollständig aufgelöste Konfiguration - (Server- und Prompt-Konfiguration) als JSON aus, **bevor** irgendeine - Docker- oder HTTP-Aktion ausgeführt wird. Damit lässt sich prüfen, welche - Werte aus welcher Quelle (Defaults/[default]/[model.*]/CLI) tatsächlich - gewonnen haben, ohne einen Container anzufassen. +- `--print-effective-config` ist eine **eigene Aktion** in der + Mutually-Exclusive-Gruppe: sie gibt die vollständig aufgelöste Konfiguration + (Server- und Prompt-Konfiguration) als JSON aus und beendet sich. Damit lässt + sich prüfen, welche Werte aus welcher Quelle (Defaults/[default]/[model.*]/CLI) + tatsächlich gewonnen haben, ohne einen Container anzufassen. + + Bis einschließlich 0.1.0 war dies ein *Flag*. Da argparse immer eine Aktion + verlangt, führte `--print-effective-config --start` die Konfigurationsausgabe + **und anschließend einen echten `do_start()`** aus — der einen laufenden + Container stillschweigend ersetzte. Die Kombination ist jetzt ein + argparse-Fehler (Exit-Code 2). - `--dry-run` (in Kombination mit `--start`/--change`) zeigt den vollständig zusammengesetzten `docker run`-Befehl (Shell-quotiert zur Anzeige) an, **ohne** ihn auszuführen. Empfohlen vor jeder Änderung an einer Produktionskonfiguration, insbesondere nach Anpassungen an `llama.cpp.config`. -Beide Flags erfordern **keinen** erreichbaren Docker-Daemon für ihre reine -Ausgabe — `main.run()` prüft `docker_available()` derzeit vor der -Konfigurationsauflösung; auf einem Host ganz ohne Docker (z. B. zur reinen -Konfigurationsvalidierung) schlägt der Aufruf entsprechend mit einer -expliziten Fehlermeldung fehl statt still falsche Annahmen zu treffen. +`--print-effective-config` erfordert **keinen** erreichbaren Docker-Daemon; +`main.run()` überspringt die `docker_available()`-Prüfung für diese Aktion. Für +`--dry-run` gilt das **nicht**: es ist ein Modifikator von `--start`/`--change`, +und die Docker-Prüfung läuft vor der Konfigurationsauflösung. Auf einem Host ganz +ohne Docker schlägt `--start --dry-run` daher mit einer expliziten Fehlermeldung +fehl, statt still falsche Annahmen zu treffen. ## 7. Betriebsmodell auf einem dedizierten GPU-Host diff --git a/src/llamacppctl/cli.py b/src/llamacppctl/cli.py index 7230f42..d32da57 100644 --- a/src/llamacppctl/cli.py +++ b/src/llamacppctl/cli.py @@ -20,6 +20,14 @@ def build_parser() -> argparse.ArgumentParser: action.add_argument("--stop", action="store_true", help="Stop/remove the container") action.add_argument("--change", action="store_true", help="Stop, reconfigure, and restart") action.add_argument("--chat", action="store_true", help="Send a prompt to a running server") + # A diagnostic action, not a modifier: combining it with --start used to print + # the config and then really start the container. It is mutually exclusive with + # the other actions so it can never have a side effect. + action.add_argument( + "--print-effective-config", + action="store_true", + help="Print the fully merged configuration as JSON and exit. Touches nothing", + ) p.add_argument("--config", default="llama.cpp.config", help="Path to llama.cpp.config") p.add_argument("--profile", help="Model profile name: [model.] in config") @@ -146,11 +154,6 @@ def build_parser() -> argparse.ArgumentParser: p.add_argument("--log-lines", type=int, default=100) p.add_argument("--dry-run", action="store_true", help="Show the docker run command, do not execute it") p.add_argument("--force", action="store_true", help="Force stop/change despite inconsistencies") - p.add_argument( - "--print-effective-config", - action="store_true", - help="Print the fully merged configuration as JSON and exit (unless combined with an action)", - ) return p diff --git a/src/llamacppctl/main.py b/src/llamacppctl/main.py index b633b96..0c82d56 100644 --- a/src/llamacppctl/main.py +++ b/src/llamacppctl/main.py @@ -3,7 +3,7 @@ Orchestration order: 1. Parse CLI arguments (cli.build_parser) 2. Run semantic validation (cli.validate_args) - 3. Check that docker is available + 3. Check that docker is available (skipped for --print-effective-config) 4. Resolve prompt sources (prompt_io) under an InputPolicy built from CLI flags 5. Resolve effective server/prompt config (config.resolve_effective_config) 6. Dispatch to the requested action (actions.do_*) @@ -55,7 +55,9 @@ def run(argv: Optional[Sequence[str]] = None) -> int: args = parser.parse_args(argv) validate_args(args, parser) - if not docker_available(): + # --print-effective-config is purely diagnostic: it must work on a host + # without a Docker daemon, and it must never touch a container. + if not args.print_effective_config and not docker_available(): print("docker is not available on PATH (or the daemon is not reachable)", file=sys.stderr) return 1 @@ -74,10 +76,7 @@ def run(argv: Optional[Sequence[str]] = None) -> int: if args.print_effective_config: actions.print_effective_config(server_cfg, prompt_cfg) - # An action is always required by argparse; only exit early if the run - # is purely diagnostic (no action would actually do anything). - if not (args.start or args.check or args.stop or args.change or args.chat): - return 0 + return 0 if args.start: return actions.do_start(server_cfg, prompt_cfg, args) diff --git a/tests/test_cli.py b/tests/test_cli.py index 051035c..b792e41 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -147,9 +147,16 @@ def test_dry_run_flag(): assert args.dry_run is True -def test_print_effective_config_flag(): - args = parse(["--start", "--print-effective-config"]) +def test_print_effective_config_is_a_standalone_action(): + args = parse(["--print-effective-config"]) assert args.print_effective_config is True + assert args.start is False + + +def test_print_effective_config_cannot_be_combined_with_start(): + # Regression: as a plain flag this printed the config and then really started + # the container, silently replacing a running one. + parse_error(["--print-effective-config", "--start"]) def test_reasoning_choice_validated(): diff --git a/tests/test_main.py b/tests/test_main.py new file mode 100644 index 0000000..6b1e554 --- /dev/null +++ b/tests/test_main.py @@ -0,0 +1,57 @@ +import sys +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parents[1] / "src")) + +from llamacppctl import actions, main as main_mod # noqa: E402 + +CONFIG = """ +[default] +hf_home = /srv/models +model_path = qwen3/default.gguf +container_name = test_main +""" + + +def _config(tmp_path: Path) -> Path: + path = tmp_path / "llama.cpp.config" + path.write_text(CONFIG, encoding="utf-8") + return path + + +def test_print_effective_config_has_no_side_effects(tmp_path, monkeypatch, capsys): + """Regression: --print-effective-config used to fall through into do_start() + because argparse always required an action, silently replacing a running + container.""" + started = [] + monkeypatch.setattr(actions, "do_start", lambda *a, **k: started.append(1)) + monkeypatch.setattr(actions, "do_change", lambda *a, **k: started.append(1)) + monkeypatch.setattr(actions, "do_stop", lambda *a, **k: started.append(1)) + + rc = main_mod.run(["--print-effective-config", "--config", str(_config(tmp_path))]) + + assert rc == 0 + assert started == [] + assert '"container_name": "test_main"' in capsys.readouterr().out + + +def test_print_effective_config_works_without_docker(tmp_path, monkeypatch, capsys): + """It is a pure config check, so it must not require a reachable daemon.""" + def boom() -> bool: + raise AssertionError("docker_available() must not be consulted") + + monkeypatch.setattr(main_mod, "docker_available", boom) + + rc = main_mod.run(["--print-effective-config", "--config", str(_config(tmp_path))]) + + assert rc == 0 + assert '"hf_home": "/srv/models"' in capsys.readouterr().out + + +def test_start_still_requires_docker(tmp_path, monkeypatch, capsys): + monkeypatch.setattr(main_mod, "docker_available", lambda: False) + + rc = main_mod.run(["--start", "--config", str(_config(tmp_path))]) + + assert rc == 1 + assert "docker is not available" in capsys.readouterr().err