Answer a command a session can never run, and put attachments under the text
Investigating a `/clear` that did nothing. What I could measure says the basic path is sound: the CLI honours `/clear` in stream-json mode -- it emits `conversation_reset`, opens a fresh session id, and the model then answers "NO CONTEXT" to a question about something it was told a moment before -- and a `/clear` sent into a running turn is queued here and applied at the boundary, with the model losing context, in two reproductions. What the investigation did find is a command that can wait forever. Held commands drain at the next idle, and a session whose process is gone has no next idle, so `/clear` sent to one sat in the queue with a waiting bubble on the phone that nothing could resolve and nothing anywhere saying why. The *message* path has always answered this case -- a message to the same session reports the exit at once -- which is what made the silence visible: one session answered one and swallowed the other. A command owes the same answer, since what makes it unanswerable is the same fact. `Unknown` still waits. It means nobody could find out whether the process is there and it resolves itself, so refusing on it would turn "we don't know" into "it's gone". `local_command` gets the `closed` check `send_user_message` has had all along, for the window between the status being read and the line being written -- a line into a fifo nothing is reading goes nowhere and looks exactly like one that arrived. Attachments now draw under the message text rather than above it: what somebody wrote is what the bubble is, and it keeps the first line of every bubble in the same place down the transcript whether or not there is an image in it.
This commit is contained in:
1 parent
da65c1571f
commit
f1a185a9cd
3 files changed
+84
-10
No files matched your search
@@ -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.
|
||||
///
|
||||
|
||||
Reference in new issue
Block a user