From 0a4d1d5f9598ca9375d6265d8f33432b7d7f2fd4 Mon Sep 17 00:00:00 2001
From: Josh Knapp <jknapp85@gmail.com>
Date: Sun, 27 Sep 2026 13:22:43 -0700
Subject: [PATCH] Projects: restore store-owned fields under the store lock on
 save (PR re-review)

update_project restored marketplace installs (and status, container id,
flags) from a copy read before validation, then wrote the whole record
later, so an install landing in between was lost. ProjectsStore gains
update_restoring, which runs restore_store_owned_fields against the record
as stored under the lock, and update_project writes through it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---
 .../src/commands/project_commands.rs          |  9 ++-
 app/src-tauri/src/storage/projects_store.rs   | 57 +++++++++++++++++++
 2 files changed, 65 insertions(+), 1 deletion(-)

diff --git a/app/src-tauri/src/commands/project_commands.rs b/app/src-tauri/src/commands/project_commands.rs
index 74c1ca6..a52a17b 100644
--- a/app/src-tauri/src/commands/project_commands.rs
+++ b/app/src-tauri/src/commands/project_commands.rs
@@ -1112,7 +1112,14 @@ pub async fn update_project(
     // for every already-running container at launch. The version of this that
     // re-asserted on every save is what turned a stale flag in a payload into a
     // restarted bridge.
-    state.projects_store.update(project)
+    //
+    // The restore above served the validation; it is redone under the store's
+    // lock against the record as it is *now*, so a marketplace install, a
+    // status change or a container id landing since `stored` was read is kept
+    // rather than written over.
+    state
+        .projects_store
+        .update_restoring(project, restore_store_owned_fields)
 }
 
 /// Restore onto `project` the fields whose value belongs to the store rather
diff --git a/app/src-tauri/src/storage/projects_store.rs b/app/src-tauri/src/storage/projects_store.rs
index 7c09b23..6a261c5 100644
--- a/app/src-tauri/src/storage/projects_store.rs
+++ b/app/src-tauri/src/storage/projects_store.rs
@@ -205,6 +205,27 @@ impl ProjectsStore {
         }
     }
 
+    /// Replace a project with `updated`, after `restore` has copied onto it
+    /// the fields the store owns from the record *as stored under the lock*.
+    /// `update_project` restores from a copy it read earlier, so an install
+    /// or a status change landing in between would otherwise be written over
+    /// (re-review round 2).
+    pub fn update_restoring(
+        &self,
+        mut updated: Project,
+        restore: impl FnOnce(&mut Project, &Project),
+    ) -> Result<Project, String> {
+        let mut projects = self.lock();
+        let p = projects
+            .iter_mut()
+            .find(|p| p.id == updated.id)
+            .ok_or_else(|| format!("Project {} not found", updated.id))?;
+        restore(&mut updated, p);
+        *p = updated.clone();
+        self.save(&projects)?;
+        Ok(updated)
+    }
+
     pub fn remove(&self, id: &str) -> Result<(), String> {
         let mut projects = self.lock();
         let initial_len = projects.len();
@@ -520,6 +541,42 @@ mod tests {
         fs::remove_dir_all(&dir).ok();
     }
 
+    #[test]
+    fn a_project_save_keeps_a_marketplace_install_made_after_it_read_the_record() {
+        // Re-review round 2: `update_project` read the stored record, then
+        // wrote the whole payload back later. An install landing in between
+        // was lost. The restore now runs against the record under the lock.
+        let dir = temp_dir("save-restore");
+        let project = Project::new("demo".to_string(), Vec::new());
+        let id = project.id.clone();
+        let store = store_over(&dir, vec![project]);
+
+        let mut payload = store.get(&id).unwrap(); // the Config tab's copy
+        payload.name = "renamed".to_string();
+        store
+            .update_marketplace_fields(&id, |installs, _| {
+                installs.push(market_install("late"));
+                Ok(())
+            })
+            .unwrap();
+
+        let saved = store
+            .update_restoring(payload, |incoming, stored| {
+                incoming.marketplace_installs = stored.marketplace_installs.clone();
+                incoming.marketplace_disabled = stored.marketplace_disabled.clone();
+            })
+            .unwrap();
+        assert_eq!(saved.name, "renamed");
+        assert_eq!(saved.marketplace_installs, vec![market_install("late")]);
+        assert_eq!(store.get(&id).unwrap().marketplace_installs, vec![market_install("late")]);
+
+        let mut ghost = Project::new("ghost".to_string(), Vec::new());
+        ghost.id = "nope".into();
+        assert!(store.update_restoring(ghost, |_, _| {}).is_err());
+
+        fs::remove_dir_all(&dir).ok();
+    }
+
     #[test]
     fn marketplace_edits_across_all_projects_touch_only_those_fields() {
         let dir = temp_dir("marketplace-all");