diff --git a/plugins/code-tidying/.claude-plugin/plugin.json b/plugins/code-tidying/.claude-plugin/plugin.json index 66024e06e..6a4f59300 100644 --- a/plugins/code-tidying/.claude-plugin/plugin.json +++ b/plugins/code-tidying/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json", "name": "code-tidying", - "version": "0.13.2", + "version": "0.13.3", "description": "Code tidying and comment hygiene: /code-tidying:tidy proactively hunts a rotated, glob-scoped lane for Beck-style tidyings under a research-backed scope budget and ships one tight PR; /code-tidying:batch-simplify sweeps a time window, a branch, or an entire repository through grouped, dependency-ordered simplification waves with a never-drop deferred-items contract; /code-tidying:dissolve-comments enforces self-describing expressive code over a diff — deletes zero-information comments, dissolves code-expressible ones into names and structure behind a tests gate (safe mode restricts applied edits to removals), and keeps only terse load-bearing comments code cannot express; /code-tidying:audit-comment-residue is a read-only classifier that flags history, plan, conversational, and ticket/PR residue in code comments for author-applied deletion. Project-specific tidy lanes are scaffolded into a tracked .claude/tidy-lanes/ config folder by a re-runnable setup skill.", "author": { "name": "Melodic Software", diff --git a/plugins/code-tidying/CHANGELOG.md b/plugins/code-tidying/CHANGELOG.md index 7c319e4e1..6aec0d4f8 100644 --- a/plugins/code-tidying/CHANGELOG.md +++ b/plugins/code-tidying/CHANGELOG.md @@ -3,6 +3,31 @@ All notable changes to the `code-tidying` plugin are documented here. Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning. +## [0.13.3] + +### Fixed + +- **`audit-comment-residue` no longer silently skips paths containing spaces + (#3126).** The default-target router parsed `git status --porcelain` with + `awk '{print $NF}'`, which split a spaced path on its space and kept git's + closing quote, so the file resolved to nothing and dropped out of the run. + The audit then reported `files=0` plus `no code targets` — a false negative + that reads as a clean tree, ending the investigation rather than prompting a + retry. `detect.sh` now slices the path out of the porcelain record, takes the + right-hand side of a rename (gated on the `R`/`C` status letter in **either** + the index or the worktree column, so an ordinary path containing `" -> "` is + left intact while an intent-to-add rename — `mv old new && git add -N new`, + which records `R` in the worktree column — still resolves), and unwraps git's + quoting. + The skill's own `Uncommitted code files` pre-computed context carried the + identical `$NF` parse and is fixed to full parity — same column handling and + the same `\"`/`\\` unescaping — rather than only to the quote-stripping half. + The test suite now **extracts** that parser out of `SKILL.md` and executes it + against the same fixtures, so the two cannot silently diverge again. Git's + octal escapes for control and non-ASCII bytes are still not decoded by either, + so such a path continues to miss; the limitation is recorded at the parse site + instead of being silent. + ## [0.13.2] ### Changed diff --git a/plugins/code-tidying/skills/audit-comment-residue/SKILL.md b/plugins/code-tidying/skills/audit-comment-residue/SKILL.md index f735e3786..ff5f0399d 100644 --- a/plugins/code-tidying/skills/audit-comment-residue/SKILL.md +++ b/plugins/code-tidying/skills/audit-comment-residue/SKILL.md @@ -13,7 +13,7 @@ metadata: ## Pre-computed context Current branch: !`git branch --show-current 2>/dev/null || echo "unknown"` -Uncommitted code files: !`git status --porcelain 2>/dev/null | awk '{print $NF}' | grep -Ei '\.(cs|ts|tsx|js|jsx|py|sh|ps1|go|rs|java|rb|lua|sql|c|h|cpp|hpp|yaml|yml|toml)$' | head -10 || echo "none"` +Uncommitted code files: !`git status --porcelain 2>/dev/null | awk '{ p = substr($0, 4); if (substr($0, 1, 2) ~ /[RC]/) sub(/^.* -> /, "", p); if (p ~ /^".*"$/) { p = substr(p, 2, length(p) - 2); gsub(/\\"/, "\"", p); gsub(/\\\\/, "\\", p) } print p }' | grep -Ei '\.(cs|ts|tsx|js|jsx|py|sh|ps1|go|rs|java|rb|lua|sql|c|h|cpp|hpp|yaml|yml|toml)$' | head -10 || echo "none"` Residue findings (sample): !`${CLAUDE_SKILL_DIR}/scripts/detect.sh 2>/dev/null | grep -E '^(Summary total:|Finding shape:)' | head -20 || echo "none"` ## Purpose diff --git a/plugins/code-tidying/skills/audit-comment-residue/scripts/detect.sh b/plugins/code-tidying/skills/audit-comment-residue/scripts/detect.sh index 7e303cb76..0de12dc18 100755 --- a/plugins/code-tidying/skills/audit-comment-residue/scripts/detect.sh +++ b/plugins/code-tidying/skills/audit-comment-residue/scripts/detect.sh @@ -97,12 +97,34 @@ if [[ ${#TARGETS[@]} -eq 0 ]]; then TARGETS+=("$(cr_anchor_path "$line")") done <"$PATHS_FILE" elif [[ -n "$repo_root" ]]; then - # Uncommitted files: modified/added/renamed/untracked, per git status. + # Uncommitted files: modified/added/renamed/untracked, per git status. Parse porcelain + # by slicing, not by splitting on whitespace, so paths containing spaces survive (#3126); + # $NF dropped everything before the last space and kept git's closing quote, so a spaced + # path resolved to a nonexistent file and vanished from the audit with no signal. while IFS= read -r line; do line="${line//$'\r'/}" [[ -z "$line" ]] && continue - TARGETS+=("$line") - done < <(git status --porcelain 2>/dev/null | awk '{print $NF}') + # XY + space + path. + status_path="${line:3}" + # Rename/copy entries read "old -> new" — audit the new path. Gated on the status + # letters so an ordinary path that happens to contain " -> " is left intact. BOTH + # columns matter: X is the index status and Y the worktree status, and a rename + # staged only as intent-to-add lands in Y (`mv old new && git add -N new` emits + # " R old -> new"). Checking X alone left that record unsplit and unresolvable. + if [[ "${line:0:1}" == [RC] || "${line:1:1}" == [RC] ]]; then + status_path="${status_path##* -> }" + fi + # Paths with spaces or other special characters arrive C-quoted: "my helper.sh". + # Unwrap and unescape. Backslash-escaped quotes and backslashes are handled; git's + # octal escapes for control and non-ASCII bytes are not, so such a path still misses. + if [[ "$status_path" == \"*\" ]]; then + status_path="${status_path#\"}" + status_path="${status_path%\"}" + status_path="${status_path//\\\"/\"}" + status_path="${status_path//\\\\/\\}" + fi + TARGETS+=("$status_path") + done < <(git status --porcelain 2>/dev/null) fi fi diff --git a/plugins/code-tidying/skills/audit-comment-residue/scripts/detect.test.sh b/plugins/code-tidying/skills/audit-comment-residue/scripts/detect.test.sh index f311c6d1e..7b8d12aaa 100755 --- a/plugins/code-tidying/skills/audit-comment-residue/scripts/detect.test.sh +++ b/plugins/code-tidying/skills/audit-comment-residue/scripts/detect.test.sh @@ -177,6 +177,134 @@ printf '%s\n' "rel.py" >"$SUBDIR/rel-paths.txt" relpf_out="$(cd "$SUBDIR" && bash "$DETECT" --paths-file rel-paths.txt)" assert_contains "relative --paths-file target audited from subdir cwd" "$relpf_out" "Finding shape: history-narration" +# --- 9. Default-target discovery parses porcelain, not whitespace fields (#3126) -------- +# With no arguments the audit discovers targets from `git status --porcelain`. A path +# containing a space arrives C-quoted ("my helper.sh"); splitting on whitespace kept the +# closing quote and dropped everything before the space, so the file resolved to nothing and +# vanished from the run — reported as a reassuring files=0 rather than as an error. A plain +# path passes either implementation, so the fixture name must contain a space. + +# The two arms live in separate repos on purpose: sharing one would let a correctly-parsed +# file keep the run's files= count above zero and mask the other arm's disappearance. + +# 9a. Spaced path is the whole tree — the broken parse reports the misleading files=0. +REPO9="$TEST_TMPDIR/repo9" +mkdir -p "$REPO9" +git -C "$REPO9" init -q +cp "$ALL_SHAPES" "$REPO9/my helper.py" + +spaced_out="$(cd "$REPO9" && bash "$DETECT")" +assert_not_contains "spaced default target is not reported as files=0" "$spaced_out" "files=0" +assert_not_contains "spaced default target does not claim a clean tree" "$spaced_out" "no code targets" +assert_contains "spaced default target audited" "$spaced_out" "Summary file: my helper.py" +assert_contains "spaced default target finds shapes" "$spaced_out" "Finding shape: history-narration" + +# 9b. Rename arm: porcelain emits "old -> new"; the new path is the one to audit. +REPO10="$TEST_TMPDIR/repo10" +mkdir -p "$REPO10" +git -C "$REPO10" init -q +cp "$ALL_SHAPES" "$REPO10/original.py" +git -C "$REPO10" add original.py +git -C "$REPO10" -c user.email=t@example.com -c user.name=t commit -qm init +git -C "$REPO10" mv original.py renamed.py + +rename_out="$(cd "$REPO10" && bash "$DETECT")" +assert_contains "renamed default target resolves to the new path" "$rename_out" "Summary file: renamed.py" +assert_not_contains "renamed default target does not audit the old path" "$rename_out" "Summary file: original.py" + +# 9c. A spaced path that is ALSO renamed exercises quote-stripping and the " -> " split +# together — the combination the two arms above each cover only half of. +REPO11="$TEST_TMPDIR/repo11" +mkdir -p "$REPO11" +git -C "$REPO11" init -q +cp "$ALL_SHAPES" "$REPO11/old name.py" +git -C "$REPO11" add "old name.py" +git -C "$REPO11" -c user.email=t@example.com -c user.name=t commit -qm init +git -C "$REPO11" mv "old name.py" "new name.py" + +spaced_rename_out="$(cd "$REPO11" && bash "$DETECT")" +assert_not_contains "spaced rename is not reported as files=0" "$spaced_rename_out" "files=0" +assert_contains "spaced rename resolves to the new path" "$spaced_rename_out" "Summary file: new name.py" + +# 9d. Porcelain is XY: X is the index status, Y the worktree status, and a rename can be +# recorded in EITHER. An intent-to-add rename (`mv old new && git add -N new`) emits +# " R old -> new" — the R is in Y, with X blank. Gating the arrow-split on X alone left +# the record unsplit, so the whole "old -> new" string became the path and resolved to +# nothing. Regression guard for that half of the rename case. +REPO12="$TEST_TMPDIR/repo12" +mkdir -p "$REPO12" +git -C "$REPO12" init -q +cp "$ALL_SHAPES" "$REPO12/old.py" +git -C "$REPO12" add old.py +git -C "$REPO12" -c user.email=t@example.com -c user.name=t commit -qm init +mv "$REPO12/old.py" "$REPO12/new.py" +git -C "$REPO12" add -N new.py + +worktree_rename_out="$(cd "$REPO12" && bash "$DETECT")" +assert_not_contains "worktree-column rename is not reported as files=0" "$worktree_rename_out" "files=0" +assert_contains "worktree-column rename resolves to the new path" "$worktree_rename_out" "Summary file: new.py" + +# --- 10. SKILL.md pre-computed-context parser stays at parity with detect.sh (#3126) ---- +# SKILL.md's `Uncommitted code files:` line re-implements the porcelain parse to preview +# targets to the model. A divergence there is a false negative on the same surface, so the +# program is EXTRACTED from SKILL.md and executed rather than being restated here — a copy +# would pass while the real line rotted. Fixture names force C-quoting through an embedded +# quote and backslash, not just a space, since quote-stripping alone passes a spaced name. + +SKILL_MD="$SCRIPT_DIR/../SKILL.md" +if [[ ! -f "$SKILL_MD" ]]; then + fail "SKILL.md located for parity check" "file at $SKILL_MD" "missing" +else + # Pull the awk program out of: ... | awk '' | grep ... + skill_awk="$(sed -n "s/^Uncommitted code files:.*| awk '\(.*\)' | grep .*$/\1/p" "$SKILL_MD")" + if [[ -z "$skill_awk" ]]; then + fail "SKILL.md awk program extracted" "non-empty program" "no match — line shape changed" + else + pass "SKILL.md awk program extracted" + + REPO13="$TEST_TMPDIR/repo13" + mkdir -p "$REPO13" + git -C "$REPO13" init -q + : >"$REPO13/quote\".py" + : >"$REPO13/back\\-slash.py" + : >"$REPO13/plain space.py" + cp "$ALL_SHAPES" "$REPO13/renamed-src.py" + git -C "$REPO13" add renamed-src.py + git -C "$REPO13" -c user.email=t@example.com -c user.name=t commit -qm init + mv "$REPO13/renamed-src.py" "$REPO13/renamed-dst.py" + git -C "$REPO13" add -N renamed-dst.py + + skill_out="$(cd "$REPO13" && git status --porcelain | awk "$skill_awk")" + + assert_contains "SKILL.md parser unescapes an embedded quote" "$skill_out" 'quote".py' + assert_contains "SKILL.md parser unescapes an embedded backslash" "$skill_out" 'back\-slash.py' + assert_contains "SKILL.md parser unwraps a spaced path" "$skill_out" 'plain space.py' + assert_contains "SKILL.md parser takes the worktree-rename new path" "$skill_out" 'renamed-dst.py' + assert_not_contains "SKILL.md parser leaves no rename arrow" "$skill_out" ' -> ' + assert_not_contains "SKILL.md parser leaves no escaped quote" "$skill_out" '\"' + + # Parity with detect.sh over the same tree: every code file detect.sh audits must also + # appear in the preview, or the model is shown a tree the audit does not agree with. + detect_out="$(cd "$REPO13" && bash "$DETECT")" + parity_ok=1 + while IFS= read -r audited; do + [[ -z "$audited" ]] && continue + case "$skill_out" in + *"$audited"*) ;; + *) + parity_ok=0 + printf ' detect.sh audited but preview missed: %s\n' "$audited" >&2 + ;; + esac + done < <(printf '%s\n' "$detect_out" | sed -n 's/^Summary file: \(.*\) | T1=.*$/\1/p') + if [[ "$parity_ok" -eq 1 ]]; then + pass "SKILL.md preview covers every file detect.sh audits" + else + fail "SKILL.md preview covers every file detect.sh audits" "full coverage" "see above" + fi + fi +fi + # --- Final report -------------------------------------------------------------------- if [[ "$FAILED" -eq 0 ]]; then