diff --git a/CLAUDE.md b/CLAUDE.md index 50ec101..3d58eaf 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -66,6 +66,10 @@ See `docs/INSTALL.md` for end-user install instructions. collisions, what blocks a kill, what `--force` retains, and what reclaim touches. Read it before changing anything that creates or removes one. +`docs/SIDEBAR.md` is the contract for session cards in the web and TUI +sidebars — line layout, activity colours, duration format, state precedence. +Read it before changing sidebar rendering. + `nix run .#dev` (or `just dev`) brings up the whole local stack under process-compose: a debug daemon on 127.0.0.1:7071, then Vite on :5173 once the daemon answers `/api/info`. The SPA proxies `/api/*` back to the daemon, so only diff --git a/Cargo.lock b/Cargo.lock index 646ccfb..7089357 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1065,6 +1065,7 @@ dependencies = [ "futures-util", "libc", "maestro-config", + "maestro-hooks", "maestro-protocol", "maestro-testing", "nix", @@ -1102,6 +1103,7 @@ dependencies = [ "futures-util", "libc", "maestro-config", + "maestro-hooks", "maestro-names", "maestro-protocol", "reqwest", @@ -1137,6 +1139,15 @@ dependencies = [ "uuid", ] +[[package]] +name = "maestro-hooks" +version = "0.1.0" +dependencies = [ + "serde_json", + "tempfile", + "thiserror 1.0.69", +] + [[package]] name = "maestro-installer" version = "0.1.0" diff --git a/Cargo.toml b/Cargo.toml index 7fb7deb..a15a776 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -18,6 +18,7 @@ dirs = "5" futures-util = { version = "0.3", default-features = false, features = ["sink", "std"] } libc = "0.2" maestro-config = { path = "crates/maestro-config" } +maestro-hooks = { path = "crates/maestro-hooks" } maestro-names = { path = "crates/maestro-names" } maestro-protocol = { path = "crates/maestro-protocol" } maestro-testing = { path = "crates/maestro-testing" } diff --git a/crates/maestro-cli/Cargo.toml b/crates/maestro-cli/Cargo.toml index 9dc2320..81720da 100644 --- a/crates/maestro-cli/Cargo.toml +++ b/crates/maestro-cli/Cargo.toml @@ -17,6 +17,7 @@ dirs.workspace = true futures-util.workspace = true libc.workspace = true maestro-config.workspace = true +maestro-hooks.workspace = true maestro-protocol = { workspace = true, features = ["client"] } nix.workspace = true serde_json.workspace = true diff --git a/crates/maestro-cli/src/cli.rs b/crates/maestro-cli/src/cli.rs index b1c7ac2..fbb7ee7 100644 --- a/crates/maestro-cli/src/cli.rs +++ b/crates/maestro-cli/src/cli.rs @@ -43,6 +43,23 @@ pub enum Command { Kill(KillArgs), /// List retained worktrees, or reclaim one by ID (#86). Worktrees(WorktreesArgs), + /// Install or update the agent activity hook (#106). + Hooks(HooksArgs), +} + +/// `maestro hooks ` +#[derive(Debug, clap::Args)] +pub struct HooksArgs { + #[command(subcommand)] + pub command: HooksCommand, +} + +#[derive(Debug, Subcommand)] +pub enum HooksCommand { + /// Write the Claude Code activity hook into ~/.claude/settings.json. + /// Idempotent: an older maestro hook is updated in place, every other + /// hook is left untouched. + Install, } /// `maestro worktrees ` @@ -275,6 +292,23 @@ mod tests { assert!(err.to_string().contains("KEY=VALUE")); } + #[test] + fn parses_hooks_install() { + let cli = Cli::try_parse_from(["maestro", "hooks", "install"]).unwrap(); + match cli.command { + Command::Hooks(h) => { + assert!(matches!(h.command, HooksCommand::Install)); + } + _ => panic!("wrong subcommand"), + } + // No subcommand → help, like `worktrees`. + let err = Cli::try_parse_from(["maestro", "hooks"]).unwrap_err(); + assert!(matches!( + err.kind(), + clap::error::ErrorKind::DisplayHelpOnMissingArgumentOrSubcommand + )); + } + #[test] fn parses_worktrees_list() { let cli = Cli::try_parse_from(["maestro", "worktrees", "list", "--json"]).unwrap(); diff --git a/crates/maestro-cli/src/dispatch.rs b/crates/maestro-cli/src/dispatch.rs index 3e2b36a..1febc52 100644 --- a/crates/maestro-cli/src/dispatch.rs +++ b/crates/maestro-cli/src/dispatch.rs @@ -24,6 +24,10 @@ pub enum CliError { #[error("{0}")] Attach(#[from] crate::attach::AttachError), #[error("{0}")] + Hooks(#[from] maestro_hooks::HooksError), + #[error("could not determine the home directory ($HOME unset?)")] + NoHome, + #[error("{0}")] Io(#[from] std::io::Error), } @@ -40,6 +44,15 @@ pub async fn dispatch( None => default_config_path()?, }; + // `hooks install` is purely local: it must work with the daemon down, so + // it never touches the client. + if let Command::Hooks(args) = &command { + match args.command { + crate::cli::HooksCommand::Install => run_hooks_install()?, + } + return Ok(()); + } + let daemon = client(&config_path, daemon_url_override)?; match command { @@ -77,6 +90,7 @@ pub async fn dispatch( ); } } + Command::Hooks(_) => unreachable!("hooks handled before the daemon client exists"), Command::Worktrees(args) => match args.command { WorktreesCommand::List(list_args) => { let mut stdout = std::io::stdout().lock(); @@ -101,6 +115,25 @@ pub async fn dispatch( Ok(()) } +/// `maestro hooks install` (#106): write the shipped hook script under the +/// user's data dir and register it in their Claude Code settings. Idempotent +/// and foreign-hook-preserving — the guarantees live in [`maestro_hooks`]. +#[allow(clippy::result_large_err)] +fn run_hooks_install() -> Result<(), CliError> { + let home = dirs::home_dir().ok_or(CliError::NoHome)?; + let changed = maestro_hooks::install_for_home(&home)?; + if changed { + println!( + "installed claude-code activity hook: {} → {}", + maestro_hooks::script_path(&home).display(), + maestro_hooks::settings_path(&home).display() + ); + } else { + println!("claude-code activity hook already up to date"); + } + Ok(()) +} + /// Default detach byte, matching `maestro attach --detach-key`'s `^q`. const CTRL_Q: u8 = 0x11; diff --git a/crates/maestro-cli/src/sessions.rs b/crates/maestro-cli/src/sessions.rs index ad43be6..12fa99b 100644 --- a/crates/maestro-cli/src/sessions.rs +++ b/crates/maestro-cli/src/sessions.rs @@ -108,6 +108,7 @@ fn print_session_table(out: &mut impl Write, sessions: &[Session], info: &Daemon ActivityState::Active => "active", ActivityState::Idle => "idle", ActivityState::AwaitingInput => "awaiting", + ActivityState::Error => "error", }; // Show the live foreground command alongside the spawn command when // the daemon has observed one that differs (NAP-340). @@ -175,7 +176,9 @@ mod tests { created_at: Utc.with_ymd_and_hms(2026, 5, 25, 12, 0, 0).unwrap(), foreground: None, activity: Default::default(), + activity_detail: None, last_activity_at: chrono::Utc::now(), + activity_since: chrono::Utc::now(), current_cwd: None, blocked_work: None, } diff --git a/crates/maestro-config/src/lib.rs b/crates/maestro-config/src/lib.rs index e7d6520..87b2bb5 100644 --- a/crates/maestro-config/src/lib.rs +++ b/crates/maestro-config/src/lib.rs @@ -64,6 +64,24 @@ pub struct MaestroConfigFile { pub tui: TuiSection, #[serde(default)] pub spawn: SpawnSection, + #[serde(default)] + pub hooks: HooksSection, +} + +/// `[hooks]` — agent-hook installation settings (#106). +#[derive(Debug, Clone, Default, PartialEq, Eq, Deserialize)] +pub struct HooksSection { + /// Whether the daemon installs the Claude Code activity hook into + /// `~/.claude/settings.json` on first start when no maestro hook is + /// there yet. Default `true`; set `auto_install = false` to opt out. + #[serde(default)] + pub auto_install: Option, +} + +impl HooksSection { + pub fn auto_install(&self) -> bool { + self.auto_install.unwrap_or(true) + } } #[derive(Debug, Clone, Default, PartialEq, Eq, Deserialize)] @@ -375,6 +393,26 @@ mod tests { assert_eq!(cfg, MaestroConfigFile::default()); } + #[test] + fn hooks_auto_install_defaults_true_and_is_opt_out() { + // Absent file / absent [hooks] ⇒ auto_install on. + assert!(HooksSection::default().auto_install()); + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("config.toml"); + std::fs::write(&path, "").unwrap(); + assert!(MaestroConfigFile::load(&path).unwrap().hooks.auto_install()); + + std::fs::write( + &path, + r#" +[hooks] +auto_install = false +"#, + ) + .unwrap(); + assert!(!MaestroConfigFile::load(&path).unwrap().hooks.auto_install()); + } + #[test] fn spawn_presets_default_to_builtins_plus_custom() { // Absent [spawn].presets → built-ins, with the implicit custom tile last. diff --git a/crates/maestro-daemon/Cargo.toml b/crates/maestro-daemon/Cargo.toml index 24c5c52..7785c96 100644 --- a/crates/maestro-daemon/Cargo.toml +++ b/crates/maestro-daemon/Cargo.toml @@ -18,6 +18,7 @@ dirs.workspace = true futures-util.workspace = true libc.workspace = true maestro-config.workspace = true +maestro-hooks.workspace = true maestro-names.workspace = true maestro-protocol.workspace = true serde.workspace = true diff --git a/crates/maestro-daemon/src/activity.rs b/crates/maestro-daemon/src/activity.rs index 7405bb5..5ece6ed 100644 --- a/crates/maestro-daemon/src/activity.rs +++ b/crates/maestro-daemon/src/activity.rs @@ -15,6 +15,44 @@ use maestro_protocol::ActivityState; /// Active↔Idle. This window *is* the debounce the issue calls for. pub const ACTIVE_WINDOW: Duration = Duration::from_secs(3); +/// How long an agent-reported state (#105) stays in force before inference +/// resumes. Long enough for a working agent's heartbeat to hold between +/// reports, short enough that an agent dying mid-run cannot pin its session +/// at `Error` forever. +pub const REPORT_TTL: Duration = Duration::from_secs(300); + +/// Server-side cap (#105) on a reported `detail`, in characters. The meta +/// line renders it inline, so an unbounded string would own the whole card. +pub const MAX_REPORT_DETAIL_CHARS: usize = 256; + +/// An agent-reported activity state (#105) still within its TTL. While one +/// is [`Self::in_force`], it outranks [`decide`]'s inference; when it +/// lapses, inference resumes and any reported detail is cleared with it. +#[derive(Debug, Clone)] +pub struct ReportedActivity { + pub state: ActivityState, + pub expires_at: std::time::Instant, +} + +impl ReportedActivity { + pub fn new(state: ActivityState, ttl: Duration) -> Self { + Self { + state, + expires_at: std::time::Instant::now() + ttl, + } + } + + pub fn in_force(&self, now: std::time::Instant) -> bool { + now < self.expires_at + } +} + +/// Cap a reported detail to [`MAX_REPORT_DETAIL_CHARS`] characters, +/// char-boundary safe. +pub fn truncate_detail(detail: Option) -> Option { + detail.map(|d| d.chars().take(MAX_REPORT_DETAIL_CHARS).collect()) +} + /// Combine the observed signals into an [`ActivityState`]: /// recent output ⇒ `Active`; else blocked on a terminal read ⇒ `AwaitingInput`; /// else `Idle`. @@ -59,4 +97,25 @@ mod tests { ActivityState::Idle ); } + + #[test] + fn report_is_in_force_until_exactly_its_expiry() { + let ttl = Duration::from_secs(300); + let report = ReportedActivity::new(ActivityState::Error, ttl); + assert!(report.in_force(report.expires_at - Duration::from_millis(1))); + assert!(!report.in_force(report.expires_at)); + } + + #[test] + fn detail_truncates_to_the_cap_without_panic_on_multibyte_input() { + let long = "é".repeat(MAX_REPORT_DETAIL_CHARS + 10); + let capped = truncate_detail(Some(long)).unwrap(); + assert_eq!(capped.chars().count(), MAX_REPORT_DETAIL_CHARS); + + assert_eq!(truncate_detail(None), None); + assert_eq!( + truncate_detail(Some("short".into())).as_deref(), + Some("short") + ); + } } diff --git a/crates/maestro-daemon/src/cli.rs b/crates/maestro-daemon/src/cli.rs index cef210d..c0b8a3f 100644 --- a/crates/maestro-daemon/src/cli.rs +++ b/crates/maestro-daemon/src/cli.rs @@ -47,6 +47,20 @@ async fn run(config_override: Option) -> Result<(), Box> { let _ = default_state_dir(); std::fs::create_dir_all(config.sidecar_dir())?; std::fs::create_dir_all(config.fifo_dir())?; + + // First-start hook auto-install (#106). Best-effort, before serving: a + // session spawned right away should already carry the hook. + if config.hooks_auto_install { + match crate::hooks::auto_install(&config.home_dir) { + crate::hooks::AutoInstallOutcome::Installed => { + tracing::info!("installed claude-code activity hook (#106)"); + } + crate::hooks::AutoInstallOutcome::Failed => { + tracing::warn!("hooks auto-install failed; run `maestro hooks install` manually"); + } + _ => {} + } + } // No worktree dir to pre-create: without an override the worktrees live // under their base repos, created on demand (#56). @@ -73,6 +87,9 @@ async fn run(config_override: Option) -> Result<(), Box> { // One store instance shared with the API handlers: forced kills (#81) // record here, `GET /api/worktrees` (#80) reads back from here. .with_retained_store((*retained).clone()); + // Spawned sessions learn where to report back (#103). Derived from the + // configured bind address; set before any spawn can happen. + manager.set_daemon_url(format!("http://{}", config.bind_addr)); let recovered = manager.recover().await?; tracing::info!(recovered, "adopted existing tmux sessions"); diff --git a/crates/maestro-daemon/src/config.rs b/crates/maestro-daemon/src/config.rs index 31bf2c5..3fc9a66 100644 --- a/crates/maestro-daemon/src/config.rs +++ b/crates/maestro-daemon/src/config.rs @@ -88,6 +88,10 @@ pub struct DaemonConfig { /// Project-discovery depth (#54): `[spawn].project_scan_depth`, default /// [`DEFAULT_PROJECT_SCAN_DEPTH`]. pub project_scan_depth: u32, + /// Whether the daemon installs the Claude Code activity hook on first + /// start when none is present (#106): `[hooks].auto_install`, default + /// `true`. + pub hooks_auto_install: bool, } impl DaemonConfig { @@ -197,6 +201,7 @@ pub fn load_config(path: &Path, env: EnvOverrides) -> Result String { mod tests { use super::*; + #[test] + fn hooks_auto_install_defaults_true_and_is_opt_out() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("config.toml"); + + std::fs::write(&path, "").unwrap(); + let cfg = load_config(&path, EnvOverrides::default()).unwrap(); + assert!(cfg.hooks_auto_install); + + std::fs::write( + &path, + r#" +[hooks] +auto_install = false +"#, + ) + .unwrap(); + let cfg = load_config(&path, EnvOverrides::default()).unwrap(); + assert!(!cfg.hooks_auto_install); + } + #[test] fn empty_file_defaults_everything() { let dir = tempfile::tempdir().unwrap(); diff --git a/crates/maestro-daemon/src/hooks.rs b/crates/maestro-daemon/src/hooks.rs new file mode 100644 index 0000000..1eba19b --- /dev/null +++ b/crates/maestro-daemon/src/hooks.rs @@ -0,0 +1,76 @@ +//! First-start hook auto-install (#106). +//! +//! The daemon runs [`auto_install`] once at startup: when the user's +//! `~/.claude/settings.json` carries no maestro hook yet (and +//! `[hooks].auto_install` hasn't opted out — checked by the caller), the same +//! installer `maestro hooks install` uses writes it. Best-effort: a failure is +//! warned about and never blocks startup. The same global hook serves every +//! Claude Code session on the machine, so nothing here targets repositories +//! or worktrees. + +use std::path::Path; + +/// Install the Claude Code activity hook for `home_dir` unless one is already +/// present or the home dir is unresolvable (`""`). Returns what happened. +pub fn auto_install(home_dir: &str) -> AutoInstallOutcome { + if home_dir.is_empty() { + return AutoInstallOutcome::Skipped; + } + let settings = maestro_hooks::settings_path(Path::new(home_dir)); + if maestro_hooks::is_installed(&settings) { + return AutoInstallOutcome::AlreadyPresent; + } + match maestro_hooks::install_for_home(Path::new(home_dir)) { + Ok(_) => AutoInstallOutcome::Installed, + Err(e) => { + tracing::warn!(error = %e, "hooks auto-install failed (non-fatal)"); + AutoInstallOutcome::Failed + } + } +} + +/// What [`auto_install`] did, so callers can log and tests can assert. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum AutoInstallOutcome { + Installed, + AlreadyPresent, + Failed, + Skipped, +} + +#[cfg(test)] +mod tests { + use super::*; + + /// Everything happens under a temp `$HOME`; the developer's own + /// `~/.claude/settings.json` is never read or written. + #[test] + fn installs_when_settings_carry_no_maestro_hook() { + let dir = tempfile::tempdir().unwrap(); + let outcome = auto_install(dir.path().to_str().unwrap()); + assert_eq!(outcome, AutoInstallOutcome::Installed); + let settings = dir.path().join(".claude/settings.json"); + assert!(maestro_hooks::is_installed(&settings)); + } + + #[test] + fn second_start_is_a_noop() { + let dir = tempfile::tempdir().unwrap(); + let home = dir.path().to_str().unwrap(); + auto_install(home); + // A foreign edit between starts survives; presence alone skips. + let settings = dir.path().join(".claude/settings.json"); + let before = std::fs::read_to_string(&settings).unwrap(); + assert_eq!(auto_install(home), AutoInstallOutcome::AlreadyPresent); + assert_eq!( + before, + std::fs::read_to_string(&settings).unwrap(), + "already-installed settings must not be rewritten" + ); + } + + #[test] + fn unresolvable_home_is_skipped() { + assert_eq!(auto_install(""), AutoInstallOutcome::Skipped); + } +} diff --git a/crates/maestro-daemon/src/lib.rs b/crates/maestro-daemon/src/lib.rs index 8f85980..8c76df9 100644 --- a/crates/maestro-daemon/src/lib.rs +++ b/crates/maestro-daemon/src/lib.rs @@ -14,6 +14,7 @@ pub mod cli; pub mod config; pub mod dirlist; pub mod gitroot; +pub mod hooks; pub mod local_api; pub mod panes; pub mod proc; @@ -27,5 +28,6 @@ pub mod tmux; pub mod worktree; pub mod worktree_probe; +pub use activity::MAX_REPORT_DETAIL_CHARS; pub use config::{default_config_path, default_state_dir, load_config, DaemonConfig, EnvOverrides}; pub use session::{SessionError, SessionManager}; diff --git a/crates/maestro-daemon/src/local_api.rs b/crates/maestro-daemon/src/local_api.rs index 6ae1c3a..06c3100 100644 --- a/crates/maestro-daemon/src/local_api.rs +++ b/crates/maestro-daemon/src/local_api.rs @@ -15,12 +15,12 @@ use std::sync::Arc; use axum::extract::{Path, Query, State}; use axum::http::StatusCode; use axum::response::{IntoResponse, Response}; -use axum::routing::{any, delete, get}; +use axum::routing::{any, delete, get, post}; use axum::{Json, Router}; use maestro_config::SpawnPreset; use maestro_protocol::{ - routes, ApiError, CatalogEvent, CreateSessionRequest, DaemonInfo, GitRoot, NameBatch, Session, - PROTOCOL_VERSION, + routes, ApiError, CatalogEvent, CreateSessionRequest, DaemonInfo, GitRoot, NameBatch, + ReportActivityRequest, Session, PROTOCOL_VERSION, }; use serde::Deserialize; use uuid::Uuid; @@ -46,6 +46,7 @@ pub struct LocalApiState { 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::DIRS, get(list_dir)) .route(routes::RECENT_DIRS, get(recent_dirs)) @@ -318,6 +319,29 @@ async fn reclaim_worktree(State(s): State, Path(id): Path) } } +/// `POST /api/sessions/:id/activity` (#105): an agent reports its own state. +/// The report holds above the daemon's inference for a TTL, then inference +/// resumes — an agent that dies mid-run cannot pin its session at `Error` +/// forever. Unknown session is 404; an unknown state value is rejected by the +/// JSON decoder (422) rather than coerced. Answers `204 No Content`. +async fn report_activity( + State(s): State, + Path(id): Path, + Json(body): Json, +) -> Response { + match s.manager.report_activity(id, body.state, body.detail).await { + Ok(()) => StatusCode::NO_CONTENT.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(), + ) + } + } +} + fn error_response(status: StatusCode, code: &str, message: &str) -> Response { (status, Json(ApiError::new(code, message))).into_response() } diff --git a/crates/maestro-daemon/src/session.rs b/crates/maestro-daemon/src/session.rs index a8150bb..7861155 100644 --- a/crates/maestro-daemon/src/session.rs +++ b/crates/maestro-daemon/src/session.rs @@ -57,6 +57,16 @@ pub struct SessionRecord { /// fall back to a probe. pub worktree_branch: Option, pub bridge: PtyBridge, + /// Agent-reported activity (#105) still within its TTL. While in force it + /// outranks the activity pass's own inference; when it lapses, inference + /// resumes and [`Session::activity_detail`] is cleared with it. This + /// field is daemon-local bookkeeping only — it never rides the wire. + pub reported_activity: Option, + /// Flipped once the natural-exit settle step (#82) has been applied or + /// refused for this session. `Dead` is observable before settling ends + /// (#115), so this is the observable that says its outcomes are safe to + /// assert on. Daemon-local bookkeeping; never rides the wire. + pub settled: bool, } #[derive(Debug, thiserror::Error)] @@ -131,6 +141,14 @@ struct Inner { /// Retained-worktree store (#78). `None` when not wired (tests); a forced /// kill (#81) without one fails rather than destroying unrecorded work. retained: Option, + /// The daemon's own HTTP base URL (#103), exported to spawned sessions as + /// `MAESTRO_URL`. `None` when unknown — those sessions simply don't get + /// the variable. A mutex because the runner sets it after construction, + /// possibly while clones are already serving. + daemon_url: std::sync::Mutex>, + /// TTL an agent-reported activity state (#105) stays in force. A const in + /// production; overridable for tests that watch the lapse happen. + report_ttl: Duration, sessions: Mutex>, events: broadcast::Sender, } @@ -170,6 +188,8 @@ impl SessionManager { replay_bytes, spawn_history: None, retained: None, + daemon_url: std::sync::Mutex::new(None), + report_ttl: crate::activity::REPORT_TTL, sessions: Mutex::new(HashMap::new()), events, }), @@ -202,6 +222,23 @@ impl SessionManager { self } + /// Override the agent-report TTL (#105). Tests pass a short one to watch + /// inference resume without waiting out [`crate::activity::REPORT_TTL`]. + pub fn with_activity_report_ttl(mut self, ttl: Duration) -> Self { + let inner = Arc::get_mut(&mut self.inner) + .expect("with_activity_report_ttl must be called before the SessionManager is cloned"); + inner.report_ttl = ttl; + self + } + + /// Set the daemon's own HTTP base URL (#103), exported to every session + /// spawned afterwards as `MAESTRO_URL`. Unlike the `with_*` builders this + /// is a plain setter: the daemon runner learns its bound address after + /// construction, and clones may already exist by then. + pub fn set_daemon_url(&self, url: String) { + *self.inner.daemon_url.lock().expect("daemon_url mutex") = Some(url); + } + /// Recent working dirs for the spawn form's picker (#24), newest-first and /// deduped by dir. Empty when no history is wired or it's disabled. pub fn recent_dirs(&self) -> Vec { @@ -262,6 +299,17 @@ impl SessionManager { sessions.get(&id).map(|r| r.session.status) } + /// Whether the natural-exit settle step (#82) has finished for this + /// session: `Some(true)` once applied or refused, `Some(false)` while + /// still pending, `None` if the session is unknown. `Dead` is observable + /// before settling ends (#115), so anything asserting on a dead + /// session's worktree waits on this rather than inferring settle + /// completion from its side effects. + pub async fn is_settled(&self, id: Uuid) -> Option { + let sessions = self.inner.sessions.lock().await; + sessions.get(&id).map(|r| r.settled) + } + /// Spawn a new session. On success returns the freshly-created `Session`. pub async fn create(&self, req: CreateSessionRequest) -> Result { let (effective_cwd, worktree_path, worktree_base_repo) = match &req.worktree { @@ -304,7 +352,7 @@ impl SessionManager { return Err(e.into()); } }; - let session_env = match sanitize_env(&req.env) { + let mut session_env = match sanitize_env(&req.env) { Ok(env) => env, Err(e) => { let _ = std::fs::remove_file(&fifo_path); @@ -312,6 +360,19 @@ impl SessionManager { return Err(e); } }; + // Identity + reporting target for the in-session hook (#106). Ours + // win over whatever the request carried, so a client cannot spoof + // another session's identity (#103). + session_env.insert("MAESTRO_SESSION_ID".into(), id.to_string()); + if let Some(url) = self + .inner + .daemon_url + .lock() + .expect("daemon_url mutex") + .clone() + { + session_env.insert("MAESTRO_URL".into(), url); + } let spawned = { let tmux = self.inner.tmux.clone(); let cwd = effective_cwd.clone(); @@ -352,7 +413,9 @@ impl SessionManager { // activity watcher refines `foreground`/`activity` on its next tick. foreground: None, activity: ActivityState::Active, + activity_detail: None, last_activity_at: Utc::now(), + activity_since: chrono::Utc::now(), current_cwd: None, // The watcher fills this in on its first blocked-work pass; a @@ -381,6 +444,8 @@ impl SessionManager { worktree_base_repo, worktree_branch: req.worktree.as_ref().map(|w| w.branch.clone()), bridge, + reported_activity: None, + settled: false, }, ); } @@ -569,9 +634,14 @@ impl SessionManager { status: SessionStatus::Running, created_at: Utc::now(), // Activity is unknown on adoption; the watcher fills it in. + // #104: the restart also resets the activity clock to now — + // accepted, since the daemon cannot know how long a + // recovered session had already been sitting idle. foreground: None, activity: ActivityState::Idle, + activity_detail: None, last_activity_at: Utc::now(), + activity_since: chrono::Utc::now(), current_cwd: None, blocked_work: None, @@ -603,6 +673,8 @@ impl SessionManager { // recovered session probes git for it at retain time. worktree_branch: None, bridge, + reported_activity: None, + settled: false, }, ); let _ = self @@ -643,6 +715,47 @@ impl SessionManager { .await .map_err(SessionError::Tmux) } + + /// 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 + /// cannot pin its session at `Error` forever. An unknown session is + /// [`SessionError::NotFound`]; the reported `detail` is capped + /// server-side. + /// + /// A report that changes the effective state restarts `activity_since` + /// (#104's clock); re-reporting an unchanged state only extends the TTL + /// and stays silent on the event stream at steady state. + pub async fn report_activity( + &self, + id: Uuid, + state: ActivityState, + detail: Option, + ) -> Result<(), SessionError> { + let updated = { + let mut sessions = self.inner.sessions.lock().await; + let rec = sessions.get_mut(&id).ok_or(SessionError::NotFound(id))?; + let detail = crate::activity::truncate_detail(detail); + let changed = rec.session.activity != state || rec.session.activity_detail != detail; + if rec.session.activity != state { + rec.session.activity_since = Utc::now(); + } + rec.session.activity = state; + rec.session.activity_detail = detail; + rec.reported_activity = Some(crate::activity::ReportedActivity::new( + state, + self.inner.report_ttl, + )); + changed.then(|| rec.session.clone()) + }; + if let Some(session) = updated { + let _ = self + .inner + .events + .send(CatalogEvent::SessionUpdated { session }); + } + Ok(()) + } } /// Run a blocking `tmux` subprocess call on the blocking thread pool so it @@ -813,8 +926,22 @@ async fn run_activity_pass(inner: &Arc) { let Some(rec) = sessions.get_mut(&id) else { continue; }; + // #105: an agent report lapses at its TTL — drop it and its + // detail so this tick's inference takes over again. + if let Some(report) = &rec.reported_activity { + if !report.in_force(std::time::Instant::now()) { + rec.reported_activity = None; + rec.session.activity_detail = None; + } + } let idle = rec.bridge.idle_for(); - let activity = crate::activity::decide(idle, awaiting); + // #105: while a report is in force it supplies the state and + // nothing here may overwrite it; only an unreported session is + // decided by inference. + let activity = match &rec.reported_activity { + Some(report) => report.state, + None => crate::activity::decide(idle, awaiting), + }; let foreground = Some(pane.command.clone()); // The pane's live cwd, drives the dir-based session tree; empty means // tmux reported none. @@ -828,6 +955,12 @@ async fn run_activity_pass(inner: &Arc) { { None } else { + // #104: stamp only when the state itself changed — never on + // foreground/cwd-only diffs, and never every tick, or the + // sidebar's duration would reset continuously and read zero. + if rec.session.activity != activity { + rec.session.activity_since = Utc::now(); + } rec.session.activity = activity; rec.session.foreground = foreground; rec.session.current_cwd = current_cwd; @@ -954,6 +1087,14 @@ async fn mark_session_dead(inner: &Arc, id: Uuid) { } } } + // Settle has now been applied or refused either way (#115): from here on + // its outcomes are safe to observe. + { + let mut sessions = inner.sessions.lock().await; + if let Some(record) = sessions.get_mut(&id) { + record.settled = true; + } + } let _ = inner.sidecar.write(&session); let _ = inner.events.send(CatalogEvent::SessionUpdated { session }); diff --git a/crates/maestro-daemon/src/tmux.rs b/crates/maestro-daemon/src/tmux.rs index e0f7720..1d278b9 100644 --- a/crates/maestro-daemon/src/tmux.rs +++ b/crates/maestro-daemon/src/tmux.rs @@ -264,6 +264,25 @@ impl Tmux { check(out) } + /// `tmux show-environment -t maestro-`, parsed into KEY=VALUE pairs. + /// Test-facing view of what a spawned session actually inherits. + pub fn show_environment(&self, id: Uuid) -> Result, TmuxError> { + let name = session_name(id); + let out = self + .cmd() + .args(["show-environment", "-t", &name]) + .output()?; + check(out.clone())?; + let stdout = String::from_utf8_lossy(&out.stdout); + let mut env = BTreeMap::new(); + for line in stdout.lines() { + if let Some((k, v)) = line.split_once('=') { + env.insert(k.to_string(), v.to_string()); + } + } + Ok(env) + } + /// `tmux kill-server`. Best-effort cleanup used mostly by tests. pub fn kill_server(&self) -> Result<(), TmuxError> { let out = self.cmd().arg("kill-server").output()?; diff --git a/crates/maestro-daemon/tests/activity.rs b/crates/maestro-daemon/tests/activity.rs index d7fe201..a3b6348 100644 --- a/crates/maestro-daemon/tests/activity.rs +++ b/crates/maestro-daemon/tests/activity.rs @@ -189,8 +189,23 @@ fn a_foreground_pipeline_whose_leader_exited_is_still_awaiting_input() { "never observed a foreground group whose leader had exited" ); - assert!( - awaiting_input(pane.pane_pid, &pane.tty, &ProcGroups::snapshot()), - "a surviving pipeline member still holds the tty" - ); + // The leader can be gone while the surviving stage is still setting up: + // `true` exits as soon as both stages are forked, before the reader has + // opened the tty and parked. Poll for the answer, but only accept it + // while the state this test exists for still holds — otherwise a shell + // that regained the terminal would satisfy it vacuously. + let deadline = std::time::Instant::now() + Duration::from_secs(5); + let mut holds = false; + while std::time::Instant::now() < deadline { + let groups = ProcGroups::snapshot(); + let still_leaderless = pgrp_and_tpgid(pane.pane_pid).is_some_and(|(pgrp, tpgid)| { + tpgid > 0 && tpgid != pgrp && pgrp_and_tpgid(tpgid).is_none() + }); + if still_leaderless && awaiting_input(pane.pane_pid, &pane.tty, &groups) { + holds = true; + break; + } + std::thread::sleep(Duration::from_millis(20)); + } + assert!(holds, "a surviving pipeline member still holds the tty"); } diff --git a/crates/maestro-daemon/tests/activity_report.rs b/crates/maestro-daemon/tests/activity_report.rs new file mode 100644 index 0000000..e494856 --- /dev/null +++ b/crates/maestro-daemon/tests/activity_report.rs @@ -0,0 +1,164 @@ +//! Agent-reported activity (#105): `POST /api/sessions/:id/activity` +//! semantics at the SessionManager level — precedence over the daemon's own +//! inference, detail handling, and lapse back to inference at the TTL. + +mod common; + +use std::sync::Arc; +use std::time::Duration; + +use common::TmuxFixture; +use maestro_daemon::sidecar::SidecarStore; +use maestro_daemon::worktree::WorktreeManager; +use maestro_daemon::SessionManager; +use maestro_protocol::{ActivityState, CreateSessionRequest}; +use tempfile::TempDir; +use tokio::time::sleep; +use uuid::Uuid; + +struct Fixture { + manager: SessionManager, + _tmux: TmuxFixture, + _state_dir: TempDir, +} + +async fn fixture(ttl: Duration) -> Fixture { + let tmux = TmuxFixture::new(); + let state_dir = TempDir::new().unwrap(); + let manager = SessionManager::new( + Arc::new(tmux.tmux.clone()), + SidecarStore::new(state_dir.path().join("sessions")), + WorktreeManager::new(None), + state_dir.path().join("fifos"), + ) + .with_activity_report_ttl(ttl); + Fixture { + manager, + _tmux: tmux, + _state_dir: state_dir, + } +} + +async fn spawn_sleeping(manager: &SessionManager) -> Uuid { + manager + .create(CreateSessionRequest { + cwd: "/tmp".into(), + command: "sleep 60".into(), + env: Default::default(), + metadata: Default::default(), + worktree: None, + }) + .await + .unwrap() + .id +} + +async fn observed(manager: &SessionManager, id: Uuid) -> maestro_protocol::Session { + manager + .snapshot() + .await + .into_iter() + .find(|s| s.id == id) + .expect("session still tracked") +} + +#[tokio::test] +async fn report_on_unknown_session_is_not_found() { + let fx = fixture(Duration::from_secs(300)).await; + let err = fx + .manager + .report_activity(Uuid::new_v4(), ActivityState::Idle, None) + .await + .unwrap_err(); + assert!(matches!(err, maestro_daemon::SessionError::NotFound(_))); +} + +#[tokio::test] +async fn report_sets_error_state_and_detail() { + let fx = fixture(Duration::from_secs(300)).await; + let id = spawn_sleeping(&fx.manager).await; + + fx.manager + .report_activity( + id, + ActivityState::Error, + Some("cargo build failed: link error".into()), + ) + .await + .unwrap(); + + let s = observed(&fx.manager, id).await; + assert_eq!(s.activity, ActivityState::Error); + assert_eq!( + s.activity_detail.as_deref(), + Some("cargo build failed: link error") + ); +} + +#[tokio::test] +async fn replacing_a_report_replaces_both_state_and_detail() { + let fx = fixture(Duration::from_secs(300)).await; + let id = spawn_sleeping(&fx.manager).await; + + fx.manager + .report_activity(id, ActivityState::Error, Some("first failure".into())) + .await + .unwrap(); + fx.manager + .report_activity( + id, + ActivityState::AwaitingInput, + Some("needs approval".into()), + ) + .await + .unwrap(); + + let s = observed(&fx.manager, id).await; + assert_eq!(s.activity, ActivityState::AwaitingInput); + assert_eq!(s.activity_detail.as_deref(), Some("needs approval")); +} + +#[tokio::test] +async fn detail_is_capped_server_side() { + let fx = fixture(Duration::from_secs(300)).await; + let id = spawn_sleeping(&fx.manager).await; + + let long = "x".repeat(10_000); + fx.manager + .report_activity(id, ActivityState::Error, Some(long)) + .await + .unwrap(); + + let s = observed(&fx.manager, id).await; + let detail = s.activity_detail.expect("detail present"); + assert_eq!( + detail.chars().count(), + maestro_daemon::MAX_REPORT_DETAIL_CHARS + ); +} + +#[tokio::test] +async fn reported_state_survives_watcher_ticks_until_the_ttl_then_inference_resumes() { + let fx = fixture(Duration::from_millis(150)).await; + let id = spawn_sleeping(&fx.manager).await; + fx.manager.start_liveness_watcher(Duration::from_millis(50)); + + fx.manager + .report_activity(id, ActivityState::Error, Some("agent died mid-run".into())) + .await + .unwrap(); + + // Several watcher ticks inside the TTL: nothing infers `Error`, so the + // report must hold above inference. + sleep(Duration::from_millis(80)).await; + let s = observed(&fx.manager, id).await; + assert_eq!(s.activity, ActivityState::Error); + assert_eq!(s.activity_detail.as_deref(), Some("agent died mid-run")); + + // After the TTL lapses, inference resumes and the detail goes with it. + common::wait_for_async(Duration::from_secs(5), || async { + let s = observed(&fx.manager, id).await; + s.activity != ActivityState::Error && s.activity_detail.is_none() + }) + .await; +} diff --git a/crates/maestro-daemon/tests/local_api.rs b/crates/maestro-daemon/tests/local_api.rs index c5d1ab7..88a712b 100644 --- a/crates/maestro-daemon/tests/local_api.rs +++ b/crates/maestro-daemon/tests/local_api.rs @@ -346,6 +346,109 @@ async fn delete_owned_session_kills_it() { assert!(!h.manager.has(session.id).await); } +/// Spawn a session through the manager and return its id. +async fn spawn_via_manager(h: &Harness) -> Uuid { + h.manager + .create(CreateSessionRequest { + cwd: "/tmp".into(), + command: "sleep 60".into(), + env: HashMap::new(), + metadata: HashMap::new(), + worktree: None, + }) + .await + .unwrap() + .id +} + +/// The agent activity endpoint on the wire (#105): a report lands in the +/// session snapshot with its detail. +#[tokio::test] +async fn post_activity_reports_error_with_detail() { + let h = spawn("naps-laptop").await; + let id = spawn_via_manager(&h).await; + + let resp = reqwest::Client::new() + .post(url(h.addr, &format!("/api/sessions/{id}/activity"))) + .json(&serde_json::json!({ + "state": "Error", + "detail": "cargo build failed" + })) + .send() + .await + .unwrap(); + assert_eq!(resp.status(), reqwest::StatusCode::NO_CONTENT); + + let sessions: Vec = reqwest::get(url(h.addr, "/api/sessions")) + .await + .unwrap() + .json() + .await + .unwrap(); + let s = sessions.iter().find(|s| s.id == id).unwrap(); + assert_eq!(s.activity, maestro_protocol::ActivityState::Error); + assert_eq!(s.activity_detail.as_deref(), Some("cargo build failed")); +} + +#[tokio::test] +async fn post_activity_to_unknown_session_returns_404() { + let h = spawn("naps-laptop").await; + let resp = reqwest::Client::new() + .post(url( + h.addr, + &format!("/api/sessions/{}/activity", Uuid::new_v4()), + )) + .json(&serde_json::json!({ "state": "Idle" })) + .send() + .await + .unwrap(); + assert_eq!(resp.status(), reqwest::StatusCode::NOT_FOUND); +} + +/// Unknown state values are rejected rather than coerced (#105). +#[tokio::test] +async fn post_activity_rejects_an_unknown_state_value() { + let h = spawn("naps-laptop").await; + let id = spawn_via_manager(&h).await; + + let resp = reqwest::Client::new() + .post(url(h.addr, &format!("/api/sessions/{id}/activity"))) + .json(&serde_json::json!({ "state": "Teleported" })) + .send() + .await + .unwrap(); + assert!(resp.status().is_client_error(), "got {}", resp.status()); +} + +#[tokio::test] +async fn post_activity_caps_an_overlong_detail_server_side() { + let h = spawn("naps-laptop").await; + let id = spawn_via_manager(&h).await; + + let resp = reqwest::Client::new() + .post(url(h.addr, &format!("/api/sessions/{id}/activity"))) + .json(&serde_json::json!({ + "state": "Error", + "detail": "x".repeat(10_000) + })) + .send() + .await + .unwrap(); + assert_eq!(resp.status(), reqwest::StatusCode::NO_CONTENT); + + let sessions: Vec = reqwest::get(url(h.addr, "/api/sessions")) + .await + .unwrap() + .json() + .await + .unwrap(); + let s = sessions.iter().find(|s| s.id == id).unwrap(); + assert_eq!( + s.activity_detail.as_ref().map(|d| d.chars().count()), + Some(maestro_daemon::MAX_REPORT_DETAIL_CHARS) + ); +} + /// The kill guard on the wire (#79): `DELETE /api/sessions/:id` answers 409 /// with the structured breakdown, and the session keeps running. #[tokio::test] diff --git a/crates/maestro-daemon/tests/session_decisions.rs b/crates/maestro-daemon/tests/session_decisions.rs index aae5475..dae0158 100644 --- a/crates/maestro-daemon/tests/session_decisions.rs +++ b/crates/maestro-daemon/tests/session_decisions.rs @@ -30,6 +30,24 @@ impl Fx { Self::with_sidecar_dir(|tmp| tmp.join("sidecars")) } + /// Variant whose manager knows the daemon's own base URL (#103), the way + /// the production runner wires it from `[daemon].bind_addr`. + fn with_daemon_url(url: &str) -> Self { + let fx = Self::new(); + fx.manager.set_daemon_url(url.to_string()); + fx + } + + /// The env map of the (single, expected) spawn call. + fn spawned_env(&self) -> BTreeMap { + let calls = self.fake.calls(); + let Some(Call::NewSessionPiped { env, .. }) = calls.first() else { + panic!("expected a spawn call, got {calls:?}"); + }; + assert_eq!(calls.len(), 1, "expected exactly one spawn: {calls:?}"); + env.clone() + } + fn with_sidecar_dir(pick: impl Fn(&std::path::Path) -> PathBuf) -> Self { let tmp = TempDir::new().expect("tempdir"); let fake = Arc::new(FakePanes::new()); @@ -261,16 +279,23 @@ async fn requested_env_reaches_tmux() { fx.manager.create(req).await.expect("create"); + let spawned = match fx.fake.calls().first() { + Some(Call::NewSessionPiped { env, .. }) => env.clone(), + other => panic!("expected a spawn call, got {other:?}"), + }; + let id = spawned + .get("MAESTRO_SESSION_ID") + .expect("spawn must export MAESTRO_SESSION_ID (#103)") + .clone(); + // The daemon-owned variable rides alongside whatever was requested. let want: BTreeMap = [ ("EMPTY".to_string(), String::new()), ("FOO".to_string(), "bar".to_string()), + ("MAESTRO_SESSION_ID".to_string(), id), ] .into_iter() .collect(); - match fx.fake.calls().first() { - Some(Call::NewSessionPiped { env, .. }) => assert_eq!(env, &want), - other => panic!("expected a spawn call, got {other:?}"), - } + assert_eq!(&spawned, &want); } #[tokio::test] @@ -338,6 +363,9 @@ async fn activity_pass_emits_updated_only_when_something_changed() { // Still inside `activity::ACTIVE_WINDOW` (3s) since the spawn, so the // change here is `foreground`, not the activity state. assert_eq!(updated.activity, ActivityState::Active); + // A foreground/cwd-only change is not a state transition (#104): the + // sidebar's duration must keep counting from the original stamp. + assert_eq!(updated.activity_since, session.activity_since); // Drain anything already queued, then confirm a steady pane stays quiet. // Stays well inside the 3s window, so no Active -> Idle transition either. @@ -349,6 +377,70 @@ async fn activity_pass_emits_updated_only_when_something_changed() { ); } +/// The sidebar's duration (#104) is time in the *current* state, so +/// `activity_since` is stamped exactly when the decided state differs from +/// the stored one — once per transition. Stamping it every tick would make +/// the duration permanently read zero. +#[tokio::test] +async fn activity_transition_stamps_activity_since_once_per_transition() { + let fx = Fx::new(); + let session = fx + .manager + .create(fx.request("sleep 300")) + .await + .expect("create"); + fx.fake.set_pane_info( + session.id, + PaneInfo { + // Nothing reads from this pid: `awaiting_input` is deterministically + // false, so past `ACTIVE_WINDOW` the session decides as Idle. + pane_pid: i32::MAX, + command: "sleep".into(), + tty: "/dev/null".into(), + current_path: String::new(), + }, + ); + let mut events = fx.manager.events(); + fx.manager.start_liveness_watcher(Duration::from_millis(25)); + + let idle = wait_for_activity(&fx.manager, session.id, ActivityState::Idle).await; + assert!( + idle.activity_since >= session.activity_since + chrono::Duration::seconds(2), + "the Active -> Idle transition must stamp a fresh activity_since \ + (spawn stamp {:?}, transition stamp {:?})", + session.activity_since, + idle.activity_since + ); + + // Steady Idle across many more ticks must not re-stamp — no further + // SessionUpdated at all, or the sidebar clock would keep resetting. + while events.try_recv().is_ok() {} + tokio::time::sleep(Duration::from_millis(300)).await; + assert!( + matches!(events.try_recv(), Err(TryRecvError::Empty)), + "a steady state must not re-emit (and re-stamp) on every tick" + ); +} + +async fn wait_for_activity( + manager: &SessionManager, + id: uuid::Uuid, + want: ActivityState, +) -> maestro_protocol::Session { + let deadline = tokio::time::Instant::now() + Duration::from_secs(6); + loop { + if tokio::time::Instant::now() >= deadline { + panic!("timed out waiting for {id} to become {want:?}"); + } + if let Some(s) = manager.snapshot().await.into_iter().find(|s| s.id == id) { + if s.activity == want { + return s; + } + } + tokio::time::sleep(Duration::from_millis(20)).await; + } +} + async fn wait_for_updated( events: &mut tokio::sync::broadcast::Receiver, ) -> maestro_protocol::Session { @@ -396,3 +488,70 @@ fn attaching_spawn_history_after_cloning_panics() { 7, )); } + +/// Every tmux-spawned session learns who it is (#103): its own UUID under +/// `MAESTRO_SESSION_ID`, so the reporting hook in #106 can name the session +/// it is running inside. +#[tokio::test] +async fn spawn_exports_maestro_session_id() { + let fx = Fx::new(); + let s = fx.manager.create(fx.request("bash")).await.expect("create"); + + let env = fx.spawned_env(); + assert_eq!( + env.get("MAESTRO_SESSION_ID").map(String::as_str), + Some(&s.id.to_string()[..]), + "MAESTRO_SESSION_ID must be the session's UUID, got {env:?}" + ); +} + +/// The hook also needs to know where to report to: `MAESTRO_URL` carries the +/// daemon's base URL derived from `[daemon].bind_addr` (#103). +#[tokio::test] +async fn spawn_exports_maestro_url_when_the_daemon_is_wired() { + let fx = Fx::with_daemon_url("http://127.0.0.1:7070"); + fx.manager.create(fx.request("bash")).await.expect("create"); + + let env = fx.spawned_env(); + assert_eq!( + env.get("MAESTRO_URL").map(String::as_str), + Some("http://127.0.0.1:7070"), + "MAESTRO_URL must be the daemon's base URL, got {env:?}" + ); +} + +/// Without a wired URL (tests, or a caller that never set one) only the +/// session id is injected — an empty or bogus `MAESTRO_URL` would send the +/// #106 hook's reports nowhere useful. +#[tokio::test] +async fn spawn_omits_maestro_url_when_not_wired() { + let fx = Fx::new(); + fx.manager.create(fx.request("bash")).await.expect("create"); + + let env = fx.spawned_env(); + assert!( + !env.contains_key("MAESTRO_URL"), + "MAESTRO_URL must be absent when no daemon URL is known, got {env:?}" + ); +} + +/// The daemon-owned variables win over anything the spawner sent: a client +/// cannot spoof another session's identity. +#[tokio::test] +async fn spawn_overrides_client_supplied_maestro_variables() { + let fx = Fx::with_daemon_url("http://127.0.0.1:7070"); + let mut req = fx.request("bash"); + req.env.insert( + "MAESTRO_SESSION_ID".into(), + "00000000-0000-0000-0000-000000000000".into(), + ); + req.env.insert("MAESTRO_URL".into(), "http://evil:1".into()); + let s = fx.manager.create(req).await.expect("create"); + + let env = fx.spawned_env(); + assert_eq!(env.get("MAESTRO_SESSION_ID"), Some(&s.id.to_string())); + assert_eq!( + env.get("MAESTRO_URL"), + Some(&"http://127.0.0.1:7070".to_string()) + ); +} diff --git a/crates/maestro-daemon/tests/session_manager.rs b/crates/maestro-daemon/tests/session_manager.rs index 0e11df4..9bcc963 100644 --- a/crates/maestro-daemon/tests/session_manager.rs +++ b/crates/maestro-daemon/tests/session_manager.rs @@ -397,6 +397,12 @@ async fn watcher_refreshes_the_blocked_work_indicator() { init_bare_repo(origin.path()); push_all_to_origin(repo.path(), origin.path()); + // Subscribe before the watcher starts so every pass's emission is + // captured — otherwise the breakdown event can land between the + // snapshot observation below and the drain, and the quiet window + // would see it as an illegitimate re-emit. + let mut events = fx.manager.events(); + fx.manager.start_liveness_watcher_with_blocked_work( Duration::from_millis(50), Duration::from_millis(100), @@ -418,14 +424,38 @@ async fn watcher_refreshes_the_blocked_work_indicator() { }) .await; - // A steady state must not re-emit every tick (#83 change-detection). - let mut events = fx.manager.events(); + // Consume through the emission that carried the breakdown (#83 + // change-detection): the quiet window below must start after the last + // legitimate change, not race it into the channel. + loop { + match tokio::time::timeout(Duration::from_secs(5), events.recv()).await { + Ok(Ok(CatalogEvent::SessionUpdated { session })) + if matches!( + &session.blocked_work, + Some(b) if b.uncommitted_files >= 1 && b.untracked_files >= 1 + ) => + { + break; + } + Ok(Ok(_)) => continue, + _ => panic!("never saw the SessionUpdated carrying the breakdown"), + } + } while events.try_recv().is_ok() {} tokio::time::sleep(Duration::from_millis(400)).await; - assert!( - !matches!(events.try_recv(), Ok(CatalogEvent::SessionUpdated { .. })), - "an unchanged worktree must not re-emit SessionUpdated" - ); + // Only a *changed* indicator counts as a re-emit (#83 change-detection). + // The activity pass may still legitimately announce e.g. Active→Idle + // (its debounce is time-based, not worktree-based), and its payload + // carries today's blocked_work unchanged. + while let Ok(CatalogEvent::SessionUpdated { session }) = events.try_recv() { + assert!( + matches!( + &session.blocked_work, + Some(b) if b.uncommitted_files >= 1 && b.untracked_files >= 1 + ), + "an unchanged worktree must not re-emit a changed indicator" + ); + } } /// Cleanup on kill is unconditional once the guard passes (#79): even a spec @@ -769,6 +799,14 @@ async fn natural_exit_of_a_guarded_worktree_retains_it_in_the_store() { "a dirty worktree must be retained on natural exit" ); // ...in the store, under the exit reason. + // (`Dead` is observable a moment before the settle step's store write lands.) + wait_for_async(Duration::from_secs(3), || async { + fx.retained_store() + .load_all() + .map(|all| all.len() == 1) + .unwrap_or(false) + }) + .await; let all = fx.retained_store().load_all().expect("load_all"); assert_eq!(all.len(), 1); assert_eq!(all[0].path, wt_path); @@ -840,6 +878,17 @@ async fn natural_exit_of_a_pristine_worktree_removes_it() { }) .await; + // (`Dead` is observable a moment before the settle step removes the tree.) + wait_for_async(Duration::from_secs(3), || async { + !wt_path.exists() + && fx + .retained_store() + .load_all() + .map(|all| all.is_empty()) + .unwrap_or(false) + }) + .await; + assert!( !wt_path.exists(), "a pristine worktree leaves nothing behind" @@ -897,6 +946,14 @@ async fn natural_exit_without_a_retained_store_destroys_nothing() { }) .await; + // The settle step has been applied or refused either way — here refused, + // since there is no store to record the tree into. Only from this point + // is "the tree survived" a settled fact rather than a race (#115). + wait_for_async(Duration::from_secs(3), || async { + manager.is_settled(s.id).await == Some(true) + }) + .await; + assert!( wt_path.exists(), "nothing may be destroyed when it cannot be recorded" @@ -1004,3 +1061,44 @@ async fn worktree_base_repo_records_the_main_checkout_not_the_request() { ); assert_ne!(wt.base_repo, inner_worktree, "the worktree is not the repo"); } + +/// #103: every tmux-spawned session carries the daemon's identity variables in +/// its environment — its UUID under `MAESTRO_SESSION_ID` and the daemon's base +/// URL under `MAESTRO_URL` — so the in-session reporting hook (#106) can find +/// both without any out-of-band channel. +#[tokio::test] +async fn spawned_sessions_export_maestro_session_id_and_url() { + let fx = DaemonFixture::new(); + fx.manager + .set_daemon_url("http://127.0.0.1:7071".to_string()); + let cwd = TempDir::new().unwrap(); + + let s = fx + .manager + .create(CreateSessionRequest { + cwd: cwd.path().to_path_buf(), + command: "sleep 30".into(), + env: HashMap::new(), + metadata: HashMap::new(), + worktree: None, + }) + .await + .expect("create"); + + let env = fx + .tmux + .tmux + .show_environment(s.id) + .expect("show-environment"); + assert_eq!( + env.get("MAESTRO_SESSION_ID"), + Some(&s.id.to_string()), + "session env: {env:?}" + ); + assert_eq!( + env.get("MAESTRO_URL"), + Some(&"http://127.0.0.1:7071".to_string()) + ); + + fx.manager.kill(s.id, false).await.expect("kill"); +} diff --git a/crates/maestro-daemon/tests/sidecar.rs b/crates/maestro-daemon/tests/sidecar.rs index bf864ed..0274844 100644 --- a/crates/maestro-daemon/tests/sidecar.rs +++ b/crates/maestro-daemon/tests/sidecar.rs @@ -26,7 +26,9 @@ fn sample(id: Uuid) -> Session { .with_timezone(&Utc), foreground: None, activity: Default::default(), + activity_detail: None, last_activity_at: chrono::Utc::now(), + activity_since: chrono::Utc::now(), current_cwd: Some(PathBuf::from("/tmp/work/crates")), blocked_work: None, } diff --git a/crates/maestro-hooks/Cargo.toml b/crates/maestro-hooks/Cargo.toml new file mode 100644 index 0000000..1c6b004 --- /dev/null +++ b/crates/maestro-hooks/Cargo.toml @@ -0,0 +1,15 @@ +[package] +name = "maestro-hooks" +version.workspace = true +edition.workspace = true +license.workspace = true +repository.workspace = true +publish.workspace = true +description = "Agent activity hooks (#106): shipped script + idempotent settings.json installer" + +[dependencies] +serde_json.workspace = true +thiserror.workspace = true + +[dev-dependencies] +tempfile.workspace = true diff --git a/crates/maestro-hooks/src/lib.rs b/crates/maestro-hooks/src/lib.rs new file mode 100644 index 0000000..fcd4359 --- /dev/null +++ b/crates/maestro-hooks/src/lib.rs @@ -0,0 +1,261 @@ +//! Claude Code activity hook (#106). +//! +//! Two halves, both shared by the `maestro hooks install` CLI subcommand and +//! the daemon's first-start auto-install: +//! +//! * [`HOOK_SCRIPT`] — the hook itself, written to disk at install time. It +//! posts an [`ActivityState`](maestro-protocol) to +//! `$MAESTRO_URL/api/sessions/$MAESTRO_SESSION_ID/activity` (#105). The same +//! global hook runs in every Claude Code session on the machine, so it exits +//! 0 and silently when either #103 env var is unset, and it never lets a +//! daemon failure escape (short curl timeout, status ignored). +//! * [`install`] — idempotent registration of that script in +//! `~/.claude/settings.json`. Only entries maestro itself wrote (identified +//! by [`HOOK_MARKER`]) are touched; every other hook and every other field +//! in the file is preserved verbatim. + +use std::path::Path; + +/// Marker substring every maestro-installed hook command carries. The +/// installer rewrites or removes exactly these entries — nothing else in the +/// user's settings file. +pub const HOOK_MARKER: &str = "maestro-claude-hook"; + +/// File name of the installed script (carries [`HOOK_MARKER`]). +pub const SCRIPT_FILE_NAME: &str = "maestro-claude-hook.sh"; + +/// Claude Code hook event → `ActivityState` spelling posted to the daemon. +/// +/// `PreToolUse` is what makes a session `Active` *during* a long tool call — +/// `PostToolUse` alone only fires once it ends (#114). `StopFailure` is +/// Claude Code's own name for "the turn ended on an API error"; there is no +/// failure signal in `Stop` or `SessionEnd` payloads, so those never report +/// `Error`. +pub const HOOK_EVENTS: [(&str, &str); 5] = [ + // Tool starting: the agent is demonstrably producing work, even if the + // tool itself runs for minutes. + ("PreToolUse", "Active"), + // Tool finished successfully. + ("PostToolUse", "Active"), + // Permission / idle notices: Claude is blocked on the user. + ("Notification", "AwaitingInput"), + // Turn ended: waiting on the user again. + ("Stop", "AwaitingInput"), + // Turn ended on an API error; the payload's `error` field is forwarded + // as the report's `detail` (#114). + ("StopFailure", "Error"), +]; + +/// The shipped hook script. POSIX sh so any `/bin/sh` works. +pub const HOOK_SCRIPT: &str = r#"#!/bin/sh +# Installed by maestro (#106): report Claude Code activity to its daemon. +# Exits silently unless running inside a session maestro spawned (#103 env). +# Event -> ActivityState mapping: +# PreToolUse -> Active (tool started; agent is working) +# PostToolUse -> Active (tool finished) +# Notification -> AwaitingInput (permission / idle notice) +# Stop -> AwaitingInput (turn ended; waiting on the user) +# StopFailure -> Error (turn ended on an API error) +state="$1" +[ -n "$MAESTRO_SESSION_ID" ] || exit 0 +[ -n "$MAESTRO_URL" ] || exit 0 +payload=$(cat 2>/dev/null) || true +body="{\"state\":\"$state\"}" +# StopFailure payloads carry the outcome in "error". Only a plain enum +# spelling is reported as the detail — anything else (quotes, backslashes, +# prose) would risk breaking the JSON body, and the spelling already names +# the failure. +detail=$(printf '%s' "$payload" | sed -n 's/.*"error"[[:space:]]*:[[:space:]]*"\([a-z_]*\)".*/\1/p') +if [ -n "$detail" ]; then + body="{\"state\":\"$state\",\"detail\":\"$detail\"}" +fi +curl -fsS --connect-timeout 1 --max-time 2 \ + -H 'Content-Type: application/json' \ + -d "$body" \ + "$MAESTRO_URL/api/sessions/$MAESTRO_SESSION_ID/activity" \ + >/dev/null 2>&1 || true +exit 0 +"#; + +#[derive(Debug, thiserror::Error)] +pub enum HooksError { + #[error("io: {0}")] + Io(#[from] std::io::Error), + #[error("json: {0}")] + Json(#[from] serde_json::Error), +} + +/// Where the script is written for a given home directory: +/// `/.local/share/maestro/maestro-claude-hook.sh`. +pub fn script_path(home: &Path) -> std::path::PathBuf { + home.join(".local/share/maestro").join(SCRIPT_FILE_NAME) +} + +/// The user-level Claude Code settings file for a given home directory. +pub fn settings_path(home: &Path) -> std::path::PathBuf { + home.join(".claude/settings.json") +} + +/// Whether `settings_path` already carries a maestro hook (#106). +pub fn is_installed(settings_path: &Path) -> bool { + settings_json(settings_path) + .map(|root| find_maestro_commands(&root).next().is_some()) + .unwrap_or(false) +} + +/// Write [`HOOK_SCRIPT`] to `path`, executable. Parent directories are +/// created as needed — this is always under the caller's chosen data dir, +/// never inside a repository. +pub fn write_script(path: &Path) -> Result<(), HooksError> { + if let Some(dir) = path.parent() { + std::fs::create_dir_all(dir)?; + } + std::fs::write(path, HOOK_SCRIPT)?; + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let mut perms = std::fs::metadata(path)?.permissions(); + perms.set_mode(0o755); + std::fs::set_permissions(path, perms)?; + } + Ok(()) +} + +/// Install the full hook setup for `home`: write the script under +/// `.local/share/maestro/` and register it in `.claude/settings.json`. +/// Idempotent; returns whether anything changed on disk. +pub fn install_for_home(home: &Path) -> Result { + let script = script_path(home); + let settings = settings_path(home); + let before = std::fs::read_to_string(&settings).unwrap_or_default(); + write_script(&script)?; + install(&settings, &script)?; + let after = std::fs::read_to_string(&settings).unwrap_or_default(); + Ok(before != after) +} + +/// Register the hook command for every event in [`HOOK_EVENTS`] in the +/// settings file at `settings_path`. Idempotent: existing maestro entries are +/// updated in place wherever they sit, stale ones removed, duplicates folded; +/// every non-maestro entry and every unrelated top-level field survives. +/// +/// Never inspects or writes anywhere but `settings_path` — callers pass the +/// user's real home or a temp one in tests, never a repository. +pub fn install(settings_path: &Path, script_path: &Path) -> Result<(), HooksError> { + let mut root = settings_json(settings_path)?; + + // Strip every maestro entry wherever it currently sits, so an older + // install under different events is replaced rather than duplicated. + strip_maestro_entries(&mut root); + + let hooks = ensure_object(&mut root, "hooks"); + for (event, state) in HOOK_EVENTS { + let groups = ensure_array(hooks, event); + let mut group = serde_json::Map::new(); + group.insert( + "hooks".into(), + serde_json::json!([{ + "type": "command", + "command": format!("{} {state}", script_path.display()), + }]), + ); + groups.push(serde_json::Value::Object(group)); + } + + write_atomic(settings_path, &root) +} + +fn settings_json(path: &Path) -> Result { + match std::fs::read_to_string(path) { + Ok(s) => Ok(serde_json::from_str(&s)?), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => { + Ok(serde_json::Value::Object(Default::default())) + } + Err(e) => Err(e.into()), + } +} + +/// Every command string in the file belonging to a maestro hook. +fn find_maestro_commands(root: &serde_json::Value) -> impl Iterator { + root.get("hooks") + .and_then(|h| h.as_object()) + .into_iter() + .flat_map(|events| events.values()) + .filter_map(|groups| groups.as_array()) + .flatten() + .filter_map(|group| group.get("hooks")) + .filter_map(|inner| inner.as_array()) + .flatten() + .filter_map(|hook| hook.get("command")) + .filter_map(|cmd| cmd.as_str()) + .filter(|cmd| cmd.contains(HOOK_MARKER)) +} + +/// Remove all maestro inner-hook entries from every event group, dropping +/// groups left empty. Non-maestro siblings survive untouched. +fn strip_maestro_entries(root: &mut serde_json::Value) { + let Some(events) = root.get_mut("hooks").and_then(|h| h.as_object_mut()) else { + return; + }; + for (_, groups) in events.iter_mut() { + let Some(list) = groups.as_array_mut() else { + continue; + }; + for group in list.iter_mut() { + let Some(inner) = group.get_mut("hooks").and_then(|h| h.as_array_mut()) else { + continue; + }; + inner.retain(|hook| { + !hook + .get("command") + .and_then(|c| c.as_str()) + .is_some_and(|c| c.contains(HOOK_MARKER)) + }); + } + list.retain(|group| { + group + .get("hooks") + .and_then(|h| h.as_array()) + .is_some_and(|h| !h.is_empty()) + }); + } + // An event left with no groups at all (ours emptied it, or the user's + // file carried an empty list) reads better gone. + events.retain(|_, groups| groups.as_array().is_some_and(|g| !g.is_empty())); +} + +fn ensure_object<'a>( + root: &'a mut serde_json::Value, + key: &str, +) -> &'a mut serde_json::Map { + if !root.get(key).map(|v| v.is_object()).unwrap_or(false) { + root[key] = serde_json::json!({}); + } + root.get_mut(key).unwrap().as_object_mut().unwrap() +} + +fn ensure_array<'a>( + parent: &'a mut serde_json::Map, + key: &str, +) -> &'a mut Vec { + let v = parent + .entry(key.to_string()) + .or_insert_with(|| serde_json::json!([])); + if !v.is_array() { + *v = serde_json::json!([]); + } + v.as_array_mut().unwrap() +} + +fn write_atomic(path: &Path, value: &serde_json::Value) -> Result<(), HooksError> { + if let Some(dir) = path.parent() { + std::fs::create_dir_all(dir)?; + } + let tmp = path.with_extension("json.maestro-tmp"); + std::fs::write(&tmp, serde_json::to_string_pretty(value)? + "\n")?; + std::fs::rename(&tmp, path)?; + Ok(()) +} + +#[cfg(test)] +mod tests; diff --git a/crates/maestro-hooks/src/tests.rs b/crates/maestro-hooks/src/tests.rs new file mode 100644 index 0000000..6f73169 --- /dev/null +++ b/crates/maestro-hooks/src/tests.rs @@ -0,0 +1,340 @@ +//! Tests for the shipped hook script (run as a real `/bin/sh` subprocess) +//! and the settings-file installer. Everything happens under a temp `$HOME`; +//! the developer's own `~/.claude/settings.json` is never touched. + +use std::io::{Read, Write}; +use std::process::{Command, Stdio}; + +use super::*; + +fn sh(script: &str, args: &[&str], envs: &[(&str, &str)], payload: &[u8]) -> std::process::Output { + let mut cmd = Command::new("/bin/sh"); + // `sh -c CMD NAME ARGS…`: NAME fills $0, so the real arguments start at $1. + let mut argv = vec!["-c", script, HOOK_MARKER]; + argv.extend(args.iter().copied()); + cmd.args(&argv); + // A clean env: only what the test passes, plus PATH so curl can spawn. + cmd.env_clear(); + cmd.env("PATH", std::env::var("PATH").unwrap_or_default()); + for (k, v) in envs { + cmd.env(k, v); + } + let mut child = cmd + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .expect("spawn /bin/sh"); + // Feed a Claude-Code-style payload on stdin; the script must drain it. + child.stdin.take().unwrap().write_all(payload).ok(); + child.wait_with_output().expect("wait") +} + +const SESSION: &str = "01234567-89ab-cdef-0123-456789abcdef"; + +/// Accept exactly one HTTP request on a throwaway listener, answer 204, and +/// hand back the raw request text. Runs `run` with the base URL once the +/// acceptor thread is in place. +fn capture_request(run: impl FnOnce(String) -> std::process::Output) -> String { + let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + let addr = listener.local_addr().unwrap(); + let handle = std::thread::spawn(move || { + let (mut sock, _) = listener.accept().unwrap(); + let mut buf = [0u8; 4096]; + let n = sock.read(&mut buf).unwrap(); + sock.write_all(b"HTTP/1.1 204 No Content\r\nContent-Length: 0\r\n\r\n") + .unwrap(); + String::from_utf8_lossy(&buf[..n]).to_string() + }); + let out = run(format!("http://{addr}")); + assert!(out.status.success(), "{:?}", out.stderr); + handle.join().unwrap() +} + +fn session_envs(url: &str) -> Vec<(&'static str, String)> { + vec![ + ("MAESTRO_URL", url.to_string()), + ("MAESTRO_SESSION_ID", SESSION.to_string()), + ] +} + +fn owned_envs<'a>(envs: &'a [(&'static str, String)]) -> Vec<(&'static str, &'a str)> { + envs.iter().map(|(k, v)| (*k, v.as_str())).collect() +} + +/// The hook must be invisible outside maestro: no env vars (#103 unset) ⇒ +/// exit 0, no output, stdin drained. +#[test] +fn script_exits_silently_without_maestro_env() { + for (event, _) in HOOK_EVENTS { + let out = sh(HOOK_SCRIPT, &[event], &[], br#"{"session_id":"x"}"#); + assert!(out.status.success(), "{event:?}: {:?}", out.stderr); + assert!(out.stdout.is_empty(), "{event:?}: {:?}", out.stdout); + assert!(out.stderr.is_empty(), "{event:?}: {:?}", out.stderr); + } +} + +/// With both env vars set the script POSTs the mapped state to the daemon's +/// activity endpoint and still exits 0. +#[test] +fn script_posts_mapped_state_to_the_activity_endpoint() { + // The installer bakes the mapped state into the command's argv; the + // script relays whatever it is given. + let request = capture_request(|url| { + sh( + HOOK_SCRIPT, + &["Active"], + &owned_envs(&session_envs(&url)), + br#"{"session_id":"x"}"#, + ) + }); + assert!(request.starts_with("POST "), "{request}"); + assert!( + request.contains(&format!("/api/sessions/{SESSION}/activity")), + "{request}" + ); + assert!(request.contains(r#""state":"Active""#), "{request}"); +} + +/// Every event maps to the state the contract specifies. +#[test] +fn event_state_mapping_matches_the_contract() { + let map: std::collections::HashMap<&str, &str> = HOOK_EVENTS.into_iter().collect(); + assert_eq!(map["PreToolUse"], "Active"); + assert_eq!(map["PostToolUse"], "Active"); + assert_eq!(map["Notification"], "AwaitingInput"); + assert_eq!(map["Stop"], "AwaitingInput"); + assert_eq!(map["StopFailure"], "Error"); +} + +/// A `StopFailure` payload names the API error that ended the turn; the +/// script reports its `error` spelling as the report's `detail` so a card +/// can show why (#114). +#[test] +fn script_reports_the_error_spelling_as_detail() { + let request = capture_request(|url| { + sh( + HOOK_SCRIPT, + &["Error"], + &owned_envs(&session_envs(&url)), + br#"{"session_id":"abc","hook_event_name":"StopFailure","error":"rate_limit","error_details":"429 Too Many Requests"}"#, + ) + }); + assert!(request.contains(r#""state":"Error""#), "{request}"); + assert!(request.contains(r#""detail":"rate_limit""#), "{request}"); +} + +/// An `error` value that is not a plain enum spelling (or a payload without +/// one at all) must never produce a broken body — no detail is posted. +#[test] +fn script_posts_no_detail_for_an_unexpected_error_value() { + let request = capture_request(|url| { + sh( + HOOK_SCRIPT, + &["Error"], + &owned_envs(&session_envs(&url)), + br#"{"hook_event_name":"StopFailure","error":"429 \"Too Many\" \\ requests"}"#, + ) + }); + assert!(request.contains(r#""state":"Error""#), "{request}"); + assert!(!request.contains("detail"), "{request}"); +} + +/// Events whose payload carries no error fields post a bare body — never an +/// empty or stale detail. +#[test] +fn script_posts_no_detail_for_events_without_an_error() { + let request = capture_request(|url| { + sh( + HOOK_SCRIPT, + &["AwaitingInput"], + &owned_envs(&session_envs(&url)), + br#"{"hook_event_name":"Stop","stop_hook_active":false}"#, + ) + }); + assert!(request.contains(r#""state":"AwaitingInput""#), "{request}"); + assert!(!request.contains("detail"), "{request}"); +} + +/// A dead or unreachable daemon must never fail — or slow down — a session. +#[test] +fn script_ignores_daemon_failure() { + // Port 1 on localhost is closed in practice; curl fails fast. + let out = sh( + HOOK_SCRIPT, + &["Stop"], + &[ + ("MAESTRO_URL", "http://127.0.0.1:1"), + ("MAESTRO_SESSION_ID", "00000000-0000-0000-0000-000000000000"), + ], + br#"{"session_id":"x"}"#, + ); + assert!(out.status.success()); +} + +// --- installer ------------------------------------------------------------- + +fn installed_commands(settings: &Path) -> Vec { + serde_json::from_str::(&std::fs::read_to_string(settings).unwrap()) + .unwrap() + .get("hooks") + .and_then(|h| h.as_object()) + .into_iter() + .flat_map(|events| events.values()) + .filter_map(|g| g.as_array()) + .flatten() + .filter_map(|g| g.get("hooks")) + .filter_map(|h| h.as_array()) + .flatten() + .filter_map(|h| h.get("command")) + .filter_map(|c| c.as_str()) + .map(str::to_string) + .collect() +} + +/// Fresh install registers exactly one command per event, all pointing at the +/// script with the right state argument. +#[test] +fn fresh_install_registers_every_event() { + let dir = tempfile::tempdir().unwrap(); + let home = dir.path(); + write_script(&script_path(home)).unwrap(); + install(&settings_path(home), &script_path(home)).unwrap(); + + let cmds = installed_commands(&settings_path(home)); + assert_eq!(cmds.len(), HOOK_EVENTS.len()); + let joined = cmds.join("\n"); + assert_eq!(joined.matches("Active").count(), 2); + assert!(joined.contains("Error")); + assert_eq!(joined.matches("AwaitingInput").count(), 2); + for cmd in &cmds { + assert!(cmd.contains(SCRIPT_FILE_NAME)); + } +} + +/// Running install twice changes nothing further — no duplicates. +#[test] +fn install_is_idempotent() { + let dir = tempfile::tempdir().unwrap(); + let home = dir.path(); + install(&settings_path(home), &script_path(home)).unwrap(); + let once = std::fs::read_to_string(settings_path(home)).unwrap(); + + install(&settings_path(home), &script_path(home)).unwrap(); + let twice = std::fs::read_to_string(settings_path(home)).unwrap(); + assert_eq!(once, twice); + assert_eq!( + installed_commands(&settings_path(home)).len(), + HOOK_EVENTS.len() + ); +} + +/// An older maestro entry under the same events is updated in place, not +/// duplicated next to the new one. +#[test] +fn install_updates_an_older_maestro_hook_in_place() { + let dir = tempfile::tempdir().unwrap(); + let settings = dir.path().join(".claude/settings.json"); + std::fs::create_dir_all(settings.parent().unwrap()).unwrap(); + let old_script = "/old/location/maestro-claude-hook.sh Active"; + std::fs::write( + &settings, + serde_json::json!({ + "model": "sonnet", + "hooks": { + "Stop": [{"hooks": [{"type": "command", "command": old_script}]}], + }, + }) + .to_string(), + ) + .unwrap(); + + let new_script = dir + .path() + .join(".local/share/maestro") + .join(SCRIPT_FILE_NAME); + install(&settings, &new_script).unwrap(); + + let cmds = installed_commands(&settings); + assert_eq!( + cmds.len(), + HOOK_EVENTS.len(), + "old entry replaced, not kept: {cmds:?}" + ); + assert!(!cmds.iter().any(|c| c == old_script)); + let root: serde_json::Value = + serde_json::from_str(&std::fs::read_to_string(&settings).unwrap()).unwrap(); + assert_eq!(root["model"], "sonnet", "unrelated fields survive"); +} + +/// A stale maestro entry under an event we no longer use is cleaned up too; +/// the user's own hooks are never touched. +#[test] +fn install_preserves_foreign_hooks_and_removes_stale_maestro_ones() { + let dir = tempfile::tempdir().unwrap(); + let settings = dir.path().join(".claude/settings.json"); + std::fs::create_dir_all(settings.parent().unwrap()).unwrap(); + std::fs::write( + &settings, + serde_json::json!({ + "hooks": { + "PreToolUse": [{"matcher": "Bash", "hooks": [ + {"type": "command", "command": "my-own-linter.sh"}, + ]}], + "UserPromptSubmit": [{"hooks": [ + {"type": "command", "command": "/old/maestro-claude-hook.sh Active"}, + ]}], + }, + }) + .to_string(), + ) + .unwrap(); + + let new_script = dir + .path() + .join(".local/share/maestro") + .join(SCRIPT_FILE_NAME); + install(&settings, &new_script).unwrap(); + + let root: serde_json::Value = + serde_json::from_str(&std::fs::read_to_string(&settings).unwrap()).unwrap(); + // Foreign hook untouched. + let pre = root["hooks"]["PreToolUse"].as_array().unwrap(); + assert_eq!(pre[0]["hooks"][0]["command"], "my-own-linter.sh"); + // Stale maestro event gone. + assert!(root["hooks"].get("UserPromptSubmit").is_none()); + // Our events present, nothing else maestro-related anywhere. + let cmds = installed_commands(&settings); + assert_eq!( + cmds.iter().filter(|c| c.contains(HOOK_MARKER)).count(), + HOOK_EVENTS.len(), + "{cmds:?}" + ); +} + +/// `is_installed` flips only once a maestro entry exists. +#[test] +fn is_installed_tracks_reality() { + let dir = tempfile::tempdir().unwrap(); + let home = dir.path(); + assert!(!is_installed(&settings_path(home))); + install_for_home(home).unwrap(); + assert!(is_installed(&settings_path(home))); +} + +/// The full home-level install leaves the script executable. +#[test] +fn install_for_home_writes_an_executable_script() { + let dir = tempfile::tempdir().unwrap(); + let home = dir.path(); + install_for_home(home).unwrap(); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let mode = std::fs::metadata(script_path(home)) + .unwrap() + .permissions() + .mode(); + assert_eq!(mode & 0o111, 0o111); + } +} diff --git a/crates/maestro-protocol/src/client.rs b/crates/maestro-protocol/src/client.rs index 61bcffd..f7d2dab 100644 --- a/crates/maestro-protocol/src/client.rs +++ b/crates/maestro-protocol/src/client.rs @@ -16,7 +16,7 @@ use uuid::Uuid; use crate::routes::{self, UrlError}; use crate::{ ApiError, BranchInfo, CreateSessionRequest, DaemonInfo, DirListing, ErrorInfo, GitRoot, - KillOutcome, Project, RecentDirs, RetainedWorktree, Session, + KillOutcome, Project, RecentDirs, ReportActivityRequest, RetainedWorktree, Session, }; /// Time budget for establishing a TCP connection to the daemon. @@ -134,6 +134,21 @@ impl DaemonClient { json(resp).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`. + pub async fn report_activity( + &self, + id: Uuid, + req: &ReportActivityRequest, + ) -> Result<(), ClientError> { + let resp = self + .send(Method::POST, &routes::session_activity(id), &[], Some(req)) + .await?; + check(resp).await?; + Ok(()) + } + pub async fn daemon_info(&self) -> Result { self.get(routes::INFO, &[]).await } diff --git a/crates/maestro-protocol/src/lib.rs b/crates/maestro-protocol/src/lib.rs index 66fb569..390c245 100644 --- a/crates/maestro-protocol/src/lib.rs +++ b/crates/maestro-protocol/src/lib.rs @@ -75,11 +75,24 @@ pub struct Session { /// byte stream. #[serde(default)] pub activity: ActivityState, + /// Short human-readable string an agent reported alongside its activity + /// (#105) — e.g. why it is in `Error`. Shown on a sidebar card's meta line + /// while the report is in force; cleared when the report lapses or is + /// replaced. `None` unless an agent reported one. + #[serde(default)] + pub activity_detail: Option, /// Wall-clock time of the session's most recent observed PTY output. /// Drives "idle for Ns" displays. Defaults to the Unix epoch on payloads /// from daemons that predate the field. #[serde(default = "epoch")] pub last_activity_at: DateTime, + /// Wall-clock time at which the session entered its current + /// [`ActivityState`] (#102) — how long it has been `Active`, `Idle` or + /// `AwaitingInput`. Distinct from [`Self::last_activity_at`], which only + /// tracks the last observed PTY output. Populated by #104; defaults to the + /// Unix epoch on payloads from daemons that predate the field. + #[serde(default = "epoch")] + pub activity_since: DateTime, /// The pane's *current* working directory (`#{pane_current_path}`) — distinct /// from [`Session::cwd`], the directory it was spawned in. The activity /// watcher refreshes it as the user `cd`s around; it drives the web UI's @@ -110,7 +123,12 @@ pub enum SessionStatus { /// Inferred session activity, orthogonal to [`SessionStatus`] (NAP-340). The /// daemon derives this from tmux pane state, PTY quiescence, and `/proc` /// inspection — never by parsing the terminal byte stream. -#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, Default)] +/// +/// Deserializes leniently (#110, SIDEBAR.md Precedence): an unknown variant +/// from a newer daemon becomes [`ActivityState::Idle`] instead of failing the +/// whole `Session`. Non-string values still error — only variant names are +/// forgiven. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Default)] pub enum ActivityState { /// Producing output, or the foreground process is busy. Active, @@ -119,6 +137,64 @@ pub enum ActivityState { Idle, /// Blocked on a `read()` from the pty — waiting on the user. AwaitingInput, + /// The agent running in the session reported a failure (#102). Never + /// inferred by the daemon; only set when an agent reports it (#105). + Error, +} + +impl<'de> Deserialize<'de> for ActivityState { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + let s = String::deserialize(deserializer)?; + Ok(match s.as_str() { + "Active" => Self::Active, + "AwaitingInput" => Self::AwaitingInput, + "Error" => Self::Error, + // Known or not, any other spelling degrades to the neutral default + _ => Self::Idle, + }) + } +} + +/// HTTP `POST /api/sessions/:id/activity` body (#105): an agent's report of +/// its own state. The daemon holds the reported state above its own inference +/// for a fixed TTL, then resumes inference. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct ReportActivityRequest { + /// Parsed strictly, unlike [`ActivityState`]'s own lenient `Deserialize` + /// (#110). Leniency exists so a client reading a *newer* daemon's snapshot + /// degrades an unrecognised state to `Idle` instead of dropping the whole + /// session. A report travelling the other way has no such excuse: an agent + /// posting a state this daemon does not know is a bug in the agent, and + /// silently recording it as `Idle` would hide that behind a 204. + #[serde(deserialize_with = "strict_activity_state")] + pub state: ActivityState, + /// Short human string shown next to the state (e.g. an error message). + /// The daemon caps its length server-side. + #[serde(default)] + pub detail: Option, +} + +/// Reject an unknown [`ActivityState`] spelling instead of degrading it to +/// `Idle`. Used only by [`ReportActivityRequest`]; see the note there. +fn strict_activity_state<'de, D>(deserializer: D) -> Result +where + D: serde::Deserializer<'de>, +{ + use serde::de::Error as _; + let s = String::deserialize(deserializer)?; + match s.as_str() { + "Active" => Ok(ActivityState::Active), + "Idle" => Ok(ActivityState::Idle), + "AwaitingInput" => Ok(ActivityState::AwaitingInput), + "Error" => Ok(ActivityState::Error), + other => Err(D::Error::unknown_variant( + other, + &["Active", "Idle", "AwaitingInput", "Error"], + )), + } } /// Optional worktree request attached to a `CreateSessionRequest`. @@ -534,9 +610,13 @@ mod tests { .with_timezone(&Utc), foreground: Some("claude".into()), activity: ActivityState::Active, + activity_detail: None, last_activity_at: DateTime::parse_from_rfc3339("2026-05-24T12:05:00Z") .unwrap() .with_timezone(&Utc), + activity_since: DateTime::parse_from_rfc3339("2026-05-24T12:04:30Z") + .unwrap() + .with_timezone(&Utc), current_cwd: Some(PathBuf::from("/home/naps62/projects/maestro/crates")), blocked_work: None, } @@ -588,6 +668,35 @@ mod tests { ); } + #[test] + fn activity_since_defaults_to_epoch_on_legacy_payload() { + // A daemon predating #102 omits `activity_since`; it must deserialize + // to the forward-compat default rather than failing, and a present + // value round-trips. + let json = serde_json::json!({ + "id": Uuid::from_u128(1), + "cwd": "/tmp", + "command": "bash", + "status": "Running", + "created_at": "2026-05-24T12:00:00Z" + }); + let s: Session = serde_json::from_value(json).unwrap(); + assert_eq!(s.activity_since, epoch()); + + let full = sample_session(); + assert_eq!(full, round_trip(&full)); + } + + // #102: nothing infers `Error`; only #105's agent-reported failure sets it. + #[test] + fn error_activity_state_serializes_pascal_case_and_idle_stays_default() { + assert_eq!( + serde_json::to_value(ActivityState::Error).unwrap(), + serde_json::json!("Error") + ); + assert_eq!(ActivityState::default(), ActivityState::Idle); + } + #[test] fn activity_state_serializes_pascal_case_and_defaults_idle() { assert_eq!( @@ -597,6 +706,78 @@ mod tests { assert_eq!(ActivityState::default(), ActivityState::Idle); } + // #110 / SIDEBAR.md Precedence: a future daemon may send an + // `ActivityState` variant this client does not know; it must degrade to + // the neutral default instead of failing the whole `Session`. + #[test] + fn unknown_activity_state_variant_deserializes_to_idle() { + assert_eq!( + serde_json::from_value::(serde_json::json!("Teleported")).unwrap(), + ActivityState::Idle + ); + + let mut future = serde_json::to_value(sample_session()).unwrap(); + future["activity"] = serde_json::json!("QuantumEntangled"); + let s: Session = serde_json::from_value(future).unwrap(); + assert_eq!(s.activity, ActivityState::Idle); + } + + #[test] + fn non_string_activity_payload_still_fails_deserialization() { + let json = serde_json::json!({ + "id": Uuid::from_u128(1), + "cwd": "/tmp", + "command": "bash", + "status": "Running", + "created_at": "2026-05-24T12:00:00Z", + "activity": 42, + }); + assert!(serde_json::from_value::(json).is_err()); + } + + #[test] + fn activity_detail_defaults_to_none_on_legacy_payload() { + // #105: a daemon predating the detail field omits it; it must + // deserialize to `None` rather than failing, and a present value + // round-trips. + let json = serde_json::json!({ + "id": Uuid::from_u128(1), + "cwd": "/tmp", + "command": "bash", + "status": "Running", + "created_at": "2026-05-24T12:00:00Z" + }); + let s: Session = serde_json::from_value(json).unwrap(); + assert_eq!(s.activity_detail, None); + + let mut reported = sample_session(); + reported.activity = ActivityState::Error; + reported.activity_detail = Some("build failed".into()); + assert_eq!(reported.clone(), round_trip(&reported)); + } + + #[test] + fn report_activity_request_defaults_detail_and_rejects_unknown_state() { + let bare: ReportActivityRequest = + serde_json::from_value(serde_json::json!({ "state": "Error" })).unwrap(); + assert_eq!(bare.state, ActivityState::Error); + assert_eq!(bare.detail, None); + + let full = ReportActivityRequest { + state: ActivityState::AwaitingInput, + detail: Some("waiting on permission".into()), + }; + assert_eq!(full, round_trip(&full)); + + // Unknown state values are rejected rather than coerced (#105). + assert!( + serde_json::from_value::(serde_json::json!({ + "state": "Teleported" + })) + .is_err() + ); + } + #[test] fn session_round_trips() { let s = sample_session(); diff --git a/crates/maestro-protocol/src/routes.rs b/crates/maestro-protocol/src/routes.rs index 7b3e024..29671b5 100644 --- a/crates/maestro-protocol/src/routes.rs +++ b/crates/maestro-protocol/src/routes.rs @@ -47,6 +47,11 @@ pub fn session(id: Uuid) -> String { format!("{SESSIONS}/{id}") } +/// `POST /api/sessions/:id/activity` (#105): an agent reports its own state. +pub fn session_activity(id: Uuid) -> String { + format!("{SESSIONS}/{id}/activity") +} + /// `DELETE /api/worktrees/:id` — reclaim one retained worktree (#80). pub fn worktree(id: Uuid) -> String { format!("{WORKTREES}/{id}") @@ -61,6 +66,7 @@ pub fn attach(id: Uuid) -> String { /// registers the same paths [`session`] and [`attach`] build. pub mod patterns { pub const SESSION: &str = "/api/sessions/:id"; + pub const SESSION_ACTIVITY: &str = "/api/sessions/:id/activity"; pub const ATTACH: &str = "/api/attach/:id"; pub const WORKTREE: &str = "/api/worktrees/:id"; } @@ -169,6 +175,10 @@ mod tests { session(id()), "/api/sessions/6f1a7c0e-0f5e-4a2b-9a3d-6b1c2d3e4f50" ); + assert_eq!( + session_activity(id()), + "/api/sessions/6f1a7c0e-0f5e-4a2b-9a3d-6b1c2d3e4f50/activity" + ); assert_eq!( attach(id()), "/api/attach/6f1a7c0e-0f5e-4a2b-9a3d-6b1c2d3e4f50" @@ -191,6 +201,10 @@ mod tests { patterns::SESSION.trim_end_matches(":id"), session(id()).trim_end_matches(&id().to_string()) ); + assert_eq!( + patterns::SESSION_ACTIVITY.trim_end_matches(":id/activity"), + session(id()).trim_end_matches(&id().to_string()) + ); assert_eq!( patterns::ATTACH.trim_end_matches(":id"), attach(id()).trim_end_matches(&id().to_string()) diff --git a/crates/maestro-testing/src/lib.rs b/crates/maestro-testing/src/lib.rs index 881fd32..8defccf 100644 --- a/crates/maestro-testing/src/lib.rs +++ b/crates/maestro-testing/src/lib.rs @@ -41,7 +41,9 @@ pub fn session(id: Uuid, cwd: &str, command: &str) -> Session { created_at: Utc::now(), foreground: None, activity: Default::default(), + activity_detail: None, last_activity_at: Utc::now(), + activity_since: chrono::Utc::now(), current_cwd: None, blocked_work: None, } diff --git a/crates/maestro-tui/src/app.rs b/crates/maestro-tui/src/app.rs index dff6470..245a628 100644 --- a/crates/maestro-tui/src/app.rs +++ b/crates/maestro-tui/src/app.rs @@ -1126,7 +1126,9 @@ mod tests { created_at: Utc::now(), foreground: None, activity: Default::default(), + activity_detail: None, last_activity_at: Utc::now(), + activity_since: chrono::Utc::now(), current_cwd: None, blocked_work: None, }], @@ -1643,7 +1645,9 @@ mod tests { created_at: Utc::now(), foreground: None, activity: Default::default(), + activity_detail: None, last_activity_at: chrono::Utc::now(), + activity_since: chrono::Utc::now(), current_cwd: None, blocked_work: None, diff --git a/crates/maestro-tui/src/catalog.rs b/crates/maestro-tui/src/catalog.rs index d2f539b..e3466f0 100644 --- a/crates/maestro-tui/src/catalog.rs +++ b/crates/maestro-tui/src/catalog.rs @@ -193,7 +193,9 @@ mod tests { created_at: Utc::now(), foreground: None, activity: Default::default(), + activity_detail: None, last_activity_at: chrono::Utc::now(), + activity_since: chrono::Utc::now(), current_cwd: None, blocked_work: None, } diff --git a/crates/maestro-tui/src/sidebar.rs b/crates/maestro-tui/src/sidebar.rs index 724eff0..0fe2dc3 100644 --- a/crates/maestro-tui/src/sidebar.rs +++ b/crates/maestro-tui/src/sidebar.rs @@ -1,10 +1,14 @@ -//! Sidebar widget — session list, navigation-only. +//! Sidebar widget — session cards per `docs/SIDEBAR.md`. //! -//! Status glyphs: ● attached / ○ live / ✗ dead. +//! Line 1: status glyph (● attached / ○ live / ✗ dead) + name. +//! Line 2: activity dot + duration in the state's colour, plus the +//! blocked-work and exited fragments when they apply. Absent entirely when +//! it has nothing to say. use std::collections::HashSet; -use maestro_protocol::{Session, SessionStatus}; +use chrono::Utc; +use maestro_protocol::{ActivityState, Session, SessionStatus}; use ratatui::layout::Rect; use ratatui::style::{Color, Modifier, Style}; use ratatui::text::{Line, Span}; @@ -12,6 +16,32 @@ use ratatui::widgets::{Block, Borders, Paragraph}; use ratatui::Frame; use uuid::Uuid; +/// Time-in-state alone earns a meta line only once the session has sat in +/// its current activity state this long (docs/SIDEBAR.md). +const META_LINE_AFTER_SECS: i64 = 30; +/// A payload predating #102 defaults `activity_since` to the Unix epoch. Per +/// docs/SIDEBAR.md such a session contributes no duration — the dot still +/// renders — so anything implausibly old counts as "never observed" rather +/// than a card announcing that the session has been idle for half a century. +/// Mirrors `PLAUSIBLE_EPOCH_MS` in `web/src/activity.ts`. +const PLAUSIBLE_SINCE_YEAR: i32 = 2010; +const INDENT: &str = " "; +const META_INDENT: &str = " "; +/// Spawn-similar and kill, shown on the highlighted card's top-right corner. +const ACTIONS: &str = "⧉ ✕"; + +/// Every card sits on this background so cards read as cards against the +/// panel; the highlighted card swaps it for the focus background instead +/// (`Modifier::REVERSED` made the card unreadable against a background). +const CARD_BG: Color = Color::Black; +fn card_base(highlighted: bool) -> Style { + if highlighted { + Style::default().fg(Color::Black).bg(Color::Cyan) + } else { + Style::default().bg(CARD_BG) + } +} + /// Renderable sidebar state. #[derive(Debug, Clone)] pub struct Sidebar { @@ -41,36 +71,185 @@ impl Sidebar { let inner = block.inner(area); f.render_widget(block, area); + let width = inner.width as usize; let mut lines: Vec = vec![Line::from(Span::styled( "sessions", Style::default().add_modifier(Modifier::BOLD), ))]; for s in &self.sessions { - let glyph = match (s.status, self.attached.contains(&s.id)) { - (SessionStatus::Dead, _) => "✗", - (SessionStatus::Running, true) => "●", - (SessionStatus::Running, false) => "○", - }; - let label = name_for(s); - // #83: a session whose worktree blocks a kill gets a warning - // glyph; the kill confirm names the specifics. - let blocked = s - .blocked_work - .filter(|b| !b.is_clear()) - .map(|_| " ⚠") - .unwrap_or_default(); - let style = if self.highlight == Some(s.id) { - Style::default().add_modifier(Modifier::REVERSED) - } else { - Style::default() - }; - lines.push(Line::from(vec![ - Span::raw(" "), - Span::styled(format!("{glyph} {label}{blocked}"), style), - ])); + let highlighted = self.highlight == Some(s.id); + let base = card_base(highlighted); + lines.push(Line::from(self.name_row(s, width))); + if let Some(meta) = self.meta_row(s, width) { + lines.push(Line::from(pad_to_width(meta, width, base))); + } } f.render_widget(Paragraph::new(lines), inner); } + + fn name_row(&self, s: &Session, width: usize) -> Vec> { + let glyph = match (s.status, self.attached.contains(&s.id)) { + (SessionStatus::Dead, _) => "✗", + (SessionStatus::Running, true) => "●", + (SessionStatus::Running, false) => "○", + }; + let highlighted = self.highlight == Some(s.id); + let base = card_base(highlighted); + // Line-1 actions are keyboard affordances shown on the highlighted + // row only; they reserve their width so the name truncates with `…` + // before it can ever reach them (docs/SIDEBAR.md). + let actions_width = if highlighted { + ACTIONS.chars().count() + } else { + 0 + }; + let usable = width.saturating_sub(actions_width); + let prefix = INDENT.chars().count() + 2; + // Keep one column of separation between a truncated name and the + // actions so the ellipsis never touches them. + let sep = if highlighted { 1 } else { 0 }; + let name_budget = usable.saturating_sub(prefix + sep); + + let mut spans: Vec> = vec![ + Span::styled(INDENT, base), + Span::styled(format!("{glyph} "), base), + ]; + spans.extend( + fit_to_width(vec![Span::raw(name_for(s))], name_budget) + .into_iter() + .map(|sp| Span::styled(sp.content, base.patch(sp.style))), + ); + let used: usize = spans.iter().map(|sp| sp.width()).sum(); + if let Some(fill) = usable.checked_sub(used).filter(|&n| n > 0) { + spans.push(Span::styled( + " ".repeat(fill), + Style::default().bg(base.bg.unwrap_or(Color::Reset)), + )); + } + if highlighted { + spans.push(Span::styled(ACTIONS, base)); + } + spans + } + + /// The adaptive second line: ` [ | exited]`. + fn meta_row(&self, s: &Session, width: usize) -> Option>> { + let now = Utc::now(); + let held = held_for(s, now); + let held_secs = held.map(|d| d.num_seconds()).unwrap_or(0); + // #83: a worktree that blocks a kill is summarised here rather than + // on line 1; the kill confirm names the specifics. + let blocked = s.blocked_work.filter(|b| !b.is_clear()); + let dead = s.status == SessionStatus::Dead; + let errored = s.activity == ActivityState::Error; + if (held.is_none() || held_secs < META_LINE_AFTER_SECS) + && blocked.is_none() + && !dead + && !errored + { + return None; + } + let base = card_base(self.highlight == Some(s.id)); + let dot_style = base.patch(Style::default().fg(activity_color(s.activity))); + let muted = Style::default().bg(base.bg.unwrap_or(Color::Reset)); + let mut spans = vec![ + Span::styled(META_INDENT, base), + Span::styled("●", dot_style), + ]; + if let Some(d) = held { + spans.push(Span::styled(" ", base)); + spans.push(Span::styled(format_state_duration(d), dot_style)); + } + // The trailing fragments are measured up front so a long detail + // truncates ahead of them instead of pushing them off the line + // (docs/SIDEBAR.md). + let mut tail: Vec> = Vec::new(); + if let Some(b) = blocked { + tail.push(Span::styled(format!(" ⚠ {b}"), muted)); + } + if dead { + tail.push(Span::styled(" exited", muted)); + } + if let Some(detail) = s.activity_detail.as_deref().filter(|d| !d.is_empty()) { + let head_w: usize = spans.iter().map(|sp| sp.width()).sum(); + let tail_w: usize = tail.iter().map(|sp| sp.width()).sum(); + let budget = width.saturating_sub(head_w + tail_w); + spans.extend(fit_to_width( + vec![Span::styled(format!(" {detail}"), muted)], + budget, + )); + } + spans.extend(tail); + Some(fit_to_width(spans, width)) + } +} + +/// ratatui colour for an [`ActivityState`], per docs/SIDEBAR.md. The state +/// word is never spelled out — this colour carries it. +fn activity_color(state: ActivityState) -> Color { + match state { + ActivityState::Active => Color::Green, + ActivityState::Idle => Color::DarkGray, + ActivityState::AwaitingInput => Color::Yellow, + ActivityState::Error => Color::Red, + } +} + +/// docs/SIDEBAR.md duration format — floored whole seconds / minutes / hours, +/// measured in the current activity state. No days, no rounding up. +/// How long the session has held its current activity state, or `None` when +/// the daemon never reported one (see [`PLAUSIBLE_SINCE_YEAR`]). +fn held_for(s: &Session, now: chrono::DateTime) -> Option { + use chrono::Datelike; + if s.activity_since.year() < PLAUSIBLE_SINCE_YEAR { + return None; + } + Some(now - s.activity_since) +} + +fn format_state_duration(elapsed: chrono::Duration) -> String { + let secs = elapsed.num_seconds().max(0); + match secs { + 0..=59 => format!("{secs}s"), + 60..=3599 => format!("{}m", secs / 60), + _ => format!("{}h", secs / 3600), + } +} + +/// Truncate a span list to `max` columns, ending it with `…` when cut. The +/// sidebar is narrow: card lines truncate, they never wrap. +fn fit_to_width(spans: Vec>, max: usize) -> Vec> { + let mut out: Vec> = Vec::with_capacity(spans.len()); + let mut used = 0usize; + for span in spans { + let w = span.width(); + if used + w <= max { + used += w; + out.push(span); + continue; + } + let budget = max.saturating_sub(used + 1); + let ellipsis = "…"; + if budget > 0 { + let text: String = span.content.chars().take(budget).collect(); + out.push(Span::styled(format!("{text}{ellipsis}"), span.style)); + } else if used < max { + out.push(Span::styled(ellipsis, span.style)); + } + return out; + } + out +} + +/// Pad a card row out to the full sidebar width so the card background +/// paints edge to edge rather than stopping at the last glyph. +fn pad_to_width(mut spans: Vec>, width: usize, base: Style) -> Vec> { + let used: usize = spans.iter().map(|sp| sp.width()).sum(); + if let Some(fill) = width.checked_sub(used).filter(|&n| n > 0) { + let pad_style = Style::default().bg(base.bg.unwrap_or(Color::Reset)); + spans.push(Span::styled(" ".repeat(fill), pad_style)); + } + spans } /// `metadata.name` if present, else the command line. @@ -85,8 +264,10 @@ fn name_for(s: &Session) -> String { #[cfg(test)] mod tests { use super::*; - use chrono::Utc; + use chrono::{DateTime, Duration}; + use maestro_protocol::InTheWay; use ratatui::backend::TestBackend; + use ratatui::buffer::Buffer; use ratatui::Terminal; use std::collections::HashMap; @@ -104,25 +285,44 @@ mod tests { created_at: Utc::now(), foreground: None, activity: Default::default(), - last_activity_at: chrono::Utc::now(), + activity_detail: None, + last_activity_at: Utc::now(), + activity_since: Utc::now(), current_cwd: None, blocked_work: None, } } - fn render_to_string(sidebar: &Sidebar, w: u16, h: u16) -> String { + fn draw(sidebar: &Sidebar, w: u16, h: u16) -> Buffer { let backend = TestBackend::new(w, h); let mut terminal = Terminal::new(backend).unwrap(); terminal.draw(|f| sidebar.render(f, f.area())).unwrap(); - let buf = terminal.backend().buffer().clone(); - let mut out = String::new(); - for y in 0..buf.area.height { - for x in 0..buf.area.width { - out.push_str(buf[(x, y)].symbol()); - } - out.push('\n'); - } - out + terminal.backend().buffer().clone() + } + + fn rows(buf: &Buffer) -> Vec { + (0..buf.area.height) + .map(|y| { + (0..buf.area.width) + .map(|x| buf[(x, y)].symbol()) + .collect::() + }) + .collect() + } + + fn render_to_string(buf: &Buffer) -> String { + rows(buf).join("\n") + } + + fn row_containing<'a>(rows: &'a [String], needle: &str) -> &'a str { + rows.iter() + .find(|l| l.contains(needle)) + .unwrap_or_else(|| panic!("no row containing {needle:?} in {rows:#?}")) + } + + /// Row text without the trailing space padding and right border. + fn content(row: &str) -> &str { + row.trim_end().trim_end_matches('│').trim_end() } #[test] @@ -132,7 +332,7 @@ mod tests { highlight: None, attached: Default::default(), }; - let rendered = render_to_string(&sb, 28, 12); + let rendered = render_to_string(&draw(&sb, 28, 12)); assert!( rendered.contains("claude"), "missing session name:\n{rendered}" @@ -151,11 +351,8 @@ mod tests { highlight: None, attached: [sid].into_iter().collect(), }; - let rendered = render_to_string(&sb, 28, 6); - let line = rendered - .lines() - .find(|l| l.contains("claude")) - .expect("claude row"); + let all = rows(&draw(&sb, 28, 6)); + let line = row_containing(&all, "claude"); assert!(line.contains('●'), "expected filled glyph in: {line:?}"); assert!( !line.contains('○'), @@ -172,21 +369,18 @@ mod tests { highlight: None, attached: Default::default(), }; - let rendered = render_to_string(&sb, 28, 6); - let line = rendered - .lines() - .find(|l| l.contains("claude")) - .expect("claude row"); + let buf = draw(&sb, 28, 6); + let all = rows(&buf); + let line = row_containing(&all, "claude"); assert!(line.contains('✗'), "expected dead glyph in: {line:?}"); } - /// #83: a session whose worktree blocks a kill carries the warning glyph; - /// a clear or unprobed one does not. + /// #83 via docs/SIDEBAR.md: blocked work lives on the meta line now, not + /// on the name row; a clear or unprobed worktree never warns. #[test] - fn blocked_session_gets_a_warning_glyph() { - use maestro_protocol::InTheWay; - + fn blocked_session_warns_on_its_meta_line() { let mut blocked = session_named(1, "claude"); + blocked.activity_since = Utc::now() - Duration::seconds(5); blocked.blocked_work = Some(InTheWay { unpushed_commits: 4, ..InTheWay::default() @@ -199,18 +393,278 @@ mod tests { highlight: None, attached: Default::default(), }; - let rendered = render_to_string(&sb, 40, 8); - let line = rendered - .lines() - .find(|l| l.contains("claude")) - .expect("blocked row"); - assert!(line.contains('⚠'), "expected warning glyph in: {line:?}"); + let all = rows(&draw(&sb, 40, 10)); + + let meta = all + .iter() + .find(|l| l.contains("⚠")) + .expect("blocked card must warn somewhere"); + assert!( + meta.contains("4 unpushed commits"), + "warning must carry InTheWay's summary: {meta:?}" + ); + let name_row = row_containing(&all, "claude"); + assert!( + !name_row.contains('⚠'), + "warning moved off the name row: {name_row:?}" + ); for name in ["bash", "vim"] { - let line = rendered - .lines() - .find(|l| l.contains(name)) - .expect("clear row"); - assert!(!line.contains('⚠'), "{name} must not warn: {line:?}"); + let idx = all.iter().position(|l| l.contains(name)).unwrap(); + assert!( + !all[idx + 1].contains('⚠'), + "{name} must not warn: {:?}", + all[idx + 1] + ); + } + } + + /// A freshly-active session stays one line — never an empty placeholder + /// row (docs/SIDEBAR.md). + #[test] + fn freshly_active_session_stays_one_line() { + let mut s = session_named(1, "claude"); + s.activity_since = Utc::now() - Duration::seconds(10); + let sb = Sidebar { + sessions: vec![s], + highlight: None, + attached: Default::default(), + }; + let all = rows(&draw(&sb, 28, 8)); + assert_eq!(content(&all[0]), "sessions"); + assert!(row_containing(&all, "claude").contains('○')); + assert!( + content(&all[2]).is_empty(), + "line 2 must be absent, got {:?}", + all[2] + ); + } + + /// Rule 1 of docs/SIDEBAR.md: held in state longer than 30s earns the + /// meta line, with the floored duration next to the dot. + #[test] + fn long_held_session_gets_a_meta_line_with_duration() { + let mut s = session_named(1, "claude"); + s.activity_since = Utc::now() - Duration::seconds(90); + let sb = Sidebar { + sessions: vec![s], + highlight: None, + attached: Default::default(), + }; + let all = rows(&draw(&sb, 28, 8)); + let idx = all.iter().position(|l| l.contains("claude")).unwrap(); + let meta = &all[idx + 1]; + assert!(meta.contains('●'), "meta line carries the dot: {meta:?}"); + assert!(meta.contains("1m"), "expected 1m in: {meta:?}"); + } + + #[test] + fn duration_format_floors_whole_units() { + let fmt = |secs| format_state_duration(Duration::seconds(secs)); + assert_eq!(fmt(0), "0s"); + assert_eq!(fmt(45), "45s"); + assert_eq!(fmt(59), "59s"); + assert_eq!(fmt(60), "1m"); + assert_eq!(fmt(240), "4m"); + assert_eq!(fmt(3599), "59m"); + assert_eq!(fmt(3600), "1h"); + assert_eq!(fmt(7200), "2h"); + } + + /// The dot and duration carry the activity state's colour; the state word + /// is never spelled out (docs/SIDEBAR.md colour table). + #[test] + fn meta_line_dot_uses_the_contract_colour_per_state() { + for (state, expected) in [ + (ActivityState::Active, Color::Green), + (ActivityState::Idle, Color::DarkGray), + (ActivityState::AwaitingInput, Color::Yellow), + (ActivityState::Error, Color::Red), + ] { + let mut s = session_named(1, "claude"); + s.activity = state; + s.activity_since = Utc::now() - Duration::seconds(120); + let sb = Sidebar { + sessions: vec![s], + highlight: None, + attached: Default::default(), + }; + let buf = draw(&sb, 28, 8); + let found = (0..buf.area.height) + .any(|y| (0..buf.area.width).any(|x| buf[(x, y)].style().fg == Some(expected))); + assert!(found, "expected some cell in fg {expected:?} for {state:?}"); + } + } + + /// Rule 3 of docs/SIDEBAR.md: a dead session always gets a meta line, + /// even freshly dead, with the muted `exited` fragment. + #[test] + fn dead_session_shows_exited_on_its_meta_line() { + let mut s = session_named(1, "claude"); + s.status = SessionStatus::Dead; + s.activity_since = Utc::now() - Duration::seconds(5); + let sb = Sidebar { + sessions: vec![s], + highlight: None, + attached: Default::default(), + }; + let all = rows(&draw(&sb, 28, 8)); + let idx = all.iter().position(|l| l.contains("claude")).unwrap(); + assert!( + all[idx + 1].contains("exited"), + "expected exited fragment in: {:?}", + all[idx + 1] + ); + } + + /// The sidebar is narrow: the meta line truncates with `…` and never + /// wraps onto a third row (docs/SIDEBAR.md). + #[test] + fn meta_line_truncates_instead_of_wrapping() { + let mut s = session_named(1, "claude"); + s.activity_since = Utc::now() - Duration::seconds(5); + s.blocked_work = Some(InTheWay { + uncommitted_files: 120, + ..InTheWay::default() + }); + let sb = Sidebar { + sessions: vec![s], + highlight: None, + attached: Default::default(), + }; + let all = rows(&draw(&sb, 28, 8)); + let idx = all.iter().position(|l| l.contains("claude")).unwrap(); + let meta = content(&all[idx + 1]); + assert!( + meta.trim_end().ends_with('…'), + "truncated meta line must end with …: {meta:?}" + ); + assert!( + content(&all[idx + 2]).trim().is_empty(), + "meta line must not wrap: {:?}", + all[idx + 2] + ); + } + + /// #112: the agent-reported detail sits right after the duration on the + /// meta line, in the muted style (docs/SIDEBAR.md). + #[test] + fn errored_session_shows_its_reported_detail_on_the_meta_line() { + let mut s = session_named(1, "claude"); + s.activity = ActivityState::Error; + s.activity_since = Utc::now() - Duration::seconds(90); + s.activity_detail = Some("cargo build failed".into()); + let sb = Sidebar { + sessions: vec![s], + highlight: None, + attached: Default::default(), + }; + let all = rows(&draw(&sb, 40, 8)); + let idx = all.iter().position(|l| l.contains("claude")).unwrap(); + let meta = &all[idx + 1]; + assert!(meta.contains('●'), "meta line carries the dot: {meta:?}"); + assert!( + meta.contains("1m"), + "duration precedes the detail: {meta:?}" + ); + assert!( + meta.contains("cargo build failed"), + "reported detail must render: {meta:?}" + ); + } + + /// A fresh Error report earns line 2 on its own (contract rule 4), with + /// the detail even though no duration has accrued. + #[test] + fn fresh_errored_session_earns_a_meta_line_for_its_detail() { + let mut s = session_named(1, "claude"); + s.activity = ActivityState::Error; + s.activity_since = Utc::now() - Duration::seconds(5); + s.activity_detail = Some("waiting for approval".into()); + let sb = Sidebar { + sessions: vec![s], + highlight: None, + attached: Default::default(), + }; + let all = rows(&draw(&sb, 40, 8)); + let idx = all.iter().position(|l| l.contains("claude")).unwrap(); + assert!( + all[idx + 1].contains("waiting for approval"), + "detail must render on a fresh report's own meta line: {:?}", + all[idx + 1] + ); + } + + /// #112 via docs/SIDEBAR.md: the detail truncates with `…` ahead of the + /// trailing fragments — a long detail never pushes blocked work or + /// `exited` off the line, and never wraps. + #[test] + fn long_detail_yields_to_blocked_work_and_exited() { + let mut s = session_named(1, "claude"); + s.status = SessionStatus::Dead; + s.activity_since = Utc::now() - Duration::seconds(5); + s.activity_detail = + Some("an absurdly long error message that cannot possibly fit ".repeat(3)); + s.blocked_work = Some(InTheWay { + unpushed_commits: 2, + ..InTheWay::default() + }); + let sb = Sidebar { + sessions: vec![s], + highlight: None, + attached: Default::default(), + }; + let all = rows(&draw(&sb, 40, 8)); + let idx = all.iter().position(|l| l.contains("claude")).unwrap(); + let meta = content(&all[idx + 1]); + assert!(meta.contains('…'), "truncated detail ends with …: {meta:?}"); + assert!( + meta.contains("2 unpushed commits"), + "blocked work stays on the line: {meta:?}" + ); + assert!( + meta.contains("exited"), + "exited fragment stays on the line: {meta:?}" + ); + assert!( + content(&all[idx + 2]).trim().is_empty(), + "meta line must not wrap: {:?}", + all[idx + 2] + ); + } + + /// docs/SIDEBAR.md: a payload predating #102 carries an epoch + /// `activity_since` and so contributes no duration — a card must not claim + /// the session has been idle since 1970. The dot still renders when some + /// other trigger puts line 2 on screen. + #[test] + fn epoch_activity_since_renders_no_duration() { + let mut legacy = session_named(1, "claude"); + legacy.activity_since = DateTime::from_timestamp(0, 0).unwrap(); + legacy.status = SessionStatus::Dead; + let sb = Sidebar { + sessions: vec![legacy], + highlight: None, + attached: Default::default(), + }; + let rendered = render_to_string(&draw(&sb, 40, 8)); + assert!( + rendered.contains("exited"), + "dead session must still show line 2:\n{rendered}" + ); + assert!( + !rendered.contains('h') || !rendered.contains("● "), + "no duration may follow the dot for an epoch stamp:\n{rendered}" + ); + for line in rendered.lines().filter(|l| l.contains('●')) { + assert!( + !line + .split('●') + .nth(1) + .unwrap_or("") + .trim_start() + .starts_with(char::is_numeric), + "duration rendered from an epoch stamp: {line:?}" + ); } } @@ -227,8 +681,9 @@ mod tests { created_at: Utc::now(), foreground: None, activity: Default::default(), - last_activity_at: chrono::Utc::now(), - + activity_detail: None, + last_activity_at: Utc::now(), + activity_since: Utc::now(), current_cwd: None, blocked_work: None, }; @@ -237,10 +692,150 @@ mod tests { highlight: None, attached: Default::default(), }; - let rendered = render_to_string(&sb, 28, 6); + let rendered = render_to_string(&draw(&sb, 28, 6)); assert!( rendered.contains("vim"), "missing fallback command name:\n{rendered}" ); } + + /// Reversed video makes a card unreadable against a background; cards + /// use explicit backgrounds instead (docs/SIDEBAR.md, #108). + #[test] + fn no_cell_is_ever_reversed() { + let mut highlighted = session_named(1, "claude"); + highlighted.activity_since = Utc::now() - Duration::seconds(120); + let sb = Sidebar { + sessions: vec![highlighted], + highlight: Some(Uuid::from_u128(1)), + attached: Default::default(), + }; + let buf = draw(&sb, 28, 8); + for y in 0..buf.area.height { + for x in 0..buf.area.width { + assert!( + !buf[(x, y)] + .style() + .add_modifier + .contains(Modifier::REVERSED), + "reversed video at ({x},{y})" + ); + } + } + } + + /// The highlighted card gets the focus background; every other card gets + /// a card background distinct from the panel's default. + #[test] + fn cards_use_explicit_backgrounds_with_a_distinct_focus_background() { + let one = session_named(1, "claude"); + let two = session_named(2, "bash"); + let sb = Sidebar { + sessions: vec![one, two], + highlight: Some(Uuid::from_u128(1)), + attached: Default::default(), + }; + let buf = draw(&sb, 28, 8); + let all = rows(&buf); + let claude_y = all.iter().position(|l| l.contains("claude")).unwrap(); + let bash_y = all.iter().position(|l| l.contains("bash")).unwrap(); + + assert_eq!( + buf[(0, claude_y as u16)].style().bg, + Some(Color::Cyan), + "focused card must sit on the focus background" + ); + assert_eq!( + buf[(0, claude_y as u16)].style().fg, + Some(Color::Black), + "focused card text must stay readable on cyan" + ); + assert_eq!( + buf[(0, bash_y as u16)].style().bg, + Some(Color::Black), + "unfocused cards must still have a card background" + ); + // The panel itself (header row) stays on the terminal default. + assert!( + matches!(buf[(0, 0)].style().bg, None | Some(Color::Reset)), + "panel must keep the terminal default background" + ); + } + + /// The card background paints the full card width, not just the glyphs. + #[test] + fn card_background_covers_the_full_card_width() { + let s = session_named(1, "claude"); + let sb = Sidebar { + sessions: vec![s], + highlight: None, + attached: Default::default(), + }; + let buf = draw(&sb, 28, 8); + let y = buf.content.iter().position(|c| c.symbol() == "c").unwrap() as u16 / buf.area.width; + for x in 0..27 { + assert_eq!( + buf[(x, y)].style().bg, + Some(Color::Black), + "gap in card background at col {x}" + ); + } + } + + /// Line 1's actions live on the highlighted row's top-right corner. + #[test] + fn highlighted_row_anchors_the_actions_top_right() { + let sb = Sidebar { + sessions: vec![session_named(1, "claude")], + highlight: Some(Uuid::from_u128(1)), + attached: Default::default(), + }; + let all = rows(&draw(&sb, 28, 8)); + let line = content(row_containing(&all, "claude")); + assert!( + line.ends_with("⧉ ✕"), + "actions must sit at the right edge: {line:?}" + ); + } + + #[test] + fn unhighlighted_rows_show_no_actions() { + let one = session_named(1, "claude"); + let two = session_named(2, "bash"); + let sb = Sidebar { + sessions: vec![one, two], + highlight: Some(Uuid::from_u128(1)), + attached: Default::default(), + }; + let all = rows(&draw(&sb, 28, 8)); + let line = row_containing(&all, "bash"); + assert!( + !line.contains('⧉') && !line.contains('✕'), + "actions are highlight-only: {line:?}" + ); + } + + /// The name truncates with `…` before it can ever reach the reserved + /// action width (docs/SIDEBAR.md — overlap is a defect). + #[test] + fn name_truncates_before_reaching_the_actions() { + let mut s = session_named(1, "claude"); + if let Some(serde_json::Value::String(name)) = s.metadata.get_mut("name") { + *name = "an-unreasonably-long-session-name".into(); + } + let sb = Sidebar { + sessions: vec![s], + highlight: Some(Uuid::from_u128(1)), + attached: Default::default(), + }; + let all = rows(&draw(&sb, 28, 8)); + let line = row_containing(&all, "an-unreasonably"); + let name_end = line.find('…').expect("name must truncate with …"); + let actions_start = line.find('⧉').expect("actions must still render"); + assert!(name_end < actions_start, "name overlaps actions: {line:?}"); + assert!( + line.split_at(actions_start).0.ends_with(' '), + "no gap before the actions: {line:?}" + ); + } } diff --git a/crates/maestro/src/main.rs b/crates/maestro/src/main.rs index 73eafe4..d571636 100644 --- a/crates/maestro/src/main.rs +++ b/crates/maestro/src/main.rs @@ -55,6 +55,9 @@ enum Command { /// Kill a session by ID. #[cfg(feature = "cli")] Kill(maestro_cli::cli::KillArgs), + /// Install or update the agent activity hook (#106). + #[cfg(feature = "cli")] + Hooks(maestro_cli::cli::HooksArgs), /// Per-host daemon: tmux wrapper + session API. #[cfg(feature = "daemon")] Daemon { @@ -123,6 +126,15 @@ async fn dispatch(cli: Cli) -> Result<(), Box> { ) .await? } + #[cfg(feature = "cli")] + Command::Hooks(args) => { + maestro_cli::dispatch( + maestro_cli::cli::Command::Hooks(args), + cli.config, + cli.daemon_url, + ) + .await? + } #[cfg(feature = "daemon")] Command::Daemon { cmd } => maestro_daemon::cli::dispatch(cli.config, cmd).await?, #[cfg(feature = "tui")] @@ -157,7 +169,11 @@ fn init_tracing(cli: &Cli) { fn is_cli_subcommand(command: &Command) -> bool { matches!( command, - Command::List(_) | Command::Spawn(_) | Command::Attach(_) | Command::Kill(_) + Command::List(_) + | Command::Spawn(_) + | Command::Attach(_) + | Command::Kill(_) + | Command::Hooks(_) ) } diff --git a/docs/SIDEBAR.md b/docs/SIDEBAR.md new file mode 100644 index 0000000..82b0522 --- /dev/null +++ b/docs/SIDEBAR.md @@ -0,0 +1,133 @@ +# SIDEBAR.md — the session card contract + +One rule for how a session is rendered as a card in the sidebar, so the web +sidebar (#107) and the TUI sidebar (#108) describe a session the same way. +Read this before changing either. Both clients implement exactly what is +written here; if it needs to change, change it here first. + +A card has **line 1** always and **line 2** only when line 2 has something to +say. + +## Line 1 — name + actions + +- Left: the session name — `metadata.name` if present, else the command line + (same fallback both clients already use). +- Right: two actions anchored to the top-right corner: **spawn similar** + (`⧉`) and **kill** (`✕`). In the TUI these are keyboard affordances shown on + the highlighted row; in the web they are buttons. The glyphs are shared so + muscle memory transfers. +- Actions reserve their fixed width; the name truncates with `…` before it can + ever reach them. **Overlap between the name and the actions is a defect** — + that overlap is what started this milestone. + +Line 1 carries no badges. The `⚠ blocked work` badge (#83) and the `exited` +badge move off this row into line 2. + +## Line 2 — meta line (adaptive) + +Rendered only when at least one of these holds: + +1. The session has been held in its current activity state longer than + **30 seconds** (`activity_since` from #102). A freshly-active session + stays one line. +2. Its worktree holds blocked work: `blocked_work` is `Some` and not clear + (`InTheWay::is_clear()`), rendered in `formatInTheWay`'s phrasing on web + and as `⚠ ` on the TUI. +3. It has exited: `status == Dead`. +4. An agent-reported state (#105) of `Error` is in force (see Precedence). + +Otherwise line 2 is absent — never an empty placeholder row. + +When present, line 2 reads left to right: + +``` + [] [ | exited] [] +``` + +`` is a single filled glyph (`●` on both clients) carrying the activity +state's colour, and `` follows in the same colour. There is no state +word and no second badge: the dot exists so the state is still legible when a +row is dimmed or the terminal renders the duration in the default fg. The dot +and duration are unconditional parts of line 2; the bracketed fragments appear +only when their trigger holds. A payload that predates #102 (no usable +`activity_since`) contributes no duration — the dot still renders. + +`` is an agent-reported string (#105) — e.g. why a session is in +`Error`. It sits right after the duration, in the muted style, and shows only +while an agent-reported state is in force (see Precedence): when the report +lapses at its TTL or is replaced, the detail goes with it. The detail +truncates with `…` ahead of the trailing fragments — a long detail never +pushes ``, `exited`, or `` off the line, and neither +client ever lets it wrap. + +`` is optional and muted: the session's command line, shown only when +the rendered name came from `metadata.name` and differs from the command (the +web has always surfaced this; the TUI may omit it). + +## Colour per activity state + +The state word is never spelled out — the colour carries it. + +| Activity state | Web token | ratatui colour | +| ---------------- | --------- | ----------------- | +| `Active` | `--live` | `Color::Green` | +| `Idle` | `--muted` | `Color::DarkGray` | +| `AwaitingInput` | `--warn` | `Color::Yellow` | +| `Error` | `--danger`| `Color::Red` | + +Web tokens resolve per theme automatically (`styles.css` defines dark, light, +and `prefers-color-scheme` variants for each). The TUI uses the plain ratatui +named colours above; do not introduce RGB values or a new palette module for +this. + +The duration text inherits its state's colour; the blocked-work, exited and +command fragments use the muted colour (`--faint` / default style), since the +⚠ glyph already signals severity. + +## Duration format + +Time spent in the **current activity state**, sourced from `activity_since` +(#102) — *not* time since spawn, and *not* time since last PTY output +(`last_activity_at`). Both clients compute it identically: + +- `< 60s` → whole seconds, floored: `30s` +- `< 60m` → whole minutes, floored: `4m` +- `≥ 60m` → whole hours, floored: `2h` + +No days unit, no seconds inside minutes, no rounding up. Floor everywhere so +the counter never shows a duration the state hasn't actually reached yet. +Clock granularity: re-render at least once every 30s while visible so the +cutover points land within one tick. + +## Precedence + +An agent-reported `ActivityState` (#105) overrides the daemon's inference for +**five minutes** (the TTL is fixed by the daemon; re-reporting extends it). +When the TTL expires, inference resumes and the session's reported detail — +if any — clears with the report. Clients never blend the two: whichever source +is in force supplies both the colour and the clock for `activity_since`. + +Two rulings specific to a reported `Error` (#114): + +- **The five-minute TTL applies to `Error` like to any other state.** A + session that failed is still failed after five minutes, but a card pinned + red forever lies once the session moves on; when the report lapses the card + returns to the daemon's own inference — usually `Idle` for a session that + has gone quiet. +- **Only a new report clears an `Error` before its TTL.** The next + `ActivityState` of any kind replaces it (with #106's hook that is typically + `PreToolUse` → `Active` when the user prompts again). PTY output and + process changes do not clear it — only the agent itself may withdraw what + the agent itself reported. Re-reporting `Error` extends the TTL rather than + stacking reports. + +If a client receives an `ActivityState` variant it does not know, it must not +fail rendering: treat it as `Idle` (neutral/dim, line-2 rules apply as usual) +until the daemon sends a variant it does know. Rust clients deserialize +activity leniently for the same reason. + +Leniency runs one way only. It covers a client *reading* a newer daemon, where +dropping the whole session would be worse than dimming one card. A report +arriving at `POST /api/sessions/:id/activity` is parsed strictly and an unknown +state is a 4xx: an agent posting a state the daemon does not know is a bug in +the agent, and answering 204 would hide it. diff --git a/web/src/activity.test.ts b/web/src/activity.test.ts new file mode 100644 index 0000000..206c97f --- /dev/null +++ b/web/src/activity.test.ts @@ -0,0 +1,101 @@ +import { describe, expect, it } from "vitest"; + +import { + activityColorSlug, + activityDurationLabel, + activityHeldMs, + showsMetaLine, +} from "./activity.js"; +import type { Session } from "./types.js"; + +describe("activityHeldMs", () => { + it("measures time spent in the current state", () => { + const now = Date.now(); + const since = new Date(now - 90_000).toISOString(); + expect(activityHeldMs(since, now)).toBe(90_000); + }); + + it("returns null for payloads that predate #102 (epoch default)", () => { + expect(activityHeldMs(undefined, Date.now())).toBeNull(); + expect(activityHeldMs(new Date(0).toISOString(), Date.now())).toBeNull(); + }); + + it("returns null for unparseable timestamps", () => { + expect(activityHeldMs("not-a-time", Date.now())).toBeNull(); + }); + + it("never goes negative", () => { + const now = Date.now(); + expect(activityHeldMs(new Date(now + 5_000).toISOString(), now)).toBe(0); + }); +}); + +describe("activityDurationLabel (#102)", () => { + it("floors whole seconds under a minute", () => { + expect(activityDurationLabel(30_400)).toBe("30s"); + expect(activityDurationLabel(59_900)).toBe("59s"); + }); + + it("floors whole minutes under an hour", () => { + expect(activityDurationLabel(60_000)).toBe("1m"); + expect(activityDurationLabel(4 * 60_000 + 59_999)).toBe("4m"); + }); + + it("floors whole hours from an hour up, with no days unit", () => { + expect(activityDurationLabel(60 * 60_000)).toBe("1h"); + expect(activityDurationLabel(2 * 3_600_000 + 59 * 60_000 + 59_999)).toBe("2h"); + expect(activityDurationLabel(26 * 3_600_000)).toBe("26h"); + }); +}); + +describe("activityColorSlug", () => { + it("maps each known state to its contract slug", () => { + expect(activityColorSlug("Active")).toBe("active"); + expect(activityColorSlug("Idle")).toBe("idle"); + expect(activityColorSlug("AwaitingInput")).toBe("awaiting-input"); + expect(activityColorSlug("Error")).toBe("error"); + }); + + it("treats an unknown variant as idle so rendering never fails", () => { + expect(activityColorSlug("Teleported")).toBe("idle"); + }); +}); + +function session(overrides: Partial): Session { + return { + id: "11111111-2222-3333-4444-555555555555", + cwd: "/home/naps/tea/maestro", + command: "claude", + env: {}, + metadata: {}, + status: "Running", + created_at: "2026-08-24T10:00:00Z", + activity: "Active", + ...overrides, + }; +} + +describe("showsMetaLine", () => { + const fresh = new Date(Date.now() - 10_000).toISOString(); + + it("stays hidden for a freshly-active session", () => { + expect(showsMetaLine(session({ activity_since: fresh }))).toBe(false); + }); + + it("appears once the state has been held past 30 seconds", () => { + const held = new Date(Date.now() - 31_000).toISOString(); + expect(showsMetaLine(session({ activity_since: held }))).toBe(true); + }); + + it("appears for blocked work, exited sessions and reported errors", () => { + expect( + showsMetaLine(session({ activity_since: fresh, blocked_work: { uncommitted_files: 1 } })), + ).toBe(true); + expect(showsMetaLine(session({ activity_since: fresh, status: "Dead" }))).toBe(true); + expect(showsMetaLine(session({ activity_since: fresh, activity: "Error" }))).toBe(true); + }); + + it("needs no activity timestamp when another trigger holds", () => { + expect(showsMetaLine(session({ activity_since: undefined, status: "Dead" }))).toBe(true); + }); +}); diff --git a/web/src/activity.ts b/web/src/activity.ts new file mode 100644 index 0000000..2d3aa83 --- /dev/null +++ b/web/src/activity.ts @@ -0,0 +1,56 @@ +// docs/SIDEBAR.md: the session card's meta line — activity colour mapping and +// the shared duration format. Both clients compute it identically; the TUI +// mirrors this in crates/maestro-tui. + +import type { ActivityState, IsoTimestamp, Session } from "./types.js"; +import { sessionBlockers } from "./types.js"; + +/** A state held shorter than this keeps the card to one line (contract rule 1). */ +const META_LINE_MIN_HOLD_MS = 30_000; + +// Payloads predating #102 default `activity_since` to the Unix epoch; anything +// implausibly old is treated as "never observed" rather than a huge duration. +const PLAUSIBLE_EPOCH_MS = Date.UTC(2010, 0, 1); + +export function activityHeldMs(since: IsoTimestamp | undefined, now: number): number | null { + if (!since) return null; + const t = Date.parse(since); + if (Number.isNaN(t) || t < PLAUSIBLE_EPOCH_MS) return null; + return Math.max(0, now - t); +} + +/** `<60s` → whole seconds, `<60m` → whole minutes, else whole hours. Floored + * everywhere so the counter never shows a duration the state hasn't reached. */ +export function activityDurationLabel(heldMs: number): string { + const s = Math.floor(heldMs / 1000); + if (s < 60) return `${s}s`; + const m = Math.floor(s / 60); + if (m < 60) return `${m}m`; + return `${Math.floor(m / 60)}h`; +} + +/** CSS hook for the state's colour (`styles.css` maps slugs to tokens). An + * unknown variant degrades to idle so rendering never fails (Precedence). */ +export function activityColorSlug(state: ActivityState | string): string { + switch (state) { + case "Active": + return "active"; + case "AwaitingInput": + return "awaiting-input"; + case "Error": + return "error"; + default: + return "idle"; + } +} + +/** Line 2 renders only when at least one contract trigger holds. */ +export function showsMetaLine(s: Session, now: number = Date.now()): boolean { + const held = activityHeldMs(s.activity_since, now); + return ( + (held !== null && held >= META_LINE_MIN_HOLD_MS) || + sessionBlockers(s) !== null || + s.status === "Dead" || + s.activity === "Error" + ); +} diff --git a/web/src/catalog.test.ts b/web/src/catalog.test.ts index b502fd7..560ee31 100644 --- a/web/src/catalog.test.ts +++ b/web/src/catalog.test.ts @@ -104,3 +104,154 @@ describe("blocked-work indicator (#83)", () => { expect(confirmations).toEqual(['Kill session "claude"?']); }); }); + +describe("agent-reported detail (#112)", () => { + const ago = (ms: number): string => new Date(Date.now() - ms).toISOString(); + + it("renders the reported detail right after the duration on line 2", () => { + const s = session({ + id: "aaaaaaaa-0000-0000-0000-000000000020", + status: "Dead", + activity: "Error", + activity_since: ago(90_000), + activity_detail: "cargo build failed", + }); + const el = renderSidebar(storeWith([s])).el; + const line2 = rowFor(el, s.id)?.querySelector(".card-line2"); + expect(line2?.querySelector(".activity-detail")?.textContent).toBe("cargo build failed"); + const order = [...line2!.children].map((c) => c.className); + expect(order.indexOf("activity-detail")).toBeGreaterThan(order.indexOf("activity-duration")); + expect(order.indexOf("activity-detail")).toBeLessThan( + order.findIndex((c) => c.startsWith("card-flag")), + ); + }); + + it("omits the detail when no report is in force", () => { + const s = session({ + id: "aaaaaaaa-0000-0000-0000-000000000021", + activity: "Active", + activity_since: ago(90_000), + }); + const el = renderSidebar(storeWith([s])).el; + expect(rowFor(el, s.id)?.querySelector(".activity-detail")).toBeNull(); + }); +}); + +describe("session cards (#107)", () => { + const ago = (ms: number): string => new Date(Date.now() - ms).toISOString(); + + it("keeps the name and the actions in the same flow line, actions reserving their width", () => { + const s = session({ id: "aaaaaaaa-0000-0000-0000-000000000010" }); + const el = renderSidebar(storeWith([s])).el; + const line1 = rowFor(el, s.id)?.querySelector(".card-line1"); + expect(line1).not.toBeNull(); + // Same flex line ⇒ the name truncates before it can ever reach the buttons. + expect(line1?.querySelector(".leaf-name")).not.toBeNull(); + expect(line1?.querySelector(".leaf-actions")).toBe(line1?.lastElementChild); + }); + + it("stays a single line while freshly active", () => { + const s = session({ + id: "aaaaaaaa-0000-0000-0000-000000000011", + activity: "Active", + activity_since: ago(5_000), + }); + const el = renderSidebar(storeWith([s])).el; + expect(rowFor(el, s.id)?.querySelector(".card-line2")).toBeNull(); + }); + + it("shows dot + floored duration once the state has been held past 30s", () => { + const s = session({ + id: "aaaaaaaa-0000-0000-0000-000000000012", + activity: "Active", + activity_since: ago(4 * 60_000 + 30_000), + }); + const el = renderSidebar(storeWith([s])).el; + const line2 = rowFor(el, s.id)?.querySelector(".card-line2"); + expect(line2?.dataset.activity).toBe("active"); + expect(line2?.querySelector(".activity-dot")).not.toBeNull(); + expect(line2?.querySelector(".activity-duration")?.textContent).toBe("4m"); + }); + + it("maps each activity state to its contract slug", () => { + const mk = (id: string, activity: string) => + session({ + id, + activity: activity as Session["activity"], + status: "Running", + activity_since: ago(90_000), + }); + const idle = mk("aaaaaaaa-0000-0000-0000-000000000013", "Idle"); + const wait = mk("aaaaaaaa-0000-0000-0000-000000000014", "AwaitingInput"); + const error = mk("aaaaaaaa-0000-0000-0000-000000000015", "Error"); + const el = renderSidebar(storeWith([idle, wait, error])).el; + expect(rowFor(el, idle.id)?.querySelector(".card-line2")?.getAttribute("data-activity")).toBe( + "idle", + ); + expect(rowFor(el, wait.id)?.querySelector(".card-line2")?.getAttribute("data-activity")).toBe( + "awaiting-input", + ); + expect(rowFor(el, error.id)?.querySelector(".card-line2")?.getAttribute("data-activity")).toBe( + "error", + ); + }); + + it("treats an unknown activity variant as idle instead of failing to render", () => { + const s = session({ + id: "aaaaaaaa-0000-0000-0000-000000000016", + activity: "Teleported" as Session["activity"], + activity_since: ago(120_000), + }); + const el = renderSidebar(storeWith([s])).el; + const line2 = rowFor(el, s.id)?.querySelector(".card-line2"); + expect(line2?.getAttribute("data-activity")).toBe("idle"); + expect(line2?.querySelector(".activity-duration")?.textContent).toBe("2m"); + }); + + it("moves blocked work onto line 2 in the shared phrasing", () => { + const s = session({ + id: "aaaaaaaa-0000-0000-0000-000000000017", + blocked_work: { uncommitted_files: 3, unpushed_commits: 2 }, + }); + const el = renderSidebar(storeWith([s])).el; + const line2 = rowFor(el, s.id)?.querySelector(".card-line2"); + expect(line2?.textContent).toContain("⚠"); + expect(line2?.textContent).toContain("3 uncommitted files, 2 unpushed commits"); + // Line 1 carries no badges. + expect(rowFor(el, s.id)?.querySelector(".leaf-badge")).toBeNull(); + }); + + it("shows exited on line 2 for dead sessions", () => { + const s = session({ + id: "aaaaaaaa-0000-0000-0000-000000000018", + status: "Dead", + }); + const el = renderSidebar(storeWith([s])).el; + const line2 = rowFor(el, s.id)?.querySelector(".card-line2"); + expect(line2?.textContent).toContain("exited"); + }); + + it("carries the state word only in tooltips and aria labels, never as text", () => { + const s = session({ + id: "aaaaaaaa-0000-0000-0000-000000000019", + command: "claude", + metadata: { name: "worker" }, + activity: "AwaitingInput", + activity_since: ago(90_000), + }); + const el = renderSidebar(storeWith([s])).el; + const row = rowFor(el, s.id) as HTMLElement; + const dot = row.querySelector(".activity-dot"); + expect(dot?.getAttribute("title")).toContain("Awaiting input"); + expect(dot?.getAttribute("aria-label")).toContain("Awaiting input"); + expect(row.textContent).not.toMatch(/awaiting.?input/i); + }); + + it("keeps the attach navigation and depth indentation", () => { + const s = session({ id: "aaaaaaaa-0000-0000-0000-00000000001a" }); + const el = renderSidebar(storeWith([s])).el; + const row = rowFor(el, s.id) as HTMLAnchorElement; + expect(row.getAttribute("href")).toBe("#/attach/aaaaaaaa-0000-0000-0000-00000000001a"); + expect(row.style.paddingLeft).toMatch(/^\d+(\.\d+)?px$/); + }); +}); diff --git a/web/src/catalog.ts b/web/src/catalog.ts index b9d4bf0..a4d517a 100644 --- a/web/src/catalog.ts +++ b/web/src/catalog.ts @@ -4,6 +4,7 @@ import { HUD_TOGGLE_EVENT, isHudEnabled, toggleHud } from "./debug.js"; import { CatalogStore } from "./events.js"; import type { Session } from "./types.js"; import { abbreviateFolderName, formatInTheWay, sessionBlockers, sessionName } from "./types.js"; +import { activityColorSlug, activityDurationLabel, activityHeldMs, showsMetaLine } from "./activity.js"; import type { TreeNode } from "./tree.js"; import { buildTree } from "./tree.js"; import { createThemeToggle } from "./theme.js"; @@ -17,6 +18,44 @@ export function requestOpenSpawn(): void { const collapsed = new Set(); +// docs/SIDEBAR.md: the state word is never rendered — the dot's colour carries +// it. It survives only in tooltips/aria labels so meaning is not colour-only. +const STATE_WORDS: Record = { + active: "Active", + idle: "Idle", + "awaiting-input": "Awaiting input", + error: "Error", +}; + +function metaLine(s: Session, now: number): string { + const name = sessionName(s); + const dead = s.status === "Dead"; + const blockers = sessionBlockers(s); + const slug = activityColorSlug(s.activity ?? "Idle"); + const stateWord = STATE_WORDS[slug] ?? STATE_WORDS.idle; + const held = activityHeldMs(s.activity_since, now); + const duration = held !== null ? activityDurationLabel(held) : null; + // The directory is the enclosing folder, so only surface the command — and + // only when it isn't already what the name shows. + const showCmd = s.command !== name; + + const fragments = [ + ``, + duration ? `${duration}` : "", + s.activity_detail + ? `${escapeHtml(s.activity_detail)}` + : "", + blockers + ? `⚠ ${escapeHtml(formatInTheWay(blockers))}` + : "", + dead ? `exited` : "", + showCmd + ? `${escapeHtml(s.command)}` + : "", + ]; + return `${fragments.join("")}`; +} + function renderNode(node: TreeNode, depth: number): HTMLElement { if (node.kind === "folder") { const wrap = document.createElement("div"); @@ -52,30 +91,34 @@ function renderNode(node: TreeNode, depth: number): HTMLElement { } const s = node.session; + const now = Date.now(); const row = document.createElement("a"); - row.className = "tree-leaf"; + row.className = "tree-leaf session-card"; row.href = `#/attach/${encodeURIComponent(s.id)}`; row.style.paddingLeft = `${depth * 16 + 8}px`; row.dataset.sessionId = s.id; const running = s.status === "Running"; row.dataset.status = running ? "running" : "dead"; const name = sessionName(s); - row.setAttribute("aria-label", `${name}, ${running ? "running" : "exited"}`); - // The directory is the enclosing folder, so only surface the command — and - // only when it isn't already what the name shows. - const showCmd = s.command !== name; - // #83: warn on the row itself, in `formatInTheWay`'s shared phrasing, so - // "this will be refused" is visible before the kill is attempted. - const blockers = sessionBlockers(s); + const stateWord = + STATE_WORDS[activityColorSlug(s.activity ?? "Idle")] ?? STATE_WORDS.idle; + row.setAttribute( + "aria-label", + `${name}, ${running ? "running" : "exited"}, ${stateWord}`, + ); + // docs/SIDEBAR.md line 1: name + top-right actions in the same flow line — + // the actions reserve their width, so the name truncates before it can ever + // reach them. Line 2 renders only when a contract trigger holds. + const showMeta = showsMetaLine(s, now); row.innerHTML = ` - ${escapeHtml(name)} - ${showCmd ? `${escapeHtml(s.command)}` : ""} - ${running ? "" : `exited`} - ${blockers ? `⚠ ${escapeHtml(formatInTheWay(blockers))}` : ""} - - - + + ${escapeHtml(name)} + + + + + ${showMeta ? metaLine(s, now) : ""} `; const similarBtn = row.querySelector(".leaf-clone") as HTMLButtonElement; similarBtn.addEventListener("click", (ev) => { @@ -395,6 +438,13 @@ export function renderSidebar(store: CatalogStore, opts: SidebarOptions = {}): S const unsubscribe = store.subscribe(refresh); + // Contract granularity: re-render at least every 30s while visible so the + // duration cutover points land within one tick. + const clock = setInterval(() => { + if (document.visibilityState !== "visible") return; + refresh(); + }, 30_000); + return { el, setActive(id: string | null) { @@ -410,6 +460,7 @@ export function renderSidebar(store: CatalogStore, opts: SidebarOptions = {}): S applyHighlights(); }, dispose() { + clearInterval(clock); document.removeEventListener("catalog-rerender", rerenderHandler); unsubscribe(); }, diff --git a/web/src/kit.ts b/web/src/kit.ts index f33759c..1d4386f 100644 --- a/web/src/kit.ts +++ b/web/src/kit.ts @@ -59,20 +59,28 @@ function leafRow(opts: { status: "running" | "dead"; active?: boolean; indent?: number; + activity?: "active" | "idle" | "awaiting-input" | "error"; + duration?: string; }): HTMLElement { - const row = h("a", "tree-leaf"); + const row = h("a", "tree-leaf session-card"); (row as HTMLAnchorElement).href = "#/kit"; row.dataset.status = opts.status; if (opts.active) row.classList.add("is-active"); row.style.paddingLeft = `${(opts.indent ?? 0) * 16 + 8}px`; + const flags = [ + opts.status === "dead" ? `exited` : "", + opts.cmd !== opts.name ? `${opts.cmd}` : "", + ].join(""); row.innerHTML = ` - ${opts.name} - ${opts.cmd !== opts.name ? `${opts.cmd}` : ""} - ${opts.status === "dead" ? `exited` : ""} - - - - `; + + ${opts.name} + + + + + + ${opts.activity ? `${opts.duration ?? ""}${flags}` : ""} + `; return row; } @@ -190,14 +198,17 @@ export function renderKit(): HTMLElement { // --- chips & badges --- { const { el, body } = section("Chips & badges", "Status-bearing pills and counts."); + const activeLine = h( + "span", + "card-line2", + `4m`, + ); + activeLine.setAttribute("data-activity", "active"); body.append( specimen("session-tally", h("span", "session-tally", "6 sessions · 5 live")), specimen("folder-count", h("span", "folder-count", "4")), - specimen("leaf-badge", h("span", "leaf-badge", "exited")), - specimen( - "attach-state · open", - h("span", "attach-state", "live"), - ), + specimen("card-line2 · active", activeLine), + specimen("attach-state · open", h("span", "attach-state", "live")), ); (body.lastElementChild?.querySelector(".attach-state") as HTMLElement)?.setAttribute( "data-state", @@ -215,19 +226,56 @@ export function renderKit(): HTMLElement { { const { el, body } = section( "Session rows — nested by host, then directory", - "The working directory (home collapsed to ~) is the nesting, and single-child chains fold into one node like ~/git/maestro. “Alive” is a faint row wash; exited rows recede with a badge; the open session is highlighted.", + "docs/SIDEBAR.md cards: line 1 is name + top-right actions (the name truncates before it can reach them); line 2 only when there's something to say — dot + duration in the state's colour, then blocked-work / exited fragments.", ); const list = h("div", "kit-tree"); list.append( folderHeader("~/git/maestro", 3, 0), folderHeader("web", 2, 1), leafRow({ name: "redesign", cmd: "claude", status: "running", indent: 2 }), - leafRow({ name: "dev-server", cmd: "vite dev", status: "running", indent: 2, active: true }), + leafRow({ + name: "dev-server", + cmd: "vite dev", + status: "running", + indent: 2, + active: true, + activity: "active", + duration: "4m", + }), folderHeader("crates/daemon", 1, 1), - leafRow({ name: "daemon-work", cmd: "claude", status: "running", indent: 2 }), + leafRow({ + name: "daemon-work", + cmd: "claude", + status: "running", + indent: 2, + activity: "awaiting-input", + duration: "12s", + }), + leafRow({ + name: "flaky-agent", + cmd: "claude", + status: "running", + indent: 2, + activity: "error", + duration: "2h", + }), folderHeader("~/work/acme", 2, 0), - leafRow({ name: "billing-api", cmd: "claude", status: "running", indent: 1 }), - leafRow({ name: "scratch", cmd: "bash", status: "dead", indent: 1 }), + leafRow({ + name: "billing-api", + cmd: "claude", + status: "running", + indent: 1, + activity: "idle", + duration: "26m", + }), + leafRow({ + name: "scratch", + cmd: "bash", + status: "dead", + indent: 1, + activity: "idle", + duration: "5h", + }), leafRow({ name: "monitor", cmd: "btop", status: "running", indent: 0 }), ); body.classList.add("kit-tree-host"); diff --git a/web/src/styles.css b/web/src/styles.css index cd89d81..ea4060e 100644 --- a/web/src/styles.css +++ b/web/src/styles.css @@ -787,60 +787,96 @@ textarea { font-weight: 400; } +/* #107: session cards per docs/SIDEBAR.md — line 1 (name + top-right actions) + always, line 2 (meta) only when it has something to say. */ +.tree-leaf.session-card { + flex-direction: column; + align-items: stretch; + gap: 1px; +} + +.card-line1 { + display: flex; + align-items: center; + gap: var(--sp-2); + min-width: 0; +} + .leaf-name { font-weight: 500; font-size: var(--fs-13); - flex: 0 1 auto; - overflow: hidden; - text-overflow: ellipsis; - white-space: nowrap; -} - -.leaf-meta { - font-size: var(--fs-11); - color: var(--muted); - margin-left: auto; - padding-left: var(--sp-2); + /* Grows to push the actions to the right edge but shrinks first, so the name + truncates before it can ever reach them — overlap is a defect (#107). */ + flex: 1 1 auto; min-width: 0; - flex: 0 1 auto; overflow: hidden; text-overflow: ellipsis; white-space: nowrap; - display: inline-flex; - align-items: baseline; +} + +/* Line 2 — dot + duration in the state's colour, then muted fragments. The + state word itself is never spelled out here; the colour carries it. */ +.card-line2 { + display: flex; + align-items: center; gap: var(--sp-2); -} - -.leaf-cmd { - color: var(--muted); - overflow: hidden; - text-overflow: ellipsis; - white-space: nowrap; -} - -.leaf-badge { font-size: var(--fs-11); - letter-spacing: 0.02em; - text-transform: uppercase; + min-width: 0; color: var(--muted); - border: 1px solid var(--border); - border-radius: var(--r-pill); - padding: 1px 8px; +} + +.card-line2[data-activity="active"] { + color: var(--live); +} +.card-line2[data-activity="idle"] { + color: var(--muted); +} +.card-line2[data-activity="awaiting-input"] { + color: var(--warn); +} +.card-line2[data-activity="error"] { + color: var(--danger); +} + +.activity-dot { + width: 7px; + height: 7px; + border-radius: 50%; + background: currentColor; flex: 0 0 auto; } -/* #83: the session row's blocked-work warning — an unforced kill will be - refused, and the operator should see that before pressing the button. */ -.leaf-badge.leaf-blocked { - color: var(--warn, #b45309); - border-color: var(--warn, #b45309); +.activity-duration { + color: inherit; + flex: 0 0 auto; } +/* Agent-reported detail (#112): sits after the duration, shrinks with an + ellipsis so it never pushes the trailing fragments off the line. */ +.activity-detail { + color: var(--faint); + flex: 0 1 auto; + min-width: 0; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; +} + +/* Blocked-work / exited / command fragments recede: the ⚠ glyph already + signals severity and "exited" dims with the row. */ +.card-flag { + color: var(--faint); + min-width: 0; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; +} + +/* Actions live in line 1's flow at its right edge — never overlaid on the + name (#107). They reserve their width even while hover-faded. */ .leaf-actions { - position: absolute; - right: var(--sp-1); - top: 50%; - transform: translateY(-50%); + flex: 0 0 auto; + margin-left: auto; display: inline-flex; gap: 2px; } diff --git a/web/src/types.ts b/web/src/types.ts index 3f15719..3731779 100644 --- a/web/src/types.ts +++ b/web/src/types.ts @@ -9,8 +9,9 @@ export type IsoTimestamp = string; export type SessionStatus = "Running" | "Dead"; /** Daemon-inferred activity dimension, orthogonal to `SessionStatus` (NAP-340). - * Absent on payloads from daemons that predate the field. */ -export type ActivityState = "Active" | "Idle" | "AwaitingInput"; + * Absent on payloads from daemons that predate the field. `Error` is only set + * when an agent reports failure (#105); the daemon never infers it. */ +export type ActivityState = "Active" | "Idle" | "AwaitingInput" | "Error"; export interface Session { id: Uuid; @@ -25,8 +26,17 @@ export interface Session { foreground?: string | null; /** Inferred activity. Defaults to `Idle` on the wire when omitted. */ activity?: ActivityState; + /** Short human string an agent reported alongside its state (#105) — e.g. + * why it is in `Error`. Shown on a sidebar card's meta line while the + * report is in force; cleared when the report lapses or is replaced. */ + activity_detail?: string | null; /** Wall-clock of the most recent observed PTY output (NAP-340). */ last_activity_at?: IsoTimestamp; + /** Wall-clock of when the session entered its current `activity` state + * (#102) — distinct from `last_activity_at`, which only tracks PTY output. + * Defaults to the Unix epoch on payloads from daemons that predate the + * field. */ + activity_since?: IsoTimestamp; /** The pane's *current* working directory, as the daemon's activity watcher * observed it (`#{pane_current_path}`) — distinct from `cwd` (the spawn dir). * Drives the directory-based session tree. Absent until first observed. */