Jelajahi Sumber

fix: prioritize package-lock.json in launcher to prevent Bun false positive

The bin/qmd wrapper checks for bun.lock to select the runtime, but since
bun.lock is committed to the repo, source builds using npm install are
incorrectly routed to Bun — causing native module ABI mismatches (#381)
and sqlite-vec crashes (#380).

Add package-lock.json as a higher-priority signal: if it exists, npm
installed the dependencies and Node should be used. Also fix
cleanupOrphanedVectors() to use the existing isSqliteVecAvailable()
guard instead of checking sqlite_master, which can report the virtual
table even when the vec0 module isn't loaded.

Fixes #381, fixes #380
Continuation of #362 (runtime detection false positives)
Ryan Malia 2 bulan lalu
induk
melakukan
28903d8eba
3 mengubah file dengan 112 tambahan dan 12 penghapusan
  1. 11 6
      bin/qmd
  2. 4 6
      src/store.ts
  3. 97 0
      test/launcher-detection.test.sh

+ 11 - 6
bin/qmd

@@ -15,12 +15,17 @@ done
 # to avoid native module ABI mismatches (e.g., better-sqlite3 compiled for bun vs node)
 DIR="$(cd -P "$(dirname "$SOURCE")/.." && pwd)"
 
-# Check if we were installed with bun (look for bun.lock or bun-lockb).
-# $BUN_INSTALL is intentionally NOT checked here — it only indicates that bun
-# exists on the system, not that it was used to install this package.  When QMD
-# is installed via npm, native modules are compiled for Node and running them
-# under bun causes ABI mismatches (e.g. sqlite-vec "no such module: vec0").
-if [ -f "$DIR/bun.lock" ] || [ -f "$DIR/bun.lockb" ]; then
+# Detect the package manager that installed dependencies by checking lockfiles.
+# $BUN_INSTALL is intentionally NOT checked — it only indicates that bun exists
+# on the system, not that it was used to install this package (see #361).
+#
+# package-lock.json takes priority: if it exists, npm installed the native
+# modules for Node.  The repo ships bun.lock, so without this check, source
+# builds that use npm would be incorrectly routed to bun, causing ABI
+# mismatches with better-sqlite3 / sqlite-vec (see #381).
+if [ -f "$DIR/package-lock.json" ]; then
+  exec node "$DIR/dist/cli/qmd.js" "$@"
+elif [ -f "$DIR/bun.lock" ] || [ -f "$DIR/bun.lockb" ]; then
   exec bun "$DIR/dist/cli/qmd.js" "$@"
 else
   exec node "$DIR/dist/cli/qmd.js" "$@"

+ 4 - 6
src/store.ts

@@ -1686,12 +1686,10 @@ export function cleanupOrphanedContent(db: Database): number {
  * Returns the number of orphaned embedding chunks deleted.
  */
 export function cleanupOrphanedVectors(db: Database): number {
-  // Check if vectors_vec table exists
-  const tableExists = db.prepare(`
-    SELECT name FROM sqlite_master WHERE type='table' AND name='vectors_vec'
-  `).get();
-
-  if (!tableExists) {
+  // sqlite-vec may not be loaded (e.g. Bun's bun:sqlite lacks loadExtension).
+  // The vectors_vec virtual table can appear in sqlite_master from a prior
+  // session, but querying it without the vec0 module loaded will crash (#380).
+  if (!isSqliteVecAvailable()) {
     return 0;
   }
 

+ 97 - 0
test/launcher-detection.test.sh

@@ -0,0 +1,97 @@
+#!/usr/bin/env bash
+# Tests for bin/qmd runtime detection logic.
+# Simulates lockfile combinations in a temp directory and verifies which
+# runtime the launcher would choose.
+#
+# Usage: bash test/launcher-detection.test.sh
+set -euo pipefail
+
+PASS=0
+FAIL=0
+TMPDIR_BASE=$(mktemp -d)
+
+cleanup() { rm -rf "$TMPDIR_BASE"; }
+trap cleanup EXIT
+
+ok()   { printf "  %-60s OK\n" "$1"; PASS=$((PASS + 1)); }
+fail() { printf "  %-60s FAIL\n" "$1 (got: $2, expected: $3)"; FAIL=$((FAIL + 1)); }
+
+# Extract the detection logic from bin/qmd into a testable function.
+# Instead of exec-ing a runtime, we echo which one would be chosen.
+detect_runtime() {
+  local DIR="$1"
+  if [ -f "$DIR/package-lock.json" ]; then
+    echo "node"
+  elif [ -f "$DIR/bun.lock" ] || [ -f "$DIR/bun.lockb" ]; then
+    echo "bun"
+  else
+    echo "node"
+  fi
+}
+
+# Verify detect_runtime matches the actual bin/qmd logic
+assert_runtime() {
+  local label="$1" dir="$2" expected="$3"
+  local got
+  got=$(detect_runtime "$dir")
+  if [[ "$got" == "$expected" ]]; then
+    ok "$label"
+  else
+    fail "$label" "$got" "$expected"
+  fi
+}
+
+echo "=== bin/qmd runtime detection tests ==="
+
+# --- Test cases ---
+
+# 1. No lockfiles → default to node
+d="$TMPDIR_BASE/no-lockfiles"
+mkdir -p "$d"
+assert_runtime "no lockfiles → node" "$d" "node"
+
+# 2. Only bun.lock → bun
+d="$TMPDIR_BASE/bun-lock-only"
+mkdir -p "$d"
+touch "$d/bun.lock"
+assert_runtime "bun.lock only → bun" "$d" "bun"
+
+# 3. Only bun.lockb → bun
+d="$TMPDIR_BASE/bun-lockb-only"
+mkdir -p "$d"
+touch "$d/bun.lockb"
+assert_runtime "bun.lockb only → bun" "$d" "bun"
+
+# 4. Only package-lock.json → node
+d="$TMPDIR_BASE/npm-only"
+mkdir -p "$d"
+touch "$d/package-lock.json"
+assert_runtime "package-lock.json only → node" "$d" "node"
+
+# 5. Both package-lock.json AND bun.lock → node (npm takes priority)
+# This is the key fix for #381: source checkouts have bun.lock committed,
+# and contributors who run npm install also create package-lock.json.
+d="$TMPDIR_BASE/both-lockfiles"
+mkdir -p "$d"
+touch "$d/package-lock.json"
+touch "$d/bun.lock"
+assert_runtime "package-lock.json + bun.lock → node (npm priority)" "$d" "node"
+
+# 6. Both package-lock.json AND bun.lockb → node (npm takes priority)
+d="$TMPDIR_BASE/both-lockfiles-b"
+mkdir -p "$d"
+touch "$d/package-lock.json"
+touch "$d/bun.lockb"
+assert_runtime "package-lock.json + bun.lockb → node (npm priority)" "$d" "node"
+
+# 7. All three lockfiles → node (npm takes priority)
+d="$TMPDIR_BASE/all-lockfiles"
+mkdir -p "$d"
+touch "$d/package-lock.json"
+touch "$d/bun.lock"
+touch "$d/bun.lockb"
+assert_runtime "all three lockfiles → node (npm priority)" "$d" "node"
+
+echo ""
+echo "=== Results: $PASS passed, $FAIL failed ==="
+[[ $FAIL -eq 0 ]]