From b88d6d6ef16bd5e6ce741704da25e6100def1a8d Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Mon, 1 Jun 2026 18:08:10 -0400 Subject: [PATCH] refactor(#588): consolidate duplicated worktree_branch_check into one canonical fragment (#589) Extracts the fail-closed worktree branch-check guard into a single canonical fragment (get-shit-done/references/worktree-branch-check.md) and repoints all five sites at it; orchestrator embeds the runnable block at dispatch. All safety invariants preserved; adversarially reviewed; full matrix green. Closes #588. --- .changeset/steady-geese-hum.md | 5 + docs/INVENTORY-MANIFEST.json | 1 + docs/INVENTORY.md | 5 +- .../references/worktree-branch-check.md | 35 +++++ .../references/worktree-path-safety.md | 30 +--- get-shit-done/workflows/diagnose-issues.md | 4 +- get-shit-done/workflows/execute-phase.md | 24 +--- get-shit-done/workflows/execute-plan.md | 2 +- get-shit-done/workflows/quick.md | 26 +--- tests/bug-3384-secondary-defects.test.cjs | 25 +++- tests/worktree-cleanup.test.cjs | 60 +++++--- tests/worktree.test.cjs | 131 ++++++++++++++---- 12 files changed, 222 insertions(+), 126 deletions(-) create mode 100644 .changeset/steady-geese-hum.md create mode 100644 get-shit-done/references/worktree-branch-check.md diff --git a/.changeset/steady-geese-hum.md b/.changeset/steady-geese-hum.md new file mode 100644 index 000000000..299f77cea --- /dev/null +++ b/.changeset/steady-geese-hum.md @@ -0,0 +1,5 @@ +--- +type: Changed +pr: 589 +--- +Consolidated the worktree_branch_check safety guard into a single canonical fragment (get-shit-done/references/worktree-branch-check.md) shared by all worktree-spawning workflows, replacing five divergent copies. No change to guard behavior. diff --git a/docs/INVENTORY-MANIFEST.json b/docs/INVENTORY-MANIFEST.json index a7d20ea31..d6063864a 100644 --- a/docs/INVENTORY-MANIFEST.json +++ b/docs/INVENTORY-MANIFEST.json @@ -257,6 +257,7 @@ "verification-patterns.md", "verify-mvp-mode.md", "workstream-flag.md", + "worktree-branch-check.md", "worktree-path-safety.md" ], "cli_modules": [ diff --git a/docs/INVENTORY.md b/docs/INVENTORY.md index 6b849034b..56870fd3d 100644 --- a/docs/INVENTORY.md +++ b/docs/INVENTORY.md @@ -262,7 +262,7 @@ Full roster at `get-shit-done/workflows/*.md`. Workflows are thin orchestrators --- -## References (62 shipped) +## References (63 shipped) Full roster at `get-shit-done/references/*.md`. References are shared knowledge documents that workflows and agents `@-reference`. The groupings below match [`docs/ARCHITECTURE.md`](ARCHITECTURE.md#references-get-shit-donereferencesmd) — core, workflow, thinking-model clusters, and the modular planner decomposition. @@ -299,6 +299,7 @@ Full roster at `get-shit-done/references/*.md`. References are shared knowledge | `scout-codebase.md` | Phase-type→codebase-map selection table for discuss-phase scout step (extracted via #2551). | | `revision-loop.md` | Plan revision iteration patterns. | | `universal-anti-patterns.md` | Universal anti-patterns to detect and avoid. | +| `worktree-branch-check.md` | Canonical spawn-time worktree HEAD/base guard (worktree_branch_check): per-agent-branch assertion, protected-ref refusal (#2924), reset --hard base correction (#2015) — embedded into worktree sub-agent prompts at dispatch. | | `worktree-path-safety.md` | Worktree guard suite: HEAD assertion, cwd-drift sentinel (step 0a, #3097), and absolute-path guard (step 0b, #3099) — loaded into executor spawn prompts via ``. | | `artifact-types.md` | Planning artifact type definitions. | | `phase-argument-parsing.md` | Phase argument parsing conventions. | @@ -359,7 +360,7 @@ The `gsd-planner` agent is decomposed into a core agent plus reference modules t | `user-story-template.md` | User story format for MVP planning — "As a / I want to / So that" structured fields. | | `spidr-splitting.md` | SPIDR splitting decomposition rules for handling large user stories in MVP mode. | -> **Subdirectory:** `get-shit-done/references/few-shot-examples/` contains additional few-shot examples (`plan-checker.md`, `verifier.md`) that are referenced from specific agents. These are not counted in the 62 top-level references. +> **Subdirectory:** `get-shit-done/references/few-shot-examples/` contains additional few-shot examples (`plan-checker.md`, `verifier.md`) that are referenced from specific agents. These are not counted in the 63 top-level references. --- diff --git a/get-shit-done/references/worktree-branch-check.md b/get-shit-done/references/worktree-branch-check.md new file mode 100644 index 000000000..c21b9ee5d --- /dev/null +++ b/get-shit-done/references/worktree-branch-check.md @@ -0,0 +1,35 @@ +# Worktree branch check (spawn-time guard) + +Canonical, fail-closed guard embedded into every worktree sub-agent prompt at dispatch. +This is the single source of truth for the `worktree_branch_check` block — do not inline +a copy elsewhere. History of coordinated edits: #2924, #2015, #3174, #48. + +**Contract for orchestrators:** before dispatch, capture `EXPECTED_BASE=$(git rev-parse HEAD)`, +then embed the block below into the sub-agent prompt verbatim, substituting `{EXPECTED_BASE}` +with that captured SHA. + + +FIRST ACTION: HEAD assertion MUST run before any reset/checkout. Worktrees +spawned by Claude Code's `isolation="worktree"` use the `worktree-agent-` +namespace. If HEAD is on a protected ref (main/master/develop/trunk/release/*) +or detached, HALT — do NOT self-recover by force-rewinding via `git update-ref`, +that destroys concurrent commits in multi-active scenarios (#2924). Only after +the HEAD assertion passes is `git reset --hard` safe (#2015 — affects all platforms). +```bash +HEAD_REF=$(git symbolic-ref --quiet HEAD || echo "DETACHED") +ACTUAL_BRANCH=$(git rev-parse --abbrev-ref HEAD) +if [ "$HEAD_REF" = "DETACHED" ] || echo "$ACTUAL_BRANCH" | grep -Eq '^(main|master|develop|trunk|release/.*)$'; then + echo "FATAL: worktree HEAD on '$ACTUAL_BRANCH' (expected worktree-agent-*); refusing to self-recover via 'git update-ref' (#2924)." >&2 + exit 1 +fi +if ! echo "$ACTUAL_BRANCH" | grep -Eq '^worktree-agent-[A-Za-z0-9._/-]+$'; then + echo "FATAL: worktree HEAD '$ACTUAL_BRANCH' is not in the worktree-agent-* namespace; refusing to commit (#2924)." >&2 + exit 1 +fi +ACTUAL_BASE=$(git merge-base HEAD {EXPECTED_BASE}) +if [ "$ACTUAL_BASE" != "{EXPECTED_BASE}" ]; then + git reset --hard {EXPECTED_BASE} + [ "$(git rev-parse HEAD)" != "{EXPECTED_BASE}" ] && { echo "ERROR: could not correct worktree base"; exit 1; } +fi +``` + diff --git a/get-shit-done/references/worktree-path-safety.md b/get-shit-done/references/worktree-path-safety.md index 8febe9629..a67279064 100644 --- a/get-shit-done/references/worktree-path-safety.md +++ b/get-shit-done/references/worktree-path-safety.md @@ -7,32 +7,10 @@ must run before any staging, Edit, or Write operation in worktree mode. ## Worktree branch check (run once at spawn-time) -FIRST ACTION: HEAD assertion MUST run before any reset/checkout. Worktrees -spawned by Claude Code's `isolation="worktree"` use the `worktree-agent-` -namespace. If HEAD is on a protected ref (main/master/develop/trunk/release/*) -or detached, HALT — do NOT self-recover by force-rewinding via `git update-ref`, -that destroys concurrent commits in multi-active scenarios (#2924). Only after -this passes is `git reset --hard` safe (#2015 — affects all platforms). - -```bash -HEAD_REF=$(git symbolic-ref --quiet HEAD || echo "DETACHED") -ACTUAL_BRANCH=$(git rev-parse --abbrev-ref HEAD) -if [ "$HEAD_REF" = "DETACHED" ] || echo "$ACTUAL_BRANCH" | grep -Eq '^(main|master|develop|trunk|release/.*)$'; then - echo "FATAL: worktree HEAD on '$ACTUAL_BRANCH' (expected worktree-agent-*); refusing to self-recover via 'git update-ref' (#2924)." >&2 - exit 1 -fi -if ! echo "$ACTUAL_BRANCH" | grep -Eq '^worktree-agent-[A-Za-z0-9._/-]+$'; then - echo "FATAL: worktree HEAD '$ACTUAL_BRANCH' is not in the worktree-agent-* namespace; refusing to commit (#2924)." >&2 - exit 1 -fi -ACTUAL_BASE=$(git merge-base HEAD {EXPECTED_BASE}) -if [ "$ACTUAL_BASE" != "{EXPECTED_BASE}" ]; then - git reset --hard {EXPECTED_BASE} - [ "$(git rev-parse HEAD)" != "{EXPECTED_BASE}" ] && { echo "ERROR: could not correct worktree base"; exit 1; } -fi -``` - -Per-commit HEAD assertion: `agents/gsd-executor.md` `` step 0. +The spawn-time HEAD/base guard now lives in the canonical fragment +`get-shit-done/references/worktree-branch-check.md`, which the orchestrator embeds directly +into your prompt at dispatch. Run that block FIRST, before any reset/checkout or staging. +If your prompt contains a `` embed instruction rather than the block itself, complete that read-and-embed step before any reset/checkout or staging. --- diff --git a/get-shit-done/workflows/diagnose-issues.md b/get-shit-done/workflows/diagnose-issues.md index 06a19ca05..bb5b961fb 100644 --- a/get-shit-done/workflows/diagnose-issues.md +++ b/get-shit-done/workflows/diagnose-issues.md @@ -98,9 +98,11 @@ For each gap, fill the debug-subagent-prompt template and spawn: Print: `◆ Spawning diagnostics agent... (each runs in a subagent — no output until they return, ~1–5 min; expected, not a freeze)` +Before spawning, materialize the guard into WORKTREE_GUARD: read `get-shit-done/references/worktree-branch-check.md`, substitute `{EXPECTED_BASE}` with `$EXPECTED_BASE`, and use the resulting `` block (the runnable guard) as WORKTREE_GUARD below. + ``` Agent( - prompt=filled_debug_subagent_prompt + "\n\n\nFIRST ACTION: assert this is a disposable worktree branch before any repair. Run:\n```bash\nHEAD_REF=$(git symbolic-ref --quiet HEAD || echo \"DETACHED\")\nACTUAL_BRANCH=$(git rev-parse --abbrev-ref HEAD)\nif [ \"$HEAD_REF\" = \"DETACHED\" ] || echo \"$ACTUAL_BRANCH\" | grep -Eq '^(main|master|develop|trunk|release/.*)$'; then\n echo \"FATAL: diagnose worktree HEAD on '$ACTUAL_BRANCH'; refusing reset --hard on a protected branch.\" >&2\n exit 1\nfi\nif ! echo \"$ACTUAL_BRANCH\" | grep -Eq '^worktree-agent-[A-Za-z0-9._/-]+$'; then\n echo \"FATAL: diagnose worktree HEAD '$ACTUAL_BRANCH' is not in the worktree-agent-* namespace; refusing reset --hard.\" >&2\n exit 1\nfi\nACTUAL_BASE=$(git merge-base HEAD {EXPECTED_BASE})\nif [ \"$ACTUAL_BASE\" != \"{EXPECTED_BASE}\" ]; then\n git reset --hard {EXPECTED_BASE}\n [ \"$(git rev-parse HEAD)\" != \"{EXPECTED_BASE}\" ] && { echo \"ERROR: Could not correct worktree base\"; exit 1; }\nfi\n```\nFixes EnterWorktree creating branches from main on all platforms while preventing protected-branch data loss.\n\n\n\n- {phase_dir}/{phase_num}-UAT.md\n- .planning/STATE.md\n\n${AGENT_SKILLS_DEBUGGER}", + prompt=filled_debug_subagent_prompt + "\n\n" + WORKTREE_GUARD + "\n\n\n- {phase_dir}/{phase_num}-UAT.md\n- .planning/STATE.md\n\n${AGENT_SKILLS_DEBUGGER}", subagent_type="gsd-debugger", ${USE_WORKTREES !== "false" ? 'isolation="worktree",' : ''} description="Debug: {truth_short}" diff --git a/get-shit-done/workflows/execute-phase.md b/get-shit-done/workflows/execute-phase.md index 67810c036..24c6932fe 100644 --- a/get-shit-done/workflows/execute-phase.md +++ b/get-shit-done/workflows/execute-phase.md @@ -556,29 +556,7 @@ increases monotonically across waves. `{status}` is `complete` (success), - FIRST ACTION: HEAD assertion MUST run before any reset/checkout. Worktrees - spawned by Claude Code's `isolation="worktree"` use the `worktree-agent-` - namespace. If HEAD is on a protected ref (main/master/develop/trunk/release/*) - or detached, HALT — do NOT self-recover by force-rewinding via `git update-ref`, - that destroys concurrent commits in multi-active scenarios (#2924). Only after - Step 1 passes is `git reset --hard` safe (#2015 — affects all platforms). - ```bash - HEAD_REF=$(git symbolic-ref --quiet HEAD || echo "DETACHED") - ACTUAL_BRANCH=$(git rev-parse --abbrev-ref HEAD) - if [ "$HEAD_REF" = "DETACHED" ] || echo "$ACTUAL_BRANCH" | grep -Eq '^(main|master|develop|trunk|release/.*)$'; then - echo "FATAL: worktree HEAD on '$ACTUAL_BRANCH' (expected worktree-agent-*); refusing to self-recover via 'git update-ref' (#2924)." >&2 - exit 1 - fi - if ! echo "$ACTUAL_BRANCH" | grep -Eq '^worktree-agent-[A-Za-z0-9._/-]+$'; then - echo "FATAL: worktree HEAD '$ACTUAL_BRANCH' is not in the worktree-agent-* namespace; refusing to commit (#2924)." >&2 - exit 1 - fi - ACTUAL_BASE=$(git merge-base HEAD {EXPECTED_BASE}) - if [ "$ACTUAL_BASE" != "{EXPECTED_BASE}" ]; then - git reset --hard {EXPECTED_BASE} - [ "$(git rev-parse HEAD)" != "{EXPECTED_BASE}" ] && { echo "ERROR: could not correct worktree base"; exit 1; } - fi - ``` + ORCHESTRATOR build-time embed (NOT a sub-agent runtime step): before this dispatch, read `get-shit-done/references/worktree-branch-check.md`, substitute `{EXPECTED_BASE}` with the base SHA captured above ({EXPECTED_BASE}), and replace this note with that fragment's `` block so the dispatched prompt carries the runnable guard verbatim — do not pass this instruction through in its place. Per-commit HEAD/cwd-drift/path-guard: `agents/gsd-executor.md` steps 0/0a/0b + `references/worktree-path-safety.md` (in ). diff --git a/get-shit-done/workflows/execute-plan.md b/get-shit-done/workflows/execute-plan.md index 704d37a72..247009b89 100644 --- a/get-shit-done/workflows/execute-plan.md +++ b/get-shit-done/workflows/execute-plan.md @@ -92,7 +92,7 @@ Otherwise: Apply checkpoint-based routing below. | Verify-only | B (segmented) | Segments between checkpoints. After none/human-verify → SUBAGENT. After decision/human-action → MAIN | | Decision | C (main) | Execute entirely in main context | -**Pattern A:** init_agent_tracking → capture `EXPECTED_BASE=$(git rev-parse HEAD)` → print `Spawning executor agent (runs in a subagent — no output until it returns, ~1–5 min; expected, not a freeze)` → spawn Agent(subagent_type="gsd-executor", model=executor_model) with prompt: execute plan at [path], autonomous, all tasks + SUMMARY + commit, follow deviation/auth rules, report: plan name, tasks, SUMMARY path, commit hash → track agent_id → wait → update tracking → report. **Include `isolation="worktree"` only if `workflow.use_worktrees` is not `false`** (read via `config-get workflow.use_worktrees`). **When using `isolation="worktree"`, include a `` block in the prompt** instructing the executor to: (1) FIRST assert `git symbolic-ref HEAD` resolves to a per-agent branch (NOT a protected ref like `main`/`master`/`develop`/`trunk`/`release/*`) and HALT with a blocker if not — never self-recover via `git update-ref refs/heads/` (#2924); (2) only after that assertion passes, run `git merge-base HEAD {EXPECTED_BASE}` and, if the result differs from `{EXPECTED_BASE}`, hard-reset the branch with `git reset --hard {EXPECTED_BASE}` before starting work, then verify with `[ "$(git rev-parse HEAD)" != "{EXPECTED_BASE}" ] && exit 1`. The HEAD assertion (Step 1) MUST run before any reset/checkout. This corrects a known issue where `EnterWorktree` creates branches from `main` instead of the feature branch HEAD (affects all platforms — #2015) and prevents the destructive HEAD-on-master self-recovery path (#2924). +**Pattern A:** init_agent_tracking → capture `EXPECTED_BASE=$(git rev-parse HEAD)` → print `Spawning executor agent (runs in a subagent — no output until it returns, ~1–5 min; expected, not a freeze)` → spawn Agent(subagent_type="gsd-executor", model=executor_model) with prompt: execute plan at [path], autonomous, all tasks + SUMMARY + commit, follow deviation/auth rules, report: plan name, tasks, SUMMARY path, commit hash → track agent_id → wait → update tracking → report. **Include `isolation="worktree"` only if `workflow.use_worktrees` is not `false`** (read via `config-get workflow.use_worktrees`). **When using `isolation="worktree"`, embed the `` block from `get-shit-done/references/worktree-branch-check.md` into the prompt, substituting `{EXPECTED_BASE}` with the captured base SHA.** That guard asserts a per-agent branch before any reset/checkout, forbids `git update-ref` self-recovery (#2924), and hard-resets to `{EXPECTED_BASE}` to correct EnterWorktree basing branches from main (affects all platforms — #2015). **Pattern B:** Execute segment-by-segment. Autonomous segments: spawn subagent for assigned tasks only (no SUMMARY/commit). Checkpoints: main context. After all segments: aggregate, create SUMMARY, commit. See segment_execution. diff --git a/get-shit-done/workflows/quick.md b/get-shit-done/workflows/quick.md index fae302a55..09c0ead92 100644 --- a/get-shit-done/workflows/quick.md +++ b/get-shit-done/workflows/quick.md @@ -677,31 +677,7 @@ Execute quick task ${quick_id}. ${USE_WORKTREES !== "false" ? ` -FIRST ACTION before any other work: verify this worktree's HEAD is bound to a per-agent -branch and that the branch is based on the correct commit. - -Step 1 — HEAD attachment assertion (MANDATORY, runs before any reset/commit): - HEAD_REF=$(git symbolic-ref --quiet HEAD || echo "DETACHED") - ACTUAL_BRANCH=$(git rev-parse --abbrev-ref HEAD) - if [ "$HEAD_REF" = "DETACHED" ] || echo "$ACTUAL_BRANCH" | grep -Eq '^(main|master|develop|trunk|release/.*)$'; then - echo "FATAL: worktree HEAD is on '$ACTUAL_BRANCH' (expected per-agent branch like worktree-agent-*)." >&2 - echo "Refusing to commit/reset on a protected ref. DO NOT self-recover via 'git update-ref refs/heads/$ACTUAL_BRANCH' — that destroys concurrent work (#2924)." >&2 - echo "Aborting before any commits. Surface as a blocker for human review." >&2 - exit 1 - fi - if ! echo "$ACTUAL_BRANCH" | grep -Eq '^worktree-agent-[A-Za-z0-9._/-]+$'; then - echo "FATAL: worktree HEAD '$ACTUAL_BRANCH' is not in the worktree-agent-* namespace (Claude Code's per-agent worktree branch namespace)." >&2 - echo "Refusing to commit; surface as blocker (#2924)." >&2 - exit 1 - fi - -Step 2 — Base correctness (only after Step 1 passes): - Run: git merge-base HEAD ${EXPECTED_BASE} - If the result differs from ${EXPECTED_BASE}, hard-reset to the correct base (safe — Step 1 confirmed HEAD is on a per-agent branch and the worktree is fresh): - git reset --hard ${EXPECTED_BASE} - Then verify: if [ "$(git rev-parse HEAD)" != "${EXPECTED_BASE}" ]; then echo "ERROR: Could not correct worktree base"; exit 1; fi - -This corrects a known issue where EnterWorktree creates branches from main instead of the feature branch HEAD (#2015) and prevents the destructive HEAD-on-master self-recovery path (#2924). +ORCHESTRATOR build-time embed (NOT a sub-agent runtime step): before this dispatch, read \`get-shit-done/references/worktree-branch-check.md\`, substitute \`{EXPECTED_BASE}\` with the base SHA captured above (${EXPECTED_BASE}), and replace this note with that fragment's \`\` block so the dispatched prompt carries the runnable guard verbatim — do not pass this instruction through in its place. ` : ''} diff --git a/tests/bug-3384-secondary-defects.test.cjs b/tests/bug-3384-secondary-defects.test.cjs index ba36f247d..54f44bb3e 100644 --- a/tests/bug-3384-secondary-defects.test.cjs +++ b/tests/bug-3384-secondary-defects.test.cjs @@ -5,6 +5,7 @@ const fs = require('node:fs'); const path = require('node:path'); const repoRoot = path.resolve(__dirname, '..'); +const WORKTREE_BRANCH_CHECK_FRAGMENT = path.join(repoRoot, 'get-shit-done', 'references', 'worktree-branch-check.md'); function read(relPath) { return fs.readFileSync(path.join(repoRoot, relPath), 'utf8'); @@ -22,14 +23,24 @@ describe('bug #3384: adjacent worktree data-loss guards', () => { }); test('diagnose-issues agents assert disposable worktree branch before reset --hard', () => { - const source = read('get-shit-done/workflows/diagnose-issues.md'); - const branchCheck = source.indexOf('HEAD_REF=$(git symbolic-ref --quiet HEAD || echo'); - const namespaceCheck = source.indexOf('worktree-agent-* namespace'); - const reset = source.indexOf('git reset --hard {EXPECTED_BASE}'); + // diagnose-issues.md now references the canonical fragment rather than + // inlining the block. Verify (a) it references the fragment and (b) the + // fragment itself has the correct ordering: symbolic-ref/HEAD assertion and + // ^worktree-agent- allow-list appear BEFORE git reset --hard. + const diagnoseSource = read('get-shit-done/workflows/diagnose-issues.md'); + assert.ok( + diagnoseSource.includes('worktree-branch-check.md'), + 'diagnose-issues.md must reference the canonical worktree-branch-check.md fragment' + ); - assert.ok(branchCheck > 0, 'diagnose prompt must assert HEAD before repair'); - assert.ok(namespaceCheck > branchCheck, 'diagnose prompt must require disposable worktree-agent branch'); - assert.ok(reset > namespaceCheck, 'reset --hard must come only after branch namespace check'); + const fragmentSource = fs.readFileSync(WORKTREE_BRANCH_CHECK_FRAGMENT, 'utf8'); + const branchCheck = fragmentSource.indexOf('HEAD_REF=$(git symbolic-ref --quiet HEAD || echo'); + const namespaceCheck = fragmentSource.indexOf('^worktree-agent-'); + const reset = fragmentSource.indexOf('git reset --hard {EXPECTED_BASE}'); + + assert.ok(branchCheck > 0, 'canonical fragment must assert HEAD before repair'); + assert.ok(namespaceCheck > branchCheck, 'canonical fragment must require disposable worktree-agent branch before reset'); + assert.ok(reset > namespaceCheck, 'reset --hard must come only after branch namespace check in canonical fragment'); }); test('remove-workspace fails closed when git worktree remove fails', () => { diff --git a/tests/worktree-cleanup.test.cjs b/tests/worktree-cleanup.test.cjs index b767dd6ba..93c89482e 100644 --- a/tests/worktree-cleanup.test.cjs +++ b/tests/worktree-cleanup.test.cjs @@ -33,6 +33,7 @@ const EXECUTE_PLAN_PATH = path.join(REPO_ROOT, 'get-shit-done', 'workflows', 'ex const QUICK_PATH = path.join(REPO_ROOT, 'get-shit-done', 'workflows', 'quick.md'); const EXECUTOR_AGENT_PATH = path.join(REPO_ROOT, 'agents', 'gsd-executor.md'); const GIT_INTEGRATION_PATH = path.join(REPO_ROOT, 'get-shit-done', 'references', 'git-integration.md'); +const WORKTREE_BRANCH_CHECK_FRAGMENT = path.join(REPO_ROOT, 'get-shit-done', 'references', 'worktree-branch-check.md'); // ─── Helpers ────────────────────────────────────────────────────────────────── @@ -118,11 +119,19 @@ function findCommandIndex(statements, predicate) { describe('bug #2924: worktree HEAD attachment + destructive recovery', () => { describe('execute-phase.md worktree_branch_check', () => { - const content = fs.readFileSync(EXECUTE_PHASE_PATH, 'utf-8'); - const block = extractNamedBlock(content, 'worktree_branch_check'); + const executePhaseContent = fs.readFileSync(EXECUTE_PHASE_PATH, 'utf-8'); + const fragmentContent = fs.readFileSync(WORKTREE_BRANCH_CHECK_FRAGMENT, 'utf-8'); + const block = extractNamedBlock(fragmentContent, 'worktree_branch_check'); - test('block exists', () => { - assert.ok(block, 'execute-phase.md must contain a block'); + test('execute-phase.md references the canonical fragment', () => { + assert.ok( + executePhaseContent.includes('worktree-branch-check.md'), + 'execute-phase.md must reference the canonical worktree-branch-check.md fragment' + ); + }); + + test('block exists in canonical fragment', () => { + assert.ok(block, 'worktree-branch-check.md must contain a block'); }); test('block invokes `git symbolic-ref` to inspect HEAD attachment', () => { @@ -269,18 +278,26 @@ describe('bug #2924: worktree HEAD attachment + destructive recovery', () => { }); describe('quick.md worktree_branch_check', () => { - const content = fs.readFileSync(QUICK_PATH, 'utf-8'); - const block = extractNamedBlock(content, 'worktree_branch_check'); + const quickContent = fs.readFileSync(QUICK_PATH, 'utf-8'); + const fragmentContent = fs.readFileSync(WORKTREE_BRANCH_CHECK_FRAGMENT, 'utf-8'); + const block = extractNamedBlock(fragmentContent, 'worktree_branch_check'); - test('block exists', () => { - assert.ok(block, 'quick.md must contain a block'); + test('quick.md references the canonical fragment', () => { + assert.ok( + quickContent.includes('worktree-branch-check.md'), + 'quick.md must reference the canonical worktree-branch-check.md fragment' + ); + }); + + test('block exists in canonical fragment', () => { + assert.ok(block, 'worktree-branch-check.md must contain a block'); }); test('block references `git symbolic-ref` for HEAD attachment assertion', () => { - // quick.md uses inline `git symbolic-ref ... HEAD` rather than a fenced - // block, so search the block as a token stream of statements. - const statements = shellStatements(block); - const idx = findCommandIndex(statements, (cmd) => + // Search the block from the canonical fragment as a token stream of statements. + const codeBlocks = extractFencedCodeBlocks(block); + const allStatements = codeBlocks.flatMap(({ body }) => shellStatements(body)); + const idx = findCommandIndex(allStatements, (cmd) => cmd[0] === 'git' && cmd[1] === 'symbolic-ref' && cmd.includes('HEAD') ); assert.notStrictEqual( @@ -290,12 +307,20 @@ describe('bug #2924: worktree HEAD attachment + destructive recovery', () => { }); test('HEAD assertion precedes `git reset --hard`', () => { - const symbolicRefByteIdx = block.indexOf('symbolic-ref'); - const resetHardByteIdx = block.indexOf('reset --hard'); - assert.notStrictEqual(symbolicRefByteIdx, -1); - assert.notStrictEqual(resetHardByteIdx, -1); + // Use shell-statement ordering on the fenced code block to avoid false matches + // from the preamble text that mentions `git reset --hard` for context. + const codeBlocks = extractFencedCodeBlocks(block); + const allStatements = codeBlocks.flatMap(({ body }) => shellStatements(body)); + const symbolicRefIdx = findCommandIndex(allStatements, (cmd) => + cmd[0] === 'git' && cmd[1] === 'symbolic-ref' && cmd.includes('HEAD') + ); + const resetHardIdx = findCommandIndex(allStatements, (cmd) => + cmd[0] === 'git' && cmd[1] === 'reset' && cmd.includes('--hard') + ); + assert.notStrictEqual(symbolicRefIdx, -1, 'symbolic-ref check must exist'); + assert.notStrictEqual(resetHardIdx, -1, 'reset --hard must exist'); assert.ok( - symbolicRefByteIdx < resetHardByteIdx, + symbolicRefIdx < resetHardIdx, 'symbolic-ref HEAD assertion must appear before `git reset --hard` in quick.md worktree_branch_check' ); }); @@ -582,6 +607,7 @@ describe('worktree commit safety hardening (#1977)', () => { test('execute-plan worktree_branch_check has no Windows-only platform qualifier', () => { const content = fs.readFileSync(EXECUTE_PLAN_PATH, 'utf-8'); assert.ok(content.includes('worktree_branch_check'), 'execute-plan.md must contain a worktree_branch_check block'); + assert.ok(content.includes('worktree-branch-check.md'), 'execute-plan.md must reference the canonical worktree-branch-check.md fragment'); const hasWindowsOnlyQualifier = ( /Windows.only/i.test(content) || /affects Windows only/i.test(content) || diff --git a/tests/worktree.test.cjs b/tests/worktree.test.cjs index 92c2b5282..3035ffd71 100644 --- a/tests/worktree.test.cjs +++ b/tests/worktree.test.cjs @@ -35,6 +35,7 @@ const QUICK_PATH = path.join(REPO_ROOT, 'get-shit-done', 'workflows', 'quick.md' const EXECUTOR_AGENT_PATH = path.join(REPO_ROOT, 'agents', 'gsd-executor.md'); const DIAGNOSE_PATH = path.join(REPO_ROOT, 'get-shit-done', 'workflows', 'diagnose-issues.md'); const GIT_INTEGRATION_PATH = path.join(REPO_ROOT, 'get-shit-done', 'references', 'git-integration.md'); +const WORKTREE_BRANCH_CHECK_FRAGMENT = path.join(REPO_ROOT, 'get-shit-done', 'references', 'worktree-branch-check.md'); const isWindows = process.platform === 'win32'; @@ -127,6 +128,59 @@ function findCommandIndex(statements, predicate) { } +// ─── Canonical fragment: single source of truth ───────────────────────────── + +describe('canonical worktree-branch-check fragment is the single source of truth', () => { + const fragmentContent = fs.readFileSync(WORKTREE_BRANCH_CHECK_FRAGMENT, 'utf-8'); + const blockMatch = fragmentContent.match(/([\s\S]*?)<\/worktree_branch_check>/); + const block = blockMatch ? blockMatch[1] : ''; + + test('fragment file exists and contains a block', () => { + assert.ok(blockMatch, 'worktree-branch-check.md must contain a block'); + }); + + test('fragment block contains reset --hard', () => { + assert.ok(block.includes('reset --hard'), 'fragment block must use reset --hard'); + }); + + test('fragment block does NOT contain reset --soft', () => { + assert.ok(!block.includes('reset --soft'), 'fragment block must not use reset --soft'); + }); + + test('fragment block protected-ref alternation contains main', () => { + assert.ok(/\bmain\b/.test(block), 'fragment protected-ref alternation must include main'); + }); + + test('fragment block protected-ref alternation contains master', () => { + assert.ok(/\bmaster\b/.test(block), 'fragment protected-ref alternation must include master'); + }); + + test('fragment block protected-ref alternation contains develop', () => { + assert.ok(/\bdevelop\b/.test(block), 'fragment protected-ref alternation must include develop'); + }); + + test('fragment block protected-ref alternation contains trunk', () => { + assert.ok(/\btrunk\b/.test(block), 'fragment protected-ref alternation must include trunk'); + }); + + test('fragment block protected-ref alternation contains release', () => { + assert.ok(/\brelease\b/.test(block), 'fragment protected-ref alternation must include release'); + }); + + test('fragment block positive allow-list matches ^worktree-agent- pattern', () => { + const allowListRe = /grep\s+-Eq?\s+'\^worktree-agent-/; + assert.ok(allowListRe.test(block), 'fragment block must enforce a positive allow-list matching ^worktree-agent-'); + }); + + test('fragment block contains update-ref prohibition text', () => { + assert.ok(block.includes('update-ref'), 'fragment block must reference update-ref prohibition'); + }); + + test('fragment block contains merge-base HEAD {EXPECTED_BASE}', () => { + assert.ok(block.includes('merge-base HEAD {EXPECTED_BASE}'), 'fragment block must contain git merge-base HEAD {EXPECTED_BASE}'); + }); +}); + const DISCOVERY_PIPELINE = 'grep "^worktree " | grep "\\.claude/worktrees/agent-" | sed \'s/^worktree //\''; @@ -163,11 +217,11 @@ function makeTempUpstreamRepo(prefix) { describe('worktree_branch_check must use reset --hard not reset --soft (#2015)', () => { test('execute-phase.md worktree_branch_check does not use reset --soft', () => { - const content = fs.readFileSync(EXECUTE_PHASE_PATH, 'utf-8'); + const fragmentContent = fs.readFileSync(WORKTREE_BRANCH_CHECK_FRAGMENT, 'utf-8'); - // Extract the worktree_branch_check block - const blockMatch = content.match(/([\s\S]*?)<\/worktree_branch_check>/); - assert.ok(blockMatch, 'execute-phase.md must contain a block'); + // Extract the worktree_branch_check block from the canonical fragment + const blockMatch = fragmentContent.match(/([\s\S]*?)<\/worktree_branch_check>/); + assert.ok(blockMatch, 'worktree-branch-check.md must contain a block'); const block = blockMatch[1]; assert.ok( @@ -177,9 +231,9 @@ describe('worktree_branch_check must use reset --hard not reset --soft (#2015)', }); test('execute-phase.md worktree_branch_check uses reset --hard for base correction', () => { - const content = fs.readFileSync(EXECUTE_PHASE_PATH, 'utf-8'); - const blockMatch = content.match(/([\s\S]*?)<\/worktree_branch_check>/); - assert.ok(blockMatch, 'execute-phase.md must contain a block'); + const fragmentContent = fs.readFileSync(WORKTREE_BRANCH_CHECK_FRAGMENT, 'utf-8'); + const blockMatch = fragmentContent.match(/([\s\S]*?)<\/worktree_branch_check>/); + assert.ok(blockMatch, 'worktree-branch-check.md must contain a block'); const block = blockMatch[1]; assert.ok( @@ -189,9 +243,9 @@ describe('worktree_branch_check must use reset --hard not reset --soft (#2015)', }); test('quick.md worktree_branch_check does not use reset --soft', () => { - const content = fs.readFileSync(QUICK_PATH, 'utf-8'); - const blockMatch = content.match(/([\s\S]*?)<\/worktree_branch_check>/); - assert.ok(blockMatch, 'quick.md must contain a block'); + const fragmentContent = fs.readFileSync(WORKTREE_BRANCH_CHECK_FRAGMENT, 'utf-8'); + const blockMatch = fragmentContent.match(/([\s\S]*?)<\/worktree_branch_check>/); + assert.ok(blockMatch, 'worktree-branch-check.md must contain a block'); const block = blockMatch[1]; assert.ok( @@ -201,9 +255,9 @@ describe('worktree_branch_check must use reset --hard not reset --soft (#2015)', }); test('quick.md worktree_branch_check uses reset --hard for base correction', () => { - const content = fs.readFileSync(QUICK_PATH, 'utf-8'); - const blockMatch = content.match(/([\s\S]*?)<\/worktree_branch_check>/); - assert.ok(blockMatch, 'quick.md must contain a block'); + const fragmentContent = fs.readFileSync(WORKTREE_BRANCH_CHECK_FRAGMENT, 'utf-8'); + const blockMatch = fragmentContent.match(/([\s\S]*?)<\/worktree_branch_check>/); + assert.ok(blockMatch, 'worktree-branch-check.md must contain a block'); const block = blockMatch[1]; assert.ok( @@ -211,6 +265,22 @@ describe('worktree_branch_check must use reset --hard not reset --soft (#2015)', 'quick.md worktree_branch_check must use reset --hard to correctly reset both HEAD and working tree' ); }); + + test('execute-phase.md references the canonical fragment', () => { + const content = fs.readFileSync(EXECUTE_PHASE_PATH, 'utf-8'); + assert.ok( + content.includes('worktree-branch-check.md'), + 'execute-phase.md must reference the canonical worktree-branch-check.md fragment' + ); + }); + + test('quick.md references the canonical fragment', () => { + const content = fs.readFileSync(QUICK_PATH, 'utf-8'); + assert.ok( + content.includes('worktree-branch-check.md'), + 'quick.md must reference the canonical worktree-branch-check.md fragment' + ); + }); }); // ─── #2075: worktree deletion safeguards ──────────────────────────────────── @@ -271,12 +341,17 @@ describe('bug-2075: worktree deletion safeguards', () => { describe('Failure Mode A: worktree_branch_check audit across all worktree-spawning workflows', () => { test('execute-phase.md has worktree_branch_check block with --hard reset', () => { - const content = fs.readFileSync(EXECUTE_PHASE_PATH, 'utf-8'); + const executePhaseContent = fs.readFileSync(EXECUTE_PHASE_PATH, 'utf-8'); + assert.ok( + executePhaseContent.includes('worktree-branch-check.md'), + 'execute-phase.md must reference the canonical worktree-branch-check.md fragment' + ); - const blockMatch = content.match(/([\s\S]*?)<\/worktree_branch_check>/); + const fragmentContent = fs.readFileSync(WORKTREE_BRANCH_CHECK_FRAGMENT, 'utf-8'); + const blockMatch = fragmentContent.match(/([\s\S]*?)<\/worktree_branch_check>/); assert.ok( blockMatch, - 'execute-phase.md must contain a block' + 'worktree-branch-check.md must contain a block' ); const block = blockMatch[1]; @@ -291,12 +366,17 @@ describe('bug-2075: worktree deletion safeguards', () => { }); test('quick.md has worktree_branch_check block with --hard reset', () => { - const content = fs.readFileSync(QUICK_PATH, 'utf-8'); + const quickContent = fs.readFileSync(QUICK_PATH, 'utf-8'); + assert.ok( + quickContent.includes('worktree-branch-check.md'), + 'quick.md must reference the canonical worktree-branch-check.md fragment' + ); - const blockMatch = content.match(/([\s\S]*?)<\/worktree_branch_check>/); + const fragmentContent = fs.readFileSync(WORKTREE_BRANCH_CHECK_FRAGMENT, 'utf-8'); + const blockMatch = fragmentContent.match(/([\s\S]*?)<\/worktree_branch_check>/); assert.ok( blockMatch, - 'quick.md must contain a block' + 'worktree-branch-check.md must contain a block' ); const block = blockMatch[1]; @@ -311,15 +391,18 @@ describe('bug-2075: worktree deletion safeguards', () => { }); test('diagnose-issues.md has worktree_branch_check instruction for spawned agents', () => { - const content = fs.readFileSync(DIAGNOSE_PATH, 'utf-8'); - + const diagnoseContent = fs.readFileSync(DIAGNOSE_PATH, 'utf-8'); assert.ok( - content.includes('worktree_branch_check'), - 'diagnose-issues.md must include worktree_branch_check instruction for spawned debug agents' + diagnoseContent.includes('worktree-branch-check.md'), + 'diagnose-issues.md must reference the canonical worktree-branch-check.md fragment for spawned debug agents' ); + const fragmentContent = fs.readFileSync(WORKTREE_BRANCH_CHECK_FRAGMENT, 'utf-8'); + const blockMatch = fragmentContent.match(/([\s\S]*?)<\/worktree_branch_check>/); + assert.ok(blockMatch, 'worktree-branch-check.md must contain a block'); + const block = blockMatch[1]; assert.ok( - content.includes('reset --hard'), + block.includes('reset --hard'), 'diagnose-issues.md worktree_branch_check must instruct agents to use git reset --hard' ); });