diff --git a/crates/goose/src/acp/server/providers.rs b/crates/goose/src/acp/server/providers.rs index 4fd4ad913a..a7b403069f 100644 --- a/crates/goose/src/acp/server/providers.rs +++ b/crates/goose/src/acp/server/providers.rs @@ -415,6 +415,40 @@ fn refresh_plan_to_response(refresh_plan: RefreshPlan) -> RefreshProviderInvento } impl GooseAcpAgent { + async fn active_session_uses_provider(&self, provider_id: &str) -> Result { + let active_sessions = { + let sessions = self.sessions.lock().await; + sessions + .iter() + .map(|(thread_id, session)| { + (thread_id.clone(), session.internal_session_id.clone()) + }) + .collect::>() + }; + + for (thread_id, internal_session_id) in active_sessions { + let session = self + .session_manager + .get_session(&internal_session_id, false) + .await + .internal_err_ctx("Failed to check active session provider")?; + if session.provider_name.as_deref() == Some(provider_id) { + return Ok(true); + } + + let thread = self + .thread_manager + .get_thread(&thread_id) + .await + .internal_err_ctx("Failed to check active thread provider")?; + if thread.metadata.provider_id.as_deref() == Some(provider_id) { + return Ok(true); + } + } + + Ok(false) + } + pub(super) async fn on_list_providers( &self, req: ListProvidersRequest, @@ -607,6 +641,7 @@ impl GooseAcpAgent { .ok() .as_deref() == Some(req.provider_id.as_str()) + || self.active_session_uses_provider(&req.provider_id).await? { return Err(sacp::Error::invalid_params().data(format!( "Cannot delete active provider: {}", diff --git a/crates/goose/src/sources.rs b/crates/goose/src/sources.rs index 1560d4a1a5..654fd4dad3 100644 --- a/crates/goose/src/sources.rs +++ b/crates/goose/src/sources.rs @@ -18,7 +18,7 @@ use tracing::warn; pub fn parse_frontmatter Deserialize<'de>>( content: &str, ) -> Result, serde_yaml::Error> { - let content = content.trim_start(); + let content = trim_frontmatter_start(content); let mut lines = content.lines(); if !lines.next().is_some_and(is_frontmatter_delimiter) { return Ok(None); @@ -38,6 +38,10 @@ pub fn parse_frontmatter Deserialize<'de>>( Ok(None) } +fn trim_frontmatter_start(content: &str) -> &str { + content.trim_start_matches(|ch: char| ch.is_whitespace() || ch == '\u{feff}') +} + fn is_frontmatter_delimiter(line: &str) -> bool { line.trim_end_matches('\r') == "---" } @@ -144,13 +148,11 @@ pub(crate) struct AgentFrontmatter { } pub(crate) fn parse_agent_markdown(raw: &str) -> Result, Error> { - if !raw.trim_start().starts_with("---") { - return Ok(None); - } - - let (mut frontmatter, body): (Mapping, String) = parse_frontmatter::(raw) + let Some((mut frontmatter, body)) = parse_frontmatter::(raw) .map_err(|e| Error::invalid_params().data(format!("Invalid agent frontmatter: {e}")))? - .ok_or_else(|| Error::invalid_params().data("Agent file is missing frontmatter"))?; + else { + return Ok(None); + }; let name = remove_string_key(&mut frontmatter, "name").unwrap_or_default(); if name.trim().is_empty() { @@ -475,7 +477,10 @@ fn scan_agents_from_dir( let mut sources = Vec::new(); for entry in entries.flatten() { let path = entry.path(); - if !path.is_file() || path.extension().and_then(|ext| ext.to_str()) != Some("md") { + let Ok(file_type) = entry.file_type() else { + continue; + }; + if !file_type.is_file() || path.extension().and_then(|ext| ext.to_str()) != Some("md") { continue; } @@ -808,6 +813,16 @@ mod tests { assert_eq!(parsed.body, "body"); } + #[test] + fn parse_frontmatter_accepts_bom_prefixed_delimiter() { + let raw = "\u{feff}---\nname: Bom Agent\ndescription: bom\n---\nbody\n"; + let parsed = parse_agent_markdown(raw).unwrap().unwrap(); + + assert_eq!(parsed.frontmatter.name, "Bom Agent"); + assert_eq!(parsed.frontmatter.description.as_deref(), Some("bom")); + assert_eq!(parsed.body, "body"); + } + #[test] fn create_list_update_delete_project_skill() { let tmp = TempDir::new().unwrap(); @@ -1234,6 +1249,28 @@ mod tests { let listed = list_sources(Some(SourceType::Agent), Some(project_dir)).unwrap(); assert!(!listed.iter().any(|source| source.name == "Nested Agent")); + #[cfg(unix)] + { + use std::os::unix::fs::symlink; + + let outside_agent = project.path().join("outside-agent.md"); + std::fs::write( + &outside_agent, + "---\nname: Symlink Agent\ndescription: linked\n---\nlinked body\n", + ) + .unwrap(); + let symlinked_agent = project + .path() + .join(".agents") + .join("agents") + .join("symlink-agent.md"); + std::fs::create_dir_all(symlinked_agent.parent().unwrap()).unwrap(); + symlink(&outside_agent, &symlinked_agent).unwrap(); + + let listed = list_sources(Some(SourceType::Agent), Some(project_dir)).unwrap(); + assert!(!listed.iter().any(|source| source.name == "Symlink Agent")); + } + let missing = project .path() .join(".agents") diff --git a/crates/goose/tests/acp_custom_requests_test.rs b/crates/goose/tests/acp_custom_requests_test.rs index 1c1dccd9b3..31b65d4f60 100644 --- a/crates/goose/tests/acp_custom_requests_test.rs +++ b/crates/goose/tests/acp_custom_requests_test.rs @@ -146,98 +146,6 @@ fn test_custom_list_builtin_skill_sources() { }); } -#[test] -fn test_custom_agent_sources_crud() { - let home = tempfile::tempdir().unwrap(); - let _env = env_lock::lock_env([("HOME", Some(home.path().to_str().unwrap()))]); - let project = tempfile::tempdir().unwrap(); - let project_dir = project.path().to_string_lossy().to_string(); - - run_test(async move { - let openai = OpenAiFixture::new(vec![], Arc::new(EnforceSessionId::default())).await; - let conn = AcpServerConnection::new(TestConnectionConfig::default(), openai).await; - - let created = send_custom( - conn.cx(), - "_goose/sources/create", - serde_json::json!({ - "type": "agent", - "name": "ACP Agent", - "description": "created through ACP", - "content": "agent instructions", - "metadata": { - "model": "gpt-4o", - "temperature": 0.1 - }, - "global": false, - "projectDir": project_dir - }), - ) - .await - .expect("agent create should succeed"); - let path = created - .pointer("/source/directory") - .and_then(|value| value.as_str()) - .expect("created source should include a path") - .to_string(); - assert!(path.ends_with(".agents/agents/acp-agent.md")); - - let listed = send_custom( - conn.cx(), - "_goose/sources/list", - serde_json::json!({ - "type": "agent", - "projectDir": project_dir - }), - ) - .await - .expect("agent list should succeed"); - assert_eq!( - listed.pointer("/sources/0/name"), - Some(&serde_json::json!("ACP Agent")) - ); - assert_eq!( - listed.pointer("/sources/0/metadata/model"), - Some(&serde_json::json!("gpt-4o")) - ); - - let updated = send_custom( - conn.cx(), - "_goose/sources/update", - serde_json::json!({ - "type": "agent", - "path": path, - "name": "ACP Agent Renamed", - "description": "updated through ACP", - "content": "updated instructions" - }), - ) - .await - .expect("agent update should succeed"); - let updated_path = updated - .pointer("/source/directory") - .and_then(|value| value.as_str()) - .expect("updated source should include a path") - .to_string(); - assert!(updated_path.ends_with(".agents/agents/acp-agent-renamed.md")); - assert_eq!( - updated.pointer("/source/metadata/model"), - Some(&serde_json::json!("gpt-4o")) - ); - - send_custom( - conn.cx(), - "_goose/sources/delete", - serde_json::json!({ - "type": "agent", - "path": updated_path - }), - ) - .await - .expect("agent delete should succeed"); - }); -} - #[test] fn test_custom_provider_inventory_includes_metadata() { run_test(async {