From da418a136f78ce0949f7ab3b3294f19829c47258 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Mon, 12 Oct 2026 17:16:08 +0300 Subject: [PATCH 01/45] test(mcp): add owner tests module for module client Register a feature-gated test module for the MCP module client so owner related tests can be compiled only when the modules feature is enabled. Auto-committed-on: dragonfly Co-authored-by: Medulla --- .../openhuman-core/src/mcp/module_client.rs | 4 + .../src/mcp/module_client_owner_tests.rs | 82 +++++++++++++++++++ 2 files changed, 86 insertions(+) create mode 100644 crates/openhuman-core/src/mcp/module_client_owner_tests.rs diff --git a/crates/openhuman-core/src/mcp/module_client.rs b/crates/openhuman-core/src/mcp/module_client.rs index 7ba7b72d373..45164fbb6fc 100644 --- a/crates/openhuman-core/src/mcp/module_client.rs +++ b/crates/openhuman-core/src/mcp/module_client.rs @@ -743,3 +743,7 @@ impl McpConfiguredClient { #[cfg(test)] #[path = "module_client_tests.rs"] mod tests; + +#[cfg(all(test, feature = "modules"))] +#[path = "module_client_owner_tests.rs"] +mod owner_tests; diff --git a/crates/openhuman-core/src/mcp/module_client_owner_tests.rs b/crates/openhuman-core/src/mcp/module_client_owner_tests.rs new file mode 100644 index 00000000000..db8d908ff04 --- /dev/null +++ b/crates/openhuman-core/src/mcp/module_client_owner_tests.rs @@ -0,0 +1,82 @@ +use super::*; +use std::sync::atomic::AtomicUsize; +use tinybus::{broker::Broker, transport::memory::MemoryBus, Connection, ObjectPath}; + +#[derive(Clone, Default)] +struct LifecycleFixture { + closes: Arc, + polls: Arc, +} + +#[tinybus::interface(name = "ai.tinyhumans.tinymcp.Mcp")] +impl LifecycleFixture { + async fn close(&self, _data_dir: String) -> tinybus::Result> { + self.closes.fetch_add(1, Ordering::SeqCst); + Ok(Some("original-close".into())) + } + + async fn transition_status(&self, _receipt: String) -> tinybus::Result { + self.polls.fetch_add(1, Ordering::SeqCst); + Ok(TransitionStatus::Complete) + } +} + +#[tokio::test] +async fn cleanup_receipts_stay_with_the_original_module_after_alias_replacement() { + let bus = MemoryBus::new(); + Broker::new().spawn(bus.clone()); + let original = Connection::connect(bus.connect().await.unwrap()) + .await + .unwrap(); + let replacement = Connection::connect(bus.connect().await.unwrap()) + .await + .unwrap(); + let caller = Connection::connect(bus.connect().await.unwrap()) + .await + .unwrap(); + let original_fixture = LifecycleFixture::default(); + let replacement_fixture = LifecycleFixture::default(); + for (connection, fixture) in [ + (&original, original_fixture.clone()), + (&replacement, replacement_fixture.clone()), + ] { + connection + .serve_at(ObjectPath::new(names::OBJECT_PATH).unwrap(), fixture) + .await + .unwrap(); + } + original.request_name(names::INTERFACE).await.unwrap(); + let root = ModuleClient::fixture( + caller + .proxy(names::INTERFACE, names::OBJECT_PATH, names::INTERFACE) + .unwrap(), + ); + let port = TinyMcpPort { root }; + // Resolve the original owner through a real bus call. No secret is sent + // to these deliberately unattested lifecycle fixtures. + assert_eq!( + port.transition_status("original-open").await, + Ok(TransitionStatus::Complete) + ); + let client = McpConfiguredClient::with_port("/test/agent".into(), Box::new(port)); + *client.state.lock().await = State::Open(OpenObject { + config: McpClientConfig::default(), + object_path: "/test/original-child".into(), + }); + original.release_name(names::INTERFACE).await.unwrap(); + replacement.request_name(names::INTERFACE).await.unwrap(); + + client.close().await.unwrap(); + assert_eq!( + original_fixture.closes.load(Ordering::SeqCst), + 1, + "cleanup must address the original module instance" + ); + assert_eq!( + original_fixture.polls.load(Ordering::SeqCst), + 2, + "the close receipt must be polled on its original owner" + ); + assert_eq!(replacement_fixture.closes.load(Ordering::SeqCst), 0); + assert_eq!(replacement_fixture.polls.load(Ordering::SeqCst), 0); +} From 88b83534b4f0d901f0f0ccdc05a2eb5666433a5f Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Mon, 12 Oct 2026 17:18:33 +0300 Subject: [PATCH 02/45] test(mcp): cover cancelled close retrying on the original owner Add a test that aborts a close while its transition poll is in flight and verifies the retry polls the retained receipt on the original module owner rather than the replacement. The lifecycle fixture gains a gate to block the close poll and signal when it has started, so the cancellation can be timed deterministically. Auto-committed-on: dragonfly Co-authored-by: Medulla --- .../src/mcp/module_client_owner_tests.rs | 78 ++++++++++++++++++- 1 file changed, 77 insertions(+), 1 deletion(-) diff --git a/crates/openhuman-core/src/mcp/module_client_owner_tests.rs b/crates/openhuman-core/src/mcp/module_client_owner_tests.rs index db8d908ff04..0ecdb11a433 100644 --- a/crates/openhuman-core/src/mcp/module_client_owner_tests.rs +++ b/crates/openhuman-core/src/mcp/module_client_owner_tests.rs @@ -6,6 +6,8 @@ use tinybus::{broker::Broker, transport::memory::MemoryBus, Connection, ObjectPa struct LifecycleFixture { closes: Arc, polls: Arc, + block_close_poll: Arc, + close_poll_started: Arc, } #[tinybus::interface(name = "ai.tinyhumans.tinymcp.Mcp")] @@ -15,12 +17,86 @@ impl LifecycleFixture { Ok(Some("original-close".into())) } - async fn transition_status(&self, _receipt: String) -> tinybus::Result { + async fn transition_status(&self, receipt: String) -> tinybus::Result { self.polls.fetch_add(1, Ordering::SeqCst); + if receipt == "original-close" && self.block_close_poll.swap(false, Ordering::SeqCst) { + self.close_poll_started.notify_one(); + std::future::pending::<()>().await; + } Ok(TransitionStatus::Complete) } } +#[tokio::test] +async fn cancelled_close_retries_its_receipt_on_the_original_owner() { + let bus = MemoryBus::new(); + Broker::new().spawn(bus.clone()); + let original = Connection::connect(bus.connect().await.unwrap()) + .await + .unwrap(); + let replacement = Connection::connect(bus.connect().await.unwrap()) + .await + .unwrap(); + let caller = Connection::connect(bus.connect().await.unwrap()) + .await + .unwrap(); + let original_fixture = LifecycleFixture::default(); + let replacement_fixture = LifecycleFixture::default(); + for (connection, fixture) in [ + (&original, original_fixture.clone()), + (&replacement, replacement_fixture.clone()), + ] { + connection + .serve_at(ObjectPath::new(names::OBJECT_PATH).unwrap(), fixture) + .await + .unwrap(); + } + original.request_name(names::INTERFACE).await.unwrap(); + let port = TinyMcpPort { + root: ModuleClient::fixture( + caller + .proxy(names::INTERFACE, names::OBJECT_PATH, names::INTERFACE) + .unwrap(), + ), + }; + assert_eq!( + port.transition_status("original-open").await, + Ok(TransitionStatus::Complete) + ); + let client = Arc::new(McpConfiguredClient::with_port( + "/test/agent".into(), + Box::new(port), + )); + *client.state.lock().await = State::Open(OpenObject { + config: McpClientConfig::default(), + object_path: "/test/original-child".into(), + }); + original_fixture + .block_close_poll + .store(true, Ordering::SeqCst); + let closing = tokio::spawn({ + let client = Arc::clone(&client); + async move { client.close().await } + }); + original_fixture.close_poll_started.notified().await; + closing.abort(); + assert!(closing.await.unwrap_err().is_cancelled()); + assert!( + matches!(&*client.state.lock().await, State::Closing { transition_id, .. } if transition_id == "original-close") + ); + original.release_name(names::INTERFACE).await.unwrap(); + replacement.request_name(names::INTERFACE).await.unwrap(); + client.close().await.unwrap(); + assert_eq!(original_fixture.closes.load(Ordering::SeqCst), 1); + assert_eq!( + original_fixture.polls.load(Ordering::SeqCst), + 3, + "retry must poll the retained receipt on the original owner" + ); + assert_eq!(replacement_fixture.closes.load(Ordering::SeqCst), 0); + assert_eq!(replacement_fixture.polls.load(Ordering::SeqCst), 0); +} + #[tokio::test] async fn cleanup_receipts_stay_with_the_original_module_after_alias_replacement() { let bus = MemoryBus::new(); From 0cf63f9882498abdf2d4444505dc2a45a2bb4651 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Mon, 12 Oct 2026 17:18:56 +0300 Subject: [PATCH 03/45] test(mcp): make close poll release deterministic in owner tests The lifecycle fixture now waits on a release notification instead of a never-resolving future, so the blocked close poll can be unblocked explicitly. The test also bounds the wait for the close receipt with a timeout and releases the poll after aborting, removing the risk of a hang and making the cancellation path deterministic. Auto-committed-on: dragonfly Co-authored-by: Medulla --- .../src/mcp/module_client_owner_tests.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/crates/openhuman-core/src/mcp/module_client_owner_tests.rs b/crates/openhuman-core/src/mcp/module_client_owner_tests.rs index 0ecdb11a433..7fa0aa26b62 100644 --- a/crates/openhuman-core/src/mcp/module_client_owner_tests.rs +++ b/crates/openhuman-core/src/mcp/module_client_owner_tests.rs @@ -8,6 +8,7 @@ struct LifecycleFixture { polls: Arc, block_close_poll: Arc, close_poll_started: Arc, + release_close_poll: Arc, } #[tinybus::interface(name = "ai.tinyhumans.tinymcp.Mcp")] @@ -21,7 +22,7 @@ impl LifecycleFixture { self.polls.fetch_add(1, Ordering::SeqCst); if receipt == "original-close" && self.block_close_poll.swap(false, Ordering::SeqCst) { self.close_poll_started.notify_one(); - std::future::pending::<()>().await; + self.release_close_poll.notified().await; } Ok(TransitionStatus::Complete) } @@ -78,9 +79,15 @@ async fn cancelled_close_retries_its_receipt_on_the_original_owner() { let client = Arc::clone(&client); async move { client.close().await } }); - original_fixture.close_poll_started.notified().await; + tokio::time::timeout( + Duration::from_secs(10), + original_fixture.close_poll_started.notified(), + ) + .await + .expect("close receipt reaches the original module"); closing.abort(); assert!(closing.await.unwrap_err().is_cancelled()); + original_fixture.release_close_poll.notify_one(); assert!( matches!(&*client.state.lock().await, State::Closing { transition_id, .. } if transition_id == "original-close") ); From a23d47ceb98a9ef81bd47e6c967eec565fc6d3e3 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Mon, 12 Oct 2026 17:32:41 +0300 Subject: [PATCH 04/45] feat(modules): pin module client to its unique peer Add pin_owner to resolve the current unique owner of a module and return a client bound to that peer, so later alias replacement cannot redirect operations. Add object_at to derive a private child client on the already captured peer. Auto-committed-on: dragonfly Co-authored-by: Medulla --- crates/openhuman-core/src/modules/client.rs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/crates/openhuman-core/src/modules/client.rs b/crates/openhuman-core/src/modules/client.rs index 4e33edb4676..9ce2849042a 100644 --- a/crates/openhuman-core/src/modules/client.rs +++ b/crates/openhuman-core/src/modules/client.rs @@ -270,6 +270,24 @@ pub(crate) struct ModuleObjectClient { #[cfg(feature = "modules")] impl ModuleObjectClient { + /// Capture the current unique peer before starting an owned lifecycle. + /// Later alias replacement cannot redirect operations on this client. + pub(crate) async fn pin_owner(&self) -> Result { + let owner = self.proxy.connection().name_owner(self.proxy.destination()).await + .map_err(|error| report_proxy_failure(self.record, &error))? + .ok_or(ModuleCallError::Unavailable)?; + let proxy = self.proxy.connection().proxy(owner.as_str(), self.proxy.path().as_str(), self.proxy.interface().as_str()) + .map_err(|error| report_proxy_failure(self.record, &error))?; + Ok(Self { record: self.record, proxy }) + } + + /// Derive a private child on the already captured module peer. + pub(crate) fn object_at(&self, path: &str) -> Result { + let proxy = self.proxy.connection().proxy(self.proxy.destination().as_str(), path, self.proxy.interface().as_str()) + .map_err(|error| report_proxy_failure(self.record, &error))?; + Ok(Self { record: self.record, proxy }) + } + pub(crate) async fn call( &self, member: &str, From 824e35466a6d06b7c49a765b55a22486762aa92c Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Mon, 12 Oct 2026 17:33:31 +0300 Subject: [PATCH 05/45] refactor(mcp): move TinyMcpPort into its own module Extract the TinyMcpPort implementation from module_client.rs into a new module_port.rs file and give it a constructor instead of exposing its root field. This keeps the client file focused on the configured client and encapsulates the port's internals. Auto-committed-on: dragonfly Co-authored-by: Medulla --- .../openhuman-core/src/mcp/module_client.rs | 116 +-------------- .../src/mcp/module_client_owner_tests.rs | 8 +- crates/openhuman-core/src/mcp/module_port.rs | 137 ++++++++++++++++++ 3 files changed, 144 insertions(+), 117 deletions(-) create mode 100644 crates/openhuman-core/src/mcp/module_port.rs diff --git a/crates/openhuman-core/src/mcp/module_client.rs b/crates/openhuman-core/src/mcp/module_client.rs index 45164fbb6fc..1c0205ad0d2 100644 --- a/crates/openhuman-core/src/mcp/module_client.rs +++ b/crates/openhuman-core/src/mcp/module_client.rs @@ -114,115 +114,9 @@ pub(crate) trait McpModulePort: Send + Sync { } } -struct TinyMcpPort { - root: ModuleClient, -} - -#[async_trait] -impl McpModulePort for TinyMcpPort { - async fn open_configured( - &self, - data_dir: &str, - config: &McpClientConfig, - ) -> Result { - self.root - .call_confidential( - MODULE, - names::methods::OPEN_CONFIGURED, - (OpenConfiguredRequest { - data_dir: data_dir.to_owned(), - client: config.clone(), - },), - ) - .await - } - - async fn close(&self, data_dir: &str) -> Result, ModuleCallError> { - self.root - .call(MODULE, names::methods::CLOSE, (data_dir.to_owned(),)) - .await - } - - async fn transition_status( - &self, - transition_id: &str, - ) -> Result { - self.root - .call( - MODULE, - names::methods::TRANSITION_STATUS, - (transition_id.to_owned(),), - ) - .await - } - - async fn call_object( - &self, - object_path: &str, - member: &str, - args: Value, - confidential: bool, - ) -> Result { - #[cfg(feature = "modules")] - { - let object = self.root.object_client(MODULE, object_path).await?; - if confidential { - object.call_confidential(member, args).await - } else { - object.call(member, args).await - } - } - #[cfg(not(feature = "modules"))] - { - let _ = (object_path, member, args, confidential); - Err(ModuleCallError::Unavailable) - } - } - - async fn prepare_agent_tools( - &self, - request: PrepareAgentToolsRequest, - ) -> Result, ModuleCallError> { - self.root - .call(MODULE, names::methods::PREPARE_AGENT_TOOLS, (request,)) - .await - } - - async fn validate_server_arguments( - &self, - request: ValidateServerArgumentsRequest, - ) -> Result { - self.root - .call_confidential( - MODULE, - names::methods::VALIDATE_SERVER_ARGUMENTS, - (request,), - ) - .await - } - - async fn normalize_tool_arguments( - &self, - arguments: Option, - ) -> Result, ModuleCallError> { - self.root - .call_confidential( - MODULE, - names::methods::NORMALIZE_TOOL_ARGUMENTS, - (arguments,), - ) - .await - } - - async fn transform_text( - &self, - request: tinymcp_bus::TransformTextRequest, - ) -> Result { - self.root - .call_confidential(MODULE, names::methods::TRANSFORM_TEXT, (request,)) - .await - } -} +#[path = "module_port.rs"] +mod module_port; +use module_port::TinyMcpPort; /// One workspace's serialized TinyMCP lifecycle and private object proxy. pub(crate) struct McpConfiguredClient { @@ -342,9 +236,7 @@ impl McpConfiguredClient { .to_owned(); Ok(Self::with_port( data_dir, - Box::new(TinyMcpPort { - root: ModuleClient::new(config.clone()), - }), + Box::new(TinyMcpPort::new(ModuleClient::new(config.clone()))), )) } diff --git a/crates/openhuman-core/src/mcp/module_client_owner_tests.rs b/crates/openhuman-core/src/mcp/module_client_owner_tests.rs index 7fa0aa26b62..4af9880fe6c 100644 --- a/crates/openhuman-core/src/mcp/module_client_owner_tests.rs +++ b/crates/openhuman-core/src/mcp/module_client_owner_tests.rs @@ -53,13 +53,11 @@ async fn cancelled_close_retries_its_receipt_on_the_original_owner() { .unwrap(); } original.request_name(names::INTERFACE).await.unwrap(); - let port = TinyMcpPort { - root: ModuleClient::fixture( + let port = TinyMcpPort::new(ModuleClient::fixture( caller .proxy(names::INTERFACE, names::OBJECT_PATH, names::INTERFACE) .unwrap(), - ), - }; + )); assert_eq!( port.transition_status("original-open").await, Ok(TransitionStatus::Complete) @@ -134,7 +132,7 @@ async fn cleanup_receipts_stay_with_the_original_module_after_alias_replacement( .proxy(names::INTERFACE, names::OBJECT_PATH, names::INTERFACE) .unwrap(), ); - let port = TinyMcpPort { root }; + let port = TinyMcpPort::new(root); // Resolve the original owner through a real bus call. No secret is sent // to these deliberately unattested lifecycle fixtures. assert_eq!( diff --git a/crates/openhuman-core/src/mcp/module_port.rs b/crates/openhuman-core/src/mcp/module_port.rs new file mode 100644 index 00000000000..c48fcd0ecfa --- /dev/null +++ b/crates/openhuman-core/src/mcp/module_port.rs @@ -0,0 +1,137 @@ +//! Bus adapter pinned to the module instance that starts its lifecycle. +use super::*; +#[cfg(feature = "modules")] +use crate::modules::client::ModuleObjectClient; + +pub(super) struct TinyMcpPort { + root: ModuleClient, + #[cfg(feature = "modules")] + pinned: tokio::sync::OnceCell, +} + +impl TinyMcpPort { + pub(super) fn new(root: ModuleClient) -> Self { + Self { root, #[cfg(feature = "modules")] pinned: tokio::sync::OnceCell::new() } + } + + #[cfg(feature = "modules")] + async fn pinned(&self) -> Result<&ModuleObjectClient, ModuleCallError> { + self.pinned.get_or_try_init(|| async { + self.root.object_client(MODULE, names::OBJECT_PATH).await?.pin_owner().await + }).await + } + + async fn root_call(&self, member: &str, args: impl Serialize, confidential: bool) -> Result { + #[cfg(feature = "modules")] + { + let root = self.pinned().await?; + if confidential { root.call_confidential(member, args).await } + else { root.call(member, args).await } + } + #[cfg(not(feature = "modules"))] + { + if confidential { self.root.call_confidential(MODULE, member, args).await } + else { self.root.call(MODULE, member, args).await } + } + } +} + +#[async_trait] +impl McpModulePort for TinyMcpPort { + async fn open_configured( + &self, + data_dir: &str, + config: &McpClientConfig, + ) -> Result { + self.root_call( + names::methods::OPEN_CONFIGURED, + (OpenConfiguredRequest { + data_dir: data_dir.to_owned(), + client: config.clone(), + },), + true, + ) + .await + } + + async fn close(&self, data_dir: &str) -> Result, ModuleCallError> { + self.root_call(names::methods::CLOSE, (data_dir.to_owned(),), false) + .await + } + + async fn transition_status( + &self, + transition_id: &str, + ) -> Result { + self.root_call( + names::methods::TRANSITION_STATUS, + (transition_id.to_owned(),), + false, + ) + .await + } + + async fn call_object( + &self, + object_path: &str, + member: &str, + args: Value, + confidential: bool, + ) -> Result { + #[cfg(feature = "modules")] + { + let object = self.pinned().await?.object_at(object_path)?; + if confidential { + object.call_confidential(member, args).await + } else { + object.call(member, args).await + } + } + #[cfg(not(feature = "modules"))] + { + let _ = (object_path, member, args, confidential); + Err(ModuleCallError::Unavailable) + } + } + + async fn prepare_agent_tools( + &self, + request: PrepareAgentToolsRequest, + ) -> Result, ModuleCallError> { + self.root_call(names::methods::PREPARE_AGENT_TOOLS, (request,), false) + .await + } + + async fn validate_server_arguments( + &self, + request: ValidateServerArgumentsRequest, + ) -> Result { + self.root_call( + names::methods::VALIDATE_SERVER_ARGUMENTS, + (request,), + true, + ) + .await + } + + async fn normalize_tool_arguments( + &self, + arguments: Option, + ) -> Result, ModuleCallError> { + self.root_call( + names::methods::NORMALIZE_TOOL_ARGUMENTS, + (arguments,), + true, + ) + .await + } + + async fn transform_text( + &self, + request: tinymcp_bus::TransformTextRequest, + ) -> Result { + self.root_call(names::methods::TRANSFORM_TEXT, (request,), true) + .await + } +} + From 53ee360857a45b198d694019e0863bc573da94ac Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Mon, 12 Oct 2026 17:33:54 +0300 Subject: [PATCH 06/45] fix(modules): preserve call timeout on pinned and derived object clients ModuleObjectClient now carries the configured timeout and reapplies it when pinning an owner or deriving a child object, so calls made through those clients no longer lose the timeout set on the root client. Auto-committed-on: dragonfly Co-authored-by: Medulla --- .../src/mcp/module_client_owner_tests.rs | 8 +- crates/openhuman-core/src/mcp/module_port.rs | 78 +++++++++++-------- crates/openhuman-core/src/modules/client.rs | 55 +++++++++++-- .../src/modules/client_tests.rs | 6 +- 4 files changed, 102 insertions(+), 45 deletions(-) diff --git a/crates/openhuman-core/src/mcp/module_client_owner_tests.rs b/crates/openhuman-core/src/mcp/module_client_owner_tests.rs index 4af9880fe6c..3d0cd37b6bb 100644 --- a/crates/openhuman-core/src/mcp/module_client_owner_tests.rs +++ b/crates/openhuman-core/src/mcp/module_client_owner_tests.rs @@ -54,10 +54,10 @@ async fn cancelled_close_retries_its_receipt_on_the_original_owner() { } original.request_name(names::INTERFACE).await.unwrap(); let port = TinyMcpPort::new(ModuleClient::fixture( - caller - .proxy(names::INTERFACE, names::OBJECT_PATH, names::INTERFACE) - .unwrap(), - )); + caller + .proxy(names::INTERFACE, names::OBJECT_PATH, names::INTERFACE) + .unwrap(), + )); assert_eq!( port.transition_status("original-open").await, Ok(TransitionStatus::Complete) diff --git a/crates/openhuman-core/src/mcp/module_port.rs b/crates/openhuman-core/src/mcp/module_port.rs index c48fcd0ecfa..76480c04752 100644 --- a/crates/openhuman-core/src/mcp/module_port.rs +++ b/crates/openhuman-core/src/mcp/module_port.rs @@ -11,27 +11,48 @@ pub(super) struct TinyMcpPort { impl TinyMcpPort { pub(super) fn new(root: ModuleClient) -> Self { - Self { root, #[cfg(feature = "modules")] pinned: tokio::sync::OnceCell::new() } + Self { + root, + #[cfg(feature = "modules")] + pinned: tokio::sync::OnceCell::new(), + } } #[cfg(feature = "modules")] async fn pinned(&self) -> Result<&ModuleObjectClient, ModuleCallError> { - self.pinned.get_or_try_init(|| async { - self.root.object_client(MODULE, names::OBJECT_PATH).await?.pin_owner().await - }).await + self.pinned + .get_or_try_init(|| async { + self.root + .object_client(MODULE, names::OBJECT_PATH) + .await? + .pin_owner() + .await + }) + .await } - async fn root_call(&self, member: &str, args: impl Serialize, confidential: bool) -> Result { + async fn root_call( + &self, + member: &str, + args: impl Serialize, + confidential: bool, + ) -> Result { #[cfg(feature = "modules")] { let root = self.pinned().await?; - if confidential { root.call_confidential(member, args).await } - else { root.call(member, args).await } + if confidential { + root.call_confidential(member, args).await + } else { + root.call(member, args).await + } } #[cfg(not(feature = "modules"))] { - if confidential { self.root.call_confidential(MODULE, member, args).await } - else { self.root.call(MODULE, member, args).await } + if confidential { + self.root.call_confidential(MODULE, member, args).await + } else { + self.root.call(MODULE, member, args).await + } } } } @@ -44,14 +65,14 @@ impl McpModulePort for TinyMcpPort { config: &McpClientConfig, ) -> Result { self.root_call( - names::methods::OPEN_CONFIGURED, - (OpenConfiguredRequest { - data_dir: data_dir.to_owned(), - client: config.clone(), - },), - true, - ) - .await + names::methods::OPEN_CONFIGURED, + (OpenConfiguredRequest { + data_dir: data_dir.to_owned(), + client: config.clone(), + },), + true, + ) + .await } async fn close(&self, data_dir: &str) -> Result, ModuleCallError> { @@ -64,11 +85,11 @@ impl McpModulePort for TinyMcpPort { transition_id: &str, ) -> Result { self.root_call( - names::methods::TRANSITION_STATUS, - (transition_id.to_owned(),), - false, - ) - .await + names::methods::TRANSITION_STATUS, + (transition_id.to_owned(),), + false, + ) + .await } async fn call_object( @@ -106,11 +127,7 @@ impl McpModulePort for TinyMcpPort { &self, request: ValidateServerArgumentsRequest, ) -> Result { - self.root_call( - names::methods::VALIDATE_SERVER_ARGUMENTS, - (request,), - true, - ) + self.root_call(names::methods::VALIDATE_SERVER_ARGUMENTS, (request,), true) .await } @@ -118,11 +135,7 @@ impl McpModulePort for TinyMcpPort { &self, arguments: Option, ) -> Result, ModuleCallError> { - self.root_call( - names::methods::NORMALIZE_TOOL_ARGUMENTS, - (arguments,), - true, - ) + self.root_call(names::methods::NORMALIZE_TOOL_ARGUMENTS, (arguments,), true) .await } @@ -134,4 +147,3 @@ impl McpModulePort for TinyMcpPort { .await } } - diff --git a/crates/openhuman-core/src/modules/client.rs b/crates/openhuman-core/src/modules/client.rs index 9ce2849042a..a18ecc2b8ff 100644 --- a/crates/openhuman-core/src/modules/client.rs +++ b/crates/openhuman-core/src/modules/client.rs @@ -165,7 +165,11 @@ impl ModuleClient { let proxy = self .timeout .map_or(proxy.clone(), |timeout| proxy.with_timeout(timeout)); - return Ok(ModuleObjectClient { record, proxy }); + return Ok(ModuleObjectClient { + record, + proxy, + timeout: self.timeout, + }); } super::ops::ensure_loaded(&self.config, module) .await @@ -181,7 +185,11 @@ impl ModuleClient { let proxy = self .timeout .map_or(proxy.clone(), |timeout| proxy.with_timeout(timeout)); - Ok(ModuleObjectClient { record, proxy }) + Ok(ModuleObjectClient { + record, + proxy, + timeout: self.timeout, + }) } /// Report a structurally invalid reply from a known module invocation. @@ -266,6 +274,7 @@ impl ModuleClient { pub(crate) struct ModuleObjectClient { record: &'static super::types::ModuleRecord, proxy: tinybus::Proxy, + timeout: Option, } #[cfg(feature = "modules")] @@ -273,19 +282,51 @@ impl ModuleObjectClient { /// Capture the current unique peer before starting an owned lifecycle. /// Later alias replacement cannot redirect operations on this client. pub(crate) async fn pin_owner(&self) -> Result { - let owner = self.proxy.connection().name_owner(self.proxy.destination()).await + let owner = self + .proxy + .connection() + .name_owner(self.proxy.destination()) + .await .map_err(|error| report_proxy_failure(self.record, &error))? .ok_or(ModuleCallError::Unavailable)?; - let proxy = self.proxy.connection().proxy(owner.as_str(), self.proxy.path().as_str(), self.proxy.interface().as_str()) + let proxy = self + .proxy + .connection() + .proxy( + owner.as_str(), + self.proxy.path().as_str(), + self.proxy.interface().as_str(), + ) .map_err(|error| report_proxy_failure(self.record, &error))?; - Ok(Self { record: self.record, proxy }) + let proxy = self + .timeout + .map_or(proxy.clone(), |timeout| proxy.with_timeout(timeout)); + Ok(Self { + record: self.record, + proxy, + timeout: self.timeout, + }) } /// Derive a private child on the already captured module peer. pub(crate) fn object_at(&self, path: &str) -> Result { - let proxy = self.proxy.connection().proxy(self.proxy.destination().as_str(), path, self.proxy.interface().as_str()) + let proxy = self + .proxy + .connection() + .proxy( + self.proxy.destination().as_str(), + path, + self.proxy.interface().as_str(), + ) .map_err(|error| report_proxy_failure(self.record, &error))?; - Ok(Self { record: self.record, proxy }) + let proxy = self + .timeout + .map_or(proxy.clone(), |timeout| proxy.with_timeout(timeout)); + Ok(Self { + record: self.record, + proxy, + timeout: self.timeout, + }) } pub(crate) async fn call( diff --git a/crates/openhuman-core/src/modules/client_tests.rs b/crates/openhuman-core/src/modules/client_tests.rs index ae607ed4f62..541c8d24f11 100644 --- a/crates/openhuman-core/src/modules/client_tests.rs +++ b/crates/openhuman-core/src/modules/client_tests.rs @@ -325,7 +325,11 @@ async fn private_object_clients_dispatch_only_to_the_configured_object() { ) .unwrap(); let record = registry::find("tinysearch").unwrap(); - let object = ModuleObjectClient { record, proxy }; + let object = ModuleObjectClient { + record, + proxy, + timeout: None, + }; assert_eq!(object.call::("Sum", (20_u32, 22_u32)).await, Ok(42)); assert_eq!(root_calls.load(std::sync::atomic::Ordering::SeqCst), 0); From b38b32c44f7fc5007f876ca5b0b6620e4bd208a0 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Mon, 12 Oct 2026 17:36:28 +0300 Subject: [PATCH 07/45] fix(mcp): reject calls to retired module clients Add a retired flag to McpConfiguredClient and check it alongside the shutdown flag in every entry point, so a client removed by agent cleanup can no longer serve calls. Client lookup now also consults the cleanup state under the same lock, preventing a previously admitted turn from creating a fresh client after removal. Auto-committed-on: dragonfly Co-authored-by: Medulla --- .../openhuman-core/src/mcp/module_client.rs | 29 +++++++++++++++---- 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/crates/openhuman-core/src/mcp/module_client.rs b/crates/openhuman-core/src/mcp/module_client.rs index 1c0205ad0d2..039ed96835d 100644 --- a/crates/openhuman-core/src/mcp/module_client.rs +++ b/crates/openhuman-core/src/mcp/module_client.rs @@ -118,11 +118,15 @@ pub(crate) trait McpModulePort: Send + Sync { mod module_port; use module_port::TinyMcpPort; +#[path = "agent_cleanup.rs"] +pub(crate) mod agent_cleanup; + /// One workspace's serialized TinyMCP lifecycle and private object proxy. pub(crate) struct McpConfiguredClient { data_dir: String, port: Box, state: tokio::sync::Mutex, + retired: AtomicBool, } impl McpConfiguredClient { @@ -144,7 +148,15 @@ impl McpConfiguredClient { let mut clients = clients .lock() .unwrap_or_else(std::sync::PoisonError::into_inner); + // Check while holding the same lock cleanup uses. A previously + // admitted turn must not create a first client after removal. + if agent_cleanup::agent_is_removed() { + return Err(ModuleCallError::Unavailable); + } if let Some(client) = clients.get(&data_dir) { + if client.retired.load(Ordering::Acquire) { + return Err(ModuleCallError::Unavailable); + } return Ok(Arc::clone(client)); } let client = Arc::new(Self::new(&data_dir, config)?); @@ -176,7 +188,7 @@ impl McpConfiguredClient { limit: usize, ) -> Result, ModuleCallError> { let state = self.state.lock().await; - if SHUTTING_DOWN.load(Ordering::Acquire) { + if SHUTTING_DOWN.load(Ordering::Acquire) || self.retired.load(Ordering::Acquire) { return Err(ModuleCallError::Unavailable); } let State::Open(object) = &*state else { @@ -266,6 +278,7 @@ impl McpConfiguredClient { data_dir, port, state: tokio::sync::Mutex::new(State::Closed), + retired: AtomicBool::new(false), } } @@ -296,7 +309,8 @@ impl McpConfiguredClient { &self, request: PrepareAgentToolsRequest, ) -> Result, ModuleCallError> { - if SHUTTING_DOWN.load(Ordering::Acquire) { + let _accepted = self.state.lock().await; + if SHUTTING_DOWN.load(Ordering::Acquire) || self.retired.load(Ordering::Acquire) { return Err(ModuleCallError::Unavailable); } self.port.prepare_agent_tools(request).await @@ -325,7 +339,8 @@ impl McpConfiguredClient { &self, request: ValidateServerArgumentsRequest, ) -> Result { - if SHUTTING_DOWN.load(Ordering::Acquire) { + let _accepted = self.state.lock().await; + if SHUTTING_DOWN.load(Ordering::Acquire) || self.retired.load(Ordering::Acquire) { return Err(ModuleCallError::Unavailable); } self.port.validate_server_arguments(request).await @@ -337,7 +352,8 @@ impl McpConfiguredClient { &self, arguments: Option, ) -> Result, ModuleCallError> { - if SHUTTING_DOWN.load(Ordering::Acquire) { + let _accepted = self.state.lock().await; + if SHUTTING_DOWN.load(Ordering::Acquire) || self.retired.load(Ordering::Acquire) { return Err(ModuleCallError::Unavailable); } self.port.normalize_tool_arguments(arguments).await @@ -348,7 +364,8 @@ impl McpConfiguredClient { &self, value: &str, ) -> Result { - if SHUTTING_DOWN.load(Ordering::Acquire) { + let _accepted = self.state.lock().await; + if SHUTTING_DOWN.load(Ordering::Acquire) || self.retired.load(Ordering::Acquire) { return Err(ModuleCallError::Unavailable); } self.port @@ -449,7 +466,7 @@ impl McpConfiguredClient { ) -> Result { let args = serde_json::to_value(args).map_err(|_| ModuleCallError::ModuleFault)?; let mut state = self.state.lock().await; - if SHUTTING_DOWN.load(Ordering::Acquire) { + if SHUTTING_DOWN.load(Ordering::Acquire) || self.retired.load(Ordering::Acquire) { return Err(ModuleCallError::Unavailable); } let object = self.ensure_open(&mut state, config).await?; From bd615a1fd8c725da394465161668a675bd777891 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Mon, 12 Oct 2026 17:37:44 +0300 Subject: [PATCH 08/45] feat(mcp): expose agent cleanup helpers and module runtime spawn Re-export the agent cleanup API from the MCP host and add a spawn helper on the module runtime so lifecycle cleanup can run on the process executor without loading an artifact. Auto-committed-on: dragonfly Co-authored-by: Medulla --- .../openhuman-core/src/mcp/agent_cleanup.rs | 92 +++++++++++++++++++ crates/openhuman-core/src/mcp/host.rs | 4 + crates/openhuman-core/src/modules/host.rs | 13 +++ 3 files changed, 109 insertions(+) create mode 100644 crates/openhuman-core/src/mcp/agent_cleanup.rs diff --git a/crates/openhuman-core/src/mcp/agent_cleanup.rs b/crates/openhuman-core/src/mcp/agent_cleanup.rs new file mode 100644 index 00000000000..688993c59ca --- /dev/null +++ b/crates/openhuman-core/src/mcp/agent_cleanup.rs @@ -0,0 +1,92 @@ +//! Captured cleanup of an agent's existing configured module object. +use super::*; + +/// A retired agent's original MCP owner and its retryable close receipts. +/// Capturing this handle never creates a client or loads a module. +#[derive(Clone)] +pub struct AgentMcpCleanup { + directory: PathBuf, + client: Arc, +} + +/// Retire the existing configured client for this exact agent directory. +/// An absent entry requires no cleanup and leaves the module loader untouched. +pub fn take_agent_cleanup(workspace: &std::path::Path, agent_id: &str) -> Option { + let directory = workspace.join("agents").join(agent_id); + let clients = CLIENTS + .get()? + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let client = Arc::clone(clients.get(&directory)?); + client.retired.store(true, Ordering::Release); + Some(AgentMcpCleanup { directory, client }) +} + +pub(super) fn agent_is_removed() -> bool { + crate::core::runtime::CoreContext::current().is_some_and(|ctx| { + ctx.session_agent().is_some() + && ctx + .host_overrides() + .and_then(|overrides| overrides.approval_scope()) + .is_some_and(|scope| scope.is_closed()) + }) +} + +impl AgentMcpCleanup { + /// Close the original object and evict it only after confirmed cleanup. + /// Cancellation or error retains the same client and transition receipt. + pub async fn finish(&self) -> Result<(), ModuleCallError> { + self.client.close().await?; + self.evict(); + Ok(()) + } + + /// Finish an unused client without starting an asynchronous executor. + /// An accepted call holding the lifecycle lock requires asynchronous drain. + pub fn finish_if_idle(&self) -> bool { + let Ok(state) = self.client.state.try_lock() else { + return false; + }; + if !matches!(*state, State::Closed) { + return false; + } + self.evict(); + true + } + + fn evict(&self) { + let Some(clients) = CLIENTS.get() else { + return; + }; + let mut clients = clients + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + if clients + .get(&self.directory) + .is_some_and(|current| Arc::ptr_eq(current, &self.client)) + { + clients.remove(&self.directory); + } + } +} + +/// Run captured cleanup independently of the caller's Tokio runtime. +/// This creates no configured client and loads no MCP artifact. +pub fn spawn_agent_cleanup( + future: F, +) -> Result, ModuleCallError> +where + F: std::future::Future + Send + 'static, + F::Output: Send + 'static, +{ + #[cfg(feature = "modules")] + { + crate::modules::host::spawn_cleanup(future).map_err(|_| ModuleCallError::TransportFailed) + } + #[cfg(not(feature = "modules"))] + { + tokio::runtime::Handle::try_current() + .map(|handle| handle.spawn(future)) + .map_err(|_| ModuleCallError::Unavailable) + } +} diff --git a/crates/openhuman-core/src/mcp/host.rs b/crates/openhuman-core/src/mcp/host.rs index 306bce8e36f..13928fc1452 100644 --- a/crates/openhuman-core/src/mcp/host.rs +++ b/crates/openhuman-core/src/mcp/host.rs @@ -9,6 +9,10 @@ use std::sync::OnceLock; use crate::config::Config; +pub use super::module_client::agent_cleanup::{ + spawn_agent_cleanup, take_agent_cleanup, AgentMcpCleanup, +}; + const PROXY_SERVICE_KEY: &str = "tool.mcp_client"; const DEFAULT_CORE_PORT: u16 = 7788; static DEFAULT_CONFIG: OnceLock = OnceLock::new(); diff --git a/crates/openhuman-core/src/modules/host.rs b/crates/openhuman-core/src/modules/host.rs index 96a5f99f40a..57fe5ccc0cd 100644 --- a/crates/openhuman-core/src/modules/host.rs +++ b/crates/openhuman-core/src/modules/host.rs @@ -111,6 +111,19 @@ impl ModuleRuntime { /// /// Does not panic: a lost initialisation race reuses the winner's runtime. pub async fn runtime() -> tinybus::Result<&'static ModuleRuntime> { + runtime_now() +} + +/// Spawn lifecycle cleanup on the process executor, without loading an artifact. +pub(crate) fn spawn_cleanup(future: F) -> tinybus::Result> +where + F: std::future::Future + Send + 'static, + F::Output: Send + 'static, +{ + Ok(runtime_now()?.spawn(future)) +} + +fn runtime_now() -> tinybus::Result<&'static ModuleRuntime> { if let Some(existing) = RUNTIME.get() { return Ok(existing); } From 7d9cab37edb4508678e5dc5f032a9e9241fcd6b9 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Mon, 12 Oct 2026 17:41:35 +0300 Subject: [PATCH 09/45] feat(agent): defer MCP cleanup when agent teardown fails Agent teardown now captures the MCP host cleanup handle instead of evicting it immediately, so a failed cleanup can be retried later while the agent's id and home stay reserved. The agent registry tracks these pending cleanups separately from live agents, allowing removal to retry detached cleanup without keeping the agent or runtime guard alive. Auto-committed-on: dragonfly Co-authored-by: Medulla --- crates/openhuman-embed/src/agent/build.rs | 1 + .../openhuman-embed/src/agent/mcp_cleanup.rs | 89 +++++++++++++++++++ crates/openhuman-embed/src/agent/mod.rs | 24 ++--- .../src/runtime/host_agents.rs | 28 +++++- crates/openhuman-embed/src/runtime/mod.rs | 3 +- 5 files changed, 128 insertions(+), 17 deletions(-) create mode 100644 crates/openhuman-embed/src/agent/mcp_cleanup.rs diff --git a/crates/openhuman-embed/src/agent/build.rs b/crates/openhuman-embed/src/agent/build.rs index cfc609e42f6..ab72d1eaad0 100644 --- a/crates/openhuman-embed/src/agent/build.rs +++ b/crates/openhuman-embed/src/agent/build.rs @@ -338,6 +338,7 @@ pub(crate) fn instantiate(runtime: &Runtime, spec: AgentSpec) -> Result, + home: &Path, + purge: bool, +) -> Result<(), AgentError> { + let mut agents = registry.lock().unwrap_or_else(std::sync::PoisonError::into_inner); + if !agents.get(id).is_some_and(|current| Weak::ptr_eq(current, instance)) { + return Ok(()); + } + // Keep the id reserved during purge. A second cleanup cannot delete the + // home of a replacement that another caller has already instantiated. + if purge && home.exists() { + std::fs::remove_dir_all(home).map_err(|source| AgentError::Workspace { + what: "delete the removed agent's home", source, + })?; + } + agents.remove(id); + Ok(()) +} + +impl AgentInner { + pub(crate) fn cleanup_is_pending(&self) -> bool { + self.mcp_cleanup.lock().unwrap_or_else(std::sync::PoisonError::into_inner).is_some() + } + + pub(crate) async fn finish_mcp_cleanup(&self) -> Result<(), AgentError> { + let cleanup = self.mcp_cleanup.lock().unwrap_or_else(std::sync::PoisonError::into_inner).clone(); + if let Some(cleanup) = cleanup { + cleanup.finish().await.map_err(AgentError::McpCleanup)?; + self.mcp_cleanup.lock().unwrap_or_else(std::sync::PoisonError::into_inner).take(); + } + Ok(()) + } + + pub(crate) fn finish_removal(&self, purge: bool) -> Result<(), AgentError> { + let instance = self.registry.lock().unwrap_or_else(std::sync::PoisonError::into_inner) + .get(&self.id).filter(|weak| std::ptr::eq(weak.as_ptr(), self)).cloned(); + if let Some(instance) = instance { + finish_removal(&self.registry, &self.id, &instance, &self.layout.home, purge)?; + } + Ok(()) + } + + /// Retry independently of the caller runtime. The task, rather than a + /// process-global callback, holds the workspace guard while cleanup runs. + pub(crate) fn defer_cleanup(&self, purge: bool) { + let cleanup = self.mcp_cleanup.lock().unwrap_or_else(std::sync::PoisonError::into_inner).clone(); + if cleanup.as_ref().is_none_or(|cleanup| cleanup.finish_if_idle()) { + if let Err(error) = self.finish_removal(purge) { + log::warn!("[embed][agent] removed home cleanup failed: {error}"); + } + return; + } + if self.cleanup_started.swap(true, Ordering::AcqRel) { return; } + let Some(cleanup) = cleanup else { return; }; + let instance = self.registry.lock().unwrap_or_else(std::sync::PoisonError::into_inner) + .get(&self.id).filter(|weak| std::ptr::eq(weak.as_ptr(), self)).cloned(); + let Some(instance) = instance else { return; }; + let registry = Arc::clone(&self.registry); + let guard = Arc::clone(&self._runtime_guard); + let home = self.layout.home.clone(); + let id = self.id.clone(); + let future = async move { + let _guard = guard; + let mut delay = std::time::Duration::from_millis(250); + loop { + let result = match cleanup.finish().await { + Ok(()) => finish_removal(®istry, &id, &instance, &home, purge), + Err(error) => Err(AgentError::McpCleanup(error)), + }; + match result { + Ok(()) => return, + Err(error) => log::warn!("[embed][agent] captured cleanup remains pending: {error}"), + } + tokio::time::sleep(delay).await; + delay = (delay * 2).min(std::time::Duration::from_secs(5)); + } + }; + if let Err(error) = openhuman_core::mcp::host::spawn_agent_cleanup(future) { + log::warn!("[embed][agent] cleanup executor unavailable: {error}"); + } + } +} diff --git a/crates/openhuman-embed/src/agent/mod.rs b/crates/openhuman-embed/src/agent/mod.rs index 13ea47e0151..9de93209568 100644 --- a/crates/openhuman-embed/src/agent/mod.rs +++ b/crates/openhuman-embed/src/agent/mod.rs @@ -19,6 +19,7 @@ mod approvals; pub use approval_handler::{ApprovalHandler, ApprovalSubscription}; mod attachments; pub(crate) mod lifecycle; +pub(crate) mod mcp_cleanup; pub use approvals::{ApprovalDecision, Approvals, ApprovalsError, PendingApproval}; pub use attachments::ToolAttachmentError; pub(crate) mod build; @@ -102,6 +103,10 @@ pub enum AgentError { /// A named built-in definition does not exist. #[error("unknown built-in agent definition {0:?}")] UnknownDefinition(String), + /// The agent is removed, but its original MCP owner still needs cleanup. + /// Its id and home remain reserved while detached cleanup retries. + #[error("agent MCP cleanup is still pending: {0}")] + McpCleanup(#[source] openhuman_core::modules::client::ModuleCallError), } /// The assembled state behind an [`Agent`], shared by every clone of it and @@ -132,6 +137,7 @@ pub(crate) struct AgentInner { pub(crate) host_tools: Option, pub(crate) hooks: openhuman_core::agent::hooks::HookScope, pub(crate) lifecycle: lifecycle::Lifecycle, + pub(crate) mcp_cleanup: std::sync::Mutex>, /// Built from [`ToolScopeSpec::HostOnly`]: every turn's session is built /// from the host tools alone. pub(crate) host_only: bool, @@ -153,14 +159,8 @@ impl AgentInner { ); self.deny_approvals(resolution); self.ctx.agent_state().clear(); - if openhuman_core::mcp::host::take_agent_host(&self.config.workspace_dir, &self.id) - .is_some() - { - log::debug!( - "[embed][agent] teardown id={} evicted its MCP host", - self.id - ); - } + *self.mcp_cleanup.lock().unwrap_or_else(std::sync::PoisonError::into_inner) = + openhuman_core::mcp::host::take_agent_cleanup(&self.config.workspace_dir, &self.id); openhuman_core::core::runtime::AgentContextRegistry::deregister(&self.id, &self.ctx); log::debug!("[embed][agent] teardown complete id={}", self.id); } @@ -189,13 +189,7 @@ impl AgentInner { impl Drop for AgentInner { fn drop(&mut self) { self.teardown("agent_dropped"); - let mut agents = self.registry.lock().unwrap_or_else(|e| e.into_inner()); - if agents - .get(&self.id) - .is_some_and(|current| std::ptr::eq(current.as_ptr(), self)) - { - agents.remove(&self.id); - } + self.defer_cleanup(false); } } diff --git a/crates/openhuman-embed/src/runtime/host_agents.rs b/crates/openhuman-embed/src/runtime/host_agents.rs index 8cadfbcdb8d..1a910aee481 100644 --- a/crates/openhuman-embed/src/runtime/host_agents.rs +++ b/crates/openhuman-embed/src/runtime/host_agents.rs @@ -15,7 +15,33 @@ use openhuman_core::agent::host_agents::{HostAgent, HostAgentResolver}; use crate::agent::AgentInner; /// The live agents of one runtime, by id. -pub(crate) type AgentMap = Mutex>>; +pub(crate) type AgentMap = Mutex; + +/// Failed cleanup retains its captured owner and purge intent, without keeping +/// an agent or the runtime guard alive. `remove_agent` can retry this record. +#[derive(Clone)] +pub(crate) struct PendingCleanup { + pub(crate) cleanup: Option, + pub(crate) instance: Weak, + pub(crate) home: std::path::PathBuf, + pub(crate) purge: bool, + pub(crate) retry_running: Arc, +} + +#[derive(Default)] +pub(crate) struct AgentRegistry { + live: HashMap>, + pub(crate) pending: HashMap, +} + +impl std::ops::Deref for AgentRegistry { + type Target = HashMap>; + fn deref(&self) -> &Self::Target { &self.live } +} + +impl std::ops::DerefMut for AgentRegistry { + fn deref_mut(&mut self) -> &mut Self::Target { &mut self.live } +} pub(crate) struct RuntimeAgents(pub(crate) Arc); diff --git a/crates/openhuman-embed/src/runtime/mod.rs b/crates/openhuman-embed/src/runtime/mod.rs index 253444024a7..5c89640207a 100644 --- a/crates/openhuman-embed/src/runtime/mod.rs +++ b/crates/openhuman-embed/src/runtime/mod.rs @@ -72,6 +72,7 @@ pub(crate) mod builder; mod defaults; mod host_agents; pub(crate) use host_agents::AgentMap; +pub(crate) use host_agents::PendingCleanup; mod info; mod lifecycle; mod live_hooks; @@ -596,7 +597,7 @@ impl Runtime { storage_info: StorageInfo, storage_backend: Option>, ) -> Self { - let agents: Arc = Arc::new(Mutex::new(HashMap::new())); + let agents: Arc = Arc::new(Mutex::new(Default::default())); let resolver: Arc = Arc::new(host_agents::RuntimeAgents(Arc::clone(&agents))); openhuman_core::agent::host_agents::install(Arc::clone(&resolver)); From 00632d50e47380b03c15643224f1ac09e2cb2bbd Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Mon, 12 Oct 2026 17:42:12 +0300 Subject: [PATCH 10/45] refactor(agent): track pending cleanup in the registry Move the captured cleanup state into a registry-side PendingCleanup record so the id reservation, purge intent and retry flag survive independently of the agent instance. Deferred cleanup now runs a bounded three-attempt retry on the process executor and leaves the record in place for a later explicit retry instead of looping forever. Auto-committed-on: dragonfly Co-authored-by: Medulla --- .../openhuman-embed/src/agent/mcp_cleanup.rs | 116 ++++++++++-------- 1 file changed, 64 insertions(+), 52 deletions(-) diff --git a/crates/openhuman-embed/src/agent/mcp_cleanup.rs b/crates/openhuman-embed/src/agent/mcp_cleanup.rs index c0065d76dd3..e78a3c7cd9b 100644 --- a/crates/openhuman-embed/src/agent/mcp_cleanup.rs +++ b/crates/openhuman-embed/src/agent/mcp_cleanup.rs @@ -1,89 +1,101 @@ //! Retiring one captured agent owner before its id or home is reusable. use super::*; +use crate::runtime::PendingCleanup; use std::sync::{atomic::Ordering, Weak}; -fn finish_removal( +pub(crate) fn finish_removal( registry: &crate::runtime::AgentMap, id: &str, - instance: &Weak, - home: &Path, - purge: bool, + pending: &PendingCleanup, ) -> Result<(), AgentError> { let mut agents = registry.lock().unwrap_or_else(std::sync::PoisonError::into_inner); - if !agents.get(id).is_some_and(|current| Weak::ptr_eq(current, instance)) { + if !agents.get(id).is_some_and(|current| Weak::ptr_eq(current, &pending.instance)) { return Ok(()); } - // Keep the id reserved during purge. A second cleanup cannot delete the - // home of a replacement that another caller has already instantiated. - if purge && home.exists() { - std::fs::remove_dir_all(home).map_err(|source| AgentError::Workspace { + let purge = pending.purge || agents.pending.get(id).is_some_and(|current| current.purge); + // Keep the id reserved throughout purge; never touch a successor's home. + if purge && pending.home.exists() { + std::fs::remove_dir_all(&pending.home).map_err(|source| AgentError::Workspace { what: "delete the removed agent's home", source, })?; } agents.remove(id); + agents.pending.remove(id); Ok(()) } +/// Run at most three retries on the process module executor. The future owns +/// the workspace guard only while running; retained records contain no guard. +pub(crate) fn defer_pending( + registry: Arc, + id: String, + pending: PendingCleanup, + guard: Arc, +) { + if pending.cleanup.as_ref().is_none_or(|cleanup| cleanup.finish_if_idle()) { + if let Err(error) = finish_removal(®istry, &id, &pending) { + log::warn!("[embed][agent] removed home cleanup remains pending: {error}"); + } + return; + } + if pending.retry_running.swap(true, Ordering::AcqRel) { return; } + let running = Arc::clone(&pending.retry_running); + let future = async move { + let _guard = guard; + for attempt in 0..3 { + let result = match &pending.cleanup { + Some(cleanup) => cleanup.finish().await.map_err(AgentError::McpCleanup), + None => Ok(()), + }.and_then(|()| finish_removal(®istry, &id, &pending)); + match result { + Ok(()) => { running.store(false, Ordering::Release); return; } + Err(error) => log::warn!("[embed][agent] captured cleanup remains pending: {error}"), + } + if attempt < 2 { + tokio::time::sleep(std::time::Duration::from_millis(250 * (1 << attempt))).await; + } + } + // The original receipt, id reservation and purge intent stay in the + // registry. A later remove_agent call retries them explicitly. + running.store(false, Ordering::Release); + }; + if let Err(error) = openhuman_core::mcp::host::spawn_agent_cleanup(future) { + pending_running_reset(®istry_placeholder()); + log::warn!("[embed][agent] cleanup executor unavailable: {error}"); + } +} + impl AgentInner { - pub(crate) fn cleanup_is_pending(&self) -> bool { - self.mcp_cleanup.lock().unwrap_or_else(std::sync::PoisonError::into_inner).is_some() + pub(crate) fn reserve_cleanup(&self, purge: bool) -> Option { + let cleanup = self.mcp_cleanup.lock().unwrap_or_else(std::sync::PoisonError::into_inner).clone(); + let mut agents = self.registry.lock().unwrap_or_else(std::sync::PoisonError::into_inner); + let instance = agents.get(&self.id).filter(|weak| std::ptr::eq(weak.as_ptr(), self)).cloned()?; + let pending = agents.pending.entry(self.id.clone()).or_insert_with(|| PendingCleanup { + cleanup, instance, home: self.layout.home.clone(), purge, + retry_running: Default::default(), + }); + pending.purge |= purge; + Some(pending.clone()) } pub(crate) async fn finish_mcp_cleanup(&self) -> Result<(), AgentError> { let cleanup = self.mcp_cleanup.lock().unwrap_or_else(std::sync::PoisonError::into_inner).clone(); if let Some(cleanup) = cleanup { cleanup.finish().await.map_err(AgentError::McpCleanup)?; - self.mcp_cleanup.lock().unwrap_or_else(std::sync::PoisonError::into_inner).take(); } Ok(()) } pub(crate) fn finish_removal(&self, purge: bool) -> Result<(), AgentError> { - let instance = self.registry.lock().unwrap_or_else(std::sync::PoisonError::into_inner) - .get(&self.id).filter(|weak| std::ptr::eq(weak.as_ptr(), self)).cloned(); - if let Some(instance) = instance { - finish_removal(&self.registry, &self.id, &instance, &self.layout.home, purge)?; + if let Some(pending) = self.reserve_cleanup(purge) { + finish_removal(&self.registry, &self.id, &pending)?; } Ok(()) } - /// Retry independently of the caller runtime. The task, rather than a - /// process-global callback, holds the workspace guard while cleanup runs. pub(crate) fn defer_cleanup(&self, purge: bool) { - let cleanup = self.mcp_cleanup.lock().unwrap_or_else(std::sync::PoisonError::into_inner).clone(); - if cleanup.as_ref().is_none_or(|cleanup| cleanup.finish_if_idle()) { - if let Err(error) = self.finish_removal(purge) { - log::warn!("[embed][agent] removed home cleanup failed: {error}"); - } - return; - } - if self.cleanup_started.swap(true, Ordering::AcqRel) { return; } - let Some(cleanup) = cleanup else { return; }; - let instance = self.registry.lock().unwrap_or_else(std::sync::PoisonError::into_inner) - .get(&self.id).filter(|weak| std::ptr::eq(weak.as_ptr(), self)).cloned(); - let Some(instance) = instance else { return; }; - let registry = Arc::clone(&self.registry); - let guard = Arc::clone(&self._runtime_guard); - let home = self.layout.home.clone(); - let id = self.id.clone(); - let future = async move { - let _guard = guard; - let mut delay = std::time::Duration::from_millis(250); - loop { - let result = match cleanup.finish().await { - Ok(()) => finish_removal(®istry, &id, &instance, &home, purge), - Err(error) => Err(AgentError::McpCleanup(error)), - }; - match result { - Ok(()) => return, - Err(error) => log::warn!("[embed][agent] captured cleanup remains pending: {error}"), - } - tokio::time::sleep(delay).await; - delay = (delay * 2).min(std::time::Duration::from_secs(5)); - } - }; - if let Err(error) = openhuman_core::mcp::host::spawn_agent_cleanup(future) { - log::warn!("[embed][agent] cleanup executor unavailable: {error}"); + if let Some(pending) = self.reserve_cleanup(purge) { + defer_pending(Arc::clone(&self.registry), self.id.clone(), pending, Arc::clone(&self._runtime_guard)); } } } From 5ade39055561901b7b573b62fea482dad47a2b6a Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Mon, 12 Oct 2026 17:42:51 +0300 Subject: [PATCH 11/45] fix(embed): retry pending agent cleanup on removal Agent removal now routes through the pending-cleanup path so a removal that finds an in-flight cleanup retries it instead of dropping the reservation, and the cleanup executor resets its running flag when spawning fails so later removals are not blocked. Auto-committed-on: dragonfly Co-authored-by: Medulla --- .../openhuman-embed/src/agent/mcp_cleanup.rs | 80 ++++++++++++++---- crates/openhuman-embed/src/agent/mod.rs | 5 +- .../src/runtime/host_agents.rs | 8 +- .../openhuman-embed/src/runtime/lifecycle.rs | 83 ++++++++++++------- .../openhuman-embed/tests/agent_lifecycle.rs | 2 +- 5 files changed, 125 insertions(+), 53 deletions(-) diff --git a/crates/openhuman-embed/src/agent/mcp_cleanup.rs b/crates/openhuman-embed/src/agent/mcp_cleanup.rs index e78a3c7cd9b..829b3cb6e8c 100644 --- a/crates/openhuman-embed/src/agent/mcp_cleanup.rs +++ b/crates/openhuman-embed/src/agent/mcp_cleanup.rs @@ -8,15 +8,21 @@ pub(crate) fn finish_removal( id: &str, pending: &PendingCleanup, ) -> Result<(), AgentError> { - let mut agents = registry.lock().unwrap_or_else(std::sync::PoisonError::into_inner); - if !agents.get(id).is_some_and(|current| Weak::ptr_eq(current, &pending.instance)) { + let mut agents = registry + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + if !agents + .get(id) + .is_some_and(|current| Weak::ptr_eq(current, &pending.instance)) + { return Ok(()); } let purge = pending.purge || agents.pending.get(id).is_some_and(|current| current.purge); // Keep the id reserved throughout purge; never touch a successor's home. if purge && pending.home.exists() { std::fs::remove_dir_all(&pending.home).map_err(|source| AgentError::Workspace { - what: "delete the removed agent's home", source, + what: "delete the removed agent's home", + source, })?; } agents.remove(id); @@ -32,24 +38,37 @@ pub(crate) fn defer_pending( pending: PendingCleanup, guard: Arc, ) { - if pending.cleanup.as_ref().is_none_or(|cleanup| cleanup.finish_if_idle()) { + if pending + .cleanup + .as_ref() + .is_none_or(|cleanup| cleanup.finish_if_idle()) + { if let Err(error) = finish_removal(®istry, &id, &pending) { log::warn!("[embed][agent] removed home cleanup remains pending: {error}"); } return; } - if pending.retry_running.swap(true, Ordering::AcqRel) { return; } + if pending.retry_running.swap(true, Ordering::AcqRel) { + return; + } let running = Arc::clone(&pending.retry_running); + let executor_failed = Arc::clone(&running); let future = async move { let _guard = guard; for attempt in 0..3 { let result = match &pending.cleanup { Some(cleanup) => cleanup.finish().await.map_err(AgentError::McpCleanup), None => Ok(()), - }.and_then(|()| finish_removal(®istry, &id, &pending)); + } + .and_then(|()| finish_removal(®istry, &id, &pending)); match result { - Ok(()) => { running.store(false, Ordering::Release); return; } - Err(error) => log::warn!("[embed][agent] captured cleanup remains pending: {error}"), + Ok(()) => { + running.store(false, Ordering::Release); + return; + } + Err(error) => { + log::warn!("[embed][agent] captured cleanup remains pending: {error}") + } } if attempt < 2 { tokio::time::sleep(std::time::Duration::from_millis(250 * (1 << attempt))).await; @@ -60,26 +79,46 @@ pub(crate) fn defer_pending( running.store(false, Ordering::Release); }; if let Err(error) = openhuman_core::mcp::host::spawn_agent_cleanup(future) { - pending_running_reset(®istry_placeholder()); + executor_failed.store(false, Ordering::Release); log::warn!("[embed][agent] cleanup executor unavailable: {error}"); } } impl AgentInner { pub(crate) fn reserve_cleanup(&self, purge: bool) -> Option { - let cleanup = self.mcp_cleanup.lock().unwrap_or_else(std::sync::PoisonError::into_inner).clone(); - let mut agents = self.registry.lock().unwrap_or_else(std::sync::PoisonError::into_inner); - let instance = agents.get(&self.id).filter(|weak| std::ptr::eq(weak.as_ptr(), self)).cloned()?; - let pending = agents.pending.entry(self.id.clone()).or_insert_with(|| PendingCleanup { - cleanup, instance, home: self.layout.home.clone(), purge, - retry_running: Default::default(), - }); + let cleanup = self + .mcp_cleanup + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .clone(); + let mut agents = self + .registry + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let instance = agents + .get(&self.id) + .filter(|weak| std::ptr::eq(weak.as_ptr(), self)) + .cloned()?; + let pending = agents + .pending + .entry(self.id.clone()) + .or_insert_with(|| PendingCleanup { + cleanup, + instance, + home: self.layout.home.clone(), + purge, + retry_running: Default::default(), + }); pending.purge |= purge; Some(pending.clone()) } pub(crate) async fn finish_mcp_cleanup(&self) -> Result<(), AgentError> { - let cleanup = self.mcp_cleanup.lock().unwrap_or_else(std::sync::PoisonError::into_inner).clone(); + let cleanup = self + .mcp_cleanup + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .clone(); if let Some(cleanup) = cleanup { cleanup.finish().await.map_err(AgentError::McpCleanup)?; } @@ -95,7 +134,12 @@ impl AgentInner { pub(crate) fn defer_cleanup(&self, purge: bool) { if let Some(pending) = self.reserve_cleanup(purge) { - defer_pending(Arc::clone(&self.registry), self.id.clone(), pending, Arc::clone(&self._runtime_guard)); + defer_pending( + Arc::clone(&self.registry), + self.id.clone(), + pending, + Arc::clone(&self._runtime_guard), + ); } } } diff --git a/crates/openhuman-embed/src/agent/mod.rs b/crates/openhuman-embed/src/agent/mod.rs index 9de93209568..011f8e2fa68 100644 --- a/crates/openhuman-embed/src/agent/mod.rs +++ b/crates/openhuman-embed/src/agent/mod.rs @@ -159,7 +159,10 @@ impl AgentInner { ); self.deny_approvals(resolution); self.ctx.agent_state().clear(); - *self.mcp_cleanup.lock().unwrap_or_else(std::sync::PoisonError::into_inner) = + *self + .mcp_cleanup + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) = openhuman_core::mcp::host::take_agent_cleanup(&self.config.workspace_dir, &self.id); openhuman_core::core::runtime::AgentContextRegistry::deregister(&self.id, &self.ctx); log::debug!("[embed][agent] teardown complete id={}", self.id); diff --git a/crates/openhuman-embed/src/runtime/host_agents.rs b/crates/openhuman-embed/src/runtime/host_agents.rs index 1a910aee481..4938340dbc7 100644 --- a/crates/openhuman-embed/src/runtime/host_agents.rs +++ b/crates/openhuman-embed/src/runtime/host_agents.rs @@ -36,11 +36,15 @@ pub(crate) struct AgentRegistry { impl std::ops::Deref for AgentRegistry { type Target = HashMap>; - fn deref(&self) -> &Self::Target { &self.live } + fn deref(&self) -> &Self::Target { + &self.live + } } impl std::ops::DerefMut for AgentRegistry { - fn deref_mut(&mut self) -> &mut Self::Target { &mut self.live } + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.live + } } pub(crate) struct RuntimeAgents(pub(crate) Arc); diff --git a/crates/openhuman-embed/src/runtime/lifecycle.rs b/crates/openhuman-embed/src/runtime/lifecycle.rs index 13587a4c90e..dfc49e42564 100644 --- a/crates/openhuman-embed/src/runtime/lifecycle.rs +++ b/crates/openhuman-embed/src/runtime/lifecycle.rs @@ -14,44 +14,37 @@ const REMOVE_IDLE_WAIT: Duration = Duration::from_secs(10); // Reserve the public id until all old-instance cleanup finishes, including // when a caller cancels the removal future after it has started. struct RemovalSlot<'a> { - runtime: &'a Runtime, + _runtime: &'a Runtime, inner: Arc, purge: bool, + finished: bool, } -impl RemovalSlot<'_> { - fn purge_home(&mut self) -> Result<(), AgentError> { - if !std::mem::take(&mut self.purge) { - return Ok(()); - } - let home = &self.inner.layout.home; - if home.exists() { - std::fs::remove_dir_all(home).map_err(|source| AgentError::Workspace { - what: "delete the removed agent's home", - source, - })?; +impl Drop for RemovalSlot<'_> { + fn drop(&mut self) { + if !self.finished { + self.inner.teardown("agent_removed"); + self.inner.defer_cleanup(self.purge); } - Ok(()) } } -impl Drop for RemovalSlot<'_> { +struct PendingRemoval<'a> { + runtime: &'a Runtime, + id: String, + pending: super::PendingCleanup, + finished: bool, +} + +impl Drop for PendingRemoval<'_> { fn drop(&mut self) { - self.inner.teardown("agent_removed"); - if let Err(error) = self.purge_home() { - log::warn!("[embed][runtime] cancelled removal purge failed: {error}"); - } - let mut agents = self - .runtime - .agents - .lock() - .unwrap_or_else(|e| e.into_inner()); - if agents - .get(&self.inner.id) - .and_then(std::sync::Weak::upgrade) - .is_some_and(|current| Arc::ptr_eq(¤t, &self.inner)) - { - agents.remove(&self.inner.id); + if !self.finished { + crate::agent::mcp_cleanup::defer_pending( + Arc::clone(&self.runtime.agents), + self.id.clone(), + self.pending.clone(), + self.runtime.guard(), + ); } } } @@ -106,6 +99,30 @@ impl Runtime { } async fn remove(&self, id: &str, purge: bool) -> Result<(), AgentError> { + let pending = { + let mut agents = self + .agents + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + agents.pending.get_mut(id).map(|pending| { + pending.purge |= purge; + pending.clone() + }) + }; + if let Some(pending) = pending { + let mut retry = PendingRemoval { + runtime: self, + id: id.to_owned(), + pending, + finished: false, + }; + if let Some(cleanup) = &retry.pending.cleanup { + cleanup.finish().await.map_err(AgentError::McpCleanup)?; + } + crate::agent::mcp_cleanup::finish_removal(&self.agents, id, &retry.pending)?; + retry.finished = true; + return Ok(()); + } let inner = { let agents = self.agents.lock().unwrap_or_else(|e| e.into_inner()); agents @@ -120,9 +137,10 @@ impl Runtime { return Err(AgentError::UnknownId(id.to_string())); } let mut removal = RemovalSlot { - runtime: self, + _runtime: self, inner, purge, + finished: false, }; let inner = &removal.inner; log::debug!("[embed][runtime] removing agent id={id} purge={purge}"); @@ -133,7 +151,10 @@ impl Runtime { ); } inner.teardown("agent_removed"); - removal.purge_home()?; + inner.reserve_cleanup(purge); + inner.finish_mcp_cleanup().await?; + inner.finish_removal(purge)?; + removal.finished = true; log::debug!("[embed][runtime] agent removed id={id}"); Ok(()) } diff --git a/crates/openhuman-embed/tests/agent_lifecycle.rs b/crates/openhuman-embed/tests/agent_lifecycle.rs index e491275512d..711a026cdb5 100644 --- a/crates/openhuman-embed/tests/agent_lifecycle.rs +++ b/crates/openhuman-embed/tests/agent_lifecycle.rs @@ -275,7 +275,7 @@ fn removing_an_agent_releases_everything_it_held() { "the context left the registry, so its cron jobs stay dormant" ); assert!( - openhuman_core::mcp::host::take_agent_host(&workspace_dir, "parker").is_none(), + openhuman_core::mcp::host::take_agent_cleanup(&workspace_dir, "parker").is_none(), "no MCP host is kept for a removed agent" ); removed(parker.turn("again").send().await, "parker"); From ec184c1df0b6f2e6ebc65b482686606637527560 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Mon, 12 Oct 2026 17:45:07 +0300 Subject: [PATCH 12/45] test(mcp): cover failed agent cleanup and retry ownership Add tests for the agent cleanup path when the module close call fails, verifying the retired owner is retained, calls stay unavailable, and a retry can complete the cleanup without evicting a later owner. Also assert that taking cleanup for an agent that never opened does not create a configured client. Auto-committed-on: dragonfly Co-authored-by: Medulla --- .../src/mcp/module_client_owner_tests.rs | 107 ++++++++++++++++++ 1 file changed, 107 insertions(+) diff --git a/crates/openhuman-core/src/mcp/module_client_owner_tests.rs b/crates/openhuman-core/src/mcp/module_client_owner_tests.rs index 3d0cd37b6bb..0c77203a05f 100644 --- a/crates/openhuman-core/src/mcp/module_client_owner_tests.rs +++ b/crates/openhuman-core/src/mcp/module_client_owner_tests.rs @@ -9,12 +9,16 @@ struct LifecycleFixture { block_close_poll: Arc, close_poll_started: Arc, release_close_poll: Arc, + fail_close: Arc, } #[tinybus::interface(name = "ai.tinyhumans.tinymcp.Mcp")] impl LifecycleFixture { async fn close(&self, _data_dir: String) -> tinybus::Result> { self.closes.fetch_add(1, Ordering::SeqCst); + if self.fail_close.load(Ordering::SeqCst) { + return Err(tinybus::Error::failed("fixture cleanup unavailable")); + } Ok(Some("original-close".into())) } @@ -28,6 +32,109 @@ impl LifecycleFixture { } } +#[tokio::test] +async fn failed_agent_cleanup_retains_the_retired_owner_and_can_retry() { + let scratch = tempfile::tempdir().unwrap(); + let directory = scratch.path().join("agents/worker"); + let bus = MemoryBus::new(); + Broker::new().spawn(bus.clone()); + let service = Connection::connect(bus.connect().await.unwrap()) + .await + .unwrap(); + let caller = Connection::connect(bus.connect().await.unwrap()) + .await + .unwrap(); + let fixture = LifecycleFixture::default(); + service + .serve_at( + ObjectPath::new(names::OBJECT_PATH).unwrap(), + fixture.clone(), + ) + .await + .unwrap(); + service.request_name(names::INTERFACE).await.unwrap(); + let port = TinyMcpPort::new(ModuleClient::fixture( + caller + .proxy(names::INTERFACE, names::OBJECT_PATH, names::INTERFACE) + .unwrap(), + )); + port.transition_status("original-open").await.unwrap(); + let client = Arc::new(McpConfiguredClient::with_port( + directory.to_str().unwrap().into(), + Box::new(port), + )); + *client.state.lock().await = State::Open(OpenObject { + config: McpClientConfig::default(), + object_path: "/test/original-child".into(), + }); + CLIENTS + .get_or_init(|| Mutex::new(HashMap::new())) + .lock() + .unwrap() + .insert(directory.clone(), Arc::clone(&client)); + let cleanup = agent_cleanup::take_agent_cleanup(scratch.path(), "worker").unwrap(); + fixture.fail_close.store(true, Ordering::SeqCst); + assert_eq!(cleanup.finish().await, Err(ModuleCallError::ModuleFault)); + assert!(Arc::ptr_eq( + CLIENTS + .get() + .unwrap() + .lock() + .unwrap() + .get(&directory) + .unwrap(), + &client + )); + assert!(matches!( + &*client.state.lock().await, + State::StartingClose(_) + )); + assert_eq!( + client + .call::(&McpClientConfig::default(), "NeverReopen", ()) + .await, + Err(ModuleCallError::Unavailable) + ); + fixture.fail_close.store(false, Ordering::SeqCst); + cleanup.finish().await.unwrap(); + assert!(!CLIENTS + .get() + .unwrap() + .lock() + .unwrap() + .contains_key(&directory)); + // Retrying the old completed handle cannot evict a later owner. + let successor = Arc::new(McpConfiguredClient::new(&directory, &Config::default()).unwrap()); + CLIENTS + .get() + .unwrap() + .lock() + .unwrap() + .insert(directory.clone(), Arc::clone(&successor)); + assert!(cleanup.finish_if_idle()); + assert!(Arc::ptr_eq( + CLIENTS + .get() + .unwrap() + .lock() + .unwrap() + .get(&directory) + .unwrap(), + &successor + )); + CLIENTS.get().unwrap().lock().unwrap().remove(&directory); +} + +#[test] +fn an_absent_agent_cleanup_does_not_create_a_configured_client() { + let scratch = tempfile::tempdir().unwrap(); + assert!(agent_cleanup::take_agent_cleanup(scratch.path(), "never-opened").is_none()); + assert!(CLIENTS.get().is_none_or(|clients| !clients + .lock() + .unwrap() + .contains_key(&scratch.path().join("agents/never-opened")))); +} + #[tokio::test] async fn cancelled_close_retries_its_receipt_on_the_original_owner() { let bus = MemoryBus::new(); From cdd6f17c5036afcecfbbb6c24daf2ee70e906357 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Mon, 12 Oct 2026 17:45:58 +0300 Subject: [PATCH 13/45] chore: files changed crates/openhuman-core/src/mcp/module_client_owner_tests.rs Auto-committed-on: dragonfly Co-authored-by: Medulla --- .../src/mcp/module_client_owner_tests.rs | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/crates/openhuman-core/src/mcp/module_client_owner_tests.rs b/crates/openhuman-core/src/mcp/module_client_owner_tests.rs index 0c77203a05f..836971b8ba5 100644 --- a/crates/openhuman-core/src/mcp/module_client_owner_tests.rs +++ b/crates/openhuman-core/src/mcp/module_client_owner_tests.rs @@ -135,6 +135,54 @@ fn an_absent_agent_cleanup_does_not_create_a_configured_client() { .contains_key(&scratch.path().join("agents/never-opened")))); } +#[tokio::test] +async fn an_old_closed_context_cannot_create_the_reused_agents_first_client() { + use crate::core::runtime::{ContextOverlay, CoreContext, DomainSet}; + use crate::tools::toolpacks::ToolGroups; + let scratch = tempfile::tempdir().unwrap(); + let mut config = Config::default(); + config.workspace_dir = scratch.path().into(); + let parent = CoreContext::for_test(DomainSet::full(), Some(config.workspace_dir.clone())); + let old_scope = Arc::new(crate::security::approval::ApprovalScope::default()); + let old_overrides = Arc::new(crate::agent::host_overrides::HostOverrides { + approval_scope: Some(Arc::clone(&old_scope)), + ..Default::default() + }); + let mut overlay = ContextOverlay::new(config.clone(), DomainSet::full(), ToolGroups::none()) + .session_agent("worker"); + overlay.host_overrides = Some(old_overrides); + let old_context = parent.derive_with(overlay); + old_scope.close("agent_removed"); + CoreContext::scope(old_context, async { + assert!(matches!( + McpConfiguredClient::for_config(&config), + Err(ModuleCallError::Unavailable) + )); + }) + .await; + let directory = config.workspace_dir.join("agents/worker"); + assert!(CLIENTS + .get() + .is_none_or(|clients| !clients.lock().unwrap().contains_key(&directory))); + let mut new_overlay = + ContextOverlay::new(config.clone(), DomainSet::full(), ToolGroups::none()) + .session_agent("worker"); + new_overlay.host_overrides = Some(Arc::new(crate::agent::host_overrides::HostOverrides { + approval_scope: Some(Arc::new(crate::security::approval::ApprovalScope::default())), + ..Default::default() + })); + CoreContext::scope(parent.derive_with(new_overlay), async { + McpConfiguredClient::for_config(&config) + .expect("the new instance has an open lifecycle scope"); + }) + .await; + assert!( + agent_cleanup::take_agent_cleanup(&config.workspace_dir, "worker") + .unwrap() + .finish_if_idle() + ); +} + #[tokio::test] async fn cancelled_close_retries_its_receipt_on_the_original_owner() { let bus = MemoryBus::new(); From c0435a6970237bce38973980ce505812a11ff5a0 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Mon, 12 Oct 2026 17:46:52 +0300 Subject: [PATCH 14/45] test(agent): add test module for mcp cleanup Wire the new mcp_cleanup_tests.rs file into the agent module behind a cfg(test) gate so the cleanup logic can be covered by unit tests. Auto-committed-on: dragonfly Co-authored-by: Medulla --- .../openhuman-embed/src/agent/mcp_cleanup.rs | 4 ++ .../src/agent/mcp_cleanup_tests.rs | 67 +++++++++++++++++++ 2 files changed, 71 insertions(+) create mode 100644 crates/openhuman-embed/src/agent/mcp_cleanup_tests.rs diff --git a/crates/openhuman-embed/src/agent/mcp_cleanup.rs b/crates/openhuman-embed/src/agent/mcp_cleanup.rs index 829b3cb6e8c..bc64b1660cf 100644 --- a/crates/openhuman-embed/src/agent/mcp_cleanup.rs +++ b/crates/openhuman-embed/src/agent/mcp_cleanup.rs @@ -143,3 +143,7 @@ impl AgentInner { } } } + +#[cfg(test)] +#[path = "mcp_cleanup_tests.rs"] +mod tests; diff --git a/crates/openhuman-embed/src/agent/mcp_cleanup_tests.rs b/crates/openhuman-embed/src/agent/mcp_cleanup_tests.rs new file mode 100644 index 00000000000..e773ba54538 --- /dev/null +++ b/crates/openhuman-embed/src/agent/mcp_cleanup_tests.rs @@ -0,0 +1,67 @@ +use super::*; + +fn pending(home: std::path::PathBuf) -> PendingCleanup { + PendingCleanup { + cleanup: None, + instance: Weak::new(), + home, + purge: true, + retry_running: Default::default(), + } +} + +#[test] +fn confirmed_cleanup_purges_before_releasing_a_dead_agent_reservation() { + let scratch = tempfile::tempdir().unwrap(); + let home = scratch.path().join("worker"); + std::fs::create_dir(&home).unwrap(); + std::fs::write(home.join("kept-until-cleanup"), "fixture").unwrap(); + let pending = pending(home.clone()); + let registry = crate::runtime::AgentMap::default(); + { + let mut agents = registry.lock().unwrap(); + agents.insert("worker".into(), pending.instance.clone()); + agents.pending.insert("worker".into(), pending.clone()); + } + finish_removal(®istry, "worker", &pending).unwrap(); + assert!(!home.exists()); + let agents = registry.lock().unwrap(); + assert!(!agents.contains_key("worker")); + assert!(!agents.pending.contains_key("worker")); +} + +#[test] +fn a_failed_purge_keeps_the_dead_agent_reservation_and_purge_intent() { + let scratch = tempfile::tempdir().unwrap(); + let home = scratch.path().join("worker"); + std::fs::write(&home, "a file cannot be purged as a directory").unwrap(); + let pending = pending(home.clone()); + let registry = crate::runtime::AgentMap::default(); + { + let mut agents = registry.lock().unwrap(); + agents.insert("worker".into(), pending.instance.clone()); + agents.pending.insert("worker".into(), pending.clone()); + } + assert!(matches!( + finish_removal(®istry, "worker", &pending), + Err(AgentError::Workspace { .. }) + )); + assert!(home.exists()); + let agents = registry.lock().unwrap(); + assert!(agents.contains_key("worker")); + assert!(agents.pending.get("worker").unwrap().purge); +} + +#[test] +fn an_already_released_reservation_does_not_purge_a_home() { + let scratch = tempfile::tempdir().unwrap(); + let home = scratch.path().join("worker"); + std::fs::create_dir(&home).unwrap(); + finish_removal( + &crate::runtime::AgentMap::default(), + "worker", + &pending(home.clone()), + ) + .unwrap(); + assert!(home.exists()); +} From 43cbf2f349dd8ea0f14b38b9c7bd2c46117b4b7b Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Mon, 12 Oct 2026 17:48:37 +0300 Subject: [PATCH 15/45] docs(embed): clarify agent teardown and MCP cleanup semantics Document that teardown captures the original MCP owner's cleanup without creating a client, and that the agent id is released only after that cleanup completes. Also note that a failed close returns AgentError::McpCleanup, leaves the id and purge intent reserved, and can be retried by calling remove again, including after the last handle is dropped. Auto-committed-on: dragonfly Co-authored-by: Medulla --- crates/openhuman-embed/src/agent/mod.rs | 8 ++++---- crates/openhuman-embed/src/runtime/lifecycle.rs | 7 ++++++- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/crates/openhuman-embed/src/agent/mod.rs b/crates/openhuman-embed/src/agent/mod.rs index 011f8e2fa68..90970b7082f 100644 --- a/crates/openhuman-embed/src/agent/mod.rs +++ b/crates/openhuman-embed/src/agent/mod.rs @@ -144,9 +144,9 @@ pub(crate) struct AgentInner { } impl AgentInner { - /// Releases what the core keeps for this agent: parked approvals are - /// denied with `resolution`, its state slots and MCP host are dropped, - /// and its context leaves the registry. Runs once. + /// Denies parked approvals, clears state slots and deregisters the context. + /// Captures the original MCP owner's cleanup without creating a client; + /// explicit removal awaits it and final drop schedules it. Runs once. pub(crate) fn teardown(&self, resolution: &str) { if !self.lifecycle.begin_teardown() { return; @@ -199,7 +199,7 @@ impl Drop for AgentInner { /// A handle to one agent on a runtime. /// /// Cheap to clone; every clone drives the same agent. The id is released for -/// reuse once the last clone is dropped. Dropping an agent does **not** remove +/// reuse after the last clone's MCP cleanup completes. Dropping an agent does **not** remove /// its directories — its transcripts and memory persist with the runtime's /// workspace. #[derive(Clone)] diff --git a/crates/openhuman-embed/src/runtime/lifecycle.rs b/crates/openhuman-embed/src/runtime/lifecycle.rs index dfc49e42564..9c0f8e95f29 100644 --- a/crates/openhuman-embed/src/runtime/lifecycle.rs +++ b/crates/openhuman-embed/src/runtime/lifecycle.rs @@ -83,10 +83,15 @@ impl Runtime { /// [`CoreError::AgentRemoved`](crate::CoreError::AgentRemoved); its parked /// approvals are denied with resolution `agent_removed`; turns in /// flight end with the same error, waited on for up to ten seconds; its - /// state slots and MCP host are dropped and its context deregistered, so + /// state slots are cleared, its original MCP object is closed, and its context deregistered, so /// its cron jobs stay dormant. The id is free for reuse once cleanup ends. /// Cancelling an already-started removal still tears down the old instance /// and performs a requested purge before releasing its id. + /// A failed close returns [`AgentError::McpCleanup`] and leaves the id and + /// purge intent reserved. Detached cleanup makes at most three attempts on + /// the process executor. Call this method again to retry the captured owner, + /// including after the last agent handle was dropped. Runtime drop does + /// not execute the process signal's shutdown hooks. /// /// Cancellation is cooperative: a tool already executing when the agent /// is removed, or a sub-agent it detached, may finish after this returns. From c4794fd6108c2732fd13b20df513d2ae55a978a5 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Mon, 12 Oct 2026 17:57:11 +0300 Subject: [PATCH 16/45] test(mcp): cover blocked close cleanup deadline and owner retry Add a lifecycle fixture hook that can hold Close open, and a test asserting that agent cleanup returns within its overall deadline even when the underlying operation timeout is much longer. The test also verifies the client stays registered in StartingClose so a retry can finish the close and evict the owner. Auto-committed-on: dragonfly Co-authored-by: Medulla --- .../src/mcp/module_client_owner_tests.rs | 93 +++++++++++++++++++ 1 file changed, 93 insertions(+) diff --git a/crates/openhuman-core/src/mcp/module_client_owner_tests.rs b/crates/openhuman-core/src/mcp/module_client_owner_tests.rs index 836971b8ba5..4ba8a6fe99b 100644 --- a/crates/openhuman-core/src/mcp/module_client_owner_tests.rs +++ b/crates/openhuman-core/src/mcp/module_client_owner_tests.rs @@ -10,12 +10,19 @@ struct LifecycleFixture { close_poll_started: Arc, release_close_poll: Arc, fail_close: Arc, + block_close: Arc, + close_started: Arc, + release_close: Arc, } #[tinybus::interface(name = "ai.tinyhumans.tinymcp.Mcp")] impl LifecycleFixture { async fn close(&self, _data_dir: String) -> tinybus::Result> { self.closes.fetch_add(1, Ordering::SeqCst); + if self.block_close.swap(false, Ordering::SeqCst) { + self.close_started.notify_one(); + self.release_close.notified().await; + } if self.fail_close.load(Ordering::SeqCst) { return Err(tinybus::Error::failed("fixture cleanup unavailable")); } @@ -316,3 +323,89 @@ async fn cleanup_receipts_stay_with_the_original_module_after_alias_replacement( assert_eq!(replacement_fixture.closes.load(Ordering::SeqCst), 0); assert_eq!(replacement_fixture.polls.load(Ordering::SeqCst), 0); } + +#[tokio::test(start_paused = true)] +async fn a_blocked_close_has_an_overall_cleanup_deadline_and_keeps_the_owner_for_retry() { + let scratch = tempfile::tempdir().unwrap(); + let directory = scratch.path().join("agents/blocked-close"); + let bus = MemoryBus::new(); + Broker::new().spawn(bus.clone()); + let service = Connection::connect(bus.connect().await.unwrap()) + .await + .unwrap(); + let caller = Connection::connect(bus.connect().await.unwrap()) + .await + .unwrap(); + let fixture = LifecycleFixture::default(); + service + .serve_at( + ObjectPath::new(names::OBJECT_PATH).unwrap(), + fixture.clone(), + ) + .await + .unwrap(); + service.request_name(names::INTERFACE).await.unwrap(); + let port = TinyMcpPort::new( + ModuleClient::fixture( + caller + .proxy(names::INTERFACE, names::OBJECT_PATH, names::INTERFACE) + .unwrap(), + ) + .with_timeout(Duration::from_secs(120)), + ); + port.transition_status("original-open").await.unwrap(); + let client = Arc::new(McpConfiguredClient::with_port( + directory.to_str().unwrap().into(), + Box::new(port), + )); + // The acknowledged object isolates lifecycle routing from module admission. + *client.state.lock().await = State::Open(OpenObject { + config: McpClientConfig::default(), + object_path: "/test/original-child".into(), + }); + CLIENTS + .get_or_init(|| Mutex::new(HashMap::new())) + .lock() + .unwrap() + .insert(directory.clone(), Arc::clone(&client)); + let cleanup = agent_cleanup::take_agent_cleanup(scratch.path(), "blocked-close").unwrap(); + fixture.block_close.store(true, Ordering::SeqCst); + let attempt = cleanup.clone(); + let mut task = tokio::spawn(async move { attempt.finish().await }); + fixture.close_started.notified().await; + let outcome = tokio::time::timeout(Duration::from_secs(31), &mut task).await; + if outcome.is_err() { + task.abort(); + let _ = task.await; + fixture.release_close.notify_one(); + CLIENTS.get().unwrap().lock().unwrap().remove(&directory); + } + let result = outcome.expect("cleanup must return within 30 seconds even when Close has a 120-second operation timeout") + .expect("cleanup task must complete normally"); + assert_eq!(result, Err(ModuleCallError::TransportFailed)); + assert!(matches!( + &*client.state.lock().await, + State::StartingClose(_) + )); + assert!(Arc::ptr_eq( + CLIENTS + .get() + .unwrap() + .lock() + .unwrap() + .get(&directory) + .unwrap(), + &client + )); + assert!(client.retired.load(Ordering::Acquire)); + // Retry the captured owner; timing out must not evict its close intent. + fixture.release_close.notify_one(); + cleanup.finish().await.unwrap(); + assert_eq!(fixture.closes.load(Ordering::SeqCst), 2); + assert!(!CLIENTS + .get() + .unwrap() + .lock() + .unwrap() + .contains_key(&directory)); +} From 0311a4281ec87746dbae41ec3dc30e8284387b87 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Mon, 12 Oct 2026 17:57:44 +0300 Subject: [PATCH 17/45] docs(mcp): clarify agent cleanup runtime requirements Document that captured cleanup runs on the process module executor when modules are enabled, and that callers without modules must keep their current Tokio runtime alive. Auto-committed-on: dragonfly Co-authored-by: Medulla --- crates/openhuman-core/src/mcp/agent_cleanup.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/openhuman-core/src/mcp/agent_cleanup.rs b/crates/openhuman-core/src/mcp/agent_cleanup.rs index 688993c59ca..98c909ec2cf 100644 --- a/crates/openhuman-core/src/mcp/agent_cleanup.rs +++ b/crates/openhuman-core/src/mcp/agent_cleanup.rs @@ -70,7 +70,8 @@ impl AgentMcpCleanup { } } -/// Run captured cleanup independently of the caller's Tokio runtime. +/// Run captured cleanup on the process module executor when modules are enabled. +/// Without modules, the caller must keep its current Tokio runtime alive. /// This creates no configured client and loads no MCP artifact. pub fn spawn_agent_cleanup( future: F, From 4c4fe6aaad131e04039484ff0482a6948187ce5f Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Mon, 12 Oct 2026 18:05:39 +0300 Subject: [PATCH 18/45] test(mcp): build host overrides via default and field assignment Reworked the test setup to construct HostOverrides with Default and then assign approval_scope, instead of struct update syntax. This keeps the tests compiling when the struct gains non-Default fields or loses the update-syntax compatibility. Auto-committed-on: dragonfly Co-authored-by: Medulla --- .../src/mcp/module_client_owner_tests.rs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/crates/openhuman-core/src/mcp/module_client_owner_tests.rs b/crates/openhuman-core/src/mcp/module_client_owner_tests.rs index 4ba8a6fe99b..fce94fcda95 100644 --- a/crates/openhuman-core/src/mcp/module_client_owner_tests.rs +++ b/crates/openhuman-core/src/mcp/module_client_owner_tests.rs @@ -151,10 +151,9 @@ async fn an_old_closed_context_cannot_create_the_reused_agents_first_client() { config.workspace_dir = scratch.path().into(); let parent = CoreContext::for_test(DomainSet::full(), Some(config.workspace_dir.clone())); let old_scope = Arc::new(crate::security::approval::ApprovalScope::default()); - let old_overrides = Arc::new(crate::agent::host_overrides::HostOverrides { - approval_scope: Some(Arc::clone(&old_scope)), - ..Default::default() - }); + let mut old_overrides = crate::agent::host_overrides::HostOverrides::default(); + old_overrides.approval_scope = Some(Arc::clone(&old_scope)); + let old_overrides = Arc::new(old_overrides); let mut overlay = ContextOverlay::new(config.clone(), DomainSet::full(), ToolGroups::none()) .session_agent("worker"); overlay.host_overrides = Some(old_overrides); @@ -174,10 +173,10 @@ async fn an_old_closed_context_cannot_create_the_reused_agents_first_client() { let mut new_overlay = ContextOverlay::new(config.clone(), DomainSet::full(), ToolGroups::none()) .session_agent("worker"); - new_overlay.host_overrides = Some(Arc::new(crate::agent::host_overrides::HostOverrides { - approval_scope: Some(Arc::new(crate::security::approval::ApprovalScope::default())), - ..Default::default() - })); + let mut new_overrides = crate::agent::host_overrides::HostOverrides::default(); + new_overrides.approval_scope = + Some(Arc::new(crate::security::approval::ApprovalScope::default())); + new_overlay.host_overrides = Some(Arc::new(new_overrides)); CoreContext::scope(parent.derive_with(new_overlay), async { McpConfiguredClient::for_config(&config) .expect("the new instance has an open lifecycle scope"); From 04f37c1b6a748146928368dc5088846168ea700c Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Mon, 12 Oct 2026 18:08:12 +0300 Subject: [PATCH 19/45] chore: files changed crates/openhuman-core/src/mcp/agent_cleanup.rs,crates/openhuman-embed/src/agent Auto-committed-on: dragonfly Co-authored-by: Medulla --- crates/openhuman-core/src/mcp/agent_cleanup.rs | 8 ++++++-- crates/openhuman-embed/src/agent/mcp_cleanup.rs | 15 +++++++++++---- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/crates/openhuman-core/src/mcp/agent_cleanup.rs b/crates/openhuman-core/src/mcp/agent_cleanup.rs index 98c909ec2cf..7324a9e04a5 100644 --- a/crates/openhuman-core/src/mcp/agent_cleanup.rs +++ b/crates/openhuman-core/src/mcp/agent_cleanup.rs @@ -34,9 +34,13 @@ pub(super) fn agent_is_removed() -> bool { impl AgentMcpCleanup { /// Close the original object and evict it only after confirmed cleanup. - /// Cancellation or error retains the same client and transition receipt. + /// The whole attempt, including lock acquisition and bus calls, has a + /// 30-second deadline. Timeout returns `TransportFailed`; cancellation or + /// error retains the same client and transition receipt for explicit retry. pub async fn finish(&self) -> Result<(), ModuleCallError> { - self.client.close().await?; + tokio::time::timeout(TRANSITION_TIMEOUT, self.client.close()) + .await + .map_err(|_| ModuleCallError::TransportFailed)??; self.evict(); Ok(()) } diff --git a/crates/openhuman-embed/src/agent/mcp_cleanup.rs b/crates/openhuman-embed/src/agent/mcp_cleanup.rs index bc64b1660cf..9d04aae03ea 100644 --- a/crates/openhuman-embed/src/agent/mcp_cleanup.rs +++ b/crates/openhuman-embed/src/agent/mcp_cleanup.rs @@ -31,7 +31,9 @@ pub(crate) fn finish_removal( } /// Run at most three retries on the process module executor. The future owns -/// the workspace guard only while running; retained records contain no guard. +/// the workspace guard through at most 90 seconds of polled MCP cleanup and +/// 750 ms of backoff. It releases the guard before filesystem purge; retained +/// records contain no guard. Executor scheduling can delay these timed waits. pub(crate) fn defer_pending( registry: Arc, id: String, @@ -54,13 +56,18 @@ pub(crate) fn defer_pending( let running = Arc::clone(&pending.retry_running); let executor_failed = Arc::clone(&running); let future = async move { - let _guard = guard; + let mut guard = Some(guard); for attempt in 0..3 { let result = match &pending.cleanup { Some(cleanup) => cleanup.finish().await.map_err(AgentError::McpCleanup), None => Ok(()), - } - .and_then(|()| finish_removal(®istry, &id, &pending)); + }; + let result = result.and_then(|()| { + // Confirmed module cleanup no longer needs the process/workspace + // owner. Filesystem purge must not extend its retention lifetime. + drop(guard.take()); + finish_removal(®istry, &id, &pending) + }); match result { Ok(()) => { running.store(false, Ordering::Release); From 4268df364e9bfd519d0f8449568ee099311bc586 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Mon, 12 Oct 2026 18:14:12 +0300 Subject: [PATCH 20/45] refactor(agent): extract guard release into complete_with_guard Cleanup completion now drops the runtime guard through a shared helper instead of inline, so the ordering is exercised by a test. The new test confirms the last guard stays live and admission stays reserved while completion is blocked, then releases both once it finishes. Auto-committed-on: dragonfly Co-authored-by: Medulla --- .../openhuman-embed/src/agent/mcp_cleanup.rs | 13 +++++-- .../src/agent/mcp_cleanup_tests.rs | 38 +++++++++++++++++++ 2 files changed, 47 insertions(+), 4 deletions(-) diff --git a/crates/openhuman-embed/src/agent/mcp_cleanup.rs b/crates/openhuman-embed/src/agent/mcp_cleanup.rs index 9d04aae03ea..3ff76b5d637 100644 --- a/crates/openhuman-embed/src/agent/mcp_cleanup.rs +++ b/crates/openhuman-embed/src/agent/mcp_cleanup.rs @@ -63,10 +63,7 @@ pub(crate) fn defer_pending( None => Ok(()), }; let result = result.and_then(|()| { - // Confirmed module cleanup no longer needs the process/workspace - // owner. Filesystem purge must not extend its retention lifetime. - drop(guard.take()); - finish_removal(®istry, &id, &pending) + complete_with_guard(&mut guard, || finish_removal(®istry, &id, &pending)) }); match result { Ok(()) => { @@ -91,6 +88,14 @@ pub(crate) fn defer_pending( } } +fn complete_with_guard( + guard: &mut Option>, + complete: impl FnOnce() -> T, +) -> T { + drop(guard.take()); + complete() +} + impl AgentInner { pub(crate) fn reserve_cleanup(&self, purge: bool) -> Option { let cleanup = self diff --git a/crates/openhuman-embed/src/agent/mcp_cleanup_tests.rs b/crates/openhuman-embed/src/agent/mcp_cleanup_tests.rs index e773ba54538..06167e7d23d 100644 --- a/crates/openhuman-embed/src/agent/mcp_cleanup_tests.rs +++ b/crates/openhuman-embed/src/agent/mcp_cleanup_tests.rs @@ -65,3 +65,41 @@ fn an_already_released_reservation_does_not_purge_a_home() { .unwrap(); assert!(home.exists()); } + +#[tokio::test] +async fn the_last_runtime_guard_keeps_admission_reserved_through_cleanup_completion() { + let runtime = crate::Runtime::builder() + .config(openhuman_core::config::Config::default()) + .workspace(crate::Workspace::Ephemeral) + .build() + .await + .expect("a real isolated runtime can be built"); + let guard = runtime.guard(); + let weak = Arc::downgrade(&guard); + drop(runtime); + let (entered_tx, entered_rx) = std::sync::mpsc::channel(); + let (release_tx, release_rx) = std::sync::mpsc::channel(); + let worker = std::thread::spawn(move || { + let mut guard = Some(guard); + complete_with_guard(&mut guard, || { + entered_tx.send(()).unwrap(); + release_rx.recv().unwrap(); + }); + }); + entered_rx.recv().unwrap(); + let guard_is_live = weak.upgrade().is_some(); + let admission_is_reserved = + crate::runtime::RUNTIME_LIVE.load(std::sync::atomic::Ordering::Acquire); + release_tx.send(()).unwrap(); + worker.join().unwrap(); + assert!( + guard_is_live, + "completion must retain the last real workspace/runtime guard" + ); + assert!( + admission_is_reserved, + "another runtime must remain excluded while cleanup completion is blocked" + ); + assert!(weak.upgrade().is_none()); + assert!(!crate::runtime::RUNTIME_LIVE.load(std::sync::atomic::Ordering::Acquire)); +} From 0c858656c4e729d606107f65d103dadb63eac634 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Mon, 12 Oct 2026 18:15:34 +0300 Subject: [PATCH 21/45] feat(harness): add mcp cleanup error variant Add a McpCleanup variant to HarnessError so that a failed confirmation of captured MCP owner cleanup during agent removal can be surfaced, and map the corresponding AgentError case onto it. Auto-committed-on: dragonfly Co-authored-by: Medulla --- crates/openhuman-embed/src/harness/error.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/crates/openhuman-embed/src/harness/error.rs b/crates/openhuman-embed/src/harness/error.rs index 3c4a2f28238..e7042793573 100644 --- a/crates/openhuman-embed/src/harness/error.rs +++ b/crates/openhuman-embed/src/harness/error.rs @@ -51,6 +51,10 @@ pub enum HarnessError { )] AlreadyRunning, + /// Removing an agent failed to confirm cleanup of its captured MCP owner. + #[error("MCP cleanup remains pending: {0}")] + McpCleanup(#[source] openhuman_core::modules::client::ModuleCallError), + /// A builder input could not be honoured. #[error("{0}")] Invalid(String), @@ -90,6 +94,7 @@ impl From for HarnessError { use crate::AgentError as A; match err { A::Call(e) => Self::Call(e), + A::McpCleanup(e) => Self::McpCleanup(e), A::Workspace { what, source } => Self::Workspace { what, source }, A::DuplicateId(_) | A::InvalidId { .. } From 4981af51ae074ae08c8052668ddafc7c21bdaad7 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Mon, 12 Oct 2026 18:16:16 +0300 Subject: [PATCH 22/45] fix(embed): map mcp cleanup harness errors Handle the new HarnessError::McpCleanup variant in the agent and runtime error mappers so the match stays exhaustive and cleanup failures surface as proper agent and runtime errors instead of failing to compile. Auto-committed-on: dragonfly Co-authored-by: Medulla --- crates/openhuman-embed/src/agent/build.rs | 1 + crates/openhuman-embed/src/runtime/build.rs | 1 + 2 files changed, 2 insertions(+) diff --git a/crates/openhuman-embed/src/agent/build.rs b/crates/openhuman-embed/src/agent/build.rs index ab72d1eaad0..e056ddab15c 100644 --- a/crates/openhuman-embed/src/agent/build.rs +++ b/crates/openhuman-embed/src/agent/build.rs @@ -472,6 +472,7 @@ fn map_harness_err(err: crate::HarnessError) -> AgentError { crate::HarnessError::Workspace { what, source } => AgentError::Workspace { what, source }, crate::HarnessError::Invalid(msg) => AgentError::Invalid(msg), crate::HarnessError::Call(e) => AgentError::Call(e), + crate::HarnessError::McpCleanup(e) => AgentError::McpCleanup(e), crate::HarnessError::Build(e) => AgentError::Invalid(format!("{e:#}")), crate::HarnessError::AlreadyRunning => AgentError::Invalid(err.to_string()), } diff --git a/crates/openhuman-embed/src/runtime/build.rs b/crates/openhuman-embed/src/runtime/build.rs index ce598b84ad5..62f10d98a7c 100644 --- a/crates/openhuman-embed/src/runtime/build.rs +++ b/crates/openhuman-embed/src/runtime/build.rs @@ -507,6 +507,7 @@ fn map_ws(err: crate::HarnessError) -> RuntimeError { crate::HarnessError::Workspace { what, source } => RuntimeError::Workspace { what, source }, crate::HarnessError::Invalid(msg) => RuntimeError::Invalid(msg), crate::HarnessError::Build(e) => RuntimeError::Build(e), + crate::HarnessError::McpCleanup(e) => RuntimeError::Build(e.into()), crate::HarnessError::Call(e) => RuntimeError::Call(e), crate::HarnessError::AlreadyRunning => RuntimeError::AlreadyRunning, } From 5d529492e23268e548bd1d5264a762b216f98756 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Mon, 12 Oct 2026 18:17:09 +0300 Subject: [PATCH 23/45] test(agent): run cleanup guard test on a dedicated runtime The last-runtime-guard cleanup test now drives its async body on a single-worker multi-thread executor with the agent worker stack size instead of the default test runtime, so the blocking cleanup path is exercised under the same stack constraints as production. Auto-committed-on: dragonfly Co-authored-by: Medulla --- .../src/agent/mcp_cleanup_tests.rs | 78 +++++++++++-------- 1 file changed, 45 insertions(+), 33 deletions(-) diff --git a/crates/openhuman-embed/src/agent/mcp_cleanup_tests.rs b/crates/openhuman-embed/src/agent/mcp_cleanup_tests.rs index 06167e7d23d..ba096aa5236 100644 --- a/crates/openhuman-embed/src/agent/mcp_cleanup_tests.rs +++ b/crates/openhuman-embed/src/agent/mcp_cleanup_tests.rs @@ -66,40 +66,52 @@ fn an_already_released_reservation_does_not_purge_a_home() { assert!(home.exists()); } -#[tokio::test] -async fn the_last_runtime_guard_keeps_admission_reserved_through_cleanup_completion() { - let runtime = crate::Runtime::builder() - .config(openhuman_core::config::Config::default()) - .workspace(crate::Workspace::Ephemeral) +#[test] +fn the_last_runtime_guard_keeps_admission_reserved_through_cleanup_completion() { + let executor = tokio::runtime::Builder::new_multi_thread() + .enable_all() + .worker_threads(1) + .thread_stack_size(openhuman_core::core::runtime::AGENT_WORKER_STACK_BYTES) .build() + .unwrap(); + executor.block_on(async { + tokio::spawn(async { + let runtime = crate::Runtime::builder() + .config(openhuman_core::config::Config::default()) + .workspace(crate::Workspace::Ephemeral) + .build() + .await + .expect("a real isolated runtime can be built"); + let guard = runtime.guard(); + let weak = Arc::downgrade(&guard); + drop(runtime); + let (entered_tx, entered_rx) = std::sync::mpsc::channel(); + let (release_tx, release_rx) = std::sync::mpsc::channel(); + let worker = std::thread::spawn(move || { + let mut guard = Some(guard); + complete_with_guard(&mut guard, || { + entered_tx.send(()).unwrap(); + release_rx.recv().unwrap(); + }); + }); + entered_rx.recv().unwrap(); + let guard_is_live = weak.upgrade().is_some(); + let admission_is_reserved = + crate::runtime::RUNTIME_LIVE.load(std::sync::atomic::Ordering::Acquire); + release_tx.send(()).unwrap(); + worker.join().unwrap(); + assert!( + guard_is_live, + "completion must retain the last real workspace/runtime guard" + ); + assert!( + admission_is_reserved, + "another runtime must remain excluded while cleanup completion is blocked" + ); + assert!(weak.upgrade().is_none()); + assert!(!crate::runtime::RUNTIME_LIVE.load(std::sync::atomic::Ordering::Acquire)); + }) .await - .expect("a real isolated runtime can be built"); - let guard = runtime.guard(); - let weak = Arc::downgrade(&guard); - drop(runtime); - let (entered_tx, entered_rx) = std::sync::mpsc::channel(); - let (release_tx, release_rx) = std::sync::mpsc::channel(); - let worker = std::thread::spawn(move || { - let mut guard = Some(guard); - complete_with_guard(&mut guard, || { - entered_tx.send(()).unwrap(); - release_rx.recv().unwrap(); - }); + .unwrap(); }); - entered_rx.recv().unwrap(); - let guard_is_live = weak.upgrade().is_some(); - let admission_is_reserved = - crate::runtime::RUNTIME_LIVE.load(std::sync::atomic::Ordering::Acquire); - release_tx.send(()).unwrap(); - worker.join().unwrap(); - assert!( - guard_is_live, - "completion must retain the last real workspace/runtime guard" - ); - assert!( - admission_is_reserved, - "another runtime must remain excluded while cleanup completion is blocked" - ); - assert!(weak.upgrade().is_none()); - assert!(!crate::runtime::RUNTIME_LIVE.load(std::sync::atomic::Ordering::Acquire)); } From c210b8378baa95b8cb16706a7a1275878c4a5618 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Mon, 12 Oct 2026 18:17:55 +0300 Subject: [PATCH 24/45] docs(agent): clarify mcp cleanup guard lifetime The doc comment on defer_pending now states that the synchronous filesystem purge and executor scheduling fall outside the MCP wait bound, and that the last guard keeps runtime admission reserved through completion. In complete_with_guard the guard is held in a local binding instead of being dropped before the callback, so a new runtime with another agent map cannot reopen the workspace mid-completion. Auto-committed-on: dragonfly Co-authored-by: Medulla --- crates/openhuman-embed/src/agent/mcp_cleanup.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/crates/openhuman-embed/src/agent/mcp_cleanup.rs b/crates/openhuman-embed/src/agent/mcp_cleanup.rs index 3ff76b5d637..ef6a09483fe 100644 --- a/crates/openhuman-embed/src/agent/mcp_cleanup.rs +++ b/crates/openhuman-embed/src/agent/mcp_cleanup.rs @@ -32,8 +32,9 @@ pub(crate) fn finish_removal( /// Run at most three retries on the process module executor. The future owns /// the workspace guard through at most 90 seconds of polled MCP cleanup and -/// 750 ms of backoff. It releases the guard before filesystem purge; retained -/// records contain no guard. Executor scheduling can delay these timed waits. +/// 750 ms of backoff, plus synchronous filesystem purge. Purge and executor +/// scheduling are outside the MCP wait bound. Retained records contain no +/// guard; the last guard keeps runtime admission reserved through completion. pub(crate) fn defer_pending( registry: Arc, id: String, @@ -92,7 +93,9 @@ fn complete_with_guard( guard: &mut Option>, complete: impl FnOnce() -> T, ) -> T { - drop(guard.take()); + // Releasing the last guard permits a new runtime with another agent map + // to reopen this workspace. Keep admission reserved through the callback. + let _guard = guard.take(); complete() } From f9a2b81866c77ac26f32fee7053f92c6c10fdea8 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Mon, 12 Oct 2026 18:18:41 +0300 Subject: [PATCH 25/45] test(agent): assert retry guard is retained after failed completion The cleanup test now returns whether the retry guard is still held from the worker thread and asserts it, so a failed completion that may retry is verified to keep runtime admission until the task ends. Auto-committed-on: dragonfly Co-authored-by: Medulla --- crates/openhuman-embed/src/agent/mcp_cleanup_tests.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/openhuman-embed/src/agent/mcp_cleanup_tests.rs b/crates/openhuman-embed/src/agent/mcp_cleanup_tests.rs index ba096aa5236..d053e3139a3 100644 --- a/crates/openhuman-embed/src/agent/mcp_cleanup_tests.rs +++ b/crates/openhuman-embed/src/agent/mcp_cleanup_tests.rs @@ -93,13 +93,15 @@ fn the_last_runtime_guard_keeps_admission_reserved_through_cleanup_completion() entered_tx.send(()).unwrap(); release_rx.recv().unwrap(); }); + guard.is_some() }); entered_rx.recv().unwrap(); let guard_is_live = weak.upgrade().is_some(); let admission_is_reserved = crate::runtime::RUNTIME_LIVE.load(std::sync::atomic::Ordering::Acquire); release_tx.send(()).unwrap(); - worker.join().unwrap(); + let retry_guard_is_retained = worker.join().unwrap(); + assert!(retry_guard_is_retained, "a failed completion may retry and must retain runtime admission until the task ends"); assert!( guard_is_live, "completion must retain the last real workspace/runtime guard" From 0363183b2b9c6e09572b8bd12c2ec76fcffb37ad Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Mon, 12 Oct 2026 18:19:44 +0300 Subject: [PATCH 26/45] test(agent): assert guard survives cleanup failure The cleanup test now makes the completion closure return an error and checks that the runtime guard is still held afterwards, so the test covers the failure path where cleanup aborts before releasing admission. Auto-committed-on: dragonfly Co-authored-by: Medulla --- crates/openhuman-embed/src/agent/mcp_cleanup_tests.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/openhuman-embed/src/agent/mcp_cleanup_tests.rs b/crates/openhuman-embed/src/agent/mcp_cleanup_tests.rs index d053e3139a3..92e7924b347 100644 --- a/crates/openhuman-embed/src/agent/mcp_cleanup_tests.rs +++ b/crates/openhuman-embed/src/agent/mcp_cleanup_tests.rs @@ -89,11 +89,12 @@ fn the_last_runtime_guard_keeps_admission_reserved_through_cleanup_completion() let (release_tx, release_rx) = std::sync::mpsc::channel(); let worker = std::thread::spawn(move || { let mut guard = Some(guard); - complete_with_guard(&mut guard, || { + let result = complete_with_guard(&mut guard, || { entered_tx.send(()).unwrap(); release_rx.recv().unwrap(); + Err::<(), ()>(()) }); - guard.is_some() + result.is_err() && guard.is_some() }); entered_rx.recv().unwrap(); let guard_is_live = weak.upgrade().is_some(); From a5aadbfe357dfa4911776f608dd2176485d0fab6 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Mon, 12 Oct 2026 18:20:06 +0300 Subject: [PATCH 27/45] fix(openhuman-embed): keep admission guard alive during completion The guard was being taken out of the option and dropped before the callback ran, so admission was released too early and a new runtime could reopen the workspace mid-completion. Borrowing the guard instead keeps it held for the duration of the callback. Auto-committed-on: dragonfly Co-authored-by: Medulla --- crates/openhuman-embed/src/agent/mcp_cleanup.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/openhuman-embed/src/agent/mcp_cleanup.rs b/crates/openhuman-embed/src/agent/mcp_cleanup.rs index ef6a09483fe..8fb0244e1ec 100644 --- a/crates/openhuman-embed/src/agent/mcp_cleanup.rs +++ b/crates/openhuman-embed/src/agent/mcp_cleanup.rs @@ -95,7 +95,7 @@ fn complete_with_guard( ) -> T { // Releasing the last guard permits a new runtime with another agent map // to reopen this workspace. Keep admission reserved through the callback. - let _guard = guard.take(); + let _guard = guard.as_ref(); complete() } From c0244a7b042af2fe6f00e1aef905c1c6626b0fa0 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Mon, 12 Oct 2026 18:25:09 +0300 Subject: [PATCH 28/45] refactor(agent): extract cleanup retry loop into a helper The deferred MCP cleanup retry loop now lives in a reusable retry_cleanup helper that takes the close and completion steps as closures, so the same retry and guard-release behaviour can be exercised directly. A test covers the exhausted-retry path, confirming the runtime guard is released while the ephemeral workspace is preserved when the close is never confirmed. Auto-committed-on: dragonfly Co-authored-by: Medulla --- .../openhuman-embed/src/agent/mcp_cleanup.rs | 58 ++++++++++++------- .../src/agent/mcp_cleanup_tests.rs | 55 ++++++++++++++++++ 2 files changed, 92 insertions(+), 21 deletions(-) diff --git a/crates/openhuman-embed/src/agent/mcp_cleanup.rs b/crates/openhuman-embed/src/agent/mcp_cleanup.rs index 8fb0244e1ec..24bffa48fb6 100644 --- a/crates/openhuman-embed/src/agent/mcp_cleanup.rs +++ b/crates/openhuman-embed/src/agent/mcp_cleanup.rs @@ -57,28 +57,20 @@ pub(crate) fn defer_pending( let running = Arc::clone(&pending.retry_running); let executor_failed = Arc::clone(&running); let future = async move { - let mut guard = Some(guard); - for attempt in 0..3 { - let result = match &pending.cleanup { - Some(cleanup) => cleanup.finish().await.map_err(AgentError::McpCleanup), - None => Ok(()), - }; - let result = result.and_then(|()| { - complete_with_guard(&mut guard, || finish_removal(®istry, &id, &pending)) - }); - match result { - Ok(()) => { - running.store(false, Ordering::Release); - return; - } - Err(error) => { - log::warn!("[embed][agent] captured cleanup remains pending: {error}") + retry_cleanup( + guard, + || { + let cleanup = pending.cleanup.clone(); + async move { + match cleanup { + Some(cleanup) => cleanup.finish().await.map_err(AgentError::McpCleanup), + None => Ok(()), + } } - } - if attempt < 2 { - tokio::time::sleep(std::time::Duration::from_millis(250 * (1 << attempt))).await; - } - } + }, + || finish_removal(®istry, &id, &pending), + ) + .await; // The original receipt, id reservation and purge intent stay in the // registry. A later remove_agent call retries them explicitly. running.store(false, Ordering::Release); @@ -89,6 +81,30 @@ pub(crate) fn defer_pending( } } +async fn retry_cleanup( + guard: Arc, + mut close: F, + mut complete: C, +) where + F: FnMut() -> Fut, + Fut: std::future::Future>, + C: FnMut() -> Result<(), AgentError>, +{ + let mut guard = Some(guard); + for attempt in 0..3 { + let result = close() + .await + .and_then(|()| complete_with_guard(&mut guard, &mut complete)); + match result { + Ok(()) => return, + Err(error) => log::warn!("[embed][agent] captured cleanup remains pending: {error}"), + } + if attempt < 2 { + tokio::time::sleep(std::time::Duration::from_millis(250 * (1 << attempt))).await; + } + } +} + fn complete_with_guard( guard: &mut Option>, complete: impl FnOnce() -> T, diff --git a/crates/openhuman-embed/src/agent/mcp_cleanup_tests.rs b/crates/openhuman-embed/src/agent/mcp_cleanup_tests.rs index 92e7924b347..68ccb92827f 100644 --- a/crates/openhuman-embed/src/agent/mcp_cleanup_tests.rs +++ b/crates/openhuman-embed/src/agent/mcp_cleanup_tests.rs @@ -118,3 +118,58 @@ fn the_last_runtime_guard_keeps_admission_reserved_through_cleanup_completion() .unwrap(); }); } + +#[test] +fn exhausted_cleanup_releases_runtime_admission_but_preserves_the_ephemeral_workspace() { + let executor = tokio::runtime::Builder::new_multi_thread() + .enable_all() + .worker_threads(1) + .thread_stack_size(openhuman_core::core::runtime::AGENT_WORKER_STACK_BYTES) + .build() + .unwrap(); + executor.block_on(async { + tokio::spawn(async { + let runtime = crate::Runtime::builder() + .config(openhuman_core::config::Config::default()) + .workspace(crate::Workspace::Ephemeral) + .build() + .await + .unwrap(); + let root = runtime.workspace_dir().to_path_buf(); + let home = root.join("agents/unresolved"); + std::fs::create_dir_all(&home).unwrap(); + std::fs::write(home.join("module-state"), "must survive unconfirmed close").unwrap(); + let guard = runtime.guard(); + let weak = Arc::downgrade(&guard); + drop(runtime); + let attempts = std::sync::atomic::AtomicUsize::new(0); + retry_cleanup( + guard, + || { + attempts.fetch_add(1, Ordering::SeqCst); + async { + Err(AgentError::McpCleanup( + openhuman_core::modules::client::ModuleCallError::ModuleFault, + )) + } + }, + || panic!("unconfirmed close must never purge"), + ) + .await; + let home_was_preserved = home.join("module-state").exists(); + let _ = std::fs::remove_dir_all(&root); + assert_eq!(attempts.load(Ordering::SeqCst), 3); + assert!( + weak.upgrade().is_none(), + "exhaustion must release the process guard" + ); + assert!(!crate::runtime::RUNTIME_LIVE.load(Ordering::Acquire)); + assert!( + home_was_preserved, + "failed cleanup must preserve the ephemeral workspace after the last guard drops" + ); + }) + .await + .unwrap(); + }); +} From 089df7ecaf98e433ca7327a910e1b5c54d57a08b Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Mon, 12 Oct 2026 18:28:40 +0300 Subject: [PATCH 29/45] test(agent): cover ephemeral workspace cleanup on cancel Add tests asserting that a confirmed cleanup removes the last owner's ephemeral workspace while dropping an unpolled cleanup future leaves it intact, so cancellation cannot silently delete a workspace that was never released. Auto-committed-on: dragonfly Co-authored-by: Medulla --- .../src/agent/mcp_cleanup_tests.rs | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/crates/openhuman-embed/src/agent/mcp_cleanup_tests.rs b/crates/openhuman-embed/src/agent/mcp_cleanup_tests.rs index 68ccb92827f..b9a8fe92b80 100644 --- a/crates/openhuman-embed/src/agent/mcp_cleanup_tests.rs +++ b/crates/openhuman-embed/src/agent/mcp_cleanup_tests.rs @@ -173,3 +173,53 @@ fn exhausted_cleanup_releases_runtime_admission_but_preserves_the_ephemeral_work .unwrap(); }); } + +fn workspace_cleanup_control(cancel: bool) { + let executor = tokio::runtime::Builder::new_multi_thread() + .enable_all() + .worker_threads(1) + .thread_stack_size(openhuman_core::core::runtime::AGENT_WORKER_STACK_BYTES) + .build() + .unwrap(); + executor.block_on(async { + tokio::spawn(async move { + let runtime = crate::Runtime::builder() + .config(openhuman_core::config::Config::default()) + .workspace(crate::Workspace::Ephemeral) + .build() + .await + .unwrap(); + let root = runtime.workspace_dir().to_path_buf(); + let guard = runtime.guard(); + let weak = Arc::downgrade(&guard); + drop(runtime); + let cleanup = retry_cleanup(guard, || async { Ok(()) }, || Ok(())); + if cancel { + // Executor rejection or an unpolled task must preserve its home. + drop(cleanup); + } else { + cleanup.await; + } + let workspace_exists = root.exists(); + let _ = std::fs::remove_dir_all(&root); + assert!(weak.upgrade().is_none()); + assert!(!crate::runtime::RUNTIME_LIVE.load(Ordering::Acquire)); + assert_eq!( + workspace_exists, cancel, + "only confirmed cleanup may remove the ephemeral workspace" + ); + }) + .await + .unwrap(); + }); +} + +#[test] +fn confirmed_cleanup_still_removes_the_last_owners_ephemeral_workspace() { + workspace_cleanup_control(false); +} + +#[test] +fn cancelling_an_unpolled_cleanup_preserves_its_ephemeral_workspace() { + workspace_cleanup_control(true); +} From 1bc675bf0d5f774f6c968e23199b550b50a90078 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Mon, 12 Oct 2026 18:30:02 +0300 Subject: [PATCH 30/45] fix(embed): preserve ephemeral workspace when cleanup is unconfirmed Agent cleanup now marks the runtime guard to preserve its ephemeral workspace whenever removal cannot be confirmed, including when a scheduled cleanup future is dropped before completion. The guard skips deleting the workspace and logs its location instead, so unconfirmed state is not erased and can be reclaimed manually. Auto-committed-on: dragonfly Co-authored-by: Medulla --- .../openhuman-embed/src/agent/mcp_cleanup.rs | 92 +++++++++++++------ crates/openhuman-embed/src/runtime/mod.rs | 44 ++++++--- 2 files changed, 95 insertions(+), 41 deletions(-) diff --git a/crates/openhuman-embed/src/agent/mcp_cleanup.rs b/crates/openhuman-embed/src/agent/mcp_cleanup.rs index 24bffa48fb6..f7288703a20 100644 --- a/crates/openhuman-embed/src/agent/mcp_cleanup.rs +++ b/crates/openhuman-embed/src/agent/mcp_cleanup.rs @@ -47,6 +47,7 @@ pub(crate) fn defer_pending( .is_none_or(|cleanup| cleanup.finish_if_idle()) { if let Err(error) = finish_removal(®istry, &id, &pending) { + guard.preserve_ephemeral_workspace(); log::warn!("[embed][agent] removed home cleanup remains pending: {error}"); } return; @@ -56,23 +57,30 @@ pub(crate) fn defer_pending( } let running = Arc::clone(&pending.retry_running); let executor_failed = Arc::clone(&running); - let future = async move { - retry_cleanup( - guard, - || { - let cleanup = pending.cleanup.clone(); - async move { - match cleanup { - Some(cleanup) => cleanup.finish().await.map_err(AgentError::McpCleanup), - None => Ok(()), - } + let pending_for_completion = pending.clone(); + // Construct the RAII owner before scheduling, so rejection or cancellation + // of an unpolled future cannot delete the unconfirmed workspace. + let cleanup = retry_cleanup( + guard, + move || { + let cleanup = pending.cleanup.clone(); + async move { + match cleanup { + Some(cleanup) => cleanup.finish().await.map_err(AgentError::McpCleanup), + None => Ok(()), } - }, - || finish_removal(®istry, &id, &pending), - ) - .await; - // The original receipt, id reservation and purge intent stay in the - // registry. A later remove_agent call retries them explicitly. + } + }, + { + let registry = Arc::clone(®istry); + let pending = pending_for_completion; + move || finish_removal(®istry, &id, &pending) + }, + ); + let future = async move { + cleanup.await; + // Retained metadata has no runtime guard. A live Runtime can explicitly + // retry; after it is gone, retained files require manual reclamation. running.store(false, Ordering::Release); }; if let Err(error) = openhuman_core::mcp::host::spawn_agent_cleanup(future) { @@ -81,26 +89,52 @@ pub(crate) fn defer_pending( } } -async fn retry_cleanup( +struct CleanupWorkspaceOwner { + guard: Option>, + confirmed: bool, +} + +impl Drop for CleanupWorkspaceOwner { + fn drop(&mut self) { + if !self.confirmed { + if let Some(guard) = &self.guard { + guard.preserve_ephemeral_workspace(); + } + } + } +} + +fn retry_cleanup( guard: Arc, mut close: F, mut complete: C, -) where +) -> impl std::future::Future +where F: FnMut() -> Fut, Fut: std::future::Future>, C: FnMut() -> Result<(), AgentError>, { - let mut guard = Some(guard); - for attempt in 0..3 { - let result = close() - .await - .and_then(|()| complete_with_guard(&mut guard, &mut complete)); - match result { - Ok(()) => return, - Err(error) => log::warn!("[embed][agent] captured cleanup remains pending: {error}"), - } - if attempt < 2 { - tokio::time::sleep(std::time::Duration::from_millis(250 * (1 << attempt))).await; + let mut owner = CleanupWorkspaceOwner { + guard: Some(guard), + confirmed: false, + }; + async move { + for attempt in 0..3 { + let result = close() + .await + .and_then(|()| complete_with_guard(&mut owner.guard, &mut complete)); + match result { + Ok(()) => { + owner.confirmed = true; + return; + } + Err(error) => { + log::warn!("[embed][agent] captured cleanup remains pending: {error}") + } + } + if attempt < 2 { + tokio::time::sleep(std::time::Duration::from_millis(250 * (1 << attempt))).await; + } } } } diff --git a/crates/openhuman-embed/src/runtime/mod.rs b/crates/openhuman-embed/src/runtime/mod.rs index 5c89640207a..82ceabc55e3 100644 --- a/crates/openhuman-embed/src/runtime/mod.rs +++ b/crates/openhuman-embed/src/runtime/mod.rs @@ -190,6 +190,8 @@ pub enum RuntimeError { pub(crate) struct CoreGuard { pub(crate) events: Arc, core: Option, + /// Terminal or cancelled agent cleanup must not erase unconfirmed state. + preserve_ephemeral_workspace: AtomicBool, /// Held for its `Drop`: an ephemeral workspace lives exactly as long as /// the last owner of this guard. workspace: ResolvedWorkspace, @@ -208,6 +210,13 @@ pub(crate) struct CoreGuard { seams: Mutex>, } +impl CoreGuard { + pub(crate) fn preserve_ephemeral_workspace(&self) { + self.preserve_ephemeral_workspace + .store(true, std::sync::atomic::Ordering::Release); + } +} + impl Drop for CoreGuard { fn drop(&mut self) { // Stop handing this runtime's agents and handlers to the core's @@ -241,21 +250,31 @@ impl Drop for CoreGuard { // writes settle before giving up. if let Some(temp) = self.workspace._temp.take() { let root = temp.keep(); - let mut quiet_passes = 0; - for _ in 0..20 { - match std::fs::remove_dir_all(&root) { - Ok(()) => quiet_passes += 1, - Err(error) if error.kind() == std::io::ErrorKind::NotFound => { - quiet_passes += 1; + if self + .preserve_ephemeral_workspace + .load(std::sync::atomic::Ordering::Acquire) + { + log::warn!( + "[embed][runtime] unresolved cleanup; preserved ephemeral workspace at {}", + root.display() + ); + } else { + let mut quiet_passes = 0; + for _ in 0..20 { + match std::fs::remove_dir_all(&root) { + Ok(()) => quiet_passes += 1, + Err(error) if error.kind() == std::io::ErrorKind::NotFound => { + quiet_passes += 1; + } + Err(_) => quiet_passes = 0, } - Err(_) => quiet_passes = 0, - } - if quiet_passes >= 5 { - break; + if quiet_passes >= 5 { + break; + } + std::thread::sleep(std::time::Duration::from_millis(50)); } - std::thread::sleep(std::time::Duration::from_millis(50)); + let _ = std::fs::remove_dir_all(&root); } - let _ = std::fs::remove_dir_all(&root); } RUNTIME_LIVE.store(false, std::sync::atomic::Ordering::Release); log::debug!("[embed][runtime] released"); @@ -606,6 +625,7 @@ impl Runtime { guard: Arc::new(CoreGuard { events: crate::events::EventHub::new(256), core: Some(core), + preserve_ephemeral_workspace: AtomicBool::new(false), workspace, session_store, previous_session_store, From 233ca437d057fbb78fada0b8cfc63ebe49fca60a Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Mon, 12 Oct 2026 18:32:28 +0300 Subject: [PATCH 31/45] test(agent): cover purge fence surviving runtime replacement Add a test asserting that a failed purge keeps its workspace directory reserved after the original runtime is dropped, so a successor runtime cannot claim it. Auto-committed-on: dragonfly Co-authored-by: Medulla --- .../src/agent/mcp_cleanup_tests.rs | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/crates/openhuman-embed/src/agent/mcp_cleanup_tests.rs b/crates/openhuman-embed/src/agent/mcp_cleanup_tests.rs index b9a8fe92b80..20cc5d25467 100644 --- a/crates/openhuman-embed/src/agent/mcp_cleanup_tests.rs +++ b/crates/openhuman-embed/src/agent/mcp_cleanup_tests.rs @@ -223,3 +223,65 @@ fn confirmed_cleanup_still_removes_the_last_owners_ephemeral_workspace() { fn cancelling_an_unpolled_cleanup_preserves_its_ephemeral_workspace() { workspace_cleanup_control(true); } + +#[test] +fn a_failed_purge_fences_the_same_directory_after_its_original_runtime_is_gone() { + let executor = tokio::runtime::Builder::new_multi_thread() + .enable_all() + .worker_threads(1) + .thread_stack_size(openhuman_core::core::runtime::AGENT_WORKER_STACK_BYTES) + .build() + .unwrap(); + executor.block_on(async { + tokio::spawn(async { + let scratch = tempfile::tempdir().unwrap(); + let make_runtime = || { + crate::Runtime::builder() + .config(openhuman_core::config::Config::default()) + .workspace(crate::Workspace::Dir(scratch.path().into())) + .access(crate::Access::full()) + }; + let spec = || { + crate::AgentSpec::new("worker") + .provider( + crate::Provider::openai_compatible("http://127.0.0.1:1", "fixture") + .model("fixture"), + ) + .access(crate::Access::full()) + }; + let runtime = make_runtime().build().await.unwrap(); + let agent = runtime.agent(spec()).unwrap(); + let home = scratch.path().join("agents/worker"); + std::fs::remove_dir_all(&home).unwrap(); + std::fs::write(&home, "force purge failure").unwrap(); + // No MCP entry means close is already confirmed. This pins the + // independent host purge fence; bus receipts have their own tests. + assert!(matches!( + runtime.remove_agent("worker").purge().await, + Err(AgentError::Workspace { .. }) + )); + drop(agent); + drop(runtime); + assert!(!crate::runtime::RUNTIME_LIVE.load(Ordering::Acquire)); + // Repair the filesystem fault without completing the old purge. + std::fs::remove_file(&home).unwrap(); + std::fs::create_dir_all(&home).unwrap(); + std::fs::write(home.join("old-state"), "must remain reserved").unwrap(); + let successor_runtime = make_runtime().build().await.unwrap(); + let successor = successor_runtime.agent(spec()); + let rejected = matches!(successor, Err(AgentError::CleanupPending)); + drop(successor); + drop(successor_runtime); + assert!( + rejected, + "failed purge must reserve this directory across runtime replacement" + ); + assert_eq!( + std::fs::read_to_string(home.join("old-state")).unwrap(), + "must remain reserved" + ); + }) + .await + .unwrap(); + }); +} From 563ded1d200c6153593e4e42f872b3a30b3ac4b0 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Mon, 12 Oct 2026 18:33:53 +0300 Subject: [PATCH 32/45] feat(mcp): reserve agent directories until host cleanup completes Agent directory reservations now stay fenced while host-owned cleanup is pending, so cached clients and reservations cannot be reused before the host purge finishes. A new token releases the reservation only after module close and host purge both succeed, and completion is wired into the embed agent removal path. Auto-committed-on: dragonfly Co-authored-by: Medulla --- .../openhuman-core/src/mcp/agent_cleanup.rs | 60 +++++++++++++++++++ .../openhuman-core/src/mcp/cached_cleanup.rs | 5 +- crates/openhuman-core/src/mcp/host.rs | 3 +- .../openhuman-core/src/mcp/module_client.rs | 1 + .../openhuman-embed/src/agent/mcp_cleanup.rs | 5 ++ .../src/agent/mcp_cleanup_tests.rs | 4 ++ .../src/runtime/host_agents.rs | 1 + 7 files changed, 77 insertions(+), 2 deletions(-) diff --git a/crates/openhuman-core/src/mcp/agent_cleanup.rs b/crates/openhuman-core/src/mcp/agent_cleanup.rs index d8ba90007ed..782a48f0992 100644 --- a/crates/openhuman-core/src/mcp/agent_cleanup.rs +++ b/crates/openhuman-core/src/mcp/agent_cleanup.rs @@ -85,3 +85,63 @@ where .map_err(|_| ModuleCallError::Unavailable) } } + +/// A process-local directory reservation with no runtime guard or callback. +/// Dropping the token retains the fence; only confirmed host completion clears it. +#[derive(Clone)] +pub struct AgentDirectoryCleanup { + directory: PathBuf, + reservation: Arc, +} + +/// Reserve an agent's directory until host-owned cleanup succeeds. +/// This never creates a client or loads a module. +pub fn reserve_agent_directory( + workspace: &std::path::Path, + agent_id: &str, +) -> AgentDirectoryCleanup { + let directory = workspace.join("agents").join(agent_id); + let mut reservations = CLEANUP_RESERVATIONS + .get_or_init(|| Mutex::new(HashMap::new())) + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let reservation = Arc::clone( + reservations + .entry(directory.clone()) + .or_insert_with(|| Arc::new(CleanupReservation::default())), + ); + reservation + .host_completion_pending + .store(true, Ordering::Release); + AgentDirectoryCleanup { + directory, + reservation, + } +} + +impl AgentDirectoryCleanup { + /// Release this exact reservation after module close and host purge succeed. + /// An independently registered canonical callback keeps its own reservation. + pub fn complete(&self) { + let after_close = self + .reservation + .after_close + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let mut reservations = CLEANUP_RESERVATIONS + .get_or_init(|| Mutex::new(HashMap::new())) + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + if reservations + .get(&self.directory) + .is_some_and(|current| Arc::ptr_eq(current, &self.reservation)) + { + self.reservation + .host_completion_pending + .store(false, Ordering::Release); + if after_close.is_none() { + reservations.remove(&self.directory); + } + } + } +} diff --git a/crates/openhuman-core/src/mcp/cached_cleanup.rs b/crates/openhuman-core/src/mcp/cached_cleanup.rs index b46984b7645..b746e902480 100644 --- a/crates/openhuman-core/src/mcp/cached_cleanup.rs +++ b/crates/openhuman-core/src/mcp/cached_cleanup.rs @@ -213,7 +213,7 @@ impl McpConfiguredClient { .unwrap_or_else(std::sync::PoisonError::into_inner) .get(data_dir) .is_some_and(|stored| Arc::ptr_eq(stored, &reservation)); - if !is_current { + if !is_current || reservation.host_completion_pending.load(Ordering::Acquire) { return false; } let mut clients = if let Some(client) = client.as_ref() { @@ -242,6 +242,9 @@ impl McpConfiguredClient { let mut reservations = reservations .lock() .unwrap_or_else(std::sync::PoisonError::into_inner); + if reservation.host_completion_pending.load(Ordering::Acquire) { + return false; + } if reservations .get(data_dir) .is_some_and(|stored| Arc::ptr_eq(stored, &reservation)) diff --git a/crates/openhuman-core/src/mcp/host.rs b/crates/openhuman-core/src/mcp/host.rs index 52d3e482d25..1192e716a48 100644 --- a/crates/openhuman-core/src/mcp/host.rs +++ b/crates/openhuman-core/src/mcp/host.rs @@ -10,7 +10,8 @@ use std::sync::OnceLock; use crate::config::Config; pub use super::module_client::agent_cleanup::{ - spawn_agent_cleanup, take_agent_cleanup, AgentMcpCleanup, + reserve_agent_directory, spawn_agent_cleanup, take_agent_cleanup, AgentDirectoryCleanup, + AgentMcpCleanup, }; const PROXY_SERVICE_KEY: &str = "tool.mcp_client"; diff --git a/crates/openhuman-core/src/mcp/module_client.rs b/crates/openhuman-core/src/mcp/module_client.rs index e739b0329e4..1bc36f1eb1a 100644 --- a/crates/openhuman-core/src/mcp/module_client.rs +++ b/crates/openhuman-core/src/mcp/module_client.rs @@ -40,6 +40,7 @@ type CleanupAction = Arc bool + Send + Sync>; #[derive(Default)] struct CleanupReservation { after_close: Mutex>, + host_completion_pending: AtomicBool, } static CLEANUP_RESERVATIONS: OnceLock>>> = diff --git a/crates/openhuman-embed/src/agent/mcp_cleanup.rs b/crates/openhuman-embed/src/agent/mcp_cleanup.rs index f7288703a20..2e09a2996d8 100644 --- a/crates/openhuman-embed/src/agent/mcp_cleanup.rs +++ b/crates/openhuman-embed/src/agent/mcp_cleanup.rs @@ -25,6 +25,7 @@ pub(crate) fn finish_removal( source, })?; } + pending.directory.complete(); agents.remove(id); agents.pending.remove(id); Ok(()) @@ -169,6 +170,10 @@ impl AgentInner { .entry(self.id.clone()) .or_insert_with(|| PendingCleanup { cleanup, + directory: openhuman_core::mcp::host::reserve_agent_directory( + &self.config.workspace_dir, + &self.id, + ), instance, home: self.layout.home.clone(), purge, diff --git a/crates/openhuman-embed/src/agent/mcp_cleanup_tests.rs b/crates/openhuman-embed/src/agent/mcp_cleanup_tests.rs index 20cc5d25467..3c22a19f905 100644 --- a/crates/openhuman-embed/src/agent/mcp_cleanup_tests.rs +++ b/crates/openhuman-embed/src/agent/mcp_cleanup_tests.rs @@ -3,6 +3,10 @@ use super::*; fn pending(home: std::path::PathBuf) -> PendingCleanup { PendingCleanup { cleanup: None, + directory: openhuman_core::mcp::host::reserve_agent_directory( + home.parent().unwrap(), + home.file_name().unwrap().to_str().unwrap(), + ), instance: Weak::new(), home, purge: true, diff --git a/crates/openhuman-embed/src/runtime/host_agents.rs b/crates/openhuman-embed/src/runtime/host_agents.rs index 4938340dbc7..0097c398a83 100644 --- a/crates/openhuman-embed/src/runtime/host_agents.rs +++ b/crates/openhuman-embed/src/runtime/host_agents.rs @@ -22,6 +22,7 @@ pub(crate) type AgentMap = Mutex; #[derive(Clone)] pub(crate) struct PendingCleanup { pub(crate) cleanup: Option, + pub(crate) directory: openhuman_core::mcp::host::AgentDirectoryCleanup, pub(crate) instance: Weak, pub(crate) home: std::path::PathBuf, pub(crate) purge: bool, From bcc25f3f8bf60cf2ac6b784389218e556a87f5c5 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Mon, 12 Oct 2026 18:36:09 +0300 Subject: [PATCH 33/45] test(mcp): cover orphaned cleanup retries and ephemeral preservation Add tests for retrying orphaned MCP cleanups on the original runtime and for keeping normal ephemeral cleanup when an agent has no MCP entry. Document that failed cleanup preserves the directory fence and ephemeral workspace, and that recovery requires explicit host cleanup. Auto-committed-on: dragonfly Co-authored-by: Medulla --- .../src/mcp/module_client_owner_tests.rs | 18 ++++ .../src/agent/mcp_cleanup_tests.rs | 99 +++++++++++++++++++ .../src/runtime/host_agents.rs | 4 +- .../openhuman-embed/src/runtime/lifecycle.rs | 11 +++ crates/openhuman-embed/src/runtime/mod.rs | 5 +- 5 files changed, 135 insertions(+), 2 deletions(-) diff --git a/crates/openhuman-core/src/mcp/module_client_owner_tests.rs b/crates/openhuman-core/src/mcp/module_client_owner_tests.rs index d6745a948c5..4328af03c88 100644 --- a/crates/openhuman-core/src/mcp/module_client_owner_tests.rs +++ b/crates/openhuman-core/src/mcp/module_client_owner_tests.rs @@ -408,3 +408,21 @@ async fn a_blocked_close_has_an_overall_cleanup_deadline_and_keeps_the_owner_for .unwrap() .contains_key(&directory)); } + +#[test] +fn orphaned_shutdown_cleanup_keeps_a_host_directory_reservation_until_confirmed() { + let scratch = tempfile::tempdir().unwrap(); + let directory = scratch.path().join("agents/purge-pending"); + let token = agent_cleanup::reserve_agent_directory(scratch.path(), "purge-pending"); + McpConfiguredClient::retry_orphaned_cleanups(); + assert!(McpConfiguredClient::agent_close_pending(&directory)); + let reservation = McpConfiguredClient::reservation_for(&directory); + assert!(!McpConfiguredClient::finish_cleanup( + &directory, + None, + reservation + )); + assert!(McpConfiguredClient::agent_close_pending(&directory)); + token.complete(); + assert!(!McpConfiguredClient::agent_close_pending(&directory)); +} diff --git a/crates/openhuman-embed/src/agent/mcp_cleanup_tests.rs b/crates/openhuman-embed/src/agent/mcp_cleanup_tests.rs index 3c22a19f905..78fdf0daa5a 100644 --- a/crates/openhuman-embed/src/agent/mcp_cleanup_tests.rs +++ b/crates/openhuman-embed/src/agent/mcp_cleanup_tests.rs @@ -289,3 +289,102 @@ fn a_failed_purge_fences_the_same_directory_after_its_original_runtime_is_gone() .unwrap(); }); } + +#[test] +fn repairing_and_retrying_purge_on_the_original_runtime_releases_the_directory() { + let executor = tokio::runtime::Builder::new_multi_thread() + .enable_all() + .worker_threads(1) + .thread_stack_size(openhuman_core::core::runtime::AGENT_WORKER_STACK_BYTES) + .build() + .unwrap(); + executor.block_on(async { + tokio::spawn(async { + let scratch = tempfile::tempdir().unwrap(); + let runtime = crate::Runtime::builder() + .config(openhuman_core::config::Config::default()) + .workspace(crate::Workspace::Dir(scratch.path().into())) + .access(crate::Access::full()) + .build() + .await + .unwrap(); + let spec = || { + crate::AgentSpec::new("worker") + .provider( + crate::Provider::openai_compatible("http://127.0.0.1:1", "fixture") + .model("fixture"), + ) + .access(crate::Access::full()) + }; + let agent = runtime.agent(spec()).unwrap(); + let home = scratch.path().join("agents/worker"); + std::fs::remove_dir_all(&home).unwrap(); + std::fs::write(&home, "force purge failure").unwrap(); + assert!(matches!( + runtime.remove_agent("worker").purge().await, + Err(AgentError::Workspace { .. }) + )); + drop(agent); + assert!(openhuman_core::mcp::host::agent_close_pending( + scratch.path(), + "worker" + )); + std::fs::remove_file(&home).unwrap(); + std::fs::create_dir_all(&home).unwrap(); + std::fs::write(home.join("old-state"), "to purge").unwrap(); + runtime.remove_agent("worker").purge().await.unwrap(); + assert!(!home.exists()); + assert!(!openhuman_core::mcp::host::agent_close_pending( + scratch.path(), + "worker" + )); + let successor = runtime + .agent(spec()) + .expect("confirmed purge releases the public id"); + drop(successor); + drop(runtime); + }) + .await + .unwrap(); + }); +} + +#[test] +fn dropping_an_agent_without_an_mcp_entry_keeps_normal_ephemeral_cleanup() { + let executor = tokio::runtime::Builder::new_multi_thread() + .enable_all() + .worker_threads(1) + .thread_stack_size(openhuman_core::core::runtime::AGENT_WORKER_STACK_BYTES) + .build() + .unwrap(); + executor.block_on(async { + tokio::spawn(async { + let runtime = crate::Runtime::builder() + .config(openhuman_core::config::Config::default()) + .workspace(crate::Workspace::Ephemeral) + .access(crate::Access::full()) + .build() + .await + .unwrap(); + let root = runtime.workspace_dir().to_path_buf(); + let agent = runtime + .agent( + crate::AgentSpec::new("unused") + .provider( + crate::Provider::openai_compatible("http://127.0.0.1:1", "fixture") + .model("fixture"), + ) + .access(crate::Access::full()), + ) + .unwrap(); + drop(agent); + drop(runtime); + assert!(!root.exists()); + assert!(!openhuman_core::mcp::host::agent_close_pending( + &root, "unused" + )); + }) + .await + .unwrap(); + }); +} diff --git a/crates/openhuman-embed/src/runtime/host_agents.rs b/crates/openhuman-embed/src/runtime/host_agents.rs index 0097c398a83..7f3a4af359d 100644 --- a/crates/openhuman-embed/src/runtime/host_agents.rs +++ b/crates/openhuman-embed/src/runtime/host_agents.rs @@ -18,7 +18,9 @@ use crate::agent::AgentInner; pub(crate) type AgentMap = Mutex; /// Failed cleanup retains its captured owner and purge intent, without keeping -/// an agent or the runtime guard alive. `remove_agent` can retry this record. +/// an agent or the runtime guard alive. The original live runtime's +/// `remove_agent` can retry this record. Its process-local directory fence +/// survives a lost record, without automatic durable retry or reclamation. #[derive(Clone)] pub(crate) struct PendingCleanup { pub(crate) cleanup: Option, diff --git a/crates/openhuman-embed/src/runtime/lifecycle.rs b/crates/openhuman-embed/src/runtime/lifecycle.rs index 9c0f8e95f29..7452a4c9b04 100644 --- a/crates/openhuman-embed/src/runtime/lifecycle.rs +++ b/crates/openhuman-embed/src/runtime/lifecycle.rs @@ -93,6 +93,17 @@ impl Runtime { /// including after the last agent handle was dropped. Runtime drop does /// not execute the process signal's shutdown hooks. /// + /// Each MCP attempt has a 30-second deadline; three detached attempts add + /// at most 750 ms of backoff. Filesystem purge and executor scheduling are + /// outside that wait bound. Failed or cancelled cleanup preserves an + /// ephemeral workspace when the runtime drops and logs its retained path. + /// That preservation is conservative for the rest of this runtime. + /// + /// The directory fence survives in this process if the original runtime's + /// retry record is lost. A new runtime cannot recover that record through + /// this method; there is no automatic durable retry or file reclamation. + /// Retained state then needs explicit host cleanup. + /// /// Cancellation is cooperative: a tool already executing when the agent /// is removed, or a sub-agent it detached, may finish after this returns. pub fn remove_agent(&self, id: &str) -> RemoveAgent<'_> { diff --git a/crates/openhuman-embed/src/runtime/mod.rs b/crates/openhuman-embed/src/runtime/mod.rs index 82ceabc55e3..65e56691ee5 100644 --- a/crates/openhuman-embed/src/runtime/mod.rs +++ b/crates/openhuman-embed/src/runtime/mod.rs @@ -179,7 +179,7 @@ pub enum RuntimeError { /// [`crate::Workspace::Ephemeral`], the workspace it lives in. /// /// Held as `Arc` by both `Runtime` and `AgentInner` so its `Drop` -/// — releasing [`RUNTIME_LIVE`] and removing an ephemeral workspace — runs +/// — releasing [`RUNTIME_LIVE`] and normally removing an ephemeral workspace — runs /// only once the *last* of them goes away. `Agent`s are owned rather than /// borrowed from `Runtime`, so a caller can drop the `Runtime` handle while /// an `Agent` (or a `Turn` in flight) is still alive; if `Runtime` tore this @@ -211,6 +211,9 @@ pub(crate) struct CoreGuard { } impl CoreGuard { + /// Keep the scratch path after an unconfirmed cleanup, while still + /// releasing the process slot. Sticky to avoid clearing another failed + /// agent's preservation request when an independent retry succeeds. pub(crate) fn preserve_ephemeral_workspace(&self) { self.preserve_ephemeral_workspace .store(true, std::sync::atomic::Ordering::Release); From 372eee9ee5aa3421db86ceddd52ddcf1adfc96ff Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Mon, 12 Oct 2026 18:47:10 +0300 Subject: [PATCH 34/45] chore: files changed crates/openhuman-core/src/mcp/module_client_tests.rs,crates/openhuman-core/src/ Auto-committed-on: dragonfly Co-authored-by: Medulla --- .../src/mcp/module_client_cleanup_tests.rs | 418 +++++++++++++++++ .../src/mcp/module_client_tests.rs | 421 +----------------- 2 files changed, 422 insertions(+), 417 deletions(-) create mode 100644 crates/openhuman-core/src/mcp/module_client_cleanup_tests.rs diff --git a/crates/openhuman-core/src/mcp/module_client_cleanup_tests.rs b/crates/openhuman-core/src/mcp/module_client_cleanup_tests.rs new file mode 100644 index 00000000000..120643bfa6f --- /dev/null +++ b/crates/openhuman-core/src/mcp/module_client_cleanup_tests.rs @@ -0,0 +1,418 @@ +use super::*; + +#[tokio::test] +async fn agent_cleanup_closes_only_the_cached_agent_and_keeps_siblings_open() { + let target_workspace = tempfile::tempdir().expect("target workspace"); + let sibling_workspace = tempfile::tempdir().expect("sibling workspace"); + let mut target_config = Config::default(); + target_config.workspace_dir = target_workspace.path().to_path_buf(); + target_config.modules.enabled = true; + let mut sibling_config = Config::default(); + sibling_config.workspace_dir = sibling_workspace.path().to_path_buf(); + sibling_config.modules.enabled = true; + + let target_port = FakePort::default(); + let target_closed = Arc::clone(&target_port.closed); + let target = McpConfiguredClient::install_test_port(&target_config, Box::new(target_port)) + .expect("install target client"); + let sibling_port = FakePort::default(); + let sibling_closed = Arc::clone(&sibling_port.closed); + let sibling = McpConfiguredClient::install_test_port(&sibling_config, Box::new(sibling_port)) + .expect("install sibling client"); + + target + .call::(&configured("target"), "ConnectedOverview", ()) + .await + .unwrap(); + sibling + .call::(&configured("sibling"), "ConnectedOverview", ()) + .await + .unwrap(); + + assert!(McpConfiguredClient::close_agent_data_dir(target_workspace.path()).await); + assert_eq!( + target_closed.lock().unwrap().as_slice(), + [target_workspace.path().to_str().unwrap()] + ); + assert!(sibling_closed.lock().unwrap().is_empty()); + assert!(McpConfiguredClient::for_config(&sibling_config).is_ok()); + assert_eq!( + target + .call::(&configured("target"), "ConnectedOverview", ()) + .await, + Err(ModuleCallError::Unavailable), + "an old client cannot reopen after acknowledged retirement" + ); +} + +#[tokio::test] +async fn failed_agent_close_stays_retired_and_can_be_retried() { + let workspace = tempfile::tempdir().expect("workspace"); + let mut config = Config::default(); + config.workspace_dir = workspace.path().to_path_buf(); + config.modules.enabled = true; + + let port = FakePort::default(); + port.fail_first_close + .store(true, std::sync::atomic::Ordering::SeqCst); + let closed = Arc::clone(&port.closed); + let client = + McpConfiguredClient::install_test_port(&config, Box::new(port)).expect("install client"); + client + .call::(&configured("server"), "ConnectedOverview", ()) + .await + .unwrap(); + + assert!(!McpConfiguredClient::close_agent_data_dir(workspace.path()).await); + assert!(McpConfiguredClient::agent_close_pending(workspace.path())); + assert!(matches!( + McpConfiguredClient::for_config(&config), + Err(ModuleCallError::Unavailable) + )); + assert_eq!(closed.lock().unwrap().len(), 1); + + assert!(McpConfiguredClient::close_agent_data_dir(workspace.path()).await); + assert!(!McpConfiguredClient::agent_close_pending(workspace.path())); + assert_eq!(closed.lock().unwrap().len(), 2); + assert!(McpConfiguredClient::for_config(&config).is_ok()); +} + +#[tokio::test] +async fn agent_id_stays_unavailable_until_its_cached_module_close_is_acknowledged() { + let workspace = tempfile::tempdir().expect("workspace"); + let alpha_dir = workspace.path().join("agents").join("alpha"); + let beta_dir = workspace.path().join("agents").join("beta"); + let mut alpha_config = Config::default(); + alpha_config.workspace_dir = alpha_dir; + alpha_config.modules.enabled = true; + let mut beta_config = Config::default(); + beta_config.workspace_dir = beta_dir; + beta_config.modules.enabled = true; + + let alpha_port = FakePort::default(); + alpha_port + .block_first_close + .store(true, std::sync::atomic::Ordering::SeqCst); + let alpha = McpConfiguredClient::install_test_port(&alpha_config, Box::new(alpha_port.clone())) + .expect("install alpha client"); + alpha + .call::(&configured("alpha"), "ConnectedOverview", ()) + .await + .unwrap(); + let beta = McpConfiguredClient::install_test_port(&beta_config, Box::new(FakePort::default())) + .expect("install beta client"); + beta.call::(&configured("beta"), "ConnectedOverview", ()) + .await + .unwrap(); + + let workspace_path = workspace.path().to_path_buf(); + let closing = + tokio::spawn(async move { crate::mcp::host::close_agent(&workspace_path, "alpha").await }); + alpha_port.close_started.notified().await; + assert!(crate::mcp::host::agent_close_pending( + workspace.path(), + "alpha" + )); + assert!(!crate::mcp::host::agent_close_pending( + workspace.path(), + "beta" + )); + assert!(matches!( + McpConfiguredClient::for_config(&alpha_config), + Err(ModuleCallError::Unavailable) + )); + assert!(McpConfiguredClient::for_config(&beta_config).is_ok()); + + alpha_port.release_close.notify_one(); + assert!(closing.await.unwrap()); + assert!(!crate::mcp::host::agent_close_pending( + workspace.path(), + "alpha" + )); + assert!(McpConfiguredClient::for_config(&alpha_config).is_ok()); +} + +#[tokio::test] +async fn deferred_agent_purge_waits_for_module_close_and_keeps_the_id_reserved() { + let workspace = tempfile::tempdir().expect("workspace"); + let agent_dir = workspace.path().join("agents").join("alpha"); + std::fs::create_dir_all(&agent_dir).expect("agent home"); + std::fs::write(agent_dir.join("transcript.jsonl"), "kept until close").unwrap(); + let mut config = Config::default(); + config.workspace_dir = agent_dir.clone(); + config.modules.enabled = true; + + let port = FakePort::default(); + port.block_first_close + .store(true, std::sync::atomic::Ordering::SeqCst); + let close_started = Arc::clone(&port.close_started); + let release_close = Arc::clone(&port.release_close); + let client = McpConfiguredClient::install_test_port(&config, Box::new(port)) + .expect("install module client"); + client + .call::(&configured("agent"), "ConnectedOverview", ()) + .await + .unwrap(); + + McpConfiguredClient::schedule_agent_close_and_cleanup( + &agent_dir, + Arc::new({ + let agent_dir = agent_dir.clone(); + move || std::fs::remove_dir_all(&agent_dir).is_ok() || !agent_dir.exists() + }), + ); + close_started.notified().await; + + assert!(agent_dir.join("transcript.jsonl").exists()); + assert!(McpConfiguredClient::agent_close_pending(&agent_dir)); + assert!(matches!( + McpConfiguredClient::for_config(&config), + Err(ModuleCallError::Unavailable) + )); + + release_close.notify_one(); + tokio::time::timeout(Duration::from_secs(1), async { + while McpConfiguredClient::agent_close_pending(&agent_dir) { + tokio::task::yield_now().await; + } + }) + .await + .expect("close and deferred cleanup finish"); + assert!(!agent_dir.exists()); + assert!(McpConfiguredClient::for_config(&config).is_ok()); +} + +#[test] +fn closed_client_cleanup_finishes_without_a_tokio_runtime() { + let workspace = tempfile::tempdir().expect("workspace"); + let agent_dir = workspace.path().join("agents").join("closed"); + std::fs::create_dir_all(&agent_dir).unwrap(); + let mut config = Config::default(); + config.workspace_dir = agent_dir.clone(); + config.modules.enabled = true; + McpConfiguredClient::install_test_port(&config, Box::new(FakePort::default())) + .expect("install closed client"); + + McpConfiguredClient::schedule_agent_close_and_cleanup( + &agent_dir, + Arc::new({ + let agent_dir = agent_dir.clone(); + move || std::fs::remove_dir_all(&agent_dir).is_ok() || !agent_dir.exists() + }), + ); + + assert!(!agent_dir.exists()); + assert!(!McpConfiguredClient::agent_close_pending(&agent_dir)); +} + +#[tokio::test] +async fn a_removed_agent_context_cannot_reopen_its_evicted_client() { + use crate::core::runtime::{ContextOverlay, CoreContext, DomainSet}; + use crate::tools::toolpacks::ToolGroups; + + let workspace = tempfile::tempdir().expect("workspace"); + let mut config = Config::default(); + config.workspace_dir = workspace.path().to_path_buf(); + config.modules.enabled = true; + let scope = Arc::new(crate::security::approval::ApprovalScope::default()); + let mut overrides = crate::agent::host_overrides::HostOverrides::default(); + overrides.approval_scope = Some(Arc::clone(&scope)); + let overrides = Arc::new(overrides); + let context = CoreContext::for_test(DomainSet::full(), Some(workspace.path().to_path_buf())) + .derive_with( + ContextOverlay::new(config.clone(), DomainSet::kernel(), ToolGroups::none()) + .session_agent("agent") + .host_overrides(overrides), + ); + scope.close("agent_removed"); + + CoreContext::scope(context, async { + assert!(matches!( + McpConfiguredClient::for_config(&config), + Err(ModuleCallError::Unavailable) + )); + }) + .await; +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn a_removed_context_waiting_for_the_client_cache_cannot_reopen_it() { + use crate::core::runtime::{ContextOverlay, CoreContext, DomainSet}; + use crate::tools::toolpacks::ToolGroups; + + let workspace = tempfile::tempdir().expect("workspace"); + let scoped_dir = workspace.path().join("agents").join("agent"); + let mut config = Config::default(); + config.workspace_dir = workspace.path().to_path_buf(); + config.modules.enabled = true; + let scope = Arc::new(crate::security::approval::ApprovalScope::default()); + let mut overrides = crate::agent::host_overrides::HostOverrides::default(); + overrides.approval_scope = Some(Arc::clone(&scope)); + let context = CoreContext::for_test(DomainSet::full(), Some(workspace.path().to_path_buf())) + .derive_with( + ContextOverlay::new(config.clone(), DomainSet::kernel(), ToolGroups::none()) + .session_agent("agent") + .host_overrides(Arc::new(overrides)), + ); + + let clients = CLIENTS.get_or_init(|| Mutex::new(HashMap::new())); + let clients_lock = clients.lock().unwrap(); + let (checked, checked_rx) = tokio::sync::oneshot::channel(); + *AFTER_CONTEXT_CHECK + .get_or_init(|| Mutex::new(None)) + .lock() + .unwrap() = Some((scoped_dir.clone(), checked)); + let request = tokio::spawn(CoreContext::scope(context, async move { + McpConfiguredClient::for_config(&config) + })); + + tokio::time::timeout(Duration::from_secs(10), checked_rx) + .await + .expect("request passed the initial context check") + .unwrap(); + scope.close("agent_removed"); + drop(clients_lock); + + let admitted = request.await.unwrap().is_ok(); + if admitted { + clients.lock().unwrap().remove(&scoped_dir); + } + assert!(!admitted, "removed context must not create a cached client"); + assert!(!clients.lock().unwrap().contains_key(&scoped_dir)); +} + +#[tokio::test] +async fn delayed_agent_close_cannot_evict_a_replacement_cache_entry() { + let workspace = tempfile::tempdir().expect("workspace"); + let mut config = Config::default(); + config.workspace_dir = workspace.path().to_path_buf(); + config.modules.enabled = true; + + let old_port = FakePort::default(); + let old_closed = Arc::clone(&old_port.closed); + let old = McpConfiguredClient::install_test_port(&config, Box::new(old_port)) + .expect("install old client"); + old.call::(&configured("server"), "ConnectedOverview", ()) + .await + .unwrap(); + let cleanup_ran = Arc::new(std::sync::atomic::AtomicBool::new(false)); + let callback_flag = Arc::clone(&cleanup_ran); + let reservation = McpConfiguredClient::reservation_with_action( + workspace.path(), + Arc::new(move || { + callback_flag.store(true, std::sync::atomic::Ordering::SeqCst); + true + }), + ); + let captured = McpConfiguredClient::retire_cached_client(workspace.path()) + .expect("retirement captures the exact old client"); + + let replacement_port = FakePort::default(); + let replacement_closed = Arc::clone(&replacement_port.closed); + let replacement = Arc::new(McpConfiguredClient::with_port( + workspace.path().to_string_lossy().into_owned(), + Box::new(replacement_port), + )); + CLIENTS + .get() + .expect("old client initialized the cache") + .lock() + .unwrap() + .insert(workspace.path().to_path_buf(), Arc::clone(&replacement)); + + assert!( + !McpConfiguredClient::close_retired_client( + workspace.path().to_path_buf(), + captured, + Arc::clone(&reservation), + ) + .await, + "a stale close completion cannot claim the replacement entry" + ); + assert!(!cleanup_ran.load(std::sync::atomic::Ordering::SeqCst)); + assert_eq!(old_closed.lock().unwrap().len(), 1); + assert!(replacement_closed.lock().unwrap().is_empty()); + let current = CLIENTS + .get() + .unwrap() + .lock() + .unwrap() + .get(workspace.path()) + .cloned() + .expect("replacement remains cached"); + assert!(Arc::ptr_eq(¤t, &replacement)); + CLEANUP_RESERVATIONS + .get() + .unwrap() + .lock() + .unwrap() + .remove(workspace.path()); +} + +#[tokio::test] +async fn failed_cleanup_actions_retain_and_retry_cached_and_orphaned_paths() { + use std::sync::atomic::{AtomicUsize, Ordering}; + + let workspace = tempfile::tempdir().expect("workspace"); + let agent_dir = workspace.path().join("agents").join("alpha"); + std::fs::create_dir_all(&agent_dir).unwrap(); + std::fs::write(agent_dir.join("state.json"), "retained until cleanup").unwrap(); + let mut config = Config::default(); + config.workspace_dir = agent_dir.clone(); + config.modules.enabled = true; + let client = McpConfiguredClient::install_test_port(&config, Box::new(FakePort::default())) + .expect("install client"); + client + .call::(&configured("alpha"), "ConnectedOverview", ()) + .await + .unwrap(); + + let attempts = Arc::new(AtomicUsize::new(0)); + let callback_attempts = Arc::clone(&attempts); + let home = agent_dir.clone(); + let action: CleanupAction = Arc::new(move || { + if callback_attempts.fetch_add(1, Ordering::SeqCst) == 0 { + return false; + } + std::fs::remove_dir_all(&home).is_ok() || !home.exists() + }); + let reservation = McpConfiguredClient::reservation_with_action(&agent_dir, action); + let captured = McpConfiguredClient::retire_cached_client(&agent_dir).unwrap(); + assert!( + !McpConfiguredClient::close_retired_client(agent_dir.clone(), captured, reservation,).await + ); + assert!(agent_dir.join("state.json").exists()); + assert!(McpConfiguredClient::agent_close_pending(&agent_dir)); + assert!(matches!( + McpConfiguredClient::for_config(&config), + Err(ModuleCallError::Unavailable) + )); + assert!(McpConfiguredClient::close_agent_data_dir(&agent_dir).await); + assert!(!agent_dir.exists()); + assert!(McpConfiguredClient::for_config(&config).is_ok()); + + let orphan_dir = workspace.path().join("agents").join("orphan"); + std::fs::create_dir_all(&orphan_dir).unwrap(); + let orphan_attempts = Arc::new(AtomicUsize::new(0)); + let callback_attempts = Arc::clone(&orphan_attempts); + let orphan_home = orphan_dir.clone(); + let reservation = McpConfiguredClient::reservation_with_action( + &orphan_dir, + Arc::new(move || { + if callback_attempts.fetch_add(1, Ordering::SeqCst) == 0 { + return false; + } + std::fs::remove_dir_all(&orphan_home).is_ok() || !orphan_home.exists() + }), + ); + assert!(!McpConfiguredClient::finish_cleanup( + &orphan_dir, + None, + reservation, + )); + assert!(McpConfiguredClient::agent_close_pending(&orphan_dir)); + assert!(orphan_dir.exists()); + McpConfiguredClient::retry_orphaned_cleanups(); + assert!(!McpConfiguredClient::agent_close_pending(&orphan_dir)); + assert!(!orphan_dir.exists()); +} diff --git a/crates/openhuman-core/src/mcp/module_client_tests.rs b/crates/openhuman-core/src/mcp/module_client_tests.rs index 7c1ee48f132..086e39aebcf 100644 --- a/crates/openhuman-core/src/mcp/module_client_tests.rs +++ b/crates/openhuman-core/src/mcp/module_client_tests.rs @@ -402,423 +402,6 @@ async fn disabling_modules_rejects_a_cached_client_but_keeps_shutdown_cleanup() assert_eq!(closed.lock().unwrap().len(), 1); } -#[tokio::test] -async fn agent_cleanup_closes_only_the_cached_agent_and_keeps_siblings_open() { - let target_workspace = tempfile::tempdir().expect("target workspace"); - let sibling_workspace = tempfile::tempdir().expect("sibling workspace"); - let mut target_config = Config::default(); - target_config.workspace_dir = target_workspace.path().to_path_buf(); - target_config.modules.enabled = true; - let mut sibling_config = Config::default(); - sibling_config.workspace_dir = sibling_workspace.path().to_path_buf(); - sibling_config.modules.enabled = true; - - let target_port = FakePort::default(); - let target_closed = Arc::clone(&target_port.closed); - let target = McpConfiguredClient::install_test_port(&target_config, Box::new(target_port)) - .expect("install target client"); - let sibling_port = FakePort::default(); - let sibling_closed = Arc::clone(&sibling_port.closed); - let sibling = McpConfiguredClient::install_test_port(&sibling_config, Box::new(sibling_port)) - .expect("install sibling client"); - - target - .call::(&configured("target"), "ConnectedOverview", ()) - .await - .unwrap(); - sibling - .call::(&configured("sibling"), "ConnectedOverview", ()) - .await - .unwrap(); - - assert!(McpConfiguredClient::close_agent_data_dir(target_workspace.path()).await); - assert_eq!( - target_closed.lock().unwrap().as_slice(), - [target_workspace.path().to_str().unwrap()] - ); - assert!(sibling_closed.lock().unwrap().is_empty()); - assert!(McpConfiguredClient::for_config(&sibling_config).is_ok()); - assert_eq!( - target - .call::(&configured("target"), "ConnectedOverview", ()) - .await, - Err(ModuleCallError::Unavailable), - "an old client cannot reopen after acknowledged retirement" - ); -} - -#[tokio::test] -async fn failed_agent_close_stays_retired_and_can_be_retried() { - let workspace = tempfile::tempdir().expect("workspace"); - let mut config = Config::default(); - config.workspace_dir = workspace.path().to_path_buf(); - config.modules.enabled = true; - - let port = FakePort::default(); - port.fail_first_close - .store(true, std::sync::atomic::Ordering::SeqCst); - let closed = Arc::clone(&port.closed); - let client = - McpConfiguredClient::install_test_port(&config, Box::new(port)).expect("install client"); - client - .call::(&configured("server"), "ConnectedOverview", ()) - .await - .unwrap(); - - assert!(!McpConfiguredClient::close_agent_data_dir(workspace.path()).await); - assert!(McpConfiguredClient::agent_close_pending(workspace.path())); - assert!(matches!( - McpConfiguredClient::for_config(&config), - Err(ModuleCallError::Unavailable) - )); - assert_eq!(closed.lock().unwrap().len(), 1); - - assert!(McpConfiguredClient::close_agent_data_dir(workspace.path()).await); - assert!(!McpConfiguredClient::agent_close_pending(workspace.path())); - assert_eq!(closed.lock().unwrap().len(), 2); - assert!(McpConfiguredClient::for_config(&config).is_ok()); -} - -#[tokio::test] -async fn agent_id_stays_unavailable_until_its_cached_module_close_is_acknowledged() { - let workspace = tempfile::tempdir().expect("workspace"); - let alpha_dir = workspace.path().join("agents").join("alpha"); - let beta_dir = workspace.path().join("agents").join("beta"); - let mut alpha_config = Config::default(); - alpha_config.workspace_dir = alpha_dir; - alpha_config.modules.enabled = true; - let mut beta_config = Config::default(); - beta_config.workspace_dir = beta_dir; - beta_config.modules.enabled = true; - - let alpha_port = FakePort::default(); - alpha_port - .block_first_close - .store(true, std::sync::atomic::Ordering::SeqCst); - let alpha = McpConfiguredClient::install_test_port(&alpha_config, Box::new(alpha_port.clone())) - .expect("install alpha client"); - alpha - .call::(&configured("alpha"), "ConnectedOverview", ()) - .await - .unwrap(); - let beta = McpConfiguredClient::install_test_port(&beta_config, Box::new(FakePort::default())) - .expect("install beta client"); - beta.call::(&configured("beta"), "ConnectedOverview", ()) - .await - .unwrap(); - - let workspace_path = workspace.path().to_path_buf(); - let closing = - tokio::spawn(async move { crate::mcp::host::close_agent(&workspace_path, "alpha").await }); - alpha_port.close_started.notified().await; - assert!(crate::mcp::host::agent_close_pending( - workspace.path(), - "alpha" - )); - assert!(!crate::mcp::host::agent_close_pending( - workspace.path(), - "beta" - )); - assert!(matches!( - McpConfiguredClient::for_config(&alpha_config), - Err(ModuleCallError::Unavailable) - )); - assert!(McpConfiguredClient::for_config(&beta_config).is_ok()); - - alpha_port.release_close.notify_one(); - assert!(closing.await.unwrap()); - assert!(!crate::mcp::host::agent_close_pending( - workspace.path(), - "alpha" - )); - assert!(McpConfiguredClient::for_config(&alpha_config).is_ok()); -} - -#[tokio::test] -async fn deferred_agent_purge_waits_for_module_close_and_keeps_the_id_reserved() { - let workspace = tempfile::tempdir().expect("workspace"); - let agent_dir = workspace.path().join("agents").join("alpha"); - std::fs::create_dir_all(&agent_dir).expect("agent home"); - std::fs::write(agent_dir.join("transcript.jsonl"), "kept until close").unwrap(); - let mut config = Config::default(); - config.workspace_dir = agent_dir.clone(); - config.modules.enabled = true; - - let port = FakePort::default(); - port.block_first_close - .store(true, std::sync::atomic::Ordering::SeqCst); - let close_started = Arc::clone(&port.close_started); - let release_close = Arc::clone(&port.release_close); - let client = McpConfiguredClient::install_test_port(&config, Box::new(port)) - .expect("install module client"); - client - .call::(&configured("agent"), "ConnectedOverview", ()) - .await - .unwrap(); - - McpConfiguredClient::schedule_agent_close_and_cleanup( - &agent_dir, - Arc::new({ - let agent_dir = agent_dir.clone(); - move || std::fs::remove_dir_all(&agent_dir).is_ok() || !agent_dir.exists() - }), - ); - close_started.notified().await; - - assert!(agent_dir.join("transcript.jsonl").exists()); - assert!(McpConfiguredClient::agent_close_pending(&agent_dir)); - assert!(matches!( - McpConfiguredClient::for_config(&config), - Err(ModuleCallError::Unavailable) - )); - - release_close.notify_one(); - tokio::time::timeout(Duration::from_secs(1), async { - while McpConfiguredClient::agent_close_pending(&agent_dir) { - tokio::task::yield_now().await; - } - }) - .await - .expect("close and deferred cleanup finish"); - assert!(!agent_dir.exists()); - assert!(McpConfiguredClient::for_config(&config).is_ok()); -} - -#[test] -fn closed_client_cleanup_finishes_without_a_tokio_runtime() { - let workspace = tempfile::tempdir().expect("workspace"); - let agent_dir = workspace.path().join("agents").join("closed"); - std::fs::create_dir_all(&agent_dir).unwrap(); - let mut config = Config::default(); - config.workspace_dir = agent_dir.clone(); - config.modules.enabled = true; - McpConfiguredClient::install_test_port(&config, Box::new(FakePort::default())) - .expect("install closed client"); - - McpConfiguredClient::schedule_agent_close_and_cleanup( - &agent_dir, - Arc::new({ - let agent_dir = agent_dir.clone(); - move || std::fs::remove_dir_all(&agent_dir).is_ok() || !agent_dir.exists() - }), - ); - - assert!(!agent_dir.exists()); - assert!(!McpConfiguredClient::agent_close_pending(&agent_dir)); -} - -#[tokio::test] -async fn a_removed_agent_context_cannot_reopen_its_evicted_client() { - use crate::core::runtime::{ContextOverlay, CoreContext, DomainSet}; - use crate::tools::toolpacks::ToolGroups; - - let workspace = tempfile::tempdir().expect("workspace"); - let mut config = Config::default(); - config.workspace_dir = workspace.path().to_path_buf(); - config.modules.enabled = true; - let scope = Arc::new(crate::security::approval::ApprovalScope::default()); - let mut overrides = crate::agent::host_overrides::HostOverrides::default(); - overrides.approval_scope = Some(Arc::clone(&scope)); - let overrides = Arc::new(overrides); - let context = CoreContext::for_test(DomainSet::full(), Some(workspace.path().to_path_buf())) - .derive_with( - ContextOverlay::new(config.clone(), DomainSet::kernel(), ToolGroups::none()) - .session_agent("agent") - .host_overrides(overrides), - ); - scope.close("agent_removed"); - - CoreContext::scope(context, async { - assert!(matches!( - McpConfiguredClient::for_config(&config), - Err(ModuleCallError::Unavailable) - )); - }) - .await; -} - -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn a_removed_context_waiting_for_the_client_cache_cannot_reopen_it() { - use crate::core::runtime::{ContextOverlay, CoreContext, DomainSet}; - use crate::tools::toolpacks::ToolGroups; - - let workspace = tempfile::tempdir().expect("workspace"); - let scoped_dir = workspace.path().join("agents").join("agent"); - let mut config = Config::default(); - config.workspace_dir = workspace.path().to_path_buf(); - config.modules.enabled = true; - let scope = Arc::new(crate::security::approval::ApprovalScope::default()); - let mut overrides = crate::agent::host_overrides::HostOverrides::default(); - overrides.approval_scope = Some(Arc::clone(&scope)); - let context = CoreContext::for_test(DomainSet::full(), Some(workspace.path().to_path_buf())) - .derive_with( - ContextOverlay::new(config.clone(), DomainSet::kernel(), ToolGroups::none()) - .session_agent("agent") - .host_overrides(Arc::new(overrides)), - ); - - let clients = CLIENTS.get_or_init(|| Mutex::new(HashMap::new())); - let clients_lock = clients.lock().unwrap(); - let (checked, checked_rx) = tokio::sync::oneshot::channel(); - *AFTER_CONTEXT_CHECK - .get_or_init(|| Mutex::new(None)) - .lock() - .unwrap() = Some((scoped_dir.clone(), checked)); - let request = tokio::spawn(CoreContext::scope(context, async move { - McpConfiguredClient::for_config(&config) - })); - - tokio::time::timeout(Duration::from_secs(10), checked_rx) - .await - .expect("request passed the initial context check") - .unwrap(); - scope.close("agent_removed"); - drop(clients_lock); - - let admitted = request.await.unwrap().is_ok(); - if admitted { - clients.lock().unwrap().remove(&scoped_dir); - } - assert!(!admitted, "removed context must not create a cached client"); - assert!(!clients.lock().unwrap().contains_key(&scoped_dir)); -} - -#[tokio::test] -async fn delayed_agent_close_cannot_evict_a_replacement_cache_entry() { - let workspace = tempfile::tempdir().expect("workspace"); - let mut config = Config::default(); - config.workspace_dir = workspace.path().to_path_buf(); - config.modules.enabled = true; - - let old_port = FakePort::default(); - let old_closed = Arc::clone(&old_port.closed); - let old = McpConfiguredClient::install_test_port(&config, Box::new(old_port)) - .expect("install old client"); - old.call::(&configured("server"), "ConnectedOverview", ()) - .await - .unwrap(); - let cleanup_ran = Arc::new(std::sync::atomic::AtomicBool::new(false)); - let callback_flag = Arc::clone(&cleanup_ran); - let reservation = McpConfiguredClient::reservation_with_action( - workspace.path(), - Arc::new(move || { - callback_flag.store(true, std::sync::atomic::Ordering::SeqCst); - true - }), - ); - let captured = McpConfiguredClient::retire_cached_client(workspace.path()) - .expect("retirement captures the exact old client"); - - let replacement_port = FakePort::default(); - let replacement_closed = Arc::clone(&replacement_port.closed); - let replacement = Arc::new(McpConfiguredClient::with_port( - workspace.path().to_string_lossy().into_owned(), - Box::new(replacement_port), - )); - CLIENTS - .get() - .expect("old client initialized the cache") - .lock() - .unwrap() - .insert(workspace.path().to_path_buf(), Arc::clone(&replacement)); - - assert!( - !McpConfiguredClient::close_retired_client( - workspace.path().to_path_buf(), - captured, - Arc::clone(&reservation), - ) - .await, - "a stale close completion cannot claim the replacement entry" - ); - assert!(!cleanup_ran.load(std::sync::atomic::Ordering::SeqCst)); - assert_eq!(old_closed.lock().unwrap().len(), 1); - assert!(replacement_closed.lock().unwrap().is_empty()); - let current = CLIENTS - .get() - .unwrap() - .lock() - .unwrap() - .get(workspace.path()) - .cloned() - .expect("replacement remains cached"); - assert!(Arc::ptr_eq(¤t, &replacement)); - CLEANUP_RESERVATIONS - .get() - .unwrap() - .lock() - .unwrap() - .remove(workspace.path()); -} - -#[tokio::test] -async fn failed_cleanup_actions_retain_and_retry_cached_and_orphaned_paths() { - use std::sync::atomic::{AtomicUsize, Ordering}; - - let workspace = tempfile::tempdir().expect("workspace"); - let agent_dir = workspace.path().join("agents").join("alpha"); - std::fs::create_dir_all(&agent_dir).unwrap(); - std::fs::write(agent_dir.join("state.json"), "retained until cleanup").unwrap(); - let mut config = Config::default(); - config.workspace_dir = agent_dir.clone(); - config.modules.enabled = true; - let client = McpConfiguredClient::install_test_port(&config, Box::new(FakePort::default())) - .expect("install client"); - client - .call::(&configured("alpha"), "ConnectedOverview", ()) - .await - .unwrap(); - - let attempts = Arc::new(AtomicUsize::new(0)); - let callback_attempts = Arc::clone(&attempts); - let home = agent_dir.clone(); - let action: CleanupAction = Arc::new(move || { - if callback_attempts.fetch_add(1, Ordering::SeqCst) == 0 { - return false; - } - std::fs::remove_dir_all(&home).is_ok() || !home.exists() - }); - let reservation = McpConfiguredClient::reservation_with_action(&agent_dir, action); - let captured = McpConfiguredClient::retire_cached_client(&agent_dir).unwrap(); - assert!( - !McpConfiguredClient::close_retired_client(agent_dir.clone(), captured, reservation,).await - ); - assert!(agent_dir.join("state.json").exists()); - assert!(McpConfiguredClient::agent_close_pending(&agent_dir)); - assert!(matches!( - McpConfiguredClient::for_config(&config), - Err(ModuleCallError::Unavailable) - )); - assert!(McpConfiguredClient::close_agent_data_dir(&agent_dir).await); - assert!(!agent_dir.exists()); - assert!(McpConfiguredClient::for_config(&config).is_ok()); - - let orphan_dir = workspace.path().join("agents").join("orphan"); - std::fs::create_dir_all(&orphan_dir).unwrap(); - let orphan_attempts = Arc::new(AtomicUsize::new(0)); - let callback_attempts = Arc::clone(&orphan_attempts); - let orphan_home = orphan_dir.clone(); - let reservation = McpConfiguredClient::reservation_with_action( - &orphan_dir, - Arc::new(move || { - if callback_attempts.fetch_add(1, Ordering::SeqCst) == 0 { - return false; - } - std::fs::remove_dir_all(&orphan_home).is_ok() || !orphan_home.exists() - }), - ); - assert!(!McpConfiguredClient::finish_cleanup( - &orphan_dir, - None, - reservation, - )); - assert!(McpConfiguredClient::agent_close_pending(&orphan_dir)); - assert!(orphan_dir.exists()); - McpConfiguredClient::retry_orphaned_cleanups(); - assert!(!McpConfiguredClient::agent_close_pending(&orphan_dir)); - assert!(!orphan_dir.exists()); -} - #[tokio::test] async fn configuration_switches_replace_the_object_before_dispatch() { let port = FakePort::default(); @@ -943,3 +526,7 @@ async fn non_utf8_workspace_paths_are_rejected_before_opening() { let result = McpConfiguredClient::new(&path, &Config::default()); assert!(matches!(result, Err(ModuleCallError::IncompatibleContract))); } + +#[cfg(test)] +#[path = "module_client_cleanup_tests.rs"] +mod cleanup_tests; From f07267de349bd4a88489a9ab3bb7d11aeec9578a Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Mon, 12 Oct 2026 18:53:06 +0300 Subject: [PATCH 35/45] refactor(mcp): move host cleanup out of the mcp feature gate Host-owned directory cleanup now lives in its own module so it stays available when the mcp feature is disabled, and agent_close_pending reports pending host cleanup instead of always returning false. The embed cleanup path also drops the owner before returning on success. Auto-committed-on: dragonfly Co-authored-by: Medulla --- .../openhuman-core/src/mcp/agent_cleanup.rs | 82 ----------- crates/openhuman-core/src/mcp/host.rs | 15 +- crates/openhuman-core/src/mcp/host_cleanup.rs | 139 ++++++++++++++++++ crates/openhuman-core/src/mcp/mod.rs | 2 + .../openhuman-core/src/mcp/module_client.rs | 12 +- .../src/mcp/module_client_owner_tests.rs | 2 +- .../openhuman-embed/src/agent/mcp_cleanup.rs | 1 + 7 files changed, 154 insertions(+), 99 deletions(-) create mode 100644 crates/openhuman-core/src/mcp/host_cleanup.rs diff --git a/crates/openhuman-core/src/mcp/agent_cleanup.rs b/crates/openhuman-core/src/mcp/agent_cleanup.rs index 782a48f0992..866484b20b9 100644 --- a/crates/openhuman-core/src/mcp/agent_cleanup.rs +++ b/crates/openhuman-core/src/mcp/agent_cleanup.rs @@ -63,85 +63,3 @@ impl AgentMcpCleanup { } } } - -/// Run captured cleanup on the process module executor when modules are enabled. -/// Without modules, the caller must keep its current Tokio runtime alive. -/// This creates no configured client and loads no MCP artifact. -pub fn spawn_agent_cleanup( - future: F, -) -> Result, ModuleCallError> -where - F: std::future::Future + Send + 'static, - F::Output: Send + 'static, -{ - #[cfg(feature = "modules")] - { - crate::modules::host::spawn_cleanup(future).map_err(|_| ModuleCallError::TransportFailed) - } - #[cfg(not(feature = "modules"))] - { - tokio::runtime::Handle::try_current() - .map(|handle| handle.spawn(future)) - .map_err(|_| ModuleCallError::Unavailable) - } -} - -/// A process-local directory reservation with no runtime guard or callback. -/// Dropping the token retains the fence; only confirmed host completion clears it. -#[derive(Clone)] -pub struct AgentDirectoryCleanup { - directory: PathBuf, - reservation: Arc, -} - -/// Reserve an agent's directory until host-owned cleanup succeeds. -/// This never creates a client or loads a module. -pub fn reserve_agent_directory( - workspace: &std::path::Path, - agent_id: &str, -) -> AgentDirectoryCleanup { - let directory = workspace.join("agents").join(agent_id); - let mut reservations = CLEANUP_RESERVATIONS - .get_or_init(|| Mutex::new(HashMap::new())) - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner); - let reservation = Arc::clone( - reservations - .entry(directory.clone()) - .or_insert_with(|| Arc::new(CleanupReservation::default())), - ); - reservation - .host_completion_pending - .store(true, Ordering::Release); - AgentDirectoryCleanup { - directory, - reservation, - } -} - -impl AgentDirectoryCleanup { - /// Release this exact reservation after module close and host purge succeed. - /// An independently registered canonical callback keeps its own reservation. - pub fn complete(&self) { - let after_close = self - .reservation - .after_close - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner); - let mut reservations = CLEANUP_RESERVATIONS - .get_or_init(|| Mutex::new(HashMap::new())) - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner); - if reservations - .get(&self.directory) - .is_some_and(|current| Arc::ptr_eq(current, &self.reservation)) - { - self.reservation - .host_completion_pending - .store(false, Ordering::Release); - if after_close.is_none() { - reservations.remove(&self.directory); - } - } - } -} diff --git a/crates/openhuman-core/src/mcp/host.rs b/crates/openhuman-core/src/mcp/host.rs index 1192e716a48..2d03ed90ecb 100644 --- a/crates/openhuman-core/src/mcp/host.rs +++ b/crates/openhuman-core/src/mcp/host.rs @@ -9,10 +9,13 @@ use std::sync::OnceLock; use crate::config::Config; -pub use super::module_client::agent_cleanup::{ - reserve_agent_directory, spawn_agent_cleanup, take_agent_cleanup, AgentDirectoryCleanup, - AgentMcpCleanup, +pub use super::host_cleanup::{ + reserve_agent_directory, spawn_agent_cleanup, AgentDirectoryCleanup, }; +#[cfg(not(feature = "mcp"))] +pub use super::host_cleanup::{take_agent_cleanup, AgentMcpCleanup}; +#[cfg(feature = "mcp")] +pub use super::module_client::agent_cleanup::{take_agent_cleanup, AgentMcpCleanup}; const PROXY_SERVICE_KEY: &str = "tool.mcp_client"; const DEFAULT_CORE_PORT: u16 = 7788; @@ -100,10 +103,10 @@ pub fn schedule_agent_close_with_cleanup( let _ = after_close(); } -/// No agent-scoped module cleanup is pending when MCP is disabled. +/// Host-owned directory cleanup can remain pending even without MCP. #[cfg(not(feature = "mcp"))] -pub fn agent_close_pending(_workspace_dir: &std::path::Path, _agent_id: &str) -> bool { - false +pub fn agent_close_pending(workspace_dir: &std::path::Path, agent_id: &str) -> bool { + super::host_cleanup::directory_pending(workspace_dir, agent_id) } /// Converts the MCP section of product configuration to the module contract. diff --git a/crates/openhuman-core/src/mcp/host_cleanup.rs b/crates/openhuman-core/src/mcp/host_cleanup.rs new file mode 100644 index 00000000000..ebb3e373751 --- /dev/null +++ b/crates/openhuman-core/src/mcp/host_cleanup.rs @@ -0,0 +1,139 @@ +//! Process-local host cleanup fencing, also used when MCP is compiled out. +use crate::modules::client::ModuleCallError; +use std::{ + collections::HashMap, + path::PathBuf, + sync::{ + atomic::{AtomicBool, Ordering}, + Arc, Mutex, OnceLock, + }, +}; + +pub(super) type CleanupAction = Arc bool + Send + Sync>; + +#[derive(Default)] +pub(super) struct CleanupReservation { + pub(super) after_close: Mutex>, + pub(super) host_completion_pending: AtomicBool, +} + +pub(super) static CLEANUP_RESERVATIONS: OnceLock>>> = + OnceLock::new(); + +pub(super) fn directory_pending(workspace: &std::path::Path, agent_id: &str) -> bool { + CLEANUP_RESERVATIONS.get().is_some_and(|reservations| { + reservations + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .contains_key(&workspace.join("agents").join(agent_id)) + }) +} + +/// Run captured cleanup on the process module executor when modules are enabled. +/// Without modules, the caller must keep its current Tokio runtime alive. +/// This creates no configured client and loads no MCP artifact. +pub fn spawn_agent_cleanup( + future: F, +) -> Result, ModuleCallError> +where + F: std::future::Future + Send + 'static, + F::Output: Send + 'static, +{ + #[cfg(feature = "modules")] + { + crate::modules::host::spawn_cleanup(future).map_err(|_| ModuleCallError::TransportFailed) + } + #[cfg(not(feature = "modules"))] + { + tokio::runtime::Handle::try_current() + .map(|handle| handle.spawn(future)) + .map_err(|_| ModuleCallError::Unavailable) + } +} + +/// A process-local directory reservation with no runtime guard or callback. +/// Dropping the token retains the fence; only confirmed host completion clears it. +#[derive(Clone)] +pub struct AgentDirectoryCleanup { + directory: PathBuf, + reservation: Arc, +} + +/// Reserve an agent's directory until host-owned cleanup succeeds. +/// This never creates a client or loads a module. +pub fn reserve_agent_directory( + workspace: &std::path::Path, + agent_id: &str, +) -> AgentDirectoryCleanup { + let directory = workspace.join("agents").join(agent_id); + let mut reservations = CLEANUP_RESERVATIONS + .get_or_init(|| Mutex::new(HashMap::new())) + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let reservation = Arc::clone( + reservations + .entry(directory.clone()) + .or_insert_with(|| Arc::new(CleanupReservation::default())), + ); + reservation + .host_completion_pending + .store(true, Ordering::Release); + AgentDirectoryCleanup { + directory, + reservation, + } +} + +impl AgentDirectoryCleanup { + /// Release this exact reservation after module close and host purge succeed. + /// An independently registered canonical callback keeps its own reservation. + pub fn complete(&self) { + let after_close = self + .reservation + .after_close + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let mut reservations = CLEANUP_RESERVATIONS + .get_or_init(|| Mutex::new(HashMap::new())) + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + if reservations + .get(&self.directory) + .is_some_and(|current| Arc::ptr_eq(current, &self.reservation)) + { + self.reservation + .host_completion_pending + .store(false, Ordering::Release); + if after_close.is_none() { + reservations.remove(&self.directory); + } + } + } +} + +/// An opaque cleanup handle for a build with no MCP client capability. +/// Such a build cannot create the handle or hold MCP resources. +#[cfg(not(feature = "mcp"))] +#[derive(Clone)] +pub struct AgentMcpCleanup { + _unconstructible: (), +} + +#[cfg(not(feature = "mcp"))] +impl AgentMcpCleanup { + /// There are no MCP resources in this compiled-out capability. + pub async fn finish(&self) -> Result<(), ModuleCallError> { + Ok(()) + } + + /// There are no MCP resources to drain or retire. + pub fn finish_if_idle(&self) -> bool { + true + } +} + +/// A build without MCP cannot contain a configured MCP client. +#[cfg(not(feature = "mcp"))] +pub fn take_agent_cleanup(_workspace: &std::path::Path, _id: &str) -> Option { + None +} diff --git a/crates/openhuman-core/src/mcp/mod.rs b/crates/openhuman-core/src/mcp/mod.rs index bc1236ad57b..fa985b1039c 100644 --- a/crates/openhuman-core/src/mcp/mod.rs +++ b/crates/openhuman-core/src/mcp/mod.rs @@ -65,6 +65,8 @@ pub fn start(config: &crate::config::Config) { #[cfg(feature = "mcp")] pub(crate) mod module_client; +mod host_cleanup; + /// Brings this domain up — the build without it, where there is nothing to do. #[cfg(not(feature = "mcp"))] pub fn start(_config: &crate::config::Config) {} diff --git a/crates/openhuman-core/src/mcp/module_client.rs b/crates/openhuman-core/src/mcp/module_client.rs index 1bc36f1eb1a..dc46de5004a 100644 --- a/crates/openhuman-core/src/mcp/module_client.rs +++ b/crates/openhuman-core/src/mcp/module_client.rs @@ -26,6 +26,8 @@ use tinymcp_bus::{ ValidateServerArgumentsRequest, }; +use super::host_cleanup::{CleanupAction, CleanupReservation, CLEANUP_RESERVATIONS}; + use crate::{ config::Config, modules::{client::ModuleCallError, client::ModuleClient}, @@ -35,16 +37,6 @@ const MODULE: &str = "tinymcp"; const TRANSITION_TIMEOUT: Duration = Duration::from_secs(30); const TRANSITION_POLL_INTERVAL: Duration = Duration::from_millis(20); static CLIENTS: OnceLock>>> = OnceLock::new(); -type CleanupAction = Arc bool + Send + Sync>; - -#[derive(Default)] -struct CleanupReservation { - after_close: Mutex>, - host_completion_pending: AtomicBool, -} - -static CLEANUP_RESERVATIONS: OnceLock>>> = - OnceLock::new(); static SHUTDOWN_REGISTERED: AtomicBool = AtomicBool::new(false); static SHUTTING_DOWN: AtomicBool = AtomicBool::new(false); diff --git a/crates/openhuman-core/src/mcp/module_client_owner_tests.rs b/crates/openhuman-core/src/mcp/module_client_owner_tests.rs index 4328af03c88..fece60a8c92 100644 --- a/crates/openhuman-core/src/mcp/module_client_owner_tests.rs +++ b/crates/openhuman-core/src/mcp/module_client_owner_tests.rs @@ -413,7 +413,7 @@ async fn a_blocked_close_has_an_overall_cleanup_deadline_and_keeps_the_owner_for fn orphaned_shutdown_cleanup_keeps_a_host_directory_reservation_until_confirmed() { let scratch = tempfile::tempdir().unwrap(); let directory = scratch.path().join("agents/purge-pending"); - let token = agent_cleanup::reserve_agent_directory(scratch.path(), "purge-pending"); + let token = crate::mcp::host::reserve_agent_directory(scratch.path(), "purge-pending"); McpConfiguredClient::retry_orphaned_cleanups(); assert!(McpConfiguredClient::agent_close_pending(&directory)); let reservation = McpConfiguredClient::reservation_for(&directory); diff --git a/crates/openhuman-embed/src/agent/mcp_cleanup.rs b/crates/openhuman-embed/src/agent/mcp_cleanup.rs index 2e09a2996d8..fab6927e9bb 100644 --- a/crates/openhuman-embed/src/agent/mcp_cleanup.rs +++ b/crates/openhuman-embed/src/agent/mcp_cleanup.rs @@ -127,6 +127,7 @@ where match result { Ok(()) => { owner.confirmed = true; + drop(owner); return; } Err(error) => { From ee29ab1785f3d9998cfaac2479785e0da8a27330 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Mon, 12 Oct 2026 18:55:56 +0300 Subject: [PATCH 36/45] chore(mcp): gate directory_pending behind non-mcp feature Add a cfg attribute so directory_pending is only compiled when the mcp feature is disabled, avoiding an unused-function warning in mcp builds. Auto-committed-on: dragonfly Co-authored-by: Medulla --- crates/openhuman-core/src/mcp/host_cleanup.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/openhuman-core/src/mcp/host_cleanup.rs b/crates/openhuman-core/src/mcp/host_cleanup.rs index ebb3e373751..3e9ff517491 100644 --- a/crates/openhuman-core/src/mcp/host_cleanup.rs +++ b/crates/openhuman-core/src/mcp/host_cleanup.rs @@ -20,6 +20,7 @@ pub(super) struct CleanupReservation { pub(super) static CLEANUP_RESERVATIONS: OnceLock>>> = OnceLock::new(); +#[cfg(not(feature = "mcp"))] pub(super) fn directory_pending(workspace: &std::path::Path, agent_id: &str) -> bool { CLEANUP_RESERVATIONS.get().is_some_and(|reservations| { reservations From 6a9d8d51255d6340ecb930208886f29adb6eeede Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Sun, 18 Oct 2026 11:42:50 +0530 Subject: [PATCH 37/45] chore(vendor): bump tinymcp submodule to 2df3935 Update the vendored tinymcp submodule to the latest upstream commit. Auto-committed-on: macbook --- vendor/tinymcp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vendor/tinymcp b/vendor/tinymcp index 06c02a6b75f..2df393596bc 160000 --- a/vendor/tinymcp +++ b/vendor/tinymcp @@ -1 +1 @@ -Subproject commit 06c02a6b75fb53739245effce47941b39a131f57 +Subproject commit 2df393596bc4a10632975fb4cad5301b0a8ce10e From 6e10fa07bf09b96b37b2352c5701f999bab04fe3 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Sun, 18 Oct 2026 11:43:15 +0530 Subject: [PATCH 38/45] test(mcp): update cleanup tests for lifecycle table API Switch the orphaned-path cleanup assertion to call finish_cleanup through the lifecycle table with a borrowed reservation, and replace negated is_some checks with is_none for clarity. Auto-committed-on: macbook --- .../openhuman-core/src/mcp/module_client_cleanup_tests.rs | 6 +----- crates/openhuman-core/src/mcp/module_client_owner_tests.rs | 4 ++-- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/crates/openhuman-core/src/mcp/module_client_cleanup_tests.rs b/crates/openhuman-core/src/mcp/module_client_cleanup_tests.rs index 1cdb2fdcbd2..15db7668c7d 100644 --- a/crates/openhuman-core/src/mcp/module_client_cleanup_tests.rs +++ b/crates/openhuman-core/src/mcp/module_client_cleanup_tests.rs @@ -114,11 +114,7 @@ async fn failed_cleanup_actions_retain_and_retry_cached_and_orphaned_paths() { std::fs::remove_dir_all(&orphan_home).is_ok() || !orphan_home.exists() }), ); - assert!(!McpConfiguredClient::finish_cleanup( - &orphan_dir, - None, - reservation, - )); + assert!(!lifecycle::table().finish_cleanup(&orphan_dir, None, &reservation,)); assert!(McpConfiguredClient::agent_close_pending(&orphan_dir)); assert!(orphan_dir.exists()); lifecycle::table().retry_orphaned(); diff --git a/crates/openhuman-core/src/mcp/module_client_owner_tests.rs b/crates/openhuman-core/src/mcp/module_client_owner_tests.rs index d0eb3503252..cd064dac85d 100644 --- a/crates/openhuman-core/src/mcp/module_client_owner_tests.rs +++ b/crates/openhuman-core/src/mcp/module_client_owner_tests.rs @@ -91,7 +91,7 @@ async fn failed_agent_cleanup_retains_the_retired_owner_and_can_retry() { ); fixture.fail_close.store(false, Ordering::SeqCst); cleanup.finish().await.unwrap(); - assert!(!cached(&directory).is_some()); + assert!(cached(&directory).is_none()); // Retrying the old completed handle cannot evict a later owner. let successor = Arc::new(McpConfiguredClient::new(&directory, &Config::default()).unwrap()); lifecycle::table().insert(directory.clone(), Arc::clone(&successor)); @@ -355,7 +355,7 @@ async fn a_blocked_close_has_an_overall_cleanup_deadline_and_keeps_the_owner_for fixture.release_close.notify_one(); cleanup.finish().await.unwrap(); assert_eq!(fixture.closes.load(Ordering::SeqCst), 2); - assert!(!cached(&directory).is_some()); + assert!(cached(&directory).is_none()); } #[test] From cf751c402274c47f1aba5537e96c5a2095d3752e Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Sun, 18 Oct 2026 11:48:47 +0530 Subject: [PATCH 39/45] test(mcp): canonicalize temp workspace paths in lifecycle tests The cleanup and owner lifecycle tests now canonicalize their temporary workspace roots before using them as cache keys, so path comparisons match the canonicalized paths the lifecycle table stores. This keeps the tests from failing on platforms where temp directories resolve through symlinks. Auto-committed-on: macbook --- .../src/mcp/module_client_cleanup_tests.rs | 33 ++++++++++------ .../src/mcp/module_client_owner_tests.rs | 39 ++++++++++++++----- 2 files changed, 52 insertions(+), 20 deletions(-) diff --git a/crates/openhuman-core/src/mcp/module_client_cleanup_tests.rs b/crates/openhuman-core/src/mcp/module_client_cleanup_tests.rs index 15db7668c7d..5f63dada6b0 100644 --- a/crates/openhuman-core/src/mcp/module_client_cleanup_tests.rs +++ b/crates/openhuman-core/src/mcp/module_client_cleanup_tests.rs @@ -3,8 +3,12 @@ use super::*; #[tokio::test] async fn delayed_agent_close_cannot_evict_a_replacement_cache_entry() { let workspace = tempfile::tempdir().expect("workspace"); + let workspace_dir = workspace + .path() + .canonicalize() + .expect("canonical fixture root"); let mut config = Config::default(); - config.workspace_dir = workspace.path().to_path_buf(); + config.workspace_dir = workspace_dir.as_path().to_path_buf(); config.modules.enabled = true; let old_port = FakePort::default(); @@ -17,27 +21,30 @@ async fn delayed_agent_close_cannot_evict_a_replacement_cache_entry() { let cleanup_ran = Arc::new(std::sync::atomic::AtomicBool::new(false)); let callback_flag = Arc::clone(&cleanup_ran); let reservation = lifecycle::table().reservation_with_action( - workspace.path(), + workspace_dir.as_path(), Arc::new(move || { callback_flag.store(true, std::sync::atomic::Ordering::SeqCst); true }), ); let captured = lifecycle::table() - .retire(workspace.path()) + .retire(workspace_dir.as_path()) .expect("retirement captures the exact old client"); let replacement_port = FakePort::default(); let replacement_closed = Arc::clone(&replacement_port.closed); let replacement = Arc::new(McpConfiguredClient::with_port( - workspace.path().to_string_lossy().into_owned(), + workspace_dir.as_path().to_string_lossy().into_owned(), Box::new(replacement_port), )); - lifecycle::table().insert(workspace.path().to_path_buf(), Arc::clone(&replacement)); + lifecycle::table().insert( + workspace_dir.as_path().to_path_buf(), + Arc::clone(&replacement), + ); assert!( !McpConfiguredClient::close_retired_client( - workspace.path().to_path_buf(), + workspace_dir.as_path().to_path_buf(), captured, Arc::clone(&reservation), ) @@ -50,12 +57,12 @@ async fn delayed_agent_close_cannot_evict_a_replacement_cache_entry() { let current = lifecycle::table() .snapshot() .into_iter() - .find(|(path, _)| path == workspace.path()) + .find(|(path, _)| path == workspace_dir.as_path()) .unwrap() .1; assert!(Arc::ptr_eq(¤t, &replacement)); - lifecycle::table().evict(workspace.path(), &replacement); - lifecycle::table().finish_cleanup(workspace.path(), None, &reservation); + lifecycle::table().evict(workspace_dir.as_path(), &replacement); + lifecycle::table().finish_cleanup(workspace_dir.as_path(), None, &reservation); } #[tokio::test] @@ -63,7 +70,11 @@ async fn failed_cleanup_actions_retain_and_retry_cached_and_orphaned_paths() { use std::sync::atomic::{AtomicUsize, Ordering}; let workspace = tempfile::tempdir().expect("workspace"); - let agent_dir = workspace.path().join("agents").join("alpha"); + let workspace_dir = workspace + .path() + .canonicalize() + .expect("canonical fixture root"); + let agent_dir = workspace_dir.as_path().join("agents").join("alpha"); std::fs::create_dir_all(&agent_dir).unwrap(); std::fs::write(agent_dir.join("state.json"), "retained until cleanup").unwrap(); let mut config = Config::default(); @@ -100,7 +111,7 @@ async fn failed_cleanup_actions_retain_and_retry_cached_and_orphaned_paths() { assert!(!agent_dir.exists()); assert!(McpConfiguredClient::for_config(&config).is_ok()); - let orphan_dir = workspace.path().join("agents").join("orphan"); + let orphan_dir = workspace_dir.as_path().join("agents").join("orphan"); std::fs::create_dir_all(&orphan_dir).unwrap(); let orphan_attempts = Arc::new(AtomicUsize::new(0)); let callback_attempts = Arc::clone(&orphan_attempts); diff --git a/crates/openhuman-core/src/mcp/module_client_owner_tests.rs b/crates/openhuman-core/src/mcp/module_client_owner_tests.rs index cd064dac85d..fdc17d8eb1b 100644 --- a/crates/openhuman-core/src/mcp/module_client_owner_tests.rs +++ b/crates/openhuman-core/src/mcp/module_client_owner_tests.rs @@ -42,7 +42,11 @@ impl LifecycleFixture { #[tokio::test] async fn failed_agent_cleanup_retains_the_retired_owner_and_can_retry() { let scratch = tempfile::tempdir().unwrap(); - let directory = scratch.path().join("agents/worker"); + let scratch_dir = scratch + .path() + .canonicalize() + .expect("canonical fixture root"); + let directory = scratch_dir.as_path().join("agents/worker"); let bus = MemoryBus::new(); Broker::new().spawn(bus.clone()); let service = Connection::connect(bus.connect().await.unwrap()) @@ -75,7 +79,7 @@ async fn failed_agent_cleanup_retains_the_retired_owner_and_can_retry() { object_path: "/test/original-child".into(), }); lifecycle::table().insert(directory.clone(), Arc::clone(&client)); - let cleanup = agent_cleanup::take_agent_cleanup(scratch.path(), "worker").unwrap(); + let cleanup = agent_cleanup::take_agent_cleanup(scratch_dir.as_path(), "worker").unwrap(); fixture.fail_close.store(true, Ordering::SeqCst); assert_eq!(cleanup.finish().await, Err(ModuleCallError::ModuleFault)); assert!(Arc::ptr_eq(cached(&directory).as_ref().unwrap(), &client)); @@ -106,8 +110,12 @@ async fn failed_agent_cleanup_retains_the_retired_owner_and_can_retry() { #[test] fn an_absent_agent_cleanup_does_not_create_a_configured_client() { let scratch = tempfile::tempdir().unwrap(); - assert!(agent_cleanup::take_agent_cleanup(scratch.path(), "never-opened").is_none()); - assert!(cached(&scratch.path().join("agents/never-opened")).is_none()); + let scratch_dir = scratch + .path() + .canonicalize() + .expect("canonical fixture root"); + assert!(agent_cleanup::take_agent_cleanup(scratch_dir.as_path(), "never-opened").is_none()); + assert!(cached(&scratch_dir.as_path().join("agents/never-opened")).is_none()); } #[tokio::test] @@ -115,8 +123,12 @@ async fn an_old_closed_context_cannot_create_the_reused_agents_first_client() { use crate::core::runtime::{ContextOverlay, CoreContext, DomainSet}; use crate::tools::toolpacks::ToolGroups; let scratch = tempfile::tempdir().unwrap(); + let scratch_dir = scratch + .path() + .canonicalize() + .expect("canonical fixture root"); let mut config = Config::default(); - config.workspace_dir = scratch.path().into(); + config.workspace_dir = scratch_dir.as_path().into(); let parent = CoreContext::for_test(DomainSet::full(), Some(config.workspace_dir.clone())); let old_scope = Arc::new(crate::security::approval::ApprovalScope::default()); let mut old_overrides = crate::agent::host_overrides::HostOverrides::default(); @@ -293,7 +305,11 @@ async fn cleanup_receipts_stay_with_the_original_module_after_alias_replacement( #[tokio::test(start_paused = true)] async fn a_blocked_close_has_an_overall_cleanup_deadline_and_keeps_the_owner_for_retry() { let scratch = tempfile::tempdir().unwrap(); - let directory = scratch.path().join("agents/blocked-close"); + let scratch_dir = scratch + .path() + .canonicalize() + .expect("canonical fixture root"); + let directory = scratch_dir.as_path().join("agents/blocked-close"); let bus = MemoryBus::new(); Broker::new().spawn(bus.clone()); let service = Connection::connect(bus.connect().await.unwrap()) @@ -330,7 +346,8 @@ async fn a_blocked_close_has_an_overall_cleanup_deadline_and_keeps_the_owner_for object_path: "/test/original-child".into(), }); lifecycle::table().insert(directory.clone(), Arc::clone(&client)); - let cleanup = agent_cleanup::take_agent_cleanup(scratch.path(), "blocked-close").unwrap(); + let cleanup = + agent_cleanup::take_agent_cleanup(scratch_dir.as_path(), "blocked-close").unwrap(); fixture.block_close.store(true, Ordering::SeqCst); let attempt = cleanup.clone(); let mut task = tokio::spawn(async move { attempt.finish().await }); @@ -361,8 +378,12 @@ async fn a_blocked_close_has_an_overall_cleanup_deadline_and_keeps_the_owner_for #[test] fn orphaned_shutdown_cleanup_keeps_a_host_directory_reservation_until_confirmed() { let scratch = tempfile::tempdir().unwrap(); - let directory = scratch.path().join("agents/purge-pending"); - let token = crate::mcp::host::reserve_agent_directory(scratch.path(), "purge-pending"); + let scratch_dir = scratch + .path() + .canonicalize() + .expect("canonical fixture root"); + let directory = scratch_dir.as_path().join("agents/purge-pending"); + let token = crate::mcp::host::reserve_agent_directory(scratch_dir.as_path(), "purge-pending"); lifecycle::table().retry_orphaned(); assert!(McpConfiguredClient::agent_close_pending(&directory)); let reservation = lifecycle::table().reservation_for(&directory); From 9fa0fda3a5a55e51cb88b811b046aa8445359f99 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Sun, 18 Oct 2026 11:57:10 +0530 Subject: [PATCH 40/45] test(agent_lifecycle): log progress while waiting for agents to park Added eprintln statements before each wait for a pending approval request so that a hanging lifecycle test shows which stage it stalled at. Auto-committed-on: macbook --- crates/openhuman-embed/tests/agent_lifecycle.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/openhuman-embed/tests/agent_lifecycle.rs b/crates/openhuman-embed/tests/agent_lifecycle.rs index 24765f083a5..d3dff31adfe 100644 --- a/crates/openhuman-embed/tests/agent_lifecycle.rs +++ b/crates/openhuman-embed/tests/agent_lifecycle.rs @@ -110,6 +110,7 @@ fn removing_an_agent_releases_everything_it_held() { let closing = closing.clone(); tokio::spawn(async move { closing.turn("write the marker").send().await }) }; + eprintln!("[agent_lifecycle] waiting for closing_request"); let closing_request = eventually("closing agent to park", || { closing.approvals().pending().ok()?.into_iter().next() }) @@ -173,6 +174,7 @@ fn removing_an_agent_releases_everything_it_held() { let replacement = replacement.clone(); tokio::spawn(async move { replacement.turn("write the marker").send().await }) }; + eprintln!("[agent_lifecycle] waiting for replacement_request"); let replacement_request = eventually("replacement to park", || { replacement.approvals().pending().ok()?.into_iter().next() }) @@ -207,6 +209,7 @@ fn removing_an_agent_releases_everything_it_held() { let parker = parker.clone(); tokio::spawn(async move { parker.turn("write the marker").send().await }) }; + eprintln!("[agent_lifecycle] waiting for request"); let request = eventually("parker to park", || { parker .approvals() From 97b406d3279c3112b8cea76b4d467bd615d31f6f Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Sun, 18 Oct 2026 11:58:42 +0530 Subject: [PATCH 41/45] test(agent_lifecycle): use a host-provided tool to park at the approval gate Replace the shell-based fixture with a WriteMarker tool supplied through HostTurnTools so the lifecycle test parks at the real approval gate without depending on native TinyBox shell analysis or artifact loading, and drop the leftover debug prints. Auto-committed-on: macbook --- .../openhuman-embed/tests/agent_lifecycle.rs | 49 +++++++++++++------ 1 file changed, 35 insertions(+), 14 deletions(-) diff --git a/crates/openhuman-embed/tests/agent_lifecycle.rs b/crates/openhuman-embed/tests/agent_lifecycle.rs index d3dff31adfe..6334638b3c4 100644 --- a/crates/openhuman-embed/tests/agent_lifecycle.rs +++ b/crates/openhuman-embed/tests/agent_lifecycle.rs @@ -14,8 +14,8 @@ use openhuman_core::core::events::DomainEvent; use openhuman_core::core::runtime::AgentContextRegistry; use openhuman_core::security::AutonomyLevel; use openhuman_embed::{ - Access, Agent, AgentDefinitionSpec, AgentError, AgentSpec, AgentTurnOrigin, CoreError, Runtime, - ToolScopeSpec, TrustedAccess, Workspace, + Access, Agent, AgentDefinitionSpec, AgentError, AgentSpec, AgentTurnOrigin, CoreError, + HostTurnTools, Runtime, Tool, ToolScopeSpec, TrustedAccess, Workspace, }; fn supervised(scratch: &std::path::Path) -> Access { @@ -30,26 +30,50 @@ fn supervised(scratch: &std::path::Path) -> Access { .trust(scratch.display().to_string(), TrustedAccess::ReadWrite) } +/// The lifecycle test needs an acting tool that parks at the real approval +/// gate, independently of native TinyBox shell analysis and artifact loading. +struct WriteMarker(std::path::PathBuf); + +#[async_trait::async_trait] +impl Tool for WriteMarker { + fn name(&self) -> &str { + "write_marker" + } + fn description(&self) -> &str { + "Write the fixture marker after approval" + } + fn parameters_schema(&self) -> serde_json::Value { + serde_json::json!({"type": "object", "properties": {}}) + } + fn permission_level(&self) -> tinytools::PermissionLevel { + tinytools::PermissionLevel::Write + } + fn external_effect(&self) -> bool { + true + } + async fn execute(&self, _: serde_json::Value) -> anyhow::Result { + std::fs::write(&self.0, "approved")?; + Ok(tinytools::ToolResult::success("marker written")) + } +} + async fn parking_agent( runtime: &Runtime, id: &str, scratch: &std::path::Path, ) -> (Agent, wiremock::MockServer) { let file = scratch.join(format!("{id}-wrote")); - let call = tool_call_completion( - "shell", - &serde_json::json!({ "command": format!("touch {}", file.display()) }).to_string(), - ); + let call = tool_call_completion("write_marker", "{}"); let provider = scripted_provider(vec![call], &format!("{id}-done")).await; let agent = runtime .agent( AgentSpec::new(id) .provider(route(&provider, &format!("{id}-model"))) .access(supervised(scratch)) - .definition( - AgentDefinitionSpec::new() - .tools(ToolScopeSpec::Named(vec!["shell".to_string()])), - ), + .definition(AgentDefinitionSpec::new().tools(ToolScopeSpec::HostOnly)) + .tools(move |_| { + HostTurnTools::advertised(vec![Box::new(WriteMarker(file.clone()))]) + }), ) .expect("agent instantiates"); (agent, provider) @@ -110,7 +134,6 @@ fn removing_an_agent_releases_everything_it_held() { let closing = closing.clone(); tokio::spawn(async move { closing.turn("write the marker").send().await }) }; - eprintln!("[agent_lifecycle] waiting for closing_request"); let closing_request = eventually("closing agent to park", || { closing.approvals().pending().ok()?.into_iter().next() }) @@ -174,7 +197,6 @@ fn removing_an_agent_releases_everything_it_held() { let replacement = replacement.clone(); tokio::spawn(async move { replacement.turn("write the marker").send().await }) }; - eprintln!("[agent_lifecycle] waiting for replacement_request"); let replacement_request = eventually("replacement to park", || { replacement.approvals().pending().ok()?.into_iter().next() }) @@ -209,7 +231,6 @@ fn removing_an_agent_releases_everything_it_held() { let parker = parker.clone(); tokio::spawn(async move { parker.turn("write the marker").send().await }) }; - eprintln!("[agent_lifecycle] waiting for request"); let request = eventually("parker to park", || { parker .approvals() @@ -241,7 +262,7 @@ fn removing_an_agent_releases_everything_it_held() { request_id: None, }, gate.intercept_forced( - "shell", + "write_marker", "late removal registration", serde_json::json!({}), ), From d9b127dd8e4189128acb04f264fc248c889bd718 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Sun, 18 Oct 2026 11:59:50 +0530 Subject: [PATCH 42/45] test(agent-lifecycle): scope parked agent tools to the named marker tool The parking agent test now declares its tool scope as the named write_marker tool instead of host-only, so the advertised tool set matches the scope the agent is given. Auto-committed-on: macbook --- crates/openhuman-embed/tests/agent_lifecycle.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/openhuman-embed/tests/agent_lifecycle.rs b/crates/openhuman-embed/tests/agent_lifecycle.rs index 6334638b3c4..93f21a06e38 100644 --- a/crates/openhuman-embed/tests/agent_lifecycle.rs +++ b/crates/openhuman-embed/tests/agent_lifecycle.rs @@ -70,7 +70,10 @@ async fn parking_agent( AgentSpec::new(id) .provider(route(&provider, &format!("{id}-model"))) .access(supervised(scratch)) - .definition(AgentDefinitionSpec::new().tools(ToolScopeSpec::HostOnly)) + .definition( + AgentDefinitionSpec::new() + .tools(ToolScopeSpec::Named(vec!["write_marker".into()])), + ) .tools(move |_| { HostTurnTools::advertised(vec![Box::new(WriteMarker(file.clone()))]) }), From 231a5b1cc2eb65b2c155ab027235e7d6ae9f404a Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Sun, 18 Oct 2026 12:01:45 +0530 Subject: [PATCH 43/45] test(openhuman-embed): gate link_tinyflows helper behind modules feature The shared test helper that registers the linked tinyflows module now only compiles when the modules feature is enabled, since it depends on module registration APIs that are unavailable otherwise. Auto-committed-on: macbook --- crates/openhuman-embed/tests/common/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/openhuman-embed/tests/common/mod.rs b/crates/openhuman-embed/tests/common/mod.rs index 7733babe5be..c22bd41c966 100644 --- a/crates/openhuman-embed/tests/common/mod.rs +++ b/crates/openhuman-embed/tests/common/mod.rs @@ -63,6 +63,7 @@ pub fn offline_config() -> Config { /// here, with a config that asks for `linked`: the resolution slot is /// process-wide, so every later caller (a profile's own config included) sees /// the linked module. Only this module changes source. +#[cfg(feature = "modules")] pub fn link_tinyflows(runtime: &tokio::runtime::Runtime) { use openhuman_core::config::schema::{ModuleOverride, ModuleSourceMode}; openhuman_embed::modules::register_linked("tinyflows", tinyflows_module::linked_module); From 3db461955bd54fd270eb7bea94917062e35ba19e Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Sun, 18 Oct 2026 12:02:54 +0530 Subject: [PATCH 44/45] test(embed): gate tinyflows linking behind modules feature The shared test runtime now only links tinyflows when the modules feature is enabled, matching the feature gating used elsewhere so the test helpers compile without it. Auto-committed-on: macbook --- crates/openhuman-embed/tests/common/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/openhuman-embed/tests/common/mod.rs b/crates/openhuman-embed/tests/common/mod.rs index c22bd41c966..b83695f387f 100644 --- a/crates/openhuman-embed/tests/common/mod.rs +++ b/crates/openhuman-embed/tests/common/mod.rs @@ -95,6 +95,7 @@ pub fn runtime_without_master_key() -> tokio::runtime::Runtime { .max_blocking_threads(MAX_BLOCKING_THREADS) .build() .expect("tokio runtime"); + #[cfg(feature = "modules")] link_tinyflows(&runtime); runtime } From 26bc6bb10fa7b86004515bf0b87693405408cd72 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Sun, 18 Oct 2026 12:15:02 +0530 Subject: [PATCH 45/45] chore(vendor): bump tinybus and tinymcp submodules Updated the vendored tinybus and tinymcp submodules to their latest commits. Auto-committed-on: macbook --- vendor/tinybus | 2 +- vendor/tinymcp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/vendor/tinybus b/vendor/tinybus index 247e4d031ba..a74c23ff013 160000 --- a/vendor/tinybus +++ b/vendor/tinybus @@ -1 +1 @@ -Subproject commit 247e4d031ba32dd4e6cad857725f2337fc851d98 +Subproject commit a74c23ff013b76630155c87913d0d2d4cc1298b2 diff --git a/vendor/tinymcp b/vendor/tinymcp index 2df393596bc..4ef02dc9a1a 160000 --- a/vendor/tinymcp +++ b/vendor/tinymcp @@ -1 +1 @@ -Subproject commit 2df393596bc4a10632975fb4cad5301b0a8ce10e +Subproject commit 4ef02dc9a1a24570b2abf64f191b82383d014e05