fix(bugs): resolve issues #2388, #2431, #2396, #2376 (#2467)

#2388 (plan-phase silently renames feature branch): add explicit Git
Branch Invariant section to plan-phase.md prohibiting branch
creation/rename/switch during planning; phase slug changes are
plan-level only and must not affect the git branch.

#2431 (worktree teardown silently swallows errors): replace
`git worktree remove --force 2>/dev/null || true` with a lock-aware
block in quick.md and execute-phase.md that detects locked worktrees,
attempts unlock+retry, and surfaces a user-visible recovery message
when removal still fails.

#2396 (hardcoded test commands bypass Makefile): add a three-tier
test command resolver (project config → Makefile/Justfile → language
sniff) in execute-phase.md, verify-phase.md, and audit-fix.md.
Makefile with a `test:` target now takes priority over npm/cargo/go.

#2376 (OpenCode @$HOME not mapped on Windows): add platform guard in
bin/install.js so OpenCode on win32 uses the absolute path instead of
`$HOME/...`, which OpenCode does not expand in @file references on
Windows.

Tests: 29 new assertions across 4 regression test files (all passing).

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

tests/bug-2376-opencode-windows-home-path.test.cjs

@@ -0,0 +1,118 @@
+/**
+ * Regression test for #2376: @$HOME not correctly mapped in OpenCode on Windows.
+ *
+ * On Windows, $HOME is not expanded by PowerShell/cmd.exe, so OpenCode cannot
+ * resolve @$HOME/... file references in installed command files.
+ *
+ * Fix: install.js must use the absolute path (not $HOME-relative) when installing
+ * for OpenCode on Windows.
+ */
+
+'use strict';
+
+const { describe, test } = require('node:test');
+const assert = require('node:assert/strict');
+const fs = require('fs');
+const path = require('path');
+
+const INSTALL_JS_PATH = path.join(__dirname, '..', 'bin', 'install.js');
+
+describe('bug-2376: OpenCode on Windows must use absolute path, not $HOME', () => {
+  test('install.js exists', () => {
+    assert.ok(fs.existsSync(INSTALL_JS_PATH), 'bin/install.js should exist');
+  });
+
+  test('install.js pathPrefix computation skips $HOME for OpenCode on Windows', () => {
+    const content = fs.readFileSync(INSTALL_JS_PATH, 'utf-8');
+
+    // The fix must include a Windows detection condition for OpenCode
+    const hasWindowsOpenCodeGuard = (
+      content.includes('isWindowsHost') ||
+      (content.includes("'win32'") && content.includes('isOpencode') && content.includes('pathPrefix'))
+    );
+    assert.ok(
+      hasWindowsOpenCodeGuard,
+      'install.js must include a Windows platform guard for OpenCode pathPrefix computation'
+    );
+  });
+
+  test('install.js pathPrefix guard excludes $HOME for OpenCode+Windows combination', () => {
+    const content = fs.readFileSync(INSTALL_JS_PATH, 'utf-8');
+
+    // The pathPrefix assignment must include a guard that prevents $HOME substitution
+    // for OpenCode on Windows (process.platform === 'win32').
+    const pathPrefixBlock = content.match(/const pathPrefix[\s\S]{0,500}resolvedTarget/);
+    assert.ok(pathPrefixBlock, 'pathPrefix assignment block should be present');
+
+    const block = pathPrefixBlock[0];
+    const excludesOpenCodeWindows = (
+      block.includes('isWindowsHost') ||
+      (block.includes('isOpencode') && block.includes('win32'))
+    );
+    assert.ok(
+      excludesOpenCodeWindows,
+      'pathPrefix computation must exclude $HOME substitution for OpenCode on Windows'
+    );
+  });
+
+  test('pathPrefix simulation: OpenCode on Windows uses absolute path', () => {
+    // Simulate the fixed pathPrefix computation for Windows+OpenCode
+    const isGlobal = true;
+    const isOpencode = true;
+    const isWindowsHost = true; // simulated Windows
+    const resolvedTarget = 'C:/Users/user/.config/opencode';
+    const homeDir = 'C:/Users/user';
+
+    const pathPrefix = isGlobal && resolvedTarget.startsWith(homeDir) && !(isOpencode && isWindowsHost)
+      ? '$HOME' + resolvedTarget.slice(homeDir.length) + '/'
+      : `${resolvedTarget}/`;
+
+    assert.strictEqual(
+      pathPrefix,
+      'C:/Users/user/.config/opencode/',
+      'OpenCode on Windows should use absolute path, not $HOME-relative'
+    );
+    assert.ok(
+      !pathPrefix.includes('$HOME'),
+      'OpenCode on Windows pathPrefix must not contain $HOME'
+    );
+  });
+
+  test('pathPrefix simulation: OpenCode on Linux/macOS still uses $HOME', () => {
+    // Non-Windows OpenCode should still use $HOME (POSIX shells expand it)
+    const isGlobal = true;
+    const isOpencode = true;
+    const isWindowsHost = false; // simulated Linux/macOS
+    const homeDir = '/home/user';
+    const resolvedTarget = '/home/user/.config/opencode';
+
+    const pathPrefix = isGlobal && resolvedTarget.startsWith(homeDir) && !(isOpencode && isWindowsHost)
+      ? '$HOME' + resolvedTarget.slice(homeDir.length) + '/'
+      : `${resolvedTarget}/`;
+
+    assert.strictEqual(
+      pathPrefix,
+      '$HOME/.config/opencode/',
+      'OpenCode on Linux/macOS should still use $HOME-relative path'
+    );
+  });
+
+  test('pathPrefix simulation: Claude Code on Windows still uses $HOME (unaffected)', () => {
+    // Claude Code on Windows is handled by Claude Code's own shell, which expands $HOME
+    const isGlobal = true;
+    const isOpencode = false; // Claude Code, not OpenCode
+    const isWindowsHost = true;
+    const homeDir = 'C:/Users/user';
+    const resolvedTarget = 'C:/Users/user/.claude';
+
+    const pathPrefix = isGlobal && resolvedTarget.startsWith(homeDir) && !(isOpencode && isWindowsHost)
+      ? '$HOME' + resolvedTarget.slice(homeDir.length) + '/'
+      : `${resolvedTarget}/`;
+
+    assert.strictEqual(
+      pathPrefix,
+      '$HOME/.claude/',
+      'Claude Code on Windows should still use $HOME-relative path (Claude Code handles this)'
+    );
+  });
+});

tests/bug-2388-plan-phase-no-branch-rename.test.cjs

@@ -0,0 +1,61 @@
+/**
+ * Regression test for #2388: plan-phase silently renames feature branch
+ * when phase slug has changed since the branch was created.
+ *
+ * Fix: plan-phase.md must include an explicit instruction not to create,
+ * rename, or switch git branches during the planning workflow.
+ */
+
+'use strict';
+
+const { describe, test } = require('node:test');
+const assert = require('node:assert/strict');
+const fs = require('fs');
+const path = require('path');
+
+const PLAN_PHASE_PATH = path.join(__dirname, '..', 'get-shit-done', 'workflows', 'plan-phase.md');
+
+describe('bug-2388: plan-phase must not rename or create git branches', () => {
+  test('plan-phase.md exists', () => {
+    assert.ok(fs.existsSync(PLAN_PHASE_PATH), 'plan-phase.md should exist');
+  });
+
+  test('plan-phase.md contains explicit no-branch-rename instruction', () => {
+    const content = fs.readFileSync(PLAN_PHASE_PATH, 'utf-8');
+    // Must say "do not" and mention branch in the context of phase slug/rename
+    const hasBranchGuard = (
+      /do not.{0,80}branch/i.test(content) ||
+      /branch.{0,80}do not/i.test(content) ||
+      /NEVER.{0,80}branch/i.test(content) ||
+      /branch.{0,80}NEVER/i.test(content)
+    );
+    assert.ok(
+      hasBranchGuard,
+      'plan-phase.md must include an explicit instruction not to create or rename git branches'
+    );
+  });
+
+  test('plan-phase.md mentions phase rename does not affect branch name', () => {
+    const content = fs.readFileSync(PLAN_PHASE_PATH, 'utf-8');
+    // Should explain that a phase rename in ROADMAP.md is plan-level, not git-level
+    const hasPlanLevelExplanation = (
+      content.includes('phase rename') ||
+      content.includes('phase_slug') ||
+      content.includes('branch identity') ||
+      content.includes('branch name')
+    );
+    assert.ok(
+      hasPlanLevelExplanation,
+      'plan-phase.md should clarify that phase slug changes do not change the git branch'
+    );
+  });
+
+  test('plan-phase.md does not contain git checkout -b instruction', () => {
+    const content = fs.readFileSync(PLAN_PHASE_PATH, 'utf-8');
+    // The workflow should not instruct the LLM to run git checkout -b
+    assert.ok(
+      !content.includes('git checkout -b'),
+      'plan-phase.md must not instruct LLM to create a new branch via git checkout -b'
+    );
+  });
+});

tests/bug-2396-makefile-test-priority.test.cjs

@@ -0,0 +1,112 @@
+/**
+ * Regression test for #2396: hardcoded host-level test commands bypass
+ * container-only project Makefiles.
+ *
+ * Fix: execute-phase.md, verify-phase.md, and audit-fix.md must check for
+ * Makefile with a test target (and other wrappers) before falling through
+ * to hardcoded language-sniffed commands.
+ */
+
+'use strict';
+
+const { describe, test } = require('node:test');
+const assert = require('node:assert/strict');
+const fs = require('fs');
+const path = require('path');
+
+const EXECUTE_PHASE_PATH = path.join(__dirname, '..', 'get-shit-done', 'workflows', 'execute-phase.md');
+const VERIFY_PHASE_PATH = path.join(__dirname, '..', 'get-shit-done', 'workflows', 'verify-phase.md');
+const AUDIT_FIX_PATH = path.join(__dirname, '..', 'get-shit-done', 'workflows', 'audit-fix.md');
+
+function assertMakefileCheckBeforeNpmTest(filePath, label) {
+  const content = fs.readFileSync(filePath, 'utf-8');
+
+  // Must check for Makefile with test target
+  const hasMakefileCheck = /Makefile.*grep.*test:|grep.*test:.*Makefile/s.test(content) ||
+    (content.includes('Makefile') && content.includes('"^test:"'));
+  assert.ok(
+    hasMakefileCheck,
+    `${label}: must check for Makefile with test: target before falling through to hardcoded commands`
+  );
+
+  // make test must appear before npm test in the file
+  const makeTestIdx = content.indexOf('make test');
+  const npmTestIdx = content.indexOf('npm test');
+  assert.ok(makeTestIdx !== -1, `${label}: must contain "make test"`);
+  assert.ok(npmTestIdx !== -1, `${label}: must still contain "npm test" as fallback`);
+  assert.ok(
+    makeTestIdx < npmTestIdx,
+    `${label}: "make test" must appear before "npm test" (Makefile takes priority)`
+  );
+}
+
+function assertConfigGetBeforeMakefile(filePath, label) {
+  const content = fs.readFileSync(filePath, 'utf-8');
+  // Must check workflow.test_command config before Makefile sniff.
+  // Verify within each bash code block: the workflow.test_command lookup
+  // appears before the Makefile grep in the same block.
+  assert.ok(
+    content.includes('workflow.test_command'),
+    `${label}: must check workflow.test_command config before Makefile/language sniff`
+  );
+
+  // Extract bash blocks to check ordering within each block.
+  // Use the actual Makefile test ([ -f "Makefile" ]) not just the word "Makefile"
+  // (which appears in comments before the config-get call).
+  const bashBlockRe = /```bash([\s\S]*?)```/g;
+  let match;
+  let anyBlockCorrectlyOrdered = false;
+  while ((match = bashBlockRe.exec(content)) !== null) {
+    const block = match[1];
+    if (block.includes('workflow.test_command') && block.includes('[ -f "Makefile"')) {
+      const configIdx = block.indexOf('workflow.test_command');
+      const makefileIdx = block.indexOf('[ -f "Makefile"');
+      if (configIdx < makefileIdx) {
+        anyBlockCorrectlyOrdered = true;
+        break;
+      }
+    }
+  }
+  assert.ok(
+    anyBlockCorrectlyOrdered,
+    `${label}: within a bash block, workflow.test_command config check must appear before Makefile test ([ -f "Makefile" ])`
+  );
+}
+
+describe('bug-2396: Makefile test target must take priority over hardcoded commands', () => {
+  test('execute-phase.md exists', () => {
+    assert.ok(fs.existsSync(EXECUTE_PHASE_PATH), 'execute-phase.md should exist');
+  });
+
+  test('verify-phase.md exists', () => {
+    assert.ok(fs.existsSync(VERIFY_PHASE_PATH), 'verify-phase.md should exist');
+  });
+
+  test('audit-fix.md exists', () => {
+    assert.ok(fs.existsSync(AUDIT_FIX_PATH), 'audit-fix.md should exist');
+  });
+
+  test('execute-phase.md: Makefile check precedes npm test (post-merge gate)', () => {
+    assertMakefileCheckBeforeNpmTest(EXECUTE_PHASE_PATH, 'execute-phase.md');
+  });
+
+  test('verify-phase.md: Makefile check precedes npm test', () => {
+    assertMakefileCheckBeforeNpmTest(VERIFY_PHASE_PATH, 'verify-phase.md');
+  });
+
+  test('audit-fix.md: Makefile check precedes npm test', () => {
+    assertMakefileCheckBeforeNpmTest(AUDIT_FIX_PATH, 'audit-fix.md');
+  });
+
+  test('execute-phase.md: workflow.test_command config checked first (within bash block)', () => {
+    assertConfigGetBeforeMakefile(EXECUTE_PHASE_PATH, 'execute-phase.md');
+  });
+
+  test('verify-phase.md: workflow.test_command config checked first (within bash block)', () => {
+    assertConfigGetBeforeMakefile(VERIFY_PHASE_PATH, 'verify-phase.md');
+  });
+
+  test('audit-fix.md: workflow.test_command config checked first (within bash block)', () => {
+    assertConfigGetBeforeMakefile(AUDIT_FIX_PATH, 'audit-fix.md');
+  });
+});

tests/bug-2431-worktree-locked-surfacing.test.cjs

@@ -0,0 +1,98 @@
+/**
+ * Regression test for #2431: quick.md and execute-phase.md worktree teardown
+ * silently accumulates locked worktrees via `2>/dev/null || true`.
+ *
+ * Fix: replace the silent-fail pattern with a lock-aware block that surfaces
+ * the error and provides a user-visible recovery message.
+ */
+
+'use strict';
+
+const { describe, test } = require('node:test');
+const assert = require('node:assert/strict');
+const fs = require('fs');
+const path = require('path');
+
+const QUICK_PATH = path.join(__dirname, '..', 'get-shit-done', 'workflows', 'quick.md');
+const EXECUTE_PHASE_PATH = path.join(__dirname, '..', 'get-shit-done', 'workflows', 'execute-phase.md');
+
+function assertNoSilentWorktreeRemove(filePath, label) {
+  const content = fs.readFileSync(filePath, 'utf-8');
+  // The old pattern: git worktree remove "$WT" --force 2>/dev/null || true
+  const silentRemovePattern = /git worktree remove[^\n]*--force\s+2>\/dev\/null\s*\|\|\s*true/;
+  assert.ok(
+    !silentRemovePattern.test(content),
+    `${label}: must not contain "git worktree remove --force 2>/dev/null || true" (silently swallows errors)`
+  );
+}
+
+function assertHasLockAwareBlock(filePath, label) {
+  const content = fs.readFileSync(filePath, 'utf-8');
+  // Fix must include: lock-aware detection (checking .git/worktrees/*/locked)
+  const hasLockCheck = content.includes('.git/worktrees/') && content.includes('locked');
+  assert.ok(
+    hasLockCheck,
+    `${label}: must include lock-aware detection (.git/worktrees/.../locked check)`
+  );
+}
+
+function assertHasWorktreeUnlock(filePath, label) {
+  const content = fs.readFileSync(filePath, 'utf-8');
+  // Fix must include a git worktree unlock attempt
+  assert.ok(
+    content.includes('git worktree unlock'),
+    `${label}: must include "git worktree unlock" retry attempt`
+  );
+}
+
+function assertHasUserVisibleWarning(filePath, label) {
+  const content = fs.readFileSync(filePath, 'utf-8');
+  // Fix must print a user-visible warning on residual worktree failure
+  const hasWarning = content.includes('Residual worktree') || content.includes('manual cleanup');
+  assert.ok(
+    hasWarning,
+    `${label}: must include user-visible warning when worktree removal fails`
+  );
+}
+
+describe('bug-2431: worktree teardown must surface locked-worktree errors', () => {
+  test('quick.md exists', () => {
+    assert.ok(fs.existsSync(QUICK_PATH), 'quick.md should exist');
+  });
+
+  test('execute-phase.md exists', () => {
+    assert.ok(fs.existsSync(EXECUTE_PHASE_PATH), 'execute-phase.md should exist');
+  });
+
+  test('quick.md: no silent worktree remove pattern', () => {
+    assertNoSilentWorktreeRemove(QUICK_PATH, 'quick.md');
+  });
+
+  test('execute-phase.md: no silent worktree remove pattern', () => {
+    assertNoSilentWorktreeRemove(EXECUTE_PHASE_PATH, 'execute-phase.md');
+  });
+
+  test('quick.md: has lock-aware detection block', () => {
+    assertHasLockAwareBlock(QUICK_PATH, 'quick.md');
+  });
+
+  test('execute-phase.md: has lock-aware detection block', () => {
+    assertHasLockAwareBlock(EXECUTE_PHASE_PATH, 'execute-phase.md');
+  });
+
+  test('quick.md: has git worktree unlock retry', () => {
+    assertHasWorktreeUnlock(QUICK_PATH, 'quick.md');
+  });
+
+  test('execute-phase.md: has git worktree unlock retry', () => {
+    assertHasWorktreeUnlock(EXECUTE_PHASE_PATH, 'execute-phase.md');
+  });
+
+  test('quick.md: has user-visible warning on residual worktree', () => {
+    assertHasUserVisibleWarning(QUICK_PATH, 'quick.md');
+  });
+
+  test('execute-phase.md: has user-visible warning on residual worktree', () => {
+    assertHasUserVisibleWarning(EXECUTE_PHASE_PATH, 'execute-phase.md');
+  });
+});