feat(claude-ops): audit plugin cache contents against the recorded commit - #3775
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Claude finished @kyle-sexton's task in 6m 31s —— View job Code review complete
This repo carries a Findings (posted inline, both anchored to
Everything else held up well: the script is genuinely read-only (verified by the test suite's own before/after hash-compare and |
|
Claude finished @kyle-sexton's task in 2m 17s —— View job Security review (
|
|
Last security-reviewed head: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: df44dc9fae
ℹ️ 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 has reviewed this PR 1 time. The lane skips further automatic reviews after 5; deleting this comment resets the count. |
3d4c8a7 to
4851338
Compare
…mmit (#3681) `claude plugin update` re-points an install record's `gitCommitSha` without rewriting the plugin's cache directory when the manifest version number is unchanged across the two commits, because the cache is keyed by version. The record then claims the new commit while the directory still holds the older build, and the version-and-sha check every delivery step relies on passes in exactly that state. On the reporting machine six plugins were in it at once. Adds `cache-content-check.sh`, a read-only per-install compare of every file in a cache directory against the recorded commit in the marketplace clone, in both directions: a changed file, a file the commit has and the cache lacks, and a file deleted at the commit but still in the cache. It runs as Step 5b of `sync` and of `audit`, ungated, because an unchanged version number is the case in which every other step reports success. Route (b) of the three the issue offered. Route (a), a repo rule that every change bumps the version, prevents nothing already delivered and depends on author discipline; route (c), an upstream report, has no date this repo controls. A standing check holds whatever upstream does, and precludes neither. The check reports and never repairs: no state file written, no cache directory removed, and no `git fetch` of a commit the clone lacks. Marketplace clones are shallow, so an install whose commit predates that window is reported `sha-not-local` and counted unverifiable rather than as a pass. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019gWgHogJFQeCne5U7vHKAE
…3681) Both siblings in this directory ship mode 100755 and the test runner invokes suites directly; the new pair landed 100644. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019gWgHogJFQeCne5U7vHKAE
…it (#3681) `git hash-object --stdin-paths` is one line of output per input path, and the cache-side table is built positionally on that. A short read would file every entry after the gap under the wrong path and report healthy files as differing, silently. Counting the hashes back turns that into a stated `hash-batch-misaligned` verdict, which is unverifiable rather than a pass. Also records in scope-semantics.md what the first real-machine run established: a marketplace clone is shallow, three commits deep when measured, so an install whose commit predates that window has no object to compare and reports `sha-not-local` through no fault of the fleet. Eleven of seventy-four user-scope installs on the authoring machine landed there. That caps what any single run can establish, and the report says so rather than leading with the match count. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019gWgHogJFQeCne5U7vHKAE
…3681) Read the plugin's source path from the marketplace.json at the recorded commit rather than the clone's current checkout, with no fallback. A directory renamed after that commit was looked up under its present-day path against an older tree, and a pathspec matching nothing is success-with-empty-output, so a healthy cache reported stale-content. A pathspec that still matches nothing at that sha now gets its own no-source-at-sha verdict, counted unverifiable. Carry pathnames NUL-separated end to end (ls-tree -z, find -print0, check-ignore -z); git otherwise quotes any name with non-ASCII, a tab, a newline or a backslash, and the quoted spelling never matched the raw path. hash-object --stdin-paths has no -z, so only a newline-bearing path falls back to a process of its own. Compare tracked symlinks mode-aware by hashing the link target text, instead of excluding them with find -type f and calling every link missing. Decode install records with a US (0x1f) separator, as fleet-state.sh does: bash collapses empty tab-separated columns, so a record with no gitCommitSha shifted installPath into the sha field and reported install-path-missing with a fabricated sha. Tests: fixtures for a renamed source directory, no-source-at-sha, a non-ASCII filename, a symlink (capability-gated with a visible skip), an absent gitCommitSha, and a stub git that truncates the hash batch to exercise hash-batch-misaligned. The process-budget probe now asserts a positive measured count so a trace it cannot parse fails instead of passing as -1. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019gWgHogJFQeCne5U7vHKAE
… byte (#3681) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019gWgHogJFQeCne5U7vHKAE
4851338 to
f81b9a4
Compare
…m and why nothing reaps them (#3786) No related issue: this records verified facts for the #3688 decision without closing it; the structural remediation stays open on that issue for the operator. ## Summary The `plugins` skill reports stale project-scope install records and states that no CLI verb removes one by path, but it never recorded where those records come from. A user reading a count in the hundreds had no way to tell a careless install habit from a repo producing them on its own. A review pass against the current Claude Code docs (plugins, plugins-reference, settings-reference, cloud-environments, discover-plugins, claude-directory, changelog through 2.1.261) established the mechanism; this PR writes it down where the skill reasons about those records. ## Fix - New `scope-semantics.md` subsection "Where project-scope records come from, and why the skill cannot reap them". A repo's committed `.claude/settings.json` `enabledPlugins` block is the documented cloud install mechanism; project scope outranks user scope in `enabledPlugins` precedence; so every project-scope `true` that duplicates a user-scope install gets its own version-pinned record keyed by that checkout's absolute path. Nothing reaps the result: `git worktree remove` does not touch `~/.claude`, no CLI verb removes a record by path, and the documented retention sweep covers nothing under `~/.claude/plugins/`. Changelog 2.1.224 shows per-project records are a live mechanism. Synced plugins (`<name>@synced`, cloud and Cowork only, no install record) are recorded as the contrast case. - Two questions are recorded as open probes, not asserted: which code path writes the records locally (one machine's 64 records sharing a single `installedAt` second is written as an observation on 2.1.261, with the probe that would settle it named), and whether a project-scope `false` writes any record. - `SKILL.md`'s stale-records section gains a short pointer to the subsection and keeps its existing boundary that the tool owning the directories is where records should be dropped. `gotchas.md` gains a gotcha on a committed block manufacturing records, citing the subsection. No remediation is proposed anywhere; that decision is open on #3688. - claude-ops `0.42.13` -> `0.42.14` with CHANGELOG entry. Docs only; no script changed; SKILL.md frontmatter unchanged. ## Verification - `markdownlint-cli2` over `plugins/claude-ops/**/*.md`: 0 issues in 54 files. `typos`: clean. `node scripts/validate-plugin-contracts.mjs`: pass (the retirements append-only sub-check reports skipped locally because `VALIDATE_CONTRACTS_BASE_REF` is unset; CI sets it). - SKILL.md frontmatter diffed byte-identical against `origin/main`. No em dashes in added prose. ## Related - #3688 (the decision this documents; stays open) - #3760 (re-verified the no-reap-by-path claim this section builds on) - #3775 (cache-content check, which skips absent project paths on the same reasoning) 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_019gWgHogJFQeCne5U7vHKAE --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Closes #3681
Summary
claude plugin updatere-points an install record'sgitCommitShaininstalled_plugins.jsonwithout rewriting the plugin's cache directory when the manifest version number is unchanged across the two commits, because the cache is keyed by version. The record then claims the new commit while the directory still holds the older build, and the version-and-sha check every delivery step relies on passes in exactly that state. On the reporting machine six plugins were in that state at once, twelve stale files in the worst case, including a reviewed hook dispatcher and twohooks.jsonfiles.The issue asked for a decision between (a) a repo rule that every plugin change bumps the version, (b) promoting the file-level compare into the claude-ops plugin-fleet audit as a standing check, or (c) an upstream report. This takes (b). (a) already exists as the CHANGELOG-parity gate and only bites when a branch is delivered without a bump, so it cannot catch the case that motivated the issue; (c) is worth filing but does nothing for machines today, and the updater's behaviour on an unchanged version may be intentional. (b) is local, cheap, and durable: every record carries the sha and the marketplace's
installLocationis a git clone, so the files can be compared byte for byte.Fix
cache-content-check.shin thepluginsskill: a read-only per-install compare of every file under a cache directory against the recorded commit in the marketplace clone. Two git processes per install (git ls-tree -r <sha>for the expected blob ids, onegit hash-object --stdin-pathsbatch for the cache files), set difference plus hash inequality. Detects changed files, files the commit has that the cache lacks, and files deleted at the commit but still in the cache. Honours the marketplace's.gitignore(a live plugin root carries__pycache__andnode_modules, which the first real run flagged as stale until this filter existed) and re-hashes mismatches with--pathso.gitattributesCRLF rules do not produce false positives. Never writes: nogit fetch(a sha absent from the clone reportssha-not-local), noclaude plugincall. Same CR-safe--idscontract asfleet-state.sh, exit 2 on usage or malformed input.sync.mdgains Step 5b running the check in bothsyncandaudit(from the run journal or scratch dir respectively), ungated, because an unchanged version number is precisely the case in which every other step reports success. SKILL.md's Report template gains aCache content:row naming affected ids and the remediation the issue proved (remove that version directory under the cache and re-runclaude plugin update <id>); the check never repairs.scope-semantics.mdrecords the mechanism (stamped Claude Code 2.1.259, issue evidence, not re-run) and the shallow-clone fact observed on 2.1.261: the marketplace clone under~/.claude/plugins/marketplaces/carries.git/shallowwith three commits, so records naming older commits are unverifiable without a fetch the audit refuses to perform.0.42.12->0.42.13with CHANGELOG entry (main took 0.42.8 through 0.42.12 mid-flight).Verification
cache-content-check.test.sh: 24 cases, 0 failed. Fixture-driven with a throwaway git repo asinstallLocationat two commits: match; older file in cache -> stale-content naming the path; file deleted at sha still in cache -> stale-content; sha not in clone -> sha-not-local;--idsemits only stale ids CR-free; malformedinstalled_plugins.json-> exit 2; unknown marketplace -> exit 2 naming it; absent project path skipped and counted; misaligned hash batch refused rather than compared (stub git truncating the hash batch); plugin directory renamed between commits still matches because the manifest is read at the recorded sha; a pathspec matching nothing at the sha isno-source-at-sha, not stale; non-ASCII filename round-trips NUL-safe; tracked symlink hashed mode-aware (skips where the host cannot create one); record without gitCommitSha yieldsno-git-commit-sha; process-count probe has a floor so it cannot pass vacuously.fleet-state.test.sh: 86 cases, 0 failed.shellcheck -S infoclean on both new scripts.typosclean.markdownlint-cli2: 0 issues in 54 files.validate-plugin-contracts.mjs: pass. Test suites are auto-discovered byscripts/run-plugin-tests.sh, no registry edit needed.--marketplace melodic-software --scope user-> checked 74, match 63, stale_content 0, unverifiable 11 (allsha-not-local, the shallow-clone case above), skipped_absent_project_paths 0.scripts/affected-tests.sh --runnot run locally (hangs oncheck-changed-skills.test.shon this host); CI is the statement of record.Related
🤖 Generated with Claude Code
https://claude.ai/code/session_019gWgHogJFQeCne5U7vHKAE