fix(repo-fleet-hygiene): degrade non-repo paths under --root by kyle-sexton · Pull Request #2630 · melodic-software/claude-code-plugins

kyle-sexton · 2026-08-14T22:54:34Z

Summary

A single non-repository or unreadable path under --root aborted the entire fleet audit, making realistic trees (e.g. a drive root) unusable.

Fix

Degrade discovery-sourced non-repo / unreadable paths per-entry (continue the audit; surface skip/unknown) instead of hard-failing the run.

Verification

See audit-fleet.test.sh on PR checks.

This PR changes audit-fleet.sh's error handling so a single non-repository or unreadable path discovered under --root degrades to a per-entry UNKNOWN/discovery-skip finding instead of aborting the whole run, plus matching doc/changelog/test updates. It's a read-only, local-filesystem audit tool (no mutating Git/GitHub operations, per its own Mode: read-only banner). Reviewed the diff for the usual concerns:

Injection: no eval, no bash -c/sh -c, no unquoted expansions introduced. New/changed printf calls all use literal %s format strings with variables passed as arguments, not as the format itself — no format-string injection even for attacker-controlled directory names.
Path/command handling: discover_repositories, add_target, reject_target, and the new record_rejected_target/discovery_skip_reason helpers pass $candidate/$dir through quoted variable expansions into git -C "$candidate" ... via the existing allowlisted run_git_probe wrapper — consistent with the pre-existing pattern, no new probe surface.
Readability checks: the new [[ ! -r "$dir" || ! -x "$dir" ]] gate only counts/skips a path; it doesn't change privilege, doesn't follow into unreadable directories, and doesn't affect the existing -L symlink exclusion.
No GitHub Actions, permissions, or secrets-handling changes — .claude-plugin/plugin.json is a version bump only; the rest of the diff is docs/CHANGELOG/tests.

No trust-boundary, authorization, or injection concerns identified in this diff.

github-actions · 2026-08-14T22:56:52Z

Last security-reviewed head: 3304b3028010812a773625bff61280c767602d5a. 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: 657901d74b

ℹ️ 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 · 2026-08-14T23:20:38Z

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

Security Review

Gather context (PR diff, changed files)
Invoke /review:security-review and follow its instructions
Report findings (or confirm none)

No security issues found.

Reviewed the full PR diff (git diff origin/main...HEAD) against /review:security-review criteria: injection surfaces, trust-boundary/authorization gaps, unsafe credential/token handling, and dangerous GitHub Actions patterns.

audit-fleet.sh (the only behavioral change): the new discovery_skip_reason, record_rejected_target, and the unreadable-directory gate in discover_repositories all operate on quoted variables ("$dir", "$candidate") inside [[ ]] tests — no unquoted expansion, no eval, no new git/chmod/external-command invocations. The one new git call path (add_target "$dir" discovery) reuses the existing run_git_probe allowlisted wrapper unchanged.
The tool remains read-only/local-filesystem only; this PR changes error-handling flow control (per-entry degrade vs. hard-fail), not what is read or executed. Symlink exclusion (! -L) and the recursion depth guard are untouched.
plugin.json, CHANGELOG.md, SKILL.md, confidence-model.md changes are docs/version-bump only.
audit-fleet.test.sh changes are test-fixture-only (mock git, root-tolerant unreadable-dir probe) — no production code path.
The one prior review comment (Codex, P2: unreadable fixture ineffective under root/CI) was a test-flakiness concern, not a security finding, and is already addressed by the follow-up commit 3304b30 (tolerate root for unreadable discovery fixture), which now dynamically probes whether chmod a-rwx actually denied access before asserting the count.

No trust-boundary, injection, or authorization concerns identified in this diff.

claude · 2026-08-14T23:43:12Z

Claude finished @cursor[bot]'s task in 3m 35s —— View job

Code Review

Gather context (PR diff, changed files)
Apply skip gate (PR is open, non-draft, non-trivial — proceeding)
Review changed files for correctness/maintainability
Independent verification pass
Report findings

Reviewed audit-fleet.sh's new discovery-origin degrade path (reject_target, record_rejected_target, discovery_skip_reason, the readability gate in discover_repositories) and the matching test/doc updates. Running one more independent verification pass on the counting/branching logic before finalizing.

github-actions · 2026-08-14T23:47:13Z

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

Closes #2598. Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>

claude · 2026-08-14T23:56:57Z

Claude finished @cursor[bot]'s task in 30s —— View job

Code Review

Gather context (PR diff, changed files)
Invoke /review:code-review and follow its instructions
Report findings

Skip gate triggered: PR is closed (merged).

Per /review:code-review's skip gate, review stops early when the PR is closed or not open — this PR was already merged, so there's no open PR to review or post findings against. No further action taken.

Rebased onto main preserving discovery-skip degradation (#2598/#2630) while adding bare-repo-with-working-tree classification (#2602). Version 0.14.2. Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>

No linked issue ## Summary A post-merge canary for the failure class in #2691: a merge that silently deletes content another recently-merged commit had just added, leaving no `Revert:` marker and no failing test. Item 4 of that issue. Detection only — it runs on `push` to `main`, is not in `ci.yml`, and is not wired into `ci-status`, so it can never gate a merge. **It also corrects the issue's premise.** This was not a stale-*base* failure, which means `strict_required_status_checks_policy` would not have prevented it. Details below. ## Fix `scripts/check-silent-revert.sh` blames the lines each merge deleted against its own parent and reports when a large block traces to a **single** commit inside a recency window. Plus `scripts/check-silent-revert.test.sh` (26 hermetic cases), two data files, and `.github/workflows/silent-revert-canary.yml`. ### Why blame-of-deleted-lines, and not the alternatives Three designs were measured against the real history before one was chosen. **Merge-base staleness — tested and rejected on evidence.** It exonerates all three real incidents: ``` $ git merge-base --is-ancestor f603880 refs/pull/2641/head && echo YES YES $ git grep -c "read-only supporting allowlist" refs/pull/2641/head -- 'plugins/disk-hygiene/**' (no output — zero occurrences) $ git grep -c "read-only supporting allowlist" f603880 -- 'plugins/disk-hygiene/**' f603880:plugins/disk-hygiene/skills/clean/SKILL.md:1 f603880:plugins/disk-hygiene/skills/clean/scripts/destructive_guard.py:1 ``` PR #2641's head had #2639 **in its ancestry** and **zero occurrences of #2639's content in its tree**. #2639 shows the identical shape against #2635. These branches were up to date with `main` in *history* and stale only in *content* — a bad conflict resolution or a force-push from an older worktree. So `strict` would have passed all three merges, and a merge queue would have too (CI was green — the tests were deleted alongside the code). **That reframes the canary: for this class it is not defence in depth behind a real fix, it is the only control that fires at all.** No ruleset is touched here and nothing in `github-iac` changes; ADR 0001 stands as written. **Curated marker strings (the issue's own suggestion 3) — rejected.** Only catches what someone pre-registered, and nobody had registered #2632, #2635 or #2639. Registration happens *after* you know a fix matters, which is the knowledge the incident destroys. **PR-creation-time overlap — rejected as non-discriminating.** At the 17-concurrent-PR rate ADR 0001 records, nearly every PR has siblings landing while it is open. ### False-positive strategy A canary that cries wolf gets disabled, which is worse than none. 1. **Volume**, aggregated **per culprit commit** — summing across culprits would re-admit ordinary iteration. 2. **Recency window** (40 first-parent commits). Stated limitation: content reverted from outside the window is missed by design. 3. **Intent, in constrained forms only** — a `Revert "` subject, a `This reverts commit <sha>` line, or an explicit `Intentional-removal:` trailer. Deliberately *not* a substring search for "revert": a body reading "this does not revert X" would silence a real finding. 4. **Non-blocking** — post-merge only, outside `ci-status`. **Measured, not assumed** — and measured by running *the shipped script itself* over the last **500** first-parent commits of `main`, not a stand-in: ``` FIRE 853 cc58cbc fix(repo-fleet-hygiene): report bare repos with live working trees (#2633) FIRE 451 9239f15 feat(disk-hygiene): verify redundant checkout evidence (#2641) FIRE 390 6f0a311 fix(repo-fleet-hygiene): restore GraphQL merge evidence and rollups after #2633 FIRE 346 f603880 fix(disk-hygiene): session-honest belt, read-only allowlist, ... (#2639) FIRE 340 91e77fc fix(hook-utils): stop a NUL in a payload value from voiding two blocking guards MEASUREMENT COMPLETE over 500 commits ``` **5 fires in 500 merges — 1%**, zero errors. Three are the confirmed incidents. The other two are real and are not detector bugs: #2640 (390) is the manual *restore* of #2633's revert, and #2135 (340) is a deliberate merge reconciliation the author argues at length in the PR body. Both are pre-recorded in the acknowledgment file so `main` starts green. **Rename detection is deliberately left ON.** An earlier revision passed `--no-renames`, which silently made the shipped detector a *different* detector from the calibrated one: without detection a `git mv` decomposes into delete + add, the delete side reaches the `--diff-filter=MD` enumeration as a whole-file removal, and relocating a large file a recent commit had added would fire. In a repo that restructures skills and docs this often, that is a live false-positive class. With detection on, the shipped script reproduces the calibration corpus exactly (the five rows above), and a test pins the rename case. The narrow cost, stated rather than hidden: content gutted in the same commit that renames its file is not attributed. **The uncomfortable part, stated plainly: 340 is the largest false positive and 346 is the smallest true one. No threshold separates them.** Picking a number inside that 2% gap would be overfitting, so the threshold is 200 — which costs nothing (200 and 300 fire on the identical five commits) and leaves headroom for a smaller future revert. Precision is traded for recall because the miss is expensive and the fire is cheap. Cheap requires a disposition path in **both** directions in time, so there are two: the prospective `Intentional-removal:` trailer (one line in the PR body, which GitHub carries into the squash message), and `scripts/silent-revert-acknowledged.txt` for a fire that can only be judged after the fact — a commit message cannot be amended post-merge. Without the second, one legitimate fire leaves the canary permanently red, and a permanently red canary is one on its way to being deleted. It matches `changelog-parity-baseline.txt` in shape, matches full 40-char SHAs only, and requires a recorded reason, so it is an audit trail rather than a mute button. ### A third incident, previously unfiled Calibration surfaced one #2691 never identified: **#2633's squash dropped #2632's finding rollups (853 lines)**, which #2640 restored by hand the same night. Nobody filed it. That is the clearest argument for automating the detection. ## Verification Real output, all from this branch. **Catches the actual incident** (requirement 1) — replayed against the real merges, and pinned in `scripts/silent-revert-incidents.txt` so CI re-proves it on every run: ``` $ scripts/check-silent-revert.sh --verify-known-incidents ok f603880 fires as recorded (#2639 dropped #2635 (346 lines), 13 minutes later) ok 9239f15 fires as recorded (#2641 dropped #2639 (451 lines), 10 minutes later) ok cc58cbc fires as recorded (#2633 dropped #2632's rollups (853 lines) -- unfiled until now) ok c8470ef stays clean as recorded (docs(conventions) rewrote 129 lines of a doc #2679 had just added) Canary reproduces every recorded incident at the shipped settings. ``` Range mode over the incident window — both fire, the interleaved unrelated merges stay clean: ``` $ scripts/check-silent-revert.sh a95f240~1..9239f15 ok a95f240 feat(disk-hygiene): prioritize tidiness over reclaimable bytes in reports (#2635) ok b7793d3 fix(repo-fleet-hygiene): degrade non-repo paths under --root (#2630) SILENT REVERT SUSPECTED removed by f603880 fix(disk-hygiene): session-honest belt, ... (#2639) content from a95f240 feat(disk-hygiene): prioritize tidiness over ... (#2635) lines lost 346 (threshold 200, window 40 commits) ok eda5ae5 feat(repo-fleet-hygiene): gather merge evidence via aliased GraphQL (#2642) SILENT REVERT SUSPECTED removed by 9239f15 feat(disk-hygiene): verify redundant checkout evidence (#2641) content from f603880 fix(disk-hygiene): session-honest belt, ... (#2639) lines lost 451 (threshold 200, window 40 commits) ``` **Actionable when it fires** (requirement 4) — it names what disappeared, which commit removed it, which commit added it, the per-file split, and the sample quotes back the very content #2691 reported as lost: ``` by file: 235 plugins/disk-hygiene/skills/clean/scripts/destructive_guard.py 156 plugins/disk-hygiene/skills/clean/scripts/test_hygiene.py 26 plugins/disk-hygiene/skills/clean/SKILL.md ... sample of the removed content: | - The skill-frontmatter guard is a fail-closed allowlist. It permits canonical bundled scan/preview | calls made from literal shell words, a small read-only supporting set for cleanup inspection ``` **Tests** (requirement 5) — `scripts/check-silent-revert.test.sh`, hermetic synthetic repos, following the `scripts/*.test.sh` + `test-git-helpers.sh` convention. 27 cases: attribution, per-culprit aggregation, the recency window, the pure-rename negative, every intent form, the "prose mentioning revert must still fire" negative, the empty-trailer negative, abbreviated-SHA rejection, whole-file deletion, the shipped default in both directions, and fail-closed exit 2 on an unresolvable range, a malformed range, and an unreachable pinned commit. ``` $ bash scripts/check-silent-revert.test.sh check-silent-revert.test.sh: 27 passed, 0 failed ``` **Verified in real CI on this PR, not just locally.** The lane runs on `pull_request` (scoped by `paths` to the canary's own files) so the detector is exercised before it lands — the scan step is gated off for PR events, so nothing on a PR inspects that PR. From the actual run log ([job](https://github.com/melodic-software/claude-code-plugins/actions/runs/31918399785/job/95094057983), 12s): ``` Test the silent-revert detector check-silent-revert.test.sh: 27 passed, 0 failed Replay the recorded incidents ok f603880 fires as recorded (#2639 dropped #2635 (346 lines), 13 minutes later) Replay the recorded incidents ok 9239f15 fires as recorded (#2641 dropped #2639 (451 lines), 10 minutes later) Replay the recorded incidents ok cc58cbc fires as recorded (#2633 dropped #2632's rollups (853 lines) -- unfiled until now) Replay the recorded incidents ok c8470ef stays clean as recorded (docs(conventions) rewrote 129 lines ...) Replay the recorded incidents Canary reproduces every recorded incident at the shipped settings. ``` That run matters for a specific reason. The blame-header pattern originally used an ERE interval (`{40}`), and interval support is an awk-implementation variable — the runner's default awk is mawk, development machines run gawk. Had it not matched, attribution would emit nothing and **every commit would report `ok`**: a false green, the exact failure class this canary exists to remove. It is now interval-free and proven against the runner's own awk above. **Cannot block a merge — verified, not just asserted.** The required contexts on `main` are: ``` $ gh api repos/melodic-software/claude-code-plugins/rules/branches/main \ --jq '.[] | select(.type=="required_status_checks") | .parameters.required_status_checks[].context' pr-title / pr-title pr-issue-linkage / pr-issue-linkage do-not-merge / do-not-merge ci-status ``` `Silent-revert canary` is not among them, and its only appearance in `ci.yml` is the `workflow_schema` filename list — never `ci-status`'s `needs`. A non-required check cannot gate a merge. **Repo gates**, all run locally on this branch: `shellcheck --rcfile=.shellcheckrc -x` (exit 0, and this repo enables `require-double-brackets` and `add-default-case`), `check-shell-portability.sh --paths` and `--all` (exit 0), `zizmor` (no findings), `actionlint` (exit 0), `typos` (exit 0), `editorconfig-checker` (exit 0), `check-jsonschema --builtin-schema vendor.github-workflows` (ok), `check-silent-skips.sh` (exit 0). The new workflow is registered in `ci.yml`'s `workflow_schema` file list; the shell scripts are committed `100755`. ### Design notes for review - **No cancelling `concurrency` group**, unlike `ci.yml` — a cancelled canary run is a silently missed detection. - **Fail-closed on an unusable push range.** `github.event.before` is all-zeros on a first push or history rewrite; the workflow falls back to the head commit and emits a `::warning::` saying earlier commits were not scanned, rather than reporting a clean scan of nothing. An unresolvable range exits **2**, never 0. - **Self-test runs before every scan**, so a broken detector cannot mask a regression behind a green canary — the same never-skip, self-test-first shape the `ci.yml` gates use. - **What runs on a PR is the detector's unit tests, never detection.** The `pull_request` trigger is `paths`-scoped to the canary's own five files, so it is inert on every other PR, and both scan steps carry `if: github.event_name != 'pull_request'`. - **Remaining limitations, stated rather than hidden:** content reverted from outside the 40-commit recency window is missed by design; content gutted in the same commit that renames its file is not attributed; and no threshold separates a large deliberate rewrite from a silent revert, which is what the acknowledgment file exists to absorb. ## Related Refs #2691 — this is item 4; the issue covers more and stays open. Refs #2713 — the docs-only silent-revert blind spot, same class from the test-coverage side. Refs melodic-software/github-iac `docs/adr/0001-relax-strict-required-status-checks.md` — unchanged; the evidence above argues it was never the relevant control for this failure mode. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>

kyle-sexton enabled auto-merge (squash) August 14, 2026 22:54

chatgpt-codex-connector Bot reviewed Aug 14, 2026

View reviewed changes

Comment thread plugins/repo-fleet-hygiene/skills/audit/scripts/audit-fleet.test.sh

cursor Bot force-pushed the cursor/fix-fleet-nonrepo-degrade-f8b0 branch 2 times, most recently from c80b244 to 35cc547 Compare August 14, 2026 23:12

kyle-sexton force-pushed the cursor/fix-fleet-nonrepo-degrade-f8b0 branch from 35cc547 to 3304b30 Compare August 14, 2026 23:20

cursor Bot force-pushed the cursor/fix-fleet-nonrepo-degrade-f8b0 branch from 3304b30 to fc668d0 Compare August 14, 2026 23:33

fix(repo-fleet-hygiene): degrade non-repo paths under --root

de55431

Closes #2598. Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>

cursor Bot force-pushed the cursor/fix-fleet-nonrepo-degrade-f8b0 branch from fc668d0 to de55431 Compare August 14, 2026 23:49

kyle-sexton merged commit b7793d3 into main Aug 14, 2026
39 checks passed

kyle-sexton deleted the cursor/fix-fleet-nonrepo-degrade-f8b0 branch August 14, 2026 23:55

claude Bot mentioned this pull request Aug 15, 2026

fix(repo-fleet-hygiene): report bare repos with live working trees #2633

Merged

claude Bot mentioned this pull request Aug 15, 2026

fix(repo-fleet-hygiene): restore GraphQL merge evidence and rollups after #2633 #2640

Merged

Navigation Menu

Uh oh!

fix(repo-fleet-hygiene): degrade non-repo paths under --root - #2630

fix(repo-fleet-hygiene): degrade non-repo paths under --root#2630
kyle-sexton merged 1 commit into
mainfrom
cursor/fix-fleet-nonrepo-degrade-f8b0

kyle-sexton commented Aug 14, 2026

Uh oh!

cursor Bot commented Aug 14, 2026

Uh oh!

claude Bot commented Aug 14, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Aug 14, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

Uh oh!

claude Bot commented Aug 14, 2026 •

edited

Loading

Uh oh!

claude Bot commented Aug 14, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Aug 14, 2026 •

edited

Loading

Uh oh!

Uh oh!

claude Bot commented Aug 14, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

kyle-sexton commented Aug 14, 2026

Summary

Fix

Verification

Related

Uh oh!

cursor Bot commented Aug 14, 2026

Uh oh!

claude Bot commented Aug 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Security Review

Uh oh!

github-actions Bot commented Aug 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

claude Bot commented Aug 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Security Review

Uh oh!

claude Bot commented Aug 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review

Uh oh!

github-actions Bot commented Aug 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

claude Bot commented Aug 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

claude Bot commented Aug 14, 2026 •

edited

Loading

github-actions Bot commented Aug 14, 2026 •

edited

Loading

claude Bot commented Aug 14, 2026 •

edited

Loading

claude Bot commented Aug 14, 2026 •

edited

Loading

github-actions Bot commented Aug 14, 2026 •

edited

Loading

claude Bot commented Aug 14, 2026 •

edited

Loading