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 67842e2..55f6beb 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/Api.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/Api.kt @@ -513,6 +513,23 @@ fun fetchTranscript( } } +/** + * Renames a session. + * + * The name is the backend's own -- it is what the list shows and it exists before any process does + * -- so this settles it rather than asking. Where the thing running the session has a name of its + * own, the backend passes it on, which is what makes a session the same session in Claude Code's + * picker and to any other agent that lists it. + */ +fun renameSession(settings: ServerSettings, sessionId: String, title: String) { + requestFromServer( + settings, + "/sessions/$sessionId/title", + method = "POST", + jsonBody = JSONObject().put("title", title).toString(), + ) {} +} + /** Switches a running session's model; the CLI changes it in place. */ fun setSessionModel(settings: ServerSettings, sessionId: String, model: String) { requestFromServer( diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/AppRoot.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/AppRoot.kt index 512a097..7baa05d 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/AppRoot.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/AppRoot.kt @@ -36,6 +36,13 @@ private sealed class Screen { */ data class Usage(val from: SessionSummary) : Screen() + /** + * What can be changed about one session. Carries the session back with it for the same reason + * [Screen.Usage] does, and carries it *out* renamed, so the session behind it shows the new + * name without waiting for a list refresh. + */ + data class SessionSettings(val from: SessionSummary) : Screen() + data object Models : Screen() data object Setups : Screen() @@ -116,6 +123,7 @@ fun AppRoot(settingsVersion: Int) { summary = here.summary, onBack = goToList, onUsage = { screen = Screen.Usage(here.summary) }, + onSettings = { screen = Screen.SessionSettings(here.summary) }, ) is Screen.Spawn -> SpawnScreen( @@ -142,6 +150,18 @@ fun AppRoot(settingsVersion: Int) { // from that session, so stepping back is the one thing Back can mean here. onBack = { screen = Screen.Session(here.from) }, ) + is Screen.SessionSettings -> + SessionSettingsScreen( + settings = current, + session = here.from, + onRenamed = { renamed -> + // The list shows the name too, so it has to refetch; and the session + // returned to is the renamed one, not the one this was opened from. + reloadToken++ + screen = Screen.Session(renamed) + }, + onBack = { screen = Screen.Session(here.from) }, + ) is Screen.Models -> ModelsScreen(settings = current, onBack = goToList) is Screen.Setups -> SetupsScreen(settings = current, onBack = goToList) is Screen.Settings -> diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/Gear.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/Gear.kt new file mode 100644 index 0000000..39a7b23 --- /dev/null +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/Gear.kt @@ -0,0 +1,59 @@ +package com.example.aiapp + +import androidx.compose.foundation.Canvas +import androidx.compose.foundation.layout.size +import androidx.compose.material3.MaterialTheme +import androidx.compose.runtime.Composable +import androidx.compose.ui.Modifier +import androidx.compose.ui.geometry.Offset +import androidx.compose.ui.graphics.Color +import androidx.compose.ui.graphics.StrokeCap +import androidx.compose.ui.graphics.drawscope.Stroke +import androidx.compose.ui.unit.dp +import kotlin.math.PI +import kotlin.math.cos +import kotlin.math.sin + +/** + * A gear: settings for the thing it sits beside. + * + * Drawn rather than set in a font, for the reason [Chevron] gives -- an icon glyph is one a system + * font may not have, and whoever gets the empty box instead is never the person who wrote it. The + * app has no icon set otherwise, and one dependency for one gear is a poor trade. + * + * A thick ring with eight blunt teeth cut into its edge. The proportions are the whole of whether + * this reads as a gear: drawn first with thin teeth standing clear of a thin hub, it was a sun -- + * unmistakably, and only once it was looked at on a screen. What separates the two shapes is that a + * gear's teeth are as heavy as its body and barely longer than they are wide, and that its centre + * is a hole rather than a dot. + * + * It draws no label, so every caller owes it a `contentDescription`: that is all assistive + * technology has, and it is also the answer to "what was that button for" six months from now. + */ +@Composable +fun Gear(modifier: Modifier = Modifier, colour: Color = MaterialTheme.colorScheme.primary) { + Canvas(modifier.size(20.dp)) { + val centre = Offset(size.width / 2, size.height / 2) + val tooth = 4.dp.toPx() + // The teeth end at the edge, so the body has to leave room for half a tooth's width + // where they meet the ring -- otherwise the widest part of the drawing is clipped. + val tip = size.minDimension / 2 + val body = tip - tooth * 0.62f + drawCircle(colour, radius = body, centre, style = Stroke(3.dp.toPx())) + repeat(TEETH) { index -> + val angle = 2 * PI * index / TEETH + val direction = Offset(cos(angle).toFloat(), sin(angle).toFloat()) + drawLine( + colour, + centre + direction * (body - 1.dp.toPx()), + centre + direction * tip, + strokeWidth = tooth, + // Square-ended, because a rounded tooth on a shape this small rounds away + // most of the tooth. + cap = StrokeCap.Butt, + ) + } + } +} + +private const val TEETH = 8 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 ae53e83..bba5a13 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/SessionScreen.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/SessionScreen.kt @@ -26,6 +26,7 @@ import androidx.compose.material3.CardDefaults import androidx.compose.material3.CircularProgressIndicator import androidx.compose.material3.DropdownMenu import androidx.compose.material3.DropdownMenuItem +import androidx.compose.material3.IconButton import androidx.compose.material3.MaterialTheme import androidx.compose.material3.OutlinedButton import androidx.compose.material3.OutlinedTextField @@ -277,6 +278,7 @@ fun SessionScreen( summary: SessionSummary, onBack: () -> Unit, onUsage: () -> Unit, + onSettings: () -> Unit, ) { val scope = rememberCoroutineScope() var items by remember { mutableStateOf(listOf()) } @@ -677,16 +679,16 @@ fun SessionScreen( // the paid service's own numbers, so a session on a provider with no such service // gets an honest "unavailable" rather than a hidden button -- a control that comes // and goes makes its absence the signal, and absence cannot say why. - // Beside the token count it acts on, which is the line to its left: compacting is - // what that number is for. Disabled rather than hidden while one is already running, - // so the button still says the session can do this and why it cannot right now. - TextButton( - onClick = { act { compactSession(settings, summary.id) } }, - enabled = status != "compacting" && status != "exited", - ) { - Text("Compact") - } TextButton(onClick = onUsage) { Text("Usage") } + // A step down from this session, so it sits at the end of the session's own row. + // The name is the whole of what it holds today, which is why it is a gear and not a + // word: there will be more, and a bar of words has nowhere to put it. + IconButton( + onClick = onSettings, + modifier = Modifier.semantics { contentDescription = "Session settings" }, + ) { + Gear() + } } (streamError ?: actionError)?.let { message -> diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/SessionSettingsScreen.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/SessionSettingsScreen.kt new file mode 100644 index 0000000..6117c18 --- /dev/null +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/SessionSettingsScreen.kt @@ -0,0 +1,123 @@ +package com.example.aiapp + +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.Row +import androidx.compose.foundation.layout.Spacer +import androidx.compose.foundation.layout.fillMaxSize +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.foundation.text.KeyboardActions +import androidx.compose.foundation.text.KeyboardOptions +import androidx.compose.foundation.verticalScroll +import androidx.compose.material3.Button +import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.OutlinedTextField +import androidx.compose.material3.Text +import androidx.compose.material3.TextButton +import androidx.compose.runtime.Composable +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.remember +import androidx.compose.runtime.rememberCoroutineScope +import androidx.compose.runtime.setValue +import androidx.compose.ui.Alignment +import androidx.compose.ui.Modifier +import androidx.compose.ui.text.input.ImeAction +import androidx.compose.ui.unit.dp +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.launch +import kotlinx.coroutines.withContext + +/** + * What can be changed about one session, as opposed to about this app. + * + * A step down from the session rather than a menu over it: the name is a text field with a keyboard + * in front of it, and that is more than belongs in a bar above a conversation. Back returns to the + * session it was opened from, which is the only thing back can mean here. + * + * The name is the one setting so far. The model and the permission mode are deliberately still on + * the session's own bar, because those are changed *while* reading a turn -- "not this model, try + * that one" -- and a control belongs with the thing it acts on. + */ +@Composable +fun SessionSettingsScreen( + settings: ServerSettings, + session: SessionSummary, + onRenamed: (SessionSummary) -> Unit, + onBack: () -> Unit, +) { + val scope = rememberCoroutineScope() + var name by remember(session.id) { mutableStateOf(session.title) } + var saving by remember { mutableStateOf(false) } + var error by remember { mutableStateOf(null) } + + // Nothing to do when the name has not changed, so the button says so rather than sending a + // request whose success would look exactly like the failure of having typed nothing. + val changed = name.trim().isNotEmpty() && name.trim() != session.title + + fun save() { + if (!changed || saving) return + val chosen = name.trim() + saving = true + error = null + scope.launch { + try { + withContext(Dispatchers.IO) { renameSession(settings, session.id, chosen) } + onRenamed(session.copy(title = chosen)) + } catch (e: ApiException) { + // Reported here, where it happened, because this screen is the only place that + // knows a rename was attempted -- the session behind it shows nothing about it. + error = e.message + saving = false + } + } + } + + Column(Modifier.fillMaxSize().verticalScroll(rememberScrollState()).padding(16.dp)) { + Row(verticalAlignment = Alignment.CenterVertically, modifier = Modifier.fillMaxWidth()) { + Text( + "Session settings", + style = MaterialTheme.typography.headlineSmall, + modifier = Modifier.weight(1f), + ) + TextButton(onClick = onBack) { Text("Back") } + } + Spacer(Modifier.height(16.dp)) + + OutlinedTextField( + value = name, + onValueChange = { name = it }, + label = { Text("Name") }, + singleLine = true, + enabled = !saving, + modifier = Modifier.fillMaxWidth(), + // The keyboard's own action does what the button does: a one-field form where the + // return key does nothing is a form people press return at anyway. + keyboardOptions = KeyboardOptions(imeAction = ImeAction.Done), + keyboardActions = KeyboardActions(onDone = { save() }), + ) + Text( + "Passed on to whatever is running this session, so Claude Code's own session " + + "picker and any agent listing sessions use the same name.", + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + modifier = Modifier.padding(top = 4.dp), + ) + Spacer(Modifier.height(16.dp)) + // Disabled rather than absent while there is nothing to save: a button that comes and + // goes makes its own presence the signal, and its absence cannot say why. + Button(onClick = { save() }, enabled = changed && !saving) { + Text(if (saving) "Saving..." else "Save") + } + error?.let { + Spacer(Modifier.height(8.dp)) + Text( + it, + color = MaterialTheme.colorScheme.error, + style = MaterialTheme.typography.bodySmall, + ) + } + } +} diff --git a/server/src/routes.rs b/server/src/routes.rs index ee0d58d..7fbaab0 100644 --- a/server/src/routes.rs +++ b/server/src/routes.rs @@ -17,6 +17,7 @@ //! POST /sessions/{id}/message {text, attachmentIds?} //! POST /sessions/{id}/answer {questionId, answer} (questions and permissions) //! POST /sessions/{id}/interrupt +//! POST /sessions/{id}/title {title} //! POST /sessions/{id}/model {model} //! POST /sessions/{id}/compact //! POST /sessions/{id}/attachments multipart image upload -> {id}, referenced by /message @@ -80,6 +81,7 @@ pub fn router(manager: Arc) -> Router { .route("/sessions/{id}/message", post(message)) .route("/sessions/{id}/answer", post(answer)) .route("/sessions/{id}/interrupt", post(interrupt)) + .route("/sessions/{id}/title", post(rename)) .route("/sessions/{id}/model", post(set_model)) .route("/sessions/{id}/permission-mode", post(set_permission_mode)) .route("/sessions/{id}/compact", post(compact)) @@ -674,6 +676,23 @@ async fn usage( Ok(axum::Json(snapshots)) } +#[derive(Deserialize)] +#[serde(deny_unknown_fields)] +struct TitleRequest { + title: String, +} + +async fn rename( + State(manager): State>, + UrlPath(id): UrlPath, + axum::Json(body): axum::Json, +) -> Result { + manager + .rename_session(&id, &body.title) + .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 99c7803..4ba9909 100644 --- a/server/src/session/claude.rs +++ b/server/src/session/claude.rs @@ -336,8 +336,19 @@ impl ClaudeDriver { if let Some(mode) = &meta.permission_mode { push("--permission-mode", mode); } - if let Some(resume) = read_resume_token(session_dir) { - push("--resume", &resume); + // Named at birth, so this session is the same session in the CLI's + // own picker and in what other agents see when they list it. + // + // Only when we are the ones creating it. A resume is a session + // that already existed -- an import, or this server starting again + // -- and it already has whatever name it was given, quite possibly + // by the person who was typing in it. Renaming that from a title + // we derived from its first message would be taking something the + // app was only ever shown. `Driver::set_title` is how it changes + // after this point, and that one is asked for. + match read_resume_token(session_dir) { + Some(resume) => push("--resume", &resume), + None => push("--name", &meta.title), } args.push("--include-partial-messages".to_string()); @@ -505,6 +516,29 @@ impl Driver for ClaudeDriver { ); } + fn set_title(&self, title: &str) { + // The CLI's own mechanism, and a local command rather than a + // control request -- `set_session_name` is not a subtype it + // knows, measured by asking. It answers this the way it answers + // `/compact`: a fresh `init`, then a `result` for a turn with no + // model call in it, so the same "a turn is in flight" bookkeeping + // applies. A name with a newline in it would be two lines and the + // second would be a message, so it is refused rather than sent. + if title.contains('\n') { + let _ = self.sink.send(Event::Error { + message: "a session name cannot contain a line break".to_string(), + }); + return; + } + self.queue.lock().unwrap().running = true; + self.send_line( + json!({"type": "user", "message": {"role": "user", "content": [ + {"type": "text", "text": format!("/rename {title}")} + ]}}) + .to_string(), + ); + } + fn compact(&self) { // A turn is in flight from here. The CLI answers `/compact` like // any other message -- a status line, a boundary, then a `result` diff --git a/server/src/session/driver.rs b/server/src/session/driver.rs index 63d318d..140cf3f 100644 --- a/server/src/session/driver.rs +++ b/server/src/session/driver.rs @@ -232,6 +232,21 @@ pub trait Driver: Send + Sync { // owes an [`Event::Error`] saying why; saying nothing leaves a phone // showing a setting nobody applied. + /// Tells the process what this conversation is called, when it has + /// somewhere to put it. + /// + /// Unlike the two above, this is not a request that can fail: the + /// rename has already happened in this server's own config, which is + /// what a phone lists and the only place the name has to be. So a + /// driver whose process has no notion of a name does nothing here and + /// says nothing -- there is no failure to report, and an error beside + /// a rename that plainly worked would be a puzzle rather than a + /// warning. + /// + /// Claude Code has one: `--name` when a session is created and + /// `/rename` afterwards, which is what puts the same name in its own + /// session picker and in what other agents see. + fn set_title(&self, title: &str); /// 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/echo.rs b/server/src/session/echo.rs index a059ab5..83e295c 100644 --- a/server/src/session/echo.rs +++ b/server/src/session/echo.rs @@ -15,6 +15,9 @@ //! - `/error [text]` -- a failure, which is otherwise awkward to cause. //! - `/peer [text]` -- a message from another agent, which otherwise takes //! two live sessions and one of them deciding to write. +//! - `/compact` -- a compaction, start to finish. Typed rather than +//! pressed, because the real dialects take it as a typed command too and +//! the phone no longer has a button for it. //! //! This is exactly the event vocabulary the real drivers produce, so a UI //! that renders echo sessions correctly renders the real thing. @@ -142,6 +145,15 @@ impl Driver for EchoDriver { return; } + // The same word the real CLI takes, so a phone drives both the same + // way. `Driver::compact` is what the manager's own route calls; + // this is the typed path onto it. + if text.trim() == "/compact" { + self.emit(Event::MessageTaken { text }); + self.compact(); + return; + } + if let Some(rest) = text.strip_prefix("/question") { let id = format!("q-{}", super::random_hex()); let prompt = if rest.trim().is_empty() { @@ -318,6 +330,11 @@ impl Driver for EchoDriver { }); } + // Nothing to forward: this process has no notion of what the + // conversation is called, and the rename it belongs to has already + // happened where the name lives. See `Driver::set_title`. + fn set_title(&self, _title: &str) {} + fn set_permission_mode(&self, mode: &str) { self.emit(Event::Error { message: format!("an echo session asks for nothing, so {mode} changes nothing"), diff --git a/server/src/session/llama.rs b/server/src/session/llama.rs index f91374d..1acb859 100644 --- a/server/src/session/llama.rs +++ b/server/src/session/llama.rs @@ -380,6 +380,11 @@ impl Driver for LlamaDriver { self.cancel.store(true, Ordering::Relaxed); } + // Nothing to forward: this process has no notion of what the + // conversation is called, and the rename it belongs to has already + // happened where the name lives. See `Driver::set_title`. + fn set_title(&self, _title: &str) {} + fn set_permission_mode(&self, _mode: &str) { let _ = self.sink.send(Event::Error { message: "a llama.cpp session runs no tools, so there is nothing for a permission \ diff --git a/server/src/session/mod.rs b/server/src/session/mod.rs index ac06ff8..76c92e0 100644 --- a/server/src/session/mod.rs +++ b/server/src/session/mod.rs @@ -118,6 +118,11 @@ pub struct LiveSession { /// change mid-session via `set_model`. struct Shared { status: Mutex, + /// What this conversation is called. Here rather than in `meta` for + /// the same reason the model is: `meta` is how the session was + /// *launched*, so reporting a name from it would show the one a + /// rename had already replaced. + title: Mutex, last_activity: Mutex, model: Mutex>, /// Beside the model and for the same reason: `meta` is the shape the @@ -220,7 +225,7 @@ impl LiveSession { provider: self.meta.provider.clone(), setup: self.meta.setup.clone(), setup_name: setup_name.to_string(), - title: self.meta.title.clone(), + title: self.shared.title.lock().unwrap().clone(), model: self.shared.model.lock().unwrap().clone(), permission_mode: self.shared.permission_mode.lock().unwrap().clone(), imported, @@ -688,6 +693,39 @@ impl SessionManager { Ok(()) } + /// Renames a session: persisted, shown, and passed on to whatever is + /// running it. + /// + /// The name is this server's own -- it is what a phone lists, it + /// exists before any process does, and every provider has one. So + /// unlike the model and the permission mode, this is settled here and + /// the driver is *told*, rather than asked and believed: see + /// [`Driver::set_title`]. + pub fn rename_session(&self, id: &str, title: &str) -> Result<()> { + let title = title.trim(); + // An empty name is not a name, and it is what a cleared field + // sends. Refused rather than accepted and papered over with the + // provider's name, which would look like the rename was ignored. + if title.is_empty() { + bail!("a session needs a name"); + } + 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.title = title.to_string(); + } + candidate.save(&self.config_path)?; + inner.config = candidate; + if let Some(session) = inner.live.get(id) { + *session.shared.title.lock().unwrap() = title.to_string(); + session.driver.set_title(title); + } + Ok(()) + } + pub fn set_session_model(&self, id: &str, model: &str) -> Result<()> { let mut inner = self.inner.write().unwrap(); if !inner.config.sessions.iter().any(|meta| meta.id == id) { @@ -940,6 +978,7 @@ fn launch( // one adopting a process that has been quiet says nothing, and // this is then the only true answer available. status: Mutex::new(transcript.last_status().unwrap_or(SessionStatus::Idle)), + title: Mutex::new(meta.title.clone()), last_activity: Mutex::new(now()), model: Mutex::new(meta.model.clone()), permission_mode: Mutex::new(meta.permission_mode.clone()), @@ -1256,6 +1295,39 @@ mod tests { assert!(manager.delete_session(&info.id).is_err()); } + #[tokio::test] + async fn renaming_a_session_persists_and_shows() { + let dir = tempfile::tempdir().expect("tempdir"); + let config_path = dir.path().join("config.ron"); + seed_echo_only(&config_path); + let manager = SessionManager::new( + config_path.clone(), + dir.path().join("sessions"), + dir.path().join("models"), + ) + .expect("manager"); + let info = manager.spawn_session(echo_spec()).expect("spawn"); + assert_eq!(info.title, "echo session"); + + manager + .rename_session(&info.id, " the one about paging ") + .expect("rename"); + // Trimmed, and reported by the live session rather than by the + // record it was launched with. + let listed = manager.sessions(); + assert_eq!(listed[0].title, "the one about paging"); + assert_eq!( + Config::load(&config_path).expect("reload").sessions[0].title, + "the one about paging" + ); + + // A name that is only spaces is not a name. + assert!(manager.rename_session(&info.id, " ").is_err()); + assert!(manager.rename_session("no-such-session", "x").is_err()); + // And the refusal changed nothing. + assert_eq!(manager.sessions()[0].title, "the one about paging"); + } + #[tokio::test] async fn questions_round_trip_through_answer() { let dir = tempfile::tempdir().expect("tempdir");