Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion plugins/repo-hygiene/.claude-plugin/plugin.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json",
"name": "repo-hygiene",
"version": "0.7.1",
"version": "0.7.2",
"description": "Repo hygiene action-router: /repo-hygiene:clean sweeps reclaimable caches, build artifacts, and stale git metadata, and can realign the working tree to a fresh-pull state — dry-run-first, with destructive tiers gated behind explicit confirmation and a session-scoped destructive-command guard. Ecosystem targets are detected at runtime; secrets, runtime dependencies, and skill data are preserved by default.",
"author": {
"name": "Melodic Software",
Expand Down
21 changes: 21 additions & 0 deletions plugins/repo-hygiene/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,27 @@
All notable changes to the `repo-hygiene` plugin are documented here. Format follows
[Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning.

## [0.7.2]

### Fixed

- **`clean-build.sh --apply` no longer accepts `caches`-tier manifest entries
without `--include-caches` (wrong-tier bypass).** Both apply paths (resume
from a caller-supplied `--manifest`, and the normal build-then-apply flow)
passed a fixed `"build caches"` allow-list to `clean_apply_manifest`
regardless of `--include-caches`, so a plain build-tier apply still accepted
and removed `caches` lines (`.pytest_cache/`, `.ruff_cache/`, …) from a
stale or caller-supplied manifest — defeating the tier-isolation guard on
the documented `--manifest` surface. The allowed classes are now derived
from the apply invocation's own `--include-caches` flag (`build` only when
unset, `build caches` when set), so a build-only apply rejects a `caches`
line as `Rejected (wrong tier)` and leaves the cache target in place.
**Caller-visible:** the manifest-flow's tier now tracks the *apply* call, not
the dry-run that built the manifest — the documented `clean-build.sh
--include-caches` build-tier flow (`SKILL.md` §3) must repeat
`--include-caches` on the `--apply --manifest <path>` step too, or the
folded-in `caches` entries are rejected instead of removed.

## [0.7.1]

### Documentation
Expand Down
2 changes: 1 addition & 1 deletion plugins/repo-hygiene/skills/clean/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ REPO_ROOT=$(git rev-parse --show-toplevel)

#### Dry-run → confirm → apply manifest flow (caches / build)

Both selective mutating tiers pay the filesystem walk **once**. `--dry-run` writes a session-scoped manifest and prints two machine-parseable lines: `Manifest: <path>` and `Summary: planned=N bytes=K` (bytes reclaimable — surface this in the confirmation gate). After the user confirms, apply the **same** manifest with `CLEAN_GUARD_ACK=1 … --apply --manifest <path>` — apply re-stats each entry (staleness guard) and removes it without re-walking, then prints `Summary: removed=N failed=M bytes=K` and exits non-zero if `failed>0`. A killed apply **resumes** by re-running the identical `--apply --manifest <path>` (already-removed entries are idempotent no-ops). Capture `<path>` from the dry-run's `Manifest:` line and thread it through unchanged; the manifest is ephemeral (mktemp default), so pass `--manifest <path>` on the dry-run too if you need a stable location.
Both selective mutating tiers pay the filesystem walk **once**. `--dry-run` writes a session-scoped manifest and prints two machine-parseable lines: `Manifest: <path>` and `Summary: planned=N bytes=K` (bytes reclaimable — surface this in the confirmation gate). After the user confirms, apply the **same** manifest with `CLEAN_GUARD_ACK=1 … --apply --manifest <path>` — apply re-stats each entry (staleness guard) and removes it without re-walking, then prints `Summary: removed=N failed=M bytes=K` and exits non-zero if `failed>0`. A killed apply **resumes** by re-running the identical `--apply --manifest <path>` (already-removed entries are idempotent no-ops). Capture `<path>` from the dry-run's `Manifest:` line and thread it through unchanged; the manifest is ephemeral (mktemp default), so pass `--manifest <path>` on the dry-run too if you need a stable location. **Build tier: repeat `--include-caches` on the apply call too** — `clean-build.sh` gates which manifest classes an apply accepts by that invocation's own `--include-caches` flag, not by what built the manifest, so an apply that omits it rejects the manifest's `caches` lines (`Rejected (wrong tier)`, `failed>0`) even though the dry-run folded them in.

### 2. Caches

Expand Down
16 changes: 14 additions & 2 deletions plugins/repo-hygiene/skills/clean/scripts/clean-build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ Manifest flow (single walk paid once):
`Summary: planned=N bytes=K`.
--apply --manifest P consumes manifest P (re-stat guard, no re-walk); resume
= re-run the same command (already-gone entries skipped).
`caches` lines in P are accepted only when
--include-caches is also passed; otherwise rejected
(wrong tier), even if P was produced elsewhere.
--apply without --manifest builds the manifest then applies it.
--manifest P on --dry-run writes to P instead of a mktemp path.

Expand Down Expand Up @@ -80,6 +83,15 @@ fi

cd "$REPO_ROOT" || exit 1

# Allowed manifest classes for this tier's apply gate — must track INCLUDE_CACHES
# on BOTH apply paths (resume-from-manifest below and the normal walk further
# down), never a fixed "build caches": otherwise a build-only apply (no
# --include-caches) would still accept `caches` lines from a caller-supplied or
# stale manifest and remove cache targets, defeating clean_apply_manifest's
# wrong-tier guard on this documented --manifest surface.
ALLOWED_CLASSES="build"
[[ "$INCLUDE_CACHES" -eq 1 ]] && ALLOWED_CLASSES="build caches"

# No build-system clean driver (e.g. `dotnet clean`): the universal bin/obj/…
# removal below already deletes everything such a driver would, and running one
# first is pure overhead (full MSBuild evaluation, minutes on a large solution)
Expand All @@ -92,7 +104,7 @@ if [[ "$DRY_RUN" -eq 0 && -n "$MANIFEST_ARG" ]]; then
echo "clean-build.sh: manifest not readable: $MANIFEST_ARG" >&2
exit 1
fi
clean_apply_manifest "$REPO_ROOT" "$MANIFEST_ARG" "build caches"
clean_apply_manifest "$REPO_ROOT" "$MANIFEST_ARG" "$ALLOWED_CLASSES"
printf 'Summary: removed=%s failed=%s bytes=%s\n' \
"$CLEAN_REMOVED_COUNT" "$CLEAN_FAILED_COUNT" "$CLEAN_REMOVED_BYTES"
[[ "$CLEAN_FAILED_COUNT" -eq 0 ]] || exit 1
Expand All @@ -117,7 +129,7 @@ if [[ "$DRY_RUN" -eq 1 ]]; then
exit 0
fi

clean_apply_manifest "$REPO_ROOT" "$MANIFEST" "build caches"
clean_apply_manifest "$REPO_ROOT" "$MANIFEST" "$ALLOWED_CLASSES"
printf 'Summary: removed=%s failed=%s bytes=%s\n' \
"$CLEAN_REMOVED_COUNT" "$CLEAN_FAILED_COUNT" "$CLEAN_REMOVED_BYTES"
[[ "$CLEAN_FAILED_COUNT" -eq 0 ]] || exit 1
Expand Down
36 changes: 34 additions & 2 deletions plugins/repo-hygiene/skills/clean/scripts/clean-build.test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -88,19 +88,51 @@ fi
assert_not_contains "nested obj not a separate entry" "$mani_body" "bin/obj"
assert_file_exists "dry-run does not mutate" "$TEST_TMPDIR/b2/bin/obj/y.dll"

out="$(run_b2 --apply --manifest "$MANI")"
# Apply must repeat --include-caches: the ALLOWED-classes gate tracks THIS
# invocation's flag, not what built the manifest (that's the wrong-tier-bypass
# fix below) — so a manifest spanning both tiers needs --include-caches on
# apply too, same as it needed it at dry-run, to remove both tiers again.
out="$(run_b2 --apply --include-caches --manifest "$MANI")"
rc=$?
assert_contains "apply-from-manifest removes bin" "$out" "Removed: bin"
assert_contains "apply summary shape" "$out" "Summary: removed=2 failed=0 bytes="
assert_exit "apply exit 0" 0 "$rc"
assert_file_absent "bin gone after apply" "$TEST_TMPDIR/b2/bin/x.dll"
assert_file_absent "folded cache gone after apply" "$TEST_TMPDIR/b2/.pytest_cache/c"

out="$(run_b2 --apply --manifest "$MANI")"
out="$(run_b2 --apply --include-caches --manifest "$MANI")"
rc=$?
assert_contains "resume removes nothing" "$out" "Summary: removed=0 failed=0 bytes=0"
assert_exit "resume exit 0" 0 "$rc"

# --- wrong-tier bypass guard: a build-only --apply must reject `caches` lines ---
# even when a caller-supplied (or stale) manifest carries them. Regression for
# the fixed bug where clean_apply_manifest's ALLOWED-classes gate was hardcoded
# to "build caches" on both apply paths regardless of --include-caches.
git init "$TEST_TMPDIR/b3" >/dev/null 2>&1
git -C "$TEST_TMPDIR/b3" config user.email "t@example.com"
git -C "$TEST_TMPDIR/b3" config user.name "Test"
mkdir -p "$TEST_TMPDIR/b3/.pytest_cache"
echo c >"$TEST_TMPDIR/b3/.pytest_cache/c"
MANI3="$TEST_TMPDIR/b3.manifest"
printf 'caches\t1\t.pytest_cache\n' >"$MANI3"

run_b3() {
bash -c "cd '$TEST_TMPDIR/b3' && bash '$BUILD' $*"
}

out="$(run_b3 --apply --manifest "$MANI3" 2>&1)"
rc=$?
assert_contains "build-only apply rejects caches-tier manifest line" "$out" "Rejected (wrong tier): .pytest_cache"
assert_contains "wrong-tier rejection counted as failed" "$out" "Summary: removed=0 failed=1 bytes=0"
assert_exit "wrong-tier rejection is a failing apply" 1 "$rc"
assert_file_exists "cache target left in place" "$TEST_TMPDIR/b3/.pytest_cache/c"

# The same manifest line IS accepted once --include-caches opts into the tier.
out="$(run_b3 --apply --include-caches --manifest "$MANI3")"
assert_contains "caches line accepted with --include-caches" "$out" "Removed: .pytest_cache"
assert_file_absent "cache target removed with --include-caches" "$TEST_TMPDIR/b3/.pytest_cache/c"

if [[ $FAILED -ne 0 ]]; then
echo "FAILED: $FAILED test(s)"
exit 1
Expand Down