fix: coerce non-numeric strings in DB columns during manifest serialization (#2962) (#3229)

SQLite can store string placeholders like "-", "N/A", or "" in INTEGER
columns after schema migrations or manual inserts. snapshotState() was
passing these through as-is via type assertions, producing JSON that
fails to parse on round-trip. Add toNumeric() helper and apply it to
all numeric columns (exit_code, duration_ms, sequence, seq).

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher 2026-03-30 16:32:58 -04:00 committed by GitHub
parent 9cebf19559
commit fb2ef25250
2 changed files with 114 additions and 5 deletions

View file

@ -12,6 +12,7 @@ import {
insertMilestone,
insertSlice,
insertTask,
_getAdapter,
} from '../gsd-db.ts';
import {
writeManifest,
@ -165,6 +166,97 @@ test('workflow-manifest: bootstrapFromManifest restores DB from manifest (round-
}
});
// ─── snapshotState: numeric column coercion (#2962) ─────────────────────
test('workflow-manifest: snapshotState coerces string placeholders in numeric columns to null (#2962)', () => {
const base = tempDir();
openDatabase(tempDbPath(base));
try {
// Set up prerequisite rows
insertMilestone({ id: 'M001' });
insertSlice({ id: 'S01', milestoneId: 'M001' });
insertTask({ id: 'T01', sliceId: 'S01', milestoneId: 'M001', title: 'Task', status: 'complete' });
// Insert verification_evidence with string placeholders in numeric columns
// This simulates what happens after schema migrations or manual inserts
const db = _getAdapter()!;
db.prepare(
`INSERT INTO verification_evidence (task_id, slice_id, milestone_id, command, exit_code, verdict, duration_ms, created_at)
VALUES (?, ?, ?, ?, ?, ?, ?, ?)`,
).run('T01', 'S01', 'M001', 'npm test', '-', 'pass', '-', new Date().toISOString());
// snapshotState should coerce "-" to null for numeric columns
const snap = snapshotState();
const ev = snap.verification_evidence[0];
assert.strictEqual(ev.exit_code, null, 'exit_code "-" should be coerced to null');
assert.strictEqual(ev.duration_ms, null, 'duration_ms "-" should be coerced to null');
// Round-trip through JSON should not throw
const json = JSON.stringify(snap, null, 2);
const reparsed = JSON.parse(json);
assert.strictEqual(reparsed.verification_evidence[0].exit_code, null);
assert.strictEqual(reparsed.verification_evidence[0].duration_ms, null);
} finally {
closeDatabase();
cleanupDir(base);
}
});
test('workflow-manifest: snapshotState coerces empty string and N/A in numeric columns (#2962)', () => {
const base = tempDir();
openDatabase(tempDbPath(base));
try {
insertMilestone({ id: 'M001' });
insertSlice({ id: 'S01', milestoneId: 'M001' });
insertTask({ id: 'T01', sliceId: 'S01', milestoneId: 'M001', title: 'Task', status: 'complete' });
const db = _getAdapter()!;
db.prepare(
`INSERT INTO verification_evidence (task_id, slice_id, milestone_id, command, exit_code, verdict, duration_ms, created_at)
VALUES (?, ?, ?, ?, ?, ?, ?, ?)`,
).run('T01', 'S01', 'M001', 'npm test', 'N/A', 'pass', '', new Date().toISOString());
const snap = snapshotState();
const ev = snap.verification_evidence[0];
assert.strictEqual(ev.exit_code, null, 'exit_code "N/A" should be coerced to null');
assert.strictEqual(ev.duration_ms, null, 'duration_ms "" should be coerced to null');
} finally {
closeDatabase();
cleanupDir(base);
}
});
test('workflow-manifest: snapshotState coerces string placeholders in sequence columns (#2962)', () => {
const base = tempDir();
openDatabase(tempDbPath(base));
try {
insertMilestone({ id: 'M001' });
// Insert a slice with a string sequence via raw SQL
const db = _getAdapter()!;
db.prepare(
`INSERT INTO slices (milestone_id, id, title, status, risk, depends, demo, created_at, sequence)
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)`,
).run('M001', 'S01', 'Test Slice', 'planned', 'low', '[]', '', new Date().toISOString(), '-');
db.prepare(
`INSERT INTO tasks (milestone_id, slice_id, id, title, status, sequence)
VALUES (?, ?, ?, ?, ?, ?)`,
).run('M001', 'S01', 'T01', 'Test Task', 'planned', 'N/A');
const snap = snapshotState();
assert.strictEqual(snap.slices[0].sequence, 0, 'slice sequence "-" should be coerced to 0');
assert.strictEqual(snap.tasks[0].sequence, 0, 'task sequence "N/A" should be coerced to 0');
// JSON round-trip must not throw
const json = JSON.stringify(snap, null, 2);
assert.doesNotThrow(() => JSON.parse(json));
} finally {
closeDatabase();
cleanupDir(base);
}
});
// ─── readManifest: version check ─────────────────────────────────────────
test('workflow-manifest: readManifest throws on unsupported version', () => {

View file

@ -42,6 +42,23 @@ function requireDb() {
return db;
}
/**
* Coerce a raw DB value to a number, returning `fallback` for
* null/undefined/non-numeric strings (e.g. "-", "N/A", "").
* SQLite can store TEXT in INTEGER columns after migrations or manual inserts.
*/
export function toNumeric(value: unknown, fallback: number | null = null): number | null {
if (value === null || value === undefined) return fallback;
if (typeof value === "number") return Number.isFinite(value) ? value : fallback;
if (typeof value === "string") {
const trimmed = value.trim();
if (trimmed === "" || trimmed === "-" || trimmed === "N/A") return fallback;
const n = Number(trimmed);
return Number.isFinite(n) ? n : fallback;
}
return fallback;
}
// ─── snapshotState ───────────────────────────────────────────────────────
/**
@ -99,7 +116,7 @@ export function snapshotState(): StateManifest {
proof_level: (r["proof_level"] as string) ?? "",
integration_closure: (r["integration_closure"] as string) ?? "",
observability_impact: (r["observability_impact"] as string) ?? "",
sequence: (r["sequence"] as number) ?? 0,
sequence: toNumeric(r["sequence"], 0) as number,
replan_triggered_at: (r["replan_triggered_at"] as string) ?? null,
}));
@ -129,12 +146,12 @@ export function snapshotState(): StateManifest {
expected_output: JSON.parse((r["expected_output"] as string) || "[]"),
observability_impact: (r["observability_impact"] as string) ?? "",
full_plan_md: (r["full_plan_md"] as string) ?? "",
sequence: (r["sequence"] as number) ?? 0,
sequence: toNumeric(r["sequence"], 0) as number,
}));
const rawDecisions = db.prepare("SELECT * FROM decisions ORDER BY seq").all() as Record<string, unknown>[];
const decisions: Decision[] = rawDecisions.map((r) => ({
seq: r["seq"] as number,
seq: toNumeric(r["seq"], 0) as number,
id: r["id"] as string,
when_context: (r["when_context"] as string) ?? "",
scope: (r["scope"] as string) ?? "",
@ -153,9 +170,9 @@ export function snapshotState(): StateManifest {
slice_id: r["slice_id"] as string,
milestone_id: r["milestone_id"] as string,
command: r["command"] as string,
exit_code: (r["exit_code"] as number) ?? null,
exit_code: toNumeric(r["exit_code"]),
verdict: (r["verdict"] as string) ?? "",
duration_ms: (r["duration_ms"] as number) ?? null,
duration_ms: toNumeric(r["duration_ms"]),
created_at: r["created_at"] as string,
}));