Skip to content

refactor: make session.name the source of truth for displayed session titles #9841

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 17, 2026
Merged

refactor: make session.name the source of truth for displayed session titles #9841

Show file tree
Hide file tree
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
5 changes: 2 additions & 3 deletions crates/goose/src/acp/response_builder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,12 +75,11 @@ pub(super) fn session_meta(session: &Session) -> serde_json::Map<String, serde_j

pub(super) fn build_session_info(session: Session) -> SessionInfo {
let meta = session_meta(&session);
let title = session.display_title();
let mut info = SessionInfo::new(SessionId::new(session.id), session.working_dir)
.updated_at(session.updated_at.to_rfc3339())
.meta(meta);
if let Some(title) = title {
info = info.title(title);
if !session.name.is_empty() {
info = info.title(session.name);
}
info
}
Expand Down
254 changes: 226 additions & 28 deletions crates/goose/src/session/session_manager.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,21 +90,6 @@ pub struct Session {
pub last_message_snippet: Option<String>,
}

impl Session {
pub fn display_title(&self) -> Option<String> {
if !self.user_set_name && self.session_type != SessionType::Scheduled {
if let Some(recipe) = &self.recipe {
return Some(recipe.title.clone());
}
}
if self.name.is_empty() {
None
} else {
Some(self.name.clone())
}
}
}

pub struct SessionUpdateBuilder<'a> {
session_manager: &'a SessionManager,
session_id: String,
Expand Down Expand Up @@ -468,6 +453,26 @@ impl SessionManager {
.await
}

async fn system_generated_name_update(
&self,
id: &str,
name: String,
) -> Result<SessionNameUpdate> {
self.update(id)
.system_generated_name(name.clone())
.apply()
.await?;

let session = self.get_session(id, false).await?;
Ok(SessionNameUpdate {
session_id: id.to_string(),
name,
updated_at: session.updated_at,
message_count: session.message_count,
user_set_name: session.user_set_name,
})
}

pub async fn maybe_update_name(
&self,
id: &str,
Expand All @@ -479,6 +484,19 @@ impl SessionManager {
return Ok(None);
}

if session.session_type == SessionType::Scheduled {
return Ok(None);
}

if let Some(recipe) = &session.recipe {

@DOsinga DOsinga Jun 17, 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.

this feels like the wrong place for this. I would consider calling user_set_name at the point where we attach the recipe?

@lifeizhou-ap lifeizhou-ap Jun 17, 2026 •
edited
Loading

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.

yeah, I also thought it is a bit strange to have the logic here while working on this PR. Ideally we can set the session name when recipe session is created. Currently this logic is in rest api and cli. Since we are moving towards to acp, that was why I did not change in the restful api. I will address this while working on recipe in acp which will be started soon

let name = recipe.title.trim().to_string();
if name.is_empty() || session.name == name {
return Ok(None);
}

return Ok(Some(self.system_generated_name_update(id, name).await?));
}

let conversation = session
.conversation
.ok_or_else(|| anyhow::anyhow!("No messages found"))?;
Expand All @@ -491,19 +509,7 @@ impl SessionManager {

if user_message_count <= MSG_COUNT_FOR_SESSION_NAME_GENERATION {
let name = provider.generate_session_name(id, &conversation).await?;
self.update(id)
.system_generated_name(name.clone())
.apply()
.await?;

let session = self.get_session(id, false).await?;
return Ok(Some(SessionNameUpdate {
session_id: id.to_string(),
name,
updated_at: session.updated_at,
message_count: session.message_count,
user_set_name: session.user_set_name,
}));
return Ok(Some(self.system_generated_name_update(id, name).await?));
}
Ok(None)
}
Expand Down Expand Up @@ -2035,10 +2041,61 @@ fn merge_tool_meta(
mod tests {
use super::*;
use crate::conversation::message::{Message, MessageContent};
use crate::providers::base::MessageStream;
use tempfile::TempDir;
use test_case::test_case;

const NUM_CONCURRENT_SESSIONS: i32 = 10;
const GENERATED_SESSION_NAME: &str = "Generated session name";

struct NamingTestProvider {
model_config: ModelConfig,
}

#[async_trait::async_trait]
impl Provider for NamingTestProvider {
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],
) -> std::result::Result<MessageStream, goose_providers::errors::ProviderError> {
unimplemented!("session naming tests override generate_session_name")
}

fn get_model_config(&self) -> ModelConfig {
self.model_config.clone()
}

async fn generate_session_name(
&self,
_session_id: &str,
_messages: &Conversation,
) -> std::result::Result<String, goose_providers::errors::ProviderError> {
Ok(GENERATED_SESSION_NAME.to_string())
}
}

fn naming_test_provider() -> Arc<dyn Provider> {
Arc::new(NamingTestProvider {
model_config: ModelConfig::new("test-model").unwrap(),
})
}

fn test_recipe(title: &str) -> Recipe {
Recipe::builder()
.title(title)
.description("Recipe description")
.instructions("Follow the recipe")
.build()
.unwrap()
}

async fn create_session_for_list(
sm: &SessionManager,
Expand Down Expand Up @@ -2115,6 +2172,147 @@ mod tests {
.unwrap();
}

async fn add_user_message(sm: &SessionManager, session_id: &str) {
sm.add_message(session_id, &Message::user().with_text("hello world"))
.await
.unwrap();
}

#[tokio::test]
async fn test_maybe_update_name_updates_eligible_session() {
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(),
"New Chat".to_string(),
SessionType::User,
GooseMode::default(),
)
.await
.unwrap();

add_user_message(&sm, &session.id).await;

let update = sm
.maybe_update_name(&session.id, naming_test_provider())
.await
.unwrap();
assert_eq!(
update.as_ref().map(|update| update.name.as_str()),
Some(GENERATED_SESSION_NAME)
);

let reloaded = sm.get_session(&session.id, false).await.unwrap();
assert_eq!(reloaded.name, GENERATED_SESSION_NAME);
assert!(!reloaded.user_set_name);
}

#[tokio::test]
async fn test_maybe_update_name_preserves_user_renamed_session() {
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(),
"New Chat".to_string(),
SessionType::User,
GooseMode::default(),
)
.await
.unwrap();

sm.update(&session.id)
.user_provided_name("Manual title".to_string())
.apply()
.await
.unwrap();
add_user_message(&sm, &session.id).await;

let update = sm
.maybe_update_name(&session.id, naming_test_provider())
.await
.unwrap();
assert!(update.is_none());

let reloaded = sm.get_session(&session.id, false).await.unwrap();
assert_eq!(reloaded.name, "Manual title");
assert!(reloaded.user_set_name);
}

#[tokio::test]
async fn test_maybe_update_name_uses_recipe_title_for_recipe_session() {
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(),
"New Chat".to_string(),
SessionType::User,
GooseMode::default(),
)
.await
.unwrap();

sm.update(&session.id)
.recipe(Some(test_recipe("Recipe title")))
.apply()
.await
.unwrap();
add_user_message(&sm, &session.id).await;

let update = sm
.maybe_update_name(&session.id, naming_test_provider())
.await
.unwrap();
assert_eq!(
update.as_ref().map(|update| update.name.as_str()),
Some("Recipe title")
);

let reloaded = sm.get_session(&session.id, false).await.unwrap();
assert_eq!(reloaded.name, "Recipe title");
assert!(!reloaded.user_set_name);

let update = sm
.maybe_update_name(&session.id, naming_test_provider())
.await
.unwrap();
assert!(update.is_none());
}

#[tokio::test]
async fn test_maybe_update_name_preserves_scheduled_session() {
let temp_dir = TempDir::new().unwrap();
let sm = SessionManager::new(temp_dir.path().to_path_buf());
let original_name = "Scheduled job: test-job";

let session = sm
.create_session(
temp_dir.path().to_path_buf(),
original_name.to_string(),
SessionType::Scheduled,
GooseMode::default(),
)
.await
.unwrap();

add_user_message(&sm, &session.id).await;

let update = sm
.maybe_update_name(&session.id, naming_test_provider())
.await
.unwrap();
assert!(update.is_none());

let reloaded = sm.get_session(&session.id, false).await.unwrap();
assert_eq!(reloaded.name, original_name);
assert!(!reloaded.user_set_name);
}

async fn create_search_session(
sm: &SessionManager,
name: &str,
Expand Down
48 changes: 8 additions & 40 deletions ui/desktop/src/__tests__/sessions.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { describe, it, expect } from 'vitest';
import { getSessionDisplayName, shouldShowNewChatTitle } from '../sessions';
import { getSessionDisplayName } from '../sessions';
import { prependUnique } from '../hooks/useNavigationSessions';
import type { Session } from '../api';
import type { SessionListItem } from '../acp/sessions';
Expand Down Expand Up @@ -30,51 +30,19 @@ function makeListItem(overrides: Partial<SessionListItem> = {}): SessionListItem
};
}

describe('shouldShowNewChatTitle', () => {
it('returns true for an empty session without a user-set name', () => {
const session = makeSession({ message_count: 0, user_set_name: false });
expect(shouldShowNewChatTitle(session)).toBe(true);
});

it('returns false when the session has messages', () => {
const session = makeSession({ message_count: 3, user_set_name: false });
expect(shouldShowNewChatTitle(session)).toBe(false);
});

it('returns false when the user has set a custom name', () => {
const session = makeSession({ message_count: 0, user_set_name: true });
expect(shouldShowNewChatTitle(session)).toBe(false);
});

it('returns false when the session has a recipe', () => {
const session = makeSession({
message_count: 0,
user_set_name: false,
recipe: { title: 'Recipe', steps: [] } as unknown as Session['recipe'],
});
expect(shouldShowNewChatTitle(session)).toBe(false);
});
});

describe('getSessionDisplayName (fix for #8865)', () => {
it('returns the user-set name for a recipe session that has been renamed', () => {
describe('getSessionDisplayName', () => {
it('returns the session name', () => {
const session = makeSession({
name: 'My Renamed Chat',
user_set_name: true,
message_count: 2,
recipe: { title: 'Some Recipe' } as unknown as Session['recipe'],
name: 'My Chat',
});
expect(getSessionDisplayName(session)).toBe('My Renamed Chat');
expect(getSessionDisplayName(session)).toBe('My Chat');
});

it('falls back to the recipe title when the user has not renamed', () => {
it('falls back to the default title when the session name is empty', () => {
const session = makeSession({
name: 'auto-generated',
user_set_name: false,
message_count: 2,
recipe: { title: 'Some Recipe' } as unknown as Session['recipe'],
name: '',
});
expect(getSessionDisplayName(session)).toBe('Some Recipe');
expect(getSessionDisplayName(session)).toBe('New Chat');
});
});

Expand Down
Loading
Loading