feat(claude-ops): audit the installed fleet, resolved per plugin not per scope (0.34.0)

[kyle-sexton]

No linked issue

Summary

audit-skill-visibility could only measure a plugins directory, defaulting to ./plugins relative to the working directory. That answers "what is in this checkout", not "what is installed" — the question the skill exists for. #3044 documented that limitation honestly; this removes it.

--installed reads ~/.claude/plugins/installed_plugins.json instead. Both answers are now available and they legitimately differ: measured here the repo holds 221 skills and the installed fleet 216, because three plugins present in the checkout (ai-slop, context-budget, improvement — 2+1+2 skills) were never installed. Neither number is wrong; they answer different questions.

Fix

The manifest lists one entry per install SCOPE, not per plugin. Here 67 plugins carried 134 entries. Since the fleet is the denominator the listing budget is measured against, counting entries would have roughly doubled the reported overflow and fabricated the headline number. resolve_installed() is pure (manifest + marketplace registry in, resolution out; no filesystem), keys by plugin identity, and reports both counts so the 134→67 collapse is auditable rather than trusted.

Multi-scope installs resolve by the documented precedence local > project > user — the record that loads is the highest-precedence applicable one, never the newest version installed. Superseded records are listed under a new Fleet resolution section so a pin being outranked stays visible. Getting this wrong is not cosmetic: 7 plugins here ship different skill sets between scopes and 19 skills different description text.

Records that cannot load here are excluded and reported. project and local installs load only in the projectPath they name; another project's records would inflate the fleet with skills the model can never see. Current project comes from CLAUDE_PROJECT_DIR, falling back to cwd — matching fleet-state.sh.

A directory-source marketplace loads its checkout, not either cached installPath — verified by a skill executing out of the marketplace directory. The plugin root for those comes from the catalog's declared source, because plugins/<name> is the common layout but not a rule. Since neither cached version is running there, those report marketplace-directory and emit no superseded pair rather than naming two versions that are both beside the point.

Supporting change: collect_fleet delegates to a new per-plugin collect_fleet_at, because installs are scattered across versioned cache paths with no single parent to walk.

Corrections during review

Five review findings, all real. Two are corrections to claims I made, and I'd rather they sit in the description than only in the thread:

1. I said scope precedence was "undocumented and not verifiable here." It is documented — in the plugin I was editing. plugins/claude-ops/skills/plugins/context/scope-semantics.md states local > project > user and warns against my exact heuristic by name: "the record that actually loads is the one at the highest-precedence scope present, never simply 'the newest version installed.'" I searched the compiled CLI, found only string constants, and concluded the answer was unknowable — without grepping the repo I was working in. My first revision shipped a lastUpdated sort, which is precisely the failure mode that document exists to prevent. Caught by Codex (P1); fixed in 0b87680a, pinned by test_scope_precedence_beats_the_newest_version (an older project pin must beat a newer user install).

2. In #3044 I deferred this work claiming "no verifiable single root to default to." Also a wrong premise — installed_plugins.json is authoritative and always was. I was looking for a single root directory rather than a manifest.

Also fixed in 0b87680a: projectPath filtering (Codex P1), catalog-declared source paths instead of a hardcoded layout (Codex P2), two dead fields that nothing downstream read (review lane), and the missing test for collect_installed — the function --installed actually wires to (review lane).

Verification

All run locally on 0b87680a:

Check	Result
test_audit_skill_visibility.py	84 passed (76 → 84)
audit_skill_visibility.test.sh	fresh-install fixture 4/4 withheld; --installed collapsed 2 records → 1 plugin
check-skill.sh --require-evals	PASS — 0 errors, 0 warnings (description 1524/1536; SKILL.md trimmed to 200 lines to clear the soft target it had crossed)
ruff check / ruff format --check	clean
shellcheck	clean
check-changelog-parity.sh --check-bump	clean
scripts/validate-plugins.sh	Validation passed

Live, both modes executed rather than asserted:

$ audit_skill_visibility.py --installed --render json
entries 134  plugins 67  superseded 0  not_applicable 0  skills 216

$ audit_skill_visibility.py --plugins-root plugins --render json
skills 221, no "fleet" key — prior behavior unchanged

Branches the live install cannot reach are covered by tests instead: precedence (local > project > user), another project's install excluded end to end, catalog-declared . source path, and an unreadable config directory.

Related

Refs #3044 — documented the cwd-relative limitation this removes, and whose "Deliberately not done" section this closes out. Refs #3035 — shipped the skill.

[claude]

Claude finished @kyle-sexton's task in 1m 30s —— View job

---

Security Review

• Gather context (PR diff, files changed)
• Review audit_skill_visibility.py changes (resolve_installed, collect_installed, collect_fleet_at, --installed flag)
• Review test additions, SKILL.md, CHANGELOG.md, plugin.json
• Post findings

No security vulnerabilities found. This PR adds read-only tooling that resolves ~/.claude/plugins/installed_plugins.json + known_marketplaces.json into a per-plugin fleet count, and reports scope conflicts instead of silently picking one.

Reviewed specifically for the usual risk classes in a change like this:

• Command/code execution — no new subprocess, os.system, eval, or exec calls. The pre-existing subprocess.run(args, ...) in read_churn (not touched by this PR) already uses a list-form argv, no shell=True.
• Unsafe deserialization — the new manifest/marketplace readers (collect_installed) use json.load only, guarded by try/except (OSError, ValueError). parse_frontmatter (pre-existing, unmodified) is a manual scalar-key reader, not yaml.load/eval.
• Path handling — resolve_installed builds root paths via os.path.join(location, "plugins", name) / installPath values pulled from installed_plugins.json / known_marketplaces.json. These are the same local, machine-trusted files Claude Code's own CLI already reads to load plugins — this PR doesn't cross a new trust boundary, and the tool is read-only (walks directories, reads SKILL.md text) rather than writing or executing anything at the resolved path. No externally-reachable input flows into these values.
• Secrets/credentials — none introduced; no new env var, token, or credential handling.
• --write output path (report_path) — unchanged by this diff.

One informational (non-blocking) note: collect_fleet_at/resolve_installed trust name/installPath from the manifest without normalizing (e.g. rejecting .. segments) before joining into a filesystem path. Given the manifest is local config already trusted elsewhere (same file the CLI itself reads to decide what to load), this isn't an exploitable escalation in the current design — flagging only as defense-in-depth if this resolver is ever reused against a less-trusted manifest source in the future.

Non-security observations (out of scope for this review, not blocking):

• Test coverage for the new resolver looks solid (ResolveInstalledTest: multi-scope collapse, ambiguous-flagging, directory-source certainty, single-scope certainty, empty/malformed manifest).
• Verification steps in the PR description (ruff, test suite, check-skill.sh --require-evals) were run locally per the author's report; not re-executed here since the review scope was security-only.

[github-actions]

Last security-reviewed head: 9e805c0007dd0bf8b3d66a2a1a5984b7dd8ec07c. On the next push, the relevance gate compares only the commits since this SHA; delete this comment to force a full re-review.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 54ecad3649

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Claude finished @kyle-sexton's task in 3m 51s —— View job

---

Claude finished reviewing this PR

• Gather context (PR body, CLAUDE.md, changed files)
• Review audit_skill_visibility.py diff for correctness/maintainability
• Review test_audit_skill_visibility.py diff for coverage
• Review SKILL.md / CHANGELOG.md / plugin.json for consistency
• Post inline findings and final summary

Summary

Reviewed the --installed fleet-resolution feature (resolve_installed, collect_installed, collect_fleet_at) against CLAUDE.md/REVIEW.md correctness-and-maintainability criteria. This is the code-review lane; security is out of scope here (there's a dedicated security lane).

The design holds up under inspection:

• resolve_installed is genuinely pure (no filesystem), matches the real on-disk known_marketplaces.json schema I cross-checked against plugins/claude-ops/skills/plugins/scripts/fixtures/known_marketplaces.sample.json and fleet-state.test.sh (nested {"source": {"source": "directory"|"github", ...}}) — the nested source.get("source") lookup in audit_skill_visibility.py#L200 is correct, not a mismatch with the PR body's simplified example.
• The scope-collapse math (manifest_entries vs plugins_resolved), the ambiguous-vs-certain branching, and the deterministic tie-break are all sound and match the new ResolveInstalledTest cases.
• argparse's nargs="?", const="" handling for --installed (bare flag vs. --installed <dir>) is correct.

Two findings posted inline, both maintainability/coverage rather than live bugs:

1. Dead fields — install_scope/install_confidence are stamped onto every denominator entry in collect_installed but never read by classify() or reachability(), so per-skill rows carry no trace of which plugins are ambiguous (only the plugin-level "Fleet resolution" section does).
2. Untested integration path — collect_installed, the function main() actually wires to the new --installed flag, has no direct test. Only the pure resolve_installed() helper is unit-tested, and the shell integration suite (audit_skill_visibility.test.sh) never exercises --installed.

Not independently verified (author-claimed only, per lane rules): the 76 passed/ruff/check-skill.sh/check-changelog-parity.sh results in the Verification table. My Bash tool in this lane is scoped to gh pr diff/view/comment/review only, so I could not execute python3, pytest, ruff, or the shell test suite myself to confirm those claims — reviewed statically instead. If test/lint execution is wanted for this lane, --allowedTools would need Bash(python3:*) / Bash(ruff:*) added.

[github-actions]

Claude has reviewed this PR 3 times. The lane skips further automatic reviews after 5; deleting this comment resets the count.

[claude]

Claude finished @kyle-sexton's task in 1m 14s —— View job

---

Security Review

• Gather context (PR diff, files changed)
• Invoke /review:security-review and follow its instructions
• Post findings

Reviewed the delta since the last security review (54ecad3 → 0b87680, the fix(claude-ops): resolve installs by documented scope precedence, not newest version commit) per /review:security-review's skip gate — that commit is new since the last successful security review, so it required a fresh pass. Scope: security only, per the lane's criteria (logic/trust-boundary flaws, injection, credential handling, Actions permission changes); no style or quality commentary.

No security vulnerabilities found.

What changed: resolve_installed now follows the documented local > project > user scope-precedence rule instead of picking the newest lastUpdated version, filters out project/local install records that belong to a different project (via CLAUDE_PROJECT_DIR/cwd matching against each record's projectPath), and resolves directory-source marketplace roots from the marketplace catalog's declared source path instead of a hardcoded plugins/<name> layout. All three were correctness fixes (from Codex + the code-review lane) to the fleet-resolution logic added in the earlier commit — none open a new trust boundary:

• Path handling — _catalog_source and _install_applies build/compare filesystem paths from known_marketplaces.json, installed_plugins.json, and now also .claude-plugin/marketplace.json read from the marketplace's own installLocation. All three are local, machine-trusted files already read by the CLI itself to decide what to load — this doesn't cross a new trust boundary versus the prior commit, and the tool remains read-only (walks directories, reads SKILL.md text). No externally-reachable input flows into these values. Same conclusion as the prior review of this PR, still holds.
• Environment input — CLAUDE_PROJECT_DIR (read at audit_skill_visibility.py#L1266) is used only as a string compared via os.path.normpath equality against each record's projectPath — no shell interpolation, no path traversal beyond what the trusted manifest already permits.
• No new code execution, deserialization, or credential handling — _load_json is the same guarded json.load path as before; no new subprocess/eval/exec.
• Test and doc changes (test_audit_skill_visibility.py, audit_skill_visibility.test.sh, SKILL.md, CHANGELOG.md) are non-executable-in-CI fixtures/prose — no security surface.

No GitHub Actions workflow files were touched in this commit.

[claude]

Claude finished @kyle-sexton's task in 5m 27s —— View job

---

Code review complete

• Gather context (PR diff at 0b87680, CLAUDE.md, prior review threads)
• Review audit_skill_visibility.py diff (resolve_installed, collect_installed, collect_fleet_at)
• Review test/doc changes (test_audit_skill_visibility.py, audit_skill_visibility.test.sh, SKILL.md, CHANGELOG.md)
• Check whether the fix commit (0b87680) addresses the Codex P1 "resolve scope precedence" finding
• Post inline findings and summary

Summary

This is the code-review lane (correctness/maintainability; security is a separate lane). Fix commit 0b87680 is a genuine, well-tested rewrite that addresses all five findings from the prior round on this PR:

• Codex P1 (scope precedence) — fixed. resolve_installed no longer sorts by lastUpdated; it now resolves via the documented local > project > user precedence (SCOPE_PRECEDENCE, audit_skill_visibility.py#L298), cross-checked against this same plugin's skills/plugins/context/scope-semantics.md, which does state that rule and explicitly warns against the newest-version heuristic.
• Codex P1 (other-project installs counted) — fixed. _install_applies (#L365-L382) excludes project/local records whose projectPath doesn't match the current project (CLAUDE_PROJECT_DIR, falling back to cwd) and reports them under not_applicable instead of silently counting them.
• Codex P2 (hardcoded directory-plugin path) — fixed. _catalog_source reads the plugin's declared source from the marketplace's own .claude-plugin/marketplace.json catalog rather than assuming plugins/<name>.
• Review-lane (dead install_scope/install_confidence fields) — fixed, fields removed entirely.
• Review-lane (untested --installed integration path) — fixed. CollectInstalledTest now covers collect_installed end to end (catalog-declared path, unreadable config dir, another project's install excluded), plus a new shell contract check in audit_skill_visibility.test.sh.

The scope-collapse math, the not_applicable/superseded reporting, and the new --installed CLI wiring in main() all check out against the new unit tests, which cover the precedence tie-break, the directory-source override, and the empty/malformed-manifest paths.

One new finding, posted inline:

• Stale docstring in resolve_installed (lines 361-369) — leftover from the pre-fix version. It still says multi-scope installs are "reported ambiguous" and a directory-source case is "marked certain," but the rewritten implementation resolves deterministically by scope precedence and reports the loser via the superseded list — there's no ambiguous/certain field in the return value anymore. The equivalent SKILL.md prose was updated correctly in this same commit; only this docstring wasn't.

Not independently verified (author-claimed only): the 84 unit tests, ruff, shellcheck, check-skill --require-evals, and changelog-parity results in the commit message. This lane's Bash tool is scoped to gh pr diff/view/comment/review, so python3/ruff/the shell suite couldn't be executed here — reviewed statically instead.

[claude]

Claude finished @kyle-sexton's task in 38s —— View job

---

Security Review

• Gather context (PR diff since last review, HEAD SHA)
• Apply skip gate
• Post result

Skip gate: stopping early. The only change since the last successful security review (0b87680 → 9e805c0) is a docstring rewrite in resolve_installed (commit docs(claude-ops): correct resolve_installed's docstring to the shipped contract). It's a comment-only edit — no code, control flow, path handling, or I/O changed — so there is no new security-relevant surface to review (skip gate condition 2).

No security vulnerabilities found. This matches the conclusion of the prior two security-review passes on this PR (54ecad3 and 0b87680): read-only tooling over local, machine-trusted config (installed_plugins.json, known_marketplaces.json), no new subprocess/eval/exec, no new deserialization, no credential handling, no GitHub Actions workflow changes.

[claude]

Claude finished @kyle-sexton's task in 1m 16s —— View job

---

Code review complete

• Gather context (PR diff at 9e805c0, prior review threads, skip gate)
• Review delta since last code-review pass (docstring fix commit 9e805c0)
• Confirm no regressions in the rest of the diff
• Post findings

Summary

This lane already reviewed 54ecad3 and 0b87680 in full (two prior passes on this PR). The only change since then is 9e805c0, a docstring-only fix to resolve_installed() in audit_skill_visibility.py, addressing the one open finding from the prior round (the docstring still described the pre-0b87680 ambiguous/certain contract).

I checked the new docstring text against the actual return value (audit_skill_visibility.py#L467-L481):

• Docstring now says the winner is chosen by SCOPE_PRECEDENCE applied to applicable records, with not_applicable and superseded as the two report lists — matches the actual keys returned (plugins, superseded, not_applicable, manifest_entries, plugins_resolved).
• No more ambiguous/certain vocabulary, which the returned dict never had after 0b87680.
• The directory-source exception (marketplace-directory scope, no superseded pair emitted) is described accurately against the code at L415-L455.

Diff is 21 insertions / 9 deletions, confined to the docstring — no behavior change, consistent with the commit message's own claim, and nothing outside that docstring was touched in this commit.

No new findings. All prior code-review-lane findings on this PR (dead install_scope/install_confidence fields, untested collect_installed integration path, and this docstring) are now resolved. Security concerns remain out of scope for this lane per REVIEW.md's split — see the dedicated claude-security-review lane's passes on this PR, both of which found nothing.

Not independently verified (author-claimed only): the "84 unit tests / ruff / check-skill --require-evals PASS" verification claims in the commit message. This lane's Bash tool is scoped to gh pr diff/view/comment/review, so python3/ruff/the shell suite could not be executed here — reviewed statically instead.