From 3143edaa3622e5997fe072a3a52a873d739bfb03 Mon Sep 17 00:00:00 2001 From: Tibsfox Date: Sun, 5 Apr 2026 14:02:20 -0700 Subject: [PATCH] fix(workflows): respect commit_docs:false in worktree merge and quick task commits (#1802) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three locations in execute-phase.md and quick.md used raw `git add .planning/` commands that bypassed the commit_docs config check. When users set commit_docs: false during project setup, these raw git commands still staged and committed .planning/ files. Add commit_docs guards (via gsd-tools.cjs config-get) around all raw git add .planning/ invocations. The gsd-tools.cjs commit wrapper already respects this flag — these were the only paths that bypassed it. Fixes #1783 Co-authored-by: Claude Opus 4.6 --- get-shit-done/workflows/execute-phase.md | 8 +- get-shit-done/workflows/quick.md | 16 +++- tests/commit-docs-bypass.test.cjs | 98 ++++++++++++++++++++++++ 3 files changed, 117 insertions(+), 5 deletions(-) create mode 100644 tests/commit-docs-bypass.test.cjs diff --git a/get-shit-done/workflows/execute-phase.md b/get-shit-done/workflows/execute-phase.md index 97d2b7970..98227de1c 100644 --- a/get-shit-done/workflows/execute-phase.md +++ b/get-shit-done/workflows/execute-phase.md @@ -526,8 +526,12 @@ Execute each selected wave in sequence. Within a wave: parallel if `PARALLELIZAT # Amend merge commit with restored files if any changed if ! git diff --quiet .planning/STATE.md .planning/ROADMAP.md 2>/dev/null || \ [ -n "$DELETED_FILES" ]; then - git add .planning/STATE.md .planning/ROADMAP.md 2>/dev/null || true - git commit --amend --no-edit 2>/dev/null || true + # Only amend the commit with .planning/ files if commit_docs is enabled (#1783) + COMMIT_DOCS=$(node "$HOME/.claude/get-shit-done/bin/gsd-tools.cjs" config-get commit_docs 2>/dev/null || echo "true") + if [ "$COMMIT_DOCS" != "false" ]; then + git add .planning/STATE.md .planning/ROADMAP.md 2>/dev/null || true + git commit --amend --no-edit 2>/dev/null || true + fi fi # Remove the worktree diff --git a/get-shit-done/workflows/quick.md b/get-shit-done/workflows/quick.md index fae32f850..0aef0995e 100644 --- a/get-shit-done/workflows/quick.md +++ b/get-shit-done/workflows/quick.md @@ -634,8 +634,11 @@ After executor returns: if ! git diff --quiet .planning/STATE.md .planning/ROADMAP.md 2>/dev/null || \ [ -n "$DELETED_FILES" ]; then - git add .planning/STATE.md .planning/ROADMAP.md 2>/dev/null || true - git commit --amend --no-edit 2>/dev/null || true + COMMIT_DOCS=$(node "$HOME/.claude/get-shit-done/bin/gsd-tools.cjs" config-get commit_docs 2>/dev/null || echo "true") + if [ "$COMMIT_DOCS" != "false" ]; then + git add .planning/STATE.md .planning/ROADMAP.md 2>/dev/null || true + git commit --amend --no-edit 2>/dev/null || true + fi fi git worktree remove "$WT" --force 2>/dev/null || true @@ -773,7 +776,14 @@ Build file list: ```bash # Explicitly stage all artifacts before commit — PLAN.md may be untracked # if the executor ran without worktree isolation and committed docs early -git add ${file_list} 2>/dev/null +# Filter .planning/ files from staging if commit_docs is disabled (#1783) +COMMIT_DOCS=$(node "$HOME/.claude/get-shit-done/bin/gsd-tools.cjs" config-get commit_docs 2>/dev/null || echo "true") +if [ "$COMMIT_DOCS" = "false" ]; then + file_list_filtered=$(echo "${file_list}" | tr ' ' '\n' | grep -v '^\.planning/' | tr '\n' ' ') + git add ${file_list_filtered} 2>/dev/null +else + git add ${file_list} 2>/dev/null +fi node "$HOME/.claude/get-shit-done/bin/gsd-tools.cjs" commit "docs(quick-${quick_id}): ${DESCRIPTION}" --files ${file_list} ``` diff --git a/tests/commit-docs-bypass.test.cjs b/tests/commit-docs-bypass.test.cjs new file mode 100644 index 000000000..045ccdbd1 --- /dev/null +++ b/tests/commit-docs-bypass.test.cjs @@ -0,0 +1,98 @@ +/** + * commit_docs bypass guard tests (#1783) + * + * When users set commit_docs: false during /gsd-new-project, .planning/ + * files should never be staged or committed. The gsd-tools.cjs commit + * wrapper already checks this flag, but three locations in execute-phase.md + * and quick.md used raw `git add .planning/` commands that bypassed it. + * + * These tests verify that every `git add .planning/` invocation (explicit + * or via file_list) is preceded by a commit_docs config check. + */ + +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 QUICK_PATH = path.join(__dirname, '..', 'get-shit-done', 'workflows', 'quick.md'); + +describe('commit_docs bypass guard (#1783)', () => { + + test('execute-phase.md: every git add .planning/ has a commit_docs guard', () => { + const content = fs.readFileSync(EXECUTE_PHASE_PATH, 'utf-8'); + const lines = content.split('\n'); + + for (let i = 0; i < lines.length; i++) { + if (/git add\b.*\.planning\//.test(lines[i])) { + // Search backwards from this line for a config-get commit_docs check + const windowStart = Math.max(0, i - 10); + const window = lines.slice(windowStart, i).join('\n'); + assert.ok( + window.includes('config-get commit_docs'), + `git add .planning/ at line ${i + 1} in execute-phase.md must be guarded by a commit_docs config check` + ); + } + } + }); + + test('quick.md: every git add .planning/ has a commit_docs guard', () => { + const content = fs.readFileSync(QUICK_PATH, 'utf-8'); + const lines = content.split('\n'); + + for (let i = 0; i < lines.length; i++) { + if (/git add\b.*\.planning\//.test(lines[i])) { + const windowStart = Math.max(0, i - 10); + const window = lines.slice(windowStart, i).join('\n'); + assert.ok( + window.includes('config-get commit_docs'), + `git add .planning/ at line ${i + 1} in quick.md must be guarded by a commit_docs config check` + ); + } + } + }); + + test('quick.md: git add ${file_list} has a commit_docs guard for .planning/ filtering', () => { + const content = fs.readFileSync(QUICK_PATH, 'utf-8'); + const lines = content.split('\n'); + + // Find the line(s) that do `git add ${file_list}` — this variable + // includes .planning/STATE.md so it needs a commit_docs guard too + for (let i = 0; i < lines.length; i++) { + if (/git add\s+\$\{?file_list/.test(lines[i])) { + const windowStart = Math.max(0, i - 10); + const window = lines.slice(windowStart, i + 1).join('\n'); + assert.ok( + window.includes('config-get commit_docs'), + `git add \${file_list} at line ${i + 1} in quick.md must be guarded by a commit_docs check ` + + `because file_list includes .planning/ files` + ); + } + } + }); + + test('no raw git add .planning/ without commit_docs guard in any workflow', () => { + const workflows = [ + { name: 'execute-phase.md', path: EXECUTE_PHASE_PATH }, + { name: 'quick.md', path: QUICK_PATH }, + ]; + + for (const wf of workflows) { + const content = fs.readFileSync(wf.path, 'utf-8'); + + // Find all occurrences of git add that reference .planning/ + const regex = /git add\b[^\n]*\.planning\//g; + let match; + while ((match = regex.exec(content)) !== null) { + // Get the 500-char window before this match + const before = content.slice(Math.max(0, match.index - 500), match.index); + assert.ok( + before.includes('config-get commit_docs'), + `${wf.name}: found unguarded git add .planning/ near offset ${match.index}. ` + + `All raw git add .planning/ commands must check commit_docs config first.` + ); + } + } + }); +});