diff --git a/AGENTS.md b/AGENTS.md index bdf14db..193cce7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -245,6 +245,57 @@ 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. +- **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 + 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 + 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, + 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 @@ -602,6 +653,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/PLAN.md b/PLAN.md index a0c9ee1..ed0064a 100644 --- a/PLAN.md +++ b/PLAN.md @@ -247,6 +247,96 @@ 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 +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 +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 +749,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 @@ -895,6 +986,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/app/androidApp/src/main/kotlin/com/example/aiapp/Api.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/Api.kt index 8d21262..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 }, @@ -492,6 +501,44 @@ 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(), + ) {} +} + +/** + * 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/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/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/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/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 e523c0d..3105593 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 @@ -245,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. @@ -256,6 +260,14 @@ 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()) } + // 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 @@ -371,6 +383,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) { @@ -486,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 @@ -890,6 +925,43 @@ 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 } + } + } + + /** 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 + } + fun act(onDone: () -> Unit = {}, action: () -> Unit) { scope.launch { try { @@ -1160,7 +1232,15 @@ 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 + // 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) }, ) } } @@ -1169,7 +1249,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( @@ -1256,7 +1343,12 @@ fun SessionScreen(settings: ServerSettings, summary: SessionSummary, onBack: () } }, image = { ref -> - SessionImage(settings, summary.id, ref) + SessionImage( + settings, + summary.id, + ref, + ::openImage, + ) }, ) is TranscriptRow.Single -> @@ -1267,6 +1359,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 @@ -1282,6 +1375,8 @@ fun SessionScreen(settings: ServerSettings, summary: SessionSummary, onBack: () AssistantMessage( item.text, replies, + openNotes = openMemories, + onToggleNote = ::toggleMemory, live = true, ) is TranscriptItem.ToolRun -> @@ -1307,7 +1402,12 @@ fun SessionScreen(settings: ServerSettings, summary: SessionSummary, onBack: () } }, image = { ref -> - SessionImage(settings, summary.id, ref) + SessionImage( + settings, + summary.id, + ref, + ::openImage, + ) }, ) is TranscriptItem.QuestionCard -> @@ -1328,7 +1428,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, @@ -1465,8 +1570,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 @@ -1539,7 +1653,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 { @@ -1618,6 +1733,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 }) } @@ -1679,6 +1797,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( @@ -1686,7 +1809,10 @@ private fun UserBubble( sessionId: String, text: String, images: List = emptyList(), + onOpenImage: (String) -> Unit, pending: Boolean = false, + refusal: String? = null, + onTakeBack: (() -> Unit)? = null, ) { Box(Modifier.fillMaxWidth()) { Card( @@ -1700,7 +1826,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 @@ -1718,15 +1856,60 @@ 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)) + 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, +) + +/** + * 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/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/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, 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) { @@ -363,6 +369,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..f55dfec 100644 --- a/server/src/routes.rs +++ b/server/src/routes.rs @@ -17,11 +17,15 @@ //! `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 //! 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) @@ -68,7 +72,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,11 +97,13 @@ 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)) .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)) @@ -123,6 +129,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 +142,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 +911,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)] @@ -1012,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/claude.rs b/server/src/session/claude.rs index 0805307..138a013 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}; @@ -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 }); + } } } @@ -595,6 +613,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/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/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..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 @@ -53,7 +55,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 @@ -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, @@ -796,6 +819,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/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; diff --git a/server/src/session/mod.rs b/server/src/session/mod.rs index 4fbac3b..c46ece6 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`]. @@ -483,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(), @@ -505,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, @@ -945,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), ), @@ -1106,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), ); @@ -1268,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 @@ -2069,10 +2158,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, @@ -2088,6 +2193,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 @@ -2137,8 +2249,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. @@ -2163,6 +2275,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. @@ -2383,28 +2502,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. @@ -2435,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 @@ -2460,6 +2594,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. /// @@ -2716,6 +2895,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");