fix: address low-priority review items from Bun→Node migration

.github/workflows/ci.yml

@@ -193,6 +193,7 @@ jobs:
       - run: bun run typecheck
       - run: bun run check:deps
       - run: bun run check:errors
+      - run: bun run check:patches
 
   test-unit:
     name: Unit Tests

.lore.md

@@ -93,7 +93,7 @@
 * **ubuntu-latest defaults to Node 20 (EOL) — E2E jobs must use actions/setup-node**: Trap: \`ubuntu-latest\` GitHub Actions runner ships Node 20 (EOL). E2E jobs that run the npm bundle via \`node -e "..."\` silently use Node 20 unless \`actions/setup-node\` is explicitly added. This looks fine because the job succeeds, but it violates \`engines.node >=22.12\`. Fix: add \`actions/setup-node\` with \`node-version: 22\` to any job that invokes \`node\` directly. The \`build-npm\` job already does this correctly; E2E jobs need the same treatment. Never add feature-detection workarounds for Node 20 — it's EOL and unsupported.
 
 <!-- lore:019e464f-8fcb-7d16-b7ee-f3b157e1565e -->
-* **whichSync must use 'command -v' not 'which' for PATH-restricted lookups**: (pattern) Bun→Node.js API replacements: \`Bun.which(cmd, {PATH})\` → \`whichSync()\` from \`src/lib/which.ts\` (uses \`command -v\` shell builtin, NOT \`which\` binary — \`which\` fails when PATH is restricted to a test dir). \`Bun.spawn\` → \`spawn\` from \`node:child\_process\`; wrap \`.exited\` as \`new Promise(r => proc.on('close', code => r(code ?? 1)))\`. \*\*CRITICAL: always attach \`proc.on('error', () => {})\` — Node crashes on unhandled spawn errors; Bun did not.\*\* \`Bun.spawnSync\` → \`spawnSync\`. \`Bun.sleep(ms)\` → \`import { setTimeout as sleepMs } from 'node:timers/promises'\`. \`new Bun.Glob(p).match(i)\` → \`picomatch(p, { dot: true })(i)\` — pre-compile matchers outside hot loops. \`Bun.randomUUIDv7()\` → \`uuidv7()\`. \`Bun.semver.order()\` → \`compare()\` from \`semver\`. Only change actual API call sites — never comments. Update type annotations.
+* **Bun.which polyfill uses 'command -v' with 5s timeout for PATH-restricted lookups**: (pattern) Bun→Node.js API replacements live in \`script/node-polyfills.ts\` (injected at bundle time via esbuild). \`Bun.which(cmd, {PATH})\` polyfill uses \`command -v\` shell builtin (NOT \`which\` binary — \`which\` fails when PATH is restricted to a test dir) with \`timeout: 5000\` to prevent indefinite blocking. Windows uses \`where\`. \`Bun.spawn\` → \`spawn\` from \`node:child\_process\`; wrap \`.exited\` as \`new Promise(r => proc.on('close', code => r(code ?? 1)))\`. \*\*CRITICAL: always attach \`proc.on('error', () => {})\` — Node crashes on unhandled spawn errors; Bun did not.\*\* \`Bun.spawnSync\` → \`spawnSync\`. \`Bun.sleep(ms)\` → \`setTimeout\` promise. \`new Bun.Glob(p).match(i)\` → \`picomatch(p, { dot: true })(i)\` — pre-compile matchers outside hot loops. \`Bun.randomUUIDv7()\` → \`uuidv7()\`. \`Bun.semver.order()\` → \`compare()\` from \`semver\`. SQLite handled by \`src/lib/db/sqlite.ts\` (not the polyfill). Only change actual API call sites — never comments. Update type annotations.
 
 <!-- lore:019db0c9-9cc7-7352-b1f2-61b34b87b252 -->
 * **Whole-buffer matchAll slower than split+test when aggregated over many files**: Grep/scan traps in \`src/lib/scan/\`: (1) Whole-buffer \`regex.exec\` 12× faster per-file but ~1.6× SLOWER over 10k files — early-exit at \`maxResults\` via \`mapFilesConcurrent.onResult\` wins. (2) Literal prefilter is FILE-LEVEL gate (\`indexOf\`→skip); per-line verify breaks cross-newline patterns and Unicode length-changing \`toLowerCase\` (Turkish \`İ\`→\`i̇\`). (3) Extractor \`hasTopLevelAlternation\`+\`skipGroup\` must call \`skipCharacterClass\` (PCRE \`\[]abc]\` ≠ JS empty class). (4) Wake-latch race: naive \`let notify=null; await new Promise(r=>notify=r)\` loses signals — use latched \`pendingWake\` flag. (5) \`mapFilesConcurrent\` filters \`null\` but NOT \`\[]\` — return \`null\` for no-op files. (6) \`collectGlob\`/\`collectGrep\` must NOT forward \`maxResults\` to iterator; drain uncapped, set \`truncated=true\`.

package.json

@@ -112,6 +112,7 @@
     "check:fragments": "bun run script/check-fragments.ts",
     "check:deps": "bun run script/check-no-deps.ts",
     "check:errors": "bun run script/check-error-patterns.ts",
+    "check:patches": "bun run script/check-patches.ts",
     "check:docs-sections": "bun run script/generate-docs-sections.ts --check"
   },
   "type": "module",

script/bundle.ts

@@ -19,34 +19,6 @@ if (!SENTRY_CLIENT_ID) {
   process.exit(1);
 }
 
-// Regex patterns for esbuild plugin (must be top-level for performance)
-const BUN_SQLITE_FILTER = /^bun:sqlite$/;
-const ANY_FILTER = /.*/;
-
-/** Plugin to replace bun:sqlite with our node:sqlite polyfill. */
-const bunSqlitePlugin: Plugin = {
-  name: "bun-sqlite-polyfill",
-  setup(pluginBuild) {
-    pluginBuild.onResolve({ filter: BUN_SQLITE_FILTER }, () => ({
-      path: "bun:sqlite",
-      namespace: "bun-sqlite-polyfill",
-    }));
-
-    pluginBuild.onLoad(
-      { filter: ANY_FILTER, namespace: "bun-sqlite-polyfill" },
-      () => ({
-        contents: `
-          // Use the polyfill injected by node-polyfills.ts
-          const polyfill = globalThis.__bun_sqlite_polyfill;
-          export const Database = polyfill.Database;
-          export default polyfill;
-        `,
-        loader: "js",
-      })
-    );
-  },
-};
-
 type InjectedFile = { jsPath: string; mapPath: string; debugId: string };
 
 /** Delete .map files after a successful upload — they shouldn't ship to users. */
@@ -180,11 +152,7 @@ const sentrySourcemapPlugin: Plugin = {
 };
 
 // Always inject debug IDs (even without auth token); upload is gated inside the plugin
-const plugins: Plugin[] = [
-  bunSqlitePlugin,
-  sentrySourcemapPlugin,
-  textImportPlugin,
-];
+const plugins: Plugin[] = [sentrySourcemapPlugin, textImportPlugin];
 
 if (process.env.SENTRY_AUTH_TOKEN) {
   console.log("  Sentry auth token found, source maps will be uploaded");
@@ -226,6 +194,10 @@ const result = await build({
   // from trying to resolve these packages in the main bundle graph.
   external: [
     "node:*",
+    // bun:sqlite is referenced as a fallback in src/lib/db/sqlite.ts (never
+    // reached on Node 22+ where node:sqlite is available). Mark external so
+    // esbuild doesn't fail trying to resolve a Bun-only module.
+    "bun:sqlite",
     "ink",
     "ink-spinner",
     "react",

script/check-patches.ts

@@ -0,0 +1,85 @@
+#!/usr/bin/env bun
+/**
+ * Check Patched Dependency Versions
+ *
+ * Verifies that pnpm patchedDependencies target versions match installed versions.
+ * Name-only keys in patchedDependencies (pnpm 10+ catalog style) apply patches
+ * to whatever version resolves. If the installed version doesn't match the
+ * patch file's target version, the patch may silently fail to apply.
+ *
+ * Mismatches are surfaced as warnings (not hard failures) because pnpm 10
+ * name-only keys intentionally support version-agnostic patching — patches
+ * often apply cleanly across minor/patch bumps. The warning ensures engineers
+ * notice and can regenerate the patch if needed.
+ *
+ * Usage:
+ *   bun run script/check-patches.ts
+ *
+ * Exit codes:
+ *   0 - All patch versions match, or mismatches are non-critical (warnings)
+ *   1 - A patched package is missing entirely
+ */
+
+const pkg: {
+  pnpm?: { patchedDependencies?: Record<string, string> };
+} = await Bun.file("package.json").json();
+
+const patches = pkg.pnpm?.patchedDependencies ?? {};
+const warnings: string[] = [];
+const errors: string[] = [];
+export {};
+
+for (const [name, patchPath] of Object.entries(patches)) {
+  // Extract version from patch path: "patches/@stricli%2Fcore@1.2.5.patch" → "1.2.5"
+  // Handles pre-release versions like "1.2.3-beta.1" by matching everything after @M.N.P until .patch
+  const versionMatch = patchPath.match(/@(\d+\.\d+\.\d+[^@]*)\.patch$/);
+  if (!versionMatch) {
+    warnings.push(
+      `  ? ${name}: could not extract version from patch path "${patchPath}"`
+    );
+    continue;
+  }
+  const patchVersion = versionMatch[1];
+
+  // Resolve installed version
+  const pkgJsonPath = `node_modules/${name}/package.json`;
+  try {
+    const installed: { version: string } = await Bun.file(pkgJsonPath).json();
+    if (installed.version !== patchVersion) {
+      warnings.push(
+        `  ${name}: patch targets ${patchVersion}, installed ${installed.version} — regenerate with: pnpm patch ${name}`
+      );
+    }
+  } catch {
+    errors.push(`  ${name}: not installed (expected ${patchVersion})`);
+  }
+}
+
+// Emit GitHub Actions annotations for CI visibility
+const isCI = !!process.env.CI;
+for (const w of warnings) {
+  if (isCI) {
+    console.log(`::warning::Patch version mismatch:${w.trim()}`);
+  } else {
+    console.warn(`⚠ ${w}`);
+  }
+}
+
+if (errors.length > 0) {
+  console.error("✗ Missing patched dependencies:");
+  console.error("");
+  for (const e of errors) {
+    console.error(e);
+  }
+  console.error("");
+  console.error("Run pnpm install to install missing dependencies.");
+  process.exit(1);
+}
+
+if (warnings.length === 0) {
+  console.log("✓ All patched dependency versions match installed versions");
+} else {
+  console.log(
+    `✓ Patches applied (${warnings.length} version mismatch warning(s) — consider regenerating)`
+  );
+}

script/node-polyfills.ts

@@ -2,7 +2,7 @@
  * Node.js polyfills for Bun APIs. Injected at bundle time via esbuild.
  */
 import {
-  execSync,
+  execFileSync,
   spawn as nodeSpawn,
   spawnSync as nodeSpawnSync,
 } from "node:child_process";
@@ -12,9 +12,6 @@ import { promisify } from "node:util";
 // biome-ignore lint/performance/noNamespaceImport: runtime access to optional `zstdCompress`/`zstdDecompress` exports
 import * as zlibModule from "node:zlib";
 import { constants as zlibConstants } from "node:zlib";
-// node:sqlite is imported lazily inside NodeDatabasePolyfill to avoid
-// crashing on Node.js versions without node:sqlite support when the
-// bundle is loaded as a library (the consumer may never use SQLite).
 
 import picomatch from "picomatch";
 import { compare as semverCompare } from "semver";
@@ -25,85 +22,6 @@ declare global {
   var Bun: typeof BunPolyfill;
 }
 
-type SqliteValue = string | number | bigint | null | Uint8Array;
-
-/** Lazy-loaded node:sqlite DatabaseSync constructor. */
-function getNodeSqlite(): typeof import("node:sqlite").DatabaseSync {
-  // eslint-disable-next-line @typescript-eslint/no-require-imports
-  return require("node:sqlite").DatabaseSync;
-}
-
-/** Wraps node:sqlite StatementSync to match bun:sqlite query() API. */
-class NodeStatementPolyfill {
-  // biome-ignore lint/suspicious/noExplicitAny: node:sqlite types loaded lazily
-  private readonly stmt: any;
-
-  // biome-ignore lint/suspicious/noExplicitAny: node:sqlite types loaded lazily
-  constructor(stmt: any) {
-    this.stmt = stmt;
-  }
-
-  get(...params: SqliteValue[]): Record<string, SqliteValue> | undefined {
-    return this.stmt.get(...params) as Record<string, SqliteValue> | undefined;
-  }
-
-  all(...params: SqliteValue[]): Record<string, SqliteValue>[] {
-    return this.stmt.all(...params) as Record<string, SqliteValue>[];
-  }
-
-  run(...params: SqliteValue[]): void {
-    this.stmt.run(...params);
-  }
-}
-
-/** Wraps node:sqlite DatabaseSync to match bun:sqlite Database API. */
-class NodeDatabasePolyfill {
-  // biome-ignore lint/suspicious/noExplicitAny: node:sqlite types loaded lazily
-  private readonly db: any;
-
-  constructor(path: string) {
-    // SQLite configuration (busy_timeout, foreign_keys, WAL mode) is applied
-    // via PRAGMA statements in src/lib/db/index.ts after construction
-    const DatabaseSync = getNodeSqlite();
-    this.db = new DatabaseSync(path);
-  }
-
-  exec(sql: string): void {
-    this.db.exec(sql);
-  }
-
-  query(sql: string): NodeStatementPolyfill {
-    return new NodeStatementPolyfill(this.db.prepare(sql));
-  }
-
-  close(): void {
-    this.db.close();
-  }
-
-  /**
-   * Wraps a function in a transaction. Returns a callable that executes
-   * the function within BEGIN/COMMIT, with ROLLBACK on error.
-   * Matches Bun's db.transaction() API.
-   */
-  transaction<T>(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;
-      }
-    };
-  }
-}
-
-const bunSqlitePolyfill = { Database: NodeDatabasePolyfill };
-(globalThis as Record<string, unknown>).__bun_sqlite_polyfill =
-  bunSqlitePolyfill;
-
 const BunPolyfill = {
   file(path: string) {
     return {
@@ -162,22 +80,39 @@ const BunPolyfill = {
   which(command: string, opts?: { PATH?: string }): string | null {
     try {
       const isWindows = process.platform === "win32";
-      const cmd = isWindows ? `where ${command}` : `which ${command}`;
       // If a custom PATH is provided, override it in the subprocess env.
       // Use !== undefined (not truthy) so empty-string PATH is respected.
       const env =
         opts?.PATH !== undefined
           ? { ...process.env, PATH: opts.PATH }
           : undefined;
-      return (
-        execSync(cmd, {
+
+      let stdout: string;
+      if (isWindows) {
+        // execFileSync bypasses the shell entirely — no injection risk
+        stdout = execFileSync("where.exe", [command], {
           encoding: "utf-8",
           stdio: ["pipe", "pipe", "ignore"],
           env,
-        })
-          .trim()
-          .split("\n")[0] || null
-      );
+          timeout: 5000,
+        });
+      } else {
+        // Pass command as a positional arg ($1) so it's never interpolated
+        // into the shell string. `command -v` is a POSIX builtin — works
+        // even when PATH is overridden to a restricted set of directories.
+        stdout = execFileSync(
+          "/bin/sh",
+          ["-c", 'command -v "$1"', "--", command],
+          {
+            encoding: "utf-8",
+            stdio: ["pipe", "pipe", "ignore"],
+            env,
+            timeout: 5000,
+          }
+        );
+      }
+
+      return stdout.trim().split("\n")[0] || null;
     } catch {
       return null;
     }