From 8dcb2f91950127d6a18da5b740a45a82319208f2 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Mon, 16 Mar 2026 23:28:08 -0400 Subject: [PATCH] fix: sync completed-units.json across worktree boundary (#769) (#780) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to #774. When GSD runs in worktree isolation mode, completed-units.json can fragment across project root and worktree locations. If a session crashes or the worktree is removed after milestone merge, keys written to the worktree are lost — causing already-completed units to be re-dispatched. Two fixes: 1. syncStateToProjectRoot() now performs a set-union merge of completed-units.json from worktree into project root. 2. After worktree entry at startup, loadPersistedKeys() runs against both project root and worktree so the in-memory completedKeySet contains the union of both locations. Co-authored-by: Lex Christopherson Co-authored-by: Claude Opus 4.6 (1M context) --- src/resources/extensions/gsd/auto.ts | 25 +++++++- .../gsd/tests/auto-recovery.test.ts | 62 +++++++++++++++++++ 2 files changed, 86 insertions(+), 1 deletion(-) diff --git a/src/resources/extensions/gsd/auto.ts b/src/resources/extensions/gsd/auto.ts index 820caa06d..8e95668b2 100644 --- a/src/resources/extensions/gsd/auto.ts +++ b/src/resources/extensions/gsd/auto.ts @@ -197,7 +197,23 @@ function syncStateToProjectRoot(worktreePath: string, projectRoot: string, miles } } catch { /* non-fatal */ } - // 3. Runtime records — unit dispatch state used by selfHealRuntimeRecords(). + // 3. Merge completed-units.json (set-union of both locations) + // Prevents already-completed units from being re-dispatched after crash/restart. + const srcKeysFile = join(wtGsd, "completed-units.json"); + const dstKeysFile = join(prGsd, "completed-units.json"); + if (existsSync(srcKeysFile)) { + try { + const srcKeys: string[] = JSON.parse(readFileSync(srcKeysFile, "utf8")); + let dstKeys: string[] = []; + if (existsSync(dstKeysFile)) { + try { dstKeys = JSON.parse(readFileSync(dstKeysFile, "utf8")); } catch { /* ignore corrupt dst */ } + } + const merged = [...new Set([...dstKeys, ...srcKeys])]; + writeFileSync(dstKeysFile, JSON.stringify(merged, null, 2)); + } catch { /* non-fatal */ } + } + + // 4. Runtime records — unit dispatch state used by selfHealRuntimeRecords(). // Without this, a crash during a unit leaves the runtime record only in the // worktree. If the next session resolves basePath before worktree re-entry, // selfHeal can't find or clear the stale record (#769). @@ -1102,6 +1118,13 @@ export async function startAuto( } // Re-register SIGTERM handler with the original basePath (lock lives there) registerSigtermHandler(originalBasePath); + + // After worktree entry, load completed keys from BOTH locations (project root + // + worktree) so the in-memory set is the union. Prevents re-dispatch of units + // completed in either location after crash/restart (#769). + if (basePath !== originalBasePath) { + loadPersistedKeys(basePath, completedKeySet); + } } catch (err) { // Worktree creation is non-fatal — continue in the project root. ctx.ui.notify( diff --git a/src/resources/extensions/gsd/tests/auto-recovery.test.ts b/src/resources/extensions/gsd/tests/auto-recovery.test.ts index ed3334d81..597669fe9 100644 --- a/src/resources/extensions/gsd/tests/auto-recovery.test.ts +++ b/src/resources/extensions/gsd/tests/auto-recovery.test.ts @@ -274,6 +274,68 @@ test("removePersistedKey is safe when file doesn't exist", () => { } }); +// ─── Dual-load across worktree boundary (#769) ─────────────────────────── + +test("loadPersistedKeys unions keys from project root and worktree", () => { + // Simulate two separate .gsd directories (project root + worktree) + // each with a different set of completed keys. Loading from both + // into the same Set should produce the union. + const projectRoot = makeTmpBase(); + const worktree = makeTmpBase(); + try { + // Persist different keys in each location + persistCompletedKey(projectRoot, "execute-task/M001/S01/T01"); + persistCompletedKey(projectRoot, "plan-slice/M001/S02"); + + persistCompletedKey(worktree, "execute-task/M001/S01/T02"); + persistCompletedKey(worktree, "plan-slice/M001/S02"); // overlap + + // Load from both into the same set (mimicking startup dual-load) + const keys = new Set(); + loadPersistedKeys(projectRoot, keys); + loadPersistedKeys(worktree, keys); + + assert.ok(keys.has("execute-task/M001/S01/T01"), "key from project root"); + assert.ok(keys.has("plan-slice/M001/S02"), "shared key"); + assert.ok(keys.has("execute-task/M001/S01/T02"), "key from worktree"); + assert.equal(keys.size, 3, "union should deduplicate overlapping keys"); + } finally { + cleanup(projectRoot); + cleanup(worktree); + } +}); + +test("completed-units.json set-union merge produces correct result", () => { + // Verify that a manual set-union merge (as done in syncStateToProjectRoot) + // correctly merges two JSON arrays of keys. + const projectRoot = makeTmpBase(); + const worktree = makeTmpBase(); + try { + // Write keys to both locations + const prKeysFile = join(projectRoot, ".gsd", "completed-units.json"); + const wtKeysFile = join(worktree, ".gsd", "completed-units.json"); + + writeFileSync(prKeysFile, JSON.stringify(["a", "b"])); + writeFileSync(wtKeysFile, JSON.stringify(["b", "c", "d"])); + + // Perform the same merge logic used in syncStateToProjectRoot + const srcKeys: string[] = JSON.parse(readFileSync(wtKeysFile, "utf8")); + let dstKeys: string[] = []; + if (existsSync(prKeysFile)) { + dstKeys = JSON.parse(readFileSync(prKeysFile, "utf8")); + } + const merged = [...new Set([...dstKeys, ...srcKeys])]; + writeFileSync(prKeysFile, JSON.stringify(merged, null, 2)); + + // Verify the merged result + const result: string[] = JSON.parse(readFileSync(prKeysFile, "utf8")); + assert.deepStrictEqual(result.sort(), ["a", "b", "c", "d"]); + } finally { + cleanup(projectRoot); + cleanup(worktree); + } +}); + // ─── verifyExpectedArtifact: parse cache collision regression ───────────── test("verifyExpectedArtifact detects roadmap [x] change despite parse cache", () => {