From 95a28a9e8feaa17af6368317caca9f71b52260db Mon Sep 17 00:00:00 2001 From: 0xLeif Date: Fri, 12 Jun 2026 08:43:54 -0600 Subject: [PATCH 1/3] =?UTF-8?q?Fix:=20atomic=20plugins.toml=20writes=20?= =?UTF-8?q?=E2=80=94=20registry=20race=20made=20plugin=20subcommands=20van?= =?UTF-8?q?ish?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit save_registry used fs::write, which truncates in place. Any fledge invocation reading the registry during a re-registration parsed partial TOML, and dispatch silently resolved every registered plugin command to 'unrecognized subcommand'. Observed in the wild: merlin's CI trust-gate failing 'fledge run augur-gate' with 'unrecognized subcommand augur' whenever a plugin registration (merlin init/update, fledge plugins install) overlapped the gate — three times in two days, each costing a misleading red build. - save_registry: write-then-rename (same-dir rename is atomic on POSIX); readers now see either the old or the new registry, never a truncated one - load_registry: one 50ms retry rides out rewrites by older (pre-atomic) writers, and persistent corruption prints a warning naming the path instead of silently resolving as an empty registry - Tests: a writer-hammer concurrency regression (400 reads against a continuous rewriter) and a corruption-surfaces-error case Co-Authored-By: Claude Fable 5 --- src/plugin/mod.rs | 44 +++++++++++++++++++++++++++++++----- src/plugin/tests.rs | 55 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 94 insertions(+), 5 deletions(-) diff --git a/src/plugin/mod.rs b/src/plugin/mod.rs index 8ba27e6..047ba08 100644 --- a/src/plugin/mod.rs +++ b/src/plugin/mod.rs @@ -169,7 +169,7 @@ impl PluginHooks { } } -#[derive(Debug, Serialize, Deserialize)] +#[derive(Debug, Serialize, Deserialize, Clone)] struct PluginsRegistry { #[serde(default)] pub(super) plugins: Vec, @@ -387,23 +387,57 @@ fn registry_path() -> PathBuf { } fn load_registry() -> Result { - let path = registry_path(); + load_registry_from(®istry_path()) +} + +fn load_registry_from(path: &Path) -> Result { if !path.exists() { return Ok(PluginsRegistry { plugins: Vec::new(), }); } - let content = fs::read_to_string(&path).context("reading plugins.toml")?; + match read_and_parse_registry(path) { + Ok(registry) => Ok(registry), + Err(first_error) => { + // A registration by an older fledge (pre-atomic-write) may be + // mid-rewrite; one short retry rides out the window instead of + // resolving every plugin subcommand to "unrecognized". + std::thread::sleep(std::time::Duration::from_millis(50)); + read_and_parse_registry(path).map_err(|_| { + eprintln!( + "warning: plugins.toml at {} is unreadable or corrupt — plugin \ + subcommands will not resolve this invocation: {first_error:#}", + path.display() + ); + first_error + }) + } + } +} + +fn read_and_parse_registry(path: &Path) -> Result { + let content = fs::read_to_string(path).context("reading plugins.toml")?; toml::from_str(&content).context("parsing plugins.toml") } fn save_registry(registry: &PluginsRegistry) -> Result<()> { - let path = registry_path(); + save_registry_to(®istry_path(), registry) +} + +fn save_registry_to(path: &Path, registry: &PluginsRegistry) -> Result<()> { if let Some(parent) = path.parent() { fs::create_dir_all(parent)?; } let content = toml::to_string_pretty(registry).context("serializing plugins.toml")?; - fs::write(&path, content).context("writing plugins.toml") + // Write-then-rename so concurrent readers never observe a truncated + // file. `fs::write` truncates in place, and any fledge invocation + // that reads the registry during the rewrite parses partial TOML — + // dispatch then reports a perfectly registered plugin command as + // "unrecognized subcommand". Same-directory rename is atomic on + // POSIX, so readers see either the old registry or the new one. + let tmp = path.with_extension(format!("toml.tmp.{}", std::process::id())); + fs::write(&tmp, content).context("writing plugins.toml temp file")?; + fs::rename(&tmp, path).context("atomically replacing plugins.toml") } // ─── Source helpers ────────────────────────────────────────────────────────── diff --git a/src/plugin/tests.rs b/src/plugin/tests.rs index 91c2271..ca58a91 100644 --- a/src/plugin/tests.rs +++ b/src/plugin/tests.rs @@ -1445,3 +1445,58 @@ fn team_tier_allows_exec() { }; assert!(check_tier_capabilities(TrustTier::Team, &caps).is_ok()); } + +#[test] +fn registry_save_is_atomic_under_concurrent_readers() { + // Regression for the "unrecognized subcommand" race: `fs::write` + // truncates plugins.toml in place, so a reader racing a re-registration + // parsed partial TOML and dispatch dropped every plugin command. With + // write-then-rename, readers must see a complete registry every time. + let tmp = tempfile::tempdir().unwrap(); + let path = tmp.path().join("plugins.toml"); + let entry = super::PluginEntry { + name: "fledge-plugin-augur".to_string(), + source: "CorvidLabs/fledge-plugin-augur".to_string(), + version: "0.2.0".to_string(), + installed: "2026-06-12".to_string(), + commands: vec!["augur".to_string(); 64], + pinned_ref: None, + capabilities: None, + runtime: None, + }; + let registry = super::PluginsRegistry { + plugins: vec![entry; 24], + }; + super::save_registry_to(&path, ®istry).unwrap(); + + let writer_path = path.clone(); + let writer_registry = registry.clone(); + let stop = std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false)); + let writer_stop = stop.clone(); + let writer = std::thread::spawn(move || { + while !writer_stop.load(std::sync::atomic::Ordering::Relaxed) { + super::save_registry_to(&writer_path, &writer_registry).unwrap(); + } + }); + + for _ in 0..400 { + let loaded = super::load_registry_from(&path) + .expect("reader must never observe a truncated registry"); + assert_eq!(loaded.plugins.len(), 24); + assert_eq!(loaded.plugins[0].commands.len(), 64); + } + stop.store(true, std::sync::atomic::Ordering::Relaxed); + writer.join().unwrap(); +} + +#[test] +fn registry_load_reports_corruption_instead_of_empty() { + let tmp = tempfile::tempdir().unwrap(); + let path = tmp.path().join("plugins.toml"); + std::fs::write(&path, "[[plugins]]\nname = \"trunc").unwrap(); + let result = super::load_registry_from(&path); + assert!( + result.is_err(), + "corrupt registry must surface an error, not parse as empty" + ); +} From a745385d7cf7c1b6284136ea5e16c4f64f0bc9d1 Mon Sep 17 00:00:00 2001 From: 0xLeif Date: Fri, 12 Jun 2026 08:51:42 -0600 Subject: [PATCH 2/3] Retry the registry rename on Windows sharing violations Co-Authored-By: Claude Fable 5 --- src/plugin/mod.rs | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/src/plugin/mod.rs b/src/plugin/mod.rs index 047ba08..fd15207 100644 --- a/src/plugin/mod.rs +++ b/src/plugin/mod.rs @@ -437,7 +437,22 @@ fn save_registry_to(path: &Path, registry: &PluginsRegistry) -> Result<()> { // POSIX, so readers see either the old registry or the new one. let tmp = path.with_extension(format!("toml.tmp.{}", std::process::id())); fs::write(&tmp, content).context("writing plugins.toml temp file")?; - fs::rename(&tmp, path).context("atomically replacing plugins.toml") + // Windows refuses the rename with a sharing violation while another + // process has the destination open for reading; retry briefly. On + // POSIX the first attempt always wins. + let mut last_error = None; + for _ in 0..20 { + match fs::rename(&tmp, path) { + Ok(()) => return Ok(()), + Err(error) => { + last_error = Some(error); + std::thread::sleep(std::time::Duration::from_millis(10)); + } + } + } + let _ = fs::remove_file(&tmp); + Err(last_error.expect("rename attempted at least once")) + .context("atomically replacing plugins.toml") } // ─── Source helpers ────────────────────────────────────────────────────────── From 623a930b6c810376767e775d8db3762ee54f72c4 Mon Sep 17 00:00:00 2001 From: 0xLeif Date: Fri, 12 Jun 2026 09:12:44 -0600 Subject: [PATCH 3/3] Clean up the temp file when the registry write itself fails Co-Authored-By: Claude Fable 5 --- src/plugin/mod.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/plugin/mod.rs b/src/plugin/mod.rs index fd15207..4a89047 100644 --- a/src/plugin/mod.rs +++ b/src/plugin/mod.rs @@ -436,7 +436,10 @@ fn save_registry_to(path: &Path, registry: &PluginsRegistry) -> Result<()> { // "unrecognized subcommand". Same-directory rename is atomic on // POSIX, so readers see either the old registry or the new one. let tmp = path.with_extension(format!("toml.tmp.{}", std::process::id())); - fs::write(&tmp, content).context("writing plugins.toml temp file")?; + if let Err(error) = fs::write(&tmp, content) { + let _ = fs::remove_file(&tmp); + return Err(error).context("writing plugins.toml temp file"); + } // Windows refuses the rename with a sharing violation while another // process has the destination open for reading; retry briefly. On // POSIX the first attempt always wins.