From 3dee4a4e47a351c3edc316f3ae0737001d805573 Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Wed, 2 Sep 2026 17:49:09 +0100 Subject: [PATCH] feat(maestro): daemon assigns the session handle The spawn form no longer prefills a name, so create() fills one in: a requested name is marked User, an absent one gets a generated handle marked Generated. Rename always settles to User. Refs #131 --- crates/maestro-daemon/src/session.rs | 55 +++++++++++++++++++--- crates/maestro-daemon/tests/rename.rs | 67 ++++++++++++++++++++++++++- 2 files changed, 114 insertions(+), 8 deletions(-) diff --git a/crates/maestro-daemon/src/session.rs b/crates/maestro-daemon/src/session.rs index c2d510f..5996858 100644 --- a/crates/maestro-daemon/src/session.rs +++ b/crates/maestro-daemon/src/session.rs @@ -12,8 +12,8 @@ use std::time::Duration; use chrono::Utc; use maestro_protocol::{ - ActivityState, CatalogEvent, CreateSessionRequest, ErrorInfo, Session, SessionStatus, - WorktreeSpec, + ActivityState, CatalogEvent, CreateSessionRequest, ErrorInfo, NameSource, Session, + SessionStatus, WorktreeSpec, NAME_KEY, NAME_SOURCE_KEY, }; use tokio::sync::{broadcast, Mutex}; use uuid::Uuid; @@ -404,6 +404,8 @@ impl SessionManager { // `cwd`), not a generated worktree checkout path — that's what the // recents picker offers to reuse. let history_cwd = req.cwd.clone(); + let mut metadata = req.metadata; + resolve_name(&mut metadata); let worktree_branch = req.worktree.as_ref().map(|w| w.branch.clone()); let session = Session { @@ -411,7 +413,7 @@ impl SessionManager { cwd: effective_cwd, command: req.command, env: req.env, - metadata: req.metadata, + metadata, worktree: worktree_path.as_ref().zip(worktree_base_repo.as_ref()).map( |(path, base_repo)| maestro_protocol::SessionWorktree { path: path.clone(), @@ -740,11 +742,22 @@ impl SessionManager { let mut sessions = self.inner.sessions.lock().await; let rec = sessions.get_mut(&id).ok_or(SessionError::NotFound(id))?; let mut renamed = rec.session.clone(); + // A person typed this, so it is settled: autonaming skips it and + // no client dims it (#131). match maestro_protocol::normalize_session_name(name) { - Some(name) => renamed - .metadata - .insert("name".into(), serde_json::Value::String(name)), - None => renamed.metadata.remove("name"), + Some(name) => { + renamed + .metadata + .insert(NAME_KEY.into(), serde_json::Value::String(name)); + renamed.metadata.insert( + NAME_SOURCE_KEY.into(), + serde_json::Value::String(NameSource::User.as_str().into()), + ); + } + None => { + renamed.metadata.remove(NAME_KEY); + renamed.metadata.remove(NAME_SOURCE_KEY); + } }; self.inner .sidecar @@ -806,6 +819,34 @@ impl SessionManager { /// milliseconds on a loaded host, and `send_keys` rides the per-keystroke hot /// path — running it inline would starve HTTP handlers, the events WS, the /// liveness watcher, and PTY readers (NAP-256). +/// Settle a spawn request's name (#131). +/// +/// A name the request carried is normalized and marked [`NameSource::User`] — +/// a person typed it, so nothing overwrites it later. A request that carried +/// none (the spawn form's name field now starts empty) gets one of the +/// daemon's own handles, marked [`NameSource::Generated`]: provisional, dimmed +/// by both sidebars, and the only kind autonaming replaces. +fn resolve_name(metadata: &mut HashMap) { + let requested = metadata + .get(NAME_KEY) + .and_then(|v| v.as_str()) + .and_then(maestro_protocol::normalize_session_name); + let (name, source) = match requested { + Some(name) => (name, NameSource::User), + None => ( + maestro_names::generate_batch(1) + .pop() + .unwrap_or_else(|| "session".into()), + NameSource::Generated, + ), + }; + metadata.insert(NAME_KEY.into(), serde_json::Value::String(name)); + metadata.insert( + NAME_SOURCE_KEY.into(), + serde_json::Value::String(source.as_str().into()), + ); +} + /// Turn a request's environment into tmux `-e KEY=VAL` material. /// /// A key containing `=` or NUL would be parsed by tmux as a different diff --git a/crates/maestro-daemon/tests/rename.rs b/crates/maestro-daemon/tests/rename.rs index 209f642..1b48655 100644 --- a/crates/maestro-daemon/tests/rename.rs +++ b/crates/maestro-daemon/tests/rename.rs @@ -10,7 +10,9 @@ use common::TmuxFixture; use maestro_daemon::sidecar::SidecarStore; use maestro_daemon::worktree::WorktreeManager; use maestro_daemon::SessionManager; -use maestro_protocol::{CatalogEvent, CreateSessionRequest, MAX_SESSION_NAME_LEN}; +use maestro_protocol::{ + display, CatalogEvent, CreateSessionRequest, NameSource, MAX_SESSION_NAME_LEN, +}; use tempfile::TempDir; use uuid::Uuid; @@ -136,3 +138,66 @@ async fn rename_broadcasts_a_session_update() { other => panic!("expected SessionUpdated, got {other:?}"), } } + +/// #131: a spawn that carried no name gets one of the daemon's own handles, +/// marked provisional so a first prompt may replace it. +#[tokio::test] +async fn a_nameless_spawn_is_given_a_provisional_handle() { + let fx = fixture(); + let id = spawn_named(&fx.manager, None).await; + + let snapshot = fx.manager.snapshot().await; + let live = snapshot.iter().find(|s| s.id == id).unwrap(); + assert!(name_of(live).is_some(), "the daemon names it"); + assert_eq!(display::name_source(live), NameSource::Generated); + assert!(display::is_provisional_name(live)); + + // It survives a restart the same way the rest of the metadata does. + let persisted = fx.sidecar.read(id).unwrap().unwrap(); + assert_eq!(name_of(&persisted), name_of(live)); + assert_eq!(display::name_source(&persisted), NameSource::Generated); +} + +/// A name the spawn request carried is a person's choice: settled, never +/// dimmed, never autonamed over (#131). +#[tokio::test] +async fn a_requested_name_is_a_user_name() { + let fx = fixture(); + let id = spawn_named(&fx.manager, Some(" amber-falcon ")).await; + + let snapshot = fx.manager.snapshot().await; + let live = snapshot.iter().find(|s| s.id == id).unwrap(); + assert_eq!(name_of(live), Some("amber-falcon")); + assert_eq!(display::name_source(live), NameSource::User); + assert!(!display::is_provisional_name(live)); +} + +/// Renaming a provisional name pins it (#131). +#[tokio::test] +async fn a_rename_settles_a_generated_name() { + let fx = fixture(); + let id = spawn_named(&fx.manager, None).await; + + let renamed = fx.manager.rename(id, "crimson-otter").await.unwrap(); + assert_eq!(display::name_source(&renamed), NameSource::User); + assert!(!display::is_provisional_name(&renamed)); + assert_eq!( + display::name_source(&fx.sidecar.read(id).unwrap().unwrap()), + NameSource::User + ); +} + +/// Clearing the name drops the source with it, so the card falling back to +/// its command carries no stale provisional marker (#131). +#[tokio::test] +async fn clearing_the_name_drops_its_source() { + let fx = fixture(); + let id = spawn_named(&fx.manager, None).await; + + let cleared = fx.manager.rename(id, " ").await.unwrap(); + assert_eq!(name_of(&cleared), None); + assert!(!cleared + .metadata + .contains_key(maestro_protocol::NAME_SOURCE_KEY)); + assert!(!display::is_provisional_name(&cleared)); +}