From c6c194b7e9f005e0ab8dfe0540d10b58bcbfac51 Mon Sep 17 00:00:00 2001 From: mastertyko <11311479+mastertyko@users.noreply.github.com> Date: Thu, 26 Mar 2026 23:17:25 +0100 Subject: [PATCH] fix(gsd): sync milestone DB status in parkMilestone and unparkMilestone (#2696) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: sync milestone DB status in parkMilestone and unparkMilestone parkMilestone only wrote the PARKED.md filesystem marker but never updated the DB milestones.status field. Similarly, unparkMilestone deleted the marker but left the DB at 'parked'. Because deriveStateFromDb checks BOTH the filesystem marker AND m.status, an unparked milestone was still skipped — the user saw 'All milestones complete' despite the milestone being unparked on disk. The fix adds updateMilestoneStatus() to gsd-db.ts and calls it from both parkMilestone (→ 'parked') and unparkMilestone (→ 'active'), guarded by isDbAvailable() with non-fatal try/catch. Closes #2694 * review: log DB sync failures instead of silently swallowing Replace empty catch blocks with process.stderr.write so park/unpark DB sync failures are visible. Matches the pattern used in gsd-db.ts for non-fatal DB errors. Addresses review feedback from igouss on PR #2696. --- src/resources/extensions/gsd/gsd-db.ts | 12 +++ .../extensions/gsd/milestone-actions.ts | 17 ++++ .../extensions/gsd/tests/park-db-sync.test.ts | 85 +++++++++++++++++++ 3 files changed, 114 insertions(+) create mode 100644 src/resources/extensions/gsd/tests/park-db-sync.test.ts diff --git a/src/resources/extensions/gsd/gsd-db.ts b/src/resources/extensions/gsd/gsd-db.ts index 20a9c11a8..7403baa6a 100644 --- a/src/resources/extensions/gsd/gsd-db.ts +++ b/src/resources/extensions/gsd/gsd-db.ts @@ -1485,6 +1485,18 @@ export function getMilestone(id: string): MilestoneRow | null { return rowToMilestone(row); } +/** + * Update a milestone's status in the database. + * Used by park/unpark to keep the DB in sync with the filesystem marker. + * See: https://github.com/gsd-build/gsd-2/issues/2694 + */ +export function updateMilestoneStatus(milestoneId: string, status: string): void { + if (!currentDb) throw new GSDError(GSD_STALE_STATE, "gsd-db: No database open"); + currentDb.prepare( + `UPDATE milestones SET status = :status WHERE id = :id`, + ).run({ ":status": status, ":id": milestoneId }); +} + export function getActiveMilestoneFromDb(): MilestoneRow | null { if (!currentDb) return null; const row = currentDb.prepare( diff --git a/src/resources/extensions/gsd/milestone-actions.ts b/src/resources/extensions/gsd/milestone-actions.ts index 79851f178..7615a1eb9 100644 --- a/src/resources/extensions/gsd/milestone-actions.ts +++ b/src/resources/extensions/gsd/milestone-actions.ts @@ -20,6 +20,7 @@ import { } from "./paths.js"; import { invalidateAllCaches } from "./cache.js"; import { loadQueueOrder, saveQueueOrder } from "./queue-order.js"; +import { isDbAvailable, updateMilestoneStatus } from "./gsd-db.js"; // ─── Park ────────────────────────────────────────────────────────────────── @@ -52,6 +53,14 @@ export function parkMilestone(basePath: string, milestoneId: string, reason: str ].join("\n"); writeFileSync(parkedPath, content, "utf-8"); + // Sync DB status so deriveStateFromDb also skips this milestone (#2694) + if (isDbAvailable()) { + try { + updateMilestoneStatus(milestoneId, "parked"); + } catch (err) { + process.stderr.write(`gsd: parkMilestone DB sync failed for ${milestoneId}: ${(err as Error).message}\n`); + } + } invalidateAllCaches(); return true; } @@ -70,6 +79,14 @@ export function unparkMilestone(basePath: string, milestoneId: string): boolean if (!existsSync(parkedPath)) return false; // not parked unlinkSync(parkedPath); + // Sync DB status so deriveStateFromDb picks up the unparked milestone (#2694) + if (isDbAvailable()) { + try { + updateMilestoneStatus(milestoneId, "active"); + } catch (err) { + process.stderr.write(`gsd: unparkMilestone DB sync failed for ${milestoneId}: ${(err as Error).message}\n`); + } + } invalidateAllCaches(); return true; } diff --git a/src/resources/extensions/gsd/tests/park-db-sync.test.ts b/src/resources/extensions/gsd/tests/park-db-sync.test.ts new file mode 100644 index 000000000..0580337e2 --- /dev/null +++ b/src/resources/extensions/gsd/tests/park-db-sync.test.ts @@ -0,0 +1,85 @@ +/** + * Regression test for #2694: parkMilestone and unparkMilestone must + * update the DB milestone status alongside the filesystem marker. + * + * Without this, deriveStateFromDb skips unparked milestones because + * the DB still has status='parked', causing "All milestones complete". + */ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from "node:fs"; +import { join } from "node:path"; +import { tmpdir } from "node:os"; + +import { parkMilestone, unparkMilestone } from "../milestone-actions.ts"; +import { + openDatabase, + closeDatabase, + insertMilestone, + getMilestone, +} from "../gsd-db.ts"; + +function createBase(): string { + const base = mkdtempSync(join(tmpdir(), "gsd-park-db-")); + mkdirSync(join(base, ".gsd", "milestones", "M001"), { recursive: true }); + writeFileSync( + join(base, ".gsd", "milestones", "M001", "M001-CONTEXT.md"), + "# M001\n\nContext.", + ); + return base; +} + +test("parkMilestone updates DB status to 'parked' (#2694)", () => { + const base = createBase(); + try { + openDatabase(":memory:"); + insertMilestone({ id: "M001", title: "Test", status: "active" }); + + assert.equal(getMilestone("M001")!.status, "active", "starts active"); + + parkMilestone(base, "M001", "deprioritized"); + + assert.equal(getMilestone("M001")!.status, "parked", "DB status should be parked"); + + closeDatabase(); + } finally { + closeDatabase(); + rmSync(base, { recursive: true, force: true }); + } +}); + +test("unparkMilestone updates DB status to 'active' (#2694)", () => { + const base = createBase(); + try { + openDatabase(":memory:"); + insertMilestone({ id: "M001", title: "Test", status: "active" }); + + // Park first + parkMilestone(base, "M001", "deprioritized"); + assert.equal(getMilestone("M001")!.status, "parked"); + + // Unpark + unparkMilestone(base, "M001"); + assert.equal(getMilestone("M001")!.status, "active", "DB status should be active after unpark"); + + closeDatabase(); + } finally { + closeDatabase(); + rmSync(base, { recursive: true, force: true }); + } +}); + +test("park/unpark are safe when DB is not available (#2694 guard)", () => { + const base = createBase(); + try { + // No openDatabase — DB not available + // park/unpark should still work (filesystem-only, no throw) + const parked = parkMilestone(base, "M001", "test"); + assert.ok(parked, "parkMilestone succeeds without DB"); + + const unparked = unparkMilestone(base, "M001"); + assert.ok(unparked, "unparkMilestone succeeds without DB"); + } finally { + rmSync(base, { recursive: true, force: true }); + } +});