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/<MID>/.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 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher 2026-03-30 15:52:21 -04:00 committed by GitHub
parent 348c399c9d
commit 16d31f2253
2 changed files with 197 additions and 4 deletions

View file

@ -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/<MID>/.gsd/gsd.db for each worktree directory.
*/
function discoverDbCompletedMilestones(basePath: string): Set<string> {
const completed = new Set<string>();
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<string>();
// 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<MergeResult[]> {
const mergeOrder = determineMergeOrder(workers, order);
const mergeOrder = determineMergeOrder(workers, order, basePath);
const results: MergeResult[] = [];
for (const mid of mergeOrder) {

View file

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