From 69cc2f9fe24a722af0661d814a1f877f8fddfe9a Mon Sep 17 00:00:00 2001 From: Colin Date: Mon, 25 May 2026 09:58:58 -0400 Subject: [PATCH] fix(issue): bug: worktree executor force-adds gitignored .planning/SUMMARY.md into worktree branch (regression of the skipped_gitignored SDK contract) --- bin/install.js | 14 ++- hooks/gsd-workflow-guard.js | 69 ++++++++++++++- .../bug-261-worktree-force-add-guard.test.cjs | 87 +++++++++++++++++++ tests/workflow-guard-registration.test.cjs | 16 +++- 4 files changed, 177 insertions(+), 9 deletions(-) create mode 100644 tests/bug-261-worktree-force-add-guard.test.cjs diff --git a/bin/install.js b/bin/install.js index 6e55320f7..c740ab4d2 100755 --- a/bin/install.js +++ b/bin/install.js @@ -9566,18 +9566,24 @@ function install(isGlobal, runtime = 'claude', options = {}) { // Configure workflow guard hook (opt-in via hooks.workflow_guard: true) // Detects file edits outside GSD workflow context and advises using - // /gsd-quick or /gsd-fast for state-tracked changes. Advisory only. + // /gsd-quick or /gsd-fast for state-tracked changes. Also hard-blocks + // unsafe Bash commands that violate worktree-agent isolation. const workflowGuardCommand = isGlobal ? buildHookCommand(targetDir, 'gsd-workflow-guard.js', hookOpts) : localCmd('gsd-workflow-guard.js'); - const hasWorkflowGuardHook = settings.hooks[preToolEvent].some(entry => + const workflowGuardMatcher = 'Bash|Edit|Write|MultiEdit'; + const workflowGuardHookEntry = settings.hooks[preToolEvent].find(entry => entry.hooks && entry.hooks.some(h => h.command && h.command.includes('gsd-workflow-guard')) ); + const hasWorkflowGuardHook = Boolean(workflowGuardHookEntry); const workflowGuardFile = path.join(targetDir, 'hooks', 'gsd-workflow-guard.js'); - if (!hasWorkflowGuardHook && fs.existsSync(workflowGuardFile) && workflowGuardCommand) { + if (hasWorkflowGuardHook && workflowGuardHookEntry.matcher !== workflowGuardMatcher) { + workflowGuardHookEntry.matcher = workflowGuardMatcher; + console.log(` ${green}✓${reset} Updated workflow guard hook matcher`); + } else if (!hasWorkflowGuardHook && fs.existsSync(workflowGuardFile) && workflowGuardCommand) { settings.hooks[preToolEvent].push({ - matcher: 'Write|Edit', + matcher: workflowGuardMatcher, hooks: [ { type: 'command', diff --git a/hooks/gsd-workflow-guard.js b/hooks/gsd-workflow-guard.js index 55ba45cf4..55743a46d 100644 --- a/hooks/gsd-workflow-guard.js +++ b/hooks/gsd-workflow-guard.js @@ -13,6 +13,57 @@ const fs = require('fs'); const path = require('path'); +const { spawnSync } = require('child_process'); +const { tokenize } = require('./lib/git-cmd.js'); + +function forceGitAddCwds(command, defaultCwd) { + const tokens = tokenize(command || ''); + const separators = new Set(['&&', '||', ';', '|']); + const cwdList = []; + for (let i = 0; i < tokens.length; i++) { + if (path.basename(tokens[i]) !== 'git') continue; + + let j = i + 1; + let gitCwd = defaultCwd; + while (j < tokens.length) { + const token = tokens[j]; + const flagName = token.includes('=') ? token.slice(0, token.indexOf('=')) : token; + if (token === '-C' && tokens[j + 1]) { + gitCwd = path.resolve(gitCwd, tokens[j + 1]); + j += 2; + continue; + } + if (['-C', '--git-dir', '--work-tree'].includes(flagName) && !token.includes('=')) { + j += 2; + continue; + } + if (['--git-dir', '--work-tree', '--no-pager', '-p', '-P'].includes(flagName)) { + j++; + continue; + } + break; + } + + if (tokens[j] !== 'add') continue; + for (let k = j + 1; k < tokens.length && !separators.has(tokens[k]); k++) { + if (tokens[k] === '--force' || tokens[k] === '-f' || /^-[A-Za-z]*f[A-Za-z]*$/.test(tokens[k])) { + cwdList.push(gitCwd); + break; + } + } + } + return cwdList; +} + +function currentBranch(cwd) { + const result = spawnSync('git', ['branch', '--show-current'], { + cwd, + encoding: 'utf8', + stdio: ['ignore', 'pipe', 'ignore'], + }); + if (result.status !== 0) return ''; + return result.stdout.trim(); +} let input = ''; const stdinTimeout = setTimeout(() => process.exit(0), 3000); @@ -23,6 +74,23 @@ process.stdin.on('end', () => { try { const data = JSON.parse(input); const toolName = data.tool_name; + const cwd = data.cwd || process.cwd(); + + if (toolName === 'Bash') { + const command = data.tool_input?.command || ''; + for (const gitCwd of forceGitAddCwds(command, cwd)) { + const branch = currentBranch(gitCwd); + if (branch.startsWith('worktree-agent-')) { + process.stdout.write(JSON.stringify({ + decision: 'block', + code: 'WORKTREE_AGENT_FORCE_ADD_FORBIDDEN', + reason: 'worktree-agent branches must not run git add -f or git add --force. Respect the SDK skipped_gitignored/skipped_commit_docs_false contract and leave gitignored files untracked.', + })); + process.exit(2); + } + } + process.exit(0); + } // Only guard Write and Edit tool calls if (toolName !== 'Write' && toolName !== 'Edit') { @@ -58,7 +126,6 @@ process.stdin.on('end', () => { } // Check if workflow guard is enabled - const cwd = data.cwd || process.cwd(); const configPath = path.join(cwd, '.planning', 'config.json'); if (fs.existsSync(configPath)) { try { diff --git a/tests/bug-261-worktree-force-add-guard.test.cjs b/tests/bug-261-worktree-force-add-guard.test.cjs new file mode 100644 index 000000000..d40f0585e --- /dev/null +++ b/tests/bug-261-worktree-force-add-guard.test.cjs @@ -0,0 +1,87 @@ +'use strict'; + +const { describe, test } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const os = require('node:os'); +const path = require('node:path'); +const { execFileSync, spawnSync } = require('node:child_process'); + +const HOOK_PATH = path.join(__dirname, '..', 'hooks', 'gsd-workflow-guard.js'); + +function git(cwd, args) { + return execFileSync('git', args, { cwd, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }); +} + +function makeRepo(branch) { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-bug-261-')); + git(dir, ['init', '-q']); + git(dir, ['config', 'user.email', 'test@example.com']); + git(dir, ['config', 'user.name', 'Test User']); + git(dir, ['config', 'commit.gpgsign', 'false']); + fs.writeFileSync(path.join(dir, 'README.md'), '# test\n'); + git(dir, ['add', 'README.md']); + git(dir, ['commit', '-q', '-m', 'chore: init']); + git(dir, ['checkout', '-q', '-b', branch]); + return dir; +} + +function runHook(cwd, command) { + return spawnSync(process.execPath, [HOOK_PATH], { + cwd, + encoding: 'utf8', + input: JSON.stringify({ + cwd, + tool_name: 'Bash', + tool_input: { command }, + }), + }); +} + +describe('bug #261: workflow guard blocks forced git add on worktree-agent branches', () => { + test('blocks git add -f on worktree-agent branch before it can stage gitignored files', () => { + const dir = makeRepo('worktree-agent-a1'); + try { + const result = runHook(dir, 'git add -f .planning/phases/01/01-01-SUMMARY.md'); + assert.strictEqual(result.status, 2); + const envelope = JSON.parse(result.stdout); + assert.strictEqual(envelope.decision, 'block'); + assert.strictEqual(envelope.code, 'WORKTREE_AGENT_FORCE_ADD_FORBIDDEN'); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + test('blocks git add --force with git global options on worktree-agent branch', () => { + const dir = makeRepo('worktree-agent-b2'); + try { + const result = runHook(path.dirname(dir), `git -C "${dir}" add --force .planning/SUMMARY.md`); + assert.strictEqual(result.status, 2); + assert.strictEqual(JSON.parse(result.stdout).code, 'WORKTREE_AGENT_FORCE_ADD_FORBIDDEN'); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + test('allows ordinary git add on worktree-agent branch', () => { + const dir = makeRepo('worktree-agent-c3'); + try { + const result = runHook(dir, 'git add .planning/SUMMARY.md'); + assert.strictEqual(result.status, 0); + assert.strictEqual(result.stdout, ''); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + test('allows git add -f outside worktree-agent branches', () => { + const dir = makeRepo('feature-docs'); + try { + const result = runHook(dir, 'git add -f .planning/SUMMARY.md'); + assert.strictEqual(result.status, 0); + assert.strictEqual(result.stdout, ''); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); +}); diff --git a/tests/workflow-guard-registration.test.cjs b/tests/workflow-guard-registration.test.cjs index b2b11c02c..d5730e68c 100644 --- a/tests/workflow-guard-registration.test.cjs +++ b/tests/workflow-guard-registration.test.cjs @@ -54,16 +54,24 @@ describe('workflow-guard hook registration (#1767)', () => { test('install.js pushes workflow-guard entry with correct matcher', () => { const content = fs.readFileSync(INSTALL_JS, 'utf-8'); - // Extract the section between "workflow-guard" command construction - // and the next console.log confirmation. The push block should have: - // matcher: 'Write|Edit' and command referencing workflow-guard + // Extract the workflow-guard registration section. It should install the + // Bash-aware matcher and upgrade old edit-only entries on reinstall. const workflowGuardSection = content.match( - /workflowGuardCommand[\s\S]*?console\.log\([^)]*workflow.guard/i + /workflowGuardCommand[\s\S]*?Configure commit validation hook/i ); assert.ok( workflowGuardSection, 'install.js must have a push block for workflow-guard with a console.log confirmation' ); + assert.ok( + workflowGuardSection[0].includes("const workflowGuardMatcher = 'Bash|Edit|Write|MultiEdit'") && + workflowGuardSection[0].includes('matcher: workflowGuardMatcher'), + 'workflow guard must be registered for Bash so worktree-agent git safety checks can run' + ); + assert.ok( + workflowGuardSection[0].includes('workflowGuardHookEntry.matcher = workflowGuardMatcher'), + 'installer must upgrade existing workflow guard hook entries to the Bash-aware matcher' + ); }); });