KiCad - the KiCad Bridge

From 4da3e46d9627c31115390b26e386690cd0a859bd Mon Sep 17 00:00:00 2001
From: Codex <codex@localhost>
Date: Sat, 5 Sep 2026 12:53:14 -0500
Subject: [PATCH] Respect development pins during explicit installs and cache
 replacement

---
 cli/src/commands.rs              |   2 +-
 src-tauri/src/bridge_cache.rs    |  47 ++++++++++---
 src-tauri/src/cache_mutation.rs  | 111 +++++++++++++++++++++++++++++++
 src-tauri/src/commands.rs        |   9 ++-
 src-tauri/src/lib.rs             |   3 +-
 tests/dev-pin-harness/Cargo.lock |  72 ++++++++++++++++++++
 tests/dev-pin-harness/Cargo.toml |   6 ++
 tests/dev-pin-harness/README.md  |  23 +++++++
 tests/dev-pin-harness/src/lib.rs |   3 +
 9 files changed, 259 insertions(+), 17 deletions(-)
 create mode 100644 src-tauri/src/cache_mutation.rs
 create mode 100644 tests/dev-pin-harness/Cargo.lock
 create mode 100644 tests/dev-pin-harness/Cargo.toml
 create mode 100644 tests/dev-pin-harness/README.md
 create mode 100644 tests/dev-pin-harness/src/lib.rs

diff --git a/cli/src/commands.rs b/cli/src/commands.rs
index 6f3c1c32..f62ab8e2 100644
--- a/cli/src/commands.rs
+++ b/cli/src/commands.rs
@@ -1534,7 +1534,7 @@ pub fn list_commands() -> Value {
                 "returns": "{success, name, paused:true, _hint}"
             },
             "bridge_dev_mode": {
-                "description": "Freeze a bridge's cache against ALL automatic updates so you can develop it without ab clobbering your changes. {name, on:true} writes a .dev-pin in the cache dir; while pinned the 4h sweep, on-launch sync, on-use auto-update AND refresh_bridges all skip it (each says outcome:'skipped: dev pin'). {name, on:false} removes the pin and updates resume. The pin survives ab restarts and shows as devPinned:true in bridge_list. bridge_install with force:true is the one deliberate override.",
+                "description": "Freeze a bridge's cache against ALL automatic updates so you can develop it without ab clobbering your changes. {name, on:true} writes a .dev-pin in the cache dir; while pinned the 4h sweep, on-launch sync, on-use auto-update AND refresh_bridges all skip it (each says outcome:'skipped: dev pin'). {name, on:false} removes the pin and updates resume. The pin survives ab restarts and shows as devPinned:true in bridge_list. Explicit bridge_install respects the pin, including force:true. Unpin with bridge_dev_mode before installing a release.",
                 "args": {"name": "bridge name (from bridge_list)", "on": "true to pin (freeze updates), false to unpin"},
                 "returns": "{ok, name, devPinned, pinPath?}",
             },
diff --git a/src-tauri/src/bridge_cache.rs b/src-tauri/src/bridge_cache.rs
index 9665b2ae..06002c3e 100644
--- a/src-tauri/src/bridge_cache.rs
+++ b/src-tauri/src/bridge_cache.rs
@@ -573,7 +573,7 @@ fn hex_encode(bytes: &[u8]) -> String {
 /// needed (cache invalid AND a bundled seed exists). Returns whether it bootstrapped.
 /// Used by the offline / bundled≥wiki branches of `sync_one` — NOT the update-download
 /// path, which would discard the (huge, for a node bridge) seed copy anyway.
-fn bootstrap_if_invalid(app: &AppHandle, bridge: &str) -> Result<bool, String> {
+async fn bootstrap_if_invalid(app: &AppHandle, bridge: &str) -> Result<bool, String> {
     let cache = cache_dir(bridge);
     if cache_is_valid(&cache) {
         return Ok(false);
@@ -584,7 +584,7 @@ fn bootstrap_if_invalid(app: &AppHandle, bridge: &str) -> Result<bool, String> {
     if !has_bundled {
         return Ok(false);
     }
-    bootstrap_cache_from_bundled(app, bridge).map_err(|e| format!("bootstrap {bridge} cache: {e}"))?;
+    bootstrap_cache_from_bundled(app, bridge).await.map_err(|e| format!("bootstrap {bridge} cache: {e}"))?;
     Ok(true)
 }
 
@@ -634,7 +634,8 @@ pub fn is_dev_pinned(bridge: &str) -> bool {
 /// resulting pinned state; unpinning an already-unpinned bridge is Ok(false).
 /// Every flip is written to the bridge's lifecycle audit so "why did/didn't my
 /// bridge update?" is answerable from one bridge_info read.
-pub fn set_dev_pin(bridge: &str, on: bool, source: &str) -> Result<bool, String> {
+pub async fn set_dev_pin(bridge: &str, on: bool, source: &str) -> Result<bool, String> {
+    let _mutation = crate::cache_mutation::lock(&cache_dir(bridge)).await;
     let pin = dev_pin_path(bridge);
     if on {
         if let Some(parent) = pin.parent() {
@@ -727,7 +728,7 @@ pub async fn sync_one(app: &AppHandle, bridge: &str) -> Result<(String, String,
     let manifest: BridgeManifest = match crate::with_session_auth(client.get(&manifest_url)).send().await {
         Ok(resp) => {
             if !resp.status().is_success() {
-                let bootstrapped = bootstrap_if_invalid(app, bridge)?;
+                let bootstrapped = bootstrap_if_invalid(app, bridge).await?;
                 let action = if bootstrapped { "bootstrapped" } else { "current_offline" };
                 record_sync_detail(bridge, detail(None,
                     "failed: manifest not served at the probed tier (bridge not published there, or the page refused)"));
@@ -737,7 +738,7 @@ pub async fn sync_one(app: &AppHandle, bridge: &str) -> Result<(String, String,
                 Ok(m) => m,
                 Err(e) => {
                     log::warn!("[bridge_cache] {bridge} manifest parse failed: {e}");
-                    let bootstrapped = bootstrap_if_invalid(app, bridge)?;
+                    let bootstrapped = bootstrap_if_invalid(app, bridge).await?;
                     let action = if bootstrapped { "bootstrapped" } else { "current_offline" };
                     record_sync_detail(bridge, detail(None, "failed: manifest unparseable"));
                     return Ok((from.clone(), from, action.to_string()));
@@ -746,7 +747,7 @@ pub async fn sync_one(app: &AppHandle, bridge: &str) -> Result<(String, String,
         }
         Err(e) => {
             log::warn!("[bridge_cache] {bridge} manifest fetch failed (offline?): {e}");
-            let bootstrapped = bootstrap_if_invalid(app, bridge)?;
+            let bootstrapped = bootstrap_if_invalid(app, bridge).await?;
             let action = if bootstrapped { "bootstrapped" } else { "current_offline" };
             record_sync_detail(bridge, detail(None, "failed: wiki unreachable (network/timeout)"));
             return Ok((from.clone(), from, action.to_string()));
@@ -771,7 +772,7 @@ pub async fn sync_one(app: &AppHandle, bridge: &str) -> Result<(String, String,
     // "current_bundled_newer"; equal is "current_bundled".
     let bundled = bundled_version(app, bridge).unwrap_or_default();
     if !bundled.is_empty() && !version_gt(&manifest.version, &bundled) {
-        let _ = bootstrap_if_invalid(app, bridge);
+        let _ = bootstrap_if_invalid(app, bridge).await;
         let action = if version_gt(&bundled, &manifest.version) {
             "current_bundled_newer"
         } else {
@@ -818,7 +819,9 @@ pub async fn sync_one(app: &AppHandle, bridge: &str) -> Result<(String, String,
 
 /// Copy the bundled bridge dir into the cache and write a .bridge-version
 /// marker.
-fn bootstrap_cache_from_bundled(app: &AppHandle, bridge: &str) -> Result<(), String> {
+async fn bootstrap_cache_from_bundled(app: &AppHandle, bridge: &str) -> Result<(), String> {
+    let _mutation = crate::cache_mutation::lock(&cache_dir(bridge)).await;
+    crate::cache_mutation::require_unpinned(&cache_dir(bridge))?;
     let bundled = bundled_dir(app, bridge)?;
     if !bundled.is_dir() {
         return Err(format!(
@@ -887,7 +890,8 @@ async fn download_and_install(
     // client-level 20s timeout, and (c) reported truncation as
     // "error decoding response body" — a misleading message. Streaming
     // avoids all three.
-    let zip_path = cache_dir(bridge).with_extension("downloading.zip");
+    let pending_id = uuid::Uuid::new_v4();
+    let zip_path = cache_dir(bridge).with_extension(format!("{pending_id}.downloading.zip"));
     let _ = std::fs::remove_file(&zip_path);
     if let Some(parent) = zip_path.parent() {
         std::fs::create_dir_all(parent).map_err(|e| format!("create cache root: {e}"))?;
@@ -905,7 +909,7 @@ async fn download_and_install(
     }
 
     // Extract to a temp dir, then atomically swap with cache.
-    let temp = cache_dir(bridge).with_extension("new");
+    let temp = cache_dir(bridge).with_extension(format!("{pending_id}.new"));
     if temp.exists() {
         let _ = std::fs::remove_dir_all(&temp);
     }
@@ -965,6 +969,15 @@ async fn download_and_install(
     std::fs::write(temp.join(".bridge-version"), manifest.version.trim())
         .map_err(|e| format!("write .bridge-version: {e}"))?;
 
+    // A pin may have been set while downloading. Serialize the check, reap,
+    // and replacement with the Pin/Unpin verb and other cache writers.
+    let _mutation = crate::cache_mutation::lock(&cache_dir(bridge)).await;
+    if let Err(error) = crate::cache_mutation::require_unpinned(&cache_dir(bridge)) {
+        let _ = std::fs::remove_dir_all(&temp);
+        let _ = std::fs::remove_file(&zip_path);
+        return Err(error);
+    }
+
     // Atomic-ish swap: rename current cache out, rename temp in, delete old.
     // v1.9.27: if the rename fails with Windows ERROR_SHARING_VIOLATION (os 32)
     // because the bridge is RUNNING and holding the cache dir as its cwd (its
@@ -1822,6 +1835,20 @@ pub async fn install_from_manifest_url(
         });
     }
 
+    // Resolve the actual bridge name from the verified archive, not the URL.
+    // Explicit installs obey the same pin as automatic updates. Unpin is a
+    // separate deliberate action; force/retries never erase development work.
+    let _mutation = crate::cache_mutation::lock(&cache_dir(&canonical_name)).await;
+    if let Err(error) = crate::cache_mutation::require_unpinned(&cache_dir(&canonical_name)) {
+        let _ = std::fs::remove_dir_all(&temp);
+        let _ = std::fs::remove_file(&zip_path);
+        return serde_json::json!({
+            "ok": false, "errorCode": "dev_pinned", "devPinned": true,
+            "bridge": canonical_name, "error": error,
+            "_hint": "Development cache preserved. Use bridge_dev_mode {name, on:false} explicitly before installing a release."
+        });
+    }
+
     // Downloaded zip is no longer needed — bytes are now in the temp tree.
     let _ = std::fs::remove_file(&zip_path);
 
diff --git a/src-tauri/src/cache_mutation.rs b/src-tauri/src/cache_mutation.rs
new file mode 100644
index 00000000..80a2e6b2
--- /dev/null
+++ b/src-tauri/src/cache_mutation.rs
@@ -0,0 +1,111 @@
+//! Serialize in-process cache replacement with development pin changes.
+//! Manual marker-file writes are observed at the commit boundary; callers that
+//! require race-free pinning must use bridge_dev_mode or the GUI pin button.
+use std::collections::HashMap;
+use std::path::{Path, PathBuf};
+use std::sync::{Arc, Mutex, OnceLock};
+use tokio::sync::{Mutex as AsyncMutex, OwnedMutexGuard};
+
+pub async fn lock(cache: &Path) -> OwnedMutexGuard<()> {
+    static LOCKS: OnceLock<Mutex<HashMap<PathBuf, Arc<AsyncMutex<()>>>>> = OnceLock::new();
+    // Windows cache names are case-insensitive, even before the directory exists.
+    let key = if cfg!(windows) {
+        PathBuf::from(cache.to_string_lossy().to_lowercase())
+    } else {
+        cache.to_path_buf()
+    };
+    let mutex = {
+        let mut locks = LOCKS
+            .get_or_init(Mutex::default)
+            .lock()
+            .unwrap_or_else(|e| e.into_inner());
+        locks.entry(key).or_default().clone()
+    };
+    mutex.lock_owned().await
+}
+
+pub fn require_unpinned(cache: &Path) -> Result<(), String> {
+    match std::fs::symlink_metadata(cache.join(".dev-pin")) {
+        Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(()),
+        Err(e) => Err(format!("Cannot inspect development pin; cache preserved: {e}")),
+        Ok(_) => Err(format!(
+            "{} is DEV-PINNED; cache and running bridge preserved. Explicitly unpin with bridge_dev_mode before installing a release.",
+            cache.display()
+        )),
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    fn fixture() -> PathBuf {
+        let path = std::env::temp_dir().join(format!(
+            "ab-pin-{}-{}",
+            std::process::id(),
+            std::time::SystemTime::now()
+                .duration_since(std::time::UNIX_EPOCH)
+                .unwrap()
+                .as_nanos()
+        ));
+        std::fs::create_dir_all(&path).unwrap();
+        path
+    }
+
+    #[tokio::test]
+    async fn pin_set_during_download_prevents_commit() {
+        let cache = fixture();
+        std::fs::write(cache.join("server.py"), "development work").unwrap();
+        require_unpinned(&cache).unwrap(); // download starts
+        {
+            let _pin = lock(&cache).await;
+            std::fs::write(cache.join(".dev-pin"), "pinned").unwrap();
+        }
+        let _commit = lock(&cache).await;
+        assert!(require_unpinned(&cache).is_err());
+        assert_eq!(
+            std::fs::read_to_string(cache.join("server.py")).unwrap(),
+            "development work"
+        );
+        std::fs::remove_dir_all(cache).unwrap();
+    }
+
+    #[tokio::test]
+    async fn pin_cannot_acknowledge_during_replacement() {
+        let cache = fixture();
+        let commit = lock(&cache).await;
+        require_unpinned(&cache).unwrap();
+        let other = cache.clone();
+        let pin = tokio::spawn(async move {
+            let _guard = lock(&other).await;
+            std::fs::write(other.join(".dev-pin"), "pinned").unwrap();
+        });
+        tokio::task::yield_now().await;
+        assert!(!pin.is_finished());
+        drop(commit);
+        pin.await.unwrap();
+        assert!(require_unpinned(&cache).is_err());
+        std::fs::remove_file(cache.join(".dev-pin")).unwrap();
+        require_unpinned(&cache).unwrap();
+        std::fs::remove_dir_all(cache).unwrap();
+    }
+
+    #[tokio::test]
+    async fn independent_bridges_do_not_block_each_other() {
+        let a = fixture();
+        let b = fixture();
+        let _a = lock(&a).await;
+        let _b = tokio::time::timeout(std::time::Duration::from_secs(1), lock(&b))
+            .await
+            .unwrap();
+        std::fs::remove_dir_all(a).unwrap();
+        std::fs::remove_dir_all(b).unwrap();
+    }
+
+    #[test]
+    fn even_a_directory_marker_blocks_install() {
+        let cache = fixture();
+        std::fs::create_dir(cache.join(".dev-pin")).unwrap();
+        assert!(require_unpinned(&cache).is_err());
+        std::fs::remove_dir_all(cache).unwrap();
+    }
+}
diff --git a/src-tauri/src/commands.rs b/src-tauri/src/commands.rs
index 1b7a2698..0ddb1c81 100644
--- a/src-tauri/src/commands.rs
+++ b/src-tauri/src/commands.rs
@@ -5957,9 +5957,8 @@ async fn handle_restart_bridge(
 /// refresh_bridges). While pinned a developer can modify the cache dir freely
 /// without ab pulling the insiders/public release over their work. It writes a
 /// `.dev-pin` marker in the cache dir, so the pin survives ab restarts and is
-/// visible (bridge_list rows show `devPinned:true`). `bridge_install ... force`
-/// is the one deliberate override — a human explicitly asking to replace the
-/// cache — but the passive sweep never wins.
+/// visible (bridge_list rows show `devPinned:true`). Explicit installs also respect the pin;
+/// unpin with bridge_dev_mode before installing a release.
 async fn handle_bridge_dev_mode(
     app: &AppHandle,
     ctx: &WsContext,
@@ -5991,11 +5990,11 @@ async fn handle_bridge_dev_mode(
     let caller_tag = crate::caller::Caller::from_args(args, Some(&ctx.server_name))
         .ai_thread
         .unwrap_or_else(|| ctx.server_name.clone());
-    let result_json = match crate::bridge_cache::set_dev_pin(&name, on, &format!("bridge_dev_mode verb, {caller_tag}")) {
+    let result_json = match crate::bridge_cache::set_dev_pin(&name, on, &format!("bridge_dev_mode verb, {caller_tag}")).await {
         Ok(true) => serde_json::json!({
             "ok": true, "name": name, "devPinned": true,
             "pinPath": crate::bridge_cache::dev_pin_path(&name).to_string_lossy(),
-            "_hint": "DEV MODE ON: every automatic update for this bridge is frozen (the 4h sweep, on-launch sync, on-use auto-update, refresh_bridges all skip it). Modify the cache dir freely. Call bridge_dev_mode {\"name\":\"...\",\"on\":false} to resume updates. bridge_install with force:true is the one deliberate override. The bridge's card in the ab window shows an amber 'dev pin' badge while pinned.",
+            "_hint": "DEV MODE ON: every automatic update for this bridge is frozen (the 4h sweep, on-launch sync, on-use auto-update, refresh_bridges all skip it). Modify the cache dir freely. Call bridge_dev_mode {\"name\":\"...\",\"on\":false} to resume updates. Explicit bridge_install also respects the pin, including force:true; unpin before installing a release. The bridge's card in the ab window shows an amber 'dev pin' badge while pinned.",
         }),
         Ok(false) => serde_json::json!({
             "ok": true, "name": name, "devPinned": false,
diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs
index 8df61912..713fd96d 100644
--- a/src-tauri/src/lib.rs
+++ b/src-tauri/src/lib.rs
@@ -2,6 +2,7 @@ mod ai_thread;
 mod auth;
 mod bridge_autoupdate;
 mod bridge_cache;
+mod cache_mutation;
 mod bridge_langs;
 mod bridge_readiness;
 mod bridge_registry;
@@ -3008,7 +3009,7 @@ async fn decline_generic_prompts(app: AppHandle) -> Result<(), String> {
 /// with one click instead of a CLI call. Returns the resulting pinned state.
 #[tauri::command]
 async fn set_bridge_dev_pin(app: AppHandle, name: String, on: bool) -> Result<bool, String> {
-    let pinned = bridge_cache::set_dev_pin(&name, on, "ab window button")?;
+    let pinned = bridge_cache::set_dev_pin(&name, on, "ab window button").await?;
     let _ = app.emit(
         "bridge-registry-changed",
         serde_json::json!({"reason": "dev-pin", "name": name, "devPinned": pinned}),
diff --git a/tests/dev-pin-harness/Cargo.lock b/tests/dev-pin-harness/Cargo.lock
new file mode 100644
index 00000000..1b950821
--- /dev/null
+++ b/tests/dev-pin-harness/Cargo.lock
@@ -0,0 +1,72 @@
+# This file is automatically @generated by Cargo.
+# It is not intended for manual editing.
+version = 4
+
+[[package]]
+name = "ab-dev-pin-tests"
+version = "0.1.0"
+dependencies = [
+ "tokio",
+]
+
+[[package]]
+name = "pin-project-lite"
+version = "0.2.17"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "a89322df9ebe1c1578d689c92318e070967d1042b512afbe49518723f4e6d5cd"
+
+[[package]]
+name = "proc-macro2"
+version = "1.0.107"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "985e7ec9bb745e6ce6535b544d84d6cd6f7ad8bd711c398938ae983b91a766d9"
+dependencies = [
+ "unicode-ident",
+]
+
+[[package]]
+name = "quote"
+version = "1.0.47"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "1fbf4db142a473a8d80c26bbf18454ed458bf8d26c8219c331daecfdbd079001"
+dependencies = [
+ "proc-macro2",
+]
+
+[[package]]
+name = "syn"
+version = "3.0.5"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "12df2e0110f65b775f769bb17ef989067a1d931b2eb822bd4346631eeada89f9"
+dependencies = [
+ "proc-macro2",
+ "quote",
+ "unicode-ident",
+]
+
+[[package]]
+name = "tokio"
+version = "1.53.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "202caea871b69668250d242070849eb495be178ed697a3e98aebce5bc81a0bed"
+dependencies = [
+ "pin-project-lite",
+ "tokio-macros",
+]
+
+[[package]]
+name = "tokio-macros"
+version = "2.7.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "78773a2a397f451582ce068015985c33193cf6dea8b74d2a639fe457b2f07b0e"
+dependencies = [
+ "proc-macro2",
+ "quote",
+ "syn",
+]
+
+[[package]]
+name = "unicode-ident"
+version = "1.0.24"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "e6e4313cd5fcd3dad5cafa179702e2b244f760991f45397d14d4ebf38247da75"
diff --git a/tests/dev-pin-harness/Cargo.toml b/tests/dev-pin-harness/Cargo.toml
new file mode 100644
index 00000000..9fa1dd7c
--- /dev/null
+++ b/tests/dev-pin-harness/Cargo.toml
@@ -0,0 +1,6 @@
+[package]
+name = "ab-dev-pin-tests"
+version = "0.1.0"
+edition = "2021"
+[dependencies]
+tokio = { version = "1", features = ["sync", "rt", "macros", "time"] }
diff --git a/tests/dev-pin-harness/README.md b/tests/dev-pin-harness/README.md
new file mode 100644
index 00000000..16b65d9e
--- /dev/null
+++ b/tests/dev-pin-harness/README.md
@@ -0,0 +1,23 @@
+# Development pin regression checks
+
+Run `cargo test --manifest-path tests/dev-pin-harness/Cargo.toml` on Linux or
+Windows. This harness compiles the production cache mutation guard directly,
+without the Windows/Tauri GUI dependency tree.
+
+The fix serializes Pin/Unpin, bundled bootstrap, automatic cache replacement,
+and explicit manifest installation. Explicit installation resolves the actual
+bridge name from the verified archive before checking the pin, and returns
+`dev_pinned` before reaping or changing the cache. `force:true` is not a bypass;
+unpinning is a separate deliberate operation. Downloads use independent staging
+paths so overlapping automatic downloads cannot overwrite each other's staging.
+
+Tests cover pins set during download, pin acknowledgement waiting for replacement,
+independent bridges, and malformed marker directories. The synchronization covers
+one ab process; hand-written marker files are checked at the commit boundary but
+cannot participate in the in-process lock. Use the verb or GUI pin button.
+
+Validation still required before release: full Windows application compilation
+and live refresh/install checks, including an install request with force:true.
+Verify cache hashes and running PID are preserved for a pinned bridge, then
+explicitly unpin and confirm a normal install succeeds. The standalone guard tests
+do not substitute for those integration checks.
diff --git a/tests/dev-pin-harness/src/lib.rs b/tests/dev-pin-harness/src/lib.rs
new file mode 100644
index 00000000..cfeb943a
--- /dev/null
+++ b/tests/dev-pin-harness/src/lib.rs
@@ -0,0 +1,3 @@
+// Compile the production guard directly without Windows/Tauri dependencies.
+#[path = "../../../src-tauri/src/cache_mutation.rs"]
+pub mod cache_mutation;
-- 
2.43.0