diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/SessionScreen.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/SessionScreen.kt index 1b26881..b3ae262 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/SessionScreen.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/SessionScreen.kt @@ -1375,13 +1375,7 @@ private fun UserBubble( modifier = Modifier.align(Alignment.CenterEnd).padding(start = 48.dp), ) { Column(Modifier.padding(12.dp)) { - // Above the words, in the order they were composed: the picture was attached - // before the sentence about it was typed, and it is what the sentence refers to. - images.forEach { ref -> - SessionImage(settings, sessionId, ref) - Spacer(Modifier.height(4.dp)) - } - // A message can be nothing but an attachment, and an empty line under a picture + // A message can be nothing but an attachment, and an empty line above a picture // is a bubble with a gap in it for a sentence nobody wrote. if (text.isNotEmpty()) { Text( @@ -1391,6 +1385,13 @@ private fun UserBubble( else MaterialTheme.colorScheme.onPrimaryContainer, ) } + // Under the words: what somebody wrote is what the bubble is, and the picture is + // what they attached to it. It also keeps the first line of every bubble at the + // same place down the transcript, whether or not there is an image in it. + images.forEachIndexed { index, ref -> + if (index > 0 || text.isNotEmpty()) Spacer(Modifier.height(4.dp)) + SessionImage(settings, sessionId, ref) + } } } } diff --git a/server/src/session/claude.rs b/server/src/session/claude.rs index ed57340..5d1ad50 100644 --- a/server/src/session/claude.rs +++ b/server/src/session/claude.rs @@ -448,7 +448,21 @@ impl ClaudeDriver { /// saying so here would be this side's guess standing in for its /// measurement. fn local_command(&self, text: String) { - self.queue.lock().unwrap().running = true; + let mut queue = self.queue.lock().unwrap(); + // The same check `send_user_message` makes, for the same reason: a + // line written into a fifo nothing is reading goes nowhere and looks + // exactly like one that arrived. `Commands::submit` refuses a + // session already known to have exited, so what this catches is the + // process going away between that check and this write. + if queue.closed { + drop(queue); + let _ = self.sink.send(Event::Error { + message: format!("this session's process has exited, so it can't run {text}"), + }); + return; + } + queue.running = true; + drop(queue); self.send_line( json!({"type": "user", "message": {"role": "user", "content": [ {"type": "text", "text": text} diff --git a/server/src/session/mod.rs b/server/src/session/mod.rs index 478fe8d..406f7c0 100644 --- a/server/src/session/mod.rs +++ b/server/src/session/mod.rs @@ -194,11 +194,28 @@ struct Commands { } impl Commands { - /// Runs `command` now if the session is between turns, and otherwise - /// holds it until it is. Either way the phone is told which happened. + /// Runs `command` now if the session is between turns, holds it until + /// it is, and refuses it outright if there will never be one. Whichever + /// happened, the phone is told. fn submit(&self, command: SessionCommand, status: SessionStatus) { let id = random_hex(); let text = command.label(); + // A session whose process is gone has no next boundary, so holding + // this would hold it forever: the phone draws a waiting bubble that + // nothing will ever resolve, and nothing anywhere says why. The + // message path has always answered this case -- see + // `ClaudeDriver::send_user_message` -- and a command owes the same + // answer, since what makes it unanswerable is the same fact. + // + // `Unknown` is not refused. It means nobody could find out whether + // the process is alive, and it resolves itself, so refusing on it + // would turn "we don't know" into "it's gone". + if status == SessionStatus::Exited { + let _ = self.sink.send(Event::Error { + message: format!("this session's process has exited, so it can't run {text}"), + }); + return; + } if status == SessionStatus::Idle { let _ = self.sink.send(Event::CommandSent { id, text }); command.apply(self.driver.as_ref()); @@ -1553,6 +1570,48 @@ mod tests { assert!(!DriverKind::LlamaCpp.keeps_own_transcript()); } + /// A command sent to a session whose process is gone says so, rather + /// than waiting for a boundary that will never come. + /// + /// Held commands drain at the next idle, and an exited session has no + /// next idle -- so this used to leave a `/clear` in the queue forever, + /// drawn on the phone as a waiting bubble with nothing to resolve it and + /// nothing anywhere saying why. A *message* sent to the same session + /// reported the exit immediately, which is what made the silence on the + /// command path visible: the same session answered one and swallowed the + /// other. + /// + /// `Unknown` still waits, deliberately: nobody could find out whether + /// the process is there, and refusing on it would turn "we don't know" + /// into "it's gone". + #[test] + fn a_command_is_refused_when_there_can_be_no_boundary() { + let dir = tempfile::tempdir().expect("tempdir"); + let (sink, mut events) = mpsc::unbounded_channel(); + let commands = Commands { + driver: Arc::new(EchoDriver::new(sink.clone(), dir.path().to_path_buf())), + sink, + waiting: Mutex::new(VecDeque::new()), + }; + + // The driver announces itself when it is built; that is not what + // this test is about. + while events.try_recv().is_ok() {} + + commands.submit(SessionCommand::Clear, SessionStatus::Exited); + assert!( + matches!(events.try_recv(), Ok(Event::Error { .. })), + "an exited session held the command instead of refusing it" + ); + assert!(commands.waiting.lock().unwrap().is_empty()); + + commands.submit(SessionCommand::Clear, SessionStatus::Running); + assert!(matches!(events.try_recv(), Ok(Event::CommandQueued { .. }))); + commands.submit(SessionCommand::Clear, SessionStatus::Unknown); + assert!(matches!(events.try_recv(), Ok(Event::CommandQueued { .. }))); + assert_eq!(commands.waiting.lock().unwrap().len(), 2); + } + /// The two transitions worth interrupting somebody for, and the ones /// that look like them and are not. ///