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");