From 48742c8099754955a004a61819070ebdf35579e2 Mon Sep 17 00:00:00 2001 From: thatdudealso Date: Tue, 21 Jul 2026 21:10:56 -0400 Subject: [PATCH 1/3] fix: load bun:sqlite at runtime with node:sqlite fallback Vitest runs the storage layer under Node, but src/storage/db.ts had a static `import { Database } from "bun:sqlite"`, which Node cannot resolve. Load bun:sqlite via createRequire at runtime and fall back to node:sqlite's DatabaseSync when Bun is unavailable, so the storage tests can run under Node/Vitest as well as Bun. --- src/storage/db.test.ts | 65 ++++++++++++++++++++++++++++++++++ src/storage/db.ts | 80 ++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 143 insertions(+), 2 deletions(-) create mode 100644 src/storage/db.test.ts diff --git a/src/storage/db.test.ts b/src/storage/db.test.ts new file mode 100644 index 000000000..44d478e08 --- /dev/null +++ b/src/storage/db.test.ts @@ -0,0 +1,65 @@ +import fs from "fs"; +import os from "os"; +import path from "path"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { closeDatabase, getDatabase, withTransaction } from "./db"; + +const originalHome = process.env.HOME; + +describe("storage database", () => { + const tempRoot = path.join(process.cwd(), ".tmp-db-tests"); + let tempHome = ""; + + beforeEach(() => { + fs.mkdirSync(tempRoot, { recursive: true }); + tempHome = fs.mkdtempSync(path.join(tempRoot, "grok-db-home-")); + process.env.HOME = tempHome; + vi.spyOn(os, "homedir").mockReturnValue(tempHome); + closeDatabase(); + }); + + afterEach(() => { + closeDatabase(); + vi.restoreAllMocks(); + process.env.HOME = originalHome; + fs.rmSync(tempRoot, { recursive: true, force: true }); + }); + + it("initializes and round-trips a write/read", () => { + const db = getDatabase(); + + db.exec(` + CREATE TABLE IF NOT EXISTS storage_smoke_test ( + id INTEGER PRIMARY KEY, + value TEXT NOT NULL + ) STRICT; + `); + + db.prepare("INSERT INTO storage_smoke_test (id, value) VALUES (?, ?)").run(1, "hello world"); + + const row = db.prepare("SELECT value FROM storage_smoke_test WHERE id = ?").get(1) as { + value: string; + }; + + expect(row.value).toBe("hello world"); + }); + + it("commits writes made inside a transaction", () => { + getDatabase().exec(` + CREATE TABLE IF NOT EXISTS storage_smoke_test ( + id INTEGER PRIMARY KEY, + value TEXT NOT NULL + ) STRICT; + `); + + withTransaction((db) => { + db.prepare("INSERT INTO storage_smoke_test (id, value) VALUES (?, ?)").run(2, "in transaction"); + }); + + const row = getDatabase().prepare("SELECT value FROM storage_smoke_test WHERE id = ?").get(2) as { + value: string; + }; + + expect(row.value).toBe("in transaction"); + }); +}); diff --git a/src/storage/db.ts b/src/storage/db.ts index dada86140..c7e611660 100644 --- a/src/storage/db.ts +++ b/src/storage/db.ts @@ -1,9 +1,12 @@ -import { Database } from "bun:sqlite"; +import type { DatabaseSync } from "node:sqlite"; import fs from "fs"; +import { createRequire } from "module"; import os from "os"; import path from "path"; import { applyMigrations } from "./migrations"; +const require = createRequire(import.meta.url); + export interface SQLiteStatement { run(...params: unknown[]): unknown; get(...params: unknown[]): unknown; @@ -53,7 +56,10 @@ class BunSqliteDatabase implements SQLiteDatabase { private readonly db: Database; constructor(filename: string) { - this.db = new Database(filename, { create: true, strict: true }); + const BunDatabase = loadBunDatabase(); + this.db = BunDatabase + ? new BunDatabase(filename, { create: true, strict: true }) + : new NodeSqliteDatabase(filename); } exec(sql: string): void { @@ -89,6 +95,76 @@ class BunSqliteDatabase implements SQLiteDatabase { } } +type Database = { + exec(sql: string): void; + run(sql: string, params?: unknown): unknown; + query(sql: string): { get(params?: unknown): unknown; all(params?: unknown): unknown[] }; + transaction(fn: () => T): () => T; + close(): void; +}; + +function loadBunDatabase(): (new (filename: string, options: { create: boolean; strict: boolean }) => Database) | null { + try { + return require("bun:sqlite").Database as new ( + filename: string, + options: { create: boolean; strict: boolean }, + ) => Database; + } catch { + return null; + } +} + +class NodeSqliteDatabase implements Database { + private readonly db: DatabaseSync; + + constructor(filename: string) { + const { DatabaseSync } = require("node:sqlite") as typeof import("node:sqlite"); + this.db = new DatabaseSync(filename); + } + + exec(sql: string): void { + this.db.exec(sql); + } + + run(sql: string, params?: unknown): unknown { + const statement = this.db.prepare(sql); + if (params === undefined) return statement.run(); + return Array.isArray(params) ? statement.run(...(params as never[])) : statement.run(params as never); + } + + query(sql: string): { get(params?: unknown): unknown; all(params?: unknown): unknown[] } { + const statement = this.db.prepare(sql); + return { + get: (params?: unknown) => { + if (params === undefined) return statement.get(); + return Array.isArray(params) ? statement.get(...(params as never[])) : statement.get(params as never); + }, + all: (params?: unknown) => { + if (params === undefined) return statement.all(); + return Array.isArray(params) ? statement.all(...(params as never[])) : statement.all(params as never); + }, + }; + } + + transaction(fn: () => T): () => T { + return () => { + this.db.exec("BEGIN"); + try { + const result = fn(); + this.db.exec("COMMIT"); + return result; + } catch (error) { + this.db.exec("ROLLBACK"); + throw error; + } + }; + } + + close(): void { + this.db.close(); + } +} + function normalizeBinding(params: unknown[]): unknown { if (params.length === 0) return undefined; return params.length === 1 ? params[0] : params; From df6e8561c96a628be0e2d2affd47bbb413815067 Mon Sep 17 00:00:00 2001 From: thatdudealso Date: Tue, 21 Jul 2026 21:11:33 -0400 Subject: [PATCH 2/3] docs: record node:sqlite fallback in AGENTS.md Add a self-governance section and note that src/storage/db.ts now supports Node via a node:sqlite fallback, and that bunx vitest still runs under Bun so the Node path requires an explicit node invocation. --- AGENTS.md | 10 +++++++++- CLAUDE.md | 1 + 2 files changed, 10 insertions(+), 1 deletion(-) create mode 120000 CLAUDE.md diff --git a/AGENTS.md b/AGENTS.md index 236be4869..39af5d575 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -27,4 +27,12 @@ Grok CLI (`@vibe-kit/grok-cli`) is a single-package TypeScript CLI tool — no d ### Environment - **Bun** must be installed (not pre-installed on Cloud VMs). The update script handles this. -- `GROK_API_KEY` environment variable is required for API calls. Set it as a secret. \ No newline at end of file +- `GROK_API_KEY` environment variable is required for API calls. Set it as a secret. +- `src/storage/db.ts` runtime-loads `bun:sqlite` and falls back to `node:sqlite` when unavailable, so the storage layer also works under Node. Note `bun run test` (`bunx vitest run`) still executes under the Bun runtime; to actually exercise the Node fallback path, run `node node_modules/.bin/vitest run`. + +## Maintaining this file + +Keep this file for knowledge useful to almost every future agent session in this project. +Do not repeat what the codebase already shows; point to the authoritative file or command instead. +Prefer rewriting or pruning existing entries over appending new ones. +When updating this file, preserve this bar for all agents and keep entries concise. diff --git a/CLAUDE.md b/CLAUDE.md new file mode 120000 index 000000000..47dc3e3d8 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1 @@ +AGENTS.md \ No newline at end of file From e221bced3fc90ab3f5f160e1cbe539b1411d27a8 Mon Sep 17 00:00:00 2001 From: thatdudealso Date: Wed, 22 Jul 2026 08:06:14 -0400 Subject: [PATCH 3/3] fix: enable bare named parameters for node:sqlite fallback The node:sqlite DatabaseSync path prepared statements without calling setAllowBareNamedParameters(true), so callers binding objects like { id } against @id placeholders (as workspaces.ts and sessions.ts do) could fail to bind under Node, unlike Bun's strict: true mode. Add a regression test that exercises the exact fallback path and verifies both persistence and that the API is actually invoked. --- AGENTS.md | 1 + src/storage/db.test.ts | 61 ++++++++++++++++++++++++++++++++++++++++++ src/storage/db.ts | 10 +++++-- 3 files changed, 70 insertions(+), 2 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 39af5d575..8ac24f6a4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -29,6 +29,7 @@ Grok CLI (`@vibe-kit/grok-cli`) is a single-package TypeScript CLI tool — no d - **Bun** must be installed (not pre-installed on Cloud VMs). The update script handles this. - `GROK_API_KEY` environment variable is required for API calls. Set it as a secret. - `src/storage/db.ts` runtime-loads `bun:sqlite` and falls back to `node:sqlite` when unavailable, so the storage layer also works under Node. Note `bun run test` (`bunx vitest run`) still executes under the Bun runtime; to actually exercise the Node fallback path, run `node node_modules/.bin/vitest run`. +- The `node:sqlite` fallback must call `statement.setAllowBareNamedParameters(true)` (see `prepareWithBareNamedParameters` in `db.ts`) to match Bun's `strict: true` binding of bare named params (e.g. `{ id }` against `@id`). Per Node's own docs the unprefixed form is not guaranteed without this call, and `node:sqlite` is still experimental — don't remove the call just because a given Node build happens to work without it. ## Maintaining this file diff --git a/src/storage/db.test.ts b/src/storage/db.test.ts index 44d478e08..c44883359 100644 --- a/src/storage/db.test.ts +++ b/src/storage/db.test.ts @@ -1,11 +1,22 @@ import fs from "fs"; +import { createRequire } from "module"; import os from "os"; import path from "path"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { closeDatabase, getDatabase, withTransaction } from "./db"; +const require = createRequire(import.meta.url); const originalHome = process.env.HOME; +function isBunSqliteAvailable(): boolean { + try { + require("bun:sqlite"); + return true; + } catch { + return false; + } +} + describe("storage database", () => { const tempRoot = path.join(process.cwd(), ".tmp-db-tests"); let tempHome = ""; @@ -44,6 +55,56 @@ describe("storage database", () => { expect(row.value).toBe("hello world"); }); + it("binds bare named parameters (e.g. { id } against @id) and persists the value", () => { + const db = getDatabase(); + + db.exec(` + CREATE TABLE IF NOT EXISTS storage_named_param_test ( + id INTEGER PRIMARY KEY, + value TEXT NOT NULL + ) STRICT; + `); + + db.prepare("INSERT INTO storage_named_param_test (id, value) VALUES (@id, @value)").run({ + id: 3, + value: "named param value", + }); + + const row = db.prepare("SELECT value FROM storage_named_param_test WHERE id = @id").get({ id: 3 }) as { + value: string | null; + }; + + expect(row).toBeDefined(); + expect(row.value).not.toBeNull(); + expect(row.value).toBe("named param value"); + }); + + it("enables bare named parameters on node:sqlite prepared statements", async () => { + if (isBunSqliteAvailable()) { + // bun:sqlite is opened with strict: true and never goes through the + // node:sqlite fallback, so there is nothing to spy on in this runtime. + return; + } + + const { StatementSync } = (await import("node:sqlite")) as typeof import("node:sqlite"); + const spy = vi.spyOn(StatementSync.prototype, "setAllowBareNamedParameters"); + + const db = getDatabase(); + db.exec(` + CREATE TABLE IF NOT EXISTS storage_named_param_spy_test ( + id INTEGER PRIMARY KEY, + value TEXT NOT NULL + ) STRICT; + `); + + db.prepare("INSERT INTO storage_named_param_spy_test (id, value) VALUES (@id, @value)").run({ + id: 1, + value: "spied value", + }); + + expect(spy).toHaveBeenCalledWith(true); + }); + it("commits writes made inside a transaction", () => { getDatabase().exec(` CREATE TABLE IF NOT EXISTS storage_smoke_test ( diff --git a/src/storage/db.ts b/src/storage/db.ts index c7e611660..6646f47ee 100644 --- a/src/storage/db.ts +++ b/src/storage/db.ts @@ -127,13 +127,13 @@ class NodeSqliteDatabase implements Database { } run(sql: string, params?: unknown): unknown { - const statement = this.db.prepare(sql); + const statement = this.prepareWithBareNamedParameters(sql); if (params === undefined) return statement.run(); return Array.isArray(params) ? statement.run(...(params as never[])) : statement.run(params as never); } query(sql: string): { get(params?: unknown): unknown; all(params?: unknown): unknown[] } { - const statement = this.db.prepare(sql); + const statement = this.prepareWithBareNamedParameters(sql); return { get: (params?: unknown) => { if (params === undefined) return statement.get(); @@ -146,6 +146,12 @@ class NodeSqliteDatabase implements Database { }; } + private prepareWithBareNamedParameters(sql: string): ReturnType { + const statement = this.db.prepare(sql); + statement.setAllowBareNamedParameters?.(true); + return statement; + } + transaction(fn: () => T): () => T { return () => { this.db.exec("BEGIN");