diff --git a/server/src/session/mod.rs b/server/src/session/mod.rs index 2830de5..24af992 100644 --- a/server/src/session/mod.rs +++ b/server/src/session/mod.rs @@ -906,9 +906,7 @@ fn launch( // that has something to say corrects this within its first poll; // one adopting a process that has been quiet says nothing, and // this is then the only true answer available. - status: Mutex::new( - transcript::last_status(&transcript_path).unwrap_or(SessionStatus::Idle), - ), + status: Mutex::new(transcript.last_status().unwrap_or(SessionStatus::Idle)), last_activity: Mutex::new(now()), model: Mutex::new(meta.model.clone()), permission_mode: Mutex::new(meta.permission_mode.clone()), diff --git a/server/src/session/transcript.rs b/server/src/session/transcript.rs index 1f1caa7..489356d 100644 --- a/server/src/session/transcript.rs +++ b/server/src/session/transcript.rs @@ -30,13 +30,23 @@ pub struct SeqEvent { pub struct Transcript { file: File, next_seq: u64, + last_status: Option, } impl Transcript { /// Opens (or creates) the log at `path`, continuing the sequence from /// the last line if one exists. pub fn open(path: &Path) -> Result { - let last_seq = last_seq(path)?; + // One pass for both answers. They are wanted at the same moment by + // the same caller, and reading the file twice to get them doubled + // the cost of starting every session -- which is paid per session, + // at the point a restart is trying to be quick. + let existing = read_after(path, 0)?; + let last_seq = existing.last().map(|entry| entry.seq).unwrap_or(0); + let last_status = existing.iter().rev().find_map(|entry| match entry.event { + Event::Status { state } => Some(state), + _ => None, + }); // Owner-only: a transcript is the whole conversation, including // whatever the session read, wrote, or was told. let file = OpenOptions::new() @@ -48,9 +58,24 @@ impl Transcript { Ok(Self { file, next_seq: last_seq + 1, + last_status, }) } + /// The state the session was last reported to be in, as of opening. + /// + /// Read from the file rather than assumed, because a server that has + /// just restarted has been told nothing yet and this is the only thing + /// it knows. Assuming idle claimed a session was waiting for you when + /// it had exited hours earlier, and would now also claim it of one + /// whose process is still mid-turn. + /// + /// `None` for a transcript that never carried a status, which is a new + /// session and genuinely has no prior state. + pub fn last_status(&self) -> Option { + self.last_status + } + /// Appends `event`, assigning it the next sequence number. Flushed per /// event: each line is tiny, and the transcript is the source of truth /// a crash must not lose the tail of. @@ -158,34 +183,6 @@ pub fn read_after(path: &Path, after: u64) -> Result> { Ok(events) } -/// The state the session was last reported to be in. -/// -/// Read from the transcript rather than assumed at launch, because a -/// server that has just restarted has been told nothing yet and the last -/// thing written down is the only thing it knows. Assuming idle claimed a -/// session was waiting for you when it had exited hours earlier, and would -/// now also claim it of one whose process is still mid-turn. -/// -/// `None` for a transcript that has never carried a status, which is a new -/// session and genuinely has no prior state. -pub fn last_status(path: &Path) -> Option { - read_after(path, 0) - .ok()? - .into_iter() - .rev() - .find_map(|entry| match entry.event { - Event::Status { state } => Some(state), - _ => None, - }) -} - -fn last_seq(path: &Path) -> Result { - Ok(read_after(path, 0)? - .last() - .map(|entry| entry.seq) - .unwrap_or(0)) -} - #[cfg(test)] mod tests { use super::*; @@ -267,6 +264,42 @@ mod tests { )); } + #[test] + fn reopening_reports_the_state_it_was_last_left_in() { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join("transcript.jsonl"); + + // Nothing recorded yet: no prior state to report, which is not the + // same as reporting idle. + assert_eq!(Transcript::open(&path).expect("open").last_status(), None); + + let mut transcript = Transcript::open(&path).expect("open"); + transcript + .append( + Event::Status { + state: SessionStatus::Running, + }, + 1.0, + ) + .expect("append"); + transcript + .append( + Event::Status { + state: SessionStatus::Exited, + }, + 2.0, + ) + .expect("append"); + // Events after the last status must not hide it. + transcript.append(text("trailing"), 3.0).expect("append"); + drop(transcript); + + let reopened = Transcript::open(&path).expect("reopen"); + assert_eq!(reopened.last_status(), Some(SessionStatus::Exited)); + // And the same pass still continues the numbering. + assert_eq!(reopened.next_seq, 4); + } + #[test] fn a_missing_file_reads_as_empty() { let dir = tempfile::tempdir().expect("tempdir");