From 4b39d454e106ca83ad7f2355c603f46ee97eceaa Mon Sep 17 00:00:00 2001 From: jaylfc Date: Thu, 9 Jul 2026 01:15:40 +0100 Subject: [PATCH 01/17] fix(rknpu): a live rkllama port is not 'installed' unless it is a managed systemd service (#1755) Audit of how backends run on the Pi found the production rkllama was a hand-started bare process (PPID 1, no unit), never adopted by systemd, so it had no boot persistence and the Activity 'Restart AI Services' recovery (routes/system.py) could not restart it. Root cause: the store wrapper short-circuited with exit 0 the moment any process answered /api/tags on 7833/8080, before delegating to install-rknpu.sh, so a hand-start permanently blocked the systemd unit from ever being created. taOS manages backends as systemd services (boot persistence + recovery + cluster placement/migration all depend on it), so the idempotent short-circuit now also requires rkllama.service to be enabled. If it is not, we fall through to install-rknpu.sh, whose install_systemd_unit adopts the running instance under systemd (ExecStartPre reaps the orphan so the unit can bind the port). Also corrects the routes/system.py comment: rkllama is a system unit (per install-rknpu.sh), not a user unit. --- scripts/install-rkllama.sh | 27 ++++++++++++++++++++------- tinyagentos/routes/system.py | 4 ++-- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/scripts/install-rkllama.sh b/scripts/install-rkllama.sh index 0ab8b104d..faab12668 100755 --- a/scripts/install-rkllama.sh +++ b/scripts/install-rkllama.sh @@ -24,16 +24,29 @@ PROJECT_DIR="${1:-$(pwd)}" PORT="${TAOS_RKLLAMA_PORT:-7833}" LEGACY_PORT=8080 -# 1. Idempotent short-circuit: a live rkllama already satisfies the install. -# Require an rkllama/Ollama-shaped /api/tags body (a "models" key), not just -# any HTTP 200 -- another local service on these ports must not be mistaken -# for an installed rkllama. Mirrors _port_responds_with_rkllama() in the -# Python installer. +# 1. Idempotent short-circuit: a live AND MANAGED rkllama already satisfies the +# install. Two conditions, both required: +# (a) an rkllama/Ollama-shaped /api/tags body (a "models" key), not just any +# HTTP 200 -- another local service on these ports must not be mistaken +# for rkllama. Mirrors _port_responds_with_rkllama() in the Python installer. +# (b) the rkllama.service systemd unit is enabled. taOS manages backends as +# systemd services (boot persistence, the Activity "Restart AI Services" +# recovery in routes/system.py, and cluster placement/migration all +# depend on it). A hand-started bare process answers (a) but has no unit, +# which used to short-circuit the install and leave rkllama permanently +# unmanaged (no reboot survival, not systemctl-controllable). If the unit +# is missing we fall through to install-rknpu.sh, whose install_systemd_unit +# adopts the running instance under systemd (its ExecStartPre reaps the +# orphan so the unit can bind the port). for p in "$PORT" "$LEGACY_PORT"; do body="$(curl -fsS --max-time 2 "http://localhost:${p}/api/tags" 2>/dev/null || true)" if printf '%s' "$body" | grep -q '"models"'; then - echo "rkllama already running on port ${p} — nothing to install" - exit 0 + if systemctl is-enabled rkllama.service >/dev/null 2>&1; then + echo "rkllama already running on port ${p} and managed by systemd — nothing to install" + exit 0 + fi + echo "rkllama answers on port ${p} but is not a managed systemd service — running the installer to adopt it under systemd" + break fi done diff --git a/tinyagentos/routes/system.py b/tinyagentos/routes/system.py index e4e77a071..18ad88ffd 100644 --- a/tinyagentos/routes/system.py +++ b/tinyagentos/routes/system.py @@ -121,8 +121,8 @@ async def _emit_fail(msg: str) -> None: # AI-stack services a recovery action restarts (issue #1743). These are the # local inference backends that can stall independently of the controller: -# rkllama (NPU model server, installed as a user unit) and qmd (shared model -# provider, a system unit). The controller itself is deliberately NOT here -- +# rkllama (NPU model server, a system unit per install-rknpu.sh) and qmd (shared +# model provider, a system unit). The controller itself is deliberately NOT here -- # bouncing it is the separate /api/system/restart/prepare path. AI_STACK_UNITS = ("rkllama.service", "qmd.service") From d8342c32226be151ec618f586a9fe144f26220b3 Mon Sep 17 00:00:00 2001 From: jaylfc Date: Thu, 9 Jul 2026 01:47:40 +0100 Subject: [PATCH 02/17] docs(design): cluster-aware backend service management spec (#1757) * docs(design): cluster-aware backend service management spec Phase 1 (build now): mandatory managed-service manifest contract + CI lint, node-local backend service manager in the worker agent (ensure/self-heal/ start/stop/restart/health), #1743 rewired onto it, and a Backend Services surface in the Cluster app. Phase 2 (spec'd, deferred): controller cross-node reconciler + adaptive host-backend migration. Grounded in the backend-lifecycle audit (qmd managed, rkllama hand-started; #1755 store-wrapper trap fix). * docs(design): correct contract to extend existing lifecycle block (not a new managed block) --- .../cluster-backend-service-management.md | 112 ++++++++++++++++++ 1 file changed, 112 insertions(+) create mode 100644 docs/design/cluster-backend-service-management.md diff --git a/docs/design/cluster-backend-service-management.md b/docs/design/cluster-backend-service-management.md new file mode 100644 index 000000000..710748afc --- /dev/null +++ b/docs/design/cluster-backend-service-management.md @@ -0,0 +1,112 @@ +# Cluster-aware Backend Service Management — Design + +**Status:** Approved design (2026-07-09). Phase 1 to build now; Phase 2 spec'd, deferred. + +## Goal + +Installing a model-inference backend (rkllama, qmd, llama.cpp, vllm, ollama, ...) must reliably make it a **managed systemd service** on the node it lands on, and because backends migrate across cluster nodes, that management is **reconciled per node** (adopt/enable on arrival, stop/disable on departure). Backends expose a uniform start/stop/restart/health interface so recovery (#1743) and cluster placement/migration work the same everywhere. + +## Motivation (audit, 2026-07-09) + +- **qmd** is a proper managed systemd unit (installed + `enable --now` by `install-server.sh`, which every box runs). It survives reboots and is `systemctl`-restartable. +- **rkllama** on the production Pi was a **hand-started bare process** (PPID 1, launched from a login session, no unit) with no boot persistence, and the Activity "Restart AI Services" (#1743) could not restart it (`_restart_ai_unit` returns `"not installed"` when no unit exists). +- Root cause: the store wrapper `scripts/install-rkllama.sh` short-circuited `exit 0` the moment any process answered the port, before delegating to `install-rknpu.sh::install_systemd_unit()` — so a hand-start permanently blocked unit creation. **Fixed in PR #1755** (a live port only counts as installed if `rkllama.service` is enabled; otherwise fall through and adopt under systemd). That fix is the first brick of this design. +- The manifest contract is inconsistent: only 1 service manifest sets `auto_manage: true` (rkllama); rk-llama-cpp's installer creates a unit but its manifest says `auto_manage: false`; most services declare neither. taOS has no reliable declared view of which backends it manages. +- No reconciler ties "desired backend placement" to "managed unit state per node". The controller is HTTP/poll-only (health via `/api/tags` or `/health`, routing via LiteLLM) and delegates lifecycle to systemd, but nothing guarantees the unit exists. + +## Existing building blocks (reuse, do not reinvent) + +- **Worker agent + router** (`tinyagentos/cluster/router.py`, `worker_protocol.py`): the controller routes to each node's worker-agent HTTP API (`worker.url`); cross-host callers go through the agent, never dial a backend URL directly. Worker endpoints are protected by HMAC (`cluster/worker_auth.py`). +- **local_worker** (`cluster/local_worker.py`): the controller's own node acts as a worker in-process. +- **Capability heartbeat** (`routes/cluster_capability.py`): nodes report capabilities/status on a cadence. +- **Placement optimiser** (`cluster/optimiser.py`): advisory `PlacementSuggestion`s (not enforced). +- **Service migrator** (`cluster/service_migrator.py`): migrates LXC-hosted services; does NOT yet cover host systemd backends like rkllama/qmd. +- **#1743 recovery** (`routes/system.py::restart_ai_stack` / `_restart_ai_unit`): fails soft per unit, resolves user-vs-system scope with `systemctl cat`; today assumes a local unit. +- **Backend adapters** (`backend_adapters.py`): `OllamaCompatAdapter.health()` (rkllama), qmd `/health`. + +## Decisions (locked) + +1. **Scope:** Phase 1 now (uniform contract + per-node ensure/self-heal + #1743 rewire + Cluster UI); Phase 2 (cross-node reconciler + adaptive migration) spec'd and deferred. +2. **Enactment:** via the existing worker-agent API (controller sends desired state; agent runs local systemctl/install; controller node uses `local_worker` in-process). No SSH, no new per-node daemon. +3. **Migration cutover:** adaptive by capability — health-gated drain where the accelerator has headroom for both model loads; stop-source-first then start-target on single-NPU / single-GPU nodes; chosen from the capability map. +4. **Contract:** mandatory + CI-gated, with a one-time normalization of existing manifests and a short grandfather allowlist. + +## Design + +### A. The managed-service manifest contract + +Service manifests already carry a `lifecycle:` block (`backend_type`, `default_url`, `auto_manage`, `start_cmd`, `stop_cmd`, `startup_timeout_seconds`). We EXTEND that block (do not invent a new one) with `unit`, `scope`, and `health`, and require it on host-managed backends: + +```yaml +lifecycle: + backend_type: rkllama + default_url: http://localhost:7833 + auto_manage: true + unit: rkllama.service # systemd unit name (must match the installer's unit) + scope: system # system | user + health: + url: "http://localhost:7833/api/tags" + expect: '"models"' # substring assertion on a 200 body + # start/stop/restart default to `systemctl `; keep start_cmd/stop_cmd only for non-systemd + startup_timeout_seconds: 60 +``` + +- The installer that provisions the backend MUST create a unit whose name/port match the manifest. +- **Current state (audit):** of the 10 `category: llm-runtime` service manifests, only rkllama sets `auto_manage: true`; llama-cpp/mlc-llm/openllm/rk-llama-cpp set `false`; exo/ezrknpu/litellm/ollama/vllm set none; NONE declare a `unit`. +- **Rule:** any `llm-runtime` service with `auto_manage: true` MUST declare `unit` + `scope` + `health`. A grandfather allowlist covers `auto_manage: false` / not-yet-migrated ones. Normalize the host-managed backends first (rkllama, qmd, rk-llama-cpp), which already ship real systemd units (`rkllama.service`, `qmd.service`, `rkllamacpp.service`). +- **CI:** a new `scripts/check_manifests.py managed-lint` (the existing `scripts/audit-manifests.py` is not run in CI) added as a step to `.github/workflows/doc-gate.yml`. + +### B. Phase 1 — node-local Backend Service Manager (in the worker agent) + +New module `cluster/backend_services.py` + worker-agent endpoints (HMAC-auth), mirrored in `local_worker`: + +- `GET /worker/backends` → for each managed backend installed on this node: `{unit, enabled, active, health: {ok, detail}}`. +- `POST /worker/backends/{unit}/ensure` → idempotent: unit missing but backend installed → run the installer's systemd path / adopt an orphan (the installer `ExecStartPre` pkill reaps the bare process so the unit can bind the port); unit present → `enable --now`. +- `POST /worker/backends/{unit}/{start|stop|restart}` → systemctl verb, health-gated, fail-soft per unit (reuse the `_restart_ai_unit` scope-resolution + kill/reap-on-timeout logic, promoted into `backend_services`). + +**Self-heal:** on worker-agent start and on each capability heartbeat, reconcile installed managed backends → enabled units. This would have auto-adopted the Pi's hand-started rkllama. + +**#1743 rewire:** `restart_ai_stack` calls the node's `POST /worker/backends/{unit}/restart` (via `local_worker` for the controller node), so recovery works uniformly cluster-wide instead of assuming a local unit. `AI_STACK_UNITS` is derived from the node's managed manifests, not hardcoded. + +### C. Cluster-app UI surface + +**Read path:** each worker object in the existing `/api/cluster/workers` payload carries per-backend `{unit, enabled, active, health}` (sourced from the agent's `GET /worker/backends`, cached from the heartbeat) — the Cluster app already fetches `/api/cluster/workers`, so no new poll. **Action path:** a dedicated admin-gated `POST /api/cluster/workers/{node}/backends/{unit}/{start|stop|restart}` that the controller proxies to that node's worker agent (or `local_worker`). In `desktop/src/apps/ClusterApp.tsx`, each worker detail card gains a **Backend Services** subsection (replacing today's read-only `worker.backends` chips): per backend a name, a unit-state pill (enabled/active/failed), a health dot, and **admin-only** per-row Restart / Stop / Start actions plus a per-node "Restart AI Services" (the #1743 action, node-scoped). + +### D. CI gate + +A manifest-lint (extend `scripts/check_doc_gate.py` or a new `scripts/check_manifests.py`) fails any PR that adds/alters a backend service manifest without a valid `managed` block. Ships with a one-time normalization of current manifests and a grandfather allowlist. Unit-tested with valid/invalid/grandfathered fixtures. + +### E. Phase 2 (spec, defer build) — cross-node reconciler + +- Extend the capability/worker registry with `desired_backends` per node, fed by the placement `optimiser`. +- A controller reconcile loop diffs desired vs actual (from the `GET /worker/backends` heartbeat) and drives `ensure`/`stop` on the owning node's agent to converge. +- Migration cutover = adaptive (decision 3), extending `service_migrator` to host (non-LXC) backends: health-gated drain where headroom exists, else stop-source-first then start-target; roll back to source if the target never reports healthy within a timeout. + +### F. Error handling + +- Every action fails soft and reports per-unit: `not-installed`, `permission-denied` (polkit / interactive auth), `timeout` (kill + reap the child), `health-never-up`. Never a 500. +- Reconcile is idempotent and convergent; an unreachable node is skipped and retried on the next heartbeat, never blocking others. +- Migration rolls back to the source backend if the target does not report healthy within the cutover timeout. + +### G. Testing + +- **Manifest-lint:** unit tests over valid / invalid / grandfathered manifest fixtures. +- **Backend Service Manager:** unit tests with mocked `systemctl` (mirroring `tests/test_routes_system.py::TestRestartAiUnit`) covering ensure / start / stop / restart / health, adopt-orphan, and every fail-soft path. +- **Cluster UI:** component tests for the Backend Services rows (state pills, health, admin-gated actions), mocking the fetch. +- **Phase 2:** reconcile convergence tests against a fake worker agent (desired vs actual). +- **Acceptance:** on the Pi, adopt the hand-started rkllama under systemd via `ensure` and confirm it survives a simulated restart and is restartable from the Cluster UI. + +## Units / boundaries (independently testable) + +1. `managed` manifest schema + CI lint. +2. `cluster/backend_services.py` node backend-service manager + worker-agent endpoints (+ `local_worker`). +3. `#1743` rewire onto the node manager. +4. Cluster-app Backend Services UI + the `/api/cluster/workers` payload extension. +5. (Phase 2) controller reconcile loop + adaptive host-backend migration. + +Phase 1 = units 1-4. Phase 2 = unit 5, its own spec/plan. + +## Out of scope + +- Non-systemd platforms (macOS/Windows launchd/services) — Phase 1 is systemd/Linux nodes; the contract's overridable start/stop leaves room for a later adapter. +- Model-weight placement/distribution (taOSnet) — separate epic; this manages the *service*, not the weights. From d8c555af95f08d987979cc4977016b8961f9b460 Mon Sep 17 00:00:00 2001 From: jaylfc Date: Thu, 9 Jul 2026 02:10:48 +0100 Subject: [PATCH 03/17] feat(catalog): managed-backend-service manifest contract + CI lint (Phase 1.1) (#1756) * feat(catalog): managed-backend-service manifest contract + CI lint Phase 1, unit 1 of the cluster-aware backend service management design (docs/design/cluster-backend-service-management.md). A service manifest whose lifecycle.auto_manage is true must now declare lifecycle.unit (systemd unit), lifecycle.scope (system|user), and lifecycle.health (url + expect), so taOS can manage the backend as a systemd service uniformly across cluster nodes. Enforced by a new scripts/check_manifests.py managed-lint wired into the doc-gate workflow (the existing audit-manifests.py is a one-off migration checker, not in CI). Normalizes rkllama (the only auto_manage:true manifest today) to declare rkllama.service / system / api-tags health. auto_manage:false backends (rk-llama-cpp, llama-cpp, mlc-llm, openllm) are grandfathered until a later slice flips them on with proper fields. 8 lint tests incl. the real catalog. * fix(manifest-lint): harden auto_manage/YAML checks (fold Kilo review on #1756) - auto_manage truthy-but-not-bool (quoted "true", 1) is now flagged instead of silently bypassing the contract via an identity check. - an unparseable or non-mapping manifest.yaml is now an error, not a silent skip that read as clean. - validate lifecycle.unit ends in .service and lifecycle.health.expect is a string (documented as a substring match on the health body). 4 new tests; 12 total, real catalog clean. --- .github/workflows/doc-gate.yml | 5 + app-catalog/services/rkllama/manifest.yaml | 7 ++ scripts/check_manifests.py | 138 +++++++++++++++++++++ tests/test_check_manifests.py | 133 ++++++++++++++++++++ 4 files changed, 283 insertions(+) create mode 100644 scripts/check_manifests.py create mode 100644 tests/test_check_manifests.py diff --git a/.github/workflows/doc-gate.yml b/.github/workflows/doc-gate.yml index 3df485caf..7a5ec4d6b 100644 --- a/.github/workflows/doc-gate.yml +++ b/.github/workflows/doc-gate.yml @@ -35,3 +35,8 @@ jobs: env: BASE_REF: ${{ github.base_ref }} run: python scripts/check_doc_gate.py diff-gate --base "origin/$BASE_REF" + + - name: Managed backend manifest lint + run: | + python -m pip install --quiet pyyaml + python scripts/check_manifests.py managed-lint diff --git a/app-catalog/services/rkllama/manifest.yaml b/app-catalog/services/rkllama/manifest.yaml index ed4627598..942209823 100644 --- a/app-catalog/services/rkllama/manifest.yaml +++ b/app-catalog/services/rkllama/manifest.yaml @@ -31,6 +31,13 @@ lifecycle: backend_type: rkllama default_url: http://localhost:7833 auto_manage: true + # Managed as a systemd service (see scripts/install-rknpu.sh install_systemd_unit). + # taOS manages the unit per node: start/stop/restart default to `systemctl `. + unit: rkllama.service + scope: system + health: + url: "http://localhost:7833/api/tags" + expect: '"models"' keep_alive_minutes: 0 start_cmd: "systemctl start rkllama" stop_cmd: "systemctl stop rkllama" diff --git a/scripts/check_manifests.py b/scripts/check_manifests.py new file mode 100644 index 000000000..fb8ccb98d --- /dev/null +++ b/scripts/check_manifests.py @@ -0,0 +1,138 @@ +#!/usr/bin/env python3 +"""Catalog manifest lints that run in CI (separate from audit-manifests.py, +which is a one-off migration checker and is not wired into CI). + +Currently implements the managed-backend-service contract lint (Phase 1 of the +cluster-aware backend service management design, +docs/design/cluster-backend-service-management.md): + + A service manifest whose ``lifecycle.auto_manage`` is true MUST declare + ``lifecycle.unit`` (systemd unit name), ``lifecycle.scope`` ("system" or + "user"), and ``lifecycle.health`` with ``url`` + ``expect``. This is what + lets taOS treat the backend as a managed systemd service uniformly across + cluster nodes (boot persistence, the Activity "Restart AI Services" + recovery, and placement/migration all depend on it). Without the lint a + backend can ship claiming auto_manage while giving taOS no unit to manage, + which is how the production rkllama ended up a hand-started orphan. + +Usage: + python scripts/check_manifests.py managed-lint [--root app-catalog] +""" +from __future__ import annotations + +import argparse +import sys +from pathlib import Path + +import yaml + +# Services that declare auto_manage but are legitimately not yet a systemd unit +# on the host (or are managed by a non-manifest installer). Empty today; add an +# id here with a one-line reason only as a deliberate, temporary exception. +GRANDFATHER: dict[str, str] = {} + +VALID_SCOPES = {"system", "user"} + + +def lint_managed(root: Path) -> list[str]: + """Return a list of human-readable errors for the managed-service contract. + + ``lifecycle.health.expect`` is a literal substring matched against the + backend's health-endpoint response body (a 200 body must contain it). + """ + errors: list[str] = [] + services_dir = root / "services" + for manifest in sorted(services_dir.glob("*/manifest.yaml")): + sid_dir = manifest.parent.name + # An unparseable or non-mapping manifest is an error, not a silent skip + # (silently skipping gave a false "clean"). + try: + data = yaml.safe_load(manifest.read_text()) + except yaml.YAMLError as exc: + first = str(exc).splitlines()[0] if str(exc) else "parse error" + errors.append(f"{sid_dir}: manifest.yaml is not valid YAML ({first[:120]})") + continue + if not isinstance(data, dict): + errors.append(f"{sid_dir}: manifest.yaml top-level is not a mapping") + continue + + sid = str(data.get("id") or sid_dir) + lifecycle = data.get("lifecycle") + if not isinstance(lifecycle, dict): + continue + + auto_manage = lifecycle.get("auto_manage") + if not auto_manage: + continue # None / False / 0 / "" -> not claiming to be managed + + if sid in GRANDFATHER: + continue + + # Truthy but not a real bool (e.g. quoted "true", 1) would otherwise slip + # past an identity check and bypass the contract -- flag it, and still + # enforce the rest since it is clearly meant to be managed. + if auto_manage is not True: + errors.append( + f"{sid}: lifecycle.auto_manage must be a boolean true, got {auto_manage!r}" + ) + + missing: list[str] = [] + unit = lifecycle.get("unit") + if not unit: + missing.append("lifecycle.unit") + elif not str(unit).endswith(".service"): + errors.append( + f"{sid}: lifecycle.unit '{unit}' should be a systemd unit name ending in .service" + ) + + scope = lifecycle.get("scope") + if not scope: + missing.append("lifecycle.scope") + elif scope not in VALID_SCOPES: + errors.append( + f"{sid}: lifecycle.scope is '{scope}', must be one of {sorted(VALID_SCOPES)}" + ) + + health = lifecycle.get("health") + if not isinstance(health, dict): + missing.append("lifecycle.health") + else: + if not health.get("url"): + missing.append("lifecycle.health.url") + expect = health.get("expect") + if not expect: + missing.append("lifecycle.health.expect") + elif not isinstance(expect, str): + errors.append( + f"{sid}: lifecycle.health.expect must be a string (a substring " + f"matched against the health response body), got {type(expect).__name__}" + ) + + if missing: + errors.append( + f"{sid}: lifecycle.auto_manage is true but is missing " + f"{', '.join(missing)} (managed backends must declare their " + f"systemd unit + health so taOS can manage them per node)" + ) + return errors + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("command", choices=["managed-lint"]) + parser.add_argument("--root", default="app-catalog", help="catalog root dir") + args = parser.parse_args(argv) + + root = Path(args.root) + errors = lint_managed(root) + if errors: + print("MANIFEST-LINT FAIL (managed-service contract):", file=sys.stderr) + for e in errors: + print(f" - {e}", file=sys.stderr) + return 1 + print("manifest managed-lint: clean") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tests/test_check_manifests.py b/tests/test_check_manifests.py new file mode 100644 index 000000000..04a245eff --- /dev/null +++ b/tests/test_check_manifests.py @@ -0,0 +1,133 @@ +"""Tests for the managed-service manifest contract lint +(scripts/check_manifests.py).""" +from __future__ import annotations + +import importlib.util +from pathlib import Path + +import yaml + +_SPEC = importlib.util.spec_from_file_location( + "check_manifests", + Path(__file__).resolve().parent.parent / "scripts" / "check_manifests.py", +) +check_manifests = importlib.util.module_from_spec(_SPEC) +_SPEC.loader.exec_module(check_manifests) # type: ignore[union-attr] + + +def _write(root: Path, sid: str, manifest: dict) -> None: + d = root / "services" / sid + d.mkdir(parents=True, exist_ok=True) + (d / "manifest.yaml").write_text(yaml.safe_dump(manifest)) + + +def _managed_ok(sid: str = "rkllama") -> dict: + return { + "id": sid, + "type": "service", + "category": "llm-runtime", + "lifecycle": { + "backend_type": "rkllama", + "auto_manage": True, + "unit": f"{sid}.service", + "scope": "system", + "health": {"url": "http://localhost:7833/api/tags", "expect": '"models"'}, + }, + } + + +def test_valid_managed_manifest_passes(tmp_path: Path) -> None: + _write(tmp_path, "rkllama", _managed_ok()) + assert check_manifests.lint_managed(tmp_path) == [] + + +def test_auto_manage_without_unit_fails(tmp_path: Path) -> None: + m = _managed_ok() + del m["lifecycle"]["unit"] + _write(tmp_path, "rkllama", m) + errors = check_manifests.lint_managed(tmp_path) + assert len(errors) == 1 + assert "lifecycle.unit" in errors[0] + + +def test_auto_manage_without_health_fails(tmp_path: Path) -> None: + m = _managed_ok() + del m["lifecycle"]["health"] + _write(tmp_path, "rkllama", m) + errors = check_manifests.lint_managed(tmp_path) + assert "lifecycle.health" in errors[0] + + +def test_invalid_scope_fails(tmp_path: Path) -> None: + m = _managed_ok() + m["lifecycle"]["scope"] = "root" + _write(tmp_path, "rkllama", m) + errors = check_manifests.lint_managed(tmp_path) + assert any("scope is 'root'" in e for e in errors) + + +def test_non_managed_service_is_ignored(tmp_path: Path) -> None: + # auto_manage false -> not claiming managed, no unit/health required + m = { + "id": "rk-llama-cpp", + "type": "service", + "category": "llm-runtime", + "lifecycle": {"backend_type": "openai-compatible", "auto_manage": False}, + } + _write(tmp_path, "rk-llama-cpp", m) + assert check_manifests.lint_managed(tmp_path) == [] + + +def test_service_without_lifecycle_is_ignored(tmp_path: Path) -> None: + _write(tmp_path, "ollama", {"id": "ollama", "type": "service"}) + assert check_manifests.lint_managed(tmp_path) == [] + + +def test_grandfathered_service_is_skipped(tmp_path: Path, monkeypatch) -> None: + m = _managed_ok("legacy-backend") + del m["lifecycle"]["unit"] # would normally fail + _write(tmp_path, "legacy-backend", m) + monkeypatch.setattr( + check_manifests, "GRANDFATHER", {"legacy-backend": "pre-systemd, tracked in #NNNN"} + ) + assert check_manifests.lint_managed(tmp_path) == [] + + +def test_non_bool_auto_manage_is_flagged_and_enforced(tmp_path: Path) -> None: + # A quoted "true" must not slip past the identity check. + m = _managed_ok() + m["lifecycle"]["auto_manage"] = "true" + _write(tmp_path, "rkllama", m) + errors = check_manifests.lint_managed(tmp_path) + assert any("must be a boolean true" in e for e in errors) + + +def test_unparseable_yaml_is_an_error(tmp_path: Path) -> None: + d = tmp_path / "services" / "broken" + d.mkdir(parents=True) + (d / "manifest.yaml").write_text("id: broken\nlifecycle: {: bad") + errors = check_manifests.lint_managed(tmp_path) + assert any("not valid YAML" in e for e in errors) + + +def test_unit_without_service_suffix_is_flagged(tmp_path: Path) -> None: + m = _managed_ok() + m["lifecycle"]["unit"] = "rkllama" # missing .service + _write(tmp_path, "rkllama", m) + errors = check_manifests.lint_managed(tmp_path) + assert any("ending in .service" in e for e in errors) + + +def test_non_string_expect_is_flagged(tmp_path: Path) -> None: + m = _managed_ok() + m["lifecycle"]["health"]["expect"] = ["models"] + _write(tmp_path, "rkllama", m) + errors = check_manifests.lint_managed(tmp_path) + assert any("expect must be a string" in e for e in errors) + + +def test_real_catalog_is_clean() -> None: + """The shipped app-catalog must pass the managed-service lint.""" + root = Path(__file__).resolve().parent.parent / "app-catalog" + errors = check_manifests.lint_managed(root) + assert errors == [], "real catalog failed managed-lint:\n" + "\n".join(errors) From 8e901b4581c12297509596d7460645cbb92dad6a Mon Sep 17 00:00:00 2001 From: jaylfc Date: Thu, 9 Jul 2026 03:12:06 +0100 Subject: [PATCH 04/17] feat(cluster): node-local backend service manager core (Phase 1.2) (#1758) Additive module tinyagentos/cluster/backend_services.py for the cluster-aware backend service management design. Reusable, node-agnostic helpers: - load_managed_backends(catalog_root): the managed backends declared in manifests (lifecycle.auto_manage + unit/scope/health). - resolve_scope / unit_state: user-vs-system scope + installed/enabled/active. - service_action(unit, verb): fail-soft systemctl start/stop/restart, promoted and generalised from routes/system.py::_restart_ai_unit (kill+reap on timeout, 'not installed' when no unit). - health_probe(url, expect): httpx GET, substring assertion on the 200 body. No existing behaviour changes; the #1743 recovery endpoint (unit 3) and the worker-agent backend endpoints (unit 2b) will build on these. 12 tests (mocked systemctl + httpx). --- tests/test_backend_services.py | 199 ++++++++++++++++++++++++ tinyagentos/cluster/backend_services.py | 193 +++++++++++++++++++++++ 2 files changed, 392 insertions(+) create mode 100644 tests/test_backend_services.py create mode 100644 tinyagentos/cluster/backend_services.py diff --git a/tests/test_backend_services.py b/tests/test_backend_services.py new file mode 100644 index 000000000..59523fb1e --- /dev/null +++ b/tests/test_backend_services.py @@ -0,0 +1,199 @@ +"""Tests for the node-local backend service manager +(tinyagentos/cluster/backend_services.py).""" +from __future__ import annotations + +import asyncio +from pathlib import Path + +import pytest +import yaml + +from tinyagentos.cluster import backend_services as bs + + +# -------------------------------------------------------------------------- +# load_managed_backends +# -------------------------------------------------------------------------- + +def _write_manifest(root: Path, sid: str, manifest: dict) -> None: + d = root / "services" / sid + d.mkdir(parents=True, exist_ok=True) + (d / "manifest.yaml").write_text(yaml.safe_dump(manifest)) + + +def test_load_managed_backends_returns_only_managed(tmp_path: Path) -> None: + _write_manifest(tmp_path, "rkllama", { + "id": "rkllama", + "lifecycle": { + "auto_manage": True, "unit": "rkllama.service", "scope": "system", + "health": {"url": "http://localhost:7833/api/tags", "expect": '"models"'}, + }, + }) + _write_manifest(tmp_path, "rk-llama-cpp", { + "id": "rk-llama-cpp", "lifecycle": {"auto_manage": False}, + }) + _write_manifest(tmp_path, "no-lifecycle", {"id": "no-lifecycle"}) + + backends = bs.load_managed_backends(tmp_path) + assert len(backends) == 1 + b = backends[0] + assert b.id == "rkllama" + assert b.unit == "rkllama.service" + assert b.scope == "system" + assert b.health_url == "http://localhost:7833/api/tags" + assert b.health_expect == '"models"' + + +def test_load_skips_managed_without_unit_or_scope(tmp_path: Path) -> None: + _write_manifest(tmp_path, "broken", { + "id": "broken", "lifecycle": {"auto_manage": True, "scope": "bogus"}, + }) + assert bs.load_managed_backends(tmp_path) == [] + + +# -------------------------------------------------------------------------- +# resolve_scope / unit_state / service_action (mocked systemctl) +# -------------------------------------------------------------------------- + +class _FakeProc: + def __init__(self, returncode: int, stderr: bytes = b""): + self.returncode = returncode + self._stderr = stderr + + async def communicate(self): + return (b"", self._stderr) + + async def wait(self): + return self.returncode + + def kill(self): + pass + + +@pytest.mark.asyncio +async def test_resolve_scope_prefers_manifest_scope(monkeypatch) -> None: + calls: list[list[str]] = [] + + async def fake_rc(args): + calls.append(args) + # unit exists in system scope only + return 0 if "--user" not in args else 1 + + monkeypatch.setattr(bs, "_rc", fake_rc) + scope = await bs.resolve_scope("rkllama.service", prefer="system") + assert scope == "system" + # prefer=system means the first cat probe is the system (no --user) one + assert "--user" not in calls[0] + + +@pytest.mark.asyncio +async def test_unit_state_reports_enabled_active(monkeypatch) -> None: + monkeypatch.setenv("INVOCATION_ID", "test") + monkeypatch.setattr(bs, "_rc", lambda args: _coro(0)) + state = await bs.unit_state("rkllama.service", prefer="system") + assert state["installed"] is True + assert state["enabled"] is True + assert state["active"] is True + assert state["scope"] == "system" + + +@pytest.mark.asyncio +async def test_unit_state_not_installed(monkeypatch) -> None: + monkeypatch.setenv("INVOCATION_ID", "test") + monkeypatch.setattr(bs, "_rc", lambda args: _coro(1)) # cat fails both scopes + state = await bs.unit_state("ghost.service") + assert state["installed"] is False + + +@pytest.mark.asyncio +async def test_service_action_restart_ok(monkeypatch) -> None: + monkeypatch.setenv("INVOCATION_ID", "test") + monkeypatch.setattr(bs, "_rc", lambda args: _coro(0)) # scope resolves + monkeypatch.setattr(asyncio, "create_subprocess_exec", + lambda *a, **k: _coro(_FakeProc(0))) + result = await bs.service_action("rkllama.service", "restart", prefer="system") + assert result == {"unit": "rkllama.service", "ok": True, "scope": "system"} + + +@pytest.mark.asyncio +async def test_service_action_failure_captures_stderr(monkeypatch) -> None: + monkeypatch.setenv("INVOCATION_ID", "test") + monkeypatch.setattr(bs, "_rc", lambda args: _coro(0)) + monkeypatch.setattr(asyncio, "create_subprocess_exec", + lambda *a, **k: _coro(_FakeProc(1, b"Interactive authentication required"))) + result = await bs.service_action("qmd.service", "restart") + assert result["ok"] is False + assert "Interactive authentication required" in result["detail"] + + +@pytest.mark.asyncio +async def test_service_action_not_installed(monkeypatch) -> None: + monkeypatch.setenv("INVOCATION_ID", "test") + monkeypatch.setattr(bs, "_rc", lambda args: _coro(1)) # cat fails -> no scope + result = await bs.service_action("ghost.service", "restart") + assert result == {"unit": "ghost.service", "ok": False, "detail": "not installed"} + + +@pytest.mark.asyncio +async def test_service_action_rejects_bad_verb() -> None: + with pytest.raises(ValueError): + await bs.service_action("x.service", "nuke") + + +# -------------------------------------------------------------------------- +# health_probe (mocked httpx) +# -------------------------------------------------------------------------- + +class _FakeResp: + def __init__(self, status_code: int, text: str): + self.status_code = status_code + self.text = text + + +class _FakeClient: + def __init__(self, resp=None, exc=None): + self._resp = resp + self._exc = exc + + async def __aenter__(self): + return self + + async def __aexit__(self, *a): + return False + + async def get(self, url): + if self._exc: + raise self._exc + return self._resp + + +@pytest.mark.asyncio +async def test_health_probe_ok(monkeypatch) -> None: + monkeypatch.setattr(bs.httpx, "AsyncClient", + lambda *a, **k: _FakeClient(_FakeResp(200, '{"models": []}'))) + assert await bs.health_probe("http://x/api/tags", '"models"') == {"ok": True} + + +@pytest.mark.asyncio +async def test_health_probe_missing_marker(monkeypatch) -> None: + monkeypatch.setattr(bs.httpx, "AsyncClient", + lambda *a, **k: _FakeClient(_FakeResp(200, "{}"))) + r = await bs.health_probe("http://x/api/tags", '"models"') + assert r["ok"] is False + + +@pytest.mark.asyncio +async def test_health_probe_connection_error(monkeypatch) -> None: + monkeypatch.setattr(bs.httpx, "AsyncClient", + lambda *a, **k: _FakeClient(exc=RuntimeError("refused"))) + r = await bs.health_probe("http://x/api/tags", "") + assert r["ok"] is False + assert "refused" in r["detail"] + + +# -------------------------------------------------------------------------- +# helper +# -------------------------------------------------------------------------- + +async def _coro(value): + return value diff --git a/tinyagentos/cluster/backend_services.py b/tinyagentos/cluster/backend_services.py new file mode 100644 index 000000000..8465f536c --- /dev/null +++ b/tinyagentos/cluster/backend_services.py @@ -0,0 +1,193 @@ +"""Node-local backend service manager. + +Phase 1, unit 2 of the cluster-aware backend service management design +(docs/design/cluster-backend-service-management.md). Reusable, node-agnostic +helpers that: + + * read the managed-backend contract from catalog manifests + (``lifecycle.auto_manage`` + ``unit`` / ``scope`` / ``health``), + * resolve a systemd unit's scope (user vs system), + * report a unit's state (installed / enabled / active) and health, and + * run a fail-soft service action (start / stop / restart). + +The systemctl logic is promoted from ``routes/system.py::_restart_ai_unit`` and +generalised to any verb so the #1743 recovery endpoint and the worker-agent +backend endpoints can share one implementation. This module is additive: it +does not change any existing behaviour on its own. +""" +from __future__ import annotations + +import asyncio +import os +from dataclasses import dataclass +from pathlib import Path + +import httpx +import yaml + +VALID_SCOPES = {"system", "user"} + + +@dataclass(frozen=True) +class ManagedBackend: + """A backend the node manages as a systemd service (from its manifest).""" + + id: str + unit: str + scope: str # "system" | "user" + health_url: str + health_expect: str + + +def load_managed_backends(catalog_root: Path) -> list[ManagedBackend]: + """Return the managed backends declared under ``catalog_root/services``. + + Only manifests with ``lifecycle.auto_manage: true`` and a valid + ``unit`` + ``scope`` are returned; the manifest lint + (scripts/check_manifests.py) guarantees those fields exist for any + auto-managed service, so a manifest missing them is skipped here rather + than raised. + """ + out: list[ManagedBackend] = [] + services_dir = catalog_root / "services" + for manifest in sorted(services_dir.glob("*/manifest.yaml")): + try: + data = yaml.safe_load(manifest.read_text()) + except yaml.YAMLError: + continue + if not isinstance(data, dict): + continue + lifecycle = data.get("lifecycle") + if not isinstance(lifecycle, dict) or lifecycle.get("auto_manage") is not True: + continue + unit = lifecycle.get("unit") + scope = lifecycle.get("scope") + if not unit or scope not in VALID_SCOPES: + continue + health = lifecycle.get("health") if isinstance(lifecycle.get("health"), dict) else {} + out.append( + ManagedBackend( + id=str(data.get("id") or manifest.parent.name), + unit=str(unit), + scope=str(scope), + health_url=str(health.get("url", "")), + health_expect=str(health.get("expect", "")), + ) + ) + return out + + +def _systemd_present() -> bool: + return bool(os.environ.get("INVOCATION_ID") or os.path.exists("/run/systemd/system")) + + +async def _rc(args: list[str]) -> int: + proc = await asyncio.create_subprocess_exec( + *args, + stdout=asyncio.subprocess.DEVNULL, + stderr=asyncio.subprocess.DEVNULL, + ) + await proc.wait() + return proc.returncode + + +async def resolve_scope(unit: str, prefer: str | None = None) -> str | None: + """Return the scope ("user"/"system") a unit file exists in, else None. + + Uses ``systemctl [--user] cat`` (returns 0 iff the unit file exists) so a + dead/crashed unit is still resolved, unlike an is-active probe. ``prefer`` + (from the manifest ``lifecycle.scope``) is tried first. + """ + order: list[str] = [] + if prefer in VALID_SCOPES: + order.append(prefer) # type: ignore[arg-type] + for s in ("user", "system"): + if s not in order: + order.append(s) + for scope in order: + flag = ["--user"] if scope == "user" else [] + if await _rc(["systemctl", *flag, "cat", unit]) == 0: + return scope + return None + + +async def unit_state(unit: str, prefer: str | None = None) -> dict: + """Report {installed, scope, enabled, active} for a unit, fail-soft.""" + if not _systemd_present(): + return {"installed": False, "enabled": False, "active": False, + "detail": "systemd not available on host"} + try: + scope = await resolve_scope(unit, prefer) + except Exception as exc: # pragma: no cover - defensive + return {"installed": False, "enabled": False, "active": False, + "detail": f"systemctl unavailable: {str(exc)[:160]}"} + if scope is None: + return {"installed": False, "enabled": False, "active": False} + flag = ["--user"] if scope == "user" else [] + enabled = await _rc(["systemctl", *flag, "is-enabled", unit]) == 0 + active = await _rc(["systemctl", *flag, "is-active", unit]) == 0 + return {"installed": True, "scope": scope, "enabled": enabled, "active": active} + + +async def service_action(unit: str, verb: str, prefer: str | None = None, + timeout: float = 45.0) -> dict: + """Run ``systemctl `` in the resolved scope, fail-soft. + + Returns a per-unit result dict (never raises): a unit that is not installed, + or that the service user lacks rights to touch (polkit / interactive auth), + is reported rather than raised. On timeout the child is killed and reaped so + it is not orphaned. + """ + if verb not in ("start", "stop", "restart"): + raise ValueError(f"unsupported verb: {verb}") + if not _systemd_present(): + return {"unit": unit, "ok": False, "detail": "systemd not available on host"} + try: + scope = await resolve_scope(unit, prefer) + except Exception as exc: # pragma: no cover - defensive + return {"unit": unit, "ok": False, "detail": f"systemctl unavailable: {str(exc)[:160]}"} + if scope is None: + return {"unit": unit, "ok": False, "detail": "not installed"} + + flag = ["--user"] if scope == "user" else [] + proc = None + try: + proc = await asyncio.create_subprocess_exec( + "systemctl", *flag, verb, unit, + stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE, + ) + _, stderr = await asyncio.wait_for(proc.communicate(), timeout=timeout) + except asyncio.TimeoutError: + if proc is not None: + try: + proc.kill() + await proc.wait() + except Exception: # pragma: no cover - defensive + pass + return {"unit": unit, "ok": False, "scope": scope, "detail": f"{verb} timed out"} + except Exception as exc: # pragma: no cover - defensive + return {"unit": unit, "ok": False, "scope": scope, "detail": str(exc)[:200]} + + if proc.returncode == 0: + return {"unit": unit, "ok": True, "scope": scope} + detail = (stderr.decode(errors="replace").strip() or f"exit code {proc.returncode}")[:200] + return {"unit": unit, "ok": False, "scope": scope, "detail": detail} + + +async def health_probe(url: str, expect: str, timeout: float = 5.0) -> dict: + """Probe a backend health endpoint. Returns {ok, detail}. + + ``expect`` is a literal substring that must appear in a 200 response body. + """ + if not url: + return {"ok": False, "detail": "no health url"} + try: + async with httpx.AsyncClient(timeout=timeout) as client: + resp = await client.get(url) + except Exception as exc: + return {"ok": False, "detail": str(exc)[:160]} + if resp.status_code != 200: + return {"ok": False, "detail": f"HTTP {resp.status_code}"} + if expect and expect not in resp.text: + return {"ok": False, "detail": "health body missing expected marker"} + return {"ok": True} From 8bb607b4940fdcf7baa33b6fc26597b6cf9acf46 Mon Sep 17 00:00:00 2001 From: jaylfc Date: Thu, 9 Jul 2026 11:39:07 +0100 Subject: [PATCH 05/17] refactor(system): derive #1743 restart targets from managed-backend contract (Phase 1.3) (#1760) Rewire the AI-stack recovery endpoint (POST /api/system/ai-stack/restart) onto the node-local backend service manager (cluster/backend_services.py, #1758) instead of the hardcoded AI_STACK_UNITS list and the private _restart_ai_unit/_systemctl_rc helpers it duplicated. The restart target set is now derived per node: core-managed units that have no store manifest (qmd, installed by install-server.sh) unioned with the store-declared managed backends (lifecycle.auto_manage in their catalog manifest, e.g. rkllama), then narrowed to units whose systemd file actually exists on this node. A backend that has migrated to another cluster node is skipped rather than reported as a spurious failure, and a newly installed managed backend joins recovery with no code change. Membership uses systemctl cat (via unit_state) so a dead/crashed unit still counts. A node with no managed backend installed now returns status "noop" instead of a misleading "failed". A broken catalog still yields the core units, so qmd never drops out of recovery. _restart_ai_unit / _systemctl_rc scope-resolution + kill-reap logic already moved to backend_services in #1758 and is tested there; the endpoint tests now stub the derivation + service_action, and a new TestManagedAiUnits covers the core+manifest union and the installed-on-this-node filter. Docs-Reviewed: no route added or removed; endpoint path, method, auth gate, and response shape are unchanged (internal target-derivation refactor only). --- tests/test_routes_system.py | 145 +++++++++++++++++++++------------- tinyagentos/routes/system.py | 146 +++++++++++++++++------------------ 2 files changed, 160 insertions(+), 131 deletions(-) diff --git a/tests/test_routes_system.py b/tests/test_routes_system.py index 1848a6061..5938698e2 100644 --- a/tests/test_routes_system.py +++ b/tests/test_routes_system.py @@ -2,7 +2,6 @@ from __future__ import annotations -import asyncio from dataclasses import dataclass, field from unittest.mock import AsyncMock, MagicMock, patch @@ -239,17 +238,6 @@ async def test_restart_status_no_orchestrator_returns_503(self, client, monkeypa assert "error" in data -class _FakeProc: - """Minimal asyncio subprocess stand-in for _restart_ai_unit tests.""" - - def __init__(self, returncode: int, stderr: bytes = b""): - self.returncode = returncode - self._stderr = stderr - - async def communicate(self): - return (b"", self._stderr) - - async def _member_client(app) -> AsyncClient: """Cookie'd client for a non-admin member session (mirrors settings authz).""" auth_mgr = app.state.auth @@ -265,15 +253,23 @@ async def _member_client(app) -> AsyncClient: class TestRestartAiStack: - """POST /api/system/ai-stack/restart -- issue #1743 backend recovery.""" + """POST /api/system/ai-stack/restart -- issue #1743 backend recovery. + + The endpoint derives its target set via ``_managed_ai_units`` (tested + separately below) and restarts each via ``backend_services.service_action``; + these tests stub both to exercise the aggregation/status logic. + """ @pytest.mark.asyncio async def test_all_ok(self, client, monkeypatch): monkeypatch.setattr( - system_routes, - "_restart_ai_unit", + system_routes, "_managed_ai_units", + AsyncMock(return_value=[("rkllama.service", "system"), ("qmd.service", "system")]), + ) + monkeypatch.setattr( + system_routes.backend_services, "service_action", AsyncMock(side_effect=[ - {"unit": "rkllama.service", "ok": True, "scope": "user"}, + {"unit": "rkllama.service", "ok": True, "scope": "system"}, {"unit": "qmd.service", "ok": True, "scope": "system"}, ]), ) @@ -287,10 +283,13 @@ async def test_all_ok(self, client, monkeypatch): @pytest.mark.asyncio async def test_partial_recovery_reports_partial(self, client, monkeypatch): monkeypatch.setattr( - system_routes, - "_restart_ai_unit", + system_routes, "_managed_ai_units", + AsyncMock(return_value=[("rkllama.service", "system"), ("qmd.service", "system")]), + ) + monkeypatch.setattr( + system_routes.backend_services, "service_action", AsyncMock(side_effect=[ - {"unit": "rkllama.service", "ok": True, "scope": "user"}, + {"unit": "rkllama.service", "ok": True, "scope": "system"}, {"unit": "qmd.service", "ok": False, "detail": "not installed"}, ]), ) @@ -304,8 +303,11 @@ async def test_partial_recovery_reports_partial(self, client, monkeypatch): @pytest.mark.asyncio async def test_all_fail_reports_failed(self, client, monkeypatch): monkeypatch.setattr( - system_routes, - "_restart_ai_unit", + system_routes, "_managed_ai_units", + AsyncMock(return_value=[("rkllama.service", "system"), ("qmd.service", "system")]), + ) + monkeypatch.setattr( + system_routes.backend_services, "service_action", AsyncMock(side_effect=[ {"unit": "rkllama.service", "ok": False, "detail": "auth required"}, {"unit": "qmd.service", "ok": False, "detail": "auth required"}, @@ -317,57 +319,92 @@ async def test_all_fail_reports_failed(self, client, monkeypatch): assert len(data["failed"]) == 2 @pytest.mark.asyncio - async def test_non_admin_rejected_403(self, client, app, monkeypatch): - # Guard should reject before any restart is attempted. + async def test_noop_when_no_managed_backends_installed(self, client, monkeypatch): + # No managed backend installed on this node (e.g. non-systemd dev host): + # report noop rather than a misleading "failed", and never call restart. monkeypatch.setattr( - system_routes, "_restart_ai_unit", AsyncMock(return_value={"ok": True}) + system_routes, "_managed_ai_units", AsyncMock(return_value=[]), ) + action = AsyncMock() + monkeypatch.setattr(system_routes.backend_services, "service_action", action) + data = (await client.post("/api/system/ai-stack/restart")).json() + assert data["status"] == "noop" + assert data["restarted"] == [] and data["failed"] == [] + action.assert_not_called() + + @pytest.mark.asyncio + async def test_non_admin_rejected_403(self, client, app, monkeypatch): + # Guard should reject before any restart is attempted. + units = AsyncMock(return_value=[("qmd.service", "system")]) + monkeypatch.setattr(system_routes, "_managed_ai_units", units) member_client = await _member_client(app) try: resp = await member_client.post("/api/system/ai-stack/restart") finally: await member_client.aclose() assert resp.status_code == 403 + units.assert_not_called() -class TestRestartAiUnit: - """_restart_ai_unit scope resolution + fail-soft behaviour.""" +class TestManagedAiUnits: + """_managed_ai_units: derive core + manifest units, filter to installed.""" + + def _request(self, tmp_path): + req = MagicMock() + req.app.state.registry.catalog_dir = tmp_path + return req @pytest.mark.asyncio - async def test_restart_user_scope_ok(self, monkeypatch): - monkeypatch.setenv("INVOCATION_ID", "test") # force systemd-present path - # cat finds the unit in --user scope (rc 0), so restart uses --user. - monkeypatch.setattr(system_routes, "_systemctl_rc", AsyncMock(return_value=0)) + async def test_unions_core_and_manifest_when_installed(self, tmp_path, monkeypatch): + from tinyagentos.cluster.backend_services import ManagedBackend + + rk = ManagedBackend( + id="rkllama", unit="rkllama.service", scope="system", + health_url="http://localhost:7833/api/tags", health_expect='"models"', + ) + monkeypatch.setattr( + system_routes.backend_services, "load_managed_backends", lambda root: [rk] + ) monkeypatch.setattr( - asyncio, "create_subprocess_exec", AsyncMock(return_value=_FakeProc(0)) + system_routes.backend_services, "unit_state", + AsyncMock(return_value={"installed": True}), ) - result = await system_routes._restart_ai_unit("rkllama.service") - assert result == {"unit": "rkllama.service", "ok": True, "scope": "user"} + units = await system_routes._managed_ai_units(self._request(tmp_path)) + # qmd (core, no manifest) + rkllama (from manifest), both installed. + assert set(u for u, _ in units) == {"qmd.service", "rkllama.service"} @pytest.mark.asyncio - async def test_restart_failure_captures_stderr(self, monkeypatch): - monkeypatch.setenv("INVOCATION_ID", "test") - monkeypatch.setattr(system_routes, "_systemctl_rc", AsyncMock(return_value=0)) + async def test_skips_units_not_installed_on_node(self, tmp_path, monkeypatch): + from tinyagentos.cluster.backend_services import ManagedBackend + + rk = ManagedBackend( + id="rkllama", unit="rkllama.service", scope="system", + health_url="", health_expect="", + ) monkeypatch.setattr( - asyncio, - "create_subprocess_exec", - AsyncMock(return_value=_FakeProc(1, b"Interactive authentication required")), + system_routes.backend_services, "load_managed_backends", lambda root: [rk] ) - result = await system_routes._restart_ai_unit("qmd.service") - assert result["ok"] is False - assert "Interactive authentication required" in result["detail"] - @pytest.mark.asyncio - async def test_not_installed_when_cat_fails_both_scopes(self, monkeypatch): - monkeypatch.setenv("INVOCATION_ID", "test") - monkeypatch.setattr(system_routes, "_systemctl_rc", AsyncMock(return_value=1)) - result = await system_routes._restart_ai_unit("rkllama.service") - assert result == {"unit": "rkllama.service", "ok": False, "detail": "not installed"} + async def fake_state(unit, prefer=None): + # rkllama has migrated to another node; only qmd is installed here. + return {"installed": unit == "qmd.service"} + + monkeypatch.setattr(system_routes.backend_services, "unit_state", fake_state) + units = await system_routes._managed_ai_units(self._request(tmp_path)) + assert [u for u, _ in units] == ["qmd.service"] @pytest.mark.asyncio - async def test_no_systemd_fails_soft(self, monkeypatch): - monkeypatch.delenv("INVOCATION_ID", raising=False) - monkeypatch.setattr(system_routes.os.path, "exists", lambda p: False) - result = await system_routes._restart_ai_unit("qmd.service") - assert result["ok"] is False - assert "systemd not available" in result["detail"] + async def test_catalog_load_failure_still_yields_core(self, tmp_path, monkeypatch): + # A broken catalog must not drop qmd from recovery. + def boom(root): + raise RuntimeError("catalog unreadable") + + monkeypatch.setattr( + system_routes.backend_services, "load_managed_backends", boom + ) + monkeypatch.setattr( + system_routes.backend_services, "unit_state", + AsyncMock(return_value={"installed": True}), + ) + units = await system_routes._managed_ai_units(self._request(tmp_path)) + assert [u for u, _ in units] == ["qmd.service"] diff --git a/tinyagentos/routes/system.py b/tinyagentos/routes/system.py index 18ad88ffd..2657c37e6 100644 --- a/tinyagentos/routes/system.py +++ b/tinyagentos/routes/system.py @@ -5,10 +5,12 @@ import os import sys from dataclasses import asdict +from pathlib import Path from fastapi import APIRouter, Request from fastapi.responses import JSONResponse +from tinyagentos.cluster import backend_services from tinyagentos.restart_orchestrator import write_pending_restart, read_pending_restart logger = logging.getLogger(__name__) @@ -119,12 +121,52 @@ async def _emit_fail(msg: str) -> None: await _emit_fail(str(exc)) -# AI-stack services a recovery action restarts (issue #1743). These are the -# local inference backends that can stall independently of the controller: -# rkllama (NPU model server, a system unit per install-rknpu.sh) and qmd (shared -# model provider, a system unit). The controller itself is deliberately NOT here -- -# bouncing it is the separate /api/system/restart/prepare path. -AI_STACK_UNITS = ("rkllama.service", "qmd.service") +# Core AI backends taOS manages as systemd services that have NO store manifest +# (installed by install-server.sh, not via the store): qmd, the shared model +# provider. Store-installed managed backends (rkllama and any future NPU/GPU +# runtime) are discovered from their catalog manifests instead +# (lifecycle.auto_manage), so new managed backends join #1743 recovery +# automatically without editing this list. The controller itself is deliberately +# NOT here -- bouncing it is the separate /api/system/restart/prepare path. +# Value is the preferred systemctl scope for scope resolution. +CORE_MANAGED_UNITS: dict[str, str] = {"qmd.service": "system"} + + +def _catalog_root(request: Request) -> Path: + """Locate the app-catalog root the same way the app wires the registry.""" + registry = getattr(request.app.state, "registry", None) + catalog_dir = getattr(registry, "catalog_dir", None) + if catalog_dir is not None: + return Path(catalog_dir) + return Path(__file__).resolve().parent.parent.parent / "app-catalog" + + +async def _managed_ai_units(request: Request) -> list[tuple[str, str | None]]: + """Return (unit, prefer_scope) for every managed AI backend installed here. + + The target set for #1743 recovery is the core-managed units (no store + manifest) unioned with the store-declared managed backends + (lifecycle.auto_manage in their catalog manifest), then narrowed to units + whose systemd file actually exists on THIS node. A backend that has migrated + to another cluster node is skipped rather than reported as a spurious + failure, and a newly installed managed backend is picked up with no code + change. Membership is resolved with ``systemctl cat`` (via + ``backend_services.unit_state``) so a dead/crashed unit -- exactly what + recovery targets -- still counts as installed. + """ + prefer: dict[str, str | None] = dict(CORE_MANAGED_UNITS) + try: + for mb in backend_services.load_managed_backends(_catalog_root(request)): + prefer.setdefault(mb.unit, mb.scope) + except Exception: # pragma: no cover - defensive + logger.exception("failed to load managed backends from catalog") + + async def _installed(unit: str, scope: str | None): + state = await backend_services.unit_state(unit, scope) + return (unit, scope) if state.get("installed") else None + + checks = await asyncio.gather(*(_installed(u, s) for u, s in prefer.items())) + return [c for c in checks if c is not None] def _is_admin_or_local_token(request: Request) -> bool: @@ -142,87 +184,37 @@ def _is_admin_or_local_token(request: Request) -> bool: return getattr(request.state, "via", None) == "local_token" -async def _systemctl_rc(args: list[str]) -> int: - proc = await asyncio.create_subprocess_exec( - *args, - stdout=asyncio.subprocess.DEVNULL, - stderr=asyncio.subprocess.DEVNULL, - ) - await proc.wait() - return proc.returncode - - -async def _restart_ai_unit(unit: str) -> dict: - """Restart one AI-stack systemd unit, resolving user vs system scope. - - Fails soft: a unit that is not installed, or that the service user lacks - rights to restart (polkit / interactive auth), is reported rather than - raised, so restarting one backend still helps when the other cannot be - touched. Scope is resolved with ``systemctl [--user] cat`` (returns 0 iff - the unit file exists) so a dead/crashed unit -- exactly what recovery - targets -- is still restarted, unlike an is-active probe. - """ - if not (os.environ.get("INVOCATION_ID") or os.path.exists("/run/systemd/system")): - return {"unit": unit, "ok": False, "detail": "systemd not available on host"} - - # Resolve scope. Guarded so a missing/unusable systemctl is reported rather - # than raised (keeps the documented "reported, not raised" contract). - scope_flag: list[str] | None = None - try: - for candidate in (["--user"], []): - if await _systemctl_rc(["systemctl", *candidate, "cat", unit]) == 0: - scope_flag = candidate - break - except Exception as exc: # pragma: no cover - defensive - return {"unit": unit, "ok": False, "detail": f"systemctl unavailable: {str(exc)[:160]}"} - if scope_flag is None: - return {"unit": unit, "ok": False, "detail": "not installed"} - - scope_name = "user" if scope_flag else "system" - proc = None - try: - proc = await asyncio.create_subprocess_exec( - "systemctl", - *scope_flag, - "restart", - unit, - stdout=asyncio.subprocess.PIPE, - stderr=asyncio.subprocess.PIPE, - ) - _, stderr = await asyncio.wait_for(proc.communicate(), timeout=45) - except asyncio.TimeoutError: - # Kill and reap the child so a hung systemctl is not orphaned. - if proc is not None: - try: - proc.kill() - await proc.wait() - except Exception: # pragma: no cover - defensive - pass - return {"unit": unit, "ok": False, "scope": scope_name, "detail": "restart timed out"} - except Exception as exc: # pragma: no cover - defensive - return {"unit": unit, "ok": False, "scope": scope_name, "detail": str(exc)[:200]} - - if proc.returncode == 0: - return {"unit": unit, "ok": True, "scope": scope_name} - detail = (stderr.decode(errors="replace").strip() or f"exit code {proc.returncode}")[:200] - return {"unit": unit, "ok": False, "scope": scope_name, "detail": detail} - - @router.post("/api/system/ai-stack/restart") async def restart_ai_stack(request: Request): - """Restart the local AI inference stack (rkllama + qmd) -- issue #1743. + """Restart the local AI inference stack -- issue #1743. A recovery action for edge devices where a model stalls (endless NPU generation, unresponsive inference) while the controller itself stays up. - Restarts the backend services without bouncing the controller or agents. - Admin (or host local token) only. Fails soft per service so a partial - recovery still reports what was restarted. The units are restarted + Restarts the managed backend services without bouncing the controller or + agents. The target set is derived from the managed-backend contract (core + units plus store manifests) and narrowed to backends actually installed on + this node, so it stays correct as backends are added or migrated across + cluster nodes. Admin (or host local token) only. Fails soft per service so a + partial recovery still reports what was restarted. Units are restarted concurrently so worst-case latency is one unit's timeout, not their sum. """ if not _is_admin_or_local_token(request): return JSONResponse({"error": "forbidden"}, status_code=403) - results = list(await asyncio.gather(*(_restart_ai_unit(u) for u in AI_STACK_UNITS))) + targets = await _managed_ai_units(request) + if not targets: + return { + "status": "noop", + "restarted": [], + "failed": [], + "results": [], + "detail": "no managed AI backends installed on this node", + } + + results = list(await asyncio.gather( + *(backend_services.service_action(unit, "restart", prefer) + for unit, prefer in targets) + )) restarted = [r["unit"] for r in results if r.get("ok")] failed = [r for r in results if not r.get("ok")] return { From 71939f1f520dd08e4de5c7b1b125cb616f8de104 Mon Sep 17 00:00:00 2001 From: hognek Date: Thu, 9 Jul 2026 13:40:19 +0200 Subject: [PATCH 06/17] feat(models): atomic VRAM check-and-reserve before model load (TOCTOU fix, #1706) (#1725) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(models): atomic VRAM check-and-reserve before model load (TOCTOU fix) Adds VramReservationManager — an asyncio.Lock-protected VRAM reservation system that atomically checks free VRAM (via nvidia-smi) and reserves the required amount before a model load begins. Concurrent callers see the in-flight reservation and back off, preventing the TOCTOU race where two model pulls both see enough free VRAM and both try to load, causing OOM. Integration points: - POST /api/models/pull — reserve before ollama pull, release after - POST /api/models/download (rkllama path) — reserve before installer, release in the background task's finally block - GET /api/models/vram-reservations — stats endpoint for monitoring New files: - tinyagentos/vram_reservation.py — atomic reserve/release manager - tests/test_vram_reservation.py — 15 tests including concurrent safety Wired as app.state.vram_reservation in the FastAPI lifespan alongside the cluster manager and resource scheduler. Issue: #1706 * docs: add TODO for per-model VRAM estimate replacement jaylfc review: min_ram_mb heuristic is safe for v1 (over-reserves, fails closed), but should be replaced with a real per-model VRAM estimate on CUDA GGUF to avoid false 503s in multi-model setups. --------- Co-authored-by: Hogne <227774406+hognek@users.noreply.github.com> --- tests/test_vram_reservation.py | 238 ++++++++++++++++++++++++++++++++ tinyagentos/app.py | 7 + tinyagentos/routes/models.py | 119 ++++++++++++---- tinyagentos/vram_reservation.py | 186 +++++++++++++++++++++++++ 4 files changed, 526 insertions(+), 24 deletions(-) create mode 100644 tests/test_vram_reservation.py create mode 100644 tinyagentos/vram_reservation.py diff --git a/tests/test_vram_reservation.py b/tests/test_vram_reservation.py new file mode 100644 index 000000000..0054b3837 --- /dev/null +++ b/tests/test_vram_reservation.py @@ -0,0 +1,238 @@ +"""Tests for atomic VRAM reservation (taOS #1706 TOCTOU fix). + +Verifies that: +- Concurrent reserve calls cannot over-commit VRAM. +- reserve() returns None when there's not enough VRAM. +- release() correctly returns VRAM to the pool. +- release() is idempotent. +- Zero-VRAM reservations always succeed. +- stats() reflects the current state. +""" + +import asyncio + +import pytest + +from tinyagentos.vram_reservation import VramReservationManager + + +# ── test helpers ──────────────────────────────────────────────────── + + +def _manager_with_probe(free_mb: int, total_mb: int) -> VramReservationManager: + """Return a manager whose _probe_vram returns fixed values.""" + mgr = VramReservationManager() + + def _fake_probe() -> tuple[int, int]: + return free_mb, total_mb + + mgr._probe_vram = staticmethod(_fake_probe) # type: ignore[method-assign] + return mgr + + +# ── basic reserve / release ───────────────────────────────────────── + + +class TestReserveRelease: + """Happy-path reserve and release.""" + + @pytest.mark.asyncio + async def test_reserve_when_vram_available(self): + """reserve() succeeds when enough VRAM is free.""" + mgr = _manager_with_probe(8192, 8192) + reservation = await mgr.reserve(4096, caller="test") + assert reservation is not None + assert reservation.vram_mb == 4096 + assert reservation.caller == "test" + assert mgr.reserved_vram_mb == 4096 + assert mgr.pending_count == 1 + + @pytest.mark.asyncio + async def test_reserve_denied_when_insufficient(self): + """reserve() returns None when VRAM is too low.""" + mgr = _manager_with_probe(2048, 8192) + reservation = await mgr.reserve(4096, caller="test") + assert reservation is None + assert mgr.reserved_vram_mb == 0 + assert mgr.pending_count == 0 + + @pytest.mark.asyncio + async def test_release_returns_vram(self): + """release() returns reserved VRAM to the pool.""" + mgr = _manager_with_probe(8192, 8192) + res = await mgr.reserve(4096, caller="test") + assert res is not None + + released = mgr.release(res.reservation_id) + assert released is True + assert mgr.reserved_vram_mb == 0 + assert mgr.pending_count == 0 + + @pytest.mark.asyncio + async def test_release_idempotent(self): + """release() is safe to call multiple times.""" + mgr = _manager_with_probe(8192, 8192) + res = await mgr.reserve(4096, caller="test") + assert res is not None + + assert mgr.release(res.reservation_id) is True + assert mgr.release(res.reservation_id) is False # already released + assert mgr.release("non-existent") is False + assert mgr.reserved_vram_mb == 0 + + @pytest.mark.asyncio + async def test_multiple_reservations_serial(self): + """Serial reservations accumulate correctly.""" + mgr = _manager_with_probe(8192, 8192) + + r1 = await mgr.reserve(2000, caller="a") + r2 = await mgr.reserve(3000, caller="b") + assert r1 is not None + assert r2 is not None + assert mgr.reserved_vram_mb == 5000 + assert mgr.pending_count == 2 + + mgr.release(r1.reservation_id) + assert mgr.reserved_vram_mb == 3000 + + mgr.release(r2.reservation_id) + assert mgr.reserved_vram_mb == 0 + + @pytest.mark.asyncio + async def test_reserve_at_boundary(self): + """Reserving exactly the free amount succeeds.""" + mgr = _manager_with_probe(4096, 8192) + res = await mgr.reserve(4096, caller="test") + assert res is not None + assert mgr.reserved_vram_mb == 4096 + + @pytest.mark.asyncio + async def test_reserve_one_more_than_free_fails(self): + """Reserving free+1 fails.""" + mgr = _manager_with_probe(4096, 8192) + res = await mgr.reserve(4097, caller="test") + assert res is None + + +# ── concurrent safety ─────────────────────────────────────────────── + + +class TestConcurrentSafety: + """Two concurrent reserve calls cannot over-commit.""" + + @pytest.mark.asyncio + async def test_concurrent_reserve_only_one_admitted(self): + """With 8192 MiB free and each needing 5000 MiB, only one admitted.""" + mgr = _manager_with_probe(8192, 8192) + results: list[bool] = [] + + async def try_reserve(caller: str): + res = await mgr.reserve(5000, caller=caller) + results.append(res is not None) + + await asyncio.gather(try_reserve("a"), try_reserve("b")) + + admitted = sum(results) + assert admitted == 1, f"Expected 1 admission, got {admitted}: {results}" + assert mgr.reserved_vram_mb == 5000 + + @pytest.mark.asyncio + async def test_concurrent_vram_accounting(self): + """After two serial reserves filling VRAM, a third fails.""" + mgr = _manager_with_probe(8192, 8192) + + r1 = await mgr.reserve(4000, caller="a") + r2 = await mgr.reserve(4000, caller="b") + assert r1 is not None + assert r2 is not None + + # Third should fail — only 192 MiB effective left. + r3 = await mgr.reserve(4000, caller="c") + assert r3 is None + assert mgr.reserved_vram_mb == 8000 + + +# ── zero / negative VRAM ──────────────────────────────────────────── + + +class TestZeroVram: + """Zero-VRAM reservations always succeed as lightweight markers.""" + + @pytest.mark.asyncio + async def test_zero_vram_reservation(self): + mgr = _manager_with_probe(0, 0) + res = await mgr.reserve(0, caller="noop") + assert res is not None + assert res.vram_mb == 0 + assert mgr.reserved_vram_mb == 0 + assert mgr.pending_count == 0 + + @pytest.mark.asyncio + async def test_negative_vram_reservation(self): + mgr = _manager_with_probe(8192, 8192) + res = await mgr.reserve(-1, caller="test") + assert res is not None + assert res.vram_mb == 0 + assert mgr.reserved_vram_mb == 0 + assert mgr.pending_count == 0 + + +# ── available_vram / stats ────────────────────────────────────────── + + +class TestAvailableVram: + """available_vram() and stats() reflect current state.""" + + @pytest.mark.asyncio + async def test_available_vram_no_reservations(self): + mgr = _manager_with_probe(8192, 16384) + free, total = mgr.available_vram() + assert free == 8192 + assert total == 16384 + + @pytest.mark.asyncio + async def test_available_vram_with_reservations(self): + mgr = _manager_with_probe(8192, 16384) + res = await mgr.reserve(3000, caller="test") + assert res is not None + + free, total = mgr.available_vram() + assert free == 5192 # 8192 - 3000 + assert total == 16384 + + @pytest.mark.asyncio + async def test_stats(self): + mgr = _manager_with_probe(8192, 16384) + + s = mgr.stats() + assert s["free_vram_mb"] == 8192 + assert s["total_vram_mb"] == 16384 + assert s["reserved_vram_mb"] == 0 + assert s["effective_free_mb"] == 8192 + assert s["pending_reservations"] == 0 + + res = await mgr.reserve(4096, caller="test") + assert res is not None + + s = mgr.stats() + assert s["reserved_vram_mb"] == 4096 + assert s["effective_free_mb"] == 4096 + assert s["pending_reservations"] == 1 + + +# ── real probe (integration smoke test) ───────────────────────────── + + +class TestRealProbe: + """Smoke-test with the real nvidia-smi probe (if available).""" + + @pytest.mark.asyncio + async def test_reserve_with_real_probe(self): + """Reserving 1 MiB should always succeed on a real GPU.""" + mgr = VramReservationManager() + res = await mgr.reserve(1, caller="smoke-test") + # On a system with nvidia-smi and some free VRAM, this should succeed. + # On a system without nvidia-smi, the probe returns (0, 0) so + # this will fail — that's fine, the test is a best-effort smoke. + if res is not None: + mgr.release(res.reservation_id) diff --git a/tinyagentos/app.py b/tinyagentos/app.py index d25166d77..bc81a1373 100644 --- a/tinyagentos/app.py +++ b/tinyagentos/app.py @@ -43,6 +43,7 @@ async def get_response(self, path, scope): from tinyagentos.backend_fallback import BackendFallback from tinyagentos.capabilities import CapabilityChecker from tinyagentos.cluster.manager import ClusterManager +from tinyagentos.vram_reservation import VramReservationManager from tinyagentos.cluster.router import TaskRouter from tinyagentos.config import auto_register_from_manifest, load_config, save_config, save_config_locked from tinyagentos.lifecycle_manager import LifecycleManager @@ -1140,6 +1141,12 @@ async def _reload_llm_proxy_on_catalog_change() -> None: except Exception: logger.exception("resource scheduler failed to build — routes will use static config") app.state.resource_scheduler = None + + # VRAM reservation manager — atomic check-and-reserve so two + # concurrent model loads cannot both pass a VRAM check before + # either consumes physical VRAM (TOCTOU race, taOS #1706). + app.state.vram_reservation = VramReservationManager() + # Detect and set container runtime from tinyagentos.containers.backend import configure_container_runtime configure_container_runtime(config) diff --git a/tinyagentos/routes/models.py b/tinyagentos/routes/models.py index 741f4d4e6..4bfbb4ed0 100644 --- a/tinyagentos/routes/models.py +++ b/tinyagentos/routes/models.py @@ -33,6 +33,7 @@ class DeleteRequest(BaseModel): class PullRequest(BaseModel): model_name: str + required_vram_mb: int = 0 router = APIRouter() @@ -376,36 +377,68 @@ async def download_model(request: Request, body: DownloadRequest): installer = RkllamaInstaller(rkllama_url=resolve_rkllama_url(None)) catalog = getattr(request.app.state, "backend_catalog", None) - async def _install_and_record(on_progress): - result = await installer.install( - body.app_id, {}, variant=variant, on_progress=on_progress + # Atomic VRAM check-and-reserve (TOCTOU fix, taOS #1706). + # rkllama pulls the model into its backend, which loads it + # into VRAM — two concurrent pulls could both pass the VRAM + # check and OOM. + vram_mgr = getattr(request.app.state, "vram_reservation", None) + reservation = None + # TODO(#1725): replace min_ram_mb heuristic with a real per-model VRAM + # estimate. On CUDA GGUF the host-RAM floor over-reserves and causes + # occasional false 503s in multi-model setups. Safe for v1 (fails + # closed), but a model-card estimate would be more accurate. + estimated_vram = int(variant.get("min_ram_mb", 0) or 0) # heuristic + if vram_mgr is not None and estimated_vram > 0: + reservation = await vram_mgr.reserve( + estimated_vram, caller=f"rkllama-pull:{body.app_id}", ) - if result.get("success"): - # rkllama pulls the weight into its own model store, not - # models_dir, so the disk scan in list_models() never sees it. - # Record the install as a registry breadcrumb (corroborated by - # live-backend evidence, see _model_to_dict) and force an - # immediate catalog refresh so the newly pulled model shows up - # in all_models() right away instead of waiting for the next - # poll tick. - try: - registry.mark_installed( - body.app_id, variant.get("id") or "latest", state="installed" - ) - except Exception: - logger.warning( - "Failed to record rkllama install for %s in registry", - body.app_id, exc_info=True, - ) - if catalog is not None: + if reservation is None: + effective_free, _total = vram_mgr.available_vram() + return JSONResponse( + { + "error": ( + f"Insufficient VRAM: need ~{estimated_vram} MiB, " + f"have {effective_free} MiB available. " + f"Try a smaller model or free VRAM first." + ), + }, + status_code=503, + ) + + async def _install_and_record(on_progress): + try: + result = await installer.install( + body.app_id, {}, variant=variant, on_progress=on_progress + ) + if result.get("success"): + # rkllama pulls the weight into its own model store, not + # models_dir, so the disk scan in list_models() never sees it. + # Record the install as a registry breadcrumb (corroborated by + # live-backend evidence, see _model_to_dict) and force an + # immediate catalog refresh so the newly pulled model shows up + # in all_models() right away instead of waiting for the next + # poll tick. try: - await catalog.refresh() + registry.mark_installed( + body.app_id, variant.get("id") or "latest", state="installed" + ) except Exception: logger.warning( - "Backend catalog refresh after rkllama install failed for %s", + "Failed to record rkllama install for %s in registry", body.app_id, exc_info=True, ) - return result + if catalog is not None: + try: + await catalog.refresh() + except Exception: + logger.warning( + "Backend catalog refresh after rkllama install failed for %s", + body.app_id, exc_info=True, + ) + return result + finally: + if vram_mgr is not None and reservation is not None: + vram_mgr.release(reservation.reservation_id) dm.start_installer_task(download_id, _install_and_record) return { @@ -611,6 +644,28 @@ async def pull_model(request: Request, body: PullRequest): model_name = body.model_name.strip() if not model_name: return JSONResponse({"error": "model_name is required"}, status_code=400) + + # Atomic VRAM check-and-reserve (TOCTOU fix, taOS #1706) — + # two concurrent pulls could both see free VRAM and OOM. + vram_mgr = getattr(request.app.state, "vram_reservation", None) + reservation = None + if vram_mgr is not None and body.required_vram_mb > 0: + reservation = await vram_mgr.reserve( + body.required_vram_mb, caller=f"ollama-pull:{model_name}", + ) + if reservation is None: + effective_free, _total = vram_mgr.available_vram() + return JSONResponse( + { + "error": ( + f"Insufficient VRAM: need {body.required_vram_mb} MiB, " + f"have {effective_free} MiB available. " + f"Try a smaller model or free VRAM first." + ), + }, + status_code=503, + ) + try: http_client = request.app.state.http_client config = request.app.state.config @@ -635,6 +690,22 @@ async def pull_model(request: Request, body: PullRequest): except Exception as e: logger.warning(f"Ollama pull failed: {e}") return JSONResponse({"error": str(e)}, status_code=502) + finally: + if vram_mgr is not None and reservation is not None: + vram_mgr.release(reservation.reservation_id) + + +@router.get("/api/models/vram-reservations") +async def vram_reservation_stats(request: Request): + """Return VRAM reservation manager stats for monitoring. + + Includes real-time free/total VRAM from nvidia-smi, in-flight + reservations, and effective free VRAM after subtracting reservations. + """ + vram_mgr = getattr(request.app.state, "vram_reservation", None) + if vram_mgr is None: + return {"available": False} + return vram_mgr.stats() @router.get("/api/models/recommended") diff --git a/tinyagentos/vram_reservation.py b/tinyagentos/vram_reservation.py new file mode 100644 index 000000000..1b2eee783 --- /dev/null +++ b/tinyagentos/vram_reservation.py @@ -0,0 +1,186 @@ +""" +Atomic VRAM reservation — check-and-reserve to prevent TOCTOU races. + +When two concurrent model loads both check available VRAM before either +consumes it, they can both pass and cause OOM. This module provides +atomic check-and-reserve semantics: check available VRAM, reserve what +you need, then load the model. Concurrent callers see the reserved +capacity and back off. + +Pattern: + + mgr = VramReservationManager() + reservation = await mgr.reserve(vram_mb=4096, caller="ollama-pull") + if reservation is not None: + try: + await load_model() + finally: + mgr.release(reservation.reservation_id) + else: + raise RuntimeError("Not enough VRAM") + +The reservation is *not* a context manager — callers must release +explicitly (typically in a ``finally`` block) so the reservation can +outlive a single ``async with`` block while the model loads. +""" + +from __future__ import annotations + +import asyncio +import logging +import secrets +import time +from dataclasses import dataclass + +logger = logging.getLogger(__name__) + + +@dataclass +class VramReservation: + """A held VRAM reservation. Release via manager.release().""" + + reservation_id: str + vram_mb: int + caller: str + created_at: float + + +class VramReservationManager: + """Atomic VRAM check-and-reserve for model loads. + + Thread-safe via ``asyncio.Lock``. Reads real-time VRAM from + nvidia-smi and subtracts in-flight reservations so concurrent + callers see the capacity already promised to others. + + Zero-VRAM reservations (``vram_mb <= 0``) always succeed as a + lightweight marker — no lock acquisition, no probe, no accounting. + """ + + def __init__(self) -> None: + self._lock = asyncio.Lock() + self._reserved_vram_mb: int = 0 + self._pending: dict[str, VramReservation] = {} + + # ── public API ────────────────────────────────────────────────── + + async def reserve( + self, + vram_mb: int, + caller: str = "", + ) -> VramReservation | None: + """Atomically check available VRAM and reserve *vram_mb* MiB. + + Returns a ``VramReservation`` if admitted, ``None`` if + insufficient VRAM. Caller **must** call :meth:`release` when + done — this object is not a context manager so the reservation + can outlive a single ``async with`` block while the model loads. + + *vram_mb* ≤ 0 always succeeds without probing VRAM or acquiring + the lock (a lightweight no-op marker). + """ + if vram_mb <= 0: + return VramReservation( + reservation_id=f"vr_{secrets.token_hex(4)}", + vram_mb=0, + caller=caller or "noop", + created_at=time.time(), + ) + + async with self._lock: + free_mb, total_mb = self._probe_vram() + effective_free = max(0, free_mb - self._reserved_vram_mb) + + if effective_free < vram_mb: + logger.debug( + "vram-reservation: denied — need %d MiB, have %d MiB " + "(%d free − %d reserved)", + vram_mb, effective_free, free_mb, self._reserved_vram_mb, + ) + return None + + reservation_id = f"vr_{secrets.token_hex(4)}" + reservation = VramReservation( + reservation_id=reservation_id, + vram_mb=vram_mb, + caller=caller, + created_at=time.time(), + ) + self._reserved_vram_mb += vram_mb + self._pending[reservation_id] = reservation + + logger.info( + "vram-reservation: granted %d MiB to %r (id=%s, " + "total reserved=%d MiB, effective free=%d MiB)", + vram_mb, caller, reservation_id, + self._reserved_vram_mb, effective_free - vram_mb, + ) + return reservation + + def release(self, reservation_id: str) -> bool: + """Release a reservation. Idempotent — safe to call repeatedly. + + Returns ``True`` if a reservation was actually released, + ``False`` if *reservation_id* was unknown or already released. + """ + reservation = self._pending.pop(reservation_id, None) + if reservation is not None: + self._reserved_vram_mb -= reservation.vram_mb + logger.debug( + "vram-reservation: released %d MiB for %r (id=%s, " + "total reserved=%d MiB)", + reservation.vram_mb, reservation.caller, + reservation_id, self._reserved_vram_mb, + ) + return True + return False + + # ── read-only accessors ───────────────────────────────────────── + + @property + def reserved_vram_mb(self) -> int: + """Total VRAM currently reserved (MiB).""" + return self._reserved_vram_mb + + @property + def pending_count(self) -> int: + """Number of active reservations.""" + return len(self._pending) + + def available_vram(self) -> tuple[int, int]: + """Return ``(effective_free_mb, total_mb)`` after subtracting + in-flight reservations from the hardware probe.""" + free_mb, total_mb = self._probe_vram() + effective_free = max(0, free_mb - self._reserved_vram_mb) + return effective_free, total_mb + + def stats(self) -> dict: + """Return a snapshot for monitoring / debug endpoints.""" + free_mb, total_mb = self._probe_vram() + return { + "free_vram_mb": free_mb, + "total_vram_mb": total_mb, + "reserved_vram_mb": self._reserved_vram_mb, + "effective_free_mb": max(0, free_mb - self._reserved_vram_mb), + "pending_reservations": self.pending_count, + } + + # ── internal ──────────────────────────────────────────────────── + + @staticmethod + def _probe_vram() -> tuple[int, int]: + """Probe real-time free/total VRAM via nvidia-smi. + + Returns ``(free_mb, total_mb)``, or ``(0, 0)`` when no + nvidia-smi is available or the probe fails. + """ + try: + from tinyagentos.system_stats import read_nvidia_vram + + pair = read_nvidia_vram() + if pair is not None: + used_mb, total_mb = pair + free_mb = max(0, total_mb - used_mb) + return free_mb, total_mb + except Exception: + logger.debug("vram-reservation: probe failed", exc_info=True) + return 0, 0 From 6b485fb4ba1c86a2e578e8e28e7be1642642865f Mon Sep 17 00:00:00 2001 From: hognek Date: Thu, 9 Jul 2026 18:10:46 +0200 Subject: [PATCH 07/17] feat(cluster): advertise available models from Skald sidecar manifest (#1762) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(cluster): advertise available models from Skald sidecar manifest Add skald_manifest.py — parses /etc/taos/skald-models.json (env-configurable) to enumerate which models a worker CAN load per the GPU catalog. - WorkerInfo gains available_models list[dict] field (defaults to []) - ClusterManager.heartbeat() derives available_models from backend entries, deduplicating model_ids across backends - WorkerAgent.detect_backends() enriches each backend with available models from the sidecar manifest, tagging each as 'loaded' or 'available' based on live probe results Tests: 42 pass — 14 parser tests, 5 field serialization tests, 5 heartbeat derivation tests, 18 existing protocol tests (0 regressions). Also verified zero regressions against 32 existing worker tests. Completes end-to-end wiring for Skald PR #361 — the controller's cluster view now shows both loaded and available models per worker. * de-skald worker manifest: rename skald_manifest → worker_manifest - Rename tinyagentos/worker/skald_manifest.py → worker_manifest.py - Default manifest path: /etc/taos/skald-models.json → /etc/taos/worker-models.json - Env var: TAOS_SKALD_MANIFEST → TAOS_WORKER_MANIFEST - Remove all Skald-specific language from docstrings, comments, and field labels - Update import in agent.py - Rename test file, update all references, 14/14 pass - Keep available_models field on WorkerInfo as generic (unchanged) Core is now fork-agnostic: any external platform can write the manifest. Skald-specific logic belongs in Skald's codebase. --------- Co-authored-by: Hogne <227774406+hognek@users.noreply.github.com> --- tests/test_cluster_worker_protocol.py | 185 ++++++++++++++++++++ tests/test_worker_manifest.py | 225 +++++++++++++++++++++++++ tinyagentos/cluster/manager.py | 12 ++ tinyagentos/cluster/worker_protocol.py | 1 + tinyagentos/worker/agent.py | 36 ++++ tinyagentos/worker/worker_manifest.py | 67 ++++++++ 6 files changed, 526 insertions(+) create mode 100644 tests/test_worker_manifest.py create mode 100644 tinyagentos/worker/worker_manifest.py diff --git a/tests/test_cluster_worker_protocol.py b/tests/test_cluster_worker_protocol.py index 6f8079691..8dc6a1503 100644 --- a/tests/test_cluster_worker_protocol.py +++ b/tests/test_cluster_worker_protocol.py @@ -201,3 +201,188 @@ async def test_vram_zero_is_stored(self): w = mgr.get_worker("w1") assert w.free_vram_mb == 0 assert w.used_vram_mb == 0 + + +# --------------------------------------------------------------------------- +# WorkerInfo available_models field (local worker manifest) +# --------------------------------------------------------------------------- + + +class TestWorkerInfoAvailableModels: + def test_default_is_empty_list(self): + """available_models defaults to empty list when not provided.""" + w = WorkerInfo(name="w", url="http://localhost:9000") + assert w.available_models == [] + + def test_custom_value_stored(self): + w = WorkerInfo( + name="w", + url="http://localhost:9000", + available_models=[ + {"model_id": "qwen3-embedding-8b", "status": "available"}, + ], + ) + assert len(w.available_models) == 1 + assert w.available_models[0]["model_id"] == "qwen3-embedding-8b" + + def test_serialises_via_asdict(self): + w = WorkerInfo( + name="w", + url="http://localhost:9000", + available_models=[ + {"model_id": "kokoro-v1.0", "status": "available"}, + ], + ) + d = asdict(w) + assert "available_models" in d + assert d["available_models"][0]["model_id"] == "kokoro-v1.0" + + def test_roundtrip(self): + models = [ + {"model_id": "a", "status": "loaded"}, + {"model_id": "b", "status": "available"}, + ] + w = WorkerInfo( + name="w", + url="http://localhost:9000", + available_models=models, + ) + d = asdict(w) + w2 = WorkerInfo(**{k: v for k, v in d.items() if not isinstance(v, bytes)}) + assert w2.available_models == models + + def test_roundtrip_default(self): + w = WorkerInfo(name="w", url="http://localhost:9000") + d = asdict(w) + w2 = WorkerInfo(**{k: v for k, v in d.items() if not isinstance(v, bytes)}) + assert w2.available_models == [] + + +# --------------------------------------------------------------------------- +# ClusterManager heartbeat available_models derivation +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +class TestHeartbeatAvailableModels: + async def _register(self, mgr: ClusterManager, name: str) -> WorkerInfo: + w = WorkerInfo(name=name, url=f"http://localhost:900{name[-1]}") + await mgr.register_worker(w) + return w + + async def test_heartbeat_derives_from_backends(self): + """available_models is derived from backend entries at heartbeat.""" + mgr = ClusterManager() + await self._register(mgr, "w1") + backends = [ + { + "name": "llama-cpp:8080", + "type": "llama-cpp", + "url": "http://localhost:8080", + "available_models": [ + {"model_id": "qwen3-embedding-8b", "status": "available"}, + {"model_id": "phi4", "status": "loaded"}, + ], + }, + ] + mgr.heartbeat("w1", backends=backends) + w = mgr.get_worker("w1") + assert len(w.available_models) == 2 + model_ids = {m["model_id"] for m in w.available_models} + assert model_ids == {"qwen3-embedding-8b", "phi4"} + + async def test_heartbeat_dedupes_across_backends(self): + """Duplicate model_ids across backends are deduplicated.""" + mgr = ClusterManager() + await self._register(mgr, "w1") + backends = [ + { + "name": "llama-cpp:8080", + "type": "llama-cpp", + "url": "http://localhost:8080", + "available_models": [ + {"model_id": "qwen3-embedding-8b", "status": "available"}, + ], + }, + { + "name": "llama-cpp:8081", + "type": "llama-cpp", + "url": "http://localhost:8081", + "available_models": [ + {"model_id": "qwen3-embedding-8b", "status": "loaded"}, + ], + }, + ] + mgr.heartbeat("w1", backends=backends) + w = mgr.get_worker("w1") + # Deduplicated: same model_id appears once. + assert len(w.available_models) == 1 + assert w.available_models[0]["model_id"] == "qwen3-embedding-8b" + + async def test_heartbeat_additive_across_backends(self): + """Different model_ids from different backends all appear.""" + mgr = ClusterManager() + await self._register(mgr, "w1") + backends = [ + { + "name": "llama-cpp:8080", + "type": "llama-cpp", + "url": "http://localhost:8080", + "available_models": [ + {"model_id": "a", "status": "available"}, + ], + }, + { + "name": "kokoro:8880", + "type": "kokoro", + "url": "http://localhost:8880", + "available_models": [ + {"model_id": "b", "status": "available"}, + ], + }, + ] + mgr.heartbeat("w1", backends=backends) + w = mgr.get_worker("w1") + assert len(w.available_models) == 2 + + async def test_heartbeat_empty_backends_clears(self): + """Heartbeat with no backends (or no available_models) clears the list.""" + mgr = ClusterManager() + await self._register(mgr, "w1") + # First heartbeat with models + mgr.heartbeat("w1", backends=[ + { + "name": "llama-cpp:8080", + "type": "llama-cpp", + "url": "http://localhost:8080", + "available_models": [ + {"model_id": "a", "status": "available"}, + ], + }, + ]) + w = mgr.get_worker("w1") + assert len(w.available_models) == 1 + + # Heartbeat with empty backends — no available_models + mgr.heartbeat("w1", backends=[]) + w = mgr.get_worker("w1") + assert w.available_models == [] + + async def test_heartbeat_no_backends_preserves_prior(self): + """Heartbeat without backends parameter preserves prior available_models.""" + mgr = ClusterManager() + await self._register(mgr, "w1") + mgr.heartbeat( + "w1", + backends=[{ + "name": "t", + "type": "llama-cpp", + "url": "http://localhost:8000", + "available_models": [{"model_id": "x", "status": "loaded"}], + }], + ) + # Sparse heartbeat — only load, no backends + mgr.heartbeat("w1", load=0.5) + w = mgr.get_worker("w1") + assert len(w.available_models) == 1 + assert w.available_models[0]["model_id"] == "x" diff --git a/tests/test_worker_manifest.py b/tests/test_worker_manifest.py new file mode 100644 index 000000000..be9a74eac --- /dev/null +++ b/tests/test_worker_manifest.py @@ -0,0 +1,225 @@ +"""Tests for tinyagentos.worker.worker_manifest — local manifest parser.""" +from __future__ import annotations + +import json +import os +import tempfile +from pathlib import Path + +import pytest + +from tinyagentos.worker.worker_manifest import ( + DEFAULT_MANIFEST_PATH, + SOFTWARE_TO_BACKEND_TYPE, + load_manifest, +) + + +# --------------------------------------------------------------------------- +# SOFTWARE_TO_BACKEND_TYPE mapping +# --------------------------------------------------------------------------- + + +class TestSoftwareMapping: + def test_llamacpp_maps_to_llama_cpp(self): + assert SOFTWARE_TO_BACKEND_TYPE["llamacpp"] == "llama-cpp" + + def test_embed_maps_to_llama_cpp(self): + assert SOFTWARE_TO_BACKEND_TYPE["embed"] == "llama-cpp" + + def test_kokoro_maps_to_kokoro(self): + assert SOFTWARE_TO_BACKEND_TYPE["kokoro"] == "kokoro" + + def test_whisper_maps_to_whisper(self): + assert SOFTWARE_TO_BACKEND_TYPE["whisper"] == "whisper" + + +# --------------------------------------------------------------------------- +# load_manifest — file-based tests +# --------------------------------------------------------------------------- + + +class TestLoadManifest: + def test_returns_empty_on_missing_file(self): + """When the manifest file does not exist, return an empty manifest.""" + result = load_manifest("/nonexistent/path/worker-models.json") + assert result == {"resource_id": "", "models": []} + + def test_parses_valid_manifest(self): + """A valid JSON manifest is parsed correctly.""" + manifest = { + "resource_id": "worker-1:gpu-cuda-0", + "models": [ + { + "model_id": "qwen3-embedding-8b", + "capability": "embed", + "software": "embed", + "port": 8080, + "vram_required_gb": 8.0, + "health_url": "http://localhost:8080/health", + }, + { + "model_id": "kokoro-v1.0", + "capability": "tts", + "software": "kokoro", + "port": 8880, + "vram_required_gb": 0.5, + "health_url": "http://localhost:8880/health", + }, + ], + } + with tempfile.NamedTemporaryFile( + mode="w", suffix=".json", delete=False + ) as f: + json.dump(manifest, f) + tmp_path = f.name + + try: + result = load_manifest(tmp_path) + assert result["resource_id"] == "worker-1:gpu-cuda-0" + assert len(result["models"]) == 2 + assert result["models"][0]["model_id"] == "qwen3-embedding-8b" + assert result["models"][1]["model_id"] == "kokoro-v1.0" + finally: + os.unlink(tmp_path) + + def test_manifest_with_empty_models(self): + """A manifest with an empty models list.""" + manifest = {"resource_id": "worker-1:gpu-cuda-0", "models": []} + with tempfile.NamedTemporaryFile( + mode="w", suffix=".json", delete=False + ) as f: + json.dump(manifest, f) + tmp_path = f.name + + try: + result = load_manifest(tmp_path) + assert result["resource_id"] == "worker-1:gpu-cuda-0" + assert result["models"] == [] + finally: + os.unlink(tmp_path) + + def test_invalid_json_raises(self): + """Invalid JSON raises json.JSONDecodeError.""" + with tempfile.NamedTemporaryFile( + mode="w", suffix=".json", delete=False + ) as f: + f.write("not valid json {{{") + tmp_path = f.name + + try: + with pytest.raises(json.JSONDecodeError): + load_manifest(tmp_path) + finally: + os.unlink(tmp_path) + + def test_env_var_overrides_path(self, monkeypatch): + """TAOS_WORKER_MANIFEST env var overrides the default path.""" + manifest = {"resource_id": "custom-path", "models": []} + with tempfile.NamedTemporaryFile( + mode="w", suffix=".json", delete=False + ) as f: + json.dump(manifest, f) + tmp_path = f.name + + try: + monkeypatch.setenv("TAOS_WORKER_MANIFEST", tmp_path) + result = load_manifest() + assert result["resource_id"] == "custom-path" + finally: + os.unlink(tmp_path) + + def test_explicit_path_overrides_env_var(self, monkeypatch): + """Explicit path argument wins over the env var.""" + manifest_env = {"resource_id": "from-env", "models": []} + manifest_arg = {"resource_id": "from-arg", "models": []} + + with tempfile.NamedTemporaryFile( + mode="w", suffix=".json", delete=False + ) as f: + json.dump(manifest_env, f) + env_path = f.name + with tempfile.NamedTemporaryFile( + mode="w", suffix=".json", delete=False + ) as f: + json.dump(manifest_arg, f) + arg_path = f.name + + try: + monkeypatch.setenv("TAOS_WORKER_MANIFEST", env_path) + result = load_manifest(arg_path) + assert result["resource_id"] == "from-arg" + finally: + os.unlink(env_path) + os.unlink(arg_path) + + def test_default_path_fallback(self, monkeypatch): + """Without env var or explicit path, DEFAULT_MANIFEST_PATH is used.""" + monkeypatch.delenv("TAOS_WORKER_MANIFEST", raising=False) + # The default path won't exist in test, so we get empty manifest. + result = load_manifest() + assert result == {"resource_id": "", "models": []} + # Verify the constant is what we expect. + assert DEFAULT_MANIFEST_PATH == "/etc/taos/worker-models.json" + + +# --------------------------------------------------------------------------- +# Edge cases +# --------------------------------------------------------------------------- + + +class TestLoadManifestEdgeCases: + def test_empty_file_raises(self): + """An empty file is not valid JSON.""" + with tempfile.NamedTemporaryFile( + mode="w", suffix=".json", delete=False + ) as f: + f.write("") + tmp_path = f.name + + try: + with pytest.raises(json.JSONDecodeError): + load_manifest(tmp_path) + finally: + os.unlink(tmp_path) + + def test_missing_model_id_handled(self): + """Model entries missing model_id are still parsed.""" + manifest = { + "resource_id": "w1", + "models": [ + {"capability": "chat", "software": "llamacpp"}, + ], + } + with tempfile.NamedTemporaryFile( + mode="w", suffix=".json", delete=False + ) as f: + json.dump(manifest, f) + tmp_path = f.name + + try: + result = load_manifest(tmp_path) + assert len(result["models"]) == 1 + # load_manifest doesn't validate individual fields — + # that's the enrichment layer's job. + finally: + os.unlink(tmp_path) + + def test_extra_fields_preserved(self): + """Unknown fields in the manifest are passed through.""" + manifest = { + "resource_id": "w1", + "models": [{"model_id": "test", "extra_field": "surprise"}], + } + with tempfile.NamedTemporaryFile( + mode="w", suffix=".json", delete=False + ) as f: + json.dump(manifest, f) + tmp_path = f.name + + try: + result = load_manifest(tmp_path) + assert "extra_field" in result["models"][0] + assert result["models"][0]["extra_field"] == "surprise" + finally: + os.unlink(tmp_path) diff --git a/tinyagentos/cluster/manager.py b/tinyagentos/cluster/manager.py index 507b353d9..ffbed738f 100644 --- a/tinyagentos/cluster/manager.py +++ b/tinyagentos/cluster/manager.py @@ -217,6 +217,18 @@ def heartbeat( if name_m and name_m not in flat_models: flat_models.append(name_m) worker.models = flat_models + # Derive available_models from the live backend catalog. + # Each backend may carry a manifest-enriched + # available_models list; flatten across all backends. + flat_available: list[dict] = [] + seen_ids: set[str] = set() + for b in backends: + for m in b.get("available_models") or []: + model_id = m.get("model_id") or "" + if model_id and model_id not in seen_ids: + seen_ids.add(model_id) + flat_available.append(m) + worker.available_models = flat_available if capabilities is not None: worker.capabilities = list(capabilities) if kv_cache_quant_support is not None: diff --git a/tinyagentos/cluster/worker_protocol.py b/tinyagentos/cluster/worker_protocol.py index 3be747f30..250659caf 100644 --- a/tinyagentos/cluster/worker_protocol.py +++ b/tinyagentos/cluster/worker_protocol.py @@ -16,6 +16,7 @@ class WorkerInfo: # Cross-host callers must route through the worker agent (worker.url) -- never dial backend url directly. backends: list[dict] = field(default_factory=list) # Available inference backends; name is "type:port" models: list[str] = field(default_factory=list) # Currently loaded models + available_models: list[dict] = field(default_factory=list) # Models this worker CAN load (from local manifest) capabilities: list[str] = field(default_factory=list) # embed, chat, rerank, image-gen, tts, etc status: str = "online" # online | offline | busy last_heartbeat: float = 0 diff --git a/tinyagentos/worker/agent.py b/tinyagentos/worker/agent.py index c137e7ec7..ac71d8f32 100644 --- a/tinyagentos/worker/agent.py +++ b/tinyagentos/worker/agent.py @@ -161,6 +161,42 @@ async def detect_backends(self) -> list[dict]: # its cluster-level kv_cache_quant_support advertisement. "kv_quant_support": kv_quant, }) + + # Enrich each backend with available models from the local worker + # manifest. The manifest declares which models this machine *can* + # run (per the GPU catalog), independently of what is currently + # loaded. The controller then sees both "loaded" and "available" + # states so the cluster-wide view reflects total capacity. + from tinyagentos.worker.worker_manifest import ( + load_manifest, + SOFTWARE_TO_BACKEND_TYPE, + ) + + manifest = load_manifest() + if manifest.get("models"): + for backend in backends: + backend_type = backend["type"] + probed_names = { + m.get("name", "") for m in backend.get("models", []) + } + available = [] + for m in manifest["models"]: + if SOFTWARE_TO_BACKEND_TYPE.get(m.get("software", "")) != backend_type: + continue + model_id = m["model_id"] + status = "loaded" if model_id in probed_names else "available" + available.append({ + "model_id": model_id, + "capability": m.get("capability", ""), + "software": m.get("software", ""), + "port": m.get("port", 0), + "vram_required_gb": m.get("vram_required_gb", 0.0), + "health_url": m.get("health_url", ""), + "status": status, + }) + if available: + backend["available_models"] = available + return backends async def _probe_kv_quant( diff --git a/tinyagentos/worker/worker_manifest.py b/tinyagentos/worker/worker_manifest.py new file mode 100644 index 000000000..bb319b20b --- /dev/null +++ b/tinyagentos/worker/worker_manifest.py @@ -0,0 +1,67 @@ +"""Read a local worker manifest that declares which models this machine +*can* run, independently of whatever is currently loaded in RAM. + +The manifest is a simple JSON file (default ``/etc/taos/worker-models.json``, +configurable via ``TAOS_WORKER_MANIFEST``). Any external platform (Skald, +a custom scheduler, a config-management tool) may write it. TAOS core +reads it to advertise *available* (not-yet-loaded) models alongside the +*loaded* models discovered by live backend probing. The controller's +cluster view then knows what a worker *could* load without having to +consult an external catalog. + +The file format is:: + + { + "resource_id": "", + "models": [ + { + "model_id": "", + "software": "llamacpp|embed|kokoro|whisper", + "port": 9090, + "vram_required_gb": 5.3, + "health_url": "http://127.0.0.1:9090/health", + "capability": "text|embed|tts|asr" + } + ] + } + +When the file is absent the worker simply reports an empty available-models +list — no error, no behaviour change for deployments that do not use the +feature. +""" + +from __future__ import annotations + +import json +import os +from pathlib import Path +from typing import Any + +DEFAULT_MANIFEST_PATH = "/etc/taos/worker-models.json" + +# Map software names to TAOS backend types so the worker can attach manifest +# entries to the correct backend dict. +SOFTWARE_TO_BACKEND_TYPE: dict[str, str] = { + "llamacpp": "llama-cpp", + "embed": "llama-cpp", + "kokoro": "kokoro", + "whisper": "whisper", +} + + +def load_manifest(path: str | None = None) -> dict[str, Any]: + """Read the worker-models manifest from disk. + + Args: + path: Override path. Falls back to ``TAOS_WORKER_MANIFEST`` env + var, then ``/etc/taos/worker-models.json``. + + Returns: + Dict with keys ``resource_id`` (str) and ``models`` (list of dicts). + Returns an empty manifest when the file is absent (the worker may + not run with a manifest). + """ + manifest_path = Path(path or os.getenv("TAOS_WORKER_MANIFEST", DEFAULT_MANIFEST_PATH)) + if not manifest_path.exists(): + return {"resource_id": "", "models": []} + return json.loads(manifest_path.read_text()) From 9ac0ff7da9bec6e888becc55d794c5d6721a0df3 Mon Sep 17 00:00:00 2001 From: jaylfc Date: Thu, 9 Jul 2026 17:45:29 +0100 Subject: [PATCH 08/17] fix(install-rknpu): pin rkllama to the 1.3.0 ref + guard the fork patches (#1764) Re-pin TAOS_RKLLAMA_REF from 58038c95 (rknn-llm 1.2.3) to dadea413 (feat/rknpu-1.3.0: upstream 1.3.0 base + the 6 taOS fork patches + the rerank-ABI adaptation for the 1.3.0 options element). This ref was deployed and smoke-verified on the RK3588 production Pi on 2026-07-09, so fresh installs now get 1.3.0 for latest-model support instead of 1.2.3. Also add a static patch-presence guard right after checkout (issue #1730): a past ref bump silently dropped --preload and broke the preloaded embed/rerank/expand models. The installer now refuses to build against any ref whose source is missing --preload or the context-overflow payload, and fails fast before the venv install and service start. Docs-Reviewed: internal dependency-ref bump plus a defensive guard; no change to documented install steps, flags, or user-facing behaviour. --- scripts/install-rknpu.sh | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/scripts/install-rknpu.sh b/scripts/install-rknpu.sh index e01eb1f99..0f8cf4a76 100755 --- a/scripts/install-rknpu.sh +++ b/scripts/install-rknpu.sh @@ -25,7 +25,7 @@ # TAOS_RKNPU_SETUP set to 1/true to skip interactive confirmation # TAOS_RKLLAMA_DIR install dir (default: ~/rkllama) # TAOS_RKLLAMA_REPO git remote (default: https://github.com/jaylfc/rkllama.git) -# TAOS_RKLLAMA_REF git ref (default: 58038c956623e9e18eb39f1ef089f812dd82b6bc) +# TAOS_RKLLAMA_REF git ref (default: dadea413fb38bc28000f18ed7b1d529bb3d18db7) # TAOS_RKLLAMA_PORT HTTP port (default: 7833) # TAOS_QMD_EXPANSION_URL override URL for qmd-query-expansion-1.7B-rk3588.rkllm # (default is the TAOS HF mirror at @@ -63,7 +63,10 @@ LIBRKNNRT_DEST="/usr/lib/librknnrt.so" LIBRKNNRT_EXPECTED_VERSION="2.3.0" RKLLAMA_REPO="${TAOS_RKLLAMA_REPO:-https://github.com/jaylfc/rkllama.git}" -RKLLAMA_REF="${TAOS_RKLLAMA_REF:-58038c956623e9e18eb39f1ef089f812dd82b6bc}" +# rknn-llm 1.3.0: feat/rknpu-1.3.0 @ dadea41 (upstream 1.3.0 base + the 6 fork +# patches + the rerank-ABI adaptation for 1.3.0). Deployed + smoke-verified on +# RK3588 2026-07-09. Was 58038c95 (1.2.3). +RKLLAMA_REF="${TAOS_RKLLAMA_REF:-dadea413fb38bc28000f18ed7b1d529bb3d18db7}" RKLLAMA_PORT="${TAOS_RKLLAMA_PORT:-7833}" # Qwen3-Embedding-0.6B rk3588 rkllm weights. @@ -448,6 +451,18 @@ install_rkllama() { run_as_user git -C "$RKLLAMA_DIR" checkout --quiet "$RKLLAMA_REF" log "rkllama pinned to $(run_as_user git -C "$RKLLAMA_DIR" rev-parse --short HEAD)" + # Guard (issue #1730): a past ref bump silently dropped the --preload flag, + # which broke the preloaded embed/rerank/expand models. Refuse to build + # against any ref that is missing the taOS fork patches we rely on. Static + # source check, so it fails fast before the venv install and service start. + local _missing_patch=() + grep -rqs -- '--preload' "$RKLLAMA_DIR/src" || _missing_patch+=("--preload flag") + grep -rqs 'context_length_exceeded' "$RKLLAMA_DIR/src" || _missing_patch+=("context-overflow payload") + if (( ${#_missing_patch[@]} )); then + die "pinned rkllama ref ${RKLLAMA_REF:0:12} is missing required taOS fork patches: ${_missing_patch[*]}. Refusing to install a ref that dropped them (issue #1730). Update taOS for a current pin." + fi + log "rkllama fork patches present (--preload, context-overflow)" + # rkllama's transitive deps (notably webrtcvad) need a C toolchain to # build wheels from source — Pi images often ship without these. Pull # them via apt before the venv install so `pip install -e .` doesn't From 58cd1598c6191c2100b82415b34957d6713c8b76 Mon Sep 17 00:00:00 2001 From: jaylfc Date: Thu, 9 Jul 2026 22:23:06 +0100 Subject: [PATCH 09/17] fix(worker,models): audit hotfixes - manifest crash, VRAM fail-open, real min_ram_mb key (#1767) Fixes the urgent findings from the full code audit of the recent merges (follow-ups #1765/#1766 track the deeper redesign parts): - worker_manifest.load_manifest: a malformed or non-object manifest file is external input and now degrades to the empty manifest with a logged warning instead of raising. Previously one typo'd comma in /etc/taos/worker-models.json crash-looped the whole worker under systemd. - worker/agent.py enrichment: skip entries without model_id (logged) and wrap the whole enrichment in a fail-soft try/except so no manifest problem can take detect_backends down. Heartbeat send failures are now logged before being swallowed so a payload-build bug no longer masquerades as controller unreachability. - vram_reservation: the probe now returns None (cannot know) distinct from (0, 0) (known-full), and reserve() fails OPEN with bookkeeping-only admission when no probe exists, matching cluster claim_lease semantics. Previously every reservation was denied 503 on AMD/Apple/Rockchip, i.e. on exactly the rkllama hardware the guard was written for. The probe also moved to asyncio.to_thread so nvidia-smi no longer blocks the event loop while holding the reservation lock. stats() gains probe_available. - routes/models.py: the rkllama pull gate now reads the real backend-level requirement (max of variant.requires.backends[].min_ram_mb, falling back to the variant key). Every rkllm variant has variant-level min_ram_mb 0, so the previous read made the TOCTOU gate dead code against the entire catalog. - cluster/backend_services.load_managed_backends: skipped manifests are now logged so a device-side catalog edit cannot silently drop a backend out of #1743 recovery. - install-rknpu.sh: replace the stale copy-of-truth comment (old port 8080 and pre-consolidation models path) with an accurate one. Tests: the two tests that blessed the crash now assert the degrade behaviour, plus new coverage for non-object manifests, no-probe fail-open, probe-unavailable stats, and a regression guard that a real probe showing insufficiency still denies. Docs-Reviewed: behaviour hardening within existing features; no documented install step, endpoint path, or response shape changed (installer diff is a comment correction only). --- scripts/install-rknpu.sh | 9 ++-- tests/test_vram_reservation.py | 43 ++++++++++++++++ tests/test_worker_manifest.py | 32 +++++++++--- tinyagentos/cluster/backend_services.py | 12 +++++ tinyagentos/routes/models.py | 15 ++++-- tinyagentos/vram_reservation.py | 60 ++++++++++++++++------ tinyagentos/worker/agent.py | 68 ++++++++++++++++--------- tinyagentos/worker/worker_manifest.py | 27 ++++++++-- 8 files changed, 206 insertions(+), 60 deletions(-) diff --git a/scripts/install-rknpu.sh b/scripts/install-rknpu.sh index 0f8cf4a76..42df4d472 100755 --- a/scripts/install-rknpu.sh +++ b/scripts/install-rknpu.sh @@ -631,10 +631,11 @@ pull_models() { install_systemd_unit() { local unit="/etc/systemd/system/rkllama.service" local exec_start - # This ExecStart line is copy-of-truth from the live orange pi: - # rkllama_server --processor rk3588 --port 8080 \ - # --models /home/jay/rkllama/models \ - # --preload qwen3-embedding-0.6b,qwen3-reranker-0.6b,qmd-query-expansion + # Matches the unit shape running on the reference RK3588 deployment + # (port 7833, unified models root, --preload of the three shared + # models). Kept in sync with the live Pi unit as of the 1.3.0 deploy; + # the audit flagged the previous comment as stale (it referenced the + # old port 8080 and the pre-consolidation models path). exec_start="$RKLLAMA_VENV/bin/python $RKLLAMA_VENV/bin/rkllama_server --processor $SOC --port $RKLLAMA_PORT --models $RKLLAMA_MODELS --preload qwen3-embedding-0.6b,qwen3-reranker-0.6b,qmd-query-expansion" log "installing $unit" diff --git a/tests/test_vram_reservation.py b/tests/test_vram_reservation.py index 0054b3837..322e7b0f1 100644 --- a/tests/test_vram_reservation.py +++ b/tests/test_vram_reservation.py @@ -236,3 +236,46 @@ async def test_reserve_with_real_probe(self): # this will fail — that's fine, the test is a best-effort smoke. if res is not None: mgr.release(res.reservation_id) + + +# ── no-probe hardware (AMD / Apple / Rockchip): fail open ──────────── + + +def _manager_without_probe() -> VramReservationManager: + """Return a manager whose probe reports no VRAM visibility (None).""" + mgr = VramReservationManager() + mgr._probe_vram = staticmethod(lambda: None) # type: ignore[method-assign] + return mgr + + +class TestNoProbeFailOpen: + """Without a VRAM probe we cannot prove insufficiency, so admission + fails OPEN (bookkeeping only), matching cluster claim_lease semantics. + A closed fail here 503s every model pull on non-NVIDIA hosts.""" + + @pytest.mark.asyncio + async def test_reserve_admits_without_probe(self): + mgr = _manager_without_probe() + res = await mgr.reserve(4096, caller="rkllama-pull:test") + assert res is not None + assert res.vram_mb == 4096 + # Bookkeeping still recorded so a future probe-aware path sees it. + assert mgr.reserved_vram_mb == 4096 + assert mgr.pending_count == 1 + mgr.release(res.reservation_id) + assert mgr.reserved_vram_mb == 0 + + @pytest.mark.asyncio + async def test_stats_reports_probe_unavailable(self): + mgr = _manager_without_probe() + s = mgr.stats() + assert s["probe_available"] is False + assert s["free_vram_mb"] == 0 and s["total_vram_mb"] == 0 + + @pytest.mark.asyncio + async def test_probe_present_still_denies_real_insufficiency(self): + # Regression guard: fail-open applies ONLY to the no-probe case; a + # real probe showing insufficient VRAM must still deny. + mgr = _manager_with_probe(free_mb=1024, total_mb=8192) + res = await mgr.reserve(4096, caller="too-big") + assert res is None diff --git a/tests/test_worker_manifest.py b/tests/test_worker_manifest.py index be9a74eac..60497d74e 100644 --- a/tests/test_worker_manifest.py +++ b/tests/test_worker_manifest.py @@ -99,8 +99,9 @@ def test_manifest_with_empty_models(self): finally: os.unlink(tmp_path) - def test_invalid_json_raises(self): - """Invalid JSON raises json.JSONDecodeError.""" + def test_invalid_json_degrades_to_empty(self): + """Invalid JSON is external input: it must degrade to the empty + manifest (logged), never raise and take the worker down.""" with tempfile.NamedTemporaryFile( mode="w", suffix=".json", delete=False ) as f: @@ -108,8 +109,22 @@ def test_invalid_json_raises(self): tmp_path = f.name try: - with pytest.raises(json.JSONDecodeError): - load_manifest(tmp_path) + result = load_manifest(tmp_path) + assert result == {"resource_id": "", "models": []} + finally: + os.unlink(tmp_path) + + def test_non_object_top_level_degrades_to_empty(self): + """A JSON array / scalar top level degrades to the empty manifest.""" + with tempfile.NamedTemporaryFile( + mode="w", suffix=".json", delete=False + ) as f: + f.write('["not", "an", "object"]') + tmp_path = f.name + + try: + result = load_manifest(tmp_path) + assert result == {"resource_id": "", "models": []} finally: os.unlink(tmp_path) @@ -169,8 +184,9 @@ def test_default_path_fallback(self, monkeypatch): class TestLoadManifestEdgeCases: - def test_empty_file_raises(self): - """An empty file is not valid JSON.""" + def test_empty_file_degrades_to_empty(self): + """An empty file is not valid JSON; it degrades to the empty + manifest rather than raising (bad input must not brick the worker).""" with tempfile.NamedTemporaryFile( mode="w", suffix=".json", delete=False ) as f: @@ -178,8 +194,8 @@ def test_empty_file_raises(self): tmp_path = f.name try: - with pytest.raises(json.JSONDecodeError): - load_manifest(tmp_path) + result = load_manifest(tmp_path) + assert result == {"resource_id": "", "models": []} finally: os.unlink(tmp_path) diff --git a/tinyagentos/cluster/backend_services.py b/tinyagentos/cluster/backend_services.py index 8465f536c..5de4d751f 100644 --- a/tinyagentos/cluster/backend_services.py +++ b/tinyagentos/cluster/backend_services.py @@ -18,6 +18,7 @@ from __future__ import annotations import asyncio +import logging import os from dataclasses import dataclass from pathlib import Path @@ -25,6 +26,8 @@ import httpx import yaml +logger = logging.getLogger(__name__) + VALID_SCOPES = {"system", "user"} @@ -54,8 +57,10 @@ def load_managed_backends(catalog_root: Path) -> list[ManagedBackend]: try: data = yaml.safe_load(manifest.read_text()) except yaml.YAMLError: + logger.warning("skipping unparseable service manifest %s", manifest) continue if not isinstance(data, dict): + logger.warning("skipping non-mapping service manifest %s", manifest) continue lifecycle = data.get("lifecycle") if not isinstance(lifecycle, dict) or lifecycle.get("auto_manage") is not True: @@ -63,6 +68,13 @@ def load_managed_backends(catalog_root: Path) -> list[ManagedBackend]: unit = lifecycle.get("unit") scope = lifecycle.get("scope") if not unit or scope not in VALID_SCOPES: + # The CI lint guarantees these fields for auto-managed services; + # a device-side catalog edit that breaks the contract would + # silently drop the backend out of recovery, so say so. + logger.warning( + "skipping managed backend %s: lifecycle.unit/scope invalid " + "(unit=%r scope=%r)", manifest, unit, scope, + ) continue health = lifecycle.get("health") if isinstance(lifecycle.get("health"), dict) else {} out.append( diff --git a/tinyagentos/routes/models.py b/tinyagentos/routes/models.py index 4bfbb4ed0..984df17ed 100644 --- a/tinyagentos/routes/models.py +++ b/tinyagentos/routes/models.py @@ -385,9 +385,18 @@ async def download_model(request: Request, body: DownloadRequest): reservation = None # TODO(#1725): replace min_ram_mb heuristic with a real per-model VRAM # estimate. On CUDA GGUF the host-RAM floor over-reserves and causes - # occasional false 503s in multi-model setups. Safe for v1 (fails - # closed), but a model-card estimate would be more accurate. - estimated_vram = int(variant.get("min_ram_mb", 0) or 0) # heuristic + # occasional false 503s in multi-model setups. + # The real requirement lives on the backend requirements, not the + # variant top level: every rkllm variant in the catalog has + # variant-level min_ram_mb 0 with the true value (e.g. 2048) under + # requires.backends[].min_ram_mb, so reading only the variant key made + # this gate dead code (audit follow-up, #1766). Take the max across + # required backends, falling back to the variant-level key. + _backend_reqs = (variant.get("requires") or {}).get("backends") or [] + estimated_vram = max( + [int(b.get("min_ram_mb", 0) or 0) for b in _backend_reqs if isinstance(b, dict)] + + [int(variant.get("min_ram_mb", 0) or 0)] + ) if vram_mgr is not None and estimated_vram > 0: reservation = await vram_mgr.reserve( estimated_vram, caller=f"rkllama-pull:{body.app_id}", diff --git a/tinyagentos/vram_reservation.py b/tinyagentos/vram_reservation.py index 1b2eee783..af300a4f1 100644 --- a/tinyagentos/vram_reservation.py +++ b/tinyagentos/vram_reservation.py @@ -87,16 +87,32 @@ async def reserve( ) async with self._lock: - free_mb, total_mb = self._probe_vram() - effective_free = max(0, free_mb - self._reserved_vram_mb) - - if effective_free < vram_mb: - logger.debug( - "vram-reservation: denied — need %d MiB, have %d MiB " - "(%d free − %d reserved)", - vram_mb, effective_free, free_mb, self._reserved_vram_mb, + # Probe in a thread: nvidia-smi is a blocking subprocess and this + # runs on a request hot path while holding the lock. + probe = await asyncio.to_thread(self._probe_vram) + if probe is None: + # No VRAM probe on this hardware (AMD/Apple/Rockchip or a + # failed nvidia-smi). Fail OPEN, matching the cluster lease + # ledger's convention (manager.claim_lease): we cannot prove + # insufficiency, so admit and keep bookkeeping only. A closed + # fail here would 503 every model pull on non-NVIDIA hosts. + free_mb, total_mb = 0, 0 + effective_free = vram_mb # unknown; grant-log then shows 0 + logger.info( + "vram-reservation: no VRAM probe available; admitting %d " + "MiB for %r unenforced (bookkeeping only)", vram_mb, caller, ) - return None + else: + free_mb, total_mb = probe + effective_free = max(0, free_mb - self._reserved_vram_mb) + + if effective_free < vram_mb: + logger.debug( + "vram-reservation: denied — need %d MiB, have %d MiB " + "(%d free − %d reserved)", + vram_mb, effective_free, free_mb, self._reserved_vram_mb, + ) + return None reservation_id = f"vr_{secrets.token_hex(4)}" reservation = VramReservation( @@ -148,15 +164,25 @@ def pending_count(self) -> int: def available_vram(self) -> tuple[int, int]: """Return ``(effective_free_mb, total_mb)`` after subtracting - in-flight reservations from the hardware probe.""" - free_mb, total_mb = self._probe_vram() + in-flight reservations from the hardware probe. + + When no probe is available, returns ``(0, 0)``; callers on that + path should already have been admitted fail-open by :meth:`reserve` + (a deny message never reports these zeros as real capacity). + """ + probe = self._probe_vram() + if probe is None: + return 0, 0 + free_mb, total_mb = probe effective_free = max(0, free_mb - self._reserved_vram_mb) return effective_free, total_mb def stats(self) -> dict: """Return a snapshot for monitoring / debug endpoints.""" - free_mb, total_mb = self._probe_vram() + probe = self._probe_vram() + free_mb, total_mb = probe if probe is not None else (0, 0) return { + "probe_available": probe is not None, "free_vram_mb": free_mb, "total_vram_mb": total_mb, "reserved_vram_mb": self._reserved_vram_mb, @@ -167,11 +193,13 @@ def stats(self) -> dict: # ── internal ──────────────────────────────────────────────────── @staticmethod - def _probe_vram() -> tuple[int, int]: + def _probe_vram() -> tuple[int, int] | None: """Probe real-time free/total VRAM via nvidia-smi. - Returns ``(free_mb, total_mb)``, or ``(0, 0)`` when no - nvidia-smi is available or the probe fails. + Returns ``(free_mb, total_mb)``, or ``None`` when no nvidia-smi + is available or the probe fails. ``None`` (cannot know) is + deliberately distinct from ``(0, 0)`` (known-full): admission + fails open on ``None``, matching cluster claim_lease semantics. """ try: from tinyagentos.system_stats import read_nvidia_vram @@ -183,4 +211,4 @@ def _probe_vram() -> tuple[int, int]: return free_mb, total_mb except Exception: logger.debug("vram-reservation: probe failed", exc_info=True) - return 0, 0 + return None diff --git a/tinyagentos/worker/agent.py b/tinyagentos/worker/agent.py index ac71d8f32..8c5f69e17 100644 --- a/tinyagentos/worker/agent.py +++ b/tinyagentos/worker/agent.py @@ -172,30 +172,44 @@ async def detect_backends(self) -> list[dict]: SOFTWARE_TO_BACKEND_TYPE, ) - manifest = load_manifest() - if manifest.get("models"): - for backend in backends: - backend_type = backend["type"] - probed_names = { - m.get("name", "") for m in backend.get("models", []) - } - available = [] - for m in manifest["models"]: - if SOFTWARE_TO_BACKEND_TYPE.get(m.get("software", "")) != backend_type: - continue - model_id = m["model_id"] - status = "loaded" if model_id in probed_names else "available" - available.append({ - "model_id": model_id, - "capability": m.get("capability", ""), - "software": m.get("software", ""), - "port": m.get("port", 0), - "vram_required_gb": m.get("vram_required_gb", 0.0), - "health_url": m.get("health_url", ""), - "status": status, - }) - if available: - backend["available_models"] = available + # The manifest is external input: a malformed file or entry must + # degrade to "no manifest" (logged), never crash detect_backends() + # and take the whole worker down with it. + try: + manifest = load_manifest() + if manifest.get("models"): + for backend in backends: + backend_type = backend["type"] + probed_names = { + m.get("name", "") for m in backend.get("models", []) + } + available = [] + for m in manifest["models"]: + if not isinstance(m, dict): + continue + if SOFTWARE_TO_BACKEND_TYPE.get(m.get("software", "")) != backend_type: + continue + model_id = m.get("model_id") + if not model_id: + logger.warning( + "skipping worker-manifest entry without model_id: %r", m + ) + continue + status = "loaded" if model_id in probed_names else "available" + available.append({ + "model_id": model_id, + "capability": m.get("capability", ""), + "software": m.get("software", ""), + "port": m.get("port", 0), + "vram_required_gb": m.get("vram_required_gb", 0.0), + "health_url": m.get("health_url", ""), + "status": status, + }) + if available: + backend["available_models"] = available + except Exception: # noqa: BLE001 - manifest must never brick the worker + logger.warning("worker-manifest enrichment failed; continuing without it", + exc_info=True) return backends @@ -567,7 +581,11 @@ async def heartbeat(self) -> int: headers=auth_headers, ) return resp.status_code - except Exception: + except Exception as exc: # noqa: BLE001 + # Log before swallowing: a payload-build bug (not just a network + # blip) would otherwise drop the worker offline with zero + # diagnostics, masquerading as controller unreachability. + logger.warning("heartbeat send failed: %s: %s", type(exc).__name__, exc) return 0 def _log_repair_instruction(self) -> None: diff --git a/tinyagentos/worker/worker_manifest.py b/tinyagentos/worker/worker_manifest.py index bb319b20b..ea7352891 100644 --- a/tinyagentos/worker/worker_manifest.py +++ b/tinyagentos/worker/worker_manifest.py @@ -33,10 +33,13 @@ from __future__ import annotations import json +import logging import os from pathlib import Path from typing import Any +logger = logging.getLogger(__name__) + DEFAULT_MANIFEST_PATH = "/etc/taos/worker-models.json" # Map software names to TAOS backend types so the worker can attach manifest @@ -58,10 +61,26 @@ def load_manifest(path: str | None = None) -> dict[str, Any]: Returns: Dict with keys ``resource_id`` (str) and ``models`` (list of dicts). - Returns an empty manifest when the file is absent (the worker may - not run with a manifest). + Returns an empty manifest when the file is absent OR unreadable/ + malformed. The manifest is external input (any platform may write + it), so a typo in it must never take the worker down -- a bad file + is logged and treated as no-manifest rather than raised. """ manifest_path = Path(path or os.getenv("TAOS_WORKER_MANIFEST", DEFAULT_MANIFEST_PATH)) + empty: dict[str, Any] = {"resource_id": "", "models": []} if not manifest_path.exists(): - return {"resource_id": "", "models": []} - return json.loads(manifest_path.read_text()) + return empty + try: + data = json.loads(manifest_path.read_text()) + except (OSError, json.JSONDecodeError) as exc: + logger.warning( + "ignoring malformed worker manifest %s: %s", manifest_path, exc + ) + return empty + if not isinstance(data, dict) or not isinstance(data.get("models", []), list): + logger.warning( + "ignoring worker manifest %s: top level must be an object with a " + "'models' list", manifest_path, + ) + return empty + return data From f221bffbd35925f958de860212cb776c87c92560 Mon Sep 17 00:00:00 2001 From: jaylfc Date: Fri, 10 Jul 2026 01:11:09 +0100 Subject: [PATCH 10/17] feat(a2a): authenticated bus send proxy (agents post as themselves, no spoofing) (#1768) * feat(a2a): authenticated bus send proxy so agents post as themselves The controller's A2A bus proxy was read-only; the only send path was the raw :7900 bus, which is unauthenticated on the LAN and trusts its `from` field. So a non-owner agent (e.g. @taOS-website-dev) had to ride the owner's account /SSH to post, and any caller could spoof any identity. Add POST /api/a2a/bus/send: - Authorized senders: an admin session/local token (may set an explicit `from`), or an agent registry JWT holding the `a2a_send` scope. - For an agent caller the bus `from` is DERIVED from the agent's own registry handle; a client-supplied `from` is ignored, so one agent cannot post as another. - Fails 502 on bus error (a send must know it did not land), unlike the read proxies which degrade to an empty 200. - Allowlisted in the auth middleware as a write path (own set), preserving the no-skeleton-key guard. Tests: 7 new cases (from-derivation/anti-spoof, scope required, no-auth 401, admin explicit-from, validation 400, bus-error 502). Docs: agent-coordination.md gains the authenticated-send path; module docstring updated. doc-gate clean. * fold Kilo review on #1768: sanitize admin from, trim body, validate reply_to - Admin-supplied "from" is stripped of non-printable chars and capped at 64 so it cannot inject newlines/control chars into the bus record or logs. - The message body is trimmed before forwarding, consistent with thread. - reply_to, when present, must be a positive message id (rejects 0/negative). - 4 new tests cover the three folds. * bound reply_to to the 64-bit id space (Kilo suggestion on #1768) --- docs/agent-coordination.md | 14 ++++ tests/test_a2a_bus_agent_auth.py | 133 +++++++++++++++++++++++++++++++ tinyagentos/auth_middleware.py | 8 +- tinyagentos/routes/a2a_bus.py | 112 ++++++++++++++++++++++++-- 4 files changed, 259 insertions(+), 8 deletions(-) diff --git a/docs/agent-coordination.md b/docs/agent-coordination.md index 8bedd7cd5..735ce81d7 100644 --- a/docs/agent-coordination.md +++ b/docs/agent-coordination.md @@ -81,6 +81,20 @@ POST /a2a/send - After posting to a specific thread, verify it landed there (read the thread back and confirm your message id is present) before assuming it was delivered. +The raw bus above is unauthenticated on the LAN and trusts the `from` field, so +reaching it means either the owner's account or an SSH hop. A registered agent +should instead post through the controller's authenticated proxy, which forces +`from` to the agent's own registry handle (no spoofing) — so it posts as itself, +not the owner: + +``` +POST /api/a2a/bus/send (Authorization: Bearer ) +{"thread": "build", "body": "...", "reply_to": ?} +``` + +An admin session may also call it and set an explicit `from`. On a bus failure +the proxy returns 502 (the read proxies degrade to an empty 200 instead). + These rules are deliberately lightweight. The goal is not process for its own sake; it is to let many hands move quickly on the same codebase without undoing each other's work. diff --git a/tests/test_a2a_bus_agent_auth.py b/tests/test_a2a_bus_agent_auth.py index 3f9e8fc02..d760fb093 100644 --- a/tests/test_a2a_bus_agent_auth.py +++ b/tests/test_a2a_bus_agent_auth.py @@ -271,3 +271,136 @@ async def test_skeleton_key_guard_agent_token_rejected_off_allowlist(self, bus_c ) # Falls through middleware to the session gate: API request -> 401. assert resp.status_code == 401 + + +def _mock_bus_post(json_payload: dict, *, raise_on_status: bool = False): + """A mock httpx.AsyncClient whose async ctx yields a client whose + .post().json() returns *json_payload*. If *raise_on_status* the mocked + response.raise_for_status() raises, simulating a bus error.""" + mock_resp = MagicMock() + if raise_on_status: + mock_resp.raise_for_status = MagicMock(side_effect=RuntimeError("bus 500")) + else: + mock_resp.raise_for_status = MagicMock() + mock_resp.json.return_value = json_payload + + mock_client = AsyncMock() + mock_client.post.return_value = mock_resp + + mock_ctx = AsyncMock() + mock_ctx.__aenter__ = AsyncMock(return_value=mock_client) + mock_ctx.__aexit__ = AsyncMock(return_value=False) + return mock_ctx, mock_client + + +@pytest.mark.asyncio +class TestBusAgentSend: + """Authenticated write path: agents post as themselves, never spoofing.""" + + async def test_agent_send_derives_from_from_registry_handle(self, bus_client): + # _make_agent_token registers handle "@bus-reader". + _cid, token = await _make_agent_token(bus_client._app, scopes=("a2a_send",)) + ctx, client = _mock_bus_post({"id": 1, "from": "@bus-reader", "thread": "build"}) + with patch(_BUS_PATCH, return_value=ctx): + async with _bare(bus_client._app) as bare: + resp = await bare.post( + "/api/a2a/bus/send", + json={"thread": "build", "body": "hi", "from": "@somebody-else"}, + headers={"Authorization": f"Bearer {token}"}, + ) + assert resp.status_code == 200 + assert resp.json()["from"] == "@bus-reader" + # Anti-spoof: the body's "from" is ignored; the bus is called with the + # agent's own registry handle. + sent = client.post.call_args.kwargs["json"] + assert sent["from"] == "@bus-reader" + assert sent["thread"] == "build" and sent["body"] == "hi" + + async def test_agent_without_send_scope_forbidden(self, bus_client): + # Only a2a_receive -> cannot send. + _cid, token = await _make_agent_token(bus_client._app, scopes=("a2a_receive",)) + async with _bare(bus_client._app) as bare: + resp = await bare.post( + "/api/a2a/bus/send", + json={"thread": "build", "body": "hi"}, + headers={"Authorization": f"Bearer {token}"}, + ) + assert resp.status_code == 403 + + async def test_no_auth_rejected(self, bus_client): + # No Bearer header at all: the auth middleware rejects the API request + # with 401 before it reaches the route (no credentials presented). The + # route's own 403 covers a *valid* token that lacks the a2a_send grant. + async with _bare(bus_client._app) as bare: + resp = await bare.post( + "/api/a2a/bus/send", + json={"thread": "build", "body": "hi"}, + ) + assert resp.status_code == 401 + + async def test_admin_may_send_with_explicit_from(self, bus_client): + ctx, client = _mock_bus_post({"id": 2, "from": "@operator", "thread": "general"}) + with patch(_BUS_PATCH, return_value=ctx): + resp = await bus_client.post( + "/api/a2a/bus/send", + json={"thread": "general", "body": "ops note", "from": "@release-bot"}, + ) + assert resp.status_code == 200 + # Admin's explicit from is honored. + assert client.post.call_args.kwargs["json"]["from"] == "@release-bot" + + async def test_missing_thread_or_body_400(self, bus_client): + _cid, token = await _make_agent_token(bus_client._app, scopes=("a2a_send",)) + async with _bare(bus_client._app) as bare: + resp = await bare.post( + "/api/a2a/bus/send", + json={"thread": " ", "body": "hi"}, + headers={"Authorization": f"Bearer {token}"}, + ) + assert resp.status_code == 400 + + async def test_reply_to_must_be_positive(self, bus_client): + _cid, token = await _make_agent_token(bus_client._app, scopes=("a2a_send",)) + async with _bare(bus_client._app) as bare: + resp = await bare.post( + "/api/a2a/bus/send", + json={"thread": "build", "body": "hi", "reply_to": 0}, + headers={"Authorization": f"Bearer {token}"}, + ) + assert resp.status_code == 400 + + async def test_body_is_trimmed_before_forwarding(self, bus_client): + _cid, token = await _make_agent_token(bus_client._app, scopes=("a2a_send",)) + ctx, client = _mock_bus_post({"id": 9, "from": "@bus-reader", "thread": "build"}) + with patch(_BUS_PATCH, return_value=ctx): + async with _bare(bus_client._app) as bare: + resp = await bare.post( + "/api/a2a/bus/send", + json={"thread": "build", "body": " padded "}, + headers={"Authorization": f"Bearer {token}"}, + ) + assert resp.status_code == 200 + assert client.post.call_args.kwargs["json"]["body"] == "padded" + + async def test_admin_from_control_chars_stripped(self, bus_client): + ctx, client = _mock_bus_post({"id": 10, "from": "@x", "thread": "general"}) + with patch(_BUS_PATCH, return_value=ctx): + resp = await bus_client.post( + "/api/a2a/bus/send", + json={"thread": "general", "body": "hi", "from": "@evil\nInjected: x"}, + ) + assert resp.status_code == 200 + # Newline/control chars removed so nothing can be injected downstream. + assert "\n" not in client.post.call_args.kwargs["json"]["from"] + + async def test_bus_error_surfaces_502(self, bus_client): + _cid, token = await _make_agent_token(bus_client._app, scopes=("a2a_send",)) + ctx, _client = _mock_bus_post({}, raise_on_status=True) + with patch(_BUS_PATCH, return_value=ctx): + async with _bare(bus_client._app) as bare: + resp = await bare.post( + "/api/a2a/bus/send", + json={"thread": "build", "body": "hi"}, + headers={"Authorization": f"Bearer {token}"}, + ) + assert resp.status_code == 502 diff --git a/tinyagentos/auth_middleware.py b/tinyagentos/auth_middleware.py index 32e813b69..02ce7991b 100644 --- a/tinyagentos/auth_middleware.py +++ b/tinyagentos/auth_middleware.py @@ -23,10 +23,16 @@ "/api/a2a/bus/channels", "/api/a2a/bus/messages", }) +# Authenticated A2A bus WRITE path: an agent may POST here with its own registry +# JWT (scope a2a_send, verified by the route, which forces the bus `from` to the +# agent's own handle so it posts as itself instead of the owner's account). +_A2A_BUS_WRITE_PATHS = frozenset({ + "/api/a2a/bus/send", +}) # Every path that accepts a registry JWT in place of the admin session. The # passthrough is allowlisted to exactly these paths -- a registry JWT must never # authenticate an arbitrary route (no skeleton key). -_AGENT_TOKEN_PATHS = _REGISTRY_FEED_PATHS | _A2A_BUS_READ_PATHS +_AGENT_TOKEN_PATHS = _REGISTRY_FEED_PATHS | _A2A_BUS_READ_PATHS | _A2A_BUS_WRITE_PATHS # Bundle assets and the SPA shell HTML must be reachable without auth so: # 1. The browser can install and cache the shell for offline / PWA use. # 2. After a backend restart the cached shell loads immediately without diff --git a/tinyagentos/routes/a2a_bus.py b/tinyagentos/routes/a2a_bus.py index cc05583f6..9a654fdaf 100644 --- a/tinyagentos/routes/a2a_bus.py +++ b/tinyagentos/routes/a2a_bus.py @@ -6,15 +6,21 @@ agents (@taOS, @taOSmd, @hermes) coordinate. This is DISTINCT from taOS's own internal per-project a2a channels (tinyagentos/projects/a2a.py). -These endpoints are a thin READ-ONLY proxy: the Messages app can list bus -channels and read messages, but there is no send/post path here. The bus is -unauthenticated on the LAN; the URL is resolved from ``TAOS_A2A_BUS_URL``. +These endpoints proxy the bus: the Messages app can list bus channels and read +messages (read paths), and a registry-authenticated agent (or an admin) can post +a message (POST /api/a2a/bus/send). The raw bus is unauthenticated on the LAN and +trusts its ``from`` field, so the send proxy is the authenticated write path -- +an agent posts as its OWN registry handle (scope ``a2a_send``), never able to +spoof another identity or borrow the owner's account. The URL is resolved from +``TAOS_A2A_BUS_URL``. Bus API (verified live): - GET {bus}/a2a/channels - -> {"channels":[{"channel","members","message_count","created_ts","last_ts"}, ...]} - GET {bus}/a2a/messages?thread={channel}&limit={n} - -> {"messages":[{"id","ts","from","body","thread","reply_to"}, ...]} + GET {bus}/a2a/channels + -> {"channels":[{"channel","members","message_count","created_ts","last_ts"}, ...]} + GET {bus}/a2a/messages?thread={channel}&limit={n} + -> {"messages":[{"id","ts","from","body","thread","reply_to"}, ...]} + POST {bus}/a2a/send {"from","thread","body","reply_to"?} + -> {"id","from","thread","reply_to"} """ from __future__ import annotations @@ -24,6 +30,7 @@ import httpx from fastapi import APIRouter, HTTPException, Request from fastapi.responses import JSONResponse +from pydantic import BaseModel, Field from tinyagentos.agent_token_auth import check_agent_scope @@ -118,3 +125,94 @@ async def bus_messages(request: Request, channel: str = "", limit: int = 100): messages = data.get("messages", []) if isinstance(data, dict) else [] return {"messages": messages, "available": True} + + +class BusSendBody(BaseModel): + """A message to post to a coordination-bus thread. + + ``from_`` (JSON key ``from``) is honored ONLY for admin callers, so an + operator can post as any handle. For an agent-token caller it is ignored and + the ``from`` is derived from the agent's own registry handle -- one agent can + never post as another. + """ + + thread: str + body: str + reply_to: int | None = None + from_: str | None = Field(default=None, alias="from") + + model_config = {"populate_by_name": True} + + +async def _resolve_send_identity(request: Request, body_from: str | None) -> str: + """Return the bus ``from`` handle authorized for this send, or raise. + + - Admin (session cookie or local token): may set an explicit ``from`` + (operator posts as any handle); defaults to ``@operator`` when omitted. + - Otherwise the caller must present a registry JWT holding an active + ``a2a_send`` grant. The ``from`` is DERIVED from that agent's registry + handle; a client-supplied ``from`` is ignored, so an agent cannot spoof + another agent's identity. ``check_agent_scope`` raises 401/403; a missing + Bearer header returns None and is rejected here as 403 (fail closed). + """ + if getattr(request.state, "is_admin", False): + # Admin may post as an explicit handle, but keep it a single clean token + # so it cannot inject newlines/control chars into the bus record or logs. + handle = (body_from or "").strip() + handle = "".join(c for c in handle if c.isprintable())[:64].strip() + return handle or "@operator" + + caller = await check_agent_scope(request, "a2a_send") + if caller is None: + raise HTTPException(status_code=403, detail="forbidden") + + registry = getattr(request.app.state, "agent_registry", None) + record = await registry.get(caller) if registry is not None else None + handle = ((record or {}).get("handle") or "").strip() + if not handle: + # An active a2a_send grant with no bus handle cannot be safely attributed. + raise HTTPException(status_code=403, detail="agent has no bus handle") + return handle + + +@router.post("/api/a2a/bus/send") +async def bus_send(request: Request, body: BusSendBody): + """Post a message to a coordination-bus thread as the authenticated identity. + + Authorized senders: an admin session / host local token (may set ``from``), + or an active agent registry JWT holding the ``a2a_send`` scope (``from`` is + forced to the agent's own handle). This is the authenticated write path so + agents post as themselves instead of sharing the owner's account. + + Unlike the read endpoints, a bus failure surfaces as 502: a caller must know + when its message did not land. + """ + from_handle = await _resolve_send_identity(request, body.from_) + + thread = body.thread.strip() + text = body.body.strip() + if not thread or not text: + return JSONResponse({"error": "thread and body required"}, status_code=400) + # reply_to is a bus message id (a SQLite rowid): must be a positive integer + # within the signed-64-bit id space. The bus owns id existence; the proxy + # only rejects values that could never be a real id. + if body.reply_to is not None and not (0 < body.reply_to <= 2**63 - 1): + return JSONResponse( + {"error": "reply_to must be a positive message id"}, status_code=400 + ) + + payload: dict = {"from": from_handle, "thread": thread, "body": text} + if body.reply_to is not None: + payload["reply_to"] = body.reply_to + + bus = _bus_url() + try: + async with httpx.AsyncClient(timeout=5.0) as client: + resp = await client.post(f"{bus}/a2a/send", json=payload) + resp.raise_for_status() + data = resp.json() + except Exception as exc: # noqa: BLE001 + logger.warning("A2A bus send failed (%s): %s", bus, exc) + raise HTTPException(status_code=502, detail="a2a bus unavailable") + + return {"ok": True, "from": from_handle, "message": data} From bd860203a198fa0e46433e0659aae17bfabdaeac Mon Sep 17 00:00:00 2001 From: jaylfc Date: Fri, 10 Jul 2026 15:41:16 +0100 Subject: [PATCH 11/17] test(designstudio): add coverage for the untested Design Studio app (#1769) * test(designstudio): add coverage for the untested Design Studio app The designstudio/ directory shipped with zero tests. Add 105 vitest cases across 11 files covering the pure logic and the view components, matching the music/game studio testing conventions (fetch stubbing, controlled-host rendering, react-konva mocked to inert passthroughs). - types: hasFill/hasStroke type guards, isValidDesignContent validation, constants - elementFactory: element creation, centering, zIndex stacking, id uniqueness, duplicate - designs-api: list/get/create/update/rename/delete fetch wiring and error messages - useElementHistory: commit/undo/redo, redo-stack clearing, empty-stack no-ops, history cap - useHtmlImage: load/error lifecycle, crossOrigin, unmount cancellation - LayersPanel: ordering, labels, select/reorder/visibility/delete, disabled edges - PropertiesPanel: per-type controls, position display, duplicate/delete, disabled chips - TemplatesView: grid, filter pills, selection dimensions - MagicView: prompt/generate/style chips, needs-model and error states, results - LibraryView: empty/error states, open, backend-confirmed delete, rename - DesignView: toolbar add/undo, auto-select, brand-swatch gating, zoom controls * fold Kilo suggestion on #1769: verify the history-cap eviction branch The undo-cap test only asserted the most-recent snapshot after undo; now it also walks undo to exhaustion and asserts the original (v0) state was evicted and the undo depth is bounded by the 100-entry cap, exercising the eviction branch. (The DesignView 'latest' state-isolation suggestion is already handled by the existing afterEach reset.) --- .../src/apps/designstudio/DesignView.test.tsx | 137 +++++++++++++ .../apps/designstudio/LayersPanel.test.tsx | 105 ++++++++++ .../apps/designstudio/LibraryView.test.tsx | 107 +++++++++++ .../src/apps/designstudio/MagicView.test.tsx | 93 +++++++++ .../designstudio/PropertiesPanel.test.tsx | 138 +++++++++++++ .../apps/designstudio/TemplatesView.test.tsx | 34 ++++ .../src/apps/designstudio/designs-api.test.ts | 125 ++++++++++++ .../apps/designstudio/elementFactory.test.ts | 141 ++++++++++++++ desktop/src/apps/designstudio/types.test.ts | 181 ++++++++++++++++++ .../designstudio/useElementHistory.test.ts | 139 ++++++++++++++ .../apps/designstudio/useHtmlImage.test.ts | 74 +++++++ 11 files changed, 1274 insertions(+) create mode 100644 desktop/src/apps/designstudio/DesignView.test.tsx create mode 100644 desktop/src/apps/designstudio/LayersPanel.test.tsx create mode 100644 desktop/src/apps/designstudio/LibraryView.test.tsx create mode 100644 desktop/src/apps/designstudio/MagicView.test.tsx create mode 100644 desktop/src/apps/designstudio/PropertiesPanel.test.tsx create mode 100644 desktop/src/apps/designstudio/TemplatesView.test.tsx create mode 100644 desktop/src/apps/designstudio/designs-api.test.ts create mode 100644 desktop/src/apps/designstudio/elementFactory.test.ts create mode 100644 desktop/src/apps/designstudio/types.test.ts create mode 100644 desktop/src/apps/designstudio/useElementHistory.test.ts create mode 100644 desktop/src/apps/designstudio/useHtmlImage.test.ts diff --git a/desktop/src/apps/designstudio/DesignView.test.tsx b/desktop/src/apps/designstudio/DesignView.test.tsx new file mode 100644 index 000000000..1c7044521 --- /dev/null +++ b/desktop/src/apps/designstudio/DesignView.test.tsx @@ -0,0 +1,137 @@ +import { describe, it, expect, vi, beforeAll, afterEach } from "vitest"; +import { useState } from "react"; +import { render, screen, fireEvent } from "@testing-library/react"; + +/* react-konva needs a real (the `canvas` npm package), which is not + * installed in the jsdom test env. Mock the primitives to inert passthroughs so + * DesignView's toolbar / layers / properties logic can be exercised without a + * Konva stage. Because the Stage/Transformer refs are never populated, the + * canvas-only effects (transformer wiring, PNG export) safely no-op. */ +vi.mock("react-konva", () => ({ + Stage: ({ children }: { children?: React.ReactNode }) =>
{children}
, + Layer: ({ children }: { children?: React.ReactNode }) =>
{children}
, + Rect: () => null, + Text: () => null, + Image: () => null, + Ellipse: () => null, + Line: () => null, + Transformer: () => null, +})); + +import { DesignView } from "./DesignView"; +import { DEFAULT_ARTBOARD, type CanvasElement } from "./types"; + +/** DesignView is a controlled component: it hands the next elements array back + * through onElementsChange and re-reads it from props. This host mirrors how + * DesignStudioApp owns that state, and exposes the latest array for assertions. */ +let latest: CanvasElement[] = []; +function Host() { + const [els, setEls] = useState([]); + latest = els; + return ; +} + +function renderView() { + return render(); +} + +beforeAll(() => { + vi.stubGlobal( + "ResizeObserver", + class { + observe() {} + unobserve() {} + disconnect() {} + }, + ); +}); + +afterEach(() => { + latest = []; + vi.clearAllMocks(); +}); + +describe("DesignView", () => { + it("renders the empty state with export/undo/redo disabled", () => { + renderView(); + expect(screen.getByText(/Add an element or generate with Magic/i)).toBeDefined(); + expect((screen.getByLabelText("Export as PNG") as HTMLButtonElement).disabled).toBe(true); + expect((screen.getByLabelText("Undo") as HTMLButtonElement).disabled).toBe(true); + expect((screen.getByLabelText("Redo") as HTMLButtonElement).disabled).toBe(true); + expect(screen.getByText(/No layers yet/i)).toBeDefined(); + }); + + it("keeps the coming-soon Star tile disabled", () => { + renderView(); + expect((screen.getByLabelText("Star") as HTMLButtonElement).disabled).toBe(true); + expect((screen.getByLabelText("Text") as HTMLButtonElement).disabled).toBe(false); + }); + + it("adding a text element updates state and adds a layer, enabling export/undo", () => { + renderView(); + fireEvent.click(screen.getByLabelText("Text")); + + expect(latest).toHaveLength(1); + expect(latest[0].type).toBe("text"); + expect(screen.getByLabelText(/Select layer/i)).toBeDefined(); + expect((screen.getByLabelText("Export as PNG") as HTMLButtonElement).disabled).toBe(false); + expect((screen.getByLabelText("Undo") as HTMLButtonElement).disabled).toBe(false); + }); + + it("adding a shape creates a rect and auto-selects it (properties panel appears)", () => { + renderView(); + fireEvent.click(screen.getByLabelText("Shape")); + + expect(latest[0].type).toBe("rect"); + // Auto-selection surfaces the shape's fill/stroke controls. + expect(screen.getByLabelText("Fill color")).toBeDefined(); + expect(screen.getByText("Rectangle")).toBeDefined(); + }); + + it("adding a circle creates an ellipse", () => { + renderView(); + fireEvent.click(screen.getByLabelText("Circle")); + expect(latest[0].type).toBe("ellipse"); + }); + + it("adding a line creates a line element", () => { + renderView(); + fireEvent.click(screen.getByLabelText("Line")); + expect(latest[0].type).toBe("line"); + }); + + it("undo removes a just-added element and returns to the empty state", () => { + renderView(); + fireEvent.click(screen.getByLabelText("Text")); + expect(latest).toHaveLength(1); + fireEvent.click(screen.getByLabelText("Undo")); + expect(latest).toHaveLength(0); + expect(screen.getByText(/Add an element or generate with Magic/i)).toBeDefined(); + }); + + it("brand swatches are disabled until a fillable element is selected", () => { + renderView(); + expect((screen.getByLabelText("Apply #6b7689") as HTMLButtonElement).disabled).toBe(true); + + fireEvent.click(screen.getByLabelText("Shape")); + expect((screen.getByLabelText("Apply #6b7689") as HTMLButtonElement).disabled).toBe(false); + }); + + it("exposes zoom controls that fit the artboard to the stage", () => { + renderView(); + const zoom = screen.getByLabelText("Zoom level") as HTMLSelectElement; + // The view fits the artboard on mount, so the zoom is a positive percentage. + expect(Number(zoom.value)).toBeGreaterThan(0); + expect(screen.getByLabelText("Zoom in")).toBeDefined(); + expect(screen.getByLabelText("Zoom out")).toBeDefined(); + expect(screen.getByLabelText("Fit to screen")).toBeDefined(); + }); + + it("shows the artboard name and dimensions in the header", () => { + renderView(); + expect(screen.getByText(DEFAULT_ARTBOARD.name)).toBeDefined(); + expect( + screen.getByText(`${DEFAULT_ARTBOARD.width} x ${DEFAULT_ARTBOARD.height}`), + ).toBeDefined(); + }); +}); diff --git a/desktop/src/apps/designstudio/LayersPanel.test.tsx b/desktop/src/apps/designstudio/LayersPanel.test.tsx new file mode 100644 index 000000000..b42a1e8e1 --- /dev/null +++ b/desktop/src/apps/designstudio/LayersPanel.test.tsx @@ -0,0 +1,105 @@ +import { describe, it, expect, vi } from "vitest"; +import { render, screen, fireEvent } from "@testing-library/react"; +import { LayersPanel } from "./LayersPanel"; +import type { CanvasElement } from "./types"; + +function text(id: string, zIndex: number, over: Partial = {}): CanvasElement { + return { + id, + type: "text", + x: 0, + y: 0, + width: 100, + height: 40, + rotation: 0, + zIndex, + visible: true, + text: "Hello world", + fontSize: 32, + fontFamily: "Inter", + fill: "#fff", + align: "left", + ...over, + } as CanvasElement; +} + +function baseProps() { + return { + onSelect: vi.fn(), + onToggleVisibility: vi.fn(), + onReorder: vi.fn(), + onDelete: vi.fn(), + }; +} + +describe("LayersPanel", () => { + it("shows an empty state with no layers", () => { + render(); + expect(screen.getByText(/No layers yet/i)).toBeDefined(); + }); + + it("lists layers top-most (highest zIndex) first", () => { + const els = [text("low", 0, { text: "Bottom" }), text("high", 5, { text: "Top" })]; + render(); + const labels = screen.getAllByText(/Top|Bottom/).map((n) => n.textContent); + expect(labels[0]).toBe("Top"); + expect(labels[1]).toBe("Bottom"); + }); + + it("labels typed layers by their content", () => { + const els = [ + text("t", 3), + { ...text("r", 2), type: "rect", fill: "#000", stroke: "#fff", strokeWidth: 0 } as CanvasElement, + { ...text("l", 1), type: "line", stroke: "#fff", strokeWidth: 3 } as CanvasElement, + ]; + render(); + expect(screen.getByText("Hello world")).toBeDefined(); + expect(screen.getByText("Rectangle")).toBeDefined(); + expect(screen.getByText("Line")).toBeDefined(); + }); + + it("fires onSelect with the clicked layer id", () => { + const props = baseProps(); + render(); + fireEvent.click(screen.getByLabelText(/Select layer/i)); + expect(props.onSelect).toHaveBeenCalledWith("a"); + }); + + it("marks the selected layer via aria-pressed", () => { + render(); + expect(screen.getByLabelText(/Select layer/i).getAttribute("aria-pressed")).toBe("true"); + }); + + it("disables move-up on the top layer and move-down on the bottom layer", () => { + const els = [text("bottom", 0, { text: "Bottom" }), text("top", 5, { text: "Top" })]; + render(); + // Top row (rendered first) can't move up; bottom row can't move down. + expect((screen.getByLabelText("Move Top up") as HTMLButtonElement).disabled).toBe(true); + expect((screen.getByLabelText("Move Bottom down") as HTMLButtonElement).disabled).toBe(true); + expect((screen.getByLabelText("Move Top down") as HTMLButtonElement).disabled).toBe(false); + }); + + it("fires onReorder with the direction", () => { + const props = baseProps(); + const els = [text("bottom", 0, { text: "Bottom" }), text("top", 5, { text: "Top" })]; + render(); + fireEvent.click(screen.getByLabelText("Move Top down")); + expect(props.onReorder).toHaveBeenCalledWith("top", "down"); + }); + + it("toggles visibility with the correct label per state", () => { + const props = baseProps(); + const els = [text("a", 0), text("b", 1, { visible: false, text: "Hidden" })]; + render(); + fireEvent.click(screen.getByLabelText("Hide Hello world")); + expect(props.onToggleVisibility).toHaveBeenCalledWith("a"); + expect(screen.getByLabelText("Show Hidden")).toBeDefined(); + }); + + it("fires onDelete for the layer", () => { + const props = baseProps(); + render(); + fireEvent.click(screen.getByLabelText("Delete Hello world")); + expect(props.onDelete).toHaveBeenCalledWith("a"); + }); +}); diff --git a/desktop/src/apps/designstudio/LibraryView.test.tsx b/desktop/src/apps/designstudio/LibraryView.test.tsx new file mode 100644 index 000000000..d8447d27a --- /dev/null +++ b/desktop/src/apps/designstudio/LibraryView.test.tsx @@ -0,0 +1,107 @@ +import { describe, it, expect, vi, afterEach } from "vitest"; +import { render, screen, fireEvent, waitFor } from "@testing-library/react"; +import { LibraryView } from "./LibraryView"; + +const DESIGN = { id: "d1", name: "My Poster", updated_at: Math.floor(Date.now() / 1000) }; + +function ok(body: unknown) { + return { ok: true, status: 200, json: () => Promise.resolve(body) } as Response; +} + +describe("LibraryView", () => { + afterEach(() => { + vi.unstubAllGlobals(); + vi.restoreAllMocks(); + }); + + it("shows an empty state when there are no saved designs", async () => { + vi.stubGlobal("fetch", vi.fn(() => Promise.resolve(ok([]))) as unknown as typeof fetch); + render(); + await waitFor(() => expect(screen.getByText(/No saved designs yet/i)).toBeDefined()); + }); + + it("renders a saved design and opens it on click", async () => { + vi.stubGlobal("fetch", vi.fn(() => Promise.resolve(ok([DESIGN]))) as unknown as typeof fetch); + const onOpen = vi.fn(); + render(); + await waitFor(() => expect(screen.getByText("My Poster")).toBeDefined()); + fireEvent.click(screen.getByLabelText("Open My Poster")); + expect(onOpen).toHaveBeenCalledWith("d1"); + }); + + it("surfaces a load error", async () => { + vi.stubGlobal( + "fetch", + vi.fn(() => Promise.resolve({ ok: false, status: 500, json: () => Promise.resolve({}) } as Response)) as unknown as typeof fetch, + ); + render(); + await waitFor(() => expect(screen.getByRole("alert").textContent).toMatch(/could not load designs/i)); + }); + + it("delete is backend-confirmed: refetches the list and reports the rename callback", async () => { + let deleted = false; + const fetchMock = vi.fn((input: RequestInfo | URL, init?: RequestInit) => { + const url = String(input); + const method = (init?.method ?? "GET").toUpperCase(); + if (url === "/api/designs" && method === "GET") { + return Promise.resolve(ok(deleted ? [] : [DESIGN])); + } + if (url === "/api/designs/d1" && method === "DELETE") { + deleted = true; + return Promise.resolve(ok({})); + } + return Promise.resolve(ok({})); + }); + vi.stubGlobal("fetch", fetchMock as unknown as typeof fetch); + vi.spyOn(window, "confirm").mockReturnValue(true); + + render(); + await waitFor(() => expect(screen.getByText("My Poster")).toBeDefined()); + + fireEvent.click(screen.getByLabelText("Delete My Poster")); + await waitFor(() => expect(screen.queryByText("My Poster")).toBeNull()); + await waitFor(() => expect(screen.getByText(/No saved designs yet/i)).toBeDefined()); + }); + + it("does not delete when the user cancels the confirm", async () => { + const fetchMock = vi.fn(() => Promise.resolve(ok([DESIGN]))); + vi.stubGlobal("fetch", fetchMock as unknown as typeof fetch); + vi.spyOn(window, "confirm").mockReturnValue(false); + + render(); + await waitFor(() => expect(screen.getByText("My Poster")).toBeDefined()); + + fireEvent.click(screen.getByLabelText("Delete My Poster")); + // No DELETE was issued -- only the initial list GET happened. + expect(fetchMock.mock.calls.every((c) => (c[1]?.method ?? "GET") === "GET")).toBe(true); + }); + + it("renames a design and notifies via onRenamed", async () => { + const fetchMock = vi.fn((input: RequestInfo | URL, init?: RequestInit) => { + const method = (init?.method ?? "GET").toUpperCase(); + if (method === "PUT") return Promise.resolve(ok({ id: "d1", name: "Renamed", content: "{}" })); + return Promise.resolve(ok([DESIGN])); + }); + vi.stubGlobal("fetch", fetchMock as unknown as typeof fetch); + vi.spyOn(window, "prompt").mockReturnValue("Renamed"); + const onRenamed = vi.fn(); + + render(); + await waitFor(() => expect(screen.getByText("My Poster")).toBeDefined()); + + fireEvent.click(screen.getByLabelText("Rename My Poster")); + await waitFor(() => expect(onRenamed).toHaveBeenCalledWith("d1", "Renamed")); + }); + + it("skips the rename when the prompt is cancelled or unchanged", async () => { + const fetchMock = vi.fn(() => Promise.resolve(ok([DESIGN]))); + vi.stubGlobal("fetch", fetchMock as unknown as typeof fetch); + vi.spyOn(window, "prompt").mockReturnValue(null); + + render(); + await waitFor(() => expect(screen.getByText("My Poster")).toBeDefined()); + + fireEvent.click(screen.getByLabelText("Rename My Poster")); + expect(fetchMock.mock.calls.every((c) => (c[1]?.method ?? "GET") === "GET")).toBe(true); + }); +}); diff --git a/desktop/src/apps/designstudio/MagicView.test.tsx b/desktop/src/apps/designstudio/MagicView.test.tsx new file mode 100644 index 000000000..7e44f0e94 --- /dev/null +++ b/desktop/src/apps/designstudio/MagicView.test.tsx @@ -0,0 +1,93 @@ +import { describe, it, expect, vi } from "vitest"; +import { render, screen, fireEvent } from "@testing-library/react"; +import { MagicView } from "./MagicView"; +import type { GeneratedImage } from "./types"; + +function props(over: Partial[0]> = {}) { + return { + prompt: "", + onPromptChange: vi.fn(), + style: null as string | null, + onStyleChange: vi.fn(), + results: [] as GeneratedImage[], + generating: false, + canGenerate: true, + error: null as string | null, + errorNeedsModel: false, + needsModel: false, + onGenerate: vi.fn(), + onPickModel: vi.fn(), + onUseResult: vi.fn(), + ...over, + }; +} + +describe("MagicView", () => { + it("edits the prompt through onPromptChange", () => { + const p = props(); + render(); + fireEvent.change(screen.getByLabelText("Design prompt"), { target: { value: "a poster" } }); + expect(p.onPromptChange).toHaveBeenCalledWith("a poster"); + }); + + it("disables Generate when canGenerate is false", () => { + render(); + expect((screen.getByRole("button", { name: /Generate/ }) as HTMLButtonElement).disabled).toBe(true); + }); + + it("fires onGenerate when enabled and clicked", () => { + const p = props({ canGenerate: true }); + render(); + fireEvent.click(screen.getByRole("button", { name: /Generate/ })); + expect(p.onGenerate).toHaveBeenCalledTimes(1); + }); + + it("shows the busy label while generating", () => { + render(); + expect(screen.getByText(/Generating\.\.\./)).toBeDefined(); + }); + + it("toggles a style chip on and off", () => { + const p = props({ style: null }); + const { rerender } = render(); + fireEvent.click(screen.getByRole("button", { name: "Bold" })); + expect(p.onStyleChange).toHaveBeenCalledWith("Bold"); + + // When a chip is already active, clicking clears it. + p.onStyleChange.mockClear(); + rerender(); + fireEvent.click(screen.getByRole("button", { name: "Bold" })); + expect(p.onStyleChange).toHaveBeenCalledWith(null); + }); + + it("shows the install-a-model prompt and wires Browse models", () => { + const p = props({ needsModel: true }); + render(); + expect(screen.getByText(/Install an image generation model/i)).toBeDefined(); + fireEvent.click(screen.getByRole("button", { name: "Browse models" })); + expect(p.onPickModel).toHaveBeenCalledTimes(1); + }); + + it("renders an error and, when it needs a model, an install action", () => { + const p = props({ error: "boom", errorNeedsModel: true }); + render(); + expect(screen.getByText("boom")).toBeDefined(); + fireEvent.click(screen.getByRole("button", { name: "Install a model" })); + expect(p.onPickModel).toHaveBeenCalledTimes(1); + }); + + it("does not show an install action for a plain error", () => { + render(); + expect(screen.getByText("just a message")).toBeDefined(); + expect(screen.queryByRole("button", { name: "Install a model" })).toBeNull(); + }); + + it("renders results and fires onUseResult with the picked image", () => { + const img: GeneratedImage = { id: "g1", url: "/x.png", prompt: "a cat poster" }; + const p = props({ results: [img] }); + render(); + const tile = screen.getByAltText("a cat poster"); + fireEvent.click(tile); + expect(p.onUseResult).toHaveBeenCalledWith(img); + }); +}); diff --git a/desktop/src/apps/designstudio/PropertiesPanel.test.tsx b/desktop/src/apps/designstudio/PropertiesPanel.test.tsx new file mode 100644 index 000000000..38d3b8305 --- /dev/null +++ b/desktop/src/apps/designstudio/PropertiesPanel.test.tsx @@ -0,0 +1,138 @@ +import { describe, it, expect, vi } from "vitest"; +import { render, screen, fireEvent } from "@testing-library/react"; +import { PropertiesPanel } from "./PropertiesPanel"; +import type { CanvasElement } from "./types"; + +function textEl(over: Partial = {}): CanvasElement { + return { + id: "t", + type: "text", + x: 12.4, + y: 40.6, + width: 100, + height: 40, + rotation: 0, + zIndex: 0, + visible: true, + text: "Hi", + fontSize: 32, + fontFamily: "Inter", + fill: "#ffffff", + align: "left", + ...over, + } as CanvasElement; +} + +function rectEl(over: Partial = {}): CanvasElement { + return { + id: "r", + type: "rect", + x: 0, + y: 0, + width: 50, + height: 60, + rotation: 0, + zIndex: 0, + visible: true, + fill: "#112233", + stroke: "#445566", + strokeWidth: 2, + ...over, + } as CanvasElement; +} + +function lineEl(): CanvasElement { + return { + id: "l", + type: "line", + x: 0, + y: 0, + width: 80, + height: 2, + rotation: 0, + zIndex: 0, + visible: true, + stroke: "#abcdef", + strokeWidth: 3, + } as CanvasElement; +} + +function props(over: Partial[0]> = {}) { + return { + onUpdate: vi.fn(), + onDuplicate: vi.fn(), + onDelete: vi.fn(), + ...over, + }; +} + +describe("PropertiesPanel", () => { + it("prompts to select an element when nothing is selected", () => { + render(); + expect(screen.getByText(/Select an element/i)).toBeDefined(); + }); + + it("shows text controls for a text element and emits font-size updates", () => { + const p = props(); + render(); + const size = screen.getByLabelText("Font size") as HTMLInputElement; + expect(size.value).toBe("32"); + fireEvent.change(size, { target: { value: "48" } }); + expect(p.onUpdate).toHaveBeenCalledWith({ fontSize: 48 }); + }); + + it("emits text color and alignment updates", () => { + const p = props(); + render(); + fireEvent.change(screen.getByLabelText("Text color"), { target: { value: "#ff0000" } }); + expect(p.onUpdate).toHaveBeenCalledWith({ fill: "#ff0000" }); + fireEvent.click(screen.getByLabelText("Align center")); + expect(p.onUpdate).toHaveBeenCalledWith({ align: "center" }); + }); + + it("reflects the active alignment via aria-pressed", () => { + render(); + expect(screen.getByLabelText("Align right").getAttribute("aria-pressed")).toBe("true"); + expect(screen.getByLabelText("Align left").getAttribute("aria-pressed")).toBe("false"); + }); + + it("shows fill/stroke controls for a rect and emits updates", () => { + const p = props(); + render(); + fireEvent.change(screen.getByLabelText("Fill color"), { target: { value: "#000000" } }); + expect(p.onUpdate).toHaveBeenCalledWith({ fill: "#000000" }); + fireEvent.change(screen.getByLabelText("Stroke width"), { target: { value: "5" } }); + expect(p.onUpdate).toHaveBeenCalledWith({ strokeWidth: 5 }); + // Text-only controls must not appear for a shape. + expect(screen.queryByLabelText("Font size")).toBeNull(); + }); + + it("shows only a stroke control for a line (no fill)", () => { + render(); + expect(screen.getByLabelText("Stroke color")).toBeDefined(); + expect(screen.queryByLabelText("Fill color")).toBeNull(); + }); + + it("rounds and displays position and size", () => { + render(); + expect(screen.getByText("X 12")).toBeDefined(); + expect(screen.getByText("Y 41")).toBeDefined(); + expect(screen.getByText("W 100")).toBeDefined(); + expect(screen.getByText("H 40")).toBeDefined(); + }); + + it("wires the duplicate and delete buttons", () => { + const p = props(); + render(); + fireEvent.click(screen.getByLabelText("Duplicate element")); + expect(p.onDuplicate).toHaveBeenCalledTimes(1); + fireEvent.click(screen.getByLabelText("Delete element")); + expect(p.onDelete).toHaveBeenCalledTimes(1); + }); + + it("renders the coming-soon Magic chips as disabled", () => { + render(); + const chip = screen.getByRole("button", { name: "Make it bolder" }) as HTMLButtonElement; + expect(chip.disabled).toBe(true); + }); +}); diff --git a/desktop/src/apps/designstudio/TemplatesView.test.tsx b/desktop/src/apps/designstudio/TemplatesView.test.tsx new file mode 100644 index 000000000..e37d70c10 --- /dev/null +++ b/desktop/src/apps/designstudio/TemplatesView.test.tsx @@ -0,0 +1,34 @@ +import { describe, it, expect, vi } from "vitest"; +import { render, screen, fireEvent } from "@testing-library/react"; +import { TemplatesView } from "./TemplatesView"; + +describe("TemplatesView", () => { + it("renders the template grid and filter pills", () => { + render(); + expect(screen.getByText("Instagram Post")).toBeDefined(); + expect(screen.getByText("Presentation")).toBeDefined(); + expect(screen.getByRole("button", { name: "All" })).toBeDefined(); + expect(screen.getByRole("button", { name: "Logos" })).toBeDefined(); + }); + + it("selecting a template reports its name and dimensions", () => { + const onSelect = vi.fn(); + render(); + fireEvent.click(screen.getByText("Instagram Post")); + expect(onSelect).toHaveBeenCalledWith({ name: "Instagram Post", width: 1080, height: 1080 }); + }); + + it("reports the correct dimensions for a non-square template", () => { + const onSelect = vi.fn(); + render(); + fireEvent.click(screen.getByText("Presentation")); + expect(onSelect).toHaveBeenCalledWith({ name: "Presentation", width: 1920, height: 1080 }); + }); + + it("does not fire selection when only switching filter pills", () => { + const onSelect = vi.fn(); + render(); + fireEvent.click(screen.getByRole("button", { name: "Posters" })); + expect(onSelect).not.toHaveBeenCalled(); + }); +}); diff --git a/desktop/src/apps/designstudio/designs-api.test.ts b/desktop/src/apps/designstudio/designs-api.test.ts new file mode 100644 index 000000000..8f36ac47e --- /dev/null +++ b/desktop/src/apps/designstudio/designs-api.test.ts @@ -0,0 +1,125 @@ +import { describe, it, expect, vi, afterEach } from "vitest"; +import { + createDesign, + deleteDesign, + getDesign, + listDesigns, + renameDesign, + updateDesign, +} from "./designs-api"; + +function jsonResponse(body: unknown, ok = true, status = ok ? 200 : 400): Response { + return { + ok, + status, + json: () => Promise.resolve(body), + } as Response; +} + +describe("designs-api", () => { + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it("listDesigns GETs /api/designs and returns the array", async () => { + const fetchMock = vi.fn(() => + Promise.resolve(jsonResponse([{ id: "d1", name: "A", updated_at: 2 }])), + ); + vi.stubGlobal("fetch", fetchMock); + + const designs = await listDesigns(); + expect(fetchMock).toHaveBeenCalledWith("/api/designs", { credentials: "include" }); + expect(designs).toEqual([{ id: "d1", name: "A", updated_at: 2 }]); + }); + + it("listDesigns throws a friendly error on a failed request", async () => { + vi.stubGlobal("fetch", vi.fn(() => Promise.resolve(jsonResponse({}, false, 500)))); + await expect(listDesigns()).rejects.toThrow(/could not load designs/i); + }); + + it("getDesign fetches a single record by (encoded) id", async () => { + const doc = { id: "d 1", name: "A", content: "{}" }; + const fetchMock = vi.fn(() => Promise.resolve(jsonResponse(doc))); + vi.stubGlobal("fetch", fetchMock); + + const got = await getDesign("d 1"); + expect(fetchMock).toHaveBeenCalledWith("/api/designs/d%201", { credentials: "include" }); + expect(got).toEqual(doc); + }); + + it("getDesign throws when the record can't be opened", async () => { + vi.stubGlobal("fetch", vi.fn(() => Promise.resolve(jsonResponse({}, false, 404)))); + await expect(getDesign("nope")).rejects.toThrow(/could not open design/i); + }); + + it("createDesign POSTs name + content and returns the saved doc", async () => { + const fetchMock = vi.fn((input: RequestInfo | URL, init?: RequestInit) => { + expect(input).toBe("/api/designs"); + expect(init?.method).toBe("POST"); + const body = JSON.parse(String(init?.body)); + expect(body).toEqual({ name: "Poster", content: "{}" }); + return Promise.resolve(jsonResponse({ id: "d1", name: "Poster", content: "{}" })); + }); + vi.stubGlobal("fetch", fetchMock); + + const saved = await createDesign("Poster", "{}"); + expect(saved.id).toBe("d1"); + }); + + it("createDesign surfaces the server's error message", async () => { + vi.stubGlobal( + "fetch", + vi.fn(() => Promise.resolve(jsonResponse({ error: "name is required" }, false, 400))), + ); + await expect(createDesign("", "{}")).rejects.toThrow("name is required"); + }); + + it("createDesign falls back to a generic message when the body has no error", async () => { + vi.stubGlobal("fetch", vi.fn(() => Promise.resolve(jsonResponse({}, false, 413)))); + await expect(createDesign("x", "{}")).rejects.toThrow(/save failed/i); + }); + + it("updateDesign PUTs only the provided fields", async () => { + const fetchMock = vi.fn((input: RequestInfo | URL, init?: RequestInit) => { + expect(input).toBe("/api/designs/d1"); + expect(init?.method).toBe("PUT"); + const body = JSON.parse(String(init?.body)); + // name was undefined, so it must be omitted from the payload. + expect(body).toEqual({ content: "{}" }); + return Promise.resolve(jsonResponse({ id: "d1", name: "A", content: "{}" })); + }); + vi.stubGlobal("fetch", fetchMock); + + await updateDesign("d1", undefined, "{}"); + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + + it("renameDesign PUTs just the name", async () => { + const fetchMock = vi.fn((input: RequestInfo | URL, init?: RequestInit) => { + const body = JSON.parse(String(init?.body)); + expect(body).toEqual({ name: "New" }); + return Promise.resolve(jsonResponse({ id: "d1", name: "New", content: "{}" })); + }); + vi.stubGlobal("fetch", fetchMock); + + const renamed = await renameDesign("d1", "New"); + expect(renamed.name).toBe("New"); + }); + + it("deleteDesign DELETEs the record", async () => { + const fetchMock = vi.fn((input: RequestInfo | URL, init?: RequestInit) => { + expect(input).toBe("/api/designs/d1"); + expect(init?.method).toBe("DELETE"); + return Promise.resolve(jsonResponse({})); + }); + vi.stubGlobal("fetch", fetchMock); + + await deleteDesign("d1"); + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + + it("deleteDesign throws on failure", async () => { + vi.stubGlobal("fetch", vi.fn(() => Promise.resolve(jsonResponse({}, false, 500)))); + await expect(deleteDesign("d1")).rejects.toThrow(/delete failed/i); + }); +}); diff --git a/desktop/src/apps/designstudio/elementFactory.test.ts b/desktop/src/apps/designstudio/elementFactory.test.ts new file mode 100644 index 000000000..d6882682c --- /dev/null +++ b/desktop/src/apps/designstudio/elementFactory.test.ts @@ -0,0 +1,141 @@ +import { describe, it, expect } from "vitest"; +import { + createImageElement, + createLineElement, + createShapeElement, + createTextElement, + duplicateElement, + nextZIndex, +} from "./elementFactory"; +import type { CanvasElement } from "./types"; + +const AW = 1080; +const AH = 1350; + +describe("nextZIndex", () => { + it("returns 0 for an empty canvas", () => { + expect(nextZIndex([])).toBe(0); + }); + + it("returns one above the current max zIndex", () => { + const els = [ + { zIndex: 0 } as CanvasElement, + { zIndex: 4 } as CanvasElement, + { zIndex: 2 } as CanvasElement, + ]; + expect(nextZIndex(els)).toBe(5); + }); +}); + +describe("createTextElement", () => { + it("creates a centered text element with sane defaults", () => { + const el = createTextElement([], AW, AH); + expect(el.type).toBe("text"); + expect(el.visible).toBe(true); + expect(el.rotation).toBe(0); + expect(el.zIndex).toBe(0); + if (el.type === "text") { + expect(el.fontSize).toBeGreaterThan(0); + expect(el.text.length).toBeGreaterThan(0); + } + // Centered horizontally: left margin + width + right margin == artboard. + expect(el.x).toBe(Math.round((AW - el.width) / 2)); + expect(el.y).toBe(Math.round((AH - el.height) / 2)); + }); + + it("caps text width at 320 for a wide artboard", () => { + const el = createTextElement([], 4000, 4000); + expect(el.width).toBe(320); + }); + + it("stacks zIndex above existing elements", () => { + const first = createTextElement([], AW, AH); + const second = createTextElement([first], AW, AH); + expect(second.zIndex).toBe(1); + }); +}); + +describe("createShapeElement", () => { + it("creates a square rect centered on the artboard", () => { + const el = createShapeElement([], "rect", AW, AH); + expect(el.type).toBe("rect"); + expect(el.width).toBe(el.height); + expect(el.x).toBe(Math.round((AW - el.width) / 2)); + }); + + it("honors the requested kind (ellipse)", () => { + const el = createShapeElement([], "ellipse", AW, AH); + expect(el.type).toBe("ellipse"); + }); + + it("caps size at 220 for a wide artboard", () => { + const el = createShapeElement([], "rect", 4000, 4000); + expect(el.width).toBe(220); + }); +}); + +describe("createLineElement", () => { + it("creates a thin horizontal line", () => { + const el = createLineElement([], AW, AH); + expect(el.type).toBe("line"); + expect(el.height).toBe(2); + expect(el.width).toBeGreaterThan(el.height); + }); +}); + +describe("createImageElement", () => { + it("creates a square image carrying its src and prompt", () => { + const el = createImageElement([], "data:image/png;base64,AA", AW, AH, "a cat"); + expect(el.type).toBe("image"); + expect(el.src).toBe("data:image/png;base64,AA"); + expect(el.prompt).toBe("a cat"); + expect(el.width).toBe(el.height); + }); + + it("leaves the prompt undefined when not supplied", () => { + const el = createImageElement([], "data:x", AW, AH); + expect(el.prompt).toBeUndefined(); + }); +}); + +describe("id uniqueness", () => { + it("every factory produces a distinct id even in the same tick", () => { + const els = [ + createTextElement([], AW, AH), + createShapeElement([], "rect", AW, AH), + createLineElement([], AW, AH), + createImageElement([], "data:x", AW, AH), + ]; + const ids = els.map((e) => e.id); + expect(new Set(ids).size).toBe(ids.length); + }); + + it("ids are prefixed by their type family", () => { + expect(createTextElement([], AW, AH).id).toMatch(/^text-/); + expect(createShapeElement([], "ellipse", AW, AH).id).toMatch(/^ellipse-/); + expect(createLineElement([], AW, AH).id).toMatch(/^line-/); + expect(createImageElement([], "data:x", AW, AH).id).toMatch(/^image-/); + }); +}); + +describe("duplicateElement", () => { + it("offsets the copy by 16px, gives it a fresh id and the top zIndex", () => { + const original = createShapeElement([], "rect", AW, AH); + const existing = [original]; + const copy = duplicateElement(original, existing); + expect(copy.id).not.toBe(original.id); + expect(copy.x).toBe(original.x + 16); + expect(copy.y).toBe(original.y + 16); + expect(copy.zIndex).toBe(nextZIndex(existing)); + // Everything else (fill, size, type) is preserved. + expect(copy.type).toBe(original.type); + expect(copy.width).toBe(original.width); + }); + + it("does not mutate the source element", () => { + const original = createShapeElement([], "rect", AW, AH); + const snapshot = { ...original }; + duplicateElement(original, [original]); + expect(original).toEqual(snapshot); + }); +}); diff --git a/desktop/src/apps/designstudio/types.test.ts b/desktop/src/apps/designstudio/types.test.ts new file mode 100644 index 000000000..c1048cce6 --- /dev/null +++ b/desktop/src/apps/designstudio/types.test.ts @@ -0,0 +1,181 @@ +import { describe, it, expect } from "vitest"; +import { + DEFAULT_ARTBOARD, + MAGIC_STYLE_CHIPS, + ZOOM_STEPS, + hasFill, + hasStroke, + isValidDesignContent, + type CanvasElement, + type DesignContent, +} from "./types"; + +function textEl(over: Partial = {}): CanvasElement { + return { + id: "t1", + type: "text", + x: 0, + y: 0, + width: 100, + height: 40, + rotation: 0, + zIndex: 0, + visible: true, + text: "Hi", + fontSize: 32, + fontFamily: "Inter", + fill: "#fff", + align: "left", + ...over, + } as CanvasElement; +} + +function rectEl(): CanvasElement { + return { + id: "r1", + type: "rect", + x: 0, + y: 0, + width: 50, + height: 50, + rotation: 0, + zIndex: 1, + visible: true, + fill: "#000", + stroke: "#fff", + strokeWidth: 1, + } as CanvasElement; +} + +function lineEl(): CanvasElement { + return { + id: "l1", + type: "line", + x: 0, + y: 0, + width: 80, + height: 2, + rotation: 0, + zIndex: 2, + visible: true, + stroke: "#fff", + strokeWidth: 3, + } as CanvasElement; +} + +function imageEl(): CanvasElement { + return { + id: "i1", + type: "image", + x: 0, + y: 0, + width: 100, + height: 100, + rotation: 0, + zIndex: 3, + visible: true, + src: "data:image/png;base64,AAAA", + } as CanvasElement; +} + +describe("hasFill", () => { + it("is true for text, rect and ellipse", () => { + expect(hasFill(textEl())).toBe(true); + expect(hasFill(rectEl())).toBe(true); + expect(hasFill({ ...rectEl(), type: "ellipse" } as CanvasElement)).toBe(true); + }); + + it("is false for line and image", () => { + expect(hasFill(lineEl())).toBe(false); + expect(hasFill(imageEl())).toBe(false); + }); +}); + +describe("hasStroke", () => { + it("is true for rect, ellipse and line", () => { + expect(hasStroke(rectEl())).toBe(true); + expect(hasStroke({ ...rectEl(), type: "ellipse" } as CanvasElement)).toBe(true); + expect(hasStroke(lineEl())).toBe(true); + }); + + it("is false for text and image", () => { + expect(hasStroke(textEl())).toBe(false); + expect(hasStroke(imageEl())).toBe(false); + }); +}); + +describe("isValidDesignContent", () => { + const valid: DesignContent = { + artboard: { name: "Poster", width: 1080, height: 1350 }, + elements: [textEl(), rectEl()], + }; + + it("accepts a well-formed design with elements", () => { + expect(isValidDesignContent(valid)).toBe(true); + }); + + it("accepts an empty elements array", () => { + expect(isValidDesignContent({ ...valid, elements: [] })).toBe(true); + }); + + it.each([ + ["null", null], + ["a string", "nope"], + ["a number", 5], + ["undefined", undefined], + ])("rejects %s", (_label, value) => { + expect(isValidDesignContent(value)).toBe(false); + }); + + it("rejects a missing artboard", () => { + expect(isValidDesignContent({ elements: [] })).toBe(false); + }); + + it("rejects an artboard with non-numeric dimensions", () => { + expect( + isValidDesignContent({ artboard: { name: "x", width: "1080", height: 1350 }, elements: [] }), + ).toBe(false); + }); + + it("rejects an artboard without a name", () => { + expect( + isValidDesignContent({ artboard: { width: 1080, height: 1350 }, elements: [] }), + ).toBe(false); + }); + + it("rejects when elements is not an array", () => { + expect(isValidDesignContent({ artboard: valid.artboard, elements: {} })).toBe(false); + }); + + it("rejects an element missing an id or type", () => { + expect( + isValidDesignContent({ artboard: valid.artboard, elements: [{ type: "text" }] }), + ).toBe(false); + expect( + isValidDesignContent({ artboard: valid.artboard, elements: [{ id: "x" }] }), + ).toBe(false); + }); + + it("rejects a non-object element (e.g. null in the array)", () => { + expect(isValidDesignContent({ artboard: valid.artboard, elements: [null] })).toBe(false); + }); +}); + +describe("constants", () => { + it("DEFAULT_ARTBOARD is a valid design's artboard", () => { + expect(isValidDesignContent({ artboard: DEFAULT_ARTBOARD, elements: [] })).toBe(true); + expect(DEFAULT_ARTBOARD.width).toBeGreaterThan(0); + expect(DEFAULT_ARTBOARD.height).toBeGreaterThan(0); + }); + + it("ZOOM_STEPS are ascending and include 100%", () => { + const arr = [...ZOOM_STEPS]; + expect(arr).toContain(1); + expect([...arr].sort((a, b) => a - b)).toEqual(arr); + }); + + it("MAGIC_STYLE_CHIPS is a non-empty unique list", () => { + expect(MAGIC_STYLE_CHIPS.length).toBeGreaterThan(0); + expect(new Set(MAGIC_STYLE_CHIPS).size).toBe(MAGIC_STYLE_CHIPS.length); + }); +}); diff --git a/desktop/src/apps/designstudio/useElementHistory.test.ts b/desktop/src/apps/designstudio/useElementHistory.test.ts new file mode 100644 index 000000000..8b4a79d09 --- /dev/null +++ b/desktop/src/apps/designstudio/useElementHistory.test.ts @@ -0,0 +1,139 @@ +import { describe, it, expect, vi } from "vitest"; +import { act, renderHook } from "@testing-library/react"; +import { useElementHistory } from "./useElementHistory"; +import type { CanvasElement } from "./types"; + +function el(id: string, zIndex = 0): CanvasElement { + return { + id, + type: "rect", + x: 0, + y: 0, + width: 10, + height: 10, + rotation: 0, + zIndex, + visible: true, + fill: "#000", + stroke: "#fff", + strokeWidth: 0, + } as CanvasElement; +} + +/** + * The hook records the *previous* elements array on commit, so undo/redo only + * make sense against a `current` value that tracks what was last applied. This + * harness re-renders the hook with the latest applied array, mirroring how + * DesignView feeds `elements` back in from parent state. + */ +function setup(initial: CanvasElement[] = []) { + let current = initial; + const onChange = vi.fn((next: CanvasElement[]) => { + current = next; + }); + const hook = renderHook(({ els }) => useElementHistory(els, onChange), { + initialProps: { els: current }, + }); + const sync = () => hook.rerender({ els: current }); + return { hook, onChange, get current() { return current; }, sync }; +} + +describe("useElementHistory", () => { + it("starts with nothing to undo or redo", () => { + const { hook } = setup(); + expect(hook.result.current.canUndo).toBe(false); + expect(hook.result.current.canRedo).toBe(false); + }); + + it("commit applies the new state and enables undo", () => { + const { hook, onChange, sync } = setup([]); + const next = [el("a")]; + act(() => hook.result.current.commit(next)); + expect(onChange).toHaveBeenLastCalledWith(next); + expect(hook.result.current.canUndo).toBe(true); + expect(hook.result.current.canRedo).toBe(false); + sync(); + }); + + it("undo restores the prior snapshot and enables redo", () => { + const start = [el("a")]; + const { hook, onChange, sync } = setup(start); + + act(() => hook.result.current.commit([el("a"), el("b", 1)])); + sync(); + + act(() => hook.result.current.undo()); + // Undo replays the array as it was *before* the commit. + expect(onChange).toHaveBeenLastCalledWith(start); + expect(hook.result.current.canRedo).toBe(true); + expect(hook.result.current.canUndo).toBe(false); + sync(); + }); + + it("redo re-applies an undone change", () => { + const start = [el("a")]; + const committed = [el("a"), el("b", 1)]; + const { hook, onChange, sync } = setup(start); + + act(() => hook.result.current.commit(committed)); + sync(); + act(() => hook.result.current.undo()); + sync(); + act(() => hook.result.current.redo()); + + expect(onChange).toHaveBeenLastCalledWith(committed); + expect(hook.result.current.canRedo).toBe(false); + expect(hook.result.current.canUndo).toBe(true); + sync(); + }); + + it("a fresh commit clears the redo stack", () => { + const { hook, sync } = setup([el("a")]); + act(() => hook.result.current.commit([el("b")])); + sync(); + act(() => hook.result.current.undo()); + sync(); + expect(hook.result.current.canRedo).toBe(true); + + act(() => hook.result.current.commit([el("c")])); + sync(); + expect(hook.result.current.canRedo).toBe(false); + }); + + it("undo/redo are no-ops when their stacks are empty", () => { + const { hook, onChange } = setup([el("a")]); + act(() => hook.result.current.undo()); + act(() => hook.result.current.redo()); + expect(onChange).not.toHaveBeenCalled(); + expect(hook.result.current.canUndo).toBe(false); + expect(hook.result.current.canRedo).toBe(false); + }); + + it("caps the undo history and still restores the most recent snapshot", () => { + const { hook, onChange, sync } = setup([el("v0")]); + // Push well past the 100-entry cap. + for (let i = 1; i <= 130; i++) { + act(() => hook.result.current.commit([el(`v${i}`)])); + sync(); + } + expect(hook.result.current.canUndo).toBe(true); + + // Undo once returns the immediately-previous applied state, not a + // corrupted/evicted one. + act(() => hook.result.current.undo()); + expect(onChange).toHaveBeenLastCalledWith([el("v129")]); + + // Eviction branch: walk undo to exhaustion. Because the history is capped, + // the oldest snapshots (including the original v0) were evicted, so we can + // never restore v0 and the undo depth is bounded by the cap, not by the 130 + // commits pushed. + let undos = 1; // the undo above + while (hook.result.current.canUndo) { + act(() => hook.result.current.undo()); + undos++; + if (undos > 200) break; // safety: never loop forever if the cap regressed + } + expect(undos).toBeLessThanOrEqual(100); + expect(onChange).not.toHaveBeenLastCalledWith([el("v0")]); + }); +}); diff --git a/desktop/src/apps/designstudio/useHtmlImage.test.ts b/desktop/src/apps/designstudio/useHtmlImage.test.ts new file mode 100644 index 000000000..0371d9871 --- /dev/null +++ b/desktop/src/apps/designstudio/useHtmlImage.test.ts @@ -0,0 +1,74 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { act, renderHook } from "@testing-library/react"; +import { useHtmlImage } from "./useHtmlImage"; + +/** A controllable stand-in for the browser's HTMLImageElement so tests can + * drive the load/error lifecycle deterministically. */ +class FakeImage { + static last: FakeImage | undefined; + crossOrigin = ""; + onload: (() => void) | null = null; + onerror: (() => void) | null = null; + private _src = ""; + constructor() { + FakeImage.last = this; + } + set src(v: string) { + this._src = v; + } + get src() { + return this._src; + } +} + +describe("useHtmlImage", () => { + beforeEach(() => { + FakeImage.last = undefined; + vi.stubGlobal("Image", FakeImage as unknown as typeof Image); + }); + afterEach(() => { + vi.unstubAllGlobals(); + vi.restoreAllMocks(); + }); + + it("returns no image and no failure for an undefined src", () => { + const { result } = renderHook(() => useHtmlImage(undefined)); + expect(result.current.image).toBeUndefined(); + expect(result.current.failed).toBe(false); + expect(FakeImage.last).toBeUndefined(); + }); + + it("exposes the loaded image once onload fires", () => { + const { result } = renderHook(() => useHtmlImage("data:image/png;base64,AAAA")); + expect(result.current.image).toBeUndefined(); + act(() => FakeImage.last!.onload!()); + expect(result.current.image).toBe(FakeImage.last); + expect(result.current.failed).toBe(false); + }); + + it("sets failed and keeps no image when onerror fires", () => { + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + const { result } = renderHook(() => useHtmlImage("data:broken")); + act(() => FakeImage.last!.onerror!()); + expect(result.current.failed).toBe(true); + expect(result.current.image).toBeUndefined(); + expect(warn).toHaveBeenCalled(); + }); + + it("requests images anonymously (crossOrigin) so exported canvases stay untainted", () => { + renderHook(() => useHtmlImage("data:x")); + expect(FakeImage.last!.crossOrigin).toBe("anonymous"); + expect(FakeImage.last!.src).toBe("data:x"); + }); + + it("ignores a late onload after the src was cleared (unmount cancellation)", () => { + const { result, rerender } = renderHook(({ src }) => useHtmlImage(src), { + initialProps: { src: "data:first" as string | undefined }, + }); + const first = FakeImage.last!; + // Switch to no src: the effect cleanup marks the first load cancelled. + rerender({ src: undefined }); + act(() => first.onload!()); + expect(result.current.image).toBeUndefined(); + }); +}); From 55b6ef364a8ef45b2bce75794b9d0b28b74bc64f Mon Sep 17 00:00:00 2001 From: jaylfc Date: Fri, 10 Jul 2026 15:41:22 +0100 Subject: [PATCH 12/17] feat(taosgo): persist per-host mesh service credentials at cluster-join (Slice 1) (#1770) * docs(design): taOSgo mesh-join foundation + Game Studio asset-generation specs Two draft design specs for Jay review: - taOSgo mesh-join foundation: unblocks Web Studio publish (#167) and off-LAN remote access. Controller has no mesh-join today; specs the credential store + poll-intercept persistence, headless Headscale join (system tailscale vs tsnet decision), and the publish caller + static-site server, in 3 shippable slices. - Game Studio asset-generation (#55): tier-aware textures/audio/(stretch)3D reusing the existing image + MusicGen backends, spec -> build -> harden. * feat(taosgo): persist per-host mesh service credentials at cluster-join (Slice 1) The controller had no writer for the get_controller_token() seam: it read a placeholder env var, so the taOSnet passkey fetch (and the future Web Studio publish caller) had no credential once a host actually joined an account mesh. Add tinyagentos/taosnet/mesh_credentials.py: a host-local 0600 store for the per-host service tokens taos.my returns in the cluster-join ready payload (controller_token = taosnet:passkey, sites_token = sites:publish). The single-use headscale preauth key is intentionally NOT persisted. get_controller_ token()/get_sites_token() read it, with the TAOS_CONTROLLER_TOKEN/TAOS_SITES_ TOKEN env overrides still winning for dev/tests. Wire it via a poll-intercept: routes/account_proxy.py::cluster_join_poll now persists the ready-payload tokens server-side and strips them from the browser- facing body, so a bearer credential never reaches browser JavaScript. Fail-soft: non-200/non-JSON/unparseable bodies and save failures leave the response untouched. passkey_client.get_controller_token() delegates to the new store. This is Slice 1 of docs/design/taosgo-mesh-join-foundation.md: credentials only, no mesh-join yet (Slice 2). It already makes the headless taOSnet passkey fetch real once a host joins. Tests: store round-trip, 0600 perms, preauth-key not persisted, idempotent re-poll, env override, and the intercept persist+strip + fail-soft matrix (13). Existing account-proxy/taosnet suites stay green. * fold Kilo review on #1770: decouple token-strip from save, harden loads - account_proxy: the service tokens are now ALWAYS stripped from the browser body once seen in a 200 JSON response, whether or not the save succeeds. The previous fail-soft returned the unstripped body on a save error, leaking the bearer token (e.g. a payload with controller_token but no host_id raised in save). Parsing no longer depends on Content-Type, so a JSON body served without one cannot bypass stripping either. - mesh_credentials._load(): a corrupt/externally-edited file that parses to a non-object (list/string) is treated as absent, so the getters can never raise AttributeError and break the headless passkey fetch. - .gitignore: data/mesh_credentials.json (never commit a bearer-token file). - 3 regression tests: strip-even-when-save-fails, corrupt-non-dict-file, json-without-content-type. 16 pass. --- .gitignore | 1 + docs/design/game-studio-asset-generation.md | 89 ++++++++++ docs/design/taosgo-mesh-join-foundation.md | 170 ++++++++++++++++++++ tests/test_mesh_credentials.py | 159 ++++++++++++++++++ tinyagentos/routes/account_proxy.py | 53 +++++- tinyagentos/taosnet/mesh_credentials.py | 133 +++++++++++++++ tinyagentos/taosnet/passkey_client.py | 12 +- 7 files changed, 611 insertions(+), 6 deletions(-) create mode 100644 docs/design/game-studio-asset-generation.md create mode 100644 docs/design/taosgo-mesh-join-foundation.md create mode 100644 tests/test_mesh_credentials.py create mode 100644 tinyagentos/taosnet/mesh_credentials.py diff --git a/.gitignore b/.gitignore index ab07259a1..bb7dfe4d3 100644 --- a/.gitignore +++ b/.gitignore @@ -23,6 +23,7 @@ data/.setup_complete data/.auth_password data/.auth_sessions data/.auth_local_token +data/mesh_credentials.json data/agents.json data/images/ data/videos/ diff --git a/docs/design/game-studio-asset-generation.md b/docs/design/game-studio-asset-generation.md new file mode 100644 index 000000000..0fb688199 --- /dev/null +++ b/docs/design/game-studio-asset-generation.md @@ -0,0 +1,89 @@ +# Game Studio offline asset-generation stack (design spec) + +Status: DRAFT for Jay review (#55). Game Studio already works as an LLM-authored +three.js maker; this adds offline generation of the game's ART + AUDIO assets so a +generated game is not limited to code-drawn primitives. Author: taOS-dev. + +## Why this exists + +Game Studio v1 is functional: an agent authors a three.js game (HTML + JS file +set) from a prompt, the game previews sandboxed, packages as `.taosapp`, and shares +to the store. Today the visuals are whatever the model can draw with three.js +primitives + procedural code, and audio is absent or web-synth. #55 adds real +asset generation - textures/sprites, sound effects + music, and (stretch) 3D +meshes - all offline, tier-aware, reusing the backends taOS already ships. This +mirrors the Images Studio pattern: the app calls tier-aware backends, degrading +gracefully on low-end hardware. + +## Asset types, in ascending difficulty + +1. **Textures / sprites / skyboxes (2D images).** Highest value, lowest new cost: + reuse the EXISTING image-generation backend (SD on the RTX 3060 per the image + stack; RK image-gen on the NPU as the arms-length low tier). A game asks for + "a mossy stone texture, tileable" or "a pixel-art spaceship sprite sheet"; the + backend returns a PNG the game references. Tileable/seamless + transparent-bg + (sprite) are prompt/pipeline options, not new models. +2. **Audio - SFX + music.** SFX (jump, hit, coin) and short loops. Reuse MusicGen + for music loops (already in the stack, license-gated CC-BY-NC per the licensing + work) and a small text-to-SFX path (e.g. a lightweight audio model or a curated + procedural/sample fallback). Output: ogg/mp3 the game loads. +3. **3D meshes (stretch, gated).** The hard one: text/image-to-3D (TripoSR, + Shap-E, InstantMesh, or image-to-mesh). Heavy, GPU-bound, quality is rough, and + glTF export + three.js loading add complexity. Ship LAST, tier-gated to the + discrete-GPU tier only, behind a clear "experimental" flag. Many good three.js + games never need generated meshes (primitives + textures go far). + +## Design (tier-aware, backend-reusing) + +- **Backend contract.** One controller route surface, `routes/game_assets.py`, + with `POST /api/games/{id}/assets/texture`, `.../assets/audio`, and (later) + `.../assets/mesh`. Each resolves the tier-appropriate backend the same way the + Images Studio edit backends do (hardware tier -> concrete backend), writes the + asset into the game's file set (so it packages + previews with the game), and + returns the asset path. No new per-asset infra: textures go through the existing + image backend, audio through MusicGen + the SFX path. +- **Frontend.** Game Studio's Editor gains an "Assets" panel: generate a + texture/sprite/audio from a prompt, preview it, and insert a reference into the + current file (the authoring agent already manages the file set, so an inserted + asset is just another file + a code reference the agent or user wires up). + Tier-gated UI: unavailable backends show a clear "needs a GPU worker" state + rather than a broken button (matches the Images Studio + reduce-effects + precedent). +- **License + provenance.** Reuse the existing license-accept gate (MusicGen / + FLUX weights are CC-BY-NC; SD/Apache-clean is the default). A game that bundles + generated assets records their provenance in the package, consistent with the + app-security-analyzer + provenance work. +- **Offline-first.** Everything runs on the local/worker GPU; no cloud. On a + GPU-less host (Pi core alone) textures fall back to the NPU RK backend or a + curated built-in pack; 3D is simply unavailable. + +## Slice plan (spec -> build -> harden, per Jay) + +- **Slice 1 - textures/sprites** (build first; highest value/lowest cost). Route + + tier-aware image-backend call + Editor Assets panel (texture/sprite) + tests. + Live-verify a generated texture appears in a previewed game on the 3060 tier. +- **Slice 2 - audio (SFX + music).** MusicGen loop + SFX path + Assets-panel audio + tab + license gate + tests. +- **Slice 3 - 3D meshes (experimental, gated).** Text/image-to-3D on the + discrete-GPU tier only, glTF export, three.js loader wiring, behind an + experimental flag. Only if Slices 1-2 land clean. +- **Harden (last, per your ordering):** test coverage across the asset routes + + panel, and screenshot-verify a real generated game with generated assets on the + Pi. + +## Open questions for Jay + +1. **SFX model choice** for Slice 2: a dedicated text-to-SFX model vs a + curated-sample + light-synth fallback. MusicGen covers music loops already; + SFX is the gap. Recommend starting with the fallback + MusicGen and adding a + model only if quality demands it. +2. **3D scope (Slice 3):** ship it experimental on the 3060 tier, or defer 3D + entirely for now and stop at textures + audio? (Textures + audio deliver most + of the value; 3D is heavy and rough.) +3. Confirm the target build/verify hardware: the RTX 3060 (Fedora worker) for the + GPU tiers, RK NPU (Pi) for the low tier - consistent with the existing image + stack. + +## Non-goals (v1) + +Animation/rigging, video, voice/TTS dialogue, and any cloud generation. diff --git a/docs/design/taosgo-mesh-join-foundation.md b/docs/design/taosgo-mesh-join-foundation.md new file mode 100644 index 000000000..27ca9359f --- /dev/null +++ b/docs/design/taosgo-mesh-join-foundation.md @@ -0,0 +1,170 @@ +# taOSgo mesh-join foundation (design spec) + +Status: DRAFT for Jay review. Unblocks Web Studio publish (#167) and, more +broadly, off-LAN remote access (taOSgo). Author: taOS-dev. + +## Why this exists + +Web Studio v1 is a complete static-site builder (generate -> edit -> preview -> +install/export). Its one missing feature, publish to `