fix: reconcile disk milestones into empty DB before deriveStateFromDb guard (#2686)
When the milestones DB table has 0 rows (e.g. failed initial migration per #2529), deriveState fell through to the filesystem path because deriveStateFromDb was only called when dbMilestones.length > 0. The reconciliation code inside deriveStateFromDb was unreachable — the very condition it was supposed to fix gated its execution. The fix moves disk→DB sync into deriveState itself: when the DB is available but empty, scan disk milestone directories and insert them before the length check. This ensures the DB path activates correctly even after a failed migration. Closes #2631
This commit is contained in:
parent
1f10ed9585
commit
53d2da15b5
3 changed files with 132 additions and 8 deletions
|
|
@ -211,7 +211,24 @@ export async function deriveState(basePath: string): Promise<GSDState> {
|
|||
|
||||
// Dual-path: try DB-backed derivation first when hierarchy tables are populated
|
||||
if (isDbAvailable()) {
|
||||
const dbMilestones = getAllMilestones();
|
||||
let dbMilestones = getAllMilestones();
|
||||
|
||||
// Disk→DB reconciliation (#2631): when the milestones table is empty
|
||||
// (e.g. failed initial migration per #2529), the reconciliation code
|
||||
// inside deriveStateFromDb is unreachable. Populate from disk here so
|
||||
// the DB path activates correctly.
|
||||
if (dbMilestones.length === 0) {
|
||||
const diskIds = findMilestoneIds(basePath);
|
||||
let synced = false;
|
||||
for (const diskId of diskIds) {
|
||||
if (!isGhostMilestone(basePath, diskId)) {
|
||||
insertMilestone({ id: diskId, status: 'active' });
|
||||
synced = true;
|
||||
}
|
||||
}
|
||||
if (synced) dbMilestones = getAllMilestones();
|
||||
}
|
||||
|
||||
if (dbMilestones.length > 0) {
|
||||
const stopDbTimer = debugTime("derive-state-db");
|
||||
result = await deriveStateFromDb(basePath);
|
||||
|
|
|
|||
|
|
@ -14,6 +14,7 @@ import {
|
|||
getAllMilestones,
|
||||
insertSlice,
|
||||
insertTask,
|
||||
updateTaskStatus,
|
||||
} from '../gsd-db.ts';
|
||||
// ─── Fixture Helpers ───────────────────────────────────────────────────────
|
||||
|
||||
|
|
@ -116,10 +117,17 @@ describe('derive-state-db', async () => {
|
|||
invalidateStateCache();
|
||||
const fileState = await deriveState(base);
|
||||
|
||||
// Now open DB, insert matching artifacts
|
||||
// Now open DB, insert matching artifacts + milestone hierarchy
|
||||
openDatabase(':memory:');
|
||||
assert.ok(isDbAvailable(), 'db-match: DB is available after open');
|
||||
|
||||
// Insert milestone hierarchy so deriveState takes the DB path (#2631 fix)
|
||||
insertMilestone({ id: 'M001', title: 'Test Milestone', status: 'active' });
|
||||
insertSlice({ id: 'S01', milestoneId: 'M001', title: 'First Slice', status: 'active', risk: 'low', depends: [] });
|
||||
insertSlice({ id: 'S02', milestoneId: 'M001', title: 'Second Slice', status: 'pending', risk: 'low', depends: ['S01'] });
|
||||
insertTask({ id: 'T01', sliceId: 'S01', milestoneId: 'M001', title: 'First Task', status: 'pending' });
|
||||
insertTask({ id: 'T02', sliceId: 'S01', milestoneId: 'M001', title: 'Done Task', status: 'complete' });
|
||||
|
||||
insertArtifactRow('milestones/M001/M001-ROADMAP.md', ROADMAP_CONTENT, {
|
||||
artifact_type: 'roadmap',
|
||||
milestone_id: 'M001',
|
||||
|
|
@ -197,18 +205,21 @@ describe('derive-state-db', async () => {
|
|||
writeFile(base, 'milestones/M001/slices/S01/tasks/.gitkeep', '');
|
||||
writeFile(base, 'milestones/M001/slices/S01/tasks/T01-PLAN.md', '# T01 Plan');
|
||||
|
||||
// Open DB but insert nothing — empty artifacts table
|
||||
// Open DB but insert nothing — empty tables.
|
||||
// With #2631 fix, deriveState will sync disk milestones into DB
|
||||
// and then take the DB path. The result should still reflect the
|
||||
// disk milestone correctly.
|
||||
openDatabase(':memory:');
|
||||
assert.ok(isDbAvailable(), 'empty-db: DB is available');
|
||||
|
||||
invalidateStateCache();
|
||||
const state = await deriveState(base);
|
||||
|
||||
// Should still work via cachedLoadFile → loadFile disk fallback
|
||||
assert.deepStrictEqual(state.phase, 'executing', 'empty-db: phase is executing');
|
||||
// Milestone should be detected (synced from disk)
|
||||
assert.deepStrictEqual(state.activeMilestone?.id, 'M001', 'empty-db: activeMilestone is M001');
|
||||
assert.deepStrictEqual(state.activeSlice?.id, 'S01', 'empty-db: activeSlice is S01');
|
||||
assert.deepStrictEqual(state.activeTask?.id, 'T01', 'empty-db: activeTask is T01');
|
||||
// The DB path without explicit slice/task rows may derive a different
|
||||
// phase than the filesystem path, but the milestone must be found.
|
||||
assert.ok(state.activeMilestone !== null, 'empty-db: activeMilestone is not null');
|
||||
|
||||
closeDatabase();
|
||||
} finally {
|
||||
|
|
@ -228,8 +239,12 @@ describe('derive-state-db', async () => {
|
|||
writeFile(base, 'milestones/M001/slices/S01/tasks/T01-PLAN.md', '# T01 Plan');
|
||||
writeFile(base, 'REQUIREMENTS.md', REQUIREMENTS_CONTENT);
|
||||
|
||||
// Open DB but only insert the roadmap — plan and requirements missing from DB
|
||||
// Open DB — insert milestone hierarchy + partial artifacts (#2631 fix)
|
||||
openDatabase(':memory:');
|
||||
insertMilestone({ id: 'M001', title: 'Test Milestone', status: 'active' });
|
||||
insertSlice({ id: 'S01', milestoneId: 'M001', title: 'First Slice', status: 'active', risk: 'low', depends: [] });
|
||||
insertTask({ id: 'T01', sliceId: 'S01', milestoneId: 'M001', title: 'First Task', status: 'pending' });
|
||||
// Only insert the roadmap artifact — plan and requirements missing from DB
|
||||
insertArtifactRow('milestones/M001/M001-ROADMAP.md', ROADMAP_CONTENT, {
|
||||
artifact_type: 'roadmap',
|
||||
milestone_id: 'M001',
|
||||
|
|
@ -314,6 +329,13 @@ describe('derive-state-db', async () => {
|
|||
|
||||
// Put roadmap content in DB only
|
||||
openDatabase(':memory:');
|
||||
// Insert milestone rows so deriveState takes the DB path (#2631 fix:
|
||||
// empty milestones table now triggers disk→DB sync, which would create
|
||||
// rows without slices — insert explicitly to get the full DB path).
|
||||
insertMilestone({ id: 'M001', title: 'First Milestone', status: 'complete' });
|
||||
insertMilestone({ id: 'M002', title: 'Second Milestone', status: 'active' });
|
||||
insertSlice({ id: 'S01', milestoneId: 'M001', title: 'Done', status: 'complete', risk: 'low', depends: [] });
|
||||
insertSlice({ id: 'S01', milestoneId: 'M002', title: 'In Progress', status: 'active', risk: 'low', depends: [] });
|
||||
insertArtifactRow('milestones/M001/M001-ROADMAP.md', completedRoadmap, {
|
||||
artifact_type: 'roadmap',
|
||||
milestone_id: 'M001',
|
||||
|
|
@ -355,6 +377,10 @@ describe('derive-state-db', async () => {
|
|||
writeFile(base, 'milestones/M001/slices/S01/tasks/T01-PLAN.md', '# T01 Plan');
|
||||
|
||||
openDatabase(':memory:');
|
||||
// Insert milestone/slice/task rows so deriveState takes the DB path (#2631 fix)
|
||||
insertMilestone({ id: 'M001', title: 'Test Milestone', status: 'active' });
|
||||
insertSlice({ id: 'S01', milestoneId: 'M001', title: 'First Slice', status: 'active', risk: 'low', depends: [] });
|
||||
insertTask({ id: 'T01', sliceId: 'S01', milestoneId: 'M001', title: 'First Task', status: 'pending' });
|
||||
insertArtifactRow('milestones/M001/M001-ROADMAP.md', ROADMAP_CONTENT, {
|
||||
artifact_type: 'roadmap',
|
||||
milestone_id: 'M001',
|
||||
|
|
@ -378,6 +404,8 @@ describe('derive-state-db', async () => {
|
|||
});
|
||||
// Also update file on disk (cachedLoadFile may read from disk for some paths)
|
||||
writeFile(base, 'milestones/M001/slices/S01/S01-PLAN.md', updatedPlan);
|
||||
// Update task status in DB so DB-path also sees completion (#2631 fix)
|
||||
updateTaskStatus('M001', 'S01', 'T01', 'complete');
|
||||
|
||||
// Without invalidation, should return cached result (T01 still active)
|
||||
const state2 = await deriveState(base);
|
||||
|
|
|
|||
|
|
@ -0,0 +1,79 @@
|
|||
/**
|
||||
* Regression test for #2631: deriveState disk→DB reconciliation must
|
||||
* run even when the milestones table starts empty.
|
||||
*
|
||||
* When getAllMilestones() returns [] (e.g. after a failed initial migration),
|
||||
* the reconciliation code inside deriveStateFromDb was unreachable because
|
||||
* deriveState only called it when dbMilestones.length > 0. The fix moves
|
||||
* disk→DB sync into deriveState itself, before the length check.
|
||||
*/
|
||||
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 { deriveState, invalidateStateCache } from "../state.ts";
|
||||
import {
|
||||
openDatabase,
|
||||
closeDatabase,
|
||||
getAllMilestones,
|
||||
} from "../gsd-db.ts";
|
||||
|
||||
test("deriveState populates empty DB from disk milestones (#2631)", async () => {
|
||||
const base = mkdtempSync(join(tmpdir(), "gsd-empty-db-"));
|
||||
mkdirSync(join(base, ".gsd", "milestones", "M001"), { recursive: true });
|
||||
|
||||
try {
|
||||
// Create a milestone on disk with a CONTEXT file (not a ghost)
|
||||
writeFileSync(
|
||||
join(base, ".gsd", "milestones", "M001", "M001-CONTEXT.md"),
|
||||
"# M001: Test Milestone\n\nSome context about this milestone.",
|
||||
);
|
||||
|
||||
// Open DB — milestones table is empty (simulating failed migration)
|
||||
openDatabase(":memory:");
|
||||
const before = getAllMilestones();
|
||||
assert.equal(before.length, 0, "DB should start with 0 milestones");
|
||||
|
||||
// deriveState should reconcile disk → DB
|
||||
invalidateStateCache();
|
||||
const state = await deriveState(base);
|
||||
|
||||
// After deriveState, the DB should now have the disk milestone
|
||||
const after = getAllMilestones();
|
||||
assert.ok(after.length > 0, "DB should have milestones after reconciliation");
|
||||
assert.equal(after[0]!.id, "M001", "reconciled milestone should be M001");
|
||||
|
||||
// State should reflect the milestone (not "No milestones found")
|
||||
assert.ok(
|
||||
state.activeMilestone !== null,
|
||||
"activeMilestone should not be null after reconciliation",
|
||||
);
|
||||
|
||||
closeDatabase();
|
||||
} finally {
|
||||
closeDatabase();
|
||||
rmSync(base, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test("deriveState does NOT insert ghost milestones into DB (#2631 guard)", async () => {
|
||||
const base = mkdtempSync(join(tmpdir(), "gsd-empty-db-"));
|
||||
// Create a ghost milestone directory (empty — no CONTEXT, no ROADMAP)
|
||||
mkdirSync(join(base, ".gsd", "milestones", "M001"), { recursive: true });
|
||||
|
||||
try {
|
||||
openDatabase(":memory:");
|
||||
invalidateStateCache();
|
||||
await deriveState(base);
|
||||
|
||||
const milestones = getAllMilestones();
|
||||
assert.equal(milestones.length, 0, "ghost milestone should NOT be inserted");
|
||||
|
||||
closeDatabase();
|
||||
} finally {
|
||||
closeDatabase();
|
||||
rmSync(base, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
Loading…
Add table
Reference in a new issue