Merge branch 'main' of git.arirex.me:iris/ai-app

server/src/session/mod.rs

@@ -204,6 +204,19 @@ impl Commands {
     /// 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.
+    ///
+    /// "Between turns" is asked of the *driver*, not of `status`. They are
+    /// two views of the same fact and only one of them is current: the
+    /// driver sets its flag the instant it writes a line, while `status` is
+    /// built from what has been recorded, so it still reads idle for the
+    /// whole round trip of a command that produces no assistant text. Two
+    /// commands in a row therefore both went out, the second landing inside
+    /// the turn the first had started, where the CLI reads it as text
+    /// instead of running it -- silently, since a message read as text
+    /// looks like a message.
+    ///
+    /// `status` is still passed, for the one question the driver's flag
+    /// cannot answer: whether there will ever *be* another boundary.
     fn submit(&self, command: SessionCommand, status: SessionStatus) {
         let id = random_hex();
         let text = command.label();
@@ -223,7 +236,7 @@ impl Commands {
             });
             return;
         }
-        if status == SessionStatus::Idle {
+        if self.driver.between_turns() {
             let _ = self.sink.send(Event::CommandSent { id, text });
             command.apply(self.driver.as_ref());
             return;
@@ -238,7 +251,16 @@ impl Commands {
     /// The turn ended, so the oldest waiting command can go. One, not all
     /// of them: running a command starts a turn of its own, and the next
     /// boundary is where the one after it belongs.
+    ///
+    /// Asks the driver again rather than trusting the idle that called this.
+    /// The recorded idle is a moment in the past by the time it gets here,
+    /// and the driver may have started something since -- a turn the CLI
+    /// 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) {
+        if !self.driver.between_turns() {
+            return;
+        }
         let Some((id, command)) = self.waiting.lock().unwrap().pop_front() else {
             return;
         };
@@ -1575,12 +1597,12 @@ mod tests {
     /// 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
+    /// Held commands drain at the next boundary, and an exited session has
+    /// none -- 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
+    /// reported the exit at once, which is what made the silence on the
+    /// command path visible: one session answered one and swallowed the
     /// other.
     ///
     /// `Unknown` still waits, deliberately: nobody could find out whether
@@ -1595,9 +1617,7 @@ mod tests {
             sink,
             waiting: Mutex::new(VecDeque::new()),
         };
-
-        // The driver announces itself when it is built; that is not what
-        // this test is about.
+        // The driver announces itself when it is built; not what this is about.
         while events.try_recv().is_ok() {}
 
         commands.submit(SessionCommand::Clear, SessionStatus::Exited);
@@ -1607,11 +1627,68 @@ mod tests {
         );
         assert!(commands.waiting.lock().unwrap().is_empty());
 
-        commands.submit(SessionCommand::Clear, SessionStatus::Running);
-        assert!(matches!(events.try_recv(), Ok(Event::CommandQueued { .. })));
+        // Not exited and the driver is between turns, so it goes now.
         commands.submit(SessionCommand::Clear, SessionStatus::Unknown);
-        assert!(matches!(events.try_recv(), Ok(Event::CommandQueued { .. })));
-        assert_eq!(commands.waiting.lock().unwrap().len(), 2);
+        assert!(matches!(events.try_recv(), Ok(Event::CommandSent { .. })));
+    }
+
+    /// A command waits for the *driver* to be between turns, not for the
+    /// recorded status to say idle.
+    ///
+    /// The two are the same fact seen at different moments, and only the
+    /// driver's is current: it moves when a line is written, while the
+    /// status moves when output comes back. Gating on the status meant two
+    /// commands in a row both went out, the second landing inside the turn
+    /// the first had started -- where the CLI reads it as text instead of
+    /// running it, which looks exactly like nothing happening.
+    #[tokio::test]
+    async fn a_command_waits_for_the_driver_rather_than_the_recorded_status() {
+        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");
+        let mut rx = session.subscribe();
+
+        // A turn long enough to submit into.
+        session.send_message("/slow 2".to_string(), Vec::new());
+        collect_until(&mut rx, |event| {
+            matches!(
+                event,
+                Event::Status {
+                    state: SessionStatus::Running
+                }
+            )
+        })
+        .await;
+
+        session.run_command(SessionCommand::Clear);
+        let held = collect_until(&mut rx, |event| {
+            matches!(
+                event,
+                Event::CommandQueued { .. } | Event::CommandSent { .. }
+            )
+        })
+        .await;
+        assert!(
+            matches!(
+                held.last().map(|entry| &entry.event),
+                Some(Event::CommandQueued { .. })
+            ),
+            "a command went out into a running turn: {held:?}"
+        );
+
+        // And it is released when the turn actually ends.
+        let after =
+            collect_until(&mut rx, |event| matches!(event, Event::CommandSent { .. })).await;
+        assert!(
+            after
+                .iter()
+                .any(|entry| matches!(entry.event, Event::CommandSent { .. }))
+        );
     }
 
     /// The two transitions worth interrupting somebody for, and the ones