diff --git a/plugins/repo-hygiene/.claude-plugin/plugin.json b/plugins/repo-hygiene/.claude-plugin/plugin.json index 688b710bc2..f4e117faa3 100644 --- a/plugins/repo-hygiene/.claude-plugin/plugin.json +++ b/plugins/repo-hygiene/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json", "name": "repo-hygiene", - "version": "0.10.19", + "version": "0.10.20", "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", diff --git a/plugins/repo-hygiene/CHANGELOG.md b/plugins/repo-hygiene/CHANGELOG.md index 27ba2cdbf6..d72ef61895 100644 --- a/plugins/repo-hygiene/CHANGELOG.md +++ b/plugins/repo-hygiene/CHANGELOG.md @@ -3,6 +3,23 @@ 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.10.20] + +### Fixed + +- **`clean`: `batch_read_lines_into` no longer treats a trailing blank line as + "file not found".** The helper's exit status is now a source-open verdict: 0 + once the selected source was opened and consumed to ordinary EOF (empty + input, blank lines, a trailing blank line, and a final non-newline-terminated + line are all success), and 1 only when a named source is missing, not a + regular file, or cannot be opened. An explicit `return 0` after EOF stops the + last `[[ -n "$line" ]]` from becoming the function result. Both + `clean-batch.sh` and `git-tree-reset-batch.sh` trust that contract, so a + `--repos-from` / `--skip-from` file ending `a\n\n` is accepted and a missing + file still errors as file-not-found. Named sources open on fixed fd 3 so a + `ulimit -n 10` runner can still read the list + ([#3482](https://github.com/melodic-software/claude-code-plugins/issues/3482)). + ## [0.10.19] ### Changed diff --git a/plugins/repo-hygiene/skills/clean/scripts/clean-batch.test.sh b/plugins/repo-hygiene/skills/clean/scripts/clean-batch.test.sh index aaf7e42777..08adf08a81 100755 --- a/plugins/repo-hygiene/skills/clean/scripts/clean-batch.test.sh +++ b/plugins/repo-hygiene/skills/clean/scripts/clean-batch.test.sh @@ -339,5 +339,67 @@ else fail "all tier plan REPO+GITDIR" "both" "$(cat "$APLAN")" fi +# --- 9. --repos-from / --skip-from source-open contract (#3482) --- +# A trailing blank is ordinary EOF success; only a missing/unopenable named +# source is "file not found". Empty input is success at the helper and surfaces +# as "no repos given", never as a missing file. +RF="$(mkrepo rf)" +printf '%s\n\n' "$RF" >"$TEST_TMPDIR/repos-trail.txt" +rc=0 +out="$(bash "$BATCH" --tier caches --repos-from "$TEST_TMPDIR/repos-trail.txt" 2>&1)" || rc=$? +assert_exit "trailing-blank --repos-from is accepted (exit 0)" 0 "$rc" +assert_not_contains "trailing blank is not reported as file-not-found" "$out" "file not found" +assert_contains "trailing-blank list enumerates the repo" "$out" "Repos: 1" + +: >"$TEST_TMPDIR/repos-empty.txt" +rc=0 +out="$(bash "$BATCH" --tier caches --repos-from "$TEST_TMPDIR/repos-empty.txt" 2>&1)" || rc=$? +assert_exit "empty --repos-from is a usage error (exit 2)" 2 "$rc" +assert_not_contains "empty file is not reported missing" "$out" "file not found" +assert_contains "empty file reaches no-repos usage" "$out" "no repos given" + +printf '%s' "$RF" >"$TEST_TMPDIR/repos-noeol.txt" +rc=0 +out="$(bash "$BATCH" --tier caches --repos-from "$TEST_TMPDIR/repos-noeol.txt" 2>&1)" || rc=$? +assert_exit "unterminated --repos-from is accepted (exit 0)" 0 "$rc" +assert_contains "unterminated list enumerates the repo" "$out" "Repos: 1" + +printf '%s\r\n' "$RF" >"$TEST_TMPDIR/repos-cr.txt" +rc=0 +out="$(bash "$BATCH" --tier caches --repos-from "$TEST_TMPDIR/repos-cr.txt" 2>&1)" || rc=$? +assert_exit "CR-terminated --repos-from is accepted (exit 0)" 0 "$rc" +assert_contains "CR-stripped list enumerates the repo" "$out" "Repos: 1" + +rc=0 +out="$(bash "$BATCH" --tier caches --repos-from "$TEST_TMPDIR/no-such-repos.txt" 2>&1)" || rc=$? +assert_exit "missing --repos-from exits 2" 2 "$rc" +assert_contains "missing file reported not found" "$out" "file not found:" + +SKIP_TRAIL="$TEST_TMPDIR/skips-trail.txt" +printf 'rf\n\n' >"$SKIP_TRAIL" +rc=0 +out="$(bash "$BATCH" --tier caches --repo "$RF" --skip-from "$SKIP_TRAIL" 2>&1)" || rc=$? +assert_exit "trailing-blank --skip-from is accepted (exit 0)" 0 "$rc" +assert_not_contains "trailing-blank skip-from is not file-not-found" "$out" "file not found" +assert_contains "trailing-blank skip-from still skips" "$out" "skip-list" + +rc=0 +out="$(bash "$BATCH" --tier caches --repo "$RF" --skip-from "$TEST_TMPDIR/no-such-skips.txt" 2>&1)" || rc=$? +assert_exit "missing --skip-from exits 2" 2 "$rc" +assert_contains "missing skip-from reported not found" "$out" "file not found:" + +UNREAD_LIST="$TEST_TMPDIR/unreadable-repos.txt" +printf '%s\n' "$RF" >"$UNREAD_LIST" +chmod 000 "$UNREAD_LIST" 2>/dev/null || true +if [[ -r "$UNREAD_LIST" ]]; then + skip_case "unopenable --repos-from: chmod 000 not enforced on this filesystem (CAP_DAC_OVERRIDE)" +else + rc=0 + out="$(bash "$BATCH" --tier caches --repos-from "$UNREAD_LIST" 2>&1)" || rc=$? + assert_exit "unopenable --repos-from exits 2" 2 "$rc" + assert_contains "unopenable file reported not found" "$out" "file not found:" +fi +chmod 644 "$UNREAD_LIST" 2>/dev/null || true + [[ $FAILED -eq 0 ]] || exit 1 echo "clean-batch.test.sh: all passed" diff --git a/plugins/repo-hygiene/skills/clean/scripts/git-tree-reset-batch.sh b/plugins/repo-hygiene/skills/clean/scripts/git-tree-reset-batch.sh index 590d54cd0f..20d7363024 100755 --- a/plugins/repo-hygiene/skills/clean/scripts/git-tree-reset-batch.sh +++ b/plugins/repo-hygiene/skills/clean/scripts/git-tree-reset-batch.sh @@ -140,8 +140,7 @@ while [[ $# -gt 0 ]]; do ;; --repos-from) [[ $# -ge 2 ]] || fail_usage "--repos-from requires a file or -" - [[ "$2" == "-" || -f "$2" ]] || fail_usage "file not found: $2" - batch_read_lines_into REPO_INPUTS "$2" + batch_read_lines_into REPO_INPUTS "$2" || fail_usage "file not found: $2" shift ;; --skip) @@ -151,8 +150,7 @@ while [[ $# -gt 0 ]]; do ;; --skip-from) [[ $# -ge 2 ]] || fail_usage "--skip-from requires a file" - [[ -f "$2" ]] || fail_usage "file not found: $2" - batch_read_lines_into SKIP_INPUTS "$2" + batch_read_lines_into SKIP_INPUTS "$2" || fail_usage "file not found: $2" shift ;; -h | --help) diff --git a/plugins/repo-hygiene/skills/clean/scripts/git-tree-reset-batch.test.sh b/plugins/repo-hygiene/skills/clean/scripts/git-tree-reset-batch.test.sh index f680c01f27..c21880f0c5 100755 --- a/plugins/repo-hygiene/skills/clean/scripts/git-tree-reset-batch.test.sh +++ b/plugins/repo-hygiene/skills/clean/scripts/git-tree-reset-batch.test.sh @@ -325,6 +325,63 @@ out="$(bash "$BATCH" --dry-run --repos-from "$NOEOL_REPOS" 2>&1)" || true assert_contains "unterminated repos-from still enumerates the repo" "$out" "Repos: 1" assert_contains "unterminated repos-from repo would reset" "$out" "would-reset" +# --- 22b. source-open contract (#3482): trailing blank / empty / CR / missing --- +# A file ending `path\n\n` is ordinary EOF success. The sibling clean-batch used +# to map that helper rc 1 onto "file not found"; tree-batch must keep accepting +# it after dropping its duplicate `-f` precheck, and a truly missing file must +# still error as file-not-found. +TRAIL_REPOS="$TEST_TMPDIR/repos-trail.txt" +printf '%s\n\n' "$CLEAN_REPO" >"$TRAIL_REPOS" +rc=0 +out="$(bash "$BATCH" --dry-run --repos-from "$TRAIL_REPOS" 2>&1)" || rc=$? +assert_exit "trailing-blank --repos-from is accepted (exit 0)" 0 "$rc" +assert_not_contains "trailing blank is not reported as file-not-found" "$out" "file not found" +assert_contains "trailing-blank list enumerates the repo" "$out" "Repos: 1" + +: >"$TEST_TMPDIR/repos-empty.txt" +rc=0 +out="$(bash "$BATCH" --dry-run --repos-from "$TEST_TMPDIR/repos-empty.txt" 2>&1)" || rc=$? +assert_exit "empty --repos-from is a usage error (exit 2)" 2 "$rc" +assert_not_contains "empty file is not reported missing" "$out" "file not found" +assert_contains "empty file reaches no-repos usage" "$out" "no repos given" + +printf '%s\r\n' "$CLEAN_REPO" >"$TEST_TMPDIR/repos-cr.txt" +rc=0 +out="$(bash "$BATCH" --dry-run --repos-from "$TEST_TMPDIR/repos-cr.txt" 2>&1)" || rc=$? +assert_exit "CR-terminated --repos-from is accepted (exit 0)" 0 "$rc" +assert_contains "CR-stripped list enumerates the repo" "$out" "Repos: 1" + +rc=0 +out="$(bash "$BATCH" --dry-run --repos-from "$TEST_TMPDIR/no-such-repos.txt" 2>&1)" || rc=$? +assert_exit "missing --repos-from exits 2" 2 "$rc" +assert_contains "missing file reported not found" "$out" "file not found:" + +TRAIL_SKIP="$TEST_TMPDIR/skips-trail.txt" +printf 'keepme\n\n' >"$TRAIL_SKIP" +rc=0 +out="$(bash "$BATCH" --dry-run --repo "$CLEAN_REPO" --repo "$SKIP_REPO" --skip-from "$TRAIL_SKIP" 2>&1)" || rc=$? +assert_exit "trailing-blank --skip-from is accepted (exit 0)" 0 "$rc" +assert_not_contains "trailing-blank skip-from is not file-not-found" "$out" "file not found" +assert_contains "trailing-blank skip-from still skips" "$out" "skip-list" + +rc=0 +out="$(bash "$BATCH" --dry-run --repo "$CLEAN_REPO" --skip-from "$TEST_TMPDIR/no-such-skips.txt" 2>&1)" || rc=$? +assert_exit "missing --skip-from exits 2" 2 "$rc" +assert_contains "missing skip-from reported not found" "$out" "file not found:" + +UNREAD_LIST="$TEST_TMPDIR/unreadable-repos.txt" +printf '%s\n' "$CLEAN_REPO" >"$UNREAD_LIST" +chmod 000 "$UNREAD_LIST" 2>/dev/null || true +if [[ -r "$UNREAD_LIST" ]]; then + skip_case "unopenable --repos-from: chmod 000 not enforced on this filesystem (CAP_DAC_OVERRIDE)" +else + rc=0 + out="$(bash "$BATCH" --dry-run --repos-from "$UNREAD_LIST" 2>&1)" || rc=$? + assert_exit "unopenable --repos-from exits 2" 2 "$rc" + assert_contains "unopenable file reported not found" "$out" "file not found:" +fi +chmod 644 "$UNREAD_LIST" 2>/dev/null || true + # --- 23. a single --repo consumes consecutive paths (shell-glob-after-expansion) --- # `--repo ~/repos/*` reaches the script as ONE --repo flag followed by N positional # paths (the shell expanded the glob before exec). A single --repo must ingest every diff --git a/plugins/repo-hygiene/skills/clean/scripts/lib/batch-common.sh b/plugins/repo-hygiene/skills/clean/scripts/lib/batch-common.sh index aec700f2b8..4276d32249 100644 --- a/plugins/repo-hygiene/skills/clean/scripts/lib/batch-common.sh +++ b/plugins/repo-hygiene/skills/clean/scripts/lib/batch-common.sh @@ -28,6 +28,15 @@ batch_normalize_input() { # batch_read_lines_into — append non-empty CR-stripped # lines. Raw (unnormalized): repo paths are normalized at resolve time, skip # entries are normalized by clean_skip_matches, so neither is pre-mangled here. +# +# Exit status is a source-open verdict, not a record-read verdict. Bash `read` +# returns nonzero at ordinary EOF (help read: "unless end-of-file is +# encountered"); that is not evidence the containing file operation failed. +# 0 the selected source was opened/accepted and consumed to ordinary EOF. +# Empty input, blank lines, a trailing blank line, and a final +# non-newline-terminated line are all success. +# 1 a named source is missing, not a regular file, or cannot be opened/read. +# No input-content shape returns 1. For `-`, ordinary stdin EOF is success. batch_read_lines_into() { local -n _dest="$1" local src="$2" line @@ -36,13 +45,24 @@ batch_read_lines_into() { line="${line%$'\r'}" [[ -n "$line" ]] && _dest+=("$line") done - else - [[ -f "$src" ]] || return 1 - while IFS= read -r line || [[ -n "$line" ]]; do - line="${line%$'\r'}" - [[ -n "$line" ]] && _dest+=("$line") - done <"$src" + return 0 fi + # Named source: reject missing/non-regular before open so a directory never + # blocks on `read`. Then open explicitly so a permission/open failure is + # retained (a `while ... done <"$src"` that never enters the body would + # otherwise look like success). + [[ -f "$src" ]] || return 1 + # Fixed fd 3: Bash `{fd}` only allocates from 10 upward, so `ulimit -n 10` + # makes that open fail even when the list file is readable. + exec 3<"$src" || return 1 + while IFS= read -r -u 3 line || [[ -n "$line" ]]; do + line="${line%$'\r'}" + [[ -n "$line" ]] && _dest+=("$line") + done + exec 3<&- + # Explicit success: do not let the last `[[ -n "$line" ]]` (false on a + # trailing blank, or on the EOF guard after a delimiter) become the result. + return 0 } # Resolve-and-dedup accumulators, populated by batch_resolve_repos. Reset per call. diff --git a/plugins/repo-hygiene/skills/clean/scripts/lib/batch-common.test.sh b/plugins/repo-hygiene/skills/clean/scripts/lib/batch-common.test.sh index a5992744d2..5a1433227e 100755 --- a/plugins/repo-hygiene/skills/clean/scripts/lib/batch-common.test.sh +++ b/plugins/repo-hygiene/skills/clean/scripts/lib/batch-common.test.sh @@ -98,15 +98,92 @@ else fail "distinct common dir" 2 "${#BATCH_GITDIR_KEYS[@]}" fi -# --- 5. batch_read_lines_into: CR-stripped, non-empty --- +# --- 5. batch_read_lines_into: CR-stripped, non-empty, rc 0 --- printf 'a\r\n\nb\n' >"$TEST_TMPDIR/lines.txt" LINES=() -batch_read_lines_into LINES "$TEST_TMPDIR/lines.txt" +rc=0 +batch_read_lines_into LINES "$TEST_TMPDIR/lines.txt" || rc=$? +assert_exit "CR-stripped file is success (rc 0)" 0 "$rc" if [[ "${#LINES[@]}" -eq 2 && "${LINES[0]}" == a && "${LINES[1]}" == b ]]; then pass "read_lines_into strips CR and empties" else fail "read_lines_into strips CR and empties" "a,b" "${LINES[*]}" fi +# --- 5b. empty file, trailing blank, missing final newline: all rc 0 --- +: >"$TEST_TMPDIR/empty.txt" +LINES=() +rc=0 +batch_read_lines_into LINES "$TEST_TMPDIR/empty.txt" || rc=$? +assert_exit "empty file is success (rc 0)" 0 "$rc" +if [[ "${#LINES[@]}" -eq 0 ]]; then + pass "empty file appends nothing" +else + fail "empty file appends nothing" 0 "${#LINES[@]}" +fi + +printf 'a\n\n' >"$TEST_TMPDIR/trail.txt" +LINES=() +rc=0 +batch_read_lines_into LINES "$TEST_TMPDIR/trail.txt" || rc=$? +assert_exit "trailing blank line is success (rc 0)" 0 "$rc" +if [[ "${#LINES[@]}" -eq 1 && "${LINES[0]}" == a ]]; then + pass "trailing blank keeps the preceding entry" +else + fail "trailing blank keeps the preceding entry" "a" "${LINES[*]}" +fi + +printf 'a' >"$TEST_TMPDIR/noeol.txt" +LINES=() +rc=0 +batch_read_lines_into LINES "$TEST_TMPDIR/noeol.txt" || rc=$? +assert_exit "unterminated final line is success (rc 0)" 0 "$rc" +if [[ "${#LINES[@]}" -eq 1 && "${LINES[0]}" == a ]]; then + pass "unterminated final line is kept" +else + fail "unterminated final line is kept" "a" "${LINES[*]}" +fi + +# Stdin (`-`): ordinary EOF is success, including a trailing blank. +LINES=() +rc=0 +batch_read_lines_into LINES - < <(printf 'a\n\n') || rc=$? +assert_exit "stdin trailing blank is success (rc 0)" 0 "$rc" +if [[ "${#LINES[@]}" -eq 1 && "${LINES[0]}" == a ]]; then + pass "stdin trailing blank keeps the preceding entry" +else + fail "stdin trailing blank keeps the preceding entry" "a" "${LINES[*]}" +fi + +# --- 5c. missing / non-regular / unopenable named sources return 1 --- +LINES=() +rc=0 +batch_read_lines_into LINES "$TEST_TMPDIR/no-such-list.txt" || rc=$? +assert_exit "missing named source returns 1" 1 "$rc" +if [[ "${#LINES[@]}" -eq 0 ]]; then + pass "missing source appends nothing" +else + fail "missing source appends nothing" 0 "${#LINES[@]}" +fi + +mkdir -p "$TEST_TMPDIR/not-a-file" +LINES=() +rc=0 +batch_read_lines_into LINES "$TEST_TMPDIR/not-a-file" || rc=$? +assert_exit "directory (non-regular) returns 1" 1 "$rc" + +UNREAD="$TEST_TMPDIR/unreadable.txt" +printf 'a\n' >"$UNREAD" +chmod 000 "$UNREAD" 2>/dev/null || true +if [[ -r "$UNREAD" ]]; then + skip_case "unopenable regular file: chmod 000 not enforced on this filesystem (CAP_DAC_OVERRIDE); missing/non-regular cases already cover rc 1" +else + LINES=() + rc=0 + batch_read_lines_into LINES "$UNREAD" || rc=$? + assert_exit "unopenable regular file returns 1" 1 "$rc" +fi +chmod 644 "$UNREAD" 2>/dev/null || true + [[ $FAILED -eq 0 ]] || exit 1 echo "batch-common.test.sh: all passed"