Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,12 @@ How to keep this current: add the entry in the same pull request as the change,

<!-- Empty. Next release starts here. -->

## 0.74.1

### Fixed

- Learning features work when Pi runs on Bun. Pi's release binaries (`pi-linux-x64.tar.gz` and the rest) are Bun `--compile` executables in which `import("node:sqlite")` fails with `No such built-in module: node:sqlite`, so `holds.db` never opened and learning was off for every release-binary user. `node:sqlite` is still tried first and is unchanged; only when that import fails and the process runs on Bun does `src/sqlite-adapter.ts` open the database through `bun:sqlite` behind the `DatabaseSync` subset learning uses. With neither module loading, learning stays off behind one warning that now names both modules.

## 0.74.0

### Added
Expand Down
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "pi-warden",
"version": "0.74.0",
"version": "0.74.1",
"description": "Makes the Pi agent follow your project's rules. Jev judges every write against your pi-warden.md and quotes the broken rule back to the agent, names slop, breaks stuck loops, calls out unverified done claims, compresses large tool output, and holds the rare destructive command. Built on pi-typesafe.",
"type": "module",
"license": "MIT",
Expand Down Expand Up @@ -54,6 +54,7 @@
"build": "tsc -p tsconfig.build.json",
"typecheck": "tsc --noEmit",
"test": "node --import tsx --test tests/*.test.ts",
"test:bun": "bun test tests/sqlite-adapter.test.ts tests/learning.test.ts",
"test:live": "node --env-file-if-exists=.env scripts/live-smoke.mjs",
"dev:pi": "node scripts/dev-pi.mjs",
"preview": "node scripts/render-preview.mjs",
Expand Down
35 changes: 35 additions & 0 deletions src/bun-sqlite.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/**
* Local types for the optional `bun:sqlite` fallback, so the TypeScript build needs no new
* dependency. Only the members `src/sqlite-adapter.ts` touches are declared; Bun ships its own
* types with its runtime. Measured on Bun 1.4.2: `exec()` answers a result object instead of
* void, `get()` answers `null` for a miss instead of undefined, and `prepare()` has no
* `pragma()` counterpart (PRAGMAs go through `exec()`).
*/
declare module "bun:sqlite" {
export interface StatementResultingChanges {
changes: number | bigint;
lastInsertRowid: number | bigint;
}

export class Statement {
run(...params: unknown[]): StatementResultingChanges;
get(...params: unknown[]): Record<string, unknown> | null;
all(...params: unknown[]): Record<string, unknown>[];
}

export interface DatabaseOptions {
/** Create the file when it does not exist. Defaults to `true`. */
create?: boolean;
/** Open read-only. */
readonly?: boolean;
/** Require named parameters without the `$` prefix. Defaults to `false`, which accepts both. */
strict?: boolean;
}

export class Database {
constructor(path: string, options?: DatabaseOptions);
exec(sql: string): unknown;
prepare(sql: string): Statement;
close(): void;
}
}
53 changes: 42 additions & 11 deletions src/learning.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,18 @@
import { createHash } from "crypto";
import { mkdirSync } from "fs";
import { dirname, join } from "path";
import type { DatabaseSync } from "node:sqlite";
import { isBunRuntime, openBunSqlite, SqliteUnavailableError } from "./sqlite-adapter.js";
import type { SqliteDb, SqliteDriver } from "./sqlite-adapter.js";
import type { HostDirs } from "./host-dirs.js";
import { defaultHostDirs } from "./host-dirs.js";
import { userConfigPath } from "./config.js";
import { redact } from "./redact.js";
import type { CallScores } from "./holds.js";

const dbs = new Map<string, Promise<DatabaseSync>>();
const dbs = new Map<string, Promise<SqliteDb>>();
let sqliteAvailable: boolean | undefined;
/** Which module opened the learning database; undefined while learning is off. */
let driver: SqliteDriver | undefined;

// --- Schema (shared constant) ---

Expand Down Expand Up @@ -46,27 +49,50 @@ const NOOP_DB = {
exec() {},
prepare() { return { run() { return { changes: 0, lastInsertRowid: 0 }; }, get() { return undefined; }, all() { return []; } }; },
pragma() {},
} as unknown as DatabaseSync;
} as unknown as SqliteDb;

/** Learning stays off: one warning names both candidate modules so the report says what to look at. */
function disableLearning(detail: unknown): SqliteDb {
sqliteAvailable = false;
console.warn("pi-warden: node:sqlite unavailable, bun:sqlite unavailable, learning features disabled:", detail);
return NOOP_DB;
}

/** Pi's release binaries are Bun --compile executables, where node:sqlite may not be a built-in
* module while bun:sqlite always is. On Bun the database opens through the adapter; with neither
* module loading, learning stays off behind one warning. */
async function openBunFallback(dbPath: string, nodeFailure: unknown): Promise<SqliteDb> {
if (!isBunRuntime()) return disableLearning(nodeFailure);
try {
const db = await openBunSqlite(dbPath);
sqliteAvailable = true;
driver = "bun:sqlite";
return db;
} catch (err) {
if (err instanceof SqliteUnavailableError) return disableLearning(err);
// bun:sqlite did load (this is Bun), so only this path failed, as on the node:sqlite path.
sqliteAvailable = true;
console.warn(`pi-warden: could not open ${dbPath}:`, err);
return NOOP_DB;
}
}

/** Open one connection; a failure is remembered per path so later calls neither warn again nor retry forever. */
async function openDb(dbPath: string): Promise<DatabaseSync> {
let opened: DatabaseSync | undefined;
async function openDb(dbPath: string): Promise<SqliteDb> {
let opened: SqliteDb | undefined;
try {
// Static import cannot work: node:sqlite is flagged experimental and loads lazily so a missing
// or broken build of it disables learning features instead of failing the whole process.
const sqlite = await import("node:sqlite").catch(err => ({ importFailed: err as unknown }));
if ("importFailed" in sqlite) {
sqliteAvailable = false;
console.warn("pi-warden: node:sqlite unavailable, learning features disabled:", sqlite.importFailed);
return NOOP_DB;
}
if ("importFailed" in sqlite) return await openBunFallback(dbPath, sqlite.importFailed);
const { DatabaseSync } = sqlite;
sqliteAvailable = true;
// DatabaseSync does not create parent directories; on a fresh machine the folder may not exist yet.
mkdirSync(dirname(dbPath), { recursive: true, mode: 0o700 });
opened = new DatabaseSync(dbPath);
opened.exec("PRAGMA journal_mode = WAL");
opened.exec("PRAGMA busy_timeout = 10000");
driver = "node:sqlite";
return opened;
} catch (err) {
// Import failures are handled above; this is a path open or PRAGMA failure for this path only.
Expand All @@ -79,14 +105,19 @@ async function openDb(dbPath: string): Promise<DatabaseSync> {
}

/** One connection per resolved path; concurrent first calls share one open. */
async function getDb(dirs: HostDirs = defaultHostDirs()): Promise<DatabaseSync> {
async function getDb(dirs: HostDirs = defaultHostDirs()): Promise<SqliteDb> {
const dbPath = process.env.PI_WARDEN_DB ?? join(dirname(userConfigPath(dirs)), "holds.db");
if (sqliteAvailable === false) return NOOP_DB;
let opening = dbs.get(dbPath);
if (!opening) dbs.set(dbPath, opening = openDb(dbPath));
return opening;
}

/** Which module opened the learning database: "node:sqlite", "bun:sqlite", or undefined while learning is off. */
export function sqliteDriver(): SqliteDriver | undefined {
return driver;
}

export async function initSchema(retentionDays = 365, dirs: HostDirs = defaultHostDirs()): Promise<void> {
try {
const d = await getDb(dirs);
Expand Down
110 changes: 110 additions & 0 deletions src/sqlite-adapter.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
// src/sqlite-adapter.ts - bun:sqlite fallback for the learning database
//
// node:sqlite stays the first choice and is not touched here. Pi's release binaries are Bun
// --compile executables in which node:sqlite may not be a built-in module, so learning needs a
// second way in: bun:sqlite, wrapped to answer like the DatabaseSync subset learning calls. One
// module owns the difference, so src/learning.ts keeps a single code path for both drivers.

import { mkdirSync } from "node:fs";
import { dirname } from "node:path";

/** The driver that opened the learning database. */
export type SqliteDriver = "node:sqlite" | "bun:sqlite";

/** Bindable values. Learning binds only positional `?` placeholders, never named keys, so the
* named-parameter prefixes never meet: Bun's default (non-strict) mode already accepts both a
* bare `name` and a `$name` key, and the adapter does not open in strict mode. */
export type SqliteValue = null | number | bigint | string | ArrayBufferView;

export type SqliteRow = Record<string, unknown>;

export interface SqliteRunResult {
changes: number | bigint;
lastInsertRowid: number | bigint;
}

export interface SqliteStatement {
run(...params: SqliteValue[]): SqliteRunResult;
get(...params: SqliteValue[]): SqliteRow | undefined;
all(...params: SqliteValue[]): SqliteRow[];
}

/** The DatabaseSync subset learning uses: schema and PRAGMAs through exec, rows through prepare. */
export interface SqliteDb {
exec(sql: string): void;
prepare(sql: string): SqliteStatement;
close(): void;
}

/** bun:sqlite's statement shape, as the adapter reads it. */
export interface BunSqliteStatement {
run(...params: unknown[]): SqliteRunResult;
get(...params: unknown[]): SqliteRow | null;
all(...params: unknown[]): SqliteRow[];
}

/** bun:sqlite's handle shape, as the adapter reads it. */
export interface BunSqliteHandle {
exec(sql: string): unknown;
prepare(sql: string): BunSqliteStatement;
close(): void;
}

/** bun:sqlite could not be loaded. The caller reports "neither module" and learning stays off. */
export class SqliteUnavailableError extends Error {
constructor(moduleName: string, cause: unknown) {
super(`${moduleName} could not be loaded`, { cause });
}
}

/** True when this process is Bun, the only runtime where bun:sqlite can be loaded. */
export function isBunRuntime(): boolean {
return typeof process.versions.bun === "string" || typeof (globalThis as { Bun?: unknown }).Bun !== "undefined";
}

/**
* Open the learning database through bun:sqlite. Throws SqliteUnavailableError when the module
* does not load; any other error is a path open or PRAGMA failure for this path only, and the
* half-open handle is closed first, as the node:sqlite path does.
*/
export async function openBunSqlite(dbPath: string): Promise<SqliteDb> {
let bunSqlite: typeof import("bun:sqlite");
try {
bunSqlite = await import("bun:sqlite");
} catch (err) {
throw new SqliteUnavailableError("bun:sqlite", err);
}
// bun:sqlite does not create parent directories either; on a fresh machine the folder is missing.
mkdirSync(dirname(dbPath), { recursive: true, mode: 0o700 });
const db = wrapBunDatabase(new bunSqlite.Database(dbPath));
try {
db.exec("PRAGMA journal_mode = WAL");
db.exec("PRAGMA busy_timeout = 10000");
} catch (err) {
try { db.close(); } catch { /* already closed */ }
throw err;
}
return db;
}

/** Present a bun:sqlite handle as the DatabaseSync subset learning uses. */
export function wrapBunDatabase(db: BunSqliteHandle): SqliteDb {
return {
exec(sql) {
db.exec(sql);
},
prepare(sql) {
const stmt = db.prepare(sql);
return {
run: (...params) => stmt.run(...params),
// Bun answers a miss with null where node:sqlite answers undefined. Learning reads no row
// as undefined, so the adapter hands back the node answer.
get: (...params) => stmt.get(...params) ?? undefined,
all: (...params) => stmt.all(...params),
};
},
close() {
db.close();
},
};
}
102 changes: 102 additions & 0 deletions tests/learning-driver.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
/**
* Which driver the learning database actually opens with, observed from a child process so the
* module-level connection cache and warnings start fresh. Both children run on Node: the Bun
* runtime is covered by tests/sqlite-adapter.test.ts, and Bun cannot be made to refuse node:sqlite
* here (Bun 1.4.2 loads it even from a --compile binary), so this file's children only run under
* Node, where `npm test` runs them.
*/
import assert from "node:assert/strict";
import { test } from "node:test";
import { execFile } from "node:child_process";
import { mkdtempSync, rmSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { fileURLToPath } from "node:url";
import { promisify } from "node:util";

const run = promisify(execFile);
const root = join(fileURLToPath(new URL(".", import.meta.url)), "..");

const hookSource = `
export async function resolve(specifier, context, next) {
if (specifier === "node:sqlite") throw new Error("blocked node:sqlite for this test");
return next(specifier, context);
}
`;

const recordHoldCall = (projectRoot: string) => `await learning.recordHold({
timestamp: Date.now(),
projectRoot: ${JSON.stringify(projectRoot)},
tool: "bash",
commandPreview: "npm test",
scores: { irreversible: 0.5, reasons: ["irreversible 0.5"] },
level: "allow",
held: true,
reasons: ["irreversible 0.5"],
});`;

/** Run a child that loads learning.ts and prints one parsed RESULT line. */
async function childResult(child: string, temp: string): Promise<{ stdout: string; stderr: string; data: Record<string, unknown> }> {
const { stdout, stderr } = await run(process.execPath, ["--import", "tsx", "--input-type=module", "-e", child], {
cwd: root,
env: { ...process.env, PI_WARDEN_DB: join(temp, "holds.db"), PI_CODING_AGENT_DIR: temp },
encoding: "utf8",
});
const line = stdout.split("\n").find(row => row.startsWith("RESULT "));
assert.ok(line, "child printed a result line; stdout was: " + stdout);
return { stdout, stderr, data: JSON.parse(line.slice("RESULT ".length)) as Record<string, unknown> };
}

const onBun = typeof process.versions.bun === "string";

if (!onBun) {
test("the Node path opens the learning database with node:sqlite", async () => {
const temp = mkdtempSync(join(tmpdir(), "pi-warden-driver-"));
try {
const child = `
const learning = await import("./src/learning.ts");
await learning.initSchema(0);
const id = ${recordHoldCall("/driver/node/project")}
const rows = await learning.queryHoldsForProject("/driver/node/project");
console.log("RESULT " + JSON.stringify({ driver: learning.sqliteDriver() ?? null, id, rows: rows.length }));
`;
const { data } = await childResult(child, temp);
assert.equal(data.driver, "node:sqlite", "node:sqlite is tried first and loads on Node");
assert.ok((data.id as number) > 0, "recordHold returned a real id, not the NOOP id");
assert.equal(data.rows, 1, "the row was written to SQLite");
} finally {
rmSync(temp, { recursive: true, force: true });
}
});

test("with node:sqlite unresolvable and no Bun, learning turns off behind one warning naming both modules", async () => {
const temp = mkdtempSync(join(tmpdir(), "pi-warden-driver-"));
try {
const child = `
import { register } from "node:module";
register("data:text/javascript," + encodeURIComponent(${JSON.stringify(hookSource)}));
const warnings = [];
const originalWarn = console.warn;
console.warn = (...args) => warnings.push(args.map(String).join(" "));
const learning = await import("./src/learning.ts");
await learning.initSchema(0);
const id = ${recordHoldCall("/driver/blocked/project")}
const rows = await learning.queryHoldsForProject("/driver/blocked/project");
console.warn = originalWarn;
console.log("RESULT " + JSON.stringify({ driver: learning.sqliteDriver() ?? null, id, rows: rows.length, warnings }));
`;
const { stderr, data } = await childResult(child, temp);
const warnings = data.warnings as string[];
assert.equal(warnings.length, 1, "exactly one warning, from the failed open");
assert.match(warnings[0]!, /node:sqlite/, "the warning names node:sqlite");
assert.match(warnings[0]!, /bun:sqlite/, "the warning names bun:sqlite");
assert.match(warnings[0]!, /learning features disabled/, "the warning says learning is off");
assert.equal(stderr.includes("pi-warden:"), false, "nothing else warned to stderr");
assert.equal(data.driver, null, "no driver opened");
assert.equal(data.id, 0, "recordHold answers the NOOP id instead of failing");
assert.equal(data.rows, 0, "queries answer nothing instead of failing");
} finally {
rmSync(temp, { recursive: true, force: true });
}
});
}
Loading
Loading