diff --git a/AGENTS.md b/AGENTS.md index 236be4869..8ac24f6a4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -27,4 +27,13 @@ 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`. +- 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 + +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 diff --git a/src/storage/db.test.ts b/src/storage/db.test.ts new file mode 100644 index 000000000..c44883359 --- /dev/null +++ b/src/storage/db.test.ts @@ -0,0 +1,126 @@ +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 = ""; + + 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("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 ( + 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..6646f47ee 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,82 @@ 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.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.prepareWithBareNamedParameters(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); + }, + }; + } + + 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"); + 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;