Skip to content

fix(repo-hygiene): finish the #3346 clean-skill ladder and make the #3852 guard prose decision-neutral - #5319

Merged
kyle-sexton merged 15 commits into
mainfrom
fix/audit-repo-hygiene
Sep 29, 2026
Merged

kyle-sexton merged 15 commits into
mainfrom
fix/audit-repo-hygiene

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Refs: #3346
Refs: #3852
Refs: #3708

Summary

Finishes the /repo-hygiene:clean ladder for #3346 (read-only scan tier, cross-repo branch and stash audits, bulk fact reads, WORKTREE branches handed to /source-control:worktree, off-default clones pointed at /repo-fleet-hygiene:sync) and makes the branch-deletion guard prose for #3852 decision-neutral: it states current coverage and the open question without recording a decision. Declares Node.js as a repo-hygiene prerequisite (#3708). Releases repo-hygiene 0.11.0.

Fix

Verification

  • All 17 plugins/repo-hygiene *.test.sh pass on the branch merged with current origin/main.
  • scripts/validate-plugins.sh passes.
  • scripts/check-changelog-parity.sh --check --check-order, --check-bump origin/main and --check-preserved origin/main pass.
  • shellcheck clean on all changed .sh; scripts/check-changed-skills.sh origin/main reported 0 failed before the merge.
  • markdownlint-cli2 clean on README, CHANGELOG, setup SKILL.md and clean/reference/invocation-forms.md; scripts/check-shell-portability.sh origin/main clean.

Related

Audit findings for #3346 and #3852 in .work/audit/REPORT.md (3b partial delivery for both, 3d decide-fresh for #3852).

Cross-group requests:

🤖 Generated with Claude Code

https://claude.ai/code/session_01EugXnFddtpHcY5gTuyEirB

kyle-sexton and others added 10 commits September 29, 2026 01:06
…ard prose

Refs #3346
- clean SKILL: git-prune, git-tree-reset, remove-path and tree-batch apply steps carry the CLEAN_GUARD_ACK prefix
- git-tree-reset context: hook interaction states that --apply is blocked without the prefix
- drop the duplicated argument list, point at the action router
- note that preflight facts are as of the dry-run and planned bytes may exceed removed bytes

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EugXnFddtpHcY5gTuyEirB
…ecision

Refs #3852
Refs #3346
- guard header, clean SKILL, README: say the guard does not match `git branch -D`/`-d` or `git push --delete` as of 2026-09-29 and that whether it should is open on #3852, replacing the settled-gap wording
- drop unsourced claims (remote deletion not a sanctioned path, permission-flow gating, undefined paid slice); keep a claim/basis/as-of/recheck record
- header: coverage is whatever is_destructive() lists; record the --apply and forced worktree remove additions with their own record
- allowed-tools-pairing test header: drop the false "matches none of these six scripts"
- destructive-guard.test.sh: retitle the allow-loop label only; is_destructive() and all assertions unchanged

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EugXnFddtpHcY5gTuyEirB
…prefix

Refs #3346

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EugXnFddtpHcY5gTuyEirB
…e, point off-default clones at /repo-fleet-hygiene:sync

The branch audit reports the checkout path of a WORKTREE branch on a
`Worktree:` line, leaving the nine-column tip capture unchanged. The git
action invokes `/source-control:worktree cleanup --dry-run` for those
branches, lets it own removal, and re-audits. A clone blocked by being off
the default branch is fixed by `/repo-fleet-hygiene:sync`, presented as a
command, not invoked.

Refs #3346

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EugXnFddtpHcY5gTuyEirB
`clean-batch.sh --tier scan` runs the unchanged scan.sh per selected repo with
the shared repo resolution and skip list, prints a per-repo `Outcome: scanned`,
and closes with `Summary: repos=N planned=0 bytes=K` (K the summed
`Total reclaimable`). It writes no plan; `--apply` and `--batch-plan` with it are
usage errors. resolve-clean-action.sh gains `scan-batch`, `scan-fleet`, and
`inventory-batch`; the router, SKILL.md, clean-batch.md, and evals follow.

Refs #3346

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EugXnFddtpHcY5gTuyEirB
git-branch-audit.sh and git-stash-audit.sh take clean-batch's repo selection
(--repo, --repos-from, --skip, --skip-from) through lib/batch-common.sh. Each
repo prints as a `Repo: <path>` block; linked worktrees sharing a git common dir
are audited once, a skipped, unresolvable, or failing repo is reported without
stopping the rest, and `FleetSummary:` closes the run with exit 0. --capture-file
with more than one repo is a usage error; each repo writes its own capture. With
no selection flag the output is unchanged.

Deletion is not batched: git-branch-delete.sh resolves the repo from the working
directory and refuses a capture whose `# common_dir:` is another repo's, so the
docs say to run it from inside the audited repo. SKILL.md, the branch-cleanup and
clean-batch context, and the README follow; the manual StashStore dedup step is
now built in.

Refs #3346

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EugXnFddtpHcY5gTuyEirB
Refs #3346

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EugXnFddtpHcY5gTuyEirB
…anch

git-branch-audit.sh spawned about six git processes per branch (tip, upstream,
commits ahead and behind, commits absent from origin/<default>, loss count).
It now reads them together:

- one for-each-ref carries each branch's tip, upstream and ahead/behind summary
- one `rev-list --parents --stdin --not origin/<default>` and one
  `... --not --remotes --tags` give the commits absent from origin/<default>
  and the LOSSY loss count for every tip (clean_unreached_counts), the number
  `git rev-list --count <tip> --not <rev>...` prints per tip; other local
  branches still do not count as "elsewhere"
- one read of the worktree porcelain gives every WORKTREE branch's path

A branch whose record cannot be trusted (a short name ambiguous with a tag or
remote, a `[gone]` upstream whose ref exists, an unrecognized ahead/behind
summary) and any pass that fails take the previous per-branch commands, so a
verdict never depends on which path answered. The LossCommit listing stays one
`git log` per LOSSY branch: a shared walk can order commits differently when
dates tie or skew.

On a 242-branch fixture the audit went from 1404 to 110 git calls, of which
83 are the per-LOSSY-branch listings; stdout and the tip capture are
identical. The suite holds the bulk facts equal to the per-branch reads on every
fixture, bounds the calls outside the listings independent of the branch
count, and checks a failed pass gives the same report.

Refs #3346

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EugXnFddtpHcY5gTuyEirB
Refs #3346
Refs #3852

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EugXnFddtpHcY5gTuyEirB
@github-actions

github-actions Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

PR body contract — issue linkage

This PR body conforms to the issue-linkage contract. Nothing to do.

…aside

Refs #3346
Refs #3852

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EugXnFddtpHcY5gTuyEirB
kyle-sexton and others added 3 commits September 29, 2026 11:18
…bash lookup

Every hook row runs `node hooks/exec-bash.mjs`, and Claude Code's native
binary neither ships nor uses Node, so without it the destructive guard does
not launch. The README gains a Requirements section, `setup check` gains a
node row probed through Bash, and its bash lookup names the PATH step the
launcher gains in #5309.

Refs #3708

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EugXnFddtpHcY5gTuyEirB
…nd invocation docs

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EugXnFddtpHcY5gTuyEirB
@kyle-sexton
kyle-sexton marked this pull request as ready for review September 29, 2026 16:04
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-29T16:08:54.965857Z 2ae4d21 Draft marked ready
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@claude

claude Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 2m 21s —— View job


Security review (/review:security-review)

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

Scope reviewed: all 26 changed files in this PR (diffed against the actual GitHub PR content via gh pr diff 5319, since origin/main in this checkout is a shallow/stale ref with no merge base — the known CI-checkout limitation the skill documents). Focus was on the security-relevant surfaces: destructive-guard.sh/.test.sh, clean-batch.sh, git-branch-audit.sh/.test.sh, git-stash-audit.sh/.test.sh, lib/batch-common.sh, lib/clean-common.sh, resolve-clean-action.sh, plus the guard-coverage prose in SKILL.md/README.md/setup/SKILL.md.

No security issues found.

Notes from the review:

  • destructive-guard.sh: the only change in this file is to header comments (lines above set -uo pipefail); is_destructive() itself is byte-for-byte unchanged, and destructive-guard.test.sh's actual assertions are unchanged (only surrounding comment text was reworded). The guard's coverage, the ack-prefix mechanism, and the confirmation-gate flow are all identical before and after.
  • Instruction-surface-deletion lens (per this skill's guidance, checked against docs/conventions/instruction-exception-register/README.md): the repo-hygiene:clean: decide whether the destructive guard should cover branch and remote-branch deletion #3852 prose change replaces "accepted coverage gap, not a defect" with "open owner decision" language in destructive-guard.sh, SKILL.md, and README.md. This reframes a decision as open rather than settled, but it does not remove or weaken any enforcement — the actual gap (git branch -D/-d, git push --delete not matched by is_destructive()) is identical before and after, and no Gate 0 class (irreversible-action, secret-handling, data-integrity, external-publication, legal-compliance, agent-authority) applied to a mechanism is deleted here, only narrative framing around a pre-existing, unchanged gap. Not a finding.
  • New fleet/bulk-read code (clean-batch.sh --tier scan, git-branch-audit.sh/git-stash-audit.sh --repo/--repos-from/--skip/--skip-from, lib/batch-common.sh batch_resolve_repos/batch_run_fleet, lib/clean-common.sh clean_unreached_counts): all repo paths are validated as real directories/git repos before use (batch_resolve_repos), interpolated only into quoted git -C "$path" / cd "$path" invocations (never eval'd or passed through a shell string), and the new bulk for-each-ref/rev-list --stdin/cat-file --batch-check parsing uses read/awk -v against git's own trusted output — no command-injection or path-traversal surface introduced. All new multi-repo paths are read-only; branch/stash deletion is explicitly kept out of the batch surface and still routes through the single-repo confirmation gate, consistent with the existing safety model.
  • remove-path.sh, git-tree-reset.sh/-batch.sh guard/gate wiring is unchanged; the diff only updates their docs to state the existing CLEAN_GUARD_ACK=1 requirement explicitly.

Branch: fix/audit-repo-hygiene

@claude

claude Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

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


Review in progress

  • Gather context (PR diff, changed files, CLAUDE.md/REVIEW.md)
  • Review shell script changes (fleet mode, bulk reads, guard prose)
  • Review docs/changelog for accuracy against code — found and verified one issue
  • Adversarially verify candidate findings
  • Post inline findings and final summary

@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: 2ae4d21ee1

ℹ️ 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-hygiene/skills/setup/SKILL.md
Add the four-part verification record (claim, basis, as-of, recheck) for the statement that Claude Code's native binary neither ships nor uses Node.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@claude

claude Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

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


Reviewing PR #5319

  • Gather context (PR diff, changed files, custom instructions)
  • Invoke /review:code-review and follow it exactly
  • Deep review of the 3 highest-risk script surfaces (git-branch-audit.sh fleet/bulk logic, git-stash-audit.sh + lib/batch-common.sh, clean-batch.sh + lib/clean-common.sh/destructive-guard.sh), running as parallel subagents
  • Manual read of docs (SKILL.md, README.md, setup/SKILL.md, CHANGELOG.md, context/*.md) — confirmed the Codex-flagged missing verification record for the Node-packaging claim was already fixed in the latest commit (aa7218b)
  • Adversarially verify candidate findings
  • Post findings
    · Branch

@claude

claude Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 2m 9s —— View job


Security review (/review:security-review)

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

Scope reviewed: this run was triggered by a new push (2ae4d21 → aa7218b) after this lane's prior review on 2ae4d21 already found no issues across all 26 files. git diff 2ae4d21..aa7218b shows the incremental commit touches exactly one file, plugins/repo-hygiene/skills/setup/SKILL.md, adding a 5-line Claim/Basis/As-of/Recheck verification record (the block Codex flagged as missing in its P1 comment) — pure prose, no script, no logic, no instruction-surface change. No security-relevant surface there.

To confirm the prior "no issues" conclusion still holds for the full diff at this head, I independently re-read the security-critical surfaces rather than taking that on faith:

  • destructive-guard.sh (lines shown in the diff): only header-comment rewording; is_destructive() is unchanged, destructive-guard.test.sh assertions are unchanged. Guard/ack-prefix/confirmation-gate mechanics are identical before and after.
  • lib/batch-common.sh (new, 103 lines) — batch_resolve_repos, batch_normalize_input, batch_skip_match, batch_add_gitdir: every path is validated with [[ -d ]] / git -C "$path" rev-parse before use, always double-quoted, never eval'd or built into a shell string. No injection or traversal surface.
  • git-branch-audit.sh bulk-read path (for-each-ref, cat-file --batch-check, clean_unreached_counts): all parsing is read/mapfile over git's own trusted output using a \x1f field separator, associative-array keyed, never interpolated into a command string. A record that doesn't fit the expected shape (bulk_record_ok, bulk_facts) falls back to the existing per-branch path rather than misclassifying.
  • clean-batch.sh --tier scan: verified directly in the checked-out script (lines 167–169) that --apply and --batch-plan are rejected with a usage error (exit 2) before any repo loop runs when --tier scan is selected — there's no path for the read-only tier to reach the apply/removal code. Consistent with the PR's stated design that deletion is never batched.
  • The #3852 guard-coverage prose change (destructive-guard.sh header, SKILL.md, README.md) reframes a decision as open rather than settled, but doesn't remove or weaken any enforcement: the actual gap (git branch -D/-d, git push --delete not matched by is_destructive()) is identical before and after, and no enforcement mechanism is deleted — narrative framing only, consistent with the earlier review's finding on this same lens.

No security issues found.

Branch: fix/audit-repo-hygiene

@kyle-sexton
kyle-sexton merged commit b5b4593 into main Sep 29, 2026
19 checks passed
@kyle-sexton
kyle-sexton deleted the fix/audit-repo-hygiene branch September 29, 2026 16:44
kyle-sexton added a commit that referenced this pull request Sep 30, 2026
…he clean skill (#5364)

Closes #3852

## Summary

The clean skill's destructive guard now matches bare `git branch
-D/-d/--delete` and remote-branch deletion (`git push --delete`, `git
push -d`, `git push origin :ref`, `git push origin +:ref`). This
implements the owner decision on #3852 (Option A): the guard stays a
best-effort net, the `CLEAN_GUARD_ACK` path lifts the block, the
`--apply` coverage from #5130 is ratified, and the host-permission
interaction stays as documented.

## Fix

- `destructive-guard.sh`: `is_destructive()` matches a `git branch`
delete-flag cluster or `--delete`, and `git push` with `--delete`, a
`-d` cluster, or a `:`- or `+:`-prefixed refspec. A quoted delete flag
(`"--delete"`, `"-d"`) matches too. `git branch
-a/-r/-vv/--list/--show-current`, `git push origin main`,
`HEAD:refs/heads/x` and `--dry-run`/`-n` deletes stay allowed; a fused
`-o<value>` push option is not read as a dry-run. Known gap, in line
with the best-effort net: a separate `-o -n` (push option value `-n`)
reads as a dry-run and is allowed. `git-branch-delete.sh` deletes with
`git update-ref -d`, so the confirmed flow is unaffected.
- The global-option prefix (`gopt`) now also accepts flag-only options
(`-p`, `-P`, `--no-pager`) beside `-C <path>`, `-c <cfg>` and
`--opt=value`. Every git pattern shares it, so `git --no-pager clean
-fd` and the other existing `clean`, `reset --hard`, `checkout --`,
`stash drop`/`clear` and `worktree remove` forms are now blocked after
those options too. The CHANGELOG records this.
- `destructive-guard.test.sh`: the four spellings pinned as allowed now
block; added block, allow and ack (Bash and PowerShell) cases, including
flag-only globals, `+:ref`, quoted flags and `-o` push options.
- Guard header, `SKILL.md` frontmatter and body, README, and
`allowed-tools-pairing.test.sh` state the new coverage together, with no
claim beyond what the guard provides: the ack prefix is described as the
documented way to run a bare `git branch -D`, not the only way (`git
--work-tree <path> branch -D x`, an alias, or disabling the guard also
run it).
- `repo-hygiene` 0.14.0 (above main's 0.13.0) with a CHANGELOG entry.

## Verification

- `bash
plugins/repo-hygiene/skills/clean/scripts/destructive-guard.test.sh`:
All 219 checks passed.
- `bash plugins/repo-hygiene/scripts/allowed-tools-pairing.test.sh`:
passed.
- All 17 repo-hygiene `*.test.sh` suites pass, including
`git-branch-delete.test.sh`.
- `scripts/check-changelog-parity.sh --check --check-order`: passed
after merging `main` (0.14.0 above 0.13.0).
- `scripts/validate-plugins.sh`: all manifests and the catalog
validated.
- Not run: `skill-quality:check` on `skills/clean`.

## Related

- Parent audit: #3346 (finding D1).
- Earlier PRs: #4966, #5130, #5319.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5.5 <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.

1 participant