fix(desktop): harden custom harness backend — validation, dangling-delete coherence, registry races

Review fixes for the BYOH generic ACP harness PR (backend layer):

- Reject commas in harness args at validate_harness_definition, the
  shared boundary used by both save_custom_harness and the disk loader,
  so a definition that would corrupt the comma-joined agent_args wire
  format can neither be saved nor loaded.
- Dangling harness references (agent pins a deleted harness id) now
  resolve to a typed sentinel: summaries render 'harness (deleted): <id>'
  instead of silently falling back to buzz-agent, and spawn converts the
  sentinel to an actionable sentence via user_facing_harness_error.
- ManagedAgentSummary carries the record-level runtime pin so the UI can
  count referencing agents in the delete confirmation.
- Collision filtering moved into load_custom_harnesses so both loaders
  inherit preset-shadowing rules; discovery publishes the registry by
  re-reading the directory under persist_mutex (lock scoped to the
  publish only, never across auth probes), with deterministic
  interleaving tests for save-during-discovery and delete-during-discovery.
- load_global_agent_config hoisted out of build_managed_agent_summary
  into a parameter: one disk read per caller instead of per record.
- Cleanup: drop discarded sweep arg from buzz_sweep_owns_process, remove
  duplicated doc paragraph and stray SAFETY comment.
- De-flake grandchild PGID test: keep the process-group leader alive
  through the getpgid assertion with a trailing sleep.

Co-authored-by: Tyler Longwell <tlongwell@block.xyz>
Signed-off-by: Tyler Longwell <tlongwell@block.xyz>

desktop/src-tauri/src/commands/agent_discovery.rs

@@ -94,12 +94,6 @@ pub async fn discover_acp_providers(
 /// merged catalog entry so the UI can update the provider list without triggering
 /// a full re-discover.
 ///
-/// Write a user-defined harness definition to `<app-data>/custom_harnesses/<id>.json`.
-///
-/// Validates the definition (id regex, builtin-id collision, non-empty command
-/// and label) before touching the filesystem. Returns the merged catalog entry
-/// so the UI can update the provider list without triggering a full re-discover.
-///
 /// `original_id` handles the rename case: when the user edits an existing
 /// harness and changes its id, pass the old id here so the old file is removed
 /// atomically as part of the write. If the id is unchanged or this is a new

desktop/src-tauri/src/commands/agent_models.rs

@@ -931,7 +931,13 @@ pub async fn update_managed_agent(
 
         let summary = {
             let personas = load_personas(&app).unwrap_or_default();
-            build_managed_agent_summary(&app, record, &runtimes, &personas)?
+            build_managed_agent_summary(
+                &app,
+                record,
+                &runtimes,
+                &personas,
+                &crate::managed_agents::load_global_agent_config(&app).unwrap_or_default(),
+            )?
         };
         let rollback = name_changed.then(|| AgentUpdateRollback::new(previous_record, record));
         (summary, sync_params, rollback)

desktop/src-tauri/src/commands/agent_settings.rs

@@ -57,7 +57,13 @@ pub async fn set_managed_agent_start_on_app_launch(
             .find(|record| record.pubkey == pubkey)
             .ok_or_else(|| format!("agent {pubkey} not found"))?;
         let personas = load_personas(&app).unwrap_or_default();
-        build_managed_agent_summary(&app, record, &runtimes, &personas)
+        build_managed_agent_summary(
+            &app,
+            record,
+            &runtimes,
+            &personas,
+            &crate::managed_agents::load_global_agent_config(&app).unwrap_or_default(),
+        )
     })
     .await
     .map_err(|e| format!("spawn_blocking failed: {e}"))?
@@ -102,7 +108,13 @@ pub async fn set_managed_agent_auto_restart(
             .find(|record| record.pubkey == pubkey)
             .ok_or_else(|| format!("agent {pubkey} not found"))?;
         let personas = load_personas(&app).unwrap_or_default();
-        build_managed_agent_summary(&app, record, &runtimes, &personas)
+        build_managed_agent_summary(
+            &app,
+            record,
+            &runtimes,
+            &personas,
+            &crate::managed_agents::load_global_agent_config(&app).unwrap_or_default(),
+        )
     })
     .await
     .map_err(|e| format!("spawn_blocking failed: {e}"))?

desktop/src-tauri/src/commands/agents.rs

@@ -380,7 +380,13 @@ pub(super) async fn start_local_agent_pairs_with_preflight(
         .iter()
         .find(|record| record.pubkey == pubkey)
         .ok_or_else(|| format!("agent {pubkey} not found"))?;
-    build_managed_agent_summary(app, record, &runtimes, &personas)
+    build_managed_agent_summary(
+        app,
+        record,
+        &runtimes,
+        &personas,
+        &crate::managed_agents::load_global_agent_config(app).unwrap_or_default(),
+    )
 }
 
 pub(super) async fn start_local_agent_with_preflight(
@@ -445,7 +451,13 @@ pub(super) async fn start_local_agent_with_preflight(
         .iter()
         .find(|record| record.pubkey == pubkey)
         .ok_or_else(|| format!("agent {pubkey} not found"))?;
-    build_managed_agent_summary(app, record, &runtimes, &personas)
+    build_managed_agent_summary(
+        app,
+        record,
+        &runtimes,
+        &personas,
+        &crate::managed_agents::load_global_agent_config(app).unwrap_or_default(),
+    )
 }
 
 /// Deploy an agent to a provider backend. Resolves the binary, calls deploy via
@@ -546,9 +558,16 @@ pub async fn list_managed_agents(app: AppHandle) -> Result<Vec<ManagedAgentSumma
         }
 
         let personas = load_personas(&app).unwrap_or_default();
+        // One disk read for the whole list — build_managed_agent_summary takes
+        // the config as a parameter precisely so this poll-every-5s call does
+        // not re-read it per record.
+        let global_config =
+            crate::managed_agents::load_global_agent_config(&app).unwrap_or_default();
         records
             .iter()
-            .map(|record| build_managed_agent_summary(&app, record, &runtimes, &personas))
+            .map(|record| {
+                build_managed_agent_summary(&app, record, &runtimes, &personas, &global_config)
+            })
             .collect()
     })
     .await
@@ -921,7 +940,13 @@ pub async fn create_managed_agent(
         retain_managed_agent_pending(&app, &state, record);
         let personas = load_personas(&app).unwrap_or_default();
         (
-            build_managed_agent_summary(&app, record, &runtimes, &personas)?,
+            build_managed_agent_summary(
+                &app,
+                record,
+                &runtimes,
+                &personas,
+                &crate::managed_agents::load_global_agent_config(&app).unwrap_or_default(),
+            )?,
             resolved_avatar_url,
         )
     };
@@ -951,7 +976,13 @@ pub async fn create_managed_agent(
                     .find(|record| record.pubkey == pubkey)
                     .ok_or_else(|| "created agent disappeared unexpectedly".to_string())?;
                 let personas = load_personas(&app).unwrap_or_default();
-                build_managed_agent_summary(&app, record, &runtimes, &personas)?
+                build_managed_agent_summary(
+                    &app,
+                    record,
+                    &runtimes,
+                    &personas,
+                    &crate::managed_agents::load_global_agent_config(&app).unwrap_or_default(),
+                )?
             }
         }
     } else {
@@ -1022,7 +1053,13 @@ pub async fn create_managed_agent(
             .find(|r| r.pubkey == pubkey)
             .ok_or_else(|| "agent disappeared".to_string())?;
         let personas = load_personas(&app).unwrap_or_default();
-        build_managed_agent_summary(&app, record, &runtimes, &personas)?
+        build_managed_agent_summary(
+            &app,
+            record,
+            &runtimes,
+            &personas,
+            &crate::managed_agents::load_global_agent_config(&app).unwrap_or_default(),
+        )?
     } else {
         agent
     };
@@ -1144,7 +1181,13 @@ pub async fn start_managed_agent(
                 .find(|r| r.pubkey == pubkey)
                 .ok_or_else(|| format!("agent {pubkey} not found"))?;
             let personas = load_personas(&app).unwrap_or_default();
-            build_managed_agent_summary(&app, record, &runtimes, &personas)
+            build_managed_agent_summary(
+                &app,
+                record,
+                &runtimes,
+                &personas,
+                &crate::managed_agents::load_global_agent_config(&app).unwrap_or_default(),
+            )
         }
         StartTarget::Provider { backend, .. } => Err(format!(
             "agent {pubkey} has unsupported backend kind: {backend:?}"
@@ -1226,7 +1269,13 @@ pub async fn stop_managed_agent(
             .find(|record| record.pubkey == pubkey)
             .ok_or_else(|| format!("agent {pubkey} not found"))?;
         let personas = load_personas(&app).unwrap_or_default();
-        build_managed_agent_summary(&app, record, &runtimes, &personas)
+        build_managed_agent_summary(
+            &app,
+            record,
+            &runtimes,
+            &personas,
+            &crate::managed_agents::load_global_agent_config(&app).unwrap_or_default(),
+        )
     })
     .await
     .map_err(|e| format!("spawn_blocking failed: {e}"))?

desktop/src-tauri/src/managed_agents/custom_harnesses.rs

@@ -75,6 +75,13 @@ pub(crate) struct HarnessDefinition {
 /// malformed file never fails discovery for the rest.  Returns only
 /// structurally valid, individually validated definitions.
 ///
+/// Filtering is applied HERE — at the loader boundary — so every consumer
+/// (discovery Phase 3, `warm_harness_registry_from_dir`, and through it the
+/// spawn/readiness registry) inherits the same guarantees:
+///   * `check_id_collision` — a hand-placed `goose.json` can never shadow a
+///     built-in or preset id, even on the cold-launch warm path;
+///   * duplicate-id dedup within the directory (first file wins).
+///
 /// **Callers must supply a fresh `dir` path on every `discover_acp_runtimes`
 /// call** — this function performs no caching, mirroring goose's
 /// `refresh_custom_providers()` pattern.
@@ -92,6 +99,7 @@ pub(crate) fn load_custom_harnesses(dir: &Path) -> Vec<HarnessDefinition> {
     };
 
     let mut definitions = Vec::new();
+    let mut seen_ids = std::collections::HashSet::new();
 
     for entry in entries.flatten() {
         let path = entry.path();
@@ -123,6 +131,24 @@ pub(crate) fn load_custom_harnesses(dir: &Path) -> Vec<HarnessDefinition> {
             continue;
         }
 
+        // A custom file must never shadow a built-in or preset id — enforced at
+        // the loader so the warm path can't admit what discovery would reject.
+        if let Err(reason) = check_id_collision(&def.id) {
+            tracing::warn!("custom_harnesses: skipping {} — {reason}", path.display());
+            continue;
+        }
+
+        // Dedup within the directory itself (a file's id is taken from its JSON
+        // content, not its filename, so two files can carry the same id).
+        if !seen_ids.insert(def.id.clone()) {
+            tracing::warn!(
+                "custom_harnesses: skipping {} — duplicate id {:?}",
+                path.display(),
+                def.id
+            );
+            continue;
+        }
+
         definitions.push(def);
     }
 
@@ -147,6 +173,19 @@ fn validate_harness_definition(def: &HarnessDefinition) -> Result<(), String> {
     if def.label.trim().is_empty() {
         return Err("label must not be empty".into());
     }
+    // Args travel to the harness through the comma-delimited
+    // `BUZZ_ACP_AGENT_ARGS` env transport (clap `value_delimiter = ','` on the
+    // buzz-acp side), so a literal comma inside one argument would silently
+    // split into two arguments at runtime. Reject at the validation boundary —
+    // shared by save (UI/Tauri) and load (hand-authored files) — so the
+    // invariant holds regardless of how the definition arrives.
+    if let Some(arg) = def.args.iter().find(|a| a.contains(',')) {
+        return Err(format!(
+            "args: argument {arg:?} contains a comma — arguments are passed via a \
+             comma-delimited transport and would be split at spawn time; \
+             use separate argument entries instead"
+        ));
+    }
     // Validate env keys through the shared boundary validator.  This closes the
     // reserved-key bypass exploit (BUZZ_AUTH_TAG=x forgery shape), rejects
     // NUL bytes that would panic Command::env, and enforces size limits.
@@ -286,6 +325,22 @@ pub(crate) fn warm_harness_registry_from_dir(custom_dir: Option<&std::path::Path
     update_loaded_harness_registry(all);
 }
 
+/// Publish the loaded-harness registry from a **fresh** directory read while
+/// holding the persist mutex — the discovery-side counterpart of
+/// `save_and_warm` / `delete_and_warm`.
+///
+/// `discover_acp_runtimes_from` runs slow auth probes between its directory
+/// scan and its registry publish. Publishing the pre-probe snapshot could
+/// clobber a `save_and_warm` that landed in that window, leaving a freshly
+/// saved harness unresolvable at spawn until the next discovery. Re-reading
+/// the directory at publish time, under the same mutex as save/delete, makes
+/// the documented guarantee ("the registry always reflects disk at warm time")
+/// actually hold. Only the publish is locked — never the probes.
+pub(crate) fn warm_harness_registry_locked(custom_dir: Option<&std::path::Path>) {
+    let _guard = persist_mutex().lock().unwrap_or_else(|e| e.into_inner());
+    warm_harness_registry_from_dir(custom_dir);
+}
+
 /// Global mutex that serializes save/delete filesystem mutations and the
 /// subsequent registry warm as a single atomic unit.
 ///
@@ -920,6 +975,110 @@ mod tests {
         );
     }
 
+    // ── Comma-in-args validation (transport-lossiness guard) ─────────────────
+
+    /// A definition whose args contain a literal comma must be rejected at the
+    /// validation boundary — the comma-delimited `BUZZ_ACP_AGENT_ARGS`
+    /// transport would silently split it into two args at spawn time.
+    #[test]
+    fn validate_rejects_comma_in_args() {
+        let mut def = make_def("comma-args", "Comma");
+        def.args = vec!["--name".to_string(), "a,b".to_string()];
+        let err = validate_harness_definition_pub(&def).unwrap_err();
+        assert!(
+            err.contains("comma"),
+            "error must explain the comma transport limit, got: {err}"
+        );
+    }
+
+    /// Comma-free args pass — including args with spaces and special chars.
+    #[test]
+    fn validate_accepts_comma_free_args() {
+        let mut def = make_def("clean-args", "Clean");
+        def.args = vec!["acp".to_string(), "--flag=x y".to_string()];
+        assert!(validate_harness_definition_pub(&def).is_ok());
+    }
+
+    /// The loader shares the same validator: a hand-authored file with a comma
+    /// in args is skipped, so the invariant holds regardless of how the
+    /// definition arrives (UI save or hand-edited JSON).
+    #[test]
+    fn load_skips_definition_with_comma_in_args() {
+        let dir = tempfile::tempdir().unwrap();
+        fs::write(
+            dir.path().join("comma.json"),
+            r#"{"id":"comma-file","label":"Comma","command":"comma-bin","args":["a,b"]}"#,
+        )
+        .unwrap();
+        assert!(
+            load_custom_harnesses(dir.path()).is_empty(),
+            "comma-in-args definition must be skipped at the loader boundary"
+        );
+    }
+
+    // ── Discovery publish under persist_mutex (stale-snapshot regression) ────
+
+    /// Discovery's registry publish must re-read the directory at publish time
+    /// (under the persist mutex), not push a snapshot taken before the auth
+    /// probes ran. Regression shape: discovery scans dir → user saves harness X
+    /// (save_and_warm warms the registry with X) → discovery finishes. If
+    /// discovery published its pre-save snapshot, X would be on disk but
+    /// unresolvable at spawn until the next discover.
+    ///
+    /// Deterministic interleaving: we simulate it by calling the publish seam
+    /// (`warm_harness_registry_locked`) after a save that happened "during"
+    /// discovery — the fresh-read semantics mean the just-saved definition
+    /// survives the publish.
+    #[test]
+    fn discovery_publish_after_concurrent_save_keeps_saved_harness() {
+        let _lock = registry_test_lock();
+        let dir = tempfile::tempdir().unwrap();
+
+        // Discovery "scans" the dir while it is empty (stale snapshot would be []).
+        let stale_snapshot = load_custom_harnesses(dir.path());
+        assert!(stale_snapshot.is_empty());
+
+        // A save lands mid-discovery (save_and_warm: write + warm).
+        let def = make_def("mid-save", "Mid Save");
+        save_and_warm(dir.path(), &def, None).unwrap();
+        assert!(lookup_loaded_harness_by_id("mid-save").is_some());
+
+        // Discovery publishes — the locked warm re-reads the directory, so the
+        // just-saved harness must survive (a stale-snapshot publish would
+        // clobber it).
+        warm_harness_registry_locked(Some(dir.path()));
+        assert!(
+            lookup_loaded_harness_by_id("mid-save").is_some(),
+            "publish must re-read the directory, not clobber the mid-discovery save"
+        );
+    }
+
+    /// Same shape for delete: a delete landing mid-discovery must not be
+    /// resurrected by the discovery publish.
+    #[test]
+    fn discovery_publish_after_concurrent_delete_keeps_harness_gone() {
+        let _lock = registry_test_lock();
+        let dir = tempfile::tempdir().unwrap();
+
+        let def = make_def("mid-delete", "Mid Delete");
+        save_and_warm(dir.path(), &def, None).unwrap();
+
+        // Discovery "scans" while the file exists (stale snapshot would contain it).
+        let stale_snapshot = load_custom_harnesses(dir.path());
+        assert_eq!(stale_snapshot.len(), 1);
+
+        // Delete lands mid-discovery.
+        delete_and_warm(dir.path(), "mid-delete").unwrap();
+        assert!(lookup_loaded_harness_by_id("mid-delete").is_none());
+
+        // Discovery publishes — fresh read keeps it gone.
+        warm_harness_registry_locked(Some(dir.path()));
+        assert!(
+            lookup_loaded_harness_by_id("mid-delete").is_none(),
+            "publish must not resurrect a harness deleted mid-discovery"
+        );
+    }
+
     // ── Registry warm path ───────────────────────────────────────────────────
 
     /// After `warm_harness_registry_from_dir` the registry contains preset +

desktop/src-tauri/src/managed_agents/discovery.rs

@@ -364,6 +364,39 @@ pub fn effective_agent_command(
 mod overrides;
 pub use overrides::{apply_agent_command_update, create_time_agent_command_override};
 
+/// Prefix of the typed dangling-harness error produced by
+/// `try_record_agent_command` / `resolve_effective_harness_descriptor`.
+///
+/// This sentinel is an internal Rust contract: user-facing surfaces must
+/// convert it to a sentence via [`user_facing_harness_error`] (spawn) or to
+/// the missing id via [`dangling_harness_id`] (summary) — never show it raw.
+pub(crate) const DANGLING_HARNESS_PREFIX: &str = "DANGLING_HARNESS_ID:";
+
+/// Extract the missing harness id from a `DANGLING_HARNESS_ID:<id>` error.
+/// Returns `None` for any other error string.
+pub(crate) fn dangling_harness_id(error: &str) -> Option<&str> {
+    error.strip_prefix(DANGLING_HARNESS_PREFIX)
+}
+
+/// Convert a harness-resolution error to a user-facing sentence. Dangling
+/// harness ids become an actionable message; other errors pass through.
+pub(crate) fn user_facing_harness_error(error: &str) -> String {
+    match dangling_harness_id(error) {
+        Some(id) => format!(
+            "harness \"{id}\" was deleted — pick a new harness for this agent or restore the harness definition"
+        ),
+        None => error.to_string(),
+    }
+}
+
+/// Summary-row display for a dangling harness id: shows the *missing* id so
+/// the agent list tells the same story as spawn (which refuses with the
+/// sentence above), rather than silently falling back to the default command
+/// as if the agent were healthy.
+pub(crate) fn dangling_harness_display(id: &str) -> String {
+    format!("harness (deleted): {id}")
+}
+
 /// Spawn-time variant of `record_agent_command` that returns a typed error when
 /// a record's `runtime` id or its persona's `runtime` id is set but cannot be
 /// resolved (i.e. the definition was deleted after the agent was created).
@@ -1496,7 +1529,7 @@ pub(crate) fn preset_harness_definitions(
 /// set from the single source of truth (`PRESET_HARNESSES`) rather than a
 /// hand-maintained copy.  Adding a preset automatically reserves its ID.
 pub(crate) fn preset_harness_ids() -> &'static [&'static str] {
-    // SAFETY: `PRESET_HARNESSES` is `'static`; we project its `id` fields.
+    // `PRESET_HARNESSES` is `'static`; we project its `id` fields.
     // Computed once via OnceLock to avoid repeated allocations on hot paths.
     use std::sync::OnceLock;
     static IDS: OnceLock<Vec<&'static str>> = OnceLock::new();
@@ -1585,10 +1618,6 @@ pub fn discover_acp_runtimes_from(
     let mut seen_ids: std::collections::HashSet<String> =
         entries.iter().map(|e| e.id.clone()).collect();
 
-    // Loaded (non-builtin) definitions collected for the registry.
-    let mut loaded_defs: Vec<crate::managed_agents::custom_harnesses::HarnessDefinition> =
-        Vec::new();
-
     // Phase 2.5: insert static preset entries (PATH-probed, not editable/deletable).
     for def in PRESET_HARNESSES {
         if seen_ids.contains(def.id) {
@@ -1636,30 +1665,15 @@ pub fn discover_acp_runtimes_from(
             // Preset entries have static, non-editable env; definition_env is empty.
             definition_env: Default::default(),
         });
-
-        // Register for spawn-time resolution.
-        loaded_defs.push(crate::managed_agents::custom_harnesses::HarnessDefinition {
-            id: def.id.to_string(),
-            label: def.label.to_string(),
-            command: def.command.to_string(),
-            args: def.args.iter().map(|s| s.to_string()).collect(),
-            env: Default::default(),
-            install_instructions_url: def.install_instructions_url.to_string(),
-            install_hint: def.install_hint.to_string(),
-        });
     }
 
     // Phase 3: load and append custom harness definitions.
     if let Some(dir) = custom_harnesses_dir {
+        // The loader applies collision + duplicate filtering at the boundary,
+        // so anything it returns is safe to surface in the catalog. Builtin
+        // shadowing is impossible here (check_id_collision covers builtins and
+        // presets); `seen_ids` guards only same-run duplicates.
         for def in crate::managed_agents::custom_harnesses::load_custom_harnesses(dir) {
-            // Collision check: a custom file must not shadow a built-in or preset id.
-            if let Err(reason) =
-                crate::managed_agents::custom_harnesses::check_id_collision(&def.id)
-            {
-                tracing::warn!("custom_harnesses: skipping {}: {reason}", def.id);
-                continue;
-            }
-            // Reject duplicates within the custom set itself or against presets.
             if !seen_ids.insert(def.id.clone()) {
                 tracing::warn!("custom_harnesses: skipping duplicate id {:?}", def.id);
                 continue;
@@ -1708,14 +1722,15 @@ pub fn discover_acp_runtimes_from(
                 // read it back — prevents silently erasing env on save.
                 definition_env: def.env.clone(),
             });
-
-            loaded_defs.push(def);
         }
     }
 
-    // Populate the loaded registry so spawn, readiness, and summary paths can
-    // resolve custom/preset harness commands without re-running discovery.
-    crate::managed_agents::custom_harnesses::update_loaded_harness_registry(loaded_defs);
+    // Publish the loaded-harness registry from a FRESH directory read under the
+    // persist mutex — never from the snapshot taken before the auth probes ran.
+    // A save/delete landing during Phase 2 already re-warmed the registry; a
+    // stale-snapshot publish here would clobber it (the just-saved harness
+    // would become unresolvable at spawn until the next discovery).
+    crate::managed_agents::custom_harnesses::warm_harness_registry_locked(custom_harnesses_dir);
 
     entries
 }

desktop/src-tauri/src/managed_agents/discovery/tests.rs

@@ -1507,6 +1507,96 @@ fn registry_edit_with_id_rename_old_dangling_new_resolved() {
     );
 }
 
+// ── Dangling-harness sentinel → user-facing surfaces ─────────────────────────
+
+/// The internal `DANGLING_HARNESS_ID:<id>` sentinel round-trips through
+/// `dangling_harness_id` and is never confused with other error strings.
+#[test]
+fn dangling_harness_id_extracts_id_and_rejects_other_errors() {
+    use super::{dangling_harness_id, DANGLING_HARNESS_PREFIX};
+    assert_eq!(
+        dangling_harness_id(&format!("{DANGLING_HARNESS_PREFIX}my-harness")),
+        Some("my-harness")
+    );
+    assert_eq!(dangling_harness_id("some other error"), None);
+    assert_eq!(dangling_harness_id(""), None);
+    // Prefix must anchor at the start — a wrapped sentinel is not a sentinel.
+    assert_eq!(
+        dangling_harness_id("cannot spawn: DANGLING_HARNESS_ID:x"),
+        None
+    );
+}
+
+/// Spawn surfaces the sentinel as an actionable sentence naming the id;
+/// non-sentinel errors pass through untouched.
+#[test]
+fn user_facing_harness_error_converts_sentinel_to_sentence() {
+    use super::{user_facing_harness_error, DANGLING_HARNESS_PREFIX};
+    let msg = user_facing_harness_error(&format!("{DANGLING_HARNESS_PREFIX}foo"));
+    assert!(
+        msg.contains("\"foo\"") && msg.contains("deleted"),
+        "sentence must name the missing harness, got: {msg}"
+    );
+    assert!(
+        !msg.contains(DANGLING_HARNESS_PREFIX),
+        "raw sentinel must never reach the user, got: {msg}"
+    );
+    assert_eq!(
+        user_facing_harness_error("plain failure"),
+        "plain failure",
+        "non-sentinel errors pass through"
+    );
+}
+
+/// Composed coherence test (delete → summary display → spawn sentence): after
+/// a harness is deleted, the single resolver errors with the sentinel, the
+/// summary path renders the *missing id* (not a silent buzz-agent fallback),
+/// and the spawn path renders the actionable sentence — both halves tell the
+/// same story from the same error.
+#[test]
+fn deleted_harness_summary_display_and_spawn_sentence_agree() {
+    use crate::managed_agents::custom_harnesses::{
+        delete_and_warm, registry_test_lock, save_and_warm,
+    };
+    use crate::managed_agents::{resolve_effective_harness_descriptor, GlobalAgentConfig};
+
+    let _lock = registry_test_lock();
+    let dir = tempfile::tempdir().unwrap();
+
+    // Save a custom harness and pin a record to it.
+    let def = crate::managed_agents::custom_harnesses::HarnessDefinition {
+        id: "doomed".to_string(),
+        label: "Doomed".to_string(),
+        command: "doomed-bin".to_string(),
+        args: vec![],
+        env: Default::default(),
+        install_instructions_url: String::new(),
+        install_hint: String::new(),
+    };
+    save_and_warm(dir.path(), &def, None).unwrap();
+    let record = record_with(Some("doomed"), None, None);
+    let global = GlobalAgentConfig::default();
+    assert!(
+        resolve_effective_harness_descriptor(&record, &[], &global).is_ok(),
+        "must resolve before delete"
+    );
+
+    // Delete it — the shared resolver used by BOTH spawn and summary errors.
+    delete_and_warm(dir.path(), "doomed").unwrap();
+    let err = resolve_effective_harness_descriptor(&record, &[], &global).unwrap_err();
+
+    // Summary half: renders the missing id, never the default command.
+    let id = super::dangling_harness_id(&err).expect("resolver must emit the typed sentinel");
+    let display = super::dangling_harness_display(id);
+    assert_eq!(display, "harness (deleted): doomed");
+    assert!(!display.contains(&default_agent_command()));
+
+    // Spawn half: renders a sentence naming the same id, no raw sentinel.
+    let sentence = super::user_facing_harness_error(&err);
+    assert!(sentence.contains("\"doomed\"") && sentence.contains("deleted"));
+    assert!(!sentence.contains(super::DANGLING_HARNESS_PREFIX));
+}
+
 // ── I2: custom catalog entry carries definition_env for the edit round-trip ───
 
 /// A custom harness definition that includes env vars must surface those vars

desktop/src-tauri/src/managed_agents/runtime.rs

@@ -418,20 +418,18 @@ pub(crate) fn valid_agent_runtime_receipt(
         receipt,
         instance_id,
         process_is_running,
-        process_belongs_to_us,
         process_has_buzz_marker,
     )
 }
 
 /// Injectable version of `valid_agent_runtime_receipt` for testing.
-/// `is_running(pid)`, `belongs_to_us(pid)`, and `has_marker(pid, instance_id)`
-/// can be substituted by test doubles without spawning real processes.
+/// `is_running(pid)` and `has_marker(pid, instance_id)` can be substituted by
+/// test doubles without spawning real processes.
 pub(crate) fn valid_agent_runtime_receipt_with(
     path: &std::path::Path,
     receipt: &super::ManagedAgentRuntimeReceipt,
     instance_id: &str,
     is_running: impl Fn(u32) -> bool,
-    belongs_to_us: impl Fn(u32) -> bool,
     has_marker: impl Fn(u32, &str) -> bool,
 ) -> bool {
     let Ok(canonical) =
@@ -445,14 +443,10 @@ pub(crate) fn valid_agent_runtime_receipt_with(
         && receipt.desktop_instance_id == instance_id
         && is_running(receipt.pid)
         // Receipts are written by THIS instance at spawn time, so they are
-        // Buzz-owned by construction. Use the shared ownership predicate
-        // (marker-only) so custom-harness binaries (not in KNOWN_AGENT_BINARIES)
-        // are not rejected here. `belongs_to_us` is passed as the fast-
-        // path hint but ignored by `buzz_sweep_owns_process`.
-        && buzz_sweep_owns_process(
-            belongs_to_us(receipt.pid),
-            has_marker(receipt.pid, &receipt.desktop_instance_id),
-        )
+        // Buzz-owned by construction. Marker-only ownership: custom-harness
+        // binaries (not in KNOWN_AGENT_BINARIES) must not be rejected by a
+        // name gate — see the sweep ownership rule above.
+        && has_marker(receipt.pid, &receipt.desktop_instance_id)
 }
 
 fn terminate_runtime_receipt_with(
@@ -610,28 +604,16 @@ const _: () = assert!(std::mem::size_of::<BSDInfo>() == 136);
 #[cfg(target_os = "macos")]
 pub(super) const PROC_PIDTBSDINFO: libc::c_int = 3;
 
-// ── Shared sweep ownership predicate ─────────────────────────────────────────
+// ── Sweep ownership rule ──────────────────────────────────────────────────────
 //
-// `BUZZ_MANAGED_AGENT` env marker is the sole authoritative ownership proof.
-// The `_belongs_to_us` name-check is passed by callers but is intentionally
-// IGNORED — custom harnesses have arbitrary binary names and would be missed
-// by a name-gated predicate.
-
-/// Returns `true` when a process should be included in the orphan sweep.
-///
-/// The `BUZZ_MANAGED_AGENT` env marker is the sole authoritative ownership
-/// proof — any process carrying it and belonging to this instance is swept,
-/// regardless of binary name.  The `_belongs_to_us` parameter is accepted
-/// for call-site symmetry but is intentionally ignored: the function returns
-/// `has_buzz_marker` unconditionally.  On Windows no `/proc`-based sweep
-/// runs, so `process_has_buzz_marker` always returns `false`.
-///
-/// This predicate is cross-platform and tested directly in the unit tests
-/// below — no `#[cfg(unix)]` guard needed here.
-pub(crate) fn buzz_sweep_owns_process(_belongs_to_us: bool, has_buzz_marker: bool) -> bool {
-    // Marker is the sole authoritative ownership gate.
-    has_buzz_marker
-}
+// The `BUZZ_MANAGED_AGENT` env marker is the SOLE authoritative ownership
+// proof for sweep/receipt decisions. Do NOT name-gate via
+// `process_belongs_to_us` here — custom harnesses use arbitrary binary names
+// and a name-gated predicate would silently leak their orphans (the old Linux
+// AND-gate bug). `process_belongs_to_us` remains in use only as a cheap
+// pre-check on paths that already know the binary (see runtime/stop.rs).
+// On Windows no `/proc`-based sweep runs, so `process_has_buzz_marker`
+// always returns `false`.
 
 /// Enumerate all processes on the system owned by the current user and kill any
 /// agent binary stamped with *this* instance's `BUZZ_MANAGED_AGENT` marker
@@ -677,13 +659,7 @@ pub(crate) fn sweep_system_agent_processes(instance_id: &str, skip_pids: &[u32])
         }
         // Custom harnesses don't match KNOWN_AGENT_BINARIES by name; the
         // BUZZ_MANAGED_AGENT env marker is the authoritative ownership proof.
-        // `buzz_sweep_owns_process` returns `has_buzz_marker` — the
-        // `_belongs_to_us` argument is accepted for call-site symmetry but
-        // is intentionally ignored (see the function's doc comment).
-        if !buzz_sweep_owns_process(
-            process_belongs_to_us(upid),
-            process_has_buzz_marker(upid, instance_id),
-        ) {
+        if !process_has_buzz_marker(upid, instance_id) {
             continue;
         }
         // Live descendants of a tracked harness are exempt — see sweep::is_live_descendant_*.
@@ -742,13 +718,9 @@ pub(crate) fn sweep_system_agent_processes(instance_id: &str, skip_pids: &[u32])
         if meta.uid() != my_uid {
             continue;
         }
-        // Same ownership predicate as macOS: marker is the authoritative gate,
-        // `_belongs_to_us` is accepted for call-site symmetry but ignored.
+        // Same ownership rule as macOS: the marker is the authoritative gate.
         // Fixes custom-harness orphan cleanup on Linux.
-        if !buzz_sweep_owns_process(
-            process_belongs_to_us(upid),
-            process_has_buzz_marker(upid, instance_id),
-        ) {
+        if !process_has_buzz_marker(upid, instance_id) {
             continue;
         }
         // Live descendants of a tracked harness are exempt — see sweep::is_live_descendant_*.
@@ -851,10 +823,7 @@ pub(crate) fn collect_same_instance_orphans(
         }
         // Custom harnesses don't match KNOWN_AGENT_BINARIES by name; the
         // BUZZ_MANAGED_AGENT env marker is the authoritative ownership proof.
-        if !buzz_sweep_owns_process(
-            process_belongs_to_us(upid),
-            process_has_buzz_marker(upid, instance_id),
-        ) {
+        if !process_has_buzz_marker(upid, instance_id) {
             continue;
         }
         // Live descendants of a tracked harness are exempt — see sweep::is_live_descendant_*.
@@ -900,13 +869,9 @@ pub(crate) fn collect_same_instance_orphans(
         if meta.uid() != my_uid {
             continue;
         }
-        // Same ownership predicate as macOS: marker is the authoritative gate,
-        // `_belongs_to_us` is accepted for call-site symmetry but ignored.
+        // Same ownership rule as macOS: the marker is the authoritative gate.
         // Fixes custom-harness orphan cleanup on Linux.
-        if !buzz_sweep_owns_process(
-            process_belongs_to_us(upid),
-            process_has_buzz_marker(upid, instance_id),
-        ) {
+        if !process_has_buzz_marker(upid, instance_id) {
             continue;
         }
         // Live descendants of a tracked harness are exempt — see sweep::is_live_descendant_*.
@@ -1469,6 +1434,7 @@ pub fn build_managed_agent_summary(
     record: &ManagedAgentRecord,
     runtimes: &HashMap<ManagedAgentRuntimeKey, ManagedAgentPairRuntime>,
     personas: &[crate::managed_agents::types::AgentDefinition],
+    global_config: &crate::managed_agents::GlobalAgentConfig,
 ) -> Result<ManagedAgentSummary, String> {
     use crate::managed_agents::BackendKind;
 
@@ -1547,12 +1513,15 @@ pub fn build_managed_agent_summary(
     // stamped at spawn.  This catches out-of-band adapter changes (manual
     // npm install/downgrade) that Phase-1 auto-restart doesn't cover.  The
     // cache is read-only here — no subprocess is spawned.
+    //
+    // Global config drives both the restart-drift hash and descriptor env
+    // layering below — the caller loads it once and passes it in, so
+    // list-style callers pay one disk read per call rather than one per record.
+
     let needs_restart = pair_key
         .as_ref()
         .and_then(|key| runtimes.get(key).map(|runtime| (key, runtime)))
         .is_some_and(|(key, runtime)| {
-            let global_for_hash =
-                crate::managed_agents::load_global_agent_config(app).unwrap_or_default();
             let teams_for_hash = crate::managed_agents::load_teams(app).unwrap_or_default();
             let hash_drift = runtime.spawn_config_hash
                 != crate::managed_agents::spawn_hash::spawn_config_hash(
@@ -1560,7 +1529,7 @@ pub fn build_managed_agent_summary(
                     personas,
                     &teams_for_hash,
                     &key.relay_url,
-                    &global_for_hash,
+                    global_config,
                 );
             let availability_drift = super::availability_drift(
                 runtime.adapter_availability.as_ref(),
@@ -1571,18 +1540,19 @@ pub fn build_managed_agent_summary(
 
     // Resolve the effective harness via the single typed descriptor — same resolver
     // as spawn, so the UI reflects the persona's current harness (or explicit pin).
-    // Global config is loaded here for descriptor parity with spawn (env layering).
-    let global_for_summary =
-        crate::managed_agents::load_global_agent_config(app).unwrap_or_default();
     let descriptor = crate::managed_agents::resolve_effective_harness_descriptor(
         record,
         personas,
-        &global_for_summary,
+        global_config,
     )
-    .unwrap_or_else(|_| {
-        // Dangling harness — show a best-effort command so the UI displays the
-        // record's configured runtime id rather than crashing.
-        let cmd = crate::managed_agents::record_agent_command(record, personas);
+    .unwrap_or_else(|e| {
+        // Dangling harness — surface the missing id so the UI tells the same
+        // story as spawn (which refuses with a sentence), rather than silently
+        // showing the default-command fallback as if the agent were healthy.
+        let cmd = match crate::managed_agents::dangling_harness_id(&e) {
+            Some(id) => crate::managed_agents::dangling_harness_display(id),
+            None => crate::managed_agents::record_agent_command(record, personas),
+        };
         let args = normalize_agent_args(&cmd, record.agent_args.clone());
         crate::managed_agents::readiness::EffectiveHarnessDescriptor {
             command: cmd,
@@ -1599,6 +1569,7 @@ pub fn build_managed_agent_summary(
         pubkey: record.pubkey.clone(),
         name: record.name.clone(),
         persona_id: record.persona_id.clone(),
+        runtime: record.runtime.clone(),
         team_id: record.team_id.clone(),
         relay_url: record.relay_url.clone(),
         acp_command: record.acp_command.clone(),
@@ -1776,7 +1747,13 @@ pub fn spawn_agent_child(
     // model probes all consume this descriptor rather than assembling values inline.
     let descriptor =
         crate::managed_agents::resolve_effective_harness_descriptor(record, &personas, &global)
-            .map_err(|e| format!("cannot spawn agent {}: {e}", record.pubkey))?;
+            .map_err(|e| {
+                format!(
+                    "cannot spawn agent {}: {}",
+                    record.pubkey,
+                    crate::managed_agents::user_facing_harness_error(&e)
+                )
+            })?;
     let effective_command = &descriptor.command;
     let agent_args = &descriptor.args;
     let resolved_acp_command = resolve_command(&record.acp_command)

desktop/src-tauri/src/managed_agents/runtime/tests.rs

@@ -693,12 +693,18 @@ fn grandchild_inherits_pgid_of_process_group_leader() {
     // spawns an intermediate child which in turn spawns a grandchild.
     // This mirrors the real tree: buzz-acp → goose → buzz-dev-mcp.
     //
-    // The intermediate `sh` uses exec to replace itself with another sh
-    // that backgrounds the grandchild, so the grandchild's ppid is the
-    // intermediate (not the harness).
+    // The intermediate `sh` backgrounds the grandchild and echoes its PID,
+    // so the grandchild's ppid is the intermediate (not the harness).
+    //
+    // The trailing `sleep 10` keeps the harness (the process group leader)
+    // alive through the assertions below: without it the harness exits as
+    // soon as the intermediate echoes, and under parallel test load it can
+    // be reaped before `getpgid(harness_pid)` runs (observed flake —
+    // getpgid returned -1). The group is killed in cleanup, so the sleep
+    // never runs to term.
     let mut harness = {
         let mut cmd = Command::new("sh");
-        cmd.args(["-c", "sh -c 'sleep 10 & echo $!' & wait $!"])
+        cmd.args(["-c", "sh -c 'sleep 10 & echo $!' & wait $!; sleep 10"])
             .stdout(std::process::Stdio::piped())
             .process_group(0);
         cmd.spawn().expect("spawn harness")
@@ -1006,99 +1012,17 @@ fn invalid_pubkey_resolves_no_pair_key() {
 
 // ── Custom-harness orphan sweep coverage ─────────────────────────────────────
 //
-// The system sweep gates must include any process carrying the
+// The sweep/receipt ownership gate must include any process carrying the
 // `BUZZ_MANAGED_AGENT` env marker, regardless of whether the binary name
 // matches `KNOWN_AGENT_BINARIES`. Custom harnesses use arbitrary binary names
 // so name-match alone would silently leak their orphans on crash.
 //
 // Previously: macOS used a two-check OR+AND pattern (equivalent to just marker),
 //             Linux used an AND-gate (name + marker) — wrong for custom harnesses.
-// Fix: all platforms use the shared `buzz_sweep_owns_process` predicate which
-//      returns `has_buzz_marker` only — the `_belongs_to_us` fast-skip is
-//      accepted for call-site symmetry but intentionally ignored.
-//
-// These tests call the production predicate directly so they fail if the
-// predicate reverts to broken logic.
-
-use super::buzz_sweep_owns_process;
-
-/// A known-binary process WITHOUT the marker must be excluded — stray processes
-/// with colliding names (e.g. another user's goose) are not ours.
-#[test]
-fn sweep_condition_known_binary_without_marker_is_excluded() {
-    assert!(
-        !buzz_sweep_owns_process(true, false),
-        "known binary without marker must be excluded"
-    );
-}
-
-/// A custom harness binary (not in KNOWN_AGENT_BINARIES) WITH the marker must
-/// be included — this is the fix for the Linux AND-gate bug.
-#[test]
-fn sweep_condition_custom_binary_with_marker_is_included() {
-    assert!(
-        buzz_sweep_owns_process(false, true),
-        "custom binary with marker must be included"
-    );
-}
-
-/// A truly foreign process (not owned by name, no marker) must remain excluded.
-#[test]
-fn sweep_condition_foreign_process_is_excluded() {
-    assert!(
-        !buzz_sweep_owns_process(false, false),
-        "foreign process must always be excluded"
-    );
-}
-
-/// A known-binary process WITH the marker is owned — must be included.
-#[test]
-fn sweep_condition_known_binary_with_marker_is_included() {
-    assert!(
-        buzz_sweep_owns_process(true, true),
-        "known binary with marker must be included"
-    );
-}
-
-// ── I3: receipt path collector decision ─────────────────────────────────────
-//
-// `valid_agent_runtime_receipt` used to AND-gate process_belongs_to_us (a
-// cheap name-check) with process_has_buzz_marker. Custom harnesses don't match
-// KNOWN_AGENT_BINARIES, so their receipts would never be valid — the receipt
-// cleanup loop would leave them running. The fix uses buzz_sweep_owns_process
-// (marker-only) in valid_agent_runtime_receipt.
-//
-// These tests verify the predicate truth table that valid_agent_runtime_receipt
-// now relies on. They would fail if the AND-gate were reinstated.
-
-/// Simulates valid_agent_runtime_receipt's ownership decision for a custom
-/// harness: belongs_to_us=false (not in KNOWN_AGENT_BINARIES), has_marker=true.
-/// Must be INCLUDED — the marker is authoritative, name is irrelevant.
-///
-/// Would fail if valid_agent_runtime_receipt used process_belongs_to_us &&
-/// process_has_buzz_marker (AND-gate).
-#[test]
-fn receipt_ownership_custom_harness_with_marker_is_valid() {
-    // Custom binary: not in KNOWN_AGENT_BINARIES (belongs_to_us = false)
-    // but carries BUZZ_MANAGED_AGENT marker (has_buzz_marker = true).
-    assert!(
-        buzz_sweep_owns_process(false, true),
-        "custom harness with marker must be valid for receipt ownership"
-    );
-}
-
-/// Simulates valid_agent_runtime_receipt's ownership decision for a known
-/// harness binary WITHOUT the marker (stray process, not owned by us).
-/// Must be EXCLUDED.
-#[test]
-fn receipt_ownership_known_binary_without_marker_is_not_valid() {
-    // Known binary name (belongs_to_us = true) but no marker.
-    // This is a stray process that happens to share a binary name — must exclude.
-    assert!(
-        !buzz_sweep_owns_process(true, false),
-        "known binary without marker must not be valid for receipt ownership"
-    );
-}
+// Fix: all platforms gate on `process_has_buzz_marker` alone; the receipt path
+//      is verified below via `valid_agent_runtime_receipt_with` (injectable),
+//      which no longer takes a name-check predicate at all — reinstating an
+//      AND-gate would be a signature change these tests would catch.
 
 // ── Collector-discriminating sweep tests (C-9 / Thufir F6) ──────────────────
 //
@@ -1204,7 +1128,7 @@ fn kill_stale_live_pair_is_not_touched() {
 
 #[test]
 fn receipt_valid_with_marker_and_running() {
-    // Custom harness receipt: belongs_to_us=false, has_marker=true, is_running=true.
+    // Custom harness receipt: custom binary (not in KNOWN_AGENT_BINARIES), has_marker=true, is_running=true.
     // Must be valid — marker is the authoritative gate.
     use crate::managed_agents::ManagedAgentRuntimeKey;
     let key = ManagedAgentRuntimeKey::new("bb".repeat(32), "wss://relay.example").unwrap();
@@ -1216,8 +1140,7 @@ fn receipt_valid_with_marker_and_running() {
         &receipt,
         "test-instance",
         |_pid| true,       // is_running
-        |_pid| false,      // belongs_to_us: false (custom binary)
-        |_pid, _iid| true, // has_marker: true
+        |_pid, _iid| true, // has_marker: true (custom binary — no name gate)
     );
     assert!(
         valid,
@@ -1238,7 +1161,6 @@ fn receipt_invalid_known_binary_without_marker() {
         &receipt,
         "test-instance",
         |_pid| true,        // is_running
-        |_pid| true,        // belongs_to_us: true (known binary)
         |_pid, _iid| false, // has_marker: false (not our process)
     );
     assert!(!valid, "known binary without marker must not be valid");
@@ -1257,7 +1179,6 @@ fn receipt_invalid_when_process_not_running() {
         &receipt,
         "test-instance",
         |_pid| false,      // is_running: false
-        |_pid| true,       // belongs_to_us
         |_pid, _iid| true, // has_marker
     );
     assert!(

desktop/src-tauri/src/managed_agents/types.rs

@@ -456,6 +456,10 @@ pub struct ManagedAgentSummary {
     pub pubkey: String,
     pub name: String,
     pub persona_id: Option<String>,
+    /// The record's harness/runtime id (mirror of `ManagedAgentRecord.runtime`).
+    /// Lets the UI count agents referencing a harness definition (e.g. in the
+    /// delete-confirmation flow). `None` = inherit from the linked persona.
+    pub runtime: Option<String>,
     pub team_id: Option<String>,
     pub relay_url: String,
     pub acp_command: String,