-
Notifications
You must be signed in to change notification settings - Fork 6k
feat(cli): add /new to start a fresh session without restarting #10767
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.
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
Changes from all commits
c42adfc
649dc12
e48aaf7
bc612f2
af42a49
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -665,6 +665,10 @@ impl CliSession { | |
| history.save(editor); | ||
| self.handle_clear().await?; | ||
| } | ||
| InputResult::New => { | ||
| history.save(editor); | ||
| self.handle_new().await?; | ||
| } | ||
| InputResult::PromptCommand(opts) => { | ||
| history.save(editor); | ||
| self.handle_prompt_command(opts).await?; | ||
|
|
@@ -1070,6 +1074,92 @@ impl CliSession { | |
| Ok(()) | ||
| } | ||
|
|
||
| async fn handle_new(&mut self) -> Result<()> { | ||
| let provider = self.agent.provider().await?; | ||
| if provider.manages_own_context() { | ||
| output::render_error(&format!( | ||
| "Starting a new session is not supported for provider '{}' because it manages its own conversation context.", | ||
| provider.get_name() | ||
| )); | ||
| return Ok(()); | ||
| } | ||
|
|
||
| let new_session_id = match self.prepare_successor_session().await { | ||
| Ok(id) => id, | ||
| Err(e) => { | ||
| output::render_error(&format!("Failed to start a new session: {}", e)); | ||
| return Ok(()); | ||
| } | ||
| }; | ||
|
|
||
| let extension_configs = self.agent.get_extension_configs().await; | ||
|
|
||
| self.agent | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't this be emitted from somewhere inside the agent already when a session ends? |
||
| .emit_hook(goose::hooks::HookEvent::SessionEnd, &self.session_id) | ||
| .await; | ||
|
|
||
| self.agent.discard_pending_steers(&self.session_id).await; | ||
|
|
||
| self.session_id = new_session_id; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When the current provider manages its own context, Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch — confirmed for ACP, gemini-cli and claude-code ( Rather than recreating the provider mid-session, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Switching Useful? React with 👍 / 👎. |
||
| self.messages.clear(); | ||
| self.run_mode = RunMode::Normal; | ||
|
Comment on lines
+1104
to
+1105
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When Useful? React with 👍 / 👎. |
||
| self.agent.set_goal(None).await; | ||
| self.agent.set_grind(None).await; | ||
|
|
||
| if let Err(e) = self | ||
| .agent | ||
| .update_goose_mode(self.agent.goose_mode().await, &self.session_id) | ||
| .await | ||
| { | ||
| output::render_error(&format!("Failed to apply the current mode: {}", e)); | ||
| } | ||
|
|
||
| if !extension_configs.is_empty() { | ||
| output::goose_mode_message("Restarting extensions for the new session..."); | ||
| } | ||
|
|
||
| // MCP clients pin themselves to the first session id they see a request for, so | ||
| // extensions must be torn down and re-added under the new session id. | ||
| for name in self.agent.list_extensions().await { | ||
| if let Err(e) = self.agent.remove_extension(&name, &self.session_id).await { | ||
| output::render_extension_error(&name, &e.to_string()); | ||
| } | ||
| } | ||
|
|
||
| let mut unavailable = Vec::new(); | ||
| for config in extension_configs { | ||
| let name = config.name(); | ||
| if let Err(e) = self.agent.add_extension(config, &self.session_id).await { | ||
| output::render_extension_error(&name, &e.to_string()); | ||
|
Comment on lines
+1132
to
+1133
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
With any configured extension whose process cannot be relaunched at Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Partly addressed. The misleading part was real, though. The confirmation now names what did not come back ("Continuing without these extensions: ..."), so it no longer reads as if everything was carried over. af42a49 |
||
| unavailable.push(name); | ||
| } | ||
| } | ||
|
|
||
| if let Err(e) = self.update_completion_cache().await { | ||
| output::render_error(&format!("Failed to refresh completions: {}", e)); | ||
| } | ||
|
|
||
| let mut started = format!("Started a new session · {}\n", self.session_id); | ||
| if !unavailable.is_empty() { | ||
| started.push_str(&format!( | ||
| "Continuing without these extensions: {}\n", | ||
| unavailable.join(", ") | ||
| )); | ||
| } | ||
| output::render_message(&Message::assistant().with_text(started), self.debug); | ||
| Ok(()) | ||
| } | ||
|
|
||
| async fn prepare_successor_session(&self) -> Result<String> { | ||
| let session_manager = &self.agent.config.session_manager; | ||
| let old_session = session_manager.get_session(&self.session_id, false).await?; | ||
| let new_session_id = | ||
| create_successor_session(session_manager, &old_session, self.agent.goose_mode().await) | ||
| .await?; | ||
| self.agent.persist_extension_state(&new_session_id).await?; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Persisting the extension configs into the new row does not rebind or restart the already-running MCP clients. Those clients record the first session id they see and assert it never changes in Useful? React with 👍 / 👎. |
||
| Ok(new_session_id) | ||
| } | ||
|
|
||
| async fn handle_recipe(&mut self, filepath_opt: Option<String>) { | ||
| println!("{}", console::style("Generating Recipe").green()); | ||
|
|
||
|
|
@@ -1972,6 +2062,40 @@ impl CliSession { | |
| } | ||
| } | ||
|
|
||
| async fn create_successor_session( | ||
| session_manager: &SessionManager, | ||
| old_session: &goose::session::Session, | ||
| goose_mode: GooseMode, | ||
| ) -> Result<String> { | ||
| let new_session = session_manager | ||
| .create_session( | ||
| old_session.working_dir.clone(), | ||
| "CLI Session".to_string(), | ||
| old_session.session_type, | ||
| goose_mode, | ||
| ) | ||
| .await?; | ||
|
|
||
| let mut builder = session_manager | ||
| .update(&new_session.id) | ||
| .recipe(old_session.recipe.clone()) | ||
| .user_recipe_values(old_session.user_recipe_values.clone()); | ||
|
|
||
| if let Some(provider_name) = old_session.provider_name.clone() { | ||
| builder = builder.provider_name(provider_name); | ||
| } | ||
| if let Some(model_config) = old_session.model_config.clone() { | ||
| builder = builder.model_config(model_config); | ||
| } | ||
| if let Some(project_id) = old_session.project_id.clone() { | ||
| builder = builder.project_id(Some(project_id)); | ||
| } | ||
|
|
||
| builder.apply().await?; | ||
|
|
||
| Ok(new_session.id) | ||
| } | ||
|
|
||
| fn message_has_text(message: &Message) -> bool { | ||
| message.content.iter().any( | ||
| |content| matches!(content, MessageContent::Text(text) if !text.text.trim().is_empty()), | ||
|
|
@@ -2853,4 +2977,77 @@ mod tests { | |
| expected | ||
| ); | ||
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn new_session_inherits_provider_model_and_working_dir() { | ||
| let temp_dir = tempfile::TempDir::new().unwrap(); | ||
| let sm = SessionManager::new(temp_dir.path().to_path_buf()); | ||
|
|
||
| let old = sm | ||
| .create_session( | ||
| temp_dir.path().to_path_buf(), | ||
| "CLI Session".to_string(), | ||
| goose::session::SessionType::User, | ||
| GooseMode::Auto, | ||
| ) | ||
| .await | ||
| .unwrap(); | ||
|
|
||
| sm.update(&old.id) | ||
| .provider_name("anthropic") | ||
| .model_config(goose_providers::model::ModelConfig::new("test-model")) | ||
| .accumulated_usage(goose_providers::conversation::token_usage::Usage::new( | ||
| Some(100), | ||
| Some(50), | ||
| Some(150), | ||
| )) | ||
| .apply() | ||
| .await | ||
| .unwrap(); | ||
|
|
||
| sm.add_message(&old.id, &Message::user().with_text("hello")) | ||
| .await | ||
| .unwrap(); | ||
|
|
||
| let mut extension_data = goose::session::ExtensionData::new(); | ||
| extension_data.set_extension_state("test", "v0", serde_json::json!("marker")); | ||
| sm.update(&old.id) | ||
| .extension_data(extension_data) | ||
| .apply() | ||
| .await | ||
| .unwrap(); | ||
|
|
||
| let old = sm.get_session(&old.id, false).await.unwrap(); | ||
|
|
||
| let new_id = create_successor_session(&sm, &old, GooseMode::Chat) | ||
| .await | ||
| .unwrap(); | ||
|
|
||
| assert_ne!(new_id, old.id); | ||
|
|
||
| let new_session = sm.get_session(&new_id, true).await.unwrap(); | ||
| assert_eq!(new_session.provider_name, old.provider_name); | ||
| assert_eq!( | ||
| new_session.model_config.as_ref().map(|m| &m.model_name), | ||
| old.model_config.as_ref().map(|m| &m.model_name) | ||
| ); | ||
| assert_eq!(new_session.goose_mode, GooseMode::Chat); | ||
| assert_eq!(new_session.working_dir, old.working_dir); | ||
| assert_eq!(new_session.session_type, old.session_type); | ||
| assert!(new_session.conversation.unwrap().messages().is_empty()); | ||
| assert_eq!(new_session.usage.total_tokens, None); | ||
| assert_eq!(old.accumulated_usage.total_tokens, Some(150)); | ||
| assert_eq!(new_session.accumulated_usage.total_tokens, None); | ||
|
|
||
| let reloaded_old = sm.get_session(&old.id, true).await.unwrap(); | ||
| let old_messages = reloaded_old.conversation.unwrap().messages().to_vec(); | ||
| assert_eq!(old_messages.len(), 1); | ||
| assert_eq!(old_messages[0].as_concat_text(), "hello"); | ||
| assert_eq!( | ||
| reloaded_old | ||
| .extension_data | ||
| .get_extension_state("test", "v0"), | ||
| Some(&serde_json::json!("marker")) | ||
| ); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because this adds a user-visible CLI feature, leaving
goose-self-test.yamlunchanged means the repo’s required feature-validation recipe will not exercise the new-session transition, such as creating a new id while clearing history/tokens and preserving provider/extensions. Please add a self-test scenario for/newbefore landing.AGENTS.md reference: AGENTS.md:L71-L71
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/newis only reachable from the interactive loop:handle_slash_commandis called frominput::get_input, which has exactly one caller —run_interactive(session/mod.rs:573). The self-test runs viagoose run --recipe, which takes theheadlesspath (session/mod.rs:1378) and never reaches that code, so a scenario there could not exercise the transition.That is also why the recipe covers no slash command today —
/clear,/modeland/compactare all absent. It exercises agent and tool capabilities, not the interactive CLI. Happy to add coverage if you would rather have it, but it would need a PTY-driven session rather than a recipe step.