diff --git a/docs/feedpak-spec-gate.md b/docs/feedpak-spec-gate.md index 3d1cadb..7df5222 100644 --- a/docs/feedpak-spec-gate.md +++ b/docs/feedpak-spec-gate.md @@ -104,18 +104,21 @@ branch. Known, and worth fixing in follow-ups rather than blocking on: -- **Layer 1 is name-heuristic.** It recognises manifest dicts bound to locals named in `MANIFEST_VARS` - (`manifest`, `mf`) plus the `load_manifest(...)` call form. This works because the loaders use a uniform - idiom, but it is fragile against a refactor that renames the local. The `readers-complete` guard limits the - blast radius — a module that touches manifest keys can't go unscanned — but a *renamed local inside a - scanned module* would still slip. The hardening step is to route all manifest access through a single - declared `KNOWN_MANIFEST_KEYS` registry in `lib/sloppak.py`; the gate then compares registry against schema - exactly instead of inferring. +- **Layer 1's receiver detection is heuristic.** Locals *assigned from* `load_manifest(...)` are discovered + flow-aware whatever they're called (chart.py's `m` taught us that), and the inline + `(load_manifest(p) or {}).get(...)` form is recognised — but a manifest that arrives as a **function + parameter** is only recognised by name (`MANIFEST_VARS`: `manifest`, `mf`). A parameter called something + else would slip. The hardening step is to route all manifest access through a single declared + `KNOWN_MANIFEST_KEYS` registry in `lib/sloppak.py`; the gate then compares registry against schema exactly + instead of inferring. - **Layer 1 covers top-level keys only.** Nested structure (`arrangements[].file`, `.id`, `.notation`) isn't checked. Extending to it means walking the schema's `$ref` subschemas. - **Layer 1 recognises `get`, `setdefault`, and subscripts** as key access. `update()` and `pop()` aren't used against a feedpak manifest anywhere in the tree, so they're deliberately not special-cased rather than - speculatively handled — but note `KEY_OPS` (which drives `readers-complete`) doesn't look for them either. + speculatively handled. `readers-complete` reuses the same scanner (`keys_touched()`), so this blind spot is + shared, not doubled: a module using only unrecognised access forms would evade both. Keys passed as literal + *call arguments* to helpers (`_gap_fill_manifest_absent(manifest, "album")` in `lib/routers/song.py`) are + likewise unseen. - **Layer 4 can't catch unknown keys**, because `manifest.schema.json` sets `additionalProperties: true` and the reference validator deliberately "treats unknown keys/files as forward-compatible". Fixing this properly belongs in the spec (tighten the schema, or give the validator a `--strict` mode). Until then, diff --git a/tests/test_spec_gate.py b/tests/test_spec_gate.py index 29027a2..dc31d24 100644 --- a/tests/test_spec_gate.py +++ b/tests/test_spec_gate.py @@ -105,6 +105,19 @@ def test_duplicate_exception_key_is_rejected(): gate._parse_exceptions(textwrap.dedent(doc), "test") +@pytest.mark.parametrize("doc", [ + "- just\n- a\n- list\n", # list at top level + "exceptions: not-a-list\n", # scalar where list expected + "exceptions:\n - just-a-string\n", # non-mapping entry + "exceptions: [\n", # invalid YAML +]) +def test_malformed_exceptions_fail_legibly(doc): + # Malformed shapes must exit with a ::error::, not an AttributeError + # traceback — CI output has to say what to fix. + with pytest.raises(SystemExit): + gate._parse_exceptions(doc, "test") + + def test_exception_without_issue_is_rejected(): # No tracking issue, no exception — entries are debt and debt is tracked. doc = """ diff --git a/tools/check_spec_conformance.py b/tools/check_spec_conformance.py index cfa31d0..9ca13b6 100644 --- a/tools/check_spec_conformance.py +++ b/tools/check_spec_conformance.py @@ -182,9 +182,25 @@ def _parse_exceptions(text: str, origin: str) -> dict[str, str]: """Parse an exceptions document into {key: tracking issue}.""" import yaml # runtime dep (PyYAML is already in requirements.txt) - data = yaml.safe_load(text) or {} + try: + data = yaml.safe_load(text) or {} + except yaml.YAMLError as e: + _fail(f"{origin}: not valid YAML — {e}") + sys.exit(1) + # A malformed shape (list/string at top level, non-mapping entry) must fail + # with a CI-legible error, not an AttributeError traceback. + if not isinstance(data, dict): + _fail(f"{origin}: top level must be a mapping with an 'exceptions' list, got {type(data).__name__}") + sys.exit(1) + entries = data.get("exceptions") or [] + if not isinstance(entries, list): + _fail(f"{origin}: 'exceptions' must be a list, got {type(entries).__name__}") + sys.exit(1) out: dict[str, str] = {} - for entry in data.get("exceptions") or []: + for entry in entries: + if not isinstance(entry, dict): + _fail(f"{origin}: each exception must be a mapping with 'key' and 'issue', got {type(entry).__name__}") + sys.exit(1) key, issue = entry.get("key"), entry.get("issue") if not key or not issue: _fail(f"{origin}: every exception needs both 'key' and 'issue'")