From 48c1827028694b2503b031a64f44945f0e687388 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sun, 31 May 2026 22:31:54 -0400 Subject: [PATCH] enhancement(#40): integrate branch pruning into /gsd-cleanup archival workflow (#564) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * enhancement(#40): integrate branch pruning into /gsd-cleanup archival workflow Adds a prune_local_branches step to cleanup.md (between archive_phases and commit) that force-deletes local branches whose upstream is gone — keeping local clones symmetric with delete_branch_on_merge on GitHub. Key design choices vs. PR #562 (the local-model draft): - dry-run step shows stale branches using cached tracking refs only; git fetch --prune is deferred to the execution step so the dry-run is non-side-effecting - awk uses { if ($1 != "*") print $1 } form to explicitly exclude the currently checked-out branch (the * prefix in git branch -vv output), not a prose note that lets xargs receive literal * as an argument - git fetch --prune runs exactly once, in prune_local_branches, eliminating the TOCTOU window between a preview fetch and an execution fetch - two new negative-contract tests: identify_completed_milestones must not run git branch commands; show_dry_run must not run git fetch --prune Closes #40 Co-Authored-By: Claude Sonnet 4.6 * chore: regenerate changeset using repo script (correct format) Replaces hand-written fragment (used `/** ... */` comment syntax) with one generated by `npm run changeset -- --type Changed --pr 562`. Co-Authored-By: Claude Sonnet 4.6 * fix: address codex review blockers — protect main/next/trunk, align dry-run with execution Codex adversarial review (pre-PR gate) flagged two blockers: 1. awk filter only excluded '*' (current branch) but not protected names. main/next/trunk/develop could be force-deleted if their upstream was gone. Fix: use !~ /^\*$|^main$|^next$|^trunk$|^develop$/ regex match. 2. Dry-run enumerated from cached tracking refs; execution re-ran git fetch --prune, creating a TOCTOU window between what the user confirmed and what got deleted. Fix: move git fetch --prune into show_dry_run (prefetch for display accuracy); prune_local_branches now enumerates from the already-fetched state with no second fetch. Updated 14 structural tests to match new design (added protected-name exclusion test; inverted show_dry_run fetch assertion). Co-Authored-By: Claude Sonnet 4.6 --------- Co-authored-by: Claude Sonnet 4.6 --- .changeset/wise-yaks-run.md | 5 + docs/COMMANDS.md | 9 +- get-shit-done/workflows/cleanup.md | 46 ++++- tests/cleanup-branch-pruning.test.cjs | 287 ++++++++++++++++++++++++++ 4 files changed, 343 insertions(+), 4 deletions(-) create mode 100644 .changeset/wise-yaks-run.md create mode 100644 tests/cleanup-branch-pruning.test.cjs diff --git a/.changeset/wise-yaks-run.md b/.changeset/wise-yaks-run.md new file mode 100644 index 000000000..3472c49d8 --- /dev/null +++ b/.changeset/wise-yaks-run.md @@ -0,0 +1,5 @@ +--- +type: Changed +pr: 562 +--- +/gsd-cleanup now prunes local branches whose upstream is gone — symmetric with delete_branch_on_merge; dry-run is non-side-effecting and current-branch exclusion is explicit in the awk filter diff --git a/docs/COMMANDS.md b/docs/COMMANDS.md index 094687698..69bdf45c4 100644 --- a/docs/COMMANDS.md +++ b/docs/COMMANDS.md @@ -776,12 +776,19 @@ v1.40.0, [#2792](https://github.com/open-gsd/gsd-core/issues/2792)). ### `/gsd-cleanup` -Archive accumulated phase directories from completed milestones. +Archive accumulated phase directories from completed milestones and prune local branches whose upstream has been deleted. ```bash /gsd-cleanup ``` +On confirmation, the workflow performs two actions: + +1. **Phase archival** — moves phase directories from `.planning/phases/` into milestone archive directories under `.planning/milestones/v{X.Y}-phases/`, using archived ROADMAP snapshots to determine phase membership. +2. **Branch pruning** — runs `git fetch --prune` to update remote-tracking refs, then identifies and force-deletes local branches whose upstream is marked gone. The currently checked-out branch is always skipped. + +The dry-run summary shows both phase directories to archive and candidate local branches for deletion before confirmation. + --- ## Spiking & Sketching Commands diff --git a/get-shit-done/workflows/cleanup.md b/get-shit-done/workflows/cleanup.md index 52dce76a6..c1ef70a72 100644 --- a/get-shit-done/workflows/cleanup.md +++ b/get-shit-done/workflows/cleanup.md @@ -85,17 +85,38 @@ These phase directories will be archived: Destination: .planning/milestones/v{X.Z}-phases/ ``` -If no phase directories remain to archive (all already moved or deleted): +**Stale local branches (upstream gone):** + +First, update remote-tracking refs so the candidate list matches the execution list exactly: + +```bash +git fetch --prune 2>/dev/null || true +``` + +Then enumerate candidates (protected branch names are excluded even if their upstream is gone): + +```bash +git branch -vv | awk '/: gone\]/ { if ($1 !~ /^\*$|^main$|^next$|^trunk$|^develop$/) print $1 }' +``` + +Show each branch name. If none, show: + +``` +No stale local branches detected. +``` + +If no phase directories remain to archive (all already moved or deleted) AND no stale branches exist: ``` No phase directories found to archive. Phases may have been removed or archived previously. +No stale local branches detected either. ``` Stop here. **Text mode (`workflow.text_mode: true` in config or `--text` flag):** Set `TEXT_MODE=true` if `--text` is present in `$ARGUMENTS` OR `text_mode` from init JSON is `true`. When TEXT_MODE is active, replace every `AskUserQuestion` call with a plain-text numbered list and ask the user to type their choice number. This is required for non-Claude runtimes (OpenAI Codex, Gemini CLI, etc.) where `AskUserQuestion` is not available. -AskUserQuestion: "Proceed with archiving?" with options: "Yes — archive listed phases" | "Cancel" +AskUserQuestion: "Proceed with archiving and pruning?" with options: "Yes — archive phases and prune stale branches" | "Cancel" If "Cancel": Stop. @@ -119,6 +140,22 @@ Repeat for all milestones in the cleanup set. + + +After phase archival, prune local branches whose upstream has been deleted. Use the same filter as the dry-run so the execution list matches exactly what the user confirmed: + +```bash +git branch -vv | awk '/: gone\]/ { if ($1 !~ /^\*$|^main$|^next$|^trunk$|^develop$/) print $1 }' | xargs -r git branch -D +``` + +Notes: +- `git fetch --prune` already ran in `show_dry_run` — the tracking refs are current and this step enumerates from the same state the user confirmed. +- `!~ /^\*$/` skips the currently checked-out branch (prefixed with `* ` in `git branch -vv` output, so `$1` yields `*`). +- `!~ /^main$|^next$|^trunk$|^develop$/` excludes protected branch names even if their upstream is gone — matches the dry-run exclusion exactly. +- `xargs -r` prevents `git branch -D` from running with no arguments when no stale branches exist. + + + Commit the changes: @@ -137,6 +174,8 @@ Archived: {For each milestone} - v{X.Y}: {N} phase directories → .planning/milestones/v{X.Y}-phases/ +Pruned: {N} local branches whose upstream is gone. + .planning/phases/ cleaned up. ``` @@ -148,8 +187,9 @@ Archived: - [ ] All completed milestones without existing phase archives identified - [ ] Phase membership determined from archived ROADMAP snapshots -- [ ] Dry-run summary shown and user confirmed +- [ ] Dry-run summary shown and user confirmed (covers both archival and pruning) - [ ] Phase directories moved to `.planning/milestones/v{X.Y}-phases/` +- [ ] Stale local branches pruned (branches whose upstream is gone) - [ ] Changes committed diff --git a/tests/cleanup-branch-pruning.test.cjs b/tests/cleanup-branch-pruning.test.cjs new file mode 100644 index 000000000..5abef4589 --- /dev/null +++ b/tests/cleanup-branch-pruning.test.cjs @@ -0,0 +1,287 @@ +// allow-test-rule: source-text-is-the-product +// Workflow markdown is the installed orchestration contract. + +'use strict'; + +/** + * Cleanup enhancement: branch pruning (#40) + * + * Seam: get-shit-done/workflows/cleanup.md + * + * Verifies that /gsd-cleanup prunes local branches whose upstream is gone, + * integrated between archive_phases and commit steps. + */ + +const { describe, test } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); + +const REPO_ROOT = path.join(__dirname, '..'); +const CLEANUP_PATH = path.join(REPO_ROOT, 'get-shit-done', 'workflows', 'cleanup.md'); + +// ─── Helpers (mirrors worktree-cleanup.test.cjs) ───────────────────────────── + +function extractNamedBlock(markdown, blockName) { + const openStep = ``; + let start = markdown.indexOf(openStep); + if (start !== -1) { + const closeTag = ''; + const end = markdown.indexOf(closeTag, start + openStep.length); + if (end !== -1) return markdown.slice(start + openStep.length, end); + } + const openBare = `<${blockName}>`; + start = markdown.indexOf(openBare); + if (start === -1) return null; + const closeBare = ``; + const end = markdown.indexOf(closeBare, start + openBare.length); + if (end === -1) return null; + return markdown.slice(start + openBare.length, end); +} + +function extractFencedCodeBlocks(markdown) { + const blocks = []; + const lines = markdown.split('\n'); + let inFence = false; + let fenceLang = ''; + let buffer = []; + for (const line of lines) { + const trimmed = line.trimStart(); + if (trimmed.startsWith('```')) { + if (!inFence) { + inFence = true; + fenceLang = trimmed.slice(3).trim(); + buffer = []; + } else { + blocks.push({ lang: fenceLang, body: buffer.join('\n') }); + inFence = false; + fenceLang = ''; + buffer = []; + } + } else if (inFence) { + buffer.push(line); + } + } + return blocks; +} + +function shellStatements(script) { + const statements = []; + const lines = script.split('\n'); + for (let raw of lines) { + const line = raw.replace(/#.*$/, '').trim(); + if (!line) continue; + const parts = line.split(/(?:&&|\|\||;)/); + for (const part of parts) { + let trimmed = part.trim(); + if (!trimmed) continue; + const assignMatch = trimmed.match(/^[A-Za-z_][A-Za-z0-9_]*=(.*)$/); + if (assignMatch) trimmed = assignMatch[1]; + const subMatch = trimmed.match(/^\$\((.*?)\)?$/); + if (subMatch) trimmed = subMatch[1]; + if (trimmed.startsWith('$(')) trimmed = trimmed.slice(2); + trimmed = trimmed.replace(/\)+\s*$/, '').trim(); + if (!trimmed) continue; + statements.push(trimmed.split(/\s+/).filter(Boolean)); + } + } + return statements; +} + +function findCommandIndex(statements, predicate) { + for (let i = 0; i < statements.length; i++) { + if (predicate(statements[i])) return i; + } + return -1; +} + +// ─── #40: prune local branches whose upstream is gone ────────────────────── + +describe('cleanup #40: prune local branches whose upstream is gone', () => { + const content = fs.readFileSync(CLEANUP_PATH, 'utf-8'); + + test('cleanup.md contains a prune_local_branches step', () => { + const block = extractNamedBlock(content, 'prune_local_branches'); + assert.ok(block, 'cleanup.md must contain a step'); + }); + + test('show_dry_run runs `git fetch --prune` so execution list matches what the user confirmed', () => { + const block = extractNamedBlock(content, 'show_dry_run'); + assert.ok(block); + const codeBlocks = extractFencedCodeBlocks(block); + const allStatements = codeBlocks.flatMap(({ body }) => shellStatements(body)); + const idx = findCommandIndex(allStatements, (cmd) => + cmd[0] === 'git' && cmd[1] === 'fetch' && (cmd.includes('--prune') || cmd.includes('-p')) + ); + assert.notStrictEqual( + idx, -1, + 'show_dry_run must run `git fetch --prune` so the candidate list shown to the user ' + + 'is drawn from the same tracking-ref state as the execution step' + ); + }); + + test('prune_local_branches does NOT re-run `git fetch --prune` (fetch already done in show_dry_run)', () => { + const block = extractNamedBlock(content, 'prune_local_branches'); + assert.ok(block); + const codeBlocks = extractFencedCodeBlocks(block); + const allStatements = codeBlocks.flatMap(({ body }) => shellStatements(body)); + const idx = findCommandIndex(allStatements, (cmd) => + cmd[0] === 'git' && cmd[1] === 'fetch' && (cmd.includes('--prune') || cmd.includes('-p')) + ); + assert.strictEqual( + idx, -1, + 'prune_local_branches must not re-run git fetch --prune — the fetch in show_dry_run ' + + 'ensures both steps use the same tracking-ref state, so re-fetching would create a ' + + 'TOCTOU window between what the user confirmed and what gets deleted' + ); + }); + + test('prune_local_branches identifies branches with gone upstream via `git branch -vv`', () => { + const block = extractNamedBlock(content, 'prune_local_branches'); + assert.ok(block); + const codeBlocks = extractFencedCodeBlocks(block); + const allStatements = codeBlocks.flatMap(({ body }) => shellStatements(body)); + const branchVvIdx = findCommandIndex(allStatements, (cmd) => + cmd[0] === 'git' && cmd[1] === 'branch' && (cmd.includes('-vv') || cmd.includes('--verbose')) + ); + assert.notStrictEqual( + branchVvIdx, -1, + 'prune_local_branches must run `git branch -vv` to find branches with gone upstream' + ); + }); + + test('prune_local_branches deletes branches marked `[gone]` using `git branch -D`', () => { + const block = extractNamedBlock(content, 'prune_local_branches'); + assert.ok(block); + const codeBlocks = extractFencedCodeBlocks(block); + const allStatements = codeBlocks.flatMap(({ body }) => shellStatements(body)); + const branchDIdx = findCommandIndex(allStatements, (cmd) => + cmd[0] === 'git' && cmd[1] === 'branch' && (cmd.includes('-D') || cmd.includes('--delete')) + ); + assert.notStrictEqual( + branchDIdx, -1, + 'prune_local_branches must run `git branch -D` to delete stale local branches' + ); + }); + + test('prune_local_branches handles empty result sets (xargs -r safety)', () => { + const block = extractNamedBlock(content, 'prune_local_branches'); + assert.ok(block); + assert.ok( + block.includes('xargs -r') || block.includes('xargs --no-run-if-empty'), + 'prune_local_branches must use `xargs -r` to handle empty branch lists safely' + ); + }); + + test('prune_local_branches appears between archive_phases and commit in ', () => { + const processBlock = extractNamedBlock(content, 'process'); + assert.ok(processBlock); + + const archivePhasesIdx = processBlock.indexOf(''); + const pruneBranchesIdx = processBlock.indexOf(''); + const commitIdx = processBlock.indexOf(''); + + assert.ok(archivePhasesIdx > -1, 'archive_phases step must exist'); + assert.ok(commitIdx > -1, 'commit step must exist'); + assert.notStrictEqual(pruneBranchesIdx, -1, 'prune_local_branches step must exist'); + assert.ok( + archivePhasesIdx < pruneBranchesIdx && pruneBranchesIdx < commitIdx, + 'prune_local_branches must appear between archive_phases and commit steps' + ); + }); + + test('dry-run output in show_dry_run mentions stale branch detection', () => { + const block = extractNamedBlock(content, 'show_dry_run'); + assert.ok(block); + // Must explicitly enumerate stale branches, not just mention the word "branch" + assert.ok( + block.includes(': gone') || block.includes('gone]') || block.includes('upstream is gone'), + 'show_dry_run must mention gone-upstream branches in the dry-run summary' + ); + }); + + test('confirmation prompt in show_dry_run covers both archiving and pruning', () => { + const block = extractNamedBlock(content, 'show_dry_run'); + assert.ok(block); + // The AskUserQuestion prompt must cover the combined action. + assert.ok( + block.includes('archive') && (block.includes('prune') || block.includes('branch')), + 'confirmation prompt must cover both phase archival and branch pruning' + ); + }); + + test('report step includes pruned-branch count', () => { + const block = extractNamedBlock(content, 'report'); + assert.ok(block); + assert.ok( + block.includes('Pruned') || block.includes('pruned'), + 'report step must include pruned branch count in the final summary' + ); + }); + + test('prune_local_branches awk pattern explicitly excludes the current branch (HEAD)', () => { + const block = extractNamedBlock(content, 'prune_local_branches'); + assert.ok(block); + // In `git branch -vv` output, the current branch is prefixed with `* `. + // awk '{print $1}' on the current branch yields `*`, NOT the branch name. + // The pipeline must explicitly exclude the `*` marker so that + // `git branch -D` is never passed `*` as a literal argument. + const hasExplicitHeadGuard = ( + block.includes('$1 != "*"') || // awk field comparison + block.includes('$1!="*"') || + block.includes('$1 !~') || // awk regex non-match (covers * and protected names) + block.includes('!/^\\*/') || // awk negation pattern + block.includes("!/^\\*") || + block.includes('--format') || // git branch --format skips * entirely + (block.includes('sed') && block.includes('\\*')) + ); + assert.ok( + hasExplicitHeadGuard, + 'prune_local_branches awk must explicitly exclude the current branch marker (`*`) ' + + 'using $1 != "*", $1 !~ /regex/, or equivalent, to prevent passing literal `*` to `git branch -D`' + ); + }); + + test('identify_completed_milestones does NOT run git branch -vv (belongs in show_dry_run)', () => { + const block = extractNamedBlock(content, 'identify_completed_milestones'); + assert.ok(block); + // Branch detection is a dry-run concern, not milestone identification. + // Including it here runs the command twice and splits responsibilities. + const codeBlocks = extractFencedCodeBlocks(block); + const allStatements = codeBlocks.flatMap(({ body }) => shellStatements(body)); + const branchVvIdx = findCommandIndex(allStatements, (cmd) => + cmd[0] === 'git' && cmd[1] === 'branch' + ); + assert.strictEqual( + branchVvIdx, -1, + 'identify_completed_milestones must not run git branch commands — ' + + 'branch detection belongs in show_dry_run where dry-run output is assembled' + ); + }); + + test('prune_local_branches awk filter excludes protected branch names (main, next, trunk, develop)', () => { + const block = extractNamedBlock(content, 'prune_local_branches'); + assert.ok(block); + // Protected branch names must be excluded even if their upstream is gone. + // Accept double-quoted "main", single-quoted 'main', or regex anchor ^main$. + assert.ok( + block.includes('"main"') || block.includes("'main'") || block.includes('^main$') || block.includes('^main|'), + 'prune_local_branches awk must exclude "main" from deletion candidates' + ); + assert.ok( + block.includes('"next"') || block.includes("'next'") || block.includes('^next$') || block.includes('^next|') || + block.includes('"trunk"') || block.includes("'trunk'") || block.includes('^trunk$') || block.includes('^trunk|'), + 'prune_local_branches awk must exclude integration branch names (next or trunk)' + ); + }); + + test('no breaking changes: existing archive_phases step is untouched', () => { + const block = extractNamedBlock(content, 'archive_phases'); + assert.ok(block, 'original archive_phases step must still exist'); + assert.ok(block.includes('mv'), 'archive_phases must still move phase directories'); + assert.ok( + block.includes('.planning/phases/') || block.includes('phases/'), + 'archive_phases must reference .planning/phases/' + ); + }); +});