From 82401cd8871aaad232bca187f97d25294c7fab65 Mon Sep 17 00:00:00 2001 From: iris <2+iris@noreply.localhost> Date: Mon, 31 Aug 2026 22:20:47 -0400 Subject: [PATCH 1/8] Select any of the transcript, and take a queued message back Two things a reader could not do to what is on screen. **Selection.** Nothing in the transcript was selectable at all, so a command, a path or an error message could be read and not copied. One `SelectionContainer` around the whole list rather than one per row: a transcript is one body of text to a reader, and a selection has to be able to run from a reply into the tool output under it. Per row it also could not, and whatever was drawn without a container would have been silently unselectable -- a state nothing on screen reports. Rows keep their tap handlers; checked on the emulator that expanding a tool call, scrolling and flinging are all unaffected, since a selection is a long press. **Taking a message back.** A message sent into a running turn sits as a bubble waiting to be read, and there was no way to change your mind: it is tappable now, and the server answers `POST /sessions/{id}/unqueue`. The answer has three states, and the middle one is the point. Claude's driver writes a steer into the CLI's stdin the instant it arrives -- that is what makes it reach the model at the next tool boundary rather than at the end of the turn, and it was measured -- so the line is already gone and `AlreadySent` is the only honest answer it can give. Holding the write until a boundary would make the drop real and cost a steer one model call, which is the latency the immediate write exists to remove; rejected on that trade, with the reasoning in PLAN.md. The refusal is drawn on the bubble that was pressed rather than in the error row under the header, a screen away from it. Where a driver really does hold its queue -- echo today -- the message goes for good, and it goes as an `Event::MessageDropped` rather than as a return value: every device watching the session loses the bubble, and a phone that reconnects and replays the `messageQueued` does not put back one that was cancelled with nothing left to resolve it. --- AGENTS.md | 14 +++ PLAN.md | 30 ++++++ .../src/main/kotlin/com/example/aiapp/Api.kt | 17 ++++ .../main/kotlin/com/example/aiapp/Events.kt | 11 +++ .../kotlin/com/example/aiapp/SessionScreen.kt | 84 +++++++++++++++- .../com/example/aiapp/TranscriptItems.kt | 3 + .../com/example/aiapp/TranscriptList.kt | 98 ++++++++++-------- server/src/routes.rs | 44 ++++++++- server/src/session/claude.rs | 20 +++- server/src/session/driver.rs | 48 +++++++++ server/src/session/echo.rs | 19 +++- server/src/session/mod.rs | 99 ++++++++++++++++++- 12 files changed, 439 insertions(+), 48 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index e9c8a95..bbb65aa 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -245,6 +245,20 @@ first if a remote spawn ever mangles an argument. on the same transcript. What that costs is that the rows below slide up under the reader's finger, so a row that has just moved ignores taps for half a second (`SETTLE_MS`). +- **All transcript text is selectable, from one `SelectionContainer` around + the whole list** (`TranscriptList.kt`). Not per row: a transcript is one + body of text to a reader, so a selection has to be able to run from a + reply into the tool output under it -- and a container per row leaves + whatever was drawn without one silently unselectable, which nothing on + screen reports. Rows keep their tap handlers; selection is a long press. +- **A queued message can be tapped to take it back**, which is + `POST /sessions/{id}/unqueue` and a `messageDropped` event -- see PLAN.md's + "Taking a queued message back". On a **Claude** session it always refuses, + and that is correct rather than broken: the driver writes a steer into the + CLI the moment it arrives, so what the bubble is waiting for is the CLI + *reading* it, not this server sending it. The refusal is drawn on the + bubble. The echo driver really does hold its queue, so that is the rig for + the case where the drop succeeds. - **Deleting a session offers to take the machine's own transcript with it.** `DELETE /sessions/{id}?deleteForeign=true`, behind a switch in the confirmation, and only where the driver keeps a record of its own diff --git a/PLAN.md b/PLAN.md index a0c9ee1..4dd4edf 100644 --- a/PLAN.md +++ b/PLAN.md @@ -247,6 +247,35 @@ turn. Claude's dialect: a `user` message on stdin mid-stream; pi's: `steer`. - Images in: base64 image content blocks in the stream-json user message. - Working directory, host, and model are spawn-screen fields. +### Taking a queued message back (decided 2026-08-31) + +A message sent into a running turn is drawn as a bubble waiting below the +transcript, and tapping it asks the server to drop it before the session +reads it — `POST /sessions/{id}/unqueue {messageId}`, answered by +`Driver::unqueue` and recorded as `Event::MessageDropped` so that every +device watching loses the bubble and a reconnect does not replay it back. + +The answer has **three** states rather than a yes/no, and that is the whole +of the design: `Dropped`, `AlreadySent`, and `Unknown`. The reason is that +the Claude driver can only ever give the middle one. It writes a steer into +the CLI's stdin the instant it arrives — that is what makes a steer reach +the model at the next tool boundary instead of at the end of the turn, and +it was measured (see `Queue`'s doc comment) — so the line is gone before the +phone could ask for it back. What waits in `awaiting` is the *announcement*, +not the message. + +Holding the write until a boundary was considered and rejected on 2026-08-31: +it would make the drop real everywhere, but it costs a steer one model call, +which is the latency the immediate write was introduced to remove. So the +refusal is the honest answer and it is reported where the reader pressed — +on the bubble itself, not in the screen's error row, which is under the +header a screen away. What a tap buys on a Claude session is therefore +knowing that the session has already been told; on a driver that really does +hold a queue (echo today) the message goes. + +`Unknown` is not "we could not find out": a driver that is gone reported +everything it was holding when it closed, so there is nothing waiting. + ### Session processes outlive the backend (decided 2026-08-29) A session's process is **left running when the backend stops, and adopted @@ -659,6 +688,7 @@ GET /sessions list (id, provider, host, title, model, st POST /sessions spawn {provider, host, model, cwd, permission_mode, title} GET /sessions/:id/events?after=N SSE: transcript replay from N, then live POST /sessions/:id/message {text, attachment_ids} +POST /sessions/:id/unqueue {message_id} take back one not read yet POST /sessions/:id/answer {question_id, answer} (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 diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/Api.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/Api.kt index 8d21262..eff62ff 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/Api.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/Api.kt @@ -492,6 +492,23 @@ fun sendMessage( ) {} } +/** + * Takes back a message the session has not read yet, named by the id its `messageQueued` carried. + * + * Throws rather than returning an outcome, because both ways of failing are things the reader has + * to be told: 409 means the session was already given it, and 404 means nothing is waiting under + * that id. The bubble disappearing is the success case and it arrives on the event stream, not from + * here -- every device drops it, not only the one that tapped. + */ +fun unqueueMessage(settings: ServerSettings, sessionId: String, messageId: String) { + requestFromServer( + settings, + "/sessions/$sessionId/unqueue", + method = "POST", + jsonBody = JSONObject().put("messageId", messageId).toString(), + ) {} +} + /** Uploads one picked image; the returned id goes into [sendMessage]. */ fun uploadAttachment( settings: ServerSettings, diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/Events.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/Events.kt index 3f612c5..771ba89 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/Events.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/Events.kt @@ -53,6 +53,16 @@ sealed class SessionEvent { data class MessageQueued(val id: String, val text: String, val images: List) : SessionEvent() + /** + * A queued message taken back before the session read it. + * + * Recorded by the server for the same reason [MessageQueued] is: a phone that reconnects + * replays both, and without this one it would put back a bubble for a message that is never + * coming -- with nothing left to resolve it, since the [UserMessage] that normally does is + * exactly what was cancelled. + */ + data class MessageDropped(val id: String) : SessionEvent() + data class AssistantText(val delta: String) : SessionEvent() data class ToolStart(val id: String, val tool: String, val input: String) : SessionEvent() @@ -183,6 +193,7 @@ fun parseSeqEvent(json: String): SeqEvent { body.getString("text"), body.stringList("images"), ) + "messageDropped" -> SessionEvent.MessageDropped(body.getString("id")) "assistantText" -> SessionEvent.AssistantText(body.getString("delta")) "toolStart" -> SessionEvent.ToolStart( 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 ce673f8..02f4f65 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/SessionScreen.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/SessionScreen.kt @@ -8,6 +8,7 @@ import androidx.activity.compose.rememberLauncherForActivityResult import androidx.activity.result.PickVisualMediaRequest import androidx.activity.result.contract.ActivityResultContracts import androidx.compose.foundation.background +import androidx.compose.foundation.clickable import androidx.compose.foundation.gestures.awaitEachGesture import androidx.compose.foundation.gestures.awaitFirstDown import androidx.compose.foundation.layout.Box @@ -371,6 +372,11 @@ fun SessionScreen(settings: ServerSettings, summary: SessionSummary, onBack: () if (event is SessionEvent.UserMessage) { queued = queued.filterNot { it.id == event.id } } + // Waiting, then taken back. From the server rather than from the tap, so every device + // drops the bubble and a reconnect does not put back one that was cancelled. + if (event is SessionEvent.MessageDropped) { + queued = queued.filterNot { it.id == event.id } + } // Waiting, then gone: a command leaves this list when the session takes it, // and the row it becomes is added by `foldEvent` in the same pass. if (event is SessionEvent.CommandQueued) { @@ -883,6 +889,33 @@ fun SessionScreen(settings: ServerSettings, summary: SessionSummary, onBack: () } } + /** + * Asks the server to take back a message the session has not read yet. + * + * Nothing is removed here. The bubble goes on the `messageDropped` the server records, which is + * what makes the cancellation the session's own fact rather than this screen's opinion of it -- + * a second device watching the same session has to lose the bubble too, and this one has to + * still lose it after a reconnect. + * + * The refusal is kept on the message it was about rather than in [actionError]: the error row + * lives under the header, and a bubble at the foot of the transcript is the thing that was + * pressed. It is the ordinary answer here rather than the exceptional one -- a Claude session + * writes a steer into the CLI the moment it arrives, so what is on screen as "waiting" is + * waiting to be *read*, not waiting to be sent. + */ + fun takeBack(messageId: String) { + scope.launch { + val refusal = + try { + withContext(Dispatchers.IO) { unqueueMessage(settings, summary.id, messageId) } + null + } catch (e: ApiException) { + e.message ?: "this message could not be taken back" + } + queued = queued.map { if (it.id == messageId) it.copy(refusal = refusal) else it } + } + } + fun act(onDone: () -> Unit = {}, action: () -> Unit) { scope.launch { try { @@ -1154,6 +1187,13 @@ fun SessionScreen(settings: ServerSettings, summary: SessionSummary, onBack: () text = waiting.text, images = waiting.images, pending = true, + refusal = waiting.refusal, + // The bubble goes away on the `messageDropped` this + // produces, not here: the server is what knows whether + // the message was still its to take back, and the + // other devices watching this session have to be told + // by the same event. + onTakeBack = { takeBack(waiting.id) }, ) } } @@ -1672,6 +1712,11 @@ private fun ProcessAction.perform(settings: ServerSettings, sessionId: String) = * * [pending] is one the server has taken and the session has not read yet -- drawn quieter, because * "said" and "heard" are different claims and the transcript must not merge them. + * + * A pending bubble is tappable: [onTakeBack] asks the server to drop the message before the session + * reads it, and [refusal] is what came back when it would not. The refusal is drawn here rather + * than with the screen's other errors because this is where the reader pressed -- the error row is + * under the header, a screen away from the bubble they were looking at. */ @Composable private fun UserBubble( @@ -1680,6 +1725,8 @@ private fun UserBubble( text: String, images: List = emptyList(), pending: Boolean = false, + refusal: String? = null, + onTakeBack: (() -> Unit)? = null, ) { Box(Modifier.fillMaxWidth()) { Card( @@ -1693,7 +1740,19 @@ private fun UserBubble( if (pending) MaterialTheme.colorScheme.surfaceVariant else MaterialTheme.colorScheme.primaryContainer ), - modifier = Modifier.align(Alignment.CenterEnd).padding(start = 48.dp), + modifier = + Modifier.align(Alignment.CenterEnd) + .padding(start = 48.dp) + .then( + if (onTakeBack == null) Modifier + else + Modifier.clickable(onClick = onTakeBack).semantics { + // The bubble is its own control and its own label; without this + // the only thing to read is the message, which does not say what + // pressing it does. + contentDescription = "Waiting to be read; tap to take it back" + } + ), ) { Column(Modifier.padding(12.dp)) { // A message can be nothing but an attachment, and an empty line above a picture @@ -1713,13 +1772,32 @@ private fun UserBubble( if (index > 0 || text.isNotEmpty()) Spacer(Modifier.height(4.dp)) SessionImage(settings, sessionId, ref) } + refusal?.let { + Spacer(Modifier.height(6.dp)) + Text( + it, + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.error, + ) + } } } } } -/** A message the server has accepted and the session has not read yet. */ -private data class QueuedMessage(val id: String, val text: String, val images: List) +/** + * A message the server has accepted and the session has not read yet. + * + * [refusal] is why taking it back did not work, kept per message rather than on the screen: two + * bubbles can be waiting at once, and an error above them both would not say which one it was + * about. + */ +private data class QueuedMessage( + val id: String, + val text: String, + val images: List, + val refusal: String? = null, +) /** * Asked before switching model, because switching is not free and the cost is invisible. diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/TranscriptItems.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/TranscriptItems.kt index 54b5b2d..c358417 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/TranscriptItems.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/TranscriptItems.kt @@ -363,6 +363,9 @@ fun foldEvent(items: List, entry: SeqEvent): List items + // The bubble goes away and nothing takes its place: the message was never read, so there + // is nothing it belongs above. + is SessionEvent.MessageDropped -> items is SessionEvent.Settings -> items is SessionEvent.Status -> items is SessionEvent.Error -> items + TranscriptItem.ErrorMsg(entry.seq, event.message) diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/TranscriptList.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/TranscriptList.kt index bb6f460..9fabfe3 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/TranscriptList.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/TranscriptList.kt @@ -7,6 +7,7 @@ import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.size import androidx.compose.foundation.lazy.LazyColumn import androidx.compose.foundation.lazy.LazyListState +import androidx.compose.foundation.text.selection.SelectionContainer import androidx.compose.material3.CircularProgressIndicator import androidx.compose.runtime.Composable import androidx.compose.ui.Alignment @@ -35,6 +36,14 @@ import androidx.compose.ui.unit.dp * What keeps a unit's arrival cheap enough to happen mid-fling: a unit is at most one block of a * reply, and its parse is already made by [warm] before the fold that introduces it -- so entering * composition costs laying out one paragraph, not parsing a message. + * + * The whole list sits in a [SelectionContainer], which is what makes every word in the transcript + * selectable by the platform's own press-and-hold. Here rather than at each place text is drawn: a + * transcript is one body of text to a reader, and a container per row would mean a selection could + * never cross from a reply into the tool output that follows it -- and would leave whatever was + * drawn without one silently unselectable, which is a state nothing on screen reports. Rows keep + * their tap handlers: selection is a long press, and the container passes an ordinary click through + * to the card under it. */ @Composable fun TranscriptList( @@ -45,51 +54,58 @@ fun TranscriptList( below: @Composable () -> Unit, unit: @Composable (TranscriptUnit) -> Unit, ) { - LazyColumn( - state = state, - reverseLayout = true, - contentPadding = TRANSCRIPT_PADDING, - modifier = - // Timed in two halves because the frame's draw phase is where Compose's measurement - // lands, and "draw is high while nothing is being recorded" does not say which half; - // see [drawAccounting]. Measure includes composing the items that scrolled in. - modifier - .layout { measurable, constraints -> - val started = System.nanoTime() - val placeable = measurable.measure(constraints) - DebugStats.record("measure: the whole transcript", System.nanoTime() - started) - layout(placeable.width, placeable.height) { - val placing = System.nanoTime() - placeable.place(0, 0) + SelectionContainer { + LazyColumn( + state = state, + reverseLayout = true, + contentPadding = TRANSCRIPT_PADDING, + modifier = + // Timed in two halves because the frame's draw phase is where Compose's measurement + // lands, and "draw is high while nothing is being recorded" does not say which + // half; + // see [drawAccounting]. Measure includes composing the items that scrolled in. + modifier + .layout { measurable, constraints -> + val started = System.nanoTime() + val placeable = measurable.measure(constraints) DebugStats.record( - "place: the whole transcript", - System.nanoTime() - placing, + "measure: the whole transcript", + System.nanoTime() - started, + ) + layout(placeable.width, placeable.height) { + val placing = System.nanoTime() + placeable.place(0, 0) + DebugStats.record( + "place: the whole transcript", + System.nanoTime() - placing, + ) + } + } + .drawWithContent { + val started = System.nanoTime() + drawContent() + DebugStats.record("draw: the whole transcript", System.nanoTime() - started) + }, + ) { + // The bottom of the screen: what is waiting to be read sits under the newest message. + item(key = "below", contentType = "below") { below() } + items(count = units.size, key = { units[it].key }, contentType = { units[it]::class }) { + val u = units[it] + DebugStats.count("unit composed") + Box(Modifier.fillMaxWidth().padding(top = u.gap)) { unit(u) } + } + // Standing in for everything not fetched yet. Only here while there is more -- its + // appearance at the top edge is also roughly when the next page is asked for, so what + // it + // reports is a fetch in flight rather than an end reached. + if (moreHistory) { + item(key = "history", contentType = "history") { + Box(Modifier.fillMaxWidth().padding(vertical = 24.dp)) { + CircularProgressIndicator( + Modifier.align(Alignment.Center).size(HISTORY_SPINNER) ) } } - .drawWithContent { - val started = System.nanoTime() - drawContent() - DebugStats.record("draw: the whole transcript", System.nanoTime() - started) - }, - ) { - // The bottom of the screen: what is waiting to be read sits under the newest message. - item(key = "below", contentType = "below") { below() } - items(count = units.size, key = { units[it].key }, contentType = { units[it]::class }) { - val u = units[it] - DebugStats.count("unit composed") - Box(Modifier.fillMaxWidth().padding(top = u.gap)) { unit(u) } - } - // Standing in for everything not fetched yet. Only here while there is more -- its - // appearance at the top edge is also roughly when the next page is asked for, so what it - // reports is a fetch in flight rather than an end reached. - if (moreHistory) { - item(key = "history", contentType = "history") { - Box(Modifier.fillMaxWidth().padding(vertical = 24.dp)) { - CircularProgressIndicator( - Modifier.align(Alignment.Center).size(HISTORY_SPINNER) - ) - } } } } diff --git a/server/src/routes.rs b/server/src/routes.rs index 5205e30..c305e9b 100644 --- a/server/src/routes.rs +++ b/server/src/routes.rs @@ -17,6 +17,8 @@ //! `reset` frame plus the newest window) //! POST /sessions/{id}/message {text, attachmentIds?} //! (starts the process first if it has exited) +//! POST /sessions/{id}/unqueue {messageId} -- take back one not read yet +//! (409 when the session already has it) //! 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 @@ -68,7 +70,7 @@ use tokio::sync::{broadcast, mpsc}; use tokio_stream::StreamExt; use tokio_stream::wrappers::{BroadcastStream, ReceiverStream}; -use crate::session::driver::SessionCommand; +use crate::session::driver::{SessionCommand, Unqueued}; use crate::session::pending::Operation; use crate::session::transcript::{CATCH_UP_LIMIT, CatchUp, SeqEvent, catch_up}; use crate::session::{LiveSession, SessionInfo, SessionManager, SpawnSpec}; @@ -93,6 +95,7 @@ pub fn router(manager: Arc) -> Router { .route("/sessions/{id}/events", get(events)) .route("/sessions/{id}/transcript", get(transcript)) .route("/sessions/{id}/message", post(message)) + .route("/sessions/{id}/unqueue", post(unqueue)) .route("/sessions/{id}/answer", post(answer)) .route("/sessions/{id}/interrupt", post(interrupt)) .route("/sessions/{id}/stop", post(stop)) @@ -123,6 +126,10 @@ enum ApiError { UnknownRoute, #[error("{0}")] BadRequest(String), + /// The request was understood and the state it names has moved on -- + /// distinct from `BadRequest`, which is a caller that got it wrong. + #[error("{0}")] + Conflict(String), #[error(transparent)] Internal(#[from] anyhow::Error), } @@ -132,6 +139,7 @@ impl IntoResponse for ApiError { let status = match self { Self::NotFound(_) | Self::UnknownRoute => StatusCode::NOT_FOUND, Self::BadRequest(_) => StatusCode::BAD_REQUEST, + Self::Conflict(_) => StatusCode::CONFLICT, Self::Internal(err) => { // The only variant whose real cause isn't safe to hand // back verbatim, and the only one worth a log line. @@ -900,6 +908,40 @@ async fn message( Ok(StatusCode::NO_CONTENT) } +#[derive(Deserialize)] +#[serde(rename_all = "camelCase")] +#[serde(deny_unknown_fields)] +struct UnqueueRequest { + /// The id the `messageQueued` event carried, which is what the bubble + /// on screen is drawn from. + message_id: String, +} + +/// Takes back a message the session has not read yet. +/// +/// The two failures are separate answers rather than one refusal, because +/// they are different things to whoever tapped: `409` means the session has +/// already been told and the message is on its way into the conversation, +/// and `404` means nothing is waiting under that id -- a bubble on screen +/// that something else has already resolved. See [`Driver::unqueue`]; the +/// Claude driver can only ever give the first, since it writes a steer into +/// the CLI the moment it arrives. +async fn unqueue( + State(manager): State>, + UrlPath(id): UrlPath, + axum::Json(body): axum::Json, +) -> Result { + match lookup(&manager, &id)?.unqueue(&body.message_id) { + Unqueued::Dropped => Ok(StatusCode::NO_CONTENT), + Unqueued::AlreadySent => Err(ApiError::Conflict( + "the session has already been given this message".to_string(), + )), + Unqueued::Unknown => Err(ApiError::NotFound( + "this message is not waiting to be read".to_string(), + )), + } +} + #[derive(Deserialize)] #[serde(rename_all = "camelCase")] #[serde(deny_unknown_fields)] diff --git a/server/src/session/claude.rs b/server/src/session/claude.rs index 0805307..a036f2a 100644 --- a/server/src/session/claude.rs +++ b/server/src/session/claude.rs @@ -57,7 +57,7 @@ use serde_json::{Value, json}; use tokio::io::AsyncWriteExt; use tokio::sync::mpsc; -use super::driver::{Driver, Event, EventSink, ImageRef, SessionStatus}; +use super::driver::{Driver, Event, EventSink, ImageRef, SessionStatus, Unqueued}; use super::process; use super::transport::{Launch, Streams, Transport}; use crate::config::{ProviderConfig, SessionConfig}; @@ -595,6 +595,24 @@ impl Driver for ClaudeDriver { self.send_line(line); } + /// Never droppable, and that is a property of the design rather than + /// an omission. + /// + /// A message queued here has already been written to the CLI's stdin + /// -- see [`Queue`], where only the *announcement* waits -- because + /// that is what makes a steer reach the model at the next tool + /// boundary instead of at the end of the turn. A line in the fifo + /// cannot be recalled, so the only honest answers are "the session has + /// already been told" and "nothing is waiting under that id". + fn unqueue(&self, id: &str) -> Unqueued { + let queue = self.queue.lock().unwrap(); + if queue.awaiting.iter().any(|(waiting, ..)| waiting == id) { + Unqueued::AlreadySent + } else { + Unqueued::Unknown + } + } + fn answer_question(&self, id: &str, answers: &[String]) { let response = { let mut state = self.state.lock().unwrap(); diff --git a/server/src/session/driver.rs b/server/src/session/driver.rs index 1eeeb80..59a1ceb 100644 --- a/server/src/session/driver.rs +++ b/server/src/session/driver.rs @@ -114,6 +114,22 @@ pub enum Event { #[serde(default, skip_serializing_if = "Vec::is_empty")] images: Vec, }, + /// A message taken out of the queue before the session read it, by + /// somebody tapping the bubble that was waiting for it. + /// + /// Recorded for the same reason `MessageQueued` is: the queue is the + /// server's, so what is waiting has to be answerable from the + /// transcript alone. Without it a phone that reconnects replays the + /// `MessageQueued` and puts back a bubble for a message that will + /// never arrive -- and nothing later would ever resolve it, since the + /// `UserMessage` that normally does is exactly what is not coming. + /// + /// Only ever sent for a message that had not been handed over. One + /// that has is not droppable and says so instead; see + /// [`Unqueued::AlreadySent`]. + MessageDropped { + id: String, + }, /// A driver has taken one of the user's messages and started reading /// it. The manager turns this into the `UserMessage` above, so it /// never reaches a phone itself. @@ -444,6 +460,25 @@ pub enum SessionStatus { Unknown, } +/// What became of a request to take a queued message back. +/// +/// Three states rather than a bool because the two failures are not the +/// same fact. A driver that writes into its session the moment a message +/// arrives -- which is what `ClaudeDriver` does, so that a steer reaches +/// the model at the next tool boundary rather than at the end of the turn +/// -- can never take one back, and a phone that was told only "no" would +/// have to guess whether it had asked too late or asked about nothing. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Unqueued { + /// Out of the queue; the session will never read it. + Dropped, + /// Already handed to the session, so there is nothing left to take + /// back. The message is on its way into the conversation. + AlreadySent, + /// Nothing is waiting under that id. + Unknown, +} + /// Where a driver reports events. Unbounded because producers are child /// processes a slow phone must never be able to stall; the transcript file /// is the backpressure-free buffer of record. @@ -463,6 +498,19 @@ pub trait Driver: Send + Sync { /// message in the transcript, so a driver that never sends it drops /// the message from the conversation entirely. fn send_user_message(&self, text: String, images: Vec); + /// Takes back a message that is still waiting, named by the id its + /// [`Event::MessageQueued`] carried. + /// + /// Answering is the whole of the contract: a driver that drops the + /// message owes an [`Event::MessageDropped`], and one that cannot must + /// say which of the two reasons it is, because they are different + /// things to a reader -- "the session has already been told" is worth + /// knowing, and "there is nothing under that id" means the bubble on + /// screen is stale. The default is the honest answer for a driver with + /// no queue at all: nothing of yours is waiting. + fn unqueue(&self, _id: &str) -> Unqueued { + Unqueued::Unknown + } /// Answers one question with everything that was chosen, in the order /// it was offered. One answer is a list of one; a driver whose dialect /// takes a single value joins them where it writes it. diff --git a/server/src/session/echo.rs b/server/src/session/echo.rs index 2f5beeb..6e57e8f 100644 --- a/server/src/session/echo.rs +++ b/server/src/session/echo.rs @@ -53,7 +53,7 @@ use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; use std::sync::{Arc, Mutex}; use std::time::Duration; -use super::driver::{Driver, Event, EventSink, ImageRef, QuestionOption, SessionStatus}; +use super::driver::{Driver, Event, EventSink, ImageRef, QuestionOption, SessionStatus, Unqueued}; /// Delay between streamed deltas -- long enough that streaming is visibly /// streaming in the UI, short enough that tests waiting on a full turn @@ -796,6 +796,23 @@ impl Driver for EchoDriver { !self.busy.load(Ordering::SeqCst) } + /// Really droppable, which is what makes this the rig for the phone's + /// side of it: the held message is this driver's own and nothing has + /// been written anywhere, so a tap here exercises the whole path + /// through to the bubble disappearing on every device. The Claude + /// driver can only ever refuse -- see its own `unqueue` -- so it + /// cannot exercise the case where the drop succeeds. + fn unqueue(&self, id: &str) -> Unqueued { + let mut queued = self.queued.lock().unwrap(); + let Some(at) = queued.iter().position(|(waiting, ..)| waiting == id) else { + return Unqueued::Unknown; + }; + queued.remove(at); + drop(queued); + self.emit(Event::MessageDropped { id: id.to_string() }); + Unqueued::Dropped + } + fn send_user_message(&self, text: String, images: Vec) { // Announced, because this is a message: every driver owes exactly // one `MessageTaken` per message, and one that quietly vanishes diff --git a/server/src/session/mod.rs b/server/src/session/mod.rs index 4fbac3b..cda3e89 100644 --- a/server/src/session/mod.rs +++ b/server/src/session/mod.rs @@ -32,7 +32,9 @@ use crate::config::{ Config, DriverKind, ProviderConfig, SessionConfig, SetupConfig, SshConfig, TokenEntry, }; use claude::ClaudeDriver; -use driver::{Driver, Event, EventSink, ImageRef, SessionCommand, SessionStatus, context_after}; +use driver::{ + Driver, Event, EventSink, ImageRef, SessionCommand, SessionStatus, Unqueued, context_after, +}; use echo::EchoDriver; use llama::LlamaDriver; use transcript::{SeqEvent, Transcript}; @@ -440,6 +442,21 @@ impl LiveSession { self.ask("be interrupted", |driver| driver.interrupt()); } + /// Takes back a message the session has not read yet, named by the id + /// its `MessageQueued` carried. See [`Driver::unqueue`] for why the + /// answer has three states. + /// + /// A session with no process answers `Unknown` rather than being + /// reported as a failure, and that is the true answer: a driver on its + /// way out already said what it was holding (`Queue::close`), so there + /// is nothing waiting to take back. + pub fn unqueue(&self, message_id: &str) -> Unqueued { + match self.driver() { + Some(driver) => driver.unqueue(message_id), + None => Unqueued::Unknown, + } + } + /// Leaves this session's process running and stops attending to it, /// for a server that is going away and means to come back. See /// [`Driver::detach`]. @@ -2716,6 +2733,86 @@ mod tests { ); } + /// A queued message can be taken back until the driver has handed it + /// over, and the taking back is an event rather than a return value -- + /// which is what makes the bubble disappear on every device watching, + /// and stay gone when one of them reconnects and replays. + /// + /// Exercised on echo because echo really holds its queue. The Claude + /// driver writes a steer into the CLI the moment it arrives, so it can + /// only ever answer `AlreadySent`; the case where a drop *succeeds* + /// has no other driver to be tested against. + #[tokio::test] + async fn a_queued_message_can_be_taken_back_until_the_session_has_it() { + let dir = tempfile::tempdir().expect("tempdir"); + seed_echo_only(&dir.path().join("config.ron")); + let manager = SessionManager::new( + dir.path().join("config.ron"), + dir.path().join("sessions"), + dir.path().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(); + + // A turn long enough that the next message has to wait behind it. + session.send_message("/slow 1".to_string(), Vec::new()); + collect_until(&mut rx, |event| { + matches!( + event, + Event::Status { + state: SessionStatus::Running + } + ) + }) + .await; + + session.send_message("second thoughts".to_string(), Vec::new()); + let seen = collect_until(&mut rx, |event| { + matches!(event, Event::MessageQueued { .. }) + }) + .await; + let Some(Event::MessageQueued { id, .. }) = + seen.iter().map(|entry| entry.event.clone()).next_back() + else { + panic!("expected the message to be queued: {seen:?}"); + }; + + assert_eq!(session.unqueue(&id), Unqueued::Dropped); + let seen = collect_until(&mut rx, |event| { + matches!(event, Event::MessageDropped { .. }) + }) + .await; + assert!( + seen.iter().any(|entry| matches!( + &entry.event, + Event::MessageDropped { id: dropped } if *dropped == id + )), + "the drop has to be recorded, not merely returned: {seen:?}" + ); + + // Gone for good: the turn ends without the message ever entering + // the conversation, and asking again says there is nothing there + // rather than dropping it twice. + assert_eq!(session.unqueue(&id), Unqueued::Unknown); + let seen = collect_until(&mut rx, |event| { + matches!( + event, + Event::Status { + state: SessionStatus::Idle + } + ) + }) + .await; + assert!( + !seen + .iter() + .any(|entry| matches!(entry.event, Event::UserMessage { .. })), + "a message taken back must never be read: {seen:?}" + ); + } + #[tokio::test] async fn a_command_on_an_idle_session_goes_straight_out() { let dir = tempfile::tempdir().expect("tempdir"); From b7277a5a040a363ff76520d6ae7e82cf7a26b43c Mon Sep 17 00:00:00 2001 From: iris <2+iris@noreply.localhost> Date: Mon, 31 Aug 2026 22:33:11 -0400 Subject: [PATCH 2/8] Let go of the bubble for a message the session died before reading `Queue::close` reports the messages a dead process never read -- they reached no transcript, so that error is the only place they are ever mentioned -- but it left each one drawn as a bubble waiting to be read, by a session that no longer exists. Nothing would ever clear it: the `UserMessage` that resolves a queued bubble is exactly what is not coming. Seen on the emulator as a grey bubble sitting under its own error message, still saying "tap to take it back", on a session reporting `exited`. It now sends the `MessageDropped` the unqueue route introduced, one per lost message, alongside the error. The error says what happened and the drop is what ends it, which is the same division of labour as the rest of this path. --- server/src/session/claude.rs | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/server/src/session/claude.rs b/server/src/session/claude.rs index a036f2a..138a013 100644 --- a/server/src/session/claude.rs +++ b/server/src/session/claude.rs @@ -150,10 +150,22 @@ impl Queue { /// Reported rather than dropped. These are messages somebody typed /// that never reached the session and never reached the transcript, so /// this is the only place they can be mentioned at all. + /// + /// Each one is also *resolved*, with the same `MessageDropped` that a + /// phone tapping the bubble produces. Without it the bubble sat there + /// for good: a message drawn as waiting to be read, by a session that + /// no longer exists, with the only thing that ever clears it -- the + /// `UserMessage` -- exactly what is not coming. The error says what + /// happened and the drop is what ends it, which is the same division + /// of labour as everywhere else here. fn close(&mut self, sink: &EventSink, why: &str) { self.closed = true; self.running = false; - let lost: Vec = self.awaiting.drain(..).map(|(_, text, _)| text).collect(); + let lost: Vec<(String, String)> = self + .awaiting + .drain(..) + .map(|(id, text, _)| (id, text)) + .collect(); if lost.is_empty() { return; } @@ -165,9 +177,15 @@ impl Queue { } else { format!("{} queued messages", lost.len()) }, - lost.join(" / ") + lost.iter() + .map(|(_, text)| text.as_str()) + .collect::>() + .join(" / ") ), }); + for (id, _) in lost { + let _ = sink.send(Event::MessageDropped { id }); + } } } From ed88bdb31fb0879102fdc1dc8a8741b0d4e64d9b Mon Sep 17 00:00:00 2001 From: iris <2+iris@noreply.localhost> Date: Mon, 31 Aug 2026 22:33:31 -0400 Subject: [PATCH 3/8] Mark the answer on the question, and close the notes that are not turns Four things the transcript and the composer said badly. **An answered question threw away the question.** It collapsed into "Answered: Deny", which does not say that Allow was the alternative -- and whether a tool was allowed or refused is what a reader comes back to that row for. The options stay now and the one that was taken is marked, in the same purple border that says "picked" while the question is still open, so it is one appearance learned once rather than two renderings of one thing. The buttons are disabled rather than removed, and state their own border and label colour, because Material dims a disabled button's and that would have taken the mark with it. Both places got it: the question card, and the permission ask on a tool row, which had the same line. An answer typed into **Other** matches no option, so nothing could mark it. That one is still written out -- it is the state the marking cannot say. **Memory notes were open.** A `` note is not part of what was said to the reader, it is a note about where a claim came from, and left open it breaks a reply in half around a card. Closed like a tool call and a peer message, with the file it came from still visible, since that is what somebody scanning for "why does it think that" is looking for. Open-ness is the screen's rather than the card's, so a note opened and scrolled past is still open on the way back. **Picking a slash command left its own suggestion up.** `/compact` is a whole command and a prefix of itself, so the list stayed with the one row already chosen -- something to dismiss, in front of the box it was about to be sent from. **A model switch warned when there was nothing to warn about.** The warning is that a cache is dropped, so it needs there to be one: a session whose process has exited has nothing holding a cache, and one reporting zero context is holding nothing. Where the figure is *unknown* the fallback is what it was -- whether anything has been said -- because unknown is not nothing, and an import nobody has measured yet is exactly where the conversation may be enormous. --- AGENTS.md | 14 ++++ .../kotlin/com/example/aiapp/AskQuestion.kt | 69 +++++++++++++++---- .../kotlin/com/example/aiapp/MemoryNote.kt | 64 ++++++++++++++--- .../kotlin/com/example/aiapp/SessionScreen.kt | 65 +++++++++++++++-- .../main/kotlin/com/example/aiapp/ToolRows.kt | 14 ++-- 5 files changed, 188 insertions(+), 38 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 82f07a2..9760f76 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -245,6 +245,20 @@ first if a remote spawn ever mangles an argument. on the same transcript. What that costs is that the rows below slide up under the reader's finger, so a row that has just moved ignores taps for half a second (`SETTLE_MS`). +- **An answered question keeps its options and marks the one that was + taken**, in the same purple that says "picked" while it is still open -- + it does not collapse into a line repeating the answer. The options are + what the question *was*, and "Deny" alone does not say that Allow was the + alternative. One rule in two places (`AskedQuestion` and `PermissionAsk`), + since a permission is a question with two bare options rather than a + different kind of thing. An answer typed into **Other** matches no option, + so that one is still written out -- the state the marking cannot say. +- **Anything that is a note *about* the conversation rather than a turn in + it is closed by default**: a tool call, a peer message, and now a memory + note (``). Open-ness is the screen's, never the card's -- a + card that remembered for itself forgets the moment the lazy list stops + composing it, so a note opened and scrolled past would shut behind the + reader. - **All transcript text is selectable, from one `SelectionContainer` around the whole list** (`TranscriptList.kt`). Not per row: a transcript is one body of text to a reader, so a selection has to be able to run from a diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/AskQuestion.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/AskQuestion.kt index 42ae30f..36300d1 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/AskQuestion.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/AskQuestion.kt @@ -11,6 +11,7 @@ import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.height import androidx.compose.foundation.layout.padding import androidx.compose.foundation.rememberScrollState +import androidx.compose.material3.ButtonDefaults import androidx.compose.material3.CardDefaults import androidx.compose.material3.MaterialTheme import androidx.compose.material3.OutlinedButton @@ -70,27 +71,38 @@ fun AskedQuestion(ask: TranscriptItem.QuestionCard, onAnswer: (List) -> } Text(ask.prompt, style = MaterialTheme.typography.bodyLarge) Spacer(Modifier.height(8.dp)) - if (ask.answers.isNotEmpty()) { - // Joined for reading only: they arrived as a list and stay one everywhere else. - Text( - "Answered: ${ask.answers.joinToString(", ")}", - style = MaterialTheme.typography.labelLarge, - color = MaterialTheme.colorScheme.onSurfaceVariant, - ) - return@Column - } - if (ask.multiSelect) { + // An answered question keeps its options and marks the one that was taken, rather than + // replacing them with a line repeating it. The options are what the question *was*, and + // dropping them leaves an answer with nothing to have been an answer to -- "Sonnet" says + // very little without the three it was chosen over. Marked in the same purple that says + // "picked" while the question is still open, so it is one appearance learned once. + val answered = ask.answers.isNotEmpty() + if (ask.multiSelect && !answered) { MultipleChoice(ask.options, onAnswer) } else if (ask.options.all { it.description == null && it.preview == null }) { // Nothing to read, so nothing to lay out: Allow and Deny are two words, and two words // do not need a card each. - AnswerOptions(ask.options, onAnswer) + AnswerOptions(ask.options, ask.answers, onAnswer.takeUnless { answered }) } else { ask.options.forEach { option -> - OptionCard(option, selected = false) { onAnswer(listOf(option.label)) } + OptionCard(option, selected = option.label in ask.answers) { + if (!answered) onAnswer(listOf(option.label)) + } } } - OtherAnswer(onAnswer) + // What was answered in the reader's own words, which no option can mark -- see + // [OtherAnswer]. Only ever the answers that match nothing offered, so a question answered + // by picking says it by the mark alone. + val inWords = ask.answers.filterNot { answer -> ask.options.any { it.label == answer } } + if (inWords.isNotEmpty()) { + Text( + "Answered: ${inWords.joinToString(", ")}", + style = MaterialTheme.typography.labelLarge, + color = MaterialTheme.colorScheme.primary, + modifier = Modifier.padding(top = 8.dp), + ) + } + if (!answered) OtherAnswer(onAnswer) } } @@ -221,14 +233,41 @@ private fun OtherAnswer(onAnswer: (List) -> Unit) { * way for a list of choices to be wrong. */ @Composable -fun AnswerOptions(options: List, onAnswer: (List) -> Unit) { +fun AnswerOptions( + options: List, + /** What was chosen, marked rather than restated; empty while the question is open. */ + answers: List = emptyList(), + /** Null once the question is answered -- the buttons stay, and stop being buttons. */ + onAnswer: ((List) -> Unit)?, +) { FlowRow( horizontalArrangement = Arrangement.spacedBy(8.dp), verticalArrangement = Arrangement.spacedBy(4.dp), modifier = Modifier.fillMaxWidth(), ) { options.forEach { option -> - OutlinedButton(onClick = { onAnswer(listOf(option.label)) }) { Text(option.label) } + val taken = option.label in answers + OutlinedButton( + onClick = { onAnswer?.invoke(listOf(option.label)) }, + // Disabled rather than removed, so an answered question still shows what it + // offered. Material dims a disabled button's own border and label, which would + // take the mark with it -- both are stated here instead. + enabled = onAnswer != null, + border = + BorderStroke( + if (taken) 2.dp else 1.dp, + if (taken) MaterialTheme.colorScheme.primary + else MaterialTheme.colorScheme.outlineVariant, + ), + colors = + ButtonDefaults.outlinedButtonColors( + disabledContentColor = + if (taken) MaterialTheme.colorScheme.primary + else MaterialTheme.colorScheme.onSurfaceVariant + ), + ) { + Text(option.label) + } } } } diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/MemoryNote.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/MemoryNote.kt index be47a74..87ae626 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/MemoryNote.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/MemoryNote.kt @@ -1,15 +1,21 @@ package com.example.aiapp +import androidx.compose.foundation.clickable import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.Row +import androidx.compose.foundation.layout.Spacer import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.width import androidx.compose.material3.Card import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Text import androidx.compose.runtime.Composable import androidx.compose.runtime.remember +import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier +import androidx.compose.ui.text.style.TextOverflow import androidx.compose.ui.unit.dp /** @@ -29,6 +35,9 @@ import androidx.compose.ui.unit.dp fun AssistantMessage( text: String, replies: ParsedReplies, + /** Which notes are open, by [MessagePart.Remembered.text] -- see [MemoryNote]. */ + openNotes: Set, + onToggleNote: (String) -> Unit, modifier: Modifier = Modifier, live: Boolean = false, ) { @@ -43,7 +52,8 @@ fun AssistantMessage( parts.forEach { part -> when (part) { is MessagePart.Prose -> BlockedMarkdown(part.text, replies, live = live) - is MessagePart.Remembered -> MemoryNote(part, replies) + is MessagePart.Remembered -> + MemoryNote(part, replies, part.text in openNotes) { onToggleNote(part.text) } } } } @@ -67,19 +77,53 @@ fun messageParts(text: String): List { return if (parts.singleOrNull() is MessagePart.Prose) listOf(MessagePart.Prose(text)) else parts } +/** + * One sentence the model attributed to a memory file, closed until somebody asks. + * + * Closed by default, like a tool call and a peer message and for the same reason: it is not part of + * what was said to the reader, it is a note about where a claim came from. Left open it breaks the + * reply in half around a card, which reads as the answer having stopped and restarted -- and these + * arrive several to a message. + * + * What stays visible is which file it came from, because that is the whole of what the note claims + * and it is the part a reader scanning for "why does it think that" is looking for. + * + * Open-ness is the screen's, keyed by the note's own text: a note opened and scrolled past has to + * still be open on the way back, and a card that remembered for itself would forget the moment the + * list stopped composing it. The text is a good enough name -- it does not change once the closing + * tag has arrived, so a note stays open across the moment its reply settles. + */ @Composable -fun MemoryNote(note: MessagePart.Remembered, replies: ParsedReplies) { - Card(Modifier.fillMaxWidth()) { +fun MemoryNote( + note: MessagePart.Remembered, + replies: ParsedReplies, + expanded: Boolean, + onToggle: () -> Unit, +) { + Card(Modifier.fillMaxWidth().clickable(onClick = onToggle)) { Column(Modifier.padding(12.dp)) { // Named, not just tinted: a colour can say "this one is different", but it cannot say // what kind of different, and "recalled from a file" is a difference in kind. - Text( - if (note.files.size == 1) "remembered from ${note.files[0]}" - else "remembered from ${note.files.joinToString(", ")}", - style = MaterialTheme.typography.labelSmall, - color = MaterialTheme.colorScheme.onSurfaceVariant, - ) - MarkdownText(note.text, replies, Modifier.padding(top = 4.dp)) + Row(verticalAlignment = Alignment.CenterVertically) { + Text( + if (note.files.size == 1) "remembered from ${note.files[0]}" + else "remembered from ${note.files.joinToString(", ")}", + style = MaterialTheme.typography.labelSmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + if (!expanded) { + Spacer(Modifier.width(8.dp)) + Text( + note.text, + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + maxLines = 1, + // The head, not the tail: a sentence is identified by how it opens. + overflow = TextOverflow.Ellipsis, + ) + } + } + if (expanded) MarkdownText(note.text, replies, Modifier.padding(top = 4.dp)) } } } 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 eece960..508cd48 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/SessionScreen.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/SessionScreen.kt @@ -246,6 +246,9 @@ fun SessionScreen(settings: ServerSettings, summary: SessionSummary, onBack: () // A model the reader has chosen and not yet confirmed. See [ModelSwitchWarning]: switching // makes the session re-read the whole conversation, which is worth asking about first. var pendingModel by remember { mutableStateOf(null) } + // What was last taken from the command suggestions, so the list closes behind it; see + // [CommandSuggestions] at its call site. + var picked by remember { mutableStateOf(null) } var expandedTools by remember { mutableStateOf(setOf()) } // Which runs of adjacent tool calls are open. Keyed by the first call's // id, so a group survives more calls arriving after it. @@ -257,6 +260,10 @@ fun SessionScreen(settings: ServerSettings, summary: SessionSummary, onBack: () // by default, which is the rule for anything new in this transcript: a screen that opens // everything it can is one nobody can scan. var expandedNotes by remember { mutableStateOf(setOf()) } + // Which memory notes are open, by the note's own text -- see [MemoryNote]. Closed by default + // like everything else new in this transcript, and held here rather than in the card so a + // note opened and scrolled past is still open on the way back. + var openMemories by remember { mutableStateOf(setOf()) } // Uploaded-but-not-yet-sent attachment ids; sent with the next message. var pendingAttachments by remember { mutableStateOf(listOf()) } // What this session is set to now, seeded from the row that opened it and @@ -923,6 +930,11 @@ fun SessionScreen(settings: ServerSettings, summary: SessionSummary, onBack: () } } + /** Opens or closes one memory note, wherever it is drawn; see [MemoryNote]. */ + fun toggleMemory(text: String) { + openMemories = if (text in openMemories) openMemories - text else openMemories + text + } + fun act(onDone: () -> Unit = {}, action: () -> Unit) { scope.launch { try { @@ -1209,7 +1221,14 @@ fun SessionScreen(settings: ServerSettings, summary: SessionSummary, onBack: () ) { unit -> when (unit) { is TranscriptUnit.Block -> MarkdownText(unit.text, replies) - is TranscriptUnit.Memory -> MemoryNote(unit.part, replies) + is TranscriptUnit.Memory -> + MemoryNote( + unit.part, + replies, + unit.part.text in openMemories, + ) { + toggleMemory(unit.part.text) + } is TranscriptUnit.Whole -> { val row = unit.row Box( @@ -1322,6 +1341,8 @@ fun SessionScreen(settings: ServerSettings, summary: SessionSummary, onBack: () AssistantMessage( item.text, replies, + openNotes = openMemories, + onToggleNote = ::toggleMemory, live = true, ) is TranscriptItem.ToolRun -> @@ -1505,8 +1526,17 @@ fun SessionScreen(settings: ServerSettings, summary: SessionSummary, onBack: () // Between the transcript and the box: above what is being typed, so the list does not // cover the thing the command is about, and below everything that explains it. CommandSuggestions( - commands = suggestedCommands(input), - onPick = { command -> input = command.typed() }, + // Nothing to suggest about a suggestion that was just taken. `/compact` is a + // whole command *and* a prefix of itself, so picking it left the list standing + // there with the one row already chosen -- the reader has to dismiss a list that + // has nothing left to offer, in front of the box they are about to send from. + // Held by what was picked rather than by a flag, so typing anything else brings + // the list back without needing a second thing to reset. + commands = if (input == picked) emptyList() else suggestedCommands(input), + onPick = { command -> + input = command.typed() + picked = command.typed() + }, ) // Always enabled -- a send while the session is running becomes a @@ -1579,7 +1609,8 @@ fun SessionScreen(settings: ServerSettings, summary: SessionSummary, onBack: () // see [ModelSwitchWarning]. onPick = { chosen -> if ( - modelLabel(chosen) == modelLabel(model) || items.isEmpty() + modelLabel(chosen) == modelLabel(model) || + !worthWarningAbout(status, contextTokens, items) ) { act { setSessionModel(settings, summary.id, chosen) } } else { @@ -1806,6 +1837,32 @@ private data class QueuedMessage( val refusal: String? = null, ) +/** + * Whether a model switch has anything to warn about -- see [ModelSwitchWarning]. + * + * What the warning is about is a *cache* being dropped, so the question is whether there is one. + * Two answers say there is not, and both used to produce the dialog anyway: + * + * A session whose process has exited has nothing running to hold a cache, so the next turn was + * always going to re-read the conversation -- the switch adds nothing to that bill. And a session + * reporting zero context is holding nothing, which is what `/clear` leaves behind. + * + * Where the figure is *unknown* rather than zero the fallback is what it always was: whether + * anything has been said at all. Unknown is not nothing, and treating it as nothing would drop the + * warning on exactly the sessions -- an import, a fresh reattach -- where nobody has measured yet + * and the conversation may be enormous. + */ +private fun worthWarningAbout( + status: String, + contextTokens: Long?, + items: List, +): Boolean = + when { + status == "exited" -> false + contextTokens != null -> contextTokens > 0 + else -> items.isNotEmpty() + } + /** * Asked before switching model, because switching is not free and the cost is invisible. * diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/ToolRows.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/ToolRows.kt index 55b6f57..8a93503 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/ToolRows.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/ToolRows.kt @@ -420,15 +420,11 @@ private fun PermissionAsk(ask: TranscriptItem.QuestionCard, onAnswer: (List Date: Mon, 31 Aug 2026 22:41:04 -0400 Subject: [PATCH 4/8] Keep the image on screen when its tool call joins a group A `Read` that returns an image is a row of one call, and the moment the session makes its next call the two become a group -- which is a different composable in a different part of the tree, so the old subtree goes and everything it remembered goes with it. The full-screen viewer was inside that subtree, so somebody looking at a screenshot was thrown back to the transcript because the session carried on working. A page of history landing does the same thing to the same row. What is open is a property of the screen rather than of whichever row happened to draw the thumbnail, so it is held there now and drawn beside the other two dialogs. Nothing that happens to rows can reach it. The cost is one fetch when it opens, since the thumbnail's decoded bitmap belongs to a row this no longer goes through. Paid deliberately rather than plumbed around: it is one request for a picture somebody asked to see, and the viewer draws the same two empty states the thumbnail does -- still coming, and never coming -- which it previously could not have, since it only ever opened on a bitmap already in hand. `/tools n gap` now puts a screenshot on its first call, so the case is reproducible rather than argued about: that command already existed to make a run *grow* while somebody watches, and the image is what made growing matter. Checked on the emulator with `/tools 3 30` -- opened the image on the lone call, and it was still open a minute later with the row by then inside a group of three, and back returned to the transcript rather than leaving the app. --- AGENTS.md | 8 ++ .../kotlin/com/example/aiapp/SessionImage.kt | 117 ++++++++++++------ .../kotlin/com/example/aiapp/SessionScreen.kt | 38 +++++- server/src/session/echo.rs | 25 +++- 4 files changed, 146 insertions(+), 42 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 9760f76..435ac11 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -259,6 +259,14 @@ first if a remote spawn ever mangles an argument. card that remembered for itself forgets the moment the lazy list stops composing it, so a note opened and scrolled past would shut behind the reader. +- **The full-screen image lives on the screen, not in the row that drew the + thumbnail** (`SessionImageViewer`). A `Read` whose result is an image is a + row of one call until the next call arrives and makes it a group -- a + different composable in a different part of the tree, so the old subtree + and everything it remembered goes, the open dialog included. Somebody + looking at a screenshot was thrown back to the transcript because the + session made another tool call. `/tools n gap` puts an image on its first + call so this is reproducible: open it, wait a gap, watch the row regroup. - **All transcript text is selectable, from one `SelectionContainer` around the whole list** (`TranscriptList.kt`). Not per row: a transcript is one body of text to a reader, so a selection has to be able to run from a diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/SessionImage.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/SessionImage.kt index e44b2d5..027be8f 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/SessionImage.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/SessionImage.kt @@ -78,12 +78,18 @@ fun rememberSessionBitmap(settings: ServerSettings, sessionId: String, ref: Stri * is what it should be. * * Four lines of body text, so a screenshot reads as an attachment beside the conversation rather - * than as a page of its own. Full size is one tap away. + * than as a page of its own. Full size is one tap away -- but the full-size view itself is not + * here. [onOpen] hands the ref to the screen, which draws [SessionImageViewer] outside the list; + * see that function for the reason. */ @Composable -fun SessionImage(settings: ServerSettings, sessionId: String, ref: String) { +fun SessionImage( + settings: ServerSettings, + sessionId: String, + ref: String, + onOpen: (String) -> Unit, +) { val (bitmap, failed) = rememberSessionBitmap(settings, sessionId, ref) - var full by remember(ref) { mutableStateOf(false) } val height = thumbnailHeight() val heightPx = with(LocalDensity.current) { height.roundToPx() } Box(Modifier.fillMaxWidth().height(height), contentAlignment = Alignment.CenterStart) { @@ -102,12 +108,60 @@ fun SessionImage(settings: ServerSettings, sessionId: String, ref: String) { contentDescription = "Attached image, tap to view full screen", contentScale = ContentScale.Fit, filterQuality = enlargingFilter(image.height, heightPx), - modifier = Modifier.fillMaxSize().clickable { full = true }, + modifier = Modifier.fillMaxSize().clickable { onOpen(ref) }, alignment = Alignment.CenterStart, ) } } - if (full) bitmap?.let { image -> ImageViewer(image) { full = false } } +} + +/** + * The image somebody opened, drawn by the screen rather than by the row it was tapped in. + * + * The row is the wrong place to hold this, and it took a real fault to see why: an image from a + * `Read` on its own is a row of one call, and the moment the next call arrives the two become a + * group -- a different composable in a different part of the tree, so everything the old subtree + * remembered goes, the dialog included. Somebody looking at a screenshot was thrown back to the + * transcript because the session made another tool call. The same happens to a row regrouped by a + * page of history landing. + * + * Held by the screen, none of that reaches it: what is open is a property of the screen, not of + * whichever row happened to draw the thumbnail. + * + * The cost is one fetch, since the thumbnail's decoded bitmap belongs to a row this does not go + * through. Paid deliberately rather than plumbed around: it is one request for a picture somebody + * asked to see, and the loading and unavailable states below are the same two the thumbnail draws. + */ +@Composable +fun SessionImageViewer( + settings: ServerSettings, + sessionId: String, + ref: String, + onClose: () -> Unit, +) { + val (bitmap, failed) = rememberSessionBitmap(settings, sessionId, ref) + Dialog( + onDismissRequest = onClose, + properties = DialogProperties(usePlatformDefaultWidth = false), + ) { + Box( + Modifier.fillMaxSize().background(Color.Black).clickable(onClick = onClose), + contentAlignment = Alignment.Center, + ) { + when (val image = bitmap) { + // Two states, not one, exactly as the thumbnail has them: still coming, and never + // coming. Stated in white because this box paints its own black behind them and a + // theme colour would be picked against a surface that is not there. + null -> + Text( + if (failed) "Image $ref is unavailable" else "Loading image…", + color = Color.White, + style = MaterialTheme.typography.bodyMedium, + ) + else -> ZoomableImage(image) + } + } + } } /** @@ -139,23 +193,23 @@ private fun enlargingFilter(sourceHeight: Int, drawnHeight: Int): FilterQuality /** * The image on its own, as large as it fits, with pinch to zoom. * - * A dialog rather than a screen, so the platform's back gesture returns to the transcript instead - * of leaving the app. It opens fitted -- the whole image visible, which is the thing a reader wants - * first -- and zoom is theirs from there. + * Inside a dialog rather than a screen -- see [SessionImageViewer] -- so the platform's back + * gesture returns to the transcript instead of leaving the app. It opens fitted, the whole image + * visible, which is the thing a reader wants first; zoom is theirs from there. */ @Composable -private fun ImageViewer(image: ImageBitmap, onClose: () -> Unit) { - Dialog( - onDismissRequest = onClose, - properties = DialogProperties(usePlatformDefaultWidth = false), - ) { - var scale by remember { mutableFloatStateOf(1f) } - var offsetX by remember { mutableFloatStateOf(0f) } - var offsetY by remember { mutableFloatStateOf(0f) } - Box( +private fun ZoomableImage(image: ImageBitmap) { + var scale by remember { mutableFloatStateOf(1f) } + var offsetX by remember { mutableFloatStateOf(0f) } + var offsetY by remember { mutableFloatStateOf(0f) } + Image( + bitmap = image, + contentDescription = "Attached image", + contentScale = ContentScale.Fit, + // Zoomed in, the reader is looking at pixels on purpose. + filterQuality = FilterQuality.None, + modifier = Modifier.fillMaxSize() - .background(Color.Black) - .clickable(onClick = onClose) .pointerInput(Unit) { detectTransformGestures { _, pan, zoom, _ -> // Floor of 1 so the image cannot be pinched smaller than fitted, which is @@ -169,23 +223,12 @@ private fun ImageViewer(image: ImageBitmap, onClose: () -> Unit) { offsetY = 0f } } + } + .graphicsLayer { + scaleX = scale + scaleY = scale + translationX = offsetX + translationY = offsetY }, - contentAlignment = Alignment.Center, - ) { - Image( - bitmap = image, - contentDescription = "Attached image", - contentScale = ContentScale.Fit, - // Zoomed in, the reader is looking at pixels on purpose. - filterQuality = FilterQuality.None, - modifier = - Modifier.fillMaxSize().graphicsLayer { - scaleX = scale - scaleY = scale - translationX = offsetX - translationY = offsetY - }, - ) - } - } + ) } 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 508cd48..d03afce 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/SessionScreen.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/SessionScreen.kt @@ -264,6 +264,10 @@ fun SessionScreen(settings: ServerSettings, summary: SessionSummary, onBack: () // like everything else new in this transcript, and held here rather than in the card so a // note opened and scrolled past is still open on the way back. var openMemories by remember { mutableStateOf(setOf()) } + // The image being looked at full screen, by ref. Here rather than in the row that drew the + // thumbnail: a row regrouped underneath the reader takes its whole subtree with it, and the + // dialog with it -- see [SessionImageViewer]. + var fullImage by remember { mutableStateOf(null) } // Uploaded-but-not-yet-sent attachment ids; sent with the next message. var pendingAttachments by remember { mutableStateOf(listOf()) } // What this session is set to now, seeded from the row that opened it and @@ -930,6 +934,11 @@ fun SessionScreen(settings: ServerSettings, summary: SessionSummary, onBack: () } } + /** Opens one image full screen, from whichever row drew it; see [SessionImageViewer]. */ + fun openImage(ref: String) { + fullImage = ref + } + /** Opens or closes one memory note, wherever it is drawn; see [MemoryNote]. */ fun toggleMemory(text: String) { openMemories = if (text in openMemories) openMemories - text else openMemories + text @@ -1205,6 +1214,7 @@ fun SessionScreen(settings: ServerSettings, summary: SessionSummary, onBack: () sessionId = summary.id, text = waiting.text, images = waiting.images, + onOpenImage = ::openImage, pending = true, refusal = waiting.refusal, // The bubble goes away on the `messageDropped` this @@ -1315,7 +1325,12 @@ fun SessionScreen(settings: ServerSettings, summary: SessionSummary, onBack: () } }, image = { ref -> - SessionImage(settings, summary.id, ref) + SessionImage( + settings, + summary.id, + ref, + ::openImage, + ) }, ) is TranscriptRow.Single -> @@ -1326,6 +1341,7 @@ fun SessionScreen(settings: ServerSettings, summary: SessionSummary, onBack: () sessionId = summary.id, text = item.text, images = item.images, + onOpenImage = ::openImage, ) is TranscriptItem.AssistantMsg -> // A whole assistant row is only ever the reply @@ -1368,7 +1384,12 @@ fun SessionScreen(settings: ServerSettings, summary: SessionSummary, onBack: () } }, image = { ref -> - SessionImage(settings, summary.id, ref) + SessionImage( + settings, + summary.id, + ref, + ::openImage, + ) }, ) is TranscriptItem.QuestionCard -> @@ -1389,7 +1410,12 @@ fun SessionScreen(settings: ServerSettings, summary: SessionSummary, onBack: () style = MaterialTheme.typography.bodyMedium, ) is TranscriptItem.ImageItem -> - SessionImage(settings, summary.id, item.ref) + SessionImage( + settings, + summary.id, + item.ref, + ::openImage, + ) is TranscriptItem.Note -> Text( item.text, @@ -1689,6 +1715,9 @@ fun SessionScreen(settings: ServerSettings, summary: SessionSummary, onBack: () } } + // Beside the other two dialogs, and outside the list for the same reason as them: what is + // open is the screen's business rather than any row's. See [SessionImageViewer]. + fullImage?.let { ref -> SessionImageViewer(settings, summary.id, ref) { fullImage = null } } if (usageOpen) { UsageDialog(settings = settings, onDismiss = { usageOpen = false }) } @@ -1762,6 +1791,7 @@ private fun UserBubble( sessionId: String, text: String, images: List = emptyList(), + onOpenImage: (String) -> Unit, pending: Boolean = false, refusal: String? = null, onTakeBack: (() -> Unit)? = null, @@ -1808,7 +1838,7 @@ private fun UserBubble( // 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) + SessionImage(settings, sessionId, ref, onOpenImage) } refusal?.let { Spacer(Modifier.height(6.dp)) diff --git a/server/src/session/echo.rs b/server/src/session/echo.rs index 6e57e8f..877518f 100644 --- a/server/src/session/echo.rs +++ b/server/src/session/echo.rs @@ -11,7 +11,9 @@ //! looks like when a screen groups them. `gap` is seconds between one //! call and the next, default none: it is what makes a run *grow* while //! somebody is looking at it, which is the only way to reach the state -//! where a call opened on its own gains a neighbour. +//! where a call opened on its own gains a neighbour. The first call +//! carries a screenshot, so that state can also be reached with an image +//! open full screen -- which is where it used to close itself. //! - `/question [text]` -- a question, exercising the answer path. //! - `/ask` -- an AskUserQuestion call: two questions on one tool call, //! with descriptions, a preview and a multi-select, which is the shape @@ -482,6 +484,27 @@ impl EchoDriver { "timeout": 5000, }), }); + // The first call carries a screenshot, and only the + // first. That is what makes this rig cover the case a + // growing run is actually about: an image opened full + // screen from a call that is alone, and then a second + // call arriving and turning that row into a group. The + // dialog used to be inside the row, so the reader was + // thrown back to the transcript by the session making + // another tool call. Any of the calls would do; the + // first is the one that is on its own for a whole + // `gap`, which is the window somebody can open it in. + if i == 1 { + let part = serde_json::json!({ + "source": {"media_type": "image/png", "data": SAMPLE_PNG} + }); + if let Some(name) = super::claude::translate::save_image(&dir, &part) { + send(Event::Image { + image: name, + about: Some(id.clone()), + }); + } + } tokio::time::sleep(DELTA_DELAY).await; send(Event::ToolEnd { id, From a1eedd7a78b5c37758617ec6d7621cbe83728a13 Mon Sep 17 00:00:00 2001 From: iris <2+iris@noreply.localhost> Date: Mon, 31 Aug 2026 22:43:24 -0400 Subject: [PATCH 5/8] Don't call a turn finished with a message still waiting behind it A message written into the tail of a turn is read the moment that turn's `result` lands: the session reports idle and is running again in the same breath. The phone that sent it got "finished" in between -- seconds before anything it asked for had been done, which is the notification arriving to say the opposite of what is happening. `notification_for` now takes how many messages the session has been given and not started reading, and a turn ending with any of them waiting is not an ending. The count is kept in `pump`, from the recorded events, because that is the one place that sees all of them in transcript order: a `messageQueued` up, and the `userMessage` that resolves it or a `messageDropped` down. Asking the driver instead would answer about the moment the question was asked rather than the moment the status was written, which is the same class of mistake as reading a session's status to decide what a queue contains. It deliberately does not suppress *awaiting input*. A question is worth interrupting somebody for whatever is queued behind it -- the queue is precisely what will not move until it is answered. Tested both halves: the decision on the number, and the number itself, where an echo turn that reads its queued message before going idle still announces its finish. That last is the case a suppression written slightly wrong silences, and it is the common one. --- PLAN.md | 13 +++++ server/src/session/mod.rs | 120 +++++++++++++++++++++++++++++++++----- 2 files changed, 118 insertions(+), 15 deletions(-) diff --git a/PLAN.md b/PLAN.md index 4dd4edf..7e37ceb 100644 --- a/PLAN.md +++ b/PLAN.md @@ -925,6 +925,19 @@ level: the session on screen is registered by the one composable that draws one, and "the app is up" *is* the banner queue being collected, since it collects only while it is on screen. +**What counts as finished** is decided in `notification_for`, and since +2026-08-31 it takes the number of messages the session has been given and +not started reading. With one waiting, a turn ending is not the work +ending: a message written into the tail of a turn is read the moment that +turn's `result` lands, so the session goes idle and immediately runs again +-- and the phone that sent it was told its work had finished, seconds +before any of it was done. The count is kept in `pump` from the recorded +events (`MessageQueued` up, the `UserMessage` that resolves it or a +`MessageDropped` down), because that is the one place that sees every event +in transcript order. It does not suppress *awaiting input*: a question is +worth saying whatever is queued behind it, and the queue is exactly what +will not move until it is answered. + The alternative considered and rejected was giving the app its own connection to `/notifications` while it is in front. That is a second stream per device saying the same thing, and it puts the "which of these two shows diff --git a/server/src/session/mod.rs b/server/src/session/mod.rs index cda3e89..c34855c 100644 --- a/server/src/session/mod.rs +++ b/server/src/session/mod.rs @@ -2086,10 +2086,26 @@ fn is_news(event: &Event, shared: &Shared) -> bool { /// adopted at startup, or because a driver announced itself, is not news /// that anything ended, and sending it would put "finished" on the phone for /// every session in the config every time the backend restarts. -fn notification_for(was: SessionStatus, now: SessionStatus) -> Option { +/// +/// `unread` is how many messages the session has been handed and not yet +/// started reading, and it suppresses *Finished* for the same reason: with +/// one waiting, the turn ending is not the work ending. A message written +/// into the tail of a turn is read as soon as that turn's `result` lands, so +/// the session goes idle and immediately runs again -- and the phone that +/// sent it was told its work had finished, seconds before anything of it had +/// been done. It cannot suppress *AwaitingInput*: a question is worth saying +/// whatever else is queued behind it, and the queue is precisely what will +/// not move until it is answered. +fn notification_for( + was: SessionStatus, + now: SessionStatus, + unread: usize, +) -> Option { match (was, now) { (_, SessionStatus::AwaitingInput) => Some(NotificationKind::AwaitingInput), - (SessionStatus::Running | SessionStatus::Compacting, SessionStatus::Idle) => { + (SessionStatus::Running | SessionStatus::Compacting, SessionStatus::Idle) + if unread == 0 => + { Some(NotificationKind::Finished) } _ => None, @@ -2105,6 +2121,13 @@ async fn pump( commands: Arc, notifications: broadcast::Sender, ) { + // Messages the session has been given and not started reading, which is + // what makes a turn ending not the same thing as the work ending; see + // `notification_for`. Counted from the recorded events rather than asked + // of the driver, because this is the one place that sees every event in + // the order the transcript has them -- and because the answer has to + // survive being asked a moment later than the driver would have said it. + let mut unread: usize = 0; while let Some(event) = source.recv().await { let ts = now(); // Taking a message is how it enters the conversation, and the @@ -2154,8 +2177,8 @@ async fn pump( // Read before it is overwritten: what makes a status // worth announcing is the transition, not the value. let was = std::mem::replace(&mut *shared.status.lock().unwrap(), *state); - if let Some(kind) = - notification_for(was, *state).filter(|_| *shared.notify.lock().unwrap()) + if let Some(kind) = notification_for(was, *state, unread) + .filter(|_| *shared.notify.lock().unwrap()) { // No subscribers is the ordinary case -- nobody has // the app open -- and it is not an error. @@ -2180,6 +2203,13 @@ async fn pump( Event::Status { state: SessionStatus::Exited, } => commands.abandon("this session's process has exited"), + // The two ends of a message's wait. A `UserMessage` with + // no id never waited -- it is one sent between turns, and + // counting it would take the total below zero. + Event::MessageQueued { .. } => unread += 1, + Event::UserMessage { id: Some(_), .. } | Event::MessageDropped { .. } => { + unread = unread.saturating_sub(1) + } _ => {} } // No subscribers is fine; the transcript already has it. @@ -2400,28 +2430,43 @@ mod tests { // Waiting on a person is worth saying however it was reached: it // will sit unanswered until somebody is told. assert_eq!( - notification_for(Running, SessionStatus::AwaitingInput), + notification_for(Running, SessionStatus::AwaitingInput, 0), Some(AwaitingInput) ); assert_eq!( - notification_for(Idle, SessionStatus::AwaitingInput), + notification_for(Idle, SessionStatus::AwaitingInput, 0), Some(AwaitingInput) ); // A turn this server watched run, ending. - assert_eq!(notification_for(Running, Idle), Some(Finished)); - assert_eq!(notification_for(Compacting, Idle), Some(Finished)); + assert_eq!(notification_for(Running, Idle, 0), Some(Finished)); + assert_eq!(notification_for(Compacting, Idle, 0), Some(Finished)); // Idle arrived at from anywhere else is not an ending. - assert_eq!(notification_for(Idle, Idle), None); - assert_eq!(notification_for(Unknown, Idle), None); - assert_eq!(notification_for(Exited, Idle), None); - assert_eq!(notification_for(SessionStatus::AwaitingInput, Idle), None); + assert_eq!(notification_for(Idle, Idle, 0), None); + assert_eq!(notification_for(Unknown, Idle, 0), None); + assert_eq!(notification_for(Exited, Idle, 0), None); + assert_eq!( + notification_for(SessionStatus::AwaitingInput, Idle, 0), + None + ); // Everything else a session does is progress nobody asked to hear. - assert_eq!(notification_for(Idle, Running), None); - assert_eq!(notification_for(Running, Compacting), None); - assert_eq!(notification_for(Running, Exited), None); + assert_eq!(notification_for(Idle, Running, 0), None); + assert_eq!(notification_for(Running, Compacting, 0), None); + assert_eq!(notification_for(Running, Exited, 0), None); + + // A turn ending with a message the session has not started reading + // is not the work ending: it goes straight back to running, and + // "finished" would arrive seconds before any of that work was done. + assert_eq!(notification_for(Running, Idle, 1), None); + assert_eq!(notification_for(Compacting, Idle, 2), None); + // A question is still worth saying with a queue behind it -- the + // queue is exactly what will not move until it is answered. + assert_eq!( + notification_for(Running, SessionStatus::AwaitingInput, 1), + Some(AwaitingInput) + ); } /// The switch reaches the running pump, not just the config file. @@ -2477,6 +2522,51 @@ mod tests { ); } + /// Counting the wait, rather than only deciding what to do about it. + /// + /// `notification_for` is tested above on the number; this is the number + /// itself, which is kept in `pump` from the recorded events and has no + /// other way to be looked at. Echo takes its queued message *before* + /// going idle -- the same order a real CLI has when the steer lands + /// inside the turn -- so the count is back to zero by the end and the + /// finish is still announced. That is the case a suppression written + /// slightly wrong silences, and it is the common one. + #[tokio::test] + async fn a_turn_that_read_its_queued_message_still_announces_its_finish() { + 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 events = session.subscribe(); + let mut notifications = manager.subscribe_notifications(); + + session.send_message("/slow 1".to_string(), Vec::new()); + collect_until(&mut events, |event| { + matches!( + event, + Event::Status { + state: SessionStatus::Running + } + ) + }) + .await; + session.send_message("and this behind it".to_string(), Vec::new()); + collect_until(&mut events, |event| { + matches!(event, Event::MessageQueued { .. }) + }) + .await; + + let announced = tokio::time::timeout(Duration::from_secs(5), notifications.recv()) + .await + .expect("a notification within five seconds") + .expect("channel open"); + assert_eq!(announced.kind, NotificationKind::Finished); + } + /// A session this app *spawned* is one it is driving, and used to look /// like somebody else's. /// From fe6a36bde4496dfe28c3864f1c737447927e019b Mon Sep 17 00:00:00 2001 From: iris <2+iris@noreply.localhost> Date: Mon, 31 Aug 2026 22:56:23 -0400 Subject: [PATCH 6/8] Page back at all, and merge the run the boundary fell through Two defects on the same path, the second found while trying to reproduce the first. Both are invisible against a loopback server and both show up at `--delay 150`, which is what a phone over the tunnel actually costs. **A run of tool calls came back as two groups.** `joinPages` heals three things across a page boundary -- a message cut in half, a call separated from its result, and the *run* a group is named after -- but the third only ran on the path where a split call had been found. A boundary landing cleanly between two finished calls, which is most of them, went straight to concatenation and left the older page's calls under the name they were folded with. On screen, one run of twelve drawn as "Called 7 tools" and "Called 5 tools", with the seam wherever the reader happened to have paged. The two early returns were an optimisation on a list the size of one page, and what they saved was the work. **And nothing older loaded at all.** The history pager fires on the first layout, before a single event has arrived: `moreHistory` starts true, so the spinner is in the list, so `visibleItemsInfo` is not empty, and with no units loaded the room ahead adds up to zero. It then asked for the events `before = 0` -- the ones before the first one, which is none -- and an empty page is precisely how this code is told it has reached the start of the conversation. So `moreHistory` latched false, racing the opening page's own write of true, and a session that lost the race stopped one page from its newest end with no spinner and nothing on screen to say why. Guarded inside `loadOlderPage`, because it is a fact about the question rather than about who asked: the post-open fetch reaches it too, on the path where the opening page failed and left `oldestSeq` unset. Checked both ways round on the emulator, with the boundary placed on purpose (the opening page is 80 events, so it is a matter of counting back from the newest): 7 + 5 without the join fix, one group of 12 with it. And the case the change had no reason to touch still holds -- a boundary that *does* split a call, which is the path that always worked, and one through a streamed reply, which `healSplitMessage` owns and this does not go near. --- AGENTS.md | 14 ++++++++++++++ .../kotlin/com/example/aiapp/SessionScreen.kt | 18 ++++++++++++++++++ .../com/example/aiapp/TranscriptItems.kt | 10 ++++++++-- 3 files changed, 40 insertions(+), 2 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 435ac11..56bdcce 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -638,6 +638,20 @@ machine belongs in `~/.claude/TOOLCHAIN.md` (toolchain versions) or further means a chunked backwards reader. `RUST_LOG=ai_server=debug` logs each page with what was asked and what came back, which is how to see a phone paging back in real time. +- **Paging back has two failures that look like "there is simply no more + history", and neither says anything on screen.** Both fixed 2026-08-31, + both invisible on a loopback server and reproducible at `--delay 150`. + The pager fires on the *first layout*, before any event has arrived -- + `moreHistory` starts true, so the history spinner is in the list and + `visibleItemsInfo` is not empty -- and `before = 0` asks for the events + before the first one, which is none, which is exactly how this code is + told it has reached the start. `loadOlderPage` refuses `oldestSeq == 0` + now. And `joinPages` only ran `adoptRun` on the path where a *split* call + had been found, so a boundary landing cleanly between two calls -- most of + them -- left one run of tool calls drawn as two groups with the seam + wherever the reader happened to have paged. Reproducing either takes a + boundary placed on purpose: the opening page is 80 events, so arrange the + transcript so that event counts back from the newest. - **A page is 800 events and a screen is a handful of rows, and the two have no fixed ratio.** A run of thirty-five tool calls is one row; a reply is hundreds of text deltas folded into one. So anything that budgets in 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 d03afce..3105593 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/SessionScreen.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/SessionScreen.kt @@ -503,6 +503,24 @@ fun SessionScreen(settings: ServerSettings, summary: SessionSummary, onBack: () * value, which does not change under a running one. */ suspend fun loadOlderPage(limit: Int = HISTORY_PAGE): Boolean { + // Nothing is loaded, so there is no "before" to ask about, and asking anyway is not a + // harmless no-op: `before = 0` fetches the events before the first one, which is none, + // and an empty page is how this function is told it has reached the start of the + // conversation -- so it would latch `moreHistory` false and the session could never be + // paged back at all. + // + // The window it fires in is the first layout. `moreHistory` starts true, which puts the + // history spinner in the list, which makes `visibleItemsInfo` non-empty before a single + // event has arrived -- and with no units loaded the room ahead adds up to zero, so the + // pager fetches. On a loopback server the opening page beat it and nothing was ever + // wrong; at `--delay 150`, which is what a phone over the tunnel actually costs, it won + // the race and the transcript stopped one page from its newest end with no spinner and + // nothing to say why. + // + // Guarded here rather than at the two callers because it is a fact about the question, + // not about who is asking: the post-open fetch reaches it too, on the path where the + // opening page failed and left `oldestSeq` unset. + if (oldestSeq == 0L) return false // The fetch *and* the fold, both off the thread that draws. Only the fetch used to be, // and the fold is the expensive half: `foldEvent` returns a new list per event, so a page // of [HISTORY_PAGE] events is that many copies of a list growing to that length -- around diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/TranscriptItems.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/TranscriptItems.kt index c358417..ea36397 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/TranscriptItems.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/TranscriptItems.kt @@ -179,18 +179,24 @@ private fun runIdFor(items: List, id: String, tool: String): Str * boundary destroys. The older row wins on what a start knows (the tool's name, its input) and the * newer on what an end knows (the output, and whether it finished), which is the only way round * that loses nothing. + * + * The third thing is the *run*, and it is the one that used to be missed. Every page ends up here, + * but [adoptRun] only ran on the path where a split call had been found -- so the boundary that + * falls cleanly between two finished calls, which is most of them, went straight to concatenation + * and left the older page's calls under the run name they were folded with. On screen: one run of + * tool calls drawn as two groups, with the seam wherever the reader happened to have paged. The two + * early returns were an optimisation on a list the size of one page, and they were skipping work + * rather than saving it. */ fun joinPages(earlier: List, later: List): List { val (older, newer) = healSplitMessage(earlier, later) val startedEarlier = older.filterIsInstance().mapTo(mutableSetOf()) { it.id } - if (startedEarlier.isEmpty()) return older + newer val endedLater = newer .filterIsInstance() .associateBy { it.id } .filterKeys { it in startedEarlier } - if (endedLater.isEmpty()) return older + newer val healed = older.map { row -> val half = (row as? TranscriptItem.ToolRun)?.let { endedLater[it.id] } if (row is TranscriptItem.ToolRun && half != null) { From 6236f0d5bd5ef3d647955f3cc5e4a5c47cc63ded Mon Sep 17 00:00:00 2001 From: iris <2+iris@noreply.localhost> Date: Mon, 31 Aug 2026 23:02:49 -0400 Subject: [PATCH 7/8] Show a message another agent sent, on a session this server is running Peer messages were only ever produced by the *import* path, reading them out of the CLI's own session file. A message another agent sent to a session this server was driving appeared nowhere, so the session started working on something nobody on the phone had asked for and there was nothing on screen to explain it. Measured rather than guessed, because the obvious place to look for it is empty: a real cross-session message sent to a real `--input-format stream-json` session on CLI 2.1.237 produces **no `user` record**, and nothing in the partial-message stream mentions it either. The whole of it arrives as an `origin` object on the turn's `result`, in exactly the shape the session file records -- so `import::peer_message` now reads both, one function for one wire format. Two copies would drift the first time a field is renamed, and the half that drifted would go on producing nothing, which is indistinguishable from nobody having sent anything. The cost is the position: the note lands after the reply it caused rather than above it, because at no earlier point in the turn does the CLI say why the turn started. Taken deliberately over the alternative -- a second reader tailing the CLI's own session file for the one record stdout does not carry, which is two sources of truth for one conversation and a poll per live session. Recorded in PLAN.md so that if the CLI ever announces the injection where it happens, the next reader knows to move it there. Both halves tested: the real record shape, and an ordinary result carrying no `origin` -- which is the half that decides whether the check is a check. Four ordinary results on a real session's stdout had none between them. --- AGENTS.md | 7 ++ PLAN.md | 25 +++++++ server/src/session/claude/translate.rs | 91 ++++++++++++++++++++++++++ server/src/session/import.rs | 11 +++- 4 files changed, 132 insertions(+), 2 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 56bdcce..0cf8e4a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -273,6 +273,13 @@ first if a remote spawn ever mangles an argument. reply into the tool output under it -- and a container per row leaves whatever was drawn without one silently unselectable, which nothing on screen reports. Rows keep their tap handlers; selection is a long press. +- **A message from another agent reaches a live session on the turn's + `result`, not before.** Measured on CLI 2.1.237 by sending a real + cross-session message to a real stream-json session: no `user` record, and + nothing in the partial-message stream -- the whole of it is an `origin` + object on the `result`, the same shape the session file records, which is + why `import::peer_message` reads both. So the note is drawn *after* the + reply it caused; that is the wire, not a bug. See PLAN.md. - **A queued message can be tapped to take it back**, which is `POST /sessions/{id}/unqueue` and a `messageDropped` event -- see PLAN.md's "Taking a queued message back". On a **Claude** session it always refuses, diff --git a/PLAN.md b/PLAN.md index 7e37ceb..eb5f2a2 100644 --- a/PLAN.md +++ b/PLAN.md @@ -247,6 +247,31 @@ turn. Claude's dialect: a `user` message on stdin mid-stream; pi's: `steer`. - Images in: base64 image content blocks in the stream-json user message. - Working directory, host, and model are spawn-screen fields. +### A message from another agent, on a live session (measured 2026-08-31) + +Peer messages were only ever produced by the *import* path, reading them out +of the CLI's own session file — so a message another agent sent a session +this server was running never appeared at all, and the session simply +started working on something nobody on the phone had asked for. + +Measured rather than guessed, by sending a real cross-session message to a +real `--input-format stream-json` session on CLI 2.1.237: the CLI emits **no +`user` record** for it, and nothing in the partial-message stream mentions +it. The whole of it arrives as an `origin` object on the turn's `result`, in +the same shape the session file records — `kind: "peer"`, the sending +session's `name`, and the message as `body` — so `import::peer_message` reads +both, and there is one function for one wire format. Only peer-caused turns +carry it: four ordinary results on a real session's stdout had no `origin` +between them. + +**The cost is the position.** The note lands after the reply it caused rather +than above it, because at no earlier point in the turn does the CLI say why +the turn started. The alternative is a second reader tailing the CLI's own +session file for the one record stdout does not carry — two sources of truth +for one conversation and a poll per live session — and it was rejected on +that. If the CLI ever announces the injection at the point it happens, this +moves to that record and the ordering comes right with it. + ### Taking a queued message back (decided 2026-08-31) A message sent into a running turn is drawn as a bubble waiting below the diff --git a/server/src/session/claude/translate.rs b/server/src/session/claude/translate.rs index b1c1b5b..17a7d51 100644 --- a/server/src/session/claude/translate.rs +++ b/server/src/session/claude/translate.rs @@ -221,6 +221,33 @@ impl Translator { .and_then(Value::as_u64) .unwrap_or(0); let mut events = Vec::new(); + // A turn another agent started, which is only knowable here. + // + // Measured against CLI 2.1.237 (2026-08-31) by sending a + // real cross-session message to a real stream-json session: + // the CLI emits no `user` record for it, and nothing in the + // partial-message stream mentions it either. The whole of + // it arrives as an `origin` object on the turn's `result`, + // in the same shape the session file records -- so this is + // `import::peer_message` reading a different record. + // + // The cost is the position: the note lands after the reply + // it caused rather than above it, because at no earlier + // point in the turn does the CLI say why the turn started. + // Taken deliberately over the alternative, which is a + // second reader tailing the CLI's own session file for the + // one record stdout does not carry -- two sources of truth + // for one conversation, and a poll per live session. What + // it buys is the thing that was missing entirely: a session + // that starts working on something nobody on this phone + // asked for is otherwise unexplainable from the phone. + // + // Only peer-caused turns carry it: measured over a real + // session's stdout, four ordinary results and no `origin` + // between them. + if let Some(peer) = crate::session::import::peer_message(message) { + events.push(peer); + } // Whichever way this result went, the interrupt it may have // been answering is now spent. let asked_to_stop = std::mem::take(&mut self.interrupting); @@ -1089,6 +1116,70 @@ mod tests { ); } + /// A turn another agent started says so, on the record that carries it. + /// + /// The line is the real shape, taken from a real cross-session message + /// sent to a real stream-json session on CLI 2.1.237 (2026-08-31) -- + /// including the `from` socket path, which is deliberately *not* what a + /// reader is shown: the sending session's `name` is what they recognise + /// it by. The `body` is the message as it was written; the content the + /// model is given beside it wraps the same text in a preamble and a + /// `` tag, which is written for the model rather + /// than for a person. + /// + /// The note comes before the usage and the idle, so it sits as close to + /// the turn it explains as the wire allows -- which is after the reply, + /// not above it. See the comment at the callsite for why that is the + /// best available position rather than an oversight. + #[test] + fn a_turn_started_by_another_agent_records_who_and_what() { + let dir = tempfile::tempdir().expect("tempdir"); + let mut translator = Translator::new(dir.path().to_path_buf()); + let events = translate_lines( + &mut translator, + &[ + r#"{"type":"result","subtype":"success","is_error":false,"session_id":"s","usage":{"input_tokens":2,"output_tokens":5},"origin":{"kind":"peer","from":"uds:/run/user/1000/cc-socks/137108.sock","verifiedPeerPid":137108,"msg_id":"1e729740","name":"ai-app-2-fb","fromMode":"prompting","body":"Reply with just the word ACK."}}"#, + ], + ); + assert_eq!( + events, + vec![ + Event::PeerMessage { + from: "ai-app-2-fb".to_string(), + text: "Reply with just the word ACK.".to_string(), + }, + Event::UsageDelta { + tokens: 7, + context: None + }, + Event::Status { + state: SessionStatus::Idle + }, + ] + ); + } + + /// And an ordinary turn does not, which is the half that decides + /// whether the check above is a check or a rubber stamp. Measured over + /// a real session's stdout: four results, no `origin` between them. + #[test] + fn an_ordinary_turn_carries_no_peer_note() { + let dir = tempfile::tempdir().expect("tempdir"); + let mut translator = Translator::new(dir.path().to_path_buf()); + let events = translate_lines( + &mut translator, + &[ + r#"{"type":"result","subtype":"success","is_error":false,"session_id":"s","usage":{"input_tokens":2,"output_tokens":5}}"#, + ], + ); + assert!( + !events + .iter() + .any(|event| matches!(event, Event::PeerMessage { .. })), + "a turn nobody else started must not be attributed to anyone: {events:?}" + ); + } + /// The context is the last assistant message's, not the result's. /// /// Real figures from a two-message haiku turn on 2.1.237, captured diff --git a/server/src/session/import.rs b/server/src/session/import.rs index 74bd783..c43f6dc 100644 --- a/server/src/session/import.rs +++ b/server/src/session/import.rs @@ -520,7 +520,7 @@ pub fn events_from(text: &str, session_dir: &std::path::Path) -> Vec { events } -/// A message from another agent, as the CLI records one. +/// A message from another agent, as the CLI reports one. /// /// Measured from a real session file (2026-08-29): the record is a `user` /// one marked `isMeta`, and its `origin` carries `kind: "peer"`, the @@ -529,7 +529,14 @@ pub fn events_from(text: &str, session_dir: &std::path::Path) -> Vec { /// preamble and a `` tag, which is written for the /// model that has to read it rather than for a person -- so the body is /// what a reader is shown, and the name is who they are told sent it. -fn peer_message(record: &Value) -> Option { +/// +/// Shared with the live driver (`claude::translate`), which finds the same +/// `origin` object on a different record -- so this reads the object and +/// not the record around it. One function because it is one wire format: +/// two copies would drift the first time the CLI renames a field, and the +/// half that drifted would go on producing nothing at all, which is +/// indistinguishable from nobody having sent anything. +pub(in crate::session) fn peer_message(record: &Value) -> Option { let origin = record.get("origin")?; if origin.get("kind").and_then(Value::as_str) != Some("peer") { return None; From deb908034cb4e59e954244b0faa617dbc29d0768 Mon Sep 17 00:00:00 2001 From: iris <2+iris@noreply.localhost> Date: Mon, 31 Aug 2026 23:16:34 -0400 Subject: [PATCH 8/8] Move a session to another working directory `POST /sessions/{id}/cwd`, behind a field in the session settings dialog. A working directory is settled when the process is spawned -- the CLI is launched with it as its cwd and there is no control request that changes one -- so this records the new one and ends the process that is in the old one. It does not start a replacement, and the field says so in a line beside it: a session with no process starts on the next message or on Start, which is this app's rule for that everywhere else, and "usually restarts" is a worse control than "always stops". The path is checked against the session's own machine and refused if it is not there. The spawn path corrects instead of refusing, because it is resuming a directory the *machine* recorded and that can be gone through nobody's fault; a path somebody has just typed is different, and a mistyped one accepted here would surface much later as a session that would not start, with nothing pointing at the typo. The refusal names the machine and the path, and is drawn under the field it is about. Nothing of Claude Code's own is moved, and that is measured rather than assumed: on CLI 2.1.237, `claude --resume ` finds a session from any working directory -- an id that does not exist answers "No conversation found with session ID", and a real one resumed from an unrelated directory did not. So the conversation continues in the new place with nothing relocated. Doing otherwise would mean reproducing a rule this app cannot see the whole of; PLAN.md records what that rule is, for whoever tries. Found while checking it: `SessionInfo.cwd` came from the snapshot a session launched with, so a moved session went on reporting its *old* directory for as long as its process lived -- a dialog showing a directory the next launch would not use, with nothing saying so. It is read from the config where the row is built now, the same way `setup_name` already was, and for the reason already written above `setup_name`: only the manager holds the config, and both of these change under a running session. Checked end to end on the emulator against a session whose process really does take a cwd: /proc said /tmp/cwd-a before and /tmp/cwd-b after, the dialog showed the new path immediately rather than after a restart, and a directory that is not there and a relative path were both refused with the session left exactly as it was. --- AGENTS.md | 8 ++ PLAN.md | 36 +++++++ .../src/main/kotlin/com/example/aiapp/Api.kt | 30 ++++++ .../example/aiapp/SessionSettingsDialog.kt | 95 +++++++++++++++++-- server/src/routes.rs | 65 +++++++++++++ server/src/session/mod.rs | 82 +++++++++++++++- 6 files changed, 305 insertions(+), 11 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 0cf8e4a..193cce7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -273,6 +273,14 @@ first if a remote spawn ever mangles an argument. reply into the tool output under it -- and a container per row leaves whatever was drawn without one silently unselectable, which nothing on screen reports. Rows keep their tap handlers; selection is a long press. +- **A session can be moved to another directory** from the settings dialog + (`POST /sessions/{id}/cwd`). It stops the process, because a working + directory is settled at spawn; the next message starts it in the new one. + **`claude --resume ` finds a session from any directory** -- measured + on 2.1.237 -- so nothing of Claude Code's is relocated, and should you ever + be tempted, its project directory is the path with every non-alphanumeric + character replaced by `-`, cut at 200 characters with a hash appended, and + overridable besides. - **A message from another agent reaches a live session on the turn's `result`, not before.** Measured on CLI 2.1.237 by sending a real cross-session message to a real stream-json session: no `user` record, and diff --git a/PLAN.md b/PLAN.md index eb5f2a2..ed0064a 100644 --- a/PLAN.md +++ b/PLAN.md @@ -247,6 +247,42 @@ turn. Claude's dialect: a `user` message on stdin mid-stream; pi's: `steer`. - Images in: base64 image content blocks in the stream-json user message. - Working directory, host, and model are spawn-screen fields. +### Moving a session to another directory (decided 2026-08-31) + +`POST /sessions/{id}/cwd {cwd}`, behind a field in the session settings +dialog. The directory is settled when the process is spawned -- the CLI is +launched with it as its cwd and there is no control request that changes one +-- so this records the new one and **ends** the process that is in the old +one. It does not start a replacement: a session with no process starts on +the next thing said to it or on Start, which is this app's rule for that +everywhere else, and "usually restarts" would be a worse control than +"always stops" (starting one here would have to wait for the recorded status +to catch up with a process already gone). + +The path is checked against the session's own machine and **refused** if it +is not there, rather than corrected. The spawn path corrects instead, +because it is resuming a directory the *machine* recorded and that can be +gone through nobody's fault; a path somebody has just typed is different, +and a mistyped one accepted here would surface much later as a session that +would not start, with nothing pointing at the typo. + +**Nothing of Claude Code's own is moved**, and that is a measurement rather +than an omission. Checked against CLI 2.1.237 on 2026-08-31: `claude +--resume ` finds a session from any working directory — an id that does +not exist answers "No conversation found with session ID", and a real one +resumed from an unrelated directory did not. So the conversation continues +in the new place with nothing relocated, and the session file stays under +the project directory the CLI made for it, which is where the CLI itself +looks. Relocating it would mean reproducing a rule this app cannot see the +whole of: the CLI's project directory is the path with every non-alphanumeric +character replaced by `-`, truncated at 200 characters with a hash of its own +appended, and an override can replace the name entirely. + +While fixing this: `SessionInfo.cwd` came from the snapshot a session +launched with, so a moved session reported its *old* directory for as long +as the process lived. It is read from the config where the row is built now, +the same way `setup_name` already was and for the same reason. + ### A message from another agent, on a live session (measured 2026-08-31) Peer messages were only ever produced by the *import* path, reading them out diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/Api.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/Api.kt index eff62ff..0f8997f 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/Api.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/Api.kt @@ -149,6 +149,14 @@ data class SessionSummary( * on when a backend is too old to say, which matches what that backend actually does. */ val notify: Boolean, + /** + * The directory the session works in, or null where it was never given one. + * + * Null is not "the home directory": it is the session never having been told, and what the + * process then starts in belongs to whatever launches it. Shown as unset rather than filled in + * with a guess, so a reader changing it is choosing rather than confirming. + */ + val cwd: String?, /** * How much context this session is holding, as the server last measured it -- see * `SessionEvent.UsageDelta`. @@ -184,6 +192,7 @@ private fun parseSession(session: JSONObject) = permissionMode = session.optString("permissionMode").ifEmpty { null }, imported = session.optBoolean("imported", false), notify = session.optBoolean("notify", true), + cwd = session.optString("cwd").ifEmpty { null }, contextTokens = if (session.has("contextTokens")) session.getLong("contextTokens") else null, maxImageEdge = session.optInt("maxImageEdge", 0).takeIf { it > 0 }, @@ -509,6 +518,27 @@ fun unqueueMessage(settings: ServerSettings, sessionId: String, messageId: Strin ) {} } +/** + * Moves a session to a different working directory. + * + * The server checks the directory is there on that machine and refuses if it is not -- a mistyped + * path accepted here would surface much later, as a session that would not start, with nothing + * pointing at the typo. + * + * Its process is **stopped**, because a working directory is settled when the process is spawned. + * The next thing said to the session starts it again in the new one, which is this app's rule for a + * session with no process everywhere else. + */ +fun setSessionCwd(settings: ServerSettings, sessionId: String, cwd: String) { + requestFromServer( + settings, + "/sessions/$sessionId/cwd", + method = "POST", + jsonBody = JSONObject().put("cwd", cwd).toString(), + readTimeoutMs = 30000, + ) {} +} + /** Uploads one picked image; the returned id goes into [sendMessage]. */ fun uploadAttachment( settings: ServerSettings, diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/SessionSettingsDialog.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/SessionSettingsDialog.kt index 7816ba0..d04f52b 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/SessionSettingsDialog.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/SessionSettingsDialog.kt @@ -68,17 +68,53 @@ fun SessionSettingsDialog( // it, which is what not knowing looks like: distinguishable from off, and from a refusal. var notify by remember(sessionId) { mutableStateOf(null) } var notifyError by remember { mutableStateOf(null) } + // Where the session works. Null until the server has been asked, for the same reason the + // switch above is: the row this dialog opened over is a snapshot, and a path drawn from it + // could be one somebody changed from another device. An empty answer is a session that was + // never given a directory, which is not the same as one whose directory is unknown -- the + // field is only enabled once one of those two is settled. + var cwd by remember(sessionId) { mutableStateOf(null) } + var typedCwd by remember(sessionId) { mutableStateOf("") } + var cwdError by remember { mutableStateOf(null) } + var movingCwd by remember { mutableStateOf(false) } LaunchedEffect(sessionId) { - notify = + try { + val fresh = withContext(Dispatchers.IO) { fetchSession(settings, sessionId) } + notify = fresh.notify + cwd = fresh.cwd.orEmpty() + typedCwd = fresh.cwd.orEmpty() + } catch (e: ApiException) { + // Left unknown rather than falling back to the stale row: the switch stays + // disabled, instead of offering a position nothing confirmed. + notifyError = e.message + notify = null + } + } + + /** + * Moves the session, which ends the process that is in the old directory. + * + * Said plainly beside the field rather than confirmed in a second dialog: what it costs is a + * process, and a stopped session is a state this app already has a word and a button for. + */ + fun moveCwd() { + val chosen = typedCwd.trim() + if (movingCwd || chosen.isEmpty() || chosen == cwd) return + movingCwd = true + cwdError = null + scope.launch { try { - withContext(Dispatchers.IO) { fetchSession(settings, sessionId).notify } + withContext(Dispatchers.IO) { setSessionCwd(settings, sessionId, chosen) } + cwd = chosen } catch (e: ApiException) { - // Left unknown rather than falling back to the stale row: the switch stays - // disabled, instead of offering a position nothing confirmed. - notifyError = e.message - null + // Where it happened: this field is the only thing on screen that knows a move was + // asked for, and the reason is usually the path itself. + cwdError = e.message + } finally { + movingCwd = false } + } } // Moved optimistically so the switch answers the finger that moved it, and put back if the @@ -167,6 +203,53 @@ fun SessionSettingsDialog( style = MaterialTheme.typography.bodySmall, ) } + Spacer(Modifier.height(8.dp)) + Row( + verticalAlignment = Alignment.CenterVertically, + modifier = Modifier.fillMaxWidth(), + ) { + OutlinedTextField( + value = typedCwd, + onValueChange = { typedCwd = it }, + label = { Text("Working directory") }, + // What the field cannot say by being empty: a session that was never + // given one starts wherever its launcher does, and this names that + // rather than showing a path nobody chose. + placeholder = { Text("wherever the session was started") }, + singleLine = true, + enabled = cwd != null && !movingCwd, + modifier = Modifier.weight(1f), + keyboardOptions = KeyboardOptions(imeAction = ImeAction.Done), + keyboardActions = KeyboardActions(onDone = { moveCwd() }), + ) + TextButton( + onClick = { moveCwd() }, + enabled = + cwd != null && + !movingCwd && + typedCwd.trim().isNotEmpty() && + typedCwd.trim() != cwd, + ) { + Text(if (movingCwd) "Moving..." else "Move") + } + } + // The whole of what pressing Move does, where it is about to be pressed. A + // directory is settled when the process is spawned, so there is no changing one + // under a running session -- it is ended, and the next thing said to the session + // starts it in the new place. + Text( + "Moving stops the session's process. It starts again in the new directory " + + "with the next message, or with Start.", + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + cwdError?.let { + Text( + it, + color = MaterialTheme.colorScheme.error, + style = MaterialTheme.typography.bodySmall, + ) + } error?.let { Spacer(Modifier.height(8.dp)) Text( diff --git a/server/src/routes.rs b/server/src/routes.rs index c305e9b..f55dfec 100644 --- a/server/src/routes.rs +++ b/server/src/routes.rs @@ -24,6 +24,8 @@ //! POST /sessions/{id}/stop end the process; the session and transcript stay //! POST /sessions/{id}/start run the process again, continuing the conversation //! POST /sessions/{id}/title {title} +//! POST /sessions/{id}/cwd {cwd} -- move it; stops the process, +//! which starts again in the new one //! POST /sessions/{id}/model {model} //! POST /sessions/{id}/command {text} -- /compact, /clear, /rename x, or the dialect's own //! (starts the process first if it has exited) @@ -101,6 +103,7 @@ pub fn router(manager: Arc) -> Router { .route("/sessions/{id}/stop", post(stop)) .route("/sessions/{id}/start", post(start)) .route("/sessions/{id}/title", post(rename)) + .route("/sessions/{id}/cwd", post(set_cwd)) .route("/sessions/{id}/model", post(set_model)) .route("/sessions/{id}/permission-mode", post(set_permission_mode)) .route("/sessions/{id}/notify", post(set_notify)) @@ -1054,6 +1057,68 @@ async fn rename( Ok(StatusCode::NO_CONTENT) } +#[derive(Deserialize)] +#[serde(rename_all = "camelCase")] +#[serde(deny_unknown_fields)] +struct CwdRequest { + cwd: PathBuf, +} + +/// Moves a session to a different working directory. +/// +/// The directory is checked here rather than in the manager because +/// checking it is an ssh round trip on a remote setup, and the manager is +/// not async -- the same division `POST /sessions` already makes for the +/// directory an import was recorded in. +/// +/// Checked rather than trusted, and refused rather than corrected: a +/// mistyped path that was accepted would leave a session recorded somewhere +/// its process cannot start, and the failure would arrive later, as a +/// session that would not come back, with nothing pointing at the typo. The +/// spawn path corrects instead because it is resuming a directory the +/// *machine* recorded, which can be gone through nobody's fault; a path +/// somebody has just typed is different. +/// +/// Note what this does not do: it does not start a replacement process. +/// See [`SessionManager::set_session_cwd`]. +async fn set_cwd( + State(manager): State>, + UrlPath(id): UrlPath, + axum::Json(body): axum::Json, +) -> Result { + let session = manager + .sessions() + .into_iter() + .find(|session| session.id == id) + .ok_or_else(|| ApiError::NotFound(format!("no session {id}")))?; + let cwd = body.cwd.to_string_lossy().trim().to_string(); + if cwd.is_empty() { + return Err(ApiError::BadRequest( + "a working directory is a path, and this one is empty".to_string(), + )); + } + // Absolute, because the alternative is relative to whatever the CLI is + // launched from, which is not something the person typing it can see. + if !cwd.starts_with('/') && !cwd.starts_with('~') { + return Err(ApiError::BadRequest(format!( + "{cwd} is not an absolute path, so where it would be depends on where the \ + session happens to start" + ))); + } + let setup = setup_by_id(&manager, &session.setup)?; + let transport = crate::session::transport::Transport::for_setup(&setup); + if !crate::session::import::directory_exists(&transport, &cwd).await { + return Err(ApiError::BadRequest(format!( + "{} has no directory {cwd}", + setup.name + ))); + } + manager + .set_session_cwd(&id, PathBuf::from(&cwd)) + .map_err(bad_request)?; + Ok(StatusCode::NO_CONTENT) +} + #[derive(Deserialize)] #[serde(deny_unknown_fields)] struct ModelRequest { diff --git a/server/src/session/mod.rs b/server/src/session/mod.rs index c34855c..c46ece6 100644 --- a/server/src/session/mod.rs +++ b/server/src/session/mod.rs @@ -500,15 +500,31 @@ impl LiveSession { Ok(name) } - /// `setup_name` is passed in rather than stored: only the manager - /// holds the config, and the label can change under a running session. + /// `setup_name` and `cwd` are passed in rather than read from the + /// snapshot this session launched with: only the manager holds the + /// config, and both of them can change under a running session. The + /// label changes when a setup is renamed; the directory changes when + /// somebody moves the session, and reading the snapshot reported the + /// old one for as long as the process lived -- a screen showing a + /// directory the next launch will not use, with nothing saying so. + /// + /// Passed rather than mirrored into `Shared`, which is where `title` + /// and `notify` live: a second copy is a second thing to keep level, + /// and this way there is one answer, read where the row is built. + /// /// `kind` rather than the facts derived from it: two of this row's /// fields are answers about the provider's *kind*, and passing them /// separately meant every caller deriving each one and a third arriving /// as a third parameter. `None` where the provider has been edited away, /// which is a session that cannot run -- so both answers are the /// cautious one rather than a guess. - fn info(&self, setup_name: &str, imported: bool, kind: Option) -> SessionInfo { + fn info( + &self, + setup_name: &str, + cwd: Option<&Path>, + imported: bool, + kind: Option, + ) -> SessionInfo { SessionInfo { id: self.meta.id.clone(), provider: self.meta.provider.clone(), @@ -522,7 +538,7 @@ impl LiveSession { max_image_edge: kind.and_then(DriverKind::max_image_edge), imported, keeps_own_transcript: kind.is_some_and(DriverKind::keeps_own_transcript), - cwd: self.meta.cwd.clone(), + cwd: cwd.map(Path::to_path_buf), status: *self.shared.status.lock().unwrap(), last_activity: *self.shared.last_activity.lock().unwrap(), created: self.meta.created, @@ -962,6 +978,7 @@ impl SessionManager { .map(|meta| match inner.live.get(&meta.id) { Some(session) => session.info( label_of(&inner.config, &meta.setup), + meta.cwd.as_deref(), import::read_cursor(&self.data_dir.join(&meta.id)).is_some(), kind_of(&inner.config, &meta.setup, &meta.provider), ), @@ -1123,6 +1140,7 @@ impl SessionManager { // listing asks of the directory a moment later. let info = session.info( &setup.name, + session.meta.cwd.as_deref(), import::read_cursor(&self.data_dir.join(&id)).is_some(), Some(provider.kind), ); @@ -1285,6 +1303,60 @@ impl SessionManager { /// for every provider that has a process at all -- so asking it here /// stops a session whose driver is in no state to be asked, and adds no /// method a new driver could implement wrongly. + /// Moves a session to a different working directory. + /// + /// The directory is settled at spawn -- the CLI is launched with it as + /// its cwd and there is no control request that changes one -- so this + /// records the new one and ends the process that is in the old one. It + /// does **not** start a replacement: a session with no process starts + /// on the next thing said to it, or on Start, which is this app's one + /// rule for that everywhere else. Starting one here would have to wait + /// for the recorded status to catch up with a process that is already + /// gone, and "usually restarts" is a worse control than "always stops". + /// + /// Nothing of Claude Code's own is moved, and that is a measurement + /// rather than an omission: `claude --resume ` finds a session from + /// any working directory (checked against 2.1.237 on 2026-08-31 -- an + /// id that does not exist says "No conversation found with session ID" + /// and a real one resumed from an unrelated directory did not), so the + /// conversation continues in the new place with nothing relocated. The + /// file stays under the project directory the CLI made for it, which is + /// where the CLI itself looks. Reimplementing that directory's name to + /// move it would mean reproducing a rule this app cannot see the whole + /// of -- the CLI truncates at 200 characters and appends a hash of its + /// own, and an override can replace the name entirely -- to relocate a + /// file the CLI is still writing. + /// + /// Whether the directory exists is the caller's question, because + /// asking it is an ssh round trip on a remote setup; see the route. + pub fn set_session_cwd(&self, id: &str, cwd: PathBuf) -> Result<()> { + { + let mut inner = self.inner.write().unwrap(); + if !inner.config.sessions.iter().any(|meta| meta.id == id) { + bail!("no session {id}"); + } + let mut candidate = inner.config.clone(); + for meta in candidate.sessions.iter_mut().filter(|meta| meta.id == id) { + meta.cwd = Some(cwd.clone()); + } + candidate.save(&self.config_path)?; + inner.config = candidate; + } + // Saved first, so a process that cannot be stopped leaves a session + // that will start in the right place rather than one recorded in a + // directory nothing agrees with. + let dir = self.data_dir.join(id); + if let Some(record) = process::live(&dir) { + tracing::info!( + "moving session {id} to {} -- stopping pid {}", + cwd.display(), + record.pid + ); + process::stop(&record, process::STOP_GRACE); + } + Ok(()) + } + pub fn stop_session(&self, id: &str) -> Result<()> { if !self .inner @@ -2497,7 +2569,7 @@ mod tests { assert_eq!(first.session_id, info.id); // The title travels with it, because the phone may have no screen // open to look one up on. - assert_eq!(first.title, session.info("m", false, None).title); + assert_eq!(first.title, session.info("m", None, false, None).title); manager.set_session_notify(&info.id, false).expect("off"); // Subscribed before the message, or the turn can finish in the gap