fix: SACP notifies clients of generated session names (#8983)

Signed-off-by: Matt Toohey <contact@matttoohey.com>
This commit is contained in:
Matt Toohey
2026-05-05 08:00:08 +10:00
committed by GitHub
parent ebe3315bdd
commit 713d9d2010
13 changed files with 364 additions and 33 deletions
+42 -7
View File
@@ -43,12 +43,13 @@ use sacp::schema::{
PermissionOption, PermissionOptionKind, PromptCapabilities, PromptRequest, PromptResponse,
RequestPermissionOutcome, RequestPermissionRequest, ResourceLink, SessionCapabilities,
SessionCloseCapabilities, SessionConfigOption, SessionConfigOptionCategory,
SessionConfigSelectOption, SessionId, SessionInfo, SessionListCapabilities, SessionMode,
SessionModeId, SessionModeState, SessionModelState, SessionNotification, SessionUpdate,
SetSessionConfigOptionRequest, SetSessionConfigOptionResponse, SetSessionModeRequest,
SetSessionModeResponse, SetSessionModelRequest, SetSessionModelResponse, StopReason,
TextContent, TextResourceContents, ToolCall, ToolCallContent, ToolCallId, ToolCallLocation,
ToolCallStatus, ToolCallUpdate, ToolCallUpdateFields, ToolKind, Usage, UsageUpdate,
SessionConfigSelectOption, SessionId, SessionInfo, SessionInfoUpdate, SessionListCapabilities,
SessionMode, SessionModeId, SessionModeState, SessionModelState, SessionNotification,
SessionUpdate, SetSessionConfigOptionRequest, SetSessionConfigOptionResponse,
SetSessionModeRequest, SetSessionModeResponse, SetSessionModelRequest, SetSessionModelResponse,
StopReason, TextContent, TextResourceContents, ToolCall, ToolCallContent, ToolCallId,
ToolCallLocation, ToolCallStatus, ToolCallUpdate, ToolCallUpdateFields, ToolKind, Usage,
UsageUpdate,
};
use sacp::util::MatchDispatchFrom;
use sacp::{
@@ -268,6 +269,36 @@ fn thread_session_meta(
meta
}
fn spawn_session_name_update_notifier(
cx: ConnectionTo<Client>,
) -> tokio::sync::mpsc::UnboundedSender<crate::session::SessionNameUpdate> {
let (tx, mut rx) = tokio::sync::mpsc::unbounded_channel::<crate::session::SessionNameUpdate>();
tokio::spawn(async move {
while let Some(update) = rx.recv().await {
let thread = update.thread;
let thread_id = thread.id.clone();
let meta = thread_session_meta(&thread);
let notification = SessionNotification::new(
SessionId::new(thread_id.clone()),
SessionUpdate::SessionInfoUpdate(
SessionInfoUpdate::new()
.title(thread.name)
.updated_at(thread.updated_at.to_rfc3339())
.meta(meta),
),
);
if let Err(error) = cx.send_notification(notification) {
warn!(
thread_id = %thread_id,
error = %error,
"Failed to send generated session name update"
);
}
}
});
tx
}
fn extract_timeout_from_meta(meta: &Option<Meta>) -> Option<u64> {
meta.as_ref()
.and_then(|m| m.get("timeout"))
@@ -1147,6 +1178,9 @@ impl GooseAcpAgent {
}
};
let session_name_update_tx =
(!disable_session_naming).then(|| spawn_session_name_update_notifier(cx.clone()));
// ── Phase 1: create agent + init provider (fast, ~55ms) ──────
let phase1: Result<Arc<Agent>, String> = async {
let agent = Arc::new(Agent::with_config(
@@ -1158,7 +1192,8 @@ impl GooseAcpAgent {
disable_session_naming,
goose_platform,
)
.with_mcp_host_info(client_mcp_host_info),
.with_mcp_host_info(client_mcp_host_info)
.with_session_name_update_tx(session_name_update_tx),
));
// Init provider — reuse the pre-resolved name + model when
+12 -2
View File
@@ -143,10 +143,20 @@ impl GooseAcpAgent {
&self,
req: RenameSessionRequest,
) -> Result<EmptyResponse, sacp::Error> {
self.thread_manager
.update_thread(&req.session_id, Some(req.title), Some(true), None)
let title = req.title;
let thread = self
.thread_manager
.update_thread(&req.session_id, Some(title.clone()), Some(true), None)
.await
.map_err(|e| sacp::Error::internal_error().data(e.to_string()))?;
if let Some(internal_session_id) = thread.current_session_id {
self.session_manager
.update(&internal_session_id)
.user_provided_name(title)
.apply()
.await
.map_err(|e| sacp::Error::internal_error().data(e.to_string()))?;
}
Ok(EmptyResponse {})
}
+22 -3
View File
@@ -50,7 +50,7 @@ use crate::security::adversary_inspector::AdversaryInspector;
use crate::security::egress_inspector::EgressInspector;
use crate::security::security_inspector::SecurityInspector;
use crate::session::extension_data::{EnabledExtensionsState, ExtensionState};
use crate::session::{Session, SessionManager};
use crate::session::{Session, SessionManager, SessionNameUpdate};
use crate::tool_inspection::ToolInspectionManager;
use crate::tool_monitor::RepetitionInspector;
use crate::utils::is_token_cancelled;
@@ -116,6 +116,7 @@ pub struct AgentConfig {
pub disable_session_naming: bool,
pub goose_platform: GoosePlatform,
pub mcp_host_info: Option<GooseMcpHostInfo>,
pub session_name_update_tx: Option<mpsc::UnboundedSender<SessionNameUpdate>>,
}
impl AgentConfig {
@@ -135,6 +136,7 @@ impl AgentConfig {
disable_session_naming,
goose_platform,
mcp_host_info: None,
session_name_update_tx: None,
}
}
@@ -142,6 +144,14 @@ impl AgentConfig {
self.mcp_host_info = mcp_host_info;
self
}
pub fn with_session_name_update_tx(
mut self,
tx: Option<mpsc::UnboundedSender<SessionNameUpdate>>,
) -> Self {
self.session_name_update_tx = tx;
self
}
}
/// The main goose Agent
@@ -1247,12 +1257,21 @@ impl Agent {
let session_id = session_config.id.clone();
if !self.config.disable_session_naming {
let manager_for_spawn = session_manager.clone();
let session_name_update_tx = self.config.session_name_update_tx.clone();
tokio::spawn(async move {
if let Err(e) = manager_for_spawn
match manager_for_spawn
.maybe_update_name(&session_id, provider)
.await
{
warn!("Failed to generate session description: {}", e);
Ok(Some(update)) => {
if let Some(tx) = session_name_update_tx {
if tx.send(update).is_err() {
warn!("Failed to publish generated session name");
}
}
}
Ok(None) => {}
Err(e) => warn!("Failed to generate session description: {}", e),
}
});
}
+1 -1
View File
@@ -11,6 +11,6 @@ pub use diagnostics::{
};
pub use extension_data::{EnabledExtensionsState, ExtensionData, ExtensionState, TodoState};
pub use session_manager::{
Session, SessionInsights, SessionManager, SessionType, SessionUpdateBuilder,
Session, SessionInsights, SessionManager, SessionNameUpdate, SessionType, SessionUpdateBuilder,
};
pub use thread_manager::{Thread, ThreadManager, ThreadMetadata};
+16 -6
View File
@@ -256,6 +256,11 @@ pub struct SessionManager {
storage: Arc<SessionStorage>,
}
#[derive(Debug, Clone)]
pub struct SessionNameUpdate {
pub thread: super::thread_manager::Thread,
}
impl SessionManager {
pub fn new(data_dir: PathBuf) -> Self {
Self {
@@ -351,11 +356,15 @@ impl SessionManager {
.await
}
pub async fn maybe_update_name(&self, id: &str, provider: Arc<dyn Provider>) -> Result<()> {
pub async fn maybe_update_name(
&self,
id: &str,
provider: Arc<dyn Provider>,
) -> Result<Option<SessionNameUpdate>> {
let session = self.get_session(id, true).await?;
if session.user_set_name {
return Ok(());
return Ok(None);
}
let conversation = session
@@ -379,15 +388,16 @@ impl SessionManager {
if let Some(ref thread_id) = session.thread_id {
let thread_mgr = super::thread_manager::ThreadManager::new(self.storage.clone());
let thread = thread_mgr.get_thread(thread_id).await?;
if !thread.user_set_name {
thread_mgr
if !thread.user_set_name && thread.name != name {
let thread = thread_mgr
.update_thread(thread_id, Some(name), Some(false), None)
.await?;
return Ok(Some(SessionNameUpdate { thread }));
}
}
Ok(())
Ok(None)
} else {
Ok(())
Ok(None)
}
}
@@ -12,6 +12,12 @@ use fs_err as fs;
use goose::acp::server::AcpProviderFactory;
use goose::config::base::CONFIG_YAML_NAME;
use goose::config::GooseMode;
use goose::conversation::message::Message;
use goose::model::ModelConfig;
use goose::providers::base::{
stream_from_single_message, MessageStream, Provider, ProviderUsage, Usage,
};
use goose::providers::errors::ProviderError;
use goose_test_support::{McpFixture, FAKE_CODE, TEST_IMAGE_B64, TEST_MODEL};
use sacp::schema::{
ListSessionsResponse, McpServer, McpServerHttp, ModelId, SessionInfo, SessionModeId,
@@ -19,6 +25,7 @@ use sacp::schema::{
};
use sqlx::sqlite::SqlitePoolOptions;
use std::sync::Arc;
use std::time::Duration;
const SHELL_TEST_CONTENT: &str = "test-shell-content-98765";
@@ -51,6 +58,46 @@ async fn new_basic_session<C: Connection>(config: TestConnectionConfig) -> Basic
BasicSession { conn, session }
}
struct NamingProvider {
model_config: ModelConfig,
}
#[async_trait::async_trait]
impl Provider for NamingProvider {
fn get_name(&self) -> &str {
"naming-test"
}
async fn stream(
&self,
_model_config: &ModelConfig,
_session_id: &str,
system: &str,
_messages: &[Message],
_tools: &[rmcp::model::Tool],
) -> Result<MessageStream, ProviderError> {
let text = if system.contains("four words or less") || system.contains("4 words or less") {
"Generated Test Title"
} else {
"2"
};
Ok(stream_from_single_message(
Message::assistant().with_text(text),
ProviderUsage::new(self.model_config.model_name.clone(), Usage::default()),
))
}
fn get_model_config(&self) -> ModelConfig {
self.model_config.clone()
}
}
fn naming_provider_factory() -> AcpProviderFactory {
Arc::new(|_provider_name, model_config, _extensions| {
Box::pin(async move { Ok(Arc::new(NamingProvider { model_config }) as Arc<dyn Provider>) })
})
}
pub async fn run_list_sessions<C: Connection>() {
let BasicSession { conn, session } =
new_basic_session::<C>(TestConnectionConfig::default()).await;
@@ -80,6 +127,56 @@ pub async fn run_list_sessions<C: Connection>() {
);
}
pub async fn run_session_name_update_notification<C: Connection>() {
let expected_session_id = C::expected_session_id();
let openai = OpenAiFixture::new(vec![], expected_session_id.clone()).await;
let config = TestConnectionConfig {
provider_factory: Some(naming_provider_factory()),
disable_session_naming: false,
..Default::default()
};
let mut conn = C::new(config, openai).await;
let SessionData { mut session, .. } = conn.new_session().await.unwrap();
expected_session_id.set(&session.session_id().0);
let output = session
.prompt(
"what should we call this conversation?",
PermissionDecision::Cancel,
)
.await
.unwrap();
assert_eq!(output.text, "2");
let mut notifications = session.notifications();
let deadline = tokio::time::Instant::now() + Duration::from_secs(1);
while !notifications
.iter()
.any(|n| matches!(n, Notification::SessionInfoUpdate { .. }))
&& tokio::time::Instant::now() < deadline
{
tokio::time::sleep(Duration::from_millis(10)).await;
notifications.extend(session.notifications());
}
let update = notifications
.iter()
.find_map(|notification| match notification {
Notification::SessionInfoUpdate {
title,
updated_at,
message_count,
user_set_name,
} => Some((title, updated_at, message_count, user_set_name)),
_ => None,
})
.expect("expected generated session name notification");
assert_eq!(update.0.as_deref(), Some("Generated Test Title"));
assert!(update.1.is_some());
assert!(update.2.unwrap_or_default() >= 1);
assert_eq!(*update.3, Some(false));
}
pub async fn run_close_session<C: Connection>() {
let BasicSession { conn, session } =
new_basic_session::<C>(TestConnectionConfig::default()).await;
+23 -1
View File
@@ -157,6 +157,7 @@ pub async fn spawn_acp_server_in_process(
goose_mode: GooseMode,
provider_factory: Option<AcpProviderFactory>,
current_model: &str,
disable_session_naming: bool,
) -> (DuplexTransport, JoinHandle<()>, Arc<PermissionManager>) {
fs::create_dir_all(data_root).unwrap();
// TODO: Paths::in_state_dir is global, ignoring per-test data_root
@@ -190,7 +191,7 @@ pub async fn spawn_acp_server_in_process(
data_root.to_path_buf(),
data_root.to_path_buf(),
goose_mode,
true,
disable_session_naming,
GoosePlatform::GooseCli,
)
.await
@@ -221,6 +222,12 @@ pub enum Notification {
AvailableCommands,
CurrentMode,
ConfigOption,
SessionInfoUpdate {
title: Option<String>,
updated_at: Option<String>,
message_count: Option<u64>,
user_set_name: Option<bool>,
},
}
pub fn to_notifications(updates: &[SessionUpdate]) -> Vec<Notification> {
@@ -266,6 +273,19 @@ pub fn to_notifications(updates: &[SessionUpdate]) -> Vec<Notification> {
SessionUpdate::AvailableCommandsUpdate(_) => out.push(Notification::AvailableCommands),
SessionUpdate::CurrentModeUpdate(_) => out.push(Notification::CurrentMode),
SessionUpdate::ConfigOptionUpdate(_) => out.push(Notification::ConfigOption),
SessionUpdate::SessionInfoUpdate(update) => {
let meta = update.meta.as_ref();
out.push(Notification::SessionInfoUpdate {
title: update.title.value().cloned(),
updated_at: update.updated_at.value().cloned(),
message_count: meta
.and_then(|m| m.get("messageCount"))
.and_then(|v| v.as_u64()),
user_set_name: meta
.and_then(|m| m.get("userSetName"))
.and_then(|v| v.as_bool()),
});
}
_ => {}
}
}
@@ -482,6 +502,7 @@ pub struct TestConnectionConfig {
pub strip_config_options: bool,
// The model the server-side provider starts with. Defaults to TEST_MODEL.
pub current_model: String,
pub disable_session_naming: bool,
}
impl Default for TestConnectionConfig {
@@ -498,6 +519,7 @@ impl Default for TestConnectionConfig {
terminal: None,
strip_config_options: false,
current_model: TEST_MODEL.to_string(),
disable_session_naming: true,
}
}
}
@@ -162,6 +162,7 @@ impl Connection for AcpProviderConnection {
goose_mode,
config.provider_factory,
&current_model,
config.disable_session_naming,
)
.await;
@@ -121,6 +121,7 @@ impl Connection for AcpServerConnection {
config.goose_mode,
config.provider_factory,
&config.current_model,
config.disable_session_naming,
)
.await;
+7 -2
View File
@@ -11,8 +11,8 @@ use common_tests::{
run_model_list, run_model_set, run_model_set_error_session_not_found,
run_new_session_returns_initial_config, run_permission_persistence, run_prompt_basic,
run_prompt_codemode, run_prompt_error, run_prompt_image, run_prompt_image_attachment,
run_prompt_mcp, run_prompt_model_mismatch, run_prompt_skill, run_shell_terminal_false,
run_shell_terminal_true,
run_prompt_mcp, run_prompt_model_mismatch, run_prompt_skill,
run_session_name_update_notification, run_shell_terminal_false, run_shell_terminal_true,
};
tests_config_option_set_error!(AcpServerConnection);
@@ -33,6 +33,11 @@ fn test_list_sessions() {
run_test(async { run_list_sessions::<AcpServerConnection>().await });
}
#[test]
fn test_session_name_update_notification() {
run_test(async { run_session_name_update_notification::<AcpServerConnection>().await });
}
#[test]
fn test_close_session() {
run_test(async { run_close_session::<AcpServerConnection>().await });
@@ -0,0 +1,95 @@
import { beforeEach, describe, expect, it } from "vitest";
import { useChatStore } from "@/features/chat/stores/chatStore";
import { useChatSessionStore } from "@/features/chat/stores/chatSessionStore";
import { handleSessionNotification } from "../acpNotificationHandler";
describe("ACP session info updates", () => {
beforeEach(() => {
useChatStore.setState({
messagesBySession: {},
sessionStateById: {},
queuedMessageBySession: {},
draftsBySession: {},
activeSessionId: null,
isConnected: false,
loadingSessionIds: new Set<string>(),
scrollTargetMessageBySession: {},
});
useChatSessionStore.setState({
sessions: [],
activeSessionId: null,
isLoading: false,
hasHydratedSessions: false,
contextPanelOpenBySession: {},
activeWorkspaceBySession: {},
});
});
it("applies generated session info updates to non-user-named sessions", async () => {
useChatSessionStore.getState().addSession({
id: "goose-session-title",
acpSessionId: "goose-session-title",
title: "New Chat",
createdAt: "2026-01-01T00:00:00.000Z",
updatedAt: "2026-01-01T00:00:00.000Z",
messageCount: 0,
userSetName: false,
});
await handleSessionNotification({
sessionId: "goose-session-title",
update: {
sessionUpdate: "session_info_update",
title: "Generated Test Title",
updatedAt: "2026-01-01T00:01:00.000Z",
_meta: {
messageCount: 1,
userSetName: false,
},
},
} as never);
expect(
useChatSessionStore.getState().getSession("goose-session-title"),
).toMatchObject({
title: "Generated Test Title",
updatedAt: "2026-01-01T00:01:00.000Z",
messageCount: 1,
userSetName: false,
});
});
it("ignores generated titles for user-named sessions", async () => {
useChatSessionStore.getState().addSession({
id: "goose-session-user-title",
acpSessionId: "goose-session-user-title",
title: "My Custom Title",
createdAt: "2026-01-01T00:00:00.000Z",
updatedAt: "2026-01-01T00:00:00.000Z",
messageCount: 0,
userSetName: true,
});
await handleSessionNotification({
sessionId: "goose-session-user-title",
update: {
sessionUpdate: "session_info_update",
title: "Generated Test Title",
updatedAt: "2026-01-01T00:01:00.000Z",
_meta: {
messageCount: 1,
userSetName: true,
},
},
} as never);
expect(
useChatSessionStore.getState().getSession("goose-session-user-title"),
).toMatchObject({
title: "My Custom Title",
updatedAt: "2026-01-01T00:01:00.000Z",
messageCount: 1,
userSetName: true,
});
});
});
@@ -30,6 +30,7 @@ import {
getTrackedReplayAssistantMessageId,
} from "./acpReplayAssistant";
import { getReplayCreated, getReplayMessageId } from "./acpReplayMetadata";
import { handleSessionInfoUpdate } from "./acpSessionInfoUpdate";
import {
getLocalSessionId,
subscribeToSessionRegistration,
@@ -487,17 +488,7 @@ function handleShared(
): void {
switch (update.sessionUpdate) {
case "session_info_update": {
const info = update as SessionUpdate & {
sessionUpdate: "session_info_update";
};
if ("title" in info && info.title) {
const session = useChatSessionStore.getState().getSession(sessionId);
if (session && !session.userSetName) {
useChatSessionStore
.getState()
.updateSession(sessionId, { title: info.title as string });
}
}
handleSessionInfoUpdate(sessionId, update);
break;
}
@@ -0,0 +1,45 @@
import type { SessionUpdate } from "@agentclientprotocol/sdk";
import { useChatSessionStore } from "@/features/chat/stores/chatSessionStore";
type SessionInfoUpdate = SessionUpdate & {
sessionUpdate: "session_info_update";
title?: unknown;
updatedAt?: unknown;
_meta?: unknown;
};
function isRecord(value: unknown): value is Record<string, unknown> {
return typeof value === "object" && value !== null;
}
export function handleSessionInfoUpdate(
sessionId: string,
update: SessionUpdate,
): void {
const info = update as SessionInfoUpdate;
const sessionStore = useChatSessionStore.getState();
const session = sessionStore.getSession(sessionId);
if (!session) {
return;
}
const meta = isRecord(info._meta) ? info._meta : {};
const patch: Parameters<typeof sessionStore.updateSession>[1] = {};
if (typeof info.title === "string" && info.title && !session.userSetName) {
patch.title = info.title;
}
if (typeof info.updatedAt === "string" && info.updatedAt) {
patch.updatedAt = info.updatedAt;
}
if (typeof meta.messageCount === "number") {
patch.messageCount = meta.messageCount;
}
if (typeof meta.userSetName === "boolean") {
patch.userSetName = meta.userSetName;
}
if (Object.keys(patch).length > 0) {
sessionStore.updateSession(sessionId, patch);
}
}