test(discovery): enforce manifest invariants in CI

.github/workflows/ci.yml

@@ -38,6 +38,31 @@ jobs:
       - name: Run tests
         run: uv run pytest tests/ -q
 
+  # The Discovery harness is standard-library only and needs no VM to check
+  # itself: the manifest invariants and the harness's own tests both run here,
+  # in well under a second. The end-to-end run this manifest describes installs
+  # real software and needs a self-hosted runner, so it is deliberately not part
+  # of per-commit CI.
+  test-discovery:
+    name: Discovery tests
+    runs-on: ubuntu-latest
+    defaults:
+      run:
+        working-directory: Discovery
+    steps:
+      - uses: actions/checkout@v7
+
+      - name: Set up Python
+        uses: actions/setup-python@v6
+        with:
+          python-version: "3.12"
+
+      - name: Check the manifest
+        run: python3 -m tests.manifest_check
+
+      - name: Run the harness's own tests
+        run: python3 -m unittest discover -s tests -t . -q
+
   test-sensor:
     name: Sensor tests (py${{ matrix.python-version }})
     runs-on: ubuntu-latest

Discovery/tests/manifest.py

@@ -259,17 +259,99 @@ def canary_names(self) -> List[str]:
     # -- validation ----------------------------------------------------
 
     def validate(self) -> None:
-        """The structural rules the format enforces, at load time.
+        """Every structural rule the format enforces, at load time.
 
         Load-time rather than run-time because the alternative is discovering a
         broken row twenty minutes into a VM run that then has to start again.
         """
         problems: List[str] = []
+        problems.extend(self.check_ids())
+        problems.extend(self.check_canaries())
         problems.extend(self._check_pins())
         problems.extend(self._check_references())
         if problems:
             raise ManifestError("manifest is invalid:\n  " + "\n  ".join(problems))
 
+    def check_ids(self) -> List[str]:
+        """Static check 2: every id unique, and every family series contiguous.
+
+        A gap in a series is almost always a row someone deleted rather than a
+        deliberate hole, and a duplicate id silently makes one of the two rows
+        unscoreable.
+        """
+        problems = []
+        seen = set()
+        series: Dict[str, List[int]] = {}
+        for entry in self.entries:
+            if entry.id in seen:
+                problems.append("duplicate id %s" % entry.id)
+            seen.add(entry.id)
+            prefix, _, number = entry.id.rpartition("-")
+            if not number.isdigit():
+                problems.append("id %s does not end in a number" % entry.id)
+                continue
+            series.setdefault(prefix, []).append(int(number))
+        for prefix, numbers in sorted(series.items()):
+            numbers.sort()
+            expected = list(range(1, len(numbers) + 1))
+            if numbers != expected:
+                missing = sorted(set(expected) - set(numbers))
+                problems.append("series %s is not contiguous: missing %s" % (prefix, missing))
+        return problems
+
+    def check_canaries(self) -> List[str]:
+        """Static check 3: no ``{{canary:x}}`` without a declared ``x``.
+
+        An undeclared canary is worse than a missing one: it is planted, never
+        searched for, and the run reports a clean redaction check it never made.
+        """
+        problems = []
+        declared = set(self.canary_names())
+        for entry in self.entries:
+            referenced = set(CANARY_REF.findall(_serialize(entry.raw)))
+            for name in sorted(referenced - declared):
+                problems.append("%s references undeclared canary %r" % (entry.id, name))
+            for name in sorted(set(entry.canaries) - referenced):
+                problems.append("%s declares canary %r it never plants" % (entry.id, name))
+            for name in sorted(referenced - set(entry.canaries)):
+                problems.append("%s plants canary %r without listing it" % (entry.id, name))
+        return problems
+
+    def check_catalog_coverage(self, catalog_ids: Iterable[str]) -> List[str]:
+        """Static check 1: every catalog entry has a manifest row.
+
+        The check that keeps the manifest honest over time. A catalog entry
+        with no row is a tool the collector claims to recognize but that
+        nothing ever verifies, and adding a tool should fail CI until someone
+        adds the row. This single test does more than any process rule.
+        """
+        covered = {entry.catalog_id for entry in self.entries if entry.catalog_id}
+        return ["catalog entry %r has no manifest row" % name
+                for name in sorted(set(catalog_ids) - covered)]
+
+    def check_sources(self) -> List[str]:
+        """Which vendor-installed entries still have no resolved download.
+
+        Reported rather than raised: an unresolved source is a harness gap that
+        blocks one entry, not a broken manifest that blocks the run.
+        """
+        pending = []
+        for entry in self.entries:
+            source = (entry.install or {}).get("source")
+            if not source or entry.family not in ("app-installer", "vendor-binary", "service", "non-ai-app"):
+                continue
+            descriptor = self.sources.get(source)
+            if descriptor is None:
+                pending.append("%s: source %r is not in sources.yaml" % (entry.id, source))
+                continue
+            for platform in entry.platforms:
+                per_os = descriptor.get(platform)
+                if per_os is None:
+                    pending.append("%s: source %r has no %s descriptor" % (entry.id, source, platform))
+                elif per_os.get("url") == "":
+                    pending.append("%s: source %r has no %s url" % (entry.id, source, platform))
+        return pending
+
     def _check_pins(self) -> List[str]:
         """Every installed package names a version.
 

Discovery/tests/manifest_check.py

@@ -0,0 +1,66 @@
+"""The manifest's static checks, as a command CI can run.
+
+These are properties of the manifest and the catalog rather than of a machine,
+so they belong in ordinary per-commit CI beside the fast tests - the expensive
+instrument should never be the thing that discovers a typo.
+
+    python3 -m tests.manifest_check [--catalog path/to/catalog.json]
+
+The catalog is an input rather than something this package goes looking for:
+the harness treats the collector as a black box and does not read its tree. When
+no catalog is given the coverage check is reported as not run, because a check
+that did not run is not a check that passed - and the difference matters when
+this gates a merge.
+"""
+
+import argparse
+import json
+import sys
+from typing import List, Optional
+
+from .manifest import ManifestError, PLATFORMS, load
+
+
+def main(argv: Optional[List[str]] = None) -> int:
+    parser = argparse.ArgumentParser(prog="manifest_check", description=__doc__.splitlines()[0])
+    parser.add_argument("--catalog", help="the collector's catalog.json, to check coverage against")
+    args = parser.parse_args(argv)
+
+    try:
+        manifest = load()
+    except ManifestError as exc:
+        print(exc, file=sys.stderr)
+        return 1
+
+    print("manifest: %d entries" % len(manifest))
+    for platform in PLATFORMS:
+        print("  %-6s %d applicable" % (platform, len(manifest.for_platform(platform))))
+
+    failures: List[str] = []
+    failures.extend(manifest.check_ids())
+    failures.extend(manifest.check_canaries())
+
+    if args.catalog:
+        with open(args.catalog, encoding="utf-8") as handle:
+            catalog = json.load(handle)
+        ids = [entry["id"] for entry in catalog.get("entries", catalog)]
+        missing = manifest.check_catalog_coverage(ids)
+        failures.extend(missing)
+        print("  catalog: %d entries, %d without a manifest row" % (len(ids), len(missing)))
+    else:
+        print("  catalog: not checked (pass --catalog path/to/catalog.json)")
+
+    # Reported rather than failed: an unresolved vendor url blocks one entry,
+    # and is a gap in the harness rather than a broken manifest.
+    pending = manifest.check_sources()
+    if pending:
+        print("  sources: %d vendor descriptors unresolved" % len(pending))
+
+    for failure in failures:
+        print("FAIL %s" % failure, file=sys.stderr)
+    print("check: %s" % ("failed" if failures else "passed"))
+    return 1 if failures else 0
+
+
+if __name__ == "__main__":
+    raise SystemExit(main())

Discovery/tests/test_manifest.py

@@ -36,6 +36,30 @@ def test_half_the_manifest_needs_no_installer(self):
                         if entry.family in ("declare-mcp", "artifact")]
         self.assertEqual(len(no_installer), 51)
 
+    def test_ids_are_unique_and_contiguous(self):
+        """Static check 2. A gap is a deleted row; a duplicate silently makes
+        one of the two rows unscoreable."""
+        self.assertEqual(self.manifest.check_ids(), [])
+
+    def test_every_canary_is_declared_and_planted(self):
+        """Static check 3. An undeclared canary is planted, never searched for,
+        and the run reports a clean redaction check it never made."""
+        self.assertEqual(self.manifest.check_canaries(), [])
+        self.assertEqual(len(self.manifest.canaries), 6)
+
+    def test_catalog_coverage_is_checkable(self):
+        """Static check 1, against a catalog supplied by the caller.
+
+        The harness does not read the collector's tree, so the catalog is an
+        input. What is asserted here is that the check *works* - a catalog entry
+        with no manifest row must be reported.
+        """
+        covered = [entry.catalog_id for entry in self.manifest if entry.catalog_id]
+        self.assertEqual(self.manifest.check_catalog_coverage(covered), [])
+        missing = self.manifest.check_catalog_coverage(covered + ["brand-new-tool"])
+        self.assertEqual(len(missing), 1)
+        self.assertIn("brand-new-tool", missing[0])
+
     def test_every_catalogued_tool_appears_once(self):
         seen = {}
         for entry in self.manifest:
@@ -62,12 +86,28 @@ def test_shapes_are_derived_not_declared(self):
         shapes = {entry.shape for entry in self.manifest}
         self.assertEqual(shapes, {"install", "declare", "create", "state"})
 
+    def test_unresolved_sources_are_reported_not_raised(self):
+        """A URL nobody has filled in blocks one entry; it does not invalidate
+        the manifest."""
+        pending = self.manifest.check_sources()
+        self.assertTrue(pending)
+        self.assertTrue(all(":" in line for line in pending))
+
 
 class ManifestValidation(unittest.TestCase):
     def test_a_bad_family_is_refused_at_load(self):
         with self.assertRaises(manifest_module.ManifestError):
             manifest_module._family({"id": "X-01", "install": {"method": "telepathy"}})
 
+    def test_an_undeclared_canary_is_a_failure(self):
+        manifest = manifest_module.load()
+        entry = manifest.by_id("S-16")
+        entry.raw = dict(entry.raw)
+        entry.raw["create"] = dict(entry.raw["create"],
+                                   command="audit.sh --token {{canary:not_declared}}")
+        problems = manifest.check_canaries()
+        self.assertTrue(any("not_declared" in problem for problem in problems))
+
 
 if __name__ == "__main__":
     unittest.main()