Skip to content

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

Merged
kyle-sexton merged 1 commit into
mainfrom
cursor/feat-fleet-worktree-conformance-f8b0
Aug 15, 2026
Merged

fix(repo-fleet-hygiene): restore GraphQL merge evidence and rollups after #2633#2640
kyle-sexton merged 1 commit into
mainfrom
cursor/feat-fleet-worktree-conformance-f8b0

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Closes #2606

Summary

Report worktree-root conformance findings for fleet audit.

Fix

See commits on this branch.

Verification

See CI checks on this PR.

Related

Refs #2597 — fleet epic.

@cursor

cursor Bot commented Aug 14, 2026

Copy link
Copy Markdown

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
kyle-sexton enabled auto-merge (squash) August 14, 2026 23:15
@claude

claude Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

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


Security Review — /review:security-review

  • Gather context (PR diff, changed files, CLAUDE.md)
  • Run /review:security-review procedure
  • Analyze changed shell scripts (audit-fleet.sh, audit-fleet.test.sh) for security issues
  • Analyze skill/docs changes
  • Post final findings

No GitHub Actions workflow files are touched in this PR, so zizmor's static lanes (supply-chain, unpinned actions, permissions, trigger patterns) don't apply here. Reviewed the new melodic.worktreeroot / worktree-root-conformance logic in audit-fleet.sh for injection, trust-boundary, and logic-flaw issues.

The report-forging defense already in place (display_value/print_field %q-escaping control-bearing values, audit-fleet.sh#L73-L85) is applied consistently to the new conformance fields, and the new convention_git_allowed/run_convention_git allowlist (audit-fleet.sh#L196-L221) correctly pins the command/flag shape so no caller-controlled Git subcommand or option can slip through.

Finding (IMPORTANT)

Any single repository in a multi-repo fleet audit can silently override the fleet-wide melodic.worktreeroot convention via its own local .git/config, because the resolver doesn't isolate to global/user scope despite documenting that intent.

  • audit-fleet.sh#L196-L221 (run_convention_git) — comment states "Do not null GIT_CONFIG_GLOBAL/SYSTEM: melodic.worktreeroot is expected outside the repo." This is the opposite hardening run_git_probe applies just above it (L184-L191), which pins GIT_CONFIG_GLOBAL=/dev/null/GIT_CONFIG_SYSTEM=/dev/null specifically so "an inherited selector or a global includeIf... can never redirect a probe away from the repository's own on-disk config." Leaving global/system unpinned for the convention read is intentional (the key is meant to live in the user's own global gitconfig) — but it has a side effect: git config --get-all also merges the audited repo's own local config, and per Git's file precedence, local config is emitted after global/system, so the last-wins loop in try_read_melodic_worktree_root (L624-L648) picks the local value over the user's actual global setting whenever both are present.
  • audit-fleet.sh#L694-L707 (resolve_configured_worktree_root) — iterates TARGETS (the full set of repositories discovered under --root, in filesystem/glob order — not trust-ranked) and stops at the first repo for which the probe returns any value, local or global.

Failure scenario: A user runs a fleet-wide audit with --root ~/code over many local checkouts, one of which is a third-party clone / fork / dependency the user doesn't fully control. If that repo's .git/config contains melodic.worktreeroot = <path> and it happens to be reached before the user's own canonical repo in TARGETS order, that value becomes CONFIGURED_WORKTREE_ROOT for the entire fleet report — silently overriding the user's real global convention. Every other repository's "expected location" / worktree-outside-configured-root / worktree-wrong-layout findings then recommend migrating worktrees under that attacker-influenced path, and the header (Worktree root: ... (source: melodic.worktreeroot; origin: file:<repo>/.git/config)) discloses the origin but not that it came from an unrelated audited repo rather than the user's own machine-level config. Since the plugin is report-only, this can't directly move files, but it can steer a user who trusts the recommendation into manually running /source-control:worktree create at a path chosen by someone else's repo.

Suggested fix: resolve melodic.worktreeroot only from a scope the user actually controls — e.g. PROJECT_DIR/an explicit --repo/global-only lookup (git config --global --get-all) — rather than scanning arbitrary fleet TARGETS, or at minimum null GIT_CONFIG_SYSTEM/keep GIT_CONFIG_GLOBAL while excluding local scope per probe (e.g. git config --get-all --show-origin filtered to origin lines starting with the known global/system file paths, discarding file:<repo>/.git/config origins).

Fix this →

No other injection, credential-handling, or trust-boundary issues found in this diff — the rest of the new code (worktree_slug, expected_worktree_dirname, is_tool_owned_worktree, the settings.json jq read) only builds display strings/path-prefix comparisons and never feeds untrusted values into command execution outside the already-allowlisted run_convention_git/run_git_probe wrappers.
· Branch

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8c1ff7ed07

ℹ️ 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".

Comment thread plugins/repo-fleet-hygiene/skills/audit/scripts/audit-fleet.sh Outdated
Comment thread plugins/repo-fleet-hygiene/skills/audit/scripts/audit-fleet.sh Outdated
Comment thread plugins/repo-fleet-hygiene/skills/audit/scripts/audit-fleet.sh Outdated
Comment thread plugins/repo-fleet-hygiene/skills/audit/scripts/audit-fleet.sh Outdated
@github-actions

github-actions Bot commented Aug 14, 2026

Copy link
Copy Markdown

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

@kyle-sexton
kyle-sexton force-pushed the cursor/feat-fleet-worktree-conformance-f8b0 branch from 8c1ff7e to 25036a7 Compare August 14, 2026 23:22
@claude

claude Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

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


Security Review — /review:security-review

  • Gather context (PR diff, changed files, CLAUDE.md)
  • Run /review:security-review procedure
  • Analyze changes since last reviewed head (8c1ff7ed0)
  • Post final findings

Diff check vs. last review: git diff origin/main...HEAD on the current head (25036a7) is byte-for-byte the same PR content already reviewed at 8c1ff7ed0 — the only difference is the base commit shifting (this branch was rebased onto the now-merged #2637), which just moved line numbers. No new lines, no changed logic. So the prior finding below still applies as-is; I'm not re-doing the full hunt, just re-anchoring it to the current head and confirming it's still unaddressed.

Finding (IMPORTANT) — still open

Any single repository in a multi-repo fleet audit can silently override the fleet-wide melodic.worktreeroot convention via its own local .git/config, because the resolver doesn't isolate to global/user scope despite documenting that intent.

  • audit-fleet.sh#L214-L227 (run_convention_git) — the comment at L222 states "Do not null GIT_CONFIG_GLOBAL/SYSTEM: melodic.worktreeroot is expected outside the repo." This is the opposite hardening run_git_probe applies just above it (L179-L191), which pins GIT_CONFIG_GLOBAL=/dev/null/GIT_CONFIG_SYSTEM=/dev/null specifically so an inherited selector or global includeIf can never redirect a probe away from the repository's own on-disk config. Leaving global/system unpinned for the convention read is intentional (the key is meant to live in the user's own global gitconfig) — but git config --get-all also merges the audited repo's own local config, and per Git's file precedence local config is emitted after global/system, so the last-wins loop in try_read_melodic_worktree_root (L619-L645) picks the local value over the user's actual global setting whenever both are present. The origin is tracked (--show-origin) but never filtered on.
  • audit-fleet.sh#L691-L705 (resolve_configured_worktree_root) — iterates TARGETS (every repository discovered under --root, in filesystem/glob order — not trust-ranked) and stops at the first repo for which the probe returns any value, local or global.

Failure scenario: A user runs a fleet-wide audit with --root ~/code over many local checkouts, one of which is a third-party clone / fork / dependency they don't fully control. If that repo's .git/config contains melodic.worktreeroot = <path> and it's reached before the user's own canonical repo in TARGETS order, that value becomes CONFIGURED_WORKTREE_ROOT for the entire fleet report, silently overriding the user's real global convention. Every other repository's worktree-outside-configured-root / worktree-wrong-layout findings then recommend migrating worktrees under that attacker-influenced path; the header discloses the origin file path but not that it came from an unrelated audited repo rather than the user's own machine-level config. Since the plugin is report-only this can't move files directly, but it can steer a user who trusts the recommendation into running /source-control:worktree create at a path chosen by someone else's repo.

Suggested fix: resolve melodic.worktreeroot only from a scope the user actually controls — e.g. PROJECT_DIR / an explicit --repo / a global-only lookup (git config --global --get-all) — rather than scanning arbitrary fleet TARGETS; or at minimum keep scanning but filter --show-origin results to origins matching the known global/system config file paths, discarding any file:<repo>/.git/config origin before applying last-wins.

Fix this →

No other injection, credential-handling, or trust-boundary issues found — the rest of the diff (worktree_slug, expected_worktree_dirname, is_tool_owned_worktree, the settings.json jq read in try_read_source_control_worktree_root) only builds display strings/path-prefix comparisons and never feeds untrusted values into command execution outside the already-allowlisted run_convention_git/run_git_probe wrappers. (Note: chatgpt-codex-connector's review comments on this PR raise correctness/robustness points — conditional roots across includeIf, symlink canonicalization, linked-worktree rollup counting, jq availability — those are code-quality/logic concerns, not security, and are out of scope for this security-only lane.)
· Branch

@cursor
cursor Bot force-pushed the cursor/feat-fleet-worktree-conformance-f8b0 branch 5 times, most recently from 266377c to 56c5653 Compare August 15, 2026 00:00
@claude

claude Bot commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

Claude finished @cursor[bot]'s task in 10m 24s —— View job


Code Review — /review:code-review

  • Gather context (PR diff, changed files, CLAUDE.md, REVIEW.md)
  • Run /review:code-review procedure (correctness, design, conventions, error handling, observability, tests, docs — security out of scope, dedicated claude-security-review lane already ran and reported)
  • Review audit-fleet.sh / audit-fleet.test.sh changes
  • Review skill/docs changes (SKILL.md, confidence-model.md, README, CHANGELOG)
  • Post inline findings + summary

Filed 4 inline findings on audit-fleet.sh. One additional finding is cross-file and goes here.

Finding (Important) — this PR's diff, as of the reviewed head, silently reverts #2642's GraphQL merged-PR evidence work

Comparing this head against origin/main, audit-fleet.sh goes from the aliased-GraphQL merged-PR implementation (landed by #2642, confirmed: #2642's diff ends at the exact blob main currently has for this file) back to the older REST gh pr list --state merged batch + privacy-gated --head fallback design. Same reversion appears consistently across SKILL.md (current L95 — "batched gh pr list" instead of "aliased gh api graphql"), confidence-model.md, audit-fleet.sh (e.g. reintroduces MERGED_PR_WINDOW=1000 at L237, the retired merged-pr-window-truncated/merge-evidence-privacy-gated findings, and the prefix-auth GraphQL-exactness test fixture removed from audit-fleet.test.sh), and even the local CHANGELOG.md at this head, whose latest entry is ## [0.16.0] (L6) — there is no 0.17.0/GraphQL entry at all at this commit, even though main's current CHANGELOG documents 0.17.0 as "Merged-PR evidence uses aliased GraphQL and retires REST window/privacy findings (#2604)."

This strongly indicates the branch was never rebased onto main after #2642 merged — its own history (b7793d3 #2630, a2f3420 #2637) predates #2642, and the worktree-conformance work here was built on the pre-#2642 REST implementation. If merged as-is, this PR would reintroduce the REST window-truncation/privacy-gating limitations GraphQL was specifically built to retire, with no CHANGELOG entry disclosing the reversion (this PR's own new changelog entry describes only the worktree-conformance feature).

Note: I refetched gh pr diff mid-review and saw the base-side version churn (0.17.00.17.3 on main) from unrelated concurrent merges to this fast-moving repo, but audit-fleet.sh's blob was stable across both fetches and matches the local checkout at the reviewed head (56c565390d473807218469b968f8e1c230145d15) exactly — this finding is anchored to that specific commit. Suggested fix: rebase this branch onto current main so the GraphQL implementation from #2642 is preserved, and layer only the worktree-root-conformance additions on top of it.

Fix this →


Other findings (inline on audit-fleet.sh)

  • Important, L654-679: try_read_source_control_worktree_root() depends on jq without declaring it or checking for it; on a jq-less Git Bash install (a documented supported platform) a real pluginConfigs.worktree_root value is silently ignored and the audit falsely reports the root as unconfigured.
  • Important, L615-620: under_configured_root()/path_key() never canonicalize symlinks, so a symlinked configured root (or ancestor) makes a correctly-placed worktree get falsely reported as worktree-outside-configured-root/worktree-wrong-layout.
  • Suggestion, L695-702: resolve_configured_worktree_root() picks the first TARGETS repo with any melodic.worktreeroot and applies it fleet-wide; a legitimate per-repo conditional root (git includeIf) from one repo gets misapplied as the "expected location" for every other repo.
  • Important, L1341-1390: the conformance-classification block runs only for linked worktrees that survive the earlier missing/unverifiable-root/not-a-root continues, so a repo whose only linked worktrees are missing/unverifiable gets no conformance rollup at all — contradicting SKILL.md's "every linked worktree is classified" claim.

No other correctness/design/convention issues found in the diff; test coverage for the new conformance logic (audit-fleet.test.sh) is otherwise thorough. Security-relevant aspects of this same code (local-config override risk in resolve_configured_worktree_root) were already reported by the security-review lane above and are out of scope here.
· Branch

@cursor
cursor Bot force-pushed the cursor/feat-fleet-worktree-conformance-f8b0 branch 3 times, most recently from 4a22450 to 63d57d5 Compare August 15, 2026 00:13
Comment thread plugins/repo-fleet-hygiene/skills/audit/scripts/audit-fleet.sh Outdated
Comment thread plugins/repo-fleet-hygiene/skills/audit/scripts/audit-fleet.sh Outdated
Comment thread plugins/repo-fleet-hygiene/skills/audit/scripts/audit-fleet.sh Outdated
Comment thread plugins/repo-fleet-hygiene/skills/audit/scripts/audit-fleet.sh Outdated
@github-actions

github-actions Bot commented Aug 15, 2026

Copy link
Copy Markdown

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

@cursor
cursor Bot force-pushed the cursor/feat-fleet-worktree-conformance-f8b0 branch 8 times, most recently from 87c5500 to 0d2d7f9 Compare August 15, 2026 01:00
@kyle-sexton kyle-sexton changed the title feat(repo-fleet-hygiene): report worktree-root conformance fix(repo-fleet-hygiene): restore GraphQL merge evidence and rollups after #2633 Aug 15, 2026
cursor Bot pushed a commit that referenced this pull request Aug 15, 2026
Re-apply the conformance report lost when #2640's squash only restored
GraphQL/rollups after the #2633 regression. Classify linked worktrees
against melodic.worktreeroot / source-control worktree_root. Version 0.20.0.

Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
cursor Bot pushed a commit that referenced this pull request Aug 15, 2026
Re-apply the conformance report lost when #2640's squash only restored
GraphQL/rollups after the #2633 regression. Classify linked worktrees
against melodic.worktreeroot / source-control worktree_root. Version 0.20.0.

Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
kyle-sexton added a commit that referenced this pull request Aug 15, 2026
<!-- CURSOR_AGENT_PR_BODY_BEGIN -->
Closes #2606

## Summary

Restores the #2606 worktree-root conformance feature that PR #2640
claimed to close but did not keep when the squash (`6f0a3110`) only
re-integrated GraphQL/rollups after the #2633 regression.

## Fix

- Per-repo and fleet counts: conforming / outside-or-wrong-layout /
tool-owned
- Non-conforming findings name the expected
`<root>/<owner>-<repo>-<slug>` location
- Codex/Cursor tool-owned paths distinguished from misplaced
- Unset root → placement only (no invented convention)
- Config reads gated on `rev-parse --git-dir`; attribution via
`--show-origin`
- Hardened evidence: physical-path compare;
`worktree-root-pluginconfigs-unreadable` when `jq` is missing
- Bump `repo-fleet-hygiene` to **0.21.0**

## Verification

```
bash plugins/repo-fleet-hygiene/skills/audit/scripts/audit-fleet.test.sh
# 133 PASS
```

## Related

Refs #2597 — fleet epic parent.
Refs #2640 — claimed Closes #2606 but landed without the feature.
Refs #2633 — regression that dropped collector surfaces.

<!-- CURSOR_AGENT_PR_BODY_END -->

<div><a
href="https://cursor.com/agents/bc-eb3f5ee5-6c9f-48a3-8e46-071bd363f8b0?cursor_ref=pr_footer&cursor_cta=open_in_web"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://cursor.com/assets/images/open-in-web-dark.png"><source
media="(prefers-color-scheme: light)"
srcset="https://cursor.com/assets/images/open-in-web-light.png"><img
alt="Open in Web" width="114" height="28"
src="https://cursor.com/assets/images/open-in-web-dark.png"></picture></a>&nbsp;<a
href="https://cursor.com/background-agent?bcId=bc-eb3f5ee5-6c9f-48a3-8e46-071bd363f8b0&cursor_ref=pr_footer&cursor_cta=open_in_cursor"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://cursor.com/assets/images/open-in-cursor-dark.png"><source
media="(prefers-color-scheme: light)"
srcset="https://cursor.com/assets/images/open-in-cursor-light.png"><img
alt="Open in Cursor" width="131" height="28"
src="https://cursor.com/assets/images/open-in-cursor-dark.png"></picture></a>&nbsp;</div>

---------

Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
cursor Bot pushed a commit that referenced this pull request Aug 15, 2026
Restore bare-live/bare-pure fixtures and assertions lost in the #2633/#2640
squash cycle, and add a finding-kind coverage gate so a self-consistent
rewrite cannot drop the defender again. Closes #2656.

Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
kyle-sexton added a commit that referenced this pull request Aug 15, 2026
#2658)

<!-- CURSOR_AGENT_PR_BODY_BEGIN -->
Closes #2656

## Summary

`bare-repo-with-working-tree` shipped on `main` with zero suite coverage
after the #2633/#2640 squash cycle. This restores the regression tests
and adds a repo-level gate outside `audit-fleet.test.sh` so a
self-consistent rewrite cannot delete the defender again.

## Fix

- Restore `bare-live` / `bare-pure` fixtures and end-to-end assertions
(positive bare+live tree with remedy, discovery under `--root`, ordinary
bare hub still rejects)
- Unit-cover `directory_has_non_git_entries` and `BARE_LIVE_TREE_*`
dedup via `record_bare_live_tree`
- Add `scripts/check-fleet-finding-test-coverage.sh` (+ baseline,
self-test, CI lane) asserting every emitted finding kind is named in the
suite or grandfathered

## Verification

```
bash plugins/repo-fleet-hygiene/skills/audit/scripts/audit-fleet.test.sh
bash scripts/check-fleet-finding-test-coverage.test.sh
scripts/check-fleet-finding-test-coverage.sh --check
```

## Related

Refs #2633 — bare-live feature squash.
Refs #2640 — recovery squash that dropped the suite coverage.

<!-- CURSOR_AGENT_PR_BODY_END -->

<div><a
href="https://cursor.com/agents/bc-eb3f5ee5-6c9f-48a3-8e46-071bd363f8b0?cursor_ref=pr_footer&cursor_cta=open_in_web"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://cursor.com/assets/images/open-in-web-dark.png"><source
media="(prefers-color-scheme: light)"
srcset="https://cursor.com/assets/images/open-in-web-light.png"><img
alt="Open in Web" width="114" height="28"
src="https://cursor.com/assets/images/open-in-web-dark.png"></picture></a>&nbsp;<a
href="https://cursor.com/background-agent?bcId=bc-eb3f5ee5-6c9f-48a3-8e46-071bd363f8b0&cursor_ref=pr_footer&cursor_cta=open_in_cursor"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://cursor.com/assets/images/open-in-cursor-dark.png"><source
media="(prefers-color-scheme: light)"
srcset="https://cursor.com/assets/images/open-in-cursor-light.png"><img
alt="Open in Cursor" width="131" height="28"
src="https://cursor.com/assets/images/open-in-cursor-dark.png"></picture></a>&nbsp;</div>

---------

Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
kyle-sexton added a commit that referenced this pull request Aug 15, 2026
)

No linked issue


## Summary

The skill body instructed an agent to **reject
`/repo-fleet-hygiene:audit D:`** — the exact invocation #2599 exists to
enable — while the bundled script accepted it. This restores the
documented grammar to match the script as it actually behaves on `main`.

## Fix

PR #2638 shipped bare positional paths and drive roots, and replaced the
project-directory scope fallback with a hard failure that names its
remedies. PR #2646 merged eight minutes later from a branch cut before
it; the rebase was textually clean but carried pre-#2638 prose forward,
reverting the skill layer while leaving the script untouched.

Restored, each verified against `origin/main`'s `audit-fleet.sh`:

| Restored | Script behaviour it now matches |
|---|---|
| bare positional `<dir>` in the argument list and `argument-hint` |
`:484` `[[ -n "$1" ]] \|\| fail "bare path requires a directory"`, with
`normalize_discovery_root` mapping `D:` → `D:/` |
| `--project-dir` documented as a config rung only | project dir is no
longer a scope target anywhere in the script |
| the no-scope paragraph, rewritten to describe the hard failure |
`:1069` `fail "no scope resolved: no bare path, --root, or --repo, and
no config-supplied fleet.root/fleet.repo"` |
| the reject-outside-the-grammar clause | now says plainly that a bare
path **is** in the grammar; only an unrecognized `-`-prefixed flag is
refused, matching the script's `-*)` arm |

Also restores the `bare-repo-with-working-tree` handoff row (#2633 /
#2602), dropped by the same revert — `git grep -c` on `main` returned 0
— with its documented remedy (`git config --local core.bare false`) and
the fact that linked worktrees keep working, since the state looks
alarming and is not.

## Also corrected: an unimplemented capability claimed as shipped

`README.md` and the plugin description both claimed **machine-wide**
repository discovery. It does not exist: `grep -n "ghq\|claude\.json"`
over the landed script returns nothing, only the configured-roots rung
is implemented, and a no-argument run hard-fails. The README row is
split into what shipped (bounded discovery via bare path, drive root,
`--root`, `--repo`, config rungs) and what did not (the ghq /
configured-roots / agent-state / bounded-sweep ladder, still unshipped
remaining contract work). The plugin description now says
"Cross-repository" rather than "Machine-wide".

`docs/CATALOG.md` regenerated from the manifest rather than hand-edited.

## Verification

```
node scripts/generate-catalog.mjs --check     → Catalog is in sync with the manifests.
node scripts/generate-cheatsheet.mjs --check  → Cheat sheet is in sync with skill frontmatter.
markdownlint-cli2 (SKILL.md, README.md)       → 0 error(s)
check-skill.sh audit                          → all 8 base-ref trigger phrases preserved
                                                description 642/1536 chars
                                                SKILL.md 302/500 lines
                                                markdownlint clean
```

The trigger-preservation check matters here specifically: the plugin
description was reworded, and that gate fails on a silently dropped
trigger phrase.

Version bumped to 0.22.1 with a CHANGELOG entry so changelog-parity
clears for the plugin-file edits.

## Why nothing caught this

`scripts/docs-only-paths.txt` contains exactly one prefix
(`docs/topics/`), so `plugin-gate` **did** run the full suite on #2646 —
all 38 checks green. There is simply no assertion tying `SKILL.md` prose
to the script's actual argument parser. That gap is worth its own issue:
this is the second silent revert on `main` in a day (the first was
#2633's stale-base squash, repaired by #2640), and both passed CI.

## Related

Refs #2599 (already closed by #2638; this PR only restores the
documented grammar that #2638 shipped).

---------

Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
cursor Bot pushed a commit that referenced this pull request Aug 15, 2026
)

No linked issue


## Summary

The skill body instructed an agent to **reject
`/repo-fleet-hygiene:audit D:`** — the exact invocation #2599 exists to
enable — while the bundled script accepted it. This restores the
documented grammar to match the script as it actually behaves on `main`.

## Fix

PR #2638 shipped bare positional paths and drive roots, and replaced the
project-directory scope fallback with a hard failure that names its
remedies. PR #2646 merged eight minutes later from a branch cut before
it; the rebase was textually clean but carried pre-#2638 prose forward,
reverting the skill layer while leaving the script untouched.

Restored, each verified against `origin/main`'s `audit-fleet.sh`:

| Restored | Script behaviour it now matches |
|---|---|
| bare positional `<dir>` in the argument list and `argument-hint` |
`:484` `[[ -n "$1" ]] \|\| fail "bare path requires a directory"`, with
`normalize_discovery_root` mapping `D:` → `D:/` |
| `--project-dir` documented as a config rung only | project dir is no
longer a scope target anywhere in the script |
| the no-scope paragraph, rewritten to describe the hard failure |
`:1069` `fail "no scope resolved: no bare path, --root, or --repo, and
no config-supplied fleet.root/fleet.repo"` |
| the reject-outside-the-grammar clause | now says plainly that a bare
path **is** in the grammar; only an unrecognized `-`-prefixed flag is
refused, matching the script's `-*)` arm |

Also restores the `bare-repo-with-working-tree` handoff row (#2633 /
#2602), dropped by the same revert — `git grep -c` on `main` returned 0
— with its documented remedy (`git config --local core.bare false`) and
the fact that linked worktrees keep working, since the state looks
alarming and is not.

## Also corrected: an unimplemented capability claimed as shipped

`README.md` and the plugin description both claimed **machine-wide**
repository discovery. It does not exist: `grep -n "ghq\|claude\.json"`
over the landed script returns nothing, only the configured-roots rung
is implemented, and a no-argument run hard-fails. The README row is
split into what shipped (bounded discovery via bare path, drive root,
`--root`, `--repo`, config rungs) and what did not (the ghq /
configured-roots / agent-state / bounded-sweep ladder, still unshipped
remaining contract work). The plugin description now says
"Cross-repository" rather than "Machine-wide".

`docs/CATALOG.md` regenerated from the manifest rather than hand-edited.

## Verification

```
node scripts/generate-catalog.mjs --check     → Catalog is in sync with the manifests.
node scripts/generate-cheatsheet.mjs --check  → Cheat sheet is in sync with skill frontmatter.
markdownlint-cli2 (SKILL.md, README.md)       → 0 error(s)
check-skill.sh audit                          → all 8 base-ref trigger phrases preserved
                                                description 642/1536 chars
                                                SKILL.md 302/500 lines
                                                markdownlint clean
```

The trigger-preservation check matters here specifically: the plugin
description was reworded, and that gate fails on a silently dropped
trigger phrase.

Version bumped to 0.22.1 with a CHANGELOG entry so changelog-parity
clears for the plugin-file edits.

## Why nothing caught this

`scripts/docs-only-paths.txt` contains exactly one prefix
(`docs/topics/`), so `plugin-gate` **did** run the full suite on #2646 —
all 38 checks green. There is simply no assertion tying `SKILL.md` prose
to the script's actual argument parser. That gap is worth its own issue:
this is the second silent revert on `main` in a day (the first was
#2633's stale-base squash, repaired by #2640), and both passed CI.

## Related

Refs #2599 (already closed by #2638; this PR only restores the
documented grammar that #2638 shipped).

---------

Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
cursor Bot pushed a commit that referenced this pull request Aug 16, 2026
Detects a merge that silently deleted content another recently-merged
commit had just added, with nothing in its own message saying it meant
to. This is item 4 of #2691.

The canary blames the lines each merge removed against its parent and
reports when a large block traces to a single commit inside a recency
window. Volume, recency, constrained intent markers, and a
non-blocking post-merge-only trigger keep the firing rate at 1% over
the last 500 first-parent commits of main.

Two designs were measured and rejected first. Merge-base staleness
exonerates all three real incidents, because every reverting branch
was up to date with main in HISTORY and stale only in CONTENT --
`git merge-base --is-ancestor f603880 refs/pull/2641/head` is true
while that branch's tree carried zero occurrences of #2639's markers.
Curated marker strings only catch what somebody pre-registered, and
nobody had registered the fixes that were lost.

That evidence also corrects the issue's premise: this was not a
stale-BASE failure, so `strict_required_status_checks_policy` would
have passed all three merges, and so would a merge queue (CI was green
-- the tests were deleted alongside the code). No ruleset is touched
here.

Calibration surfaced a third, previously unfiled incident: #2633's
squash dropped #2632's rollups (853 lines), which #2640 restored by
hand.

Refs #2691

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018S8a1S71VxhLTRWBtMuEvp
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 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 16, 2026
…proves

Verification of the previous commit caught the setup paragraph still asserting
the thesis the conclusion now demolishes:

  "The highest verified-legitimate commit scores well below the threshold
   below. The separation is what makes the canary livable."

Both halves are false at the shipped settings. The highest verified-legitimate
fire is 390 (#2640) against a threshold of 200 -- nearly double it, not well
below. And there is no separation to rest on: the smallest true finding is
301, under both legitimate fires (340, 390), so the populations overlap.

Replaced with what the measurements actually support -- content-based
detection fires on all three incidents where the two rejected designs
exonerate them, and it is the disposition path plus the non-blocking posture,
not any threshold, that keeps the canary livable.

Both sentences predate #2832 and were already false on main (340 > 200 there
too). They are repaired here because this branch rewrote the paragraph they
sit in, and because leaving the file self-contradictory on its central claim
is the same defect class this PR exists to close.

Comments only. The non-comment body of check-silent-revert.sh remains
byte-identical to origin/main, both fixtures' parsed fields are unchanged, and
27/27 detector tests plus --verify-known-incidents pass at the unchanged
shipped settings.

Refs: #2846

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
kyle-sexton added a commit that referenced this pull request Aug 16, 2026
…alsified

Recording #2642 as a second victim of #2633's squash added a fourth true
finding at 301 lines, and two surviving sentences still described a corpus
that no longer exists:

  check-silent-revert.sh    "340 is the largest false positive and 346 is the
                             smallest true one ... that 2% gap"
  silent-revert-acknowledged.txt
                            "At 340 lines this is the largest measured false
                             positive, six lines below the smallest real
                             incident (346)"

Measured at the shipped settings, the smallest true finding is 301, and BOTH
verified-legitimate fires score above it -- 340 (#2135) and 390 (#2640). The
relationship is inverted, not narrowed: the ranges overlap outright and there
is no gap to split. That strengthens the existing conclusion rather than
changing it, so the threshold stays at 200 and the disposition path stays the
thing that makes the canary livable. Also records that 200 and 300 still fire
on the identical five commits, with the 301 finding surviving 300 by one line.

Separately, #2656 was mischaracterized. It does record the event as a silent
revert -- "#2633 was a stale-base squash that silently reverted two merged
features" -- and it is what it ASKED for, not what it recorded, that was
coverage-shaped. Corrected and quoted.

Comments and fixture note text only. The non-comment body of
check-silent-revert.sh is byte-identical to origin/main, both fixtures' parsed
fields (expect/sha and sha) are unchanged, and 27/27 detector tests plus
--verify-known-incidents pass at the unchanged shipped settings.

Refs: #2831

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
kyle-sexton added a commit that referenced this pull request Aug 16, 2026
…proves

Verification of the previous commit caught the setup paragraph still asserting
the thesis the conclusion now demolishes:

  "The highest verified-legitimate commit scores well below the threshold
   below. The separation is what makes the canary livable."

Both halves are false at the shipped settings. The highest verified-legitimate
fire is 390 (#2640) against a threshold of 200 -- nearly double it, not well
below. And there is no separation to rest on: the smallest true finding is
301, under both legitimate fires (340, 390), so the populations overlap.

Replaced with what the measurements actually support -- content-based
detection fires on all three incidents where the two rejected designs
exonerate them, and it is the disposition path plus the non-blocking posture,
not any threshold, that keeps the canary livable.

Both sentences predate #2832 and were already false on main (340 > 200 there
too). They are repaired here because this branch rewrote the paragraph they
sit in, and because leaving the file self-contradictory on its central claim
is the same defect class this PR exists to close.

Comments only. The non-comment body of check-silent-revert.sh remains
byte-identical to origin/main, both fixtures' parsed fields are unchanged, and
27/27 detector tests plus --verify-known-incidents pass at the unchanged
shipped settings.

Refs: #2846

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
kyle-sexton added a commit that referenced this pull request Aug 17, 2026
…alsified

Recording #2642 as a second victim of #2633's squash added a fourth true
finding at 301 lines, and two surviving sentences still described a corpus
that no longer exists:

  check-silent-revert.sh    "340 is the largest false positive and 346 is the
                             smallest true one ... that 2% gap"
  silent-revert-acknowledged.txt
                            "At 340 lines this is the largest measured false
                             positive, six lines below the smallest real
                             incident (346)"

Measured at the shipped settings, the smallest true finding is 301, and BOTH
verified-legitimate fires score above it -- 340 (#2135) and 390 (#2640). The
relationship is inverted, not narrowed: the ranges overlap outright and there
is no gap to split. That strengthens the existing conclusion rather than
changing it, so the threshold stays at 200 and the disposition path stays the
thing that makes the canary livable. Also records that 200 and 300 still fire
on the identical five commits, with the 301 finding surviving 300 by one line.

Separately, #2656 was mischaracterized. It does record the event as a silent
revert -- "#2633 was a stale-base squash that silently reverted two merged
features" -- and it is what it ASKED for, not what it recorded, that was
coverage-shaped. Corrected and quoted.

Comments and fixture note text only. The non-comment body of
check-silent-revert.sh is byte-identical to origin/main, both fixtures' parsed
fields (expect/sha and sha) are unchanged, and 27/27 detector tests plus
--verify-known-incidents pass at the unchanged shipped settings.

Refs: #2831

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
kyle-sexton added a commit that referenced this pull request Aug 17, 2026
…proves

Verification of the previous commit caught the setup paragraph still asserting
the thesis the conclusion now demolishes:

  "The highest verified-legitimate commit scores well below the threshold
   below. The separation is what makes the canary livable."

Both halves are false at the shipped settings. The highest verified-legitimate
fire is 390 (#2640) against a threshold of 200 -- nearly double it, not well
below. And there is no separation to rest on: the smallest true finding is
301, under both legitimate fires (340, 390), so the populations overlap.

Replaced with what the measurements actually support -- content-based
detection fires on all three incidents where the two rejected designs
exonerate them, and it is the disposition path plus the non-blocking posture,
not any threshold, that keeps the canary livable.

Both sentences predate #2832 and were already false on main (340 > 200 there
too). They are repaired here because this branch rewrote the paragraph they
sit in, and because leaving the file self-contradictory on its central claim
is the same defect class this PR exists to close.

Comments only. The non-comment body of check-silent-revert.sh remains
byte-identical to origin/main, both fixtures' parsed fields are unchanged, and
27/27 detector tests plus --verify-known-incidents pass at the unchanged
shipped settings.

Refs: #2846

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
kyle-sexton added a commit that referenced this pull request Aug 17, 2026
…tector

PR #2843 pinned attribute_file's diff and blame flags to git's own defaults.
Three of the figures the calibration prose quotes move under those pins,
because the original calibration was taken on a machine carrying
diff.algorithm = histogram: #2640 reads 447 rather than 390, #2135 reads 323
rather than 340, and #2642's share of #2633's squash reads 298 rather than 301.
A calibration comment that states a number the shipped detector no longer
produces is the defect this branch exists to fix, so every sentence carrying a
figure was re-measured rather than patched.

The overlap argument survives and is stated more strongly. The smallest true
finding is 298 and both cleared fires score above it, at 323 and 447, so the
populations invert rather than merely abut.

The 200-versus-300 sentence inverted outright and was rewritten from
measurement. It claimed the finding survived a threshold of 300 by a single
line; under the pins that finding is 298 and disappears instead. The commit set
at 200 and 300 is still identical, because cc58cbc keeps its 853-line
finding, but the finding set is not, and running the replay at 300 reports
cc58cbc as firing but not as recorded.

Three further corrections the re-measurement surfaced. The corpus endpoint is
now named as a sha rather than written as "the last 500 first-parent commits",
which is a moving window that falsifies itself on the next merge. Detection and
disposition are now distinguished: five commits cross the threshold but two are
cleared by the acknowledgment file, so a reader sees three. And the two recall
gaps are acknowledged rather than implied, so the corpus figures read as floors
by construction: attribute_file swallows git's stderr (#2880), and paths marked
-diff or binary in .gitattributes contribute nothing (#2883).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LwdkpWf6bptu3AqTMoeg2H
kyle-sexton added a commit that referenced this pull request Aug 17, 2026
…tor's counts (#2847)

Closes #2846.

## Summary

The calibration comments in `scripts/check-silent-revert.sh` carry the
argument that
justifies the 200-line threshold. Two of their sentences depended on
which finding is
the smallest, and #2832 had already falsified both by recording a fourth
true finding.
This PR repairs them — and re-derives every figure they rest on against
the detector as
PR #2843 pins it, because three of those figures move under the pins.

Found by fresh-context verification of #2832, after it had merged.

## Why the numbers moved

PR #2843 pins `attribute_file`'s git invocations to git's own defaults

(`--diff-algorithm=myers`, `--no-ext-diff`, `--no-textconv`,
`--no-ignore-revs-file`,
`-M`). The original calibration was taken on a machine carrying
`diff.algorithm = histogram`, and the algorithm choice changes which
lines a hunk calls
deleted. Every figure below was re-measured against the pinned detector
and reproduced
byte-identically with `GIT_CONFIG_GLOBAL` emptied, which is the property
`t_counts_are_immune_to_ambient_git_config` asserts.

| commit | PR | pre-pin | pinned | class |
| --- | --- | ---: | ---: | --- |
| `cc58cbc53` | #2633 | 853 | **853** | incident |
| `cc58cbc53` | #2633 (#2642's share) | 301 | **298** | incident |
| `9239f1541` | #2641 | 451 | **451** | incident |
| `f603880da` | #2639 | 346 | **346** | incident |
| `6f0a31109` | #2640 | 390 | **447** | cleared |
| `91e77fc16` | #2135 | 340 | **323** | cleared |

## Fix

**The overlap argument survives; every sentence stating it was
re-derived.** The
smallest true finding is 298 and both cleared fires score above it, at
323 and 447. The
relationship is an inversion, not a narrow gap — so `NO THRESHOLD
SEPARATES THEM` is
now true by a wider margin than the six-line version it replaces.
THRESHOLD stays 200.

**The 200-vs-300 sentence inverted and was rewritten from measurement,
not patched.**
The old text said "at 300 the 301-line finding survives by a single
line". Under the
pins that finding is 298, so at 300 it VANISHES. The COMMIT set at 200
and 300 is still
identical — `cc58cbc53` keeps its 853-line finding — but the FINDING set
is not, and
that is the sharper argument against tuning. Measured:

```
$ SILENT_REVERT_THRESHOLD=300 scripts/check-silent-revert.sh --verify-known-incidents
FAIL cc58cbc fires, but NOT as recorded
     recorded attribution:
       bfb66be 853
       eda5ae5 298
     what the detector reported:
       bfb66be 853
EXIT=1
```

Without #2833's attribution expectations that row would have passed on
the surviving
853-line finding and announced a reproduction it never performed. The
passage now says
that, and cites `t_replay_asserts_the_recorded_attribution`, which pins
the same
two-culprit shape.

**Detection and disposition are now distinguished.** The corpus sentence
said the canary
"fires on 5 commits" and left a reader to assume that is what CI shows.
It is not:
`6f0a31109` and `91e77fc16` are in
`scripts/silent-revert-acknowledged.txt`, so
`scan_commit` clears each before it attributes a line. Five commits
cross the threshold;
three print `SILENT REVERT SUSPECTED`. The prose now states both and
says which one the
threshold is calibrated against.

**The corpus endpoint is pinned.** "the last 500 first-parent commits of
main" is a
moving window that falsifies itself on the next merge — the same defect
class this PR
closes. It now reads "the 500 first-parent commits of main ending at
`738791c45`".

**Two recall gaps are acknowledged rather than implied.**

- `attribute_file` swallows `git` stderr on both commands that produce a
count, so a
failed diff or blame is indistinguishable from nothing-to-attribute and
can only
subtract. Every corpus enumeration is therefore a floor, and the prose
is worded so
  the caveat is structural rather than an appended qualifier (#2880).
- Paths the repository's `.gitattributes` marks `-diff` or `binary`
produce no hunks, so
their deletions attribute to zero on every machine including CI (#2883).
This is a
RECALL gap, not a calibration one: no path of that class appears in any
commit whose
figure is quoted, and the largest such deletion anywhere in the sweep
was 72 lines
  from a `package-lock.json` — well under the threshold.

**Also corrects a mischaracterization of #2656** that #2832 introduced,
which said that
issue recorded the event "rather than as a silent revert". Its Evidence
section opens
with *"#2633 was a stale-base squash that silently reverted two merged
features."* It
recorded it exactly as a silent revert — what was coverage-shaped was
what it **asked
for**. Now quoted rather than paraphrased.

## Not fixed here

The fresh-context verifier confirmed each of these; every one sits in
prose this PR does
not own, and each needs a rewording rather than a renumber.

- **"main's 1527-commit history"** (twice). Reproduces at no named
endpoint — measured
1546 at `738791c45`, 1549 at `origin/main`. The claims it supports are
unaffected and
do reproduce: `Revert "` = 0, `revert:` = 1, that one being `1d1fca6e8`
(#1839).
Renumbering it would be falsified by the next merge, which is the same
moving-window
  defect this PR removes elsewhere.
- **"the replay exited 0 for 31h28m"**. The duration reproduces exactly,
but it is the
content-absence window; the replay itself only existed for about 6h13m
of it. #2873's
prose, and the same conflation appears once in
`silent-revert-incidents.txt`.
- **"a four-row corpus"**. There are 5 `marker` rows, and 3 `fires` + 1
`clean`
expectation rows; the sentence's own unit is one read per marker, so 5.
#2873's prose.
- **The repo-wide-grep counterfactual.** Its present-tense half holds,
but at the tree
where both markers were actually missing, a repo-wide grep would have
falsely cleared
only the README half — the CHANGELOG copy that makes the claim true
today was added by
  the restore commit itself. #2873's prose.
- **The `clean` row's "129 lines"** reads 136 under the pins. Issue
#2865 owns that row;
  correcting it here would collide.

## Related

- PR #2843 — merged ahead of this one; it added the pins that move three
of the figures
here, and its pin table records the pre-pin and pinned columns side by
side. This PR
  layers prose on top of it and changes no pin.
- PR #2873 — merged ahead of both; added the restoration markers and the
`marker) continue ;;` arm. Untouched here and verified intact after the
rebase.
- Refs #2880 — `attribute_file` swallows git stderr, which is why the
corpus figures are
worded as floors rather than exact counts. Acknowledged here, not fixed.
- Refs #2883 — paths marked `-diff` or `binary` contribute zero to
attribution.
  Acknowledged here as a recall gap, not fixed.
- Refs #2865 — owns the `clean` row whose "129 lines" reads 136 under
the pins.
  Deliberately left to that lane.
- Refs #2832 — the corpus attribution correction that added the fourth
finding and
  falsified the two sentences this PR repairs.

## Verification

- Every figure re-measured against the shipped detector on this branch,
twice — once
inheriting ambient config and once with `GIT_CONFIG_GLOBAL` emptied —
with identical
  results.
- `scripts/check-silent-revert.test.sh` (101 passed, 0 failed) and both
replay modes run
  green against the merged content.
- An independent fresh-context verifier re-measured every number in the
calibration
comments without access to this reasoning, running the full 500-commit
corpus sweep
rather than per-commit checks alone. Every figure this PR states
reproduced. It raised
two defects in the new prose, both fixed here: "the number a reader sees
in CI is 3"
read as findings when it means commits (three commits, four findings
between them),
and "roughly once a month" was 12-19x off — the corpus spans 7.9 days
with four of its
five crossings inside 76 minutes, so that rate claim was removed rather
than
renumbered, because the corpus measures a burst and no per-month figure
is defensible
from it. Its full verdict, including drift it confirmed in prose this PR
does not own,
  is recorded in the PR comments.

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
kyle-sexton added a commit that referenced this pull request Aug 17, 2026
Re-pinning `_FIND_SIDE_EFFECT_PRIMARIES` and `MERGED_PR_GRAPHQL_ALIAS_PAGE`
from bare identifiers to their definition lines closed only one direction. A
definition line occurs exactly once and a use-site-only re-land cannot contain
it, so the use-only false green is gone -- but the inverse opened in its place:
a re-land that restores the definition and never reconnects the use satisfies
the row while the guard stays disconnected.

So each constant is now TWO marker rows, its definition and its only use, and
both must hold. A definition-only re-land passes the definition row and fails
the use row; a use-only re-land does the reverse.

Measured for both added literals, and re-measured independently for all seven
marker rows now in the corpus: each occurs exactly once in the file it binds
to on the working tree and at origin/main, zero times in that path at its
recorded incident sha, and at least once at that sha's first parent.

Also corrects the note above the 9239f15 row. It credited #2640 with the
destructive_guard.py restore; #2640 (6f0a311) changes no file under
plugins/disk-hygiene, both markers are absent at the only two intervening
commits that touched the file (#2671, #2676), and both first appear at
de26c27 (#2706). The #2640 credit on the cc58cbc row is correct and is
left alone.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LwdkpWf6bptu3AqTMoeg2H
kyle-sexton added a commit that referenced this pull request Aug 17, 2026
…two loose markers (#2930)

## What

Two related fixes to the silent-revert canary, both about claims that do
not hold: four comment claims that no longer reproduce, and two `marker`
literals that bind more loosely than the corpus prose implies.

## Part 1 — four comment claims re-derived (#2917)

None is executable, so nothing failed — which is the problem: they read
as measured, so the next person to touch the calibration would have
built on them. Each is **rewritten to a claim that stays true**, not
renumbered to a figure that rots again. That follows the standard set in
#2847, which removed a "fires roughly once a month" claim rather than
renumbering it, because the corpus measures a burst and no honest
per-month rate exists.

1. **"1527-commit history", stated twice.** Reproduced at no named
endpoint — 1546 at `738791c45`, 1549 shortly after. Now "every
first-parent commit of main". The result the pin rests on (`Revert "` 0,
`revert:` 1) is unchanged and now leads the sentence.
2. **"the replay exited 0 for 31h28m."** Credited the replay with a
silence it was not present for. 31h28m is the **content-absence**
window; this canary merged part-way through it at `7b47d2253`, 6h13m
before the restore. The passage now separates the two windows and states
what was true over the tail: the `f603880da` row reproduced the removal
exactly as recorded while the content was still gone. The row was never
wrong about what it asserted — it asserted the other question.
3. **The repo-wide-grep counterfactual.** Mixed tenses. The
present-tense half holds; the past-tense half did not, because
`CHANGELOG.md` got its copy of the marker from `534eac138` — the restore
commit itself — so during the absence a repo-wide grep for the evals
marker still matched nothing.
4. **"A four-row corpus" / "all five markers."** Counts that go stale as
rows are added, neither load-bearing. Now "per marker row" and "every
marker".

## Part 2 — two loose markers split into definition and use rows (#2915)

`--verify-restoration` tests a marker with `grep -qF`, which answers "is
this string anywhere in this file". Two markers were bare identifiers
occurring **twice** in their bound file:

| marker | definition | use site |
|---|---|---|
| `_FIND_SIDE_EFFECT_PRIMARIES` | `destructive_guard.py:1010` | `:1047`
|
| `MERGED_PR_GRAPHQL_ALIAS_PAGE` | `audit-fleet.sh:286` | `:2257` |

A re-land restoring only the use site would satisfy the grep and the row
would report `ok` — a partial re-land reported as restored, exactly the
failure the assertion exists to catch (#2828 was a partial re-land
nobody noticed).

The first pass narrowed each marker to its definition line. Review
pointed out that this closes only one direction: a re-land restoring the
definition while omitting the use would still report `ok`, leaving the
guard defined but disconnected. Narrowing cannot fix that — a single row
can only bind one of the two halves.

So each constant is now recorded as **two rows, a definition row and a
use row**:

```
marker 9239f15… destructive_guard.py _FIND_SIDE_EFFECT_PRIMARIES = frozenset(
marker 9239f15… destructive_guard.py name.casefold() for name in _FIND_SIDE_EFFECT_PRIMARIES
marker cc58cbc… audit-fleet.sh       MERGED_PR_GRAPHQL_ALIAS_PAGE=100
marker cc58cbc… audit-fleet.sh       gql_page_end=$((gql_page_start + MERGED_PR_GRAPHQL_ALIAS_PAGE))
```

A definition-only re-land passes the definition row and fails the use
row; a use-only re-land does the reverse. Each literal is the whole
expression rather than a bare identifier, and none depends on leading
whitespace — `parse_incidents_file` splits with `read -r kind sha path
rest`, so leading and trailing whitespace on the literal is stripped and
interior whitespace preserved.

**Being precise about what was broken:** both bare forms *did* still
discriminate the recorded incidents, because those reverts removed
definition and use together. Measured at the recorded revs, each bare
marker is absent at the reverting commit and present at its parent. The
weakness was never in what they assert about the past — it was that the
future shapes the assertion exists to catch could slip past them.

## Measurements behind this PR

All by `grep -oF … | wc -l` against the bound file, `git show
<rev>:<path>`, and `git show <rev>^:<path>`. Every marker row now in the
corpus, not only the changed ones:

| literal | bound file | worktree | `origin/main` | at parent | at rev |
|---|---|---|---|---|---|
| `Safe tidiness is the primary objective` | `disk-hygiene/README.md` |
1 | 1 | 1 | 0 |
| `empty-directories-remain-first-class-tidiness-findings` |
`clean/evals/evals.json` | 1 | 1 | 1 | 0 |
| `_FIND_SIDE_EFFECT_PRIMARIES = frozenset(` | `destructive_guard.py` |
1 | 1 | 1 | 0 |
| `name.casefold() for name in _FIND_SIDE_EFFECT_PRIMARIES` |
`destructive_guard.py` | 1 | 1 | 1 | 0 |
| `--apply-plan may be supplied only once` | `audit-fleet.sh` | 1 | 1 |
1 | 0 |
| `MERGED_PR_GRAPHQL_ALIAS_PAGE=100` | `audit-fleet.sh` | 1 | 1 | 1 | 0
|
| `gql_page_end=$((gql_page_start + MERGED_PR_GRAPHQL_ALIAS_PAGE))` |
`audit-fleet.sh` | 1 | 1 | 1 | 0 |

Every literal occurs exactly once in the file it binds to, is present at
the parent of its reverting commit, and is absent at the reverting
commit — the discrimination a marker must have. Both new literals were
additionally swept commit-by-commit and are present at every
first-parent commit that has touched their file since the restore.

## One attribution corrected

The corpus prose above the `9239f1541` row credited **#2640** with
restoring `destructive_guard.py`. Measured: #2640 (`6f0a31109`) touches
four files, all under `plugins/repo-fleet-hygiene`, none under
`plugins/disk-hygiene`. Both markers are absent at the two intervening
first-parent commits that touched the file (`224b04070` #2671,
`1a612904b` #2676) and first appear at `de26c27b4` (#2706). The note now
credits #2706 and records the correction in the file's own established
shape. The #2640 credits on the `cc58cbc53` row are correct —
`6f0a31109` *is* the `audit-fleet.sh` restore — and were left alone.

## Verification note

The comment changes touch no executable behavior. The marker rows are
data the replay reads, so `--verify-restoration` is the check that
matters and it runs in CI on this PR. The suite was **not** run locally
against this tree — CI is the verification of record.

Closes #2917
Closes #2915

## Related

- #2880 — `attribute_file` swallows blame stderr, so counts are lower
bounds
- #2879 — `clean` rows carry no attribution field, so nothing in CI
asserts their figures

---------

Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

repo-fleet-hygiene: no conformance report against the configured worktree root, so ~245 misplaced worktrees are invisible

1 participant