From 4fdd3a93387cb2a35c3d217937213f2ed601bdb7 Mon Sep 17 00:00:00 2001 From: srstomp Date: Sun, 26 Apr 2026 14:52:40 +0200 Subject: [PATCH 01/11] chore: bump engines.node to >=22.16.0 Required for node:sqlite DatabaseSync({ timeout }) constructor option, added in Node 22.16. Phase 2 of the data-integrity plan. --- packages/ohno-cli/package.json | 2 +- packages/ohno-core/package.json | 2 +- packages/ohno-mcp/package.json | 2 +- packages/package-lock.json | 6 +++--- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/ohno-cli/package.json b/packages/ohno-cli/package.json index 3769680..5a4d35c 100644 --- a/packages/ohno-cli/package.json +++ b/packages/ohno-cli/package.json @@ -52,7 +52,7 @@ "typescript": "^5.7.0" }, "engines": { - "node": ">=18.0.0" + "node": ">=22.16.0" }, "license": "MIT" } diff --git a/packages/ohno-core/package.json b/packages/ohno-core/package.json index bfb1a75..986c95d 100644 --- a/packages/ohno-core/package.json +++ b/packages/ohno-core/package.json @@ -44,7 +44,7 @@ "typescript": "^5.7.0" }, "engines": { - "node": ">=18.0.0" + "node": ">=22.16.0" }, "license": "MIT" } diff --git a/packages/ohno-mcp/package.json b/packages/ohno-mcp/package.json index d54510a..1e5b45e 100644 --- a/packages/ohno-mcp/package.json +++ b/packages/ohno-mcp/package.json @@ -45,7 +45,7 @@ "typescript": "^5.7.0" }, "engines": { - "node": ">=18.0.0" + "node": ">=22.16.0" }, "license": "MIT" } diff --git a/packages/package-lock.json b/packages/package-lock.json index e02d6ad..a3d2a2d 100644 --- a/packages/package-lock.json +++ b/packages/package-lock.json @@ -3859,7 +3859,7 @@ "typescript": "^5.7.0" }, "engines": { - "node": ">=18.0.0" + "node": ">=22.16.0" } }, "ohno-core": { @@ -3875,7 +3875,7 @@ "typescript": "^5.7.0" }, "engines": { - "node": ">=18.0.0" + "node": ">=22.16.0" } }, "ohno-mcp": { @@ -3896,7 +3896,7 @@ "typescript": "^5.7.0" }, "engines": { - "node": ">=18.0.0" + "node": ">=22.16.0" } } } From 65f19376ed49dd30a6541ef545e3cd8c70c6a909 Mon Sep 17 00:00:00 2001 From: srstomp Date: Sun, 26 Apr 2026 15:06:46 +0200 Subject: [PATCH 02/11] feat: add Node.js version guard to cli and mcp entries Both ohno-cli and ohno-mcp depend on node:sqlite (Node 22.16+). Without this guard, users on older Node would see "Cannot find module 'node:sqlite'" instead of an actionable error. The guard self-executes on import and is the first import in each entry point, so it runs before any sqlite-bearing code loads. Pure helper checkNodeVersion(version) is unit-tested with 6 synthetic version strings. Phase 2 of the data-integrity plan, task-791a15c4. Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/ohno-cli/src/index.ts | 1 + packages/ohno-cli/src/node-guard.test.ts | 15 ++++++++++++ packages/ohno-cli/src/node-guard.ts | 30 ++++++++++++++++++++++++ packages/ohno-mcp/src/index.ts | 1 + packages/ohno-mcp/src/node-guard.ts | 30 ++++++++++++++++++++++++ 5 files changed, 77 insertions(+) create mode 100644 packages/ohno-cli/src/node-guard.test.ts create mode 100644 packages/ohno-cli/src/node-guard.ts create mode 100644 packages/ohno-mcp/src/node-guard.ts diff --git a/packages/ohno-cli/src/index.ts b/packages/ohno-cli/src/index.ts index 84e48db..81a89c6 100644 --- a/packages/ohno-cli/src/index.ts +++ b/packages/ohno-cli/src/index.ts @@ -3,6 +3,7 @@ * ohno-cli entry point */ +import "./node-guard.js"; import { createCli } from "./cli.js"; const program = createCli(); diff --git a/packages/ohno-cli/src/node-guard.test.ts b/packages/ohno-cli/src/node-guard.test.ts new file mode 100644 index 0000000..e99695b --- /dev/null +++ b/packages/ohno-cli/src/node-guard.test.ts @@ -0,0 +1,15 @@ +import { describe, it, expect } from 'vitest'; +import { checkNodeVersion } from './node-guard.js'; + +describe('checkNodeVersion', () => { + it('ok for 22.16.0', () => expect(checkNodeVersion('22.16.0').ok).toBe(true)); + it('ok for 22.16.5', () => expect(checkNodeVersion('22.16.5').ok).toBe(true)); + it('ok for 24.0.0', () => expect(checkNodeVersion('24.0.0').ok).toBe(true)); + it('not-ok 22.15.0', () => { + const r = checkNodeVersion('22.15.0'); + expect(r.ok).toBe(false); + if (!r.ok) expect(r.message).toContain('22.16.0'); + }); + it('not-ok 20.18.0', () => expect(checkNodeVersion('20.18.0').ok).toBe(false)); + it('not-ok 18.0.0', () => expect(checkNodeVersion('18.0.0').ok).toBe(false)); +}); diff --git a/packages/ohno-cli/src/node-guard.ts b/packages/ohno-cli/src/node-guard.ts new file mode 100644 index 0000000..74e3d12 --- /dev/null +++ b/packages/ohno-cli/src/node-guard.ts @@ -0,0 +1,30 @@ +/** + * Node.js version guard for ohno-cli. + * Must be imported before any module that transitively imports node:sqlite. + */ + +export function checkNodeVersion( + version: string +): { ok: true } | { ok: false; message: string } { + const [major, minor] = version.split(".").map(Number); + if (major > 22 || (major === 22 && minor >= 16)) { + return { ok: true }; + } + return { + ok: false, + message: + `ohno 1.0.0+ requires Node.js >= 22.16.0. You're on v${version}.\n` + + `Either upgrade Node, or pin to @stevestomp/ohno-cli@^0.20 / @stevestomp/ohno-mcp@^0.20.`, + }; +} + +export function runNodeGuard(): void { + const result = checkNodeVersion(process.versions.node); + if (!result.ok) { + process.stderr.write(result.message + "\n"); + process.exit(1); + } +} + +// Self-execute so that a bare `import './node-guard.js'` runs the guard. +runNodeGuard(); diff --git a/packages/ohno-mcp/src/index.ts b/packages/ohno-mcp/src/index.ts index f3efd42..961486d 100644 --- a/packages/ohno-mcp/src/index.ts +++ b/packages/ohno-mcp/src/index.ts @@ -11,6 +11,7 @@ * OHNO_DB_PATH - Path to tasks.db file */ +import "./node-guard.js"; import { runServer } from "./server.js"; // Parse args diff --git a/packages/ohno-mcp/src/node-guard.ts b/packages/ohno-mcp/src/node-guard.ts new file mode 100644 index 0000000..b0cddb6 --- /dev/null +++ b/packages/ohno-mcp/src/node-guard.ts @@ -0,0 +1,30 @@ +/** + * Node.js version guard for ohno-mcp. + * Must be imported before any module that transitively imports node:sqlite. + */ + +export function checkNodeVersion( + version: string +): { ok: true } | { ok: false; message: string } { + const [major, minor] = version.split(".").map(Number); + if (major > 22 || (major === 22 && minor >= 16)) { + return { ok: true }; + } + return { + ok: false, + message: + `ohno 1.0.0+ requires Node.js >= 22.16.0. You're on v${version}.\n` + + `Either upgrade Node, or pin to @stevestomp/ohno-cli@^0.20 / @stevestomp/ohno-mcp@^0.20.`, + }; +} + +export function runNodeGuard(): void { + const result = checkNodeVersion(process.versions.node); + if (!result.ok) { + process.stderr.write(result.message + "\n"); + process.exit(1); + } +} + +// Self-execute so that a bare `import './node-guard.js'` runs the guard. +runNodeGuard(); From 948ba18313228019fdd49c8029c137eca779d662 Mon Sep 17 00:00:00 2001 From: srstomp Date: Sun, 26 Apr 2026 16:37:35 +0200 Subject: [PATCH 03/11] feat(core): replace sql.js with node:sqlite Rewrites TaskDatabase on top of Node 22.16+'s built-in node:sqlite (DatabaseSync). All ~40 query methods translated from sql.js patterns (db.run + getRowsModified, db.exec returning {columns,values}, prepare/bind/step/getAsObject/free) to node:sqlite's prepare(sql).all/get/run(). - open() asserts journal_mode = DELETE and quick_check = ok with verbatim error messages; closes db before throwing on either path - 5-second busy_timeout via DatabaseSync({ timeout: 5000 }) - save()/getSqlJs()/resultToObjects removed; reload() kept as no-op (cli.test.ts call sites removed in a later task) - dbPath exposed as a public readonly property for diagnostics - 4 white-box test sites in db.test.ts updated to the new API - vitest.config.ts in core and cli get a Vite plugin to work around vite-node 2.1.x stripping the node: prefix before resolving builtins Tests: 250/250 ohno-core + 156/156 ohno-cli pass. db.ts net 1874 -> 1587. Closes task-a5f53eb6 and task-e77e86ca. Phase 2 of the data-integrity plan. Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/ohno-cli/vitest.config.ts | 29 +- packages/ohno-core/src/db.test.ts | 33 +- packages/ohno-core/src/db.ts | 613 ++++++++-------------------- packages/ohno-core/vitest.config.ts | 30 +- 4 files changed, 235 insertions(+), 470 deletions(-) diff --git a/packages/ohno-cli/vitest.config.ts b/packages/ohno-cli/vitest.config.ts index c840770..e516ef5 100644 --- a/packages/ohno-cli/vitest.config.ts +++ b/packages/ohno-cli/vitest.config.ts @@ -1,6 +1,33 @@ -import { defineConfig } from "vitest/config"; +import { defineConfig, type Plugin } from "vitest/config"; + +// node:sqlite is a Node 22.5+ built-in added after builtinModules was frozen. +// vite-node's normalizeModuleId() strips the "node:" prefix, so "node:sqlite" +// becomes "sqlite", which Vite can't find as a file. This plugin intercepts +// the stripped id and provides a virtual module that loads the real built-in. +const nodeSqlitePlugin: Plugin = { + name: "node-sqlite-external", + enforce: "pre", + resolveId(id) { + if (id === "sqlite") { + return "\0virtual:sqlite"; + } + }, + load(id) { + if (id === "\0virtual:sqlite") { + return ` +import { createRequire } from 'node:module'; +const _require = createRequire(import.meta.url); +const mod = _require('node:sqlite'); +export const DatabaseSync = mod.DatabaseSync; +export const StatementSync = mod.StatementSync; +export default mod; +`; + } + }, +}; export default defineConfig({ + plugins: [nodeSqlitePlugin], test: { globals: false, environment: "node", diff --git a/packages/ohno-core/src/db.test.ts b/packages/ohno-core/src/db.test.ts index e2c8e2e..9f00a86 100644 --- a/packages/ohno-core/src/db.test.ts +++ b/packages/ohno-core/src/db.test.ts @@ -6,6 +6,7 @@ import { describe, it, expect, beforeEach, afterEach } from "vitest"; import { mkdtempSync, rmSync } from "fs"; import { tmpdir } from "os"; import { join } from "path"; +import type { DatabaseSync } from "node:sqlite"; import { TaskDatabase } from "./db.js"; import type { TaskStatus } from "./types.js"; @@ -755,25 +756,22 @@ describe("TaskDatabase", () => { // Helper function to set up hierarchy: epic -> story -> tasks const setupHierarchy = () => { // Access the private db property for direct SQL - const dbInstance = db as unknown as { db: { run: (sql: string, params?: unknown[]) => void } }; + const dbInstance = db as unknown as { db: DatabaseSync }; // Create an epic - dbInstance.db.run( - "INSERT INTO epics (id, title, priority) VALUES (?, ?, ?)", - ["epic-1", "Epic 1", "P0"] - ); + dbInstance.db.prepare( + "INSERT INTO epics (id, title, priority) VALUES (?, ?, ?)" + ).run("epic-1", "Epic 1", "P0"); // Create a story - dbInstance.db.run( - "INSERT INTO stories (id, epic_id, title) VALUES (?, ?, ?)", - ["story-1", "epic-1", "Story 1"] - ); + dbInstance.db.prepare( + "INSERT INTO stories (id, epic_id, title) VALUES (?, ?, ?)" + ).run("story-1", "epic-1", "Story 1"); // Create another story in the same epic - dbInstance.db.run( - "INSERT INTO stories (id, epic_id, title) VALUES (?, ?, ?)", - ["story-2", "epic-1", "Story 2"] - ); + dbInstance.db.prepare( + "INSERT INTO stories (id, epic_id, title) VALUES (?, ?, ?)" + ).run("story-2", "epic-1", "Story 2"); }; describe("isStoryCompleted", () => { @@ -1544,11 +1542,10 @@ describe("TaskDatabase", () => { const taskId = db.createTask({ title: "Invalid WIP" }); // Manually insert invalid JSON (edge case) - const dbInstance = db as unknown as { db: { run: (sql: string, params?: unknown[]) => void } }; - dbInstance.db.run( - "UPDATE tasks SET work_in_progress = ? WHERE id = ?", - ["invalid json{", taskId] - ); + const dbInstance = db as unknown as { db: DatabaseSync }; + dbInstance.db.prepare( + "UPDATE tasks SET work_in_progress = ? WHERE id = ?" + ).run("invalid json{", taskId); // Should treat as empty object and continue const result = db.updateTaskWip(taskId, { phase: "recovery" }); diff --git a/packages/ohno-core/src/db.ts b/packages/ohno-core/src/db.ts index 3666e23..060e4bd 100644 --- a/packages/ohno-core/src/db.ts +++ b/packages/ohno-core/src/db.ts @@ -1,11 +1,10 @@ /** * TaskDatabase - Core database operations for ohno * - * Uses sql.js (pure JavaScript SQLite) for maximum compatibility. - * No native bindings required - works on any Node.js version and platform. + * Uses Node's built-in node:sqlite (DatabaseSync). Requires Node >= 22.16. */ -import initSqlJs, { type Database as SqlJsDatabase } from "sql.js"; +import { DatabaseSync } from 'node:sqlite'; import * as fs from "fs"; import * as path from "path"; import type { @@ -66,105 +65,69 @@ import { FIELD_SETS, } from "./schema.js"; -// Cache the SQL.js initialization promise -let sqlJsPromise: Promise | null = null; - -/** - * Initialize sql.js (cached) - */ -async function getSqlJs(): Promise { - if (!sqlJsPromise) { - sqlJsPromise = initSqlJs(); - } - return sqlJsPromise; -} - -/** - * Convert sql.js result to array of objects - */ -function resultToObjects(result: initSqlJs.QueryExecResult[]): T[] { - if (result.length === 0) return []; - const { columns, values } = result[0]; - return values.map((row: initSqlJs.SqlValue[]) => { - const obj: Record = {}; - columns.forEach((col: string, i: number) => { - obj[col] = row[i]; - }); - return obj as T; - }); -} - export class TaskDatabase { - private db: SqlJsDatabase; - private dbPath: string; - - /** - * Private constructor - use TaskDatabase.open() instead - */ - private constructor(db: SqlJsDatabase, dbPath: string) { - this.db = db; - this.dbPath = dbPath; - } + private constructor(private db: DatabaseSync, public readonly dbPath: string) {} /** * Open or create a database (async factory) */ static async open(dbPath: string): Promise { - const SQL = await getSqlJs(); - - let db: SqlJsDatabase; - - // Load existing database or create new one - if (fs.existsSync(dbPath)) { - const buffer = fs.readFileSync(dbPath); - db = new SQL.Database(buffer); - } else { - // Ensure directory exists - const dir = path.dirname(dbPath); - if (!fs.existsSync(dir)) { - fs.mkdirSync(dir, { recursive: true }); - } - db = new SQL.Database(); + fs.mkdirSync(path.dirname(dbPath), { recursive: true }); + const db = new DatabaseSync(dbPath, { timeout: 5000 }); + + // Set and verify journal mode. PRAGMA can silently fail (e.g., if the file + // is in use by another process holding it in WAL); we assert the result. + const journalRows = db.prepare('PRAGMA journal_mode = DELETE').all() as Array<{ journal_mode: string }>; + const actualMode = journalRows[0]?.journal_mode; + if (actualMode !== 'delete') { + db.close(); + throw new Error( + `Failed to set journal_mode = DELETE on ${dbPath}. ` + + `SQLite returned mode '${actualMode}'. ` + + `This usually means another process has the database open in a different journal mode. ` + + `Close other ohno processes and retry.` + ); + } + + db.exec('PRAGMA foreign_keys = ON'); + db.exec('PRAGMA synchronous = NORMAL'); + + const checkRows = db.prepare('PRAGMA quick_check').all() as Array<{ quick_check: string }>; + if (checkRows[0]?.quick_check !== 'ok') { + db.close(); + throw new Error( + `Database integrity check failed on ${dbPath}: ${checkRows[0]?.quick_check}. ` + + `Run 'sqlite3 ${dbPath} ".recover"' to attempt recovery.` + ); } const instance = new TaskDatabase(db, dbPath); instance.ensureTables(); - instance.save(); // Save initial state - return instance; } - /** - * Save database to disk - */ - private save(): void { - const data = this.db.export(); - const buffer = Buffer.from(data); - fs.writeFileSync(this.dbPath, buffer); - } - /** * Ensure all required tables and columns exist */ private ensureTables(): void { // Create hierarchy tables - this.db.run(CREATE_PROJECTS_TABLE); - this.db.run(CREATE_EPICS_TABLE); - this.db.run(CREATE_STORIES_TABLE); + this.db.prepare(CREATE_PROJECTS_TABLE).run(); + this.db.prepare(CREATE_EPICS_TABLE).run(); + this.db.prepare(CREATE_STORIES_TABLE).run(); // Create core tables - this.db.run(CREATE_TASKS_TABLE); - this.db.run(CREATE_TASK_ACTIVITY_TABLE); - this.db.run(CREATE_TASK_FILES_TABLE); - this.db.run(CREATE_TASK_DEPENDENCIES_TABLE); - this.db.run(CREATE_TASK_FAILURES_TABLE); - this.db.run(CREATE_TASK_HANDOFFS_TABLE); - this.db.run(CREATE_WORK_QUEUE_TABLE); + this.db.prepare(CREATE_TASKS_TABLE).run(); + this.db.prepare(CREATE_TASK_ACTIVITY_TABLE).run(); + this.db.prepare(CREATE_TASK_FILES_TABLE).run(); + this.db.prepare(CREATE_TASK_DEPENDENCIES_TABLE).run(); + this.db.prepare(CREATE_TASK_FAILURES_TABLE).run(); + this.db.prepare(CREATE_TASK_HANDOFFS_TABLE).run(); + this.db.prepare(CREATE_WORK_QUEUE_TABLE).run(); // Add extended columns if missing (backwards compatibility) for (const [colName, colType] of EXTENDED_TASK_COLUMNS) { try { - this.db.run(`ALTER TABLE tasks ADD COLUMN ${colName} ${colType}`); + this.db.prepare(`ALTER TABLE tasks ADD COLUMN ${colName} ${colType}`).run(); } catch { // Column already exists } @@ -172,7 +135,7 @@ export class TaskDatabase { // Create indexes for (const sql of CREATE_INDEXES) { - this.db.run(sql); + this.db.prepare(sql).run(); } } @@ -180,29 +143,19 @@ export class TaskDatabase { * Close the database connection */ close(): void { - this.save(); this.db.close(); } /** * Reload the database from disk (useful for tests) - * This discards any in-memory changes and re-reads from the file. + * No-op: with node:sqlite, reads always see the latest committed state. + * Kept for backwards compat with tests that called this with sql.js. + * To be removed once cli.test.ts call sites are deleted. */ async reload(): Promise { - const SQL = await getSqlJs(); - - // Close current db - this.db.close(); - - // Reload from disk - if (fs.existsSync(this.dbPath)) { - const buffer = fs.readFileSync(this.dbPath); - this.db = new SQL.Database(buffer); - } else { - this.db = new SQL.Database(); - this.ensureTables(); - this.save(); - } + // No-op: with node:sqlite, reads always see the latest committed state. + // Kept for backwards compat with tests that called this with sql.js. + // To be removed once cli.test.ts call sites are deleted. } // ========================================================================== @@ -213,8 +166,7 @@ export class TaskDatabase { * Get aggregated project status */ getProjectStatus(): ProjectStatus { - const result = this.db.exec(GET_PROJECT_STATUS); - const rows = resultToObjects>(result); + const rows = this.db.prepare(GET_PROJECT_STATUS).all() as Array>; if (rows.length === 0) { return { @@ -299,17 +251,7 @@ export class TaskDatabase { sql += " LIMIT ?"; params.push(limit); - const stmt = this.db.prepare(sql); - stmt.bind(params as initSqlJs.BindParams); - - const rows: Task[] = []; - while (stmt.step()) { - const row = stmt.getAsObject() as unknown as Task; - rows.push(row); - } - stmt.free(); - - return rows; + return this.db.prepare(sql).all(...(params as never[])) as unknown as Task[]; } /** @@ -348,12 +290,8 @@ export class TaskDatabase { sql += ` WHERE ${conditions.join(" AND ")}`; - const stmt = this.db.prepare(sql); - stmt.bind(params as initSqlJs.BindParams); - stmt.step(); - const count = (stmt.getAsObject() as { count: number }).count; - stmt.free(); - return count; + const row = this.db.prepare(sql).get(...(params as never[])) as { count: number } | undefined; + return row?.count ?? 0; } /** @@ -372,17 +310,7 @@ export class TaskDatabase { WHERE t.id = ? `; - const stmt = this.db.prepare(sql); - stmt.bind([taskId]); - - if (stmt.step()) { - const row = stmt.getAsObject() as unknown as Task; - stmt.free(); - return row; - } - - stmt.free(); - return null; + return (this.db.prepare(sql).get(taskId) as Task | undefined) ?? null; } /** @@ -436,17 +364,14 @@ export class TaskDatabase { LIMIT ? `; - const stmt = this.db.prepare(sql); - stmt.bind([maxSize]); const batch: BatchTask[] = []; - while (stmt.step()) { - const task = stmt.getAsObject() as unknown as Task; + const rows = this.db.prepare(sql).all(maxSize) as unknown as Task[]; + for (const task of rows) { batch.push({ ...task, failure_context: task.needs_rework ? this.getTaskFailures(task.id) : undefined, }); } - stmt.free(); // Fallback: if queue is empty (first run or stale), rebuild and retry if (batch.length === 0) { @@ -478,8 +403,7 @@ export class TaskDatabase { t.created_at ASC `; - const result = this.db.exec(sql); - const candidates = resultToObjects(result); + const candidates = this.db.prepare(sql).all() as unknown as Task[]; const available = candidates.filter((task) => !this.isTaskBlockedByDependencies(task.id)); const batch = available.slice(0, maxSize); @@ -518,65 +442,29 @@ export class TaskDatabase { ORDER BY created_at DESC LIMIT ? `; - const stmt = this.db.prepare(sql); - stmt.bind([taskId, limit]); - - const rows: TaskActivity[] = []; - while (stmt.step()) { - rows.push(stmt.getAsObject() as unknown as TaskActivity); - } - stmt.free(); - - return rows; + return this.db.prepare(sql).all(taskId, limit) as unknown as TaskActivity[]; } /** * Get recent activity across all tasks */ getRecentActivity(limit = 10): TaskActivity[] { - const stmt = this.db.prepare(GET_RECENT_ACTIVITY); - stmt.bind([limit]); - - const rows: TaskActivity[] = []; - while (stmt.step()) { - rows.push(stmt.getAsObject() as unknown as TaskActivity); - } - stmt.free(); - - return rows; + return this.db.prepare(GET_RECENT_ACTIVITY).all(limit) as unknown as TaskActivity[]; } /** * Get dependencies for a task */ getTaskDependencies(taskId: string): TaskDependency[] { - const stmt = this.db.prepare(GET_TASK_DEPENDENCIES); - stmt.bind([taskId]); - - const rows: TaskDependency[] = []; - while (stmt.step()) { - rows.push(stmt.getAsObject() as unknown as TaskDependency); - } - stmt.free(); - - return rows; + return this.db.prepare(GET_TASK_DEPENDENCIES).all(taskId) as unknown as TaskDependency[]; } /** * Get blocking (unfinished) dependencies for a task */ getBlockingDependencies(taskId: string): string[] { - const stmt = this.db.prepare(GET_BLOCKING_DEPENDENCIES); - stmt.bind([taskId]); - - const rows: string[] = []; - while (stmt.step()) { - const obj = stmt.getAsObject() as unknown as { depends_on_task_id: string }; - rows.push(obj.depends_on_task_id); - } - stmt.free(); - - return rows; + const rows = this.db.prepare(GET_BLOCKING_DEPENDENCIES).all(taskId) as unknown as Array<{ depends_on_task_id: string }>; + return rows.map((row) => row.depends_on_task_id); } /** @@ -596,17 +484,8 @@ export class TaskDatabase { WHERE story_id = ? AND status NOT IN ('done', 'archived') `; - const stmt = this.db.prepare(sql); - stmt.bind([storyId]); - - let incompleteCount = 0; - if (stmt.step()) { - const row = stmt.getAsObject() as { incomplete_count: number }; - incompleteCount = row.incomplete_count; - } - stmt.free(); - - return incompleteCount === 0; + const row = this.db.prepare(sql).get(storyId) as { incomplete_count: number } | undefined; + return (row?.incomplete_count ?? 0) === 0; } /** @@ -620,17 +499,8 @@ export class TaskDatabase { WHERE s.epic_id = ? AND t.status NOT IN ('done', 'archived') `; - const stmt = this.db.prepare(sql); - stmt.bind([epicId]); - - let incompleteCount = 0; - if (stmt.step()) { - const row = stmt.getAsObject() as { incomplete_count: number }; - incompleteCount = row.incomplete_count; - } - stmt.free(); - - return incompleteCount === 0; + const row = this.db.prepare(sql).get(epicId) as { incomplete_count: number } | undefined; + return (row?.incomplete_count ?? 0) === 0; } /** @@ -677,7 +547,7 @@ export class TaskDatabase { VALUES (?, ?, ?, 'todo', ?, ?, ?, ?, ?, ?, ?) `; - this.db.run(sql, [ + this.db.prepare(sql).run( taskId, opts.story_id ?? null, opts.title, @@ -688,7 +558,7 @@ export class TaskDatabase { timestamp, opts.actor ?? null, opts.source ?? "human", - ]); + ); // Log activity this.addTaskActivity(taskId, "created", `Task created: ${opts.title}`, opts.actor); @@ -696,7 +566,6 @@ export class TaskDatabase { // Add to work queue this.recomputeQueueEntry(taskId); - this.save(); return taskId; } @@ -719,16 +588,15 @@ export class TaskDatabase { VALUES (?, ?, ?, ?, 'todo', ?, ?) `; - this.db.run(sql, [ + this.db.prepare(sql).run( storyId, opts.epic_id ?? null, opts.title, opts.description ?? null, timestamp, timestamp, - ]); + ); - this.save(); return storyId; } @@ -737,17 +605,7 @@ export class TaskDatabase { */ getStory(storyId: string): { id: string; epic_id?: string; title: string; description?: string; status?: string; created_at?: string; updated_at?: string } | null { const sql = "SELECT * FROM stories WHERE id = ?"; - const stmt = this.db.prepare(sql); - stmt.bind([storyId]); - - if (stmt.step()) { - const row = stmt.getAsObject() as { id: string; epic_id?: string; title: string; description?: string; status?: string; created_at?: string; updated_at?: string }; - stmt.free(); - return row; - } - - stmt.free(); - return null; + return (this.db.prepare(sql).get(storyId) as { id: string; epic_id?: string; title: string; description?: string; status?: string; created_at?: string; updated_at?: string } | undefined) ?? null; } /** @@ -779,14 +637,9 @@ export class TaskDatabase { params.push(storyId); const sql = `UPDATE stories SET ${setClauses.join(", ")} WHERE id = ?`; - this.db.run(sql, params as initSqlJs.BindParams); - - const changes = this.db.getRowsModified(); - if (changes > 0) { - this.save(); - } + const result = this.db.prepare(sql).run(...(params as never[])); - return changes > 0; + return Number(result.changes) > 0; } /** @@ -822,16 +675,7 @@ export class TaskDatabase { sql += " LIMIT ? OFFSET ?"; params.push(limit, offset); - const stmt = this.db.prepare(sql); - stmt.bind(params as initSqlJs.BindParams); - - const rows: Story[] = []; - while (stmt.step()) { - rows.push(stmt.getAsObject() as unknown as Story); - } - stmt.free(); - - return rows; + return this.db.prepare(sql).all(...(params as never[])) as unknown as Story[]; } /** @@ -839,17 +683,7 @@ export class TaskDatabase { */ getEpic(epicId: string): Epic | null { const sql = "SELECT * FROM epics WHERE id = ?"; - const stmt = this.db.prepare(sql); - stmt.bind([epicId]); - - if (stmt.step()) { - const row = stmt.getAsObject() as unknown as Epic; - stmt.free(); - return row; - } - - stmt.free(); - return null; + return (this.db.prepare(sql).get(epicId) as Epic | undefined) ?? null; } /** @@ -880,16 +714,7 @@ export class TaskDatabase { sql += " LIMIT ?"; params.push(limit); - const stmt = this.db.prepare(sql); - stmt.bind(params as initSqlJs.BindParams); - - const rows: Epic[] = []; - while (stmt.step()) { - rows.push(stmt.getAsObject() as unknown as Epic); - } - stmt.free(); - - return rows; + return this.db.prepare(sql).all(...(params as never[])) as unknown as Epic[]; } /** @@ -911,7 +736,7 @@ export class TaskDatabase { VALUES (?, ?, ?, ?, ?, 'todo', ?, ?) `; - this.db.run(sql, [ + this.db.prepare(sql).run( epicId, opts.project_id ?? null, opts.title, @@ -919,9 +744,8 @@ export class TaskDatabase { opts.priority ?? "P2", timestamp, timestamp, - ]); + ); - this.save(); return epicId; } @@ -954,14 +778,9 @@ export class TaskDatabase { params.push(epicId); const sql = `UPDATE epics SET ${setClauses.join(", ")} WHERE id = ?`; - this.db.run(sql, params as initSqlJs.BindParams); + const result = this.db.prepare(sql).run(...(params as never[])); - const changes = this.db.getRowsModified(); - if (changes > 0) { - this.save(); - } - - return changes > 0; + return Number(result.changes) > 0; } /** @@ -993,16 +812,13 @@ export class TaskDatabase { params.push(taskId); const sql = `UPDATE tasks SET ${setClauses.join(", ")} WHERE id = ?`; - this.db.run(sql, params as initSqlJs.BindParams); + const result = this.db.prepare(sql).run(...(params as never[])); - const changes = this.db.getRowsModified(); - - if (changes > 0) { + if (Number(result.changes) > 0) { this.addTaskActivity(taskId, "updated", "Task updated", actor); - this.save(); } - return changes > 0; + return Number(result.changes) > 0; } /** @@ -1032,8 +848,8 @@ export class TaskDatabase { WHERE id = ? `; - this.db.run(sql, [status, timestamp, notes ?? null, taskId]); - const changes = this.db.getRowsModified(); + const result = this.db.prepare(sql).run(status, timestamp, notes ?? null, taskId); + const changes = Number(result.changes); if (changes > 0) { this.addTaskActivity( @@ -1056,8 +872,6 @@ export class TaskDatabase { this.recomputeQueueDependents(taskId); } - this.save(); - // Return boundary metadata when marking as done or archived if (status === "done" || status === "archived") { const boundaries = this.getCompletionBoundaries(taskId); @@ -1075,12 +889,11 @@ export class TaskDatabase { */ setHandoffNotes(taskId: string, notes: string, actor?: string): boolean { const sql = `UPDATE tasks SET handoff_notes = ?, updated_at = ? WHERE id = ?`; - this.db.run(sql, [notes, getTimestamp(), taskId]); - const changes = this.db.getRowsModified(); + const result = this.db.prepare(sql).run(notes, getTimestamp(), taskId); + const changes = Number(result.changes); if (changes > 0) { this.addTaskActivity(taskId, "note", "Handoff notes updated", actor); - this.save(); } return changes > 0; @@ -1101,12 +914,11 @@ export class TaskDatabase { params.push(taskId); const sql = `UPDATE tasks SET ${setClauses.join(", ")} WHERE id = ?`; - this.db.run(sql, params as initSqlJs.BindParams); - const changes = this.db.getRowsModified(); + const result = this.db.prepare(sql).run(...(params as never[])); + const changes = Number(result.changes); if (changes > 0) { this.addTaskActivity(taskId, "progress", `Progress updated to ${percent}%`, actor); - this.save(); } return changes > 0; @@ -1122,12 +934,11 @@ export class TaskDatabase { WHERE id = ? `; - this.db.run(sql, [reason, getTimestamp(), taskId]); - const changes = this.db.getRowsModified(); + const result = this.db.prepare(sql).run(reason, getTimestamp(), taskId); + const changes = Number(result.changes); if (changes > 0) { this.addTaskActivity(taskId, "blocker_set", `Blocked: ${reason}`, actor); - this.save(); } return changes > 0; @@ -1143,12 +954,11 @@ export class TaskDatabase { WHERE id = ? `; - this.db.run(sql, [getTimestamp(), taskId]); - const changes = this.db.getRowsModified(); + const result = this.db.prepare(sql).run(getTimestamp(), taskId); + const changes = Number(result.changes); if (changes > 0) { this.addTaskActivity(taskId, "blocker_resolved", "Blocker resolved", actor); - this.save(); } return changes > 0; @@ -1169,8 +979,8 @@ export class TaskDatabase { WHERE id = ? `; - this.db.run(sql, [value ? 1 : 0, getTimestamp(), taskId]); - const changes = this.db.getRowsModified(); + const result = this.db.prepare(sql).run(value ? 1 : 0, getTimestamp(), taskId); + const changes = Number(result.changes); if (changes > 0) { this.addTaskActivity( @@ -1180,7 +990,6 @@ export class TaskDatabase { actor ); this.recomputeQueueEntry(taskId); - this.save(); } return changes > 0; @@ -1210,12 +1019,11 @@ export class TaskDatabase { const timestamp = getTimestamp(); const sql = `UPDATE tasks SET work_in_progress = ?, wip_updated_at = ?, updated_at = ? WHERE id = ?`; - this.db.run(sql, [JSON.stringify(merged), timestamp, timestamp, taskId]); - const changes = this.db.getRowsModified(); + const result = this.db.prepare(sql).run(JSON.stringify(merged), timestamp, timestamp, taskId); + const changes = Number(result.changes); if (changes > 0) { this.addTaskActivity(taskId, "wip_update", "Work in progress updated", actor); - this.save(); } return changes > 0; @@ -1231,8 +1039,8 @@ export class TaskDatabase { WHERE id = ? `; - this.db.run(sql, [getTimestamp(), taskId]); - const changes = this.db.getRowsModified(); + const result = this.db.prepare(sql).run(getTimestamp(), taskId); + const changes = Number(result.changes); if (changes > 0) { this.addTaskActivity( @@ -1242,7 +1050,6 @@ export class TaskDatabase { actor ); this.recomputeQueueEntry(taskId); - this.save(); } return changes > 0; @@ -1267,8 +1074,8 @@ export class TaskDatabase { WHERE id = ? `; - this.db.run(sql, [getTimestamp(), taskId]); - const changes = this.db.getRowsModified(); + const result = this.db.prepare(sql).run(getTimestamp(), taskId); + const changes = Number(result.changes); if (changes > 0) { this.addTaskActivity( @@ -1279,7 +1086,6 @@ export class TaskDatabase { ); this.recomputeQueueEntry(taskId); this.recomputeQueueDependents(taskId); - this.save(); } return changes > 0; @@ -1290,19 +1096,13 @@ export class TaskDatabase { */ deleteTask(taskId: string): boolean { // Delete related records first - this.db.run("DELETE FROM task_activity WHERE task_id = ?", [taskId]); - this.db.run("DELETE FROM task_files WHERE task_id = ?", [taskId]); - this.db.run("DELETE FROM task_dependencies WHERE task_id = ? OR depends_on_task_id = ?", [taskId, taskId]); - this.db.run("DELETE FROM work_queue WHERE task_id = ?", [taskId]); - - this.db.run("DELETE FROM tasks WHERE id = ?", [taskId]); - const changes = this.db.getRowsModified(); - - if (changes > 0) { - this.save(); - } + this.db.prepare("DELETE FROM task_activity WHERE task_id = ?").run(taskId); + this.db.prepare("DELETE FROM task_files WHERE task_id = ?").run(taskId); + this.db.prepare("DELETE FROM task_dependencies WHERE task_id = ? OR depends_on_task_id = ?").run(taskId, taskId); + this.db.prepare("DELETE FROM work_queue WHERE task_id = ?").run(taskId); - return changes > 0; + const result = this.db.prepare("DELETE FROM tasks WHERE id = ?").run(taskId); + return Number(result.changes) > 0; } /** @@ -1330,18 +1130,13 @@ export class TaskDatabase { // Delete all stories in this epic for (const story of stories) { - this.db.run("DELETE FROM stories WHERE id = ?", [story.id]); + this.db.prepare("DELETE FROM stories WHERE id = ?").run(story.id); } // Delete the epic - this.db.run("DELETE FROM epics WHERE id = ?", [epicId]); - const changes = this.db.getRowsModified(); - - if (changes > 0) { - this.save(); - } + const result = this.db.prepare("DELETE FROM epics WHERE id = ?").run(epicId); - return changes > 0; + return Number(result.changes) > 0; } /** @@ -1363,14 +1158,9 @@ export class TaskDatabase { } // Delete the story - this.db.run("DELETE FROM stories WHERE id = ?", [storyId]); - const changes = this.db.getRowsModified(); - - if (changes > 0) { - this.save(); - } + const result = this.db.prepare("DELETE FROM stories WHERE id = ?").run(storyId); - return changes > 0; + return Number(result.changes) > 0; } // ========================================================================== @@ -1394,14 +1184,11 @@ export class TaskDatabase { const depId = generateDependencyId(taskId, dependsOnTaskId); // Check if already exists - const stmt = this.db.prepare( + const existing = this.db.prepare( "SELECT id FROM task_dependencies WHERE task_id = ? AND depends_on_task_id = ?" - ); - stmt.bind([taskId, dependsOnTaskId]); - const exists = stmt.step(); - stmt.free(); + ).get(taskId, dependsOnTaskId); - if (exists) { + if (existing) { return null; } @@ -1410,14 +1197,12 @@ export class TaskDatabase { VALUES (?, ?, ?, ?, ?) `; - this.db.run(sql, [depId, taskId, dependsOnTaskId, dependencyType, getTimestamp()]); + this.db.prepare(sql).run(depId, taskId, dependsOnTaskId, dependencyType, getTimestamp()); // Recompute both tasks in work queue this.recomputeQueueEntry(taskId); this.recomputeQueueEntry(dependsOnTaskId); - this.save(); - return depId; } @@ -1425,18 +1210,16 @@ export class TaskDatabase { * Remove a dependency */ removeDependency(taskId: string, dependsOnTaskId: string): boolean { - this.db.run( - "DELETE FROM task_dependencies WHERE task_id = ? AND depends_on_task_id = ?", - [taskId, dependsOnTaskId] - ); + const result = this.db.prepare( + "DELETE FROM task_dependencies WHERE task_id = ? AND depends_on_task_id = ?" + ).run(taskId, dependsOnTaskId); - const changes = this.db.getRowsModified(); + const changes = Number(result.changes); if (changes > 0) { // Recompute both tasks in work queue this.recomputeQueueEntry(taskId); this.recomputeQueueEntry(dependsOnTaskId); - this.save(); } return changes > 0; @@ -1465,7 +1248,7 @@ export class TaskDatabase { VALUES (?, ?, ?, ?, ?, ?, ?, ?) `; - this.db.run(sql, [ + const result = this.db.prepare(sql).run( actId, taskId, activityType, @@ -1474,9 +1257,9 @@ export class TaskDatabase { newValue ?? null, actor ?? null, timestamp, - ]); + ); - return this.db.getRowsModified() > 0; + return Number(result.changes) > 0; } /** @@ -1499,20 +1282,18 @@ export class TaskDatabase { const summary = lines.join("\n"); // Store summary on task - this.db.run("UPDATE tasks SET activity_summary = ? WHERE id = ?", [summary, taskId]); + this.db.prepare("UPDATE tasks SET activity_summary = ? WHERE id = ?").run(summary, taskId); // Optionally delete old entries (keep last 3) if (deleteRaw && activities.length > 3) { const keepIds = activities.slice(0, 3).map((a) => a.id); const placeholders = keepIds.map(() => "?").join(","); - this.db.run( - `DELETE FROM task_activity WHERE task_id = ? AND id NOT IN (${placeholders})`, - [taskId, ...keepIds] - ); + this.db.prepare( + `DELETE FROM task_activity WHERE task_id = ? AND id NOT IN (${placeholders})` + ).run(...([taskId, ...keepIds] as never[])); } - this.save(); return summary; } @@ -1537,16 +1318,15 @@ export class TaskDatabase { VALUES (?, ?, ?, ?, ?, ?) `; - this.db.run(sql, [ + this.db.prepare(sql).run( failureId, taskId, failureType, failureReason, attempt ?? null, timestamp, - ]); + ); - this.save(); return failureId; } @@ -1559,16 +1339,7 @@ export class TaskDatabase { WHERE task_id = ? ORDER BY created_at DESC `; - const stmt = this.db.prepare(sql); - stmt.bind([taskId]); - - const rows: TaskFailure[] = []; - while (stmt.step()) { - rows.push(stmt.getAsObject() as unknown as TaskFailure); - } - stmt.free(); - - return rows; + return this.db.prepare(sql).all(taskId) as unknown as TaskFailure[]; } // ========================================================================== @@ -1591,19 +1362,15 @@ export class TaskDatabase { INSERT OR REPLACE INTO task_handoffs (task_id, status, summary, files_changed, full_details, created_at) VALUES (?, ?, ?, ?, ?, ?) `; - this.db.run(sql, [ + const result = this.db.prepare(sql).run( taskId, status, summary, filesChanged ? JSON.stringify(filesChanged) : null, fullDetails ?? null, timestamp, - ]); - const changes = this.db.getRowsModified(); - if (changes > 0) { - this.save(); - } - return changes > 0; + ); + return Number(result.changes) > 0; } /** @@ -1614,23 +1381,17 @@ export class TaskDatabase { const sql = includeDetails ? `SELECT * FROM task_handoffs WHERE task_id = ?` : `SELECT task_id, status, summary, files_changed, created_at, compacted_at FROM task_handoffs WHERE task_id = ?`; - const stmt = this.db.prepare(sql); - stmt.bind([taskId]); - if (stmt.step()) { - const row = stmt.getAsObject(); - stmt.free(); - return { - task_id: row.task_id as string, - status: row.status as HandoffStatus, - summary: row.summary as string, - files_changed: row.files_changed ? JSON.parse(row.files_changed as string) : undefined, - full_details: includeDetails ? (row.full_details !== null ? row.full_details as string : null) : undefined, - created_at: row.created_at as string ?? undefined, - compacted_at: row.compacted_at !== null ? row.compacted_at as string : null, - }; - } - stmt.free(); - return null; + const row = this.db.prepare(sql).get(taskId) as Record | undefined; + if (!row) return null; + return { + task_id: row.task_id as string, + status: row.status as HandoffStatus, + summary: row.summary as string, + files_changed: row.files_changed ? JSON.parse(row.files_changed as string) : undefined, + full_details: includeDetails ? (row.full_details !== null ? row.full_details as string : null) : undefined, + created_at: row.created_at as string ?? undefined, + compacted_at: row.compacted_at !== null ? row.compacted_at as string : null, + }; } // ========================================================================== @@ -1645,18 +1406,7 @@ export class TaskDatabase { compactStoryHandoffs(storyId: string): number { // Get all tasks in the story const sql = `SELECT id, status FROM tasks WHERE story_id = ?`; - const stmt = this.db.prepare(sql); - stmt.bind([storyId]); - - const taskRows: Array<{ id: string; status: string }> = []; - while (stmt.step()) { - const row = stmt.getAsObject(); - taskRows.push({ - id: row.id as string, - status: row.status as string, - }); - } - stmt.free(); + const taskRows = this.db.prepare(sql).all(storyId) as Array<{ id: string; status: string }>; let compacted = 0; const timestamp = getTimestamp(); @@ -1674,16 +1424,14 @@ export class TaskDatabase { if (handoff.full_details === null || handoff.full_details === undefined) continue; // Compact: null out full_details, set compacted_at - this.db.run( - `UPDATE task_handoffs SET full_details = NULL, compacted_at = ? WHERE task_id = ?`, - [timestamp, task.id] - ); - if (this.db.getRowsModified() > 0) { + const result = this.db.prepare( + `UPDATE task_handoffs SET full_details = NULL, compacted_at = ? WHERE task_id = ?` + ).run(timestamp, task.id); + if (Number(result.changes) > 0) { compacted++; } } - if (compacted > 0) this.save(); return compacted; } @@ -1695,33 +1443,14 @@ export class TaskDatabase { deleteEpicHandoffs(epicId: string): number { // Get all stories in the epic const storySql = `SELECT id FROM stories WHERE epic_id = ?`; - const storyStmt = this.db.prepare(storySql); - storyStmt.bind([epicId]); - - const storyRows: Array<{ id: string }> = []; - while (storyStmt.step()) { - const row = storyStmt.getAsObject(); - storyRows.push({ id: row.id as string }); - } - storyStmt.free(); + const storyRows = this.db.prepare(storySql).all(epicId) as Array<{ id: string }>; let deleted = 0; for (const story of storyRows) { // Get all tasks in the story const taskSql = `SELECT id, status FROM tasks WHERE story_id = ?`; - const taskStmt = this.db.prepare(taskSql); - taskStmt.bind([story.id]); - - const taskRows: Array<{ id: string; status: string }> = []; - while (taskStmt.step()) { - const row = taskStmt.getAsObject(); - taskRows.push({ - id: row.id as string, - status: row.status as string, - }); - } - taskStmt.free(); + const taskRows = this.db.prepare(taskSql).all(story.id) as Array<{ id: string; status: string }>; for (const task of taskRows) { // Skip blocked tasks @@ -1733,14 +1462,13 @@ export class TaskDatabase { if (handoff.status === "FAIL" || handoff.status === "BLOCKED") continue; // Delete handoff - this.db.run(`DELETE FROM task_handoffs WHERE task_id = ?`, [task.id]); - if (this.db.getRowsModified() > 0) { + const result = this.db.prepare(`DELETE FROM task_handoffs WHERE task_id = ?`).run(task.id); + if (Number(result.changes) > 0) { deleted++; } } } - if (deleted > 0) this.save(); return deleted; } @@ -1778,13 +1506,8 @@ export class TaskDatabase { SELECT COUNT(*) as count FROM task_dependencies WHERE depends_on_task_id = ? `; - const stmt = this.db.prepare(blocksSql); - stmt.bind([taskId]); - if (stmt.step()) { - const row = stmt.getAsObject() as { count: number }; - score += (row.count ?? 0) * 50; - } - stmt.free(); + const row = this.db.prepare(blocksSql).get(taskId) as { count: number } | undefined; + score += (row?.count ?? 0) * 50; return Math.round(score * 100) / 100; } @@ -1796,14 +1519,14 @@ export class TaskDatabase { recomputeQueueEntry(taskId: string): void { const task = this.getTask(taskId); if (!task) { - this.db.run("DELETE FROM work_queue WHERE task_id = ?", [taskId]); + this.db.prepare("DELETE FROM work_queue WHERE task_id = ?").run(taskId); return; } // Only queue tasks that are todo or needs_rework const eligible = (task.status === "todo" || (task.needs_rework && task.status !== "archived")); if (!eligible) { - this.db.run("DELETE FROM work_queue WHERE task_id = ?", [taskId]); + this.db.prepare("DELETE FROM work_queue WHERE task_id = ?").run(taskId); return; } @@ -1817,11 +1540,10 @@ export class TaskDatabase { const priorityMap: Record = { P0: 0, P1: 1, P2: 2, P3: 3 }; const batchGroup = priorityMap[task.epic_priority ?? "P2"] ?? 2; - this.db.run( + this.db.prepare( `INSERT OR REPLACE INTO work_queue (task_id, priority_score, batch_group, blocked_by, ready, computed_at) - VALUES (?, ?, ?, ?, ?, ?)`, - [taskId, score, batchGroup, blockedBy, ready, timestamp] - ); + VALUES (?, ?, ?, ?, ?, ?)` + ).run(taskId, score, batchGroup, blockedBy, ready, timestamp); } /** @@ -1833,14 +1555,8 @@ export class TaskDatabase { SELECT task_id FROM task_dependencies WHERE depends_on_task_id = ? `; - const stmt = this.db.prepare(sql); - stmt.bind([taskId]); - const dependentIds: string[] = []; - while (stmt.step()) { - const row = stmt.getAsObject() as { task_id: string }; - dependentIds.push(row.task_id); - } - stmt.free(); + const rows = this.db.prepare(sql).all(taskId) as Array<{ task_id: string }>; + const dependentIds = rows.map((row) => row.task_id); for (const depId of dependentIds) { this.recomputeQueueEntry(depId); @@ -1853,7 +1569,7 @@ export class TaskDatabase { */ rebuildWorkQueue(): void { // Clear existing queue - this.db.run("DELETE FROM work_queue"); + this.db.prepare("DELETE FROM work_queue").run(); // Get all eligible tasks const sql = ` @@ -1862,13 +1578,10 @@ export class TaskDatabase { WHERE (t.status = 'todo' OR t.needs_rework = 1) AND t.status != 'archived' `; - const result = this.db.exec(sql); - const tasks = resultToObjects<{ id: string }>(result); + const tasks = this.db.prepare(sql).all() as Array<{ id: string }>; for (const task of tasks) { this.recomputeQueueEntry(task.id); } - - this.save(); } } diff --git a/packages/ohno-core/vitest.config.ts b/packages/ohno-core/vitest.config.ts index 2e456d6..87f6a60 100644 --- a/packages/ohno-core/vitest.config.ts +++ b/packages/ohno-core/vitest.config.ts @@ -1,6 +1,34 @@ -import { defineConfig } from "vitest/config"; +import { defineConfig, type Plugin } from "vitest/config"; + +// node:sqlite is a Node 22.5+ built-in added after builtinModules was frozen. +// vite-node's normalizeModuleId() strips the "node:" prefix, so "node:sqlite" +// becomes "sqlite", which Vite can't find as a file. This plugin intercepts +// the stripped id and provides a virtual module that loads the real built-in +// via a dynamic import (which bypasses vite-node's normalisation). +const nodeSqlitePlugin: Plugin = { + name: "node-sqlite-external", + enforce: "pre", + resolveId(id) { + if (id === "sqlite") { + return "\0virtual:sqlite"; + } + }, + load(id) { + if (id === "\0virtual:sqlite") { + return ` +import { createRequire } from 'node:module'; +const _require = createRequire(import.meta.url); +const mod = _require('node:sqlite'); +export const DatabaseSync = mod.DatabaseSync; +export const StatementSync = mod.StatementSync; +export default mod; +`; + } + }, +}; export default defineConfig({ + plugins: [nodeSqlitePlugin], test: { globals: false, environment: "node", From a0196e6e599760be6e1c0d0694cfebe19c610c46 Mon Sep 17 00:00:00 2001 From: srstomp Date: Sun, 26 Apr 2026 16:43:26 +0200 Subject: [PATCH 04/11] chore(ci): drop Node 20 from CI matrix, add Node 22 and 24 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After bumping engines.node to >=22.16.0, update CI to match. - ci.yml: convert single Node 20 to a matrix of 22.x + 24.x with fail-fast: false so we see all version failures - release.yml: pin publish to 22.x (the engine floor — publishing on the lowest supported version catches accidentally-newer syntax) Phase 2 of the data-integrity plan, task-54d3aaf3. Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/ci.yml | 9 +++++++-- .github/workflows/release.yml | 2 +- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 85b63e2..3460cb8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -10,14 +10,19 @@ jobs: test: runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + node-version: ['22.x', '24.x'] + steps: - name: Checkout uses: actions/checkout@v4 - - name: Setup Node.js + - name: Setup Node.js ${{ matrix.node-version }} uses: actions/setup-node@v4 with: - node-version: '20' + node-version: ${{ matrix.node-version }} cache: 'npm' cache-dependency-path: packages/package-lock.json diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index db34ea6..fb22c76 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -28,7 +28,7 @@ jobs: - name: Setup Node.js uses: actions/setup-node@v4 with: - node-version: '20' + node-version: '22.x' cache: 'npm' cache-dependency-path: packages/package-lock.json registry-url: 'https://registry.npmjs.org' From 7e01c049e00672670b37fdea268a9c912cc9ae71 Mon Sep 17 00:00:00 2001 From: srstomp Date: Sun, 26 Apr 2026 17:13:59 +0200 Subject: [PATCH 05/11] feat(cli): migrate kanban.ts off sql.js to node:sqlite kanban.ts had its own independent sql.js usage (initSqlJs, getSqlJs cache, queryToObjects helper, fs.readFileSync into Uint8Array) that bypassed ohno-core's TaskDatabase. Migrate to node:sqlite's DatabaseSync in readonly mode with PRAGMA query_only = ON. - Open DB once per request via new DatabaseSync(dbPath, { readOnly: true }) - All 7 SELECT queries use db.prepare(sql).all() - queryToObjects helper deleted - try/finally ensures db.close() on the error path Smoke test: exportDatabase against .ohno/tasks.db returns expected counts (60 tasks, 47 done, 78%, 9 stories, 2 epics). task-fddb12c3. Unblocks task-860301ca (drop sql.js dep). Phase 2 of the data-integrity plan. Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/ohno-cli/src/kanban.ts | 239 ++++++++++++++------------------ 1 file changed, 105 insertions(+), 134 deletions(-) diff --git a/packages/ohno-cli/src/kanban.ts b/packages/ohno-cli/src/kanban.ts index fca1163..a7d7d57 100644 --- a/packages/ohno-cli/src/kanban.ts +++ b/packages/ohno-cli/src/kanban.ts @@ -1,25 +1,17 @@ /** * Kanban board generation + * + * Read-only view of the ohno tasks database for the `ohno serve` HTTP UI. + * Uses Node's built-in node:sqlite (DatabaseSync) in readonly mode. */ -import initSqlJs from "sql.js"; -import * as fs from "fs"; +import { DatabaseSync } from "node:sqlite"; import { createRequire } from "module"; import { KANBAN_TEMPLATE } from "./template.js"; const require = createRequire(import.meta.url); const pkg = require("../package.json"); -// Cache sql.js initialization -let sqlJsPromise: Promise | null = null; - -async function getSqlJs(): Promise { - if (!sqlJsPromise) { - sqlJsPromise = initSqlJs(); - } - return sqlJsPromise; -} - export interface KanbanData { synced_at: string; version: string; @@ -54,134 +46,113 @@ export interface KanbanData { }; } -/** - * Convert sql.js query result to array of objects - */ -function queryToObjects(db: initSqlJs.Database, sql: string): T[] { - try { - const result = db.exec(sql); - if (result.length === 0) return []; - const { columns, values } = result[0]; - return values.map((row) => { - const obj: Record = {}; - columns.forEach((col, i) => { - obj[col] = row[i]; - }); - return obj as T; - }); - } catch { - return []; - } -} - /** * Export database to JSON structure for kanban */ export async function exportDatabase(dbPath: string): Promise { - const SQL = await getSqlJs(); - const buffer = fs.readFileSync(dbPath); - const db = new SQL.Database(buffer); - - const data: KanbanData = { - synced_at: new Date().toISOString(), - version: pkg.version, - projects: [], - epics: [], - stories: [], - tasks: [], - dependencies: [], - task_activity: [], - task_files: [], - task_dependencies: [], - stats: { - total_tasks: 0, - done_tasks: 0, - blocked_tasks: 0, - in_progress_tasks: 0, - review_tasks: 0, - todo_tasks: 0, - completion_percent: 0, - total_stories: 0, - done_stories: 0, - total_epics: 0, - done_epics: 0, - p0_tasks: 0, - p1_tasks: 0, - total_estimate_hours: 0, - total_actual_hours: 0, - tasks_with_details: 0, - tasks_with_activity: 0, - tasks_with_files: 0, - tasks_with_dependencies: 0, - }, - }; + const db = new DatabaseSync(dbPath, { readOnly: true }); + db.exec("PRAGMA query_only = ON"); - // Export tables - data.projects = queryToObjects(db, "SELECT * FROM projects"); - data.epics = queryToObjects(db, "SELECT * FROM epics"); - data.stories = queryToObjects(db, "SELECT * FROM stories"); - - // Get tasks with joined info - data.tasks = queryToObjects(db, ` - SELECT - t.*, - s.title as story_title, - e.id as epic_id, - e.title as epic_title, - e.priority as epic_priority - FROM tasks t - LEFT JOIN stories s ON t.story_id = s.id - LEFT JOIN epics e ON s.epic_id = e.id - WHERE t.status != 'archived' - ORDER BY t.updated_at DESC - `); - - data.task_activity = queryToObjects(db, ` - SELECT a.*, t.title as task_title - FROM task_activity a - JOIN tasks t ON a.task_id = t.id - ORDER BY a.created_at DESC - LIMIT 100 - `); - - data.task_files = queryToObjects(db, "SELECT * FROM task_files"); - data.task_dependencies = queryToObjects(db, ` - SELECT d.*, t.title as depends_on_title, t.status as depends_on_status - FROM task_dependencies d - JOIN tasks t ON d.depends_on_task_id = t.id - `); - - // Compute stats - const tasks = data.tasks as { status: string; epic_priority?: string; estimate_hours?: number; actual_hours?: number; description?: string }[]; - - data.stats.total_tasks = tasks.length; - data.stats.done_tasks = tasks.filter((t) => t.status === "done").length; - data.stats.blocked_tasks = tasks.filter((t) => t.status === "blocked").length; - data.stats.in_progress_tasks = tasks.filter((t) => t.status === "in_progress").length; - data.stats.review_tasks = tasks.filter((t) => t.status === "review").length; - data.stats.todo_tasks = tasks.filter((t) => t.status === "todo").length; - - if (data.stats.total_tasks > 0) { - data.stats.completion_percent = Math.round( - (data.stats.done_tasks / data.stats.total_tasks) * 100 - ); + try { + const data: KanbanData = { + synced_at: new Date().toISOString(), + version: pkg.version, + projects: [], + epics: [], + stories: [], + tasks: [], + dependencies: [], + task_activity: [], + task_files: [], + task_dependencies: [], + stats: { + total_tasks: 0, + done_tasks: 0, + blocked_tasks: 0, + in_progress_tasks: 0, + review_tasks: 0, + todo_tasks: 0, + completion_percent: 0, + total_stories: 0, + done_stories: 0, + total_epics: 0, + done_epics: 0, + p0_tasks: 0, + p1_tasks: 0, + total_estimate_hours: 0, + total_actual_hours: 0, + tasks_with_details: 0, + tasks_with_activity: 0, + tasks_with_files: 0, + tasks_with_dependencies: 0, + }, + }; + + data.projects = db.prepare("SELECT * FROM projects").all() as unknown[]; + data.epics = db.prepare("SELECT * FROM epics").all() as unknown[]; + data.stories = db.prepare("SELECT * FROM stories").all() as unknown[]; + + data.tasks = db.prepare(` + SELECT + t.*, + s.title as story_title, + e.id as epic_id, + e.title as epic_title, + e.priority as epic_priority + FROM tasks t + LEFT JOIN stories s ON t.story_id = s.id + LEFT JOIN epics e ON s.epic_id = e.id + WHERE t.status != 'archived' + ORDER BY t.updated_at DESC + `).all() as unknown[]; + + data.task_activity = db.prepare(` + SELECT a.*, t.title as task_title + FROM task_activity a + JOIN tasks t ON a.task_id = t.id + ORDER BY a.created_at DESC + LIMIT 100 + `).all() as unknown[]; + + data.task_files = db.prepare("SELECT * FROM task_files").all() as unknown[]; + data.task_dependencies = db.prepare(` + SELECT d.*, t.title as depends_on_title, t.status as depends_on_status + FROM task_dependencies d + JOIN tasks t ON d.depends_on_task_id = t.id + `).all() as unknown[]; + + const tasks = data.tasks as { status: string; epic_priority?: string; estimate_hours?: number; actual_hours?: number; description?: string }[]; + + data.stats.total_tasks = tasks.length; + data.stats.done_tasks = tasks.filter((t) => t.status === "done").length; + data.stats.blocked_tasks = tasks.filter((t) => t.status === "blocked").length; + data.stats.in_progress_tasks = tasks.filter((t) => t.status === "in_progress").length; + data.stats.review_tasks = tasks.filter((t) => t.status === "review").length; + data.stats.todo_tasks = tasks.filter((t) => t.status === "todo").length; + + if (data.stats.total_tasks > 0) { + data.stats.completion_percent = Math.round( + (data.stats.done_tasks / data.stats.total_tasks) * 100 + ); + } + + data.stats.total_stories = (data.stories as unknown[]).length; + data.stats.total_epics = (data.epics as unknown[]).length; + data.stats.p0_tasks = tasks.filter((t) => t.epic_priority === "P0").length; + data.stats.p1_tasks = tasks.filter((t) => t.epic_priority === "P1").length; + + data.stats.total_estimate_hours = tasks.reduce((sum, t) => sum + (t.estimate_hours ?? 0), 0); + data.stats.total_actual_hours = tasks.reduce((sum, t) => sum + (t.actual_hours ?? 0), 0); + + data.stats.tasks_with_details = tasks.filter((t) => t.description).length; + data.stats.tasks_with_activity = new Set((data.task_activity as { task_id: string }[]).map((a) => a.task_id)).size; + data.stats.tasks_with_files = new Set((data.task_files as { task_id: string }[]).map((f) => f.task_id)).size; + data.stats.tasks_with_dependencies = new Set((data.task_dependencies as { task_id: string }[]).map((d) => d.task_id)).size; + + return data; + } finally { + db.close(); } - - data.stats.total_stories = (data.stories as unknown[]).length; - data.stats.total_epics = (data.epics as unknown[]).length; - data.stats.p0_tasks = tasks.filter((t) => t.epic_priority === "P0").length; - data.stats.p1_tasks = tasks.filter((t) => t.epic_priority === "P1").length; - - data.stats.total_estimate_hours = tasks.reduce((sum, t) => sum + (t.estimate_hours ?? 0), 0); - data.stats.total_actual_hours = tasks.reduce((sum, t) => sum + (t.actual_hours ?? 0), 0); - - data.stats.tasks_with_details = tasks.filter((t) => t.description).length; - data.stats.tasks_with_activity = new Set((data.task_activity as { task_id: string }[]).map((a) => a.task_id)).size; - data.stats.tasks_with_files = new Set((data.task_files as { task_id: string }[]).map((f) => f.task_id)).size; - data.stats.tasks_with_dependencies = new Set((data.task_dependencies as { task_id: string }[]).map((d) => d.task_id)).size; - - db.close(); - return data; } /** From 0108e0f387033795be6a18bd3bf144e80c180696 Mon Sep 17 00:00:00 2001 From: srstomp Date: Sun, 26 Apr 2026 17:20:55 +0200 Subject: [PATCH 06/11] chore: drop sql.js dependency from ohno-core and ohno-cli MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All call sites migrated to node:sqlite (DatabaseSync) in prior commits. - ohno-core: removed sql.js dep + @types/sql.js devDep (the dependencies block is now empty — node:sqlite is built-in) - ohno-cli: removed sql.js (was a leftover direct dep from when kanban.ts imported it independently) - Lockfile shrinks ~30 lines as sql.js + transitive deps are pruned Verified: all 3 packages build clean, 250 ohno-core + 156 ohno-cli tests pass, grep returns zero sql.js references in src or lockfile. task-860301ca. Phase 2 of the data-integrity plan. Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/ohno-cli/package.json | 3 +-- packages/ohno-core/package.json | 4 ---- packages/package-lock.json | 31 +------------------------------ 3 files changed, 2 insertions(+), 36 deletions(-) diff --git a/packages/ohno-cli/package.json b/packages/ohno-cli/package.json index 5a4d35c..5f63b9a 100644 --- a/packages/ohno-cli/package.json +++ b/packages/ohno-cli/package.json @@ -41,8 +41,7 @@ "chokidar": "^4.0.0", "commander": "^12.1.0", "ink": "^4.4.1", - "react": "^18.3.1", - "sql.js": "^1.11.0" + "react": "^18.3.1" }, "devDependencies": { "@types/node": "^22.10.0", diff --git a/packages/ohno-core/package.json b/packages/ohno-core/package.json index 986c95d..e670a4c 100644 --- a/packages/ohno-core/package.json +++ b/packages/ohno-core/package.json @@ -35,11 +35,7 @@ "test:watch": "vitest", "clean": "rm -rf dist" }, - "dependencies": { - "sql.js": "^1.11.0" - }, "devDependencies": { - "@types/sql.js": "^1.4.9", "@types/node": "^22.10.0", "typescript": "^5.7.0" }, diff --git a/packages/package-lock.json b/packages/package-lock.json index a3d2a2d..0629314 100644 --- a/packages/package-lock.json +++ b/packages/package-lock.json @@ -896,13 +896,6 @@ "resolved": "ohno-mcp", "link": true }, - "node_modules/@types/emscripten": { - "version": "1.41.5", - "resolved": "https://registry.npmjs.org/@types/emscripten/-/emscripten-1.41.5.tgz", - "integrity": "sha512-cMQm7pxu6BxtHyqJ7mQZ2kXWV5SLmugybFdHCBbJ5eHzOo6VhBckEgAT3//rP5FwPHNPeEiq4SmQ5ucBwsOo4Q==", - "dev": true, - "license": "MIT" - }, "node_modules/@types/estree": { "version": "1.0.8", "resolved": "https://registry.npmjs.org/@types/estree/-/estree-1.0.8.tgz", @@ -939,17 +932,6 @@ "csstype": "^3.2.2" } }, - "node_modules/@types/sql.js": { - "version": "1.4.9", - "resolved": "https://registry.npmjs.org/@types/sql.js/-/sql.js-1.4.9.tgz", - "integrity": "sha512-ep8b36RKHlgWPqjNG9ToUrPiwkhwh0AEzy883mO5Xnd+cL6VBH1EvSjBAAuxLUFF2Vn/moE3Me6v9E1Lo+48GQ==", - "dev": true, - "license": "MIT", - "dependencies": { - "@types/emscripten": "*", - "@types/node": "*" - } - }, "node_modules/@vitest/expect": { "version": "2.1.9", "resolved": "https://registry.npmjs.org/@vitest/expect/-/expect-2.1.9.tgz", @@ -2820,12 +2802,6 @@ "node": ">=0.10.0" } }, - "node_modules/sql.js": { - "version": "1.13.0", - "resolved": "https://registry.npmjs.org/sql.js/-/sql.js-1.13.0.tgz", - "integrity": "sha512-RJbVP1HRDlUUXahJ7VMTcu9Rm1Nzw+EBpoPr94vnbD4LwR715F3CcxE2G2k45PewcaZ57pjetYa+LoSJLAASgA==", - "license": "MIT" - }, "node_modules/stack-utils": { "version": "2.0.6", "resolved": "https://registry.npmjs.org/stack-utils/-/stack-utils-2.0.6.tgz", @@ -3845,8 +3821,7 @@ "chokidar": "^4.0.0", "commander": "^12.1.0", "ink": "^4.4.1", - "react": "^18.3.1", - "sql.js": "^1.11.0" + "react": "^18.3.1" }, "bin": { "ohno": "dist/index.js" @@ -3866,12 +3841,8 @@ "name": "@stevestomp/ohno-core", "version": "0.20.0", "license": "MIT", - "dependencies": { - "sql.js": "^1.11.0" - }, "devDependencies": { "@types/node": "^22.10.0", - "@types/sql.js": "^1.4.9", "typescript": "^5.7.0" }, "engines": { From e2153553367121431fa8ddc9b4170cfdd4dc869b Mon Sep 17 00:00:00 2001 From: srstomp Date: Sun, 26 Apr 2026 17:34:12 +0200 Subject: [PATCH 07/11] feat(core): wrap multi-statement writes in BEGIN IMMEDIATE transactions Adds private withTransaction(fn) helper using BEGIN IMMEDIATE so the write lock is acquired at transaction start, eliminating mid-transaction upgrade races between concurrent writers. The catch block attempts ROLLBACK in a nested try/catch so a throwing rollback doesn't shadow the original error. Wrapped 20 public multi-write methods: createTask, updateTask, updateTaskStatus, setHandoffNotes, updateTaskProgress, setBlocker, resolveBlocker, setNeedsRework, updateTaskWip, archiveTask, reopenTask, deleteTask, deleteEpic, deleteStory, addDependency, removeDependency, compactStoryHandoffs, deleteEpicHandoffs, rebuildWorkQueue, and summarizeTaskActivity. Extracted two private impl methods to avoid nested BEGIN IMMEDIATE when internal callers already hold a transaction: - _deleteTaskImpl: called by deleteEpic/deleteStory when cascading - _summarizeTaskActivityImpl: called by updateTaskStatus on auto-summarize Methods with a single write (createStory, updateStory, createEpic, updateEpic, addTaskActivity, recomputeQueueEntry) are intentionally unwrapped. Tests: 250/250 ohno-core + 156/156 ohno-cli pass. task-aba4f48b. Phase 2 of the data-integrity plan. Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/ohno-core/src/db.ts | 590 ++++++++++++++++++++--------------- 1 file changed, 337 insertions(+), 253 deletions(-) diff --git a/packages/ohno-core/src/db.ts b/packages/ohno-core/src/db.ts index 060e4bd..10e5cf8 100644 --- a/packages/ohno-core/src/db.ts +++ b/packages/ohno-core/src/db.ts @@ -68,6 +68,27 @@ import { export class TaskDatabase { private constructor(private db: DatabaseSync, public readonly dbPath: string) {} + /** + * Execute fn inside a BEGIN IMMEDIATE transaction. + * Acquires the write lock at start, avoiding mid-transaction upgrade races. + * If ROLLBACK itself throws, the original error propagates (not the rollback error). + */ + private withTransaction(fn: () => T): T { + this.db.exec('BEGIN IMMEDIATE'); + try { + const result = fn(); + this.db.exec('COMMIT'); + return result; + } catch (e) { + try { + this.db.exec('ROLLBACK'); + } catch { + // Swallow rollback errors so the original cause propagates. + } + throw e; + } + } + /** * Open or create a database (async factory) */ @@ -542,31 +563,33 @@ export class TaskDatabase { taskId = generateTaskId(opts.title, opts.story_id ?? null, `${timestamp}-${counter}`); } - const sql = ` - INSERT INTO tasks (id, story_id, title, status, task_type, description, estimate_hours, created_at, updated_at, created_by, source) - VALUES (?, ?, ?, 'todo', ?, ?, ?, ?, ?, ?, ?) - `; + return this.withTransaction(() => { + const sql = ` + INSERT INTO tasks (id, story_id, title, status, task_type, description, estimate_hours, created_at, updated_at, created_by, source) + VALUES (?, ?, ?, 'todo', ?, ?, ?, ?, ?, ?, ?) + `; - this.db.prepare(sql).run( - taskId, - opts.story_id ?? null, - opts.title, - opts.task_type ?? "feature", - opts.description ?? null, - opts.estimate_hours ?? null, - timestamp, - timestamp, - opts.actor ?? null, - opts.source ?? "human", - ); + this.db.prepare(sql).run( + taskId, + opts.story_id ?? null, + opts.title, + opts.task_type ?? "feature", + opts.description ?? null, + opts.estimate_hours ?? null, + timestamp, + timestamp, + opts.actor ?? null, + opts.source ?? "human", + ); - // Log activity - this.addTaskActivity(taskId, "created", `Task created: ${opts.title}`, opts.actor); + // Log activity + this.addTaskActivity(taskId, "created", `Task created: ${opts.title}`, opts.actor); - // Add to work queue - this.recomputeQueueEntry(taskId); + // Add to work queue + this.recomputeQueueEntry(taskId); - return taskId; + return taskId; + }); } /** @@ -811,14 +834,16 @@ export class TaskDatabase { params.push(getTimestamp()); params.push(taskId); - const sql = `UPDATE tasks SET ${setClauses.join(", ")} WHERE id = ?`; - const result = this.db.prepare(sql).run(...(params as never[])); + return this.withTransaction(() => { + const sql = `UPDATE tasks SET ${setClauses.join(", ")} WHERE id = ?`; + const result = this.db.prepare(sql).run(...(params as never[])); - if (Number(result.changes) > 0) { - this.addTaskActivity(taskId, "updated", "Task updated", actor); - } + if (Number(result.changes) > 0) { + this.addTaskActivity(taskId, "updated", "Task updated", actor); + } - return Number(result.changes) > 0; + return Number(result.changes) > 0; + }); } /** @@ -834,69 +859,73 @@ export class TaskDatabase { const oldStatus = task.status; const timestamp = getTimestamp(); - // Clear needs_rework when completing or archiving task - const clearNeedsRework = status === "done" || status === "archived"; - const sql = clearNeedsRework - ? ` - UPDATE tasks - SET status = ?, updated_at = ?, handoff_notes = COALESCE(?, handoff_notes), needs_rework = 0 - WHERE id = ? - ` - : ` - UPDATE tasks - SET status = ?, updated_at = ?, handoff_notes = COALESCE(?, handoff_notes) - WHERE id = ? - `; - - const result = this.db.prepare(sql).run(status, timestamp, notes ?? null, taskId); - const changes = Number(result.changes); - - if (changes > 0) { - this.addTaskActivity( - taskId, - "status_change", - `Status changed from ${oldStatus} to ${status}`, - actor, - oldStatus, - status - ); - - // Auto-summarize on completion - if (status === "done" || status === "archived") { - this.summarizeTaskActivity(taskId); - } + return this.withTransaction(() => { + // Clear needs_rework when completing or archiving task + const clearNeedsRework = status === "done" || status === "archived"; + const sql = clearNeedsRework + ? ` + UPDATE tasks + SET status = ?, updated_at = ?, handoff_notes = COALESCE(?, handoff_notes), needs_rework = 0 + WHERE id = ? + ` + : ` + UPDATE tasks + SET status = ?, updated_at = ?, handoff_notes = COALESCE(?, handoff_notes) + WHERE id = ? + `; + + const result = this.db.prepare(sql).run(status, timestamp, notes ?? null, taskId); + const changes = Number(result.changes); + + if (changes > 0) { + this.addTaskActivity( + taskId, + "status_change", + `Status changed from ${oldStatus} to ${status}`, + actor, + oldStatus, + status + ); + + // Auto-summarize on completion (use unwrapped impl to avoid nested BEGIN IMMEDIATE) + if (status === "done" || status === "archived") { + this._summarizeTaskActivityImpl(taskId, false, 5); + } - // Update work queue: remove completed/in-progress tasks, recompute dependents - this.recomputeQueueEntry(taskId); - if (status === "done" || status === "archived") { - this.recomputeQueueDependents(taskId); - } + // Update work queue: remove completed/in-progress tasks, recompute dependents + this.recomputeQueueEntry(taskId); + if (status === "done" || status === "archived") { + this.recomputeQueueDependents(taskId); + } - // Return boundary metadata when marking as done or archived - if (status === "done" || status === "archived") { - const boundaries = this.getCompletionBoundaries(taskId); - if (boundaries) { - return { success: true, boundaries }; + // Return boundary metadata when marking as done or archived + if (status === "done" || status === "archived") { + const boundaries = this.getCompletionBoundaries(taskId); + if (boundaries) { + return { success: true, boundaries }; + } } } - } - return { success: changes > 0 }; + return { success: changes > 0 }; + }); } /** * Set handoff notes for a task */ setHandoffNotes(taskId: string, notes: string, actor?: string): boolean { - const sql = `UPDATE tasks SET handoff_notes = ?, updated_at = ? WHERE id = ?`; - const result = this.db.prepare(sql).run(notes, getTimestamp(), taskId); - const changes = Number(result.changes); + return this.withTransaction(() => { + const sql = `UPDATE tasks SET handoff_notes = ?, updated_at = ? WHERE id = ?`; + const result = this.db.prepare(sql).run(notes, getTimestamp(), taskId); + const changes = Number(result.changes); - if (changes > 0) { - this.addTaskActivity(taskId, "note", "Handoff notes updated", actor); - } + if (changes > 0) { + this.addTaskActivity(taskId, "note", "Handoff notes updated", actor); + } - return changes > 0; + return changes > 0; + }); } /** @@ -913,55 +942,61 @@ export class TaskDatabase { params.push(taskId); - const sql = `UPDATE tasks SET ${setClauses.join(", ")} WHERE id = ?`; - const result = this.db.prepare(sql).run(...(params as never[])); - const changes = Number(result.changes); + return this.withTransaction(() => { + const sql = `UPDATE tasks SET ${setClauses.join(", ")} WHERE id = ?`; + const result = this.db.prepare(sql).run(...(params as never[])); + const changes = Number(result.changes); - if (changes > 0) { - this.addTaskActivity(taskId, "progress", `Progress updated to ${percent}%`, actor); - } + if (changes > 0) { + this.addTaskActivity(taskId, "progress", `Progress updated to ${percent}%`, actor); + } - return changes > 0; + return changes > 0; + }); } /** * Set a blocker on a task */ setBlocker(taskId: string, reason: string, actor?: string): boolean { - const sql = ` - UPDATE tasks - SET status = 'blocked', blockers = ?, updated_at = ? - WHERE id = ? - `; + return this.withTransaction(() => { + const sql = ` + UPDATE tasks + SET status = 'blocked', blockers = ?, updated_at = ? + WHERE id = ? + `; - const result = this.db.prepare(sql).run(reason, getTimestamp(), taskId); - const changes = Number(result.changes); + const result = this.db.prepare(sql).run(reason, getTimestamp(), taskId); + const changes = Number(result.changes); - if (changes > 0) { - this.addTaskActivity(taskId, "blocker_set", `Blocked: ${reason}`, actor); - } + if (changes > 0) { + this.addTaskActivity(taskId, "blocker_set", `Blocked: ${reason}`, actor); + } - return changes > 0; + return changes > 0; + }); } /** * Resolve a blocker */ resolveBlocker(taskId: string, actor?: string): boolean { - const sql = ` - UPDATE tasks - SET status = 'in_progress', blockers = NULL, updated_at = ? - WHERE id = ? - `; + return this.withTransaction(() => { + const sql = ` + UPDATE tasks + SET status = 'in_progress', blockers = NULL, updated_at = ? + WHERE id = ? + `; - const result = this.db.prepare(sql).run(getTimestamp(), taskId); - const changes = Number(result.changes); + const result = this.db.prepare(sql).run(getTimestamp(), taskId); + const changes = Number(result.changes); - if (changes > 0) { - this.addTaskActivity(taskId, "blocker_resolved", "Blocker resolved", actor); - } + if (changes > 0) { + this.addTaskActivity(taskId, "blocker_resolved", "Blocker resolved", actor); + } - return changes > 0; + return changes > 0; + }); } /** @@ -973,26 +1008,28 @@ export class TaskDatabase { return false; } - const sql = ` - UPDATE tasks - SET needs_rework = ?, updated_at = ? - WHERE id = ? - `; - - const result = this.db.prepare(sql).run(value ? 1 : 0, getTimestamp(), taskId); - const changes = Number(result.changes); + return this.withTransaction(() => { + const sql = ` + UPDATE tasks + SET needs_rework = ?, updated_at = ? + WHERE id = ? + `; - if (changes > 0) { - this.addTaskActivity( - taskId, - "updated", - `Task ${value ? "marked as" : "cleared from"} needs rework`, - actor - ); - this.recomputeQueueEntry(taskId); - } + const result = this.db.prepare(sql).run(value ? 1 : 0, getTimestamp(), taskId); + const changes = Number(result.changes); + + if (changes > 0) { + this.addTaskActivity( + taskId, + "updated", + `Task ${value ? "marked as" : "cleared from"} needs rework`, + actor + ); + this.recomputeQueueEntry(taskId); + } - return changes > 0; + return changes > 0; + }); } /** @@ -1017,42 +1054,46 @@ export class TaskDatabase { } const merged = { ...existing, ...wipData }; - const timestamp = getTimestamp(); - const sql = `UPDATE tasks SET work_in_progress = ?, wip_updated_at = ?, updated_at = ? WHERE id = ?`; - const result = this.db.prepare(sql).run(JSON.stringify(merged), timestamp, timestamp, taskId); - const changes = Number(result.changes); + return this.withTransaction(() => { + const timestamp = getTimestamp(); + const sql = `UPDATE tasks SET work_in_progress = ?, wip_updated_at = ?, updated_at = ? WHERE id = ?`; + const result = this.db.prepare(sql).run(JSON.stringify(merged), timestamp, timestamp, taskId); + const changes = Number(result.changes); - if (changes > 0) { - this.addTaskActivity(taskId, "wip_update", "Work in progress updated", actor); - } + if (changes > 0) { + this.addTaskActivity(taskId, "wip_update", "Work in progress updated", actor); + } - return changes > 0; + return changes > 0; + }); } /** * Archive a task */ archiveTask(taskId: string, reason?: string, actor?: string): boolean { - const sql = ` - UPDATE tasks - SET status = 'archived', updated_at = ? - WHERE id = ? - `; - - const result = this.db.prepare(sql).run(getTimestamp(), taskId); - const changes = Number(result.changes); + return this.withTransaction(() => { + const sql = ` + UPDATE tasks + SET status = 'archived', updated_at = ? + WHERE id = ? + `; - if (changes > 0) { - this.addTaskActivity( - taskId, - "status_change", - `Task archived${reason ? `: ${reason}` : ""}`, - actor - ); - this.recomputeQueueEntry(taskId); - } + const result = this.db.prepare(sql).run(getTimestamp(), taskId); + const changes = Number(result.changes); + + if (changes > 0) { + this.addTaskActivity( + taskId, + "status_change", + `Task archived${reason ? `: ${reason}` : ""}`, + actor + ); + this.recomputeQueueEntry(taskId); + } - return changes > 0; + return changes > 0; + }); } /** @@ -1068,33 +1109,37 @@ export class TaskDatabase { return false; } - const sql = ` - UPDATE tasks - SET status = 'todo', activity_summary = NULL, needs_rework = 0, updated_at = ? - WHERE id = ? - `; - - const result = this.db.prepare(sql).run(getTimestamp(), taskId); - const changes = Number(result.changes); + return this.withTransaction(() => { + const sql = ` + UPDATE tasks + SET status = 'todo', activity_summary = NULL, needs_rework = 0, updated_at = ? + WHERE id = ? + `; - if (changes > 0) { - this.addTaskActivity( - taskId, - "reopen", - `Task reopened from ${task.status}${notes ? `: ${notes}` : ""}`, - actor - ); - this.recomputeQueueEntry(taskId); - this.recomputeQueueDependents(taskId); - } + const result = this.db.prepare(sql).run(getTimestamp(), taskId); + const changes = Number(result.changes); + + if (changes > 0) { + this.addTaskActivity( + taskId, + "reopen", + `Task reopened from ${task.status}${notes ? `: ${notes}` : ""}`, + actor + ); + this.recomputeQueueEntry(taskId); + this.recomputeQueueDependents(taskId); + } - return changes > 0; + return changes > 0; + }); } /** - * Delete a task (hard delete) + * Internal unwrapped implementation for deleting a task. + * Called by deleteTask (which wraps it) and by deleteEpic/deleteStory + * (which wrap their own outer transaction and call this directly to avoid nesting). */ - deleteTask(taskId: string): boolean { + private _deleteTaskImpl(taskId: string): boolean { // Delete related records first this.db.prepare("DELETE FROM task_activity WHERE task_id = ?").run(taskId); this.db.prepare("DELETE FROM task_files WHERE task_id = ?").run(taskId); @@ -1105,6 +1150,13 @@ export class TaskDatabase { return Number(result.changes) > 0; } + /** + * Delete a task (hard delete) + */ + deleteTask(taskId: string): boolean { + return this.withTransaction(() => this._deleteTaskImpl(taskId)); + } + /** * Delete an epic (hard delete) * Also deletes all stories and tasks associated with the epic @@ -1115,28 +1167,31 @@ export class TaskDatabase { return false; } - // Get all stories in this epic + // Get all stories in this epic before entering the transaction const stories = this.getStories({ epic_id: epicId }); - // Delete all tasks in each story - for (const story of stories) { - const tasks = this.getTasks({ limit: 1000 }); - for (const task of tasks) { - if (task.story_id === story.id) { - this.deleteTask(task.id); + return this.withTransaction(() => { + // Delete all tasks in each story + for (const story of stories) { + const tasks = this.getTasks({ limit: 1000 }); + for (const task of tasks) { + if (task.story_id === story.id) { + // Use unwrapped impl to avoid nested BEGIN IMMEDIATE + this._deleteTaskImpl(task.id); + } } } - } - // Delete all stories in this epic - for (const story of stories) { - this.db.prepare("DELETE FROM stories WHERE id = ?").run(story.id); - } + // Delete all stories in this epic + for (const story of stories) { + this.db.prepare("DELETE FROM stories WHERE id = ?").run(story.id); + } - // Delete the epic - const result = this.db.prepare("DELETE FROM epics WHERE id = ?").run(epicId); + // Delete the epic + const result = this.db.prepare("DELETE FROM epics WHERE id = ?").run(epicId); - return Number(result.changes) > 0; + return Number(result.changes) > 0; + }); } /** @@ -1149,18 +1204,23 @@ export class TaskDatabase { return false; } - // Delete all tasks in this story + // Get tasks before entering the transaction const tasks = this.getTasks({ limit: 1000 }); - for (const task of tasks) { - if (task.story_id === storyId) { - this.deleteTask(task.id); + + return this.withTransaction(() => { + // Delete all tasks in this story + for (const task of tasks) { + if (task.story_id === storyId) { + // Use unwrapped impl to avoid nested BEGIN IMMEDIATE + this._deleteTaskImpl(task.id); + } } - } - // Delete the story - const result = this.db.prepare("DELETE FROM stories WHERE id = ?").run(storyId); + // Delete the story + const result = this.db.prepare("DELETE FROM stories WHERE id = ?").run(storyId); - return Number(result.changes) > 0; + return Number(result.changes) > 0; + }); } // ========================================================================== @@ -1192,37 +1252,41 @@ export class TaskDatabase { return null; } - const sql = ` - INSERT INTO task_dependencies (id, task_id, depends_on_task_id, dependency_type, created_at) - VALUES (?, ?, ?, ?, ?) - `; + return this.withTransaction(() => { + const sql = ` + INSERT INTO task_dependencies (id, task_id, depends_on_task_id, dependency_type, created_at) + VALUES (?, ?, ?, ?, ?) + `; - this.db.prepare(sql).run(depId, taskId, dependsOnTaskId, dependencyType, getTimestamp()); + this.db.prepare(sql).run(depId, taskId, dependsOnTaskId, dependencyType, getTimestamp()); - // Recompute both tasks in work queue - this.recomputeQueueEntry(taskId); - this.recomputeQueueEntry(dependsOnTaskId); + // Recompute both tasks in work queue + this.recomputeQueueEntry(taskId); + this.recomputeQueueEntry(dependsOnTaskId); - return depId; + return depId; + }); } /** * Remove a dependency */ removeDependency(taskId: string, dependsOnTaskId: string): boolean { - const result = this.db.prepare( - "DELETE FROM task_dependencies WHERE task_id = ? AND depends_on_task_id = ?" - ).run(taskId, dependsOnTaskId); + return this.withTransaction(() => { + const result = this.db.prepare( + "DELETE FROM task_dependencies WHERE task_id = ? AND depends_on_task_id = ?" + ).run(taskId, dependsOnTaskId); - const changes = Number(result.changes); + const changes = Number(result.changes); - if (changes > 0) { - // Recompute both tasks in work queue - this.recomputeQueueEntry(taskId); - this.recomputeQueueEntry(dependsOnTaskId); - } + if (changes > 0) { + // Recompute both tasks in work queue + this.recomputeQueueEntry(taskId); + this.recomputeQueueEntry(dependsOnTaskId); + } - return changes > 0; + return changes > 0; + }); } // ========================================================================== @@ -1263,9 +1327,19 @@ export class TaskDatabase { } /** - * Summarize task activity into a compressed text + * Summarize task activity into a compressed text. + * When deleteRaw=true, this performs 2 writes (summary update + raw delete); + * the public method wraps in a transaction. Internal callers that already + * hold a transaction (e.g., updateTaskStatus) call _summarizeTaskActivityImpl + * directly to avoid nested BEGIN IMMEDIATE. */ summarizeTaskActivity(taskId: string, deleteRaw = false, minEntries = 5): string | null { + return this.withTransaction(() => + this._summarizeTaskActivityImpl(taskId, deleteRaw, minEntries) + ); + } + + private _summarizeTaskActivityImpl(taskId: string, deleteRaw: boolean, minEntries: number): string | null { const activities = this.getTaskActivity(taskId, 100); if (activities.length < minEntries) { @@ -1404,35 +1478,38 @@ export class TaskDatabase { * Skips blocked/failed tasks (preserve for debugging). */ compactStoryHandoffs(storyId: string): number { - // Get all tasks in the story + // Get all tasks in the story and their handoffs before the transaction const sql = `SELECT id, status FROM tasks WHERE story_id = ?`; const taskRows = this.db.prepare(sql).all(storyId) as Array<{ id: string; status: string }>; - let compacted = 0; - const timestamp = getTimestamp(); - - for (const task of taskRows) { - // Skip blocked tasks - if (task.status === "blocked") continue; - - // Check handoff - need full details to see if already compacted + // Filter eligible tasks outside the transaction (reads) + const eligible = taskRows.filter((task) => { + if (task.status === "blocked") return false; const handoff = this.getTaskHandoff(task.id, true); - if (!handoff) continue; - if (handoff.status === "FAIL" || handoff.status === "BLOCKED") continue; - - // Skip if already compacted (full_details is already null or undefined) - if (handoff.full_details === null || handoff.full_details === undefined) continue; - - // Compact: null out full_details, set compacted_at - const result = this.db.prepare( - `UPDATE task_handoffs SET full_details = NULL, compacted_at = ? WHERE task_id = ?` - ).run(timestamp, task.id); - if (Number(result.changes) > 0) { - compacted++; + if (!handoff) return false; + if (handoff.status === "FAIL" || handoff.status === "BLOCKED") return false; + if (handoff.full_details === null || handoff.full_details === undefined) return false; + return true; + }); + + if (eligible.length === 0) return 0; + + return this.withTransaction(() => { + let compacted = 0; + const timestamp = getTimestamp(); + + for (const task of eligible) { + // Compact: null out full_details, set compacted_at + const result = this.db.prepare( + `UPDATE task_handoffs SET full_details = NULL, compacted_at = ? WHERE task_id = ?` + ).run(timestamp, task.id); + if (Number(result.changes) > 0) { + compacted++; + } } - } - return compacted; + return compacted; + }); } /** @@ -1445,31 +1522,36 @@ export class TaskDatabase { const storySql = `SELECT id FROM stories WHERE epic_id = ?`; const storyRows = this.db.prepare(storySql).all(epicId) as Array<{ id: string }>; - let deleted = 0; - + // Collect eligible task IDs outside the transaction (reads) + const eligibleTaskIds: string[] = []; for (const story of storyRows) { - // Get all tasks in the story const taskSql = `SELECT id, status FROM tasks WHERE story_id = ?`; const taskRows = this.db.prepare(taskSql).all(story.id) as Array<{ id: string; status: string }>; for (const task of taskRows) { - // Skip blocked tasks if (task.status === "blocked") continue; - - // Check handoff status - skip FAIL and BLOCKED const handoff = this.getTaskHandoff(task.id, false); if (!handoff) continue; if (handoff.status === "FAIL" || handoff.status === "BLOCKED") continue; + eligibleTaskIds.push(task.id); + } + } + + if (eligibleTaskIds.length === 0) return 0; + + return this.withTransaction(() => { + let deleted = 0; + for (const taskId of eligibleTaskIds) { // Delete handoff - const result = this.db.prepare(`DELETE FROM task_handoffs WHERE task_id = ?`).run(task.id); + const result = this.db.prepare(`DELETE FROM task_handoffs WHERE task_id = ?`).run(taskId); if (Number(result.changes) > 0) { deleted++; } } - } - return deleted; + return deleted; + }); } // ========================================================================== @@ -1568,10 +1650,7 @@ export class TaskDatabase { * Called on first use or when queue is detected as stale. */ rebuildWorkQueue(): void { - // Clear existing queue - this.db.prepare("DELETE FROM work_queue").run(); - - // Get all eligible tasks + // Get all eligible tasks before the transaction const sql = ` SELECT t.id FROM tasks t @@ -1580,8 +1659,13 @@ export class TaskDatabase { `; const tasks = this.db.prepare(sql).all() as Array<{ id: string }>; - for (const task of tasks) { - this.recomputeQueueEntry(task.id); - } + this.withTransaction(() => { + // Clear existing queue + this.db.prepare("DELETE FROM work_queue").run(); + + for (const task of tasks) { + this.recomputeQueueEntry(task.id); + } + }); } } From 981b1d65ccd59f6ce29d7c011173450a3509cac3 Mon Sep 17 00:00:00 2001 From: srstomp Date: Sun, 26 Apr 2026 17:59:30 +0200 Subject: [PATCH 08/11] feat: map SQLITE_BUSY/LOCKED errors to user-facing messages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a typed OhnoDatabaseLockedError class (exported from ohno-core) and wires SQLite error mapping across the three layers that can hit a lock: db.ts internals, the CLI bin entry, and the MCP tool dispatcher. ohno-core (db.ts): - Add private mapSqliteError(e): never helper that inspects e.errcode (numeric, not e.code which is always 'ERR_SQLITE_ERROR') and throws OhnoDatabaseLockedError for BUSY (5) and BUSY_SNAPSHOT (261), a plain bug-flagged Error for LOCKED (6), and re-throws other SQLite errors with [errcode=N] preserved in the message - Wrap open()'s DatabaseSync construction + early PRAGMA block so a BUSY at open time maps cleanly (close db before mapping; pass-through for our own journal_mode/quick_check assertion errors) - Wrap withTransaction's BEGIN IMMEDIATE and the inner body so both can surface as OhnoDatabaseLockedError; rollback errors still swallowed - Wrap the four single-write public methods (createStory, updateStory, createEpic, updateEpic) — other writes inherit mapping via withTransaction ohno-cli (index.ts): - main().catch() handles OhnoDatabaseLockedError: stderr by default, { success: false, error, errorCode } JSON to stdout when --json is in argv (commander state isn't available at the global handler) ohno-mcp (server.ts): - handleTool wraps the switch in try/catch; OhnoDatabaseLockedError becomes { success: false, error, errorCode } in the tool response; other errors re-throw to the framework Drive-by fix: ohno-mcp's vitest.config.ts was missing the node-sqlite- external plugin, so its tests weren't actually running. Plugin added; 5 pre-existing whitebox sites in server.test.ts that used the old sql.js db.run(sql, [array]) shape now use prepare(sql).run(...args) with a DatabaseSync type cast, matching the same fix already done in db.test.ts. Spec said e.code is the SQLite code string; verified experimentally that node:sqlite (Node 22.16+/24) actually uses e.errcode (numeric) — the implementation matches reality and the test at db.test.ts:2083 provokes a real SQLITE_BUSY to lock that in. Tests: 258 ohno-core + 159 ohno-cli + 232 ohno-mcp = 649 pass. task-1064ddb2. Phase 2 of the data-integrity plan. Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/ohno-cli/src/cli.test.ts | 87 +++++++++++- packages/ohno-cli/src/index.ts | 25 +++- packages/ohno-core/src/db.test.ts | 104 ++++++++++++++- packages/ohno-core/src/db.ts | 193 ++++++++++++++++++++------- packages/ohno-core/src/index.ts | 2 +- packages/ohno-mcp/src/server.test.ts | 102 +++++++++----- packages/ohno-mcp/src/server.ts | 13 +- packages/ohno-mcp/vitest.config.ts | 31 ++++- 8 files changed, 471 insertions(+), 86 deletions(-) diff --git a/packages/ohno-cli/src/cli.test.ts b/packages/ohno-cli/src/cli.test.ts index 6ee9ebd..3548d51 100644 --- a/packages/ohno-cli/src/cli.test.ts +++ b/packages/ohno-cli/src/cli.test.ts @@ -7,7 +7,7 @@ import { mkdtempSync, rmSync, mkdirSync } from "fs"; import { tmpdir } from "os"; import { join } from "path"; import { createRequire } from "module"; -import { TaskDatabase } from "@stevestomp/ohno-core"; +import { TaskDatabase, OhnoDatabaseLockedError } from "@stevestomp/ohno-core"; import { createCli } from "./cli.js"; const require = createRequire(import.meta.url); @@ -1347,4 +1347,89 @@ describe("CLI Commands", () => { expect(watchOption).toBeDefined(); }); }); + + describe("OhnoDatabaseLockedError - CLI entry point error handler", () => { + it("MUST: OhnoDatabaseLockedError is a real error class importable from ohno-core", () => { + const err = new OhnoDatabaseLockedError("SQLITE_BUSY", "test"); + expect(err).toBeInstanceOf(OhnoDatabaseLockedError); + expect(err.sqliteCode).toBe("SQLITE_BUSY"); + }); + + it("MUST: CLI error handler emits stderr message for OhnoDatabaseLockedError without --json", () => { + // Test the logic of the main().catch() handler by simulating it directly. + const stderrSpy = vi.spyOn(process.stderr, "write").mockImplementation(() => true); + const stdoutSpy = vi.spyOn(process.stdout, "write").mockImplementation(() => true); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const exitSpy = vi.spyOn(process, "exit").mockImplementation((() => {}) as any); + + const err = new OhnoDatabaseLockedError( + "SQLITE_BUSY", + "Database is locked by another ohno process; retry timed out after 5s. Try again, or check for stale ohno-mcp processes with 'ps aux | grep ohno-mcp'." + ); + + // Simulate the catch handler from index.ts (without --json in argv) + const originalArgv = process.argv; + process.argv = ["node", "ohno", "tasks"]; // no --json + if (err instanceof OhnoDatabaseLockedError) { + if (process.argv.includes('--json')) { + process.stdout.write(JSON.stringify({ + success: false, + error: err.message, + errorCode: err.sqliteCode, + }) + '\n'); + } else { + process.stderr.write(err.message + '\n'); + } + process.exit(1); + } + process.argv = originalArgv; + + expect(stderrSpy).toHaveBeenCalledWith(expect.stringContaining("retry timed out after 5s")); + expect(stdoutSpy).not.toHaveBeenCalled(); + expect(exitSpy).toHaveBeenCalledWith(1); + + stderrSpy.mockRestore(); + stdoutSpy.mockRestore(); + exitSpy.mockRestore(); + }); + + it("MUST: CLI error handler emits JSON to stdout for OhnoDatabaseLockedError with --json", () => { + const stderrSpy = vi.spyOn(process.stderr, "write").mockImplementation(() => true); + const stdoutSpy = vi.spyOn(process.stdout, "write").mockImplementation(() => true); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const exitSpy = vi.spyOn(process, "exit").mockImplementation((() => {}) as any); + + const err = new OhnoDatabaseLockedError( + "SQLITE_BUSY", + "Database is locked by another ohno process; retry timed out after 5s. Try again, or check for stale ohno-mcp processes with 'ps aux | grep ohno-mcp'." + ); + + const originalArgv = process.argv; + process.argv = ["node", "ohno", "tasks", "--json"]; + if (err instanceof OhnoDatabaseLockedError) { + if (process.argv.includes('--json')) { + process.stdout.write(JSON.stringify({ + success: false, + error: err.message, + errorCode: err.sqliteCode, + }) + '\n'); + } else { + process.stderr.write(err.message + '\n'); + } + process.exit(1); + } + process.argv = originalArgv; + + const jsonOutput = JSON.parse(stdoutSpy.mock.calls[0]?.[0] as string); + expect(jsonOutput.success).toBe(false); + expect(jsonOutput.error).toContain("retry timed out after 5s"); + expect(jsonOutput.errorCode).toBe("SQLITE_BUSY"); + expect(stderrSpy).not.toHaveBeenCalled(); + expect(exitSpy).toHaveBeenCalledWith(1); + + stderrSpy.mockRestore(); + stdoutSpy.mockRestore(); + exitSpy.mockRestore(); + }); + }); }); \ No newline at end of file diff --git a/packages/ohno-cli/src/index.ts b/packages/ohno-cli/src/index.ts index 81a89c6..5682b9c 100644 --- a/packages/ohno-cli/src/index.ts +++ b/packages/ohno-cli/src/index.ts @@ -5,6 +5,27 @@ import "./node-guard.js"; import { createCli } from "./cli.js"; +import { OhnoDatabaseLockedError } from "@stevestomp/ohno-core"; -const program = createCli(); -program.parse(); +async function main() { + const program = createCli(); + await program.parseAsync(); +} + +main().catch((err) => { + if (err instanceof OhnoDatabaseLockedError) { + if (process.argv.includes('--json')) { + process.stdout.write(JSON.stringify({ + success: false, + error: err.message, + errorCode: err.sqliteCode, + }) + '\n'); + } else { + process.stderr.write(err.message + '\n'); + } + process.exit(1); + } + // Re-throw or print other errors using existing semantics + process.stderr.write(String(err?.stack ?? err) + '\n'); + process.exit(1); +}); diff --git a/packages/ohno-core/src/db.test.ts b/packages/ohno-core/src/db.test.ts index 9f00a86..b54cf40 100644 --- a/packages/ohno-core/src/db.test.ts +++ b/packages/ohno-core/src/db.test.ts @@ -7,7 +7,7 @@ import { mkdtempSync, rmSync } from "fs"; import { tmpdir } from "os"; import { join } from "path"; import type { DatabaseSync } from "node:sqlite"; -import { TaskDatabase } from "./db.js"; +import { TaskDatabase, OhnoDatabaseLockedError } from "./db.js"; import type { TaskStatus } from "./types.js"; describe("TaskDatabase", () => { @@ -2031,4 +2031,106 @@ describe("TaskDatabase", () => { }); }); }); + + // ========================================================================== + // SQLite Error Mapping Tests + // ========================================================================== + + describe("OhnoDatabaseLockedError - class shape", () => { + it("MUST: OhnoDatabaseLockedError is exported and constructable", () => { + const err = new OhnoDatabaseLockedError("SQLITE_BUSY", "test message"); + expect(err).toBeInstanceOf(OhnoDatabaseLockedError); + expect(err).toBeInstanceOf(Error); + }); + + it("MUST: OhnoDatabaseLockedError has correct name", () => { + const err = new OhnoDatabaseLockedError("SQLITE_BUSY", "test message"); + expect(err.name).toBe("OhnoDatabaseLockedError"); + }); + + it("MUST: OhnoDatabaseLockedError carries sqliteCode property", () => { + const err = new OhnoDatabaseLockedError("SQLITE_BUSY", "test message"); + expect(err.sqliteCode).toBe("SQLITE_BUSY"); + }); + + it("MUST: OhnoDatabaseLockedError message matches constructor arg", () => { + const err = new OhnoDatabaseLockedError("SQLITE_BUSY", "Database is locked by another ohno process"); + expect(err.message).toBe("Database is locked by another ohno process"); + }); + }); + + describe("SQLITE_BUSY error message shape", () => { + it("MUST: SQLITE_BUSY OhnoDatabaseLockedError has exact spec wording with sqliteCode SQLITE_BUSY", () => { + const err = new OhnoDatabaseLockedError( + "SQLITE_BUSY", + "Database is locked by another ohno process; retry timed out after 5s. Try again, or check for stale ohno-mcp processes with 'ps aux | grep ohno-mcp'." + ); + expect(err.sqliteCode).toBe("SQLITE_BUSY"); + expect(err.message).toContain("retry timed out after 5s"); + expect(err.message).toContain("ps aux | grep ohno-mcp"); + }); + + it("MUST: SQLITE_BUSY_SNAPSHOT maps to OhnoDatabaseLockedError with documented retry wording", () => { + const err = new OhnoDatabaseLockedError( + "SQLITE_BUSY_SNAPSHOT", + "Database changed during transaction; retry" + ); + expect(err.sqliteCode).toBe("SQLITE_BUSY_SNAPSHOT"); + expect(err.message).toBe("Database changed during transaction; retry"); + }); + }); + + describe("node:sqlite error property verification", () => { + it("MUST: node:sqlite throws errors with .errcode=5 for SQLITE_BUSY and .code='ERR_SQLITE_ERROR'", () => { + // Use two DatabaseSync connections to the same file with timeout:0 to get immediate SQLITE_BUSY + const { DatabaseSync } = require("node:sqlite") as { DatabaseSync: typeof import("node:sqlite").DatabaseSync }; + const busyDbPath = join(tempDir, "busy-verify.db"); + const conn1 = new DatabaseSync(busyDbPath, { timeout: 0 }); + const conn2 = new DatabaseSync(busyDbPath, { timeout: 0 }); + conn1.exec("BEGIN EXCLUSIVE"); + let caughtError: unknown; + try { + conn2.exec("BEGIN EXCLUSIVE"); + } catch (e) { + caughtError = e; + } finally { + conn1.exec("ROLLBACK"); + conn1.close(); + conn2.close(); + } + // Verify node:sqlite uses .errcode (numeric, 5=SQLITE_BUSY) not .code (which is 'ERR_SQLITE_ERROR') + expect(caughtError).toBeDefined(); + expect((caughtError as { code?: string })?.code).toBe("ERR_SQLITE_ERROR"); + expect((caughtError as { errcode?: number })?.errcode).toBe(5); // SQLITE_BUSY = 5 + }); + + it("MUST: mapSqliteError converts SQLITE_BUSY (errcode=5) to OhnoDatabaseLockedError via withTransaction", () => { + // This is tested indirectly: when BEGIN IMMEDIATE fails on a locked db, + // withTransaction should throw OhnoDatabaseLockedError. + // We simulate SQLITE_BUSY by having one connection hold an exclusive lock + // and then trying to createTask (which uses withTransaction -> BEGIN IMMEDIATE). + const { DatabaseSync } = require("node:sqlite") as { DatabaseSync: typeof import("node:sqlite").DatabaseSync }; + const conn1 = new DatabaseSync(dbPath, { timeout: 0 }); + conn1.exec("BEGIN EXCLUSIVE"); + let caught: unknown; + try { + // db has timeout: 5000 but we need immediate failure for the test. + // Use a fresh db connection with timeout:0 to the same file. + const freshDb = new DatabaseSync(dbPath, { timeout: 0 }); + try { + freshDb.exec("BEGIN IMMEDIATE"); + } catch (e) { + caught = e; + } finally { + freshDb.close(); + } + } finally { + conn1.exec("ROLLBACK"); + conn1.close(); + } + // The raw error has errcode=5 (SQLITE_BUSY) + expect(caught).toBeDefined(); + expect((caught as { errcode?: number })?.errcode).toBe(5); + }); + }); }); diff --git a/packages/ohno-core/src/db.ts b/packages/ohno-core/src/db.ts index 10e5cf8..b389589 100644 --- a/packages/ohno-core/src/db.ts +++ b/packages/ohno-core/src/db.ts @@ -65,6 +65,56 @@ import { FIELD_SETS, } from "./schema.js"; +// SQLite numeric error codes (node:sqlite uses .errcode, not .code, for the SQLite-specific code) +const SQLITE_BUSY = 5; +const SQLITE_BUSY_SNAPSHOT = 261; // SQLITE_BUSY | (1 << 8) +const SQLITE_LOCKED = 6; + +/** + * Typed error for database lock contention that is recoverable by the user. + * Thrown when node:sqlite reports SQLITE_BUSY or SQLITE_BUSY_SNAPSHOT. + */ +export class OhnoDatabaseLockedError extends Error { + constructor(public readonly sqliteCode: string, message: string) { + super(message); + this.name = 'OhnoDatabaseLockedError'; + } +} + +/** + * Map a node:sqlite error to a typed application error. + * node:sqlite errors have .code = 'ERR_SQLITE_ERROR' and .errcode = . + * Always throws; callers use it in catch blocks: try { ... } catch (e) { mapSqliteError(e); } + */ +function mapSqliteError(e: unknown): never { + const errcode = (e as { errcode?: number })?.errcode; + + if (errcode === SQLITE_BUSY) { + throw new OhnoDatabaseLockedError( + 'SQLITE_BUSY', + "Database is locked by another ohno process; retry timed out after 5s. Try again, or check for stale ohno-mcp processes with 'ps aux | grep ohno-mcp'." + ); + } + if (errcode === SQLITE_BUSY_SNAPSHOT) { + throw new OhnoDatabaseLockedError( + 'SQLITE_BUSY_SNAPSHOT', + 'Database changed during transaction; retry' + ); + } + if (errcode === SQLITE_LOCKED) { + // Programmer-level bug, NOT an OhnoDatabaseLockedError + const err = new Error('Internal lock conflict (SQLITE_LOCKED) — please report this as a bug.'); + (err as Error & { sqliteCode?: string }).sqliteCode = 'SQLITE_LOCKED'; + throw err; + } + // Pass-through: re-throw with errcode preserved in message if not already there + const isNodeSqliteError = (e as { code?: string })?.code === 'ERR_SQLITE_ERROR'; + if (isNodeSqliteError && errcode !== undefined && e instanceof Error && !e.message.includes(String(errcode))) { + throw new Error(`${e.message} [errcode=${errcode}]`); + } + throw e; +} + export class TaskDatabase { private constructor(private db: DatabaseSync, public readonly dbPath: string) {} @@ -72,9 +122,15 @@ export class TaskDatabase { * Execute fn inside a BEGIN IMMEDIATE transaction. * Acquires the write lock at start, avoiding mid-transaction upgrade races. * If ROLLBACK itself throws, the original error propagates (not the rollback error). + * SQLite lock errors (BUSY, BUSY_SNAPSHOT) are mapped to OhnoDatabaseLockedError. */ private withTransaction(fn: () => T): T { - this.db.exec('BEGIN IMMEDIATE'); + try { + this.db.exec('BEGIN IMMEDIATE'); + } catch (e) { + mapSqliteError(e); + } + try { const result = fn(); this.db.exec('COMMIT'); @@ -85,6 +141,10 @@ export class TaskDatabase { } catch { // Swallow rollback errors so the original cause propagates. } + // Map SQLite errors before propagating; non-SQLite errors pass through + if ((e as { code?: string })?.code === 'ERR_SQLITE_ERROR') { + mapSqliteError(e); + } throw e; } } @@ -94,32 +154,43 @@ export class TaskDatabase { */ static async open(dbPath: string): Promise { fs.mkdirSync(path.dirname(dbPath), { recursive: true }); - const db = new DatabaseSync(dbPath, { timeout: 5000 }); - - // Set and verify journal mode. PRAGMA can silently fail (e.g., if the file - // is in use by another process holding it in WAL); we assert the result. - const journalRows = db.prepare('PRAGMA journal_mode = DELETE').all() as Array<{ journal_mode: string }>; - const actualMode = journalRows[0]?.journal_mode; - if (actualMode !== 'delete') { - db.close(); - throw new Error( - `Failed to set journal_mode = DELETE on ${dbPath}. ` + - `SQLite returned mode '${actualMode}'. ` + - `This usually means another process has the database open in a different journal mode. ` + - `Close other ohno processes and retry.` - ); - } + let db: DatabaseSync | undefined; + try { + db = new DatabaseSync(dbPath, { timeout: 5000 }); + + // Set and verify journal mode. PRAGMA can silently fail (e.g., if the file + // is in use by another process holding it in WAL); we assert the result. + const journalRows = db.prepare('PRAGMA journal_mode = DELETE').all() as Array<{ journal_mode: string }>; + const actualMode = journalRows[0]?.journal_mode; + if (actualMode !== 'delete') { + db.close(); + throw new Error( + `Failed to set journal_mode = DELETE on ${dbPath}. ` + + `SQLite returned mode '${actualMode}'. ` + + `This usually means another process has the database open in a different journal mode. ` + + `Close other ohno processes and retry.` + ); + } - db.exec('PRAGMA foreign_keys = ON'); - db.exec('PRAGMA synchronous = NORMAL'); + db.exec('PRAGMA foreign_keys = ON'); + db.exec('PRAGMA synchronous = NORMAL'); - const checkRows = db.prepare('PRAGMA quick_check').all() as Array<{ quick_check: string }>; - if (checkRows[0]?.quick_check !== 'ok') { - db.close(); - throw new Error( - `Database integrity check failed on ${dbPath}: ${checkRows[0]?.quick_check}. ` + - `Run 'sqlite3 ${dbPath} ".recover"' to attempt recovery.` - ); + const checkRows = db.prepare('PRAGMA quick_check').all() as Array<{ quick_check: string }>; + if (checkRows[0]?.quick_check !== 'ok') { + db.close(); + throw new Error( + `Database integrity check failed on ${dbPath}: ${checkRows[0]?.quick_check}. ` + + `Run 'sqlite3 ${dbPath} ".recover"' to attempt recovery.` + ); + } + } catch (e) { + // Close db if it was opened before the error occurred + try { db?.close(); } catch { /* ignore close errors */ } + // Only remap SQLite-coded errors; let our own Error messages pass through unchanged + if ((e as { code?: string })?.code === 'ERR_SQLITE_ERROR') { + mapSqliteError(e); + } + throw e; } const instance = new TaskDatabase(db, dbPath); @@ -611,14 +682,21 @@ export class TaskDatabase { VALUES (?, ?, ?, ?, 'todo', ?, ?) `; - this.db.prepare(sql).run( - storyId, - opts.epic_id ?? null, - opts.title, - opts.description ?? null, - timestamp, - timestamp, - ); + try { + this.db.prepare(sql).run( + storyId, + opts.epic_id ?? null, + opts.title, + opts.description ?? null, + timestamp, + timestamp, + ); + } catch (e) { + if ((e as { code?: string })?.code === 'ERR_SQLITE_ERROR') { + mapSqliteError(e); + } + throw e; + } return storyId; } @@ -660,9 +738,15 @@ export class TaskDatabase { params.push(storyId); const sql = `UPDATE stories SET ${setClauses.join(", ")} WHERE id = ?`; - const result = this.db.prepare(sql).run(...(params as never[])); - - return Number(result.changes) > 0; + try { + const result = this.db.prepare(sql).run(...(params as never[])); + return Number(result.changes) > 0; + } catch (e) { + if ((e as { code?: string })?.code === 'ERR_SQLITE_ERROR') { + mapSqliteError(e); + } + throw e; + } } /** @@ -759,15 +843,22 @@ export class TaskDatabase { VALUES (?, ?, ?, ?, ?, 'todo', ?, ?) `; - this.db.prepare(sql).run( - epicId, - opts.project_id ?? null, - opts.title, - opts.description ?? null, - opts.priority ?? "P2", - timestamp, - timestamp, - ); + try { + this.db.prepare(sql).run( + epicId, + opts.project_id ?? null, + opts.title, + opts.description ?? null, + opts.priority ?? "P2", + timestamp, + timestamp, + ); + } catch (e) { + if ((e as { code?: string })?.code === 'ERR_SQLITE_ERROR') { + mapSqliteError(e); + } + throw e; + } return epicId; } @@ -801,9 +892,15 @@ export class TaskDatabase { params.push(epicId); const sql = `UPDATE epics SET ${setClauses.join(", ")} WHERE id = ?`; - const result = this.db.prepare(sql).run(...(params as never[])); - - return Number(result.changes) > 0; + try { + const result = this.db.prepare(sql).run(...(params as never[])); + return Number(result.changes) > 0; + } catch (e) { + if ((e as { code?: string })?.code === 'ERR_SQLITE_ERROR') { + mapSqliteError(e); + } + throw e; + } } /** diff --git a/packages/ohno-core/src/index.ts b/packages/ohno-core/src/index.ts index 145e6b0..9a59cdb 100644 --- a/packages/ohno-core/src/index.ts +++ b/packages/ohno-core/src/index.ts @@ -2,7 +2,7 @@ * ohno-core - Core database layer for ohno task management */ -export { TaskDatabase } from "./db.js"; +export { TaskDatabase, OhnoDatabaseLockedError } from "./db.js"; export type { Task, diff --git a/packages/ohno-mcp/src/server.test.ts b/packages/ohno-mcp/src/server.test.ts index 18e83c8..2cf8a1e 100644 --- a/packages/ohno-mcp/src/server.test.ts +++ b/packages/ohno-mcp/src/server.test.ts @@ -6,7 +6,8 @@ import { describe, it, expect, beforeEach, afterEach } from "vitest"; import { mkdtempSync, rmSync } from "fs"; import { tmpdir } from "os"; import { join } from "path"; -import { TaskDatabase } from "@stevestomp/ohno-core"; +import { TaskDatabase, OhnoDatabaseLockedError } from "@stevestomp/ohno-core"; +import type { DatabaseSync } from "node:sqlite"; import { ZodError } from "zod"; import { handleTool, @@ -1335,11 +1336,10 @@ describe("MCP Server", () => { it("should filter by epic_id", async () => { // Create an epic - const dbInstance = db as unknown as { db: { run: (sql: string, params?: unknown[]) => void } }; - dbInstance.db.run( - "INSERT INTO epics (id, title, priority) VALUES (?, ?, ?)", - ["epic-1", "Epic 1", "P0"] - ); + const dbInstance = db as unknown as { db: DatabaseSync }; + dbInstance.db.prepare( + "INSERT INTO epics (id, title, priority) VALUES (?, ?, ?)" + ).run("epic-1", "Epic 1", "P0"); // Create stories with different epics db.createStory({ title: "Story in epic 1", epic_id: "epic-1" }); @@ -1389,11 +1389,10 @@ describe("MCP Server", () => { }); it("should combine filters", async () => { - const dbInstance = db as unknown as { db: { run: (sql: string, params?: unknown[]) => void } }; - dbInstance.db.run( - "INSERT INTO epics (id, title, priority) VALUES (?, ?, ?)", - ["epic-1", "Epic 1", "P0"] - ); + const dbInstance = db as unknown as { db: DatabaseSync }; + dbInstance.db.prepare( + "INSERT INTO epics (id, title, priority) VALUES (?, ?, ?)" + ).run("epic-1", "Epic 1", "P0"); const story1 = db.createStory({ title: "Story 1", epic_id: "epic-1" }); const story2 = db.createStory({ title: "Story 2", epic_id: "epic-1" }); @@ -1469,11 +1468,10 @@ describe("MCP Server", () => { }); it("should update story epic_id", async () => { - const dbInstance = db as unknown as { db: { run: (sql: string, params?: unknown[]) => void } }; - dbInstance.db.run( - "INSERT INTO epics (id, title, priority) VALUES (?, ?, ?)", - ["epic-1", "Epic 1", "P0"] - ); + const dbInstance = db as unknown as { db: DatabaseSync }; + dbInstance.db.prepare( + "INSERT INTO epics (id, title, priority) VALUES (?, ?, ?)" + ).run("epic-1", "Epic 1", "P0"); const storyId = db.createStory({ title: "Test story" }); const result = await handleTool("update_story", { @@ -1487,11 +1485,10 @@ describe("MCP Server", () => { }); it("should unassign story from epic with null", async () => { - const dbInstance = db as unknown as { db: { run: (sql: string, params?: unknown[]) => void } }; - dbInstance.db.run( - "INSERT INTO epics (id, title, priority) VALUES (?, ?, ?)", - ["epic-1", "Epic 1", "P0"] - ); + const dbInstance = db as unknown as { db: DatabaseSync }; + dbInstance.db.prepare( + "INSERT INTO epics (id, title, priority) VALUES (?, ?, ?)" + ).run("epic-1", "Epic 1", "P0"); const storyId = db.createStory({ title: "Test story", epic_id: "epic-1" }); const result = await handleTool("update_story", { @@ -1706,15 +1703,13 @@ describe("MCP Server", () => { it("should return boundaries when completing task with story", async () => { // Set up hierarchy - const dbInstance = db as unknown as { db: { run: (sql: string, params?: unknown[]) => void } }; - dbInstance.db.run( - "INSERT INTO epics (id, title, priority) VALUES (?, ?, ?)", - ["epic-1", "Epic 1", "P0"] - ); - dbInstance.db.run( - "INSERT INTO stories (id, epic_id, title) VALUES (?, ?, ?)", - ["story-1", "epic-1", "Story 1"] - ); + const dbInstance = db as unknown as { db: DatabaseSync }; + dbInstance.db.prepare( + "INSERT INTO epics (id, title, priority) VALUES (?, ?, ?)" + ).run("epic-1", "Epic 1", "P0"); + dbInstance.db.prepare( + "INSERT INTO stories (id, epic_id, title) VALUES (?, ?, ?)" + ).run("story-1", "epic-1", "Story 1"); const taskId = db.createTask({ title: "Test", story_id: "story-1" }); const result = await handleTool("update_task_status", { @@ -2321,4 +2316,51 @@ describe("MCP Server", () => { await expect(handleTool("get_task", {})).rejects.toThrow(ZodError); }); }); + + describe("OhnoDatabaseLockedError handling in handleTool", () => { + it("MUST: handleTool returns structured error response when OhnoDatabaseLockedError is thrown", async () => { + // Simulate OhnoDatabaseLockedError being thrown inside handleTool + // by closing the db and trying to use it via a mock db that throws + const lockedError = new OhnoDatabaseLockedError( + "SQLITE_BUSY", + "Database is locked by another ohno process; retry timed out after 5s. Try again, or check for stale ohno-mcp processes with 'ps aux | grep ohno-mcp'." + ); + + // Create a mock db that throws OhnoDatabaseLockedError on getProjectStatus + const mockDb = { + getProjectStatus: () => { throw lockedError; }, + } as unknown as TaskDatabase; + setDb(mockDb); + + const result = await handleTool("get_project_status", {}); + expect(result).toEqual({ + success: false, + error: lockedError.message, + errorCode: "SQLITE_BUSY", + }); + }); + + it("MUST: handleTool does not catch non-OhnoDatabaseLockedError errors (lets them propagate)", async () => { + const regularError = new Error("Some other error"); + const mockDb = { + getProjectStatus: () => { throw regularError; }, + } as unknown as TaskDatabase; + setDb(mockDb); + + await expect(handleTool("get_project_status", {})).rejects.toThrow("Some other error"); + }); + + it("MUST: structured error response has success:false, error message, and errorCode fields", async () => { + const lockedError = new OhnoDatabaseLockedError("SQLITE_BUSY_SNAPSHOT", "Database changed during transaction; retry"); + const mockDb = { + getProjectStatus: () => { throw lockedError; }, + } as unknown as TaskDatabase; + setDb(mockDb); + + const result = await handleTool("get_project_status", {}) as Record; + expect(result.success).toBe(false); + expect(result.error).toBe("Database changed during transaction; retry"); + expect(result.errorCode).toBe("SQLITE_BUSY_SNAPSHOT"); + }); + }); }); diff --git a/packages/ohno-mcp/src/server.ts b/packages/ohno-mcp/src/server.ts index dbe066c..27fce52 100644 --- a/packages/ohno-mcp/src/server.ts +++ b/packages/ohno-mcp/src/server.ts @@ -14,7 +14,7 @@ import { ListToolsRequestSchema, } from "@modelcontextprotocol/sdk/types.js"; import { z } from "zod"; -import { TaskDatabase, findDbPath, type TaskStatus, type DependencyType } from "@stevestomp/ohno-core"; +import { TaskDatabase, OhnoDatabaseLockedError, findDbPath, type TaskStatus, type DependencyType } from "@stevestomp/ohno-core"; // Zod schemas for tool parameters const GetTasksSchema = z.object({ @@ -713,6 +713,7 @@ export function setDb(database: TaskDatabase | null): void { export async function handleTool(name: string, args: Record): Promise { const database = await getDb(); + try { switch (name) { case "get_project_status": return database.getProjectStatus(); @@ -1015,6 +1016,16 @@ export async function handleTool(name: string, args: Record): P default: throw new Error(`Unknown tool: ${name}`); } + } catch (e) { + if (e instanceof OhnoDatabaseLockedError) { + return { + success: false, + error: e.message, + errorCode: e.sqliteCode, + }; + } + throw e; + } } export async function createServer(): Promise { diff --git a/packages/ohno-mcp/vitest.config.ts b/packages/ohno-mcp/vitest.config.ts index 94d18ee..d738fbd 100644 --- a/packages/ohno-mcp/vitest.config.ts +++ b/packages/ohno-mcp/vitest.config.ts @@ -1,9 +1,36 @@ -import { defineConfig } from "vitest/config"; +import { defineConfig, type Plugin } from "vitest/config"; + +// node:sqlite is a Node 22.5+ built-in added after builtinModules was frozen. +// vite-node's normalizeModuleId() strips the "node:" prefix, so "node:sqlite" +// becomes "sqlite", which Vite can't find as a file. This plugin intercepts +// the stripped id and provides a virtual module that loads the real built-in. +const nodeSqlitePlugin: Plugin = { + name: "node-sqlite-external", + enforce: "pre", + resolveId(id) { + if (id === "sqlite") { + return "\0virtual:sqlite"; + } + }, + load(id) { + if (id === "\0virtual:sqlite") { + return ` +import { createRequire } from 'node:module'; +const _require = createRequire(import.meta.url); +const mod = _require('node:sqlite'); +export const DatabaseSync = mod.DatabaseSync; +export const StatementSync = mod.StatementSync; +export default mod; +`; + } + }, +}; export default defineConfig({ + plugins: [nodeSqlitePlugin], test: { globals: false, environment: "node", include: ["src/**/*.test.ts"], }, -}); \ No newline at end of file +}); From eaaf3585044a983ebeafefd78749afbdfd6e1ba8 Mon Sep 17 00:00:00 2001 From: srstomp Date: Sun, 26 Apr 2026 18:42:27 +0200 Subject: [PATCH 09/11] fix: address sqlite review feedback --- packages/ohno-cli/src/kanban.ts | 3 +- packages/ohno-core/src/db.test.ts | 66 ++++++++++++++++++++--------- packages/ohno-core/src/db.ts | 44 ++++++++----------- packages/ohno-core/vitest.config.ts | 2 +- packages/ohno-mcp/src/server.ts | 16 +++---- 5 files changed, 73 insertions(+), 58 deletions(-) diff --git a/packages/ohno-cli/src/kanban.ts b/packages/ohno-cli/src/kanban.ts index a7d7d57..1e0353d 100644 --- a/packages/ohno-cli/src/kanban.ts +++ b/packages/ohno-cli/src/kanban.ts @@ -51,9 +51,10 @@ export interface KanbanData { */ export async function exportDatabase(dbPath: string): Promise { const db = new DatabaseSync(dbPath, { readOnly: true }); - db.exec("PRAGMA query_only = ON"); try { + db.exec("PRAGMA query_only = ON"); + const data: KanbanData = { synced_at: new Date().toISOString(), version: pkg.version, diff --git a/packages/ohno-core/src/db.test.ts b/packages/ohno-core/src/db.test.ts index b54cf40..6c214b7 100644 --- a/packages/ohno-core/src/db.test.ts +++ b/packages/ohno-core/src/db.test.ts @@ -6,7 +6,7 @@ import { describe, it, expect, beforeEach, afterEach } from "vitest"; import { mkdtempSync, rmSync } from "fs"; import { tmpdir } from "os"; import { join } from "path"; -import type { DatabaseSync } from "node:sqlite"; +import { DatabaseSync } from "node:sqlite"; import { TaskDatabase, OhnoDatabaseLockedError } from "./db.js"; import type { TaskStatus } from "./types.js"; @@ -1803,6 +1803,42 @@ describe("TaskDatabase", () => { }); describe("Memory Decay", () => { + describe("hierarchy deletion", () => { + it("should delete archived tasks when deleting a story", () => { + const storyId = db.createStory({ title: "Story with archived task" }); + const taskId = db.createTask({ title: "Archived task", story_id: storyId }); + db.archiveTask(taskId); + + expect(db.deleteStory(storyId)).toBe(true); + expect(db.getTask(taskId)).toBeNull(); + expect(db.getStory(storyId)).toBeNull(); + }); + + it("should delete every task in a story without a 1000 task limit", () => { + const storyId = db.createStory({ title: "Large story" }); + const taskIds: string[] = []; + + for (let i = 0; i < 1005; i++) { + taskIds.push(db.createTask({ title: `Task ${i}`, story_id: storyId })); + } + + expect(db.deleteStory(storyId)).toBe(true); + expect(taskIds.every((taskId) => db.getTask(taskId) === null)).toBe(true); + }); + + it("should delete archived story tasks when deleting an epic", () => { + const epicId = db.createEpic({ title: "Epic with archived task" }); + const storyId = db.createStory({ title: "Child story", epic_id: epicId }); + const taskId = db.createTask({ title: "Archived task", story_id: storyId }); + db.archiveTask(taskId); + + expect(db.deleteEpic(epicId)).toBe(true); + expect(db.getTask(taskId)).toBeNull(); + expect(db.getStory(storyId)).toBeNull(); + expect(db.getEpic(epicId)).toBeNull(); + }); + }); + describe("compactStoryHandoffs", () => { it("should compact handoffs for all tasks in a completed story", () => { const storyId = db.createStory({ title: "Test story" }); @@ -2083,7 +2119,6 @@ describe("TaskDatabase", () => { describe("node:sqlite error property verification", () => { it("MUST: node:sqlite throws errors with .errcode=5 for SQLITE_BUSY and .code='ERR_SQLITE_ERROR'", () => { // Use two DatabaseSync connections to the same file with timeout:0 to get immediate SQLITE_BUSY - const { DatabaseSync } = require("node:sqlite") as { DatabaseSync: typeof import("node:sqlite").DatabaseSync }; const busyDbPath = join(tempDir, "busy-verify.db"); const conn1 = new DatabaseSync(busyDbPath, { timeout: 0 }); const conn2 = new DatabaseSync(busyDbPath, { timeout: 0 }); @@ -2104,33 +2139,22 @@ describe("TaskDatabase", () => { expect((caughtError as { errcode?: number })?.errcode).toBe(5); // SQLITE_BUSY = 5 }); - it("MUST: mapSqliteError converts SQLITE_BUSY (errcode=5) to OhnoDatabaseLockedError via withTransaction", () => { - // This is tested indirectly: when BEGIN IMMEDIATE fails on a locked db, - // withTransaction should throw OhnoDatabaseLockedError. - // We simulate SQLITE_BUSY by having one connection hold an exclusive lock - // and then trying to createTask (which uses withTransaction -> BEGIN IMMEDIATE). - const { DatabaseSync } = require("node:sqlite") as { DatabaseSync: typeof import("node:sqlite").DatabaseSync }; + it("MUST: withTransaction maps SQLITE_BUSY to OhnoDatabaseLockedError", async () => { + const lockedDb = await TaskDatabase.open(dbPath); const conn1 = new DatabaseSync(dbPath, { timeout: 0 }); conn1.exec("BEGIN EXCLUSIVE"); let caught: unknown; try { - // db has timeout: 5000 but we need immediate failure for the test. - // Use a fresh db connection with timeout:0 to the same file. - const freshDb = new DatabaseSync(dbPath, { timeout: 0 }); - try { - freshDb.exec("BEGIN IMMEDIATE"); - } catch (e) { - caught = e; - } finally { - freshDb.close(); - } + lockedDb.deleteTask("task-locked"); + } catch (e) { + caught = e; } finally { conn1.exec("ROLLBACK"); conn1.close(); + lockedDb.close(); } - // The raw error has errcode=5 (SQLITE_BUSY) - expect(caught).toBeDefined(); - expect((caught as { errcode?: number })?.errcode).toBe(5); + expect(caught).toBeInstanceOf(OhnoDatabaseLockedError); + expect((caught as OhnoDatabaseLockedError).sqliteCode).toBe("SQLITE_BUSY"); }); }); }); diff --git a/packages/ohno-core/src/db.ts b/packages/ohno-core/src/db.ts index b389589..ab30eef 100644 --- a/packages/ohno-core/src/db.ts +++ b/packages/ohno-core/src/db.ts @@ -141,11 +141,8 @@ export class TaskDatabase { } catch { // Swallow rollback errors so the original cause propagates. } - // Map SQLite errors before propagating; non-SQLite errors pass through - if ((e as { code?: string })?.code === 'ERR_SQLITE_ERROR') { - mapSqliteError(e); - } - throw e; + // Map SQLite errors before propagating; non-SQLite errors pass through. + mapSqliteError(e); } } @@ -186,11 +183,8 @@ export class TaskDatabase { } catch (e) { // Close db if it was opened before the error occurred try { db?.close(); } catch { /* ignore close errors */ } - // Only remap SQLite-coded errors; let our own Error messages pass through unchanged - if ((e as { code?: string })?.code === 'ERR_SQLITE_ERROR') { - mapSqliteError(e); - } - throw e; + // Remap SQLite-coded errors; let our own Error messages pass through unchanged. + mapSqliteError(e); } const instance = new TaskDatabase(db, dbPath); @@ -1254,6 +1248,11 @@ export class TaskDatabase { return this.withTransaction(() => this._deleteTaskImpl(taskId)); } + private getTaskIdsByStory(storyId: string): string[] { + const rows = this.db.prepare("SELECT id FROM tasks WHERE story_id = ?").all(storyId) as Array<{ id: string }>; + return rows.map((row) => row.id); + } + /** * Delete an epic (hard delete) * Also deletes all stories and tasks associated with the epic @@ -1264,18 +1263,14 @@ export class TaskDatabase { return false; } - // Get all stories in this epic before entering the transaction - const stories = this.getStories({ epic_id: epicId }); - return this.withTransaction(() => { + const stories = this.getStories({ epic_id: epicId }); + // Delete all tasks in each story for (const story of stories) { - const tasks = this.getTasks({ limit: 1000 }); - for (const task of tasks) { - if (task.story_id === story.id) { - // Use unwrapped impl to avoid nested BEGIN IMMEDIATE - this._deleteTaskImpl(task.id); - } + for (const taskId of this.getTaskIdsByStory(story.id)) { + // Use unwrapped impl to avoid nested BEGIN IMMEDIATE + this._deleteTaskImpl(taskId); } } @@ -1301,16 +1296,11 @@ export class TaskDatabase { return false; } - // Get tasks before entering the transaction - const tasks = this.getTasks({ limit: 1000 }); - return this.withTransaction(() => { // Delete all tasks in this story - for (const task of tasks) { - if (task.story_id === storyId) { - // Use unwrapped impl to avoid nested BEGIN IMMEDIATE - this._deleteTaskImpl(task.id); - } + for (const taskId of this.getTaskIdsByStory(storyId)) { + // Use unwrapped impl to avoid nested BEGIN IMMEDIATE + this._deleteTaskImpl(taskId); } // Delete the story diff --git a/packages/ohno-core/vitest.config.ts b/packages/ohno-core/vitest.config.ts index 87f6a60..5e2a2ad 100644 --- a/packages/ohno-core/vitest.config.ts +++ b/packages/ohno-core/vitest.config.ts @@ -4,7 +4,7 @@ import { defineConfig, type Plugin } from "vitest/config"; // vite-node's normalizeModuleId() strips the "node:" prefix, so "node:sqlite" // becomes "sqlite", which Vite can't find as a file. This plugin intercepts // the stripped id and provides a virtual module that loads the real built-in -// via a dynamic import (which bypasses vite-node's normalisation). +// via createRequire(...), which bypasses vite-node's normalisation. const nodeSqlitePlugin: Plugin = { name: "node-sqlite-external", enforce: "pre", diff --git a/packages/ohno-mcp/src/server.ts b/packages/ohno-mcp/src/server.ts index 27fce52..05ae62b 100644 --- a/packages/ohno-mcp/src/server.ts +++ b/packages/ohno-mcp/src/server.ts @@ -711,12 +711,12 @@ export function setDb(database: TaskDatabase | null): void { * Tool handler - exported for testing */ export async function handleTool(name: string, args: Record): Promise { - const database = await getDb(); - try { - switch (name) { - case "get_project_status": - return database.getProjectStatus(); + const database = await getDb(); + + switch (name) { + case "get_project_status": + return database.getProjectStatus(); case "get_session_context": return database.getSessionContext(); @@ -1013,9 +1013,9 @@ export async function handleTool(name: string, args: Record): P return { deleted: count }; } - default: - throw new Error(`Unknown tool: ${name}`); - } + default: + throw new Error(`Unknown tool: ${name}`); + } } catch (e) { if (e instanceof OhnoDatabaseLockedError) { return { From 6ad8a30edad1ce4529ab462f75e1b87b46e4ed4f Mon Sep 17 00:00:00 2001 From: srstomp Date: Sun, 26 Apr 2026 19:19:32 +0200 Subject: [PATCH 10/11] fix: handle second sqlite review pass --- packages/ohno-core/src/db.test.ts | 42 +++++++++++++++++ packages/ohno-core/src/db.ts | 76 +++++++++++++++++++------------ packages/ohno-mcp/src/server.ts | 8 ++-- 3 files changed, 92 insertions(+), 34 deletions(-) diff --git a/packages/ohno-core/src/db.test.ts b/packages/ohno-core/src/db.test.ts index 6c214b7..bae19c2 100644 --- a/packages/ohno-core/src/db.test.ts +++ b/packages/ohno-core/src/db.test.ts @@ -1837,6 +1837,21 @@ describe("TaskDatabase", () => { expect(db.getStory(storyId)).toBeNull(); expect(db.getEpic(epicId)).toBeNull(); }); + + it("should delete every story in an epic without a 50 story limit", () => { + const epicId = db.createEpic({ title: "Large epic" }); + const storyIds: string[] = []; + + for (let i = 0; i < 55; i++) { + const storyId = db.createStory({ title: `Story ${i}`, epic_id: epicId }); + storyIds.push(storyId); + db.createTask({ title: `Task ${i}`, story_id: storyId }); + } + + expect(db.deleteEpic(epicId)).toBe(true); + expect(storyIds.every((storyId) => db.getStory(storyId) === null)).toBe(true); + expect(db.getTasks({ limit: 100 }).filter((task) => storyIds.includes(task.story_id ?? "")).length).toBe(0); + }); }); describe("compactStoryHandoffs", () => { @@ -2156,5 +2171,32 @@ describe("TaskDatabase", () => { expect(caught).toBeInstanceOf(OhnoDatabaseLockedError); expect((caught as OhnoDatabaseLockedError).sqliteCode).toBe("SQLITE_BUSY"); }); + + it("MUST: direct write methods map SQLITE_BUSY to OhnoDatabaseLockedError", async () => { + const lockedDb = await TaskDatabase.open(dbPath); + const conn1 = new DatabaseSync(dbPath, { timeout: 0 }); + + for (const write of [ + () => lockedDb.addTaskActivity("task-locked", "note", "Blocked activity"), + () => lockedDb.addTaskFailure("task-locked", "implementation", "Blocked failure", 1), + () => lockedDb.setTaskHandoff("task-locked", "PASS", "Blocked handoff"), + ]) { + conn1.exec("BEGIN EXCLUSIVE"); + let caught: unknown; + try { + write(); + } catch (e) { + caught = e; + } finally { + conn1.exec("ROLLBACK"); + } + + expect(caught).toBeInstanceOf(OhnoDatabaseLockedError); + expect((caught as OhnoDatabaseLockedError).sqliteCode).toBe("SQLITE_BUSY"); + } + + conn1.close(); + lockedDb.close(); + }); }); }); diff --git a/packages/ohno-core/src/db.ts b/packages/ohno-core/src/db.ts index ab30eef..892f780 100644 --- a/packages/ohno-core/src/db.ts +++ b/packages/ohno-core/src/db.ts @@ -110,7 +110,7 @@ function mapSqliteError(e: unknown): never { // Pass-through: re-throw with errcode preserved in message if not already there const isNodeSqliteError = (e as { code?: string })?.code === 'ERR_SQLITE_ERROR'; if (isNodeSqliteError && errcode !== undefined && e instanceof Error && !e.message.includes(String(errcode))) { - throw new Error(`${e.message} [errcode=${errcode}]`); + throw new Error(`${e.message} [errcode=${errcode}]`, { cause: e }); } throw e; } @@ -1253,6 +1253,10 @@ export class TaskDatabase { return rows.map((row) => row.id); } + private getStoriesByEpic(epicId: string): Array<{ id: string }> { + return this.db.prepare("SELECT id FROM stories WHERE epic_id = ? ORDER BY updated_at DESC, created_at DESC").all(epicId) as Array<{ id: string }>; + } + /** * Delete an epic (hard delete) * Also deletes all stories and tasks associated with the epic @@ -1264,7 +1268,7 @@ export class TaskDatabase { } return this.withTransaction(() => { - const stories = this.getStories({ epic_id: epicId }); + const stories = this.getStoriesByEpic(epicId); // Delete all tasks in each story for (const story of stories) { @@ -1399,18 +1403,22 @@ export class TaskDatabase { VALUES (?, ?, ?, ?, ?, ?, ?, ?) `; - const result = this.db.prepare(sql).run( - actId, - taskId, - activityType, - description, - oldValue ?? null, - newValue ?? null, - actor ?? null, - timestamp, - ); + try { + const result = this.db.prepare(sql).run( + actId, + taskId, + activityType, + description, + oldValue ?? null, + newValue ?? null, + actor ?? null, + timestamp, + ); - return Number(result.changes) > 0; + return Number(result.changes) > 0; + } catch (e) { + mapSqliteError(e); + } } /** @@ -1479,14 +1487,18 @@ export class TaskDatabase { VALUES (?, ?, ?, ?, ?, ?) `; - this.db.prepare(sql).run( - failureId, - taskId, - failureType, - failureReason, - attempt ?? null, - timestamp, - ); + try { + this.db.prepare(sql).run( + failureId, + taskId, + failureType, + failureReason, + attempt ?? null, + timestamp, + ); + } catch (e) { + mapSqliteError(e); + } return failureId; } @@ -1523,15 +1535,19 @@ export class TaskDatabase { INSERT OR REPLACE INTO task_handoffs (task_id, status, summary, files_changed, full_details, created_at) VALUES (?, ?, ?, ?, ?, ?) `; - const result = this.db.prepare(sql).run( - taskId, - status, - summary, - filesChanged ? JSON.stringify(filesChanged) : null, - fullDetails ?? null, - timestamp, - ); - return Number(result.changes) > 0; + try { + const result = this.db.prepare(sql).run( + taskId, + status, + summary, + filesChanged ? JSON.stringify(filesChanged) : null, + fullDetails ?? null, + timestamp, + ); + return Number(result.changes) > 0; + } catch (e) { + mapSqliteError(e); + } } /** diff --git a/packages/ohno-mcp/src/server.ts b/packages/ohno-mcp/src/server.ts index 05ae62b..14d5fa5 100644 --- a/packages/ohno-mcp/src/server.ts +++ b/packages/ohno-mcp/src/server.ts @@ -715,8 +715,8 @@ export async function handleTool(name: string, args: Record): P const database = await getDb(); switch (name) { - case "get_project_status": - return database.getProjectStatus(); + case "get_project_status": + return database.getProjectStatus(); case "get_session_context": return database.getSessionContext(); @@ -1013,8 +1013,8 @@ export async function handleTool(name: string, args: Record): P return { deleted: count }; } - default: - throw new Error(`Unknown tool: ${name}`); + default: + throw new Error(`Unknown tool: ${name}`); } } catch (e) { if (e instanceof OhnoDatabaseLockedError) { From c53d808e776f55e3e82a206e99dfd0386d039257 Mon Sep 17 00:00:00 2001 From: srstomp Date: Sun, 26 Apr 2026 19:42:56 +0200 Subject: [PATCH 11/11] fix: address sqlite review edge cases --- packages/ohno-core/src/db.test.ts | 36 +++++++++++++++++++++++++++++++ packages/ohno-core/src/db.ts | 13 ++++++++--- 2 files changed, 46 insertions(+), 3 deletions(-) diff --git a/packages/ohno-core/src/db.test.ts b/packages/ohno-core/src/db.test.ts index bae19c2..a524e64 100644 --- a/packages/ohno-core/src/db.test.ts +++ b/packages/ohno-core/src/db.test.ts @@ -42,6 +42,15 @@ describe("TaskDatabase", () => { expect(db2).toBeDefined(); db2.close(); }); + + it("should not swallow schema migration errors other than duplicate columns", async () => { + const malformedDbPath = join(tempDir, "malformed-schema.db"); + const malformedDb = new DatabaseSync(malformedDbPath); + malformedDb.exec("CREATE VIEW tasks AS SELECT 'task-1' AS id"); + malformedDb.close(); + + await expect(TaskDatabase.open(malformedDbPath)).rejects.toThrow("Cannot add a column to a view"); + }); }); describe("Task CRUD Operations", () => { @@ -2172,6 +2181,33 @@ describe("TaskDatabase", () => { expect((caught as OhnoDatabaseLockedError).sqliteCode).toBe("SQLITE_BUSY"); }); + it("MUST: withTransaction maps SQLITE_BUSY_SNAPSHOT errcode 517 to OhnoDatabaseLockedError", () => { + const dbInstance = db as unknown as { db: DatabaseSync }; + const originalDb = dbInstance.db; + dbInstance.db = { + exec(sql: string) { + if (sql === "BEGIN IMMEDIATE") { + const err = new Error("database snapshot is stale") as Error & { code: string; errcode: number }; + err.code = "ERR_SQLITE_ERROR"; + err.errcode = 517; + throw err; + } + }, + } as DatabaseSync; + + let caught: unknown; + try { + db.deleteTask("task-locked"); + } catch (e) { + caught = e; + } finally { + dbInstance.db = originalDb; + } + + expect(caught).toBeInstanceOf(OhnoDatabaseLockedError); + expect((caught as OhnoDatabaseLockedError).sqliteCode).toBe("SQLITE_BUSY_SNAPSHOT"); + }); + it("MUST: direct write methods map SQLITE_BUSY to OhnoDatabaseLockedError", async () => { const lockedDb = await TaskDatabase.open(dbPath); const conn1 = new DatabaseSync(dbPath, { timeout: 0 }); diff --git a/packages/ohno-core/src/db.ts b/packages/ohno-core/src/db.ts index 892f780..db8cbe3 100644 --- a/packages/ohno-core/src/db.ts +++ b/packages/ohno-core/src/db.ts @@ -67,7 +67,7 @@ import { // SQLite numeric error codes (node:sqlite uses .errcode, not .code, for the SQLite-specific code) const SQLITE_BUSY = 5; -const SQLITE_BUSY_SNAPSHOT = 261; // SQLITE_BUSY | (1 << 8) +const SQLITE_BUSY_SNAPSHOT = 517; // SQLITE_BUSY | (2 << 8) const SQLITE_LOCKED = 6; /** @@ -115,6 +115,10 @@ function mapSqliteError(e: unknown): never { throw e; } +function isDuplicateColumnError(e: unknown): boolean { + return e instanceof Error && e.message.toLowerCase().includes("duplicate column name"); +} + export class TaskDatabase { private constructor(private db: DatabaseSync, public readonly dbPath: string) {} @@ -214,8 +218,11 @@ export class TaskDatabase { for (const [colName, colType] of EXTENDED_TASK_COLUMNS) { try { this.db.prepare(`ALTER TABLE tasks ADD COLUMN ${colName} ${colType}`).run(); - } catch { - // Column already exists + } catch (e) { + if (isDuplicateColumnError(e)) { + continue; + } + mapSqliteError(e); } }