From 851f00a6b68d0b0954c53ce766005f836d8e9d39 Mon Sep 17 00:00:00 2001 From: daniel Date: Sun, 4 Oct 2026 18:02:31 +0100 Subject: [PATCH 1/2] fix: retry failed subagent directory initialization --- src/tools/subagent.rs | 34 +++++++++++++--------- src/tools/subagent/tests.rs | 56 +++++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 13 deletions(-) diff --git a/src/tools/subagent.rs b/src/tools/subagent.rs index 9be409f..ec27a62 100644 --- a/src/tools/subagent.rs +++ b/src/tools/subagent.rs @@ -41,22 +41,30 @@ pub(crate) struct Permit { _slot: std::fs::File, } -type TreeSlots = Arc>>; +#[derive(Debug, Default)] +struct TreeSlotDirectory { + path: std::sync::OnceLock, + initialized: std::sync::OnceLock<()>, +} + +type TreeSlots = Arc; /// Slot directory shared by every Kit process in one delegation tree. fn tree_slot_directory(slots: &TreeSlots) -> Result<&PathBuf, ChildError> { - slots - .get_or_init(|| { - let directory = match std::env::var_os(TREE_SLOTS_ENV) { - Some(directory) => PathBuf::from(directory), - None => std::env::temp_dir().join(format!("kit-subagents-{}", session::new_id())), - }; - std::fs::create_dir_all(&directory) - .map_err(|error| format!("could not create {}: {error}", directory.display()))?; - Ok(directory) - }) - .as_ref() - .map_err(|error| ChildError::Failed(error.clone())) + let directory = slots.path.get_or_init(|| match std::env::var_os(TREE_SLOTS_ENV) { + Some(directory) => PathBuf::from(directory), + None => std::env::temp_dir().join(format!("kit-subagents-{}", session::new_id())), + }); + // Cache only success: failed attempts retry the same path. Once initialized, + // callers can reuse the path without new filesystem errors after reserving a slot. + if slots.initialized.get().is_none() { + std::fs::create_dir_all(directory).map_err(|error| { + ChildError::Failed(format!("could not create {}: {error}", directory.display())) + })?; + // Concurrent successful creators publish the same fact; neither owns cleanup. + let _ = slots.initialized.set(()); + } + Ok(directory) } /// Holds one of the tree-wide slots until dropped or the process exits. diff --git a/src/tools/subagent/tests.rs b/src/tools/subagent/tests.rs index 3a6d92e..f9c5677 100644 --- a/src/tools/subagent/tests.rs +++ b/src/tools/subagent/tests.rs @@ -2,6 +2,62 @@ use std::path::{Path, PathBuf}; use super::*; +#[test] +fn tree_slot_directory_failure_is_retryable() { + let root = tempfile::tempdir().unwrap(); + let blocker = root.path().join("blocked"); + std::fs::write(&blocker, b"not a directory").unwrap(); + let directory = blocker.join("slots"); + let manager = manager_with_generic_harness(root.path(), Vec::new()); + manager.tree_slots.path.set(directory.clone()).unwrap(); + + for _ in 0..2 { + let error = manager.acquire_permit().unwrap_err(); + assert!(error.to_string().contains("could not create")); + assert_eq!(manager.capacity.available_permits(), MAX_LIVE_SUBAGENTS); + } + + std::fs::remove_file(&blocker).unwrap(); + let permit = manager.acquire_permit().unwrap(); + assert!(directory.is_dir()); + assert_eq!(tree_slot_directory(&manager.tree_slots).unwrap(), &directory); + drop(permit); + assert_eq!(manager.capacity.available_permits(), MAX_LIVE_SUBAGENTS); +} + +#[test] +fn tree_slot_directory_is_shared_across_concurrent_callers() { + let root = tempfile::tempdir().unwrap(); + let directory = root.path().join("slots"); + let slots = TreeSlots::default(); + slots.path.set(directory.clone()).unwrap(); + + std::thread::scope(|scope| { + for _ in 0..8 { + let slots = Arc::clone(&slots); + let expected = &directory; + scope.spawn(move || { + assert_eq!(tree_slot_directory(&slots).unwrap(), expected); + let _permit = tree_slot(expected).unwrap(); + }); + } + }); + assert!(directory.is_dir()); +} + +#[test] +fn tree_slot_directory_success_does_not_recheck_filesystem() { + let root = tempfile::tempdir().unwrap(); + let directory = root.path().join("slots"); + let slots = TreeSlots::default(); + slots.path.set(directory.clone()).unwrap(); + assert_eq!(tree_slot_directory(&slots).unwrap(), &directory); + + std::fs::remove_dir(&directory).unwrap(); + std::fs::write(&directory, b"not a directory").unwrap(); + assert_eq!(tree_slot_directory(&slots).unwrap(), &directory); +} + fn listing_id(value: &SubagentListing) -> &str { &value.id } From 2ca3fc0cb780438db347b44df87a9b0806769bab Mon Sep 17 00:00:00 2001 From: daniel Date: Sun, 4 Oct 2026 18:03:51 +0100 Subject: [PATCH 2/2] style: format subagent initialization regression tests --- src/tools/subagent.rs | 10 ++++++---- src/tools/subagent/tests.rs | 5 ++++- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/tools/subagent.rs b/src/tools/subagent.rs index ec27a62..b158a2d 100644 --- a/src/tools/subagent.rs +++ b/src/tools/subagent.rs @@ -51,10 +51,12 @@ type TreeSlots = Arc; /// Slot directory shared by every Kit process in one delegation tree. fn tree_slot_directory(slots: &TreeSlots) -> Result<&PathBuf, ChildError> { - let directory = slots.path.get_or_init(|| match std::env::var_os(TREE_SLOTS_ENV) { - Some(directory) => PathBuf::from(directory), - None => std::env::temp_dir().join(format!("kit-subagents-{}", session::new_id())), - }); + let directory = slots + .path + .get_or_init(|| match std::env::var_os(TREE_SLOTS_ENV) { + Some(directory) => PathBuf::from(directory), + None => std::env::temp_dir().join(format!("kit-subagents-{}", session::new_id())), + }); // Cache only success: failed attempts retry the same path. Once initialized, // callers can reuse the path without new filesystem errors after reserving a slot. if slots.initialized.get().is_none() { diff --git a/src/tools/subagent/tests.rs b/src/tools/subagent/tests.rs index f9c5677..920fc0f 100644 --- a/src/tools/subagent/tests.rs +++ b/src/tools/subagent/tests.rs @@ -20,7 +20,10 @@ fn tree_slot_directory_failure_is_retryable() { std::fs::remove_file(&blocker).unwrap(); let permit = manager.acquire_permit().unwrap(); assert!(directory.is_dir()); - assert_eq!(tree_slot_directory(&manager.tree_slots).unwrap(), &directory); + assert_eq!( + tree_slot_directory(&manager.tree_slots).unwrap(), + &directory + ); drop(permit); assert_eq!(manager.capacity.available_permits(), MAX_LIVE_SUBAGENTS); }