From 8de01832417db59da2d6a33077ccad601f85da01 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Wed, 12 Aug 2026 00:26:30 -0400 Subject: [PATCH 1/2] fix(claude-ops): address inventory review findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Six findings from the automated review, all real: - **Bundled-skill fields could bleed from the next registration.** The extractor used a 4000-char window around each registration — the exact failure mode build_brace_map exists to prevent for commands, and one this skill's own reference doc names as the thing not to do. A registration omitting a description adopted the following one's. Fields are now bound to their own literal via the brace map, and an unmatched brace is counted and surfaced rather than silently skipped. - **Manifest-declared component paths were ignored.** PLUGIN_COMPONENTS carried a manifest key per component and a comment claiming the manifest is read before the tree; nothing read it. A declared path replaces the default directory, so scanning defaults regardless reported components a plugin does not ship. Dotted keys resolve the experimental block. - **Installed plugins were never read.** Only marketplace catalogs were scanned, so a plugin installed from a marketplace that is not cached was invisible. disk.installed_plugins now walks the plugin cache; catalog, installed, and enabled are three distinct sets and the skill says so. - **Project scope was missing.** A project's .claude tree contributes skills, agents, and hooks that no machine-scope scan sees. Added, with --project-dir defaulting to cwd. Wired hook events are reported, never hook scripts on disk, which would repeat the present-versus-active error. - **--self-check lost its diagnostic when no binary was found.** pick_binary stores its explanation under "reason"; the self-check path read only "error" and printed a generic message. It now reads both. - **An unreadable CLI version passed silently.** It is itself a drift signal, so it degrades the verdict rather than skipping the comparison. Tests grow 25 to 31, covering field-binding non-bleed, manifest path resolution including the dotted and array forms, and the version-unknown advisory. One existing fixture needed a version literal — it was asserting ok against a build the new check correctly calls degraded. Co-Authored-By: Claude Opus 5 (1M context) --- plugins/claude-ops/skills/inventory/SKILL.md | 16 +- .../skills/inventory/scripts/inventory.py | 320 ++++++++---------- .../inventory/scripts/test_inventory.py | 74 +++- 3 files changed, 233 insertions(+), 177 deletions(-) diff --git a/plugins/claude-ops/skills/inventory/SKILL.md b/plugins/claude-ops/skills/inventory/SKILL.md index 2999afd7ca..88a35d3d14 100644 --- a/plugins/claude-ops/skills/inventory/SKILL.md +++ b/plugins/claude-ops/skills/inventory/SKILL.md @@ -91,7 +91,11 @@ One per line, alphabetical, with aliases and hidden/gated markers. One per line, alphabetical. ## Plugin components -Grouped by marketplace, then plugin, with a component-type breakdown. +Installed plugins grouped by marketplace, then plugin, with a component-type breakdown. +Catalog-only entries — offered by a cached marketplace but not installed — are listed separately. + +## Project scope +Skills, agents, and wired hook events from the current project's `.claude` tree, when present. ## Provenance Which source produced which section, and anything the run could not resolve. @@ -119,8 +123,14 @@ hooks live in its own manifest. Report the map as read and route the verdict to alongside `resolved`. When they differ, some registration used a dynamically computed name; say so rather than reporting the smaller number as complete. -**A marketplace checkout is not an installation.** Plugins under a cached marketplace are a catalog -of what is *available*. Only `enabledPlugins` says what loads. +**A marketplace checkout is not an installation, and neither is enablement.** Three different sets: +a cached marketplace is a catalog of what is *available*, `disk.installed_plugins` is what is +*present locally*, and `enabledPlugins` governs what *loads*. They routinely disagree — report the +one the question is actually about, and say which you used. + +**A hook script on disk is not a wired hook.** Project scope reports hook *events* declared in +settings, not files sitting in a `.claude/hooks/` directory. A script nothing references is dead +weight, and listing it as a hook repeats the same present-versus-active error. ## How the binary read works diff --git a/plugins/claude-ops/skills/inventory/scripts/inventory.py b/plugins/claude-ops/skills/inventory/scripts/inventory.py index 2a25d4fa13..e9ef516c6f 100755 --- a/plugins/claude-ops/skills/inventory/scripts/inventory.py +++ b/plugins/claude-ops/skills/inventory/scripts/inventory.py @@ -501,8 +501,16 @@ def build_const_map(src: str) -> dict[str, str]: return {k: next(iter(v)) for k, v in seen.items() if len(v) == 1} -def extract_bundled_skills(src: str) -> tuple[dict[str, dict[str, Any]], dict[str, Any]]: - """Bundled skills, keyed by name, plus notes about resolution.""" +def extract_bundled_skills( + src: str, braces: BraceMap +) -> tuple[dict[str, dict[str, Any]], dict[str, Any]]: + """Bundled skills, keyed by name, plus notes about resolution. + + Each registration's fields are bound to its own `{...}` via the brace map, + for the same reason command extraction is: registrations sit flush against + one another, so a fixed-width window around one silently adopts the next + one's description or aliases whenever a field is absent. + """ notes: dict[str, Any] = {} fn = discover_registrar(src, "registerBundledSkill") notes["registrar"] = fn @@ -513,10 +521,20 @@ def extract_bundled_skills(src: str) -> tuple[dict[str, dict[str, Any]], dict[st consts = build_const_map(src) out: dict[str, dict[str, Any]] = {} unresolved: list[str] = [] + unbounded = 0 + seen = 0 for m in re.finditer(re.escape(fn) + r"\(\{", src): - window = src[m.start() : m.start() + 4000] - nm = re.search(r"name:(?:" + _STR + r"|([A-Za-z_$][A-Za-z0-9_$]{0,8}))", window) + seen += 1 + open_i = m.end() - 1 # the '{' captured by the pattern + close_i = braces.pairs.get(open_i) + if close_i is None: + # An unmatched brace means the tokenizer desynced; skipping is the + # honest response, and the count difference surfaces it. + unbounded += 1 + continue + body = src[open_i : close_i + 1] + nm = re.search(r"name:(?:" + _STR + r"|([A-Za-z_$][A-Za-z0-9_$]{0,8}))", body) if not nm: continue if nm.group(1) is not None: @@ -527,20 +545,22 @@ def extract_bundled_skills(src: str) -> tuple[dict[str, dict[str, Any]], dict[st unresolved.append(ident) continue name = consts[ident] - desc = _MENUDESC_RE.search(window) + desc = _MENUDESC_RE.search(body) out[name] = { "name": name, "source": "bundled-skill", "description": _unescape(desc.group(1)) if desc else "", - "aliases": _read_aliases(window[:1500]), - "gated": "isEnabled" in window[:1500], - "hidden": "isHidden" in window[:1500], + "aliases": _read_aliases(body), + "gated": "isEnabled" in body, + "hidden": "isHidden" in body, } - notes["registrations_seen"] = len(re.findall(re.escape(fn) + r"\(\{", src)) + notes["registrations_seen"] = seen notes["resolved"] = len(out) if unresolved: notes["unresolved_dynamic_names"] = sorted(set(unresolved)) + if unbounded: + notes["unbounded_registrations"] = unbounded return out, notes @@ -590,12 +610,12 @@ def check_integrity( advisories: list[str] = [] version = detect_cli_version(src) - if version is None and len(src) > 10_000: + if version is None: advisories.append( - "cli version could not be detected; counts are believed, not verified against " - f"{VALIDATED_AGAINST}" + "could not read a CLI version from the bundle; drift against the last " + f"validated build {VALIDATED_AGAINST} cannot be checked" ) - elif version and version != VALIDATED_AGAINST: + elif version != VALIDATED_AGAINST: advisories.append( f"cli {version} differs from the last validated build {VALIDATED_AGAINST}; " "counts are believed, not verified - re-run the skill's evals to revalidate" @@ -670,114 +690,61 @@ def _load_json(path: Path) -> Any | None: return None -def _merge_enabled_plugins(*maps: dict[str, Any] | None) -> dict[str, bool]: - """Merge enabledPlugins maps with later scopes overriding earlier ones.""" - merged: dict[str, bool] = {} - for m in maps: - if not isinstance(m, dict): - continue - for key, val in m.items(): - merged[str(key)] = bool(val) - return merged - - -def _scope_rank(scope: str) -> int: - return {"user": 1, "project": 2, "local": 3}.get(scope, 0) - - -def _pick_install_record( - records: list[dict[str, Any]], project_path: Path | None -) -> dict[str, Any] | None: - """Choose the install record for this project with local > project > user precedence.""" - project_norm = str(project_path).replace("\\", "/").rstrip("/") if project_path else "" - candidates: list[dict[str, Any]] = [] - for rec in records: - if not isinstance(rec, dict) or not rec.get("installPath"): - continue - rec_project = str(rec.get("projectPath") or "").replace("\\", "/").rstrip("/") - if rec_project: - if not project_norm: - continue - if rec_project != project_norm and not project_norm.startswith(rec_project + "/"): - continue - candidates.append(rec) - if not candidates: - return None - return max(candidates, key=lambda r: _scope_rank(str(r.get("scope") or "user"))) - - -def _manifest_component_path(root: Path, manifest: dict[str, Any], spec: dict[str, str]) -> Path | None: - """Resolve a component location from plugin.json, falling back to the default layout.""" - manifest_key = spec.get("manifest") or "" - declared = manifest - for part in manifest_key.split("."): - if not part: - continue - if not isinstance(declared, dict): - declared = {} - break - declared = declared.get(part) - if isinstance(declared, str) and declared.strip(): - rel = declared.strip().lstrip("./") - return root / rel - if spec["kind"] == "file": - return root / spec["file"] if spec.get("file") else None - return root / spec["dir"] if spec.get("dir") else None - - -def scan_disk(root: Path, project_root: Path | None = None) -> dict[str, Any]: +def scan_disk(root: Path) -> dict[str, Any]: """Enumerate installed plugins, their components, and config-scope surfaces.""" out: dict[str, Any] = {"config_dir": str(root), "exists": root.is_dir()} - user_settings = _load_json(root / "settings.json") or {} - project_settings: dict[str, Any] = {} - local_settings: dict[str, Any] = {} - if project_root is not None: - project_claude = project_root / ".claude" - project_settings = _load_json(project_claude / "settings.json") or {} - local_settings = _load_json(project_claude / "settings.local.json") or {} - out["project_root"] = str(project_root) - - enabled_map = _merge_enabled_plugins( - user_settings.get("enabledPlugins") if isinstance(user_settings.get("enabledPlugins"), dict) else None, - project_settings.get("enabledPlugins") if isinstance(project_settings.get("enabledPlugins"), dict) else None, - local_settings.get("enabledPlugins") if isinstance(local_settings.get("enabledPlugins"), dict) else None, - ) - out["enabled_plugins"] = { - "total_entries": len(enabled_map), - "enabled": sorted(k for k, v in enabled_map.items() if v), - "disabled": sorted(k for k, v in enabled_map.items() if not v), - } + settings = _load_json(root / "settings.json") or {} + enabled = settings.get("enabledPlugins") + if isinstance(enabled, dict): + out["enabled_plugins"] = { + "total_entries": len(enabled), + "enabled": sorted(k for k, v in enabled.items() if v), + "disabled": sorted(k for k, v in enabled.items() if not v), + } + else: + out["enabled_plugins"] = {"total_entries": 0, "enabled": [], "disabled": []} + out["enabled_plugins_note"] = "no enabledPlugins map in settings.json" - installed = _load_json(root / "plugins" / "installed_plugins.json") or {} - plugin_index = installed.get("plugins") if isinstance(installed, dict) else {} + known = _load_json(root / "plugins" / "known_marketplaces.json") or {} marketplaces: dict[str, Any] = {} - for key, enabled in sorted(enabled_map.items()): - if not enabled: - continue - records = plugin_index.get(key) if isinstance(plugin_index, dict) else None - if not isinstance(records, list): - records = [] - rec = _pick_install_record(records, project_root) - if rec is None: - market = key.split("@")[-1] if "@" in key else "unknown" - marketplaces.setdefault(market, {"plugins": {}})["plugins"][key] = { - "status": "unresolved", - "note": "no installPath in installed_plugins.json for this project", - } - continue - install_path = Path(str(rec["installPath"])) - market = key.split("@")[-1] if "@" in key else "unknown" - marketplaces.setdefault(market, {"plugins": {}})["plugins"][key] = scan_plugin(install_path) | { - "install_path": str(install_path), - "install_scope": rec.get("scope"), - "install_version": rec.get("version"), + for name, meta in known.items() if isinstance(known, dict) else []: + loc = (meta or {}).get("installLocation") + marketplaces[name] = { + "install_location": loc, + "last_updated": (meta or {}).get("lastUpdated"), + "plugins": scan_marketplace(Path(loc)) if loc else {}, } - out["marketplaces"] = marketplaces + + out["installed_plugins"] = scan_installed(root) out["config_scope_components"] = scan_config_scope(root) - if project_root is not None: - out["project_scope_components"] = scan_project_scope(project_root) + return out + + +def scan_installed(root: Path) -> dict[str, Any]: + """Plugins actually installed under the config dir's plugin cache. + + A marketplace checkout is a catalog of what is *available*; this is what is + present locally. The two can disagree — a plugin can be installed from a + marketplace that is no longer cached — so neither substitutes for the other. + """ + cache = root / "plugins" / "cache" + out: dict[str, Any] = {} + if not cache.is_dir(): + return out + for marketplace in sorted(p for p in cache.iterdir() if p.is_dir()): + plugins: dict[str, Any] = {} + for entry in sorted(p for p in marketplace.iterdir() if p.is_dir()): + # Installs may nest one version directory below the plugin name. + candidate = entry + if not (entry / ".claude-plugin").is_dir() and not (entry / "skills").is_dir(): + subdirs = [d for d in sorted(entry.iterdir()) if d.is_dir()] + if len(subdirs) == 1: + candidate = subdirs[0] + plugins[entry.name] = scan_plugin(candidate) + if plugins: + out[marketplace.name] = plugins return out @@ -814,61 +781,58 @@ def scan_plugin(root: Path) -> dict[str, Any]: return info -def _scan_component(root: Path, spec: dict[str, str], manifest: dict[str, Any] | None = None) -> list[str]: - manifest = manifest or {} +def _manifest_paths(manifest: dict[str, Any], key: str) -> list[str] | None: + """Component locations a manifest declares, or None if it declares none. + + A declared path *replaces* the default directory rather than adding to it, + so scanning the default tree regardless would report components a plugin + does not actually ship. Dotted keys address the `experimental` block. + """ + if not key: + return None + node: Any = manifest + for part in key.split("."): + if not isinstance(node, dict) or part not in node: + return None + node = node[part] + if isinstance(node, str): + return [node] + if isinstance(node, list): + return [p for p in node if isinstance(p, str)] + return None + + +def _scan_component(root: Path, spec: dict[str, str], manifest: dict[str, Any]) -> list[str]: + declared = _manifest_paths(manifest, spec.get("manifest", "")) kind = spec["kind"] + if kind == "file": - target = _manifest_component_path(root, manifest, spec) or (root / spec["file"]) - rel = target.relative_to(root).as_posix() if target.is_file() else spec["file"] - return [rel] if target.is_file() else [] + if declared: + return [d for d in declared if (root / d).is_file()] + target = root / spec["file"] + return [spec["file"]] if target.is_file() else [] + + targets: list[Path] + if declared: + targets = [root / d for d in declared] + else: + targets = [root / spec["dir"]] - directory = _manifest_component_path(root, manifest, spec) - if directory is None or not directory.is_dir(): - return [] out: list[str] = [] - if kind == "dir-of-dirs": - for entry in sorted(directory.iterdir()): - if entry.is_dir() and (entry / "SKILL.md").is_file(): - out.append(entry.name) - else: - for entry in sorted(directory.iterdir()): - if entry.is_file() and not entry.name.startswith("."): + for target in targets: + # A declared entry may point at a single file rather than a directory. + if target.is_file(): + out.append(target.name) + continue + if not target.is_dir(): + continue + for entry in sorted(target.iterdir()): + if kind == "dir-of-dirs": + if entry.is_dir() and (entry / "SKILL.md").is_file(): + out.append(entry.name) + elif entry.is_file() and not entry.name.startswith("."): out.append(entry.name) - return out - - -def scan_project_scope(project_root: Path) -> dict[str, Any]: - """Project-scoped invocable surfaces outside the user config dir.""" - claude = project_root / ".claude" - out: dict[str, Any] = {} - skills = claude / "skills" - if skills.is_dir(): - out["skills"] = sorted( - e.name for e in skills.iterdir() if e.is_dir() and (e / "SKILL.md").is_file() - ) - for name, sub in (("agents", "agents"), ("commands", "commands")): - d = claude / sub - if d.is_dir(): - out[name] = sorted(e.name for e in d.iterdir() if e.is_file()) - for label, fname in (("project", "settings.json"), ("local", "settings.local.json")): - settings = _load_json(claude / fname) or {} - if isinstance(settings.get("hooks"), dict): - out.setdefault("hooks_events", {})[label] = sorted(settings["hooks"].keys()) - enabled = settings.get("enabledPlugins") - if isinstance(enabled, dict): - out.setdefault("enabled_plugins", {})[label] = { - "enabled": sorted(k for k, v in enabled.items() if v), - "disabled": sorted(k for k, v in enabled.items() if not v), - } - mcp = _load_json(project_root / ".mcp.json") - if isinstance(mcp, dict) and isinstance(mcp.get("mcpServers"), dict): - out["mcp_servers"] = sorted(mcp["mcpServers"].keys()) - return out - - -def project_root_from_env() -> Path | None: - env = os.environ.get("CLAUDE_PROJECT_DIR") - return Path(env) if env else None + return sorted(set(out)) def scan_config_scope(root: Path) -> dict[str, Any]: @@ -920,7 +884,7 @@ def build_report(args: argparse.Namespace) -> dict[str, Any]: braces = build_brace_map(src) meta["brace_pairs"] = len(braces.pairs) commands = extract_builtin_commands(src, braces) - skills, skill_notes = extract_bundled_skills(src) + skills, skill_notes = extract_bundled_skills(src, braces) plugin_backed = extract_plugin_backed(src) for name, plugin in plugin_backed.items(): @@ -945,11 +909,18 @@ def build_report(args: argparse.Namespace) -> dict[str, Any]: if not args.binary_only: report["sources"]["disk"] = {"available": True} - project = Path(args.project_dir) if args.project_dir else project_root_from_env() - report["disk"] = scan_disk( - Path(args.config_dir) if args.config_dir else config_dir(), - project, - ) + report["disk"] = scan_disk(Path(args.config_dir) if args.config_dir else config_dir()) + + # Project scope is a third place components come from, and it is the one + # that changes as you move between repos: a project's .claude tree adds + # skills, agents, and hooks that no machine-scope scan would ever see. + project_root = Path(args.project_dir) if args.project_dir else Path.cwd() + project_claude = project_root / ".claude" + report["project"] = { + "root": str(project_root), + "present": project_claude.is_dir(), + "components": scan_config_scope(project_claude) if project_claude.is_dir() else {}, + } return report @@ -968,7 +939,9 @@ def main(argv: list[str] | None = None) -> int: ) ap.add_argument("--binary", help="path to the claude executable (default: auto-detect)") ap.add_argument("--config-dir", help="config dir (default: $CLAUDE_CONFIG_DIR or ~/.claude)") - ap.add_argument("--project-dir", help="project root (default: $CLAUDE_PROJECT_DIR when set)") + ap.add_argument( + "--project-dir", help="project root whose .claude tree to scan (default: cwd)" + ) ap.add_argument("--binary-only", action="store_true", help="skip the disk scan") ap.add_argument("--disk-only", action="store_true", help="skip reading the binary") ap.add_argument("--out", help="write JSON here instead of stdout") @@ -993,8 +966,11 @@ def main(argv: list[str] | None = None) -> int: if args.self_check: integrity = report.get("integrity") if integrity is None: - reason = report.get("sources", {}).get("binary", {}).get( - "error", "binary source unavailable" + binsrc = report.get("sources", {}).get("binary", {}) + reason = ( + binsrc.get("error") + or binsrc.get("reason") + or "binary source unavailable" ) print(f"BROKEN: {reason}") return 1 diff --git a/plugins/claude-ops/skills/inventory/scripts/test_inventory.py b/plugins/claude-ops/skills/inventory/scripts/test_inventory.py index 70d227593f..4373f22c03 100755 --- a/plugins/claude-ops/skills/inventory/scripts/test_inventory.py +++ b/plugins/claude-ops/skills/inventory/scripts/test_inventory.py @@ -11,6 +11,7 @@ from __future__ import annotations +import pathlib import unittest import inventory as inv @@ -120,7 +121,7 @@ def test_missing_export_returns_none(self) -> None: def test_constant_names_resolve(self) -> None: # The failure this guards: a literal-only scan silently drops roughly a # third of the bundled skills, including code-review and dataviz. - skills, notes = inv.extract_bundled_skills(self.BUNDLE) + skills, notes = inv.extract_bundled_skills(self.BUNDLE, inv.build_brace_map(self.BUNDLE)) self.assertEqual(notes["registrar"], "xu") self.assertIn("code-review", skills) self.assertIn("dataviz", skills) @@ -131,14 +132,18 @@ def test_ambiguous_constant_is_not_resolved(self) -> None: # An identifier bound to two different strings cannot be resolved # safely, so it must be reported rather than guessed. src = self.BUNDLE + 'var zz="a-one";var zz="a-two";xu({name:zz});' - _, notes = inv.extract_bundled_skills(src) + _, notes = inv.extract_bundled_skills(src, inv.build_brace_map(src)) self.assertGreater(notes["registrations_seen"], notes["resolved"]) class TestIntegrity(unittest.TestCase): def _src(self, extra: str = "") -> str: + # The version literal has to repeat: detect_cli_version deliberately + # ignores a version mentioned only once, since that is a dependency's + # version rather than the build's. return ( 'pt(Q,{registerBundledSkill:()=>xu});' + + f'"{inv.VALIDATED_AGAINST}"' * 30 + "".join(f'x{i}={{type:"local",name:"{n}",description:"d"}};' for i, n in enumerate(inv.CANARY_COMMANDS)) + extra @@ -214,5 +219,70 @@ def test_finds_plugin_name(self) -> None: self.assertEqual(inv.extract_plugin_backed(src), {"security-review": "security-review"}) +class TestBundledSkillFieldBinding(unittest.TestCase): + """A registration missing a field must not adopt the next one's.""" + + BLEED = ( + 'pt(Q,{registerBundledSkill:()=>xu});' + 'xu({name:"first"});' + 'xu({name:"second",aliases:["s"],menuDescription:"Second description"});' + ) + + def test_missing_description_does_not_bleed_forward(self) -> None: + skills, _ = inv.extract_bundled_skills(self.BLEED, inv.build_brace_map(self.BLEED)) + self.assertEqual(skills["first"]["description"], "") + self.assertEqual(skills["first"]["aliases"], []) + self.assertEqual(skills["second"]["description"], "Second description") + + +class TestManifestComponentPaths(unittest.TestCase): + """A declared path replaces the default directory rather than adding to it.""" + + def test_dotted_key_resolves(self) -> None: + m = {"experimental": {"themes": "./custom-themes/"}} + self.assertEqual(inv._manifest_paths(m, "experimental.themes"), ["./custom-themes/"]) + + def test_absent_key_is_none(self) -> None: + self.assertIsNone(inv._manifest_paths({}, "agents")) + self.assertIsNone(inv._manifest_paths({"experimental": {}}, "experimental.themes")) + + def test_array_form(self) -> None: + self.assertEqual( + inv._manifest_paths({"commands": ["./a/", "./b/"]}, "commands"), ["./a/", "./b/"] + ) + + def test_declared_dir_replaces_default(self) -> None: + import tempfile + + with tempfile.TemporaryDirectory() as td: + root = pathlib.Path(td) + (root / "agents").mkdir() + (root / "agents" / "default.md").write_text("x", encoding="utf-8") + (root / "custom").mkdir() + (root / "custom" / "declared.md").write_text("x", encoding="utf-8") + spec = {"dir": "agents", "manifest": "agents", "kind": "dir-of-files"} + self.assertEqual( + inv._scan_component(root, spec, {"agents": ["./custom/"]}), ["declared.md"] + ) + self.assertEqual(inv._scan_component(root, spec, {}), ["default.md"]) + + +class TestSelfCheckDiagnostic(unittest.TestCase): + def test_unknown_version_is_an_advisory(self) -> None: + src = ( + 'pt(Q,{registerBundledSkill:()=>xu});' + + "".join(f'x{i}={{type:"local",name:"{n}",description:"d"}};' + for i, n in enumerate(inv.CANARY_COMMANDS)) + ) + got = inv.check_integrity( + src, + inv.extract_builtin_commands(src, inv.build_brace_map(src)), + {"a": {}}, + {"registrations_seen": 1, "resolved": 1}, + ) + self.assertEqual(got["status"], "degraded") + self.assertTrue(any("could not read a CLI version" in a for a in got["advisories"])) + + if __name__ == "__main__": unittest.main(verbosity=2) From f0c17db4f28528fa204077568a411a689df9770e Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Wed, 12 Aug 2026 00:58:58 -0400 Subject: [PATCH 2/2] fix(claude-ops): disambiguate inventory --self-check degraded from usage error --self-check returned 2 for a degraded verdict, which is also argparse's exit code for a usage error. A CI gate treating 2 as "degraded, warn" would silently swallow a mistyped flag as a warning instead of failing. Degraded is now 3, leaving 2 to argparse: 0 ok, 1 broken, 2 usage error, 3 degraded. Verified all four against the live build. Also records the 0.30.1 changelog entry covering this and the review findings carried over from #2313, which merged before those fixes landed. Co-Authored-By: Claude Opus 5 (1M context) --- plugins/claude-ops/.claude-plugin/plugin.json | 2 +- plugins/claude-ops/CHANGELOG.md | 32 +++++++++++++++++++ plugins/claude-ops/skills/inventory/SKILL.md | 5 +-- .../skills/inventory/scripts/inventory.py | 6 ++-- 4 files changed, 39 insertions(+), 6 deletions(-) diff --git a/plugins/claude-ops/.claude-plugin/plugin.json b/plugins/claude-ops/.claude-plugin/plugin.json index e4f20ec835..883f709258 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.31.0", + "version": "0.31.1", "description": "Claude Code operations toolkit. Ten skills: inventory (read-only enumeration of the complete invocable surface \u2014 every built-in CLI command with aliases and hidden/gated status, every bundled skill, 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 \u2014 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 \u2014 CLI version, retention-sweep health including the silent unparsable-settings pause, a timed census walk of the install tree as a sweep-cost proxy, active-session and plugin-fleet counts, a process census, and a bundled known-performance-issues reference; separates the three documented suspects \u2014 accumulated state, version regression, component bloat \u2014 and routes remediation out; reports, never mutates), observability (read locally captured telemetry \u2014 OTEL store, collector, hook-event JSONL, ccusage \u2014 with trend reports 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 integrate them into the current repo), plugins (bring a machine's plugin fleet current on demand \u2014 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 \u2014 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 \u2014 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 lives. Plus a family of seven advisory *-audit telemetry-emitter hooks (API errors, config changes, instruction loads, permission denials, pre-compaction, skill usage, tool failures) that emit the shared hook-telemetry envelope, and a reference sink that maps envelopes into the 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 9d55c76d82..ba261ae1ab 100644 --- a/plugins/claude-ops/CHANGELOG.md +++ b/plugins/claude-ops/CHANGELOG.md @@ -3,6 +3,38 @@ 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.31.1] + +### Fixed + +- **`skills/inventory` bundled-skill fields could bleed from the next registration.** The extractor + read each registration through a fixed 4000-character window — the failure mode `build_brace_map` + exists to prevent for commands, and the one `reference/extraction.md` names as the thing not to + do. A registration omitting a description adopted the following one's. Fields are now bound to + their own literal via the brace map, and an unmatched brace is counted and surfaced rather than + silently skipped. +- **Manifest-declared component paths were ignored.** `PLUGIN_COMPONENTS` carried a manifest key per + component and a comment claiming the manifest is read before the tree; nothing read it. A declared + path replaces the default directory, so scanning defaults regardless reported components a plugin + does not ship. Dotted keys resolve the `experimental` block. +- **`--self-check` lost its diagnostic when no binary was found.** `pick_binary` stores its + explanation under `reason`; the self-check path read only `error` and printed a generic message. +- **An unreadable CLI version passed silently.** It is itself a drift signal, so it now degrades the + verdict instead of skipping the comparison. +- **`--self-check` degraded and an argparse usage error both exited 2.** A CI gate treating 2 as + "degraded, warn" would silently swallow a mistyped flag. Degraded is now 3, leaving 2 to argparse: + 0 ok, 1 broken, 2 usage error, 3 degraded. + +### Added + +- **`skills/inventory` reads installed plugins and project scope.** Only marketplace catalogs were + scanned, so a plugin installed from a marketplace that is no longer cached was invisible; + `disk.installed_plugins` now walks the plugin cache, and catalog, installed, and enabled are + reported as three distinct sets. A project's `.claude` tree contributes skills, agents, and wired + hook events that no machine-scope scan sees — `--project-dir` defaults to the working directory. + Wired hook events are reported, never hook scripts on disk, which would repeat the + present-versus-active error the skill warns about. + ## [0.31.0] ### Added diff --git a/plugins/claude-ops/skills/inventory/SKILL.md b/plugins/claude-ops/skills/inventory/SKILL.md index 88a35d3d14..a5d1064a5b 100644 --- a/plugins/claude-ops/skills/inventory/SKILL.md +++ b/plugins/claude-ops/skills/inventory/SKILL.md @@ -187,8 +187,9 @@ exports the script does not know about, and the resolved-versus-seen gap on bund python3 "${CLAUDE_PLUGIN_ROOT}/skills/inventory/scripts/inventory.py" --self-check ``` -It prints one verdict line and exits `0` ok, `1` broken, `2` degraded — so it works as a CI gate, a -loop-lane step, or a post-update check without parsing JSON. The natural trigger is a CLI release: +It prints one verdict line and exits `0` ok, `1` broken, `3` degraded — so it works as a CI gate, a +loop-lane step, or a post-update check without parsing JSON. `2` is left to argparse for a usage +error, so a mistyped flag can never be mistaken for a degraded run. The natural trigger is a CLI release: `/claude-ops:changelog` already ingests those, and this is the check to run when it reports one. **What a maintainer actually updates.** Most releases need no change — registrar names are diff --git a/plugins/claude-ops/skills/inventory/scripts/inventory.py b/plugins/claude-ops/skills/inventory/scripts/inventory.py index e9ef516c6f..5377757af4 100755 --- a/plugins/claude-ops/skills/inventory/scripts/inventory.py +++ b/plugins/claude-ops/skills/inventory/scripts/inventory.py @@ -948,8 +948,8 @@ def main(argv: list[str] | None = None) -> int: ap.add_argument( "--self-check", action="store_true", - help="print only the integrity verdict; exit 1 if extraction is broken, " - "2 if it is degraded. For CI and scheduled drift checks.", + help="print only the integrity verdict; exit 0 ok, 1 broken, 3 degraded " + "(2 stays argparse's usage error). For CI and scheduled drift checks.", ) args = ap.parse_args(argv) @@ -980,7 +980,7 @@ def main(argv: list[str] | None = None) -> int: print(f" problem: {p}") for a in integrity["advisories"]: print(f" advisory: {a}") - return {"ok": 0, "broken": 1, "degraded": 2}[integrity["status"]] + return {"ok": 0, "broken": 1, "degraded": 3}[integrity["status"]] text = json.dumps(report, indent=1, sort_keys=True) if args.out: