From 4da3e46d9627c31115390b26e386690cd0a859bd Mon Sep 17 00:00:00 2001 From: Codex 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 { +async fn bootstrap_if_invalid(app: &AppHandle, bridge: &str) -> Result { 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 { 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 { +pub async fn set_dev_pin(bridge: &str, on: bool, source: &str) -> Result { + 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>>>> = 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 { - 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