From 6af8abccd32bf7fd236bf1413d1a8a0d06605d9a Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Mon, 24 Aug 2026 19:17:04 +0100 Subject: [PATCH 01/37] docs(maestro): add SIDEBAR.md session card contract --- CLAUDE.md | 4 +++ docs/SIDEBAR.md | 92 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 96 insertions(+) create mode 100644 docs/SIDEBAR.md 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/docs/SIDEBAR.md b/docs/SIDEBAR.md new file mode 100644 index 0000000..c3e5eb4 --- /dev/null +++ b/docs/SIDEBAR.md @@ -0,0 +1,92 @@ +# 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] +``` + +## 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 and exited +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 +the TTL the report carries; when the TTL expires, inference resumes. Clients +never blend the two: whichever source is in force supplies both the colour and +the clock for `activity_since`. + +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. From b95b5c04c19049618e4550155526f8526902f60f Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Mon, 24 Aug 2026 19:19:33 +0100 Subject: [PATCH 02/37] feat(maestro): add activity_since to the session wire --- crates/maestro-cli/src/sessions.rs | 1 + crates/maestro-daemon/src/session.rs | 2 ++ crates/maestro-daemon/tests/sidecar.rs | 1 + crates/maestro-protocol/src/lib.rs | 29 ++++++++++++++++++++++++++ crates/maestro-testing/src/lib.rs | 1 + crates/maestro-tui/src/app.rs | 2 ++ crates/maestro-tui/src/catalog.rs | 1 + crates/maestro-tui/src/sidebar.rs | 2 ++ 8 files changed, 39 insertions(+) diff --git a/crates/maestro-cli/src/sessions.rs b/crates/maestro-cli/src/sessions.rs index ad43be6..f58fd98 100644 --- a/crates/maestro-cli/src/sessions.rs +++ b/crates/maestro-cli/src/sessions.rs @@ -176,6 +176,7 @@ mod tests { foreground: None, activity: Default::default(), last_activity_at: chrono::Utc::now(), + activity_since: chrono::Utc::now(), current_cwd: None, blocked_work: None, } diff --git a/crates/maestro-daemon/src/session.rs b/crates/maestro-daemon/src/session.rs index a8150bb..549d1be 100644 --- a/crates/maestro-daemon/src/session.rs +++ b/crates/maestro-daemon/src/session.rs @@ -353,6 +353,7 @@ impl SessionManager { foreground: None, activity: ActivityState::Active, 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 @@ -572,6 +573,7 @@ impl SessionManager { foreground: None, activity: ActivityState::Idle, last_activity_at: Utc::now(), + activity_since: chrono::Utc::now(), current_cwd: None, blocked_work: None, diff --git a/crates/maestro-daemon/tests/sidecar.rs b/crates/maestro-daemon/tests/sidecar.rs index bf864ed..2d88ff4 100644 --- a/crates/maestro-daemon/tests/sidecar.rs +++ b/crates/maestro-daemon/tests/sidecar.rs @@ -27,6 +27,7 @@ fn sample(id: Uuid) -> Session { foreground: None, activity: Default::default(), 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-protocol/src/lib.rs b/crates/maestro-protocol/src/lib.rs index 66fb569..391d0bd 100644 --- a/crates/maestro-protocol/src/lib.rs +++ b/crates/maestro-protocol/src/lib.rs @@ -80,6 +80,13 @@ pub struct Session { /// 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 @@ -537,6 +544,9 @@ mod tests { 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 +598,25 @@ 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)); + } + #[test] fn activity_state_serializes_pascal_case_and_defaults_idle() { assert_eq!( diff --git a/crates/maestro-testing/src/lib.rs b/crates/maestro-testing/src/lib.rs index 881fd32..d5a6d47 100644 --- a/crates/maestro-testing/src/lib.rs +++ b/crates/maestro-testing/src/lib.rs @@ -42,6 +42,7 @@ pub fn session(id: Uuid, cwd: &str, command: &str) -> Session { foreground: None, activity: Default::default(), 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..3034aa4 100644 --- a/crates/maestro-tui/src/app.rs +++ b/crates/maestro-tui/src/app.rs @@ -1127,6 +1127,7 @@ mod tests { foreground: None, activity: Default::default(), last_activity_at: Utc::now(), + activity_since: chrono::Utc::now(), current_cwd: None, blocked_work: None, }], @@ -1644,6 +1645,7 @@ mod tests { foreground: None, activity: Default::default(), 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..178a559 100644 --- a/crates/maestro-tui/src/catalog.rs +++ b/crates/maestro-tui/src/catalog.rs @@ -194,6 +194,7 @@ mod tests { foreground: None, activity: Default::default(), 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..cd17e98 100644 --- a/crates/maestro-tui/src/sidebar.rs +++ b/crates/maestro-tui/src/sidebar.rs @@ -105,6 +105,7 @@ mod tests { foreground: None, activity: Default::default(), last_activity_at: chrono::Utc::now(), + activity_since: chrono::Utc::now(), current_cwd: None, blocked_work: None, } @@ -228,6 +229,7 @@ mod tests { foreground: None, activity: Default::default(), last_activity_at: chrono::Utc::now(), + activity_since: chrono::Utc::now(), current_cwd: None, blocked_work: None, From fc6f3469b0dd2a1fe1b778b4ec793d9e4ce12160 Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Mon, 24 Aug 2026 19:21:44 +0100 Subject: [PATCH 03/37] feat(maestro): add Error activity state to the wire --- crates/maestro-cli/src/sessions.rs | 1 + crates/maestro-protocol/src/lib.rs | 13 +++++++++++++ 2 files changed, 14 insertions(+) diff --git a/crates/maestro-cli/src/sessions.rs b/crates/maestro-cli/src/sessions.rs index f58fd98..ed51946 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). diff --git a/crates/maestro-protocol/src/lib.rs b/crates/maestro-protocol/src/lib.rs index 391d0bd..e960175 100644 --- a/crates/maestro-protocol/src/lib.rs +++ b/crates/maestro-protocol/src/lib.rs @@ -126,6 +126,9 @@ 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, } /// Optional worktree request attached to a `CreateSessionRequest`. @@ -617,6 +620,16 @@ mod tests { 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!( From 1a0d1284e1993fb67a421fa7419902b522958535 Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Mon, 24 Aug 2026 19:22:47 +0100 Subject: [PATCH 04/37] feat(maestro): mirror activity_since and Error state in web types --- web/src/types.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/web/src/types.ts b/web/src/types.ts index 3f15719..c03aebd 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; @@ -27,6 +28,11 @@ export interface Session { activity?: ActivityState; /** 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. */ From 7d06bec21e1fa0a8b9e679bcc8d02978938a4f51 Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Mon, 24 Aug 2026 19:30:11 +0100 Subject: [PATCH 05/37] feat(maestro): export MAESTRO_SESSION_ID and MAESTRO_URL into spawned sessions Every tmux-spawned session now gets its UUID under MAESTRO_SESSION_ID and, when the runner has wired its base URL from [daemon].bind_addr, that URL under MAESTRO_URL (#103). The daemon-owned values override anything the spawn request carried. Adopted sessions stay without them by design. --- crates/maestro-daemon/src/cli.rs | 3 + crates/maestro-daemon/src/session.rs | 29 ++++- .../maestro-daemon/tests/session_decisions.rs | 100 +++++++++++++++++- 3 files changed, 127 insertions(+), 5 deletions(-) diff --git a/crates/maestro-daemon/src/cli.rs b/crates/maestro-daemon/src/cli.rs index cef210d..04f2077 100644 --- a/crates/maestro-daemon/src/cli.rs +++ b/crates/maestro-daemon/src/cli.rs @@ -73,6 +73,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/session.rs b/crates/maestro-daemon/src/session.rs index a8150bb..4f16692 100644 --- a/crates/maestro-daemon/src/session.rs +++ b/crates/maestro-daemon/src/session.rs @@ -131,6 +131,11 @@ 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>, sessions: Mutex>, events: broadcast::Sender, } @@ -170,6 +175,7 @@ impl SessionManager { replay_bytes, spawn_history: None, retained: None, + daemon_url: std::sync::Mutex::new(None), sessions: Mutex::new(HashMap::new()), events, }), @@ -202,6 +208,14 @@ impl SessionManager { 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 { @@ -304,7 +318,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 +326,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(); diff --git a/crates/maestro-daemon/tests/session_decisions.rs b/crates/maestro-daemon/tests/session_decisions.rs index aae5475..3b81634 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] @@ -396,3 +421,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()) + ); +} From b3c73fd2cbb1c1c21ad0dc65f618feb72f19fff9 Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Mon, 24 Aug 2026 19:30:11 +0100 Subject: [PATCH 06/37] test(maestro): assert the exported session env end to end through real tmux --- crates/maestro-daemon/src/tmux.rs | 19 +++++++++ .../maestro-daemon/tests/session_manager.rs | 41 +++++++++++++++++++ 2 files changed, 60 insertions(+) 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/session_manager.rs b/crates/maestro-daemon/tests/session_manager.rs index 0e11df4..c90d9cc 100644 --- a/crates/maestro-daemon/tests/session_manager.rs +++ b/crates/maestro-daemon/tests/session_manager.rs @@ -1004,3 +1004,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"); +} From 581ec2949283aee8e6716f2fe528c5f6454e25df Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Mon, 24 Aug 2026 19:32:08 +0100 Subject: [PATCH 07/37] docs(maestro): name the activity dot in the card contract --- docs/SIDEBAR.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/docs/SIDEBAR.md b/docs/SIDEBAR.md index c3e5eb4..55cbd49 100644 --- a/docs/SIDEBAR.md +++ b/docs/SIDEBAR.md @@ -41,9 +41,14 @@ Otherwise line 2 is absent — never an empty placeholder row. When present, line 2 reads left to right: ``` - [] [ | exited] + [ | 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. + ## Colour per activity state The state word is never spelled out — the colour carries it. From d7b15a131f6ae15061492eafde0ff4af7e0ab828 Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Mon, 24 Aug 2026 19:42:33 +0100 Subject: [PATCH 08/37] feat(maestro): activity duration + colour helpers per the sidebar card contract --- web/src/activity.test.ts | 101 +++++++++++++++++++++++++++++++++++++++ web/src/activity.ts | 56 ++++++++++++++++++++++ 2 files changed, 157 insertions(+) create mode 100644 web/src/activity.test.ts create mode 100644 web/src/activity.ts 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" + ); +} From bb28fe1d6e193931c080fe72f3f26cd9a392c70b Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Mon, 24 Aug 2026 19:48:23 +0100 Subject: [PATCH 09/37] feat(maestro): render an adaptive meta line under TUI session cards Per docs/SIDEBAR.md: line 2 carries the activity dot + floored duration in the state's colour, plus the blocked-work and exited fragments; it appears only when time-in-state exceeds 30s, work is blocked, the session is dead, or an Error state is in force. Blocked work moves off line 1. Card lines truncate with an ellipsis instead of wrapping. --- crates/maestro-tui/src/sidebar.rs | 380 ++++++++++++++++++++++++------ 1 file changed, 311 insertions(+), 69 deletions(-) diff --git a/crates/maestro-tui/src/sidebar.rs b/crates/maestro-tui/src/sidebar.rs index cd17e98..a0fab9e 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,12 @@ 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; +const INDENT: &str = " "; +const META_INDENT: &str = " "; + /// Renderable sidebar state. #[derive(Debug, Clone)] pub struct Sidebar { @@ -41,36 +51,119 @@ 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), - ])); + lines.push(Line::from(self.name_row(s, width))); + if let Some(meta) = self.meta_row(s, width) { + lines.push(Line::from(meta)); + } } 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 style = if highlighted { + Style::default().add_modifier(Modifier::REVERSED) + } else { + Style::default() + }; + let row = vec![ + Span::raw(INDENT), + Span::raw(format!("{glyph} ")), + Span::raw(name_for(s)), + ]; + fit_to_width(row, width) + .into_iter() + .map(|sp| Span::styled(sp.content, style.patch(sp.style))) + .collect() + } + + /// The adaptive second line: ` [ | exited]`. + fn meta_row(&self, s: &Session, width: usize) -> Option>> { + let now = Utc::now(); + let held_secs = (now - s.activity_since).num_seconds(); + // #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_secs < META_LINE_AFTER_SECS && blocked.is_none() && !dead && !errored { + return None; + } + let dot_style = Style::default().fg(activity_color(s.activity)); + let muted = Style::default(); + let duration = format_state_duration(now - s.activity_since); + let mut spans = vec![ + Span::raw(META_INDENT), + Span::styled("●", dot_style), + Span::raw(" "), + Span::styled(duration, dot_style), + ]; + if let Some(b) = blocked { + spans.push(Span::styled(format!(" ⚠ {b}"), muted)); + } + if dead { + spans.push(Span::styled(" exited", muted)); + } + 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. +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 } /// `metadata.name` if present, else the command line. @@ -85,8 +178,10 @@ fn name_for(s: &Session) -> String { #[cfg(test)] mod tests { use super::*; - use chrono::Utc; + use chrono::Duration; + use maestro_protocol::InTheWay; use ratatui::backend::TestBackend; + use ratatui::buffer::Buffer; use ratatui::Terminal; use std::collections::HashMap; @@ -104,26 +199,43 @@ mod tests { created_at: Utc::now(), foreground: None, activity: Default::default(), - last_activity_at: chrono::Utc::now(), - activity_since: chrono::Utc::now(), + 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] @@ -133,7 +245,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}" @@ -152,11 +264,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('○'), @@ -173,21 +282,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() @@ -200,21 +306,158 @@ 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] + ); + } + #[test] fn falls_back_to_command_when_name_metadata_is_absent() { let s = Session { @@ -228,9 +471,8 @@ mod tests { created_at: Utc::now(), foreground: None, activity: Default::default(), - last_activity_at: chrono::Utc::now(), - activity_since: chrono::Utc::now(), - + last_activity_at: Utc::now(), + activity_since: Utc::now(), current_cwd: None, blocked_work: None, }; @@ -239,7 +481,7 @@ 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}" From 002c128ac82288062195f2dd2ed6f4351a2f89f3 Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Mon, 24 Aug 2026 19:49:55 +0100 Subject: [PATCH 10/37] feat(maestro): stamp activity_since only on state transitions (#104) The activity pass now sets activity_since only when the decided ActivityState differs from the stored one; foreground/cwd-only diffs and steady-state ticks leave the clock alone, so the sidebar duration counts time in the current state instead of reading zero. Daemon restarts still reset the clock to the restart time (stated in recover()); the daemon cannot know how long a recovered session had already been idle. --- crates/maestro-daemon/src/session.rs | 9 +++ .../maestro-daemon/tests/session_decisions.rs | 67 +++++++++++++++++++ 2 files changed, 76 insertions(+) diff --git a/crates/maestro-daemon/src/session.rs b/crates/maestro-daemon/src/session.rs index c2229ee..4070b24 100644 --- a/crates/maestro-daemon/src/session.rs +++ b/crates/maestro-daemon/src/session.rs @@ -597,6 +597,9 @@ 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, last_activity_at: Utc::now(), @@ -857,6 +860,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; diff --git a/crates/maestro-daemon/tests/session_decisions.rs b/crates/maestro-daemon/tests/session_decisions.rs index 3b81634..dae0158 100644 --- a/crates/maestro-daemon/tests/session_decisions.rs +++ b/crates/maestro-daemon/tests/session_decisions.rs @@ -363,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. @@ -374,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 { From 5be3e33c94ba7d34bb2bc3e7cdedcfe8b9915a47 Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Mon, 24 Aug 2026 19:50:24 +0100 Subject: [PATCH 11/37] feat(maestro): render sidebar sessions as two-line cards per docs/SIDEBAR.md --- web/src/catalog.test.ts | 119 ++++++++++++++++++++++++++++++++++++++++ web/src/catalog.ts | 78 +++++++++++++++++++++----- web/src/kit.ts | 84 ++++++++++++++++++++++------ web/src/styles.css | 101 +++++++++++++++++++++------------- 4 files changed, 311 insertions(+), 71 deletions(-) diff --git a/web/src/catalog.test.ts b/web/src/catalog.test.ts index b502fd7..57fa9a1 100644 --- a/web/src/catalog.test.ts +++ b/web/src/catalog.test.ts @@ -104,3 +104,122 @@ describe("blocked-work indicator (#83)", () => { expect(confirmations).toEqual(['Kill session "claude"?']); }); }); + +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..cefca49 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,41 @@ 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}` : "", + 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 +88,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 +435,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 +457,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..4b533e9 100644 --- a/web/src/styles.css +++ b/web/src/styles.css @@ -787,60 +787,85 @@ 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; } +/* 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; } From 323ead5a83afaae62844f29184dfc7da215bdf38 Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Mon, 24 Aug 2026 19:50:37 +0100 Subject: [PATCH 12/37] feat(maestro): give TUI cards explicit card and focus backgrounds Reversed video made a highlighted card unreadable. Cards now sit on a black card background painted edge to edge; the highlighted card swaps it for black-on-cyan, matching the status bar's focus treatment. --- crates/maestro-tui/src/sidebar.rs | 133 +++++++++++++++++++++++++++--- 1 file changed, 121 insertions(+), 12 deletions(-) diff --git a/crates/maestro-tui/src/sidebar.rs b/crates/maestro-tui/src/sidebar.rs index a0fab9e..fd0cd5e 100644 --- a/crates/maestro-tui/src/sidebar.rs +++ b/crates/maestro-tui/src/sidebar.rs @@ -22,6 +22,18 @@ const META_LINE_AFTER_SECS: i64 = 30; const INDENT: &str = " "; const META_INDENT: &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 { @@ -57,9 +69,15 @@ impl Sidebar { Style::default().add_modifier(Modifier::BOLD), ))]; for s in &self.sessions { - lines.push(Line::from(self.name_row(s, width))); + let highlighted = self.highlight == Some(s.id); + let base = card_base(highlighted); + lines.push(Line::from(pad_to_width( + self.name_row(s, width), + width, + base, + ))); if let Some(meta) = self.meta_row(s, width) { - lines.push(Line::from(meta)); + lines.push(Line::from(pad_to_width(meta, width, base))); } } f.render_widget(Paragraph::new(lines), inner); @@ -72,11 +90,7 @@ impl Sidebar { (SessionStatus::Running, false) => "○", }; let highlighted = self.highlight == Some(s.id); - let style = if highlighted { - Style::default().add_modifier(Modifier::REVERSED) - } else { - Style::default() - }; + let base = card_base(highlighted); let row = vec![ Span::raw(INDENT), Span::raw(format!("{glyph} ")), @@ -84,7 +98,7 @@ impl Sidebar { ]; fit_to_width(row, width) .into_iter() - .map(|sp| Span::styled(sp.content, style.patch(sp.style))) + .map(|sp| Span::styled(sp.content, base.patch(sp.style))) .collect() } @@ -100,13 +114,14 @@ impl Sidebar { if held_secs < META_LINE_AFTER_SECS && blocked.is_none() && !dead && !errored { return None; } - let dot_style = Style::default().fg(activity_color(s.activity)); - let muted = Style::default(); + 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 duration = format_state_duration(now - s.activity_since); let mut spans = vec![ - Span::raw(META_INDENT), + Span::styled(META_INDENT, base), Span::styled("●", dot_style), - Span::raw(" "), + Span::styled(" ", base), Span::styled(duration, dot_style), ]; if let Some(b) = blocked { @@ -166,6 +181,17 @@ fn fit_to_width(spans: Vec>, max: usize) -> Vec> { 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. fn name_for(s: &Session) -> String { if let Some(serde_json::Value::String(name)) = s.metadata.get("name") { @@ -487,4 +513,87 @@ mod tests { "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}" + ); + } + } } From f6a97c4695fbe902ea13bdae5a34d1fba18dfcc7 Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Mon, 24 Aug 2026 19:51:06 +0100 Subject: [PATCH 13/37] =?UTF-8?q?docs(maestro):=20settle=20line-2=20ruling?= =?UTF-8?q?s=20=E2=80=94=20command=20fragment,=20pre-#102=20payloads,=20un?= =?UTF-8?q?conditional=20dot?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/SIDEBAR.md | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/docs/SIDEBAR.md b/docs/SIDEBAR.md index 55cbd49..1ac2442 100644 --- a/docs/SIDEBAR.md +++ b/docs/SIDEBAR.md @@ -41,13 +41,20 @@ Otherwise line 2 is absent — never an empty placeholder row. When present, line 2 reads left to right: ``` - [ | exited] + [ | 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. +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 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 @@ -65,9 +72,9 @@ 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 and exited -fragments use the muted colour (`--faint` / default style), since the ⚠ glyph -already signals severity. +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 From c7c5b2e0ec25ae137b978da8f957f7b28867bcb3 Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Mon, 24 Aug 2026 19:53:27 +0100 Subject: [PATCH 14/37] feat(maestro): show spawn-similar and kill affordances on the highlighted card Line-1 actions anchor to the card's top-right corner on the highlighted row only, reserving their width so the name truncates with an ellipsis before it can ever reach them. --- crates/maestro-tui/src/sidebar.rs | 107 ++++++++++++++++++++++++++---- 1 file changed, 94 insertions(+), 13 deletions(-) diff --git a/crates/maestro-tui/src/sidebar.rs b/crates/maestro-tui/src/sidebar.rs index fd0cd5e..9e2bda0 100644 --- a/crates/maestro-tui/src/sidebar.rs +++ b/crates/maestro-tui/src/sidebar.rs @@ -21,6 +21,8 @@ use uuid::Uuid; const META_LINE_AFTER_SECS: i64 = 30; 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 @@ -71,11 +73,7 @@ impl Sidebar { for s in &self.sessions { let highlighted = self.highlight == Some(s.id); let base = card_base(highlighted); - lines.push(Line::from(pad_to_width( - self.name_row(s, width), - width, - base, - ))); + 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))); } @@ -91,15 +89,41 @@ impl Sidebar { }; let highlighted = self.highlight == Some(s.id); let base = card_base(highlighted); - let row = vec![ - Span::raw(INDENT), - Span::raw(format!("{glyph} ")), - Span::raw(name_for(s)), + // 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), ]; - fit_to_width(row, width) - .into_iter() - .map(|sp| Span::styled(sp.content, base.patch(sp.style))) - .collect() + 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]`. @@ -596,4 +620,61 @@ mod tests { ); } } + + /// 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:?}" + ); + } } From 766d2569707149be6bdf03d7736b3e89d1541163 Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Mon, 24 Aug 2026 19:57:58 +0100 Subject: [PATCH 15/37] fix(maestro): suppress epoch durations on TUI cards --- crates/maestro-tui/src/sidebar.rs | 70 ++++++++++++++++++++++++++++--- 1 file changed, 64 insertions(+), 6 deletions(-) diff --git a/crates/maestro-tui/src/sidebar.rs b/crates/maestro-tui/src/sidebar.rs index 9e2bda0..4a02ca2 100644 --- a/crates/maestro-tui/src/sidebar.rs +++ b/crates/maestro-tui/src/sidebar.rs @@ -19,6 +19,12 @@ 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. @@ -129,25 +135,31 @@ impl Sidebar { /// The adaptive second line: ` [ | exited]`. fn meta_row(&self, s: &Session, width: usize) -> Option>> { let now = Utc::now(); - let held_secs = (now - s.activity_since).num_seconds(); + 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_secs < META_LINE_AFTER_SECS && blocked.is_none() && !dead && !errored { + 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 duration = format_state_duration(now - s.activity_since); let mut spans = vec![ Span::styled(META_INDENT, base), Span::styled("●", dot_style), - Span::styled(" ", base), - Span::styled(duration, dot_style), ]; + if let Some(d) = held { + spans.push(Span::styled(" ", base)); + spans.push(Span::styled(format_state_duration(d), dot_style)); + } if let Some(b) = blocked { spans.push(Span::styled(format!(" ⚠ {b}"), muted)); } @@ -171,6 +183,16 @@ fn activity_color(state: ActivityState) -> Color { /// 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 { @@ -228,7 +250,7 @@ fn name_for(s: &Session) -> String { #[cfg(test)] mod tests { use super::*; - use chrono::Duration; + use chrono::{DateTime, Duration}; use maestro_protocol::InTheWay; use ratatui::backend::TestBackend; use ratatui::buffer::Buffer; @@ -508,6 +530,42 @@ mod tests { ); } + /// 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:?}" + ); + } + } + #[test] fn falls_back_to_command_when_name_metadata_is_absent() { let s = Session { From 71add0215a041102da2c38ddb039ab50d657ee45 Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Mon, 24 Aug 2026 21:28:30 +0100 Subject: [PATCH 16/37] fix(maestro): deserialize unknown ActivityState variants as Idle A newer daemon's unknown activity variant failed the whole Session deserialization on older clients. Hand-written Deserialize maps any unknown string to Idle; non-string values still error. (#110) --- crates/maestro-protocol/src/lib.rs | 52 +++++++++++++++++++++++++++++- 1 file changed, 51 insertions(+), 1 deletion(-) diff --git a/crates/maestro-protocol/src/lib.rs b/crates/maestro-protocol/src/lib.rs index e960175..9b370d1 100644 --- a/crates/maestro-protocol/src/lib.rs +++ b/crates/maestro-protocol/src/lib.rs @@ -117,7 +117,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, @@ -131,6 +136,22 @@ pub enum ActivityState { 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, + }) + } +} + /// Optional worktree request attached to a `CreateSessionRequest`. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct WorktreeSpec { @@ -639,6 +660,35 @@ 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 session_round_trips() { let s = sample_session(); From 5c4e083c2d129e345ba86452be687213215ffb2b Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Mon, 24 Aug 2026 21:32:27 +0100 Subject: [PATCH 17/37] feat(maestro): add activity_detail to Session and the activity-report wire types --- crates/maestro-cli/src/sessions.rs | 1 + crates/maestro-daemon/src/session.rs | 2 + crates/maestro-daemon/tests/sidecar.rs | 1 + crates/maestro-protocol/src/client.rs | 17 ++++++- crates/maestro-protocol/src/lib.rs | 62 ++++++++++++++++++++++++++ crates/maestro-protocol/src/routes.rs | 14 ++++++ crates/maestro-testing/src/lib.rs | 1 + crates/maestro-tui/src/app.rs | 2 + crates/maestro-tui/src/catalog.rs | 1 + crates/maestro-tui/src/sidebar.rs | 2 + 10 files changed, 102 insertions(+), 1 deletion(-) diff --git a/crates/maestro-cli/src/sessions.rs b/crates/maestro-cli/src/sessions.rs index ed51946..12fa99b 100644 --- a/crates/maestro-cli/src/sessions.rs +++ b/crates/maestro-cli/src/sessions.rs @@ -176,6 +176,7 @@ 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, diff --git a/crates/maestro-daemon/src/session.rs b/crates/maestro-daemon/src/session.rs index 4070b24..82c1a94 100644 --- a/crates/maestro-daemon/src/session.rs +++ b/crates/maestro-daemon/src/session.rs @@ -379,6 +379,7 @@ 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(), @@ -602,6 +603,7 @@ impl SessionManager { // 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(), diff --git a/crates/maestro-daemon/tests/sidecar.rs b/crates/maestro-daemon/tests/sidecar.rs index 2d88ff4..0274844 100644 --- a/crates/maestro-daemon/tests/sidecar.rs +++ b/crates/maestro-daemon/tests/sidecar.rs @@ -26,6 +26,7 @@ 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")), 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 e960175..e1e7973 100644 --- a/crates/maestro-protocol/src/lib.rs +++ b/crates/maestro-protocol/src/lib.rs @@ -75,6 +75,12 @@ 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. @@ -131,6 +137,18 @@ pub enum ActivityState { Error, } +/// 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 { + 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, +} + /// Optional worktree request attached to a `CreateSessionRequest`. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct WorktreeSpec { @@ -544,6 +562,7 @@ 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), @@ -639,6 +658,49 @@ mod tests { assert_eq!(ActivityState::default(), ActivityState::Idle); } + #[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 d5a6d47..8defccf 100644 --- a/crates/maestro-testing/src/lib.rs +++ b/crates/maestro-testing/src/lib.rs @@ -41,6 +41,7 @@ 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, diff --git a/crates/maestro-tui/src/app.rs b/crates/maestro-tui/src/app.rs index 3034aa4..245a628 100644 --- a/crates/maestro-tui/src/app.rs +++ b/crates/maestro-tui/src/app.rs @@ -1126,6 +1126,7 @@ 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, @@ -1644,6 +1645,7 @@ 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(), diff --git a/crates/maestro-tui/src/catalog.rs b/crates/maestro-tui/src/catalog.rs index 178a559..e3466f0 100644 --- a/crates/maestro-tui/src/catalog.rs +++ b/crates/maestro-tui/src/catalog.rs @@ -193,6 +193,7 @@ 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, diff --git a/crates/maestro-tui/src/sidebar.rs b/crates/maestro-tui/src/sidebar.rs index 4a02ca2..2737d12 100644 --- a/crates/maestro-tui/src/sidebar.rs +++ b/crates/maestro-tui/src/sidebar.rs @@ -271,6 +271,7 @@ mod tests { created_at: Utc::now(), foreground: None, activity: Default::default(), + activity_detail: None, last_activity_at: Utc::now(), activity_since: Utc::now(), current_cwd: None, @@ -579,6 +580,7 @@ mod tests { created_at: Utc::now(), foreground: None, activity: Default::default(), + activity_detail: None, last_activity_at: Utc::now(), activity_since: Utc::now(), current_cwd: None, From 8370ddf066fd81cada556d320d9beca7d0d1c9a5 Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Mon, 24 Aug 2026 21:36:56 +0100 Subject: [PATCH 18/37] feat(maestro): hold agent-reported activity above inference for a TTL --- crates/maestro-daemon/src/activity.rs | 59 +++++++ crates/maestro-daemon/src/lib.rs | 1 + crates/maestro-daemon/src/session.rs | 78 ++++++++- .../maestro-daemon/tests/activity_report.rs | 164 ++++++++++++++++++ 4 files changed, 301 insertions(+), 1 deletion(-) create mode 100644 crates/maestro-daemon/tests/activity_report.rs 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/lib.rs b/crates/maestro-daemon/src/lib.rs index 8f85980..b92689e 100644 --- a/crates/maestro-daemon/src/lib.rs +++ b/crates/maestro-daemon/src/lib.rs @@ -27,5 +27,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/session.rs b/crates/maestro-daemon/src/session.rs index 82c1a94..0575760 100644 --- a/crates/maestro-daemon/src/session.rs +++ b/crates/maestro-daemon/src/session.rs @@ -57,6 +57,11 @@ 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, } #[derive(Debug, thiserror::Error)] @@ -136,6 +141,9 @@ struct Inner { /// 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, } @@ -176,6 +184,7 @@ impl SessionManager { spawn_history: None, retained: None, daemon_url: std::sync::Mutex::new(None), + report_ttl: crate::activity::REPORT_TTL, sessions: Mutex::new(HashMap::new()), events, }), @@ -208,6 +217,15 @@ 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 @@ -410,6 +428,7 @@ impl SessionManager { worktree_base_repo, worktree_branch: req.worktree.as_ref().map(|w| w.branch.clone()), bridge, + reported_activity: None, }, ); } @@ -637,6 +656,7 @@ impl SessionManager { // recovered session probes git for it at retain time. worktree_branch: None, bridge, + reported_activity: None, }, ); let _ = self @@ -677,6 +697,48 @@ 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 now = std::time::Instant::now(); + 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 @@ -847,8 +909,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. 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; +} From 5a594b2b7f45c995b98572668b4be56b739d64c5 Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Mon, 24 Aug 2026 21:38:42 +0100 Subject: [PATCH 19/37] feat(maestro): serve POST /api/sessions/:id/activity --- crates/maestro-daemon/src/local_api.rs | 30 ++++++- crates/maestro-daemon/tests/local_api.rs | 103 +++++++++++++++++++++++ 2 files changed, 130 insertions(+), 3 deletions(-) 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/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] From 015f2be5fb9c5a6da1df66adbb41a6c3b21a5694 Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Mon, 24 Aug 2026 21:39:15 +0100 Subject: [PATCH 20/37] docs(maestro): record the activity-detail meta-line slot and report TTL --- docs/SIDEBAR.md | 14 ++++++++++---- web/src/types.ts | 4 ++++ 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/docs/SIDEBAR.md b/docs/SIDEBAR.md index 1ac2442..cab0444 100644 --- a/docs/SIDEBAR.md +++ b/docs/SIDEBAR.md @@ -41,7 +41,7 @@ Otherwise line 2 is absent — never an empty placeholder row. When present, line 2 reads left to right: ``` - [ | exited] [] + [] [ | exited] [] ``` `` is a single filled glyph (`●` on both clients) carrying the activity @@ -52,6 +52,11 @@ 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. + `` 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). @@ -94,9 +99,10 @@ cutover points land within one tick. ## Precedence An agent-reported `ActivityState` (#105) overrides the daemon's inference for -the TTL the report carries; when the TTL expires, inference resumes. Clients -never blend the two: whichever source is in force supplies both the colour and -the clock for `activity_since`. +**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`. 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) diff --git a/web/src/types.ts b/web/src/types.ts index c03aebd..3731779 100644 --- a/web/src/types.ts +++ b/web/src/types.ts @@ -26,6 +26,10 @@ 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 From 202b5008b865cedca54acd8ccc0807cbbec5ae22 Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Mon, 24 Aug 2026 21:40:34 +0100 Subject: [PATCH 21/37] fix(maestro): drop an unused instant from report_activity --- crates/maestro-daemon/src/session.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/maestro-daemon/src/session.rs b/crates/maestro-daemon/src/session.rs index 0575760..9ea3942 100644 --- a/crates/maestro-daemon/src/session.rs +++ b/crates/maestro-daemon/src/session.rs @@ -714,7 +714,6 @@ impl SessionManager { state: ActivityState, detail: Option, ) -> Result<(), SessionError> { - let now = std::time::Instant::now(); let updated = { let mut sessions = self.inner.sessions.lock().await; let rec = sessions.get_mut(&id).ok_or(SessionError::NotFound(id))?; From cfebfd441b19bc1ce7e75e69a8d03e92cc28d20e Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Mon, 24 Aug 2026 22:03:56 +0100 Subject: [PATCH 22/37] fix(maestro): wait for retained-store write in natural-exit test Dead status can be observed before the settle step's store write lands, so the test's store read raced the write under parallel load. Wait for the store to have the entry before asserting on it, same as the settle-state wait a few lines below. Fixes #111 --- crates/maestro-daemon/tests/session_manager.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/crates/maestro-daemon/tests/session_manager.rs b/crates/maestro-daemon/tests/session_manager.rs index c90d9cc..e0ec6cf 100644 --- a/crates/maestro-daemon/tests/session_manager.rs +++ b/crates/maestro-daemon/tests/session_manager.rs @@ -769,6 +769,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); From 4f0e887a5049a2d3f8cecb6559d76c4201061dd5 Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Mon, 24 Aug 2026 22:05:40 +0100 Subject: [PATCH 23/37] fix(maestro): parse reported activity states strictly --- crates/maestro-protocol/src/lib.rs | 27 +++++++++++++++++++++++++++ docs/SIDEBAR.md | 6 ++++++ 2 files changed, 33 insertions(+) diff --git a/crates/maestro-protocol/src/lib.rs b/crates/maestro-protocol/src/lib.rs index 80f3e79..390c245 100644 --- a/crates/maestro-protocol/src/lib.rs +++ b/crates/maestro-protocol/src/lib.rs @@ -163,6 +163,13 @@ impl<'de> Deserialize<'de> for ActivityState { /// 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. @@ -170,6 +177,26 @@ pub struct ReportActivityRequest { 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`. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct WorktreeSpec { diff --git a/docs/SIDEBAR.md b/docs/SIDEBAR.md index cab0444..f1d5a23 100644 --- a/docs/SIDEBAR.md +++ b/docs/SIDEBAR.md @@ -108,3 +108,9 @@ 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. From 75ccfe10d25d0fca8f05dc2c2ba1588ad1388b83 Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Mon, 24 Aug 2026 22:13:04 +0100 Subject: [PATCH 24/37] docs(maestro): pin detail truncation ahead of trailing meta fragments --- docs/SIDEBAR.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/docs/SIDEBAR.md b/docs/SIDEBAR.md index f1d5a23..e2341c4 100644 --- a/docs/SIDEBAR.md +++ b/docs/SIDEBAR.md @@ -55,7 +55,10 @@ only when their trigger holds. A payload that predates #102 (no usable `` 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. +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 From c24a2133d4cc0a26730ba6f0c13a7553d03a0bb4 Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Mon, 24 Aug 2026 22:13:59 +0100 Subject: [PATCH 25/37] feat(maestro): render agent-reported detail on web session cards --- web/src/catalog.test.ts | 32 ++++++++++++++++++++++++++++++++ web/src/catalog.ts | 3 +++ web/src/styles.css | 11 +++++++++++ 3 files changed, 46 insertions(+) diff --git a/web/src/catalog.test.ts b/web/src/catalog.test.ts index 57fa9a1..560ee31 100644 --- a/web/src/catalog.test.ts +++ b/web/src/catalog.test.ts @@ -105,6 +105,38 @@ describe("blocked-work indicator (#83)", () => { }); }); +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(); diff --git a/web/src/catalog.ts b/web/src/catalog.ts index cefca49..a4d517a 100644 --- a/web/src/catalog.ts +++ b/web/src/catalog.ts @@ -42,6 +42,9 @@ function metaLine(s: Session, now: number): string { const fragments = [ ``, duration ? `${duration}` : "", + s.activity_detail + ? `${escapeHtml(s.activity_detail)}` + : "", blockers ? `⚠ ${escapeHtml(formatInTheWay(blockers))}` : "", diff --git a/web/src/styles.css b/web/src/styles.css index 4b533e9..ea4060e 100644 --- a/web/src/styles.css +++ b/web/src/styles.css @@ -851,6 +851,17 @@ textarea { 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 { From 4c9409a785162555d973557acb10ccd40a60b3f1 Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Mon, 24 Aug 2026 22:14:15 +0100 Subject: [PATCH 26/37] feat(maestro): add [hooks] auto_install config section --- crates/maestro-config/src/lib.rs | 38 ++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) 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. From eb605a3d05bd59d5013f1f678ee6062e4d166a92 Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Mon, 24 Aug 2026 22:18:23 +0100 Subject: [PATCH 27/37] feat(maestro): ship claude-code activity hook + idempotent installer --- Cargo.lock | 9 + Cargo.toml | 1 + crates/maestro-hooks/Cargo.toml | 15 ++ crates/maestro-hooks/src/lib.rs | 239 ++++++++++++++++++++++++++ crates/maestro-hooks/src/tests.rs | 268 ++++++++++++++++++++++++++++++ 5 files changed, 532 insertions(+) create mode 100644 crates/maestro-hooks/Cargo.toml create mode 100644 crates/maestro-hooks/src/lib.rs create mode 100644 crates/maestro-hooks/src/tests.rs diff --git a/Cargo.lock b/Cargo.lock index 646ccfb..5db5452 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1137,6 +1137,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-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..429f317 --- /dev/null +++ b/crates/maestro-hooks/src/lib.rs @@ -0,0 +1,239 @@ +//! 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. +pub const HOOK_EVENTS: [(&str, &str); 3] = [ + // Tool finished: the agent is demonstrably producing work. + ("PostToolUse", "Active"), + // Permission / idle notices: Claude is blocked on the user. + ("Notification", "AwaitingInput"), + // Turn ended: waiting on the user again. + ("Stop", "AwaitingInput"), +]; + +/// 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: +# PostToolUse -> Active (tool finished; agent is working) +# Notification -> AwaitingInput (permission / idle notice) +# Stop -> AwaitingInput (turn ended; waiting on the user) +state="$1" +[ -n "$MAESTRO_SESSION_ID" ] || exit 0 +[ -n "$MAESTRO_URL" ] || exit 0 +# Drain the hook payload so the parent never sees a broken pipe. +cat >/dev/null 2>&1 +curl -fsS --connect-timeout 1 --max-time 2 \ + -H 'Content-Type: application/json' \ + -d "{\"state\":\"$state\"}" \ + "$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..79acd80 --- /dev/null +++ b/crates/maestro-hooks/src/tests.rs @@ -0,0 +1,268 @@ +//! 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)]) -> 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(br#"{"session_id":"x"}"#) + .ok(); + child.wait_with_output().expect("wait") +} + +/// 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], &[]); + 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() { + let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + let addr = listener.local_addr().unwrap(); + let server = move || { + let (mut sock, _) = listener.accept().unwrap(); + let mut buf = [0u8; 4096]; + let n = sock.read(&mut buf).unwrap(); + let raw = String::from_utf8_lossy(&buf[..n]).to_string(); + sock.write_all(b"HTTP/1.1 204 No Content\r\nContent-Length: 0\r\n\r\n") + .unwrap(); + raw + }; + let handle = std::thread::spawn(server); + + let url = format!("http://{addr}"); + let session = "01234567-89ab-cdef-0123-456789abcdef"; + // The installer bakes the mapped state into the command's argv; the + // script relays whatever it is given. + let out = sh( + HOOK_SCRIPT, + &["Active"], + &[("MAESTRO_URL", &url), ("MAESTRO_SESSION_ID", session)], + ); + assert!(out.status.success()); + + let request = handle.join().unwrap(); + 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 issue specifies. +#[test] +fn event_state_mapping_matches_the_contract() { + let map: std::collections::HashMap<&str, &str> = HOOK_EVENTS.into_iter().collect(); + assert_eq!(map["PostToolUse"], "Active"); + assert_eq!(map["Stop"], "AwaitingInput"); + assert_eq!(map["Notification"], "AwaitingInput"); +} + +/// 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"), + ], + ); + 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(), 3); + let joined = cmds.join("\n"); + assert!(joined.contains("Active")); + 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(), 3); +} + +/// 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(), 3, "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 three events present, nothing else maestro-related anywhere. + let cmds = installed_commands(&settings); + assert_eq!( + cmds.iter().filter(|c| c.contains(HOOK_MARKER)).count(), + 3, + "{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); + } +} From 43946c3c46145bbf2b0232e60292716a074ee48a Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Mon, 24 Aug 2026 22:22:06 +0100 Subject: [PATCH 28/37] feat(maestro): render agent-reported detail on TUI session cards --- crates/maestro-tui/src/sidebar.rs | 105 +++++++++++++++++++++++++++++- 1 file changed, 103 insertions(+), 2 deletions(-) diff --git a/crates/maestro-tui/src/sidebar.rs b/crates/maestro-tui/src/sidebar.rs index 2737d12..0fe2dc3 100644 --- a/crates/maestro-tui/src/sidebar.rs +++ b/crates/maestro-tui/src/sidebar.rs @@ -160,12 +160,26 @@ impl Sidebar { 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 { - spans.push(Span::styled(format!(" ⚠ {b}"), muted)); + tail.push(Span::styled(format!(" ⚠ {b}"), muted)); } if dead { - spans.push(Span::styled(" exited", muted)); + 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)) } } @@ -531,6 +545,93 @@ mod tests { ); } + /// #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 From 3c5bb51e884e64e4eaf7e83deee83a9510ebf953 Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Mon, 24 Aug 2026 22:22:49 +0100 Subject: [PATCH 29/37] feat(maestro): add `maestro hooks install` subcommand --- Cargo.lock | 1 + crates/maestro-cli/Cargo.toml | 1 + crates/maestro-cli/src/cli.rs | 34 ++++++++++++++++++++++++++++++ crates/maestro-cli/src/dispatch.rs | 33 +++++++++++++++++++++++++++++ crates/maestro/src/main.rs | 18 +++++++++++++++- 5 files changed, 86 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 5db5452..76adbd6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1065,6 +1065,7 @@ dependencies = [ "futures-util", "libc", "maestro-config", + "maestro-hooks", "maestro-protocol", "maestro-testing", "nix", 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/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(_) ) } From 1b289a0450ca501b5d195cca142a77ea74eea3f7 Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Mon, 24 Aug 2026 22:25:54 +0100 Subject: [PATCH 30/37] feat(maestro): daemon auto-installs claude-code hook on first start --- Cargo.lock | 1 + crates/maestro-daemon/Cargo.toml | 1 + crates/maestro-daemon/src/cli.rs | 14 ++++++ crates/maestro-daemon/src/config.rs | 26 ++++++++++ crates/maestro-daemon/src/hooks.rs | 76 +++++++++++++++++++++++++++++ crates/maestro-daemon/src/lib.rs | 1 + 6 files changed, 119 insertions(+) create mode 100644 crates/maestro-daemon/src/hooks.rs diff --git a/Cargo.lock b/Cargo.lock index 76adbd6..7089357 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1103,6 +1103,7 @@ dependencies = [ "futures-util", "libc", "maestro-config", + "maestro-hooks", "maestro-names", "maestro-protocol", "reqwest", 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/cli.rs b/crates/maestro-daemon/src/cli.rs index 04f2077..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). 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 b92689e..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; From efbccb4e1138bb6ff23a2285c0f62807dc3a8e21 Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Mon, 24 Aug 2026 22:41:54 +0100 Subject: [PATCH 31/37] fix(maestro): wait for settle in pristine natural-exit test Dead status is observable a moment before the settle step removes the worktree, so the assertions raced the removal under parallel load. Wait for the tree to be gone and the store to stay empty, same as the retained-store wait cfebfd4 added to the guarded sibling. Fixes #113 --- crates/maestro-daemon/tests/session_manager.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/crates/maestro-daemon/tests/session_manager.rs b/crates/maestro-daemon/tests/session_manager.rs index e0ec6cf..ece2cf1 100644 --- a/crates/maestro-daemon/tests/session_manager.rs +++ b/crates/maestro-daemon/tests/session_manager.rs @@ -848,6 +848,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" From 54d84e3eaa32ecb1a499532d45de48dbec779613 Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Mon, 24 Aug 2026 22:46:46 +0100 Subject: [PATCH 32/37] docs(maestro): settle Error report TTL and clearing rulings --- docs/SIDEBAR.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/docs/SIDEBAR.md b/docs/SIDEBAR.md index e2341c4..82b0522 100644 --- a/docs/SIDEBAR.md +++ b/docs/SIDEBAR.md @@ -107,6 +107,20 @@ 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 From 88233cb96dc46673def697562c857b327659b3ad Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Mon, 24 Aug 2026 22:50:02 +0100 Subject: [PATCH 33/37] fix(maestro): sync watcher re-emit test to real event flow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two races. First, the watcher updates the snapshot and emits SessionUpdated as two steps, so the drain could run between them: the legitimate breakdown emission then arrived inside the quiet window. Subscribe before the watcher starts and consume through the breakdown event so the window begins after the last real change. Second, the assertion treated any SessionUpdated as a failed suppression, but the activity pass may legitimately announce an Active→Idle transition in that window (ACTIVE_WINDOW is 3s), carrying today's blocked_work unchanged. Only a changed indicator counts as a re-emit now. Fixes #113 --- .../maestro-daemon/tests/session_manager.rs | 42 ++++++++++++++++--- 1 file changed, 36 insertions(+), 6 deletions(-) diff --git a/crates/maestro-daemon/tests/session_manager.rs b/crates/maestro-daemon/tests/session_manager.rs index ece2cf1..1ecc734 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 From f689c916bfe0dcba6bb57de0674ea0aa5802c045 Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Mon, 24 Aug 2026 22:50:02 +0100 Subject: [PATCH 34/37] fix(maestro): poll awaiting-input until leaderless state holds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pipeline leader exits as soon as both stages are forked, which can beat the surviving stage opening the tty and parking on read — the one- shot assertion then ran against a group with no parked member yet. Poll for the answer, accepting it only while the foreground group is still leaderless so a shell that regained the tty cannot satisfy it. Fixes #113 --- crates/maestro-daemon/tests/activity.rs | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) 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"); } From 2a43c76176758379a61e303d46d90b562b9f1fb3 Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Mon, 24 Aug 2026 22:52:51 +0100 Subject: [PATCH 35/37] feat(maestro): report Active while a long tool call is still running --- crates/maestro-hooks/src/lib.rs | 13 +++- crates/maestro-hooks/src/tests.rs | 117 ++++++++++++++++++++---------- 2 files changed, 87 insertions(+), 43 deletions(-) diff --git a/crates/maestro-hooks/src/lib.rs b/crates/maestro-hooks/src/lib.rs index 429f317..1b70b11 100644 --- a/crates/maestro-hooks/src/lib.rs +++ b/crates/maestro-hooks/src/lib.rs @@ -25,8 +25,14 @@ pub const HOOK_MARKER: &str = "maestro-claude-hook"; pub const SCRIPT_FILE_NAME: &str = "maestro-claude-hook.sh"; /// Claude Code hook event → `ActivityState` spelling posted to the daemon. -pub const HOOK_EVENTS: [(&str, &str); 3] = [ - // Tool finished: the agent is demonstrably producing work. +/// +/// `PreToolUse` is what makes a session `Active` *during* a long tool call — +/// `PostToolUse` alone only fires once it ends (#114). +pub const HOOK_EVENTS: [(&str, &str); 4] = [ + // 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"), @@ -39,7 +45,8 @@ 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: -# PostToolUse -> Active (tool finished; agent is working) +# PreToolUse -> Active (tool started; agent is working) +# PostToolUse -> Active (tool finished) # Notification -> AwaitingInput (permission / idle notice) # Stop -> AwaitingInput (turn ended; waiting on the user) state="$1" diff --git a/crates/maestro-hooks/src/tests.rs b/crates/maestro-hooks/src/tests.rs index 79acd80..0d79533 100644 --- a/crates/maestro-hooks/src/tests.rs +++ b/crates/maestro-hooks/src/tests.rs @@ -7,7 +7,7 @@ use std::process::{Command, Stdio}; use super::*; -fn sh(script: &str, args: &[&str], envs: &[(&str, &str)]) -> std::process::Output { +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]; @@ -26,21 +26,48 @@ fn sh(script: &str, args: &[&str], envs: &[(&str, &str)]) -> std::process::Outpu .spawn() .expect("spawn /bin/sh"); // Feed a Claude-Code-style payload on stdin; the script must drain it. - child - .stdin - .take() - .unwrap() - .write_all(br#"{"session_id":"x"}"#) - .ok(); + 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], &[]); + 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); @@ -51,46 +78,48 @@ fn script_exits_silently_without_maestro_env() { /// activity endpoint and still exits 0. #[test] fn script_posts_mapped_state_to_the_activity_endpoint() { - let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); - let addr = listener.local_addr().unwrap(); - let server = move || { - let (mut sock, _) = listener.accept().unwrap(); - let mut buf = [0u8; 4096]; - let n = sock.read(&mut buf).unwrap(); - let raw = String::from_utf8_lossy(&buf[..n]).to_string(); - sock.write_all(b"HTTP/1.1 204 No Content\r\nContent-Length: 0\r\n\r\n") - .unwrap(); - raw - }; - let handle = std::thread::spawn(server); - - let url = format!("http://{addr}"); - let session = "01234567-89ab-cdef-0123-456789abcdef"; // The installer bakes the mapped state into the command's argv; the // script relays whatever it is given. - let out = sh( - HOOK_SCRIPT, - &["Active"], - &[("MAESTRO_URL", &url), ("MAESTRO_SESSION_ID", session)], - ); - assert!(out.status.success()); - - let request = handle.join().unwrap(); + 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.contains(&format!("/api/sessions/{SESSION}/activity")), "{request}" ); assert!(request.contains(r#""state":"Active""#), "{request}"); } -/// Every event maps to the state the issue specifies. +/// 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["Stop"], "AwaitingInput"); assert_eq!(map["Notification"], "AwaitingInput"); + assert_eq!(map["Stop"], "AwaitingInput"); +} + +/// 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. @@ -104,6 +133,7 @@ fn script_ignores_daemon_failure() { ("MAESTRO_URL", "http://127.0.0.1:1"), ("MAESTRO_SESSION_ID", "00000000-0000-0000-0000-000000000000"), ], + br#"{"session_id":"x"}"#, ); assert!(out.status.success()); } @@ -138,9 +168,9 @@ fn fresh_install_registers_every_event() { install(&settings_path(home), &script_path(home)).unwrap(); let cmds = installed_commands(&settings_path(home)); - assert_eq!(cmds.len(), 3); + assert_eq!(cmds.len(), HOOK_EVENTS.len()); let joined = cmds.join("\n"); - assert!(joined.contains("Active")); + assert_eq!(joined.matches("Active").count(), 2); assert_eq!(joined.matches("AwaitingInput").count(), 2); for cmd in &cmds { assert!(cmd.contains(SCRIPT_FILE_NAME)); @@ -158,7 +188,10 @@ fn install_is_idempotent() { 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(), 3); + assert_eq!( + installed_commands(&settings_path(home)).len(), + HOOK_EVENTS.len() + ); } /// An older maestro entry under the same events is updated in place, not @@ -188,7 +221,11 @@ fn install_updates_an_older_maestro_hook_in_place() { install(&settings, &new_script).unwrap(); let cmds = installed_commands(&settings); - assert_eq!(cmds.len(), 3, "old entry replaced, not kept: {cmds:?}"); + 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(); @@ -231,11 +268,11 @@ fn install_preserves_foreign_hooks_and_removes_stale_maestro_ones() { assert_eq!(pre[0]["hooks"][0]["command"], "my-own-linter.sh"); // Stale maestro event gone. assert!(root["hooks"].get("UserPromptSubmit").is_none()); - // Our three events present, nothing else maestro-related anywhere. + // Our events present, nothing else maestro-related anywhere. let cmds = installed_commands(&settings); assert_eq!( cmds.iter().filter(|c| c.contains(HOOK_MARKER)).count(), - 3, + HOOK_EVENTS.len(), "{cmds:?}" ); } From 6471da2624648652dd57fdcd7a8ccc2da183a21d Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Mon, 24 Aug 2026 22:54:21 +0100 Subject: [PATCH 36/37] feat(maestro): report StopFailure as Error with the error spelling as detail --- crates/maestro-hooks/src/lib.rs | 25 +++++++++++++++++----- crates/maestro-hooks/src/tests.rs | 35 +++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 5 deletions(-) diff --git a/crates/maestro-hooks/src/lib.rs b/crates/maestro-hooks/src/lib.rs index 1b70b11..fcd4359 100644 --- a/crates/maestro-hooks/src/lib.rs +++ b/crates/maestro-hooks/src/lib.rs @@ -27,8 +27,11 @@ 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). -pub const HOOK_EVENTS: [(&str, &str); 4] = [ +/// `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"), @@ -38,6 +41,9 @@ pub const HOOK_EVENTS: [(&str, &str); 4] = [ ("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. @@ -49,14 +55,23 @@ pub const HOOK_SCRIPT: &str = r#"#!/bin/sh # 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 -# Drain the hook payload so the parent never sees a broken pipe. -cat >/dev/null 2>&1 +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 "{\"state\":\"$state\"}" \ + -d "$body" \ "$MAESTRO_URL/api/sessions/$MAESTRO_SESSION_ID/activity" \ >/dev/null 2>&1 || true exit 0 diff --git a/crates/maestro-hooks/src/tests.rs b/crates/maestro-hooks/src/tests.rs index 0d79533..6f73169 100644 --- a/crates/maestro-hooks/src/tests.rs +++ b/crates/maestro-hooks/src/tests.rs @@ -104,6 +104,40 @@ fn event_state_mapping_matches_the_contract() { 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 @@ -171,6 +205,7 @@ fn fresh_install_registers_every_event() { 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)); From bbafff5a98b5fd0fd4efa88974a66a905405ba55 Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Mon, 24 Aug 2026 23:12:46 +0100 Subject: [PATCH 37/37] fix(maestro): add settle-finished observable for natural-exit test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The no-retained-store test asserts a negative — the tree survives — so waiting on that negative proves nothing: settle could still remove the tree one instant after the assertion passes. mark_session_dead now flips a per-record settled flag once the exit rule has been applied or refused, exposed as SessionManager::is_settled; the test waits on it before asserting. Refs #115 --- crates/maestro-daemon/src/session.rs | 26 +++++++++++++++++++ .../maestro-daemon/tests/session_manager.rs | 8 ++++++ 2 files changed, 34 insertions(+) diff --git a/crates/maestro-daemon/src/session.rs b/crates/maestro-daemon/src/session.rs index 9ea3942..7861155 100644 --- a/crates/maestro-daemon/src/session.rs +++ b/crates/maestro-daemon/src/session.rs @@ -62,6 +62,11 @@ pub struct SessionRecord { /// 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)] @@ -294,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 { @@ -429,6 +445,7 @@ impl SessionManager { worktree_branch: req.worktree.as_ref().map(|w| w.branch.clone()), bridge, reported_activity: None, + settled: false, }, ); } @@ -657,6 +674,7 @@ impl SessionManager { worktree_branch: None, bridge, reported_activity: None, + settled: false, }, ); let _ = self @@ -1069,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/tests/session_manager.rs b/crates/maestro-daemon/tests/session_manager.rs index 1ecc734..9bcc963 100644 --- a/crates/maestro-daemon/tests/session_manager.rs +++ b/crates/maestro-daemon/tests/session_manager.rs @@ -946,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"