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 b094d34..129712f 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/Events.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/Events.kt @@ -48,6 +48,14 @@ sealed class SessionEvent { data class Status(val state: String) : SessionEvent() + /** + * What the session is set to, as the session itself reports it. + * + * Either field alone: the two are confirmed separately and by different things. Asking for a + * change is not having one, so this -- not the request -- is what the pickers show. + */ + data class Settings(val model: String?, val permissionMode: String?) : SessionEvent() + data class UsageDelta(val tokens: Long) : SessionEvent() /** @@ -108,6 +116,11 @@ fun parseSeqEvent(json: String): SeqEvent { "peerMessage" -> SessionEvent.PeerMessage(body.getString("from"), body.getString("text")) "status" -> SessionEvent.Status(body.getString("state")) + "settings" -> + SessionEvent.Settings( + model = body.optString("model").ifEmpty { null }, + permissionMode = body.optString("permissionMode").ifEmpty { null }, + ) "usageDelta" -> SessionEvent.UsageDelta(body.getLong("tokens")) "compacted" -> SessionEvent.Compacted( diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/ModelName.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/ModelName.kt new file mode 100644 index 0000000..0e3b3a2 --- /dev/null +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/ModelName.kt @@ -0,0 +1,25 @@ +package com.example.aiapp + +/** + * A model's name as a person reads it. + * + * Providers answer with their own full identifier -- Claude Code resolves `haiku` to + * `claude-haiku-4-5-20251001` and reports that, which is the honest answer to "what is this session + * using" and far too long for a button in a row that also has to hold Stop and Send. + * + * So the two ends that identify nothing are dropped and nothing else is: the vendor prefix, which + * is the same on every model this app can show, and the release date, which distinguishes builds of + * one model rather than one model from another. What is left is the part somebody chose -- + * `haiku-4-5` -- and anything that does not look like that is returned untouched, since a name this + * does not recognise is a name it has no business editing. + * + * A display decision, not a correction: the full name is what the session reports and what a reader + * is shown when there is room for it. + */ +fun modelLabel(model: String?): String { + val name = model?.takeIf { it.isNotBlank() } ?: return "default" + return name.removePrefix("claude-").replace(DATED_SUFFIX, "") +} + +/** A trailing `-YYYYMMDD`, which is how these identifiers carry their release date. */ +private val DATED_SUFFIX = Regex("""-\d{8}$""") diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/SessionListScreen.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/SessionListScreen.kt index 5daebe4..9816659 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/SessionListScreen.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/SessionListScreen.kt @@ -228,7 +228,7 @@ private fun SessionCard( listOfNotNull( session.provider, "on ${session.setupName}", - session.model, + session.model?.let { modelLabel(it) }, ) .joinToString(" · "), style = MaterialTheme.typography.bodySmall, 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 1c11191..ae53e83 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/SessionScreen.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/SessionScreen.kt @@ -233,6 +233,8 @@ fun foldEvent(items: List, entry: SeqEvent): List items + TranscriptItem.PeerNote(entry.seq, event.from, event.text) + // Screen-level state, not transcript rows -- see SessionScreen. + is SessionEvent.Settings -> items is SessionEvent.Status -> items is SessionEvent.Error -> items + TranscriptItem.ErrorMsg(entry.seq, event.message) is SessionEvent.Image -> @@ -346,6 +348,12 @@ fun SessionScreen( when (val event = entry.event) { is SessionEvent.UsageDelta -> totalTokens += event.tokens else -> { + // What the session says it is set to now, which is the only thing that + // says it: picking from either menu asks, and the answer comes back here. + if (event is SessionEvent.Settings) { + event.model?.let { model = it } + event.permissionMode?.let { permissionMode = it } + } if (event is SessionEvent.Status) { // Started here, or nowhere. `ready` is what separates the live stream from // the page of history the screen opens with, and a compaction found in that @@ -651,7 +659,9 @@ fun SessionScreen( listOfNotNull( summary.provider, "on ${summary.setupName}", - summary.model, + // What the session says it is set to now, which is the same fact + // the picker below shows and has to be the same answer. + model?.let { modelLabel(it) }, if (totalTokens > 0) "$totalTokens tok" else null, ) .joinToString(" · "), @@ -931,10 +941,13 @@ fun SessionScreen( ) { if (offeredModels.isNotEmpty()) { PickerButton( - current = model ?: "default", + current = modelLabel(model), options = offeredModels, + // Not set here. The button follows what the session reports it + // is set to, which arrives a moment later and is sometimes a + // different answer -- a name the CLI resolved, or no change at all + // on a provider whose model is fixed when it starts. onPick = { chosen -> - model = chosen act { setSessionModel(settings, summary.id, chosen) } }, ) @@ -943,7 +956,6 @@ fun SessionScreen( current = permissionMode, options = PERMISSION_MODES, onPick = { chosen -> - permissionMode = chosen act { setSessionPermissionMode(settings, summary.id, chosen) } }, ) diff --git a/server/src/session/claude.rs b/server/src/session/claude.rs index 037d87c..2b57f02 100644 --- a/server/src/session/claude.rs +++ b/server/src/session/claude.rs @@ -61,7 +61,7 @@ use super::driver::{Driver, Event, EventSink, ImageRef, SessionStatus}; use super::process; use super::transport::{Launch, Streams, Transport}; use crate::config::{ProviderConfig, SessionConfig}; -use translate::{AnswerOutcome, Translator}; +use translate::{AnswerOutcome, Setting, Translator}; /// How much of a failing process's stderr the exit report carries. /// @@ -387,8 +387,26 @@ impl ClaudeDriver { let _ = self.to_child.send(line); } - fn send_control(&self, request: Value) { - let id = format!("req-{}", super::now() as u64); + /// Sends a control request, remembering what it asked for. + /// + /// `confirms` is the setting this request will have made if the CLI + /// answers success -- see [`Translator::expect_setting`], and + /// `Driver::set_model` for why a request is not a confirmation. + /// `None` for the ones that change no setting, like an interrupt. + /// + /// The id is random rather than the clock it used to be: two requests + /// in the same second shared an id, which was harmless while nothing + /// looked one up and is not any more. + fn send_control(&self, request: Value, confirms: Option) { + let id = format!("req-{}", super::random_hex()); + if let Some(setting) = confirms { + // Before the line goes out: the reader thread is already + // running, and a fast answer to a slow lock arrives first. + self.state + .lock() + .unwrap() + .expect_setting(id.clone(), setting); + } self.send_line( json!({"type": "control_request", "request_id": id, "request": request}).to_string(), ); @@ -470,15 +488,21 @@ impl Driver for ClaudeDriver { /// typed deliberately, and dropping it would lose a message that never /// reached the transcript, with nothing on screen to say so. fn interrupt(&self) { - self.send_control(json!({"subtype": "interrupt"})); + self.send_control(json!({"subtype": "interrupt"}), None); } fn set_permission_mode(&self, mode: &str) { - self.send_control(json!({"subtype": "set_permission_mode", "mode": mode})); + self.send_control( + json!({"subtype": "set_permission_mode", "mode": mode}), + Some(Setting::PermissionMode(mode.to_string())), + ); } fn set_model(&self, model: &str) { - self.send_control(json!({"subtype": "set_model", "model": model})); + self.send_control( + json!({"subtype": "set_model", "model": model}), + Some(Setting::Model(model.to_string())), + ); } fn compact(&self) { diff --git a/server/src/session/claude/translate.rs b/server/src/session/claude/translate.rs index 0bd0087..e2d2e98 100644 --- a/server/src/session/claude/translate.rs +++ b/server/src/session/claude/translate.rs @@ -28,6 +28,19 @@ pub(super) enum AnswerOutcome { Unknown, } +/// A setting a control request asked for, held until the CLI says +/// whether it took. +/// +/// The CLI answers `set_model` with a bare success -- no value -- so the +/// only way to report what was accepted is to remember what was asked. +/// `set_permission_mode` does echo its mode back, and so does a +/// `system/status` line a moment later; both are handled where they +/// arrive, and this covers the one that says nothing. +pub(super) enum Setting { + Model(String), + PermissionMode(String), +} + /// A `can_use_tool` request we've surfaced to the phone and not yet /// answered. For plain permissions there is one implicit question /// (Allow/Deny); for AskUserQuestion, one per entry in `questions`. @@ -47,6 +60,10 @@ struct PendingRequest { pub(super) struct Translator { pub(super) session_id: Option, pending: HashMap, + /// Settings asked for and not yet answered, by request id. Its path + /// out is the response: every entry is removed when one arrives, + /// whether it succeeded or failed. + asked: HashMap, session_dir: PathBuf, } @@ -55,9 +72,19 @@ impl Translator { Self { session_id: None, pending: HashMap::new(), + asked: HashMap::new(), session_dir, } } + + /// Remembers what a control request was for, so its answer can say so. + /// + /// Called before the request goes out, not after: the reader thread is + /// already running and a fast CLI can answer before this side gets + /// back to it. + pub(super) fn expect_setting(&mut self, request_id: String, setting: Setting) { + self.asked.insert(request_id, setting); + } pub(super) fn translate(&mut self, message: &Value) -> Vec { // Events from subagents (Task tool internals) carry a // parent_tool_use_id; the transcript shows the Task tool's own @@ -76,16 +103,45 @@ impl Translator { Some("control_request") => self.translate_control_request(message), Some("control_response") => { let response = &message["response"]; + // Answered either way, so the request stops being pending + // either way -- a rejected setting that stayed here would + // be applied by the next request that reused its id. + let asked = response + .get("request_id") + .and_then(Value::as_str) + .and_then(|id| self.asked.remove(id)); if response.get("subtype").and_then(Value::as_str) == Some("error") { let error = response .get("error") .and_then(Value::as_str) .unwrap_or("unknown"); - vec![Event::Error { + return vec![Event::Error { message: format!("claude rejected a request: {error}"), - }] - } else { - Vec::new() + }]; + } + // Success, so the setting this request asked for is now + // the session's, and this is the only place that says so: + // the response carries no value of its own for a model. + match asked { + Some(Setting::Model(model)) => vec![Event::Settings { + model: Some(model), + permission_mode: None, + }], + Some(Setting::PermissionMode(mode)) => vec![Event::Settings { + model: None, + // The CLI echoes this one, and its answer wins: + // `auto` and `manual` are names it accepts on the + // way in and reports back under another name, so + // repeating the request here would show a mode the + // session is not in. + permission_mode: Some( + response["response"]["mode"] + .as_str() + .map(str::to_string) + .unwrap_or(mode), + ), + }], + None => Vec::new(), } } Some("result") => { @@ -150,7 +206,21 @@ impl Translator { if let Some(id) = message.get("session_id").and_then(Value::as_str) { self.session_id = Some(id.to_string()); } - Vec::new() + // The CLI's own account of what it is set to, and the only + // one that resolves an alias: a session launched with + // `--model haiku` reports `claude-haiku-4-5-20251001` + // here. It arrives again after a compaction, which is + // free -- the manager drops a setting it is already in. + vec![Event::Settings { + model: message + .get("model") + .and_then(Value::as_str) + .map(str::to_string), + permission_mode: message + .get("permissionMode") + .and_then(Value::as_str) + .map(str::to_string), + }] } Some("status") => self.translate_status(message), Some("compact_boundary") => { @@ -178,6 +248,18 @@ impl Translator { /// state this build does not recognise is left alone rather than /// mapped onto the nearest one we do. fn translate_status(&self, message: &Value) -> Vec { + // A mode change the CLI has made, announced a moment after it + // answers the request that asked for it. Measured on 2.1.237: + // `{"subtype":"status","status":null,"permissionMode":"plan"}`, + // which is a leaving edge carrying no compaction result -- so it + // is checked before the compaction reading below, which would + // otherwise fall through to nothing. + if let Some(mode) = message.get("permissionMode").and_then(Value::as_str) { + return vec![Event::Settings { + model: None, + permission_mode: Some(mode.to_string()), + }]; + } if let Some(status) = message.get("status").and_then(Value::as_str) { return match status { "compacting" => vec![Event::Status { @@ -471,17 +553,123 @@ mod tests { } #[test] - fn captures_the_resume_token_from_init() { + fn captures_the_resume_token_and_the_settings_from_init() { let dir = tempfile::tempdir().expect("tempdir"); let mut translator = Translator::new(dir.path().to_path_buf()); let events = translate_lines( &mut translator, &[ - r#"{"type":"system","subtype":"init","cwd":"/x","session_id":"5ecf21da-d53f","tools":[],"model":"claude-haiku-4-5-20251001"}"#, + r#"{"type":"system","subtype":"init","cwd":"/x","session_id":"5ecf21da-d53f","tools":[],"model":"claude-haiku-4-5-20251001","permissionMode":"acceptEdits"}"#, ], ); - assert!(events.is_empty()); assert_eq!(translator.session_id.as_deref(), Some("5ecf21da-d53f")); + // The resolved model, which is the point: a session launched with + // `--model haiku` is reported by its full name here, and that is + // the name the phone should be showing. + assert_eq!( + events, + vec![Event::Settings { + model: Some("claude-haiku-4-5-20251001".to_string()), + permission_mode: Some("acceptEdits".to_string()), + }] + ); + } + + #[test] + fn a_setting_is_reported_when_the_cli_accepts_it_and_not_before() { + let dir = tempfile::tempdir().expect("tempdir"); + let mut translator = Translator::new(dir.path().to_path_buf()); + + // What `set_model` does: remember, send, and say nothing yet. + translator.expect_setting("req-a".to_string(), Setting::Model("sonnet".to_string())); + translator.expect_setting( + "req-b".to_string(), + Setting::PermissionMode("plan".to_string()), + ); + + // Success carries no model of its own -- measured on 2.1.237 -- + // so what was asked for is the only answer available. + let events = translate_lines( + &mut translator, + &[ + r#"{"type":"control_response","response":{"subtype":"success","request_id":"req-a"}}"#, + ], + ); + assert_eq!( + events, + vec![Event::Settings { + model: Some("sonnet".to_string()), + permission_mode: None, + }] + ); + + // A mode the CLI answers with a value of its own is taken from + // that value: `auto` on the way in is `default` coming back, and + // the request is not the answer. + translator.expect_setting( + "req-c".to_string(), + Setting::PermissionMode("auto".to_string()), + ); + let events = translate_lines( + &mut translator, + &[ + r#"{"type":"control_response","response":{"subtype":"success","request_id":"req-c","response":{"mode":"default"}}}"#, + ], + ); + assert_eq!( + events, + vec![Event::Settings { + model: None, + permission_mode: Some("default".to_string()), + }] + ); + + // A refusal changes nothing, and says why rather than claiming a + // setting that was rejected. + let events = translate_lines( + &mut translator, + &[ + r#"{"type":"control_response","response":{"subtype":"error","request_id":"req-b","error":"unknown mode"}}"#, + ], + ); + assert_eq!( + events, + vec![Event::Error { + message: "claude rejected a request: unknown mode".to_string() + }] + ); + + // And neither request is still waiting: a second answer to either + // id reports nothing at all. + let events = translate_lines( + &mut translator, + &[ + r#"{"type":"control_response","response":{"subtype":"success","request_id":"req-a"}}"#, + r#"{"type":"control_response","response":{"subtype":"success","request_id":"req-b"}}"#, + ], + ); + assert!(events.is_empty(), "{events:?}"); + } + + #[test] + fn a_mode_the_cli_announces_is_taken_from_the_announcement() { + // The line it sends just after answering `set_permission_mode`, + // which is also how a mode changed from the terminal arrives. + let dir = tempfile::tempdir().expect("tempdir"); + let mut translator = Translator::new(dir.path().to_path_buf()); + let events = translate_lines( + &mut translator, + &[ + r#"{"type":"system","subtype":"status","status":null,"permissionMode":"plan","session_id":"s"}"#, + ], + ); + assert_eq!( + events, + vec![Event::Settings { + model: None, + permission_mode: Some("plan".to_string()), + }] + ); } #[test] diff --git a/server/src/session/driver.rs b/server/src/session/driver.rs index 1c36f9e..63d318d 100644 --- a/server/src/session/driver.rs +++ b/server/src/session/driver.rs @@ -127,6 +127,25 @@ pub enum Event { Status { state: SessionStatus, }, + /// What the session is set to, as the session itself reports it. + /// + /// Asking for a change and having one are different things, and only + /// this one is a measurement: a model name the dialect does not know, + /// a mode it refuses, or a driver whose model is fixed at startup all + /// leave a request that was sent and nothing that changed. Reporting + /// from the request instead put the answer on the phone before the + /// question had been answered, and left it there when the answer was + /// no. + /// + /// Either field alone, because the two are confirmed separately and + /// by different things -- the CLI echoes a mode change, and names the + /// model it resolved an alias to when a session starts. + Settings { + #[serde(default, skip_serializing_if = "Option::is_none")] + model: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + permission_mode: Option, + }, /// Per-turn token counts, where the dialect reports them. UsageDelta { tokens: u64, @@ -206,6 +225,13 @@ pub trait Driver: Send + Sync { /// spawn-only: the answer changes with what is being done, and a phone /// is the worst place to answer "may I run this?" forty times. fn set_permission_mode(&self, mode: &str); + // Both of the above are requests, and neither reports the outcome by + // returning. A driver that actually changes the setting owes an + // [`Event::Settings`] once it has -- that event, and not the request, + // is what the manager and the phone read. One that cannot change it + // owes an [`Event::Error`] saying why; saying nothing leaves a phone + // showing a setting nobody applied. + /// pi: native compaction; claude: `/compact`. fn compact(&self); /// Stop attending to the process but leave it running, because this diff --git a/server/src/session/mod.rs b/server/src/session/mod.rs index f13d5e5..ac06ff8 100644 --- a/server/src/session/mod.rs +++ b/server/src/session/mod.rs @@ -678,7 +678,11 @@ impl SessionManager { candidate.save(&self.config_path)?; inner.config = candidate; if let Some(session) = inner.live.get(id) { - *session.shared.permission_mode.lock().unwrap() = Some(mode.to_string()); + // Asked for, not recorded: what the session is actually set to + // comes back as an `Event::Settings` if the driver makes the + // 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); } Ok(()) @@ -696,7 +700,8 @@ impl SessionManager { candidate.save(&self.config_path)?; inner.config = candidate; if let Some(session) = inner.live.get(id) { - *session.shared.model.lock().unwrap() = Some(model.to_string()); + // See `set_session_permission_mode`: the driver reports what + // it is set to, this only asks. session.driver.set_model(model); } Ok(()) @@ -1000,6 +1005,29 @@ fn launch( /// The appends are synchronous file writes from an async task, /// deliberately: each is one small line on a local disk, and funneling /// them through one task is what makes the sequence numbering safe. +/// Whether this event tells anyone anything they do not already know. +/// +/// Only the two events that report state rather than something that +/// happened can fail this: everything else is an occurrence, and an +/// occurrence is news by existing. A `Settings` naming one field is +/// judged on that field alone, since the other is not a claim that it is +/// unset. +fn is_news(event: &Event, shared: &Shared) -> bool { + match event { + Event::Status { state } => *shared.status.lock().unwrap() != *state, + Event::Settings { + model, + permission_mode, + } => { + let model_changed = model.is_some() && *shared.model.lock().unwrap() != *model; + let mode_changed = permission_mode.is_some() + && *shared.permission_mode.lock().unwrap() != *permission_mode; + model_changed || mode_changed + } + _ => true, + } +} + async fn pump( mut transcript: Transcript, mut source: mpsc::UnboundedReceiver, @@ -1016,18 +1044,32 @@ async fn pump( Event::MessageTaken { text } => Event::UserMessage { text }, other => other, }; - // A status the session is already in is not news. Imported - // sessions make this the common case rather than a rarity: each - // sync reads the turn state off the file's newest record, and - // most of them find the same answer as the sync before -- which - // would otherwise be a transcript entry, a broadcast, and a + // Nothing changed, so there is nothing to record. Both of these + // repeat: an imported session reads the turn state off its file's + // newest record on every sync and mostly finds the answer it found + // last time, and the CLI restates its model and mode at every + // `init`, which includes the one after every compaction. Recording + // those would be a transcript entry, a broadcast and a // recomposition on every phone, several times a minute, to say // nothing at all. - if let Event::Status { state } = &event - && *shared.status.lock().unwrap() == *state - { + if !is_news(&event, &shared) { continue; } + if let Event::Settings { + model, + permission_mode, + } = &event + { + // The session's own account of what it is set to, which is + // what the list and the session screen show. Not written + // where the change is *asked for* -- see `Event::Settings`. + if let Some(model) = model { + *shared.model.lock().unwrap() = Some(model.clone()); + } + if let Some(mode) = permission_mode { + *shared.permission_mode.lock().unwrap() = Some(mode.clone()); + } + } match transcript.append(event, ts) { Ok(entry) => { if let Event::Status { state } = &entry.event {