tkmind/tkmind_go
1
0
Fork 0
Code Issues Pull Requests Actions 4 Packages Projects Releases Wiki Activity

fix(acp): ignores acp provider generated title and uses goose generate title (#11197)

Browse Source
This commit is contained in:
Lifei Zhou
2026-08-13 02:45:24 +00:00
committed by GitHub
parent 9b70cc4774
commit 11deb564d0
4 changed files with 0 additions and 156 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/goose-provider-types/src/base.rs
-3
View File
@@ -5,7 +5,6 @@ use serde::{Deserialize, Serialize};
use serde_json::Value;
use std::collections::HashMap;
use std::pin::Pin;
use std::sync::Arc;
use crate::{
canonical::{map_to_canonical_model, CanonicalModelRegistry},
@@ -591,8 +590,6 @@ pub trait Provider: Send + Sync {
false
}
fn set_session_title_callback(&self, _callback: Arc<dyn Fn(String) + Send + Sync>) {}
/// Configure OAuth authentication for this provider
///
/// This method is called when a provider has configuration keys marked with oauth_flow = true.
crates/goose/src/acp/provider.rs
-58
View File
@@ -149,30 +149,6 @@ struct HandoffContextClaim {
include_context: bool,
}
type SessionTitleCallback = Arc<dyn Fn(String) + Send + Sync>;
#[derive(Clone, Default)]
struct SessionTitlePublisher {
callback: Arc<Mutex<Option<SessionTitleCallback>>>,
}
impl SessionTitlePublisher {
fn set_callback(&self, callback: SessionTitleCallback) {
*self.callback.lock().unwrap() = Some(callback);
}
fn publish(&self, title: &str) {
let title = title.trim();
if title.is_empty() {
return;
}
if let Some(callback) = self.callback.lock().unwrap().clone() {
callback(title.to_string());
}
}
}
pub struct AcpProvider {
name: String,
goose_mode: Arc<Mutex<GooseMode>>,
@@ -189,7 +165,6 @@ pub struct AcpProvider {
/// in which case `get_context_limit()` falls back to the supplied model
/// configuration's context limit.
context_size: Arc<AtomicU64>,
session_title_publisher: SessionTitlePublisher,
/// Config option id used to select the model, if this agent supports it.
model_config_option_id: Option<String>,
@@ -277,13 +252,11 @@ impl AcpProvider {
let pending_tool_updates: Arc<Mutex<HashMap<String, AccumulatedToolCall>>> =
Arc::new(Mutex::new(HashMap::new()));
let context_size = Arc::new(AtomicU64::new(0));
let session_title_publisher = SessionTitlePublisher::default();
let client_loop = AcpClientLoop::new(
config,
goose_mode_shared.clone(),
pending_tool_updates.clone(),
context_size.clone(),
session_title_publisher.clone(),
);
let loop_thread = spawn_client_loop(run(client_loop, rx, init_tx));
@@ -316,7 +289,6 @@ impl AcpProvider {
pending_tool_updates,
handoff_context_sent: AtomicBool::new(false),
context_size,
session_title_publisher,
model_config_option_id,
applied_model: Arc::new(Mutex::new(applied_model)),
tx: Some(tx),
@@ -542,10 +514,6 @@ impl Provider for AcpProvider {
true
}
fn set_session_title_callback(&self, callback: Arc<dyn Fn(String) + Send + Sync>) {
self.session_title_publisher.set_callback(callback);
}
async fn handle_permission_confirmation(
&self,
request_id: &str,
@@ -786,7 +754,6 @@ struct AcpClientLoop {
prompt_response_tx: Arc<Mutex<Option<mpsc::Sender<AcpUpdate>>>>,
pending_tool_updates: Arc<Mutex<HashMap<String, AccumulatedToolCall>>>,
context_size: Arc<AtomicU64>,
session_title_publisher: SessionTitlePublisher,
}
impl AcpClientLoop {
@@ -795,7 +762,6 @@ impl AcpClientLoop {
goose_mode: Arc<Mutex<GooseMode>>,
pending_tool_updates: Arc<Mutex<HashMap<String, AccumulatedToolCall>>>,
context_size: Arc<AtomicU64>,
session_title_publisher: SessionTitlePublisher,
) -> Self {
Self {
config,
@@ -803,7 +769,6 @@ impl AcpClientLoop {
prompt_response_tx: Arc::new(Mutex::new(None)),
pending_tool_updates,
context_size,
session_title_publisher,
}
}
@@ -858,7 +823,6 @@ impl AcpClientLoop {
prompt_response_tx,
pending_tool_updates,
context_size,
session_title_publisher,
} = self;
let notification_callback = config.notification_callback.clone();
let reverse_modes = reverse_mode_mapping(&config.mode_mapping);
@@ -872,7 +836,6 @@ impl AcpClientLoop {
let goose_mode = goose_mode.clone();
let pending_tool_updates = pending_tool_updates.clone();
let context_size = context_size.clone();
let session_title_publisher = session_title_publisher.clone();
async move |notification: SessionNotification, _cx| {
if let Some(ref cb) = notification_callback {
cb(notification.clone());
@@ -909,11 +872,6 @@ impl AcpClientLoop {
SessionUpdate::UsageUpdate(usage) => {
context_size.store(usage.size, Ordering::Relaxed);
}
SessionUpdate::SessionInfoUpdate(update) => {
if let Some(title) = update.title.value() {
session_title_publisher.publish(title);
}
}
_ => {}
}
if let Some(tx) = prompt_response_tx
@@ -1858,21 +1816,6 @@ mod tests {
test_provider_with_tx(None)
}
#[test]
fn session_title_publisher_forwards_non_empty_titles() {
let publisher = SessionTitlePublisher::default();
let titles = Arc::new(Mutex::new(Vec::new()));
let received = titles.clone();
publisher.set_callback(Arc::new(move |title| {
received.lock().unwrap().push(title);
}));
publisher.publish(" Generated title ");
publisher.publish(" ");
assert_eq!(*titles.lock().unwrap(), vec!["Generated title"]);
}
fn test_provider_with_tx(
tx: Option<mpsc::Sender<ClientRequest>>,
) -> (AcpProvider, ModelConfig) {
@@ -1889,7 +1832,6 @@ mod tests {
pending_tool_updates: Arc::new(Mutex::new(HashMap::new())),
handoff_context_sent: AtomicBool::new(false),
context_size: Arc::new(AtomicU64::new(0)),
session_title_publisher: SessionTitlePublisher::default(),
model_config_option_id: None,
applied_model: Arc::new(Mutex::new(None)),
tx,
crates/goose/src/agents/agent.rs
-23
View File
@@ -3406,29 +3406,6 @@ impl Agent {
session_id: &str,
) -> Result<()> {
let provider_name = provider.get_name().to_string();
let session_manager = self.config.session_manager.clone();
let session_name_update_tx = self.config.session_name_update_tx.clone();
let session_id_for_title = session_id.to_string();
let runtime = tokio::runtime::Handle::current();
provider.set_session_title_callback(Arc::new(move |title| {
let session_manager = session_manager.clone();
let session_name_update_tx = session_name_update_tx.clone();
let session_id = session_id_for_title.clone();
runtime.spawn(async move {
match session_manager
.update_name_from_provider(&session_id, title)
.await
{
Ok(Some(update)) => {
if let Some(tx) = session_name_update_tx {
let _ = tx.send(update);
}
}
Ok(None) => {}
Err(error) => warn!(%error, "Failed to apply provider session title"),
}
});
}));
// Normalize against the provider entry so custom/declarative providers
// backfill `context_limit` from their known models before the config is
crates/goose/src/session/session_manager.rs
-72
View File
@@ -557,24 +557,6 @@ impl SessionManager {
})
}
pub async fn update_name_from_provider(
&self,
id: &str,
name: String,
) -> Result<Option<SessionNameUpdate>> {
let name = name.trim().to_string();
if name.is_empty() {
return Ok(None);
}
let session = self.get_session(id, false).await?;
if session.user_set_name || session.name == name {
return Ok(None);
}
Ok(Some(self.system_generated_name_update(id, name).await?))
}
pub async fn maybe_update_name(
&self,
id: &str,
@@ -3221,60 +3203,6 @@ mod tests {
assert_eq!(update.name, "investigate session naming with");
}
#[tokio::test]
async fn test_provider_name_replaces_generated_name_but_not_user_name() {
let temp_dir = TempDir::new().unwrap();
let sm = SessionManager::new(temp_dir.path().to_path_buf());
let session = sm
.create_session(
temp_dir.path().to_path_buf(),
"Local fallback".to_string(),
SessionType::User,
GooseMode::default(),
)
.await
.unwrap();
let update = sm
.update_name_from_provider(&session.id, " Better ACP title ".to_string())
.await
.unwrap()
.unwrap();
assert_eq!(update.name, "Better ACP title");
sm.update(&session.id)
.model_config(ModelConfig::new("test-model"))
.apply()
.await
.unwrap();
add_user_message(&sm, &session.id).await;
add_user_message(&sm, &session.id).await;
assert!(sm
.maybe_update_name(&session.id, Arc::new(StatefulNamingTestProvider))
.await
.unwrap()
.is_none());
assert_eq!(
sm.get_session(&session.id, false).await.unwrap().name,
"Better ACP title"
);
sm.update(&session.id)
.user_provided_name("Manual title")
.apply()
.await
.unwrap();
assert!(sm
.update_name_from_provider(&session.id, "Another ACP title".to_string())
.await
.unwrap()
.is_none());
assert_eq!(
sm.get_session(&session.id, false).await.unwrap().name,
"Manual title"
);
}
#[tokio::test]
async fn test_maybe_update_name_preserves_user_renamed_session() {
let temp_dir = TempDir::new().unwrap();