Skip to content

fix (acp): Refresh provider when session working directory changes #9883

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lifeizhou-ap merged 1 commit into from
Jun 19, 2026
Merged

fix (acp): Refresh provider when session working directory changes #9883

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
41 changes: 32 additions & 9 deletions crates/goose/src/acp/server/manage_sessions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,20 +13,43 @@ impl GooseAcpAgent {
let path = std::path::PathBuf::from(&working_dir);
validate_absolute_cwd(&path)?;
let session_id = &req.session_id;

let session = self
.session_manager
.get_session(session_id, false)
.await
.map_err(|_| {
agent_client_protocol::Error::resource_not_found(Some(session_id.to_string()))
.data(format!("Session not found: {}", session_id))
})?;

if path == session.working_dir {
return Ok(EmptyResponse {});
}

self.session_manager
.update(session_id)
.working_dir(path.clone())
.working_dir(path)
.apply()
.await
.internal_err()?;
.internal_err_ctx("Failed to update session working directory")?;

if let Some(session) = self.sessions.lock().await.get(session_id) {
session
.agent
.extension_manager
.update_working_dir(&path)
.await;
}
let session = self
.session_manager
.get_session(session_id, false)
.await
.internal_err_ctx("Failed to reload session")?;

let agent = self.get_session_agent(session_id).await?;

@chatgpt-codex-connector chatgpt-codex-connector Bot Jun 19, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Skip agent startup for inactive cwd updates

When this request targets a persisted session that is not currently registered in self.sessions, get_session_agent falls back to activating the session (via activate_acp_session in server.rs) just to refresh a provider. In that context there is no live provider to refresh, so changing metadata for an inactive/listed session can unexpectedly spawn provider subprocesses and extensions (and then recreate the provider again here), whereas the previous path only notified an already-active ACP session. Please only refresh the provider when an agent is already registered, and otherwise leave the persisted cwd update as a metadata-only operation.

Useful? React with 👍 / 👎.

@lifeizhou-ap lifeizhou-ap Jun 19, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The activation is intentional for the current desktop migration path: the UI can update cwd through ACP for a session originally created through REST, so absence fromself.sessions does not mean “inactive/list-only.” We can change it when the migration is done

@jh-block jh-block Jun 19, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It makes sense that activation is intentional but in case get_session_agent created an ACP provider (it calls create_agent_locked), the new restore_provider_from_session call below will immediately create it again. So if CWD is updated and the session was not already active, then the ACP provider is created twice in a row.

agent
.restore_provider_from_session(&session)
.await
.internal_err_ctx("Failed to refresh provider from session")?;

agent
.extension_manager
.update_working_dir(&session.working_dir)
.await;

Ok(EmptyResponse {})
}
Expand Down
Loading