From 511c900052cf3de392f1ab98ae8547ca3c94d8d3 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Tue, 8 Sep 2026 12:39:34 -0400 Subject: [PATCH] fix(#4458): reuse detectSubRepos for new-project.md's sub-repo detection (#4548) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(#4458): reuse detectSubRepos for new-project.md's sub-repo detection new-project.md's Step 5.1 (Sub-Repo Detection) ran its own bash predicate: find . -maxdepth 1 -type d -not -name ".*" -not -name "node_modules" \ -exec test -d "{}/.git" \; -print `test -d` requires .git to be a DIRECTORY. A linked git worktree's .git is a FILE (a `gitdir: ` pointer), so this predicate silently excluded valid linked-worktree children while still finding ordinary clones. src/core-utils.cts's detectSubRepos(cwd) already handles this correctly (fs.existsSync, type-agnostic) but had zero callers anywhere in the codebase -- orphaned logic the workflow never actually used, despite duplicating a narrower version of the same check inline. Wired detectSubRepos into cmdInitNewProject's JSON output as a new sub_repos_detected field (matching the file's existing pattern of similar directory-scan-derived fields like has_existing_code/ is_brownfield/has_codebase_map) and replaced the workflow's raw find fence with a gsd_run query init.new-project call reading that field -- removing the duplicate, narrower detection logic entirely rather than patching it in place, per the issue's own "reuse a central policy" framing. Added the missing .git-as-FILE test case to the existing tests/core-utils.test.cjs detectSubRepos coverage (proving the helper was already correct -- the defect was entirely in the unwired workflow predicate) plus CLI-level end-to-end coverage in tests/init-manager.test.cjs using a REAL `git worktree add` fixture, matching the issue's own reproduction steps, alongside an ordinary child-clone case and a non-repository-directory negative case. Refreshed tests/fixtures/compact-content-benchmark-baseline.json (gsd-test caught the drift from new-project.md's byte-count change; the benchmark script itself always exits 0 -- report, not gate -- but the wrapper test enforces the committed baseline stays in sync). Emitted-Drift-Ack-Growth: new-project.md — #4458 replaces the raw find predicate in Step 5.1 with a gsd_run query call reading the new sub_repos_detected field, net +140 bytes Co-Authored-By: Claude Sonnet 5 * docs(#4458): backfill changeset PR number pr: 0 -> pr: 4548 Co-Authored-By: Claude Sonnet 5 * fix(#4458): bound the new git worktree add spawn with a named timeout CI's lint-tests caught two ESLint findings my local gsd-test run couldn't see (gsd-test's matrix doesn't run npm run lint:ci -- same gap already observed on #4456's PR): - local/no-unbounded-spawn: the new execFileSync('git', ['worktree', 'add', ...]) call had no timeout, an indefinite-hang risk. - local/no-adhoc-timeout-literal: my first fix (a bare `timeout: 15_000` literal) was itself flagged -- two independent hardcoded copies of a guessed timeout can silently drift or collide (this repo hit exactly that on 2026-09-06, PR #4428). Fixed by importing GIT_FIXTURE_TIMEOUT_MS from tests/helpers/timeouts.cjs -- `git worktree add` checks out files into a new working tree, the same "construction" weight class as init/config/add/commit that constant already covers, not plain plumbing (GIT_TIMEOUT_MS's class). Verified via `npm run lint` directly (clean) before re-running gsd-test. Co-Authored-By: Claude Sonnet 5 * fix(#4458): reduce redundant git subprocess overhead in new tests CI's full-test Windows shard 2/3 failed: chunk 1/9 (306 files) exceeded its internal 600s budget and was force-killed, with an unrelated file (codex-config.test.cjs) in flight at the moment of the kill -- meaning the chunk's AGGREGATE runtime, not any single hang, blew the budget. This PR's own three new tests each independently called createTempGitProject() (git init + a commit), and one of them also runs git worktree add -- real subprocess spawns, each Defender-scanned on Windows CI (tests/helpers/timeouts.cjs's own documented rationale for why Windows spawn classes get generous budgets). That's a genuine, quantifiable overhead addition to the exact chunk that timed out, not something to wave off as unrelated flake without checking. Two of the three tests never actually needed a real git repo -- detectSubRepos only inspects a CHILD directory's own .git, never the root's git state, and the existing SUBCOMMANDS loop earlier in this same file already proves `init new-project` succeeds against a plain, non-git createTempProject() fixture. Switched those two tests to the lighter fixture, leaving only the one test that genuinely needs a real repo (git worktree add requires one) on createTempGitProject(). Co-Authored-By: Claude Sonnet 5 --------- Co-authored-by: sim Co-authored-by: Claude Sonnet 5 --- .changeset/graceful-mice-chatter.md | 5 ++ gsd-core/workflows/new-project.md | 11 +-- src/init.cts | 8 +++ tests/core-utils.test.cjs | 13 ++++ .../compact-content-benchmark-baseline.json | 10 +-- tests/init-manager.test.cjs | 71 +++++++++++++++++++ 6 files changed, 109 insertions(+), 9 deletions(-) create mode 100644 .changeset/graceful-mice-chatter.md diff --git a/.changeset/graceful-mice-chatter.md b/.changeset/graceful-mice-chatter.md new file mode 100644 index 000000000..402fd0480 --- /dev/null +++ b/.changeset/graceful-mice-chatter.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 4548 +--- +**`/gsd-new-project`'s sub-repo detection now finds linked git worktrees** — a linked worktree's `.git` is a file rather than a directory, and the previous detection predicate silently excluded it from the multi-repo prompt. diff --git a/gsd-core/workflows/new-project.md b/gsd-core/workflows/new-project.md index 3b03cefbc..5414184e5 100644 --- a/gsd-core/workflows/new-project.md +++ b/gsd-core/workflows/new-project.md @@ -629,15 +629,18 @@ gsd_run query commit "chore: add project config" --files .planning/config.json **Detect multi-repo workspace:** -Check for directories with their own `.git` folders (separate repos within the workspace): +Check for directories with their own `.git` (separate repos within the workspace — +this also finds linked git worktree children, whose `.git` is a file rather than a +directory, unlike a plain `find -type d` predicate would): ```bash -find . -maxdepth 1 -type d -not -name ".*" -not -name "node_modules" -exec test -d "{}/.git" \; -print +gsd_run query init.new-project ``` -**If sub-repos found:** +Read the `sub_repos_detected` array from the JSON output — each entry is a bare +directory name already relative to the workspace root (e.g. `"backend"`). -Strip the `./` prefix to get directory names (e.g., `./backend` → `backend`). +**If sub-repos found:** Use AskUserQuestion: diff --git a/src/init.cts b/src/init.cts index ae761d7d6..a927c1678 100644 --- a/src/init.cts +++ b/src/init.cts @@ -1398,6 +1398,14 @@ function cmdInitNewProject(cwd: string, raw: boolean, options: Record { assert.deepEqual(coreUtils.detectSubRepos(tmpDir), ['myrepo']); }); + // #4458: a linked git worktree's .git is a FILE (a `gitdir: ` pointer), + // not a directory. detectSubRepos uses fs.existsSync (type-agnostic), so this + // was already correct before #4458 — this test proves it explicitly, since + // the actual #4458 defect was new-project.md's own `find -exec test -d + // "{}/.git"` predicate never calling this helper at all. + test('detects directory with .git as a FILE (linked worktree) as sub-repo', () => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-cu-test-')); + const subDir = path.join(tmpDir, 'myworktree'); + fs.mkdirSync(subDir); + fs.writeFileSync(path.join(subDir, '.git'), 'gitdir: /some/main/repo/.git/worktrees/myworktree\n'); + assert.deepEqual(coreUtils.detectSubRepos(tmpDir), ['myworktree']); + }); + test('excludes hidden directories', () => { tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-cu-test-')); const hiddenDir = path.join(tmpDir, '.hidden'); diff --git a/tests/fixtures/compact-content-benchmark-baseline.json b/tests/fixtures/compact-content-benchmark-baseline.json index b75f9be26..391c5a5ba 100644 --- a/tests/fixtures/compact-content-benchmark-baseline.json +++ b/tests/fixtures/compact-content-benchmark-baseline.json @@ -23,9 +23,9 @@ "reductionPct": 8.78 }, "new-project": { - "offTokens": 14097, - "onTokens": 12153, - "reductionPct": 13.79 + "offTokens": 14112, + "onTokens": 12168, + "reductionPct": 13.78 }, "plan-phase": { "offTokens": 27637, @@ -39,8 +39,8 @@ } }, "aggregate": { - "offTokens": 106907, - "onTokens": 90259, + "offTokens": 106922, + "onTokens": 90274, "reductionPct": 15.57 } } diff --git a/tests/init-manager.test.cjs b/tests/init-manager.test.cjs index bf23627e1..89d8b1282 100644 --- a/tests/init-manager.test.cjs +++ b/tests/init-manager.test.cjs @@ -1236,6 +1236,77 @@ describe('init subcommands sharing the project_exists/project_path PROJECT.md pa }); }); +// #4458: init new-project's sub_repos_detected field reuses core-utils.cts's +// detectSubRepos instead of new-project.md's own (now-removed) narrower `find +// -exec test -d "{}/.git"` predicate, which required .git to be a DIRECTORY and +// so silently excluded linked git worktree children (.git is a FILE there). +describe('init new-project: sub_repos_detected (#4458)', () => { + const { execFileSync } = require('child_process'); + const { createTempGitProject } = require('./helpers.cjs'); + const { GIT_FIXTURE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); + + let tmpDir; + + afterEach(() => { + if (tmpDir) { cleanup(tmpDir); tmpDir = null; } + }); + + test('detects a REAL linked git worktree child, matching the issue #4458 repro exactly', () => { + // `git worktree add` genuinely needs a real git repo -- this is the only + // one of these three tests that does (createTempGitProject spawns + // `git init` + a commit, real subprocess overhead on Windows CI's + // Defender-scanned spawns; the other two tests below use the plain, + // no-git createTempProject fixture instead, matching the pattern + // already proven safe by the SUBCOMMANDS loop above running `init + // new-project` against a non-git tmpDir). + tmpDir = fs.realpathSync(createTempGitProject()); + const worktreeDir = path.join(tmpDir, 'child-wt'); + // `git worktree add` checks out files into a new working tree — the same + // "construction" weight class as init/config/add/commit, not plain + // plumbing (rev-parse/branch/log), so GIT_FIXTURE_TIMEOUT_MS is the + // correct shared norm here (tests/helpers/timeouts.cjs). + execFileSync('git', ['worktree', 'add', '-b', 'wt-branch', worktreeDir], { cwd: tmpDir, stdio: 'pipe', timeout: GIT_FIXTURE_TIMEOUT_MS }); + + // Confirm the fixture actually reproduces the reported shape before + // trusting the assertion below: a linked worktree's .git is a FILE. + assert.ok(fs.statSync(path.join(worktreeDir, '.git')).isFile(), + 'fixture setup: linked worktree .git must be a file, not a directory'); + + const result = runGsdTools('init new-project', tmpDir); + assert.ok(result.success, `init new-project failed: ${result.error}`); + const output = JSON.parse(result.output); + assert.ok(Array.isArray(output.sub_repos_detected), 'sub_repos_detected must be an array'); + assert.ok(output.sub_repos_detected.includes('child-wt'), + `sub_repos_detected must include the linked worktree child, got: ${JSON.stringify(output.sub_repos_detected)}`); + }); + + test('detects an ordinary child clone (.git as a directory) — no regression', () => { + // detectSubRepos only inspects the CHILD directory's .git, not the + // root's own git state -- a real outer repo isn't needed here, matching + // the SUBCOMMANDS loop above. + tmpDir = fs.realpathSync(createTempProject()); + const cloneDir = path.join(tmpDir, 'child-clone'); + fs.mkdirSync(path.join(cloneDir, '.git'), { recursive: true }); + + const result = runGsdTools('init new-project', tmpDir); + assert.ok(result.success, `init new-project failed: ${result.error}`); + const output = JSON.parse(result.output); + assert.ok(output.sub_repos_detected.includes('child-clone'), + `sub_repos_detected must include the ordinary child clone, got: ${JSON.stringify(output.sub_repos_detected)}`); + }); + + test('does not report an ordinary non-repository directory as a sub-repo', () => { + tmpDir = fs.realpathSync(createTempProject()); + fs.mkdirSync(path.join(tmpDir, 'not-a-repo')); + + const result = runGsdTools('init new-project', tmpDir); + assert.ok(result.success, `init new-project failed: ${result.error}`); + const output = JSON.parse(result.output); + assert.ok(!output.sub_repos_detected.includes('not-a-repo'), + `sub_repos_detected must not include a plain non-repo directory, got: ${JSON.stringify(output.sub_repos_detected)}`); + }); +}); + // ──────────────────────────────────────────────────────────────────────── // Folded from tests/bug-3584-runtime-slash-emitters.test.cjs — consolidation epic #1969 (B2 #1971)