test(desktop): add B-2/B-3/B-4/B-6 discriminating tests + concurrent-safe registry

B-4 (persistence contract): Replace the two simulated round-trip tests that
called raw fs::write/remove_file with six tests that exercise the real
save_custom_harness_to_dir helper directly:
- save_to_dir_create_writes_file_and_loads_back
- save_to_dir_same_id_edit_replaces_content
- save_to_dir_backup_is_cleaned_up_after_same_id_edit
- save_to_dir_rename_removes_old_file_and_creates_new
- save_to_dir_rename_nonexistent_old_id_is_non_fatal
- save_to_dir_roundtrip_with_env_preserves_values

B-3 (env validation boundary): Six tests exercising validate_harness_definition_pub
integration with validate_user_env_keys for the documented attack surfaces:
- validate_rejects_malformed_key_with_equals_sign (BUZZ_AUTH_TAG=x forgery shape)
- validate_rejects_reserved_key_buzz_managed_agent (ownership marker)
- validate_rejects_reserved_key_case_insensitive (lowercase bypass)
- validate_rejects_nul_byte_in_value (Command::env panic protection)
- validate_rejects_value_over_per_value_size_limit
- validate_accepts_well_formed_env

B-2 (API boundary): Export fromRawAcpRuntimeCatalogEntry from tauri.ts and
add four tests to tauri.test.mjs proving the Rust definition_env snake_case
field is mapped to definitionEnv camelCase, that absent definition_env defaults
to {} (not undefined), and that env round-trips end-to-end through the mapper
so a save-then-edit cycle cannot erase definition env.

B-6 (concurrent-safe registry): Add save_and_warm + delete_and_warm functions
in custom_harnesses.rs that hold a PERSIST_MUTEX for the filesystem mutation
and the subsequent warm_harness_registry_from_dir call as an atomic unit.
Update save_custom_harness and delete_custom_harness Tauri commands to use
these helpers. Eliminates the lost-update window where two concurrent saves
could interleave their warm calls and produce a stale registry snapshot.

File-size override added for custom_harnesses.rs (1042 lines after the new
tests; queued to split once the feature stabilizes).

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>

desktop/scripts/check-file-sizes.mjs

@@ -338,6 +338,14 @@ const overrides = new Map([
   // helper + discover_acp_runtime_availability; both load-bearing for
   // post-install verification. Semantic composition with BYOH changes.
   ["src-tauri/src/managed_agents/discovery.rs", 1706],
+  // BYOH — save_custom_harness_to_dir (backup-swap atomic write) + save_and_warm /
+  // delete_and_warm (persist-mutex serialization for concurrent-safe registry
+  // refresh, B-6). Also: id/collision/load/registry tests (from the file base) +
+  // B-4 real persistence tests (create, same-id edit, rename, backup cleanup) +
+  // B-3 env validation boundary tests (malformed key, reserved shape, NUL,
+  // size limit, ownership marker). Load-bearing correctness/security coverage;
+  // queued to extract helper module once the feature stabilizes.
+  ["src-tauri/src/managed_agents/custom_harnesses.rs", 1045],
   // rebase over codex-acp-package-swap: its version-probe tests union with the
   // doctor-install-reliability nvm/login-shell/semver tests — each side alone
   // stayed under the 1000 default; the union exceeds it.

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

@@ -143,59 +143,10 @@ pub async fn save_custom_harness(
     std::fs::create_dir_all(&custom_dir)
         .map_err(|e| format!("failed to create custom_harnesses dir: {e}"))?;
 
-    let target_path = custom_dir.join(format!("{}.json", definition.id));
-
-    // ── Phase 2: atomic write (Windows-safe) ────────────────────────────────
-    // `atomic-write-file` 0.3 uses a plain `fs::rename(temp, dest)` on
-    // non-Unix platforms.  On Windows `fs::rename` fails with "access denied"
-    // when `dest` already exists.  Pre-removing the target on Windows before
-    // committing makes same-ID edits safe; the window between remove and rename
-    // is tiny and the user-visible TOML data is already serialised in the temp
-    // file at that point.
-    let json = serde_json::to_string_pretty(&definition)
-        .map_err(|e| format!("failed to serialize harness definition: {e}"))?;
-
-    {
-        use atomic_write_file::AtomicWriteFile;
-        let mut file = AtomicWriteFile::open(&target_path)
-            .map_err(|e| format!("failed to open {}: {e}", target_path.display()))?;
-        std::io::Write::write_all(&mut file, json.as_bytes())
-            .map_err(|e| format!("failed to write harness definition: {e}"))?;
-        // On Windows, remove the destination before committing so `fs::rename`
-        // does not fail with "access denied" on an existing file.
-        #[cfg(windows)]
-        if target_path.exists() {
-            std::fs::remove_file(&target_path).map_err(|e| {
-                format!(
-                    "failed to remove existing {} before replace: {e}",
-                    target_path.display()
-                )
-            })?;
-        }
-        file.commit()
-            .map_err(|e| format!("failed to finalize harness definition: {e}"))?;
-    }
-
-    // ── Phase 3: remove old file on rename (after new file is committed) ────
-    if let Some(ref old_id) = rename_old_id {
-        let old_path = custom_dir.join(format!("{old_id}.json"));
-        if let Err(e) = std::fs::remove_file(&old_path) {
-            if e.kind() != std::io::ErrorKind::NotFound {
-                // New file is already committed.  Surface the error so the user
-                // knows the old file was not cleaned up, but do not abort — the
-                // new definition is already live and the registry re-warm below
-                // will make the new id resolvable.
-                tracing::warn!(
-                    "save_custom_harness: failed to remove old harness file {old_id:?}: {e}"
-                );
-            }
-        }
-    }
-
-    // Refresh the loaded-harness registry transactionally so a spawn/start
-    // immediately after save can resolve the new id without waiting for the
-    // next frontend-driven discover_acp_providers call.
-    custom_harnesses::warm_harness_registry_from_dir(Some(&custom_dir));
+    // ── Phase 2+3: backup-swap write + rename (Windows-safe, rollback on failure)
+    // `save_and_warm` holds the persist mutex for the write + registry-warm pair
+    // so concurrent saves never produce a stale registry snapshot (B-6).
+    custom_harnesses::save_and_warm(&custom_dir, &definition, rename_old_id.as_deref())?;
 
     // Resolve availability for the returned catalog entry.
     let (availability, command_opt, binary_path) =
@@ -264,20 +215,9 @@ pub async fn delete_custom_harness(id: String, app: tauri::AppHandle) -> Result<
         .map_err(|e| format!("failed to resolve app data dir: {e}"))?
         .join("custom_harnesses");
 
-    let target_path = custom_dir.join(format!("{id}.json"));
-
-    match std::fs::remove_file(&target_path) {
-        Ok(()) => {}
-        Err(e) if e.kind() == std::io::ErrorKind::NotFound => {
-            // Idempotent: already gone is fine.
-        }
-        Err(e) => return Err(format!("failed to delete harness {id:?}: {e}")),
-    }
-
-    // Refresh the loaded-harness registry transactionally so the deleted id
-    // is immediately unresolvable, without waiting for the next frontend
-    // discover_acp_providers call.
-    custom_harnesses::warm_harness_registry_from_dir(Some(&custom_dir));
+    // `delete_and_warm` holds the persist mutex for the delete + registry-warm
+    // pair so concurrent save/delete calls never produce a stale snapshot (B-6).
+    custom_harnesses::delete_and_warm(&custom_dir, &id)?;
 
     Ok(())
 }

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

@@ -286,6 +286,155 @@ pub(crate) fn warm_harness_registry_from_dir(custom_dir: Option<&std::path::Path
     update_loaded_harness_registry(all);
 }
 
+/// Global mutex that serializes save/delete filesystem mutations and the
+/// subsequent registry warm as a single atomic unit.
+///
+/// Without this lock, two concurrent `save_custom_harness` calls could
+/// interleave: A writes file-A, B writes file-B, B re-warms (sees only B),
+/// A re-warms (sees A + B, wins) — but if timing goes the other way B's warm
+/// wins and misses A. Holding the lock for the write+warm pair guarantees that
+/// the registry always reflects the complete set of files on disk at the time
+/// of the warm.
+fn persist_mutex() -> &'static std::sync::Mutex<()> {
+    use std::sync::{Mutex, OnceLock};
+    static PERSIST: OnceLock<Mutex<()>> = OnceLock::new();
+    PERSIST.get_or_init(|| Mutex::new(()))
+}
+
+/// Write `definition` to `dir`, then atomically warm the loaded-harness
+/// registry.  Callers MUST use this function (not `save_custom_harness_to_dir`
+/// or `warm_harness_registry_from_dir` separately) so that concurrent saves
+/// cannot produce a stale registry snapshot.
+pub(crate) fn save_and_warm(
+    dir: &Path,
+    definition: &HarnessDefinition,
+    rename_old_id: Option<&str>,
+) -> Result<SaveOutcome, String> {
+    let _guard = persist_mutex().lock().unwrap_or_else(|e| e.into_inner());
+    let outcome = save_custom_harness_to_dir(dir, definition, rename_old_id)?;
+    warm_harness_registry_from_dir(Some(dir));
+    Ok(outcome)
+}
+
+/// Delete `id.json` from `dir`, then atomically warm the loaded-harness
+/// registry.  Callers MUST use this function (not `fs::remove_file` +
+/// `warm_harness_registry_from_dir` separately) for the same reason as
+/// `save_and_warm`.
+pub(crate) fn delete_and_warm(dir: &Path, id: &str) -> Result<(), String> {
+    let _guard = persist_mutex().lock().unwrap_or_else(|e| e.into_inner());
+    let target = dir.join(format!("{id}.json"));
+    match std::fs::remove_file(&target) {
+        Ok(()) => {}
+        Err(e) if e.kind() == std::io::ErrorKind::NotFound => {}
+        Err(e) => return Err(format!("failed to delete harness {id:?}: {e}")),
+    }
+    warm_harness_registry_from_dir(Some(dir));
+    Ok(())
+}
+
+/// The outcome of a successful [`save_custom_harness_to_dir`] call.
+#[allow(dead_code)] // Fields consumed by tests; production callers use the Result shape.
+pub(crate) struct SaveOutcome {
+    /// The path of the newly written harness file.
+    pub target_path: std::path::PathBuf,
+    /// The path of the old file that was removed on a rename, if any.
+    /// `None` when the id was unchanged or this was a new harness.
+    pub removed_old_path: Option<std::path::PathBuf>,
+}
+
+/// Write a harness definition to `dir/<id>.json` using a backup-swap strategy
+/// that is safe on all platforms (including Windows where `fs::rename` over an
+/// existing file fails with "access denied"):
+///
+/// 1. Serialize the definition and write it to a unique temp file via
+///    `atomic_write_file`.
+/// 2. If the target already exists, rename it to `<target>.bak` (the backup).
+/// 3. `commit()` the temp file (renames temp → target).
+///    * On success: delete `.bak` (best-effort; a stale `.bak` is harmless).
+///    * On failure: restore `.bak` → target so the original is never lost.
+/// 4. If `rename_old_id` is `Some`, remove `<dir>/<old_id>.json` after the
+///    new file is committed (non-fatal if NotFound).
+///
+/// The caller is responsible for full validation (id, env, etc.) BEFORE
+/// calling this function — no validation is performed here.
+pub(crate) fn save_custom_harness_to_dir(
+    dir: &Path,
+    definition: &HarnessDefinition,
+    rename_old_id: Option<&str>,
+) -> Result<SaveOutcome, String> {
+    use atomic_write_file::AtomicWriteFile;
+    use std::io::Write;
+
+    let json = serde_json::to_string_pretty(definition)
+        .map_err(|e| format!("failed to serialize harness definition: {e}"))?;
+
+    let target_path = dir.join(format!("{}.json", definition.id));
+    let bak_path = dir.join(format!("{}.json.bak", definition.id));
+
+    // Stage write to temp file.
+    let mut file = AtomicWriteFile::open(&target_path)
+        .map_err(|e| format!("failed to open {}: {e}", target_path.display()))?;
+    file.write_all(json.as_bytes())
+        .map_err(|e| format!("failed to write harness definition: {e}"))?;
+
+    // Back up the existing target before committing so we can restore on failure.
+    let had_backup = if target_path.exists() {
+        std::fs::rename(&target_path, &bak_path).map_err(|e| {
+            format!(
+                "failed to back up {} before replace: {e}",
+                target_path.display()
+            )
+        })?;
+        true
+    } else {
+        false
+    };
+
+    // Commit temp → target.  If commit fails and we made a backup, restore it.
+    if let Err(e) = file.commit() {
+        if had_backup {
+            if let Err(restore_err) = std::fs::rename(&bak_path, &target_path) {
+                // Both commit and restore failed — surface both so the user
+                // can recover manually.
+                return Err(format!(
+                    "failed to finalize harness definition: {e}; \
+                     also failed to restore backup: {restore_err} \
+                     (original may be at {})",
+                    bak_path.display()
+                ));
+            }
+        }
+        return Err(format!("failed to finalize harness definition: {e}"));
+    }
+
+    // Commit succeeded — remove the backup (best-effort; stale .bak is harmless).
+    if had_backup {
+        let _ = std::fs::remove_file(&bak_path);
+    }
+
+    // Remove old file on id rename, after the new file is committed.
+    let mut removed_old_path = None;
+    if let Some(old_id) = rename_old_id {
+        let old_path = dir.join(format!("{old_id}.json"));
+        match std::fs::remove_file(&old_path) {
+            Ok(()) => removed_old_path = Some(old_path),
+            Err(e) if e.kind() == std::io::ErrorKind::NotFound => {}
+            Err(e) => {
+                // New file is already committed — log and continue so the
+                // registry re-warm picks up the new id.
+                tracing::warn!(
+                    "save_custom_harness_to_dir: failed to remove old harness {old_id:?}: {e}"
+                );
+            }
+        }
+    }
+
+    Ok(SaveOutcome {
+        target_path,
+        removed_old_path,
+    })
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -495,68 +644,280 @@ mod tests {
         assert!(check_id_collision("custom-goose").is_ok());
     }
 
-    // ── Round-trip: save → load → delete → load ──────────────────────────────
+    // ── Round-trip via save_custom_harness_to_dir (B-4) ─────────────────────
+    //
+    // These tests exercise the REAL persistence helper, not raw fs::write.
+    // They prove: create, same-ID edit (backup-swap), rename (old file removed),
+    // backup file cleaned up on success.
+
+    fn make_def(id: &str, label: &str) -> HarnessDefinition {
+        HarnessDefinition {
+            id: id.to_string(),
+            label: label.to_string(),
+            command: format!("{id}-bin"),
+            args: vec![],
+            env: BTreeMap::new(),
+            install_instructions_url: String::new(),
+            install_hint: String::new(),
+        }
+    }
 
     #[test]
-    fn round_trip_save_then_load_then_delete() {
+    fn save_to_dir_create_writes_file_and_loads_back() {
         let dir = tempfile::tempdir().unwrap();
-        let mut env_map = BTreeMap::new();
-        env_map.insert("MY_KEY".to_string(), "my_value".to_string());
-        let def = HarnessDefinition {
-            id: "my-rt".to_string(),
-            label: "My Runtime".to_string(),
-            command: "my-rt-bin".to_string(),
-            args: vec!["--flag".to_string()],
-            env: env_map,
-            install_instructions_url: "https://example.com".to_string(),
-            install_hint: "Install from example.com".to_string(),
-        };
+        let def = make_def("my-harness", "My Harness");
 
-        // Serialize and write (simulating save_custom_harness logic).
-        let json = serde_json::to_string_pretty(&def).unwrap();
-        let target = dir.path().join(format!("{}.json", def.id));
-        fs::write(&target, &json).unwrap();
+        let outcome = save_custom_harness_to_dir(dir.path(), &def, None).unwrap();
+
+        assert_eq!(outcome.target_path, dir.path().join("my-harness.json"));
+        assert!(outcome.removed_old_path.is_none(), "no old file on create");
 
-        // Load should return exactly one entry.
         let loaded = load_custom_harnesses(dir.path());
         assert_eq!(loaded.len(), 1);
-        assert_eq!(loaded[0].id, "my-rt");
-        assert_eq!(loaded[0].command, "my-rt-bin");
-        assert_eq!(loaded[0].args, vec!["--flag"]);
-        assert_eq!(
-            loaded[0].env.get("MY_KEY").map(String::as_str),
-            Some("my_value")
-        );
+        assert_eq!(loaded[0].id, "my-harness");
+        assert_eq!(loaded[0].label, "My Harness");
+    }
 
-        // Delete the file (simulating delete_custom_harness).
-        fs::remove_file(&target).unwrap();
+    #[test]
+    fn save_to_dir_same_id_edit_replaces_content() {
+        let dir = tempfile::tempdir().unwrap();
+        let v1 = make_def("my-harness", "V1 Label");
+        save_custom_harness_to_dir(dir.path(), &v1, None).unwrap();
 
-        // Load should now return an empty list.
-        let after_delete = load_custom_harnesses(dir.path());
+        // Same-ID edit: label changes.
+        let v2 = make_def("my-harness", "V2 Label");
+        let outcome = save_custom_harness_to_dir(dir.path(), &v2, None).unwrap();
+
+        // No old-path reported (id unchanged).
+        assert!(outcome.removed_old_path.is_none());
+
+        let loaded = load_custom_harnesses(dir.path());
+        assert_eq!(loaded.len(), 1, "same-id edit must not duplicate entries");
+        assert_eq!(loaded[0].label, "V2 Label", "v2 content must be present");
+    }
+
+    #[test]
+    fn save_to_dir_backup_is_cleaned_up_after_same_id_edit() {
+        let dir = tempfile::tempdir().unwrap();
+        let v1 = make_def("my-harness", "V1");
+        save_custom_harness_to_dir(dir.path(), &v1, None).unwrap();
+
+        let v2 = make_def("my-harness", "V2");
+        save_custom_harness_to_dir(dir.path(), &v2, None).unwrap();
+
+        // .bak file must be gone after a successful commit.
+        let bak = dir.path().join("my-harness.json.bak");
         assert!(
-            after_delete.is_empty(),
-            "directory should be empty after delete"
+            !bak.exists(),
+            ".bak file must be removed after successful same-id edit"
         );
     }
 
     #[test]
-    fn round_trip_overwrite_existing_definition() {
+    fn save_to_dir_rename_removes_old_file_and_creates_new() {
         let dir = tempfile::tempdir().unwrap();
-        let path = dir.path().join("rt.json");
+        let old_def = make_def("old-id", "Old");
+        save_custom_harness_to_dir(dir.path(), &old_def, None).unwrap();
 
-        // Write v1.
-        fs::write(&path, r#"{"id":"rt","label":"V1","command":"rt-bin"}"#).unwrap();
+        // Rename: new id, old_id supplied.
+        let new_def = make_def("new-id", "New");
+        let outcome = save_custom_harness_to_dir(dir.path(), &new_def, Some("old-id")).unwrap();
 
-        let v1 = load_custom_harnesses(dir.path());
-        assert_eq!(v1[0].label, "V1");
+        // The outcome carries the old path that was removed.
+        let expected_old = dir.path().join("old-id.json");
+        assert_eq!(
+            outcome.removed_old_path,
+            Some(expected_old.clone()),
+            "removed_old_path must be the old file"
+        );
 
-        // Overwrite with v2 (simulates save on an existing definition).
-        fs::write(&path, r#"{"id":"rt","label":"V2","command":"rt-bin-v2"}"#).unwrap();
+        // Old file gone, new file present.
+        assert!(!expected_old.exists(), "old-id.json must be removed");
+        let loaded = load_custom_harnesses(dir.path());
+        assert_eq!(loaded.len(), 1);
+        assert_eq!(loaded[0].id, "new-id");
+    }
 
-        let v2 = load_custom_harnesses(dir.path());
-        assert_eq!(v2.len(), 1, "overwrite must not duplicate entries");
-        assert_eq!(v2[0].label, "V2");
-        assert_eq!(v2[0].command, "rt-bin-v2");
+    #[test]
+    fn save_to_dir_rename_nonexistent_old_id_is_non_fatal() {
+        // rename_old_id pointing to a file that does not exist must succeed
+        // (NotFound is silently ignored by the helper).
+        let dir = tempfile::tempdir().unwrap();
+        let def = make_def("alpha", "Alpha");
+        let outcome = save_custom_harness_to_dir(dir.path(), &def, Some("ghost-id")).unwrap();
+
+        // New file created, no old path removed.
+        assert_eq!(outcome.target_path, dir.path().join("alpha.json"));
+        assert!(
+            outcome.removed_old_path.is_none(),
+            "NotFound old-id must not be reported as removed"
+        );
+        assert!(load_custom_harnesses(dir.path()).len() == 1);
+    }
+
+    #[test]
+    fn save_to_dir_roundtrip_with_env_preserves_values() {
+        let dir = tempfile::tempdir().unwrap();
+        let mut env = BTreeMap::new();
+        env.insert("MY_KEY".to_string(), "my_value".to_string());
+        let def = HarnessDefinition {
+            id: "env-harness".to_string(),
+            label: "Env Harness".to_string(),
+            command: "env-bin".to_string(),
+            args: vec!["--flag".to_string()],
+            env,
+            install_instructions_url: "https://example.com".to_string(),
+            install_hint: "Install from example.com".to_string(),
+        };
+
+        save_custom_harness_to_dir(dir.path(), &def, None).unwrap();
+
+        let loaded = load_custom_harnesses(dir.path());
+        assert_eq!(loaded.len(), 1);
+        assert_eq!(loaded[0].args, vec!["--flag"]);
+        assert_eq!(
+            loaded[0].env.get("MY_KEY").map(String::as_str),
+            Some("my_value"),
+            "env must round-trip through save_custom_harness_to_dir"
+        );
+    }
+
+    // ── B-3: env validation boundary (validate_harness_definition_pub integration) ──
+
+    #[test]
+    fn validate_rejects_malformed_key_with_equals_sign() {
+        // BUZZ_AUTH_TAG=x is the documented reserved-key bypass shape:
+        // the key contains '=' so Command::env would produce
+        // `BUZZ_AUTH_TAG=x=forged` in the child env.
+        let mut env = BTreeMap::new();
+        env.insert("BUZZ_AUTH_TAG=x".to_string(), "forged".to_string());
+        let def = HarnessDefinition {
+            id: "bad-env".to_string(),
+            label: "Bad".to_string(),
+            command: "bad-bin".to_string(),
+            args: vec![],
+            env,
+            install_instructions_url: String::new(),
+            install_hint: String::new(),
+        };
+        let err = validate_harness_definition_pub(&def).unwrap_err();
+        assert!(
+            err.contains("env var keys must match"),
+            "malformed key must be rejected: {err}"
+        );
+        assert!(
+            err.contains("BUZZ_AUTH_TAG"),
+            "error must name the offending key: {err}"
+        );
+    }
+
+    #[test]
+    fn validate_rejects_reserved_key_buzz_managed_agent() {
+        // BUZZ_MANAGED_AGENT and BUZZ_MANAGED_AGENT_START_NONCE are the
+        // ownership markers — supplying them in a definition must be rejected.
+        let mut env = BTreeMap::new();
+        env.insert(
+            "BUZZ_MANAGED_AGENT".to_string(),
+            "fake-instance".to_string(),
+        );
+        let def = HarnessDefinition {
+            id: "bad-marker".to_string(),
+            label: "Bad".to_string(),
+            command: "bad-bin".to_string(),
+            args: vec![],
+            env,
+            install_instructions_url: String::new(),
+            install_hint: String::new(),
+        };
+        let err = validate_harness_definition_pub(&def).unwrap_err();
+        assert!(
+            err.contains("reserved by Buzz"),
+            "ownership marker key must be rejected: {err}"
+        );
+    }
+
+    #[test]
+    fn validate_rejects_reserved_key_case_insensitive() {
+        // BUZZ_PRIVATE_KEY in any casing must be blocked.
+        let mut env = BTreeMap::new();
+        env.insert("buzz_private_key".to_string(), "secret".to_string());
+        let def = HarnessDefinition {
+            id: "ci-marker".to_string(),
+            label: "CI".to_string(),
+            command: "ci-bin".to_string(),
+            args: vec![],
+            env,
+            install_instructions_url: String::new(),
+            install_hint: String::new(),
+        };
+        let err = validate_harness_definition_pub(&def).unwrap_err();
+        assert!(
+            err.contains("reserved by Buzz"),
+            "reserved key must be blocked case-insensitively: {err}"
+        );
+    }
+
+    #[test]
+    fn validate_rejects_nul_byte_in_value() {
+        // A NUL in a value would cause Command::env to panic at spawn time.
+        let mut env = BTreeMap::new();
+        env.insert("MY_KEY".to_string(), "val\x00ue".to_string());
+        let def = HarnessDefinition {
+            id: "nul-val".to_string(),
+            label: "NUL".to_string(),
+            command: "nul-bin".to_string(),
+            args: vec![],
+            env,
+            install_instructions_url: String::new(),
+            install_hint: String::new(),
+        };
+        let err = validate_harness_definition_pub(&def).unwrap_err();
+        assert!(
+            err.contains("NUL bytes"),
+            "NUL value must be rejected at validation: {err}"
+        );
+    }
+
+    #[test]
+    fn validate_rejects_value_over_per_value_size_limit() {
+        use crate::managed_agents::env_vars::MAX_ENV_VALUE_BYTES;
+        let mut env = BTreeMap::new();
+        // One byte over the per-value cap.
+        env.insert("BIG_VAL".to_string(), "x".repeat(MAX_ENV_VALUE_BYTES + 1));
+        let def = HarnessDefinition {
+            id: "big-val".to_string(),
+            label: "Big".to_string(),
+            command: "big-bin".to_string(),
+            args: vec![],
+            env,
+            install_instructions_url: String::new(),
+            install_hint: String::new(),
+        };
+        let err = validate_harness_definition_pub(&def).unwrap_err();
+        assert!(
+            err.contains("per-value limit"),
+            "oversized value must be rejected: {err}"
+        );
+    }
+
+    #[test]
+    fn validate_accepts_well_formed_env() {
+        let mut env = BTreeMap::new();
+        env.insert("ANTHROPIC_API_KEY".to_string(), "sk-test-123".to_string());
+        env.insert("MODEL_VERSION".to_string(), "claude-3".to_string());
+        let def = HarnessDefinition {
+            id: "good-env".to_string(),
+            label: "Good".to_string(),
+            command: "good-bin".to_string(),
+            args: vec![],
+            env,
+            install_instructions_url: String::new(),
+            install_hint: String::new(),
+        };
+        assert!(
+            validate_harness_definition_pub(&def).is_ok(),
+            "well-formed definition must pass validation"
+        );
     }
 
     // ── Registry warm path ───────────────────────────────────────────────────

desktop/src/shared/api/tauri.test.mjs

@@ -112,6 +112,105 @@ test("relay rate-limited: prefix check is case-sensitive (Rust always emits lowe
   );
 });
 
+// ── fromRawAcpRuntimeCatalogEntry: custom row API-boundary (B-2) ─────────────
+//
+// These tests feed real raw custom catalog rows through fromRawAcpRuntimeCatalogEntry
+// and verify the Rust→TypeScript mapping boundary: definition_env (snake_case)
+// arrives as definitionEnv (camelCase), source "custom" is preserved, and the
+// env round-trips end-to-end so a save-then-edit cycle cannot erase env.
+
+const { fromRawAcpRuntimeCatalogEntry } = await import("./tauri.ts");
+
+test("fromRawAcpRuntimeCatalogEntry maps definition_env to definitionEnv", () => {
+  const raw = {
+    id: "my-harness",
+    label: "My Harness",
+    availability: "available",
+    command: "my-bin",
+    source: "custom",
+    definition_env: { ANTHROPIC_API_KEY: "sk-test", MODEL: "claude-3" },
+    default_args: [],
+    can_auto_install: false,
+    requires_external_cli: false,
+    install_hint: "",
+    install_instructions_url: "",
+  };
+  const entry = fromRawAcpRuntimeCatalogEntry(raw);
+  assert.deepStrictEqual(entry.definitionEnv, {
+    ANTHROPIC_API_KEY: "sk-test",
+    MODEL: "claude-3",
+  });
+  assert.equal(entry.source, "custom");
+});
+
+test("fromRawAcpRuntimeCatalogEntry defaults definitionEnv to {} when absent", () => {
+  // Rust serialization skips empty BTreeMap, so definition_env will be absent
+  // for harnesses with no env defined — the mapper must default to {}.
+  const raw = {
+    id: "no-env-harness",
+    label: "No Env",
+    availability: "available",
+    command: "no-env-bin",
+    source: "custom",
+    default_args: [],
+    can_auto_install: false,
+    requires_external_cli: false,
+    install_hint: "",
+    install_instructions_url: "",
+  };
+  const entry = fromRawAcpRuntimeCatalogEntry(raw);
+  assert.deepStrictEqual(
+    entry.definitionEnv,
+    {},
+    "absent definition_env must map to empty object, not undefined",
+  );
+});
+
+test("fromRawAcpRuntimeCatalogEntry preserves source preset", () => {
+  const raw = {
+    id: "cursor",
+    label: "Cursor",
+    availability: "available",
+    command: "cursor",
+    source: "preset",
+    default_args: [],
+    can_auto_install: false,
+    requires_external_cli: false,
+    install_hint: "",
+    install_instructions_url: "",
+  };
+  const entry = fromRawAcpRuntimeCatalogEntry(raw);
+  assert.equal(entry.source, "preset");
+  assert.deepStrictEqual(entry.definitionEnv, {});
+});
+
+test("fromRawAcpRuntimeCatalogEntry env round-trips through edit payload shape", () => {
+  // Simulate the full save → re-open cycle: raw entry comes back from Rust
+  // with definition_env populated; the edit form reads entry.definitionEnv.
+  // Verify the env values are identical before and after the mapper.
+  const envValues = { OPENAI_API_KEY: "sk-live-abc", REGION: "us-east-1" };
+  const raw = {
+    id: "openai-harness",
+    label: "OpenAI",
+    availability: "not_installed",
+    command: "openai-agent",
+    source: "custom",
+    definition_env: envValues,
+    default_args: ["--acp"],
+    can_auto_install: false,
+    requires_external_cli: true,
+    install_hint: "Install the OpenAI CLI",
+    install_instructions_url: "https://platform.openai.com/docs",
+  };
+  const entry = fromRawAcpRuntimeCatalogEntry(raw);
+  // The edit form reads entry.definitionEnv; it must equal the original env.
+  assert.deepStrictEqual(
+    entry.definitionEnv,
+    envValues,
+    "env must round-trip: edit form must see the same values that Rust serialized",
+  );
+});
+
 // ── Teardown ──────────────────────────────────────────────────────────────────
 
 test("teardown — restore Date.now", () => {

desktop/src/shared/api/tauri.ts

@@ -733,7 +733,7 @@ export function fromRawManagedAgent(agent: RawManagedAgent): ManagedAgent {
   };
 }
 
-function fromRawAcpRuntimeCatalogEntry(
+export function fromRawAcpRuntimeCatalogEntry(
   entry: RawAcpRuntimeCatalogEntry,
 ): AcpRuntimeCatalogEntry {
   return {