fix(gsd): consistency and cleanup (wave 5/5)

Five consistency fixes to eliminate divergence sources:

1. workflow-projections.ts: Direct string comparisons for task/slice status
   replaced with isClosedStatus() from status-guards.ts. Skipped tasks now
   correctly show checked checkboxes in PLAN.md and ROADMAP.md.

2. gsd-db.ts upsertDecision: INSERT OR REPLACE changed to INSERT ... ON
   CONFLICT(id) DO UPDATE SET. Preserves the seq column so decision ordering
   in DECISIONS.md is stable after reconcile replay.

3. state.ts: Duplicate private isStatusDone() removed, replaced with alias
   to isClosedStatus from status-guards.ts. Single source of truth for
   "what counts as closed."

4. gsd-db.ts migrateSchema: Database is now backed up to
   gsd.db.backup-v{currentVersion} before running migration steps. A mid-
   migration crash no longer leaves a partially-migrated DB with no recovery.

5. workflow-events.ts: WorkflowEvent interface now includes optional v field
   (schema version). New events are written with v:2. Legacy events (no v
   field) are still accepted. Prevents future cmd-format drift from requiring
   another dual-read fix.
This commit is contained in:
Jeremy 2026-04-07 12:42:53 -05:00
parent 0dd7c31213
commit 5c9ee8f10d
4 changed files with 38 additions and 13 deletions

View file

@ -451,6 +451,19 @@ function migrateSchema(db: DbAdapter): void {
const currentVersion = row ? (row["v"] as number) : 0;
if (currentVersion >= SCHEMA_VERSION) return;
// Backup database before migration so a mid-migration crash doesn't
// leave a partially-migrated DB with no recovery path.
if (currentPath && currentPath !== ":memory:" && existsSync(currentPath)) {
try {
const backupPath = `${currentPath}.backup-v${currentVersion}`;
if (!existsSync(backupPath)) {
copyFileSync(currentPath, backupPath);
}
} catch {
// Non-fatal — proceed with migration even if backup fails
}
}
db.exec("BEGIN");
try {
if (currentVersion < 2) {
@ -999,9 +1012,21 @@ export function _resetProvider(): void {
export function upsertDecision(d: Omit<Decision, "seq">): void {
if (!currentDb) throw new GSDError(GSD_STALE_STATE, "gsd-db: No database open");
// Use ON CONFLICT DO UPDATE instead of INSERT OR REPLACE to preserve the
// seq column. INSERT OR REPLACE deletes then reinserts, resetting seq and
// corrupting decision ordering in DECISIONS.md after reconcile replay.
currentDb.prepare(
`INSERT OR REPLACE INTO decisions (id, when_context, scope, decision, choice, rationale, revisable, made_by, superseded_by)
VALUES (:id, :when_context, :scope, :decision, :choice, :rationale, :revisable, :made_by, :superseded_by)`,
`INSERT INTO decisions (id, when_context, scope, decision, choice, rationale, revisable, made_by, superseded_by)
VALUES (:id, :when_context, :scope, :decision, :choice, :rationale, :revisable, :made_by, :superseded_by)
ON CONFLICT(id) DO UPDATE SET
when_context = excluded.when_context,
scope = excluded.scope,
decision = excluded.decision,
choice = excluded.choice,
rationale = excluded.rationale,
revisable = excluded.revisable,
made_by = excluded.made_by,
superseded_by = excluded.superseded_by`,
).run({
":id": d.id,
":when_context": d.when_context,

View file

@ -304,12 +304,9 @@ function extractContextTitle(content: string | null, fallback: string): string {
// ─── DB-backed State Derivation ────────────────────────────────────────────
/**
* Helper: check if a DB status counts as "done" (handles K002 ambiguity).
*/
function isStatusDone(status: string): boolean {
return status === 'complete' || status === 'done' || status === 'skipped';
}
// isStatusDone replaced by isClosedStatus from status-guards.ts (single source of truth).
// Alias kept for backward compatibility within this file.
const isStatusDone = isClosedStatus;
/**
* Derive GSD state from the milestones/slices/tasks DB tables.

View file

@ -19,10 +19,11 @@ export function getSessionId(): string {
// ─── Event Types ─────────────────────────────────────────────────────────
export interface WorkflowEvent {
cmd: string; // e.g. "complete_task"
v?: number; // schema version — omitted in v1 (legacy), 2 for current format
cmd: string; // e.g. "complete-task" (canonical: hyphens; legacy: underscores — both accepted by replay)
params: Record<string, unknown>;
ts: string; // ISO 8601
hash: string; // content hash (hex, 16 chars)
ts: string; // ISO 8601
hash: string; // content hash (hex, 16 chars)
actor: "agent" | "system";
actor_name?: string; // e.g. "executor-agent-01" — caller-provided identity
trigger_reason?: string; // e.g. "plan-phase complete" — caller-provided causation
@ -46,6 +47,7 @@ export function appendEvent(
.slice(0, 16);
const fullEvent: WorkflowEvent = {
v: 2,
...event,
hash,
session_id: ENGINE_SESSION_ID,

View file

@ -16,6 +16,7 @@ import { atomicWriteSync } from "./atomic-write.js";
import { join } from "node:path";
import { mkdirSync, existsSync } from "node:fs";
import { logWarning } from "./workflow-logger.js";
import { isClosedStatus } from "./status-guards.js";
import { deriveState } from "./state.js";
import type { GSDState } from "./types.js";
@ -55,7 +56,7 @@ export function renderPlanContent(sliceRow: SliceRow, taskRows: TaskRow[]): stri
lines.push("## Tasks");
for (const task of taskRows) {
const checkbox = task.status === "done" || task.status === "complete" ? "[x]" : "[ ]";
const checkbox = isClosedStatus(task.status) ? "[x]" : "[ ]";
lines.push(`- ${checkbox} **${task.id}: ${task.title}** \u2014 ${task.description}`);
// Estimate subline (always present if non-empty)
@ -125,7 +126,7 @@ export function renderRoadmapContent(milestoneRow: MilestoneRow, sliceRows: Slic
lines.push("|----|-------|------|---------|------|------------|");
for (const slice of sliceRows) {
const done = slice.status === "done" || slice.status === "complete" ? "\u2705" : "\u2B1C";
const done = isClosedStatus(slice.status) ? "\u2705" : "\u2B1C";
// depends is already parsed to string[] by rowToSlice
let depends = "\u2014";