Merge pull request #1424 from jeremymcs/fix/doctor-gap-coverage
fix: close 5 doctor coverage gaps — providers, lock dir, integration branch, orphaned worktrees
This commit is contained in:
commit
aa6f41c256
6 changed files with 332 additions and 11 deletions
|
|
@ -1,15 +1,15 @@
|
|||
import { existsSync, lstatSync, readdirSync, readFileSync, realpathSync, statSync } from "node:fs";
|
||||
import { join, sep } from "node:path";
|
||||
import { existsSync, lstatSync, readdirSync, readFileSync, realpathSync, rmSync, statSync } from "node:fs";
|
||||
import { basename, dirname, join, sep } from "node:path";
|
||||
|
||||
import type { DoctorIssue, DoctorIssueCode } from "./doctor-types.js";
|
||||
import { loadFile, parseRoadmap } from "./files.js";
|
||||
import { resolveMilestoneFile, milestonesDir, gsdRoot, resolveGsdRootFile, relGsdRootFile } from "./paths.js";
|
||||
import { deriveState, isMilestoneComplete } from "./state.js";
|
||||
import { saveFile } from "./files.js";
|
||||
import { listWorktrees, resolveGitDir } from "./worktree-manager.js";
|
||||
import { listWorktrees, resolveGitDir, worktreesDir } from "./worktree-manager.js";
|
||||
import { abortAndReset } from "./git-self-heal.js";
|
||||
import { RUNTIME_EXCLUSION_PATHS } from "./git-service.js";
|
||||
import { nativeIsRepo, nativeWorktreeRemove, nativeBranchList, nativeBranchDelete, nativeLsFiles, nativeRmCached } from "./native-git-bridge.js";
|
||||
import { RUNTIME_EXCLUSION_PATHS, readIntegrationBranch } from "./git-service.js";
|
||||
import { nativeIsRepo, nativeBranchExists, nativeWorktreeList, nativeWorktreeRemove, nativeBranchList, nativeBranchDelete, nativeLsFiles, nativeRmCached } from "./native-git-bridge.js";
|
||||
import { readCrashLock, isLockProcessAlive, clearLock } from "./crash-recovery.js";
|
||||
import { ensureGitignore } from "./gitignore.js";
|
||||
import { readAllSessionStatuses, isSessionStale, removeSessionStatus } from "./session-status-io.js";
|
||||
|
|
@ -215,6 +215,70 @@ export async function checkGitHealth(
|
|||
} catch {
|
||||
// git branch list failed — skip
|
||||
}
|
||||
|
||||
// ── Integration branch existence ──────────────────────────────────────
|
||||
// For each active (non-complete) milestone, verify the stored integration
|
||||
// branch still exists in git. A missing integration branch blocks merge-back
|
||||
// and causes the next merge operation to fail silently.
|
||||
try {
|
||||
const state = await deriveState(basePath);
|
||||
for (const milestone of state.registry) {
|
||||
if (milestone.status === "complete") continue;
|
||||
const integrationBranch = readIntegrationBranch(basePath, milestone.id);
|
||||
if (!integrationBranch) continue; // No stored branch — skip (not yet set)
|
||||
if (!nativeBranchExists(basePath, integrationBranch)) {
|
||||
issues.push({
|
||||
severity: "error",
|
||||
code: "integration_branch_missing",
|
||||
scope: "milestone",
|
||||
unitId: milestone.id,
|
||||
message: `Milestone ${milestone.id} recorded integration branch "${integrationBranch}" but that branch no longer exists in git. Merge-back will fail.`,
|
||||
fixable: false,
|
||||
});
|
||||
}
|
||||
}
|
||||
} catch {
|
||||
// Non-fatal — integration branch check failed
|
||||
}
|
||||
|
||||
// ── Orphaned worktree directories ────────────────────────────────────
|
||||
// Worktree removal can fail after a branch delete, leaving a directory
|
||||
// that is no longer registered with git. These orphaned dirs cause
|
||||
// "already exists" errors when re-creating the same worktree name.
|
||||
try {
|
||||
const wtDir = worktreesDir(basePath);
|
||||
if (existsSync(wtDir)) {
|
||||
const registeredPaths = new Set(
|
||||
nativeWorktreeList(basePath).map(entry => entry.path),
|
||||
);
|
||||
for (const entry of readdirSync(wtDir)) {
|
||||
const fullPath = join(wtDir, entry);
|
||||
try {
|
||||
if (!statSync(fullPath).isDirectory()) continue;
|
||||
} catch { continue; }
|
||||
if (!registeredPaths.has(fullPath)) {
|
||||
issues.push({
|
||||
severity: "warning",
|
||||
code: "worktree_directory_orphaned",
|
||||
scope: "project",
|
||||
unitId: entry,
|
||||
message: `Worktree directory ${fullPath} exists on disk but is not registered with git. Run "git worktree prune" or doctor --fix to remove it.`,
|
||||
fixable: true,
|
||||
});
|
||||
if (shouldFix("worktree_directory_orphaned")) {
|
||||
try {
|
||||
rmSync(fullPath, { recursive: true, force: true });
|
||||
fixesApplied.push(`removed orphaned worktree directory ${fullPath}`);
|
||||
} catch {
|
||||
fixesApplied.push(`failed to remove orphaned worktree directory ${fullPath}`);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
} catch {
|
||||
// Non-fatal — orphaned worktree directory check failed
|
||||
}
|
||||
}
|
||||
|
||||
// ── Runtime Health Checks ──────────────────────────────────────────────────
|
||||
|
|
@ -255,6 +319,44 @@ export async function checkRuntimeHealth(
|
|||
// Non-fatal — crash lock check failed
|
||||
}
|
||||
|
||||
// ── Stranded lock directory ────────────────────────────────────────────
|
||||
// proper-lockfile creates a `.gsd.lock/` directory as the OS-level lock
|
||||
// mechanism. If the process was SIGKILLed or crashed hard, this directory
|
||||
// can remain on disk without any live process holding it. The next session
|
||||
// fails to acquire the lock until the directory is removed (#1245).
|
||||
try {
|
||||
const lockDir = join(dirname(root), `${basename(root)}.lock`);
|
||||
if (existsSync(lockDir)) {
|
||||
const statRes = statSync(lockDir);
|
||||
if (statRes.isDirectory()) {
|
||||
// Check if any live process actually holds this lock
|
||||
const lock = readCrashLock(basePath);
|
||||
const lockHolderAlive = lock ? isLockProcessAlive(lock) : false;
|
||||
if (!lockHolderAlive) {
|
||||
issues.push({
|
||||
severity: "error",
|
||||
code: "stranded_lock_directory",
|
||||
scope: "project",
|
||||
unitId: "project",
|
||||
message: `Stranded lock directory "${lockDir}" exists but no live process holds the session lock. This blocks new auto-mode sessions from starting.`,
|
||||
file: lockDir,
|
||||
fixable: true,
|
||||
});
|
||||
if (shouldFix("stranded_lock_directory")) {
|
||||
try {
|
||||
rmSync(lockDir, { recursive: true, force: true });
|
||||
fixesApplied.push(`removed stranded lock directory ${lockDir}`);
|
||||
} catch {
|
||||
fixesApplied.push(`failed to remove stranded lock directory ${lockDir}`);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
} catch {
|
||||
// Non-fatal — stranded lock directory check failed
|
||||
}
|
||||
|
||||
// ── Stale parallel sessions ────────────────────────────────────────────
|
||||
try {
|
||||
const parallelStatuses = readAllSessionStatuses(basePath);
|
||||
|
|
|
|||
|
|
@ -20,6 +20,9 @@ import { gsdRoot, resolveGsdRootFile } from "./paths.js";
|
|||
import { readCrashLock, isLockProcessAlive, clearLock } from "./crash-recovery.js";
|
||||
import { abortAndReset } from "./git-self-heal.js";
|
||||
import { rebuildState } from "./doctor.js";
|
||||
import { deriveState } from "./state.js";
|
||||
import { readIntegrationBranch } from "./git-service.js";
|
||||
import { nativeBranchExists, nativeIsRepo } from "./native-git-bridge.js";
|
||||
|
||||
// ── Health Score Tracking ──────────────────────────────────────────────────
|
||||
|
||||
|
|
@ -191,6 +194,27 @@ export async function preDispatchHealthGate(basePath: string): Promise<PreDispat
|
|||
// Non-fatal — dispatch continues without STATE.md if rebuild fails
|
||||
}
|
||||
|
||||
// ── Integration branch existence check ──
|
||||
// If the active milestone's recorded integration branch no longer exists in
|
||||
// git, the merge-back at the end of the milestone will fail. Block dispatch
|
||||
// now to surface this before work is lost.
|
||||
try {
|
||||
if (nativeIsRepo(basePath)) {
|
||||
const state = await deriveState(basePath);
|
||||
if (state.activeMilestone) {
|
||||
const integrationBranch = readIntegrationBranch(basePath, state.activeMilestone.id);
|
||||
if (integrationBranch && !nativeBranchExists(basePath, integrationBranch)) {
|
||||
issues.push(
|
||||
`Integration branch "${integrationBranch}" for milestone ${state.activeMilestone.id} no longer exists in git. ` +
|
||||
`Restore the branch or update the integration branch before dispatching. Run /gsd doctor for details.`,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
} catch {
|
||||
// Non-fatal — dispatch continues if state/branch check fails
|
||||
}
|
||||
|
||||
// If we had critical issues that couldn't be auto-healed, block dispatch
|
||||
if (issues.length > 0) {
|
||||
return {
|
||||
|
|
|
|||
|
|
@ -45,7 +45,15 @@ export type DoctorIssueCode =
|
|||
| "env_python"
|
||||
| "env_cargo"
|
||||
| "env_go"
|
||||
| "env_git_remote";
|
||||
| "env_git_remote"
|
||||
// Provider / auth checks
|
||||
| "provider_key_missing"
|
||||
| "provider_key_backedoff"
|
||||
// Lock infrastructure checks
|
||||
| "stranded_lock_directory"
|
||||
// Git / worktree integrity checks
|
||||
| "integration_branch_missing"
|
||||
| "worktree_directory_orphaned";
|
||||
|
||||
/**
|
||||
* Issue codes that represent expected completion-transition states.
|
||||
|
|
|
|||
|
|
@ -11,6 +11,7 @@ import type { DoctorIssue, DoctorIssueCode } from "./doctor-types.js";
|
|||
import { COMPLETION_TRANSITION_CODES } from "./doctor-types.js";
|
||||
import { checkGitHealth, checkRuntimeHealth } from "./doctor-checks.js";
|
||||
import { checkEnvironmentHealth } from "./doctor-environment.js";
|
||||
import { runProviderChecks } from "./doctor-providers.js";
|
||||
|
||||
// ── Re-exports ─────────────────────────────────────────────────────────────
|
||||
// All public types and functions from extracted modules are re-exported here
|
||||
|
|
@ -406,6 +407,40 @@ export async function runGSDDoctor(basePath: string, options?: { fix?: boolean;
|
|||
issues.push(...auditRequirements(requirementsContent));
|
||||
|
||||
const state = await deriveState(basePath);
|
||||
|
||||
// Provider / auth health checks — only relevant when there is active work to dispatch.
|
||||
// Skipped for idle projects (no active milestone) to avoid noise in environments
|
||||
// where CI/test runners have no API key configured.
|
||||
if (state.activeMilestone) {
|
||||
try {
|
||||
const providerResults = runProviderChecks();
|
||||
for (const result of providerResults) {
|
||||
if (!result.required) continue;
|
||||
if (result.status === "error") {
|
||||
issues.push({
|
||||
severity: "warning",
|
||||
code: "provider_key_missing",
|
||||
scope: "project",
|
||||
unitId: "project",
|
||||
message: result.message + (result.detail ? ` — ${result.detail}` : ""),
|
||||
fixable: false,
|
||||
});
|
||||
} else if (result.status === "warning") {
|
||||
issues.push({
|
||||
severity: "warning",
|
||||
code: "provider_key_backedoff",
|
||||
scope: "project",
|
||||
unitId: "project",
|
||||
message: result.message + (result.detail ? ` — ${result.detail}` : ""),
|
||||
fixable: false,
|
||||
});
|
||||
}
|
||||
}
|
||||
} catch {
|
||||
// Non-fatal — provider check failure should not block other checks
|
||||
}
|
||||
}
|
||||
|
||||
for (const milestone of state.registry) {
|
||||
const milestoneId = milestone.id;
|
||||
const milestonePath = resolveMilestonePath(basePath, milestoneId);
|
||||
|
|
|
|||
|
|
@ -2,9 +2,10 @@
|
|||
* doctor-git.test.ts — Integration tests for doctor git health checks.
|
||||
*
|
||||
* Creates real temp git repos with deliberate broken state, runs runGSDDoctor,
|
||||
* and asserts correct detection and fixing of all 4 git issue codes:
|
||||
* and asserts correct detection and fixing of git issue codes:
|
||||
* orphaned_auto_worktree, stale_milestone_branch,
|
||||
* corrupt_merge_state, tracked_runtime_files
|
||||
* corrupt_merge_state, tracked_runtime_files,
|
||||
* integration_branch_missing, worktree_directory_orphaned
|
||||
*/
|
||||
|
||||
import { mkdtempSync, mkdirSync, writeFileSync, rmSync, existsSync, realpathSync } from "node:fs";
|
||||
|
|
@ -299,6 +300,101 @@ async function main(): Promise<void> {
|
|||
console.log("\n=== none-mode skips stale branch (skipped on Windows) ===");
|
||||
}
|
||||
|
||||
// ─── Test: Integration branch missing ──────────────────────────────
|
||||
if (process.platform !== "win32") {
|
||||
console.log("\n=== integration_branch_missing ===");
|
||||
{
|
||||
const dir = createRepoWithActiveMilestone();
|
||||
cleanups.push(dir);
|
||||
|
||||
// Write integration branch metadata for M001 pointing to a non-existent branch
|
||||
const metaPath = join(dir, ".gsd", "milestones", "M001", "M001-META.json");
|
||||
writeFileSync(metaPath, JSON.stringify({ integrationBranch: "feat/does-not-exist" }, null, 2));
|
||||
|
||||
const detect = await runGSDDoctor(dir);
|
||||
const missingBranchIssues = detect.issues.filter(i => i.code === "integration_branch_missing");
|
||||
assertTrue(missingBranchIssues.length > 0, "detects missing integration branch");
|
||||
assertTrue(
|
||||
missingBranchIssues[0]?.message.includes("feat/does-not-exist"),
|
||||
"message includes the missing branch name",
|
||||
);
|
||||
assertEq(missingBranchIssues[0]?.fixable, false, "integration_branch_missing is not auto-fixable");
|
||||
assertEq(missingBranchIssues[0]?.severity, "error", "severity is error");
|
||||
}
|
||||
} else {
|
||||
console.log("\n=== integration_branch_missing (skipped on Windows) ===");
|
||||
}
|
||||
|
||||
// ─── Test: Integration branch present — no false positive ──────────
|
||||
if (process.platform !== "win32") {
|
||||
console.log("\n=== integration_branch_missing (no false positive) ===");
|
||||
{
|
||||
const dir = createRepoWithActiveMilestone();
|
||||
cleanups.push(dir);
|
||||
|
||||
// Write integration branch metadata for M001 pointing to "main" (which exists)
|
||||
const metaPath = join(dir, ".gsd", "milestones", "M001", "M001-META.json");
|
||||
writeFileSync(metaPath, JSON.stringify({ integrationBranch: "main" }, null, 2));
|
||||
|
||||
const detect = await runGSDDoctor(dir);
|
||||
const missingBranchIssues = detect.issues.filter(i => i.code === "integration_branch_missing");
|
||||
assertEq(missingBranchIssues.length, 0, "existing integration branch NOT flagged");
|
||||
}
|
||||
} else {
|
||||
console.log("\n=== integration_branch_missing (no false positive — skipped on Windows) ===");
|
||||
}
|
||||
|
||||
// ─── Test: Orphaned worktree directory ─────────────────────────────
|
||||
if (process.platform !== "win32") {
|
||||
console.log("\n=== worktree_directory_orphaned ===");
|
||||
{
|
||||
const dir = createRepoWithActiveMilestone();
|
||||
cleanups.push(dir);
|
||||
|
||||
// Create a worktrees/ dir with an entry that is NOT in git worktree list
|
||||
const orphanDir = join(dir, ".gsd", "worktrees", "orphan-feature");
|
||||
mkdirSync(orphanDir, { recursive: true });
|
||||
writeFileSync(join(orphanDir, "some-file.txt"), "leftover content\n");
|
||||
|
||||
const detect = await runGSDDoctor(dir);
|
||||
const orphanDirIssues = detect.issues.filter(i => i.code === "worktree_directory_orphaned");
|
||||
assertTrue(orphanDirIssues.length > 0, "detects orphaned worktree directory");
|
||||
assertTrue(
|
||||
orphanDirIssues[0]?.message.includes("orphan-feature"),
|
||||
"message includes the orphaned directory name",
|
||||
);
|
||||
assertTrue(orphanDirIssues[0]?.fixable === true, "worktree_directory_orphaned is fixable");
|
||||
|
||||
const fixed = await runGSDDoctor(dir, { fix: true });
|
||||
assertTrue(
|
||||
fixed.fixesApplied.some(f => f.includes("removed orphaned worktree directory")),
|
||||
"fix removes orphaned worktree directory",
|
||||
);
|
||||
assertTrue(!existsSync(orphanDir), "orphaned directory removed after fix");
|
||||
}
|
||||
} else {
|
||||
console.log("\n=== worktree_directory_orphaned (skipped on Windows) ===");
|
||||
}
|
||||
|
||||
// ─── Test: Registered worktree NOT flagged as orphaned ─────────────
|
||||
if (process.platform !== "win32") {
|
||||
console.log("\n=== worktree_directory_orphaned (registered worktree not flagged) ===");
|
||||
{
|
||||
const dir = createRepoWithActiveMilestone();
|
||||
cleanups.push(dir);
|
||||
|
||||
// Create a real registered worktree under .gsd/worktrees/
|
||||
mkdirSync(join(dir, ".gsd", "worktrees"), { recursive: true });
|
||||
run("git worktree add -b worktree/feature-1 .gsd/worktrees/feature-1", dir);
|
||||
|
||||
const detect = await runGSDDoctor(dir);
|
||||
const orphanDirIssues = detect.issues.filter(i => i.code === "worktree_directory_orphaned");
|
||||
assertEq(orphanDirIssues.length, 0, "registered worktree NOT flagged as orphaned");
|
||||
}
|
||||
} else {
|
||||
console.log("\n=== worktree_directory_orphaned (registered worktree not flagged — skipped on Windows) ===");
|
||||
}
|
||||
|
||||
// ─── Test 9: none-mode still detects corrupt merge state ───────────
|
||||
console.log("\n=== none-mode keeps corrupt merge state ===");
|
||||
{
|
||||
|
|
|
|||
|
|
@ -2,9 +2,9 @@
|
|||
* doctor-runtime.test.ts — Tests for doctor runtime health checks.
|
||||
*
|
||||
* Tests detection and auto-fix of:
|
||||
* stale_crash_lock, orphaned_completed_units, stale_hook_state,
|
||||
* activity_log_bloat, state_file_missing, state_file_stale,
|
||||
* gitignore_missing_patterns
|
||||
* stale_crash_lock, stranded_lock_directory, orphaned_completed_units,
|
||||
* stale_hook_state, activity_log_bloat, state_file_missing,
|
||||
* state_file_stale, gitignore_missing_patterns
|
||||
*/
|
||||
|
||||
import { mkdtempSync, mkdirSync, writeFileSync, rmSync, existsSync, readFileSync, realpathSync } from "node:fs";
|
||||
|
|
@ -290,6 +290,62 @@ node_modules/
|
|||
assertEq(content.length, 0, "all orphaned keys removed");
|
||||
}
|
||||
|
||||
// ─── Test: Stranded lock directory detection & fix ────────────────
|
||||
// Skip on Windows: proper-lockfile uses advisory file locking on Windows,
|
||||
// not the directory-based mechanism. The .gsd.lock/ directory pattern is
|
||||
// a POSIX-specific lockfile implementation detail.
|
||||
if (process.platform !== "win32") {
|
||||
console.log("\n=== stranded_lock_directory ===");
|
||||
{
|
||||
const dir = createMinimalProject();
|
||||
cleanups.push(dir);
|
||||
|
||||
// Create the proper-lockfile lock directory without a live lock holder.
|
||||
// The lock dir sits at <parent of .gsd>/.gsd.lock (i.e., <basePath>/.gsd.lock).
|
||||
const lockDir = join(dir, ".gsd.lock");
|
||||
mkdirSync(lockDir, { recursive: true });
|
||||
|
||||
const detect = await runGSDDoctor(dir);
|
||||
const strandedIssues = detect.issues.filter(i => i.code === "stranded_lock_directory");
|
||||
assertTrue(strandedIssues.length > 0, "detects stranded lock directory");
|
||||
assertTrue(strandedIssues[0]?.message.includes("lock directory"), "message describes stranded lock directory");
|
||||
assertTrue(strandedIssues[0]?.fixable === true, "stranded lock dir is fixable");
|
||||
|
||||
const fixed = await runGSDDoctor(dir, { fix: true });
|
||||
assertTrue(
|
||||
fixed.fixesApplied.some(f => f.includes("removed stranded lock directory")),
|
||||
"fix removes stranded lock directory",
|
||||
);
|
||||
assertTrue(!existsSync(lockDir), "lock directory removed after fix");
|
||||
}
|
||||
|
||||
// ─── Test: Stranded lock dir with live lock holder — NOT flagged ───
|
||||
console.log("\n=== stranded_lock_directory (live holder not flagged) ===");
|
||||
{
|
||||
const dir = createMinimalProject();
|
||||
cleanups.push(dir);
|
||||
|
||||
// Create lock dir + auto.lock with PID 1 (init/launchd — always alive, never our own PID)
|
||||
const lockDir = join(dir, ".gsd.lock");
|
||||
mkdirSync(lockDir, { recursive: true });
|
||||
const liveLockData = {
|
||||
pid: 1,
|
||||
startedAt: new Date().toISOString(),
|
||||
unitType: "execute-task",
|
||||
unitId: "M001/S01/T01",
|
||||
unitStartedAt: new Date().toISOString(),
|
||||
completedUnits: 1,
|
||||
};
|
||||
writeFileSync(join(dir, ".gsd", "auto.lock"), JSON.stringify(liveLockData, null, 2));
|
||||
|
||||
const detect = await runGSDDoctor(dir);
|
||||
const strandedIssues = detect.issues.filter(i => i.code === "stranded_lock_directory");
|
||||
assertEq(strandedIssues.length, 0, "live lock holder: stranded_lock_directory NOT detected");
|
||||
}
|
||||
} else {
|
||||
console.log("\n=== stranded_lock_directory (skipped on Windows) ===");
|
||||
}
|
||||
|
||||
} finally {
|
||||
for (const dir of cleanups) {
|
||||
try { rmSync(dir, { recursive: true, force: true }); } catch { /* ignore */ }
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue