feat(repo-hygiene): stash audit, worktree/no-upstream branch classes, single-walk scan, resolver notes - #1066
Conversation
…mary Rework the clean skill's build/caches enumeration and apply flow as one change. #993 — the selective caches/build tiers ran ~10 unpruned full-tree `find` walks per repo; the `! -path` exclusions filtered output but did not `-prune`, so every walk re-descended `.git/`, `node_modules/`, and `.venv/`. A single pruned walk per tier (`clean_enumerate`) now prunes those three trees once and `-print`s all dir-name and file-glob matches, then classifies on the result list. Measured on a large .NET + node repo (Windows/NTFS): one pruned walk incl. `du` sizing ~17s vs a 10-walk unpruned dry-run that exceeded 10 min (killed). #995 — the dry-run now writes a session-scoped manifest (`<class>\t<bytes>\t<relpath>` per eligible target), prints `Manifest: <path>` and `Summary: planned=N bytes=K` so the gate can state reclaimable space, and `--apply --manifest <path>` consumes it with a re-stat + re-classify staleness guard instead of re-walking. Resume = re-run the same command (already-gone entries are idempotent). `--apply` without a manifest builds one then applies it, preserving the standalone CLI contract; `--include-caches` folds the caches tier into the one build manifest (no subprocess). #1002 — each `--apply` ends with `Summary: removed=N failed=M bytes=K` and exits non-zero when failed>0, so a fleet sweep no longer needs per-log grepping. Cross-tier nested-target dedup drops any eligible path under an eligible ancestor so byte totals never double-count. scan.sh keeps the old walk pattern for now, tracked as TODO(#1011). Closes #993 Closes #995 Closes #1002 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016D1oCPX8LaUKnLUi3TXmih
…ding The manifest is a consumed contract (#994 parses it), so assert the exact class<TAB>bytes<TAB>relpath shape per tier rather than substrings, and cover --include-caches folding both a build and a caches entry into one manifest. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016D1oCPX8LaUKnLUi3TXmih
`clean-build.sh --apply` ran `dotnet clean <solution>` before removing bin/obj wholesale, but the universal artifact removal already deletes everything the driver would — running it first was pure overhead: a full MSBuild evaluation (minutes on a large solution) that also re-created obj/ evaluation artifacts. One walk + rm is strictly faster and equally complete. Removes the driver and its `Planned: dotnet clean …` (dry-run) / `DRIVER_FAILED:` (apply) output markers, and the now-stale driver references in the config/ecosystem/README docs. Closes #999 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016D1oCPX8LaUKnLUi3TXmih
The repo's comment-hygiene gate forbids tracker/issue references and TODO markers in code comments; version control, the CHANGELOG, and the tracker own that history. Strips the `(#NNN)` refs and the scan.sh `TODO(#…)` note from the clean scripts and their comments — no behavior change. The scan.sh migration deferral remains tracked by its own issue and this PR. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016D1oCPX8LaUKnLUi3TXmih
…edup The `--apply --manifest <path>` surface is documented and caller-supplied, so `clean_apply_manifest` now treats the manifest as untrusted input: - Containment: an entry whose repo-relative path is absolute or traverses a parent (`..`, either separator) is rejected before any stat/rm and counted as a failure — an entry like `../outside` can no longer make `rm -rf` escape the repository. - Byte field as data: the size field is validated as an unsigned decimal before it reaches Bash arithmetic (which evaluates array subscripts recursively), so a crafted value cannot execute embedded command substitution. - Fail closed: `--apply --manifest <missing>` now exits non-zero with a clear message instead of printing `removed=0 failed=0` and exiting 0, which would let automation treat a mistyped path as a successful sweep. Also replaces the O(n^2) nested-target dedup with an O(n log n) sort pass: each path is sorted under a trailing-'/' key so an ancestor sorts immediately before all its descendants (and never swallows a sibling like `buildstuff`), then a single scan drops anything under the last kept ancestor — the dry-run no longer spends minutes deduplicating a monorepo's thousands of candidates before it can show the confirmation plan. Regression tests cover containment rejection, arithmetic-injection refusal via a sentinel, and the missing-manifest fail-closed exit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016D1oCPX8LaUKnLUi3TXmih
…rules The manifest containment guard stopped `..`/absolute escapes but still trusted that any in-repo path a manifest listed was a real cleanup target. A caller- supplied or concurrently-altered entry such as `caches\t1\tnotes` therefore passed the protection gate (an ordinary untracked dir is unprotected) and was removed while the run reported success. `clean_apply_manifest` now takes the tier's allowed classes and, per entry, rejects (fail closed, counted as a failure) any entry whose class the tier does not produce, and any path that is not a legitimate target for its class per the same candidate rules enumeration uses to find targets (explicit repo-root paths, a recognized dir-name leaf, or a file-glob leaf). clean-caches accepts only `caches`; clean-build accepts `build` and the caches it folds in. Regression tests cover a non-target untracked dir (`notes/`) and a wrong-tier entry — both rejected, preserved, non-zero exit. The rm-failure case now targets a valid explicit cache so it exercises the genuine Unremovable branch rather than target rejection. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016D1oCPX8LaUKnLUi3TXmih
…rwrite Two more hardenings of the untrusted --manifest surface: - Target type: manifest target validation now also checks the filesystem type each candidate rule emits (dir names / explicit dirs are found -type d, globs / explicit files -type f). A regular file merely named `bin` (or `__pycache__`) can no longer be removed by a crafted `build\t1\timportant/bin` — the dry-run could never have planned it. The existence check now precedes validation so a resumed, already-removed entry stays an idempotent no-op rather than a rejection. - Manifest overwrite: `--manifest <path>` is a caller-supplied input, so the dry-run build refuses to truncate an existing NON-manifest file (e.g. a mistyped `~/.config/app/settings`) and exits non-zero instead of silently erasing it; an absent path or an existing manifest-format file is still (re)written. Regression tests cover a file-as-dir-name target rejection and the refuse-to-overwrite guard (including that a real manifest is still rewritable). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016D1oCPX8LaUKnLUi3TXmih
…orever A trailing `--manifest` (e.g. `clean-caches.sh --apply --manifest`) hung: `$2` is empty so `shift 2` fails on the single remaining arg, and without `set -e` `$#` is unchanged, so the case loop reprocesses `--manifest` forever. Both entry scripts now require a value to follow `--manifest` and exit with the documented usage error (2) otherwise. Regression test guards with a `timeout` so a regression fails loudly instead of spinning. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016D1oCPX8LaUKnLUi3TXmih
Enumeration prunes .git/node_modules/.venv and never emits a target inside them, but the apply path did not re-exclude them: a manifest entry such as `caches\t1\t.git/objects/__pycache__` passed every guard (its basename is a valid target name and .git is not in the protected-substring set) and was removed — removing files under .git can corrupt the repository. Lift the prune set into a single CLEAN_PRUNE_DIRS SSOT that the single-walk enumeration builds its `-prune` alternation from AND the apply path rejects against, so the prune set is enforced uniformly on both sides. Any manifest entry with a pruned segment is now rejected (fail closed). Regression test covers a `.git/...` entry. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016D1oCPX8LaUKnLUi3TXmih
… records Two manifest-parsing gaps that let a bad manifest report success: - A valid final record with no trailing newline (common in caller-written files) was dropped: `read` returns nonzero at EOF so the loop body never ran, and `--apply` reported `removed=0 failed=0` while leaving the target in place. The loop now processes a nonempty final record via `|| [[ -n … ]]`. - A nonblank but truncated record missing the path field (e.g. `caches<TAB>1`, or a partial write by a concurrent process) was silently skipped. It is now a malformed-record failure (fail closed); only genuinely blank lines are ignored. Regression tests cover both. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016D1oCPX8LaUKnLUi3TXmih
…targets Two more manifest-surface hardenings, both keeping apply to exactly what the dry-run could have emitted: - Manifest creation: clean_manifest_path only truncated its target and never checked the result, so `--dry-run --manifest <unwritable-or-missing-parent>` printed a `Manifest:`/`Summary: planned=` plan for a file that was never written, handing apply a bogus path. It now fails (non-zero, prints nothing on stdout) when the file cannot be created. - Symlinked targets: the enumerator matches with `find -type d`/`-type f`, which never follow a symlink or Windows junction, but Bash `[[ -d ]]`/`[[ -f ]]` do. Target validation now requires a plain dir/file (not a symlink/reparse point), so a manifest naming a link (or a path swapped for one after the dry-run) is rejected instead of having its link removed. Regression tests cover the uncreatable-manifest failure and the symlinked-target rejection (the latter skips where the FS cannot create a symlink). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016D1oCPX8LaUKnLUi3TXmih
…ly parity Keeps the dry-run plan and the apply outcome faithful to each other and to the filesystem: - Apply loop fd isolation (root-cause bug): a per-entry validator shells out to `fsutil` (the reparse-point check), which drains stdin. Read on fd 0, it swallowed the rest of the manifest — every entry after the first was silently skipped, so `clean-build --apply --include-caches` removed the first target and reported success while leaving the rest. The loop now reads the manifest on a dedicated fd (3), and the reparse check gives fsutil its own `</dev/null`. - Reclaimed bytes are re-measured from the filesystem at removal time instead of trusting the manifest's dry-run byte field: the target may have changed since the plan, and a caller-supplied value never reaches the summary arithmetic (no overflow, no evaluation of untrusted input). - Dry-run/apply parity for explicit caches: an explicit cache path that is a file or symlink (not the plain dir `find` would emit) is filtered at planning, so the advertised plan is always applyable rather than being rejected at --apply. - Unencodable paths: a candidate whose path contains a tab or newline cannot be encoded in the tab-delimited, newline-terminated manifest, so it is skipped with a warning rather than written as a record that maps to the wrong path. Regression tests cover the file-named explicit cache, the recomputed bytes, and the unencodable-path skip; the existing include-caches apply test now exercises the multi-entry loop that the fd bug broke. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016D1oCPX8LaUKnLUi3TXmih
Comment-hygiene forbids tracker references in code comments; reword to name the downstream consumer generically. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016D1oCPX8LaUKnLUi3TXmih
`typos` flagged `applyable` and `mis-mapped` in the new comments; reword to
plain phrasing ("the plan can still be applied", "not mapped to a wrong path").
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016D1oCPX8LaUKnLUi3TXmih
The `..`/absolute containment guard and the final-component reparse check did not cover a symlinked ANCESTOR: a manifest naming `link/__pycache__`, where `link` points outside the repo, passed every check and `rm -rf` followed the ancestor symlink to delete the external directory while reporting success. Enumeration uses `find -type d`, which never descends a symlinked dir, so such a path is never one the dry-run emitted. Apply now walks every component of `root/rel` and rejects the entry (fail closed) if any ancestor or the target is a symlink/reparse point — junction-aware via the existing fsutil check, so it also catches Windows reparse-point ancestors that `realpath`/`-L` miss. Regression test covers a symlinked ancestor pointing outside the repo (skips where the FS cannot create a symlink). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016D1oCPX8LaUKnLUi3TXmih
The new symlinked-ancestor walk also catches a symlinked final target (it walks every component including the leaf), which fires before the type check — so a symlinked `.turbo` is now rejected as a symlinked path, not "not a caches target". Rename the marker to `Rejected (symlinked path)` (accurate for both a symlinked ancestor and a symlinked target) and align the two symlink regression tests, which now run on Linux CI. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016D1oCPX8LaUKnLUi3TXmih
… single-walk scan, resolver notes Add a stash audit action and script (age, source branch, diffstat, PR/merge signal, per-stash keep/drop advisory; never auto-drops; deduped across linked worktrees by --git-common-dir). Give branches checked out in a linked worktree their own WORKTREE bucket routed to the worktree tool, and surface never-pushed branches' unpushed commits (no-upstream count vs origin/<default>) as their own class plus an Unpushed line. Migrate scan.sh's per-pattern unpruned find walks onto the shared clean_enumerate engine so the read-only inventory and the mutating tiers share one prune set. Teach the resolver to echo trailing free text after an action token as an advisory Note. Closes #996 Closes #997 Closes #998 Closes #1000 Closes #1011 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016D1oCPX8LaUKnLUi3TXmih
|
Claude finished @kyle-sexton's task in 4m 1s —— View job PR Review — feat(repo-hygiene): stash audit, worktree/no-upstream branch classes, single-walk scan, resolver notesStatus: Complete
Overall this is a well-structured batch. The manifest security hardening is thorough, the test suite is excellent, and the design decisions (read-only stash audit, WORKTREE bucket priority ordering, exit-status gate for upstream detection) are all correct. Three items worth addressing before merge. Findings1.
|
|
Claude finished @kyle-sexton's task in 6m 22s —— View job Security Review
No security vulnerabilities found. The changes are defensively written and address the relevant threat models. Below is a full account of the surfaces I examined and my reasoning. Surfaces audited
|
| Gate | What it blocks |
|---|---|
clean_manifest_rel_safe |
absolute paths, ../ traversal, empty entries |
clean_path_has_pruned_segment |
paths inside .git/, node_modules/, .venv/ |
clean_path_has_reparse_ancestor |
symlink/junction ancestors that rm -rf would follow out of the repo |
clean_manifest_target_valid |
arbitrary untracked paths not in the known-safe pattern set; also rejects symlinks/junctions via clean_is_plain_dir/clean_is_plain_file |
clean_target_eligible |
re-runs the protection gate at apply time (staleness guard) |
A few specific checks I verified:
- Manifest fd isolation —
3<"$manifest"/<&3keeps the manifest on a dedicated fd so that tools called per-entry (e.g.fsutil reparsepoint query) cannot drain stdin and silently skip later entries. The PR also adds</dev/nullto thefsutilcall inclean_path_is_reparse_pointfor the same reason. bytesfield from manifest never reaches arithmetic —du -skre-measures size at removal time; the manifest's byte field is read intobytesbut discarded. A caller-supplied overflowed or injected value cannot affect the summary counters.duoutput validated before arithmetic —[[ "$kb" =~ ^[0-9]+$ ]] || kb=0inclean_apply_manifestguards against non-numericduoutput.- Dedup prefix check —
[[ "$key" == "$last_key"* ]]after an LC_ALL=C sort with/-appended keys. The trailing/correctly preventsbuild/from swallowing a siblingbuildstuff/(the pattern/repo/buildstuff/does not start with/repo/build/because the char afterbuildin the LHS iss, not/). - Manifest writable-target guard —
clean_manifest_writable_targetvalidates an existing file at the caller-supplied path matches^(caches|build)\t[0-9]+\ton every non-blank line before truncating it, so a mistyped path pointing at an unrelated file is refused rather than silently erased. clean_enumeratefindinvocation — built entirely from hardcoded constants (CLEAN_PRUNE_DIRS,CLEAN_CACHE_FIND_DIR_NAMES, etc.) plus a repo-root path from git. No user-controlled data reachesfind's arguments.
One acknowledged TOCTOU — between dry-run manifest creation and --apply consumption, a concurrent process could modify the manifest or swap a validated path for a symlink. The per-entry validation at apply time is the stated mitigation, and it is comprehensive. This is the documented design constraint, not a new gap introduced by this PR. SUGGESTION severity; no fix required.
scan.sh (modified)
emit_path_line now takes an absolute path directly from clean_caches_candidates/clean_build_candidates and derives rel via ${abs#"$REPO_ROOT"/}. The ${kb:-0} guard on the du output before $((...)) arithmetic is a minor correctness improvement over the old bare-subshell form. No injection surface.
No GitHub Actions files changed
No .github/workflows/ files were modified; the Actions-specific injection patterns (untrusted github context in run: steps, pull_request_target + secrets, unpinned action refs) are not in scope for this PR.
Summary: All external data (git output, gh API responses, stash subjects, manifest entries) is handled through double-quoted expansions, printf %s format strings, or explicit validation chains before any destructive operation. No command injection, path traversal, or privilege escalation vectors were found.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a331f34f08
ℹ️ 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".
|
🔒 babysit-prs lane claiming this PR for on-branch fix work this cycle (amendment-round: 16, safe tier). Will fix clear branch-owned findings and push; will not resolve threads or merge (safe tier). |
Bring the PR up to date with main (now containing #1023). Conflicts resolved: plugin.json keeps the 0.6.0 bump over main's 0.5.0; clean-common.sh takes main's finalized version; cleanup-paths.sh, CHANGELOG.md, action-router.md keep this branch's Batch-D additions layered over main's content. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016D1oCPX8LaUKnLUi3TXmih
The merge inadvertently carried this branch's older base snapshot of the source-control plugin (0.18.0), reverting main's 0.19.0. Reset those files to origin/main so the PR diff is exactly the Batch-D repo-hygiene changes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016D1oCPX8LaUKnLUi3TXmih
|
Claude encountered an error after 0s —— View job I'll analyze this and get back to you. |
|
Claude encountered an error after 0s —— View job I'll analyze this and get back to you. |
|
Warning Automated review did not complete — this is an infrastructure failure, not a review verdict. Treat any Claude comment on this PR (including a placeholder like "I'll analyze this and get back to you") as incomplete, not "no findings."
Re-running the job, or pushing a new commit, will retry the review. |
|
Warning Automated security review did not complete — this is an infrastructure failure, not a review verdict. Treat any Claude comment on this PR (including a placeholder like "I'll analyze this and get back to you") as incomplete, not "no findings."
Re-running the job, or pushing a new commit, will retry the review. |
…sh-preflight-resolver
|
Claude encountered an error after 0s —— View job I'll analyze this and get back to you. |
|
Claude encountered an error after 0s —— View job I'll analyze this and get back to you. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 564f36cd01
ℹ️ 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".
Initialize the stash-audit and branch-audit test repositories with `git init -b main` so the `main`-branch assumptions hold on machines where `git init` still defaults to `master` — otherwise the stash on the initial branch is attributed to `master`, flagged possibly-superseded against origin/main, and the case fails before validating the intended behavior. Verified green under a forced init.defaultBranch=master environment. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016D1oCPX8LaUKnLUi3TXmih
|
Claude encountered an error after 0s —— View job I'll analyze this and get back to you. |
|
Claude encountered an error after 0s —— View job I'll analyze this and get back to you. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 03c63e1aaf
ℹ️ 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".
Address Codex review threads on the Batch-D PR:
- git-branch-audit: a branch whose upstream is gone but that still carries
commits absent from origin/<default> is now REVIEW, not the LIKELY-SAFE
deletion candidate the bare gone-check assigned (prevents losing unmerged
local commits). Ahead-of-default is computed for every branch.
- git-stash-audit: emit each stash's stable commit id (Commit:) so multi-drop
workflows do not rely on the volatile stash@{n} selector that renumbers after
every drop.
- destructive-guard: gate `git stash drop`/`clear` behind the CLEAN_GUARD_ACK
acknowledgement, closing the fail-open where a confirmed-then-mistaken drop
could discard saved WIP.
- Portability: the branch-audit example table uses origin/<default> instead of a
hardcoded origin/main; new audit scripts get the executable bit.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016D1oCPX8LaUKnLUi3TXmih
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4b7c976389
ℹ️ 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".
…ght-resolver Batch C's fleet-batch mode merged to main first (first-ready-wins). Rebase this PR's version to 0.7.0 over C's 0.6.0 and reorder the CHANGELOG so 0.7.0 (stash audit, worktree/no-upstream branch classes, resolver notes, single-walk scan) sits above 0.6.0 (fleet batch). Shared-file conflicts resolved to carry BOTH feature sets: SKILL.md description/argument-hint list the stash action and the caches/build/git/all batch forms; resolve-clean-action.sh keeps the action-token note short-circuit and adopts C's reset-intent split (tree-batch vs all-batch). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016D1oCPX8LaUKnLUi3TXmih
|
Claude encountered an error after 1s —— View job I'll analyze this and get back to you. |
|
Claude encountered an error after 0s —— View job I'll analyze this and get back to you. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 08be94ae7d
ℹ️ 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".
…outing
Two review-thread fixes on the reconcile push:
- git-branch-audit: a gone-upstream branch with no fetched origin/<default> to
compare against was still LIKELY-SAFE (a deletion candidate) despite unprovable
safety. Classify it REVIEW ("upstream gone, cannot compare against
origin/<default>") — fail closed so local-only commits are never lost.
- resolve-clean-action: gate the advisory-note short-circuit on
`! has_fleet_indicator` (not just `! is_fleet_phrase`), so a fleet request the
phrase list misses ("all across the fleet") is no longer preempted into a
single-repo action with the fleet words demoted to a note; it routes to the
fleet/batch resolution instead.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016D1oCPX8LaUKnLUi3TXmih
…sh-preflight-resolver
|
Claude encountered an error after 0s —— View job I'll analyze this and get back to you. |
|
Claude encountered an error after 1s —— View job I'll analyze this and get back to you. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d4d8cf0eff
ℹ️ 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".
| | main | PROTECTED | 0d | — | 0 ahead of origin/<default> | default branch | | ||
| | feat/parked | WORKTREE | 3d | — | 0 ahead of origin/feat/parked | checked out in worktree — clean up the worktree first | | ||
| | feat/old-thing | SAFE | 45d | #123 MERGED | 0 ahead of origin/feat/old-thing | PR merged | | ||
| | refactor/x | LIKELY-SAFE | 12d | — | no upstream (no origin/<default> to compare) | upstream gone | |
There was a problem hiding this comment.
Fresh evidence after the gone-branch fix: this updated report example still presents Unpushed: no upstream (no origin/<default> to compare) as LIKELY-SAFE, even though the script now correctly classifies that scenario as REVIEW because it cannot prove the branch is merged. Since the same section defines deletion candidates as SAFE + LIKELY-SAFE, agents following this context can offer an uncompareable gone branch for deletion despite the fail-closed behavior the script now enforces.
Useful? React with 👍 / 👎.
| second_lower="$(printf '%s' "$2" | tr '[:upper:]' '[:lower:]')" | ||
| second_action="$(resolve_one "$second_lower")" | ||
| fi | ||
| if [[ -n "$first_action" && $# -ge 2 && -z "$second_action" ]] && ! has_fleet_indicator "$joined"; then |
There was a problem hiding this comment.
When the first token is already an explicit batch/fleet action, this full-input has_fleet_indicator guard treats that action token itself as a fleet indicator and disables note extraction, so inputs like caches-batch mind the live sessions or tree-batch skip repo X resolve to only Action: ... and silently drop the trailing constraint. Check the fleet indicator on the suffix after the action token instead, so explicit batch actions can still surface advisory Note: text while phrases like all across the fleet remain protected from single-repo routing.
Useful? React with 👍 / 👎.
1 participant
Summary
Batch D small/medium fixes for the
repo-hygieneclean skill, delivered as one PR. Version bumped0.5.0→0.6.0with a matching CHANGELOG entry.Closes #996
Closes #997
Closes #998
Closes #1000
Closes #1011
git-stash-audit.sh(+ sibling test): per-stash age, source branch, untracked-inclusive diffstat, PR/merge signal, and a per-stash keep/drop advisory. Never drops a stash — the agent confirms keep-or-drop per entry, even for asupersededadvisory. Deduped across linked worktrees by the--git-common-dirStashStore:key. Runs standalone (stashaction) and as part of thegittier. Related metadata gaps (reflog expiry,git maintenance, fsck sweep) are intentionally NOT folded in — they stay tracked in the issue.git-branch-audit.shsubtracts branches checked out in linked worktrees into a distinctWORKTREEtier (reason "clean up the worktree first"), routed to the worktree-management tool instead of being lumped intoPROTECTEDor offered forgit branch -d. Protected-name checks now rank above the worktree check.Summary:gains aworktree=count.git rev-listcount againstorigin/<default>, surfaced as their own REVIEW class and a per-branchUnpushed:line (N ahead of <upstream>/no upstream, M commits not on origin/<default>). Also fixed a latent bug whererev-parse --abbrev-refechoes its input on failure (a configured-but-unfetched upstream) and was mistaken for a real upstream.resolve-clean-action.shemitsNote: <trailing text>when a leading action token is followed by advisory free text (a question or a live-session constraint), documented in SKILL.md as context the agent must address. Trailing text is no longer re-interpreted as an action token, so a note mentioning an action word (e.g. "…include stashes?") no longer forces a false conflict.scan.shmigrated off its per-pattern unprunedfindwalks onto the sharedclean_enumerate/clean_caches_candidates/clean_build_candidatesengine, so the read-only inventory and the mutating caches/build tiers share one prune set (no longer descends.git/,node_modules/,.venv/). Removed the now-unusedCLEAN_FIND_EXCLUDE_VENV/CLEAN_FIND_EXCLUDE_NODE_MODULESvars.Verification
repo-hygieneclean*.test.shgreen; shellcheck (--rcfile .shellcheckrc),shfmt -i 2, markdownlint, andcheck-changelog-parity --check-bumpclean. No issue-refs / TODO markers in code comments (comment-hygiene).WORKTREEbucket and surfaced no-upstream branches' unpushed commit counts.Related
feat/repo-hygiene-fleet-batch) also editsresolve-clean-action.sh,SKILL.md,action-router.md, and the CHANGELOG, so merge-order conflicts there are expected and accepted (kept edits surgical to minimize them).feat/repo-hygiene-single-walk-manifest) — rebase--onto origin/mainafter feat(repo-hygiene): single pruned walk + dry-run manifest + apply summary #1023 squash-merges; at that point reconcile the version/CHANGELOG heading against then-current main (bump to0.7.0if Batch C's0.6.0landed first).🤖 Generated with Claude Code