fix(FN-743): prevent duplicate comments from steeringComments merge in rowToTask

- Stop merging steeringComments into comments array in rowToTask to prevent duplication
- Add regression tests verifying comments and steeringComments remain separate
- Add changeset for the comments deduplication fix
This commit is contained in:
gsxdsm
2026-04-02 20:17:01 -07:00
parent dacd074700
commit 4d46b99460
3 changed files with 81 additions and 5 deletions

View File

@@ -1976,6 +1976,71 @@ Task with acceptance criteria
expect(fetched.steeringComments![0].text).toBe("Focus on error handling");
});
it("steering comments do not duplicate in comments across read-write cycles", async () => {
const task = await createTestTask();
// Add a steering comment (writes to both comments and steeringComments columns)
await store.addSteeringComment(task.id, "Focus on error handling");
// Read the task back — comments should have exactly 1 entry
const read1 = await store.getTask(task.id);
expect(read1.comments).toHaveLength(1);
expect(read1.steeringComments).toHaveLength(1);
// Simulate a write-back (updateTask writes via upsertTask)
await store.updateTask(task.id, { status: "specifying" });
// Read again — should still have exactly 1 comment, not 2
const read2 = await store.getTask(task.id);
expect(read2.comments).toHaveLength(1);
expect(read2.comments![0].text).toBe("Focus on error handling");
});
it("no duplication accumulation over multiple read-write cycles with steering comments", async () => {
const task = await createTestTask();
await store.addSteeringComment(task.id, "Comment A");
await store.addSteeringComment(task.id, "Comment B");
// Perform 5 read-write cycles
for (let i = 0; i < 5; i++) {
const fetched = await store.getTask(task.id);
expect(fetched.comments).toHaveLength(2);
expect(fetched.steeringComments).toHaveLength(2);
// Write back via an innocuous update
await store.updateTask(task.id, { status: "specifying" });
}
// Final read — still exactly 2 comments
const final = await store.getTask(task.id);
expect(final.comments).toHaveLength(2);
expect(final.comments!.map(c => c.text).sort()).toEqual(["Comment A", "Comment B"]);
});
it("mixed regular and steering comments maintain correct counts through cycles", async () => {
const task = await createTestTask();
// Add 1 regular comment and 1 steering comment
await store.addTaskComment(task.id, "Regular note", "alice");
await store.addSteeringComment(task.id, "Steering note");
// Should have 2 comments total, 1 steering comment
const read1 = await store.getTask(task.id);
expect(read1.comments).toHaveLength(2);
expect(read1.steeringComments).toHaveLength(1);
// Perform 3 read-write cycles
for (let i = 0; i < 3; i++) {
const fetched = await store.getTask(task.id);
expect(fetched.comments).toHaveLength(2);
await store.updateTask(task.id, { status: "specifying" });
}
const final = await store.getTask(task.id);
expect(final.comments).toHaveLength(2);
expect(final.steeringComments).toHaveLength(1);
});
it("regular addComment on done task still creates refinement", async () => {
const task = await store.createTask({ description: "Original task" });
await store.moveTask(task.id, "todo");

View File

@@ -193,11 +193,17 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
return sc && sc.length > 0 ? sc : undefined;
})(),
comments: (() => {
// Merge legacy steeringComments and comments into unified comments field
const legacySteering = fromJson<Array<{ id: string; text: string; createdAt: string; author: string }>>(row.steeringComments) || [];
const regularComments = fromJson<import("./types.js").TaskComment[]>(row.comments) || [];
const merged = [...legacySteering, ...regularComments];
return merged.length > 0 ? merged : undefined;
// Comments column already contains steering comments (addSteeringComment calls addComment).
// Do NOT merge steeringComments here — that caused duplication on every read-write cycle.
const c = fromJson<import("./types.js").TaskComment[]>(row.comments) || [];
// Deduplicate by id to recover from prior corruption
const seen = new Set<string>();
const deduped = c.filter(entry => {
if (seen.has(entry.id)) return false;
seen.add(entry.id);
return true;
});
return deduped.length > 0 ? deduped : undefined;
})(),
workflowStepResults: (() => { const w = fromJson<import("./types.js").WorkflowStepResult[]>(row.workflowStepResults); return w && w.length > 0 ? w : undefined; })(),
prInfo: fromJson<import("./types.js").PrInfo>(row.prInfo),