fix(gsd): promote milestone status from queued to active in plan-milestone (#3317)

* fix(gsd): promote milestone status from queued to active in plan-milestone

upsertMilestonePlanning() did not include title or status in its UPDATE
statement. When a milestone row was pre-created by ensureMilestoneDbRow
with status "queued", the INSERT OR IGNORE in insertMilestone() silently
skipped the row, and upsertMilestonePlanning() never updated the status.
This left the milestone permanently stuck as "queued", preventing proper
state-machine phase transitions during milestone completion.

Add title and status columns to the upsertMilestonePlanning() UPDATE
statement and pass them from handlePlanMilestone(). Uses COALESCE with
NULLIF to preserve existing values when empty strings are passed.

Closes #3022

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(ts): remove extra title arg from upsertMilestonePlanning call

* fix(test): move title into planning object for 2-param upsertMilestonePlanning

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: trek-e <trek-e@users.noreply.github.com>
This commit is contained in:
Tom Boucher 2026-04-05 01:27:25 -04:00 committed by GitHub
parent 1c95b94720
commit 5fa34d438b
4 changed files with 36 additions and 6 deletions

View file

@ -1136,11 +1136,12 @@ export function insertMilestone(m: {
});
}
export function upsertMilestonePlanning(milestoneId: string, planning: Partial<MilestonePlanningRecord>, title?: string): void {
export function upsertMilestonePlanning(milestoneId: string, planning: Partial<MilestonePlanningRecord> & { title?: string; status?: string }): void {
if (!currentDb) throw new GSDError(GSD_STALE_STATE, "gsd-db: No database open");
currentDb.prepare(
`UPDATE milestones SET
title = COALESCE(:title, title),
title = COALESCE(NULLIF(:title, ''), title),
status = COALESCE(NULLIF(:status, ''), status),
vision = COALESCE(:vision, vision),
success_criteria = COALESCE(:success_criteria, success_criteria),
key_risks = COALESCE(:key_risks, key_risks),
@ -1155,7 +1156,8 @@ export function upsertMilestonePlanning(milestoneId: string, planning: Partial<M
WHERE id = :id`,
).run({
":id": milestoneId,
":title": title ?? null,
":title": planning.title ?? "",
":status": planning.status ?? "",
":vision": planning.vision ?? null,
":success_criteria": planning.successCriteria ? JSON.stringify(planning.successCriteria) : null,
":key_risks": planning.keyRisks ? JSON.stringify(planning.keyRisks) : null,

View file

@ -38,8 +38,9 @@ test("upsertMilestonePlanning updates title when DB row pre-exists with empty ti
// Step 3: upsertMilestonePlanning should update the title
upsertMilestonePlanning("M099", {
title: "My Important Milestone",
vision: "Test vision",
}, "My Important Milestone");
});
const afterUpsert = getMilestone("M099");
assert.ok(afterUpsert);
assert.equal(

View file

@ -4,7 +4,7 @@ import { mkdtempSync, mkdirSync, rmSync, readFileSync, existsSync, writeFileSync
import { join } from 'node:path';
import { tmpdir } from 'node:os';
import { openDatabase, closeDatabase, getMilestone, getMilestoneSlices, getSlice, updateSliceStatus, deleteSlice } from '../gsd-db.ts';
import { openDatabase, closeDatabase, getMilestone, getMilestoneSlices, getSlice, updateSliceStatus, deleteSlice, insertMilestone } from '../gsd-db.ts';
import { handlePlanMilestone } from '../tools/plan-milestone.ts';
import { parseRoadmap } from '../parsers-legacy.ts';
@ -268,3 +268,28 @@ test('plan-milestone re-plan preserves completed status and updates slice fields
cleanup(base);
}
});
test('handlePlanMilestone promotes pre-existing queued milestone to active (#3022)', async () => {
const base = makeTmpBase();
const dbPath = join(base, '.gsd', 'gsd.db');
openDatabase(dbPath);
try {
// Simulate ensureMilestoneDbRow: pre-create row with status "queued"
// (this is what gsd_milestone_generate_id does)
insertMilestone({ id: 'M001', status: 'queued' });
const before = getMilestone('M001');
assert.equal(before?.status, 'queued', 'pre-condition: milestone should start as queued');
// Now plan the milestone — status should be promoted to "active"
const result = await handlePlanMilestone(validParams(), base);
assert.ok(!('error' in result), `unexpected error: ${'error' in result ? result.error : ''}`);
const after = getMilestone('M001');
assert.equal(after?.status, 'active', 'milestone status should be promoted from queued to active');
assert.equal(after?.title, 'DB-backed planning', 'milestone title should be set');
} finally {
cleanup(base);
}
});

View file

@ -241,6 +241,8 @@ export async function handlePlanMilestone(
});
upsertMilestonePlanning(params.milestoneId, {
title: params.title,
status: params.status ?? "active",
vision: params.vision,
successCriteria: params.successCriteria,
keyRisks: params.keyRisks,
@ -252,7 +254,7 @@ export async function handlePlanMilestone(
definitionOfDone: params.definitionOfDone,
requirementCoverage: params.requirementCoverage,
boundaryMapMarkdown: params.boundaryMapMarkdown,
}, params.title);
});
for (const slice of params.slices) {
// Preserve completed/done status on re-plan (#2558).