diff --git a/server/src/session/claude.rs b/server/src/session/claude.rs index a25ca42..09d8621 100644 --- a/server/src/session/claude.rs +++ b/server/src/session/claude.rs @@ -817,10 +817,36 @@ async fn follow( queue.lock().unwrap().close(&sink, "the session ended"); if !process::stopping(&session_dir) { let detail = stderr_tail(&stderr_path); + // A token the CLI will not accept leaves the chat + // *unrecoverable* rather than merely failed: every later + // start passes the same `--resume` and dies the same way. + // So it is forgotten, and `Cleared` marks where the model's + // context stopped -- as for a Codex thread with no rollout. + let refused = + missing_conversation(&detail) && read_resume_token(&session_dir).is_some(); + // Both the message and the divider follow the removal + // rather than the diagnosis: one that failed leaves the + // session as stuck as it was. + let recovered = refused && forget_resume_token(&session_dir); if !detail.is_empty() { - let _ = sink.send(Event::Error { - message: format!("{label} exited:\n{detail}"), - }); + // The CLI's sentence reads like the chat is lost when + // one more message is all it needs, and this is the + // only place anyone sees it. Its words stay on top, + // being what a person would search for. + let message = if recovered { + format!( + "{label} exited:\n{detail}\n\nThat conversation is gone from the \ + CLI, so this session has stopped trying to resume it. Send a \ + message to carry on in a new one -- everything above is kept, \ + but the model starts without it." + ) + } else { + format!("{label} exited:\n{detail}") + }; + let _ = sink.send(Event::Error { message }); + } + if recovered { + let _ = sink.send(Event::Cleared); } } let _ = sink.send(Event::Status { @@ -1083,6 +1109,38 @@ pub(super) fn write_resume_token(session_dir: &Path, session_id: &str) { } } +/// Drop a resume token the CLI has refused, so the next start makes a session +/// rather than repeating the failure. Answers whether it is really gone: the +/// caller promises somebody the session has stopped resuming, and a failed +/// removal would make that a promise this server cannot keep. +fn forget_resume_token(session_dir: &Path) -> bool { + let path = session_dir.join(RESUME_FILE); + match std::fs::remove_file(&path) { + Ok(()) => true, + Err(err) if err.kind() == std::io::ErrorKind::NotFound => true, + Err(err) => { + tracing::error!( + "couldn't forget the refused resume token at {}: {err}", + path.display() + ); + false + } + } +} + +/// Whether the CLI refused `--resume` because the token names nothing it has. +/// +/// Two sentences say it and both recover the same way; matched on the phrases +/// that carry the fact, since the rest names the id. Measured against 2.1.237: +/// a valid UUID with nothing behind it gives the first, a non-UUID matching no +/// title the second. +fn missing_conversation(detail: &str) -> bool { + let detail = detail.to_ascii_lowercase(); + detail.contains("no conversation found with session id") + || (detail.contains("--resume requires a valid session id") + && detail.contains("does not match any session title")) +} + /// Where an uploaded attachment is, as a path the CLI can be told. /// /// Absolute, because the CLI's working directory is the session's and the @@ -1556,4 +1614,147 @@ mod tests { assert!(received.try_recv().is_err()); assert!(queue.closed); } + + /// Both refusals verbatim from 2.1.237, and the exits that must *not* be + /// read as one: a session that merely failed still has a conversation, and + /// forgetting its token would discard the model's context for nothing. + #[test] + fn both_refused_resume_tokens_are_recognised() { + assert!(missing_conversation( + "No conversation found with session ID: a1c6c855-86cd-4f47-8b50-eaea85be3579" + )); + assert!(missing_conversation( + "Error: --resume requires a valid session ID or session title when used with \ + --print. Usage: claude -p --resume . Provided value \ + \"not-a-real-session\" is not a UUID and does not match any session title." + )); + assert!(!missing_conversation("Error: connection closed")); + assert!(!missing_conversation( + "Credit balance is too low to run this request" + )); + // The usage line alone is a different complaint: `--resume` was passed + // wrongly, not given a token that named nothing. + assert!(!missing_conversation( + "Error: --resume requires a valid session ID or session title when used with --print." + )); + } + + /// The composition the reader performs, not just the matcher: `stderr_tail` + /// both truncates and trims, so the phrase can survive the CLI and still + /// not reach `missing_conversation`. + #[test] + fn the_refusal_survives_the_stderr_tail_that_carries_it() { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join("stderr.log"); + // Verbatim from 2.1.237, trailing newline included -- a shell's error + // ends with one, which is exactly what `tail_of` exists to trim. + std::fs::write( + &path, + "No conversation found with session ID: a1c6c855-86cd-4f47-8b50-eaea85be3579\n", + ) + .expect("write stderr"); + assert!(missing_conversation(&stderr_tail(&path))); + + // And still found when noise precedes it, since the refusal is the last + // thing the CLI writes and the tail is taken from the end. + let mut noisy = "some earlier warning\n".repeat(STDERR_LINES_KEPT * 2); + noisy.push_str("No conversation found with session ID: a1c6c855\n"); + std::fs::write(&path, noisy).expect("write noisy stderr"); + assert!(missing_conversation(&stderr_tail(&path))); + } + + /// A dead process, a refusal in its stderr log, and a token on disk -- + /// driven through the reader that performs the recovery rather than through + /// its parts, because the wiring is what the parts cannot check. + async fn exit_with_stderr(stderr: &str) -> (tempfile::TempDir, Vec) { + let dir = tempfile::tempdir().expect("tempdir"); + // Empty but present: an unreadable stdout is a different path that + // returns before any of this. + std::fs::write(dir.path().join(STDOUT_LOG), "").expect("stdout log"); + std::fs::write(dir.path().join(STDERR_LOG), stderr).expect("stderr log"); + write_resume_token(dir.path(), "a1c6c855-86cd-4f47-8b50-eaea85be3579"); + + let (sink, mut events) = mpsc::unbounded_channel(); + follow( + dir.path().to_path_buf(), + // No `/proc` entry, so `liveness()` reads `Dead` -- the state this + // arm exists for, without having to kill anything. + process::Record { + pid: u32::MAX, + started: 0, + detail: process::Detail::Stdio { stdout_read: 0 }, + }, + 0, + Arc::new(Mutex::new(Translator::new( + dir.path().to_path_buf(), + Arc::new(Subagents::new(dir.path().to_path_buf())), + ))), + sink, + Arc::new(Mutex::new(Queue::default())), + Arc::new(AtomicBool::new(true)), + "claude-cli on vm".to_string(), + ) + .await; + let seen = std::iter::from_fn(|| events.try_recv().ok()).collect(); + (dir, seen) + } + + #[tokio::test] + async fn a_session_whose_conversation_vanished_is_left_able_to_start_again() { + let (dir, seen) = exit_with_stderr( + "No conversation found with session ID: a1c6c855-86cd-4f47-8b50-eaea85be3579\n", + ) + .await; + + let reported = seen + .iter() + .find_map(|event| match event { + Event::Error { message } => Some(message.clone()), + _ => None, + }) + .expect("the exit is reported"); + // The CLI's own words, and then what to do about them. + assert!( + reported.contains("No conversation found with session ID"), + "{reported}" + ); + assert!( + reported.contains("Send a message to carry on"), + "{reported}" + ); + // The divider, so the transcript says where the model's context ended. + assert!(seen.contains(&Event::Cleared), "{seen:?}"); + // And the point of all of it: the next start has no token to repeat. + assert_eq!(read_resume_token(dir.path()), None); + } + + /// The same reader on an exit that is *not* a refusal. The token is what + /// the session is still worth resuming from, so it has to survive. + #[tokio::test] + async fn an_ordinary_failure_keeps_the_token_it_can_still_resume_from() { + let (dir, seen) = exit_with_stderr("Error: connection closed\n").await; + + assert!(!seen.contains(&Event::Cleared), "{seen:?}"); + assert_eq!( + read_resume_token(dir.path()).as_deref(), + Some("a1c6c855-86cd-4f47-8b50-eaea85be3579") + ); + } + + #[test] + fn a_refused_token_is_forgotten_so_the_next_start_makes_a_session() { + let dir = tempfile::tempdir().expect("tempdir"); + write_resume_token(dir.path(), "a1c6c855-86cd-4f47-8b50-eaea85be3579"); + assert!(read_resume_token(dir.path()).is_some()); + + assert!(forget_resume_token(dir.path())); + + // Gone, so `launch` pushes `--name` instead of `--resume` and the CLI + // is asked for a session it can actually make. + assert_eq!(read_resume_token(dir.path()), None); + // Removing an absent one succeeds too -- the check and the remove are + // separate, so the file can go between them. + assert!(forget_resume_token(dir.path())); + assert_eq!(read_resume_token(dir.path()), None); + } }