From 16d31f2253afaa721488f8ed19c11e43a33225da Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Mon, 30 Mar 2026 15:52:21 -0400 Subject: [PATCH] fix(parallel): check worktree DB for milestone completion in merge (#2812) (#3256) determineMergeOrder relied solely on orchestrator WorkerInfo.state being "stopped" to find mergeable milestones. When the orchestrator state drifts (worker respawned, status.json deleted, etc.), completed milestones become invisible to the merge command. Now scans .gsd/worktrees//.gsd/gsd.db for milestones with status='complete', using the same subprocess-sqlite3 pattern as the parallel-monitor-overlay. The worktree DB is the ground truth. Co-authored-by: Claude Opus 4.6 --- .../extensions/gsd/parallel-merge.ts | 91 ++++++++++++++- .../tests/integration/parallel-merge.test.ts | 110 ++++++++++++++++++ 2 files changed, 197 insertions(+), 4 deletions(-) diff --git a/src/resources/extensions/gsd/parallel-merge.ts b/src/resources/extensions/gsd/parallel-merge.ts index 74b526fdd..e777a5a35 100644 --- a/src/resources/extensions/gsd/parallel-merge.ts +++ b/src/resources/extensions/gsd/parallel-merge.ts @@ -5,6 +5,9 @@ * with safety checks for parallel execution context. */ +import { existsSync, readdirSync } from "node:fs"; +import { join } from "node:path"; +import { spawnSync } from "node:child_process"; import { loadFile } from "./files.js"; import { resolveMilestoneFile } from "./paths.js"; import { mergeMilestoneToMain } from "./auto-worktree.js"; @@ -28,22 +31,102 @@ export type MergeOrder = "sequential" | "by-completion"; // ─── Merge Queue ─────────────────────────────────────────────────────────── +/** + * Check whether a milestone is complete by querying its worktree SQLite DB. + * Uses a subprocess to avoid disrupting the global DB singleton. + * Returns true when milestones.status = 'complete' in the worktree's gsd.db. + */ +export function isMilestoneCompleteInWorktreeDb(basePath: string, mid: string): boolean { + const dbPath = join(basePath, ".gsd", "worktrees", mid, ".gsd", "gsd.db"); + if (!existsSync(dbPath)) return false; + + try { + const result = spawnSync( + "sqlite3", + [dbPath, `SELECT status FROM milestones WHERE id='${mid}' LIMIT 1`], + { timeout: 3000, encoding: "utf-8" }, + ); + return (result.stdout || "").trim() === "complete"; + } catch { + return false; + } +} + +/** + * Discover milestone IDs with status='complete' in their worktree DB, + * scanning .gsd/worktrees//.gsd/gsd.db for each worktree directory. + */ +function discoverDbCompletedMilestones(basePath: string): Set { + const completed = new Set(); + const worktreeDir = join(basePath, ".gsd", "worktrees"); + try { + for (const entry of readdirSync(worktreeDir)) { + if (entry.startsWith("M") && isMilestoneCompleteInWorktreeDb(basePath, entry)) { + completed.add(entry); + } + } + } catch { + // worktrees dir may not exist + } + return completed; +} + /** * Determine safe merge order for completed milestones. * Sequential: merge in milestone ID order (M001 before M002). * By-completion: merge in the order milestones finished. + * + * When basePath is provided, also checks worktree SQLite DBs as the + * source of truth — workers with stale orchestrator state (e.g. "error") + * are included if their worktree DB shows status='complete'. + * See: https://github.com/gsd-build/gsd-2/issues/2812 */ export function determineMergeOrder( workers: WorkerInfo[], order: MergeOrder = "sequential", + basePath?: string, ): string[] { - const completed = workers.filter(w => w.state === "stopped"); + // Start with workers the orchestrator already knows are stopped + const stoppedIds = new Set( + workers.filter(w => w.state === "stopped").map(w => w.milestoneId), + ); + + // When basePath is available, also check worktree DBs for milestones + // whose orchestrator state is stale but are actually complete (#2812) + const dbCompleted = basePath ? discoverDbCompletedMilestones(basePath) : new Set(); + + // Union: milestone is mergeable if stopped OR DB-complete + const mergeableIds = new Set([...stoppedIds, ...dbCompleted]); + + // Build the list from tracked workers + any DB-discovered milestones + // not tracked by the orchestrator at all + const workerMap = new Map(workers.map(w => [w.milestoneId, w])); + const allMergeable: WorkerInfo[] = []; + for (const mid of mergeableIds) { + const w = workerMap.get(mid); + if (w) { + allMergeable.push(w); + } else { + // Milestone discovered from worktree DB but not in workers list + allMergeable.push({ + milestoneId: mid, + title: mid, + pid: 0, + process: null, + worktreePath: basePath ? join(basePath, ".gsd", "worktrees", mid) : "", + startedAt: 0, + state: "stopped", + cost: 0, + }); + } + } + if (order === "by-completion") { - return completed + return allMergeable .sort((a, b) => a.startedAt - b.startedAt) // earliest first .map(w => w.milestoneId); } - return completed + return allMergeable .sort((a, b) => a.milestoneId.localeCompare(b.milestoneId)) .map(w => w.milestoneId); } @@ -114,7 +197,7 @@ export async function mergeAllCompleted( workers: WorkerInfo[], order: MergeOrder = "sequential", ): Promise { - const mergeOrder = determineMergeOrder(workers, order); + const mergeOrder = determineMergeOrder(workers, order, basePath); const results: MergeResult[] = []; for (const mid of mergeOrder) { diff --git a/src/resources/extensions/gsd/tests/integration/parallel-merge.test.ts b/src/resources/extensions/gsd/tests/integration/parallel-merge.test.ts index 038f40f44..b438d5fa6 100644 --- a/src/resources/extensions/gsd/tests/integration/parallel-merge.test.ts +++ b/src/resources/extensions/gsd/tests/integration/parallel-merge.test.ts @@ -38,6 +38,12 @@ import { writeSessionStatus, readSessionStatus, } from "../../session-status-io.ts"; +import { + openDatabase, + closeDatabase, + insertMilestone, + updateMilestoneStatus, +} from "../../gsd-db.ts"; // ─── Helpers ────────────────────────────────────────────────────────────────── @@ -465,3 +471,107 @@ test("mergeAllCompleted — by-completion order respects startedAt", async () => cleanup(repo); } }); + +// ═══════════════════════════════════════════════════════════════════════════════ +// Bug #2812 — determineMergeOrder should use worktree DB as source of truth +// ═══════════════════════════════════════════════════════════════════════════════ + +/** Set up a worktree DB with a milestone marked complete */ +function setupWorktreeDb(basePath: string, mid: string): void { + const wtGsdDir = join(basePath, ".gsd", "worktrees", mid, ".gsd"); + mkdirSync(wtGsdDir, { recursive: true }); + const dbPath = join(wtGsdDir, "gsd.db"); + openDatabase(dbPath); + insertMilestone({ id: mid, title: `Milestone ${mid}`, status: "complete" }); + updateMilestoneStatus(mid, "complete", new Date().toISOString()); + closeDatabase(); +} + +test("determineMergeOrder — finds milestones completed in worktree DB even when worker state is 'error' (#2812)", () => { + const base = realpathSync(mkdtempSync(join(tmpdir(), "merge-db-bug-"))); + try { + // Simulate the bug scenario: orchestrator has stale "error" state + // but the worktree DB shows milestone is actually complete. + setupWorktreeDb(base, "M011"); + + const workers = [ + makeWorker({ milestoneId: "M010", state: "error" }), + makeWorker({ milestoneId: "M011", state: "error" }), // stale — actually complete in DB + makeWorker({ milestoneId: "M012", state: "running" }), + ]; + + const order = determineMergeOrder(workers, "sequential", base); + + // M011 should be included because its worktree DB says status='complete' + assert.ok( + order.includes("M011"), + `Expected M011 in merge order (worktree DB says complete), got: [${order}]`, + ); + // M010 and M012 should NOT be included (no worktree DB with complete status) + assert.ok(!order.includes("M010"), "M010 should not be in merge order (error, no DB)"); + assert.ok(!order.includes("M012"), "M012 should not be in merge order (running, no DB)"); + } finally { + cleanup(base); + } +}); + +test("determineMergeOrder — workers with state='stopped' still included without basePath", () => { + // Backward compatibility: existing behavior still works when basePath is omitted + const workers = [ + makeWorker({ milestoneId: "M001", state: "stopped" }), + makeWorker({ milestoneId: "M002", state: "error" }), + ]; + const order = determineMergeOrder(workers, "sequential"); + assert.deepEqual(order, ["M001"]); +}); + +test("determineMergeOrder — combines stopped workers and DB-complete milestones without duplicates", () => { + const base = realpathSync(mkdtempSync(join(tmpdir(), "merge-dedup-"))); + try { + // M001 is stopped in orchestrator AND complete in worktree DB + setupWorktreeDb(base, "M001"); + + const workers = [ + makeWorker({ milestoneId: "M001", state: "stopped" }), + makeWorker({ milestoneId: "M002", state: "running" }), + ]; + + const order = determineMergeOrder(workers, "sequential", base); + // M001 should appear exactly once + assert.deepEqual(order, ["M001"]); + } finally { + cleanup(base); + } +}); + +test("mergeAllCompleted — discovers DB-complete milestones when workers show error (#2812)", async () => { + const savedCwd = process.cwd(); + const repo = createTempRepo(); + + try { + // Create milestone branch with a file + createMilestoneBranch(repo, "M011", [ + { name: "feature.ts", content: "export const feature = true;\n" }, + ]); + setupRoadmap(repo, "M011", "Feature System", ["S01: Feature module"]); + + // Set up worktree DB showing M011 is complete + setupWorktreeDb(repo, "M011"); + + // Orchestrator thinks M011 is in error (stale state) + const workers = [ + makeWorker({ milestoneId: "M011", state: "error" }), + ]; + + process.chdir(repo); + const results = await mergeAllCompleted(repo, workers, "sequential"); + + // Should find and merge M011 despite orchestrator "error" state + assert.equal(results.length, 1, "should have one result"); + assert.equal(results[0]!.milestoneId, "M011"); + assert.equal(results[0]!.success, true, `M011 merge should succeed: ${results[0]!.error}`); + } finally { + process.chdir(savedCwd); + cleanup(repo); + } +});