From 24202feee61a1cf5348de524d6d66b8e2806fa9c Mon Sep 17 00:00:00 2001 From: dschlueter Date: Mon, 6 Jul 2026 16:57:47 +0200 Subject: [PATCH] feat: harden and extend the CLI (security, UX, robustness, tests) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sieben Verbesserungen; die Dateien überschneiden sich thematisch, daher ein Commit (jeder Commit bleibt grün: 115 Tests). - #1 --check ist scriptbar: Exit 0 wenn Container läuft und erreichbar, sonst 5 (check_exit_code / CheckResult). - #2 --force implementiert: Bypass eines belegten Locks mit Warnung (_container_lock) und stop_container(force=…) schluckt Inkonsistenzen. - #3 stille Trunkierung behoben: chat_completion_text liefert ChatReply (content + finish_reason); bei finish_reason=length Hinweis auf stderr, --chat gibt Exit 1 bei leerem Content zurück. - #4 keine vermeidbare Downtime: --change validiert den Modellpfad VOR dem Entfernen des laufenden Containers. - #5 Netzwerk dicht: Port-Publish standardmäßig nur auf 127.0.0.1 (--expose/expose für alle Interfaces), optionaler --api-key/api_key (Server --api-key + Bearer-Token auf allen Requests). - #6 --stream: Chat-Reply token-weise via SSE auf stdout (stream_chat). - #7 tests/test_actions.py: Orchestrierungs-Ebene (dry-run-Nebenwirkungen, Lock, validate-before-remove, chat/stream/exit-codes). Doku aktualisiert (Manpage, README, llama.cpp.config.example). Co-Authored-By: Claude Opus 4.8 --- README.md | 9 +++ llama.cpp.config.example | 6 ++ man/llamacppctl.1 | 17 ++++- src/llamacppctl/actions.py | 99 +++++++++++++++++++++++--- src/llamacppctl/cli.py | 17 +++++ src/llamacppctl/config.py | 6 ++ src/llamacppctl/docker_ops.py | 33 ++++++--- src/llamacppctl/http_ops.py | 89 +++++++++++++++++++++--- src/llamacppctl/main.py | 2 +- src/llamacppctl/schema.py | 15 ++++ tests/test_actions.py | 127 ++++++++++++++++++++++++++++++++++ tests/test_config.py | 24 +++++++ tests/test_docker_ops.py | 22 +++++- tests/test_http_ops.py | 51 +++++++++++++- 14 files changed, 485 insertions(+), 32 deletions(-) create mode 100644 tests/test_actions.py diff --git a/README.md b/README.md index c6e0168..380a8b8 100644 --- a/README.md +++ b/README.md @@ -98,6 +98,10 @@ llamacppctl --change --config llama.cpp.config --profile deepseek llamacppctl --chat --config llama.cpp.config --profile qwen35 \ -s "Du antwortest kurz." -p "Was ist 2+2?" +# Chat mit Live-Streaming und großem Antwortbudget (lange Texte) +llamacppctl --chat --config llama.cpp.config --profile qwen35 \ + --stream --max-tokens 8000 -p "Schreibe eine kurze Rede zum Jahreswechsel." + # System-Prompt aus einer lokalen Datei, User-Prompt als Literal llamacppctl --start --config llama.cpp.config --profile qwen35 \ --system-file ./prompts/coding.md -p "Refaktoriere diese Funktion." @@ -125,6 +129,11 @@ Für Details siehe [`docs/SECURITY_AND_OPERATIONS.md`](docs/SECURITY_AND_OPERATI der URL, erlauben standardmäßig keine Redirects (jeder Hop wird bei Aktivierung erneut validiert), erzwingen eine Content-Type-Allowlist und ein hartes Größenlimit sowohl über `Content-Length` als auch beim Streaming. +- Der Port wird standardmäßig **nur auf `127.0.0.1`** veröffentlicht — der + ungeauthentifizierte OpenAI-Endpoint ist also nicht im LAN erreichbar. + `--expose`/`expose = true` bindet auf alle Interfaces; dann sollte per + `--api-key`/`api_key` ein Schlüssel gesetzt werden (Server verlangt ihn, das + Tool sendet ihn als Bearer-Token). - `container_name` ist verpflichtend und muss pro parallel betriebenem Profil eindeutig sein, um Docker-Namenskollisionen und ungewollte Container-Übernahmen zu verhindern. diff --git a/llama.cpp.config.example b/llama.cpp.config.example index aa9f8c8..6940e7e 100644 --- a/llama.cpp.config.example +++ b/llama.cpp.config.example @@ -49,6 +49,12 @@ models_endpoint = /v1/models chat_endpoint = /v1/chat/completions timeout = 300 poll_interval = 2 +# Port nur auf localhost veröffentlichen (Default). expose = true bindet auf +# alle Interfaces (LAN) -> dann unbedingt api_key setzen. +expose = false +# api_key: leer = keine Authentifizierung. Gesetzt -> Server verlangt ihn und +# das Tool sendet ihn als Bearer-Token. +api_key = # Chat-Antwortbudget (wirkt auf --chat / --start-Antwort, nicht auf den Container). # Reasoning-Modelle brauchen viel Budget; für lange Texte hochsetzen. max_tokens = 2048 diff --git a/man/llamacppctl.1 b/man/llamacppctl.1 index ff78c7c..ca0eee8 100644 --- a/man/llamacppctl.1 +++ b/man/llamacppctl.1 @@ -23,7 +23,8 @@ an open port. .TP .B \-\-check Report Docker container status, HTTP health/model endpoint reachability, and -(if a user prompt is supplied) whether a live chat completion succeeds. +(if a user prompt is supplied) whether a live chat completion succeeds. Exits +0 when the container is running and reachable, otherwise 5 (scriptable). .TP .B \-\-stop Stop and remove the container. No-op if it does not exist. @@ -118,6 +119,16 @@ GPU device id passed to \fB\-\-gpus device=\fR. .TP .B \-\-restart\-policy \fIPOLICY\fR Docker \fB\-\-restart\fR policy. +.TP +.B \-\-expose\fR / \fB\-\-no\-expose +Publish the port on all host interfaces (LAN-reachable) or on loopback only. +Default: loopback only, so the unauthenticated API is not exposed to the +network. Config key: \fBexpose\fR. +.TP +.B \-\-api\-key \fIKEY\fR +Require an API key on the server (\fB\-\-api\-key\fR) and send it as a Bearer +token on every request. Config key: \fBapi_key\fR. Recommended together with +\fB\-\-expose\fR. .SS llama.cpp runtime parameters .B \-\-ctx\-size\fR, \fB\-\-n\-predict\fR, \fB\-\-temp\fR, \fB\-\-top\-p\fR, \fB\-\-top\-k\fR, \fB\-\-min\-p\fR, \fB\-\-repeat\-penalty\fR, @@ -143,6 +154,10 @@ answer, so a small budget yields empty content). Must be > 0. .B \-\-chat\-temp \fIFLOAT\fR Temperature for the chat request. If unset, the request omits temperature and the server's configured \fB\-\-temp\fR applies (single source of truth). +.TP +.B \-\-stream +Stream the \fB\-\-chat\fR reply token-by-token to stdout as it is generated, +instead of waiting for the full response. .SS Security / behavior flags (prompt input) .TP .B \-\-allow\-insecure\-http diff --git a/src/llamacppctl/actions.py b/src/llamacppctl/actions.py index 7072718..d0a3587 100644 --- a/src/llamacppctl/actions.py +++ b/src/llamacppctl/actions.py @@ -14,8 +14,10 @@ from __future__ import annotations import json import sys +from contextlib import contextmanager from dataclasses import asdict from pathlib import Path +from typing import Iterator from .docker_ops import ( DockerError, @@ -28,9 +30,9 @@ from .docker_ops import ( start_llama_container, stop_container, ) -from .http_ops import HttpError, base_url, chat_completion_text, wait_until_ready +from .http_ops import HttpError, base_url, chat_completion_text, stream_chat, wait_until_ready from .lock_ops import FileLock, LockError -from .schema import CheckResult, PromptConfig, ServerConfig +from .schema import ChatReply, CheckResult, PromptConfig, ServerConfig EXIT_OK = 0 EXIT_GENERAL = 1 @@ -40,6 +42,42 @@ EXIT_HTTP_NOT_READY = 5 EXIT_LOCK_BUSY = 6 +@contextmanager +def _container_lock(cfg: ServerConfig, args) -> Iterator[None]: + """Hold the container's file lock. With --force, a busy lock is bypassed + with a warning instead of aborting (for stuck/stale locks).""" + lock = FileLock(cfg.lock_file) + try: + lock.__enter__() + except LockError: + if not getattr(args, "force", False): + raise + print(f"warning: --force: proceeding despite busy lock {cfg.lock_file}", file=sys.stderr) + yield + return + try: + yield + finally: + lock.__exit__(None, None, None) + + +def _warn_if_truncated(reply: ChatReply, prompt_cfg: PromptConfig) -> None: + if reply.finish_reason == "length": + print( + f"Hinweis: Antwort bei max_tokens={prompt_cfg.max_tokens} abgeschnitten " + f"(finish_reason=length). Erhöhe --max-tokens für vollständige Ausgabe.", + file=sys.stderr, + ) + + +def check_exit_code(result: CheckResult) -> int: + """Map a CheckResult to a process exit code so --check is scriptable: + 0 = running and reachable, EXIT_HTTP_NOT_READY otherwise.""" + if result.container_running and result.http_ok: + return EXIT_OK + return EXIT_HTTP_NOT_READY + + def validate_model_path(cfg: ServerConfig) -> None: model_path = Path(cfg.model_path) if model_path.is_absolute(): @@ -100,7 +138,9 @@ def print_check_result(cfg: ServerConfig, result: CheckResult) -> None: def do_stop(cfg: ServerConfig, args) -> int: try: - removed = stop_container(cfg.container_name, remove=True) + removed = stop_container( + cfg.container_name, remove=True, force=getattr(args, "force", False) + ) except DockerError as exc: print(f"docker error: {exc}", file=sys.stderr) return EXIT_DOCKER_ERROR @@ -118,7 +158,7 @@ def do_start(cfg: ServerConfig, prompt_cfg: PromptConfig, args) -> int: print(format_command_for_display(build_run_command(cfg))) return EXIT_OK try: - with FileLock(cfg.lock_file): + with _container_lock(cfg, args): return _start_locked(cfg, prompt_cfg, args) except LockError as exc: print(str(exc), file=sys.stderr) @@ -154,9 +194,11 @@ def _start_locked(cfg: ServerConfig, prompt_cfg: PromptConfig, args) -> int: if prompt_cfg.user_prompt: try: - content = chat_completion_text(cfg, prompt_cfg) + reply = chat_completion_text(cfg, prompt_cfg) print("---- response ----") - print(content) + if reply.content: + print(reply.content) + _warn_if_truncated(reply, prompt_cfg) except HttpError as exc: print(f"chat request failed: {exc}", file=sys.stderr) @@ -169,7 +211,14 @@ def do_change(cfg: ServerConfig, prompt_cfg: PromptConfig, args) -> int: print(format_command_for_display(build_run_command(cfg))) return EXIT_OK try: - with FileLock(cfg.lock_file): + with _container_lock(cfg, args): + # Validate BEFORE removing the running container, so a bad model + # path / config does not cause avoidable downtime. + try: + validate_model_path(cfg) + except FileNotFoundError as exc: + print(str(exc), file=sys.stderr) + return EXIT_MODEL_PATH_MISSING remove_container(cfg.container_name) return _start_locked(cfg, prompt_cfg, args) except LockError as exc: @@ -184,10 +233,42 @@ def do_chat(cfg: ServerConfig, prompt_cfg: PromptConfig, args) -> int: if not prompt_cfg.user_prompt: print("--chat requires a user prompt", file=sys.stderr) return EXIT_GENERAL + + if getattr(args, "stream", False): + return _do_chat_stream(cfg, prompt_cfg, args) + try: - content = chat_completion_text(cfg, prompt_cfg) + reply = chat_completion_text(cfg, prompt_cfg) except HttpError as exc: print(f"chat request failed: {exc}", file=sys.stderr) return EXIT_HTTP_NOT_READY - print(content) + if reply.content: + print(reply.content) + _warn_if_truncated(reply, prompt_cfg) + if not reply.content and reply.finish_reason == "length": + return EXIT_GENERAL + return EXIT_OK + + +def _do_chat_stream(cfg: ServerConfig, prompt_cfg: PromptConfig, args) -> int: + got_content = False + finish_reason = "" + try: + for kind, value in stream_chat(cfg, prompt_cfg): + if kind == "content": + sys.stdout.write(value) + sys.stdout.flush() + got_content = True + elif kind == "finish": + finish_reason = value + except HttpError as exc: + if got_content: + print() + print(f"chat request failed: {exc}", file=sys.stderr) + return EXIT_HTTP_NOT_READY + if got_content: + print() + _warn_if_truncated(ChatReply(content="", finish_reason=finish_reason), prompt_cfg) + if not got_content and finish_reason == "length": + return EXIT_GENERAL return EXIT_OK diff --git a/src/llamacppctl/cli.py b/src/llamacppctl/cli.py index 6755c94..2a99f98 100644 --- a/src/llamacppctl/cli.py +++ b/src/llamacppctl/cli.py @@ -52,6 +52,18 @@ def build_parser() -> argparse.ArgumentParser: p.add_argument("--model-alias", help="Model alias exposed via the OpenAI-compatible API") p.add_argument("--gpu", dest="gpu_device", help="GPU device id passed to --gpus device=") p.add_argument("--restart-policy", help="Docker --restart policy") + p.add_argument( + "--expose", + dest="expose", + action="store_true", + default=None, + help="Publish the port on all interfaces (LAN-reachable). Default: loopback only", + ) + p.add_argument("--no-expose", dest="expose", action="store_false") + p.add_argument( + "--api-key", + help="API key required by the server and sent as a Bearer token on requests", + ) # --- llama.cpp runtime parameters --- p.add_argument("--ctx-size", type=int) @@ -105,6 +117,11 @@ def build_parser() -> argparse.ArgumentParser: type=float, help="Temperature for the chat request (default: use the server's configured --temp)", ) + p.add_argument( + "--stream", + action="store_true", + help="Stream the --chat reply token-by-token to stdout as it is generated", + ) # --- Behavior / diagnostics --- p.add_argument("--timeout", type=int, help="Readiness timeout in seconds") diff --git a/src/llamacppctl/config.py b/src/llamacppctl/config.py index bc00a5a..353f910 100644 --- a/src/llamacppctl/config.py +++ b/src/llamacppctl/config.py @@ -66,6 +66,8 @@ def builtin_defaults() -> dict: "cont_batching": "true", "no_context_shift": "true", "host": "0.0.0.0", + "expose": "false", # publish only on loopback unless true + "api_key": "", "health_endpoint": "/health", "models_endpoint": "/v1/models", "chat_endpoint": "/v1/chat/completions", @@ -130,6 +132,8 @@ def _apply_cli_overrides(merged: dict, args) -> None: "timeout": getattr(args, "timeout", None), "poll_interval": getattr(args, "poll_interval", None), "lock_file": getattr(args, "lock_file", None), + "expose": getattr(args, "expose", None), + "api_key": getattr(args, "api_key", None), } for key, value in overrides.items(): if value is not None: @@ -184,6 +188,8 @@ def build_server_config(merged: dict) -> ServerConfig: cont_batching=_to_bool(merged["cont_batching"], "cont_batching"), no_context_shift=_to_bool(merged["no_context_shift"], "no_context_shift"), host=str(merged["host"]), + expose=_to_bool(merged.get("expose", "false"), "expose"), + api_key=str(merged.get("api_key", "")), health_endpoint=str(merged["health_endpoint"]), models_endpoint=str(merged["models_endpoint"]), chat_endpoint=str(merged["chat_endpoint"]), diff --git a/src/llamacppctl/docker_ops.py b/src/llamacppctl/docker_ops.py index ae84077..46ccfaf 100644 --- a/src/llamacppctl/docker_ops.py +++ b/src/llamacppctl/docker_ops.py @@ -79,14 +79,21 @@ def remove_container(name: str) -> bool: return True -def stop_container(name: str, remove: bool = True) -> bool: - """Stop (and by default remove) a container. No-op if it does not exist.""" - if not container_exists(name): +def stop_container(name: str, remove: bool = True, force: bool = False) -> bool: + """Stop (and by default remove) a container. No-op if it does not exist. + + With force=True, skip the existence check and swallow docker errors (e.g. a + half-created / inconsistent container), returning False instead of raising. + """ + if not force and not container_exists(name): return False - if remove: - _run(["docker", "rm", "-f", name], check=True) - else: - _run(["docker", "stop", name], check=True) + cmd = ["docker", "rm", "-f", name] if remove else ["docker", "stop", name] + try: + _run(cmd, check=True) + except DockerError: + if force: + return False + raise return True @@ -106,6 +113,14 @@ def _resolve_model_path_in_container(cfg: ServerConfig) -> str: return f"/hf_home/{cfg.model_path}" +def _port_publish(cfg: ServerConfig) -> str: + """Port publish spec. Default binds to loopback only so the unauthenticated + API is not reachable from the LAN; cfg.expose opts into all interfaces.""" + if cfg.expose: + return f"{cfg.host_port}:{cfg.container_port}" + return f"127.0.0.1:{cfg.host_port}:{cfg.container_port}" + + def build_run_command(cfg: ServerConfig) -> list: """Build the `docker run` argument list for the llama.cpp server. @@ -122,7 +137,7 @@ def build_run_command(cfg: ServerConfig) -> list: "--restart", cfg.restart_policy, "-e", "HF_HOME=/hf_home", "-v", f"{cfg.hf_home}:/hf_home:ro", - "-p", f"{cfg.host_port}:{cfg.container_port}", + "-p", _port_publish(cfg), cfg.image, "-m", model_in_container, "--alias", cfg.model_alias, @@ -155,6 +170,8 @@ def build_run_command(cfg: ServerConfig) -> list: cmd.append("--kv-unified") if cfg.cont_batching: cmd.append("--cont-batching") + if cfg.api_key: + cmd += ["--api-key", cfg.api_key] cmd.extend(cfg.extra_args) return cmd diff --git a/src/llamacppctl/http_ops.py b/src/llamacppctl/http_ops.py index 3fe14e9..8fb2abf 100644 --- a/src/llamacppctl/http_ops.py +++ b/src/llamacppctl/http_ops.py @@ -2,12 +2,13 @@ from __future__ import annotations +import json import time -from typing import Optional +from typing import Iterator, Optional, Tuple import requests -from .schema import PromptConfig, ServerConfig +from .schema import ChatReply, PromptConfig, ServerConfig class HttpError(RuntimeError): @@ -18,6 +19,10 @@ def base_url(cfg: ServerConfig) -> str: return f"http://127.0.0.1:{cfg.host_port}/v1" +def _auth_headers(cfg: ServerConfig) -> dict: + return {"Authorization": f"Bearer {cfg.api_key}"} if cfg.api_key else {} + + def chat_url(cfg: ServerConfig) -> str: """Chat completions endpoint, honoring the configurable cfg.chat_endpoint (consistent with health_endpoint / models_endpoint). Always talks to the @@ -27,7 +32,11 @@ def chat_url(cfg: ServerConfig) -> str: def check_health(cfg: ServerConfig, timeout: float = 3.0) -> bool: try: - r = requests.get(f"http://127.0.0.1:{cfg.host_port}{cfg.health_endpoint}", timeout=timeout) + r = requests.get( + f"http://127.0.0.1:{cfg.host_port}{cfg.health_endpoint}", + headers=_auth_headers(cfg), + timeout=timeout, + ) return r.ok except requests.RequestException: return False @@ -35,7 +44,11 @@ def check_health(cfg: ServerConfig, timeout: float = 3.0) -> bool: def check_models(cfg: ServerConfig, timeout: float = 3.0) -> bool: try: - r = requests.get(f"http://127.0.0.1:{cfg.host_port}{cfg.models_endpoint}", timeout=timeout) + r = requests.get( + f"http://127.0.0.1:{cfg.host_port}{cfg.models_endpoint}", + headers=_auth_headers(cfg), + timeout=timeout, + ) return r.ok except requests.RequestException: return False @@ -69,18 +82,25 @@ def _chat_payload(cfg: ServerConfig, prompt_cfg: PromptConfig) -> dict: def chat_completion(cfg: ServerConfig, prompt_cfg: PromptConfig) -> requests.Response: """Low-level chat call returning the raw Response (never raises on HTTP status). Used by readiness polling, which must treat non-200 as 'not yet'.""" - return requests.post(chat_url(cfg), json=_chat_payload(cfg, prompt_cfg), timeout=10) + return requests.post( + chat_url(cfg), json=_chat_payload(cfg, prompt_cfg), headers=_auth_headers(cfg), timeout=10 + ) def chat_completion_text( cfg: ServerConfig, prompt_cfg: PromptConfig, timeout: float = 30.0 -) -> str: +) -> ChatReply: """High-level chat helper for --chat and the --start/--check post-checks. - Returns the assistant message content, raising HttpError on transport, - HTTP status, JSON decoding, or malformed-response failures.""" + Returns a ChatReply (content + finish_reason), raising HttpError on + transport, HTTP status, JSON decoding, or malformed-response failures.""" try: - resp = requests.post(chat_url(cfg), json=_chat_payload(cfg, prompt_cfg), timeout=timeout) + resp = requests.post( + chat_url(cfg), + json=_chat_payload(cfg, prompt_cfg), + headers=_auth_headers(cfg), + timeout=timeout, + ) except requests.RequestException as exc: raise HttpError(f"chat completion request failed: {exc}") from exc @@ -93,9 +113,56 @@ def chat_completion_text( raise HttpError("chat completion response was not valid JSON") from exc try: - return data["choices"][0]["message"]["content"] + choice = data["choices"][0] + message = choice["message"] except (KeyError, IndexError, TypeError) as exc: - raise HttpError("chat completion response missing choices[0].message.content") from exc + raise HttpError("chat completion response missing choices[0].message") from exc + + content = message.get("content") or "" + finish_reason = choice.get("finish_reason") or "" + return ChatReply(content=content, finish_reason=finish_reason) + + +def stream_chat( + cfg: ServerConfig, prompt_cfg: PromptConfig, timeout: Optional[tuple] = None +) -> Iterator[Tuple[str, str]]: + """Stream a chat completion via SSE, yielding (kind, value) events: + ("content", ) for each token chunk and a final + ("finish", ). Raises HttpError on transport/HTTP failures.""" + payload = _chat_payload(cfg, prompt_cfg) + payload["stream"] = True + try: + resp = requests.post( + chat_url(cfg), + json=payload, + headers=_auth_headers(cfg), + stream=True, + timeout=timeout or (10.0, 600.0), + ) + except requests.RequestException as exc: + raise HttpError(f"chat completion request failed: {exc}") from exc + + if resp.status_code != 200: + raise HttpError(f"chat completion returned HTTP {resp.status_code}: {resp.text[:500]}") + + finish_reason = "" + for line in resp.iter_lines(decode_unicode=True): + if not line or not line.startswith("data:"): + continue + chunk = line[len("data:"):].strip() + if chunk == "[DONE]": + break + try: + obj = json.loads(chunk) + choice = obj["choices"][0] + except (ValueError, KeyError, IndexError, TypeError): + continue + piece = (choice.get("delta") or {}).get("content") + if piece: + yield ("content", piece) + if choice.get("finish_reason"): + finish_reason = choice["finish_reason"] + yield ("finish", finish_reason) def wait_until_ready(cfg: ServerConfig, poll_prompt: PromptConfig) -> bool: diff --git a/src/llamacppctl/main.py b/src/llamacppctl/main.py index ed759f3..3bf16c1 100644 --- a/src/llamacppctl/main.py +++ b/src/llamacppctl/main.py @@ -84,7 +84,7 @@ def run(argv: Optional[Sequence[str]] = None) -> int: if args.check: result = actions.do_check(server_cfg, prompt_cfg, args) actions.print_check_result(server_cfg, result) - return 0 + return actions.check_exit_code(result) if args.stop: return actions.do_stop(server_cfg, args) if args.change: diff --git a/src/llamacppctl/schema.py b/src/llamacppctl/schema.py index e6d6209..ab057d7 100644 --- a/src/llamacppctl/schema.py +++ b/src/llamacppctl/schema.py @@ -53,6 +53,13 @@ class ServerConfig: poll_interval: float lock_file: Path + # Publish the port on all host interfaces (LAN-reachable) instead of just + # loopback. Default False keeps the unauthenticated API local-only. + expose: bool = False + # Optional API key; when set it is passed to the server (--api-key) and sent + # as a Bearer token on every request. + api_key: str = "" + extra_args: list = field(default_factory=list) @@ -68,6 +75,14 @@ class PromptConfig: stream: bool = False +@dataclass +class ChatReply: + """Result of a non-streaming chat completion.""" + + content: str + finish_reason: str = "" + + @dataclass class CheckResult: container_exists: bool diff --git a/tests/test_actions.py b/tests/test_actions.py new file mode 100644 index 0000000..400df48 --- /dev/null +++ b/tests/test_actions.py @@ -0,0 +1,127 @@ +import sys +from pathlib import Path +from types import SimpleNamespace + +sys.path.insert(0, str(Path(__file__).resolve().parents[1] / "src")) + +from llamacppctl import actions # noqa: E402 +from llamacppctl.http_ops import HttpError # noqa: E402 +from llamacppctl.schema import ChatReply, CheckResult # noqa: E402 +from tests.test_docker_ops import make_cfg # noqa: E402 + + +def _cfg(tmp_path): + return make_cfg(lock_file=tmp_path / "test.lock") + + +# --- dry-run must be a pure preview (no side effects) --------------------- + + +def test_do_start_dry_run_does_not_start(tmp_path, monkeypatch, capsys): + cfg = _cfg(tmp_path) + called = {"start": 0} + monkeypatch.setattr(actions, "start_llama_container", lambda c: called.__setitem__("start", 1)) + rc = actions.do_start(cfg, SimpleNamespace(), SimpleNamespace(dry_run=True)) + assert rc == actions.EXIT_OK + assert called["start"] == 0 + assert "docker run" in capsys.readouterr().out + + +def test_do_change_dry_run_does_not_remove(tmp_path, monkeypatch): + cfg = _cfg(tmp_path) + called = {"remove": 0} + monkeypatch.setattr(actions, "remove_container", lambda n: called.__setitem__("remove", 1)) + rc = actions.do_change(cfg, SimpleNamespace(), SimpleNamespace(dry_run=True)) + assert rc == actions.EXIT_OK + assert called["remove"] == 0 + + +# --- --change must validate BEFORE removing the running container ---------- + + +def test_do_change_validates_before_remove(tmp_path, monkeypatch): + cfg = _cfg(tmp_path) + called = {"remove": 0, "start": 0} + monkeypatch.setattr(actions, "remove_container", lambda n: called.__setitem__("remove", 1)) + monkeypatch.setattr(actions, "start_llama_container", lambda c: called.__setitem__("start", 1)) + + def missing(_cfg): + raise FileNotFoundError("model file not found: /x") + + monkeypatch.setattr(actions, "validate_model_path", missing) + rc = actions.do_change(cfg, SimpleNamespace(), SimpleNamespace(dry_run=False, force=False)) + assert rc == actions.EXIT_MODEL_PATH_MISSING + assert called["remove"] == 0 # container was NOT touched + assert called["start"] == 0 + + +# --- do_chat ------------------------------------------------------------- + + +def test_do_chat_prints_content(tmp_path, monkeypatch, capsys): + cfg = _cfg(tmp_path) + monkeypatch.setattr(actions, "chat_completion_text", lambda c, p: ChatReply("Hallo", "stop")) + prompt = SimpleNamespace(user_prompt="hi", max_tokens=2048) + rc = actions.do_chat(cfg, prompt, SimpleNamespace(stream=False)) + assert rc == actions.EXIT_OK + assert capsys.readouterr().out.strip() == "Hallo" + + +def test_do_chat_truncated_empty_warns_and_fails(tmp_path, monkeypatch, capsys): + cfg = _cfg(tmp_path) + monkeypatch.setattr(actions, "chat_completion_text", lambda c, p: ChatReply("", "length")) + prompt = SimpleNamespace(user_prompt="hi", max_tokens=128) + rc = actions.do_chat(cfg, prompt, SimpleNamespace(stream=False)) + assert rc == actions.EXIT_GENERAL + err = capsys.readouterr().err + assert "max_tokens=128" in err and "abgeschnitten" in err + + +def test_do_chat_http_error_returns_not_ready(tmp_path, monkeypatch): + cfg = _cfg(tmp_path) + + def boom(c, p): + raise HttpError("down") + + monkeypatch.setattr(actions, "chat_completion_text", boom) + prompt = SimpleNamespace(user_prompt="hi", max_tokens=2048) + rc = actions.do_chat(cfg, prompt, SimpleNamespace(stream=False)) + assert rc == actions.EXIT_HTTP_NOT_READY + + +def test_do_chat_stream(tmp_path, monkeypatch, capsys): + cfg = _cfg(tmp_path) + monkeypatch.setattr( + actions, + "stream_chat", + lambda c, p: iter([("content", "Hal"), ("content", "lo"), ("finish", "stop")]), + ) + prompt = SimpleNamespace(user_prompt="hi", max_tokens=2048) + rc = actions.do_chat(cfg, prompt, SimpleNamespace(stream=True)) + assert rc == actions.EXIT_OK + assert capsys.readouterr().out.strip() == "Hallo" + + +# --- exit-code mapping & force stop -------------------------------------- + + +def test_check_exit_code(): + ok = CheckResult(True, True, True, True, True, "u", "") + down = CheckResult(True, False, False, False, False, "u", "") + missing = CheckResult(False, False, False, False, False, "u", "") + assert actions.check_exit_code(ok) == actions.EXIT_OK + assert actions.check_exit_code(down) == actions.EXIT_HTTP_NOT_READY + assert actions.check_exit_code(missing) == actions.EXIT_HTTP_NOT_READY + + +def test_do_stop_passes_force(tmp_path, monkeypatch): + cfg = _cfg(tmp_path) + seen = {} + + def fake_stop(name, remove=True, force=False): + seen["force"] = force + return True + + monkeypatch.setattr(actions, "stop_container", fake_stop) + actions.do_stop(cfg, SimpleNamespace(force=True)) + assert seen["force"] is True diff --git a/tests/test_config.py b/tests/test_config.py index 7ed12f6..5054745 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -206,6 +206,30 @@ def test_chat_params_from_config(tmp_path): assert prompt_cfg.temperature == 0.6 +def test_expose_and_api_key_default_and_config(tmp_path): + cfg_path = _write_config(tmp_path) + server_cfg, _ = resolve_effective_config(_parse(["--start", "--config", str(cfg_path)])) + assert server_cfg.expose is False # loopback-only default + assert server_cfg.api_key == "" + + cfg = CONFIG_BASIC.replace( + "host = 0.0.0.0", "host = 0.0.0.0\nexpose = true\napi_key = s3cr3t" + ) + cfg_path2 = _write_config(tmp_path, cfg) + server_cfg, _ = resolve_effective_config(_parse(["--start", "--config", str(cfg_path2)])) + assert server_cfg.expose is True + assert server_cfg.api_key == "s3cr3t" + + +def test_expose_cli_overrides_config(tmp_path): + cfg = CONFIG_BASIC.replace("host = 0.0.0.0", "host = 0.0.0.0\nexpose = true") + cfg_path = _write_config(tmp_path, cfg) + server_cfg, _ = resolve_effective_config( + _parse(["--start", "--config", str(cfg_path), "--no-expose"]) + ) + assert server_cfg.expose is False + + def test_chat_params_cli_overrides_config(tmp_path): cfg = CONFIG_BASIC.replace( "poll_interval = 2", diff --git a/tests/test_docker_ops.py b/tests/test_docker_ops.py index 6ceab97..ce21f5c 100644 --- a/tests/test_docker_ops.py +++ b/tests/test_docker_ops.py @@ -116,7 +116,8 @@ def test_build_run_command_contains_key_flags(): assert "--name" in cmd assert "test_llm" in cmd assert "-p" in cmd - assert "8001:8000" in cmd + # loopback-only by default (expose=False) + assert "127.0.0.1:8001:8000" in cmd assert "--jinja" in cmd assert "--reasoning" in cmd assert "on" in cmd @@ -125,6 +126,25 @@ def test_build_run_command_contains_key_flags(): assert "--cont-batching" in cmd +def test_build_run_command_expose_binds_all_interfaces(): + cfg = make_cfg(expose=True) + cmd = build_run_command(cfg) + assert "8001:8000" in cmd + assert "127.0.0.1:8001:8000" not in cmd + + +def test_build_run_command_api_key_passed_to_server(): + cfg = make_cfg(api_key="secret123") + cmd = build_run_command(cfg) + assert "--api-key" in cmd + assert "secret123" in cmd + + +def test_build_run_command_no_api_key_by_default(): + cfg = make_cfg() + assert "--api-key" not in build_run_command(cfg) + + def test_build_run_command_respects_disabled_flags(): cfg = make_cfg(jinja=False, kv_unified=False, cont_batching=False, no_context_shift=False, fa=False) cmd = build_run_command(cfg) diff --git a/tests/test_http_ops.py b/tests/test_http_ops.py index 185dffb..0085d1a 100644 --- a/tests/test_http_ops.py +++ b/tests/test_http_ops.py @@ -64,7 +64,9 @@ def test_chat_completion_text_success(): mock_resp = MagicMock(status_code=200) mock_resp.json.return_value = {"choices": [{"message": {"content": "ok"}}]} mock_post.return_value = mock_resp - assert chat_completion_text(cfg, prompt_cfg) == "ok" + reply = chat_completion_text(cfg, prompt_cfg) + assert reply.content == "ok" + assert reply.finish_reason == "" def test_chat_completion_text_honors_chat_endpoint(): @@ -116,6 +118,53 @@ def test_chat_completion_text_malformed_response(): pass +def test_chat_completion_text_reports_finish_reason(): + cfg = make_cfg() + prompt_cfg = PromptConfig(user_prompt="hi") + with patch("requests.post") as mock_post: + mock_resp = MagicMock(status_code=200) + mock_resp.json.return_value = { + "choices": [{"finish_reason": "length", "message": {"content": ""}}] + } + mock_post.return_value = mock_resp + reply = chat_completion_text(cfg, prompt_cfg) + assert reply.content == "" + assert reply.finish_reason == "length" + + +def test_api_key_sent_as_bearer(): + cfg = make_cfg(api_key="secret123") + prompt_cfg = PromptConfig(user_prompt="hi") + with patch("requests.post") as mock_post: + mock_resp = MagicMock(status_code=200) + mock_resp.json.return_value = {"choices": [{"message": {"content": "x"}}]} + mock_post.return_value = mock_resp + chat_completion_text(cfg, prompt_cfg) + _, kwargs = mock_post.call_args + assert kwargs["headers"]["Authorization"] == "Bearer secret123" + + +def test_stream_chat_yields_content_and_finish(): + cfg = make_cfg() + prompt_cfg = PromptConfig(user_prompt="hi", stream=True) + lines = [ + 'data: {"choices":[{"delta":{"content":"Hal"}}]}', + 'data: {"choices":[{"delta":{"content":"lo"}}]}', + 'data: {"choices":[{"delta":{},"finish_reason":"stop"}]}', + "data: [DONE]", + ] + mock_resp = MagicMock(status_code=200) + mock_resp.iter_lines.return_value = iter(lines) + with patch("requests.post", return_value=mock_resp): + from llamacppctl.http_ops import stream_chat + + events = list(stream_chat(cfg, prompt_cfg)) + contents = [v for k, v in events if k == "content"] + finishes = [v for k, v in events if k == "finish"] + assert "".join(contents) == "Hallo" + assert finishes == ["stop"] + + def test_wait_until_ready_succeeds_immediately(): cfg = make_cfg(timeout=5, poll_interval=0.01) probe = PromptConfig(user_prompt="ping", max_tokens=1)