From fc71cb440315335a1bd137c8b1738d7c2ba89d45 Mon Sep 17 00:00:00 2001 From: iris <2+iris@noreply.localhost> Date: Sat, 29 Aug 2026 14:50:20 -0400 Subject: [PATCH] Say "unknown" for a session we are not driving but cannot bury A session in the config with no live entry reported `Exited`, whatever the reason. That covers three different situations -- one that genuinely ended, one that failed to relaunch, and one whose process could not be checked -- and the wrong one is the expensive one. `Exited` reads as "this conversation is over", and what a reader does about it is start a fresh session. If the process is in fact still running, that is a second CLI against a conversation that already has one: the exact fault `session::process` exists to prevent, arriving through the status field instead of through a spawn. So it is said only when the process is known to be gone. A record that cannot be checked reports `Unknown`, and so does one that is still alive -- this server is not driving it, so it genuinely does not know what that process is doing, and the honest word is the one meaning "wait" rather than the one meaning "act". A session with no record at all is still `Exited`: an echo session, or one already stopped and cleaned up, and known to be. The distinction was available all along -- `process::recorded` returns the liveness -- which makes this the same mistake as the other five today: reporting what was convenient to compute rather than what was measured. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01VETa8afmpWaYezLCqJhDB8 --- server/src/session/mod.rs | 60 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 59 insertions(+), 1 deletion(-) diff --git a/server/src/session/mod.rs b/server/src/session/mod.rs index c9e1023..6a5bce6 100644 --- a/server/src/session/mod.rs +++ b/server/src/session/mod.rs @@ -539,7 +539,7 @@ impl SessionManager { permission_mode: meta.permission_mode.clone(), imported: import::read_cursor(&self.data_dir.join(&meta.id)).is_some(), cwd: meta.cwd.clone(), - status: SessionStatus::Exited, + status: status_of_unlaunched(&self.data_dir.join(&meta.id)), last_activity: meta.created, created: meta.created, }, @@ -727,6 +727,32 @@ impl SessionManager { } } +/// What to report for a session that is in the config but has no live +/// entry -- one that failed to relaunch, or whose process this server +/// never took charge of. +/// +/// This said `Exited` for all of them, which is the enumeration mistake in +/// its most consequential form. `Exited` reads as "this conversation is +/// over", and the thing a reader does about it is start a new session -- +/// which, if the process is in fact still running, is a second CLI against +/// a conversation that already has one. That is the fault the whole +/// `process` module exists to prevent, arriving through the status field. +/// +/// So it is only said when the process is known to be gone. A record that +/// cannot be checked reports `Unknown`, and a record that is still alive +/// reports `Unknown` too: this server is not driving it, so it genuinely +/// does not know what it is doing -- and that is worth a word that means +/// "wait", not one that means "act". +fn status_of_unlaunched(session_dir: &Path) -> SessionStatus { + match process::recorded(session_dir) { + // Nothing was ever recorded: an echo session, or one whose + // process was stopped and cleaned up. Gone, and known to be. + None => SessionStatus::Exited, + Some((_, process::Liveness::Dead)) => SessionStatus::Exited, + Some((_, process::Liveness::Alive | process::Liveness::Unknown)) => SessionStatus::Unknown, + } +} + /// The provider and host a session's config names, or a message saying /// which one is missing. Both are looked up fresh at every launch, so /// editing either takes effect on the next respawn. @@ -1081,6 +1107,38 @@ mod tests { .expect("seed config"); } + #[test] + fn a_session_we_are_not_driving_says_exited_only_when_it_is_gone() { + let dir = tempfile::tempdir().expect("tempdir"); + let session = dir.path().join("s1"); + std::fs::create_dir_all(&session).expect("mkdir"); + + // Nothing recorded: an echo session, or one already cleaned up. + assert_eq!(status_of_unlaunched(&session), SessionStatus::Exited); + + // A record naming a process that is definitely gone. + process::write( + &session, + &process::Record { + pid: 0, + started: 1, + detail: process::Detail::Stdio { stdout_read: 0 }, + }, + ); + assert_eq!(status_of_unlaunched(&session), SessionStatus::Exited); + + // A record naming a process that is definitely there, which this + // server is nonetheless not driving. `Exited` here would invite + // starting a second one against the same conversation. + let live = process::Record::of( + std::process::id(), + process::Detail::Stdio { stdout_read: 0 }, + ) + .expect("start time"); + process::write(&session, &live); + assert_eq!(status_of_unlaunched(&session), SessionStatus::Unknown); + } + #[tokio::test] async fn spawn_message_and_delete_round_trip() { let dir = tempfile::tempdir().expect("tempdir");