diff --git a/CHANGELOG.md b/CHANGELOG.md index f8c0555..67e65e1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,9 +12,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 an open format with its own repo, normative spec, JSON Schemas, and reference validator — but nothing stopped core from reading a manifest key the spec never defined, which is exactly what happened with `original_audio` (#583 → #933). `tools/check_spec_conformance.py` now enforces three surface properties - in CI: (1) **key-coverage** — every manifest key core reads is declared in the spec's + in CI: (1) **key-coverage** — every manifest key core reads *or writes* is declared in the spec's `manifest.schema.json`, found by walking the AST of `lib/sloppak.py`, `lib/enrichment.py`, and - `lib/songmeta.py`; (2) **forward** — core's `load_song()` ingests every example pack the spec ships; + `lib/songmeta.py` (writes are gated too, and reported separately: a key core writes lands in every pack + we emit, so an undeclared one seeds the ecosystem with non-spec data); (2) **forward** — core's + `load_song()` ingests every example pack the spec ships; (3) **reverse** — every pack committed here passes the spec's own `tools/validate.py` (7/7 pass today). The spec is pinned by SHA in `.feedpak-spec-ref` so a change over there can't redden an unrelated PR here; bump it in its own PR, and a red result is the signal that core doesn't satisfy the new spec. diff --git a/feedpak-spec-exceptions.yml b/feedpak-spec-exceptions.yml index b0ef1a2..b2ccd52 100644 --- a/feedpak-spec-exceptions.yml +++ b/feedpak-spec-exceptions.yml @@ -1,4 +1,9 @@ -# Manifest keys core reads that the feedpak spec does not (yet) define. +# Manifest keys core reads OR WRITES that the feedpak spec does not (yet) define. +# +# Both directions are gated, and both are allowlisted here. A key core *writes* +# (`manifest["k"] = v`) is spec surface pointed outward — it lands in every pack +# we emit — so an undeclared one needs an entry here just as much as one core +# reads. # # This file exists so the spec-conformance gate (tools/check_spec_conformance.py) # can be honest instead of being switched off. A gate with no legitimate way to @@ -10,8 +15,10 @@ # - Every entry needs a tracking issue. No issue, no exception. # - Entries are debt, not policy. The fix is to land the key in the spec # (github.com/got-feedback/feedpak-spec) and delete the entry. +# - One entry per key. A duplicate is an error, not a last-one-wins. # - The gate fails if an entry goes stale — i.e. the spec caught up, or core -# stopped reading the key. The allowlist must never become a hiding place. +# no longer reads or writes the key. The allowlist must never become a +# hiding place. # # For a key that is genuinely experimental and not yet ready for the spec, # prefer the reserved `x-` prefix (e.g. `x-my_new_key`) over an exception: the diff --git a/tools/check_spec_conformance.py b/tools/check_spec_conformance.py index 46126e3..769c485 100644 --- a/tools/check_spec_conformance.py +++ b/tools/check_spec_conformance.py @@ -130,12 +130,20 @@ def load_exceptions() -> dict[str, str]: if not key or not issue: _fail(f"{EXCEPTIONS_FILE.name}: every exception needs both 'key' and 'issue'") sys.exit(1) + # A duplicate would silently take the last issue link, quietly retargeting + # the debt this file exists to track. Fail instead. + if key in out: + _fail( + f"{EXCEPTIONS_FILE.name}: '{key}' is listed more than once. " + f"Keep one entry per key so the tracking issue is unambiguous." + ) + sys.exit(1) out[key] = issue return out def check_key_coverage(spec: Path) -> bool: - """Layer 1 — core must not read a manifest key the spec does not declare.""" + """Layer 1 — core must not read or write a manifest key the spec does not declare.""" schema = json.loads((spec / "schemas" / "manifest.schema.json").read_text(encoding="utf-8")) declared = set(schema.get("properties") or {}) if not declared: @@ -209,8 +217,17 @@ def check_forward(spec: Path) -> bool: if not examples_dir.is_dir(): _fail(f"{examples_dir} is missing — wrong path or bad checkout?") return False + # rglob, not iterdir: the contract is "every example pack the spec ships", so + # a pack nested under examples// must not slip through. + # + # Deliberately NOT filtered by is_file(): a feedpak is dual-form — a zip + # (`foo.feedpak`) *or* a directory (`foo.feedpak/`) — and the spec's own + # examples ship as directories today. An is_file() guard here would silently + # match zero packs. Matching on the suffix covers both forms, and rglob does + # not smuggle in a pack's innards because files inside a pack don't carry a + # pack suffix. examples = sorted( - p for p in examples_dir.iterdir() + p for p in examples_dir.rglob("*") if p.suffix in (".feedpak", ".sloppak") ) if not examples: @@ -218,7 +235,14 @@ def check_forward(spec: Path) -> bool: return False sys.path.insert(0, str(REPO / "lib")) - import sloppak # noqa: E402 (path must be set first — flat imports, no package) + try: + import sloppak # noqa: E402 (path must be set first — flat imports, no package) + except Exception as e: + _fail( + f"could not import core's sloppak loader ({type(e).__name__}: {e}). " + f"Are requirements.txt deps installed?" + ) + return False ok = True with tempfile.TemporaryDirectory() as tmp: