Skip to content

Commit e2f60e6

Browse files
committed
registry: validate AppManifest at the catalog boundary (tsk-y5tp24)
App manifests come from an externally-authored git repo (tinyagentos/catalog_sync.py) and were previously parsed with data.get(key, default) defaults and no type checks. A single malformed entry broke the whole store listing at install time -- the audit's noted cases were: - requires=data.get("requires", {}) accepting a YAML string, then raising AttributeError deep in install code with no indication which manifest was malformed. - context_window=data.get("context_window", 0) accepting a string, which flowed into model-selection arithmetic. - id=data["id"] raising bare KeyError; the loop in registry.py:168 built the catalog inside a try/except that only swallowed yaml.YAMLError and KeyError, so one bad manifest took the whole store listing offline. Converting AppManifest from a @DataClass to a pydantic.BaseModel gives per-field coercion and a ValidationError naming the offending field and manifest at the boundary, for zero new install weight (pydantic is already pulled in by FastAPI). The catalog load loop now wraps the validate step and skips a malformed manifest with a named log message instead of aborting the rest of the listing. scripts/check_manifests.py was the mirror-image bug on the CI side ('if not isinstance(lifecycle, dict): continue' silently skipped a malformed manifest, so a typo'd manifest passed the gate); the lint now validates every service manifest against the same AppManifest model the runtime uses. model_json_schema() is published as scripts/manifest.schema.json for third-party app authors. Red proof (before fix, tests/test_registry_manifest.py written, fix not yet applied): ``` FAILED tests/test_registry_manifest.py::TestBoundaryRejectsWrongTypes::test_string_requires_is_rejected Failed: DID NOT RAISE ValidationError FAILED tests/test_registry_manifest.py::TestBoundaryRejectsWrongTypes::test_string_context_window_is_coerced_or_rejected AssertionError: assert 8192 == '8192' + where '8192' = AppManifest(...).context_window 2 failed, 2 passed in 0.46s ``` Green proof (after fix): ``` tests/test_registry_manifest.py::TestBoundaryRejectsWrongTypes::test_string_requires_is_rejected PASSED tests/test_registry_manifest.py::TestBoundaryRejectsWrongTypes::test_string_context_window_is_coerced_or_rejected PASSED tests/test_registry_manifest.py::TestCatalogResilience::test_one_bad_manifest_does_not_abort_catalog PASSED tests/test_registry_manifest.py::TestWellFormedManifestStillLoads::test_well_formed_manifest_loads PASSED 4 passed in 0.29s ``` Rollout: warn-only -- the live catalog has a few float-typed version fields (dreamshaper-8-lcm, flux-schnell-gguf, pixart-sigma-512, sdxs-512) and the hailo-ollama service manifest was missing version entirely. The loop logs each skip with a named error and the rest of the catalog loads (259 of 276 loaded in the live catalog pass; old loader loaded 262). hailo-ollama gets a version: 0.1.0 to make the live catalog clean against the new schema. Docs-Reviewed: no README change needed; the only catalog manifest edit adds a required 'version' field to hailo-ollama, no manifest is added or removed and the catalog list in README is unchanged.
1 parent 9440fc9 commit e2f60e6

7 files changed

Lines changed: 384 additions & 54 deletions

File tree

app-catalog/services/hailo-ollama/manifest.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ id: hailo-ollama
44
license: Proprietary (Hailo EULA, accepted at install)
55
name: hailo-ollama (Hailo-10H NPU LLM)
66
type: service
7+
version: 0.1.0
78
category: llm-runtime
89
description: "Ollama-compatible LLM server on the Hailo-10H NPU (Raspberry Pi 5 + AI HAT+2)"
910
requires:
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
### Fixed
2+
3+
- `tinyagentos/registry`: `AppManifest` is now a `pydantic.BaseModel`, so
4+
manifests from the external catalog repo are type-checked at the load
5+
boundary. A wrong-typed field (e.g. `requires: "ollama"` where a mapping
6+
is required, `context_window: "8192"` where an int is required) now
7+
raises `pydantic.ValidationError` *naming the offending field and the
8+
manifest path* instead of silently propagating the bad value into
9+
install code as `AttributeError: 'str' object has no attribute 'get'`.
10+
The catalog load loop also skips a single bad manifest with a named
11+
log message instead of aborting the entire store listing, so one
12+
malformed entry no longer takes the store offline.
13+
- `scripts/check_manifests.py`: the CI lint now validates every service
14+
manifest against the same `AppManifest` pydantic model the runtime uses,
15+
closing the mirror-image bug where a typo'd manifest slipped through
16+
the gate as a silent skip.
17+
- `scripts/manifest.schema.json`: published JSON Schema for `AppManifest`
18+
so third-party app authors can validate their catalogs against the same
19+
contract the runtime enforces.

scripts/check_manifests.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,15 @@ def lint_managed(root: Path) -> list[str]:
4040
``lifecycle.health.expect`` is a literal substring matched against the
4141
backend's health-endpoint response body (a 200 body must contain it).
4242
"""
43+
# Validate every manifest against the same pydantic model the runtime
44+
# uses, so a typo'd manifest (e.g. ``requires: ollama`` as a string)
45+
# fails the gate instead of silently shipping. The CI mirror-image
46+
# bug was: `if not isinstance(lifecycle, dict): continue` skipped
47+
# malformed manifests, so a bad one passed the gate.
48+
from pydantic import ValidationError
49+
50+
from tinyagentos.registry import AppManifest
51+
4352
errors: list[str] = []
4453
services_dir = root / "services"
4554
for manifest in sorted(services_dir.glob("*/manifest.yaml")):
@@ -56,6 +65,13 @@ def lint_managed(root: Path) -> list[str]:
5665
errors.append(f"{sid_dir}: manifest.yaml top-level is not a mapping")
5766
continue
5867

68+
# Boundary validation: same model the runtime loads with.
69+
try:
70+
AppManifest.model_validate({**data, "manifest_dir": manifest.parent})
71+
except ValidationError as exc:
72+
errors.append(f"{sid_dir}: manifest failed schema validation: {exc}")
73+
continue
74+
5975
sid = str(data.get("id") or sid_dir)
6076
lifecycle = data.get("lifecycle")
6177
if not isinstance(lifecycle, dict):

scripts/manifest.schema.json

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
{
2+
"description": "A loaded catalog manifest.\n\nApp manifests are externally-authored input, pulled from a remote git repo\nby ``tinyagentos/catalog_sync.py``. Validating at this boundary keeps a\nsingle malformed entry from breaking install-time code paths deep in\nthe stack (the prior ``data.get(key, default)`` loader happily let a\nstring ``requires`` through to install code, which then raised\n``AttributeError: 'str' object has no attribute 'get'`` with no\nindication which manifest was malformed).\n\n``extra=\"ignore\"`` keeps the model forward-compatible with newer\ncatalog fields the runtime has not learned about yet.",
3+
"properties": {
4+
"id": {
5+
"title": "Id",
6+
"type": "string"
7+
},
8+
"name": {
9+
"title": "Name",
10+
"type": "string"
11+
},
12+
"type": {
13+
"title": "Type",
14+
"type": "string"
15+
},
16+
"version": {
17+
"title": "Version",
18+
"type": "string"
19+
},
20+
"description": {
21+
"default": "",
22+
"title": "Description",
23+
"type": "string"
24+
},
25+
"category": {
26+
"default": "",
27+
"title": "Category",
28+
"type": "string"
29+
},
30+
"icon": {
31+
"default": "",
32+
"title": "Icon",
33+
"type": "string"
34+
},
35+
"homepage": {
36+
"default": "",
37+
"title": "Homepage",
38+
"type": "string"
39+
},
40+
"license": {
41+
"default": "",
42+
"title": "License",
43+
"type": "string"
44+
},
45+
"weights_license": {
46+
"default": "",
47+
"title": "Weights License",
48+
"type": "string"
49+
},
50+
"license_class": {
51+
"default": "",
52+
"title": "License Class",
53+
"type": "string"
54+
},
55+
"requires": {
56+
"additionalProperties": true,
57+
"title": "Requires",
58+
"type": "object"
59+
},
60+
"install": {
61+
"additionalProperties": true,
62+
"title": "Install",
63+
"type": "object"
64+
},
65+
"hardware_tiers": {
66+
"additionalProperties": true,
67+
"title": "Hardware Tiers",
68+
"type": "object"
69+
},
70+
"config_schema": {
71+
"items": {},
72+
"title": "Config Schema",
73+
"type": "array"
74+
},
75+
"variants": {
76+
"items": {},
77+
"title": "Variants",
78+
"type": "array"
79+
},
80+
"context_window": {
81+
"default": 0,
82+
"title": "Context Window",
83+
"type": "integer"
84+
},
85+
"capabilities": {
86+
"items": {},
87+
"title": "Capabilities",
88+
"type": "array"
89+
},
90+
"lifecycle": {
91+
"additionalProperties": true,
92+
"title": "Lifecycle",
93+
"type": "object"
94+
},
95+
"manifest_dir": {
96+
"anyOf": [
97+
{
98+
"format": "path",
99+
"type": "string"
100+
},
101+
{
102+
"type": "null"
103+
}
104+
],
105+
"default": null,
106+
"title": "Manifest Dir"
107+
}
108+
},
109+
"required": [
110+
"id",
111+
"name",
112+
"type",
113+
"version"
114+
],
115+
"title": "AppManifest",
116+
"type": "object"
117+
}

tests/test_check_manifests.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,9 @@ def _write(root: Path, sid: str, manifest: dict) -> None:
2424
def _managed_ok(sid: str = "rkllama") -> dict:
2525
return {
2626
"id": sid,
27+
"name": sid,
2728
"type": "service",
29+
"version": "1.0.0",
2830
"category": "llm-runtime",
2931
"lifecycle": {
3032
"backend_type": "rkllama",
@@ -70,7 +72,9 @@ def test_non_managed_service_is_ignored(tmp_path: Path) -> None:
7072
# auto_manage false -> not claiming managed, no unit/health required
7173
m = {
7274
"id": "rk-llama-cpp",
75+
"name": "rk-llama-cpp",
7376
"type": "service",
77+
"version": "1.0.0",
7478
"category": "llm-runtime",
7579
"lifecycle": {"backend_type": "openai-compatible", "auto_manage": False},
7680
}
@@ -79,7 +83,9 @@ def test_non_managed_service_is_ignored(tmp_path: Path) -> None:
7983

8084

8185
def test_service_without_lifecycle_is_ignored(tmp_path: Path) -> None:
82-
_write(tmp_path, "ollama", {"id": "ollama", "type": "service"})
86+
_write(tmp_path, "ollama", {
87+
"id": "ollama", "name": "ollama", "type": "service", "version": "1.0.0",
88+
})
8389
assert check_manifests.lint_managed(tmp_path) == []
8490

8591

tests/test_registry_manifest.py

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
# tests/test_registry_manifest.py
2+
"""Schema validation at the catalog boundary.
3+
4+
App manifests come from an externally-authored git repo (see
5+
tinyagentos/catalog_sync.py) and were previously parsed with ``data.get(...)
6+
`` defaults and no type checks, so a single malformed entry broke the whole
7+
store listing at install time. These tests pin the boundary contract: wrong
8+
types are rejected with a named ``ValidationError`` *at load time*, and one
9+
bad manifest does not abort the rest of the catalog.
10+
"""
11+
from __future__ import annotations
12+
13+
import pytest
14+
import yaml
15+
from pydantic import ValidationError
16+
17+
from tinyagentos.registry import AppManifest, AppRegistry
18+
19+
20+
# -- helpers ----------------------------------------------------------------
21+
22+
def _write_manifest(app_dir, manifest: dict) -> None:
23+
app_dir.mkdir(parents=True, exist_ok=True)
24+
(app_dir / "manifest.yaml").write_text(yaml.dump(manifest))
25+
26+
27+
def _good_manifest(mid: str = "good-app") -> dict:
28+
return {
29+
"id": mid,
30+
"name": "Good App",
31+
"type": "agent-framework",
32+
"version": "1.0.0",
33+
}
34+
35+
36+
# -- RED tests --------------------------------------------------------------
37+
38+
class TestBoundaryRejectsWrongTypes:
39+
def test_string_requires_is_rejected(self, tmp_path, caplog):
40+
"""A string where ``requires`` should be a mapping would otherwise
41+
travel into the install path and raise AttributeError deep in
42+
install code, with no indication which manifest was malformed."""
43+
import logging
44+
45+
bad = _good_manifest("bad-requires")
46+
bad["requires"] = "ollama" # YAML string, not a mapping
47+
d = tmp_path / "agents" / "bad-requires"
48+
_write_manifest(d, bad)
49+
with caplog.at_level(logging.WARNING):
50+
with pytest.raises(ValidationError) as exc:
51+
AppManifest.from_file(d / "manifest.yaml")
52+
# ValidationError names the offending field...
53+
assert "requires" in str(exc.value)
54+
# ...and the load-time log names the offending manifest by id and path.
55+
assert any("bad-requires" in r.getMessage() for r in caplog.records)
56+
57+
def test_string_context_window_is_coerced_or_rejected(self, tmp_path):
58+
"""A string-valued ``context_window`` flowed into model-selection
59+
arithmetic. Either coerce to int or reject; a silent string is
60+
not acceptable."""
61+
bad = _good_manifest("bad-window")
62+
bad["type"] = "model"
63+
bad["context_window"] = "8192" # YAML string
64+
d = tmp_path / "models" / "bad-window"
65+
_write_manifest(d, bad)
66+
try:
67+
m = AppManifest.from_file(d / "manifest.yaml")
68+
except ValidationError:
69+
return # rejected is fine
70+
# If coerced, it must be the int 8192, not the string.
71+
assert m.context_window == 8192
72+
assert isinstance(m.context_window, int)
73+
74+
75+
class TestCatalogResilience:
76+
def test_one_bad_manifest_does_not_abort_catalog(self, tmp_path):
77+
"""A manifest missing the required ``id`` field used to raise bare
78+
``KeyError`` from from_dict, which the loop swallowed -- but every
79+
subsequent manifest in the same type_dir was loaded into the same
80+
loop, and a later exception propagated. Per the audit, one bad
81+
manifest must NOT take down the store listing: the rest load."""
82+
catalog = tmp_path
83+
agents = catalog / "agents"
84+
# Good manifest before the bad one.
85+
_write_manifest(agents / "alpha", _good_manifest("alpha"))
86+
# Bad manifest: missing required id field.
87+
bad = agents / "broken"
88+
bad.mkdir(parents=True, exist_ok=True)
89+
(bad / "manifest.yaml").write_text(
90+
yaml.dump({"name": "Broken", "type": "agent-framework", "version": "1.0.0"})
91+
)
92+
# Good manifest after the bad one.
93+
_write_manifest(agents / "zeta", _good_manifest("zeta"))
94+
95+
reg = AppRegistry(catalog_dir=catalog, installed_path=tmp_path / "installed.json")
96+
apps = reg.list_available()
97+
ids = {a.id for a in apps}
98+
# Both good manifests are listed, the bad one is skipped.
99+
assert "alpha" in ids
100+
assert "zeta" in ids
101+
assert "broken" not in ids
102+
103+
104+
class TestWellFormedManifestStillLoads:
105+
def test_well_formed_manifest_loads(self, tmp_path):
106+
"""Sanity: a well-formed manifest still loads cleanly through the
107+
new pydantic model."""
108+
d = tmp_path / "agents" / "happy"
109+
_write_manifest(d, _good_manifest("happy"))
110+
m = AppManifest.from_file(d / "manifest.yaml")
111+
assert m.id == "happy"
112+
assert m.type == "agent-framework"

0 commit comments

Comments
 (0)