Let a session be renamed, under the same name everywhere
A gear at the end of the session's own bar opens what can be changed about that session; the name is the first thing there. Compact is gone from that bar -- `/compact` typed into the message box is the CLI's own way to ask and it already worked, so the button was a second way to say one thing. Echo takes the typed word too now, since it is the rig the compaction display is checked against and losing the button would have taken that with it. The name is this server's, not a driver's: it is what the list shows, it exists before any process does, and every provider has one. So it is settled in the config and the driver is *told* -- which is the opposite of the model and the permission mode, and the difference is written down at `Driver::set_title`. A driver whose process has no notion of a name does nothing and says nothing, because there is no failure to report. Claude Code has one, so the name reaches it: `--name` for a session we create, and `/rename` afterwards, which is a local command rather than a control request -- `set_session_name` is not a subtype it knows, which I established by asking it. A resumed session is deliberately not renamed at launch: an import already has a name, quite possibly one the person typing in it chose, and taking that would be helping itself to something the app was only shown. Verified end to end rather than argued: renaming from the phone put "Session renamed to: paging and scroll" in the CLI's own session file, and the session now lists under that name to other agents. The gear is drawn rather than set in a font, for the reason Chevron gives. It was a sun on the first attempt -- thin teeth standing clear of a thin hub -- which no amount of reading the diff would have shown.
This commit is contained in:
1 parent
1629e0911e
commit
d3fff3d229
11 files changed
+395
-12
No files matched your search
@@ -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`
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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"),
|
||||
|
||||
@@ -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 \
|
||||
|
||||
@@ -118,6 +118,11 @@ pub struct LiveSession {
|
||||
/// change mid-session via `set_model`.
|
||||
struct Shared {
|
||||
status: Mutex<SessionStatus>,
|
||||
/// 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<String>,
|
||||
last_activity: Mutex<f64>,
|
||||
model: Mutex<Option<String>>,
|
||||
/// 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");
|
||||
|
||||
Reference in new issue
Block a user