refactor: make session.name the source of truth for displayed session titles (#9841)
Some checks are pending
Canary / bundle-desktop-windows-cuda (push) Blocked by required conditions
Canary / Prepare Version (push) Waiting to run
Canary / build-cli (push) Blocked by required conditions
Canary / Upload Install Script (push) Blocked by required conditions
Canary / bundle-desktop (push) Blocked by required conditions
Canary / bundle-desktop-intel (push) Blocked by required conditions
Canary / bundle-desktop-linux (push) Blocked by required conditions
Canary / bundle-desktop-windows (push) Blocked by required conditions
Canary / Release (push) Blocked by required conditions
Cargo Deny / deny (push) Waiting to run
Unused Dependencies / machete (push) Waiting to run
CI / changes (push) Waiting to run
CI / Check Rust Code Format (push) Blocked by required conditions
CI / Build and Test Rust Project (push) Blocked by required conditions
CI / Build Rust Project on Windows (push) Waiting to run
CI / Check MSRV (push) Blocked by required conditions
CI / Lint Rust Code (push) Blocked by required conditions
CI / Check Generated Schemas are Up-to-Date (push) Blocked by required conditions
CI / Test and Lint Electron Desktop App (push) Blocked by required conditions
Deploy Documentation / deploy (push) Waiting to run
Create Minor Release PR / check-version-bump-pr (push) Waiting to run
Create Minor Release PR / release (push) Blocked by required conditions
Live Provider Tests / check-fork (push) Waiting to run
Live Provider Tests / changes (push) Blocked by required conditions
Live Provider Tests / Build Binary (push) Blocked by required conditions
Live Provider Tests / Smoke Tests (push) Blocked by required conditions
Live Provider Tests / Smoke Tests (Code Execution) (push) Blocked by required conditions
Live Provider Tests / Compaction Tests (push) Blocked by required conditions
Live Provider Tests / goose server HTTP integration tests (push) Blocked by required conditions
Publish Ask AI Bot Docker Image / docker (push) Waiting to run
Publish Docker Image / docker (push) Waiting to run
Scorecard supply-chain security / Scorecard analysis (push) Waiting to run

This commit is contained in:
Lifei Zhou 2026-06-18 08:18:28 +10:00 committed by GitHub
parent 69b4d3686c
commit 109bda35ac
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 237 additions and 88 deletions

View file

@ -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
}

View file

@ -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,
@ -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,
@ -479,6 +484,19 @@ impl SessionManager {
return Ok(None);
}
if session.session_type == SessionType::Scheduled {
return Ok(None);
}
if let Some(recipe) = &session.recipe {
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"))?;
@ -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)
}
@ -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,
@ -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,

View file

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

View file

@ -10,24 +10,8 @@ import type { FixedExtensionEntry } from './components/ConfigContext';
import { AppEvents } from './constants/events';
import { decodeRecipe, Recipe } from './recipe';
export function shouldShowNewChatTitle(session: Session): boolean {
if (session.recipe) {
return false;
}
return !session.user_set_name && session.message_count === 0;
}
export function getSessionDisplayName(session: Session): string {
if (session.user_set_name) {
return session.name;
}
if (session.recipe?.title) {
return session.recipe.title;
}
if (shouldShowNewChatTitle(session)) {
return DEFAULT_CHAT_TITLE;
}
return session.name;
return session.name || DEFAULT_CHAT_TITLE;
}
export function resumeSession(session: Session, setView: setViewType) {