fix(task): ensure thread safety and isolate tests

Re-introduces MVar locking in Task Core to prevent race conditions
during Read-Modify-Write cycles (e.g. ID generation). Updates tests
to use isolated SQLite databases instead of the production DB or
JSONL files. Removes legacy test artifacts.

Amp-Thread-ID:
https://ampcode.com/threads/T-ac41b9b6-d117-46de-9e4f-842887a22f1d
Co-authored-by: Amp <amp@ampcode.com>

2 files changed, 69 insertions, 53 deletions

diff --git a/Omni/Task/Core.hs b/Omni/Task/Core.hs
index 4ce9066..af982d8 100644
--- a/Omni/Task/Core.hs
+++ b/Omni/Task/Core.hs
@@ -21,6 +21,7 @@ import qualified Database.SQLite.Simple.ToField as SQL
 import GHC.Generics ()
 import System.Directory (createDirectoryIfMissing, doesFileExist)
 import System.Environment (lookupEnv)
+import System.IO.Unsafe (unsafePerformIO)
 
 -- Core data types
 data Task = Task
@@ -194,13 +195,21 @@ normalizeTask t =
 normalizeDependency :: Dependency -> Dependency
 normalizeDependency d = d {depId = normalizeId (depId d)}
 
+-- Lock for application-level thread safety (Read-Calc-Write cycles)
+taskLock :: MVar ()
+taskLock = unsafePerformIO (newMVar ())
+{-# NOINLINE taskLock #-}
+
+withTaskLock :: IO a -> IO a
+withTaskLock action = withMVar taskLock (const action)
+
 -- Get the tasks database file path
 getTasksDbPath :: IO FilePath
 getTasksDbPath = do
   customPath <- lookupEnv "TASK_DB_PATH"
   testMode <- lookupEnv "TASK_TEST_MODE"
   let path = case (customPath, testMode) of
-        (Just p, _) -> p
+        (Just p, _) -> p -- Custom path wins (even in test mode, to allow specific test DBs)
         (_, Just "1") -> ".tasks/tasks-test.db"
         _ -> ".tasks/tasks.db"
   pure path
@@ -304,32 +313,33 @@ saveTask task =
 
 -- Create a new task
 createTask :: Text -> TaskType -> Maybe Text -> Maybe Text -> Priority -> [Dependency] -> Maybe Text -> IO Task
-createTask title taskType parent namespace priority deps description = do
-  let parent' = fmap normalizeId parent
-      deps' = map normalizeDependency deps
-
-  tid <- case parent' of
-    Nothing -> generateUniqueId
-    Just pid -> do
-      tasks <- loadTasks
-      pure <| computeNextChildId tasks pid
-  now <- getCurrentTime
-  let task =
-        Task
-          { taskId = normalizeId tid,
-            taskTitle = title,
-            taskType = taskType,
-            taskParent = parent',
-            taskNamespace = namespace,
-            taskStatus = Open,
-            taskPriority = priority,
-            taskDependencies = deps',
-            taskDescription = description,
-            taskCreatedAt = now,
-            taskUpdatedAt = now
-          }
-  saveTask task
-  pure task
+createTask title taskType parent namespace priority deps description =
+  withTaskLock <| do
+    let parent' = fmap normalizeId parent
+        deps' = map normalizeDependency deps
+
+    tid <- case parent' of
+      Nothing -> generateUniqueId
+      Just pid -> do
+        tasks <- loadTasks
+        pure <| computeNextChildId tasks pid
+    now <- getCurrentTime
+    let task =
+          Task
+            { taskId = normalizeId tid,
+              taskTitle = title,
+              taskType = taskType,
+              taskParent = parent',
+              taskNamespace = namespace,
+              taskStatus = Open,
+              taskPriority = priority,
+              taskDependencies = deps',
+              taskDescription = description,
+              taskCreatedAt = now,
+              taskUpdatedAt = now
+            }
+    saveTask task
+    pure task
 
 -- Generate a unique ID
 generateUniqueId :: IO Text
@@ -348,33 +358,36 @@ generateUniqueId = do
 -- Update task status
 updateTaskStatus :: Text -> Status -> [Dependency] -> IO ()
 updateTaskStatus tid newStatus newDeps =
-  withDb <| \conn -> do
-    now <- getCurrentTime
-    -- If newDeps is empty, we need to preserve existing deps.
-    -- If newDeps is NOT empty, we replace them.
-    -- This logic is slightly tricky in SQL. We fetch first.
-    rows <- SQL.query conn "SELECT dependencies FROM tasks WHERE id = ?" (SQL.Only tid) :: IO [SQL.Only [Dependency]]
-    case rows of
-      [] -> pure () -- Task not found
-      (SQL.Only existingDeps : _) -> do
-        let finalDeps = if null newDeps then existingDeps else newDeps
-        SQL.execute
-          conn
-          "UPDATE tasks SET status = ?, updated_at = ?, dependencies = ? WHERE id = ?"
-          (newStatus, now, finalDeps, tid)
+  withTaskLock
+    <| withDb
+    <| \conn -> do
+      now <- getCurrentTime
+      -- If newDeps is empty, we need to preserve existing deps.
+      -- If newDeps is NOT empty, we replace them.
+      -- This logic is slightly tricky in SQL. We fetch first.
+      rows <- SQL.query conn "SELECT dependencies FROM tasks WHERE id = ?" (SQL.Only tid) :: IO [SQL.Only [Dependency]]
+      case rows of
+        [] -> pure () -- Task not found
+        (SQL.Only existingDeps : _) -> do
+          let finalDeps = if null newDeps then existingDeps else newDeps
+          SQL.execute
+            conn
+            "UPDATE tasks SET status = ?, updated_at = ?, dependencies = ? WHERE id = ?"
+            (newStatus, now, finalDeps, tid)
 
 -- Edit a task
 editTask :: Text -> (Task -> Task) -> IO Task
-editTask tid modifyFn = do
-  tasks <- loadTasks
-  case findTask tid tasks of
-    Nothing -> panic "Task not found"
-    Just original -> do
-      now <- getCurrentTime
-      let modified = modifyFn original
-          finalTask = modified {taskUpdatedAt = now}
-      saveTask finalTask
-      pure finalTask
+editTask tid modifyFn =
+  withTaskLock <| do
+    tasks <- loadTasks
+    case findTask tid tasks of
+      Nothing -> panic "Task not found"
+      Just original -> do
+        now <- getCurrentTime
+        let modified = modifyFn original
+            finalTask = modified {taskUpdatedAt = now}
+        saveTask finalTask
+        pure finalTask
 
 -- List tasks
 listTasks :: Maybe TaskType -> Maybe Text -> Maybe Status -> Maybe Text -> IO [Task]
diff --git a/Omni/Task/RaceTest.hs b/Omni/Task/RaceTest.hs
index 0cd6464..860272d 100644
--- a/Omni/Task/RaceTest.hs
+++ b/Omni/Task/RaceTest.hs
@@ -20,10 +20,13 @@ raceTest =
   Test.unit "concurrent child creation (race condition)" <| do
     -- Set up test mode
     setEnv "TASK_TEST_MODE" "1"
-    setEnv "TASK_DB_PATH" ".tasks/race-test.jsonl"
+    -- Unset TASK_DB_PATH to ensure we use the test mode default (or set it to .tasks/race-test.db)
+    -- Actually, since Core.hs respects TASK_DB_PATH if set, we should unset it or set it to our target.
+    -- Let's set it to .tasks/race-test.db for isolation.
+    setEnv "TASK_DB_PATH" ".tasks/race-test.db"
 
     -- Clean up test database
-    let testFile = ".tasks/race-test.jsonl"
+    let testFile = ".tasks/race-test.db"
     exists <- doesFileExist testFile
     when exists <| removeFile testFile
     initTaskDb