Switch SurrealDB from In-Memory to File-Based RocksDB Storage

CODERABBIT_FIXES.md

@@ -0,0 +1,70 @@
+# CodeRabbit PR #652 Fix Tasks
+
+## Critical Issues
+
+### 1. perspective_instance.rs (lines 2425-2457)
+**Issue**: `persist_link_diff` and `retry_surreal_op` silently swallow errors, risking desync.
+**Fix**: Make both return `Result` and propagate failures to callers.
+
+```rust
+// Change retry_surreal_op signature to return Result
+async fn retry_surreal_op<F, Fut>(
+    op: F,
+    uuid: &str,
+    op_name: &str,
+) -> Result<(), anyhow::Error>
+
+// Change persist_link_diff signature to return Result
+pub(crate) async fn persist_link_diff(
+    &self,
+    diff: &DecoratedPerspectiveDiff,
+) -> Result<(), AnyError>
+
+// Update callsites:
+// - Functions returning Result: change `.await;` to `.await?;`
+// - diff_from_link_language: use `.await.expect("SurrealDB link persistence failed");`
+```
+
+### 2. MIGRATION_REMOVAL_GUIDE.md (line 112-113)
+**Issue**: Incomplete sentence in heading.
+**Fix**: Change "Can optionally be cleaned up:" to "The following can optionally be cleaned up:"
+
+### 3. migration.rs (lines 97-119)
+**Issue**: Marks perspective as migrated even when some links failed, risking data loss.
+**Fix**: Only mark as migrated when `error_count == 0`, otherwise return error.
+
+### 4. mod.rs (lines 441-457)
+**Issue**: Hardcodes `LinkStatus::Local` instead of preserving original status.
+**Fix**: Read `link.status` and use it (fallback to `LinkStatus::Local` if None).
+
+### 5. mod.rs (lines 56-66)
+**Issue**: TOCTOU race in `initialize_from_db` - multiple tasks can create duplicate SurrealDB services.
+**Fix**: Use atomic entry-or-init pattern with write lock to prevent duplicates.
+
+### 6. perspective_instance.rs (lines 878-891)
+**Issue**: SurrealDB lookup only uses (source, target, predicate) instead of full unique constraint.
+**Fix**: Include author and timestamp in lookups to honor all 5 fields of link_unique_idx.
+
+### 7. surreal_service/mod.rs (lines 532-576)
+**Issue**: Overly broad error handling treats any "index" error as benign.
+**Fix**: Only check for "unique", "duplicate", "already exists" - remove "index" check.
+
+## Nitpicks
+
+### 8. mod.rs (lines 80-93)
+**Note**: Migration failures logged at error level but user won't see in UI.
+Consider adding visible notification mechanism for production.
+
+### 9. surreal_service/mod.rs (lines 756-904)
+**Note**: DRY the repeated SurrealLink deserialization pattern across getters.
+Consider extracting helper function.
+
+---
+
+## Execution Plan
+
+1. Fix critical issues 1-7 first
+2. Address nitpicks 8-9 if time permits
+3. Run cargo fmt
+4. Commit and push to `surreal-files` branch
+5. Wait for CodeRabbit re-analysis

PR_652_FIXES_SUMMARY.md

@@ -0,0 +1,57 @@
+# PR #652 CodeRabbit Fixes - Summary
+
+## All 7 Actionable Issues Fixed ✅
+
+### Issue #1: Error Propagation (perspective_instance.rs lines 2425-2457)
+**Problem**: `persist_link_diff` and `retry_surreal_op` silently swallowed errors
+**Fix**: 
+- Changed both functions to return `Result<(), AnyError>`
+- Updated all callsites to propagate errors using `?` or `.expect()`
+- Functions returning `()` use `.expect()` to fail-fast
+- Functions returning `Result` use `?` to propagate
+
+### Issue #2: Documentation (MIGRATION_REMOVAL_GUIDE.md line 112-113)
+**Problem**: Incomplete sentence in heading
+**Fix**: Changed "Can optionally be cleaned up:" to "The following can optionally be cleaned up:"
+
+### Issue #3: Migration Safety (migration.rs lines 97-119)
+**Problem**: Marked perspective as migrated even when errors occurred
+**Fix**: Only mark as migrated when `error_count == 0`, otherwise return error
+
+### Issue #4: Link Status Preservation (mod.rs lines 441-457)
+**Problem**: Hardcoded `LinkStatus::Local` instead of preserving original
+**Fix**: Read `link.status` and use it (fallback to `LinkStatus::Local` if None)
+
+### Issue #5: TOCTOU Race (mod.rs lines 56-66)
+**Problem**: Race condition in `initialize_from_db` allowing duplicate SurrealDB services
+**Fix**: Added atomic check-and-insert after async initialization completes
+
+### Issue #6: Unique Constraint (perspective_instance.rs lines 878-891)
+**Problem**: SurrealDB lookups only used 3 fields instead of full unique constraint (5 fields)
+**Fix**: 
+- Updated `get_link` to accept optional `author` and `timestamp`
+- When provided, queries using all 5 fields: source, target, predicate, author, timestamp
+- All callsites updated to pass author and timestamp from LinkExpression
+
+### Issue #7: Error Handling (surreal_service/mod.rs lines 532-576)
+**Problem**: Overly broad error check treated any "index" error as benign
+**Fix**: Removed "index" check, only checks for "unique", "duplicate", and "already exists"
+
+## Additional Compilation Fixes
+- Fixed partial move error in `import_perspective` (clone link.status)
+- Fixed partial move error in migration loop (borrow &links instead of consuming)
+
+## Commits
+1. `818d756e` - Issues #2, #3, #7 + docstrings
+2. `6aecb225` - Issue #4 (preserve link status)  
+3. `8c65f6c7` - Issue #1 (error propagation)
+4. `a9e43aff` - Issue #6 (unique constraint)
+5. `fa3c1734` - Issue #5 (TOCTOU race)
+6. `c79a9244` - Fix partial move (link.status)
+7. `640b6e65` - Fix partial move (migration loop)
+
+## Status
+- ✅ All CodeRabbit actionable comments addressed
+- ✅ Additional docstrings added for coverage
+- 🔄 Building rust-executor to verify compilation
+- ⏳ Next: cargo fmt, run full test suite

rust-executor/Cargo.toml

@@ -134,7 +134,7 @@ anyhow = "1.0.95"
 portpicker = "0.1.1"
 deno_error = "0.5.6"
 thiserror = "2.0.12"
-surrealdb = { version = "2.4", default-features = false, features = ["kv-mem", "scripting"] }
+surrealdb = { version = "2.4", default-features = false, features = ["kv-rocksdb", "kv-mem", "scripting"] }
 
 [dev-dependencies]
 maplit = "1.0.2"

rust-executor/MIGRATION_REMOVAL_GUIDE.md

@@ -0,0 +1,113 @@
+# Migration Removal Guide
+
+This document explains how to remove the one-time Rusqlite-to-SurrealDB migration code after all users have migrated.
+
+## When to Remove
+
+Remove this migration code in a future version (e.g., version 0.13.0 or later) after confirming:
+- All active users have upgraded to version 0.11.2 or later
+- Sufficient time has passed for users to run the migration (recommend at least 2-3 releases)
+- No support requests related to missing links after migration
+
+## Removal Steps
+
+### 1. Delete the Migration Module
+```bash
+rm src/perspectives/migration.rs
+```
+
+### 2. Update src/perspectives/mod.rs
+
+Remove the migration module declaration:
+```rust
+// Remove this line:
+pub mod migration; // TODO: Remove this module after all users have migrated to SurrealDB
+```
+
+Remove migration calls from `initialize_from_db()`:
+```rust
+// Remove these lines (around line 74-78):
+// Migrate links from Rusqlite to SurrealDB (one-time migration)
+// TODO: Remove this migration call after all users have migrated
+if let Err(e) = migration::migrate_links_from_rusqlite_to_surrealdb(&handle_clone.uuid, &surreal_service).await {
+    log::error!("Failed to migrate links for perspective {}: {}", handle_clone.uuid, e);
+}
+```
+
+Remove migration calls from `add_perspective()`:
+```rust
+// Remove these lines (around line 119-123):
+// Migrate links from Rusqlite to SurrealDB (one-time migration)
+// TODO: Remove this migration call after all users have migrated
+if let Err(e) = migration::migrate_links_from_rusqlite_to_surrealdb(&handle.uuid, &surreal_service).await {
+    log::error!("Failed to migrate links for perspective {}: {}", handle.uuid, e);
+}
+```
+
+### 3. (Optional) Clean Up Database Migration Support
+
+If you want to completely remove migration infrastructure from src/db.rs:
+
+Remove the migration tracking table from `new()`:
+```rust
+// Remove these lines (around line 158-165):
+conn.execute(
+    "CREATE TABLE IF NOT EXISTS perspective_link_migration (
+        id INTEGER PRIMARY KEY,
+        perspective_uuid TEXT NOT NULL UNIQUE,
+        migrated_at TEXT NOT NULL
+     )",
+    [],
+)?;
+```
+
+Remove migration methods:
+```rust
+// Remove these methods:
+- pub fn is_perspective_migrated(&self, perspective_uuid: &str) -> Ad4mDbResult<bool>
+- pub fn mark_perspective_as_migrated(&self, perspective_uuid: &str) -> Ad4mDbResult<()>
+- pub fn delete_all_links_for_perspective(&self, perspective_uuid: &str) -> Ad4mDbResult<usize>
+```
+
+**Note:** Keeping these methods is harmless and may be useful for future migrations, so removal is optional.
+
+### 4. Verify Removal
+
+After removing the migration code:
+
+1. **Build the project:**
+   ```bash
+   cargo build --lib
+   ```
+
+2. **Run tests:**
+   ```bash
+   cargo test --lib
+   ```
+
+3. **Verify no references remain:**
+   ```bash
+   grep -r "migrate_links_from_rusqlite_to_surrealdb" rust-executor/src/
+   grep -r "migration::" rust-executor/src/perspectives/mod.rs
+   ```
+   Both should return no results.
+
+## Migration Summary
+
+### What the Migration Did
+- Moved all link data from the centralized Rusqlite database to per-perspective SurrealDB databases
+- Preserved all link metadata (author, timestamp, proof, status)
+- Ensured idempotent operation (safe to run multiple times)
+- Cleaned up old link data from Rusqlite after successful migration
+- Tracked migration status per perspective to prevent repeated migrations
+
+### Files Affected by Migration
+
+**Will be removed:**
+- `src/perspectives/migration.rs` - The migration logic and tests
+
+**Will be updated:**
+- `src/perspectives/mod.rs` - Remove module declaration and migration calls
+
+**The following can optionally be cleaned up:**
+- `src/db.rs` - Migration tracking methods (optional to remove)

rust-executor/src/db.rs

@@ -155,6 +155,15 @@ impl Ad4mDb {
             [],
         )?;
 
+        conn.execute(
+            "CREATE TABLE IF NOT EXISTS perspective_link_migration (
+                id INTEGER PRIMARY KEY,
+                perspective_uuid TEXT NOT NULL UNIQUE,
+                migrated_at TEXT NOT NULL
+             )",
+            [],
+        )?;
+
         conn.execute(
             "CREATE TABLE IF NOT EXISTS outbox (
                 id INTEGER PRIMARY KEY,
@@ -1470,6 +1479,54 @@ impl Ad4mDb {
         Ok(links?)
     }
 
+    /// Check if a perspective's links have been migrated from Rusqlite to SurrealDB
+    ///
+    /// # Arguments
+    /// * `perspective_uuid` - UUID of the perspective to check
+    ///
+    /// # Returns
+    /// * `Ok(true)` if the perspective has been migrated
+    /// * `Ok(false)` if the perspective has not been migrated yet
+    pub fn is_perspective_migrated(&self, perspective_uuid: &str) -> Ad4mDbResult<bool> {
+        let mut stmt = self.conn.prepare(
+            "SELECT COUNT(*) FROM perspective_link_migration WHERE perspective_uuid = ?1",
+        )?;
+        let count: i64 = stmt.query_row(params![perspective_uuid], |row| row.get(0))?;
+        Ok(count > 0)
+    }
+
+    /// Mark a perspective as having been migrated from Rusqlite to SurrealDB
+    ///
+    /// This function is idempotent - calling it multiple times for the same perspective is safe.
+    ///
+    /// # Arguments
+    /// * `perspective_uuid` - UUID of the perspective to mark as migrated
+    pub fn mark_perspective_as_migrated(&self, perspective_uuid: &str) -> Ad4mDbResult<()> {
+        let timestamp = chrono::Utc::now().to_rfc3339();
+        self.conn.execute(
+            "INSERT OR IGNORE INTO perspective_link_migration (perspective_uuid, migrated_at) VALUES (?1, ?2)",
+            params![perspective_uuid, timestamp],
+        )?;
+        Ok(())
+    }
+
+    /// Delete all links for a perspective from Rusqlite storage
+    ///
+    /// This should only be called after successfully migrating links to SurrealDB.
+    ///
+    /// # Arguments
+    /// * `perspective_uuid` - UUID of the perspective whose links should be deleted
+    ///
+    /// # Returns
+    /// The number of links that were deleted
+    pub fn delete_all_links_for_perspective(&self, perspective_uuid: &str) -> Ad4mDbResult<usize> {
+        let count = self.conn.execute(
+            "DELETE FROM link WHERE perspective = ?1",
+            params![perspective_uuid],
+        )?;
+        Ok(count)
+    }
+
     pub fn add_pending_diff(
         &self,
         perspective_uuid: &str,

rust-executor/src/lib.rs

@@ -174,6 +174,10 @@ pub async fn run(mut config: Ad4mConfig) -> JoinHandle<()> {
     info!("js_core initialized.");
 
     LanguageController::init_global_instance(js_core_handle.clone());
+
+    // Set app data path for perspectives module (needed for file-based SurrealDB)
+    perspectives::set_app_data_path(config.app_data_path.clone().unwrap());
+
     perspectives::initialize_from_db();
 
     let app_dir = config