Skip to content

fix(worker): manifest path crashes worker on malformed input + availability depends on backend liveness (audit follow-up to #1762) #1765

Description

@jaylfc

Follow-up to #1762 from a code audit of the merged tree. Two real issues plus two smaller ones in the worker manifest path, all in tinyagentos/worker/worker_manifest.py + tinyagentos/worker/agent.py. Over to @hognek since this is your feature and you know it best.

1. HIGH: a malformed manifest crashes the worker process. load_manifest() calls json.loads unguarded (worker_manifest.py, final line of the loader), so invalid JSON raises JSONDecodeError; and the enrichment loop does an unchecked m["model_id"] (agent.py, enrichment loop) that raises KeyError on an entry missing the key. Both fire inside detect_backends(), which register() calls with no try/except, and __main__._run_headless only catches KeyboardInterrupt, so the whole worker dies (crash-loop under systemd). The docstring says the file may be written by any external platform, exactly the input class that will eventually be malformed. Failure scenario: an admin typos one comma in /etc/taos/worker-models.json and the worker crash-loops on start, or, if the file goes bad while running, heartbeat()'s blanket except Exception: return 0 swallows it with no log line and the worker silently drops offline.
Fix shape: wrap the manifest read + enrichment in try/except, log a warning with the parse error, and fall back to no-manifest behaviour. Also add a log line in the heartbeat swallow. Please update the two tests that currently bless the crash (test_invalid_json_raises, test_missing_model_id_handled).

2. MEDIUM: manifest models only surface when a matching backend is already live. The module promises the controller learns what a worker COULD load independently of what is loaded, but enrichment attaches manifest entries only to backends that passed the live probe, so if llama.cpp is stopped (the exact could-load-but-not-loaded case) its manifest entries vanish from the heartbeat. Fix shape: emit manifest entries for declared-but-not-probed backends too (e.g. a synthetic backend entry or a top-level available_models fallback), so availability does not depend on liveness.

3. Dead mappings: SOFTWARE_TO_BACKEND_TYPE maps kokoro/whisper to backend types that appear nowhere in the probe candidates list, so those manifest entries are unreachable, despite tests asserting the mappings. Either add the probe candidates or drop the mappings.

4. Nit: status = "loaded" is derived from the backend's models catalog, not its loaded_models residency list, so it reports "loaded" for models that are merely present on disk.

Acceptance: a corrupted manifest file leaves the worker registered and heartbeating (with a logged warning); a manifest entry for a stopped-but-installed backend still reaches the controller; tests cover both. Usual bar: wait for Kilo/CodeRabbit and resolve every finding. No rush.

Metadata

Metadata

Assignees

No one assigned

    Labels

    backend-drivenBackend-driven discoverybugSomething isn't workingfork-auditTracking issue for fork-vs-upstream drift auditskilo-auto-fixAuto-generated label by Kilokilo-triagedAuto-generated label by KilomodelsModel management and inferencetestingTesting and QA

    Projects

    Status
    Todo

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions