diff --git a/AGENTS.md b/AGENTS.md index c07e0d3..a778e54 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -305,6 +305,24 @@ day: telling, so a failure there says the telling failed rather than the rename. So the Start button is for when you want a process and nothing to say to it yet. +- **A backend start adopts and starts nothing** (2026-08-30). It picks up + the processes still running and leaves every other session as it found + it: listed, with its transcript and its stream, reporting `exited`, with + no process and no driver until somebody asks for one. Restarting the + server used to relaunch a driver for every session, which started a CLI + for each one that had none — so a session stopped on purpose came back at + the next rebuild, and the `Idle` the new driver announced stamped every + row as active just now. If you are looking for a stopped session's + process after a restart, there is deliberately none; press Start, or send + it anything. +- **A launch never moves a session's clock.** A status it has to correct is + written at the time of the last thing the session actually did, not at + `now()`, and a session that has never done anything reports + `SessionConfig::created` rather than the clock — its transcript is empty, + since a driver announcing the state it starts in is not news, so there is + no line to read a time off. Both are the same rule as + `Transcript::last_activity`: a restart has been told nothing, so it must + not claim anything happened. - **Each session directory now holds `process.json`, `stdin.fifo`, `stdout.log` and `stderr.log`.** `stdout.log` is the driver's input, read from the byte offset in `process.json`; removing either by hand while the diff --git a/PLAN.md b/PLAN.md index 123e936..67c65e2 100644 --- a/PLAN.md +++ b/PLAN.md @@ -240,8 +240,9 @@ turn. Claude's dialect: a `user` message on stdin mid-stream; pi's: `steer`. installed CLI doesn't support it, fall back to `shutdown` + respawn with `--resume --model ` — cheap, since Claude persists sessions in `~/.claude/projects` anyway. That resume path is the recovery - story for a process that has genuinely died; a backend restart no longer - uses it, because the process is still there to be adopted (see below). + story for a process that has genuinely died; a backend restart never takes + it — it adopts the process that is still there, and starts nothing for the + session that has none (see below). **Resuming is only ever safe when nothing else has that session open.** - Images in: base64 image content blocks in the stream-json user message. - Working directory, host, and model are spawn-screen fields. @@ -420,6 +421,58 @@ running (interrupt — the process stays), a red stop when it is not (end the process), and a green play when it has exited (start it again). One button rather than three that come and go, so its presence is never the signal. +### A backend start adopts, and starts nothing (decided 2026-08-30) + +Starting the server is not something a session should be able to tell +happened. `SessionManager::new` takes charge of the processes that are still +running and **leaves every other session exactly as it found it** — listed, +with its transcript, its event pump and the SSE stream a phone reads, and no +driver at all until somebody asks for one. + +What it did before was launch a driver for every session in the config, and +`ClaudeDriver::launch` starts a process when there is none to adopt. So a +session somebody had deliberately stopped came back at the next rebuild, +which is the decision Stop exists to make being undone by an unrelated +event — and since a driver announces `Idle` for a process it started, the +session was also stamped as active at the moment of the restart. On the +phone that read as *every* session idle and "just now" after every restart, +with the list — sorted by that time — in an order that meant nothing. + +- **`Launching` is the parameter that says which it is**, and the seed an + import carries rides on the asked-for variant, because a restart re-seeding + a transcript would write the imported conversation into it twice. +- **A session with no process has no driver.** `DriverCell` is an option + rather than a driver whose requests go nowhere, so "nothing is running + this" is a state the code can be asked about instead of one it discovers by + sending into a dead fifo. `LiveSession::ask` is the one place that answers + it, with an `Event::Error` naming what could not happen — a request nobody + can carry out is reported, never swallowed. +- **`--resume` on a crashed session is now a press rather than a restart.** + That is the whole of what is given up, and it is small: a session whose CLI + died reports `Exited` and draws the Start button, and *sending it anything + at all* starts it (above). What is bought is that the two are told apart by + who asked, rather than a restart guessing that everything it found should be + running. +- **What a launch settles the status to is written into the transcript, at + the time of the last thing the session actually did.** Adopting, the + transcript's word stands except for the `Exited` a live process disproves. + Taking charge of nothing, every word but `Exited` is disproved at once — a + backend killed mid-turn leaves a transcript saying `Running`, and that + draws a stop button for a turn that ended hours ago. The correction goes in + the transcript because the list reads the manager's status and the session + screen replays the file; it is stamped with the transcript's own last time + because it is not something the session did — this server noticed, at a + moment of its own choosing, and `now` there is the same lie in the same + field that `Transcript::last_activity` exists to prevent. +- **A session that has never done anything reports when it was created.** Its + transcript is empty — a driver announcing the state it starts in is not + news, so nothing is written — which makes it the one session with no line to + read a time off. The clock was the fallback, so a session nobody had sent + anything to climbed to the top of the list at every restart. Not the + transcript file's mtime, which is the same instant for an empty file and a + worse answer for a shared checkout that can be copied or touched; + `SessionConfig::created` is recorded rather than inferred. + ### Importing refuses a session that is already open (decided 2026-08-29) Claude Code keeps a descriptor per live session at diff --git a/server/src/session/mod.rs b/server/src/session/mod.rs index 5daaf08..86d8c0b 100644 --- a/server/src/session/mod.rs +++ b/server/src/session/mod.rs @@ -169,7 +169,7 @@ pub struct SessionInfo { pub created: f64, } -/// What is running a session at this moment. +/// What is running a session at this moment, and `None` when nothing is. /// /// Behind a lock because a session outlives its process: stopping one and /// starting it again replaces the driver while the transcript, the event @@ -177,7 +177,15 @@ pub struct SessionInfo { /// were. Shared with [`Commands`] rather than copied into it, because two /// holders of "the driver" are two answers to that question the moment one /// of them is replaced. -type DriverCell = Arc>>; +/// +/// An option because a session outlives its process in the other direction +/// too: one that was stopped, or whose process died while this server was +/// down, is a session with a transcript, a pump and a phone reading it, and +/// nothing running it. A driver is how a process is spoken to, so where +/// there is no process there is no driver -- rather than a driver whose +/// requests go nowhere, which is the same thing with nobody able to say so. +/// See [`Launching`]. +type DriverCell = Arc>>>; /// A running session: its driver plus the shared state the event pump /// keeps current. Cheap to clone-by-`Arc` into request handlers. @@ -211,8 +219,9 @@ struct Commands { } impl Commands { - /// Whatever is driving the session now -- see [`DriverCell`]. - fn driver(&self) -> Arc { + /// Whatever is driving the session now, if anything -- see + /// [`DriverCell`]. + fn driver(&self) -> Option> { self.driver.lock().unwrap().clone() } @@ -251,7 +260,16 @@ impl Commands { }); return; } - let driver = self.driver(); + // The same answer for the same reason one step earlier: a session + // with no driver has no process to have a boundary. The status + // above is what says so in the ordinary case; this is the session + // whose process went between that word being written and now. + let Some(driver) = self.driver() else { + let _ = self.sink.send(Event::Error { + message: format!("this session has no process running, so it can't run {text}"), + }); + return; + }; if driver.between_turns() { let _ = self.sink.send(Event::CommandSent { id, text }); command.apply(driver.as_ref()); @@ -274,7 +292,12 @@ impl Commands { /// began by itself, which it does: a background task finishing makes it /// pick the conversation back up with nothing written to it. fn take_one(&self) { - let driver = self.driver(); + // Nothing to run it against. Held rather than abandoned: what ends + // a session's process announces `Exited`, and that is what empties + // the queue -- see `abandon`. + let Some(driver) = self.driver() else { + return; + }; if !driver.between_turns() { return; } @@ -353,11 +376,38 @@ struct Shared { } impl LiveSession { - /// Whatever is driving this session now -- see [`DriverCell`]. - fn driver(&self) -> Arc { + /// Whatever is driving this session now, if anything -- see + /// [`DriverCell`]. + fn driver(&self) -> Option> { self.driver.lock().unwrap().clone() } + /// Asks whatever is running this session to do something, and says so + /// when nothing is. + /// + /// Every caller here is relaying a request from a person, and a + /// request that reaches no process has to be reported rather than + /// swallowed: `Event::Error` is where the session screen shows what + /// did not happen, and silence would leave somebody watching for a + /// reply to a message nothing was ever given. The requests that mean + /// "do this now" start a process before they get here -- see + /// [`SessionManager::start_if_exited`] -- so what lands in the `None` + /// arm is the one that arrived just as the process went, or one aimed + /// at a session nobody has started. + /// + /// `what` completes "this session has no process running, so it + /// can't ...". + fn ask(&self, what: &str, request: impl FnOnce(&dyn Driver)) { + match self.driver() { + Some(driver) => request(driver.as_ref()), + None => { + let _ = self.sink.send(Event::Error { + message: format!("this session has no process running, so it can't {what}"), + }); + } + } + } + /// Hands the user's message to the driver, which records it in the /// transcript by reporting that it has taken it -- see `MessageTaken`. /// @@ -370,7 +420,9 @@ impl LiveSession { // drew a person's screenshot as a row floating above the bubble // that sent it, and left the phone inferring from adjacency which // message an image went with -- a thing the sender already knew. - self.driver().send_user_message(text, images); + self.ask("take a message", |driver| { + driver.send_user_message(text, images) + }); } pub fn answer_question(&self, question_id: &str, answers: &[String]) { @@ -378,18 +430,25 @@ impl LiveSession { id: question_id.to_string(), answers: answers.to_vec(), }); - self.driver().answer_question(question_id, answers); + self.ask("answer that", |driver| { + driver.answer_question(question_id, answers) + }); } pub fn interrupt(&self) { - self.driver().interrupt(); + self.ask("be interrupted", |driver| driver.interrupt()); } /// Leaves this session's process running and stops attending to it, /// for a server that is going away and means to come back. See /// [`Driver::detach`]. pub fn detach(&self) { - self.driver().detach(); + // Nothing to let go of is not worth reporting: this is the server + // shutting down, and a session with no process is already in the + // state detaching leaves one in. + if let Some(driver) = self.driver() { + driver.detach(); + } } pub fn subscribe(&self) -> broadcast::Receiver { @@ -475,11 +534,15 @@ pub struct SessionManager { } impl SessionManager { - /// Loads the config and relaunches a driver for every persisted - /// session -- for the real drivers that is the `--resume`/session-file - /// crash-recovery story; the echo driver just starts fresh over the - /// same transcript. Must be called inside a tokio runtime (each - /// session spawns its event pump). + /// Loads the config and brings every persisted session back: its + /// transcript, its event pump, and the process it left running, where + /// it left one. Sessions with no process are listed as what they are + /// and nothing is started for them -- see [`Launching`], which is the + /// difference between a backend that restarts and one that restarts + /// everything it finds. + /// + /// Must be called inside a tokio runtime (each session spawns its + /// event pump). pub fn new(config_path: PathBuf, data_dir: PathBuf, models_dir: PathBuf) -> Result { let config = Config::load(&config_path)?; wg_app_link::private::create_dir(&data_dir)?; @@ -498,8 +561,11 @@ impl SessionManager { &provider, &data_dir, &models_dir, - None, notifications.clone(), + // Nothing is started here. See `Launching`: a restart + // picks up the processes that are still running and + // leaves the rest as it found them. + Launching::Restart, ) }) { Ok(session) => { @@ -898,8 +964,8 @@ impl SessionManager { &provider, &self.data_dir, &self.models_dir, - seed, self.notifications.clone(), + Launching::Asked(seed), )?; let mut candidate = inner.config.clone(); candidate.sessions.push(meta); @@ -950,7 +1016,9 @@ impl SessionManager { // change, and as an error if it cannot. The config above is a // different question -- what to launch this session with next // time -- and it is answered by the request. - session.driver().set_permission_mode(mode); + session.ask("change how much it asks", |driver| { + driver.set_permission_mode(mode) + }); } Ok(()) } @@ -1054,7 +1122,7 @@ impl SessionManager { if let Some(session) = inner.live.get(id) { // See `set_session_permission_mode`: the driver reports what // it is set to, this only asks. - session.driver().set_model(model); + session.ask("change model", |driver| driver.set_model(model)); } Ok(()) } @@ -1254,8 +1322,10 @@ impl SessionManager { // the tasks that do it. Its process has exited -- that is // how this line was reached -- so there is nothing left to // preserve and `detach` is the whole of what it is owed. - session.driver().detach(); - *session.driver.lock().unwrap() = make_driver( + if let Some(driver) = session.driver() { + driver.detach(); + } + *session.driver.lock().unwrap() = Some(make_driver( &meta, &setup, &provider, @@ -1263,7 +1333,7 @@ impl SessionManager { session.dir(), session.transcript_path(), &session.sink, - )?; + )?); } // Nothing is live for this one -- a session whose launch failed // when the server started, which has no pump either. That is the @@ -1275,8 +1345,8 @@ impl SessionManager { &provider, &self.data_dir, &self.models_dir, - None, self.notifications.clone(), + Launching::Asked(None), )?; inner.live.insert(id.to_string(), session); } @@ -1305,7 +1375,9 @@ impl SessionManager { // Stopped, not detached: this is the one exit where the // process must not survive, because the conversation it // belongs to is being removed. See `Driver::stop`. - session.driver().stop(); + if let Some(driver) = session.driver() { + driver.stop(); + } } let dir = self.data_dir.join(id); if dir.exists() { @@ -1354,25 +1426,41 @@ impl SessionManager { /// reader -- three presses put three interleaved copies of one reply on /// screen. fn corrected(status: SessionStatus, session_dir: &Path) -> SessionStatus { - if status != SessionStatus::Exited { - return status; - } - match process::recorded(session_dir) { - Some((_, process::Liveness::Alive | process::Liveness::Unknown)) => SessionStatus::Unknown, - Some((_, process::Liveness::Dead)) | None => SessionStatus::Exited, + if status == SessionStatus::Exited && adoptable(session_dir) { + SessionStatus::Unknown + } else { + status } } 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, + if adoptable(session_dir) { + SessionStatus::Unknown + } else { + SessionStatus::Exited } } +/// Whether this session has a process worth taking charge of. +/// +/// "Running" and "this machine will not say" are one answer here, and that +/// is the module's central rule wearing its third hat: starting a second +/// CLI against a conversation that may already have one is the expensive +/// fault, so anything short of *known to be gone* is treated as a process. +/// `None` -- nothing ever recorded -- is an echo session, or one whose +/// process was stopped and cleaned up: gone, and known to be. +/// +/// One function because it is one question asked in three places: what a +/// [`launch`] can adopt, what a session nobody launched reports, and which +/// `Exited` is a lie. Three copies of it would be three chances to answer +/// the same thing differently. +fn adoptable(session_dir: &Path) -> bool { + matches!( + process::recorded(session_dir), + Some((_, process::Liveness::Alive | process::Liveness::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. @@ -1448,9 +1536,6 @@ fn unique_id(config: &Config) -> String { } } -/// Creates the session directory, opens its transcript (continuing the -/// sequence numbering if one exists), starts the driver, and spawns the -/// event pump connecting them. /// Keeps an imported session's transcript level with the file the CLI /// writes. /// @@ -1546,14 +1631,50 @@ pub struct Seed { pub records: String, } +/// Why a session is being launched, which is what decides whether a +/// process may be started for one that has none. +/// +/// That distinction is the whole of what a backend restart is allowed to do +/// to the sessions it finds, and starting the server is not something a +/// session should be able to tell happened. A session whose process is gone +/// is usually gone because somebody pressed Stop, so starting one back +/// because the server was rebuilt undoes that decision silently -- and, +/// since a driver announces `Idle` for a process it started, it also moves +/// the session's last-activity time to the restart, so every row on the +/// phone reads "just now" and a list sorted by that time means nothing. +/// +/// What starts a process is somebody asking for one: spawning a session, +/// pressing Start, or sending it anything at all -- see +/// [`SessionManager::start_if_exited`], which is the one place that +/// decides. +/// +/// The import's history rides on the asked-for variant rather than beside +/// it because it belongs to exactly that case: a seed is a session being +/// created, and a restart re-seeding a transcript would write the imported +/// conversation into it a second time. +enum Launching { + /// Somebody asked for this session to be running -- it was just + /// spawned, or its Start button was pressed. Takes charge of a process + /// that is running and starts one where there is none. + Asked(Option), + /// The backend has just started. Takes charge of the processes that are + /// still running and leaves every other session exactly as it was + /// found, with no driver at all. + Restart, +} + +/// Creates the session directory, opens its transcript (continuing the +/// sequence numbering if one exists), settles what the session is doing, +/// and spawns the event pump -- with a driver behind it where there is a +/// process for it to speak to. See [`Launching`] for when that is. fn launch( meta: SessionConfig, setup: &SetupConfig, provider: &ProviderConfig, data_dir: &Path, models_dir: &Path, - seed: Option, notifications: broadcast::Sender, + why: Launching, ) -> Result> { let dir = data_dir.join(&meta.id); wg_app_link::private::create_dir(&dir)?; @@ -1562,7 +1683,7 @@ fn launch( let last_status = transcript.last_status().unwrap_or(SessionStatus::Idle); // Before the driver starts, so the token is there when it looks and // the history is already in the transcript a phone will read. - if let Some(seed) = seed { + if let Launching::Asked(Some(seed)) = &why { claude::write_resume_token(&dir, &seed.resume); import::write_cursor(&dir, &seed.cursor); let at = now(); @@ -1571,23 +1692,71 @@ fn launch( } } + // Whether this launch is to have a process behind it. Answered before + // anything else is built, because it is also what the session's status + // is: a driver is how a process is spoken to, and a session with + // neither is one somebody has to start. + let driving = match why { + Launching::Asked(_) => true, + Launching::Restart => adoptable(&dir), + }; + + // What this server can say the session is, which is not always what + // the transcript last said about it. + // + // Adopting, the transcript's word stands except for the one that a + // live process disproves -- see `corrected`. Taking charge of nothing, + // every word except `Exited` is disproved at once: `Idle` and + // `Running` are claims about a process, and this session has none, so + // a transcript left saying `Running` by a backend that was killed + // mid-turn would otherwise draw a stop button for a turn that ended + // hours ago. + let status = if driving { + corrected(last_status, &dir) + } else { + SessionStatus::Exited + }; + // Written into the transcript rather than sent through the sink, and + // written at the time of the last thing the session actually did. + // + // In the transcript because the session list reads the status below + // and the session screen replays the transcript, so a correction that + // reaches one of them is two screens describing one session + // differently -- which is what a stop button that turns into a play + // button a moment after the screen opens is. + // + // At the old time because this is not something the session did. It is + // this server noticing, at a moment of its own choosing, and stamping + // it `now` says the session was active the instant the server started + // -- the same lie in the same field that `Transcript::last_activity` + // exists to prevent, arriving by the other route. + if status != last_status { + let at = transcript.last_activity().unwrap_or(meta.created); + transcript.append(Event::Status { state: status }, at)?; + } + let (sink, source) = mpsc::unbounded_channel(); let (events, _) = broadcast::channel(EVENT_BUFFER); let shared = Arc::new(Shared { // What it was last known to be doing, not an assumption. A driver // 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. Where it is *not* - // the true answer, the correction goes through the sink below - // rather than being written here, so that it reaches the - // transcript too -- see there. - status: Mutex::new(last_status), + // this is then the only true answer available. + status: Mutex::new(status), title: Mutex::new(meta.title.clone()), // What the transcript last recorded, not the clock: this server has // just been told nothing, and `now()` claimed every relaunched // session had been active this instant -- see // `Transcript::last_activity`. - last_activity: Mutex::new(transcript.last_activity().unwrap_or_else(now)), + // + // A session that has never done anything has an empty transcript, + // and its answer is when it was created rather than when this + // server last started. The clock was the fallback here, which meant + // a session nobody had sent anything to climbed back to the top of + // a list sorted by activity at every rebuild -- the same lie in the + // same field, reached by the one route that had no line to read it + // from. + last_activity: Mutex::new(transcript.last_activity().unwrap_or(meta.created)), model: Mutex::new(meta.model.clone()), permission_mode: Mutex::new(meta.permission_mode.clone()), context_tokens: Mutex::new(transcript.context_tokens()), @@ -1635,31 +1804,21 @@ fn launch( ); } - // Before the driver, because the driver may start a process and say so, - // and that has to be the later word of the two. - // - // Sent rather than written into `shared`: the session screen replays the - // transcript and the session list reads `shared`, so a correction made - // in only one of them is the two of them describing one session - // differently -- which is how the phone came to show a Start button on a - // running session in the first place. One event, and the pump puts it in - // both. - let corrected_status = corrected(last_status, &dir); - if corrected_status != last_status { - let _ = sink.send(Event::Status { - state: corrected_status, - }); - } - - let driver = Arc::new(Mutex::new(make_driver( - &meta, - setup, - provider, - models_dir, - &dir, - &transcript_path, - &sink, - )?)); + let driver = Arc::new(Mutex::new( + driving + .then(|| { + make_driver( + &meta, + setup, + provider, + models_dir, + &dir, + &transcript_path, + &sink, + ) + }) + .transpose()?, + )); let commands = Arc::new(Commands { driver: Arc::clone(&driver), @@ -1979,10 +2138,10 @@ mod tests { let dir = tempfile::tempdir().expect("tempdir"); let (sink, mut events) = mpsc::unbounded_channel(); let commands = Commands { - driver: Arc::new(Mutex::new(Arc::new(EchoDriver::new( + driver: Arc::new(Mutex::new(Some(Arc::new(EchoDriver::new( sink.clone(), dir.path().to_path_buf(), - )))), + ))))), sink, waiting: Mutex::new(VecDeque::new()), }; @@ -2569,6 +2728,148 @@ mod tests { assert_eq!(manager.sessions()[0].context_tokens, None); } + /// A session that has never done anything says when it was made, not + /// when this server last started. + /// + /// Its transcript is empty -- a driver announcing the state it starts + /// in is not news, so nothing is written -- which makes it the one + /// session with no line to read a time off. The clock was the fallback, + /// so every session nobody had sent anything to climbed back to the top + /// of a list sorted by activity at every rebuild, reporting a moment + /// nothing happened in. + #[tokio::test] + async fn a_session_that_has_done_nothing_reports_when_it_was_made() { + let dir = tempfile::tempdir().expect("tempdir"); + let config_path = dir.path().join("config.ron"); + let data_dir = dir.path().join("sessions"); + seed_echo_only(&config_path); + + let manager = SessionManager::new( + config_path.clone(), + data_dir.clone(), + data_dir.join("models"), + ) + .expect("manager"); + let info = manager.spawn_session(echo_spec()).expect("spawn"); + assert_eq!(info.last_activity, info.created); + drop(manager); + + let manager = SessionManager::new(config_path, data_dir.clone(), data_dir.join("models")) + .expect("manager restart"); + let listed = manager.sessions(); + assert_eq!( + listed[0].last_activity, info.created, + "a session that has done nothing reported {} rather than the {} it was created at", + listed[0].last_activity, info.created, + ); + } + + /// Starting the backend is not something a session should be able to + /// tell happened. + /// + /// Two halves of one question, because a restart meets sessions in two + /// states and used to get both of them wrong in the same direction. A + /// process that is still there is adopted and nothing is said about it. + /// A session that has *no* process -- somebody pressed Stop, or the CLI + /// died while this server was down -- is left alone: relaunching it + /// started a second CLI on the conversation, which is exactly what Stop + /// was pressed to prevent, and the `Idle` the new driver announced + /// stamped the session as active at the moment of the restart. On the + /// phone that was every session reading "idle, just now" after every + /// rebuild, with the list -- sorted by that time -- in an order that + /// meant nothing. + /// + /// Echo is the session with nothing to adopt: it never records a + /// process, which is the same thing a stopped one leaves behind. + #[tokio::test] + async fn a_restart_starts_nothing_and_moves_no_clock() { + let dir = tempfile::tempdir().expect("tempdir"); + let config_path = dir.path().join("config.ron"); + let data_dir = dir.path().join("sessions"); + seed_echo_only(&config_path); + + let manager = SessionManager::new( + config_path.clone(), + data_dir.clone(), + data_dir.join("models"), + ) + .expect("manager"); + let info = manager.spawn_session(echo_spec()).expect("spawn"); + let session = manager.session(&info.id).expect("live session"); + let mut rx = session.subscribe(); + session.send_message("something".to_string(), Vec::new()); + collect_turn(&mut rx).await; + drop(rx); + drop(session); + drop(manager); + + let session_dir = data_dir.join(&info.id); + let transcript = session_dir.join("transcript.jsonl"); + // Far enough back that a restart taking the clock cannot pass by + // being fast, as in the test above. + let long_ago = now() - 86_400.0; + rewrite_transcript_times(&transcript, long_ago); + + // A process still running: this test's own, which is the one + // certain to be there when the launch looks. + let record = process::Record::of( + std::process::id(), + process::Detail::Stdio { stdout_read: 0 }, + ) + .expect("record this process"); + process::write(&session_dir, &record); + + let manager = SessionManager::new( + config_path.clone(), + data_dir.clone(), + data_dir.join("models"), + ) + .expect("manager restart"); + let listed = manager.sessions(); + assert_eq!(listed[0].status, SessionStatus::Idle); + assert!( + (listed[0].last_activity - long_ago).abs() < 1.0, + "adopting a running process reported {} instead of the {long_ago} the transcript \ + records", + listed[0].last_activity, + ); + drop(manager); + + // And now the same session with nothing to adopt, which is what a + // stopped one looks like. + process::clear(&session_dir); + let manager = SessionManager::new(config_path, data_dir.clone(), data_dir.join("models")) + .expect("manager restart"); + let listed = manager.sessions(); + assert_eq!( + listed[0].status, + SessionStatus::Exited, + "a session with no process was reported as though something were running it", + ); + assert!( + (listed[0].last_activity - long_ago).abs() < 1.0, + "a session nothing has run since reported {} instead of the {long_ago} the \ + transcript records", + listed[0].last_activity, + ); + // The same word in the transcript, at the same time: the list reads + // the status above and the session screen replays the file, and a + // correction that reaches one of them is two screens describing one + // session differently. + let reopened = Transcript::open(&transcript).expect("reopen transcript"); + assert_eq!(reopened.last_status(), Some(SessionStatus::Exited)); + assert!( + (reopened.last_activity().expect("lines") - long_ago).abs() < 1.0, + "the correction was written at the clock rather than at the time of the last thing \ + the session did", + ); + // Nothing was started, so the way back is asking for one. + let mut rx = manager.session(&info.id).expect("live session").subscribe(); + manager.start_session(&info.id).expect("start again"); + collect_until(&mut rx, is_idle).await; + assert_eq!(manager.sessions()[0].status, SessionStatus::Idle); + } + /// Backdates every line in a transcript, so a restart has something to /// report that the clock could not have produced. fn rewrite_transcript_times(path: &Path, ts: f64) { @@ -2880,7 +3181,13 @@ mod tests { assert_eq!(listed[0].id, info.id); let session = manager.session(&info.id).expect("relaunched session"); let mut rx = session.subscribe(); - session.send_message("second".to_string(), Vec::new()); + // Through the manager, which is the message path a phone takes and + // the one that starts a process for a session that has none -- see + // `Launching`. A restart adopts what is running and starts nothing, + // and echo has nothing to adopt. + manager + .send_message(&info.id, "second".to_string(), Vec::new()) + .expect("send after restart"); let seen = collect_turn(&mut rx).await; assert!(seen.first().expect("events").seq > last_seq); } diff --git a/server/src/session/transcript.rs b/server/src/session/transcript.rs index f2f841d..249844d 100644 --- a/server/src/session/transcript.rs +++ b/server/src/session/transcript.rs @@ -97,8 +97,10 @@ impl Transcript { /// untouched for days. /// /// `None` for a transcript with no lines in it, which is a session that - /// genuinely has not done anything yet; its caller uses the clock, which - /// is right there and only there. + /// genuinely has not done anything yet. Its caller answers that with + /// when the session was created -- not with the clock, which would say + /// a session nobody has ever sent anything to was active a moment ago, + /// every time this server started. pub fn last_activity(&self) -> Option { self.last_activity }