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); + } +});