awesome-bookmarks/goose
3
0
Fork 0
mirror of https://github.com/block/goose.git synced 2026-07-17 12:56:20 +02:00
Code Issues Packages Projects Releases Wiki Activity

refactor(acp): use mode negotiation, reduce session overhead, handle session titles

Browse Source 
Signed-off-by: Adrian Cole <adrian@tetrate.io>
This commit is contained in:
Adrian Cole
2026-04-02 10:37:18 -04:00
parent e94f304780
commit fec3e7bb65
12 changed files with 373 additions and 119 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Cargo.lock
Generated
+8 -1
View File
@@ -3224,6 +3224,12 @@ version = "0.11.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "1435fa1053d8b2fbbe9be7e97eca7f33d37b28409959813daefc1446a14247f1"
[[package]]
name = "downcast-rs"
version = "2.0.2"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "117240f60069e65410b3ae1bb213295bd828f707b5bec6596a1afc8793ce0cbc"
[[package]]
name = "dprint-swc-ext"
version = "0.26.0"
@@ -4352,6 +4358,7 @@ dependencies = [
"dashmap 6.1.0",
"dirs 5.0.1",
"dotenvy",
"downcast-rs",
"encoding_rs",
"env-lock",
"etcetera 0.11.0",
@@ -4573,7 +4580,7 @@ dependencies = [
[[package]]
name = "goose-sdk"
version = "1.29.0"
version = "1.30.0"
dependencies = [
"agent-client-protocol-schema",
"sacp",
crates/goose-acp/src/server.rs
+21 -1
View File
@@ -3,7 +3,7 @@ use crate::fs::AcpTools;
use crate::tools::AcpAwareToolMeta;
use anyhow::Result;
use fs_err as fs;
use goose::acp::PermissionDecision;
use goose::acp::{AcpProvider, PermissionDecision};
use goose::agents::extension::{Envs, PLATFORM_EXTENSIONS};
use goose::agents::mcp_client::McpClientTrait;
use goose::agents::platform_extensions::developer::DeveloperClient;
@@ -67,6 +67,7 @@ struct GooseAcpSession {
pub struct GooseAcpAgent {
sessions: Arc<Mutex<HashMap<String, GooseAcpSession>>>,
provider_factory: ProviderConstructor,
provider: Mutex<Option<Arc<dyn Provider>>>,
builtins: Vec<String>,
client_fs_capabilities: OnceCell<FileSystemCapabilities>,
client_terminal: OnceCell<bool>,
@@ -400,6 +401,7 @@ impl GooseAcpAgent {
Ok(Self {
sessions: Arc::new(Mutex::new(HashMap::new())),
provider_factory,
provider: Mutex::new(None),
builtins,
client_fs_capabilities: OnceCell::new(),
client_terminal: OnceCell::new(),
@@ -882,6 +884,16 @@ impl GooseAcpAgent {
Self::add_mcp_extensions(&agent, args.mcp_servers, &goose_session.id).await?;
// Pre-populate the downstream session so build_model_state
// reads cached model info instead of creating a throwaway session.
if let Ok(acp) = provider.clone().downcast_arc::<AcpProvider>() {
acp.ensure_session(Some(&goose_session.id))
.await
.map_err(|e| {
sacp::Error::internal_error().data(format!("Failed to ensure session: {}", e))
})?;
}
let session = GooseAcpSession {
agent,
messages: Conversation::new_unvalidated(Vec::new()),
@@ -909,6 +921,11 @@ impl GooseAcpAgent {
}
async fn init_provider(&self, agent: &Agent, session: &Session) -> Result<Arc<dyn Provider>> {
let mut cached = self.provider.lock().await;
if let Some(provider) = cached.as_ref() {
agent.update_provider(provider.clone(), &session.id).await?;
return Ok(provider.clone());
}
let model_config = match &session.model_config {
Some(config) => config.clone(),
None => {
@@ -921,6 +938,7 @@ impl GooseAcpAgent {
};
let provider = (self.provider_factory)(model_config, Vec::new()).await?;
agent.update_provider(provider.clone(), &session.id).await?;
*cached = Some(provider.clone());
Ok(provider)
}
@@ -1216,6 +1234,8 @@ impl GooseAcpAgent {
sacp::Error::internal_error().data(format!("Failed to create provider: {}", e))
})?;
*self.provider.lock().await = Some(provider.clone());
let agent = self.get_session_agent(session_id, None).await?;
agent
.update_provider(provider, session_id)
crates/goose/Cargo.toml
+1
View File
@@ -61,6 +61,7 @@ native-tls = [
workspace = true
[dependencies]
downcast-rs = "2.0.2"
lru = { workspace = true }
rmcp = { workspace = true, features = [
"client",
crates/goose/src/acp/provider.rs
+259 -37
View File
@@ -1,3 +1,4 @@
use crate::config::search_path::SearchPaths;
use agent_client_protocol_schema::AGENT_METHOD_NAMES;
use anyhow::{Context, Result};
use async_stream::try_stream;
@@ -27,10 +28,12 @@ use tokio_util::compat::{TokioAsyncReadCompatExt, TokioAsyncWriteCompatExt};
use crate::acp::{map_permission_response, PermissionDecision, PermissionMapping};
use crate::config::{ExtensionConfig, GooseMode};
use crate::conversation::message::{Message, MessageContent};
use crate::conversation::Conversation;
use crate::model::ModelConfig;
use crate::permission::permission_confirmation::PrincipalType;
use crate::permission::{Permission, PermissionConfirmation};
use crate::providers::base::{MessageStream, PermissionRouting, Provider};
use crate::providers::cli_common::generate_simple_session_description;
use crate::providers::errors::ProviderError;
use crate::subprocess::configure_subprocess;
@@ -38,7 +41,7 @@ use crate::subprocess::configure_subprocess;
pub const ACP_CURRENT_MODEL: &str = "current";
pub struct AcpProviderConfig {
pub command: PathBuf,
pub command: String,
pub args: Vec<String>,
pub env: Vec<(String, String)>,
pub env_remove: Vec<String>,
@@ -121,7 +124,7 @@ enum AcpUpdate {
pub struct AcpProvider {
name: String,
model: ModelConfig,
model: Mutex<ModelConfig>,
goose_mode: Arc<Mutex<GooseMode>>,
tx: Option<mpsc::Sender<ClientRequest>>,
loop_thread: Option<JoinHandle<()>>,
@@ -136,7 +139,11 @@ pub struct AcpProvider {
session_model: Arc<TokioMutex<HashMap<String, String>>>,
auth_methods: Vec<AuthMethod>,
supports_close: bool,
supports_list: bool,
// Cached NewSessionResponse for model list and config resolution.
// Populated from whichever comes first: ensure_session or get_init_session.
init_session: OnceCell<NewSessionResponse>,
session_titles: Arc<TokioMutex<HashMap<String, String>>>,
}
impl std::fmt::Debug for AcpProvider {
@@ -214,6 +221,8 @@ impl AcpProvider {
let permission_mapping = config.permission_mapping.clone();
let rejected_tool_calls = Arc::new(TokioMutex::new(HashSet::new()));
let goose_mode = Arc::new(Mutex::new(goose_mode));
let session_titles = Arc::new(TokioMutex::new(HashMap::new()));
let acp_to_goose_id = Arc::new(TokioMutex::new(HashMap::new()));
let client_loop = AcpClientLoop::new(config, goose_mode.clone());
let loop_thread = spawn_client_loop(run(client_loop, rx, init_tx));
@@ -221,12 +230,10 @@ impl AcpProvider {
.await
.context("ACP client initialization cancelled")??;
let supports_close = init_response
.agent_capabilities
.session_capabilities
.close
.is_some();
let mut provider = Self::new_with_runtime(
let session_caps = &init_response.agent_capabilities.session_capabilities;
let supports_close = session_caps.close.is_some();
let supports_list = session_caps.list.is_some();
let provider = Self::new_with_runtime(
name,
model,
goose_mode,
@@ -237,13 +244,10 @@ impl AcpProvider {
rejected_tool_calls,
init_response.auth_methods,
supports_close,
supports_list,
session_titles,
acp_to_goose_id,
);
if provider.model.model_name == ACP_CURRENT_MODEL {
let response = provider.get_init_session().await?;
let (current_model, _) = resolve_model_info(&provider.name, response)?;
tracing::info!(from = ACP_CURRENT_MODEL, to = %current_model, "resolved ACP model");
provider.model.model_name = current_model;
}
Ok(provider)
}
@@ -259,10 +263,13 @@ impl AcpProvider {
rejected_tool_calls: Arc<TokioMutex<HashSet<String>>>,
auth_methods: Vec<AuthMethod>,
supports_close: bool,
supports_list: bool,
session_titles: Arc<TokioMutex<HashMap<String, String>>>,
acp_to_goose_id: Arc<TokioMutex<HashMap<String, String>>>,
) -> Self {
Self {
name,
model,
model: Mutex::new(model),
goose_mode,
tx: Some(tx),
loop_thread: Some(loop_thread),
@@ -271,11 +278,13 @@ impl AcpProvider {
rejected_tool_calls,
pending_confirmations: Arc::new(TokioMutex::new(HashMap::new())),
goose_to_acp_id: Arc::new(TokioMutex::new(HashMap::new())),
acp_to_goose_id: Arc::new(TokioMutex::new(HashMap::new())),
acp_to_goose_id,
session_model: Arc::new(TokioMutex::new(HashMap::new())),
auth_methods,
supports_close,
supports_list,
init_session: OnceCell::new(),
session_titles,
}
}
@@ -472,7 +481,10 @@ impl AcpProvider {
.lock()
.await
.entry(session_id.to_string())
.or_insert(current_model);
.or_insert(current_model.clone());
self.cache_session_metadata(response.clone(), current_model)
.await;
}
Ok(response)
@@ -497,6 +509,14 @@ impl AcpProvider {
Ok(response_rx)
}
async fn cache_session_metadata(&self, response: NewSessionResponse, model: String) {
self.init_session.get_or_init(|| async { response }).await;
let mut m = self.model.lock().unwrap();
if m.model_name == ACP_CURRENT_MODEL {
m.model_name = model;
}
}
async fn get_init_session(&self) -> Result<&NewSessionResponse> {
self.init_session
.get_or_try_init(|| async {
@@ -505,6 +525,12 @@ impl AcpProvider {
self.close_session_by_acp_id(response.session_id.clone())
.await?;
}
let (model, _) =
resolve_model_info(&self.name, &response).map_err(anyhow::Error::from)?;
let mut m = self.model.lock().unwrap();
if m.model_name == ACP_CURRENT_MODEL {
m.model_name = model;
}
Ok(response)
})
.await
@@ -532,7 +558,45 @@ impl Provider for AcpProvider {
}
fn get_model_config(&self) -> ModelConfig {
self.model.clone()
self.model.lock().unwrap().clone()
}
async fn generate_session_name(
&self,
session_id: &str,
messages: &Conversation,
) -> Result<String, ProviderError> {
// No downstream session yet; nothing to name.
if self.goose_to_acp_id.lock().await.is_empty() {
return Ok(String::new());
}
if self.supports_list {
if let Some(title) = self.session_titles.lock().await.get(session_id) {
return Ok(title.clone());
}
// Fall back to session/list (returns goose-remapped IDs).
// TODO: replace with session/get when available
// https://github.com/agentclientprotocol/agent-client-protocol/discussions/60
if let Ok(list) = self.list_sessions().await {
let mut titles = self.session_titles.lock().await;
if let Some(title) = resolve_title_from_list(&list, &mut titles, session_id) {
return Ok(title);
}
}
// No title yet. system_generated_name skips empty, maybe_update_name retries.
return Ok(String::new());
}
// No list capability: use first user message as title.
// https://github.com/google-gemini/gemini-cli/issues/22919
let (msg, _) = generate_simple_session_description(
&self.model.lock().unwrap().model_name,
messages.messages(),
)?;
Ok(msg.as_concat_text())
}
async fn update_mode(&self, session_id: &str, mode: GooseMode) -> Result<(), ProviderError> {
@@ -570,6 +634,11 @@ impl Provider for AcpProvider {
Ok(())
}
// Agentic providers own their own metadata.
fn skip_moim(&self) -> bool {
true
}
fn permission_routing(&self) -> PermissionRouting {
PermissionRouting::ActionRequired
}
@@ -592,11 +661,11 @@ impl Provider for AcpProvider {
) -> Result<MessageStream, ProviderError> {
let response = self.ensure_session(Some(session_id)).await?;
// Provider trait has no update_model — stream() is the only place to forward model changes.
// Forward model changes to the agent. Skip "current" (resolved lazily by ensure_session).
{
let new_model = &model_config.model_name;
let tracked = self.session_model.lock().await.get(session_id).cloned();
if tracked.as_deref() != Some(new_model) {
if new_model != ACP_CURRENT_MODEL && tracked.as_deref() != Some(new_model) {
if self
.session_has_config_option(session_id, SessionConfigOptionCategory::Model)
.await
@@ -807,6 +876,7 @@ impl AcpClientLoop {
{
let prompt_response_tx = prompt_response_tx.clone();
let reverse_modes = reverse_modes.clone();
let goose_mode = goose_mode.clone();
async move |notification: SessionNotification, _cx| {
if let Some(ref cb) = notification_callback {
cb(notification.clone());
@@ -915,7 +985,7 @@ impl AcpClientLoop {
sacp::on_receive_request!(),
)
.connect_with(transport, async move |cx: ConnectionTo<Agent>| {
handle_requests(config, cx, rx, prompt_response_tx, init_tx).await
handle_requests(config, goose_mode, cx, rx, prompt_response_tx, init_tx).await
})
.await?;
@@ -924,7 +994,9 @@ impl AcpClientLoop {
}
async fn spawn_acp_process(config: &AcpProviderConfig) -> Result<Child> {
let mut cmd = Command::new(&config.command);
// with_npm() includes npm global bin dir (desktop app PATH may not)
let resolved = SearchPaths::builder().with_npm().resolve(&config.command)?;
let mut cmd = Command::new(&resolved);
cmd.args(&config.args)
.stdin(Stdio::piped())
.stdout(Stdio::piped())
@@ -952,6 +1024,7 @@ fn log_undelivered<E: std::fmt::Debug>(result: Result<(), E>, method: &str) {
async fn handle_requests(
config: AcpProviderConfig,
goose_mode: Arc<Mutex<GooseMode>>,
cx: ConnectionTo<Agent>,
rx: &mut mpsc::Receiver<ClientRequest>,
prompt_response_tx: Arc<Mutex<Option<mpsc::Sender<AcpUpdate>>>>,
@@ -997,13 +1070,18 @@ async fn handle_requests(
let result = match session {
Ok(session) => {
session_ids.push(session.session_id.clone());
apply_session_mode(&config, &cx, session).await
let mode_id = goose_mode
.lock()
.ok()
.and_then(|m| config.mode_mapping.get(&*m).cloned());
match mode_id {
Some(id) => apply_session_mode(id, &cx, session).await,
None => Err(anyhow::anyhow!("mode_mapping lookup failed")),
}
}
Err(err) => Err(anyhow::anyhow!(
"ACP {} failed: {err}",
AGENT_METHOD_NAMES.session_new
)),
};
Err(err) => Err(anyhow::anyhow!("{err}")),
}
.map_err(|e| anyhow::anyhow!("ACP {} failed: {e}", AGENT_METHOD_NAMES.session_new));
log_undelivered(response_tx.send(result), AGENT_METHOD_NAMES.session_new);
}
ClientRequest::ListSessions { response_tx } => {
@@ -1143,29 +1221,29 @@ async fn handle_requests(
}
async fn apply_session_mode(
config: &AcpProviderConfig,
desired_mode_id: String,
cx: &ConnectionTo<Agent>,
session: NewSessionResponse,
) -> Result<NewSessionResponse> {
if let (Some(mode_id), Some(modes)) = (config.session_mode_id.clone(), session.modes.as_ref()) {
if modes.current_mode_id.0.as_ref() != mode_id.as_str() {
if let Some(modes) = session.modes.as_ref() {
if modes.current_mode_id.0.as_ref() != desired_mode_id.as_str() {
let available: Vec<String> = modes
.available_modes
.iter()
.map(|mode| mode.id.0.to_string())
.collect();
if !available.iter().any(|id| id == &mode_id) {
if !available.iter().any(|id| id == &desired_mode_id) {
return Err(anyhow::anyhow!(
"Requested mode '{}' not offered by agent. Available modes: {}",
mode_id,
desired_mode_id,
available.join(", ")
));
}
let _: SetSessionModeResponse = cx
.send_request(SetSessionModeRequest::new(
session.session_id.clone(),
mode_id,
desired_mode_id,
))
.block_task()
.await
@@ -1398,6 +1476,22 @@ fn resolve_mode(
}
}
// Caches titles from session/list and returns the title for session_id if found.
fn resolve_title_from_list(
response: &ListSessionsResponse,
titles: &mut HashMap<String, String>,
session_id: &str,
) -> Option<String> {
for info in &response.sessions {
if let Some(ref title) = info.title {
titles
.entry(info.session_id.0.to_string())
.or_insert_with(|| title.clone());
}
}
titles.get(session_id).cloned()
}
fn permission_decision_from_mode(goose_mode: GooseMode) -> Option<PermissionDecision> {
match goose_mode {
GooseMode::Auto => Some(PermissionDecision::AllowOnce),
@@ -1406,8 +1500,8 @@ fn permission_decision_from_mode(goose_mode: GooseMode) -> Option<PermissionDeci
}
}
// TODO: ID mapping is in-memory only — sessions from prior runs or other clients are dropped.
// Persisting requires a schema change to map goose↔ACP IDs in the session DB.
// TODO: ID mapping is in-memory only. Sessions from prior runs or other clients are dropped.
// Persistence is a non-goal as we want to change to honoring agent-supplied IDs.
fn map_sessions_to_goose_ids(
response: ListSessionsResponse,
acp_to_goose: &HashMap<String, String>,
@@ -1428,7 +1522,10 @@ fn map_sessions_to_goose_ids(
mod tests {
use super::*;
use crate::agents::extension::Envs;
use sacp::schema::{SessionConfigOption, SessionConfigSelectOption, SessionInfo};
use sacp::schema::{
AgentCapabilities, SessionCapabilities, SessionConfigOption, SessionConfigSelectOption,
SessionInfo, SessionListCapabilities,
};
use test_case::test_case;
#[test_case(
@@ -1743,4 +1840,129 @@ mod tests {
assert_eq!(result, expected);
}
}
#[test_case(
vec![SessionInfo::new(SessionId::new("s1"), "/tmp").title("Hello")],
HashMap::new(),
"s1",
Some("Hello".to_string())
; "returns matching title"
)]
#[test_case(
vec![SessionInfo::new(SessionId::new("s1"), "/tmp").title("Hello")],
HashMap::new(),
"s2",
None
; "returns none for missing session"
)]
#[test_case(
vec![SessionInfo::new(SessionId::new("s1"), "/tmp")],
HashMap::new(),
"s1",
None
; "returns none when session has no title"
)]
#[test_case(
vec![SessionInfo::new(SessionId::new("s1"), "/tmp").title("From list")],
HashMap::from([("s1".to_string(), "From notification".to_string())]),
"s1",
Some("From notification".to_string())
; "existing title not overwritten by list"
)]
fn test_resolve_title_from_list(
sessions: Vec<SessionInfo>,
mut existing: HashMap<String, String>,
session_id: &str,
expected: Option<String>,
) {
let result = resolve_title_from_list(
&ListSessionsResponse::new(sessions),
&mut existing,
session_id,
);
assert_eq!(result, expected);
}
#[test_case(
HashMap::new(),
SessionCapabilities::new().list(SessionListCapabilities::new()),
HashMap::new(),
"s1", Conversation::new_unvalidated(vec![Message::user().with_text("hello")]),
Ok("".to_string())
; "no downstream session returns empty"
)]
#[test_case(
HashMap::from([("s1".to_string(), NewSessionResponse::new(SessionId::new("acp-1")))]),
SessionCapabilities::new().list(SessionListCapabilities::new()),
HashMap::from([("s1".to_string(), "cached title".to_string())]),
"s1", Conversation::new_unvalidated(vec![]),
Ok("cached title".to_string())
; "returns cached session title"
)]
#[test_case(
HashMap::from([("s1".to_string(), NewSessionResponse::new(SessionId::new("acp-1")))]),
SessionCapabilities::new(),
HashMap::new(),
"s1", Conversation::new_unvalidated(vec![Message::user().with_text("say hello")]),
Ok("say hello".to_string())
; "no list uses first user message"
)]
#[test_case(
HashMap::from([("s1".to_string(), NewSessionResponse::new(SessionId::new("acp-1")))]),
SessionCapabilities::new(),
HashMap::new(),
"s1", Conversation::new_unvalidated(vec![]),
Ok("Simple task".to_string())
; "no list no messages returns simple task"
)]
#[test_case(
HashMap::from([("s1".to_string(), NewSessionResponse::new(SessionId::new("acp-1")))]),
SessionCapabilities::new(),
HashMap::new(),
"s1", Conversation::new_unvalidated(vec![Message::user().with_text("one two three four five six")]),
Ok("one two three four".to_string())
; "no list truncates to 4 words"
)]
#[tokio::test]
async fn test_generate_session_name(
goose_to_acp: HashMap<String, NewSessionResponse>,
session_caps: SessionCapabilities,
session_titles: HashMap<String, String>,
session_id: &str,
conv: Conversation,
expected: Result<String, ProviderError>,
) {
let init_response = InitializeResponse::new(ProtocolVersion::LATEST)
.agent_capabilities(AgentCapabilities::new().session_capabilities(session_caps));
let provider = AcpProvider::start(
"test".into(),
ModelConfig::new("test-model").unwrap(),
GooseMode::Auto,
AcpProviderConfig {
command: "true".to_string(),
args: vec![],
env: vec![],
env_remove: vec![],
work_dir: PathBuf::from("."),
mcp_servers: vec![],
session_mode_id: None,
mode_mapping: HashMap::new(),
permission_mapping: PermissionMapping::default(),
notification_callback: None,
},
Box::new(move |_cl, _rx, init_tx| {
Box::pin(async move {
let _ = init_tx.send(Ok(init_response));
})
}),
)
.await
.unwrap();
*provider.goose_to_acp_id.lock().await = goose_to_acp;
*provider.session_titles.lock().await = session_titles;
assert_eq!(
provider.generate_session_name(session_id, &conv).await,
expected
);
}
}
crates/goose/src/agents/agent.rs
+13 -8
View File
@@ -1255,18 +1255,23 @@ impl Agent {
break;
}
let conversation_with_moim = super::moim::inject_moim(
&session_config.id,
conversation.clone(),
&self.extension_manager,
&working_dir,
).await;
let provider = self.provider().await?;
let messages = if provider.skip_moim() {
conversation.clone()
} else {
super::moim::inject_moim(
&session_config.id,
conversation.clone(),
&self.extension_manager,
&working_dir,
).await
};
let mut stream = Self::stream_response_from_provider(
self.provider().await?,
provider,
&session_config.id,
&system_prompt,
conversation_with_moim.messages(),
messages.messages(),
&tools,
&toolshim_tools,
).await?;
crates/goose/src/providers/base.rs
+7 -1
View File
@@ -1,5 +1,6 @@
use anyhow::Result;
use async_trait::async_trait;
use downcast_rs::{impl_downcast, DowncastSync};
use futures::future::BoxFuture;
use futures::Stream;
use serde::{Deserialize, Serialize};
@@ -502,7 +503,7 @@ pub enum PermissionRouting {
/// Base trait for AI providers (OpenAI, Anthropic, etc)
#[async_trait]
pub trait Provider: Send + Sync {
pub trait Provider: DowncastSync {
/// Get the name of this provider instance
fn get_name(&self) -> &str;
@@ -588,6 +589,10 @@ pub trait Provider: Send + Sync {
false
}
fn skip_moim(&self) -> bool {
false
}
/// Fetch models filtered by canonical registry and usability
async fn fetch_recommended_models(&self) -> Result<Vec<String>, ProviderError> {
let all_models = self.fetch_supported_models().await?;
@@ -783,6 +788,7 @@ pub trait Provider: Send + Sync {
false
}
}
impl_downcast!(sync Provider);
/// A message stream yields partial text content but complete tool calls, all within the Message object
/// So a message with text will contain potentially just a word of a longer response, but tool calls
crates/goose/src/providers/claude_acp.rs
+1 -6
View File
@@ -7,7 +7,6 @@ use crate::acp::{
extension_configs_to_mcp_servers, AcpProvider, AcpProviderConfig, PermissionMapping,
ACP_CURRENT_MODEL,
};
use crate::config::search_path::SearchPaths;
use crate::config::{Config, GooseMode};
use crate::model::ModelConfig;
use crate::providers::base::{ProviderDef, ProviderMetadata};
@@ -45,10 +44,6 @@ impl ProviderDef for ClaudeAcpProvider {
) -> BoxFuture<'static, Result<AcpProvider>> {
Box::pin(async move {
let config = Config::global();
// with_npm() includes npm global bin dir (desktop app PATH may not)
let resolved_command = SearchPaths::builder()
.with_npm()
.resolve(CLAUDE_ACP_BINARY)?;
let goose_mode = config.get_goose_mode().unwrap_or(GooseMode::Auto);
// claude-agent-acp permission option_ids
@@ -70,7 +65,7 @@ impl ProviderDef for ClaudeAcpProvider {
]);
let provider_config = AcpProviderConfig {
command: resolved_command,
command: CLAUDE_ACP_BINARY.to_string(),
args: vec![],
env: vec![],
// Prevent nested-session detection in claude-agent-acp (wraps Claude Code)
crates/goose/src/providers/claude_code.rs
+4
View File
@@ -689,6 +689,10 @@ impl Provider for ClaudeCodeProvider {
Ok(())
}
fn skip_moim(&self) -> bool {
true
}
fn permission_routing(&self) -> PermissionRouting {
PermissionRouting::ActionRequired
}
crates/goose/src/providers/codex.rs
+4
View File
@@ -730,6 +730,10 @@ impl Provider for CodexProvider {
))
}
fn skip_moim(&self) -> bool {
true
}
async fn update_mode(&self, session_id: &str, mode: GooseMode) -> Result<(), ProviderError> {
self.mode_by_session
.write()
crates/goose/src/providers/codex_acp.rs
+9 -55
View File
@@ -7,7 +7,6 @@ use crate::acp::{
extension_configs_to_mcp_servers, AcpProvider, AcpProviderConfig, PermissionMapping,
ACP_CURRENT_MODEL,
};
use crate::config::search_path::SearchPaths;
use crate::config::{Config, GooseMode};
use crate::model::ModelConfig;
use crate::providers::base::{ProviderDef, ProviderMetadata};
@@ -44,36 +43,20 @@ impl ProviderDef for CodexAcpProvider {
) -> BoxFuture<'static, Result<AcpProvider>> {
Box::pin(async move {
let config = Config::global();
// with_npm() includes npm global bin dir (desktop app PATH may not)
let resolved_command = SearchPaths::builder()
.with_npm()
.resolve(CODEX_ACP_PROVIDER_NAME)?;
let work_dir = std::env::current_dir().unwrap_or_else(|_| PathBuf::from("."));
let env = vec![];
let goose_mode = config.get_goose_mode().unwrap_or(GooseMode::Auto);
let mcp_servers = extension_configs_to_mcp_servers(&extensions);
// fixed goose mode via -c overrides until session/set-mode works
let (approval_policy, sandbox_mode) = map_goose_mode(goose_mode);
let mut args = vec![
// Seed read-only so modes() returns a preset match and session/new
// includes modes. session_mode_id upgrades to the real mode.
// https://github.com/zed-industries/codex-acp/issues/221
let args = vec![
"-c".to_string(),
format!("approval_policy={approval_policy}"),
"sandbox_mode=read-only".to_string(),
"-c".to_string(),
format!("sandbox_mode={sandbox_mode}"),
"sandbox_workspace_write.network_access=true".to_string(),
];
// Codex sandbox blocks network by default. Enable it when HTTP MCP
// servers are configured so codex-acp can connect to them.
let has_http_mcp = mcp_servers
.iter()
.any(|s| matches!(s, sacp::schema::McpServer::Http(_)));
if has_http_mcp {
args.extend([
"-c".to_string(),
"sandbox_workspace_write.network_access=true".to_string(),
]);
}
// codex-acp permission option_ids
let permission_mapping = PermissionMapping {
allow_option_id: Some("approved".to_string()),
@@ -90,14 +73,13 @@ impl ProviderDef for CodexAcpProvider {
]);
let provider_config = AcpProviderConfig {
command: resolved_command,
command: CODEX_ACP_PROVIDER_NAME.to_string(),
args,
env,
env: vec![],
env_remove: vec![],
work_dir,
mcp_servers,
// Disabled until https://github.com/zed-industries/codex-acp/issues/179 is fixed.
session_mode_id: None,
session_mode_id: Some(mode_mapping[&goose_mode].clone()),
mode_mapping,
permission_mapping,
notification_callback: None,
@@ -108,31 +90,3 @@ impl ProviderDef for CodexAcpProvider {
})
}
}
// Codex sandbox scope determines what needs approval: operations within the
// sandbox are auto-approved, operations outside it trigger on-request prompts.
// So Approve uses read-only sandbox to force write approvals through goose.
fn map_goose_mode(goose_mode: GooseMode) -> (&'static str, &'static str) {
match goose_mode {
GooseMode::Auto => ("never", "danger-full-access"),
GooseMode::SmartApprove => ("on-request", "workspace-write"),
GooseMode::Approve => ("on-request", "read-only"),
GooseMode::Chat => ("never", "read-only"),
}
}
#[cfg(test)]
mod tests {
use super::*;
use test_case::test_case;
#[test_case(GooseMode::Auto, "never", "danger-full-access")]
#[test_case(GooseMode::SmartApprove, "on-request", "workspace-write")]
#[test_case(GooseMode::Approve, "on-request", "read-only")]
#[test_case(GooseMode::Chat, "never", "read-only")]
fn test_map_goose_mode(mode: GooseMode, expected_approval: &str, expected_sandbox: &str) {
let (approval, sandbox) = map_goose_mode(mode);
assert_eq!(approval, expected_approval);
assert_eq!(sandbox, expected_sandbox);
}
}
crates/goose/src/providers/copilot_acp.rs
+1 -6
View File
@@ -7,7 +7,6 @@ use crate::acp::{
extension_configs_to_mcp_servers, AcpProvider, AcpProviderConfig, PermissionMapping,
ACP_CURRENT_MODEL,
};
use crate::config::search_path::SearchPaths;
use crate::config::{Config, GooseMode};
use crate::model::ModelConfig;
use crate::providers::base::{ProviderDef, ProviderMetadata};
@@ -48,10 +47,6 @@ impl ProviderDef for CopilotAcpProvider {
) -> BoxFuture<'static, Result<AcpProvider>> {
Box::pin(async move {
let config = Config::global();
// with_npm() includes npm global bin dir (desktop app PATH may not)
let resolved_command = SearchPaths::builder()
.with_npm()
.resolve(COPILOT_ACP_BINARY)?;
let goose_mode = config.get_goose_mode().unwrap_or(GooseMode::Auto);
// Copilot uses standard ACP permission option IDs (allow_once,
@@ -74,7 +69,7 @@ impl ProviderDef for CopilotAcpProvider {
]);
let provider_config = AcpProviderConfig {
command: resolved_command,
command: COPILOT_ACP_BINARY.to_string(),
args,
env: vec![],
env_remove: vec![],
crates/goose/tests/providers.rs
+45 -4
View File
@@ -5,6 +5,7 @@ use goose::acp::ACP_CURRENT_MODEL;
use goose::agents::{Agent, AgentConfig, AgentEvent, GoosePlatform, PromptManager, SessionConfig};
use goose::config::{ExtensionConfig, GooseMode, PermissionManager};
use goose::conversation::message::{ActionRequiredData, Message, MessageContent};
use goose::conversation::Conversation;
use goose::permission::permission_confirmation::PrincipalType;
use goose::permission::{Permission, PermissionConfirmation};
use goose::providers::anthropic::ANTHROPIC_DEFAULT_MODEL;
@@ -182,6 +183,11 @@ impl ProviderTestConfig {
self
}
fn test_mode_update(mut self, v: bool) -> Self {
self.test_mode_update = v;
self
}
fn expect_context_length_exceeded(mut self, v: bool) -> Self {
self.expect_context_length_exceeded = v;
self
@@ -198,7 +204,6 @@ impl ProviderTestConfig {
skip,
expected_session_id: || Arc::new(IgnoreSessionId),
test_smart_approve: false,
test_mode_update: false,
test_context_length_exceeded: false,
..Self::with_llm_provider(name, model_name, &[])
}
@@ -605,16 +610,46 @@ impl ProviderFixture {
self.agent
.update_goose_mode(GooseMode::Approve, &self.session_id)
.await?;
// Verify tool call now requires permission (ActionRequired).
// Cancel prevents the task from completing → tool fails.
// Use a write prompt so ACP agents trigger sandbox permission requests.
let test_file = tempfile::NamedTempFile::new()?;
self.run_permission_test(
Permission::Cancel,
true,
"Use the get_code tool and output only its result.",
&format!("Write the word 'hello' to {}", test_file.path().display()),
"mode_update",
)
.await
}
async fn test_session_naming(&self) -> Result<()> {
let model_config = self.provider.get_model_config();
let mut conv = Conversation::default();
let prompts = ["say hello", "what is 1+1", "say goodbye"];
for prompt in &prompts {
let user_msg = Message::user().with_text(*prompt);
conv.push(user_msg.clone());
let (response, _) = self
.provider
.complete(
&model_config,
&self.session_id,
"You are a helpful assistant.",
conv.messages(),
&[],
)
.await?;
conv.push(response);
let name = self
.provider
.generate_session_name(&self.session_id, &conv)
.await?;
if !name.is_empty() {
println!("=== {}::session_naming === {}", self.name, name);
return Ok(());
}
}
panic!("{}: session name not set after 3 prompts", self.name);
}
}
fn load_env() {
@@ -703,6 +738,10 @@ async fn test_provider(config: ProviderTestConfig) -> Result<()> {
if config.test_mode_update {
run_test(GooseMode::Auto).await?.test_mode_update().await?;
}
run_test(GooseMode::Auto)
.await?
.test_session_naming()
.await?;
Ok(())
}
.await;
@@ -871,6 +910,7 @@ async fn test_xai_provider() -> Result<()> {
async fn test_claude_code_provider() -> Result<()> {
ProviderTestConfig::with_agentic_provider("claude-code", CLAUDE_CODE_DEFAULT_MODEL, "claude")
.model_switch_name("sonnet")
.test_mode_update(false)
.run()
.await
}
@@ -879,6 +919,7 @@ async fn test_claude_code_provider() -> Result<()> {
async fn test_codex_provider() -> Result<()> {
ProviderTestConfig::with_agentic_provider("codex", CODEX_DEFAULT_MODEL, "codex")
.test_permissions(false)
.test_mode_update(false)
.run()
.await
}