Marketplace sync: keep installs the host could not build (final review M3)

An install the host skipped (pinned commit missing from the cache, cache
unreadable, item failing a tightened validation rule) never reached the
manifest, so sync.sh treated it as deselected and deleted it from the
container. The manifest now carries `held`: the state ids of such
installs ("plugin:<slug>/<key>" for plugins). The script counts them as
still selected and carries their records forward, as it already does
for items that fail inside the container. A removed marketplace is the
one skip that still removes; a malformed `held` list changes nothing.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

app/src-tauri/src/marketplace/payload.rs

@@ -100,6 +100,10 @@ pub fn build_payload(input: &PayloadInput) -> Result<Payload, String> {
     let mut tar = TarWriter::new();
     let mut items: Vec<Value> = Vec::new();
     let mut skipped: Vec<SkippedItem> = Vec::new();
+    // State ids (see sync.sh) of installs the host could not build this time:
+    // the container keeps what it has for them instead of treating them as
+    // deselected (final review M3). Only a removed source really removes.
+    let mut held: BTreeSet<String> = BTreeSet::new();
     let mut plugin_groups: BTreeMap<String, PluginGroup> = BTreeMap::new();
     // Non-plugin items share one namespace in ~/.claude; plugins are namespaced
     // by their per-marketplace catalog, so they never collide.
@@ -122,10 +126,22 @@ pub fn build_payload(input: &PayloadInput) -> Result<Payload, String> {
             skip("its marketplace has been removed".to_string());
             continue;
         };
-        if !is_valid_item_key(&inst.key) || !is_valid_commit(&inst.commit) {
+        let state_id = match inst.kind {
+            ItemKind::Plugin => format!("plugin:{}/{}", marketplace_slug(&m.id), inst.key),
+            _ => label.clone(),
+        };
+        let mut hold = |reason: String| {
+            held.insert(state_id.clone());
+            skip(reason)
+        };
+        if !is_valid_item_key(&inst.key) {
             skip("the saved install entry is invalid".to_string());
             continue;
         }
+        if !is_valid_commit(&inst.commit) {
+            hold("the saved install entry is invalid".to_string());
+            continue;
+        }
         if inst.kind != ItemKind::Plugin && taken.contains(&(inst.kind, inst.key.clone())) {
             skip(format!(
                 "another marketplace's {label} is already installed"
@@ -134,7 +150,7 @@ pub fn build_payload(input: &PayloadInput) -> Result<Payload, String> {
         }
         let repo = git::cache_path(input.data_root, &m.id);
         if !git::has_commit(&repo, &inst.commit) {
-            skip(format!(
+            hold(format!(
                 "pinned commit {} is not in the local cache of \"{}\" — refresh the marketplace",
                 &inst.commit[..8],
                 m.name
@@ -144,7 +160,7 @@ pub fn build_payload(input: &PayloadInput) -> Result<Payload, String> {
         let (tree, files) = match install_files(&repo, inst) {
             Ok(v) => v,
             Err(e) => {
-                skip(e);
+                hold(e);
                 continue;
             }
         };
@@ -164,7 +180,7 @@ pub fn build_payload(input: &PayloadInput) -> Result<Payload, String> {
                     "commands"
                 };
                 let Some(f) = files.first() else {
-                    skip("has no files".to_string());
+                    hold("has no files".to_string());
                     continue;
                 };
                 let path = format!("{dir}/{key}.md");
@@ -181,7 +197,7 @@ pub fn build_payload(input: &PayloadInput) -> Result<Payload, String> {
                     match rendered_hook_settings(&tree, key) {
                         Ok(settings) => item["settings"] = settings,
                         Err(e) => {
-                            skip(e);
+                            hold(e);
                             continue;
                         }
                     }
@@ -195,7 +211,7 @@ pub fn build_payload(input: &PayloadInput) -> Result<Payload, String> {
                 let mut entry = match plugin_catalog_entry(&tree, key) {
                     Ok(e) => e,
                     Err(e) => {
-                        skip(e);
+                        hold(e);
                         continue;
                     }
                 };
@@ -242,8 +258,12 @@ pub fn build_payload(input: &PayloadInput) -> Result<Payload, String> {
             .push(json!({ "slug": slug, "dir": format!("plugins/{slug}"), "plugins": group.keys }));
     }
 
-    let manifest =
-        json!({ "version": 1, "items": items, "plugin_marketplaces": plugin_marketplaces });
+    let manifest = json!({
+        "version": 1,
+        "items": items,
+        "plugin_marketplaces": plugin_marketplaces,
+        "held": held,
+    });
     let bytes = serde_json::to_vec_pretty(&manifest).map_err(|e| e.to_string())?;
     tar.file("manifest.json", &bytes, false)?;
 
@@ -466,6 +486,37 @@ mod tests {
             p.skipped[1].reason
         );
         assert_eq!(p.manifest["items"].as_array().unwrap().len(), 1);
+        // Final review M3: host-side failures are held (the container keeps
+        // what it has); only a removed source really removes.
+        assert_eq!(
+            p.manifest["held"],
+            json!(["agent:code-reviewer", "agent:does-not-exist"])
+        );
+    }
+
+    #[test]
+    fn a_plugin_that_cannot_be_built_is_held_under_its_marketplace() {
+        let Some(fx) = GitFixture::new() else { return };
+        fx.with_all_kinds();
+        let data = tempfile::tempdir().unwrap();
+        cache(&fx, data.path());
+        let installs = vec![inst(ItemKind::Plugin, "example-plugin", &"0".repeat(40))];
+        let marketplaces = vec![market("m1aaaaaaaa")];
+        let p = build_payload(&PayloadInput {
+            installs: &installs,
+            marketplaces: &marketplaces,
+            data_root: data.path(),
+        })
+        .unwrap();
+        assert_eq!(p.skipped.len(), 1);
+        assert_eq!(
+            p.manifest["held"],
+            json!([format!(
+                "plugin:{}/example-plugin",
+                marketplace_slug("m1aaaaaaaa")
+            )])
+        );
+        assert_eq!(p.manifest["plugin_marketplaces"], json!([]));
     }
 
     #[test]
@@ -489,6 +540,7 @@ mod tests {
         assert_eq!(p.manifest["items"].as_array().unwrap().len(), 1);
         assert_eq!(p.skipped.len(), 1);
         assert!(p.skipped[0].reason.contains("another marketplace"));
+        assert_eq!(p.manifest["held"], json!([]));
     }
 
     #[test]
@@ -502,7 +554,7 @@ mod tests {
         .unwrap();
         assert_eq!(
             p.manifest,
-            json!({ "version": 1, "items": [], "plugin_marketplaces": [] })
+            json!({ "version": 1, "items": [], "plugin_marketplaces": [], "held": [] })
         );
         assert!(unpack(&p.tar).contains_key("manifest.json"));
     }