From 4a122f7b2565a1e78792f2ec691fd0a43de4f827 Mon Sep 17 00:00:00 2001 From: iris <2+iris@noreply.localhost> Date: Sun, 30 Aug 2026 13:56:28 -0400 Subject: [PATCH] Start a stopped session's process when a message is sent to it Refusing was work handed back: read the status word, find the other button, press it, type the message again. Sending plainly means "do this now", and `--resume` puts the new process on the same conversation, so nothing about the message changes -- only whether there was anything there to read it. `POST /sessions/{id}/message` now goes through the manager, which starts a process first when the session is known to have exited. Only on `exited`: `unknown` has a process that may well be reading its fifo, and starting a second CLI on that guess is the fault `session::process` exists to prevent, so the message goes to the driver as it always did. The Start button and this ask one function, `start_if_exited`, and want opposite answers from it -- "there is already a process" is a refusal worth showing to somebody who pressed Start, and nothing at all to a message being sent. Deciding it in one place under the one write lock is also what keeps two requests that arrive together from starting two CLIs. Verified over the API and on the emulator: with the session reporting `exited` and the composer showing a play button, typing a message and pressing Send started the process, delivered the message and ran the turn -- transcript order `idle`, `userMessage`, `running`, `idle` -- and the button became a stop. Co-Authored-By: Claude Opus 5 --- AGENTS.md | 6 ++ PLAN.md | 14 +++++ server/src/routes.rs | 10 ++- server/src/session/mod.rs | 124 +++++++++++++++++++++++++++++++++----- 4 files changed, 137 insertions(+), 17 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 96eccf9..8762055 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -291,6 +291,12 @@ day: which keeps the session and its transcript, and `POST .../start` brings the process back on the same conversation — or delete the session, which ends the conversation too. +- **Sending a message to a stopped session starts it.** `POST + .../message` goes through `SessionManager::send_message`, which starts a + process first when the session is known to have exited and then delivers + the message to the driver that has one behind it. Only on `exited`: + `unknown` has a process that may well be reading its fifo. So the Start + button is for when you want a process and nothing to say to it yet. - **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 d069af7..7b1c30f 100644 --- a/PLAN.md +++ b/PLAN.md @@ -375,6 +375,20 @@ Two rules come out of it, and neither is optional: existed for the backend going away — and it is the whole of what a driver whose process has exited is owed. +**Sending a message starts the process if there isn't one** (decided +2026-08-30). Refusing was work handed back: read the status word, find the +other button, press it, type the message again. Sending plainly means "do +this now", and `--resume` puts the new process on the same conversation, so +nothing about the message changes — only whether there was anything there to +read it. The manager's `send_message` and the Start button ask one function +(`start_if_exited`) and want opposite answers from it: "there is already a +process" is a refusal worth showing to somebody who pressed Start, and +nothing at all to a message. Deciding it in one place under one write lock is +also what stops two requests arriving together from starting two CLIs. Only +`Exited` starts anything, for the reason above — `Unknown` has a process that +may well be reading its fifo, and the message goes to the driver as it always +did. + The phone's half is that the process button is disabled while its own request is in flight, so a second press cannot be decided against a status the first one has not changed yet. That is a courtesy rather than the fix: the server diff --git a/server/src/routes.rs b/server/src/routes.rs index a369fff..7de102f 100644 --- a/server/src/routes.rs +++ b/server/src/routes.rs @@ -16,6 +16,7 @@ //! (a backlog past CATCH_UP_LIMIT arrives as a //! `reset` frame plus the newest window) //! POST /sessions/{id}/message {text, attachmentIds?} +//! (starts the process first if it has exited) //! POST /sessions/{id}/answer {questionId, answers} (questions and permissions) //! POST /sessions/{id}/interrupt stop the running turn; the process stays //! POST /sessions/{id}/stop end the process; the session and transcript stay @@ -640,11 +641,16 @@ async fn message( UrlPath(id): UrlPath, axum::Json(body): axum::Json, ) -> Result { - let session = lookup(&manager, &id)?; + // For the 404 a session that is not here has always answered with; the + // send itself goes through the manager, which may have to start a + // process before there is anything to send to. + lookup(&manager, &id)?; if body.text.trim().is_empty() && body.attachment_ids.is_empty() { return Err(ApiError::BadRequest("message is empty".to_string())); } - session.send_message(body.text, body.attachment_ids); + manager + .send_message(&id, body.text, body.attachment_ids) + .map_err(bad_request)?; Ok(StatusCode::NO_CONTENT) } diff --git a/server/src/session/mod.rs b/server/src/session/mod.rs index f4fe012..d3a31a4 100644 --- a/server/src/session/mod.rs +++ b/server/src/session/mod.rs @@ -1100,9 +1100,14 @@ impl SessionManager { Ok(()) } - /// Starts a process for a session whose process has ended, continuing - /// the same conversation -- for Claude Code, the `--resume` that crash - /// recovery already uses. + /// Starts a process for a session whose process has ended, for somebody + /// who asked for exactly that. + /// + /// Anything other than a session known to have exited is a refusal to + /// report, because the person pressing this expects a process to appear + /// and is owed the reason one did not. [`SessionManager::send_message`] + /// asks the same question of [`SessionManager::start_if_exited`] and + /// wants the opposite answer. /// /// Only the driver is new. The transcript, the event pump and the stream /// every open phone is reading stay as they were, so this is not a @@ -1110,10 +1115,56 @@ impl SessionManager { /// writer of the transcript, which relaunching the whole session would /// not be. /// - /// Refused unless the session is *known* to have exited. `Unknown` means - /// nobody could find out whether the process is alive, and starting one - /// on that is precisely the second-CLI-on-one-conversation fault that - /// `session::process` exists to prevent. + pub fn start_session(&self, id: &str) -> Result<()> { + match self.start_if_exited(id)? { + SessionStatus::Exited => Ok(()), + SessionStatus::Unknown => { + bail!("there is still a process recorded for this session, so nothing was started") + } + _ => bail!("this session is already running"), + } + } + + /// Hands a message to a session, starting its process first if that + /// session has none. + /// + /// Sending is the one instruction that plainly means "do this now", so a + /// session whose CLI has ended starts it rather than answering that it + /// cannot -- which left the person holding the phone to read a status + /// word, find a second button, press it, and type the message again. + /// `--resume` puts the new process on the same conversation, so nothing + /// about the message changes; only whether there was anything there to + /// read it. + /// + /// Started before the message rather than after, because starting + /// replaces the driver and the driver that takes the message has to be + /// the one with a process behind it. + pub fn send_message(&self, id: &str, text: String, images: Vec) -> Result<()> { + // Only `Exited` starts anything -- see `start_if_exited`. A session + // this cannot say has exited keeps the behaviour it always had: the + // message goes to the driver, which answers for it. + self.start_if_exited(id)?; + self.session(id) + .with_context(|| format!("no session {id}"))? + .send_message(text, images); + Ok(()) + } + + /// Starts a process for the session if it is known to have exited, and + /// reports what the session was found to be doing either way. `Exited` + /// is therefore the one returned value that means something was started. + /// + /// One decision with two callers who want opposite things from it: a + /// Start button treats "there is already a process" as a refusal worth + /// showing, and a message being sent treats it as nothing at all. + /// Deciding it here, under the one write lock, is also what stops two + /// requests that arrive together from starting two CLIs on one + /// conversation. + /// + /// Nothing is started on `Unknown`. That means nobody could find out + /// whether the process is alive, and starting one on that is precisely + /// the second-CLI-on-one-conversation fault `session::process` exists to + /// prevent. /// /// What the session then *reports* is the driver's to say, not this /// function's: the phone's list reads the manager's status and the @@ -1122,7 +1173,7 @@ impl SessionManager { /// is what a status set here without an event produced, visible as a /// stop button that turned into a play button a moment after the screen /// opened. - pub fn start_session(&self, id: &str) -> Result<()> { + fn start_if_exited(&self, id: &str) -> Result { let mut inner = self.inner.write().unwrap(); let meta = inner .config @@ -1151,12 +1202,8 @@ impl SessionManager { } None => status_of_unlaunched(&dir), }; - match status { - SessionStatus::Exited => {} - SessionStatus::Unknown => { - bail!("there is still a process recorded for this session, so nothing was started") - } - _ => bail!("this session is already running"), + if status != SessionStatus::Exited { + return Ok(status); } // Fresh from the config, like every other launch: a model or a // permission mode changed while the session was stopped is what it @@ -1202,7 +1249,7 @@ impl SessionManager { // here as well would be a second writer of the same fact, and the // one that cannot see whether the process it is describing is still // there. - Ok(()) + Ok(SessionStatus::Exited) } /// Kills the process, releases everything the spawn created, and @@ -2565,6 +2612,53 @@ mod tests { )); } + /// Sending is an instruction that means "now", so it does not answer + /// that the session's process has gone -- it starts one and delivers + /// the message to it. + /// + /// Refusing was the old behaviour and it was work handed back: read the + /// status word, find the other button, press it, type the message + /// again. `--resume` puts the new process on the same conversation, so + /// the message it reads is the one that was typed. + #[tokio::test] + async fn a_message_starts_the_process_a_stopped_session_has_not_got() { + 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, 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(); + + let _ = session.sink.send(Event::Status { + state: SessionStatus::Exited, + }); + collect_until(&mut rx, |event| { + matches!( + event, + Event::Status { + state: SessionStatus::Exited + } + ) + }) + .await; + + manager + .send_message(&info.id, "carry on".to_string(), Vec::new()) + .expect("send to a stopped session"); + collect_until( + &mut rx, + |event| matches!(event, Event::UserMessage { text, .. } if text == "carry on"), + ) + .await; + // And the session is running again, not merely written to: a message + // delivered to a session still reporting `exited` is one the phone + // draws under a Start button. + assert_ne!(manager.sessions()[0].status, SessionStatus::Exited); + } + /// The status is a claim about a process, and the process record is /// what settles it. ///