From c9fb623e0221bab184de9850a48eb7ab32d2bbbf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9CBeeRad=E2=80=9D?= Date: Sat, 25 Apr 2026 16:09:14 +1000 Subject: [PATCH] fix: align WAL multi-surface DB safety - align app and standalone MCP busy-timeout and retry behavior - guard live DB dev and restore flows - document WAL maintenance boundaries --- apps/mcp-server-standalone/README.md | 1 + .../services/edgeService.js | 20 ++-- .../services/sqlite-client.js | 64 ++++++++++--- docs/8_mcp.md | 9 ++ scripts/database/rah-db-owners.sh | 30 ++++++ scripts/database/sqlite-backup.sh | 7 ++ scripts/database/sqlite-restore.sh | 8 ++ scripts/dev/guard-live-db-dev.sh | 37 +++++++ scripts/dev/run-tauri-dev.sh | 2 + src/services/database/sqlite-client.ts | 96 ++++++++++++++----- 10 files changed, 226 insertions(+), 48 deletions(-) create mode 100755 scripts/database/rah-db-owners.sh create mode 100755 scripts/dev/guard-live-db-dev.sh diff --git a/apps/mcp-server-standalone/README.md b/apps/mcp-server-standalone/README.md index 15384a6..91f5330 100644 --- a/apps/mcp-server-standalone/README.md +++ b/apps/mcp-server-standalone/README.md @@ -28,6 +28,7 @@ Important contract: - exact versions are only for release/debug reproducibility - standalone MCP reads and writes node data directly - standalone MCP does not own chunking, embeddings, or live schema migration on an existing DB +- standalone MCP and the app use SQLite WAL; normal short reads/writes can coexist, while restore/replace/checkpoint/delete-WAL maintenance requires all DB owners to be closed first ## Configure Claude Code / Claude Desktop diff --git a/apps/mcp-server-standalone/services/edgeService.js b/apps/mcp-server-standalone/services/edgeService.js index cefa7bf..e8de447 100644 --- a/apps/mcp-server-standalone/services/edgeService.js +++ b/apps/mcp-server-standalone/services/edgeService.js @@ -1,6 +1,6 @@ 'use strict'; -const { query, getDb } = require('./sqlite-client'); +const { query, getDb, runWithBusyRetry } = require('./sqlite-client'); /** * Get all edges. @@ -74,12 +74,14 @@ function createEdge(edgeData) { VALUES (?, ?, ?, ?, ?) `); - const result = stmt.run( - from_node_id, - to_node_id, - JSON.stringify(context), - source, - now + const result = runWithBusyRetry(() => stmt.run( + from_node_id, + to_node_id, + JSON.stringify(context), + source, + now + ), + 'createEdge' ); const edgeId = Number(result.lastInsertRowid); @@ -109,14 +111,14 @@ function updateEdge(id, updates) { }; const stmt = db.prepare('UPDATE edges SET context = ? WHERE id = ?'); - stmt.run(JSON.stringify(newContext), id); + runWithBusyRetry(() => stmt.run(JSON.stringify(newContext), id), 'updateEdge'); } else if (contextUpdates) { const newContext = { ...existing.context, ...contextUpdates }; const stmt = db.prepare('UPDATE edges SET context = ? WHERE id = ?'); - stmt.run(JSON.stringify(newContext), id); + runWithBusyRetry(() => stmt.run(JSON.stringify(newContext), id), 'updateEdge'); } return getEdgeById(id); diff --git a/apps/mcp-server-standalone/services/sqlite-client.js b/apps/mcp-server-standalone/services/sqlite-client.js index d52b57f..7fa3c8b 100644 --- a/apps/mcp-server-standalone/services/sqlite-client.js +++ b/apps/mcp-server-standalone/services/sqlite-client.js @@ -89,11 +89,11 @@ function initDatabase() { db = new Database(dbPath); - // Configure SQLite for performance + // Configure SQLite for WAL-first app/MCP coexistence. db.pragma('journal_mode = WAL'); db.pragma('synchronous = NORMAL'); db.pragma('cache_size = 5000'); - db.pragma('busy_timeout = 5000'); + db.pragma('busy_timeout = 10000'); validateExistingRahSchema(db); @@ -254,19 +254,21 @@ function getDb() { * Execute a query and return rows. */ function query(sql, params = []) { - const database = getDb(); - const stmt = database.prepare(sql); + return runWithBusyRetry(() => { + const database = getDb(); + const stmt = database.prepare(sql); - const sqlLower = sql.trim().toLowerCase(); - if (sqlLower.startsWith('select') || sqlLower.startsWith('with') || sqlLower.startsWith('pragma') || sqlLower.includes('returning')) { - return params.length > 0 ? stmt.all(...params) : stmt.all(); - } else { - const result = params.length > 0 ? stmt.run(...params) : stmt.run(); - return { - changes: result.changes, - lastInsertRowid: Number(result.lastInsertRowid) - }; - } + const sqlLower = sql.trim().toLowerCase(); + if (sqlLower.startsWith('select') || sqlLower.startsWith('with') || sqlLower.startsWith('pragma') || sqlLower.includes('returning')) { + return params.length > 0 ? stmt.all(...params) : stmt.all(); + } else { + const result = params.length > 0 ? stmt.run(...params) : stmt.run(); + return { + changes: result.changes, + lastInsertRowid: Number(result.lastInsertRowid) + }; + } + }, 'query'); } /** @@ -275,7 +277,38 @@ function query(sql, params = []) { function transaction(callback) { const database = getDb(); const txn = database.transaction(callback); - return txn(); + return runWithBusyRetry(() => txn(), 'transaction'); +} + +function isBusyOrLocked(error) { + const message = error instanceof Error ? error.message : String(error); + const code = error && error.code ? String(error.code) : ''; + return code === 'SQLITE_BUSY' || code === 'SQLITE_LOCKED' || /database is locked|database busy/i.test(message); +} + +function sleepSync(ms) { + const buffer = new SharedArrayBuffer(4); + const view = new Int32Array(buffer); + Atomics.wait(view, 0, 0, ms); +} + +function runWithBusyRetry(operation, label) { + const maxAttempts = 4; + let lastError; + for (let attempt = 1; attempt <= maxAttempts; attempt += 1) { + try { + return operation(); + } catch (error) { + if (!isBusyOrLocked(error) || attempt === maxAttempts) { + throw error; + } + lastError = error; + const delayMs = 100 * attempt * attempt; + console.warn(`[RA-H MCP] SQLite ${label} hit ${error instanceof Error ? error.message : String(error)}; retrying in ${delayMs}ms (${attempt}/${maxAttempts})`); + sleepSync(delayMs); + } + } + throw lastError; } /** @@ -293,6 +326,7 @@ module.exports = { getDb, query, transaction, + runWithBusyRetry, closeDatabase, getDatabasePath }; diff --git a/docs/8_mcp.md b/docs/8_mcp.md index 7410242..c8cb176 100644 --- a/docs/8_mcp.md +++ b/docs/8_mcp.md @@ -46,6 +46,15 @@ Important runtime distinction: - standalone MCP can read and write nodes/edges without the app running, but it does not own chunking or embedding - if standalone MCP writes `nodes.source` while the app is closed, the app later processes that node through startup recovery +## WAL / Multi-Surface Safety + +- RA-H uses SQLite WAL as the normal local multi-surface model. +- The app, standalone MCP, Claude/Codex, and normal reads can coexist. +- Normal writes should be short; lock collisions wait through SQLite `busy_timeout` plus retry/backoff where supported. +- Standalone MCP remains first-class and does not require the app to be open. +- Dangerous maintenance is different from normal reads/writes: restore, DB replacement, checkpoint/truncate, and deleting `rah.sqlite-wal` / `rah.sqlite-shm` require all app/dev/MCP DB owners to be closed first. +- Use `scripts/database/rah-db-owners.sh "$HOME/Library/Application Support/RA-H/db/rah.sqlite"` before destructive maintenance. + ## Core MCP Contract - `queryNodes` is the primary tool for direct node retrieval when the user is trying to find a specific existing node. diff --git a/scripts/database/rah-db-owners.sh b/scripts/database/rah-db-owners.sh new file mode 100755 index 0000000..8713540 --- /dev/null +++ b/scripts/database/rah-db-owners.sh @@ -0,0 +1,30 @@ +#!/usr/bin/env bash +set -euo pipefail + +DB_PATH="${1:-}" +if [ -z "$DB_PATH" ]; then + echo "Usage: $0 " >&2 + exit 2 +fi + +if ! command -v lsof >/dev/null 2>&1; then + echo "lsof is required to inspect live RA-H database owners." >&2 + exit 3 +fi + +paths=("$DB_PATH" "${DB_PATH}-wal" "${DB_PATH}-shm") +existing=() +for candidate in "${paths[@]}"; do + if [ -e "$candidate" ]; then + existing+=("$candidate") + fi +done + +if [ "${#existing[@]}" -eq 0 ]; then + exit 0 +fi + +lsof -FnPc -- "${existing[@]}" 2>/dev/null | awk ' + /^p/ { pid=substr($0, 2); next } + /^c/ { cmd=substr($0, 2); if (pid != "") print pid "\t" cmd } +' | sort -u diff --git a/scripts/database/sqlite-backup.sh b/scripts/database/sqlite-backup.sh index 4214533..c72e450 100644 --- a/scripts/database/sqlite-backup.sh +++ b/scripts/database/sqlite-backup.sh @@ -26,6 +26,13 @@ fi echo "Resolved DB path: $DB_PATH" +OWNERS="$(bash "$(dirname "$0")/rah-db-owners.sh" "$DB_PATH" || true)" +if [ -n "$OWNERS" ]; then + echo "⚠️ Live RA-H DB owners detected while creating a VACUUM INTO backup:" + echo "$OWNERS" | sed 's/^/ /' + echo " Continuing because this backup path does not replace/delete DB, WAL, or SHM files." +fi + BACKUP_DIR="$(dirname "$0")/../backups" mkdir -p "$BACKUP_DIR" diff --git a/scripts/database/sqlite-restore.sh b/scripts/database/sqlite-restore.sh index bdd0427..f980c83 100644 --- a/scripts/database/sqlite-restore.sh +++ b/scripts/database/sqlite-restore.sh @@ -32,6 +32,14 @@ mkdir -p "$DB_DIR" echo "Source backup: $SRC" echo "Target DB: $DB_PATH" +OWNERS="$(bash "$(dirname "$0")/rah-db-owners.sh" "$DB_PATH" || true)" +if [ -n "$OWNERS" ] && [ "${RAH_ALLOW_LIVE_DB_RESTORE:-}" != "true" ]; then + echo "❌ Refusing restore while RA-H DB owners are live:" >&2 + echo "$OWNERS" | sed 's/^/ /' >&2 + echo "Close RA-H/MCP/dev processes first, or set RAH_ALLOW_LIVE_DB_RESTORE=true if you have deliberately quiesced them." >&2 + exit 4 +fi + echo "Verifying backup integrity before restore..." ICHECK=$(sqlite3 "$SRC" "PRAGMA integrity_check;") echo " $ICHECK" diff --git a/scripts/dev/guard-live-db-dev.sh b/scripts/dev/guard-live-db-dev.sh new file mode 100755 index 0000000..a2ff1fc --- /dev/null +++ b/scripts/dev/guard-live-db-dev.sh @@ -0,0 +1,37 @@ +#!/usr/bin/env bash +set -euo pipefail + +ROOT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" +DEFAULT_DB="$HOME/Library/Application Support/RA-H/db/rah.sqlite" + +DB_PATH="${SQLITE_DB_PATH:-}" +if [ -z "$DB_PATH" ] && [ -f "${ROOT_DIR}/.env.local" ]; then + DB_PATH=$(grep -E '^SQLITE_DB_PATH=' "${ROOT_DIR}/.env.local" | sed 's/^SQLITE_DB_PATH=//' | sed 's/^"\(.*\)"$/\1/') || true +fi +if [ -z "$DB_PATH" ]; then + DB_PATH="$DEFAULT_DB" +fi + +if [ "$DB_PATH" != "$DEFAULT_DB" ]; then + exit 0 +fi + +echo "⚠️ dev:mac is using the packaged live RA-H DB:" +echo " $DB_PATH" +echo " Prefer SQLITE_DB_PATH= for isolated development when possible." + +owners="$("${ROOT_DIR}/scripts/database/rah-db-owners.sh" "$DB_PATH" || true)" +if [ -z "$owners" ]; then + exit 0 +fi + +if [ "${RAH_ALLOW_LIVE_DB_DEV:-}" = "true" ]; then + echo "RAH_ALLOW_LIVE_DB_DEV=true set; continuing despite live DB owners:" + echo "$owners" | sed 's/^/ /' + exit 0 +fi + +echo "❌ Refusing to start dev server against the live DB while these processes have it open:" +echo "$owners" | sed 's/^/ /' +echo "Close RA-H/MCP owners first, set SQLITE_DB_PATH to a dev DB, or override with RAH_ALLOW_LIVE_DB_DEV=true." +exit 1 diff --git a/scripts/dev/run-tauri-dev.sh b/scripts/dev/run-tauri-dev.sh index 68f58f7..f9bb88b 100755 --- a/scripts/dev/run-tauri-dev.sh +++ b/scripts/dev/run-tauri-dev.sh @@ -9,6 +9,8 @@ NEXT_LOG="${REPO_DIR}/logs/next-dev.log" mkdir -p "${REPO_DIR}/logs" +"${REPO_DIR}/scripts/dev/guard-live-db-dev.sh" + cleanup() { if [[ -n "${NEXT_PID:-}" ]]; then echo "Shutting down Next.js dev server (pid ${NEXT_PID})" diff --git a/src/services/database/sqlite-client.ts b/src/services/database/sqlite-client.ts index 80c9a0f..0026c7b 100644 --- a/src/services/database/sqlite-client.ts +++ b/src/services/database/sqlite-client.ts @@ -47,11 +47,13 @@ class SQLiteClient { private db: Database.Database; private config: SQLiteConfig; private readonly readOnly: boolean; + private readonly maintenanceMode: boolean; private integrityReport: DatabaseIntegrityReport | null = null; private constructor() { this.config = this.getSQLiteConfig(); this.readOnly = process.env.SQLITE_READONLY === 'true'; + this.maintenanceMode = process.env.RAH_DB_MAINTENANCE === 'true'; // Initialize database connection const dbDirectory = path.dirname(this.config.dbPath); @@ -84,11 +86,11 @@ class SQLiteClient { this.db.pragma('synchronous = NORMAL'); this.db.pragma('cache_size = 10000'); this.db.pragma('temp_store = memory'); - this.db.pragma('busy_timeout = 5000'); + this.db.pragma('busy_timeout = 10000'); this.withStartupWriteLock(() => { this.ensureCoreSchema(); - // Ensure vector virtual tables are present and healthy + // Ensure vector virtual tables are present. Repair/rebuild is maintenance-only. this.ensureVectorTables(); this.healVectorTablesIfCorrupt(); @@ -139,26 +141,28 @@ class SQLiteClient { params?: any[] ): SQLiteQueryResult { try { - const sqlLower = sql.trim().toLowerCase(); - - // Handle different query types - if (sqlLower.startsWith('select') || - sqlLower.startsWith('with') || - sqlLower.includes('returning')) { - // SELECT queries and queries with RETURNING clause - const stmt = this.db.prepare(sql); - const rows = params ? stmt.all(...params) : stmt.all(); - return { rows: rows as T[] }; - } else { - // INSERT/UPDATE/DELETE queries without RETURNING - const stmt = this.db.prepare(sql); - const result = params ? stmt.run(...params) : stmt.run(); - return { - rows: [], - changes: result.changes, - lastInsertRowid: Number(result.lastInsertRowid) - }; - } + return this.runWithBusyRetry(() => { + const sqlLower = sql.trim().toLowerCase(); + + // Handle different query types + if (sqlLower.startsWith('select') || + sqlLower.startsWith('with') || + sqlLower.includes('returning')) { + // SELECT queries and queries with RETURNING clause + const stmt = this.db.prepare(sql); + const rows = params ? stmt.all(...params) : stmt.all(); + return { rows: rows as T[] }; + } else { + // INSERT/UPDATE/DELETE queries without RETURNING + const stmt = this.db.prepare(sql); + const result = params ? stmt.run(...params) : stmt.run(); + return { + rows: [], + changes: result.changes, + lastInsertRowid: Number(result.lastInsertRowid) + }; + } + }, 'query'); } catch (error) { this.refreshIntegrityReportForCorruptionError(error); console.error('SQLite query error:', error); @@ -178,17 +182,55 @@ class SQLiteClient { details: 'Transactions are not allowed in read-only mode' } as DatabaseError; } - // Proactively validate/repair vec vtables before any write transaction + // Proactively validate vec vtables before any write transaction. Destructive + // repair/rebuild is only allowed in explicit DB maintenance mode. this.healVectorTablesIfCorrupt(); const txn = this.db.transaction(callback); try { - return txn(); + return this.runWithBusyRetry(() => txn(), 'transaction'); } catch (error) { this.refreshIntegrityReportForCorruptionError(error); throw this.handleError(error); } } + private isBusyOrLocked(error: unknown): boolean { + const message = error instanceof Error ? error.message : String(error); + const code = + typeof error === 'object' && + error !== null && + 'code' in error && + typeof (error as { code?: unknown }).code === 'string' + ? String((error as { code: string }).code) + : ''; + return code === 'SQLITE_BUSY' || code === 'SQLITE_LOCKED' || /database is locked|database busy/i.test(message); + } + + private sleepSync(ms: number): void { + const buffer = new SharedArrayBuffer(4); + const view = new Int32Array(buffer); + Atomics.wait(view, 0, 0, ms); + } + + private runWithBusyRetry(operation: () => T, label: string): T { + const maxAttempts = 4; + let lastError: unknown; + for (let attempt = 1; attempt <= maxAttempts; attempt += 1) { + try { + return operation(); + } catch (error) { + if (!this.isBusyOrLocked(error) || attempt === maxAttempts) { + throw error; + } + lastError = error; + const delayMs = 100 * attempt * attempt; + console.warn(`[SQLite] ${label} hit ${error instanceof Error ? error.message : String(error)}; retrying in ${delayMs}ms (${attempt}/${maxAttempts})`); + this.sleepSync(delayMs); + } + } + throw lastError; + } + public async testConnection(): Promise { try { const result = this.query('SELECT datetime() as current_time'); @@ -908,6 +950,12 @@ class SQLiteClient { const msg = String(e?.message || ''); const code = (e && e.code) ? String(e.code) : ''; if (code === 'SQLITE_CORRUPT_VTAB' || msg.includes('database disk image is malformed') || msg.includes('CORRUPT_VTAB')) { + if (!this.maintenanceMode) { + console.warn( + `Detected corrupted virtual table ${table}, but repair is disabled outside RAH_DB_MAINTENANCE=true. Use explicit maintenance/rebuild flow.` + ); + return; + } console.warn(`Detected corrupted virtual table ${table} (${code || 'error'}). Recreating...`); try { this.db.exec(`DROP TABLE IF EXISTS ${table};`);