fix(src): close KB watcher start/start TOCTOU race

start_kb_watcher_impl previously performed the lifecycle steps in this
order:

  1. KbWatcher::new(&path)
  2. watcher.start()        (spawns notify threads, starts watching)
  3. KB_WATCHER.lock()
  4. *guard = Some(watcher)

Between 2 and 3 the watcher is already running on the filesystem but
unreferenced by the global slot. Two concurrent start_kb_watcher
invocations — e.g., a settings "Start" click that double-fires, or a
frontend that retries after a timeout while the first call is still in
flight — could each pass their own check-then-act without observing
each other, and both reach step 4 with their own watcher instance. The
second guard assignment would overwrite the first, but the first
watcher's notify threads keep running on the filesystem as an orphan
with no handle for stop_kb_watcher to reach.

Fix: hold the KB_WATCHER StdMutex guard across the full
check/create/start/store sequence. A racing second starter now observes
the populated slot and returns Ok(false) without ever constructing a
second KbWatcher. KbWatcher::new and watcher.start() are synchronous,
so holding the guard across them does not block other async tasks on
the runtime; the tokio::spawn event-forwarder is moved outside the
critical section so receiving events never contends with KB commands.

Added a SAFETY-style explanatory comment inline; no API or behavior
change for callers other than the correct race resolution.

cargo check --all-targets clean; cargo test --lib passes 311/312 (one
pre-existing #[ignore]'d model-download test).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: saagar210

src-tauri/src/commands/kb_commands.rs

@@ -260,17 +260,32 @@ pub(crate) async fn start_kb_watcher_impl(
 
     let validated_path = validate_stored_kb_path(&folder_path)?;
 
-    // Create and start watcher
-    let mut watcher = KbWatcher::new(&validated_path).map_err(|e| e.to_string())?;
-    let mut rx = watcher.start().map_err(|e| e.to_string())?;
-
-    // Store watcher instance
-    {
+    // Acquire the global watcher slot up-front, then create and start the
+    // watcher *inside* the critical section. This closes a TOCTOU race
+    // where two concurrent start_kb_watcher_impl calls could each
+    // successfully construct + start their own KbWatcher instance before
+    // either reached the KB_WATCHER.lock() — one of the instances would
+    // then be overwritten in the slot while still running on the
+    // filesystem as an orphan notify thread. With the lock held for the
+    // full init sequence, a racing second starter observes the populated
+    // slot and returns Ok(false) without ever creating its own watcher.
+    //
+    // KbWatcher::new and watcher.start() are synchronous — no .await is
+    // performed while the StdMutex guard is held, so this is safe under
+    // tokio's work-stealing scheduler.
+    let mut rx = {
         let mut guard = KB_WATCHER.lock().map_err(|e| e.to_string())?;
+        if guard.is_some() {
+            return Ok(false);
+        }
+        let mut watcher = KbWatcher::new(&validated_path).map_err(|e| e.to_string())?;
+        let rx = watcher.start().map_err(|e| e.to_string())?;
         *guard = Some(watcher);
-    }
+        rx
+    };
 
-    // Spawn event handler
+    // Spawn event handler outside the lock so receiving doesn't starve
+    // other KB commands.
     let window_clone = window.clone();
     tokio::spawn(async move {
         while let Some(event) = rx.recv().await {