diff --git a/src/bin/record_vcr.rs b/src/bin/record_vcr.rs index ca09f13..e1a336c 100644 --- a/src/bin/record_vcr.rs +++ b/src/bin/record_vcr.rs @@ -6,6 +6,7 @@ use tokio::sync::{Semaphore, mpsc}; use tokio::task::LocalSet; use coven::commands; +use coven::session::runner::find_session_file; use coven::vcr::{DEFAULT_TEST_MODEL, Io, MultiStep, TestCase, TriggerController, VcrContext}; /// Writes to stderr with a `[prefix] ` prepended to each line. @@ -356,10 +357,55 @@ async fn record_case(case_dir: &Path, name: &str) -> Result<()> { } vcr.write_recording(&vcr_path)?; + + // Post-recording: verify session files were saved. + verify_session_files(&vcr_path, name)?; + std::fs::remove_dir_all(&tmp_dir).ok(); Ok(()) } +/// Extract session IDs from a VCR recording by scanning for result events +/// that contain a `session_id` field. +fn extract_session_ids(vcr_path: &Path) -> Result> { + let content = std::fs::read_to_string(vcr_path)?; + let mut ids = Vec::new(); + for line in content.lines() { + let entry: serde_json::Value = serde_json::from_str(line)?; + if let Some(result) = entry.get("result") + && let Some(ok) = result.get("Ok") + && let Some(claude) = ok.get("Claude").and_then(|c| c.get("Claude")) + && claude.get("type").and_then(|t| t.as_str()) == Some("result") + && let Some(id) = claude.get("session_id").and_then(|s| s.as_str()) + && !ids.contains(&id.to_string()) + { + ids.push(id.to_string()); + } + } + Ok(ids) +} + +/// After recording, verify that session files were saved for all sessions +/// in the recording. Logs results but does not fail the recording. +fn verify_session_files(vcr_path: &Path, name: &str) -> Result<()> { + let session_ids = extract_session_ids(vcr_path)?; + for id in &session_ids { + match find_session_file(id) { + Ok(Some(path)) => { + let meta = std::fs::metadata(&path)?; + eprintln!(" Session file for {name} ({id}): {} bytes", meta.len()); + } + Ok(None) => { + eprintln!(" WARNING: session file NOT found for {name} ({id})"); + } + Err(e) => { + eprintln!(" WARNING: session file check failed for {name}: {e}"); + } + } + } + Ok(()) +} + /// Record a multi-step test case. Steps are executed sequentially unless they /// share a `concurrent_group`, in which case they run concurrently. /// Each step writes its own VCR file: `__.vcr`. diff --git a/src/commands/ralph.rs b/src/commands/ralph.rs index 165e1de..5ce917e 100644 --- a/src/commands/ralph.rs +++ b/src/commands/ralph.rs @@ -203,9 +203,10 @@ pub async fn ralph( &features, ) .await?; - // Kill the CLI process immediately to prevent async task - // notifications from triggering an invisible continuation. - runner.kill().await?; + // The event loop has received the Result event (turn complete). + // Shut down gracefully: close stdin and wait for the process to + // exit (saving session state). Falls back to SIGKILL on timeout. + runner.shutdown().await?; match handle_session_outcome( outcome, diff --git a/src/commands/run.rs b/src/commands/run.rs index 3482381..1ed562d 100644 --- a/src/commands/run.rs +++ b/src/commands/run.rs @@ -112,8 +112,7 @@ pub async fn run( } } - runner.close_input(); - let _ = runner.wait().await; + runner.shutdown().await?; Ok(renderer.into_messages()) } @@ -139,8 +138,7 @@ async fn handle_outcome( { FollowUpAction::Sent => Ok(true), FollowUpAction::Interactive => { - runner.close_input(); - let _ = runner.wait().await; + runner.shutdown().await?; let Some(session_id) = state.session_id.take() else { return Ok(false); }; @@ -165,7 +163,7 @@ async fn handle_outcome( resume_after_pause(session_id, base_session_cfg, runner, state, ctx).await } SessionOutcome::Reload { .. } => { - runner.kill().await?; + runner.shutdown().await?; let Some(session_id) = state.session_id.take() else { return Ok(false); }; diff --git a/src/commands/worker.rs b/src/commands/worker.rs index 221b7f4..811d4f6 100644 --- a/src/commands/worker.rs +++ b/src/commands/worker.rs @@ -840,9 +840,10 @@ async fn run_phase_session( ) .await?; - // Kill the CLI process immediately to prevent async task - // notifications from triggering an invisible continuation. - runner.kill().await?; + // The event loop has received the Result event (turn complete). + // Kill the process to prevent async task notifications from + // triggering an invisible continuation. + runner.shutdown().await?; match outcome { SessionOutcome::Completed { result_text } => { diff --git a/src/session/event_loop.rs b/src/session/event_loop.rs index 4a9cb5d..04636e4 100644 --- a/src/session/event_loop.rs +++ b/src/session/event_loop.rs @@ -500,9 +500,10 @@ async fn execute_fork( bail!("fork detected but fork_config is None"); }; - // Kill the parent CLI process to prevent async task notifications - // from triggering an invisible continuation while fork children run. - runner.kill().await?; + // The event loop has received the Result event (turn complete, with + // fork tags). Kill the parent before fork children run to prevent + // async task notifications from triggering an invisible continuation. + runner.shutdown().await?; let msg = fork::run_fork(&session_id, tasks, fork_cfg, renderer, vcr).await?; diff --git a/src/session/runner.rs b/src/session/runner.rs index a0d1912..330c9f4 100644 --- a/src/session/runner.rs +++ b/src/session/runner.rs @@ -1,5 +1,6 @@ -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::process::Stdio; +use std::time::Duration; use anyhow::{Context, Result}; use serde::{Deserialize, Serialize}; @@ -183,7 +184,7 @@ impl SessionRunner { } } - /// Kill the claude process. No-op on stubs. + /// Kill the claude process immediately (SIGKILL). No-op on stubs. pub async fn kill(&mut self) -> Result<()> { if let Some(child) = &mut self.child { child.kill().await?; @@ -191,6 +192,27 @@ impl SessionRunner { Ok(()) } + /// Shut down the claude process after the current turn has completed. + /// + /// Callers must ensure the event loop has already received a `Result` + /// event (confirming the turn is complete) before calling this. Closes + /// stdin and waits for the process to exit so it can persist session + /// state. Falls back to SIGKILL if the process doesn't exit within + /// the timeout (e.g., an async task notification triggered a new turn). + pub async fn shutdown(&mut self) -> Result<()> { + self.close_input(); + if let Some(child) = &mut self.child { + match tokio::time::timeout(Duration::from_secs(5), child.wait()).await { + Ok(_) => {} // Process exited — session state saved. + Err(_) => { + // Timeout — kill to prevent invisible continuation. + child.kill().await.ok(); + } + } + } + Ok(()) + } + fn spawn_reader( stdout: ChildStdout, stderr: ChildStderr, @@ -245,6 +267,33 @@ pub(crate) fn has_flag(args: &[String], flag: &str) -> bool { .any(|a| a == flag || a.starts_with(&format!("{flag}="))) } +/// Find the claude session file for the given session ID. +/// +/// Searches `~/.claude/projects/*/sessions/` for a file whose stem matches +/// the session ID. Returns the file path if found. +pub fn find_session_file(session_id: &str) -> Result> { + let home = std::env::var("HOME").context("HOME not set")?; + let base = Path::new(&home).join(".claude/projects"); + if !base.exists() { + return Ok(None); + } + for project_entry in std::fs::read_dir(&base)? { + let project_dir = project_entry?.path(); + let sessions_dir = project_dir.join("sessions"); + if !sessions_dir.exists() { + continue; + } + for entry in std::fs::read_dir(&sessions_dir)? { + let path = entry?.path(); + let stem = path.file_stem().and_then(|s| s.to_str()).unwrap_or(""); + if stem == session_id { + return Ok(Some(path)); + } + } + } + Ok(None) +} + #[cfg(test)] mod tests { use super::*; diff --git a/tests/cases/ralph/ralph_session_save/ralph_session_save.toml b/tests/cases/ralph/ralph_session_save/ralph_session_save.toml new file mode 100644 index 0000000..04dc483 --- /dev/null +++ b/tests/cases/ralph/ralph_session_save/ralph_session_save.toml @@ -0,0 +1,7 @@ +[ralph] +prompt = "What is 2+2? Answer briefly then stop." +break_tag = "break" + +# No files needed — this is a simple Q&A to verify session shutdown behavior. +# After the model answers and outputs , the session should be saved +# before the process is killed.