From b361938c21bece1a33732225f8b8feae2122b8f8 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Thu, 1 Oct 2026 17:45:47 -0400 Subject: [PATCH 1/7] test(claude-ops): add the reader comparison harness and pin open #5640 findings compare_reports.py classifies every changed value between two inventory reports and fails on a value->value change not named in --allow. test_reader_findings.py pins findings 3-5 as expected failures. test_fixture_parse.py checks the test_inventory fixtures parse under acorn per module and lists the 39 tests whose fixtures do not. Part of #5640 (P0). Co-Authored-By: Claude Opus 5.5 --- plugins/claude-ops/.claude-plugin/plugin.json | 2 +- plugins/claude-ops/CHANGELOG.md | 15 ++ .../inventory/scripts/compare_reports.py | 245 ++++++++++++++++++ .../inventory/scripts/test_compare_reports.py | 222 ++++++++++++++++ .../inventory/scripts/test_fixture_parse.py | 186 +++++++++++++ .../inventory/scripts/test_reader_findings.py | 108 ++++++++ 6 files changed, 777 insertions(+), 1 deletion(-) create mode 100644 plugins/claude-ops/skills/inventory/scripts/compare_reports.py create mode 100644 plugins/claude-ops/skills/inventory/scripts/test_compare_reports.py create mode 100644 plugins/claude-ops/skills/inventory/scripts/test_fixture_parse.py create mode 100644 plugins/claude-ops/skills/inventory/scripts/test_reader_findings.py diff --git a/plugins/claude-ops/.claude-plugin/plugin.json b/plugins/claude-ops/.claude-plugin/plugin.json index 4a0c382668..9f9b1a40a3 100644 --- a/plugins/claude-ops/.claude-plugin/plugin.json +++ b/plugins/claude-ops/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json", "name": "claude-ops", - "version": "0.80.1", + "version": "0.80.2", "description": "Claude Code operations toolkit. Fifteen skills: audit-skill-visibility (audit whether each installed skill is actually VISIBLE to the model, and diagnose why most of a fleet never gets used: a skill is invisible when its description is dropped by Claude Code's skill-listing context budget, which sheds descriptions lowest-score-first so an unused skill loses the keywords that would let it be matched, from skills genuinely not wanted, from skills the run cannot observe at all; computes whether the listing overflows from documented settings, and withholds every cold verdict the data cannot support rather than reporting absence of data as absence of use), inventory (read-only enumeration of the complete invocable surface: every built-in CLI command with aliases and hidden/gated status, every bundled skill, every built-in subagent and tool, and every component of every installed plugin across all marketplaces; reads the shipped binary because upstream publishes no built-in command list, and carries an integrity verdict so a drifted build reports counts as floors rather than silently short totals), audit-install-state (read-only audit of the machine-scope ~/.claude installation directory and ~/.claude.json: full inventory split into an authored surface and rolled-up bulk trees, product-managed retention vs genuinely unmanaged state, filename-scheme resolution before any process-liveness check, and deliberate/mid-experiment detection; reports, never deletes), audit-performance (read-only slowness-diagnostic capture run at the moment the machine or a session feels slow: CLI version, retention-sweep health including the unparsable-settings pause, which warns in /status, a timed census walk of the install tree as a sweep-cost proxy, active-session and plugin-fleet counts, a process census, and the fan-out layer, which covers a load-labeled no-op spawn baseline, every hook that will fire bucketed per-tool-call versus per-turn with its invocation shape, the configured statusline, subagent concurrency and spawn-depth ceilings against documented defaults, whether running sessions predate the settings file they are judged by, and orphan attribution by parent liveness rather than age, plus on Windows a kernel-object census (Token objects against uptime, paged pool) that names a host-level leak beneath all four suspects; read against a bundled known-performance-issues reference that also records the causes tested and cleared; separates the four documented suspects of accumulated state, version regression, component bloat, and per-spawn fan-out cost, and routes remediation out; reports, never mutates, and never executes a discovered hook or statusline command), audit-native-overlap (map native Claude Code surfaces, namely built-in CLI commands, bundled skills, plugin-backed built-ins, and session-provided skills, against the current repo's plugin skills and agents, so a custom component never silently duplicates what Claude Code itself ships; bare invocation is a read-only overlap report carrying the extraction's integrity floors and a shared-listing-budget exposure section, verdicts are human-gated in a committed store rendered into a generated registry whose every row carries an observable recheck trigger, and only an explicit apply step bakes presence-gated native references into descriptions and Boundary sections), observability (read locally captured telemetry from the OTEL store, the collector, the per-session hook event log and hook-event JSONL, and ccusage, with trend reports, a per-session report of what fired, what was blocked and the event timeline, and store pruning), known-issues (search known Claude product GitHub bugs, check service health, maintain a persistent tracked-issue registry), changelog (ingest Claude Code changelog entries and turn them into decisions: apply executes those in scope one PR per owner plugin and hands larger ones off as work items, then re-extract the native surface and file its drift as work items), prerequisites (read-only table of external binaries declared by enabled plugins; never installs), check (read-only check that node and jq resolve for the claude-ops hooks; never installs), machine-profile (discover this machine's facts and per-tree identity domains, store them as a re-runnable profile with the observation behind every value, and diff the stored profile against the host now; read-only unless the operator confirms a write, never installs and never reapplies a stored value on its own), plugins (bring a machine's plugin fleet current on demand: marketplace refresh, effective-scope updates including in-repo project/local installs, new-plugin install per policy, scope-divergence detection and explicit convergence), morning-brief (read-only gh-based operator morning view: queue-label counts, merge-ready PRs, parked decisions with their RECOMMENDED lines, and loop-lane telemetry freshness), lanes (start/restart/stop/status loop lanes as named background Claude Code sessions seeded from canonical prompt files, with per-lane model/effort, a repo-pull + marketplace-refresh launch step, and a consume-restarts action, an OS-schedulable reader that relaunches stopped lanes whose telemetry carries a restart_request), and a re-runnable setup action that settles where the known-issues registry, the skill-usage log and the hook log root live, places the root's self-ignoring guard, and detects retired conventions. Plus an opt-in, default-off per-session hook event log (one JSON line per hook event on every event the generated registry marks observable, written to /sessions/.jsonl, with SessionEnd retention by session count or age and an optional detached pre-prune command), a family of eight advisory *-audit hooks (API errors, config changes, instruction loads, permission denials, pre-compaction, skill usage, tool failures, and unsurfaced hook failures. The last also warns the user via systemMessage, since a hook that fails to launch enforces nothing and Claude Code surfaces the failure to nobody) that emit the shared hook-telemetry envelope, and a reference sink that routes envelopes under the same root: per session when the envelope carries a session id, else into the shared hook-events.jsonl the observability skill reads.", "author": { "name": "Melodic Software", diff --git a/plugins/claude-ops/CHANGELOG.md b/plugins/claude-ops/CHANGELOG.md index 22b11073ad..aa77b66f4a 100644 --- a/plugins/claude-ops/CHANGELOG.md +++ b/plugins/claude-ops/CHANGELOG.md @@ -3,6 +3,21 @@ All notable changes to the `claude-ops` plugin are documented here. Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning. +## [0.80.2] - 2026-10-01 + +### Added + +- **A harness for proving an inventory reader change alters only the values it means to.** + `compare_reports.py` diffs two inventory reports, ignores run metadata, and classifies each + changed value as `wrong->unresolved`, `unresolved->resolved`, `value->value`, added or + removed; a `value->value` change exits 1 unless an `--allow` file names its JSON pointer + with a reason. `test_reader_findings.py` pins the open #5640 findings (array mutation + through a method call or call argument, an arrow earlier in the statement leaving a spread + partial, and declaration text in a string or comment hiding a write) as expected failures + that flip when a reader fix lands. `test_fixture_parse.py` checks every JavaScript fixture + the inventory tests feed the reader parses as a module under acorn, when node and acorn + resolve, and lists the 39 tests whose fixtures do not yet. + ## [0.80.1] - 2026-10-01 ### Changed diff --git a/plugins/claude-ops/skills/inventory/scripts/compare_reports.py b/plugins/claude-ops/skills/inventory/scripts/compare_reports.py new file mode 100644 index 0000000000..85f659d5d2 --- /dev/null +++ b/plugins/claude-ops/skills/inventory/scripts/compare_reports.py @@ -0,0 +1,245 @@ +#!/usr/bin/env python3 +"""Diff two inventory JSON reports and classify every changed value. + +Built to gate a reader change: run `inventory.py --binary-only --binary ` +before and after, then compare. A value the reader stopped reading or newly +reads is expected; a value that changed into a different value is a wrong +value somewhere, so it fails unless an `--allow` file names it. + +Classes, per changed leaf (a JSON pointer, RFC 6901): + + wrong->unresolved a concrete value became unresolved + unresolved->resolved an unresolved value became concrete + value->value a concrete value became a different concrete value, + or an unresolved one a different unresolved one + added / removed the key or list item exists on one side only + +A value is unresolved when its field's sibling `_source` is `partial` +or `unresolved`, when it is that source field itself holding one of those, or +when it is a string holding the reader's `…` runtime placeholder. + +Run metadata is ignored: `/host`, and `elapsed_seconds`, `path` and +`selected_by` anywhere under `/sources`. + +Allow file: a JSON list of {"pointer": "/a/b", "reason": "..."}. An entry +allows a value->value change at that pointer or under it. + +Exit: 0 no disallowed change, 1 a disallowed value->value change, 2 usage or +input error. + +Run: python3 compare_reports.py before.json after.json [--allow allow.json] [--json] +""" + +from __future__ import annotations + +import argparse +import json +import sys +from typing import Any + +UNRESOLVED_SOURCES = frozenset({"partial", "unresolved"}) +ELLIPSIS = "…" +METADATA_KEYS = frozenset({"elapsed_seconds", "path", "selected_by"}) +CLASSES = ( + "value->value", + "wrong->unresolved", + "unresolved->resolved", + "added", + "removed", +) + +_MISSING = object() + + +def _escape(key: str) -> str: + return key.replace("~", "~0").replace("/", "~1") + + +def _ignored(pointer: str, key: str) -> bool: + if pointer == "" and key == "host": + return True + return pointer.startswith("/sources") and key in METADATA_KEYS + + +def _unresolved(value: Any, source: Any) -> bool: + if source in UNRESOLVED_SOURCES: + return True + if isinstance(value, list): + return ELLIPSIS in value + return isinstance(value, str) and ELLIPSIS in value + + +def _scalar(value: Any) -> bool: + return not isinstance(value, (dict, list)) + + +def _diff( + old: Any, new: Any, pointer: str, src_old: Any, src_new: Any, out: list +) -> None: + if isinstance(old, dict) and isinstance(new, dict): + for key in sorted(old.keys() | new.keys()): + if _ignored(pointer, key): + continue + child = pointer + "/" + _escape(key) + a, b = old.get(key, _MISSING), new.get(key, _MISSING) + if a is _MISSING or b is _MISSING: + out.append(_change(child, a, b)) + continue + if key.endswith("_source"): + sa, sb = a, b + else: + sa = old.get(key + "_source", src_old) + sb = new.get(key + "_source", src_new) + _diff(a, b, child, sa, sb, out) + return + if isinstance(old, list) and isinstance(new, list): + # A list of names reads as a whole, so an inserted name does not + # shift every later index into a spurious value->value. + if all(map(_scalar, old + new)): + if old != new: + out.append(_change(pointer, old, new, src_old, src_new)) + return + for i in range(max(len(old), len(new))): + a = old[i] if i < len(old) else _MISSING + b = new[i] if i < len(new) else _MISSING + child = f"{pointer}/{i}" + if a is _MISSING or b is _MISSING: + out.append(_change(child, a, b)) + else: + _diff(a, b, child, src_old, src_new, out) + return + if old == new and type(old) is type(new): + return + out.append(_change(pointer, old, new, src_old, src_new)) + + +def _change( + pointer: str, old: Any, new: Any, src_old: Any = None, src_new: Any = None +) -> dict: + if old is _MISSING: + cls = "added" + elif new is _MISSING: + cls = "removed" + else: + was, now = _unresolved(old, src_old), _unresolved(new, src_new) + if was == now: + cls = "value->value" + elif now: + cls = "wrong->unresolved" + else: + cls = "unresolved->resolved" + rec: dict[str, Any] = {"pointer": pointer, "class": cls} + if old is not _MISSING: + rec["old"] = old + if new is not _MISSING: + rec["new"] = new + return rec + + +def _covers(allowed: str, pointer: str) -> bool: + return pointer == allowed or pointer.startswith(allowed.rstrip("/") + "/") + + +def compare(old: Any, new: Any, allow: list[dict] | None = None) -> dict: + """The classified changes from `old` to `new`, with `failed` set when a + value->value change is not covered by `allow`.""" + changes: list[dict] = [] + _diff(old, new, "", None, None, changes) + allow = allow or [] + used: set[str] = set() + for rec in changes: + if rec["class"] != "value->value": + continue + for entry in allow: + if _covers(entry["pointer"], rec["pointer"]): + rec["allowed"] = entry.get("reason", "") + used.add(entry["pointer"]) + break + counts = {c: sum(1 for r in changes if r["class"] == c) for c in CLASSES} + disallowed = [ + r for r in changes if r["class"] == "value->value" and "allowed" not in r + ] + return { + "counts": counts, + "changes": changes, + "disallowed": len(disallowed), + "unused_allow": [e["pointer"] for e in allow if e["pointer"] not in used], + "failed": bool(disallowed), + } + + +def _load_allow(path: str) -> list[dict]: + with open(path, encoding="utf-8") as fh: + data = json.load(fh) + if not isinstance(data, list) or not all( + isinstance(e, dict) + and isinstance(e.get("pointer"), str) + and isinstance(e.get("reason"), str) + and e["reason"].strip() + for e in data + ): + raise ValueError( + 'allow file must be a JSON list of {"pointer": str, "reason": non-empty str}' + ) + return data + + +def _short(value: Any) -> str: + text = json.dumps(value, ensure_ascii=False) + return text if len(text) <= 120 else text[:117] + "..." + + +def render(result: dict) -> str: + lines = ["changes: " + ", ".join(f"{c} {n}" for c, n in result["counts"].items())] + for cls in CLASSES: + group = [r for r in result["changes"] if r["class"] == cls] + if not group: + continue + lines.append(f"\n{cls} ({len(group)})") + for r in group: + tail = f" [allowed: {r['allowed']}]" if "allowed" in r else "" + lines.append(f" {r['pointer']}{tail}") + if "old" in r: + lines.append(f" - {_short(r['old'])}") + if "new" in r: + lines.append(f" + {_short(r['new'])}") + for p in result["unused_allow"]: + lines.append(f"\nwarning: allow entry {p} matched no value->value change") + if result["failed"]: + n = result["disallowed"] + lines.append(f"\nFAIL: {n} value->value change(s) not in --allow") + else: + lines.append("\nOK: no disallowed value->value change") + return "\n".join(lines) + + +def main(argv: list[str] | None = None) -> int: + ap = argparse.ArgumentParser( + description="Classify the value changes between two inventory reports." + ) + ap.add_argument("before", help="report from the current reader") + ap.add_argument("after", help="report from the changed reader") + ap.add_argument( + "--allow", help="JSON list of {pointer, reason} for intended value changes" + ) + ap.add_argument("--json", action="store_true", help="print the result as JSON") + args = ap.parse_args(argv) + try: + with open(args.before, encoding="utf-8") as fh: + old = json.load(fh) + with open(args.after, encoding="utf-8") as fh: + new = json.load(fh) + allow = _load_allow(args.allow) if args.allow else [] + except (OSError, ValueError) as exc: + print(f"error: {exc}", file=sys.stderr) + return 2 + result = compare(old, new, allow) + if args.json: + print(json.dumps(result, indent=2, ensure_ascii=False)) + else: + print(render(result)) + return 1 if result["failed"] else 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/plugins/claude-ops/skills/inventory/scripts/test_compare_reports.py b/plugins/claude-ops/skills/inventory/scripts/test_compare_reports.py new file mode 100644 index 0000000000..65a147edf9 --- /dev/null +++ b/plugins/claude-ops/skills/inventory/scripts/test_compare_reports.py @@ -0,0 +1,222 @@ +#!/usr/bin/env python3 +"""Tests for compare_reports.py. + +Run: python3 -m unittest test_compare_reports +""" + +from __future__ import annotations + +import contextlib +import io +import json +import pathlib +import tempfile +import unittest + +import compare_reports as cr + + +def _report(**agent: object) -> dict: + rec = { + "description": "Fast search", + "description_source": "constant", + "disallowed_tools": ["Agent", "Edit"], + "disallowed_tools_source": "literal", + } + rec.update(agent) + return { + "schema": 1, + "host": {"python": "3.14.0"}, + "sources": { + "binary": { + "path": "/a", + "elapsed_seconds": 1.0, + "selected_by": "x", + "size": 9, + } + }, + "builtin_agents": {"Explore": rec}, + } + + +def _classes(old: dict, new: dict, allow: list | None = None) -> dict[str, str]: + result = cr.compare(old, new, allow) + return {r["pointer"]: r["class"] for r in result["changes"]} + + +AGENT = "/builtin_agents/Explore" + + +class TestClassification(unittest.TestCase): + def test_identical_reports_have_no_change(self) -> None: + result = cr.compare(_report(), _report()) + self.assertEqual(result["changes"], []) + self.assertFalse(result["failed"]) + + def test_run_metadata_is_ignored(self) -> None: + new = _report() + new["host"] = {"python": "3.15.0"} + new["sources"]["binary"].update(path="/b", elapsed_seconds=2.0, selected_by="y") + self.assertEqual(cr.compare(_report(), new)["changes"], []) + + def test_metadata_keys_outside_sources_are_compared(self) -> None: + old, new = _report(path="/a"), _report(path="/b") + self.assertEqual(_classes(old, new), {AGENT + "/path": "value->value"}) + + def test_a_concrete_value_that_changes_is_value_to_value_and_fails(self) -> None: + result = cr.compare(_report(), _report(description="Slow search")) + self.assertEqual( + {r["pointer"]: r["class"] for r in result["changes"]}, + {AGENT + "/description": "value->value"}, + ) + self.assertTrue(result["failed"]) + + def test_a_list_that_turns_partial_is_wrong_to_unresolved(self) -> None: + new = _report(disallowed_tools=["Agent"], disallowed_tools_source="partial") + result = cr.compare(_report(), new) + self.assertEqual( + {r["pointer"]: r["class"] for r in result["changes"]}, + { + AGENT + "/disallowed_tools": "wrong->unresolved", + AGENT + "/disallowed_tools_source": "wrong->unresolved", + }, + ) + self.assertFalse(result["failed"]) + + def test_a_partial_list_that_resolves_is_unresolved_to_resolved(self) -> None: + old = _report(disallowed_tools=["Agent"], disallowed_tools_source="partial") + new = _report(disallowed_tools=["Agent", "Edit", "Artifact"]) + self.assertEqual(set(_classes(old, new).values()), {"unresolved->resolved"}) + + def test_an_ellipsis_placeholder_marks_a_string_unresolved(self) -> None: + old = _report(description="Use … here") + new = _report(description="Use Grep here") + self.assertEqual( + _classes(old, new), {AGENT + "/description": "unresolved->resolved"} + ) + + def test_an_ellipsis_element_marks_a_list_unresolved(self) -> None: + old = _report(disallowed_tools=["…", "Agent"]) + self.assertEqual( + _classes(old, _report()), + {AGENT + "/disallowed_tools": "unresolved->resolved"}, + ) + + def test_two_different_partial_values_are_value_to_value(self) -> None: + old = _report(disallowed_tools=["A"], disallowed_tools_source="partial") + new = _report(disallowed_tools=["B"], disallowed_tools_source="partial") + self.assertEqual( + _classes(old, new), {AGENT + "/disallowed_tools": "value->value"} + ) + + def test_an_inserted_name_is_one_change_not_a_shift(self) -> None: + new = _report(disallowed_tools=["Agent", "Artifact", "Edit"]) + self.assertEqual( + _classes(_report(), new), {AGENT + "/disallowed_tools": "value->value"} + ) + + def test_added_and_removed_keys(self) -> None: + old, new = _report(), _report() + del old["builtin_agents"]["Explore"]["description"] + new["builtin_agents"]["Plan"] = {"tools": []} + self.assertEqual( + _classes(old, new), + { + AGENT + "/description": "added", + "/builtin_agents/Plan": "added", + }, + ) + self.assertEqual( + _classes(new, old), + {AGENT + "/description": "removed", "/builtin_agents/Plan": "removed"}, + ) + self.assertFalse(cr.compare(old, new)["failed"]) + + def test_list_items_of_objects_diff_by_index(self) -> None: + old = {"rows": [{"a": 1}, {"a": 2}]} + new = {"rows": [{"a": 1}, {"a": 3}, {"a": 4}]} + self.assertEqual( + _classes(old, new), {"/rows/1/a": "value->value", "/rows/2": "added"} + ) + + def test_a_type_change_is_a_change(self) -> None: + self.assertEqual(_classes({"n": 1}, {"n": True}), {"/n": "value->value"}) + + def test_pointer_segments_are_escaped(self) -> None: + self.assertEqual( + _classes({"a/b~c": 1}, {"a/b~c": 2}), {"/a~1b~0c": "value->value"} + ) + + +class TestAllow(unittest.TestCase): + def test_an_allowed_pointer_does_not_fail(self) -> None: + allow = [{"pointer": AGENT + "/description", "reason": "fix 5"}] + result = cr.compare(_report(), _report(description="Slow"), allow) + self.assertFalse(result["failed"]) + self.assertEqual(result["changes"][0]["allowed"], "fix 5") + self.assertEqual(result["unused_allow"], []) + + def test_an_allowed_prefix_covers_its_subtree_only(self) -> None: + allow = [{"pointer": AGENT, "reason": "fix"}] + self.assertFalse( + cr.compare(_report(), _report(description="x"), allow)["failed"] + ) + old, new = {"Explorer": 1, "Explore": 1}, {"Explorer": 2, "Explore": 1} + allow = [{"pointer": "/Explore", "reason": "fix"}] + self.assertTrue(cr.compare(old, new, allow)["failed"]) + + def test_an_unused_allow_entry_is_reported(self) -> None: + allow = [{"pointer": "/nowhere", "reason": "stale"}] + self.assertEqual( + cr.compare(_report(), _report(), allow)["unused_allow"], ["/nowhere"] + ) + + +class TestCli(unittest.TestCase): + def setUp(self) -> None: + self.dir = pathlib.Path(tempfile.mkdtemp()) + + def _write(self, name: str, data: object) -> str: + path = self.dir / name + path.write_text(json.dumps(data), encoding="utf-8") + return str(path) + + def _run(self, *argv: str) -> tuple[int, str, str]: + out, err = io.StringIO(), io.StringIO() + with contextlib.redirect_stdout(out), contextlib.redirect_stderr(err): + code = cr.main(list(argv)) + return code, out.getvalue(), err.getvalue() + + def test_exit_codes_and_text_output(self) -> None: + a = self._write("a.json", _report()) + b = self._write("b.json", _report(description="Slow")) + code, out, _ = self._run(a, a) + self.assertEqual(code, 0) + self.assertIn("OK: no disallowed value->value change", out) + code, out, _ = self._run(a, b) + self.assertEqual(code, 1) + self.assertIn("FAIL: 1 value->value change(s) not in --allow", out) + self.assertIn(AGENT + "/description", out) + allow = self._write( + "allow.json", [{"pointer": AGENT + "/description", "reason": "fix"}] + ) + self.assertEqual(self._run(a, b, "--allow", allow)[0], 0) + + def test_json_output(self) -> None: + a = self._write("a.json", _report()) + b = self._write("b.json", _report(description="…")) + code, out, _ = self._run(a, b, "--json") + self.assertEqual(code, 0) + data = json.loads(out) + self.assertEqual(data["counts"]["wrong->unresolved"], 1) + self.assertEqual(data["changes"][0]["new"], "…") + + def test_bad_input_exits_2(self) -> None: + a = self._write("a.json", _report()) + bad = self._write("bad.json", [{"pointer": "/x"}]) + self.assertEqual(self._run(a, a, "--allow", bad)[0], 2) + self.assertEqual(self._run(a, str(self.dir / "missing.json"))[0], 2) + + +if __name__ == "__main__": + unittest.main() diff --git a/plugins/claude-ops/skills/inventory/scripts/test_fixture_parse.py b/plugins/claude-ops/skills/inventory/scripts/test_fixture_parse.py new file mode 100644 index 0000000000..09892a33c8 --- /dev/null +++ b/plugins/claude-ops/skills/inventory/scripts/test_fixture_parse.py @@ -0,0 +1,186 @@ +#!/usr/bin/env python3 +"""Every JavaScript fixture test_inventory.py feeds the reader parses as a +module under acorn. + +A parser-backed reader (#5640) reads only what parses, so a fixture that is +not valid JavaScript tests a shape no real bundle has. The fixtures are +collected by running test_inventory.py with `inventory.build_brace_map` +wrapped: every extraction builds the brace map from its source first, so +the wrapper sees exactly the text each test hands the reader, including +text a test assembles in a loop. + +Fixtures known not to parse are listed in KNOWN_UNPARSEABLE by test id, for +P2 to rewrite; the check fails when a new one appears or a listed one starts +parsing, so the list stays exact. + +Needs `node` and an `acorn` package that `require("acorn")` resolves (set +NODE_PATH to its node_modules directory, e.g. after +`npm install --prefix acorn@8`); otherwise the check skips. + +Run: python3 -m unittest test_fixture_parse +""" + +from __future__ import annotations + +import json +import re +import shutil +import subprocess +import tempfile +import unittest + +import inventory as inv +import test_inventory + +PARSE_JS = r""" +const acorn = require("acorn"); +const fixtures = JSON.parse(require("fs").readFileSync(process.argv[1], "utf8")); +const failed = []; +for (const [i, src] of fixtures.entries()) { + try { + acorn.parse(src, { ecmaVersion: "latest", sourceType: "module" }); + } catch (e) { + failed.push([i, e.message]); + } +} +process.stdout.write(JSON.stringify({ acorn: acorn.version, failed })); +""" + +# Four shapes, all fixture shortcuts no real bundle has: an `export{eo as ...}` +# header with no `eo` declared; `z` padding run straight into the next +# token (`zzz…function f`); the version anchor repeated as adjacent string +# literals (`"2.1.287""2.1.287"`); and a bare object literal as a statement. +KNOWN_UNPARSEABLE: frozenset[str] = frozenset( + { + "TestAgentAndToolIntegrity.test_a_missing_canary_breaks_only_that_lane", + "TestAgentAndToolIntegrity.test_an_empty_lane_is_broken", + "TestAgentAndToolIntegrity.test_both_lanes_ok", + "TestAgentAndToolIntegrity.test_factories_do_not_degrade", + "TestAgentAndToolIntegrity.test_lanes_absent_when_not_extracted", + "TestAgentAndToolIntegrity.test_unresolved_names_and_a_missing_roster_degrade", + "TestBundledWorkflows.test_lane_breaks_on_a_missing_canary_and_others_stand", + "TestBundledWorkflows.test_lane_ok_with_the_canary", + "TestCommandExtraction.test_a_name_bound_to_a_conditional_is_not_resolved", + "TestCommandExtraction.test_shell_builtin_is_not_a_command", + "TestIntegrity.test_canary_missing_breaks_the_builtin_lane_only", + "TestIntegrity.test_degraded_on_unknown_registrar", + "TestIntegrity.test_degraded_when_registrations_exceed_resolved", + "TestIntegrity.test_dynamic_roster_is_an_advisory_not_an_unresolved_name", + "TestIntegrity.test_esm_export_list_feeds_the_registrar_advisory", + "TestIntegrity.test_every_lane_broken_is_broken", + "TestIntegrity.test_low_yield_breaks_the_builtin_lane", + "TestIntegrity.test_missing_plugin_backed_canary_breaks_that_lane", + "TestIntegrity.test_no_skills_breaks_the_bundled_lane_only", + "TestIntegrity.test_ok_when_everything_resolves", + "TestIntegrity.test_registrations_below_the_floor_degrade_the_bundled_lane", + "TestInvocationFieldsAndCollisions.test_a_flag_driven_twin_of_a_constant_field_is_a_collision", + "TestInvocationFieldsAndCollisions.test_a_function_valued_field_reads_as_true_and_flag_driven", + "TestInvocationFieldsAndCollisions.test_invocation_fields_are_read_when_present", + "TestInvocationFieldsAndCollisions.test_the_same_registration_seen_twice_is_not_a_collision", + "TestInvocationFieldsAndCollisions.test_two_registrations_sharing_a_name_are_both_kept", + "TestNameLocality.test_a_binding_after_the_registration_does_not_resolve_it", + "TestNameLocality.test_a_descriptor_member_name_resolves", + "TestNameLocality.test_a_function_whose_name_ends_in_the_registrar_is_not_a_call", + "TestNameLocality.test_a_lone_far_binding_of_a_short_identifier_is_not_resolved", + "TestNameLocality.test_a_long_identifier_bound_far_ahead_resolves", + "TestNameLocality.test_a_loop_over_a_literal_table_is_enumerated", + "TestNameLocality.test_a_loop_registration_is_a_dynamic_roster", + "TestNameLocality.test_a_nearer_non_constant_binding_shadows_a_constant", + "TestNameLocality.test_a_same_identifier_call_without_a_name_is_not_a_registration", + "TestNameLocality.test_a_short_identifier_bound_nearby_resolves", + "TestNameLocality.test_a_template_literal_name_is_a_dynamic_roster", + "TestNameLocality.test_nearest_preceding_binding_wins_over_a_farther_one", + "TestRegistrarRoutes.test_route_is_recorded_in_the_notes", + } +) + + +def collect_fixtures() -> dict[str, set[str]]: + """Each distinct source test_inventory passes the reader, mapped to the + ids of the tests that passed it.""" + seen: dict[str, set[str]] = {} + current = [""] + original = inv.build_brace_map + + def recording(src: str, *args, **kwargs): + seen.setdefault(src, set()).add(current[0]) + return original(src, *args, **kwargs) + + class Result(unittest.TestResult): + def startTest(self, test: unittest.TestCase) -> None: + current[0] = test.id().removeprefix("test_inventory.") + super().startTest(test) + + suite = unittest.defaultTestLoader.loadTestsFromModule(test_inventory) + inv.build_brace_map = recording + try: + result = Result() + suite.run(result) + finally: + inv.build_brace_map = original + if result.errors or result.failures: + raise AssertionError( + "test_inventory must pass before its fixtures are checked: " + + "; ".join(t.id() for t, _ in result.errors + result.failures) + ) + return seen + + +def modules(src: str) -> list[str]: + """`src` split into its modules at each `// @bun` header, the unit a real + bundle is parsed in: concatenated modules repeat top-level names.""" + starts = [0] + [m.start() for m in re.finditer("\n// @bun", src) if m.start()] + return [src[a:b] for a, b in zip(starts, starts[1:] + [len(src)])] + + +def _acorn_skip_reason() -> str | None: + node = shutil.which("node") + if node is None: + return "node is not on PATH" + probe = subprocess.run( + [node, "-e", 'require("acorn")'], capture_output=True, text=True, check=False + ) + if probe.returncode != 0: + return 'node cannot require("acorn"); set NODE_PATH to a node_modules holding acorn@8' + return None + + +def unparseable(fixtures: list[str]) -> tuple[str, list[tuple[int, str]]]: + """Acorn's version, and (index, error) for each fixture that fails.""" + with tempfile.NamedTemporaryFile("w", suffix=".json", encoding="utf-8") as fh: + json.dump(fixtures, fh) + fh.flush() + run = subprocess.run( + ["node", "-e", PARSE_JS, fh.name], + capture_output=True, + text=True, + check=True, + ) + out = json.loads(run.stdout) + return out["acorn"], [tuple(f) for f in out["failed"]] + + +class TestFixturesParse(unittest.TestCase): + def test_every_fixture_parses_as_a_module(self) -> None: + reason = _acorn_skip_reason() + if reason: + self.skipTest(reason) + seen = collect_fixtures() + owners: dict[str, set[str]] = {} + for src, tests in seen.items(): + for module in modules(src): + owners.setdefault(module, set()).update(tests) + fixtures = sorted(owners) + _, failed = unparseable(fixtures) + bad_tests: dict[str, list[str]] = {} + for i, error in failed: + for test in owners[fixtures[i]]: + bad_tests.setdefault(test, []).append(error) + lines = [f"{t}: {'; '.join(e)}" for t, e in sorted(bad_tests.items())] + self.assertEqual( + sorted(bad_tests), sorted(KNOWN_UNPARSEABLE), "\n" + "\n".join(lines) + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/plugins/claude-ops/skills/inventory/scripts/test_reader_findings.py b/plugins/claude-ops/skills/inventory/scripts/test_reader_findings.py new file mode 100644 index 0000000000..6827c482b9 --- /dev/null +++ b/plugins/claude-ops/skills/inventory/scripts/test_reader_findings.py @@ -0,0 +1,108 @@ +#!/usr/bin/env python3 +"""The open wrong-value and unresolved-only findings on #5640, pinned. + +Each test builds a synthetic bundle the way test_inventory.py does and +asserts what JavaScript computes. They are expected failures while the regex +reader stands; a reader fix flips one to an unexpected success, which fails +the run until its `@unittest.expectedFailure` is removed. + +A wrong-value test accepts either the JavaScript result or the reader +declining to read (`partial`): an unresolved value is honest, a wrong +literal is the bug. + +Run: python3 -m unittest test_reader_findings +""" + +from __future__ import annotations + +import unittest + +import inventory as inv +from test_inventory import AGENT_SRC + +# AGENT_SRC binds xt="Edit", yt="Agent". +PROBE = ( + 'var SP={agentType:"spread-probe",whenToUse:"s",source:"built-in",' + 'disallowedTools:[yt,...pY],getSystemPrompt:()=>""};' +) + + +def _probe(prelude: str) -> dict: + src = AGENT_SRC + prelude + PROBE + return inv.extract_builtin_agents(src, inv.build_brace_map(src))[0]["spread-probe"] + + +class TestOpenFindings(unittest.TestCase): + def assert_not_wrong(self, prelude: str, js_value: list[str]) -> None: + rec = _probe(prelude) + if rec["disallowed_tools_source"] != "literal": + return + self.assertEqual(rec["disallowed_tools"], js_value, prelude) + + @unittest.expectedFailure + def test_finding_3_push_mutates_a_spread_array(self) -> None: + """A `push` on the spread binding is not modelled, so the literal + misses the pushed name. + https://github.com/melodic-software/claude-code-plugins/issues/5640#issuecomment-5934412887 + """ + self.assert_not_wrong( + 'var pY=[xt,"Artifact"];pY.push("B");', + ["Agent", "Edit", "Artifact", "B"], + ) + + @unittest.expectedFailure + def test_finding_3_other_mutations_of_a_spread_array(self) -> None: + """The other mutating shapes finding 3 names, plus a call argument + (operator decision: any call argument counts as possible mutation) + and a push inside a called function. + https://github.com/melodic-software/claude-code-plugins/issues/5640#issuecomment-5934412887 + """ + for prelude, js_value in ( + ( + 'var pY=[xt,"Artifact"];pY.unshift("B");', + ["Agent", "B", "Edit", "Artifact"], + ), + ('var pY=[xt,"Artifact"];pY.splice(0,1);', ["Agent", "Artifact"]), + ('var pY=[xt,"Artifact"];pY.length=0;', ["Agent"]), + ( + 'var pY=[xt,"Artifact"];function g(a){a.push("B")}g(pY);', + ["Agent", "Edit", "Artifact", "B"], + ), + ( + 'var pY=[xt,"Artifact"];function g(){pY.push("B")}g();', + ["Agent", "Edit", "Artifact", "B"], + ), + ): + with self.subTest(prelude=prelude): + self.assert_not_wrong(prelude, js_value) + + @unittest.expectedFailure + def test_finding_4_an_arrow_earlier_in_the_statement_leaves_a_spread_partial( + self, + ) -> None: + """Unresolved-only: `=>` anywhere earlier in the statement counts as + the binding sitting in an arrow body, so `...pY` stays partial though + JavaScript has a constant list. Pinned as expected-partial today. + https://github.com/melodic-software/claude-code-plugins/issues/5640#issuecomment-5934863409 + """ + rec = _probe('var f=()=>0,pY=[xt,"Artifact"];') + self.assertEqual( + (rec["disallowed_tools"], rec["disallowed_tools_source"]), + (["Agent", "Edit", "Artifact"], "literal"), + ) + + @unittest.expectedFailure + def test_finding_5_declaration_text_in_a_string_suppresses_a_write(self) -> None: + """`_written_elsewhere` scans raw source for shadowing declarations, + so `let pY` inside a string or comment hides the function's write to + the outer `pY`. + https://github.com/melodic-software/claude-code-plugins/issues/5640#issuecomment-5936100666 + """ + for inner in ('"let pY";', "/*let pY*/", "`let pY`;"): + prelude = 'var pY=[xt,"Artifact"];function f(){' + inner + 'pY=["B"]}f();' + with self.subTest(inner=inner): + self.assert_not_wrong(prelude, ["Agent", "B"]) + + +if __name__ == "__main__": + unittest.main() From fc6542e76d059b461c96be478bba33e6334a05b0 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Thu, 1 Oct 2026 18:16:11 -0400 Subject: [PATCH 2/7] test(claude-ops): pin each #5640 finding shape in its own expected failure Grouped subtests under one expectedFailure stop counting at the first failing case, so a fix for some shapes stayed hidden. Each finding 3 and finding 5 shape is now its own test: 10 expected failures. Co-Authored-By: Claude Opus 5.5 --- .../inventory/scripts/test_reader_findings.py | 101 +++++++++++------- 1 file changed, 61 insertions(+), 40 deletions(-) diff --git a/plugins/claude-ops/skills/inventory/scripts/test_reader_findings.py b/plugins/claude-ops/skills/inventory/scripts/test_reader_findings.py index 6827c482b9..bdefb586d0 100644 --- a/plugins/claude-ops/skills/inventory/scripts/test_reader_findings.py +++ b/plugins/claude-ops/skills/inventory/scripts/test_reader_findings.py @@ -39,42 +39,52 @@ def assert_not_wrong(self, prelude: str, js_value: list[str]) -> None: return self.assertEqual(rec["disallowed_tools"], js_value, prelude) + # Finding 3: mutation of the spread array is not modelled, so the literal + # keeps the initializer. The call-argument case follows the operator + # decision that any call argument counts as possible mutation. + @unittest.expectedFailure - def test_finding_3_push_mutates_a_spread_array(self) -> None: - """A `push` on the spread binding is not modelled, so the literal - misses the pushed name. - https://github.com/melodic-software/claude-code-plugins/issues/5640#issuecomment-5934412887 - """ + def test_finding_3_push(self) -> None: + """https://github.com/melodic-software/claude-code-plugins/issues/5640#issuecomment-5934412887""" + self.assert_not_wrong( + 'var pY=[xt,"Artifact"];pY.push("B");', ["Agent", "Edit", "Artifact", "B"] + ) + + @unittest.expectedFailure + def test_finding_3_unshift(self) -> None: + """https://github.com/melodic-software/claude-code-plugins/issues/5640#issuecomment-5934412887""" + self.assert_not_wrong( + 'var pY=[xt,"Artifact"];pY.unshift("B");', + ["Agent", "B", "Edit", "Artifact"], + ) + + @unittest.expectedFailure + def test_finding_3_splice(self) -> None: + """https://github.com/melodic-software/claude-code-plugins/issues/5640#issuecomment-5934412887""" self.assert_not_wrong( - 'var pY=[xt,"Artifact"];pY.push("B");', + 'var pY=[xt,"Artifact"];pY.splice(0,1);', ["Agent", "Artifact"] + ) + + @unittest.expectedFailure + def test_finding_3_length_assignment(self) -> None: + """https://github.com/melodic-software/claude-code-plugins/issues/5640#issuecomment-5934412887""" + self.assert_not_wrong('var pY=[xt,"Artifact"];pY.length=0;', ["Agent"]) + + @unittest.expectedFailure + def test_finding_3_call_argument(self) -> None: + """https://github.com/melodic-software/claude-code-plugins/issues/5640#issuecomment-5934412887""" + self.assert_not_wrong( + 'var pY=[xt,"Artifact"];function g(a){a.push("B")}g(pY);', ["Agent", "Edit", "Artifact", "B"], ) @unittest.expectedFailure - def test_finding_3_other_mutations_of_a_spread_array(self) -> None: - """The other mutating shapes finding 3 names, plus a call argument - (operator decision: any call argument counts as possible mutation) - and a push inside a called function. - https://github.com/melodic-software/claude-code-plugins/issues/5640#issuecomment-5934412887 - """ - for prelude, js_value in ( - ( - 'var pY=[xt,"Artifact"];pY.unshift("B");', - ["Agent", "B", "Edit", "Artifact"], - ), - ('var pY=[xt,"Artifact"];pY.splice(0,1);', ["Agent", "Artifact"]), - ('var pY=[xt,"Artifact"];pY.length=0;', ["Agent"]), - ( - 'var pY=[xt,"Artifact"];function g(a){a.push("B")}g(pY);', - ["Agent", "Edit", "Artifact", "B"], - ), - ( - 'var pY=[xt,"Artifact"];function g(){pY.push("B")}g();', - ["Agent", "Edit", "Artifact", "B"], - ), - ): - with self.subTest(prelude=prelude): - self.assert_not_wrong(prelude, js_value) + def test_finding_3_push_in_a_called_function(self) -> None: + """https://github.com/melodic-software/claude-code-plugins/issues/5640#issuecomment-5934412887""" + self.assert_not_wrong( + 'var pY=[xt,"Artifact"];function g(){pY.push("B")}g();', + ["Agent", "Edit", "Artifact", "B"], + ) @unittest.expectedFailure def test_finding_4_an_arrow_earlier_in_the_statement_leaves_a_spread_partial( @@ -91,17 +101,28 @@ def test_finding_4_an_arrow_earlier_in_the_statement_leaves_a_spread_partial( (["Agent", "Edit", "Artifact"], "literal"), ) + # Finding 5: `_written_elsewhere` scans raw source for shadowing + # declarations, so `let pY` inside quoted text hides the function's write + # to the outer `pY`. + + def assert_text_does_not_shadow(self, inner: str) -> None: + prelude = 'var pY=[xt,"Artifact"];function f(){' + inner + 'pY=["B"]}f();' + self.assert_not_wrong(prelude, ["Agent", "B"]) + @unittest.expectedFailure - def test_finding_5_declaration_text_in_a_string_suppresses_a_write(self) -> None: - """`_written_elsewhere` scans raw source for shadowing declarations, - so `let pY` inside a string or comment hides the function's write to - the outer `pY`. - https://github.com/melodic-software/claude-code-plugins/issues/5640#issuecomment-5936100666 - """ - for inner in ('"let pY";', "/*let pY*/", "`let pY`;"): - prelude = 'var pY=[xt,"Artifact"];function f(){' + inner + 'pY=["B"]}f();' - with self.subTest(inner=inner): - self.assert_not_wrong(prelude, ["Agent", "B"]) + def test_finding_5_declaration_text_in_a_string(self) -> None: + """https://github.com/melodic-software/claude-code-plugins/issues/5640#issuecomment-5936100666""" + self.assert_text_does_not_shadow('"let pY";') + + @unittest.expectedFailure + def test_finding_5_declaration_text_in_a_comment(self) -> None: + """https://github.com/melodic-software/claude-code-plugins/issues/5640#issuecomment-5936100666""" + self.assert_text_does_not_shadow("/*let pY*/") + + @unittest.expectedFailure + def test_finding_5_declaration_text_in_a_template(self) -> None: + """https://github.com/melodic-software/claude-code-plugins/issues/5640#issuecomment-5936100666""" + self.assert_text_does_not_shadow("`let pY`;") if __name__ == "__main__": From a57bc5f0e93bb794a854cbee6e23ead56b1060b3 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Thu, 1 Oct 2026 18:25:30 -0400 Subject: [PATCH 3/7] fix(claude-ops): mark every covering allow entry used and read an ellipsis inside a list element MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A narrower --allow entry under a broader one was reported unused because the match loop stopped at the first covering entry. A list of names whose element holds the `…` placeholder (description_variants) read as resolved, so its resolution classified as value->value instead of unresolved->resolved. Co-Authored-By: Claude Opus 5.5 --- .../skills/inventory/scripts/compare_reports.py | 11 +++++------ .../inventory/scripts/test_compare_reports.py | 16 ++++++++++++++++ 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/plugins/claude-ops/skills/inventory/scripts/compare_reports.py b/plugins/claude-ops/skills/inventory/scripts/compare_reports.py index 85f659d5d2..f943ad53db 100644 --- a/plugins/claude-ops/skills/inventory/scripts/compare_reports.py +++ b/plugins/claude-ops/skills/inventory/scripts/compare_reports.py @@ -65,7 +65,7 @@ def _unresolved(value: Any, source: Any) -> bool: if source in UNRESOLVED_SOURCES: return True if isinstance(value, list): - return ELLIPSIS in value + return any(isinstance(v, str) and ELLIPSIS in v for v in value) return isinstance(value, str) and ELLIPSIS in value @@ -150,11 +150,10 @@ def compare(old: Any, new: Any, allow: list[dict] | None = None) -> dict: for rec in changes: if rec["class"] != "value->value": continue - for entry in allow: - if _covers(entry["pointer"], rec["pointer"]): - rec["allowed"] = entry.get("reason", "") - used.add(entry["pointer"]) - break + covering = [e for e in allow if _covers(e["pointer"], rec["pointer"])] + if covering: + rec["allowed"] = covering[0]["reason"] + used.update(e["pointer"] for e in covering) counts = {c: sum(1 for r in changes if r["class"] == c) for c in CLASSES} disallowed = [ r for r in changes if r["class"] == "value->value" and "allowed" not in r diff --git a/plugins/claude-ops/skills/inventory/scripts/test_compare_reports.py b/plugins/claude-ops/skills/inventory/scripts/test_compare_reports.py index 65a147edf9..b81ab860cf 100644 --- a/plugins/claude-ops/skills/inventory/scripts/test_compare_reports.py +++ b/plugins/claude-ops/skills/inventory/scripts/test_compare_reports.py @@ -102,6 +102,13 @@ def test_an_ellipsis_element_marks_a_list_unresolved(self) -> None: {AGENT + "/disallowed_tools": "unresolved->resolved"}, ) + def test_an_element_holding_an_ellipsis_marks_a_list_unresolved(self) -> None: + old = {"description_variants": ["Use … here", "Short"]} + new = {"description_variants": ["Use Grep here", "Short"]} + self.assertEqual( + _classes(old, new), {"/description_variants": "unresolved->resolved"} + ) + def test_two_different_partial_values_are_value_to_value(self) -> None: old = _report(disallowed_tools=["A"], disallowed_tools_source="partial") new = _report(disallowed_tools=["B"], disallowed_tools_source="partial") @@ -165,6 +172,15 @@ def test_an_allowed_prefix_covers_its_subtree_only(self) -> None: allow = [{"pointer": "/Explore", "reason": "fix"}] self.assertTrue(cr.compare(old, new, allow)["failed"]) + def test_a_narrower_entry_under_a_broader_one_is_used(self) -> None: + allow = [ + {"pointer": AGENT, "reason": "broad"}, + {"pointer": AGENT + "/description", "reason": "narrow"}, + ] + result = cr.compare(_report(), _report(description="x"), allow) + self.assertFalse(result["failed"]) + self.assertEqual(result["unused_allow"], []) + def test_an_unused_allow_entry_is_reported(self) -> None: allow = [{"pointer": "/nowhere", "reason": "stale"}] self.assertEqual( From 4834b8e21d629796dbb4d425b39e21e2e1792541 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Thu, 1 Oct 2026 18:49:40 -0400 Subject: [PATCH 4/7] fix(claude-ops): match allow pointers as written and track unparseable fixtures per module An --allow entry was right-stripped of `/` before matching, so `/a/` (the empty-key child of `/a`) covered all of `/a` and `/` covered the whole report. Entries now match RFC 6901 pointers as written. KNOWN_UNPARSEABLE listed test ids, so a module that broke or was repaired inside an already-listed test left the comparison unchanged. Keys are now `#`. Co-Authored-By: Claude Opus 5.5 --- .../inventory/scripts/compare_reports.py | 4 +- .../inventory/scripts/test_compare_reports.py | 11 +++ .../inventory/scripts/test_fixture_parse.py | 96 ++++++++++--------- 3 files changed, 64 insertions(+), 47 deletions(-) diff --git a/plugins/claude-ops/skills/inventory/scripts/compare_reports.py b/plugins/claude-ops/skills/inventory/scripts/compare_reports.py index f943ad53db..e1bc905cd2 100644 --- a/plugins/claude-ops/skills/inventory/scripts/compare_reports.py +++ b/plugins/claude-ops/skills/inventory/scripts/compare_reports.py @@ -137,7 +137,9 @@ def _change( def _covers(allowed: str, pointer: str) -> bool: - return pointer == allowed or pointer.startswith(allowed.rstrip("/") + "/") + # RFC 6901: `/a/` names the empty-key child of `/a`, and `/` is not the + # whole document, so the entry is matched as written. + return pointer == allowed or pointer.startswith(allowed + "/") def compare(old: Any, new: Any, allow: list[dict] | None = None) -> dict: diff --git a/plugins/claude-ops/skills/inventory/scripts/test_compare_reports.py b/plugins/claude-ops/skills/inventory/scripts/test_compare_reports.py index b81ab860cf..6845e3351c 100644 --- a/plugins/claude-ops/skills/inventory/scripts/test_compare_reports.py +++ b/plugins/claude-ops/skills/inventory/scripts/test_compare_reports.py @@ -172,6 +172,17 @@ def test_an_allowed_prefix_covers_its_subtree_only(self) -> None: allow = [{"pointer": "/Explore", "reason": "fix"}] self.assertTrue(cr.compare(old, new, allow)["failed"]) + def test_allow_pointers_keep_rfc_6901_meaning(self) -> None: + old, new = {"a": {"": 1, "b": 1}}, {"a": {"": 2, "b": 2}} + result = cr.compare(old, new, [{"pointer": "/a/", "reason": "empty key"}]) + self.assertEqual( + {r["pointer"]: "allowed" in r for r in result["changes"]}, + {"/a/": True, "/a/b": False}, + ) + result = cr.compare(old, new, [{"pointer": "/", "reason": "not all"}]) + self.assertEqual(result["disallowed"], 2) + self.assertEqual(result["unused_allow"], ["/"]) + def test_a_narrower_entry_under_a_broader_one_is_used(self) -> None: allow = [ {"pointer": AGENT, "reason": "broad"}, diff --git a/plugins/claude-ops/skills/inventory/scripts/test_fixture_parse.py b/plugins/claude-ops/skills/inventory/scripts/test_fixture_parse.py index 09892a33c8..94bb3cbc1c 100644 --- a/plugins/claude-ops/skills/inventory/scripts/test_fixture_parse.py +++ b/plugins/claude-ops/skills/inventory/scripts/test_fixture_parse.py @@ -9,9 +9,11 @@ the wrapper sees exactly the text each test hands the reader, including text a test assembles in a loop. -Fixtures known not to parse are listed in KNOWN_UNPARSEABLE by test id, for -P2 to rewrite; the check fails when a new one appears or a listed one starts -parsing, so the list stays exact. +Modules known not to parse are listed in KNOWN_UNPARSEABLE as +`#`, for P2 to rewrite. The +check fails when a new key appears or a listed one is gone, so a module that +breaks or is repaired inside an already-listed test changes the comparison; +the failure message prints every current key. Needs `node` and an `acorn` package that `require("acorn")` resolves (set NODE_PATH to its node_modules directory, e.g. after @@ -22,6 +24,7 @@ from __future__ import annotations +import hashlib import json import re import shutil @@ -52,45 +55,45 @@ # literals (`"2.1.287""2.1.287"`); and a bare object literal as a statement. KNOWN_UNPARSEABLE: frozenset[str] = frozenset( { - "TestAgentAndToolIntegrity.test_a_missing_canary_breaks_only_that_lane", - "TestAgentAndToolIntegrity.test_an_empty_lane_is_broken", - "TestAgentAndToolIntegrity.test_both_lanes_ok", - "TestAgentAndToolIntegrity.test_factories_do_not_degrade", - "TestAgentAndToolIntegrity.test_lanes_absent_when_not_extracted", - "TestAgentAndToolIntegrity.test_unresolved_names_and_a_missing_roster_degrade", - "TestBundledWorkflows.test_lane_breaks_on_a_missing_canary_and_others_stand", - "TestBundledWorkflows.test_lane_ok_with_the_canary", - "TestCommandExtraction.test_a_name_bound_to_a_conditional_is_not_resolved", - "TestCommandExtraction.test_shell_builtin_is_not_a_command", - "TestIntegrity.test_canary_missing_breaks_the_builtin_lane_only", - "TestIntegrity.test_degraded_on_unknown_registrar", - "TestIntegrity.test_degraded_when_registrations_exceed_resolved", - "TestIntegrity.test_dynamic_roster_is_an_advisory_not_an_unresolved_name", - "TestIntegrity.test_esm_export_list_feeds_the_registrar_advisory", - "TestIntegrity.test_every_lane_broken_is_broken", - "TestIntegrity.test_low_yield_breaks_the_builtin_lane", - "TestIntegrity.test_missing_plugin_backed_canary_breaks_that_lane", - "TestIntegrity.test_no_skills_breaks_the_bundled_lane_only", - "TestIntegrity.test_ok_when_everything_resolves", - "TestIntegrity.test_registrations_below_the_floor_degrade_the_bundled_lane", - "TestInvocationFieldsAndCollisions.test_a_flag_driven_twin_of_a_constant_field_is_a_collision", - "TestInvocationFieldsAndCollisions.test_a_function_valued_field_reads_as_true_and_flag_driven", - "TestInvocationFieldsAndCollisions.test_invocation_fields_are_read_when_present", - "TestInvocationFieldsAndCollisions.test_the_same_registration_seen_twice_is_not_a_collision", - "TestInvocationFieldsAndCollisions.test_two_registrations_sharing_a_name_are_both_kept", - "TestNameLocality.test_a_binding_after_the_registration_does_not_resolve_it", - "TestNameLocality.test_a_descriptor_member_name_resolves", - "TestNameLocality.test_a_function_whose_name_ends_in_the_registrar_is_not_a_call", - "TestNameLocality.test_a_lone_far_binding_of_a_short_identifier_is_not_resolved", - "TestNameLocality.test_a_long_identifier_bound_far_ahead_resolves", - "TestNameLocality.test_a_loop_over_a_literal_table_is_enumerated", - "TestNameLocality.test_a_loop_registration_is_a_dynamic_roster", - "TestNameLocality.test_a_nearer_non_constant_binding_shadows_a_constant", - "TestNameLocality.test_a_same_identifier_call_without_a_name_is_not_a_registration", - "TestNameLocality.test_a_short_identifier_bound_nearby_resolves", - "TestNameLocality.test_a_template_literal_name_is_a_dynamic_roster", - "TestNameLocality.test_nearest_preceding_binding_wins_over_a_farther_one", - "TestRegistrarRoutes.test_route_is_recorded_in_the_notes", + "TestAgentAndToolIntegrity.test_a_missing_canary_breaks_only_that_lane#b5868a7055b1", + "TestAgentAndToolIntegrity.test_an_empty_lane_is_broken#b5868a7055b1", + "TestAgentAndToolIntegrity.test_both_lanes_ok#b5868a7055b1", + "TestAgentAndToolIntegrity.test_factories_do_not_degrade#b5868a7055b1", + "TestAgentAndToolIntegrity.test_lanes_absent_when_not_extracted#b5868a7055b1", + "TestAgentAndToolIntegrity.test_unresolved_names_and_a_missing_roster_degrade#b5868a7055b1", + "TestBundledWorkflows.test_lane_breaks_on_a_missing_canary_and_others_stand#b5868a7055b1", + "TestBundledWorkflows.test_lane_ok_with_the_canary#b5868a7055b1", + "TestCommandExtraction.test_a_name_bound_to_a_conditional_is_not_resolved#85ce46d58655", + "TestCommandExtraction.test_shell_builtin_is_not_a_command#fec1bb3c5c58", + "TestIntegrity.test_canary_missing_breaks_the_builtin_lane_only#55213d5c1103", + "TestIntegrity.test_degraded_on_unknown_registrar#3618f81b8f33", + "TestIntegrity.test_degraded_when_registrations_exceed_resolved#b5868a7055b1", + "TestIntegrity.test_dynamic_roster_is_an_advisory_not_an_unresolved_name#b5868a7055b1", + "TestIntegrity.test_esm_export_list_feeds_the_registrar_advisory#e5c5402a79c1", + "TestIntegrity.test_every_lane_broken_is_broken#55213d5c1103", + "TestIntegrity.test_low_yield_breaks_the_builtin_lane#b88b934b8a7b", + "TestIntegrity.test_missing_plugin_backed_canary_breaks_that_lane#b5868a7055b1", + "TestIntegrity.test_no_skills_breaks_the_bundled_lane_only#b5868a7055b1", + "TestIntegrity.test_ok_when_everything_resolves#b5868a7055b1", + "TestIntegrity.test_registrations_below_the_floor_degrade_the_bundled_lane#b5868a7055b1", + "TestInvocationFieldsAndCollisions.test_a_flag_driven_twin_of_a_constant_field_is_a_collision#e502c2ef5e4a", + "TestInvocationFieldsAndCollisions.test_a_function_valued_field_reads_as_true_and_flag_driven#44b9fec2dc50", + "TestInvocationFieldsAndCollisions.test_invocation_fields_are_read_when_present#ce769dbf090c", + "TestInvocationFieldsAndCollisions.test_the_same_registration_seen_twice_is_not_a_collision#0d36306e877f", + "TestInvocationFieldsAndCollisions.test_two_registrations_sharing_a_name_are_both_kept#ce69f83385dc", + "TestNameLocality.test_a_binding_after_the_registration_does_not_resolve_it#9a6e9362fb32", + "TestNameLocality.test_a_descriptor_member_name_resolves#55be871fc0c5", + "TestNameLocality.test_a_function_whose_name_ends_in_the_registrar_is_not_a_call#cafe65dd1b41", + "TestNameLocality.test_a_lone_far_binding_of_a_short_identifier_is_not_resolved#0c5d756b8eac", + "TestNameLocality.test_a_long_identifier_bound_far_ahead_resolves#41443f4f85cd", + "TestNameLocality.test_a_loop_over_a_literal_table_is_enumerated#8fd1aba220f9", + "TestNameLocality.test_a_loop_registration_is_a_dynamic_roster#f666325cf42d", + "TestNameLocality.test_a_nearer_non_constant_binding_shadows_a_constant#48defb31bfb4", + "TestNameLocality.test_a_same_identifier_call_without_a_name_is_not_a_registration#9c35b1ff9865", + "TestNameLocality.test_a_short_identifier_bound_nearby_resolves#553470597e43", + "TestNameLocality.test_a_template_literal_name_is_a_dynamic_roster#1dd6d1208661", + "TestNameLocality.test_nearest_preceding_binding_wins_over_a_farther_one#a68e67dc1cbc", + "TestRegistrarRoutes.test_route_is_recorded_in_the_notes#700bc7dd62e2", } ) @@ -172,13 +175,14 @@ def test_every_fixture_parses_as_a_module(self) -> None: owners.setdefault(module, set()).update(tests) fixtures = sorted(owners) _, failed = unparseable(fixtures) - bad_tests: dict[str, list[str]] = {} + bad: dict[str, str] = {} for i, error in failed: + digest = hashlib.sha256(fixtures[i].encode()).hexdigest()[:12] for test in owners[fixtures[i]]: - bad_tests.setdefault(test, []).append(error) - lines = [f"{t}: {'; '.join(e)}" for t, e in sorted(bad_tests.items())] + bad[f"{test}#{digest}"] = error + lines = [f'"{k}", # {e}' for k, e in sorted(bad.items())] self.assertEqual( - sorted(bad_tests), sorted(KNOWN_UNPARSEABLE), "\n" + "\n".join(lines) + sorted(bad), sorted(KNOWN_UNPARSEABLE), "\n" + "\n".join(lines) ) From 7f03a392d1058ed73d87fa12fc04baaac6ccb886 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Thu, 1 Oct 2026 21:54:41 -0400 Subject: [PATCH 5/7] test(claude-ops): name inventory.py in the reader suites so affected-tests selects them test_reader_findings.py and test_fixture_parse.py reach the reader through `import inventory`, so they never spelled the basename and scripts/affected-tests.sh (R3, a suite that names the file) skipped them on a reader-only change. Each module docstring now names inventory.py. Co-Authored-By: Claude Opus 5.5 --- .../skills/inventory/scripts/test_fixture_parse.py | 5 +++-- .../skills/inventory/scripts/test_reader_findings.py | 4 +++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/plugins/claude-ops/skills/inventory/scripts/test_fixture_parse.py b/plugins/claude-ops/skills/inventory/scripts/test_fixture_parse.py index 195e07eabe..86b751393b 100644 --- a/plugins/claude-ops/skills/inventory/scripts/test_fixture_parse.py +++ b/plugins/claude-ops/skills/inventory/scripts/test_fixture_parse.py @@ -1,6 +1,7 @@ #!/usr/bin/env python3 -"""Every JavaScript fixture test_inventory.py feeds the reader parses as a -module under acorn. +"""Every JavaScript fixture test_inventory.py feeds the reader in +inventory.py parses as a module under acorn. Naming inventory.py here is +what makes scripts/affected-tests.sh select this suite when it changes. A parser-backed reader (#5640) reads only what parses, so a fixture that is not valid JavaScript tests a shape no real bundle has. The fixtures are diff --git a/plugins/claude-ops/skills/inventory/scripts/test_reader_findings.py b/plugins/claude-ops/skills/inventory/scripts/test_reader_findings.py index bdefb586d0..97e8d5b849 100644 --- a/plugins/claude-ops/skills/inventory/scripts/test_reader_findings.py +++ b/plugins/claude-ops/skills/inventory/scripts/test_reader_findings.py @@ -1,5 +1,7 @@ #!/usr/bin/env python3 -"""The open wrong-value and unresolved-only findings on #5640, pinned. +"""The open wrong-value and unresolved-only findings on #5640 in the bundle +reader of inventory.py, pinned. Naming inventory.py here is what makes +scripts/affected-tests.sh select this suite when the reader changes. Each test builds a synthetic bundle the way test_inventory.py does and asserts what JavaScript computes. They are expected failures while the regex From fbd67773401c5f621208d095a642b22731456aa3 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Thu, 1 Oct 2026 23:07:36 -0400 Subject: [PATCH 6/7] fix(harness-ops): mark the reader harness scripts executable Co-Authored-By: Claude Opus 5.5 --- plugins/harness-ops/skills/inventory/scripts/compare_reports.py | 0 .../harness-ops/skills/inventory/scripts/test_compare_reports.py | 0 .../harness-ops/skills/inventory/scripts/test_fixture_parse.py | 0 .../harness-ops/skills/inventory/scripts/test_reader_findings.py | 0 4 files changed, 0 insertions(+), 0 deletions(-) mode change 100644 => 100755 plugins/harness-ops/skills/inventory/scripts/compare_reports.py mode change 100644 => 100755 plugins/harness-ops/skills/inventory/scripts/test_compare_reports.py mode change 100644 => 100755 plugins/harness-ops/skills/inventory/scripts/test_fixture_parse.py mode change 100644 => 100755 plugins/harness-ops/skills/inventory/scripts/test_reader_findings.py diff --git a/plugins/harness-ops/skills/inventory/scripts/compare_reports.py b/plugins/harness-ops/skills/inventory/scripts/compare_reports.py old mode 100644 new mode 100755 diff --git a/plugins/harness-ops/skills/inventory/scripts/test_compare_reports.py b/plugins/harness-ops/skills/inventory/scripts/test_compare_reports.py old mode 100644 new mode 100755 diff --git a/plugins/harness-ops/skills/inventory/scripts/test_fixture_parse.py b/plugins/harness-ops/skills/inventory/scripts/test_fixture_parse.py old mode 100644 new mode 100755 diff --git a/plugins/harness-ops/skills/inventory/scripts/test_reader_findings.py b/plugins/harness-ops/skills/inventory/scripts/test_reader_findings.py old mode 100644 new mode 100755 From 00e9c9e9b34ff8a5c5699f74866e7da7dc2b83e8 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Thu, 1 Oct 2026 23:21:53 -0400 Subject: [PATCH 7/7] fix(harness-ops): spell unparsable and modeled as the typos check expects KNOWN_UNPARSEABLE becomes KNOWN_UNPARSABLE and unparseable() becomes unparsable(); "modelled" becomes "modeled". Co-Authored-By: Claude Opus 5.5 --- .../skills/inventory/scripts/test_fixture_parse.py | 12 +++++------- .../skills/inventory/scripts/test_reader_findings.py | 2 +- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/plugins/harness-ops/skills/inventory/scripts/test_fixture_parse.py b/plugins/harness-ops/skills/inventory/scripts/test_fixture_parse.py index 86b751393b..8d913fae9f 100755 --- a/plugins/harness-ops/skills/inventory/scripts/test_fixture_parse.py +++ b/plugins/harness-ops/skills/inventory/scripts/test_fixture_parse.py @@ -10,7 +10,7 @@ the wrapper sees exactly the text each test hands the reader, including text a test assembles in a loop. -Modules known not to parse are listed in KNOWN_UNPARSEABLE as +Modules known not to parse are listed in KNOWN_UNPARSABLE as `#`, for P2 to rewrite. The check fails when a new key appears or a listed one is gone, so a module that breaks or is repaired inside an already-listed test changes the comparison; @@ -54,7 +54,7 @@ # header with no `eo` declared; `z` padding run straight into the next # token (`zzz…function f`); the version anchor repeated as adjacent string # literals (`"2.1.287""2.1.287"`); and a bare object literal as a statement. -KNOWN_UNPARSEABLE: frozenset[str] = frozenset( +KNOWN_UNPARSABLE: frozenset[str] = frozenset( { "TestAgentAndToolIntegrity.test_a_missing_canary_breaks_only_that_lane#b5868a7055b1", "TestAgentAndToolIntegrity.test_an_empty_lane_is_broken#b5868a7055b1", @@ -157,7 +157,7 @@ def _acorn_skip_reason() -> str | None: return None -def unparseable(fixtures: list[str]) -> tuple[str, list[tuple[int, str]]]: +def unparsable(fixtures: list[str]) -> tuple[str, list[tuple[int, str]]]: """Acorn's version, and (index, error) for each fixture that fails.""" with tempfile.NamedTemporaryFile("w", suffix=".json", encoding="utf-8") as fh: json.dump(fixtures, fh) @@ -183,16 +183,14 @@ def test_every_fixture_parses_as_a_module(self) -> None: for module in modules(src): owners.setdefault(module, set()).update(tests) fixtures = sorted(owners) - _, failed = unparseable(fixtures) + _, failed = unparsable(fixtures) bad: dict[str, str] = {} for i, error in failed: digest = hashlib.sha256(fixtures[i].encode()).hexdigest()[:12] for test in owners[fixtures[i]]: bad[f"{test}#{digest}"] = error lines = [f'"{k}", # {e}' for k, e in sorted(bad.items())] - self.assertEqual( - sorted(bad), sorted(KNOWN_UNPARSEABLE), "\n" + "\n".join(lines) - ) + self.assertEqual(sorted(bad), sorted(KNOWN_UNPARSABLE), "\n" + "\n".join(lines)) if __name__ == "__main__": diff --git a/plugins/harness-ops/skills/inventory/scripts/test_reader_findings.py b/plugins/harness-ops/skills/inventory/scripts/test_reader_findings.py index c9171f76a2..d7c9458996 100755 --- a/plugins/harness-ops/skills/inventory/scripts/test_reader_findings.py +++ b/plugins/harness-ops/skills/inventory/scripts/test_reader_findings.py @@ -47,7 +47,7 @@ def assert_still_open( "#5640 finding is fixed, so replace this pin with that result.", ) - # Finding 3: mutation of the spread array is not modelled, so the literal + # Finding 3: mutation of the spread array is not modeled, so the literal # keeps the initializer. The call-argument case follows the operator # decision that any call argument counts as possible mutation.