diff --git a/.changeset/lucky-eagles-hop.md b/.changeset/lucky-eagles-hop.md new file mode 100644 index 000000000..6dcf1c51b --- /dev/null +++ b/.changeset/lucky-eagles-hop.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 2909 +--- +**`findProjectRoot` no longer silently resolves to a parent project across a git-repo boundary** — when invoked from a nested git repository that has no `.planning/` of its own, resolution stays within the caller's repo (or falls back to the start directory) instead of crossing into an ancestor GSD project. The existing plain-descendant and co-located `.git`+`.planning` cases are unchanged. diff --git a/src/project-root.cts b/src/project-root.cts index 821e93b9a..dd2555fab 100644 --- a/src/project-root.cts +++ b/src/project-root.cts @@ -57,6 +57,30 @@ export function findProjectRoot(startDir: string): string { return false; } + // #2843: nearest ancestor (including `from` itself) that contains a `.git`, + // bounded by `upTo` (exclusive). Returns the git-repo root, or null if none + // exists before `upTo` / the filesystem root. Used to detect a NESTED child + // repo whose root is strictly below a candidate ancestor `.planning/` — in + // that case the caller's repo boundary sits between start and the ancestor, + // so trusting the ancestor's `.planning/` would silently cross into a + // different project. (No `git` subprocess — fs walk only, matching + // isInsideGitRepo's deliberate no-spawn contract.) + function nearestGitRoot(from: string, upTo: string): string | null { + let d = from; + while (d !== fsRoot) { + if (d === upTo) break; + try { + if (fs.existsSync(d + path.sep + '.git')) return d; + } catch { + // ignore + } + const next = path.dirname(d); + if (next === d) break; + d = next; + } + return null; + } + let dir = resolvedStart; let depth = 0; @@ -107,6 +131,18 @@ export function findProjectRoot(startDir: string): string { // claims our startDir — explicit sub_repos config takes precedence over the // implicit .git signal. (#1422) if (isInsideGitRepo(parent)) { + // #2843: do NOT cross a git-repo boundary. If the caller is inside its + // OWN nested repo whose root is strictly below `parent`, trusting + // `parent`'s .planning/ would silently resolve to a DIFFERENT project. + // isInsideGitRepo only proved SOME .git exists between start and parent; + // verify that .git is parent's own (or absent between), not a nested + // child repo. If nearestGitRoot finds a .git strictly below parent, + // the boundary is crossed — fall through (do not return parent). + if (nearestGitRoot(resolvedStart, parent) !== null) { + dir = parent; + depth += 1; + continue; + } // Lookahead: walk ancestors above `parent` to find a sub_repos claim. let ancestor = path.dirname(parent); let ancestorDepth = 0; @@ -162,6 +198,15 @@ export function findProjectRoot(startDir: string): string { try { const candidatePlanning = parent2 + path.sep + '.planning'; if (fs.existsSync(candidatePlanning) && fs.statSync(candidatePlanning).isDirectory()) { + // #2843: do not cross a git-repo boundary. If the caller is inside its + // own nested repo (a .git strictly below parent2), parent2's .planning/ + // belongs to a DIFFERENT project — keep walking is wrong; stop and fall + // through to the startDir fallback instead of silently resolving to the + // ancestor project. (Reached only when no .git exists anywhere in the + // chain per the triage, but guard defensively.) + if (nearestGitRoot(resolvedStart, parent2) !== null) { + break; + } return parent2; } } catch { diff --git a/tests/project-root.test.cjs b/tests/project-root.test.cjs index 0942a4fab..b226bceeb 100644 --- a/tests/project-root.test.cjs +++ b/tests/project-root.test.cjs @@ -275,4 +275,44 @@ describe('findProjectRoot nearest-.planning resolution (#1414)', () => { assert.strictEqual(result, nested, 'Should return startDir unchanged when no ancestor has .planning/ within the depth bound'); }); + + // #2843: findProjectRoot must NOT cross a git-repo boundary. A nested child + // repo (own .git, no .planning of its own) under an ancestor GSD project must + // NOT resolve to the ancestor's root — that would silently return a different + // project's identity with exit 0. Pre-fix heuristic (3)'s isInsideGitRepo only + // checked "does SOME .git exist between start and the ancestor" — the child's + // own .git satisfied it, crossing the boundary. + test('#2843 does not resolve to an ancestor project across a nested child .git boundary', () => { + // tmpDir/ (plain — not a git repo, not HOME) + // parent-gsd/.planning/ ← ancestor GSD project + // parent-gsd/child-app/.git ← nested child repo, NO .planning of its own + // parent-gsd/child-app/src/ ← startDir + const parentGsd = mkDeep(tmpDir, 'parent-gsd'); + fs.mkdirSync(path.join(parentGsd, '.planning'), { recursive: true }); + const childApp = mkDeep(parentGsd, 'child-app'); + fs.mkdirSync(path.join(childApp, '.git'), { recursive: true }); + const startDir = mkDeep(childApp, 'src'); + + const result = findProjectRoot(startDir); + assert.notStrictEqual(result, parentGsd, + 'must NOT cross child-app\'s .git boundary to resolve to the ancestor parent-gsd project'); + // The child repo has no .planning of its own, so resolution falls back to + // the startDir (or a path within child-app) — never the ancestor. + assert.ok( + result === startDir || result === childApp, + `expected to stay within the child repo (startDir or childApp fallback), got: ${result}`, + ); + }); + + test('#2843 negative-space: a co-located .git + .planning (normal single-repo) still resolves to the project root', () => { + // The normal case: .git and .planning at the SAME level. The caller's .git + // IS the project's .git, so the boundary check passes (no nested child repo). + fs.mkdirSync(path.join(tmpDir, '.git'), { recursive: true }); + fs.mkdirSync(path.join(tmpDir, '.planning'), { recursive: true }); + const nested = mkDeep(tmpDir, 'src', 'lib'); + + const result = findProjectRoot(nested); + assert.strictEqual(result, tmpDir, + 'a co-located .git + .planning (single-repo project) must still resolve to the project root'); + }); });