From 5f6fd34abaa4a07a2df88a2ef36e0fa7bd864a17 Mon Sep 17 00:00:00 2001 From: iris <2+iris@noreply.localhost> Date: Sat, 29 Aug 2026 20:20:49 -0400 Subject: [PATCH] Record the clear that happened, not the one that was asked for `ClaudeDriver::clear` emitted `Event::Cleared` beside the `/clear` it sent, so the divider recorded a request. A reader scrolling back takes that mark as a fact about the conversation -- the session no longer has what is above this -- and a request is a different claim from a result. Compaction already gets this right by taking its mark from the CLI's own `compact_boundary` rather than from somebody pressing Compact; this is the same rule, and it was the one place left applying it to the request. Caught in review by the session this was measured against, which also established that the CLI's `/clear` is declared `supportsNonInteractive` and returns empty text with no result line -- so a fresh `init` bearing a different `session_id` is the only trace it leaves. The reader already watches for exactly that in order to persist the resume token, so the mark now goes out there. It has to be a *replacement* rather than any change, and the tests pin both ways of getting that wrong. The first `init` sets the id from nothing, which would otherwise open every session with a divider announcing a clear that never happened. And a compaction re-announces `init` carrying the *same* id, which would otherwise draw a clear on top of the compaction's own mark -- that one was found by writing the test rather than by reasoning about the change. 73 tests, clippy clean, rustfmt clean. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01VETa8afmpWaYezLCqJhDB8 --- server/src/session/claude.rs | 98 +++++++++++++++++++++++++++++++++--- 1 file changed, 90 insertions(+), 8 deletions(-) diff --git a/server/src/session/claude.rs b/server/src/session/claude.rs index 8eeddaf..3631fd4 100644 --- a/server/src/session/claude.rs +++ b/server/src/session/claude.rs @@ -570,12 +570,13 @@ impl Driver for ClaudeDriver { } fn clear(&self) { - // The CLI answers this with a fresh `init` carrying a new - // `session_id`, and the reader persists that as the resume token - // the moment it changes -- so the next launch of this session - // resumes the cleared conversation rather than the old one, with - // nothing here to keep in step. - let _ = self.sink.send(Event::Cleared); + // Nothing is emitted here on purpose. The CLI answers this with a + // fresh `init` carrying a new `session_id`, the reader persists + // that as the resume token, and `Event::Cleared` is emitted at + // that observation -- so the transcript records a clear that + // happened rather than one that was asked for, and the next + // launch resumes the cleared conversation with nothing here to + // keep in step. self.local_command("/clear".to_string()); } @@ -789,16 +790,36 @@ fn translate_line( tracing::warn!("unparseable claude output line: {shown}"); return true; }; - let (events, new_session_id) = { + let (events, new_session_id, replaced) = { let mut state = state.lock().unwrap(); let before = state.session_id.clone(); let events = state.translate(&message); let after = state.session_id.clone(); - (events, if before != after { after } else { None }) + // A *replacement* -- an id where there already was a different + // one -- rather than merely a change. The first `init` of a + // session sets the id from nothing, which is a session starting + // and not a conversation being dropped. + let replaced = before.is_some() && before != after; + (events, if before != after { after } else { None }, replaced) }; if let Some(session_id) = new_session_id { write_resume_token(session_dir, &session_id); } + // Here rather than where `/clear` is sent, because this is the CLI + // saying it happened and that is a different claim from us having + // asked. The divider is read as a fact about the conversation -- a + // reader scrolling back takes it to mean the session no longer has + // what is above it -- so it has to be anchored to the observation, + // the same way `Compacted` is anchored to `compact_boundary` rather + // than to somebody pressing Compact. + // + // The CLI's `/clear` returns empty text and leaves no result line, so + // a fresh `init` bearing a different `session_id` is the only trace + // it leaves. That is this, and it is already being watched for in + // order to persist the resume token. + if replaced && sink.send(Event::Cleared).is_err() { + return false; + } for event in events { // Anything the CLI says after a steer was written is proof it has // been round the model again, and the steer went with it -- so @@ -1016,6 +1037,67 @@ fn attachment_block(session_dir: &Path, id: &str) -> Result { mod tests { use super::*; + /// Drives real CLI output lines through the reader and collects what + /// came out, which is the only way to check the wiring between "the + /// CLI said this" and "the transcript records that". + fn events_from_lines(lines: &[&str]) -> Vec { + let dir = tempfile::tempdir().expect("temp dir"); + let state = Arc::new(Mutex::new(Translator::new(dir.path().to_path_buf()))); + let queue = Arc::new(Mutex::new(Queue::default())); + let (sink, mut out) = mpsc::unbounded_channel::(); + for line in lines { + assert!(translate_line(line, dir.path(), &state, &sink, &queue)); + } + drop(sink); + let mut events = Vec::new(); + while let Ok(event) = out.try_recv() { + events.push(event); + } + events + } + + /// A session starting is not a conversation being dropped. + /// + /// The first `init` sets the session id from nothing, which looks like + /// a change and is not a replacement -- reading it as one would open + /// every session with a divider announcing a clear that never + /// happened. + #[test] + fn the_first_init_is_not_a_clear() { + let events = events_from_lines(&[ + r#"{"type":"system","subtype":"init","session_id":"first","tools":[],"model":"claude-haiku-4-5-20251001"}"#, + ]); + assert!(!events.contains(&Event::Cleared), "got {events:?}"); + } + + /// The divider is anchored to the CLI saying it happened, not to us + /// asking. `/clear` returns empty text and leaves no result line, so a + /// fresh `init` bearing a *different* session id is the only trace it + /// leaves -- and that trace is what records the clear. + #[test] + fn a_second_init_with_a_new_id_is_a_clear() { + let events = events_from_lines(&[ + r#"{"type":"system","subtype":"init","session_id":"first","tools":[],"model":"claude-haiku-4-5-20251001"}"#, + r#"{"type":"system","subtype":"init","session_id":"second","tools":[],"model":"claude-haiku-4-5-20251001"}"#, + ]); + assert_eq!( + events.iter().filter(|e| **e == Event::Cleared).count(), + 1, + "got {events:?}" + ); + } + + /// A compaction also re-announces `init`, carrying the *same* session + /// id. That must not draw a clear on top of the compaction's own mark. + #[test] + fn an_init_repeating_the_same_id_is_not_a_clear() { + let events = events_from_lines(&[ + r#"{"type":"system","subtype":"init","session_id":"same","tools":[],"model":"claude-haiku-4-5-20251001"}"#, + r#"{"type":"system","subtype":"init","session_id":"same","tools":[],"model":"claude-haiku-4-5-20251001"}"#, + ]); + assert!(!events.contains(&Event::Cleared), "got {events:?}"); + } + /// The failure this exists for: a shell's complaint ends with a blank /// line, so reporting "the last line of stderr" reported nothing, and /// the phone showed a bare exit status while the reason sat in the