From eba0c9969800e47e33ee6c851431c93941afedda Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Thu, 23 Apr 2026 11:58:23 -0400 Subject: [PATCH] fix(#2623): resolve parent .planning root for sub_repos workspaces in SDK query dispatch (#2629) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(#2623): resolve parent .planning root for sub_repos workspaces in SDK query dispatch When `gsd-sdk query` is invoked from inside a `sub_repos`-listed child repo, `projectDir` defaulted to `process.cwd()` which pointed at the child repo, not the parent workspace that owns `.planning/`. Handlers then directly checked `${projectDir}/.planning` and reported `project_exists: false`. The legacy `gsd-tools.cjs` CLI does not have this gap — it calls `findProjectRoot(cwd)` from `bin/lib/core.cjs`, which walks up from the starting directory checking each ancestor's `.planning/config.json` for a `sub_repos` entry that lists the starting directory's top-level segment. This change ports that walk-up as a new `findProjectRoot` helper in `sdk/src/query/helpers.ts` and applies it once in `cli.ts:main()` before dispatching `query`, `run`, `init`, or `auto`. Resolution is idempotent: if `projectDir` already owns `.planning/` (including an explicit `--project-dir` pointing at the workspace root), the helper returns it unchanged. The walk is capped at 10 parent levels and never crosses `$HOME`. All filesystem errors are swallowed. Regression coverage: - `helpers.test.ts` — 8 unit tests covering own-`.planning` guard (#1362), sub_repos match, nested-path match, `planning.sub_repos` shape, heuristic fallback, unparseable config, legacy `multiRepo: true`. - `sub-repos-root.integration.test.ts` — end-to-end baseline (reproduces the bug without the walk-up) and fixed behavior (walk-up + dispatch of `init.new-milestone` reports `project_exists: true` with the parent workspace as `project_root`). sdk vitest: 1511 pass / 24 fail (all 24 failures pre-existing on main, baseline is 26 failing — `comm -23` against baseline produces zero new failures). CJS: 5410 pass / 0 fail. Closes #2623 Co-Authored-By: Claude Opus 4.7 (1M context) * fix(#2623): remove stray .planing typo from integration test setup Address CodeRabbit nitpick: the mkdir('.planing') call on line 23 was dead code from a typo, with errors silently swallowed via .catch(() => {}). The test already creates '.planning' correctly on the next line. Co-Authored-By: Claude Opus 4.7 (1M context) --------- Co-authored-by: Claude Opus 4.7 (1M context) --- CHANGELOG.md | 1 + docs/CONFIGURATION.md | 11 ++ sdk/src/cli.ts | 15 ++ sdk/src/query/helpers.test.ts | 117 +++++++++++++++- sdk/src/query/helpers.ts | 131 +++++++++++++++++- .../query/sub-repos-root.integration.test.ts | 79 +++++++++++ 6 files changed, 352 insertions(+), 2 deletions(-) create mode 100644 sdk/src/query/sub-repos-root.integration.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ca40000..679a638e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,7 @@ If you use GSD **as a workflow**—milestones, phases, `.planning/` artifacts, b ### Fixed +- **`gsd-sdk query` now resolves parent `.planning/` root in multi-repo (`sub_repos`) workspaces** — when invoked from inside a `sub_repos`-listed child repo (e.g. `workspace/app/`), the SDK now walks up to the parent workspace that owns `.planning/`, matching the legacy `gsd-tools.cjs` `findProjectRoot` behavior. Previously `gsd-sdk query init.new-milestone` reported `project_exists: false` from the sub-repo, while `gsd-tools.cjs` resolved the parent root correctly. Resolution happens once in `cli.ts` before dispatch; if `projectDir` already owns `.planning/` (including explicit `--project-dir`), the walk is a no-op. Ported as `findProjectRoot` in `sdk/src/query/helpers.ts` with the same detection order (own `.planning/` wins, then parent `sub_repos` match, then legacy `multiRepo: true`, then `.git` heuristic), capped at 10 parent levels and never crossing `$HOME`. Closes #2623. - **Shell hooks falsely flagged as stale on every session** — `gsd-phase-boundary.sh`, `gsd-session-state.sh`, and `gsd-validate-commit.sh` now ship with a `# gsd-hook-version: {{GSD_VERSION}}` header; the installer substitutes `{{GSD_VERSION}}` in `.sh` hooks the same way it does for `.js` hooks; and the stale-hook detector in `gsd-check-update.js` now matches bash `#` comment syntax in addition to JS `//` syntax. All three changes are required together — neither the regex fix alone nor the install fix alone is sufficient to resolve the false positive (#2136, #2206, #2209, #2210, #2212) ## [1.38.2] - 2026-04-19 diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index 47987191..8b9c2704 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -227,6 +227,17 @@ All workflow toggles follow the **absent = enabled** pattern. If a key is missin | `planning.search_gitignored` | boolean | `false` | Add `--no-ignore` to broad searches to include `.planning/` | | `planning.sub_repos` | array of strings | `[]` | Paths of nested sub-repos relative to the project root. When set, GSD-aware tooling scopes phase-lookup, path-resolution, and commit operations per sub-repo instead of treating the outer repo as a monorepo | +### Project-Root Resolution in Multi-Repo Workspaces + +When `sub_repos` is set and `gsd-tools.cjs` or `gsd-sdk query` is invoked from inside a listed child repo, both CLIs walk up to the parent workspace that owns `.planning/` before dispatching handlers. Resolution order (checked at each ancestor up to 10 levels, never above `$HOME`): + +1. If the starting directory already has its own `.planning/`, it is the project root (no walk-up). +2. Parent has `.planning/config.json` listing the starting directory's top-level segment in `sub_repos` (or the legacy `planning.sub_repos` shape). +3. Parent has `.planning/config.json` with legacy `multiRepo: true` and the starting directory is inside a git repo. +4. Parent has `.planning/` and an ancestor up to the candidate parent contains `.git` (heuristic fallback). + +If none match, the starting directory is returned unchanged. Explicit `--project-dir /path/to/workspace` is idempotent under this resolution. + ### Auto-Detection If `.planning/` is in `.gitignore`, `commit_docs` is automatically `false` regardless of config.json. This prevents git errors. diff --git a/sdk/src/cli.ts b/sdk/src/cli.ts index d68f2ed3..94211c56 100644 --- a/sdk/src/cli.ts +++ b/sdk/src/cli.ts @@ -341,6 +341,21 @@ export async function main(argv: string[] = process.argv.slice(2)): Promise { expect(resolveAgentsDir('codex')).toBe(join('/codex', 'agents')); }); }); + +// ─── findProjectRoot (issue #2623) ───────────────────────────────────────── + +describe('findProjectRoot (multi-repo .planning resolution)', () => { + let workspace: string; + + beforeEach(async () => { + workspace = await mkdtemp(join(tmpdir(), 'gsd-find-root-')); + }); + + afterEach(async () => { + await rm(workspace, { recursive: true, force: true }); + }); + + it('returns startDir unchanged when startDir has its own .planning/', async () => { + await mkdir(join(workspace, '.planning'), { recursive: true }); + expect(findProjectRoot(workspace)).toBe(workspace); + }); + + it('returns startDir unchanged when no ancestor has .planning/', () => { + expect(findProjectRoot(workspace)).toBe(workspace); + }); + + it('walks up to parent .planning/ when config lists the child in sub_repos (#2623)', async () => { + // workspace/.planning/{config.json, PROJECT.md} + // workspace/app/.git/ + await mkdir(join(workspace, '.planning'), { recursive: true }); + await writeFile( + join(workspace, '.planning', 'config.json'), + JSON.stringify({ sub_repos: ['app'] }), + 'utf-8', + ); + const app = join(workspace, 'app'); + await mkdir(join(app, '.git'), { recursive: true }); + + expect(findProjectRoot(app)).toBe(workspace); + }); + + it('resolves parent root from deeply nested dir inside a sub_repo', async () => { + await mkdir(join(workspace, '.planning'), { recursive: true }); + await writeFile( + join(workspace, '.planning', 'config.json'), + JSON.stringify({ sub_repos: ['app'] }), + 'utf-8', + ); + const nested = join(workspace, 'app', 'src', 'modules'); + await mkdir(join(workspace, 'app', '.git'), { recursive: true }); + await mkdir(nested, { recursive: true }); + + expect(findProjectRoot(nested)).toBe(workspace); + }); + + it('supports planning.sub_repos nested config shape', async () => { + await mkdir(join(workspace, '.planning'), { recursive: true }); + await writeFile( + join(workspace, '.planning', 'config.json'), + JSON.stringify({ planning: { sub_repos: ['app'] } }), + 'utf-8', + ); + const app = join(workspace, 'app'); + await mkdir(join(app, '.git'), { recursive: true }); + + expect(findProjectRoot(app)).toBe(workspace); + }); + + it('falls back to .git heuristic when parent has .planning/ but no matching sub_repos', async () => { + await mkdir(join(workspace, '.planning'), { recursive: true }); + // Config doesn't list the child, but child has .git and parent has .planning/. + await writeFile( + join(workspace, '.planning', 'config.json'), + JSON.stringify({ sub_repos: [] }), + 'utf-8', + ); + const app = join(workspace, 'app'); + await mkdir(join(app, '.git'), { recursive: true }); + + expect(findProjectRoot(app)).toBe(workspace); + }); + + it('swallows unparseable config.json and falls back to .git heuristic', async () => { + await mkdir(join(workspace, '.planning'), { recursive: true }); + await writeFile(join(workspace, '.planning', 'config.json'), '{ not json', 'utf-8'); + const app = join(workspace, 'app'); + await mkdir(join(app, '.git'), { recursive: true }); + + expect(findProjectRoot(app)).toBe(workspace); + }); + + it('supports legacy multiRepo: true when child is inside a git repo', async () => { + await mkdir(join(workspace, '.planning'), { recursive: true }); + await writeFile( + join(workspace, '.planning', 'config.json'), + JSON.stringify({ multiRepo: true }), + 'utf-8', + ); + const app = join(workspace, 'app'); + await mkdir(join(app, '.git'), { recursive: true }); + + expect(findProjectRoot(app)).toBe(workspace); + }); + + it('does not walk up when child has its own .planning/ (#1362 guard)', async () => { + await mkdir(join(workspace, '.planning'), { recursive: true }); + await writeFile( + join(workspace, '.planning', 'config.json'), + JSON.stringify({ sub_repos: ['app'] }), + 'utf-8', + ); + const app = join(workspace, 'app'); + await mkdir(join(app, '.planning'), { recursive: true }); + + expect(findProjectRoot(app)).toBe(app); + }); +}); diff --git a/sdk/src/query/helpers.ts b/sdk/src/query/helpers.ts index 3e5272c8..e82b0004 100644 --- a/sdk/src/query/helpers.ts +++ b/sdk/src/query/helpers.ts @@ -17,8 +17,9 @@ * ``` */ -import { join, dirname, relative, resolve, isAbsolute, normalize } from 'node:path'; +import { join, dirname, relative, resolve, isAbsolute, normalize, parse as parsePath, sep as pathSep } from 'node:path'; import { realpath } from 'node:fs/promises'; +import { existsSync, statSync, readFileSync } from 'node:fs'; import { homedir } from 'node:os'; import { GSDError, ErrorClassification } from '../errors.js'; import { relPlanningPath } from '../workstream-utils.js'; @@ -428,6 +429,134 @@ export function planningPaths(projectDir: string, workstream?: string): Planning }; } +// ─── findProjectRoot (multi-repo .planning resolution) ───────────────────── + +/** + * Maximum number of parent directories to walk when searching for a + * multi-repo `.planning/` root. Bounded to avoid scanning to the filesystem + * root in pathological cases. + */ +const FIND_PROJECT_ROOT_MAX_DEPTH = 10; + +/** + * Walk up from `startDir` to find the project root that owns `.planning/`. + * + * Ported from `get-shit-done/bin/lib/core.cjs:findProjectRoot` so that + * `gsd-sdk query` resolves the same parent `.planning/` root as the legacy + * `gsd-tools.cjs` CLI when invoked inside a `sub_repos`-listed child repo. + * + * Detection strategy (checked in order for each ancestor, up to + * `FIND_PROJECT_ROOT_MAX_DEPTH` levels): + * 1. `startDir` itself has `.planning/` — return it unchanged (#1362). + * 2. Parent has `.planning/config.json` with `sub_repos` listing the + * immediate child segment of the starting directory. + * 3. Parent has `.planning/config.json` with `multiRepo: true` (legacy). + * 4. Parent has `.planning/` AND an ancestor of `startDir` (up to the + * candidate parent) contains `.git` — heuristic fallback. + * + * Returns `startDir` unchanged when no ancestor `.planning/` is found + * (first-run or single-repo projects). Never walks above the user's home + * directory. + * + * All filesystem errors are swallowed — a missing or unparseable + * `config.json` falls back to the `.git` heuristic, and unreadable + * directories terminate the walk at that level. + */ +export function findProjectRoot(startDir: string): string { + let resolvedStart: string; + try { + resolvedStart = resolve(startDir); + } catch { + return startDir; + } + const fsRoot = parsePath(resolvedStart).root; + const home = homedir(); + + // If startDir already contains .planning/, it IS the project root. + try { + const ownPlanning = join(resolvedStart, '.planning'); + if (existsSync(ownPlanning) && statSync(ownPlanning).isDirectory()) { + return startDir; + } + } catch { + // fall through + } + + // Walk upward, mirroring isInsideGitRepo from the CJS reference. + function isInsideGitRepo(candidateParent: string): boolean { + let d = resolvedStart; + while (d !== fsRoot) { + try { + if (existsSync(join(d, '.git'))) return true; + } catch { + // ignore + } + if (d === candidateParent) break; + const next = dirname(d); + if (next === d) break; + d = next; + } + return false; + } + + let dir = resolvedStart; + let depth = 0; + while (dir !== fsRoot && depth < FIND_PROJECT_ROOT_MAX_DEPTH) { + const parent = dirname(dir); + if (parent === dir) break; + if (parent === home) break; + + const parentPlanning = join(parent, '.planning'); + let parentPlanningIsDir = false; + try { + parentPlanningIsDir = existsSync(parentPlanning) && statSync(parentPlanning).isDirectory(); + } catch { + parentPlanningIsDir = false; + } + + if (parentPlanningIsDir) { + const configPath = join(parentPlanning, 'config.json'); + let matched = false; + try { + const raw = readFileSync(configPath, 'utf-8'); + const config = JSON.parse(raw) as { + sub_repos?: unknown; + planning?: { sub_repos?: unknown }; + multiRepo?: unknown; + }; + const subReposValue = + (config.sub_repos as unknown) ?? (config.planning && config.planning.sub_repos); + const subRepos = Array.isArray(subReposValue) ? (subReposValue as unknown[]) : []; + + if (subRepos.length > 0) { + const relPath = relative(parent, resolvedStart); + const topSegment = relPath.split(pathSep)[0]; + if (subRepos.includes(topSegment)) { + return parent; + } + } + + if (config.multiRepo === true && isInsideGitRepo(parent)) { + matched = true; + } + } catch { + // config.json missing or unparseable — fall through to .git heuristic. + } + + if (matched) return parent; + + // Heuristic: parent has .planning/ and we're inside a git repo. + if (isInsideGitRepo(parent)) { + return parent; + } + } + + dir = parent; + depth += 1; + } + return startDir; +} + // ─── resolvePathUnderProject ─────────────────────────────────────────────── /** diff --git a/sdk/src/query/sub-repos-root.integration.test.ts b/sdk/src/query/sub-repos-root.integration.test.ts new file mode 100644 index 00000000..60142f74 --- /dev/null +++ b/sdk/src/query/sub-repos-root.integration.test.ts @@ -0,0 +1,79 @@ +/** + * Regression: issue #2623 — `gsd-sdk query` must resolve the parent + * `.planning/` root when invoked from a `sub_repos`-listed child repo. + * + * Exercises the end-to-end path: findProjectRoot(startDir) -> registry dispatch + * of `init.new-milestone`, and asserts the handler reports the parent workspace + * as `project_root` with `project_exists: true`. + */ + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { mkdtemp, rm, writeFile, mkdir } from 'node:fs/promises'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; +import { findProjectRoot } from './helpers.js'; +import { createRegistry } from './index.js'; + +describe('issue #2623 — sub_repos project-root resolution through query dispatch', () => { + let workspace: string; + let appDir: string; + + beforeEach(async () => { + workspace = await mkdtemp(join(tmpdir(), 'gsd-2623-')); + await mkdir(join(workspace, '.planning'), { recursive: true }); + await mkdir(join(workspace, '.planning', 'phases'), { recursive: true }); + await writeFile( + join(workspace, '.planning', 'config.json'), + JSON.stringify({ sub_repos: ['app'] }), + 'utf-8', + ); + await writeFile(join(workspace, '.planning', 'PROJECT.md'), '# Project\n', 'utf-8'); + await writeFile( + join(workspace, '.planning', 'ROADMAP.md'), + '# Roadmap\n\n## Milestone v1.0.0 — Bootstrap\n', + 'utf-8', + ); + await writeFile( + join(workspace, '.planning', 'STATE.md'), + '---\ncurrent_phase: 01-bootstrap\n---\n', + 'utf-8', + ); + + appDir = join(workspace, 'app'); + await mkdir(join(appDir, '.git'), { recursive: true }); + }); + + afterEach(async () => { + await rm(workspace, { recursive: true, force: true }); + }); + + it('findProjectRoot(app) resolves to the parent workspace that owns .planning/', () => { + expect(findProjectRoot(appDir)).toBe(workspace); + }); + + it('init.new-milestone dispatched with resolved root reports project_exists:true', async () => { + // Simulate the CLI path: user starts inside the sub_repo. + const resolved = findProjectRoot(appDir); + expect(resolved).toBe(workspace); + + const registry = createRegistry(); + const result = await registry.dispatch('init.new-milestone', [], resolved, undefined); + const data = result.data as Record; + + expect(data.project_exists).toBe(true); + expect(data.roadmap_exists).toBe(true); + expect(data.state_exists).toBe(true); + expect(data.project_root).toBe(workspace); + }); + + it('without findProjectRoot walk-up, the same handler reports project_exists:false (baseline)', async () => { + // Proves the walk-up is load-bearing — invoking from the child directly + // reproduces the bug described in #2623. + const registry = createRegistry(); + const result = await registry.dispatch('init.new-milestone', [], appDir, undefined); + const data = result.data as Record; + + expect(data.project_exists).toBe(false); + expect(data.project_root).toBe(appDir); + }); +});