feat(cleanup): prune orphaned git worktrees on gsd-cleanup

[rsieb]

Problem

GSD creates git worktrees automatically during phase execution but never cleans them up. Worktrees accumulate indefinitely across projects. One vault I work in had 41 registered worktrees with 32 branches already merged into main — none of them were ever removed because there was no cleanup path.

gsd-cleanup already handles phase directory archival and stale branch pruning. Worktrees are the missing third category.

What this changes

Extends get-shit-done/workflows/cleanup.md with a prune_orphaned_worktrees step:

Detection (in show_dry_run):

• Enumerates all non-primary worktrees via git worktree list
• Cross-references against git branch --merged main to identify branches that are safe to delete
• Also runs git worktree prune --dry-run to surface metadata-only stubs (directories already gone)
• Shows the candidate list before any action

Execution (new prune_orphaned_worktrees step, runs after prune_local_branches):

• git worktree remove <path> --force — removes the directory and .git/worktrees/<name> metadata
• git branch -d <branch> — deletes the local branch (lowercase -d refuses unmerged branches as a safety net)
• git worktree prune — cleans any remaining dangling metadata stubs

Confirmation gate: Extended the existing AskUserQuestion to cover all three actions (archive phases, prune branches, remove worktrees) in a single confirm.

Definition of "orphaned"

A worktree is considered orphaned if its branch appears in git branch --merged main (excluding protected names: main, trunk, develop, next). This is the same standard used for branch pruning in the existing workflow.

Test plan

• Run gsd-cleanup in a repo with merged worktrees — verify dry-run shows the orphans
• Confirm and verify the worktrees + branches are removed
• Run in a repo with no worktrees — verify "No orphaned worktrees detected." and no errors
• Run git worktree list after cleanup — verify only non-merged worktrees remain

🤖 Generated with Claude Code
via Happy

---

^{Need help on this PR? Tag /codesmith with what you need. Autofix is disabled.}

[github-actions]

PR template required

This PR does not follow one of the required pull request templates from the contribution guidelines.

Please update the PR description to use the correct typed template:

• Fix PR template
• Enhancement PR template
• Feature PR template

This check only validates template format. It does not close PRs for an unfilled issue number such as Fixes #; the issue-link workflow handles issue references separately after the PR exists.

Detected problem: PR body does not match the fix, enhancement, or feature template.

[Solvely-Colin]

@rsieb

Thanks for tackling this. Retargeting to next has the PR scoped correctly now: the diff is just get-shit-done/workflows/cleanup.md, and the feature direction makes sense.

Requesting changes on the cleanup implementation before merge:

1. The new git worktree remove --force path can delete dirty or untracked files in a merged worktree. A branch being merged into main does not guarantee the worktree has no local-only changes. Please make this fail closed: detect dirty/untracked worktrees and report them for manual cleanup, or route through the existing safety-aware worktree helper.

2. The workflow parses git worktree list with awk '{print $1, $3}', which is unsafe for paths containing spaces. Please use git worktree list --porcelain or the existing parser/helper path.

3. The merged-branch check hard-codes main and does not protect master. Please resolve the default branch dynamically, or at least include master in protected branch handling.

Also, the PR template check is still failing, so the PR body needs to be updated to one of the required templates.

Once those are addressed, I’d be happy to re-review. The feature is useful; the main thing is making sure cleanup never becomes a data-loss path.

[trek-e]

please read and use the repo guidelines for formatting of the PR, these are important for how change logs and documentation are updated.

[trek-e]

Combined review — /code-review + /adversarial-review (Codex) + /security-review + Artificer laws

Thank you for this, Roland — orphaned-worktree pruning is a genuinely useful addition to gsd-cleanup, and I want to call out the parts you clearly thought hard about: the dry-run preview, the explicit "re-enumerate at execution so it matches what the user confirmed" intent, and the deliberate choice of -d (safe-delete) over -D for branches. That instinct toward safety is exactly right for destructive automation.

That said, I'm requesting changes — this touches destructive operations (removing worktrees, deleting branches), and both my review and an independent Codex pass surfaced several ways it can silently destroy uncommitted work or target the wrong thing. I want to make sure you get full value from a rebase, so everything's below.

🚧 Structural blockers (independent of the logic)

1. Branch is 40 commits behind next and predates the get-shit-done → gsd-core rename. This PR edits get-shit-done/workflows/cleanup.md, but on next that file now lives at gsd-core/workflows/cleanup.md (the old path is gone, per the ADR-0008 migration). As-is, these edits won't land on the canonical file. Please rebase onto current next and re-apply the change to gsd-core/workflows/cleanup.md — which has also evolved, so expect to reconcile.
2. No changeset. This is a user-facing feat; please add a .changeset/*.md fragment (the Changeset-Required gate will want it).
3. (Carryover from my earlier note) please align the PR description with the repo's PR template / contribution guidelines.

🔴 Critical — data-loss & self-destruct (both reviewers, confirmed against the lines)

• git worktree remove "$wt_path" --force 2>/dev/null discards uncommitted work (:217, :231). The comment justifies --force as safe because "the branch is merged" — but merged says nothing about uncommitted/unstaged/untracked files in that worktree (WIP edits, a local .env, an un-applied patch). --force bypasses git's dirty-worktree protection and the 2>/dev/null hides the only signal you'd lose work. Don't --force: try the non-force git worktree remove and, on failure, skip and report the worktree as "dirty — left in place" rather than forcing.
• No guard against removing the currently-active worktree (:214, only tail -n +2 skips the primary). gsd-cleanup frequently runs from inside a linked worktree (e.g. a claude/... session worktree). If that worktree's branch is considered merged, the loop will try to git worktree remove the directory it's running in. Add an explicit guard: resolve git rev-parse --show-toplevel and skip any worktree whose path equals it (and the primary).

🔴 High — wrong targets

• Hardcoded git branch --merged main (:120, :207). This repo's default branch is next, not main. A branch merged into next (the real integration branch) won't appear; worse, if local main is stale or only-via-main, you can flag branches that aren't actually integrated into next and remove their worktrees. Resolve the default dynamically: git symbolic-ref --short refs/remotes/origin/HEAD (fallback to next/main), and compare against the remote-tracking ref after a fetch.
• Fragile git worktree list | awk '{print $1, $3}' | tr -d '[]' (:114, :214). Breaks on worktree paths containing spaces (fields shift → $1 truncated, $3 is the SHA, and you then pass a wrong path to remove) and on detached-HEAD worktrees (no branch field). Use git worktree list --porcelain (ideally -z) and parse the worktree/branch records — this also removes the tr -d '[]' hack.
• Dry-run/execution race (:203 claims "matches exactly what the user confirmed", but :207/:214 recompute). Between the preview and execution, other steps (prune_local_branches does git fetch --prune) can change the merged set, so a worktree never shown to the user can be removed. Capture the confirmed list once in the dry-run and operate only on that captured set (intersected with still-present worktrees), rather than recomputing.

🟡 Medium

• grep -qx "$branch" treats the branch name as a regex (:215). Valid branch names with . (e.g. feat/foo.bar) can match the wrong branch (feat/fooXbar). Use grep -Fqx -- "$branch". (Note: feat/[x] isn't a valid git ref, but . absolutely is, so this is real.)
• git worktree remove "$wt_path" --force — arg order works on modern git (verified locally on 2.50.1) but the canonical safe form is git worktree remove --force "$wt_path"; prefer it for older-git portability.

🟢 Low

• The report count conflates git worktree prune (metadata stubs for already-gone dirs) with git worktree remove (destructive). Consider reporting them separately so "Worktrees removed: N" reflects only the destructive count.

Security review

No classic vulnerabilities (no untrusted network/subprocess input) — branch names aren't a meaningful injection vector here. The real risk class is data loss / unintended destruction (the Critical items above), which I'd treat with the same seriousness.

Artificer laws

• Kernighan's Law — "debugging is twice as hard as writing it." The awk | sed | tr | grep parsing pipeline is exactly the kind of clever text-munging that's hard to get right; the --porcelain route is both simpler and correct.
• Merged-as-proxy-for-safe (Goodhart-flavored) — "branch is merged" is being used as a proxy for "safe to delete the worktree," but the two diverge precisely when there's uncommitted local state. Guard the real property (clean worktree), not the proxy.

---

Verdict: 🔧 Changes requested. The feature is worth landing, but destructive logic needs to be safe-by-default first. Highest priorities: (1) never --force away a dirty worktree, (2) never remove the active/primary worktree, (3) resolve the real default branch, (4) parse via --porcelain. After the rebase onto gsd-core/workflows/cleanup.md + changeset, I'm happy to do a fast re-review. Really appreciate you tackling this — the dry-run design shows you're aiming at the right target.

[jslitzkerttcu]

Hey Roland, worktree cleanup is a real pain point and your dry run plus safe delete instincts are spot on.

Wanted to raise one thing before you sink time into the rebase, because the tree has moved a lot since this branch was cut and it changes the shape of the work.

Since this PR was opened, the repo grew a dedicated Worktree Safety Policy Module at gsd-core/bin/lib/worktree-safety.cjs. It already implements the safe versions of basically everything the review flagged:

• parseWorktreePorcelain does the --porcelain parsing, so no more awk | tr | grep (handles paths with spaces and detached HEAD)
• planWorktreePrune is non destructive by default and reapOrphanWorktrees skips dirty worktrees and the active/primary one instead of force removing them
• inspectWorktreeHealth does orphan and stale detection (the W017 class)

It is even already exposed as a command: node gsd-tools.cjs worktree reap-orphans. So the safe primitive your PR is hand rolling in bash basically exists in tree now.

Here is the gap though, and why I am not sure this should just be closed: the /gsd-cleanup workflow (gsd-core/workflows/cleanup.md) only prunes stale branches today. It never calls worktree reap-orphans, so the actual user facing outcome you are after (orphaned worktrees getting cleaned up during gsd-cleanup) still is not wired up.

So my suggestion: rather than rebasing the bash implementation and re-fixing the four data loss issues by hand, the lighter and safer path is to wire the existing gsd-tools worktree reap-orphans command into cleanup.md (with the dry run preview you already designed). You get all the safety guarantees for free and the diff shrinks to mostly workflow wiring plus a changeset.

Two questions for you and the maintainers:

1. Are you up for reworking this to drive the existing module instead of the standalone bash? Happy to point at the exact dispatch and the cleanup.md step where it would slot in.
2. For maintainers: would you rather keep this PR open for that rework, or close it in favor of a fresh issue scoped to "wire reap-orphans into /gsd-cleanup"?

Either way the original instinct here was right, it is mostly that the safe machinery landed in parallel.

[trek-e]

Summary

This PR adds a prune_orphaned_worktrees step to get-shit-done/workflows/cleanup.md so that /gsd-cleanup removes git worktrees whose branch is merged into main. The motivation (worktrees accumulating indefinitely) is real and worth solving. However, the change cannot be accepted in its current form: it reverses an Accepted ADR decision and bypasses the established worktree-safety module seam, makes a destructive git worktree remove --force the default behavior, and fails several hard contribution gates (issue link, ADR, documentation, changeset). Requesting changes.

Per the review directive, the control flow stops at the failed issue-link gate (§0.3) and ADR gate (§2B.1), so the downstream /security-review, /adversarial-review, /code-review, and /rubber-duck passes were not run — the gate blockers below are dispositive. The technical findings (§Findings) are included so they can be addressed in the same revision.

Classification & gate compliance

• Track: Feature (feat(cleanup):). The behavior is also governed by an existing Accepted ADR, so the enhancement-track ADR rule applies too.
• Issue link — FAIL (blocker). PR body contains no closes/fixes/resolves #<issue>. CI.GATE.issue-link-required=hard-fail if PR body lacks closes/fixes/resolves #<issue>.
• Classification label — FAIL. PR has no labels; there is no approved-feature/approved-enhancement, and no linked issue to verify approval. RULESET.CONTRIB.CLASSIFY.feature=requires approved-feature before implementation.
• ADR gate — FAIL (blocker). This behavior is owned by docs/adr/0004-worktree-workstream-seam-module.md (Status: Accepted). No ADR addendum is included or signed off. Per §2B.1, an existing ADR covering the changed behavior requires a maintainer-signed addendum before review proceeds.
• Documentation gate — FAIL (hard blocker). No docs/ files changed. docs/COMMANDS.md /gsd-cleanup (around line 809) still describes only phase archival + branch pruning. RULESET.PR-FLOW.docs-by-changeset-type (changed command behavior → docs/USER-GUIDE.md + docs/COMMANDS.md).
• Changeset — FAIL. No fragment under .changeset/ and no no-changelog label (CI.GATE.changeset-lint). A feat needs an Added fragment.
• Branch naming — FAIL. feat/worktree-cleanup violates RULESET.CONTRIB.BRANCH.format=branch must be <prefix>/NNN-slug (no issue number).
• PR template — FAIL. Body uses the codesmith Problem/What/Test-plan layout, not the repo .github/ PR template (PRED.k328.audit-list).
• Scope: OK — single concern.
• Competing PRs: none (only this PR references worktree cleanup).

Findings by severity

Blocker

1. Reverses Accepted ADR-0004 + bypasses the canonical worktree seam.
   ADR-0004 states: "Worktree metadata cleanup remains non-destructive by default: pruneOrphanedWorktrees runs git worktree prune only and does not remove linked worktree directories." CONTEXT.md reinforces this:

   • WORKTREE.SEAM.default-prune-policy=metadata_prune_only (non-destructive)
   • WORKTREE.SEAM.decision-1=retain non-destructive default; destructive path only as explicit future opt-in scaffold

   This PR makes destructive git worktree remove --force the default behavior of /gsd-cleanup — a direct reversal of an Accepted ADR decision, without an ADR addendum. The canonical seam is the Worktree Safety Policy Module (src/worktree-safety.cts → get-shit-done/bin/lib/worktree-safety.cjs), which already exposes inspectWorktreeHealth (orphan + stale detection), planWorktreePrune, executeWorktreePrunePlan, parseWorktreePorcelain, and snapshotWorktreeInventory, all with bounded git subprocess calls that return git_timed_out instead of throwing. The PR reimplements orphan detection with ad-hoc bash, violating WORKTREE.SEAM.caller-rule=...no ad-hoc porcelain parsing in callers and the safety invariant WORKTREE.SEAM.invariant=parser failure must degrade to metadata_prune_only and never escalate to destructive removal (the ad-hoc pipeline, on a parse miss, still proceeds to git worktree remove --force).

2. Data-loss risk in git worktree remove --force.
   The step note claims: "--force handles worktrees that have uncommitted changes but are confirmed merged — the branch content is already in main." This reasoning is incorrect. "Branch merged into main" means the committed history is in main; it says nothing about uncommitted, unstaged, or untracked changes in the worktree's working directory. git worktree remove --force deletes those irrecoverably. A worktree on a merged branch can still hold un-pushed local edits.

3. Missing issue link (CI.GATE.issue-link-required) — see gate table.

4. Missing documentation (hard blocker, §2B.2). The new destructive capability is undocumented end-to-end. Please use the /writing-documentation-with-diataxis skill: this needs reference coverage (update docs/COMMANDS.md /gsd-cleanup entry) and a how-to (e.g. docs/USER-GUIDE.md: how to prune orphaned worktrees, including the safety/confirmation semantics). Keep each doc to a single Diátaxis mode and don't mix them. Framework reference: https://diataxis.fr/

5. Missing/unsigned ADR addendum (§2B.1) — see gate table.

Major

6. git worktree list --porcelain is the documented script interface; this scrapes human output (§4).
   git worktree list | tail -n +2 | awk '{print $1, $3}' | tr -d '[]' parses the human-readable format, which git documents as unstable for scripts. Use git worktree list --porcelain (the seam's parseWorktreePorcelain already does this). The current parse breaks on detached-HEAD worktrees ((detached HEAD) → $3='(detached'), paths with spaces, and locked/prunable entries.

7. Hardcoded git branch --merged main; repo default branch is next.
   git branch --merged main 2>/dev/null returns nothing when main doesn't exist (the error is silenced), so on next/master-default repos no orphans are detected — or, worse, mis-evaluated. The protected-name list already includes next, which shows awareness, yet the merge base stays hardcoded to main. Resolve the default branch dynamically (the seam centralizes worktree-root/git-dir logic via resolveWorktreeRoot).

8. Unbounded git subprocesses reintroduce DEFECT.UNBOUNDED-SUBPROCESS ("git/npm subprocess shelled out without timeout; CLI hangs indefinitely on stuck remote, large repo, or missing network"). The seam bounds every git call and degrades to git_timed_out; the ad-hoc bash has no timeout.

9. Windows portability. The entire step is POSIX bash (awk/sed/tr/tail/grep -qx/while read). This file already carries a "Text mode for non-Claude runtimes (Codex, Gemini)" note, so cross-platform/non-bash runtimes are in scope; this step will not run on Windows or in those runtimes.

Minor

10. No test/regression coverage. Worktree-pruning logic has test anchors (tests/worktree-safety.test.cjs, tests/orphan-worktree-detection.test.cjs); routing through the seam would let this behavior be pinned there. WORKTREE.SEAM.test-policy=cover all decision branches in policy module before changing prune behavior.

Nit

11. The PR's own Test plan checkboxes are all unchecked — no evidence the manual plan was executed.

What's right

• Using git worktree remove / git worktree prune (documented git commands) rather than rm -rf of .git/worktrees/* is the correct primitive choice — the issue is the destructive --force default and bypassing the safety module, not the command selection.
• git branch -d (lowercase) as a merge safety net is a reasonable touch.

Recommended path forward

1. File/link an issue and get an approved-enhancement (this touches an Accepted ADR) before implementation.
2. Add a maintainer-signed addendum to ADR-0004 deciding whether destructive directory removal becomes a default or an explicit opt-in (WORKTREE.SEAM.decision-1 framed it as opt-in scaffold).
3. Implement detection/removal through the Worktree Safety Policy Module (inspectWorktreeHealth + planWorktreePrune/executeWorktreePrunePlan), not ad-hoc workflow bash — preserving bounded timeouts, porcelain parsing, and the degrade-to-metadata-prune invariant.
4. Drop --force as default, or gate it behind explicit confirmation that surfaces uncommitted/untracked changes.
5. Add Diátaxis docs, a changeset fragment, a conforming branch name, and the repo PR template.

Verdict

Request changes. Multiple hard-gate failures (issue link, ADR/addendum, documentation, changeset) plus a destructive default that reverses Accepted ADR-0004 and bypasses the worktree-safety seam.