Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 59 additions & 4 deletions src/agent/manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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?;

Expand Down Expand Up @@ -603,6 +600,33 @@ 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<PathBuf> {
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<String> {
Expand Down Expand Up @@ -894,6 +918,37 @@ 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();
Expand Down
Loading