fix: preserve milestone title in upsertMilestonePlanning when DB row pre-exists (#2879) (#3247)

State reconciliation inserts milestone rows with empty titles via INSERT
OR IGNORE. When gsd_plan_milestone later calls upsertMilestonePlanning,
the UPDATE statement did not include the title column, so it stayed empty
permanently. Add title as a COALESCE-guarded column in the UPDATE and
pass it from the plan-milestone handler.

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher 2026-03-30 16:29:50 -04:00 committed by GitHub
parent c602b244c6
commit fc813bc657
3 changed files with 74 additions and 2 deletions

View file

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

View file

@ -0,0 +1,70 @@
/**
* Regression test for #2879: gsd_plan_milestone silently drops milestone title
* when the DB row pre-exists from state reconciliation.
*
* Scenario: state reconciliation inserts a milestone row with an empty title
* (INSERT OR IGNORE). When gsd_plan_milestone is called later with a title,
* the title must be persisted not silently dropped.
*/
import test from "node:test";
import assert from "node:assert/strict";
import {
openDatabase,
closeDatabase,
insertMilestone,
getMilestone,
upsertMilestonePlanning,
} from "../gsd-db.ts";
test("upsertMilestonePlanning updates title when DB row pre-exists with empty title (#2879)", () => {
try {
openDatabase(":memory:");
// Step 1: Simulate state reconciliation — inserts milestone with empty title
insertMilestone({ id: "M099", status: "active" });
const before = getMilestone("M099");
assert.ok(before, "milestone row should exist after insertMilestone");
assert.equal(before.title, "", "title should be empty after reconciliation insert");
// Step 2: Simulate gsd_plan_milestone — insertMilestone is called again
// with a title, but INSERT OR IGNORE skips it since the row exists.
insertMilestone({ id: "M099", title: "My Important Milestone", status: "active" });
const afterInsert = getMilestone("M099");
assert.ok(afterInsert);
// The INSERT OR IGNORE means title is still empty — this is the known limitation
assert.equal(afterInsert.title, "", "INSERT OR IGNORE does not update existing row");
// Step 3: upsertMilestonePlanning should update the title
upsertMilestonePlanning("M099", {
vision: "Test vision",
}, "My Important Milestone");
const afterUpsert = getMilestone("M099");
assert.ok(afterUpsert);
assert.equal(
afterUpsert.title,
"My Important Milestone",
"title must be updated by upsertMilestonePlanning when row pre-exists",
);
} finally {
closeDatabase();
}
});
test("upsertMilestonePlanning preserves existing title when no title argument provided", () => {
try {
openDatabase(":memory:");
// Insert milestone with a title
insertMilestone({ id: "M100", title: "Original Title", status: "active" });
// Call upsertMilestonePlanning without a title — should preserve existing
upsertMilestonePlanning("M100", { vision: "Updated vision" });
const after = getMilestone("M100");
assert.ok(after);
assert.equal(after.title, "Original Title", "existing title must be preserved when no title argument given");
} finally {
closeDatabase();
}
});

View file

@ -235,7 +235,7 @@ export async function handlePlanMilestone(
definitionOfDone: params.definitionOfDone,
requirementCoverage: params.requirementCoverage,
boundaryMapMarkdown: params.boundaryMapMarkdown,
});
}, params.title);
for (const slice of params.slices) {
insertSlice({