fix: clear task verification status on revert
This commit is contained in:
parent
3e002ca698
commit
4289946e11
2 changed files with 224 additions and 1 deletions
|
|
@ -166,12 +166,24 @@ export function updateTaskStatus(
|
|||
typeof purposeTrace === "string" && purposeTrace.trim().length > 0
|
||||
? purposeTrace.trim()
|
||||
: null;
|
||||
// R072: When reverting a task to a non-complete status (e.g. the safety
|
||||
// file-change guard or hook retry path calls updateTaskStatus("pending")),
|
||||
// clear verification_status so it cannot drift out of sync with status.
|
||||
// Without this, completeTaskAtomic sets both fields atomically but a later
|
||||
// status-revert leaves verification_status=all_pass while status=pending,
|
||||
// which triggers the status-completion-drift quarantine in state-db.js and
|
||||
// prevents the task from being re-dispatched. Completion writes must go
|
||||
// through completeTaskAtomic; this function is revert/lifecycle only.
|
||||
const isComplete = status === "complete" || status === "done";
|
||||
const verificationStatusUpdate = isComplete
|
||||
? "" // completing via this function is non-standard; don't touch verification_status
|
||||
: ", verification_status = ''";
|
||||
currentDb
|
||||
.prepare(`UPDATE tasks SET
|
||||
status = :status,
|
||||
completed_at = :completed_at,
|
||||
task_status = :task_status,
|
||||
purpose_trace = COALESCE(:purpose_trace, purpose_trace)
|
||||
purpose_trace = COALESCE(:purpose_trace, purpose_trace)${verificationStatusUpdate}
|
||||
WHERE milestone_id = :milestone_id AND slice_id = :slice_id AND id = :id`)
|
||||
.run({
|
||||
":status": status,
|
||||
|
|
|
|||
|
|
@ -0,0 +1,211 @@
|
|||
/**
|
||||
* atomic-completion-writer-drift.test.mjs — R072 regression guard.
|
||||
*
|
||||
* Purpose: prove that the live M048/S02/T01 drift pattern cannot recur:
|
||||
* - completeTaskAtomic writes status=complete + verification_status atomically
|
||||
* - updateTaskStatus("pending") revert clears verification_status so the row
|
||||
* cannot enter the status-completion-drift quarantine in state-db.js
|
||||
*
|
||||
* Consumer: CI, operator-run after any change to sf-db-tasks.js write paths.
|
||||
*
|
||||
* Contract: after completeTaskAtomic + updateTaskStatus("pending"):
|
||||
* status=pending, completed_at=null, verification_status=''
|
||||
*/
|
||||
import assert from "node:assert/strict";
|
||||
import { mkdirSync, mkdtempSync, rmSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { afterEach, test } from "vitest";
|
||||
import {
|
||||
closeDatabase,
|
||||
completeTaskAtomic,
|
||||
getTask,
|
||||
insertMilestone,
|
||||
insertSlice,
|
||||
insertTask,
|
||||
openDatabase,
|
||||
updateTaskStatus,
|
||||
} from "../sf-db.js";
|
||||
|
||||
const tmpDirs = [];
|
||||
|
||||
afterEach(() => {
|
||||
closeDatabase();
|
||||
while (tmpDirs.length > 0) {
|
||||
const dir = tmpDirs.pop();
|
||||
if (dir) rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
function makeProject(milestoneId = "M048", sliceId = "S02") {
|
||||
const project = mkdtempSync(join(tmpdir(), "sf-r072-drift-"));
|
||||
tmpDirs.push(project);
|
||||
mkdirSync(
|
||||
join(project, ".sf", "milestones", milestoneId, "slices", sliceId),
|
||||
{
|
||||
recursive: true,
|
||||
},
|
||||
);
|
||||
assert.equal(openDatabase(join(project, ".sf", "sf.db")), true);
|
||||
insertMilestone({ id: milestoneId, title: milestoneId, status: "active" });
|
||||
insertSlice({
|
||||
milestoneId,
|
||||
id: sliceId,
|
||||
title: sliceId,
|
||||
status: "active",
|
||||
sequence: 1,
|
||||
});
|
||||
return project;
|
||||
}
|
||||
|
||||
// ── Live repro scenario: M048/S02/T01 ────────────────────────────────────────
|
||||
|
||||
test("R072_repro_completeTaskAtomic_then_status_revert_must_clear_verification_status", () => {
|
||||
// Arrange: task starts pending (as inserted by plan-slice/plan-task)
|
||||
makeProject("M048", "S02");
|
||||
insertTask({
|
||||
milestoneId: "M048",
|
||||
sliceId: "S02",
|
||||
id: "T01",
|
||||
title: "Created smoke-fixture-constants.js",
|
||||
status: "pending",
|
||||
});
|
||||
|
||||
// Act 1: executor calls complete_task → completeTaskAtomic fires
|
||||
completeTaskAtomic("M048", "S02", "T01", {
|
||||
status: "complete",
|
||||
completedAt: "2026-05-17T12:07:48.587Z",
|
||||
narrative: "Smoke fixture constants created.",
|
||||
verificationResult: "all tests pass",
|
||||
verificationStatus: "all_pass",
|
||||
summaryMd: "---\ncompleted_at: 2026-05-17T12:07:48.587Z\n---",
|
||||
});
|
||||
|
||||
const afterComplete = getTask("M048", "S02", "T01");
|
||||
assert.equal(
|
||||
afterComplete.status,
|
||||
"complete",
|
||||
"must be complete after completeTaskAtomic",
|
||||
);
|
||||
assert.equal(
|
||||
afterComplete.verification_status,
|
||||
"all_pass",
|
||||
"verification_status must be all_pass",
|
||||
);
|
||||
assert.ok(afterComplete.completed_at, "completed_at must be set");
|
||||
|
||||
// Act 2: safety file-change guard or hook retry path reverts via updateTaskStatus
|
||||
// (this is the exact call at auto-post-unit.js:1012 and :1579)
|
||||
updateTaskStatus("M048", "S02", "T01", "pending", null);
|
||||
|
||||
// Assert: no drift — both status AND verification_status are reset
|
||||
const afterRevert = getTask("M048", "S02", "T01");
|
||||
assert.equal(afterRevert.status, "pending", "status must revert to pending");
|
||||
assert.equal(afterRevert.completed_at, null, "completed_at must be cleared");
|
||||
assert.equal(
|
||||
afterRevert.verification_status,
|
||||
"",
|
||||
"verification_status must be cleared on revert — R072 drift guard",
|
||||
);
|
||||
});
|
||||
|
||||
// ── Negative: outcome=continue must not trigger completion ────────────────────
|
||||
|
||||
test("R072_task_with_outcome_continue_must_not_be_marked_complete", () => {
|
||||
makeProject("M048", "S02");
|
||||
insertTask({
|
||||
milestoneId: "M048",
|
||||
sliceId: "S02",
|
||||
id: "T02",
|
||||
title: "Wire last-green ledger",
|
||||
status: "pending",
|
||||
});
|
||||
|
||||
// outcome=continue: executor does NOT call complete_task, no DB write happens
|
||||
// The task must remain pending with empty verification_status
|
||||
const task = getTask("M048", "S02", "T02");
|
||||
assert.equal(task.status, "pending");
|
||||
assert.equal(task.verification_status, "");
|
||||
assert.equal(task.completed_at, null);
|
||||
});
|
||||
|
||||
// ── Negative: completeTaskAtomic without verificationEvidence ─────────────────
|
||||
|
||||
test("R072_completeTaskAtomic_without_verification_evidence_sets_empty_status", () => {
|
||||
makeProject("M048", "S02");
|
||||
insertTask({
|
||||
milestoneId: "M048",
|
||||
sliceId: "S02",
|
||||
id: "T03",
|
||||
title: "No evidence task",
|
||||
status: "pending",
|
||||
});
|
||||
|
||||
// complete_task called with no verificationEvidence → verificationStatus="" (not all_pass)
|
||||
completeTaskAtomic("M048", "S02", "T03", {
|
||||
status: "complete",
|
||||
narrative: "Done.",
|
||||
// No verificationResult, no verificationStatus
|
||||
});
|
||||
|
||||
const task = getTask("M048", "S02", "T03");
|
||||
assert.equal(task.status, "complete");
|
||||
// When no verification evidence supplied, verificationStatus should be empty string
|
||||
assert.equal(
|
||||
task.verification_status,
|
||||
"",
|
||||
"empty verification_status when no evidence",
|
||||
);
|
||||
});
|
||||
|
||||
// ── Positive: updateTaskStatus with complete status leaves verification_status alone ──
|
||||
|
||||
test("R072_updateTaskStatus_to_complete_does_not_clear_verification_status", () => {
|
||||
// Scenario: some code path calls updateTaskStatus("complete") on a row
|
||||
// that already has verification_status set; it must be preserved.
|
||||
makeProject("M048", "S02");
|
||||
insertTask({
|
||||
milestoneId: "M048",
|
||||
sliceId: "S02",
|
||||
id: "T04",
|
||||
title: "Already has verif status",
|
||||
status: "pending",
|
||||
verificationStatus: "all_pass",
|
||||
});
|
||||
|
||||
updateTaskStatus("M048", "S02", "T04", "complete", new Date().toISOString());
|
||||
|
||||
const task = getTask("M048", "S02", "T04");
|
||||
assert.equal(task.status, "complete");
|
||||
// verification_status should be preserved when completing (not our concern to clear it here)
|
||||
assert.equal(
|
||||
task.verification_status,
|
||||
"all_pass",
|
||||
"must preserve verification_status on complete",
|
||||
);
|
||||
});
|
||||
|
||||
// ── Idempotent: double revert must keep verification_status cleared ────────────
|
||||
|
||||
test("R072_double_revert_keeps_verification_status_empty", () => {
|
||||
makeProject("M048", "S02");
|
||||
insertTask({
|
||||
milestoneId: "M048",
|
||||
sliceId: "S02",
|
||||
id: "T05",
|
||||
title: "Double revert task",
|
||||
status: "pending",
|
||||
});
|
||||
|
||||
completeTaskAtomic("M048", "S02", "T05", {
|
||||
status: "complete",
|
||||
verificationResult: "passed",
|
||||
verificationStatus: "all_pass",
|
||||
});
|
||||
updateTaskStatus("M048", "S02", "T05", "pending", null);
|
||||
updateTaskStatus("M048", "S02", "T05", "pending", null); // second revert
|
||||
|
||||
const task = getTask("M048", "S02", "T05");
|
||||
assert.equal(task.status, "pending");
|
||||
assert.equal(task.verification_status, "");
|
||||
});
|
||||
Loading…
Add table
Reference in a new issue