Stop and start a session's process from the composer
The composer's second button now says what pressing it would do to the process behind the session, in one place that is always there: an orange pause while a turn is running (interrupt, the process stays), a red stop when it is not (end the process), a green play when it has exited (start it again on the same conversation). Send is disabled while there is nothing to send, rather than pressable and silent. Behind it, two routes. `stop` signals the recorded process and says nothing else -- the driver's own reader already reports a death correctly, and announcing it here would be a guess ahead of the measurement. `start` replaces the driver and nothing else, so the transcript, the pump and every open phone's stream stay where they were and there is still one writer of the transcript; it is refused unless the session is known to have exited, since starting on `Unknown` is the two-CLIs-on-one-conversation fault. That last rule found a bug in the launch path: a relaunched session took its status from the transcript, so one whose process had died before a backend restart reported `exited` while the launch had just started a new process -- which refuses every command and offers a phone the chance to start a second CLI on a live conversation. A launch that leaves a process running now says idle. The icon font moves to the Mono face, where every glyph is one em square, so two icon buttons are the same width without either being told one; the proportional advances ran 0.46 to 0.92 em and Send came out visibly wider than Stop. GLYPH_SIZE comes down to match, since a glyph that fills its em draws bigger at the same point size. Verified against a stand-in CLI on the emulator: idle -> stop -> exited -> start -> idle, a turn interrupted from the pause button, and both buttons measured at 171x105 device pixels. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
317ad29d85
commit
6154cb1949
13 files changed
+553
-70
No files matched your search
@@ -89,7 +89,11 @@ repo is in PLAN.md's "Backend layout" section.
|
||||
codepoint in the Kotlin that the script did not subset is a glyph that
|
||||
silently isn't there. Rerun the script and commit its output when adding
|
||||
one; it needs network access. `md-cog` and `md-refresh` are deliberately
|
||||
the same codepoints dev-updater uses and must not drift from it.
|
||||
the same codepoints dev-updater uses and must not drift from it. The
|
||||
subset is the **Mono** face, where every glyph is one em square — that is
|
||||
what makes two icon buttons the same width without either being given
|
||||
one, and it is why `GLYPH_SIZE` is smaller than it looks like it should
|
||||
be.
|
||||
- `.dev-updater.ron` — what Dev Updater is asked to do with this checkout:
|
||||
the server (built in `server/`, run as `service: Managed(...)`) and the
|
||||
APK (built in `app/`), built in parallel. The project it serves is the
|
||||
@@ -280,15 +284,21 @@ day:
|
||||
- **Stopping the server no longer stops the sessions.** After `pkill
|
||||
ai-server` the `claude` processes are still there, on purpose, and the
|
||||
next start picks them up (`reattaching to the claude-cli it left
|
||||
running` in the log). To actually end one, delete the session — that is
|
||||
the only path that stops a process.
|
||||
running` in the log). To end one, either `POST /sessions/{id}/stop` —
|
||||
which keeps the session and its transcript, and `POST .../start` brings
|
||||
the process back on the same conversation — or delete the session, which
|
||||
ends the conversation too.
|
||||
- **Each session directory now holds `process.json`, `stdin.fifo`,
|
||||
`stdout.log` and `stderr.log`.** `stdout.log` is the driver's input, read
|
||||
from the byte offset in `process.json`; removing either by hand while the
|
||||
session is live loses output or replays it.
|
||||
- **`--resume` only ever runs when nothing is running.** That check is the
|
||||
fix for the incident below, and the reason there is one entry point
|
||||
(`ClaudeDriver::launch`) rather than a spawn and an attach.
|
||||
(`ClaudeDriver::launch`) rather than a spawn and an attach. The status a
|
||||
launch reports obeys the same rule: a session recorded as `exited` whose
|
||||
launch has just started a process reports `idle`, because `exited` is the
|
||||
word that refuses every command and offers a phone the chance to start a
|
||||
second CLI on a live conversation.
|
||||
- Remote sessions are adopted too. The pid recorded for one is the **`ssh`
|
||||
client's**, on this machine — that is the process the backend owns, and it
|
||||
lives as long as the remote command does. (This said "local only" until
|
||||
|
||||
@@ -298,6 +298,51 @@ Two consequences worth stating:
|
||||
going away and means to come back) and `stop` (the session is being deleted,
|
||||
so the process must not survive). Every driver owes exactly one of them.
|
||||
|
||||
### Stopping and starting a session's process (decided 2026-08-30)
|
||||
|
||||
If a session outlives the backend, the person holding the phone needs the
|
||||
other direction too: **end the process without ending the session, and start
|
||||
it again on the same conversation.** `POST /sessions/:id/stop` and
|
||||
`/start`.
|
||||
|
||||
Three decisions worth not undoing:
|
||||
|
||||
- **Stop signals the recorded process and says nothing else.** It does not
|
||||
go through the driver and it does not announce `Exited`. The record is the
|
||||
session's rather than any dialect's, so signalling it here works for a
|
||||
session whose driver is in no state to be asked and adds no trait method a
|
||||
new driver could implement wrongly — and the driver's own reader already
|
||||
reports the death correctly, draining the last output and recording the
|
||||
status. Announcing it from here would be a guess arriving ahead of the
|
||||
measurement, and wrong for the grace period a process that ignores SIGTERM
|
||||
keeps running.
|
||||
- **Start replaces the driver and nothing else.** The transcript, the event
|
||||
pump and the SSE stream every open phone is reading stay where they were,
|
||||
so starting a session again is not a reconnect for anybody watching, and
|
||||
there is still exactly one writer of the transcript — which relaunching
|
||||
the whole `LiveSession` would not be, since the old pump outlives its
|
||||
session and an imported session's sync task would go on feeding it.
|
||||
`LiveSession` and `Commands` therefore share one `Mutex<Arc<dyn Driver>>`
|
||||
rather than each holding a copy.
|
||||
- **Start is refused unless the session is *known* to have exited.**
|
||||
`Unknown` means nobody could find out whether the process is alive, and
|
||||
starting one on that is exactly the two-CLIs-on-one-conversation fault
|
||||
`session::process` exists to prevent.
|
||||
|
||||
That last rule found a real bug in the launch path, which is where the phone
|
||||
would have hit it: a relaunched session took its status from the transcript,
|
||||
so one whose process had died before a backend restart reported `Exited`
|
||||
while the launch it had just gone through was starting a new process. The
|
||||
status now says `Idle` when a launch leaves a process running — `Exited`
|
||||
there is not merely stale, it is the word that refuses every command and
|
||||
invites somebody to start a second process against a live conversation.
|
||||
|
||||
On the phone this is one button in the composer, left of Send, whose mark and
|
||||
colour say what pressing it would do now: an orange pause while a turn is
|
||||
running (interrupt — the process stays), a red stop when it is not (end the
|
||||
process), and a green play when it has exited (start it again). One button
|
||||
rather than three that come and go, so its presence is never the signal.
|
||||
|
||||
### Importing refuses a session that is already open (decided 2026-08-29)
|
||||
|
||||
Claude Code keeps a descriptor per live session at
|
||||
@@ -448,7 +493,9 @@ POST /sessions spawn {provider, host, model, cwd, permiss
|
||||
GET /sessions/:id/events?after=N SSE: transcript replay from N, then live
|
||||
POST /sessions/:id/message {text, attachment_ids}
|
||||
POST /sessions/:id/answer {question_id, answer} (questions and permissions)
|
||||
POST /sessions/:id/interrupt
|
||||
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/model {model}
|
||||
POST /sessions/:id/compact (llama sessions)
|
||||
POST /sessions/:id/attachments multipart upload → id (referenced by /message)
|
||||
|
||||
@@ -558,6 +558,22 @@ fun interruptSession(settings: ServerSettings, sessionId: String) {
|
||||
requestFromServer(settings, "/sessions/$sessionId/interrupt", method = "POST") {}
|
||||
}
|
||||
|
||||
/**
|
||||
* Ends the process behind a session, leaving the session and its transcript.
|
||||
*
|
||||
* Not a delete and not an interrupt: the conversation stays exactly where it is and [startSession]
|
||||
* picks it back up. The server reports what it could not do -- there was nothing running, or the
|
||||
* machine would not say whether there was -- rather than answering the same way either way.
|
||||
*/
|
||||
fun stopSession(settings: ServerSettings, sessionId: String) {
|
||||
requestFromServer(settings, "/sessions/$sessionId/stop", method = "POST") {}
|
||||
}
|
||||
|
||||
/** Starts the process again on the conversation it left. See [stopSession]. */
|
||||
fun startSession(settings: ServerSettings, sessionId: String) {
|
||||
requestFromServer(settings, "/sessions/$sessionId/start", method = "POST") {}
|
||||
}
|
||||
|
||||
/**
|
||||
* Removes a Claude Code session from the machine.
|
||||
*
|
||||
|
||||
@@ -31,6 +31,12 @@ import androidx.compose.ui.unit.sp
|
||||
* tofu. Adding one means adding its codepoint in *both* places; a codepoint here that the script
|
||||
* did not subset is a glyph that silently isn't there.
|
||||
*
|
||||
* The subset is the font's **Mono** face, where every glyph is exactly one em wide and one em tall.
|
||||
* That is what makes two icons the same size without either of them being given a size: the
|
||||
* proportional face's advances run from 0.46 em to 0.92 em, so a Send button and a Stop button side
|
||||
* by side came out visibly different widths, and matching them at the call site would have meant
|
||||
* one hardcoded measurement per pair. [GLYPH_SIZE] carries the cost.
|
||||
*
|
||||
* The same arrangement as dev-updater, down to the cog and the refresh arrow being the same two
|
||||
* Material Design codepoints. Those two must not drift: an icon that means "settings" in one app
|
||||
* and something else in the other is the failure this is worth preventing. The script is copied
|
||||
@@ -52,9 +58,27 @@ val REFRESH_GLYPH = glyph(0xF0450)
|
||||
/** `md-send` -- the filled paper plane: submit what is in the composer. */
|
||||
val SEND_GLYPH = glyph(0xF048A)
|
||||
|
||||
/** `md-stop` -- a filled square: interrupt the turn that is running. */
|
||||
/**
|
||||
* `md-stop` -- a filled square: end the process behind this session.
|
||||
*
|
||||
* The square is what stop has meant since tape decks, and it is spent here on the thing that
|
||||
* actually stops rather than on pausing. [PAUSE_GLYPH] is the turn; this is the session.
|
||||
*/
|
||||
val STOP_GLYPH = glyph(0xF04DB)
|
||||
|
||||
/**
|
||||
* `md-pause` -- two bars: take the running turn away and leave the session there.
|
||||
*
|
||||
* The pair with [STOP_GLYPH] and [PLAY_GLYPH] is the point: one button in the composer says what
|
||||
* pressing it now would do to the process, and the three marks are the three answers. An interrupt
|
||||
* ends a turn and nothing else -- the CLI is still there and still holds the conversation -- which
|
||||
* is a pause, not a stop, and drawing it as a square said otherwise.
|
||||
*/
|
||||
val PAUSE_GLYPH = glyph(0xF03E4)
|
||||
|
||||
/** `md-play` -- start the process again, on the conversation it left. See [PAUSE_GLYPH]. */
|
||||
val PLAY_GLYPH = glyph(0xF040A)
|
||||
|
||||
/**
|
||||
* `md-send_clock` -- the same paper plane with a clock on it: this message will wait its turn.
|
||||
*
|
||||
@@ -142,5 +166,13 @@ fun Glyph(
|
||||
Text(glyph, fontFamily = NerdIcons, fontSize = size, color = colour, modifier = modifier)
|
||||
}
|
||||
|
||||
/** The size an icon draws at beside a line of text. */
|
||||
private val GLYPH_SIZE = 20.sp
|
||||
/**
|
||||
* The size an icon draws at beside a line of text.
|
||||
*
|
||||
* 17 rather than the 20 it was while the font was the proportional face. A glyph there filled at
|
||||
* most 0.83 em of its point size and most filled a good deal less, so the number was standing in
|
||||
* for the headroom above the tallest one; in the Mono face every glyph fills its em exactly, and
|
||||
* keeping 20 would have made every icon in the app step up by a fifth for no reason anybody asked
|
||||
* for. This is what the largest of them already drew at.
|
||||
*/
|
||||
private val GLYPH_SIZE = 17.sp
|
||||
@@ -1358,30 +1358,44 @@ fun SessionScreen(
|
||||
},
|
||||
)
|
||||
}
|
||||
if (running) {
|
||||
// The same filled shape as the button beside it, not an outlined one: these
|
||||
// are two things you can do about the turn that is running, and weighting one
|
||||
// of them as secondary said they were a primary action and its qualifier.
|
||||
// What separates them is the colour and the mark, which is what they mean.
|
||||
// The same filled shape as the button beside it, not an outlined one: these are
|
||||
// two things you can do about the session, and weighting one of them as secondary
|
||||
// said they were a primary action and its qualifier. What separates them is the
|
||||
// colour and the mark, which is what they mean.
|
||||
//
|
||||
// Always here, rather than arriving with the turn as it used to. A control that
|
||||
// comes and goes makes its own presence the signal, and its absence could not say
|
||||
// whether there was nothing to do; a button that is always in the same place also
|
||||
// cannot push Send off the end of the row by turning up.
|
||||
val process =
|
||||
when {
|
||||
running -> ProcessAction.Pause
|
||||
status == "exited" -> ProcessAction.Start
|
||||
else -> ProcessAction.Stop
|
||||
}
|
||||
Button(
|
||||
onClick = { act { interruptSession(settings, summary.id) } },
|
||||
colors = actionButtonColors(stopColor),
|
||||
onClick = { act { process.perform(settings, summary.id) } },
|
||||
colors = actionButtonColors(process.colour()),
|
||||
) {
|
||||
// A filled square, which is what stop has looked like since tape decks.
|
||||
Glyph(
|
||||
STOP_GLYPH,
|
||||
process.glyph,
|
||||
colour = LocalContentColor.current,
|
||||
modifier = Modifier.semantics { contentDescription = "Stop" },
|
||||
modifier = Modifier.semantics { contentDescription = process.label },
|
||||
)
|
||||
}
|
||||
Spacer(Modifier.width(8.dp))
|
||||
}
|
||||
// The paper plane, with a clock on it while a turn is in flight: sending then
|
||||
// queues the message for the next tool boundary rather than starting a turn of
|
||||
// its own, and the two have to be told apart at a glance. The label says the same
|
||||
// thing to a screen reader, which has nothing else to read.
|
||||
//
|
||||
// Disabled while there is nothing to send, rather than pressable and silent:
|
||||
// `send` has always returned early on an empty composer, so the button promised
|
||||
// something it would not do, and the only feedback was the ripple. Disabled and
|
||||
// not hidden, for the reason the button beside it is always here.
|
||||
Button(
|
||||
onClick = { send() },
|
||||
enabled = input.isNotBlank() || pendingAttachments.isNotEmpty(),
|
||||
colors = actionButtonColors(if (running) queueColor else sendColor),
|
||||
) {
|
||||
Glyph(
|
||||
@@ -1402,6 +1416,39 @@ fun SessionScreen(
|
||||
/** What pressing Send does right now, said the same way to the eye and to a screen reader. */
|
||||
private fun sendLabel(running: Boolean) = if (running) "Queue" else "Send"
|
||||
|
||||
/**
|
||||
* What the composer's process button would do if it were pressed now.
|
||||
*
|
||||
* One value rather than four parallel conditions over the status, because the mark, the colour, the
|
||||
* name a screen reader is given and the request that goes out are four halves of one decision. A
|
||||
* button drawn as a pause that terminates the CLI is the worst bug available here, and separate
|
||||
* branches over the same condition are how that happens -- these three each have to cover every
|
||||
* case, and the compiler says so.
|
||||
*/
|
||||
private enum class ProcessAction(val glyph: String, val label: String) {
|
||||
/** A turn is running: take it back, and leave the process holding the conversation. */
|
||||
Pause(PAUSE_GLYPH, "Pause"),
|
||||
/** Nothing is running, but the process behind the session is: end it. */
|
||||
Stop(STOP_GLYPH, "Stop"),
|
||||
/** The process is gone: start it again, on the conversation it left. */
|
||||
Start(PLAY_GLYPH, "Start"),
|
||||
}
|
||||
|
||||
@Composable
|
||||
private fun ProcessAction.colour() =
|
||||
when (this) {
|
||||
ProcessAction.Pause -> pauseColor
|
||||
ProcessAction.Stop -> stopColor
|
||||
ProcessAction.Start -> startColor
|
||||
}
|
||||
|
||||
private fun ProcessAction.perform(settings: ServerSettings, sessionId: String) =
|
||||
when (this) {
|
||||
ProcessAction.Pause -> interruptSession(settings, sessionId)
|
||||
ProcessAction.Stop -> stopSession(settings, sessionId)
|
||||
ProcessAction.Start -> startSession(settings, sessionId)
|
||||
}
|
||||
|
||||
/**
|
||||
* An inline transcript image, fetched (authenticated, pinned) from the session's files route. The
|
||||
* bitmap is remembered per ref, so scrolling doesn't refetch.
|
||||
|
||||
@@ -213,13 +213,14 @@ val overLimitColor: Color
|
||||
@Composable get() = MaterialTheme.colorScheme.error
|
||||
|
||||
/**
|
||||
* The composer's three buttons, coloured by what pressing one does rather than by where it sits.
|
||||
* The composer's buttons, coloured by what pressing one does rather than by where it sits.
|
||||
*
|
||||
* Green sends now, blue sends later, red takes the running turn away. The pair of greens and the
|
||||
* pair of reds elsewhere in this file are deliberate near-collisions worth naming: [runningColor]
|
||||
* is green because a session is working, and [failedColor] is red because one fell over -- those
|
||||
* are *states*, and these are *actions*. A reader never has to tell them apart, because nothing
|
||||
* here is a state and nothing there is pressable.
|
||||
* Green makes something happen now, blue makes it happen later, orange takes back what is in
|
||||
* flight, red ends the process. The near-collisions with the states above are deliberate and worth
|
||||
* naming rather than collapsing: [runningColor] is green because a session is working,
|
||||
* [failedColor] is red because one fell over, [awaitingColor] is the same orange because a session
|
||||
* is waiting on somebody -- those are *states*, and these are *actions*. A reader never has to tell
|
||||
* them apart, because nothing here is a state and nothing there is pressable.
|
||||
*/
|
||||
val sendColor: Color
|
||||
@Composable get() = Mocha.Green
|
||||
@@ -228,10 +229,30 @@ val sendColor: Color
|
||||
val queueColor: Color
|
||||
@Composable get() = Mocha.Blue
|
||||
|
||||
/** Interrupting the running turn -- the one button here that takes something away. */
|
||||
/**
|
||||
* Interrupting the running turn: the work stops and the session stays.
|
||||
*
|
||||
* Orange rather than red because of how much it takes: only what is in flight. The process is still
|
||||
* there holding the conversation, and the next message starts a turn as though nothing had
|
||||
* happened. Red is spent on [stopColor], which is the same button in the same place when what it
|
||||
* would end is the session's process.
|
||||
*/
|
||||
val pauseColor: Color
|
||||
@Composable get() = Mocha.Peach
|
||||
|
||||
/** Ending the session's process -- the one button here that takes something away. */
|
||||
val stopColor: Color
|
||||
@Composable get() = Mocha.Red
|
||||
|
||||
/**
|
||||
* Starting the process again, on the conversation it left.
|
||||
*
|
||||
* The same green as [sendColor] on purpose: both mean "this happens now", and they are never the
|
||||
* same button -- the process button only offers to start when there is nothing running to stop.
|
||||
*/
|
||||
val startColor: Color
|
||||
@Composable get() = Mocha.Green
|
||||
|
||||
/**
|
||||
* A filled button in one of the action colours above.
|
||||
*
|
||||
|
||||
Binary file not shown.
+16
-3
@@ -32,6 +32,8 @@ GLYPHS=(
|
||||
U+F0450 # md-refresh
|
||||
U+F048A # md-send
|
||||
U+F04DB # md-stop
|
||||
U+F03E4 # md-pause
|
||||
U+F040A # md-play
|
||||
U+F1163 # md-send_clock
|
||||
U+F0156 # md-close
|
||||
U+F004D # md-arrow_left
|
||||
@@ -52,9 +54,20 @@ python3 -m venv "$work/venv"
|
||||
|
||||
unicodes="$(IFS=,; echo "${GLYPHS[*]}")"
|
||||
mkdir -p "$(dirname "$out")"
|
||||
# The proportional face rather than the Mono one: these are drawn inline
|
||||
# beside text, where a fixed advance would pad each icon out to a cell.
|
||||
"$work/venv/bin/pyftsubset" "$work/SymbolsNerdFont-Regular.ttf" \
|
||||
# The Mono face rather than the proportional one, which this used until
|
||||
# 2026-08-30. Every glyph in it is one em wide and one em tall, so two
|
||||
# icons drawn at the same size are the same size -- which is what makes two
|
||||
# icon buttons beside each other match without either of them being told a
|
||||
# width. In the proportional face the advances run from 0.46 em (play) to
|
||||
# 0.92 em (line chart), so the composer's Send button came out visibly wider
|
||||
# than the Stop button next to it, and any fix at the call site would have
|
||||
# been one measurement hardcoded per pair.
|
||||
#
|
||||
# The trade is the one the old comment named: an icon inline beside text is
|
||||
# padded out to a cell. That is worth it, and it is also why GLYPH_SIZE in
|
||||
# NerdIcons.kt came down when this changed -- a glyph that fills its em
|
||||
# draws bigger at the same point size than one that does not.
|
||||
"$work/venv/bin/pyftsubset" "$work/SymbolsNerdFontMono-Regular.ttf" \
|
||||
--unicodes="$unicodes" \
|
||||
--layout-features= \
|
||||
--drop-tables+=DSIG \
|
||||
|
||||
+30
-1
@@ -17,7 +17,9 @@
|
||||
//! `reset` frame plus the newest window)
|
||||
//! POST /sessions/{id}/message {text, attachmentIds?}
|
||||
//! POST /sessions/{id}/answer {questionId, answers} (questions and permissions)
|
||||
//! POST /sessions/{id}/interrupt
|
||||
//! 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}/model {model}
|
||||
//! POST /sessions/{id}/command {text} -- /compact, /clear, /rename x, or the dialect's own
|
||||
@@ -87,6 +89,8 @@ pub fn router(manager: Arc<SessionManager>) -> Router {
|
||||
.route("/sessions/{id}/message", post(message))
|
||||
.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}/model", post(set_model))
|
||||
.route("/sessions/{id}/permission-mode", post(set_permission_mode))
|
||||
@@ -678,6 +682,31 @@ async fn interrupt(
|
||||
Ok(StatusCode::NO_CONTENT)
|
||||
}
|
||||
|
||||
/// Ends the session's process. The session stays, and `start` brings it
|
||||
/// back -- see [`SessionManager::stop_session`].
|
||||
///
|
||||
/// Not `lookup`ed: a session that failed to relaunch has no live entry and
|
||||
/// may still have a process running, which is exactly one worth being able
|
||||
/// to stop.
|
||||
async fn stop(
|
||||
State(manager): State<Arc<SessionManager>>,
|
||||
UrlPath(id): UrlPath<String>,
|
||||
) -> Result<StatusCode, ApiError> {
|
||||
manager.stop_session(&id).map_err(bad_request)?;
|
||||
Ok(StatusCode::NO_CONTENT)
|
||||
}
|
||||
|
||||
/// Starts a process for a session that has none, continuing the same
|
||||
/// conversation -- see [`SessionManager::start_session`], which refuses
|
||||
/// unless the session is known to have exited.
|
||||
async fn start(
|
||||
State(manager): State<Arc<SessionManager>>,
|
||||
UrlPath(id): UrlPath<String>,
|
||||
) -> Result<StatusCode, ApiError> {
|
||||
manager.start_session(&id).map_err(bad_request)?;
|
||||
Ok(StatusCode::NO_CONTENT)
|
||||
}
|
||||
|
||||
/// The usage screen needs two things that live in different places: the
|
||||
/// cache, and the current list of machines to ask. Carried together rather
|
||||
/// than the monitor holding the manager, which would point the dependency
|
||||
|
||||
@@ -100,12 +100,6 @@ pub(super) mod translate;
|
||||
|
||||
const RESUME_FILE: &str = "claude-session.json";
|
||||
|
||||
/// Grace period between asking a process to stop and killing it.
|
||||
///
|
||||
/// Only [`Driver::stop`] uses it -- a session being deleted. Detaching
|
||||
/// does not stop anything, so it has no grace period and needs none.
|
||||
const STOP_GRACE: std::time::Duration = std::time::Duration::from_secs(5);
|
||||
|
||||
/// Messages handed to the CLI that it has not visibly acted on yet.
|
||||
///
|
||||
/// The CLI *does* take a message written mid-turn: it goes into the next
|
||||
@@ -673,7 +667,7 @@ impl Driver for ClaudeDriver {
|
||||
fn stop(&self) {
|
||||
self.reading.store(false, Ordering::SeqCst);
|
||||
if let Some(record) = process::live(&self.session_dir) {
|
||||
process::stop(&record, STOP_GRACE);
|
||||
process::stop(&record, process::STOP_GRACE);
|
||||
}
|
||||
process::clear(&self.session_dir);
|
||||
}
|
||||
|
||||
@@ -269,9 +269,6 @@ const SERVER_LOG: &str = "llama-server.log";
|
||||
/// seconds late costs nothing.
|
||||
const WATCH_INTERVAL: std::time::Duration = std::time::Duration::from_secs(2);
|
||||
|
||||
/// Grace period between asking llama-server to stop and killing it.
|
||||
const STOP_GRACE: std::time::Duration = std::time::Duration::from_secs(5);
|
||||
|
||||
/// An owner-only log opened for appending, so the two streams pointed at
|
||||
/// it do not overwrite each other and a reattach keeps what came before.
|
||||
fn log_file(path: &Path) -> Result<std::fs::File> {
|
||||
@@ -447,7 +444,7 @@ impl Driver for LlamaDriver {
|
||||
fn stop(&self) {
|
||||
self.cancel.store(true, Ordering::Relaxed);
|
||||
if let Some(record) = process::live(&self.session_dir) {
|
||||
process::stop(&record, STOP_GRACE);
|
||||
process::stop(&record, process::STOP_GRACE);
|
||||
}
|
||||
process::clear(&self.session_dir);
|
||||
}
|
||||
|
||||
+299
-29
@@ -169,11 +169,21 @@ pub struct SessionInfo {
|
||||
pub created: f64,
|
||||
}
|
||||
|
||||
/// What is running a session at this moment.
|
||||
///
|
||||
/// Behind a lock because a session outlives its process: stopping one and
|
||||
/// starting it again replaces the driver while the transcript, the event
|
||||
/// pump and the stream every open phone is reading stay exactly where they
|
||||
/// were. Shared with [`Commands`] rather than copied into it, because two
|
||||
/// holders of "the driver" are two answers to that question the moment one
|
||||
/// of them is replaced.
|
||||
type DriverCell = Arc<Mutex<Arc<dyn Driver>>>;
|
||||
|
||||
/// A running session: its driver plus the shared state the event pump
|
||||
/// keeps current. Cheap to clone-by-`Arc` into request handlers.
|
||||
pub struct LiveSession {
|
||||
meta: SessionConfig,
|
||||
driver: Arc<dyn Driver>,
|
||||
driver: DriverCell,
|
||||
/// Commands asked for and not yet run, oldest first, with the pump
|
||||
/// that will run them. Shared with that pump, which is what notices
|
||||
/// the boundary.
|
||||
@@ -195,12 +205,17 @@ pub struct LiveSession {
|
||||
/// for the turn to end. Drivers therefore never have to think about it,
|
||||
/// and a new provider cannot get it wrong by omission.
|
||||
struct Commands {
|
||||
driver: Arc<dyn Driver>,
|
||||
driver: DriverCell,
|
||||
sink: EventSink,
|
||||
waiting: Mutex<VecDeque<(String, SessionCommand)>>,
|
||||
}
|
||||
|
||||
impl Commands {
|
||||
/// Whatever is driving the session now -- see [`DriverCell`].
|
||||
fn driver(&self) -> Arc<dyn Driver> {
|
||||
self.driver.lock().unwrap().clone()
|
||||
}
|
||||
|
||||
/// Runs `command` now if the session is between turns, holds it until
|
||||
/// it is, and refuses it outright if there will never be one. Whichever
|
||||
/// happened, the phone is told.
|
||||
@@ -236,9 +251,10 @@ impl Commands {
|
||||
});
|
||||
return;
|
||||
}
|
||||
if self.driver.between_turns() {
|
||||
let driver = self.driver();
|
||||
if driver.between_turns() {
|
||||
let _ = self.sink.send(Event::CommandSent { id, text });
|
||||
command.apply(self.driver.as_ref());
|
||||
command.apply(driver.as_ref());
|
||||
return;
|
||||
}
|
||||
let _ = self.sink.send(Event::CommandQueued {
|
||||
@@ -258,7 +274,8 @@ impl Commands {
|
||||
/// began by itself, which it does: a background task finishing makes it
|
||||
/// pick the conversation back up with nothing written to it.
|
||||
fn take_one(&self) {
|
||||
if !self.driver.between_turns() {
|
||||
let driver = self.driver();
|
||||
if !driver.between_turns() {
|
||||
return;
|
||||
}
|
||||
let Some((id, command)) = self.waiting.lock().unwrap().pop_front() else {
|
||||
@@ -268,7 +285,7 @@ impl Commands {
|
||||
id,
|
||||
text: command.label(),
|
||||
});
|
||||
command.apply(self.driver.as_ref());
|
||||
command.apply(driver.as_ref());
|
||||
}
|
||||
|
||||
/// Gives up on everything held, because the session cannot run them.
|
||||
@@ -336,6 +353,11 @@ struct Shared {
|
||||
}
|
||||
|
||||
impl LiveSession {
|
||||
/// Whatever is driving this session now -- see [`DriverCell`].
|
||||
fn driver(&self) -> Arc<dyn Driver> {
|
||||
self.driver.lock().unwrap().clone()
|
||||
}
|
||||
|
||||
/// Hands the user's message to the driver, which records it in the
|
||||
/// transcript by reporting that it has taken it -- see `MessageTaken`.
|
||||
///
|
||||
@@ -348,7 +370,7 @@ impl LiveSession {
|
||||
// drew a person's screenshot as a row floating above the bubble
|
||||
// that sent it, and left the phone inferring from adjacency which
|
||||
// message an image went with -- a thing the sender already knew.
|
||||
self.driver.send_user_message(text, images);
|
||||
self.driver().send_user_message(text, images);
|
||||
}
|
||||
|
||||
pub fn answer_question(&self, question_id: &str, answers: &[String]) {
|
||||
@@ -356,7 +378,7 @@ impl LiveSession {
|
||||
id: question_id.to_string(),
|
||||
answers: answers.to_vec(),
|
||||
});
|
||||
self.driver.answer_question(question_id, answers);
|
||||
self.driver().answer_question(question_id, answers);
|
||||
}
|
||||
|
||||
/// Asks the session to run a command on itself, now or at the next
|
||||
@@ -367,14 +389,14 @@ impl LiveSession {
|
||||
}
|
||||
|
||||
pub fn interrupt(&self) {
|
||||
self.driver.interrupt();
|
||||
self.driver().interrupt();
|
||||
}
|
||||
|
||||
/// 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`].
|
||||
pub fn detach(&self) {
|
||||
self.driver.detach();
|
||||
self.driver().detach();
|
||||
}
|
||||
|
||||
/// Compacts at the next boundary. Through the command queue like
|
||||
@@ -942,7 +964,7 @@ impl SessionManager {
|
||||
// change, and as an error if it cannot. The config above is a
|
||||
// different question -- what to launch this session with next
|
||||
// time -- and it is answered by the request.
|
||||
session.driver.set_permission_mode(mode);
|
||||
session.driver().set_permission_mode(mode);
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
@@ -1025,11 +1047,141 @@ impl SessionManager {
|
||||
if let Some(session) = inner.live.get(id) {
|
||||
// See `set_session_permission_mode`: the driver reports what
|
||||
// it is set to, this only asks.
|
||||
session.driver.set_model(model);
|
||||
session.driver().set_model(model);
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Ends this session's process, leaving the session -- its transcript,
|
||||
/// its place in the list, everything a phone is watching -- exactly
|
||||
/// where it is. [`SessionManager::start_session`] is the way back.
|
||||
///
|
||||
/// The signal is all this does. Whether the process actually went, what
|
||||
/// it said on the way out, and the `Exited` that follows are reported by
|
||||
/// the path a session that died on its own already takes: the driver's
|
||||
/// own reader notices within a poll, drains what was still unread, and
|
||||
/// records it. Announcing it from here would be this side's guess
|
||||
/// arriving ahead of the measurement, and it would be wrong for the five
|
||||
/// seconds a process that ignores SIGTERM keeps running.
|
||||
///
|
||||
/// Deliberately not routed through the driver. The record is the
|
||||
/// session's rather than any dialect's -- `session::process` writes it
|
||||
/// 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.
|
||||
pub fn stop_session(&self, id: &str) -> Result<()> {
|
||||
if !self
|
||||
.inner
|
||||
.read()
|
||||
.unwrap()
|
||||
.config
|
||||
.sessions
|
||||
.iter()
|
||||
.any(|meta| meta.id == id)
|
||||
{
|
||||
bail!("no session {id}");
|
||||
}
|
||||
// Three answers, and they are three different things to tell
|
||||
// somebody: it is running (stop it), it is not (nothing to do), and
|
||||
// nobody could find out (nothing was signalled, and saying "nothing
|
||||
// is running" would be inventing the answer).
|
||||
let record = match process::recorded(&self.data_dir.join(id)) {
|
||||
Some((record, process::Liveness::Alive)) => record,
|
||||
Some((_, process::Liveness::Unknown)) => bail!(
|
||||
"this machine won't say whether this session's process is still running, so it \
|
||||
wasn't signalled"
|
||||
),
|
||||
Some((_, process::Liveness::Dead)) | None => {
|
||||
bail!("this session has no process running")
|
||||
}
|
||||
};
|
||||
tracing::info!("stopping session {id} (pid {})", record.pid);
|
||||
process::stop(&record, process::STOP_GRACE);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Starts a process for a session whose process has ended, continuing
|
||||
/// the same conversation -- for Claude Code, the `--resume` that crash
|
||||
/// recovery already uses.
|
||||
///
|
||||
/// Only the driver is new. The transcript, the event pump and the stream
|
||||
/// every open phone is reading stay as they were, so this is not a
|
||||
/// reconnect for anybody watching -- and there is still exactly one
|
||||
/// writer of the transcript, which relaunching the whole session would
|
||||
/// not be.
|
||||
///
|
||||
/// Refused unless the session is *known* to have exited. `Unknown` means
|
||||
/// nobody could find out whether the process is alive, and starting one
|
||||
/// on that is precisely the second-CLI-on-one-conversation fault that
|
||||
/// `session::process` exists to prevent.
|
||||
pub fn start_session(&self, id: &str) -> Result<()> {
|
||||
let mut inner = self.inner.write().unwrap();
|
||||
let meta = inner
|
||||
.config
|
||||
.sessions
|
||||
.iter()
|
||||
.find(|meta| meta.id == id)
|
||||
.with_context(|| format!("no session {id}"))?
|
||||
.clone();
|
||||
let existing = inner.live.get(id).cloned();
|
||||
let status = match &existing {
|
||||
Some(session) => *session.shared.status.lock().unwrap(),
|
||||
None => status_of_unlaunched(&self.data_dir.join(id)),
|
||||
};
|
||||
match status {
|
||||
SessionStatus::Exited => {}
|
||||
SessionStatus::Unknown => bail!(
|
||||
"this machine won't say whether this session's process is still running, so \
|
||||
nothing was started"
|
||||
),
|
||||
_ => bail!("this session is already running"),
|
||||
}
|
||||
// Fresh from the config, like every other launch: a model or a
|
||||
// permission mode changed while the session was stopped is what it
|
||||
// starts with.
|
||||
let (setup, provider) = resolve(&inner.config, &meta)?;
|
||||
let session = match existing {
|
||||
Some(session) => {
|
||||
*session.driver.lock().unwrap() = make_driver(
|
||||
&meta,
|
||||
&setup,
|
||||
&provider,
|
||||
&self.models_dir,
|
||||
session.dir(),
|
||||
session.transcript_path(),
|
||||
&session.sink,
|
||||
)?;
|
||||
session
|
||||
}
|
||||
// Nothing is live for this one -- a session whose launch failed
|
||||
// when the server started, which has no pump either. That is the
|
||||
// whole of `launch`, and the same call the server start makes.
|
||||
None => {
|
||||
let session = launch(
|
||||
meta,
|
||||
&setup,
|
||||
&provider,
|
||||
&self.data_dir,
|
||||
&self.models_dir,
|
||||
None,
|
||||
self.notifications.clone(),
|
||||
)?;
|
||||
inner.live.insert(id.to_string(), Arc::clone(&session));
|
||||
session
|
||||
}
|
||||
};
|
||||
// The recorded status is `Exited` and this has just made it untrue.
|
||||
// Said here because nothing else will say it: a CLI that has been
|
||||
// given no work writes nothing, so the session would sit at
|
||||
// `Exited` -- refusing every command, refusing every message, and
|
||||
// showing a phone an offer to start a second process against the
|
||||
// conversation this one is already running.
|
||||
let _ = session.sink.send(Event::Status {
|
||||
state: SessionStatus::Idle,
|
||||
});
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Kills the process, releases everything the spawn created, and
|
||||
/// deletes the transcript and files -- the complete path out.
|
||||
pub fn delete_session(&self, id: &str) -> Result<()> {
|
||||
@@ -1045,7 +1197,7 @@ impl SessionManager {
|
||||
// Stopped, not detached: this is the one exit where the
|
||||
// process must not survive, because the conversation it
|
||||
// belongs to is being removed. See `Driver::stop`.
|
||||
session.driver.stop();
|
||||
session.driver().stop();
|
||||
}
|
||||
let dir = self.data_dir.join(id);
|
||||
if dir.exists() {
|
||||
@@ -1339,25 +1491,38 @@ fn launch(
|
||||
);
|
||||
}
|
||||
|
||||
let driver: Arc<dyn Driver> = match provider.kind {
|
||||
DriverKind::Echo => Arc::new(EchoDriver::new(sink.clone(), dir.clone())),
|
||||
DriverKind::LlamaCpp => Arc::new(LlamaDriver::launch(
|
||||
let driver = Arc::new(Mutex::new(make_driver(
|
||||
&meta,
|
||||
setup,
|
||||
provider,
|
||||
&Transport::for_setup(setup),
|
||||
models_dir,
|
||||
&dir,
|
||||
&transcript_path,
|
||||
&dir,
|
||||
sink.clone(),
|
||||
)?),
|
||||
DriverKind::ClaudeCli => Arc::new(ClaudeDriver::launch(
|
||||
&meta,
|
||||
provider,
|
||||
&Transport::for_setup(setup),
|
||||
&dir,
|
||||
sink.clone(),
|
||||
)?),
|
||||
};
|
||||
&sink,
|
||||
)?));
|
||||
|
||||
// The transcript's last word is what this session was doing when
|
||||
// something was last watching it, and building the driver above may have
|
||||
// just made it untrue: a session recorded as `Exited` with a process
|
||||
// running again is one this launch has started. Carrying `Exited`
|
||||
// forward is not merely stale -- it is the word that refuses every
|
||||
// command sent to the session, and the word that invites somebody to
|
||||
// start a *second* process against a conversation that already has one,
|
||||
// which is the fault `session::process` exists to prevent. It reached a
|
||||
// phone as a play button on a session that was already running.
|
||||
//
|
||||
// Idle is what is true: there is a process, and nothing has asked it for
|
||||
// anything. Written rather than announced, because nobody watched a
|
||||
// transition -- this is the state the session is being restored in, and
|
||||
// an event would put a status change in the transcript that never
|
||||
// happened. A record nobody could check stays as it was and is corrected
|
||||
// by the driver's first poll, which reports `Unknown` for it.
|
||||
{
|
||||
let mut status = shared.status.lock().unwrap();
|
||||
if *status == SessionStatus::Exited && process::live(&dir).is_some() {
|
||||
*status = SessionStatus::Idle;
|
||||
}
|
||||
}
|
||||
|
||||
let commands = Arc::new(Commands {
|
||||
driver: Arc::clone(&driver),
|
||||
@@ -1386,6 +1551,44 @@ fn launch(
|
||||
}))
|
||||
}
|
||||
|
||||
/// Whatever runs this session's provider, pointed at the session's own
|
||||
/// directory and reporting into `sink`.
|
||||
///
|
||||
/// Split out of [`launch`] because a session outlives its process: it is
|
||||
/// also what [`SessionManager::start_session`] builds when somebody starts a
|
||||
/// stopped session again. That path replaces the driver and nothing else, so
|
||||
/// it has to construct one the same way rather than becoming a second answer
|
||||
/// to "what runs this".
|
||||
fn make_driver(
|
||||
meta: &SessionConfig,
|
||||
setup: &SetupConfig,
|
||||
provider: &ProviderConfig,
|
||||
models_dir: &Path,
|
||||
dir: &Path,
|
||||
transcript_path: &Path,
|
||||
sink: &EventSink,
|
||||
) -> Result<Arc<dyn Driver>> {
|
||||
Ok(match provider.kind {
|
||||
DriverKind::Echo => Arc::new(EchoDriver::new(sink.clone(), dir.to_path_buf())),
|
||||
DriverKind::LlamaCpp => Arc::new(LlamaDriver::launch(
|
||||
meta,
|
||||
provider,
|
||||
&Transport::for_setup(setup),
|
||||
models_dir,
|
||||
transcript_path,
|
||||
dir,
|
||||
sink.clone(),
|
||||
)?),
|
||||
DriverKind::ClaudeCli => Arc::new(ClaudeDriver::launch(
|
||||
meta,
|
||||
provider,
|
||||
&Transport::for_setup(setup),
|
||||
dir,
|
||||
sink.clone(),
|
||||
)?),
|
||||
})
|
||||
}
|
||||
|
||||
/// The one writer of a session's transcript: assigns sequence numbers,
|
||||
/// appends, updates the shared status/activity view, fans out. Ends when
|
||||
/// every sender is dropped -- i.e. when the session is deleted and its
|
||||
@@ -1639,7 +1842,10 @@ mod tests {
|
||||
let dir = tempfile::tempdir().expect("tempdir");
|
||||
let (sink, mut events) = mpsc::unbounded_channel();
|
||||
let commands = Commands {
|
||||
driver: Arc::new(EchoDriver::new(sink.clone(), dir.path().to_path_buf())),
|
||||
driver: Arc::new(Mutex::new(Arc::new(EchoDriver::new(
|
||||
sink.clone(),
|
||||
dir.path().to_path_buf(),
|
||||
)))),
|
||||
sink,
|
||||
waiting: Mutex::new(VecDeque::new()),
|
||||
};
|
||||
@@ -2233,6 +2439,70 @@ mod tests {
|
||||
std::fs::write(path, rewritten).expect("write transcript");
|
||||
}
|
||||
|
||||
/// Stopping and starting a session is about its *process*, and the two
|
||||
/// refusals are the whole of what keeps starting one from becoming a
|
||||
/// second one on the same conversation.
|
||||
///
|
||||
/// Echo has no process, which makes it the right session to ask the
|
||||
/// first question of: "there is nothing to stop" is an answer, and
|
||||
/// reporting success would leave a phone showing a session it believes
|
||||
/// it stopped. The second question is asked of a session that has been
|
||||
/// told it exited, since the guard is on the *status* rather than on
|
||||
/// which driver it is.
|
||||
#[tokio::test]
|
||||
async fn a_session_is_started_again_only_once_it_is_known_to_have_exited() {
|
||||
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 session");
|
||||
let mut rx = session.subscribe();
|
||||
|
||||
let refused = manager.stop_session(&info.id).expect_err("nothing to stop");
|
||||
assert!(
|
||||
refused.to_string().contains("no process running"),
|
||||
"said: {refused:#}"
|
||||
);
|
||||
let refused = manager
|
||||
.start_session(&info.id)
|
||||
.expect_err("already running");
|
||||
assert!(
|
||||
refused.to_string().contains("already running"),
|
||||
"said: {refused:#}"
|
||||
);
|
||||
|
||||
// What a driver reports when its process goes, without a process
|
||||
// to go: the guard reads the recorded status, so this is the same
|
||||
// state a stopped claude session reaches.
|
||||
let _ = session.sink.send(Event::Status {
|
||||
state: SessionStatus::Exited,
|
||||
});
|
||||
collect_until(&mut rx, |event| {
|
||||
matches!(
|
||||
event,
|
||||
Event::Status {
|
||||
state: SessionStatus::Exited
|
||||
}
|
||||
)
|
||||
})
|
||||
.await;
|
||||
|
||||
manager.start_session(&info.id).expect("start again");
|
||||
collect_until(&mut rx, is_idle).await;
|
||||
// Idle rather than exited, and said by the start rather than left
|
||||
// for a driver that has been given no work to say for itself.
|
||||
assert_eq!(manager.sessions()[0].status, SessionStatus::Idle);
|
||||
// The same live session throughout: only the driver was replaced,
|
||||
// so nothing a phone is reading was interrupted.
|
||||
assert!(Arc::ptr_eq(
|
||||
&session,
|
||||
&manager.session(&info.id).expect("still live")
|
||||
));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn a_restart_relaunches_sessions_and_continues_the_numbering() {
|
||||
let dir = tempfile::tempdir().expect("tempdir");
|
||||
|
||||
@@ -204,6 +204,13 @@ pub fn clear(session_dir: &Path) {
|
||||
}
|
||||
}
|
||||
|
||||
/// Grace period between asking a session's process to stop and killing it.
|
||||
///
|
||||
/// Here rather than beside each caller: it is a property of stopping one of
|
||||
/// these, and two drivers plus the manager had written the same five seconds
|
||||
/// down separately, which is three places for it to drift.
|
||||
pub const STOP_GRACE: std::time::Duration = std::time::Duration::from_secs(5);
|
||||
|
||||
/// Asks it to stop, then makes sure. Used where a leaked process must
|
||||
/// actually end: a deleted session, or one being replaced.
|
||||
///
|
||||
|
||||
Reference in new issue
Block a user