From 0c7427114824650befd2c894d715d8f9d99f0cdc Mon Sep 17 00:00:00 2001 From: Fullstop000 Date: Sat, 23 May 2026 11:17:12 +0800 Subject: [PATCH 1/2] fix(agent): scope runtime agent dir by workspace_id from the store MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The runtime `Agent` arrives over the bridge with an empty `workspace_id` (the `bridge.target` frame omits it; `target_to_agent` fills `String::new()`), so `start_agent`'s inline path join produced `agents/{name}-{id}` — dropping the `{workspace_id}` segment. But workspace delete (`agents.rs`) and the agent-workspace file API (`agent_workspace.rs`) build `agents/{workspace_id}/{name}-{id}` via `AgentWorkspace` using the DB `workspace_id`. The mismatch orphaned agent runtime directories on workspace delete and pointed the file API at the wrong directory. Resolve `workspace_id` from the store in `start_agent` and build the path via `AgentWorkspace::path_for`, so creation matches the delete/file-API paths. Found by dogfooding the autonomous task loop. Co-Authored-By: Claude Opus 4.7 --- src/agent/manager.rs | 60 +++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 56 insertions(+), 4 deletions(-) diff --git a/src/agent/manager.rs b/src/agent/manager.rs index 50d739df..5a499071 100644 --- a/src/agent/manager.rs +++ b/src/agent/manager.rs @@ -228,10 +228,7 @@ impl AgentManager { let rt = AgentRuntime::parse(&agent.runtime) .ok_or_else(|| anyhow::anyhow!("Unknown runtime: {}", agent.runtime))?; - let agent_data_dir = self - .data_dir - .join(&agent.workspace_id) - .join(format!("{}-{}", agent.name, agent.id)); + let agent_data_dir = self.agent_workspace_dir(&agent.id, &agent.name)?; tokio::fs::create_dir_all(&agent_data_dir).await?; @@ -603,6 +600,28 @@ impl AgentManager { mgr } + /// Resolve an agent's on-disk runtime/workspace directory, scoped by the + /// `workspace_id` looked up from the store. + /// + /// The runtime [`Agent`] that arrives over the bridge does NOT carry + /// `workspace_id` — the `bridge.target` frame omits it and + /// `target_to_agent` fills `String::new()`. Building the path off that + /// empty field drops the `{workspace_id}` segment, so the runtime would + /// create `agents/{name}-{id}` while every platform-side path (workspace + /// delete + the agent-workspace file API) uses + /// [`AgentWorkspace::path_for`] = `agents/{workspace_id}/{name}-{id}`. + /// That mismatch orphans the directory on workspace delete and hides the + /// agent's real files. Resolve the workspace_id from the store so creation + /// and the platform-side paths agree. + fn agent_workspace_dir(&self, agent_id: &str, agent_name: &str) -> anyhow::Result { + let workspace_id = self + .store + .agent_workspace_id(agent_id)? + .ok_or_else(|| anyhow::anyhow!("agent {agent_id} has no workspace_id"))?; + Ok(crate::agent::workspace::AgentWorkspace::new(&self.data_dir) + .path_for(&workspace_id, agent_name, agent_id)) + } + /// Resolve the shared bridge endpoint. Fails loudly if no bridge is /// running — there is no stdio fallback anymore. fn resolve_bridge_endpoint(&self) -> anyhow::Result { @@ -894,6 +913,39 @@ mod tests { let _ = manager.stop_agent(&agent_id).await; } + #[test] + fn agent_workspace_dir_is_scoped_by_store_workspace_id() { + // Regression: the runtime Agent arrives over the bridge with an empty + // workspace_id (target_to_agent), so the create path must resolve the + // real workspace_id from the store — otherwise it drops the + // {workspace_id} segment and diverges from the delete/file-API paths, + // orphaning the dir on workspace delete. + let dir = tempdir().unwrap(); + let store = Arc::new(Store::open(dir.path().join("chorus.db").to_str().unwrap()).unwrap()); + insert_codex_agent(&store); + let agent = store.get_agent("v2bot").unwrap().unwrap(); + assert!( + !agent.workspace_id.is_empty(), + "seeded agent should have a real workspace_id" + ); + + let manager = make_test_manager(store, dir.path()); + let got = manager + .agent_workspace_dir(&agent.id, &agent.name) + .unwrap(); + + // Must match the canonical path the delete + file-API paths use. + let expected = crate::agent::workspace::AgentWorkspace::new(&dir.path().join("agents")) + .path_for(&agent.workspace_id, &agent.name, &agent.id); + assert_eq!(got, expected); + // ...and must actually include the workspace_id segment (the bug + // silently dropped it because the runtime Agent's field was empty). + assert!( + got.to_string_lossy().contains(&agent.workspace_id), + "agent dir must be scoped by workspace_id, got {got:?}" + ); + } + #[tokio::test] async fn stop_agent_idempotent() { let dir = tempdir().unwrap(); From b717eccf210756ed2ed821d0a54d7170d792d21c Mon Sep 17 00:00:00 2001 From: Fullstop000 Date: Sat, 23 May 2026 11:19:57 +0800 Subject: [PATCH 2/2] style: cargo fmt (agent_workspace_dir helper + test) --- src/agent/manager.rs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/agent/manager.rs b/src/agent/manager.rs index 5a499071..14668f8b 100644 --- a/src/agent/manager.rs +++ b/src/agent/manager.rs @@ -618,8 +618,13 @@ impl AgentManager { .store .agent_workspace_id(agent_id)? .ok_or_else(|| anyhow::anyhow!("agent {agent_id} has no workspace_id"))?; - Ok(crate::agent::workspace::AgentWorkspace::new(&self.data_dir) - .path_for(&workspace_id, agent_name, agent_id)) + Ok( + crate::agent::workspace::AgentWorkspace::new(&self.data_dir).path_for( + &workspace_id, + agent_name, + agent_id, + ), + ) } /// Resolve the shared bridge endpoint. Fails loudly if no bridge is @@ -930,9 +935,7 @@ mod tests { ); let manager = make_test_manager(store, dir.path()); - let got = manager - .agent_workspace_dir(&agent.id, &agent.name) - .unwrap(); + let got = manager.agent_workspace_dir(&agent.id, &agent.name).unwrap(); // Must match the canonical path the delete + file-API paths use. let expected = crate::agent::workspace::AgentWorkspace::new(&dir.path().join("agents"))