feat(disk-hygiene): prioritize tidiness over reclaimable bytes in reports - #2635
Merged
Merged
Conversation
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
kyle-sexton
enabled auto-merge (squash)
August 14, 2026 23:00
|
Claude finished @kyle-sexton's task in 1m 54s —— View job Security review — no issues found
This PR only touches I read the deletion-safety path in
No injection, authorization, path-traversal, or trust-boundary issues found in this diff. |
|
Last security-reviewed head: |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 61c2b8c83b
ℹ️ 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".
cursor
Bot
force-pushed
the
cursor/feat-disk-hygiene-tidiness-report-f8b0
branch
2 times, most recently
from
August 14, 2026 23:15
e3cbd05 to
fe9070e
Compare
kyle-sexton
force-pushed
the
cursor/feat-disk-hygiene-tidiness-report-f8b0
branch
from
August 14, 2026 23:21
fe9070e to
3f313be
Compare
|
Claude finished @kyle-sexton's task in 1m 59s —— View job Security review — no issues found
This PR's diff ( A prior review comment on this thread reviewed an earlier commit on this branch ( Deletion-safety path re-confirmed at the new head:
On the two Codex review comments ( No injection, authorization, path-traversal, or trust-boundary issues found in this diff. |
|
Claude finished @kyle-sexton's task in 1m 37s —— View job Security review — no issues found
This PR only touches I re-verified the deletion-safety path end to end against the new head, including the changes since the last review:
No injection, authorization, path-traversal, or trust-boundary issues found in this diff. |
cursor
Bot
force-pushed
the
cursor/feat-disk-hygiene-tidiness-report-f8b0
branch
from
August 14, 2026 23:33
83d3b14 to
d5c7014
Compare
…orts Closes #2590. Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
cursor
Bot
force-pushed
the
cursor/feat-disk-hygiene-tidiness-report-f8b0
branch
from
August 14, 2026 23:49
d5c7014 to
61d7f5b
Compare
|
Claude finished @cursor[bot]'s task in 23s —— View job CI code review — skipped
Skip gate triggered: PR is closed (merged). Per |
|
Claude has reviewed this PR 1 time. The lane skips further automatic reviews after 5; deleting this comment resets the count. |
This was referenced Aug 15, 2026
Closed
cursor Bot
pushed a commit
that referenced
this pull request
Aug 15, 2026
…elt's documented posture Re-lands the report-ordering fix from #2635 (reverted by #2639's stale base) and the session-lifetime honesty from #2639 (reverted by #2641's stale base), then corrects three further documentation defects in the clean skill's two operator- facing markdown surfaces. F1 (#2590) — reports are ordered by tier and evidence strength, never by byte size: provenance/what-it-is/why-removable/risk lead, bytes come last; empty directories are first-class findings distinguished from not-walked coverage gaps; `provenance` and `risk` return to the plan schema; §5's preview table and §6's apply summary lead with tidiness rather than bytes. F5(a) (#2618) — the frontmatter PreToolUse belt is session-lifetime, not skill-scoped. Both markdown sites now say so, sourced from the skills reference ("registers when the skill is invoked and keeps running for the rest of the session"), and name the allowed-tools/hooks asymmetry that made the narrower claim plausible. The third site (destructive_guard.py's resolve_mode docstring) is covered by a sibling PR. F6 (#2618) — SKILL.md claimed the belt "still launches in exec form via python3", contradicting its own frontmatter and safety-model.md: it has been shell form since 0.17.9 (#2568). The false statement and the conclusions drawn from it (a silently-inert belt, defense-in-depth "lost, not preserved", #2568 unconverted) are removed rather than reworded; safety-model.md's accurate account, including the real residual fail-open, is the single copy. F4 (#2618) — step 6.2 listed three ways reversible removal silently becomes permanent but omitted path length, which fails differently: beyond MAX_PATH (260) a path cannot reach the Recycle Bin at all, so the only fallback is a permanent delete through a long-path API. That fallback now requires its own explicit irreversible-action approval instead of inheriting the tier approval given for reversible removals. F3 (#2618) — §3 now states that relocation is out of scope: the skill offers keep-or-delete only, and a move is the operator's own action outside the workflow. F7 (#2618) — the Gotchas section carried ~56 lines of harness mechanics already documented in full by reference/safety-model.md; hand-maintained duplication is how F6's stale bullet survived a fix to the reference. Those bullets are replaced with load-when pointers, leaving the engine-behavior and operator- actionable gotchas in place. Gotchas 77 -> 35 lines; SKILL.md 490 -> 495 net, the other findings having added required content, and back under the 500-line skill-quality cap it had 10 lines of headroom against. Also corrects safety-model.md's claim that Move-Item/Rename-Item "reach the tool with no guard verdict at all" — stale in the unsafe-sounding direction since #2470 gated move/rename/overwrite/volume spellings and closed #387. The lane is still enumerated rather than fail-closed, so the residuals are named concretely. Closes #2590 Refs #2618 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Aug 16, 2026
kyle-sexton
added a commit
that referenced
this pull request
Aug 16, 2026
## Summary #2590 stayed open: #2714 restored skill prose, but the engine half from #2635 (`empty_directory_count`, plan `provenance`/`risk`, preview/apply tidiness fields) was still absent on `main`. Re-lands that engine work and tests. ## Test plan - [x] `python3 -m unittest test_hygiene.HygieneTests` — 94/94 pass (focused empty-dir / provenance / apply cases included) ## Related Closes #2590 Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
kyle-sexton
added a commit
that referenced
this pull request
Aug 16, 2026
## Summary Adds a PR CI gate (`scripts/check-stale-base-overlap.sh`) that fails when the head is behind its base on overlapping paths — the failure mode that silently reverted #2635/#2639 with CI green. Documents the babysit/pull-request rule: never squash-merge while behind, even if `mergeStateStatus` is CLEAN under a non-strict ruleset. Points at `requiredStatusChecks.strict` in `melodic-software/github-iac` as the durable prevention. ## Test plan - [x] `bash scripts/check-stale-base-overlap.test.sh` (fresh / overlap / disjoint / bad-ref) - [ ] CI `stale-base-overlap-gate` on this PR ## Related Closes #2691 --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
kyle-sexton
added a commit
that referenced
this pull request
Aug 16, 2026
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
pushed a commit
that referenced
this pull request
Aug 16, 2026
…fers to The 0.54.9 entry quoted the removed "#2635 then #2639" citation and then said "all three incidents" without naming them. Name the three reverting merges (#2633, #2639, #2641) and mark the earlier numbers as a quotation of the text being removed. Changelog prose only. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Aug 16, 2026
Merged
kyle-sexton
added a commit
that referenced
this pull request
Aug 16, 2026
Review found that eval 12 as #2635 wrote it marks the correct gated response as a failure. Its prompt targeted `C:\` directly, but SKILL.md requires an OS-managed volume root to be addressed through `--root-children` with explicit `--root-child` names after the confirmation gate clears, and the engine rejects a direct root scan. An eval whose expected output is only reachable by bypassing that gate rewards bypassing it. Point the prompt at five explicitly selected children so the tidiness ranking is reachable inside the documented safe flow, and add a fourth expectation pinning that the ranking request is never authority to skip the gate. Root gating itself stays eval 11's subject. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RPRAvSLuRmR6rPQeCvdok6
kyle-sexton
added a commit
that referenced
this pull request
Aug 16, 2026
…trict recommendation (#2830) ## What Wording-only correction to two defects in how `scripts/check-stale-base-overlap.sh` (shipped in #2799) describes itself, and to the same framing where it had propagated. The gate itself is unchanged and stays required. It correctly detects the stale-**base** class. ## Defect 1 — false coverage citation The header cited `claude-code-plugins#2691: #2635 then #2639 lost inside ten minutes` as its motivating incident. Those incidents are a different class: ``` git merge-base --is-ancestor f603880 refs/pull/2641/head # exit 0 — #2639 WAS in the ancestry git grep -c "read-only supporting allowlist" refs/pull/2641/head # zero — the tree lacked the work ``` The bases were fresh; the trees were stale. Run against those exact branches this gate exits 0 ("fresh"). A required check was naming incidents it does not detect. The class that covers them is `scripts/check-silent-revert.sh` (#2808). The two cover **disjoint** classes; neither subsumes the other. Every touched surface now says that explicitly. ## Defect 2 — a recommendation barred by an accepted ADR The header called GitHub's `requiredStatusChecks.strict` "the durable prevention". That is wrong twice over: - It is barred. `melodic-software/github-iac` → `docs/adr/0001-relax-strict-required-status-checks.md` is ACCEPTED and implemented as a non-strict ruleset; its revisit trigger prescribes per-repo granularity, not a global flip. - It is false. The measurement above shows strict — and a merge queue — would have **passed** all three recorded incidents. Replaced with a neutral statement of scope. Nothing here endorses the setting, softly or otherwise, and nothing recommends flipping it. `.github/workflows/silent-revert-canary.yml` and `scripts/check-silent-revert.sh` already carried the accurate framing and were deliberately left untouched. ## Files | File | Change | | --- | --- | | `scripts/check-stale-base-overlap.sh` | Header "Why" rewritten with an explicit SCOPE paragraph; the "Durable prevention" stderr line replaced with a neutral scope statement | | `.github/workflows/ci.yml` | Job comment only — no logic, wiring, or `needs` touched | | `plugins/source-control/skills/babysit-prs/reference/freshness.md` | Dropped the "#2635 then #2639 ... inside ten minutes" citation and the "when the durable `strict` setting is not yet on" framing | | `plugins/source-control/skills/pull-request/reference/merge.md` | Dropped "the durable fix is `strict: true`" | | `plugins/source-control/CHANGELOG.md` | New 0.54.9 entry; 0.54.7's trailing clause amended because it actively recommended the barred setting rather than merely recording a release | | `plugins/source-control/.claude-plugin/plugin.json` | 0.54.8 → 0.54.9 (changelog-parity) | ## Verification - `bash -n scripts/check-stale-base-overlap.sh` — OK. The header sits directly above `set -uo pipefail`; parsing is intact. - `bash scripts/check-stale-base-overlap.test.sh` — **ALL PASS**, exit 0 (all five cases). - No behavioral change. The only non-comment lines in the diff are stderr prose inside two `echo` strings. - `plugins/disk-hygiene/CHANGELOG.md:83` describes the incidents as "squash-merged from stale bases" — the same wrong premise, but it is another plugin's release history and makes no claim about this gate's coverage. Left alone deliberately. ## Related No linked issue. This corrects wording on surfaces shipped by an already-merged PR; nothing here closes a GitHub issue. - #2799 — merged the gate and the incorrect header wording this PR corrects. - #2808 — merged `scripts/check-silent-revert.sh`, the detector that actually covers the disjoint class. - #2691 — the audit whose incidents were miscited. CLOSED as COMPLETED and deliberately left closed; referenced for context only, with no closing keyword. - `melodic-software/github-iac` → `docs/adr/0001-relax-strict-required-status-checks.md` — the ACCEPTED ADR that bars the recommendation removed here. Not touched by this PR. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
kyle-sexton
added a commit
that referenced
this pull request
Aug 16, 2026
…hangelog The entry said eval 12 "is back" while scoping the not-restored-verbatim caveat to the README approval bullet alone. Three of the four restored surfaces are corrected rather than restored: the approval bullet, the deletion-report enumeration, and eval 12's prompt. Only eval 1's expectation is byte-identical to #2635. In a repo that keeps scripts/silent-revert-incidents.txt precisely because "restored" records went unaudited, a changelog that misstates what was restored is the failure mode this change exists to correct. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RPRAvSLuRmR6rPQeCvdok6
kyle-sexton
added a commit
that referenced
this pull request
Aug 16, 2026
…2828) (#2829) ## Summary PR #2635 changed **seven** files. The stale-base squash in #2639 deleted them all 13 minutes later — the incident already recorded in `scripts/silent-revert-incidents.txt`. PR #2714 restored four and PR #2803 restored four; the overlap left `plugins/disk-hygiene/README.md` and `plugins/disk-hygiene/skills/clean/evals/evals.json` unrestored on `main`. The silent-revert canary cannot see this. It detects deleting commits, and the deleting commit here (`f603880da`) is already an acknowledged incident — it has no mechanism for noticing that a recorded incident's restoration was only **partial**. ## What changed - **`README.md`** — all three #2635 hunks: - the overview now states that safe tidiness is the primary objective and reclaimable bytes are a secondary signal; - the approval-contract bullet now names provenance, what the entry is, why it is removable, and risk, with byte counts last; - the deletion-report paragraph now leads with tidiness outcomes and records bytes as secondary. - **`evals.json`** — eval 12 `empty-directories-remain-first-class-tidiness-findings` restored, and eval 1's third expectation restored to the provenance-first wording. - Version bumped to `0.20.10` with a CHANGELOG entry. ## Three of the four surfaces are corrected, not restored verbatim Only eval 1's expectation is byte-identical to #2635. The other three each deviate deliberately, and each deviation is disclosed in the CHANGELOG as well as here. **1. The README approval bullet — #2635's own hunk was malformed.** It rewrote the bullet while leaving the original two trailing lines in place and duplicating the following "Filesystem roots..." bullet in truncated form. Restoring it byte-for-byte would re-introduce a broken list. The replacement matches `SKILL.md` §5, which is the authority on what the approval table names: > show a table naming every path with provenance, what it is, why removable, risk, whether it is an > empty directory, the single tier, and only then logical / reclaimable bytes That is also why `and its logical byte count` survives in the bullet rather than being dropped the way #2635 dropped it — the approval genuinely still names byte counts, last. **2. The README deletion-report paragraph keeps its skip enumeration.** #2635 collapsed "removed, locked, changed, protected, needs-elevation, and unverified outcomes" to the single word "skips". This PR keeps the enumeration, which matches `SKILL.md` §6. **3. Eval 12's prompt is retargeted through the documented `--root-children` selection.** As #2635 wrote it, the prompt scanned `C:\` directly. `SKILL.md` is explicit that `--root-children` is the only way to address an OS-managed volume root, and the engine has always enforced it — `hygiene.py` raises `OS-managed roots are not valid audit targets`, and `--root-children` without names returns `root-children-selection-required`. Both were already true at #2635's own commit. So the eval's expected output was reachable only by bypassing the confirmation gate, which means the eval graded the correct gated refusal as a failure. The prompt now names five explicitly selected children, and a fourth expectation pins that a ranking request is never authority to skip the gate. Root gating itself stays eval 11's subject. ## Scope note The engine half is already complete on `main` and is not touched here. Verified against merged content, not PR state: `empty_directory_count` (`hygiene.py:852`, emitted at `:1446` and `:3432`), preview `empty_directory` / `empty_directories` (`:2639`, `:2653`), apply `paths_removed` / `empty_directories_removed` (`:3170-3171`), and `validate_plan`'s `provenance`/`risk` enforcement (`:1609-1652`). The four tidiness tests were executed against `origin/main` content and pass (two skip on a documented Linux-only platform gate). ## Test plan - [x] `scripts/validate-plugins.sh` — all manifests and the catalog validate - [x] `scripts/check-changelog-parity.sh --check` — pass - [x] `scripts/check-changelog-parity.sh --check-order` — pass, 78 changelogs newest-first - [x] `scripts/check-changelog-parity.sh --check-bump origin/main` — pass - [x] `scripts/check-changelog-parity.sh --check-preserved origin/main` — pass, 64 headings compared - [x] `plugins/skill-quality/scripts/check-evals-quality.sh` — pass (1 pre-existing warning on eval 9) - [x] `evals.json` validates against `plugins/skill-quality/reference/evals.schema.json` — 0 errors - [x] `markdownlint-cli2` on `README.md` and `CHANGELOG.md` — 0 errors - [x] `biome check` on both changed JSON files — clean ## Related Closes #2828. Refs #2590, #2635, #2639, #2691, #2714, #2803. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
kyle-sexton
added a commit
that referenced
this pull request
Aug 16, 2026
…2832) Closes #2831. The silent-revert canary merged in #2808 ships an incident corpus that misattributes one of its three incidents. Since `--verify-known-incidents` replays that corpus on every `push: main`, the corpus is the guard's own honesty proof, and a wrong row costs it the credibility it exists to earn. The guard already contradicts its own fixture. Running the shipped detector on the incident commit prints: ``` $ scripts/check-silent-revert.sh --commit cc58cbc content from bfb66be feat(repo-fleet-hygiene): add finding rollups and scalable handoff plans (#2644) lines lost 853 content from eda5ae5 feat(repo-fleet-hygiene): gather merge evidence via aliased GraphQL (#2642) lines lost 301 ``` while the corpus recorded that commit as `#2633 dropped #2632's rollups (853 lines) -- unfiled until now`. ## What was wrong, and what it is now **The named victim never merged.** PR #2632 is CLOSED, not merged: `mergeCommit` null, `mergedAt` null, closed unmerged 2026-08-14T23:19:00Z, head `2b9391be` not an ancestor of `origin/main`, and zero occurrences of `rollup` in its diff. **There were two victims, and neither was #2632.** Attributing each of the 1165 lines `cc58cbc53` (#2633) deleted to the commit `git blame` credits it to: | culprit | PR | lines | | --- | --- | ---: | | `bfb66beb8` | #2644 — finding rollups and handoff plans | 853 | | `eda5ae5ed` | #2642 — aliased GraphQL merge evidence | 301 | | 11 other commits, ≤4 lines each | — | 11 | | **total** | | **1165** | 1165 matches the squash diffstat exactly (`6 files changed, 549 insertions(+), 1165 deletions(-)`). #2642 was not mentioned in the corpus at all, despite the detector reporting it as a separate finding. Corroboration: `plugins/repo-fleet-hygiene/skills/audit/scripts/audit-fleet.sh` went 2178 lines / 8 `graphql` / 9 `rollup` at `bfb66beb8` → 1700 / 0 / 0 at `cc58cbc53` → 2348 with both restored at `6f0a31109` (#2640) → 2933 on current `origin/main`. **853 was correct but unqualified.** It is a blame attribution against one culprit — not the deleting squash's diffstat total (1165) and not what #2644 added (942 insertions in its own squash commit, per `git diff --shortstat d55ffbf bfb66be`). Both 853 and 301 now say what they measure, in the fixture and in the script header. Worth noting for anyone re-measuring: `gh pr view 2644 --json additions` says 947, not 942, because the PR-level count is three-dot against the merge base while the commit diffstat is two-dot against the parent, and main moved under the branch in between. #2640 diverges the same way (`+1281/−463` as a commit, `1269/451` as a PR). The branch text avoids the trap by citing no lines-added figure at all — it just says the blame counts are not that. **"Unfiled until now" was false.** #2656 recorded this merge event, pinned to the same commits, roughly 23 hours before #2808 merged — from the test-coverage angle rather than as a silent revert. The corpus now cites it and frames the canary's contribution as detection speed and automation rather than discovery. ## Two judgement calls worth reviewing **The `#2632` string still appears twice**, in a new note recording that the row previously named it and why that was wrong. That is a refutation, not an attribution; it is there so the error is not silently reintroduced. Say the word if you would rather it be dropped entirely. **`check-silent-revert.sh` said the canary "fires 5 times" over 500 commits.** Measured per commit: `f603880da` 1 finding, `9239f1541` 1, `cc58cbc53` 2, `6f0a31109` 1 (pre-acknowledgment, 390 lines), `91e77fc16` 1 (pre-acknowledgment, 340 lines) — 5 commits, 6 findings. The number was counting commits, so the count is unchanged; the units are now stated so the 6-vs-5 gap is not read as an error. ## Blast radius Comments and free-text fixture notes only. - `diff <(git show HEAD~1:scripts/check-silent-revert.sh | grep -v '^\s*#') <(grep -v '^\s*#' scripts/check-silent-revert.sh)` is empty — no non-comment line of the detector changed. - No threshold, window, or expectation moved. The fixture parser reads `expect sha note`; only `note` text changed, and both `expect` and `sha` are untouched. - `shellcheck`, `bash -n`, `actionlint`, and `typos` all pass. `scripts/check-silent-revert.sh --verify-known-incidents` after the change: ``` 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 #2644's rollups (853 blamed lines) and #2642's GraphQL merge evidence (301); already recorded in #2656) 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. ``` ## Related - Refs #2808 — the PR that merged the canary and introduced the misattributed corpus row. - Refs #2691 — the original silent-revert audit the corpus is built from. Its two incidents (#2639/#2635 and #2641/#2639) were already recorded correctly and are untouched here. - Refs #2656 — the pre-existing record of this merge event, now cited by the corpus in place of the "unfiled until now" claim. Stays CLOSED/COMPLETED. - Refs #2644, #2642, #2633, #2640 — the merges measured above. Refs #2632, which is not one of them: it never merged, which is the whole point. - Refs #2833 — follow-up raised in review here: the replay checks only that a recorded commit still fires, never which culprit or how many lines, so a row can keep printing a reproduction it no longer performs. Pre-existing, and out of scope for a change constrained to leave the detector untouched. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_018S8a1S71VxhLTRWBtMuEvp --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
kyle-sexton
added a commit
that referenced
this pull request
Aug 17, 2026
…ributions (#2843) Closes #2837. Closes #2833. ## Summary Three defects in the silent-revert canary, plus one unfiled harness-safety fix in a file this change already owns. Branched off `2de57a379` (#2832), which had already merged. ## Fix ## 1. `declares_removal()` could not read the only revert subject that merges here (#2837) The detector accepted three intent forms. This repo is squash-only with `squash_merge_commit_title: PR_TITLE`, so the squash subject is the PR title, and `.github/workflows/pr-title.yml` gates every title through a required Conventional-Commits check whose default type list is all-lowercase — it admits `revert:` and contains nothing a `Revert "…"` subject could match. So a deliberate revert reached `main` wearing a subject the detector could not read. `declares_removal()` now also accepts the Conventional-Commits revert type, anchored at the start of the **subject**, requiring the literal lowercase token, its optional `(scope)` and/or `!`, its colon, and a non-empty description: ``` ^revert(\([^()]+\))?!?:[[:space:]]*[^[:space:]] ``` Never a substring search for "revert" — kept exactly as constrained as the three forms beside it. ### Before / after on the repo's one real deliberate revert `1d1fca6e8` — `revert: remove Cursor dual-target marketplace manifests (#1835) (#1839)` Before (at `2de57a379`, shipped detector, thresholds unmodified): ``` $ bash scripts/check-silent-revert.sh --commit 1d1fca6 SILENT REVERT SUSPECTED removed by 1d1fca6 revert: remove Cursor dual-target marketplace manifests (#1835) (#1839) 2026-07-30 19:41:47 -0400 content from b6c4b58 feat: add Cursor dual-target marketplace manifests (#1835) 2026-07-30 18:17:36 -0400 (1 commit(s) earlier on main) lines lost 3361 (threshold 200, window 40 commits) [... 102 files, sample block and "What to do" block elided; 130 lines total ...] EXIT=1 ``` After: ``` $ bash scripts/check-silent-revert.sh --commit 1d1fca6 declared 1d1fca6 revert: remove Cursor dual-target marketplace manifests (#1835) (#1839) removal is declared: the subject carries the Conventional-Commits revert type EXIT=0 ``` ### Suppression is not widened over anything the corpus records - No recorded incident has a `revert`-prefixed subject — `f603880da` `fix(disk-hygiene): …`, `9239f1541` `feat(disk-hygiene): …`, `cc58cbc53` `fix(repo-fleet-hygiene): …`, `c8470efd0` `docs(conventions): …`. All four rows still hold (green run below). - Neither acknowledgment-file commit is revert-prefixed either — `6f0a31109` `fix(repo-fleet-hygiene): …`, `91e77fc16` `fix(hook-utils): …` — so no ack row goes dead now that `declares_removal()` short-circuits ahead of `ack_reason()`. - The header's "fires on 5 commits — 1%" calibration figure is therefore unchanged; none of those five is revert-prefixed. New tests pin both directions. `revert:`, `revert(scope):`, `revert!:` and `revert(scope)!:` suppress; `feat: do not revert the alpha guard (#99)`, `reverted: drop the alpha guard (#99)`, `Revert: drop the alpha guard (#99)` and a bare `revert:` with no description all still fire. The pre-existing case only covered a *body* mention of "revert"; the subject is what the new form reads, so that is where a substring bug would widen. ## 2. `verify_known_incidents` asserted only "something fired" (#2833) A `fires` row passed on `scan_commit`'s exit status while the note beside it named a specific culprit and a specific line count that nothing checked. On `cc58cbc53`, whose deletions trace to two culprits, losing the `eda5ae5ed` attribution entirely would still have printed `ok` on the surviving `bfb66beb8` finding — the canary announcing a reproduction it did not perform. A `fires` row may now carry a bracketed attribution expectation after its sha: ``` fires <sha> [<culprit-full-sha>=<blamed-lines>,<culprit-full-sha>=<blamed-lines>] <note> ``` and the replay asserts the run's findings are **exactly** that set — same culprits, same per-culprit counts, no extras, no omissions. Full 40-character culprit shas only, the same discipline `silent-revert-acknowledged.txt` uses. Counts come from a new `FINDINGS_SINK` file that `report_finding` appends `<full-culprit-sha> <count>` to, not from scraping the human report — the report prints a 9-character abbreviation, which is not enough sha to assert on. Nothing else sets `FINDINGS_SINK`, so `--commit` and range mode are byte-identical. Every recorded figure was **measured, not transcribed from the notes** — the sink was wired first and the observed values recorded: ``` f603880 -> a95f240 346 9239f15 -> f603880 451 cc58cbc -> bfb66be 853 eda5ae5 298 ``` Three of the four agree with the notes #2832 corrected; the fourth is 298 rather than 301, for the reason in section 5. A malformed field is exit 2 (cannot run), never a FAIL and never a pass — a silently misread expectation is the same false green this file exists to remove. The field is optional so a row can be pinned before its attribution is measured, but a new well-formedness assertion requires every *shipped* `fires` row to carry one. ## 3. The workflow header stated a reason that was not true (unfiled) `.github/workflows/silent-revert-canary.yml` asserted *"There is no `pull_request` trigger, so it can never gate a PR"* — while its own `on:` block has a paths-filtered `pull_request` trigger, added by #2808 to run the detector's unit tests, and explained at length 20 lines further down in the same file. The conclusion holds for a different reason: both scan steps are gated `if: github.event_name != 'pull_request'`, and the lane sits outside `ci.yml` and its `ci-status` aggregate. Wording corrected to the actual reason. **No trigger and no `if:` changed.** `git diff` on that file is comment lines only — one hunk, `@@ -13,6 +13,9 @@`, entirely inside the `#` header. ## 4. The detector's line counts depended on ambient git config (unfiled, found by #2833's new assertion) The first CI run of this branch went red, and the failure is the most valuable thing in this PR. The runner reported the `eda5ae5ed` attribution as **298** lines where my machine measured **301**: ``` FAIL cc58cbc fires, but NOT as recorded recorded attribution: eda5ae5… 301 what the detector reported: eda5ae5… 298 ``` Cause: `attribute_file` called bare `git diff --unified=0`, so it inherited whatever `diff.algorithm` the caller's config carried. I have `diff.algorithm = histogram` set globally; CI has nothing set and therefore uses git's default `myers`. The algorithm changes which lines a hunk calls deleted, so it changes the per-culprit counts this canary **thresholds on**. Reproduced directly: ``` $ GIT_CONFIG_COUNT=1 GIT_CONFIG_KEY_0=diff.algorithm GIT_CONFIG_VALUE_0=myers \ FINDINGS_SINK=… scripts/check-silent-revert.sh --commit cc58cbc… bfb66be… 853 eda5ae5… 298 # 301 under histogram ``` Three lines is harmless in itself. The principle is not: the same drift can carry a count across the 200-line threshold, so a commit could fire on one machine and stay silent on another, and the header's "fires on 5 commits over 500 — 1%" calibration only ever described one algorithm. `attribute_file` now pins `--diff-algorithm=myers -M` explicitly, and the file enumeration pins `-M` too. **Both are git's defaults, so this does not change what CI detects today** — CI already had no `diff.algorithm` set. It makes a local run match CI, not the reverse. `-M` covers the same exposure for rename detection, which the existing header calls "load-bearing rather than incidental": `diff.renames = false` in a developer's config would decompose a `git mv` into delete + add and make relocating a large recent file fire. The recorded figure is now **298**, and the prose figures in the script header and the corpus are corrected with the refutation attached, so nobody re-measuring on a histogram machine "corrects" it back to 301. Worth stating plainly: nothing asked for this. #2833's exact-count assertion turned a silent, config-dependent divergence into a red build on its first run — which is precisely the argument for asserting attributions instead of exit status. ## 5. The test harness could commit the developer's work as `test <t@t.test>` (unfiled) Found the hard way while developing this. `mk_repo` is called as `repo="$(mk_repo)"`, so a `return 1` inside the command substitution cannot abort the suite — the caller just gets `""`. And `""` is not inert: `git -C ""` is documented as a no-op, so the next `add -A` + `commit` staged and committed my uncommitted work into the checkout, authored `test <t@t.test>`. Such a commit cannot be pushed here — it fails `required_signatures` with `no_user`. `mk_repo` now yields a path derived from `SELF_DIR` that does not exist, so every git call against it fails loudly and the assertions go red — fail-closed, which is what a harness that cannot build its fixture should do. Scoped to this file only; if the same `repo="$(mk_repo)"` shape exists in sibling harnesses that is a separate follow-up. ## Verification `scripts/check-silent-revert.sh --verify-known-incidents`, run on a machine with `diff.algorithm = histogram` set globally — it now agrees with CI exactly, because the flags are pinned: ``` ok f603880 fires as recorded, 1 attribution(s) reproduced exactly (#2639 dropped #2635 (346 blamed lines), 13 minutes later) ok 9239f15 fires as recorded, 1 attribution(s) reproduced exactly (#2641 dropped #2639 (451 blamed lines), 10 minutes later) ok cc58cbc fires as recorded, 2 attribution(s) reproduced exactly (#2633 dropped #2644's rollups (853 blamed lines) and #2642's GraphQL merge evidence (298); already recorded in #2656) 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. ``` Injected failure against the **shipped** corpus (`853` changed to `852` in a copy) — the mechanism is proven on real rows, not only on synthetic fixtures: ``` $ SILENT_REVERT_INCIDENTS=<copy with 298 -> 297> scripts/check-silent-revert.sh --verify-known-incidents ok f603880 fires as recorded, 1 attribution(s) reproduced exactly (...) ok 9239f15 fires as recorded, 1 attribution(s) reproduced exactly (...) FAIL cc58cbc fires, but NOT as recorded (#2633 dropped #2644's rollups (853 blamed lines) and #2642's GraphQL merge evidence (298); already recorded in #2656) recorded attribution: bfb66be 853 eda5ae5 297 what the detector reported: bfb66be 853 eda5ae5 298 A row that fires for the wrong reason is not a reproduction. Do NOT edit the row to match; find out why the attribution moved. ok c8470ef stays clean as recorded (...) The canary no longer reproduces the incidents it was built for. Do not relax the recorded expectations to make this pass. EXIT=1 ``` The commit still fires — exit status alone would have passed this row. Note the row is red on a **one-line** discrepancy in one of two attributions, which is exactly the regression #2833 describes. `scripts/check-silent-revert.test.sh`: **49 passed, 0 failed**, including `replay fails when the finding is attributed to a different culprit`, `replay fails when the recorded line count no longer reproduces`, and `replay fails when one of two recorded attributions stops reproducing`. So the assertion is proven by permanent tests, not only by a one-off injection. ## 6. Review follow-up: an unterminated attribution field read as *absent* Both automated review lanes independently flagged the same real gap, and they were right. A `fires` row whose field opened with `[` but never closed it failed the `[[ "$rest" == \[*\]* ]]` glob, so `attribution` stayed empty, the remainder became free-text `note`, and the row fell back to passing on exit status alone — reintroducing the exact pre-#2833 gap by the one route nobody would look at, and contradicting the contract documented directly above it. A leading `[` now COMMITS the row to carrying an attribution; unterminated takes the malformed path. Reproduced against the real corpus with the closing bracket stripped from the `f603880da` row: ``` check-silent-revert: unterminated attribution field for f603880… (no closing ']'): [a95f240…=346 #2639 dropped #2635 … EXIT=2 ``` The four malformed shapes already pinned all carried a closing `]`, so this one was untested; `[<sha>=40` and a bare `[` are now pinned too. The shipped corpus was never at risk — `t_shipped_data_files_are_wellformed`'s regex covers it — but that is a separate layer and does not hold for a custom `SILENT_REVERT_INCIDENTS`. ## 7. Verification follow-up: `git blame` was still ambient-config dependent Fresh-context verification of section 4 found that fix was only half of one. Pinning the diff flags left `attribute_file`'s `git blame` call bare, so `blame.ignoreRevsFile` — an ordinary setting in any repo carrying a bulk-reformat commit — still decided the per-culprit counts the replay now asserts on. On the real corpus, with that setting naming `bfb66beb8`: | culprit | pinned | with `blame.ignoreRevsFile` | | --- | ---: | ---: | | `bfb66beb8…` | 853 | **259** | | `eda5ae5ed…` | 298 | **322** | On a synthetic fixture it is worse than a wrong number. With the pin absent and that config present, the detector reports **no finding at all** on a genuine silent revert: | | clean config | hostile config | | --- | --- | --- | | pinned | `culprit 40` | `culprit 40` | | unpinned | `culprit 40` | **(nothing — the canary goes silent)** | That is a false green reached through the developer's own gitconfig — the precise failure this canary exists to remove. **The obvious fix does not work, and the comment says so.** `-c blame.ignoreRevsFile=` does *not* clear it: the documented "an empty file name resets the list" applies to the **option**, and the `-c` form was measured leaving the hostile value fully in effect (853 → 259 with the reset supposedly applied). Only `--no-ignore-revs-file` actually resets. The symmetry with the `-c` pins above is wrong here and is deliberately not used. `t_counts_are_immune_to_ambient_git_config` pins the property: the same fixture scanned twice, once under a hostile `GIT_CONFIG_GLOBAL` setting `blame.ignoreRevsFile`, `diff.algorithm` and `diff.renames`, asserting identical exit status and identical per-culprit findings. **Confirmed discriminating** — with the blame pin stripped it fails, and it fails because the hostile run reports nothing at all. `shellcheck`, `actionlint`, `typos`, `bash -n` and `scripts/check-shell-portability.sh` all pass. ## Test plan Run from a clean checkout of this branch, with `unset GIT_DIR GIT_WORK_TREE`: 1. `bash scripts/check-silent-revert.test.sh` — the detector's own unit suite. Expect **49 passed, 0 failed**. Covers all four `revert:` spellings, the four negative subject cases (`feat: do not revert ...`, `reverted:`, `Revert:`, bare `revert:`), the wrong-culprit / wrong-count / missing- attribution replay regressions, all six malformed-field shapes, and `t_counts_are_immune_to_ambient_git_config`. 2. `bash scripts/check-silent-revert.sh --verify-known-incidents` — the real corpus. Expect exit 0 with all three `fires` rows reporting `attribution(s) reproduced exactly` and the `clean` row staying clean. 3. Negative control for step 2: edit `scripts/silent-revert-incidents.txt` to inject a wrong culprit sha (leaving the count correct, so the commit still fires) and, separately, a wrong line count. Each must exit **1** with `fires, but NOT as recorded`. Restore the file afterwards. 4. Set `diff.algorithm = histogram` in global git config and repeat steps 1-2. The numbers must not move — that is what the new flag pins buy. 5. `git diff origin/main...HEAD -- .github/workflows/silent-revert-canary.yml` must show comment-only changes; no executable YAML line may differ. ## Related - Refs #2808 — the PR that merged the canary and its three intent forms. - Refs #2832 / #2831 — the corpus-attribution correction this branches off; #2833 was raised in its review. - Refs #2691 — the original silent-revert audit the corpus is built from. - Refs #1839 — the deliberate revert (`1d1fca6e8`) used as the real-history fixture for #2837. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #2590
Summary
Disk-hygiene reporting was organized around reclaimable bytes, so empty/zero-byte directories effectively dropped out — inverting the operator goal of safe tidiness.
Fix
Make tidiness/provenance the primary reporting frame; keep bytes as a secondary signal so empty residue remains visible and actionable.
Verification
See PR checks / plugin tests.
Related
Refs #2588 — root-children mode (related volume-root cleanup workflow).