bugfix: memory migration crash

Author: sysread

lib/store/project/conversation/task_list_status_migration.ex

@@ -1,20 +1,26 @@
 defmodule Store.Project.Conversation.TaskListStatusMigration do
   @moduledoc """
-  Helpers to heal and migrate conversation task-list statuses and shapes.
+  Helpers to best-effort heal and migrate conversation task-list statuses and shapes.
 
   These functions are intentionally minimal and safe: they only alter the
   `tasks` map of the decoded conversation JSON if legacy shapes are detected
   (for example the value is a bare list instead of a map), or if a `status`
-  field is missing or non-canonical. The migration is performed in-place via
-  an atomic write that preserves the original timestamp prefix used by the
-  Store.Project.Conversation format.
+  field is missing or non-canonical. Repairs are applied in-memory, and the
+  module attempts an atomic write of the repaired JSON back to disk, but if
+  persistence fails, processing continues without raising. The timestamp prefix
+  used by the Store.Project.Conversation format is preserved.
   """
 
   @doc """
   Inspect the decoded JSON map `original_json_map` for legacy task-list
-  shapes and/or non-canonical status values. If repairs are needed, atomically
-  write the repaired JSON back to disk using the provided `conversation` and
-  the original `timestamp_str` (which is the unix timestamp string prefix).
+  shapes and/or non-canonical status values. If repairs are needed, apply them
+  in-memory and attempt to atomically persist the repaired JSON to disk using
+  the provided `conversation` and the original `timestamp_str` (the unix
+  timestamp string prefix).
+
+  Persistence is best-effort: if the write fails (for example due to concurrent
+  access or filesystem issues), this function will warn and continue without
+  raising so callers can keep reading the conversation.
 
   This function is safe to call from Store.Project.Conversation.read/1; it is
   conservative and only writes when it detects a change.
@@ -56,7 +62,17 @@ defmodule Store.Project.Conversation.TaskListStatusMigration do
       end)
 
     if changed do
-      write_file_with_ts(conversation, timestamp_str, repaired)
+      case write_file_with_ts(conversation, timestamp_str, repaired) do
+        :ok ->
+          :ok
+
+        {:error, reason} ->
+          UI.warn(
+            "Could not persist healed tasks for conversation #{conversation.id}: #{inspect(reason)}"
+          )
+
+          :ok
+      end
     else
       :ok
     end
@@ -82,14 +98,41 @@ defmodule Store.Project.Conversation.TaskListStatusMigration do
     end
   end
 
+  @spec write_file_with_ts(Store.Project.Conversation.t(), String.t(), map()) ::
+          :ok | {:error, term()}
   defp write_file_with_ts(conversation, timestamp_str, data_map) do
     conversation.project_home
     |> Path.join("conversations")
     |> File.mkdir_p()
 
-    json = SafeJson.encode!(data_map)
     tmp = conversation.store_path <> ".tmp"
-    File.write!(tmp, "#{timestamp_str}:" <> json)
-    File.rename!(tmp, conversation.store_path)
+
+    with {:ok, json} <- SafeJson.encode(data_map),
+         :ok <- File.write(tmp, "#{timestamp_str}:" <> json),
+         :ok <- safe_rename(tmp, conversation.store_path) do
+      :ok
+    else
+      {:error, reason} ->
+        File.rm(tmp)
+        {:error, reason}
+    end
+  end
+
+  @spec safe_rename(String.t(), String.t()) :: :ok | {:error, term()}
+  defp safe_rename(tmp, dest) do
+    case File.rename(tmp, dest) do
+      :ok ->
+        :ok
+
+      {:error, :enoent} ->
+        if File.exists?(tmp) do
+          {:error, :dest_missing}
+        else
+          {:error, :tmp_missing}
+        end
+
+      {:error, reason} ->
+        {:error, reason}
+    end
   end
 end

test/store/project/conversation_test.exs

@@ -163,6 +163,42 @@ defmodule Store.Project.ConversationTest do
     assert [%{id: "a1", data: "info", outcome: :done, result: "ok"}] = task_list
   end
 
+  test "read/1 does not crash when migration write fails", ctx do
+    # Prepare a legacy JSON file to trigger migration
+    id = "mig_fail"
+    convo = Conversation.new(id, ctx.project)
+    File.mkdir_p!(Path.dirname(convo.store_path))
+
+    legacy_tasks = [
+      %{"id" => "t1", "data" => "d1", "outcome" => "todo", "result" => nil}
+    ]
+
+    legacy_data = %{
+      "messages" => [],
+      "metadata" => %{},
+      "memory" => [],
+      "tasks" => %{"123" => legacy_tasks}
+    }
+
+    timestamp = 99
+    File.write!(convo.store_path, "#{timestamp}:" <> SafeJson.encode!(legacy_data))
+
+    # Create a directory at .tmp to cause migration write to fail
+    tmp_path = convo.store_path <> ".tmp"
+    File.mkdir_p!(tmp_path)
+
+    before_contents = File.read!(convo.store_path)
+    # Ensure read/1 still succeeds
+    assert {:ok, %{tasks: tasks_map}} = Conversation.read(convo)
+
+    assert Map.has_key?(tasks_map, "123")
+    assert %{tasks: tasks_list, description: nil} = tasks_map["123"]
+    assert [%{id: "t1", data: "d1", outcome: :todo, result: nil}] = tasks_list
+
+    # File contents remain unchanged
+    assert File.read!(convo.store_path) == before_contents
+  end
+
   test "list/1 returns conversations in descending timestamp order", ctx do
     id1 = "one"
     id2 = "two"