handle non-existent sessions gracefully instead of panicking

crates/goose-cli/src/cli.rs

@@ -67,17 +67,27 @@ jsonl' -> '20250325_200615')."
     path: Option<PathBuf>,
 }
 
-async fn get_session_id(identifier: Identifier) -> Result<String> {
+async fn get_session_id(identifier: Identifier, must_exist: bool) -> Result<String> {
     if let Some(session_id) = identifier.session_id {
         Ok(session_id)
     } else if let Some(name) = identifier.name {
         let sessions = SessionManager::list_sessions().await?;
 
-        sessions
+        match sessions
             .into_iter()
             .find(|s| s.id == name || s.description.contains(&name))
             .map(|s| s.id)
-            .ok_or_else(|| anyhow::anyhow!("No session found with name '{}'", name))
+        {
+            Some(id) => Ok(id),
+            None => {
+                if must_exist {
+                    Err(anyhow::anyhow!("No session found with name '{}'", name))
+                } else {
+                    // Return the name as-is to be used as a new session ID/description
+                    Ok(name)
+                }
+            }
+        }
     } else if let Some(path) = identifier.path {
         path.file_stem()
             .and_then(|s| s.to_str())
@@ -826,7 +836,7 @@ pub async fn cli() -> Result<()> {
                     format,
                 }) => {
                     let session_identifier = if let Some(id) = identifier {
-                        get_session_id(id).await?
+                        get_session_id(id, true).await?
                     } else {
                         // If no identifier is provided, prompt for interactive selection
                         match crate::commands::session::prompt_interactive_session_selection().await
@@ -859,7 +869,7 @@ pub async fn cli() -> Result<()> {
                     );
 
                     let session_id = if let Some(id) = identifier {
-                        Some(get_session_id(id).await?)
+                        Some(get_session_id(id, resume).await?)
                     } else {
                         None
                     };
@@ -1057,7 +1067,7 @@ pub async fn cli() -> Result<()> {
                 }
             };
             let session_id = if let Some(id) = identifier {
-                Some(get_session_id(id).await?)
+                Some(get_session_id(id, resume).await?)
             } else {
                 None
             };
@@ -1281,3 +1291,176 @@ pub async fn cli() -> Result<()> {
     }
     Ok(())
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[tokio::test]
+    async fn test_get_session_id_with_direct_session_id() {
+        // Test that session_id field is returned as-is
+        let identifier = Identifier {
+            name: None,
+            session_id: Some("test_session_123".to_string()),
+            path: None,
+        };
+
+        let result = get_session_id(identifier, false).await;
+        assert!(result.is_ok());
+        assert_eq!(result.unwrap(), "test_session_123");
+    }
+
+    #[tokio::test]
+    async fn test_get_session_id_with_path() {
+        // Test that session ID is extracted from path
+        let identifier = Identifier {
+            name: None,
+            session_id: None,
+            path: Some(PathBuf::from("/some/path/20250107_001.jsonl")),
+        };
+
+        let result = get_session_id(identifier, false).await;
+        assert!(result.is_ok());
+        assert_eq!(result.unwrap(), "20250107_001");
+    }
+
+    #[tokio::test]
+    async fn test_get_session_id_must_exist_false_with_nonexistent_name() {
+        // Test that when must_exist=false, non-existent session names are returned as-is
+        let identifier = Identifier {
+            name: Some("nonexistent_session_xyz".to_string()),
+            session_id: None,
+            path: None,
+        };
+
+        let result = get_session_id(identifier, false).await;
+        assert!(result.is_ok());
+        // Should return the name as-is since it doesn't exist but must_exist=false
+        assert_eq!(result.unwrap(), "nonexistent_session_xyz");
+    }
+
+    #[tokio::test]
+    async fn test_get_session_id_must_exist_true_with_nonexistent_name() {
+        // Test that when must_exist=true, non-existent session names cause an error
+        let identifier = Identifier {
+            name: Some("definitely_nonexistent_session_999".to_string()),
+            session_id: None,
+            path: None,
+        };
+
+        let result = get_session_id(identifier, true).await;
+        assert!(result.is_err());
+        assert!(result
+            .unwrap_err()
+            .to_string()
+            .contains("No session found with name"));
+    }
+
+    #[tokio::test]
+    async fn test_get_session_id_must_exist_true_with_existing_session() {
+        // Test that when must_exist=true and session exists, it returns the session ID
+
+        // Create a test session
+        let session = match SessionManager::create_session(
+            std::env::current_dir().unwrap(),
+            "Test session for get_session_id".to_string(),
+        )
+        .await
+        {
+            Ok(s) => s,
+            Err(_) => {
+                // Skip test if we can't create a session (e.g., in CI without proper setup)
+                return;
+            }
+        };
+
+        let session_id = session.id.clone();
+
+        // Test with the actual session ID
+        let identifier = Identifier {
+            name: Some(session_id.clone()),
+            session_id: None,
+            path: None,
+        };
+
+        let result = get_session_id(identifier, true).await;
+        if result.is_ok() {
+            assert_eq!(result.unwrap(), session_id);
+        }
+
+        // Clean up
+        let _ = SessionManager::delete_session(&session_id).await;
+    }
+
+    #[tokio::test]
+    async fn test_get_session_id_finds_by_description() {
+        // Test that get_session_id can find sessions by description
+
+        // Create a test session with a specific description
+        let test_description = "Unique test description 12345";
+        let session = match SessionManager::create_session(
+            std::env::current_dir().unwrap(),
+            test_description.to_string(),
+        )
+        .await
+        {
+            Ok(s) => s,
+            Err(_) => {
+                // Skip test if we can't create a session
+                return;
+            }
+        };
+
+        let session_id = session.id.clone();
+
+        // Test searching by part of the description
+        let identifier = Identifier {
+            name: Some("Unique test description".to_string()),
+            session_id: None,
+            path: None,
+        };
+
+        let result = get_session_id(identifier, true).await;
+        if result.is_ok() {
+            assert_eq!(result.unwrap(), session_id);
+        }
+
+        // Clean up
+        let _ = SessionManager::delete_session(&session_id).await;
+    }
+
+    #[tokio::test]
+    async fn test_get_session_id_must_exist_false_with_existing_session() {
+        // Test that when must_exist=false and session exists, it still returns the session ID
+
+        // Create a test session
+        let session = match SessionManager::create_session(
+            std::env::current_dir().unwrap(),
+            "Test existing session".to_string(),
+        )
+        .await
+        {
+            Ok(s) => s,
+            Err(_) => {
+                // Skip test if we can't create a session
+                return;
+            }
+        };
+
+        let session_id = session.id.clone();
+
+        let identifier = Identifier {
+            name: Some(session_id.clone()),
+            session_id: None,
+            path: None,
+        };
+
+        let result = get_session_id(identifier, false).await;
+        if result.is_ok() {
+            assert_eq!(result.unwrap(), session_id);
+        }
+
+        // Clean up
+        let _ = SessionManager::delete_session(&session_id).await;
+    }
+}

crates/goose-cli/src/session/builder.rs

@@ -309,7 +309,26 @@ pub async fn build_session(session_config: SessionBuilderConfig) -> CliSession {
             }
         }
     } else if let Some(session_id) = session_config.session_id {
-        Some(session_id)
+        // Check if the session exists; if not, we'll create a new one with this ID later
+        match SessionManager::get_session(&session_id, false).await {
+            Ok(_) => {
+                // Session exists, resume it (for backward compatibility)
+                Some(session_id)
+            }
+            Err(_) => {
+                // Session doesn't exist, create a new one with this ID
+                let session = SessionManager::create_session(
+                    std::env::current_dir().unwrap(),
+                    session_id.clone(),
+                )
+                .await
+                .unwrap_or_else(|e| {
+                    output::render_error(&format!("Failed to create session: {}", e));
+                    process::exit(1);
+                });
+                Some(session.id)
+            }
+        }
     } else {
         let session = SessionManager::create_session(
             std::env::current_dir().unwrap(),

crates/goose-cli/src/session/mod.rs

@@ -130,10 +130,13 @@ impl CliSession {
         let messages = if let Some(session_id) = &session_id {
             tokio::task::block_in_place(|| {
                 tokio::runtime::Handle::current().block_on(async {
-                    SessionManager::get_session(session_id, true)
-                        .await
-                        .map(|session| session.conversation.unwrap_or_default())
-                        .unwrap()
+                    match SessionManager::get_session(session_id, true).await {
+                        Ok(session) => session.conversation.unwrap_or_default(),
+                        Err(e) => {
+                            tracing::warn!("Failed to load session '{}': {}. Starting with empty conversation.", session_id, e);
+                            Conversation::new_unvalidated(Vec::new())
+                        }
+                    }
                 })
             })
         } else {
@@ -1748,4 +1751,101 @@ mod tests {
         let duration = Duration::from_millis(60500);
         assert_eq!(format_elapsed_time(duration), "1m 00s");
     }
+
+    #[tokio::test(flavor = "multi_thread")]
+    async fn test_cli_session_new_with_nonexistent_session() {
+        // Test that CliSession::new handles non-existent sessions gracefully
+        // without panicking, even when a session_id is provided
+
+        let agent = Agent::new();
+        let fake_session_id = Some("nonexistent_session_12345".to_string());
+
+        // This should not panic - it should create a session with empty messages
+        let session = CliSession::new(
+            agent,
+            fake_session_id.clone(),
+            false,
+            None,
+            None,
+            None,
+            None,
+        );
+
+        // Verify the session was created with the provided ID
+        assert_eq!(session.session_id, fake_session_id);
+
+        // Verify messages are empty (not loaded from non-existent session)
+        assert_eq!(session.messages.len(), 0);
+    }
+
+    #[test]
+    fn test_cli_session_new_without_session_id() {
+        // Test that CliSession::new works correctly when no session_id is provided
+
+        let agent = Agent::new();
+
+        let session = CliSession::new(
+            agent,
+            None,
+            false,
+            None,
+            None,
+            None,
+            None,
+        );
+
+        // Verify no session ID is set
+        assert_eq!(session.session_id, None);
+
+        // Verify messages are empty
+        assert_eq!(session.messages.len(), 0);
+    }
+
+    #[tokio::test(flavor = "multi_thread")]
+    async fn test_cli_session_new_with_existing_session() {
+        // Test that CliSession::new loads messages from an existing session
+
+        // First, create a session in the database
+        let session = match SessionManager::create_session(
+            std::env::current_dir().unwrap(),
+            "Test session for loading".to_string(),
+        )
+        .await
+        {
+            Ok(s) => s,
+            Err(_) => {
+                // Skip test if we can't create a session
+                return;
+            }
+        };
+
+        let session_id = session.id.clone();
+
+        // Add a test message to the session
+        let test_message = Message::user().with_text("Test message");
+        if let Err(_) = SessionManager::add_message(&session_id, &test_message).await {
+            // Clean up and skip if we can't add message
+            let _ = SessionManager::delete_session(&session_id).await;
+            return;
+        }
+
+        // Now create a CliSession with this session ID
+        let agent = Agent::new();
+        let cli_session = CliSession::new(
+            agent,
+            Some(session_id.clone()),
+            false,
+            None,
+            None,
+            None,
+            None,
+        );
+
+        // Verify the session was loaded with messages
+        assert_eq!(cli_session.session_id, Some(session_id.clone()));
+        assert_eq!(cli_session.messages.len(), 1);
+
+        // Clean up: delete the test session
+        let _ = SessionManager::delete_session(&session_id).await;
+    }
 }