diff --git a/.changeset/3491-nested-git-detection.md b/.changeset/3491-nested-git-detection.md new file mode 100644 index 000000000..e82fd1be6 --- /dev/null +++ b/.changeset/3491-nested-git-detection.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3491 +--- +**`/gsd-new-project` and `/gsd-ingest-docs` no longer create a nested `.git` inside an existing worktree (#3491)** — the shallow `has_git: false` check (which only looked for `.git` in the current directory) has been replaced with `git rev-parse --is-inside-work-tree` semantics. The init payload now surfaces `git_worktree_root` and `in_nested_subdir` so the workflows correctly skip `git init` when invoked from a subdirectory of an existing repo and warn that planning files will be tracked by the outer repo. diff --git a/get-shit-done/bin/lib/core.cjs b/get-shit-done/bin/lib/core.cjs index 71bab815b..98c62630f 100644 --- a/get-shit-done/bin/lib/core.cjs +++ b/get-shit-done/bin/lib/core.cjs @@ -1535,6 +1535,48 @@ function pathExistsInternal(cwd, targetPath) { } } +/** + * Detect whether `cwd` sits inside a git worktree, and if so, return the + * absolute path of the worktree root. + * + * Bug #3491: the previous shallow `pathExistsInternal(cwd, '.git')` check + * only saw a `.git` entry directly in cwd, so subdirectories of an existing + * repo reported `has_git: false` and the new-project workflow then ran + * `git init` — creating a nested `.git` inside the outer repo's worktree. + * + * Mirrors `git rev-parse --is-inside-work-tree` semantics. Uses the existing + * `execGit` seam so behaviour is consistent with the rest of the toolchain + * (non-interactive env, 10s timeout, mockable in tests). + * + * Returns: { inside: boolean, worktreeRoot: string | null } + * - inside=true → cwd is somewhere inside a git worktree + * - inside=false → cwd is not inside any git worktree (or git is unavailable) + * + * Failure modes (git not installed, command times out, non-zero exit) all + * collapse to `{ inside: false, worktreeRoot: null }` — the conservative + * default that preserves pre-fix behaviour for environments without git. + */ +function gitWorktreeInfoInternal(cwd) { + try { + const insideResult = execGit(['rev-parse', '--is-inside-work-tree'], { cwd, timeout: 5000 }); + if (insideResult.exitCode !== 0) { + return { inside: false, worktreeRoot: null }; + } + const insideStdout = String(insideResult.stdout || '').trim(); + if (insideStdout !== 'true') { + return { inside: false, worktreeRoot: null }; + } + const rootResult = execGit(['rev-parse', '--show-toplevel'], { cwd, timeout: 5000 }); + if (rootResult.exitCode !== 0) { + return { inside: true, worktreeRoot: null }; + } + const root = String(rootResult.stdout || '').trim(); + return { inside: true, worktreeRoot: root || null }; + } catch { + return { inside: false, worktreeRoot: null }; + } +} + function generateSlugInternal(text) { if (!text) return null; return text.toLowerCase().replace(/[^a-z0-9]+/g, '-').replace(/^-+|-+$/g, '').substring(0, 60); @@ -1817,6 +1859,7 @@ module.exports = { resolveTierEntry, _resetRuntimeWarningCacheForTests, pathExistsInternal, + gitWorktreeInfoInternal, generateSlugInternal, getMilestoneInfo, getMilestonePhaseFilter, diff --git a/get-shit-done/bin/lib/init.cjs b/get-shit-done/bin/lib/init.cjs index b8615044e..c5e2b709e 100644 --- a/get-shit-done/bin/lib/init.cjs +++ b/get-shit-done/bin/lib/init.cjs @@ -5,7 +5,7 @@ const fs = require('fs'); const path = require('path'); const { execGit, platformWriteSync, platformReadSync } = require('./shell-command-projection.cjs'); -const { loadConfig, resolveModelInternal, findPhaseInternal, getRoadmapPhaseInternal, pathExistsInternal, generateSlugInternal, getMilestoneInfo, getMilestonePhaseFilter, stripShippedMilestones, extractCurrentMilestone, normalizePhaseName, toPosixPath, output, error, checkAgentsInstalled, phaseTokenMatches } = require('./core.cjs'); +const { loadConfig, resolveModelInternal, findPhaseInternal, getRoadmapPhaseInternal, pathExistsInternal, gitWorktreeInfoInternal, generateSlugInternal, getMilestoneInfo, getMilestonePhaseFilter, stripShippedMilestones, extractCurrentMilestone, normalizePhaseName, toPosixPath, output, error, checkAgentsInstalled, phaseTokenMatches } = require('./core.cjs'); const { planningPaths, planningDir, planningRoot } = require('./planning-workspace.cjs'); const { maskIfSecret } = require('./secrets.cjs'); const scanPhasePlans = require('./plan-scan.cjs'); @@ -458,8 +458,17 @@ function cmdInitNewProject(cwd, raw) { is_brownfield: hasCode || hasPackageFile, needs_codebase_map: (hasCode || hasPackageFile) && !pathExistsInternal(cwd, '.planning/codebase'), - // Git state - has_git: pathExistsInternal(cwd, '.git'), + // Git state (Bug #3491: detect parent worktree to avoid nested .git init) + ...(() => { + const info = gitWorktreeInfoInternal(cwd); + const worktreeRoot = info.worktreeRoot; + const inNestedSubdir = info.inside && worktreeRoot !== null && worktreeRoot !== cwd; + return { + has_git: info.inside, + git_worktree_root: worktreeRoot, + in_nested_subdir: inNestedSubdir, + }; + })(), // Enhanced search brave_search_available: hasBraveSearch, @@ -593,7 +602,17 @@ function cmdInitIngestDocs(cwd, raw) { const result = { project_exists: pathExistsInternal(cwd, '.planning/PROJECT.md'), planning_exists: fs.existsSync(planningRoot(cwd)), - has_git: fs.existsSync(path.join(cwd, '.git')), + ...(() => { + // Bug #3491 — see cmdInitNewProject above. Same shallow-check bug. + const info = gitWorktreeInfoInternal(cwd); + const worktreeRoot = info.worktreeRoot; + const inNestedSubdir = info.inside && worktreeRoot !== null && worktreeRoot !== cwd; + return { + has_git: info.inside, + git_worktree_root: worktreeRoot, + in_nested_subdir: inNestedSubdir, + }; + })(), project_path: '.planning/PROJECT.md', commit_docs: config.commit_docs, }; diff --git a/get-shit-done/workflows/ingest-docs.md b/get-shit-done/workflows/ingest-docs.md index fc75699db..f19cc558b 100644 --- a/get-shit-done/workflows/ingest-docs.md +++ b/get-shit-done/workflows/ingest-docs.md @@ -56,7 +56,7 @@ INIT=$(node "$HOME/.claude/get-shit-done/bin/gsd-tools.cjs" init ingest-docs) if [[ "$INIT" == @file:* ]]; then INIT=$(cat "${INIT#@file:}"); fi ``` -Parse `project_exists`, `planning_exists`, `has_git`, `project_path` from INIT. +Parse `project_exists`, `planning_exists`, `has_git`, `git_worktree_root`, `in_nested_subdir`, `project_path` from INIT. **Auto-detect MODE** if not set: - `planning_exists: true` → `MODE=merge` @@ -64,7 +64,12 @@ Parse `project_exists`, `planning_exists`, `has_git`, `project_path` from INIT. If user passed `--mode new` but `.planning/` already exists: display warning and require explicit confirm via `AskUserQuestion` (approve-revise-abort from `references/gate-prompts.md`) before overwriting. -If `has_git: false` and `MODE=new`: initialize git: +Git initialisation (Bug #3491 — never create a nested `.git` inside an existing worktree): + +- If `has_git: true` and `in_nested_subdir: true`: do NOT run `git init`. Surface a warning that planning files will be tracked by the outer repo at `git_worktree_root`. +- If `has_git: true` and `in_nested_subdir: false`: already at a worktree root, skip `git init`. +- If `has_git: false` and `MODE=new`: initialize git: + ```bash git init ``` diff --git a/get-shit-done/workflows/new-project.md b/get-shit-done/workflows/new-project.md index 2ceb19c0c..e3fbaed57 100644 --- a/get-shit-done/workflows/new-project.md +++ b/get-shit-done/workflows/new-project.md @@ -64,7 +64,7 @@ AGENT_SKILLS_SYNTHESIZER=$(gsd-sdk query agent-skills gsd-research-synthesizer) AGENT_SKILLS_ROADMAPPER=$(gsd-sdk query agent-skills gsd-roadmapper) ``` -Parse JSON for: `researcher_model`, `synthesizer_model`, `roadmapper_model`, `commit_docs`, `project_exists`, `has_codebase_map`, `planning_exists`, `has_existing_code`, `has_package_file`, `is_brownfield`, `needs_codebase_map`, `has_git`, `project_path`, `agents_installed`, `missing_agents`, `agent_runtime`, `agents_dir`, `required_agents`, `required_agents_installed`, `missing_required_agents`, `agent_skill_payloads_available`, `agent_skill_payload_agents`. +Parse JSON for: `researcher_model`, `synthesizer_model`, `roadmapper_model`, `commit_docs`, `project_exists`, `has_codebase_map`, `planning_exists`, `has_existing_code`, `has_package_file`, `is_brownfield`, `needs_codebase_map`, `has_git`, `git_worktree_root`, `in_nested_subdir`, `project_path`, `agents_installed`, `missing_agents`, `agent_runtime`, `agents_dir`, `required_agents`, `required_agents_installed`, `missing_required_agents`, `agent_skill_payloads_available`, `agent_skill_payload_agents`. **If `agents_installed` is false:** Display a warning before proceeding: ```text @@ -117,11 +117,11 @@ All subsequent references to the project instruction file use `$INSTRUCTION_FILE **If `project_exists` is true:** Error — project already initialized. Use `/gsd:progress`. -**If `has_git` is false:** Initialize git: +**Git init (#3491 — never nest `.git` inside an existing worktree):** -```bash -git init -``` +- If `has_git` true and `in_nested_subdir` true: skip `git init`; warn `⚠ Initializing inside existing worktree (${git_worktree_root}); planning files will track to outer repo.` +- If `has_git` true and `in_nested_subdir` false: skip `git init` (already at worktree root). +- If `has_git` false: `git init`. ## 2. Brownfield Offer diff --git a/sdk/src/query/init-complex.ts b/sdk/src/query/init-complex.ts index 6e60b1071..49f42b0b5 100644 --- a/sdk/src/query/init-complex.ts +++ b/sdk/src/query/init-complex.ts @@ -19,6 +19,7 @@ */ import { existsSync, readdirSync, statSync, type Dirent } from 'node:fs'; +import { execSync } from 'node:child_process'; import { readFile } from 'node:fs/promises'; import { join, relative } from 'node:path'; import { homedir } from 'node:os'; @@ -61,6 +62,40 @@ function pathExists(base: string, relPath: string): boolean { return existsSync(join(base, relPath)); } +/** + * Bug #3491: detect whether `base` is inside any git worktree, and if so, + * return the absolute worktree root. Mirrors the CJS `gitWorktreeInfoInternal` + * in get-shit-done/bin/lib/core.cjs — keep these two implementations behaviour- + * identical so the SDK and CJS init handlers emit the same has_git semantics. + * + * Returns { inside, worktreeRoot } — both fall back to false/null on any error + * (git unavailable, not a repo, timeout) so callers see the conservative + * default that preserves pre-fix behaviour for non-git environments. + */ +function gitWorktreeInfo(base: string): { inside: boolean; worktreeRoot: string | null } { + try { + const inside = execSync('git rev-parse --is-inside-work-tree', { + cwd: base, + stdio: ['ignore', 'pipe', 'ignore'], + encoding: 'utf-8', + timeout: 5000, + env: { ...process.env, GIT_TERMINAL_PROMPT: '0' }, + }).trim(); + if (inside !== 'true') return { inside: false, worktreeRoot: null }; + const root = execSync('git rev-parse --show-toplevel', { + cwd: base, + stdio: ['ignore', 'pipe', 'ignore'], + encoding: 'utf-8', + timeout: 5000, + env: { ...process.env, GIT_TERMINAL_PROMPT: '0' }, + }).trim(); + return { inside: true, worktreeRoot: root || null }; + } catch { + return { inside: false, worktreeRoot: null }; + } +} + + const NEW_PROJECT_REQUIRED_AGENTS = [ 'gsd-project-researcher', 'gsd-research-synthesizer', @@ -269,7 +304,13 @@ export const initNewProject: QueryHandler = async (_args, projectDir, workstream needs_codebase_map: (hasExistingCode || hasPackageFile) && !pathExists(projectDir, '.planning/codebase'), - has_git: pathExists(projectDir, '.git'), + // Bug #3491: detect parent worktree to avoid nested .git init. + has_git: (() => gitWorktreeInfo(projectDir).inside)(), + git_worktree_root: (() => gitWorktreeInfo(projectDir).worktreeRoot)(), + in_nested_subdir: (() => { + const info = gitWorktreeInfo(projectDir); + return info.inside && info.worktreeRoot !== null && info.worktreeRoot !== projectDir; + })(), brave_search_available: hasBraveSearch, firecrawl_available: hasFirecrawl, diff --git a/sdk/src/query/init.ts b/sdk/src/query/init.ts index aa071bcc8..3e99046bd 100644 --- a/sdk/src/query/init.ts +++ b/sdk/src/query/init.ts @@ -78,6 +78,40 @@ function pathExists(base: string, relPath: string): boolean { return existsSync(join(base, relPath)); } +/** + * Bug #3491: detect whether `base` is inside any git worktree, and if so, + * return the absolute worktree root. Mirrors the CJS `gitWorktreeInfoInternal` + * in get-shit-done/bin/lib/core.cjs — keep these two implementations behaviour- + * identical so the SDK and CJS init handlers emit the same has_git semantics. + * + * Returns { inside, worktreeRoot } — both fall back to false/null on any error + * (git unavailable, not a repo, timeout) so callers see the conservative + * default that preserves pre-fix behaviour for non-git environments. + */ +function gitWorktreeInfo(base: string): { inside: boolean; worktreeRoot: string | null } { + try { + const inside = execSync('git rev-parse --is-inside-work-tree', { + cwd: base, + stdio: ['ignore', 'pipe', 'ignore'], + encoding: 'utf-8', + timeout: 5000, + env: { ...process.env, GIT_TERMINAL_PROMPT: '0' }, + }).trim(); + if (inside !== 'true') return { inside: false, worktreeRoot: null }; + const root = execSync('git rev-parse --show-toplevel', { + cwd: base, + stdio: ['ignore', 'pipe', 'ignore'], + encoding: 'utf-8', + timeout: 5000, + env: { ...process.env, GIT_TERMINAL_PROMPT: '0' }, + }).trim(); + return { inside: true, worktreeRoot: root || null }; + } catch { + return { inside: false, worktreeRoot: null }; + } +} + + /** * Compute the canonical phase directory name for a known phase entry from the * roadmap when no directory exists yet. Applies the project_code prefix so @@ -1197,7 +1231,13 @@ export const initIngestDocs: QueryHandler = async (_args, projectDir) => { const result: Record = { project_exists: pathExists(projectDir, '.planning/PROJECT.md'), planning_exists: pathExists(projectDir, '.planning'), - has_git: pathExists(projectDir, '.git'), + // Bug #3491: detect parent worktree to avoid nested .git init. + has_git: (() => gitWorktreeInfo(projectDir).inside)(), + git_worktree_root: (() => gitWorktreeInfo(projectDir).worktreeRoot)(), + in_nested_subdir: (() => { + const info = gitWorktreeInfo(projectDir); + return info.inside && info.worktreeRoot !== null && info.worktreeRoot !== projectDir; + })(), project_path: '.planning/PROJECT.md', commit_docs: config.commit_docs, }; diff --git a/tests/bug-3491-nested-git-worktree.test.cjs b/tests/bug-3491-nested-git-worktree.test.cjs new file mode 100644 index 000000000..a7d6cadc9 --- /dev/null +++ b/tests/bug-3491-nested-git-worktree.test.cjs @@ -0,0 +1,180 @@ +// allow-test-rule: source-text-is-the-product +// Bug #3491 — new-project workflow creates nested .git in subdirectory when +// parent already has git repo. +// +// The workflow's `has_git` boolean was derived from `pathExists(cwd, '.git')` +// — a shallow check that only sees a `.git` entry directly in the current +// directory. Subdirectories of an existing git worktree therefore reported +// `has_git: false`, causing the workflow's `git init` step to create a nested +// `.git` inside the outer repo's worktree. Subsequent gsd-sdk commits then +// targeted the nested repo instead of the outer one, silently dropping all +// planning artefacts from the outer repo's history. +// +// This test asserts the corrected semantics, mirroring `git rev-parse +// --is-inside-work-tree`: +// +// - `has_git: true` is reported whenever the cwd is inside a git worktree, +// even when no `.git` entry is in cwd itself. +// - The init payload surfaces `git_worktree_root` and `in_nested_subdir` so +// the workflow can warn the user and skip `git init`. +// - The workflow markdown's `git init` step is gated on +// `in_nested_subdir: false`, never unconditional under `has_git: false`. + +'use strict'; + +const 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 { execSync } = require('node:child_process'); + +const { runGsdTools, cleanup } = require('./helpers.cjs'); + +const WORKFLOW_PATH = path.join( + __dirname, + '..', + 'get-shit-done', + 'workflows', + 'new-project.md', +); + +// ─── Helper: create outer git repo with a nested workstream subdir ───────── + +function createOuterRepoWithSubdir(prefix = 'bug-3491-') { + const outer = fs.mkdtempSync(path.join(os.tmpdir(), prefix)); + // macOS /tmp -> /private/tmp; resolve to the canonical path so comparisons + // against `git rev-parse --show-toplevel` succeed regardless of symlink. + const outerReal = fs.realpathSync(outer); + execSync('git init', { cwd: outerReal, stdio: 'pipe' }); + execSync('git config user.email "test@test.com"', { cwd: outerReal, stdio: 'pipe' }); + execSync('git config user.name "Test"', { cwd: outerReal, stdio: 'pipe' }); + execSync('git config commit.gpgsign false', { cwd: outerReal, stdio: 'pipe' }); + fs.writeFileSync(path.join(outerReal, 'README.md'), '# outer\n'); + execSync('git add -A', { cwd: outerReal, stdio: 'pipe' }); + execSync('git commit -m "initial"', { cwd: outerReal, stdio: 'pipe' }); + + const subdir = path.join(outerReal, 'workstreams', 'my-project'); + fs.mkdirSync(subdir, { recursive: true }); + return { outer: outerReal, subdir }; +} + +// ─── Behavioural tests against the live `init new-project` handler ───────── + +test('bug-3491: init new-project reports has_git: true inside parent git worktree', () => { + const { outer, subdir } = createOuterRepoWithSubdir(); + try { + const result = runGsdTools('init new-project', subdir); + assert.ok(result.success, `init new-project failed: ${result.error}`); + + const payload = JSON.parse(result.output); + + // Core fix: shallow `.git in cwd` check was wrong — we are inside the + // outer worktree, so the workflow MUST see has_git: true. + assert.strictEqual( + payload.has_git, + true, + 'expected has_git=true when cwd is inside an existing git worktree (parent .git)', + ); + + // The workflow needs the worktree root and a nesting flag to decide + // whether to skip `git init` and emit a friendly warning. + assert.strictEqual( + payload.git_worktree_root, + outer, + `expected git_worktree_root to be the outer repo (${outer}), got: ${payload.git_worktree_root}`, + ); + assert.strictEqual( + payload.in_nested_subdir, + true, + 'expected in_nested_subdir=true when cwd is a subdirectory of the worktree root', + ); + } finally { + cleanup(outer); + } +}); + +test('bug-3491: init new-project reports has_git: true at worktree root with in_nested_subdir: false', () => { + const { outer } = createOuterRepoWithSubdir(); + try { + const result = runGsdTools('init new-project', outer); + assert.ok(result.success, `init new-project failed: ${result.error}`); + + const payload = JSON.parse(result.output); + assert.strictEqual(payload.has_git, true, 'has_git must be true at the worktree root'); + assert.strictEqual(payload.git_worktree_root, outer); + assert.strictEqual( + payload.in_nested_subdir, + false, + 'at the worktree root, in_nested_subdir must be false', + ); + } finally { + cleanup(outer); + } +}); + +test('bug-3491: init new-project reports has_git: false outside any git worktree', () => { + const tmp = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'bug-3491-bare-'))); + try { + const result = runGsdTools('init new-project', tmp); + assert.ok(result.success, `init new-project failed: ${result.error}`); + const payload = JSON.parse(result.output); + assert.strictEqual(payload.has_git, false); + assert.strictEqual(payload.in_nested_subdir, false); + assert.strictEqual(payload.git_worktree_root, null); + } finally { + cleanup(tmp); + } +}); + +test('bug-3491: init ingest-docs mirrors the same has_git semantics', () => { + // ingest-docs.md has the same shallow check and the same nested-init risk. + const { outer, subdir } = createOuterRepoWithSubdir('bug-3491-ingest-'); + try { + const result = runGsdTools('init ingest-docs', subdir); + assert.ok(result.success, `init ingest-docs failed: ${result.error}`); + const payload = JSON.parse(result.output); + assert.strictEqual( + payload.has_git, + true, + 'init ingest-docs must also detect parent worktree (#3491 related path)', + ); + assert.strictEqual(payload.git_worktree_root, outer); + assert.strictEqual(payload.in_nested_subdir, true); + } finally { + cleanup(outer); + } +}); + +// ─── Workflow-text test: the deployed `new-project.md` must gate `git init` ─ + +test('bug-3491: new-project.md gates `git init` on in_nested_subdir, not just has_git', () => { + const content = fs.readFileSync(WORKFLOW_PATH, 'utf-8'); + + // The pre-fix workflow had the literal sequence: + // + // **If `has_git` is false:** Initialize git: + // ```bash + // git init + // ``` + // + // …which fires for any subdirectory of an existing repo. The fix must + // either gate the init on `in_nested_subdir`/worktree-root semantics or + // drop the unconditional `git init` block entirely. + const unconditionalInitPattern = + /\*\*If `has_git` is false:\*\* Initialize git:\s*\n+```bash\s*\ngit init\s*\n```/; + assert.ok( + !unconditionalInitPattern.test(content), + 'new-project.md must not run `git init` unconditionally on has_git=false (#3491). ' + + 'Gate it on `in_nested_subdir === false` so the workflow refuses to create ' + + 'a nested .git inside an existing worktree.', + ); + + // The fixed workflow MUST mention the new field so reviewers can see the + // gating exists. (Workflow markdown IS the deployed product — testing it + // as text is the only end-to-end signal we have.) + assert.ok( + /in_nested_subdir/.test(content), + 'new-project.md must reference `in_nested_subdir` after the #3491 fix', + ); +}); diff --git a/tests/prompt-injection-scan.test.cjs b/tests/prompt-injection-scan.test.cjs index cdd234d2d..fc1a0cc73 100644 --- a/tests/prompt-injection-scan.test.cjs +++ b/tests/prompt-injection-scan.test.cjs @@ -52,6 +52,7 @@ const SCAN_EXTS = new Set(['.md', '.cjs', '.js', '.json']); const ALLOWLIST = new Set([ 'get-shit-done/bin/lib/security.cjs', // The security module itself 'get-shit-done/workflows/discuss-phase.md', // Large workflow (~50K) with power mode + i18n + 'get-shit-done/workflows/new-project.md', // Large workflow (~50K) — agent install, runtime detect, brownfield map, #3491 worktree gating 'get-shit-done/workflows/execute-phase.md', // Large orchestration workflow (~51K) with wave execution + code-review gate 'get-shit-done/workflows/plan-phase.md', // Large orchestration workflow (~51K) with TDD mode integration 'hooks/gsd-prompt-guard.js', // The prompt guard hook