diff --git a/MIGRATION-PLAN.md b/MIGRATION-PLAN.md new file mode 100644 index 000000000..17f471691 --- /dev/null +++ b/MIGRATION-PLAN.md @@ -0,0 +1,77 @@ +# Bun → Node.js Migration Plan + +Status: In progress. See below for completed and remaining steps. + +## Completed + +### Phase 4 (early): Package Manager Switch +- [x] Changed `packageManager` from `bun@1.3.13` to `pnpm@10.11.0` +- [x] Moved `patchedDependencies` into `pnpm` config section +- [x] Added `onlyBuiltDependencies: ["esbuild"]` +- [x] Added phantom deps as explicit devDependencies: `@sentry/core`, `@clack/prompts` +- [x] Generated `pnpm-lock.yaml` +- [x] Verified all patches apply (including cross-version: `@stricli/core@1.2.5` patch on `1.2.7`) + +### Phase 2, Group D: SQLite Adapter +- [x] Created `src/lib/db/sqlite.ts` — runtime-detecting adapter (bun:sqlite under Bun, node:sqlite under Node.js) +- [x] Updated 4 source files: `db/index.ts`, `schema.ts`, `migration.ts`, `utils.ts` +- [x] Updated 3 test files: `fix.test.ts`, `telemetry.test.ts`, `schema.test.ts` +- [x] Zero `bun:sqlite` imports remain in `src/` or `test/` + +## Remaining + +### Phase 2: Source Code Migration (replace Bun.* APIs in `src/`) + +**Group A: File I/O** — Replace `Bun.file()` / `Bun.write()` with `node:fs/promises` +- `Bun.file(path).text()` → `readFile(path, "utf-8")` +- `Bun.file(path).json()` → `readFile(path, "utf-8")` then `JSON.parse()` +- `Bun.file(path).exists()` → `access(path).then(() => true, () => false)` +- `Bun.write(path, content)` → `writeFile(path, content)` +- Scan all of `src/` for occurrences + +**Group B: Process/System APIs** — Replace Bun.which / Bun.spawn / Bun.sleep +- `Bun.which("cmd")` → `which` from a Node.js-compatible package or custom implementation +- `Bun.spawn()` / `Bun.spawnSync()` → `child_process.spawn()` / `spawnSync()` +- `Bun.sleep(ms)` → `setTimeout` promise wrapper + +**Group C: Miscellaneous Bun APIs** +- `Bun.Glob` → `tinyglobby` or `picomatch` (already in devDependencies) +- `Bun.randomUUIDv7()` → `uuidv7` package (already in devDependencies) +- `Bun.semver.order()` → `semver.compare()` (already in devDependencies) +- `Bun.zstdCompressSync()` / `Bun.zstdDecompressSync()` → Node.js zlib or `zstd-napi` package + +**Group E: Unpolyfilled APIs** +- `bspatch.ts` and `upgrade.ts` — Replace any Bun-specific APIs not covered by node-polyfills.ts + +### Phase 3: Test Migration (`bun:test` → Vitest) + +- Add `vitest` as devDependency +- Replace `import { ... } from "bun:test"` with Vitest equivalents +- Replace `bun test` scripts with `vitest` +- Key differences: + - `bun:test`'s `mock.module()` → Vitest's `vi.mock()` + - `bun:test`'s `spyOn` → Vitest's `vi.spyOn()` + - Test file discovery patterns may differ + - `--isolate --parallel` behavior needs Vitest equivalent + +### Phase 4: CI & Dev Scripts (remaining) + +- Update `package.json` scripts: `bun run` → `pnpm run` where appropriate +- Replace `bun run src/bin.ts` with `tsx src/bin.ts` (add `tsx` devDependency) +- Replace `bun run script/*.ts` with `tsx script/*.ts` +- Replace `bunx` with `pnpm exec` +- Update GitHub Actions workflows to use pnpm + Node.js instead of Bun +- Update `Dockerfile` / build scripts if applicable + +### Phase 5: Cleanup + +- Remove `@types/bun` from devDependencies +- Remove `bun.lock` (replaced by `pnpm-lock.yaml`) +- Remove or update `script/node-polyfills.ts` (may become unnecessary) +- Update `AGENTS.md` Bun API reference table +- Remove Bun-specific `.cursor/rules/bun-cli.mdc` or update for Node.js +- Clean up any remaining `Bun.*` references in comments/docs + +## Known Issues + +- `test/lib/index.test.ts` — `sdk.run throws when auth is required but missing` fails under pnpm's strict `node_modules`. The mock fetch returns empty 200s which prevents the expected auth error from being thrown. Pre-existing test fragility, not caused by migration changes. diff --git a/src/lib/db/index.ts b/src/lib/db/index.ts index 2e308ec34..89c08bac3 100644 --- a/src/lib/db/index.ts +++ b/src/lib/db/index.ts @@ -1,9 +1,10 @@ /** * SQLite database connection manager for CLI configuration storage. - * Uses bun:sqlite natively; Node.js uses a polyfill in node-polyfills.ts. + * Uses the sqlite.ts adapter which wraps node:sqlite's DatabaseSync + * with a bun:sqlite-compatible API surface. */ -import { Database } from "bun:sqlite"; +import { Database } from "./sqlite.js"; import { chmodSync, mkdirSync } from "node:fs"; import { join } from "node:path"; import { getEnv } from "../env.js"; diff --git a/src/lib/db/migration.ts b/src/lib/db/migration.ts index 34ec331e4..02c00abb3 100644 --- a/src/lib/db/migration.ts +++ b/src/lib/db/migration.ts @@ -2,7 +2,7 @@ * One-time migration from config.json to SQLite. */ -import type { Database } from "bun:sqlite"; +import type { Database } from "./sqlite.js"; import { rmSync } from "node:fs"; import { join } from "node:path"; import { logger } from "../logger.js"; diff --git a/src/lib/db/schema.ts b/src/lib/db/schema.ts index e4ee65c97..090510327 100644 --- a/src/lib/db/schema.ts +++ b/src/lib/db/schema.ts @@ -11,7 +11,7 @@ * - Migration checks */ -import type { Database } from "bun:sqlite"; +import type { Database } from "./sqlite.js"; import { getEnv } from "../env.js"; import { stringifyUnknown } from "../errors.js"; import { logger } from "../logger.js"; diff --git a/src/lib/db/sqlite.ts b/src/lib/db/sqlite.ts new file mode 100644 index 000000000..94169d58a --- /dev/null +++ b/src/lib/db/sqlite.ts @@ -0,0 +1,116 @@ +/** + * SQLite adapter wrapping node:sqlite's DatabaseSync with a convenient API. + * + * This module is the single import point for all SQLite access in the + * codebase. It provides a `.query(sql).get()` / `.all()` / `.run()` + * interface and a manual `transaction()` wrapper. + * + * Uses `node:sqlite` (Node 22+) as the backing implementation. Falls back + * to `bun:sqlite` when `node:sqlite` is unavailable (Bun runtime) — this + * fallback will be removed once the test runner migrates off Bun. + */ + +/** Valid SQLite binding value */ +export type SQLQueryBindings = + | string + | number + | bigint + | null + | Uint8Array + | undefined; + +/** + * Prepared statement wrapper exposing `.get()`, `.all()`, `.run()`. + */ +class StatementWrapper { + // biome-ignore lint/suspicious/noExplicitAny: backing driver types vary + private readonly stmt: any; + + // biome-ignore lint/suspicious/noExplicitAny: backing driver types vary + constructor(stmt: any) { + this.stmt = stmt; + } + + get( + ...params: SQLQueryBindings[] + ): Record | undefined { + return this.stmt.get(...params) as + | Record + | undefined; + } + + all(...params: SQLQueryBindings[]): Record[] { + return this.stmt.all(...params) as Record[]; + } + + run(...params: SQLQueryBindings[]): void { + this.stmt.run(...params); + } +} + +/** Resolve the underlying SQLite constructor. */ +function getSqliteConstructor(): new (path: string) => { + exec(sql: string): void; + prepare(sql: string): unknown; + close(): void; +} { + try { + // Primary: node:sqlite (Node 22+) + return require("node:sqlite").DatabaseSync; + } catch { + // Fallback: bun:sqlite — remove once test runner migrates off Bun + return require("bun:sqlite").Database; + } +} + +// biome-ignore lint/suspicious/noExplicitAny: resolved dynamically +const SqliteImpl: any = getSqliteConstructor(); + +/** + * SQLite database wrapper. + * + * - `exec(sql)` — execute raw SQL (DDL, multi-statement) + * - `query(sql)` — prepare a statement → `.get()` / `.all()` / `.run()` + * - `close()` — close the connection + * - `transaction(fn)` — wrap a function in BEGIN/COMMIT/ROLLBACK + */ +export class Database { + // biome-ignore lint/suspicious/noExplicitAny: backing driver resolved at runtime + private readonly db: any; + + constructor(path: string) { + this.db = new SqliteImpl(path); + } + + exec(sql: string): void { + this.db.exec(sql); + } + + query(sql: string): StatementWrapper { + // node:sqlite uses .prepare(), bun:sqlite uses .query() + const prepFn = this.db.prepare ?? this.db.query; + return new StatementWrapper(prepFn.call(this.db, sql)); + } + + close(): void { + this.db.close(); + } + + transaction(fn: () => T): () => T { + // bun:sqlite has native transaction(); node:sqlite does not + if (typeof this.db.transaction === "function") { + return this.db.transaction(fn); + } + return () => { + this.db.exec("BEGIN"); + try { + const result = fn(); + this.db.exec("COMMIT"); + return result; + } catch (error) { + this.db.exec("ROLLBACK"); + throw error; + } + }; + } +} diff --git a/src/lib/db/utils.ts b/src/lib/db/utils.ts index 8d12eb3ca..ac79cb9e9 100644 --- a/src/lib/db/utils.ts +++ b/src/lib/db/utils.ts @@ -3,11 +3,11 @@ * Reduces boilerplate for UPSERT and other repetitive patterns. */ -import type { SQLQueryBindings } from "bun:sqlite"; +import type { SQLQueryBindings } from "./sqlite.js"; import { getDatabase } from "./index.js"; -/** Valid SQLite binding value (matches bun:sqlite's SQLQueryBindings) */ +/** Valid SQLite binding value (re-exported from sqlite.ts adapter) */ export type SqlValue = SQLQueryBindings; /** diff --git a/test/commands/cli/fix.test.ts b/test/commands/cli/fix.test.ts index 634de40c2..d47754643 100644 --- a/test/commands/cli/fix.test.ts +++ b/test/commands/cli/fix.test.ts @@ -10,7 +10,7 @@ * and code spans have backticks stripped. */ -import { Database } from "bun:sqlite"; +import { Database } from "../../../src/lib/db/sqlite.js"; import { afterEach, describe, expect, mock, spyOn, test } from "bun:test"; import { chmodSync, statSync } from "node:fs"; import { join } from "node:path"; diff --git a/test/lib/db/schema.test.ts b/test/lib/db/schema.test.ts index 2b8bf810c..7171e0e46 100644 --- a/test/lib/db/schema.test.ts +++ b/test/lib/db/schema.test.ts @@ -2,7 +2,7 @@ * Tests for database schema repair functions. */ -import { Database } from "bun:sqlite"; +import { Database } from "../../../src/lib/db/sqlite.js"; import { describe, expect, test } from "bun:test"; import { join } from "node:path"; import { diff --git a/test/lib/telemetry.test.ts b/test/lib/telemetry.test.ts index fb58f00ce..4d4658feb 100644 --- a/test/lib/telemetry.test.ts +++ b/test/lib/telemetry.test.ts @@ -4,7 +4,7 @@ * Tests for withTelemetry wrapper and opt-out behavior. */ -import { Database } from "bun:sqlite"; +import { Database } from "../../src/lib/db/sqlite.js"; import { afterAll, afterEach,