fix: worktree health check walks parent dirs for monorepo support (#3313)
* fix: worktree health check walks parent dirs for monorepo support (#2347) The health check only looked for project markers (package.json, Cargo.toml, etc.) in s.basePath directly. In monorepos, these files live in a parent directory, causing false rejections. Now walks up parent directories to find project markers before triggering the greenfield warning. Fixes #2347 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(health-check): cap parent walk at git repo boundary The parent directory walk in the worktree health check was unbounded, walking all the way to the filesystem root. Ancestor directories like ~ or /usr/local may contain unrelated package.json files, causing false positive health checks. Now stops at the .git boundary. Also adds a test assertion verifying the .git boundary guard exists. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(ts): remove duplicate imports introduced during rebase --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: trek-e <trek-e@users.noreply.github.com>
This commit is contained in:
parent
5fa34d438b
commit
56617891d9
2 changed files with 92 additions and 1 deletions
|
|
@ -26,7 +26,7 @@ import { runUnit } from "./run-unit.js";
|
|||
import { debugLog } from "../debug-logger.js";
|
||||
import { PROJECT_FILES } from "../detection.js";
|
||||
import { MergeConflictError } from "../git-service.js";
|
||||
import { join, basename } from "node:path";
|
||||
import { join, basename, dirname, parse as parsePath } from "node:path";
|
||||
import { existsSync, cpSync } from "node:fs";
|
||||
import { logWarning, logError } from "../workflow-logger.js";
|
||||
import { gsdRoot } from "../paths.js";
|
||||
|
|
@ -949,7 +949,25 @@ export async function runUnitPhase(
|
|||
}
|
||||
const hasProjectFile = PROJECT_FILES.some((f) => deps.existsSync(join(s.basePath, f)));
|
||||
const hasSrcDir = deps.existsSync(join(s.basePath, "src"));
|
||||
// Monorepo support (#2347): if no project files in the worktree directory,
|
||||
// walk parent directories up to the filesystem root. In monorepos,
|
||||
// package.json / Cargo.toml etc. live in a parent directory.
|
||||
let hasProjectFileInParent = false;
|
||||
if (!hasProjectFile && !hasSrcDir) {
|
||||
let checkDir = dirname(s.basePath);
|
||||
const { root } = parsePath(checkDir);
|
||||
while (checkDir !== root) {
|
||||
// Stop at git repository boundary — ancestors above the repo root
|
||||
// (e.g. ~ or /usr/local) may contain unrelated project files.
|
||||
if (deps.existsSync(join(checkDir, ".git"))) break;
|
||||
if (PROJECT_FILES.some((f) => deps.existsSync(join(checkDir, f)))) {
|
||||
hasProjectFileInParent = true;
|
||||
break;
|
||||
}
|
||||
checkDir = dirname(checkDir);
|
||||
}
|
||||
}
|
||||
if (!hasProjectFile && !hasSrcDir && !hasProjectFileInParent) {
|
||||
// Greenfield projects won't have project files yet — the first task creates them.
|
||||
// Log a warning but allow execution to proceed. The .git check above is sufficient
|
||||
// to ensure we're in a valid working directory.
|
||||
|
|
|
|||
|
|
@ -0,0 +1,73 @@
|
|||
/**
|
||||
* worktree-health-monorepo.test.ts — #2347
|
||||
*
|
||||
* The worktree health check in auto/phases.ts falsely rejects monorepos
|
||||
* where package.json (or other project markers) is in a parent directory.
|
||||
* This test verifies that the health check walks parent directories.
|
||||
*/
|
||||
|
||||
import { readFileSync } from "node:fs";
|
||||
import { join } from "node:path";
|
||||
import { createTestContext } from "./test-helpers.ts";
|
||||
|
||||
const { assertTrue, report } = createTestContext();
|
||||
|
||||
const srcPath = join(import.meta.dirname, "..", "auto", "phases.ts");
|
||||
const src = readFileSync(srcPath, "utf-8");
|
||||
|
||||
console.log("\n=== #2347: Worktree health check supports monorepos ===");
|
||||
|
||||
// ── Test 1: The health check region exists ──────────────────────────────
|
||||
|
||||
const healthCheckIdx = src.indexOf("Worktree health check");
|
||||
assertTrue(healthCheckIdx > 0, "auto/phases.ts has worktree health check section");
|
||||
|
||||
const healthCheckRegion = src.slice(healthCheckIdx, healthCheckIdx + 2000);
|
||||
|
||||
// ── Test 2: The check walks parent directories for project markers ──────
|
||||
|
||||
// The fix should check parent directories for project files, not just s.basePath.
|
||||
// Look for patterns like: walking up directories, dirname, parent, or a helper
|
||||
// function that checks ancestors.
|
||||
const checksParentDirs =
|
||||
healthCheckRegion.includes("dirname") ||
|
||||
healthCheckRegion.includes("parent") ||
|
||||
healthCheckRegion.includes("ancestor") ||
|
||||
healthCheckRegion.includes("walk") ||
|
||||
// Or a helper function that's called with the base path
|
||||
/hasProjectFileInAncestor|findProjectRoot|checkParent/i.test(healthCheckRegion);
|
||||
|
||||
assertTrue(
|
||||
checksParentDirs,
|
||||
"Health check should walk parent directories for project markers (monorepo support) (#2347)",
|
||||
);
|
||||
|
||||
// ── Test 3: The parent walk stops at a .git boundary ──────────────────
|
||||
|
||||
// The parent directory walk must not escape the git repository root.
|
||||
// Without this guard, ancestor directories like ~ or /usr/local that
|
||||
// happen to contain package.json would cause false positive health checks.
|
||||
const hasGitBoundary = healthCheckRegion.includes('.git') &&
|
||||
(healthCheckRegion.includes('break') || healthCheckRegion.includes('stop'));
|
||||
|
||||
assertTrue(
|
||||
hasGitBoundary,
|
||||
"Parent directory walk must stop at .git repository boundary to prevent false positives",
|
||||
);
|
||||
|
||||
// ── Test 4: The greenfield warning should only trigger when no parent has markers ─
|
||||
|
||||
// The original code was:
|
||||
// const hasProjectFile = PROJECT_FILES.some((f) => deps.existsSync(join(s.basePath, f)));
|
||||
// The fix should check parents too, so the greenfield warning only fires
|
||||
// when NO ancestor directory has project markers either.
|
||||
const hasParentCheck = healthCheckRegion.includes("parent") ||
|
||||
healthCheckRegion.includes("dirname") ||
|
||||
/ancestor|walk.*up/i.test(healthCheckRegion);
|
||||
|
||||
assertTrue(
|
||||
hasParentCheck,
|
||||
"Greenfield check should consider parent directories before warning (#2347)",
|
||||
);
|
||||
|
||||
report();
|
||||
Loading…
Add table
Reference in a new issue