diff --git a/crates/maestro-daemon/src/local_api.rs b/crates/maestro-daemon/src/local_api.rs index 06c3100..eb90e73 100644 --- a/crates/maestro-daemon/src/local_api.rs +++ b/crates/maestro-daemon/src/local_api.rs @@ -20,7 +20,7 @@ use axum::{Json, Router}; use maestro_config::SpawnPreset; use maestro_protocol::{ routes, ApiError, CatalogEvent, CreateSessionRequest, DaemonInfo, GitRoot, NameBatch, - ReportActivityRequest, Session, PROTOCOL_VERSION, + RenameSessionRequest, ReportActivityRequest, Session, PROTOCOL_VERSION, }; use serde::Deserialize; use uuid::Uuid; @@ -47,7 +47,10 @@ pub fn router(state: LocalApiState) -> Router { Router::new() .route(routes::SESSIONS, get(list_sessions).post(create_session)) .route(routes::patterns::SESSION_ACTIVITY, post(report_activity)) - .route(routes::patterns::SESSION, delete(kill_session)) + .route( + routes::patterns::SESSION, + delete(kill_session).patch(rename_session), + ) .route(routes::DIRS, get(list_dir)) .route(routes::RECENT_DIRS, get(recent_dirs)) .route(routes::PROJECTS, get(projects)) @@ -213,6 +216,28 @@ async fn kill_session( } } +/// `PATCH /api/sessions/:id` (#125): rename a session. The name is trimmed +/// and capped server-side; a blank one clears `metadata.name` so the session +/// falls back to its command. Answers with the updated session, which also +/// goes out as `SessionUpdated` to every events subscriber. +async fn rename_session( + State(s): State, + Path(id): Path, + Json(body): Json, +) -> Response { + match s.manager.rename(id, &body.name).await { + Ok(session) => (StatusCode::OK, Json(session)).into_response(), + Err(e) => { + let info = e.to_error_info(); + error_response( + session_error_status(&info.code), + info.code.as_str(), + info.message.as_str(), + ) + } + } +} + /// `GET /api/worktrees` (#80): every retained worktree, with the in-the-way /// breakdown recomputed at read time — never stored, so a tree whose work has /// since been committed or pushed says so. Entries whose directory somebody diff --git a/crates/maestro-daemon/src/session.rs b/crates/maestro-daemon/src/session.rs index 7861155..4a91011 100644 --- a/crates/maestro-daemon/src/session.rs +++ b/crates/maestro-daemon/src/session.rs @@ -716,6 +716,38 @@ impl SessionManager { .map_err(SessionError::Tmux) } + /// Rename a session (#125): set `metadata.name`, or clear it when the + /// requested name is blank so the session falls back to its command. + /// + /// The new name is persisted to the sidecar before the event goes out — a + /// rename the operator can see but that a daemon restart would undo is + /// worse than one that failed outright. + pub async fn rename(&self, id: Uuid, name: &str) -> Result { + // The sidecar write happens under the sessions lock so what is on disk + // and what is in memory can never disagree about the newest rename. + let session = { + 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(); + match maestro_protocol::normalize_session_name(name) { + Some(name) => renamed + .metadata + .insert("name".into(), serde_json::Value::String(name)), + None => renamed.metadata.remove("name"), + }; + self.inner + .sidecar + .write(&renamed) + .map_err(SessionError::Sidecar)?; + rec.session = renamed.clone(); + renamed + }; + let _ = self.inner.events.send(CatalogEvent::SessionUpdated { + session: session.clone(), + }); + Ok(session) + } + /// Accept an agent-reported activity state (#105), the only path that can /// produce `ActivityState::Error`. The report outranks the activity pass's /// own inference until its TTL lapses — so an agent that dies mid-run diff --git a/crates/maestro-daemon/tests/local_api.rs b/crates/maestro-daemon/tests/local_api.rs index 88a712b..a90af29 100644 --- a/crates/maestro-daemon/tests/local_api.rs +++ b/crates/maestro-daemon/tests/local_api.rs @@ -323,6 +323,48 @@ async fn delete_unknown_session_returns_404() { assert_eq!(resp.status(), reqwest::StatusCode::NOT_FOUND); } +/// #125: the rename endpoint answers with the updated session, and the name +/// it stored is the trimmed one. +#[tokio::test] +async fn patch_session_renames_it() { + let h = spawn("naps-laptop").await; + let session = h + .manager + .create(CreateSessionRequest { + cwd: "/tmp".into(), + command: "sleep 60".into(), + env: HashMap::new(), + metadata: HashMap::new(), + worktree: None, + }) + .await + .unwrap(); + let resp = reqwest::Client::new() + .patch(url(h.addr, &format!("/api/sessions/{}", session.id))) + .json(&serde_json::json!({ "name": " crimson-otter " })) + .send() + .await + .unwrap(); + assert_eq!(resp.status(), reqwest::StatusCode::OK); + let updated: Session = resp.json().await.unwrap(); + assert_eq!( + updated.metadata.get("name").and_then(|v| v.as_str()), + Some("crimson-otter") + ); +} + +#[tokio::test] +async fn patch_unknown_session_returns_404() { + let h = spawn("naps-laptop").await; + let resp = reqwest::Client::new() + .patch(url(h.addr, &format!("/api/sessions/{}", Uuid::new_v4()))) + .json(&serde_json::json!({ "name": "nope" })) + .send() + .await + .unwrap(); + assert_eq!(resp.status(), reqwest::StatusCode::NOT_FOUND); +} + #[tokio::test] async fn delete_owned_session_kills_it() { let h = spawn("naps-laptop").await; diff --git a/crates/maestro-daemon/tests/rename.rs b/crates/maestro-daemon/tests/rename.rs new file mode 100644 index 0000000..209f642 --- /dev/null +++ b/crates/maestro-daemon/tests/rename.rs @@ -0,0 +1,138 @@ +//! Renaming a live session (#125): `SessionManager::rename` semantics — +//! what lands in `metadata.name`, what reaches the sidecar, and what goes out +//! on the event stream. + +mod common; + +use std::sync::Arc; + +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 tempfile::TempDir; +use uuid::Uuid; + +struct Fixture { + manager: SessionManager, + sidecar: SidecarStore, + _tmux: TmuxFixture, + _state_dir: TempDir, +} + +fn fixture() -> Fixture { + let tmux = TmuxFixture::new(); + let state_dir = TempDir::new().unwrap(); + let sidecar = SidecarStore::new(state_dir.path().join("sessions")); + let manager = SessionManager::new( + Arc::new(tmux.tmux.clone()), + sidecar.clone(), + WorktreeManager::new(None), + state_dir.path().join("fifos"), + ); + Fixture { + manager, + sidecar, + _tmux: tmux, + _state_dir: state_dir, + } +} + +async fn spawn_named(manager: &SessionManager, name: Option<&str>) -> Uuid { + let metadata = match name { + Some(n) => std::collections::HashMap::from([( + "name".to_string(), + serde_json::Value::String(n.to_string()), + )]), + None => Default::default(), + }; + manager + .create(CreateSessionRequest { + cwd: "/tmp".into(), + command: "sleep 60".into(), + env: Default::default(), + metadata, + worktree: None, + }) + .await + .unwrap() + .id +} + +fn name_of(session: &maestro_protocol::Session) -> Option<&str> { + session.metadata.get("name").and_then(|v| v.as_str()) +} + +#[tokio::test] +async fn rename_on_unknown_session_is_not_found() { + let fx = fixture(); + let err = fx + .manager + .rename(Uuid::new_v4(), "whatever") + .await + .unwrap_err(); + assert!(matches!(err, maestro_daemon::SessionError::NotFound(_))); +} + +#[tokio::test] +async fn rename_sets_the_name_in_memory_and_on_disk() { + let fx = fixture(); + let id = spawn_named(&fx.manager, Some("amber-falcon")).await; + + let renamed = fx.manager.rename(id, " crimson-otter ").await.unwrap(); + assert_eq!(name_of(&renamed), Some("crimson-otter")); + + let snapshot = fx.manager.snapshot().await; + let live = snapshot.iter().find(|s| s.id == id).unwrap(); + assert_eq!(name_of(live), Some("crimson-otter")); + + // A rename a daemon restart would undo is worse than one that failed. + let persisted = fx.sidecar.read(id).unwrap().unwrap(); + assert_eq!(name_of(&persisted), Some("crimson-otter")); +} + +#[tokio::test] +async fn a_blank_rename_clears_the_name() { + let fx = fixture(); + let id = spawn_named(&fx.manager, Some("amber-falcon")).await; + + let renamed = fx.manager.rename(id, " ").await.unwrap(); + assert_eq!(name_of(&renamed), None); + // Line 1 falls back to the command, per docs/SIDEBAR.md. + assert_eq!(renamed.command, "sleep 60"); + assert_eq!(name_of(&fx.sidecar.read(id).unwrap().unwrap()), None); +} + +#[tokio::test] +async fn rename_caps_a_long_name() { + let fx = fixture(); + let id = spawn_named(&fx.manager, None).await; + let long = "x".repeat(MAX_SESSION_NAME_LEN + 20); + let renamed = fx.manager.rename(id, &long).await.unwrap(); + assert_eq!( + name_of(&renamed).unwrap().chars().count(), + MAX_SESSION_NAME_LEN + ); +} + +#[tokio::test] +async fn rename_broadcasts_a_session_update() { + let fx = fixture(); + let id = spawn_named(&fx.manager, Some("amber-falcon")).await; + let mut events = fx.manager.events(); + + fx.manager.rename(id, "crimson-otter").await.unwrap(); + + let event = tokio::time::timeout(std::time::Duration::from_secs(5), events.recv()) + .await + .expect("an update within the timeout") + .expect("the event channel stays open"); + match event { + CatalogEvent::SessionUpdated { session } => { + assert_eq!(session.id, id); + assert_eq!(name_of(&session), Some("crimson-otter")); + } + other => panic!("expected SessionUpdated, got {other:?}"), + } +} diff --git a/crates/maestro-protocol/src/client.rs b/crates/maestro-protocol/src/client.rs index f7d2dab..3caff1c 100644 --- a/crates/maestro-protocol/src/client.rs +++ b/crates/maestro-protocol/src/client.rs @@ -16,7 +16,8 @@ use uuid::Uuid; use crate::routes::{self, UrlError}; use crate::{ ApiError, BranchInfo, CreateSessionRequest, DaemonInfo, DirListing, ErrorInfo, GitRoot, - KillOutcome, Project, RecentDirs, ReportActivityRequest, RetainedWorktree, Session, + KillOutcome, Project, RecentDirs, RenameSessionRequest, ReportActivityRequest, + RetainedWorktree, Session, }; /// Time budget for establishing a TCP connection to the daemon. @@ -134,6 +135,17 @@ impl DaemonClient { json(resp).await } + /// `PATCH /api/sessions/:id` (#125): rename a live session. A blank name + /// clears the custom one, so the session falls back to its command. The + /// daemon trims and caps the name and answers with the updated session. + pub async fn rename_session(&self, id: Uuid, name: &str) -> Result { + let req = RenameSessionRequest { name: name.into() }; + let resp = self + .send(Method::PATCH, &routes::session(id), &[], Some(&req)) + .await?; + json(check(resp).await?).await + } + /// `POST /api/sessions/:id/activity` (#105): report an agent-observed /// state. The daemon holds it above its own inference for a TTL, then /// resumes inference. Answers `204 No Content`. diff --git a/crates/maestro-protocol/src/lib.rs b/crates/maestro-protocol/src/lib.rs index 8286ea7..6f33ea0 100644 --- a/crates/maestro-protocol/src/lib.rs +++ b/crates/maestro-protocol/src/lib.rs @@ -495,6 +495,31 @@ pub struct NameBatch { pub names: Vec, } +/// Longest session name the daemon stores (#125), in characters. A name is a +/// sidebar card's line 1; past this it only truncates on every client anyway. +pub const MAX_SESSION_NAME_LEN: usize = 64; + +/// HTTP `PATCH /api/sessions/:id` body (#125): rename a live session. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct RenameSessionRequest { + pub name: String, +} + +/// Trim a requested session name and cap it at [`MAX_SESSION_NAME_LEN`] +/// characters. `None` when nothing is left — that clears `metadata.name` and +/// the card falls back to the command, the same rule the spawn form uses when +/// its name field is blank. +/// +/// Shared so the daemon and every Rust client agree on what a name becomes; +/// `web/src/types.ts` carries the same function for the browser. +pub fn normalize_session_name(name: &str) -> Option { + let trimmed = name.trim(); + if trimmed.is_empty() { + return None; + } + Some(trimmed.chars().take(MAX_SESSION_NAME_LEN).collect()) +} + // --------------------------------------------------------------------------- // Catalog events (`WS /api/events`) // --------------------------------------------------------------------------- @@ -964,4 +989,42 @@ mod tests { assert_eq!(c, round_trip(&c)); } } + + /// #125: a rename request is trimmed and capped; a blank one clears the + /// name so the card falls back to the command. + #[test] + fn session_names_are_trimmed_capped_and_blankable() { + assert_eq!( + normalize_session_name(" amber-falcon "), + Some("amber-falcon".into()) + ); + assert_eq!(normalize_session_name(""), None); + assert_eq!(normalize_session_name(" \t "), None); + let long = "x".repeat(MAX_SESSION_NAME_LEN + 10); + assert_eq!( + normalize_session_name(&long).unwrap().chars().count(), + MAX_SESSION_NAME_LEN + ); + } + + /// The cap counts characters, not bytes — a multi-byte name must not be + /// sliced mid-codepoint. + #[test] + fn session_name_cap_counts_characters() { + let name = "é".repeat(MAX_SESSION_NAME_LEN + 5); + let capped = normalize_session_name(&name).unwrap(); + assert_eq!(capped.chars().count(), MAX_SESSION_NAME_LEN); + } + + #[test] + fn rename_request_round_trips() { + let req = RenameSessionRequest { + name: "amber-falcon".into(), + }; + assert_eq!(req, round_trip(&req)); + assert_eq!( + serde_json::to_value(&req).unwrap(), + serde_json::json!({"name": "amber-falcon"}) + ); + } }