From a503ec7c2b4719dab4765dcedd82664604ce40d6 Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Mon, 31 Aug 2026 14:51:24 +0100 Subject: [PATCH 1/2] refactor(maestro): drop per-session colour and pane glow The sidebar row dot, the pane top strip and the mobile pill dot all painted a hashed per-session colour; the focused pane added an outer accent glow on top of its ring. Removes the colour concept entirely along with the glow, leaving the flat accent ring as the only focus mark. --- web/src/catalog.ts | 12 +++-------- web/src/multiview.ts | 18 +++-------------- web/src/sessioncolor.ts | 23 --------------------- web/src/styles.css | 45 ++++------------------------------------- 4 files changed, 10 insertions(+), 88 deletions(-) delete mode 100644 web/src/sessioncolor.ts diff --git a/web/src/catalog.ts b/web/src/catalog.ts index 2f1a4f2..0d58035 100644 --- a/web/src/catalog.ts +++ b/web/src/catalog.ts @@ -327,12 +327,10 @@ export function renderTopbar(store: CatalogStore, onToggleSidebar?: () => void): } /** Two-tier sidebar highlight state (issue #33): which sessions are open in some - * pane (quiet), which one is focused (loud), and a stable per-session colour so - * a row maps to its pane. */ + * pane (quiet), and which one is focused (loud). */ export interface SidebarHighlights { open: Set; focused: string | null; - colors: Map; } /** Live handle to the persistent sidebar (built once, survives navigation). */ @@ -342,7 +340,7 @@ export interface SidebarHandle { /** Highlight the row for `id` (or clear it with `null`) as the focused session. * Back-compat shim over {@link setHighlights}. */ setActive(id: string | null): void; - /** Apply the full open/focused/colour highlight state. */ + /** Apply the full open/focused highlight state. */ setHighlights(h: SidebarHighlights): void; /** Tear down the store subscription + listeners. */ dispose(): void; @@ -451,7 +449,7 @@ export function renderSidebar(store: CatalogStore, opts: SidebarOptions = {}): S refresh(); }); - let highlights: SidebarHighlights = { open: new Set(), focused: null, colors: new Map() }; + let highlights: SidebarHighlights = { open: new Set(), focused: null }; const applyHighlights = () => { for (const n of treeHost.querySelectorAll(".tree-leaf")) { const id = n.dataset.sessionId; @@ -460,9 +458,6 @@ export function renderSidebar(store: CatalogStore, opts: SidebarOptions = {}): S const open = highlights.open.has(id); n.classList.toggle("is-active", focused); n.classList.toggle("is-open", open && !focused); - const color = highlights.colors.get(id); - if (color && (open || focused)) n.style.setProperty("--mv-color", color); - else n.style.removeProperty("--mv-color"); } }; @@ -515,7 +510,6 @@ export function renderSidebar(store: CatalogStore, opts: SidebarOptions = {}): S highlights = { open: id ? new Set([id]) : new Set(), focused: id, - colors: new Map(), }; applyHighlights(); }, diff --git a/web/src/multiview.ts b/web/src/multiview.ts index f19de89..3da26a8 100644 --- a/web/src/multiview.ts +++ b/web/src/multiview.ts @@ -22,7 +22,6 @@ import { removeLeaf, splitLeaf, } from "./layout.js"; -import { sessionColor } from "./sessioncolor.js"; import type { AttachInfo } from "./attach.js"; import { createSpawnForm, type SpawnFormHandle } from "./spawnForm.js"; @@ -97,12 +96,6 @@ export function createMultiView(opts: MultiViewOptions): MultiView { // ---- highlight helpers ---- - function colorMap(): Map { - const m = new Map(); - for (const id of openSessions(layout)) m.set(id, sessionColor(id)); - return m; - } - function focusedLeafSession(): string | null { const tab = activeTab(layout); const fl = tab.focusedLeafId ? findLeaf(tab.root, tab.focusedLeafId) : null; @@ -120,7 +113,6 @@ export function createMultiView(opts: MultiViewOptions): MultiView { sidebar.setHighlights({ open: openSessions(layout), focused: focusedLeafSession(), - colors: colorMap(), }); } @@ -142,10 +134,7 @@ export function createMultiView(opts: MultiViewOptions): MultiView { const pane = document.createElement("div"); pane.className = "mv-pane"; pane.dataset.leafId = leaf.id; - if (leaf.sessionId) { - pane.dataset.sessionId = leaf.sessionId; - pane.style.setProperty("--mv-color", sessionColor(leaf.sessionId)); - } + if (leaf.sessionId) pane.dataset.sessionId = leaf.sessionId; const host = document.createElement("div"); host.className = "mv-pane-host"; @@ -482,7 +471,7 @@ export function createMultiView(opts: MultiViewOptions): MultiView { pool.setVisible([]); pool.setFocus(null); canvas.appendChild(renderWelcome()); - sidebar.setHighlights({ open: openSessions(layout), focused: null, colors: colorMap() }); + sidebar.setHighlights({ open: openSessions(layout), focused: null }); persist(); return; } @@ -523,7 +512,6 @@ export function createMultiView(opts: MultiViewOptions): MultiView { pill.type = "button"; pill.className = "mv-pill" + (l.id === focused.id ? " is-active" : ""); pill.textContent = l.sessionId ? (infoFor(l.sessionId)?.name ?? l.sessionId.slice(0, 8)) : "empty"; - if (l.sessionId) pill.style.setProperty("--mv-color", sessionColor(l.sessionId)); pill.addEventListener("click", () => { tab.focusedLeafId = l.id; render(); @@ -543,7 +531,7 @@ export function createMultiView(opts: MultiViewOptions): MultiView { focused.sessionId ? [{ id: focused.sessionId, pane: pane.host, info: infoFor(focused.sessionId) }] : [], ); pool.setFocus(focused.sessionId ?? null); - sidebar.setHighlights({ open: openSessions(layout), focused: focused.sessionId ?? null, colors: colorMap() }); + sidebar.setHighlights({ open: openSessions(layout), focused: focused.sessionId ?? null }); } function closeSession(sessionId: string): void { diff --git a/web/src/sessioncolor.ts b/web/src/sessioncolor.ts deleted file mode 100644 index a61caab..0000000 --- a/web/src/sessioncolor.ts +++ /dev/null @@ -1,23 +0,0 @@ -// Stable per-session accent colour, used to map a sidebar row to the pane -// showing that session (issue #33). Hash the session id into a fixed OKLCH -// palette tuned to the maestro tokens; collisions are tolerated when there are -// more open sessions than hues — the colour is a hint, not an identity. - -import type { Uuid } from "./types.js"; - -const PALETTE = [ - "oklch(0.80 0.13 152)", // green (the house accent) - "oklch(0.80 0.12 90)", // amber - "oklch(0.76 0.13 30)", // coral - "oklch(0.78 0.11 210)", // teal - "oklch(0.74 0.13 265)", // blue - "oklch(0.76 0.14 320)", // magenta - "oklch(0.80 0.11 60)", // gold - "oklch(0.78 0.12 185)", // cyan -]; - -export function sessionColor(id: Uuid): string { - let h = 0; - for (let i = 0; i < id.length; i++) h = (h * 31 + id.charCodeAt(i)) >>> 0; - return PALETTE[h % PALETTE.length]; -} diff --git a/web/src/styles.css b/web/src/styles.css index ddb2e04..0f729a4 100644 --- a/web/src/styles.css +++ b/web/src/styles.css @@ -2301,20 +2301,10 @@ input.leaf-rename:focus { min-width: 0; min-height: 0; } -/* per-session colour strip — maps a pane to its sidebar row */ -.mv-pane::before { - content: ""; - position: absolute; - inset: 0 0 auto 0; - height: 2px; - background: var(--mv-color, transparent); - z-index: 3; - pointer-events: none; -} -/* focused pane: accent ring + glow, full strength */ +/* focused pane: accent ring, full strength */ .mv-pane.is-focused { border-color: var(--accent); - box-shadow: inset 0 0 0 1px var(--accent), 0 0 22px -8px var(--accent-ring); + box-shadow: inset 0 0 0 1px var(--accent); } /* unfocused panes recede so the eye finds the live one */ .mv-pane:not(.is-focused) { @@ -2417,31 +2407,12 @@ input.leaf-rename:focus { border-radius: var(--r-1); } -/* sidebar two-tier highlight + colour dot (issue #33). +/* sidebar two-tier highlight (issue #33). .is-active = focused pane (loud, from existing rule); .is-open = open - elsewhere (quiet). The dot uses the session's colour so a row maps to a pane. */ + elsewhere (quiet). */ .tree-leaf.is-open { background: color-mix(in oklab, var(--accent) 6%, transparent); } -.tree-leaf.is-open::after, -.tree-leaf.is-active::after { - content: ""; - position: absolute; - right: 9px; - top: 50%; - width: 7px; - height: 7px; - margin-top: -3.5px; - border-radius: 50%; - background: var(--mv-color, var(--accent)); - pointer-events: none; -} -.tree-leaf.is-open::after { - opacity: 0.7; -} -.tree-leaf.is-active::after { - box-shadow: 0 0 8px var(--mv-color, var(--accent-ring)); -} /* ========================================================================= multi-view mobile (#35): below 860px the split tree projects to a pill strip @@ -2486,14 +2457,6 @@ input.leaf-rename:focus { cursor: pointer; border: 1px solid transparent; } -.mv-pill::before { - content: ""; - width: 7px; - height: 7px; - border-radius: 50%; - background: var(--mv-color, var(--faint)); - flex: none; -} .mv-pill.is-active { background: var(--accent-tint); color: var(--accent-strong); From 1589d58ca0449fd0b89c484a9f810568724892fa Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Mon, 31 Aug 2026 14:54:54 +0100 Subject: [PATCH 2/2] feat(maestro): sidebar cards show diff stats and tool icons MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Line 2 of a session card spelled out its blocked work ("1 uncommitted file") and its command ("claude"). Both now render as marks: the breakdown as diff notation, and the command's leading program as its tool's icon in the web client. The probe carries the line delta of the tracked changes — staged and unstaged together, against HEAD — so `git add` never makes the counter drop. Untracked files stay a file count; counting their lines means reading every one off disk. A tracked change with no line delta falls back to `~N` rather than claiming `+0 -0`. The TUI keeps its command as text: a terminal cannot be assumed to have a font carrying tool glyphs. --- crates/maestro-daemon/src/worktree_probe.rs | 80 +++++++++++ crates/maestro-protocol/src/lib.rs | 105 ++++++++++++++- crates/maestro-tui/src/sidebar.rs | 86 +++++++++--- docs/SIDEBAR.md | 43 +++++- web/src/catalog.test.ts | 71 ++++++++-- web/src/catalog.ts | 30 ++++- web/src/mock.ts | 18 ++- web/src/styles.css | 40 ++++++ web/src/toolIcon.test.ts | 58 ++++++++ web/src/toolIcon.ts | 141 ++++++++++++++++++++ web/src/types.ts | 36 +++++ 11 files changed, 665 insertions(+), 43 deletions(-) create mode 100644 web/src/toolIcon.test.ts create mode 100644 web/src/toolIcon.ts diff --git a/crates/maestro-daemon/src/worktree_probe.rs b/crates/maestro-daemon/src/worktree_probe.rs index 440378d..0e732cc 100644 --- a/crates/maestro-daemon/src/worktree_probe.rs +++ b/crates/maestro-daemon/src/worktree_probe.rs @@ -45,6 +45,25 @@ pub fn in_the_way(path: &Path) -> Result { breakdown.uncommitted_files += 1; } } + // The line delta behind the sidebar's `+N −M` (docs/SIDEBAR.md). Diffing + // the working tree against HEAD covers staged and unstaged changes in one + // pass, so a `git add` never makes the counter drop. + let numstat = run_git(path, ["diff", "--numstat", "HEAD"]); + if let Ok(numstat) = numstat { + for line in String::from_utf8_lossy(&numstat.stdout).lines() { + let mut cols = line.split('\t'); + // A binary file reports `-` for both columns; it has no lines to + // add to either total. + let added: usize = cols.next().and_then(|c| c.parse().ok()).unwrap_or(0); + let removed: usize = cols.next().and_then(|c| c.parse().ok()).unwrap_or(0); + breakdown.insertions += added; + breakdown.deletions += removed; + } + } + // A repo with no commits yet has no HEAD to diff against, and the counts + // stay zero rather than failing the whole probe — the file counts above + // already reported the work. + // Resolved inside the worktree so a detached HEAD still has a starting // point; asking the base repo for `refs/heads/` fails there. let head = run_git(path, ["rev-parse", "--abbrev-ref", "HEAD"])?; @@ -210,6 +229,67 @@ mod tests { assert_eq!(dirty.untracked_files, 1); } + /// docs/SIDEBAR.md: the sidebar renders `+N −M`, so the probe carries the + /// line delta of the tracked changes — staged and unstaged together, + /// against HEAD. + #[test] + fn staged_and_unstaged_lines_both_count() { + let r = repo_with_worktree(); + std::fs::write(r.wt().join("staged.txt"), b"a\nb\n").unwrap(); + git(&r.wt(), &["add", "staged.txt"]); + // README loses its one line and gains three, unstaged. + std::fs::write(r.wt().join("README"), b"x\ny\nz\n").unwrap(); + + let breakdown = in_the_way(&r.wt()).expect("probe"); + assert_eq!(breakdown.insertions, 5, "2 staged + 3 unstaged"); + assert_eq!(breakdown.deletions, 1); + } + + /// Untracked files are counted as files and never as lines — reading each + /// one off disk is the cost the sidebar contract declines to pay. + #[test] + fn untracked_files_contribute_no_lines() { + let r = repo_with_worktree(); + std::fs::write(r.wt().join("scratch.txt"), b"a\nb\nc\n").unwrap(); + let breakdown = in_the_way(&r.wt()).expect("probe"); + assert_eq!(breakdown.untracked_files, 1); + assert_eq!(breakdown.insertions, 0); + assert_eq!(breakdown.deletions, 0); + } + + /// A binary file's numstat is `-\t-`. It must not poison the totals, and + /// it must not abort the probe either. + #[test] + fn binary_changes_add_no_lines() { + let r = repo_with_worktree(); + std::fs::write(r.wt().join("blob.bin"), [0u8, 159, 0, 12]).unwrap(); + git(&r.wt(), &["add", "blob.bin"]); + git(&r.wt(), &["commit", "-qm", "blob"]); + std::fs::write(r.wt().join("blob.bin"), [0u8, 1, 2, 3, 4]).unwrap(); + + let breakdown = in_the_way(&r.wt()).expect("probe"); + assert_eq!(breakdown.uncommitted_files, 1); + assert_eq!(breakdown.insertions, 0); + assert_eq!(breakdown.deletions, 0); + } + + /// A line delta alone never decides `is_clear` — the file counts do — so a + /// mode change with no line delta still blocks a kill. + #[test] + fn a_line_less_change_still_blocks() { + let r = repo_with_worktree(); + let script = r.wt().join("run.sh"); + std::fs::write(&script, b"echo hi\n").unwrap(); + git(&r.wt(), &["add", "run.sh"]); + git(&r.wt(), &["commit", "-qm", "script"]); + git(&r.wt(), &["update-index", "--chmod=+x", "run.sh"]); + + let breakdown = in_the_way(&r.wt()).expect("probe"); + assert_eq!(breakdown.insertions, 0); + assert_eq!(breakdown.deletions, 0); + assert!(!breakdown.is_clear()); + } + #[test] fn ignored_files_never_count() { let r = repo_with_worktree(); diff --git a/crates/maestro-protocol/src/lib.rs b/crates/maestro-protocol/src/lib.rs index 2dfbed6..2e6cdfa 100644 --- a/crates/maestro-protocol/src/lib.rs +++ b/crates/maestro-protocol/src/lib.rs @@ -236,12 +236,71 @@ pub struct InTheWay { /// Commits on the branch reachable from no remote-tracking ref. #[serde(default)] pub unpushed_commits: usize, + /// Lines added across the tracked changes — staged and unstaged alike, + /// measured against `HEAD`. Untracked files contribute nothing: counting + /// their lines means reading every one off disk, and they already have + /// their own count above (docs/SIDEBAR.md). + #[serde(default)] + pub insertions: usize, + /// Lines removed across the same tracked changes. + #[serde(default)] + pub deletions: usize, } impl InTheWay { /// Nothing blocks any more — the work has since been committed and pushed. + /// + /// The line counts are not consulted: they describe the same tracked + /// changes `uncommitted_files` counts, and a change with no line delta — + /// a mode change, a rename — still blocks. pub fn is_clear(&self) -> bool { - *self == Self::default() + self.uncommitted_files == 0 && self.untracked_files == 0 && self.unpushed_commits == 0 + } +} + +/// Which fragment of the compact blocked-work stat a value is, and so which +/// colour the client paints it (docs/SIDEBAR.md). +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum StatKind { + /// Lines added. + Added, + /// Lines removed. + Removed, + /// Changed tracked files, when the change carries no line delta. + Files, + /// Untracked files. + Untracked, + /// Unpushed commits. + Unpushed, +} + +impl InTheWay { + /// The breakdown as diff notation: `+124`, `-18`, `?3`, `↑2`. + /// + /// The line counts cover tracked changes only, so a tree whose only + /// tracked change carries no line delta — a mode change, a rename — + /// reports the file count (`~2`) rather than claiming `+0 -0`. Both + /// sidebars render exactly these fragments; [`Display`](std::fmt::Display) + /// remains the spelled-out phrasing for the kill refusal and the tooltip. + pub fn stats(&self) -> Vec<(StatKind, String)> { + let mut out = Vec::new(); + if self.insertions > 0 || self.deletions > 0 { + if self.insertions > 0 { + out.push((StatKind::Added, format!("+{}", self.insertions))); + } + if self.deletions > 0 { + out.push((StatKind::Removed, format!("-{}", self.deletions))); + } + } else if self.uncommitted_files > 0 { + out.push((StatKind::Files, format!("~{}", self.uncommitted_files))); + } + if self.untracked_files > 0 { + out.push((StatKind::Untracked, format!("?{}", self.untracked_files))); + } + if self.unpushed_commits > 0 { + out.push((StatKind::Unpushed, format!("↑{}", self.unpushed_commits))); + } + out } } @@ -544,6 +603,50 @@ mod tests { assert!(is_compatible("unknown")); } + /// docs/SIDEBAR.md: the sidebars render the breakdown as diff notation. + #[test] + fn stats_read_as_a_diff() { + let w = InTheWay { + uncommitted_files: 3, + untracked_files: 2, + unpushed_commits: 1, + insertions: 124, + deletions: 18, + }; + let text: Vec = w.stats().into_iter().map(|(_, t)| t).collect(); + assert_eq!(text, ["+124", "-18", "?2", "↑1"]); + let kinds: Vec = w.stats().into_iter().map(|(k, _)| k).collect(); + assert_eq!( + kinds, + [ + StatKind::Added, + StatKind::Removed, + StatKind::Untracked, + StatKind::Unpushed + ] + ); + } + + /// A change with no line delta — a mode change — still has to say + /// something, and `+0 -0` would be a lie about the tree. + #[test] + fn stats_fall_back_to_the_file_count_without_a_line_delta() { + let w = InTheWay { + uncommitted_files: 2, + ..InTheWay::default() + }; + let text: Vec = w.stats().into_iter().map(|(_, t)| t).collect(); + assert_eq!(text, ["~2"]); + assert!(!w.is_clear()); + } + + /// A clear tree says nothing at all — the fragment is omitted, not empty. + #[test] + fn a_clear_tree_has_no_stats() { + assert!(InTheWay::default().stats().is_empty()); + assert!(InTheWay::default().is_clear()); + } + fn round_trip(value: &T) -> T where T: Serialize + for<'de> Deserialize<'de>, diff --git a/crates/maestro-tui/src/sidebar.rs b/crates/maestro-tui/src/sidebar.rs index 5b37b09..5e79422 100644 --- a/crates/maestro-tui/src/sidebar.rs +++ b/crates/maestro-tui/src/sidebar.rs @@ -9,7 +9,7 @@ use std::path::PathBuf; use chrono::Utc; use maestro_protocol::display::{abbreviate_dir, abbreviate_folder_name, session_tree_path}; -use maestro_protocol::{ActivityState, Session, SessionStatus}; +use maestro_protocol::{ActivityState, Session, SessionStatus, StatKind}; use ratatui::layout::Rect; use ratatui::style::{Color, Modifier, Style}; use ratatui::text::{Line, Span}; @@ -283,7 +283,16 @@ impl Sidebar { } let mut tail: Vec> = Vec::new(); if let Some(b) = blocked { - tail.push(Span::styled(format!(" ⚠ {b}"), muted)); + let palette = palette::active(); + let surface = palette.surface(highlighted && focused); + for (kind, text) in b.stats() { + let style = match kind { + StatKind::Added => muted.fg(surface.active), + StatKind::Removed => muted.fg(surface.error), + _ => muted, + }; + tail.push(Span::styled(format!(" {text}"), style)); + } } if dead { tail.push(Span::styled(" exited", muted)); @@ -627,14 +636,18 @@ mod tests { } /// #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. + /// on the name row, and reads as diff notation; a clear or unprobed + /// worktree says nothing. #[test] - fn blocked_session_warns_on_its_meta_line() { + fn blocked_session_states_its_diff_on_the_meta_line() { let mut blocked = session_named(1, "claude"); blocked.activity_since = Utc::now() - Duration::seconds(5); blocked.blocked_work = Some(InTheWay { + uncommitted_files: 2, + untracked_files: 3, unpushed_commits: 4, - ..InTheWay::default() + insertions: 124, + deletions: 18, }); let mut clear = session_named(2, "bash"); clear.blocked_work = Some(InTheWay::default()); @@ -644,31 +657,66 @@ mod tests { highlight: None, attached: Default::default(), }; - let all = rows(&draw(&sb, 40, 10)); + let all = rows(&draw(&sb, 44, 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:?}" - ); + .find(|l| l.contains("+124")) + .expect("blocked card must state its diff somewhere"); + for fragment in ["-18", "?3", "↑4"] { + assert!( + meta.contains(fragment), + "meta line must carry {fragment}: {meta:?}" + ); + } let name_row = row_containing(&all, "claude"); assert!( - !name_row.contains('⚠'), - "warning moved off the name row: {name_row:?}" + !name_row.contains("+124"), + "the diff stays off the name row: {name_row:?}" ); for name in ["bash", "vim"] { let idx = all.iter().position(|l| l.contains(name)).unwrap(); assert!( - !all[idx + 1].contains('⚠'), - "{name} must not warn: {:?}", + !all[idx + 1].contains('+') && !all[idx + 1].contains('?'), + "{name} must not state a diff: {:?}", all[idx + 1] ); } } + /// The additions and deletions are the only coloured fragments: they are + /// what a reader scans for, and colouring the rest would make the line + /// compete with the activity dot. + #[test] + fn the_diff_colours_only_its_additions_and_deletions() { + let mut s = session_named(1, "claude"); + s.blocked_work = Some(InTheWay { + uncommitted_files: 1, + untracked_files: 0, + unpushed_commits: 2, + insertions: 7, + deletions: 3, + }); + let sb = Sidebar { + sessions: vec![s], + highlight: None, + attached: Default::default(), + }; + let buf = draw(&sb, 44, 8); + let palette = palette::active(); + let surface = palette.surface(false); + let colour_of = |needle: char| { + (0..buf.area.width) + .flat_map(|x| (0..buf.area.height).map(move |y| (x, y))) + .find(|&(x, y)| buf[(x, y)].symbol() == needle.to_string()) + .map(|(x, y)| buf[(x, y)].fg) + .expect("fragment must be drawn") + }; + assert_eq!(colour_of('+'), surface.active); + assert_eq!(colour_of('-'), surface.error); + assert_eq!(colour_of('↑'), surface.muted); + } + /// A fresh session still gets its unconditional second line, but no /// duration until it has been held in state for 30 seconds. #[test] @@ -929,7 +977,7 @@ mod tests { let meta = content(&all[idx + 1]); assert!(meta.contains('…'), "truncated detail ends with …: {meta:?}"); assert!( - meta.contains("2 unpushed commits"), + meta.contains("↑2"), "blocked work stays on the line: {meta:?}" ); assert!( @@ -1062,7 +1110,7 @@ mod tests { attached: Default::default(), }; let all = rows(&draw(&sb, 29, 6)); - assert!(all[3].contains("⚠ 1 uncommitted file"), "{:?}", all[3]); + assert!(all[3].contains("~1"), "{:?}", all[3]); let meta = content(&all[3]); assert!(meta.ends_with('…'), "command must truncate: {meta:?}"); } @@ -1081,7 +1129,7 @@ mod tests { attached: Default::default(), }; let all = rows(&draw(&sb, 40, 6)); - assert!(all[3].contains("⚠ 1 uncommitted file"), "{:?}", all[3]); + assert!(all[3].contains("~1"), "{:?}", all[3]); assert!( all[3].contains("bash"), "command must survive: {:?}", diff --git a/docs/SIDEBAR.md b/docs/SIDEBAR.md index b2f7bc0..c4c4e30 100644 --- a/docs/SIDEBAR.md +++ b/docs/SIDEBAR.md @@ -37,7 +37,7 @@ itself is not. Line 2 reads left to right: ``` - [] [] [ | exited] + [] [] [ | exited] ``` Its minimum content is therefore the dot and the command. @@ -64,12 +64,46 @@ 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 the blocked-work breakdown (#83) in diff notation, replacing +the spelled-out `⚠ 3 uncommitted files, 2 unpushed commits`. It renders only +while something would block an unforced kill, and reads: + +``` ++ - ? ↑ +``` + +Each fragment is omitted when its count is zero. The line counts cover +**tracked** changes only — staged and unstaged together, measured against +`HEAD` — so a `git add` never makes the counter drop. Untracked files are +counted as files, not lines: counting their lines means reading every one off +disk, and `?3` already says what is there. A tree whose only tracked change +carries no line delta — a mode change, a rename — shows `~` +instead of the additions and deletions, since `+0 -0` would describe it +wrongly. + +Only `+` and `-` take colour (`--live` / `--danger` in the web client, the +surface's `active` / `error` roles in the TUI); `~`, `?` and `↑` stay muted, so +the fragment reads as a diff rather than as an alarm. The spelled-out phrasing +of `maestro_protocol::InTheWay`'s `Display` does not disappear: it is the web +card's tooltip and the wording of the kill refusal, so no two clients describe +the same tree differently. + `` is unconditional and muted: the session's command line, shown on every card in both clients — no omission in the TUI, no name-vs-command comparison gating it. A session named by its command line then shows the same string on both lines; that is accepted, the price of a row that never changes height. +In the **web client** the command's leading program is replaced by that tool's +icon when it is one we have a mark for — Claude Code, Codex, opencode, Gemini +CLI, Aider, Cursor, pi, and any shell — and only the remaining words of the +command line follow as text. Recognition looks through a path, leading +`FOO=bar` assignments and wrappers (`sudo`, `env`, `npx`, `pnpm dlx`, …); an +unrecognised program keeps its full text, so the card never shows a mark +standing for the wrong tool. The full command line stays in the card's tooltip +either way. The **TUI** keeps the command as text: a terminal cannot be assumed +to have a font carrying tool glyphs. + ## Colour per activity state The state word is never spelled out — the colour carries it. @@ -95,9 +129,10 @@ surface; a colour chosen for the card background is never painted onto the selection background. Contrast is a property of the palette (values are RGB) and is asserted in the palette's tests, not left to the terminal theme. -The duration text inherits its state's colour; the blocked-work, exited and -command fragments use the surface's `muted` role, since the ⚠ glyph already -signals severity. +The duration text inherits its state's colour; the exited and command +fragments use the surface's `muted` role, as do the diff stat's `~`, `?` and +`↑` fragments. Only `+` and `-` step outside the surface's muted role, and they +take the `active` and `error` roles of the same surface. ## Duration format diff --git a/web/src/catalog.test.ts b/web/src/catalog.test.ts index 8990206..00afeeb 100644 --- a/web/src/catalog.test.ts +++ b/web/src/catalog.test.ts @@ -47,15 +47,33 @@ function rowFor(el: HTMLElement, id: string): HTMLElement | null { } describe("blocked-work indicator (#83)", () => { - it("marks the row of a blocked session with the shared phrasing", () => { + it("marks the row of a blocked session in diff notation", () => { const blocked = session({ id: "aaaaaaaa-0000-0000-0000-000000000001", - blocked_work: { uncommitted_files: 3, unpushed_commits: 2 }, + blocked_work: { + uncommitted_files: 3, + unpushed_commits: 2, + insertions: 124, + deletions: 18, + }, }); const el = renderSidebar(storeWith([blocked])).el; const badge = rowFor(el, blocked.id)?.querySelector(".leaf-blocked"); - expect(badge?.textContent).toContain("⚠"); - expect(badge?.textContent).toContain("3 uncommitted files, 2 unpushed commits"); + expect(badge?.textContent).toBe("+124-18↑2"); + // The spelled-out breakdown survives in the tooltip. + expect(badge?.getAttribute("title")).toContain( + "3 uncommitted files, 2 unpushed commits", + ); + }); + + it("falls back to the file count when tracked changes carry no lines", () => { + const blocked = session({ + id: "aaaaaaaa-0000-0000-0000-00000000001a", + blocked_work: { uncommitted_files: 2, untracked_files: 1 }, + }); + const el = renderSidebar(storeWith([blocked])).el; + const badge = rowFor(el, blocked.id)?.querySelector(".leaf-blocked"); + expect(badge?.textContent).toBe("~2?1"); }); it("does not mark clear or unprobed sessions", () => { @@ -161,7 +179,7 @@ describe("session cards (#107)", () => { expect(line2).not.toBeNull(); expect(line2?.querySelector(".activity-dot")).not.toBeNull(); expect(line2?.querySelector(".activity-duration")).toBeNull(); - expect(line2?.querySelector(".leaf-cmd")?.textContent).toBe("claude"); + expect(line2?.querySelector(".leaf-cmd")).not.toBeNull(); }); it("shows dot + floored duration once the state has been held past 30s", () => { @@ -177,14 +195,40 @@ describe("session cards (#107)", () => { expect(line2?.querySelector(".activity-duration")?.textContent).toBe("4m"); }); - it("renders the command when the session name equals it", () => { + it("renders the command as its tool icon when the tool is known", () => { const s = session({ id: "aaaaaaaa-0000-0000-0000-00000000001b", command: "claude", metadata: { name: "claude" }, }); const el = renderSidebar(storeWith([s])).el; - expect(rowFor(el, s.id)?.querySelector(".card-line2 .leaf-cmd")?.textContent).toBe("claude"); + const cmd = rowFor(el, s.id)?.querySelector(".card-line2 .leaf-cmd"); + expect(cmd?.querySelector(".tool-icon")?.getAttribute("data-tool")).toBe("claude"); + expect(cmd?.getAttribute("title")).toBe("claude"); + // The icon said it all: no text left over. + expect(cmd?.querySelector(".leaf-cmd-rest")).toBeNull(); + }); + + it("keeps the command's arguments as text beside the icon", () => { + const s = session({ + id: "aaaaaaaa-0000-0000-0000-00000000001c", + command: "claude --resume", + }); + const el = renderSidebar(storeWith([s])).el; + const cmd = rowFor(el, s.id)?.querySelector(".card-line2 .leaf-cmd"); + expect(cmd?.querySelector(".tool-icon")?.getAttribute("data-tool")).toBe("claude"); + expect(cmd?.querySelector(".leaf-cmd-rest")?.textContent).toBe("--resume"); + }); + + it("keeps an unknown command as plain text", () => { + const s = session({ + id: "aaaaaaaa-0000-0000-0000-00000000001d", + command: "hx src/main.rs", + }); + const el = renderSidebar(storeWith([s])).el; + const cmd = rowFor(el, s.id)?.querySelector(".card-line2 .leaf-cmd"); + expect(cmd?.querySelector(".tool-icon")).toBeNull(); + expect(cmd?.querySelector(".leaf-cmd-rest")?.textContent).toBe("hx src/main.rs"); }); it("maps each activity state to its contract slug", () => { @@ -222,15 +266,20 @@ describe("session cards (#107)", () => { expect(line2?.querySelector(".activity-duration")?.textContent).toBe("2m"); }); - it("moves blocked work onto line 2 in the shared phrasing", () => { + it("moves blocked work onto line 2 as diff notation", () => { const s = session({ id: "aaaaaaaa-0000-0000-0000-000000000017", - blocked_work: { uncommitted_files: 3, unpushed_commits: 2 }, + blocked_work: { + uncommitted_files: 3, + unpushed_commits: 2, + insertions: 124, + deletions: 18, + }, }); 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"); + expect(line2?.textContent).toContain("+124"); + expect(line2?.textContent).toContain("-18"); // Line 1 carries no badges. expect(rowFor(el, s.id)?.querySelector(".leaf-badge")).toBeNull(); }); diff --git a/web/src/catalog.ts b/web/src/catalog.ts index febb706..65d8efd 100644 --- a/web/src/catalog.ts +++ b/web/src/catalog.ts @@ -2,9 +2,10 @@ import { escapeHtml } from "./html.js"; import { getDaemonInfo, killSession, renameSession } from "./api.js"; import { openContextMenu } from "./contextMenu.js"; import { CatalogStore } from "./events.js"; -import type { Session } from "./types.js"; +import type { InTheWay, Session } from "./types.js"; import { abbreviateFolderName, + blockerStats, customSessionName, formatInTheWay, MAX_SESSION_NAME_LEN, @@ -12,6 +13,7 @@ import { sessionBlockers, sessionName, } from "./types.js"; +import { splitCommand, toolIconSvg } from "./toolIcon.js"; import { activityColorSlug, activityDurationLabel, @@ -60,15 +62,33 @@ function metaLine(s: Session, now: number): string { s.activity_detail ? `${escapeHtml(s.activity_detail)}` : "", - blockers - ? `⚠ ${escapeHtml(formatInTheWay(blockers))}` - : "", + blockers ? blockedFragment(blockers) : "", dead ? `exited` : "", - `${escapeHtml(s.command)}`, + commandFragment(s.command), ]; return `${fragments.join("")}`; } +/** docs/SIDEBAR.md: blocked work renders as diff notation — `+124 -18 ?3 ↑2` + * — with the spelled-out breakdown kept in the tooltip. */ +function blockedFragment(blockers: InTheWay): string { + const stats = blockerStats(blockers) + .map((st) => `${escapeHtml(st.text)}`) + .join(""); + const title = `Kill will be refused unless forced — ${formatInTheWay(blockers)}`; + return `${stats}`; +} + +/** docs/SIDEBAR.md: the command's leading program becomes its tool icon when + * we have a mark for it, and only the remaining words stay as text. The full + * command line stays in the tooltip either way. */ +function commandFragment(command: string): string { + const { tool, rest } = splitCommand(command); + const icon = tool ? toolIconSvg(tool) : null; + const text = icon ? rest : command; + return `${icon ?? ""}${text ? `${escapeHtml(text)}` : ""}`; +} + function renderNode(node: TreeNode, depth: number): HTMLElement { if (node.kind === "folder") { const wrap = document.createElement("div"); diff --git a/web/src/mock.ts b/web/src/mock.ts index dc9fc6f..597f958 100644 --- a/web/src/mock.ts +++ b/web/src/mock.ts @@ -14,6 +14,7 @@ function sess( status: "Running" | "Dead", cwd: string, name?: string, + blocked_work?: Session["blocked_work"], ): Session { const seed = parseInt(id.slice(-1), 16) || 0; return { @@ -30,6 +31,7 @@ function sess( last_activity_at: new Date(Date.now() - seed * 11_000).toISOString(), // The session tree nests by current_cwd; default it to the spawn dir. current_cwd: cwd, + blocked_work, }; } @@ -109,16 +111,26 @@ function stubSpawnApi(): void { export function seedMock(store: CatalogStore): void { const sessions: Session[] = [ // ~/git/maestro/web/* — deep dir chain; "~/git/maestro" collapses to one node. - sess("a1b2c3d4-0000-0000-0000-000000000001", "claude", "Running", "/home/dev/git/maestro/web", "redesign"), + sess("a1b2c3d4-0000-0000-0000-000000000001", "claude", "Running", "/home/dev/git/maestro/web", "redesign", { + uncommitted_files: 4, + untracked_files: 2, + unpushed_commits: 1, + insertions: 312, + deletions: 47, + }), sess("a1b2c3d4-0000-0000-0000-000000000002", "vite dev", "Running", "/home/dev/git/maestro/web", "dev-server"), sess("a1b2c3d4-0000-0000-0000-000000000003", "bash", "Dead", "/home/dev/git/maestro/web", "scratch"), - sess("a1b2c3d4-0000-0000-0000-000000000004", "claude", "Running", "/home/dev/git/maestro/web/tests", "e2e-fixes"), + sess("a1b2c3d4-0000-0000-0000-000000000004", "codex --resume", "Running", "/home/dev/git/maestro/web/tests", "e2e-fixes", { + uncommitted_files: 1, + }), // ~/git/maestro/crates/* — sibling subtree under the same repo. sess("a1b2c3d4-0000-0000-0000-000000000005", "claude", "Running", "/home/dev/git/maestro/crates/daemon", "daemon-work"), sess("a1b2c3d4-0000-0000-0000-000000000006", "cargo watch -x test", "Running", "/home/dev/git/maestro/crates/daemon", "test-loop"), sess("a1b2c3d4-0000-0000-0000-000000000007", "cargo run -- daemon", "Running", "/home/dev/git/maestro/crates/daemon", "daemon"), // ~/work/acme/* — a separate top-level dir on the same host. - sess("a1b2c3d4-0000-0000-0000-000000000008", "claude", "Running", "/home/dev/work/acme/api", "billing-api"), + sess("a1b2c3d4-0000-0000-0000-000000000008", "gemini", "Running", "/home/dev/work/acme/api", "billing-api", { + untracked_files: 3, + }), sess("a1b2c3d4-0000-0000-0000-000000000009", "pnpm dev", "Dead", "/home/dev/work/acme/web", "dashboard"), // ~/git/dotfiles — single session, fully collapsed chain. sess("a1b2c3d4-0000-0000-0000-000000000010", "opencode", "Running", "/home/dev/git/dotfiles", "dotfiles"), diff --git a/web/src/styles.css b/web/src/styles.css index 67fabfb..cca53fd 100644 --- a/web/src/styles.css +++ b/web/src/styles.css @@ -940,6 +940,46 @@ input.leaf-rename:focus { truncation target when even the complete trailing set cannot fit. */ .leaf-cmd { flex: 0 1 auto; + display: inline-flex; + align-items: center; + gap: 4px; + min-width: 0; +} + +/* The tool's mark stands in for the program the command line starts with + (docs/SIDEBAR.md). It keeps its own colours where the tool has brand + colours, and inherits the muted fragment colour where it does not. */ +.tool-icon { + flex: 0 0 auto; + opacity: 0.9; +} + +.leaf-cmd-rest { + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; +} + +/* Blocked work as diff notation: `+124 -18 ?3 ↑2` (docs/SIDEBAR.md). The + spelled-out breakdown is the tooltip; only the additions and deletions take + colour, so the ⚠-less fragment still reads as a diff and not as an alarm. */ +.leaf-blocked { + display: inline-flex; + align-items: center; + gap: 4px; + font-variant-numeric: tabular-nums; +} + +.blocked-stat[data-stat="add"] { + color: var(--live); +} +.blocked-stat[data-stat="del"] { + color: var(--danger); +} +.blocked-stat[data-stat="files"], +.blocked-stat[data-stat="untracked"], +.blocked-stat[data-stat="unpushed"] { + color: var(--faint); } /* Actions live in line 1's flow at its right edge — never overlaid on the diff --git a/web/src/toolIcon.test.ts b/web/src/toolIcon.test.ts new file mode 100644 index 0000000..0af0745 --- /dev/null +++ b/web/src/toolIcon.test.ts @@ -0,0 +1,58 @@ +import { describe, expect, it } from "vitest"; +import { splitCommand, toolIconSvg } from "./toolIcon.js"; + +describe("tool icons on the command fragment (docs/SIDEBAR.md)", () => { + it("recognises the agents and shells sessions are spawned with", () => { + const cases: Array<[string, string]> = [ + ["claude", "claude"], + ["codex", "codex"], + ["opencode", "opencode"], + ["gemini", "gemini"], + ["aider", "aider"], + ["cursor-agent", "cursor"], + ["pi", "pi"], + ["zsh", "shell"], + ["bash", "shell"], + ["fish", "shell"], + ]; + for (const [command, tool] of cases) { + expect(splitCommand(command).tool, command).toBe(tool); + } + }); + + it("keeps everything after the program as text", () => { + expect(splitCommand("claude --resume -p hi")).toEqual({ + tool: "claude", + rest: "--resume -p hi", + }); + expect(splitCommand(" zsh -l ")).toEqual({ tool: "shell", rest: "-l" }); + }); + + it("looks through paths, env assignments and wrappers", () => { + expect(splitCommand("/usr/local/bin/claude").tool).toBe("claude"); + expect(splitCommand("FOO=1 BAR=2 claude").tool).toBe("claude"); + expect(splitCommand("npx opencode").tool).toBe("opencode"); + expect(splitCommand("pnpm dlx opencode run").tool).toBe("opencode"); + expect(splitCommand("pnpm dlx opencode run").rest).toBe("run"); + }); + + it("leaves an unrecognised program as text, mark-less", () => { + expect(splitCommand("hx src/main.rs")).toEqual({ + tool: null, + rest: "hx src/main.rs", + }); + // A wrapper with nothing after it is not a tool either. + expect(splitCommand("sudo").tool).toBeNull(); + expect(splitCommand("").tool).toBeNull(); + }); + + it("gives every recognised tool a labelled mark", () => { + for (const tool of ["claude", "codex", "opencode", "gemini", "aider", "cursor", "pi", "shell"]) { + const svg = toolIconSvg(tool); + expect(svg, tool).toContain(" = { + claude: "claude", + "claude-code": "claude", + codex: "codex", + opencode: "opencode", + gemini: "gemini", + aider: "aider", + cursor: "cursor", + "cursor-agent": "cursor", + pi: "pi", + bash: "shell", + zsh: "shell", + sh: "shell", + fish: "shell", + dash: "shell", + ksh: "shell", + nu: "shell", + xonsh: "shell", +}; + +/** Human name behind an icon id, for the tooltip and the aria label. */ +export const TOOL_LABELS: Record = { + claude: "Claude Code", + codex: "Codex", + opencode: "opencode", + gemini: "Gemini CLI", + aider: "Aider", + cursor: "Cursor", + pi: "pi", + shell: "Shell", +}; + +export interface CommandParts { + /** Icon id, or `null` when the program is not one we have a mark for. */ + tool: string | null; + /** What is left of the command line once the icon stands for the program. + * Empty when the icon says everything the command line said. */ + rest: string; +} + +/** Split a command line into the tool its icon stands for and the remainder. + * + * `claude --resume` → `{tool: "claude", rest: "--resume"}`; an unrecognised + * `hx src/` keeps the whole line as `rest` with no tool. */ +export function splitCommand(command: string): CommandParts { + const words = command.trim().split(/\s+/).filter(Boolean); + let i = 0; + // Leading `FOO=bar` assignments and wrappers stand in front of the real + // program; `pnpm dlx x` / `yarn dlx x` need one more word skipped. + while (i < words.length) { + const word = words[i]; + if (/^[A-Za-z_][A-Za-z0-9_]*=/.test(word)) { + i += 1; + continue; + } + const program = basename(word); + if (WRAPPERS.has(program)) { + i += 1; + if ((program === "pnpm" || program === "yarn") && words[i] === "dlx") i += 1; + continue; + } + break; + } + if (i >= words.length) return { tool: null, rest: command.trim() }; + const tool = BY_PROGRAM[basename(words[i])] ?? null; + if (!tool) return { tool: null, rest: command.trim() }; + return { tool, rest: words.slice(i + 1).join(" ") }; +} + +function basename(word: string): string { + const cut = word.lastIndexOf("/"); + return (cut >= 0 ? word.slice(cut + 1) : word).toLowerCase(); +} + +/** Anthropic's burst: tapered rays around a centre, long ones on the axes. */ +function burst(): string { + const rays = Array.from({ length: 8 }, (_, k) => { + const long = k % 2 === 0; + const tip = long ? 1.1 : 3.4; + const waist = long ? 6.3 : 6.6; + const half = long ? 1.05 : 0.85; + return ``; + }).join(""); + return `${rays}`; +} + +const SVGS: Record = { + claude: burst(), + // A hexafoil knot in the shape of OpenAI's mark. The petals stay thin and + // wide apart: three fatter loops merge into a blob at 13px. + codex: ` + + + + `, + gemini: ``, + // An isometric cube, the shape Cursor's mark is built from. + cursor: ` + + + + `, + shell: ` + + + `, + opencode: monogram("oc"), + aider: monogram("ai"), + pi: monogram("π"), +}; + +/** Tools whose own mark is a wordmark get their initials instead — inventing + * a logo for them would be worse than saying which letters they start with. + * No chip around the letters: a border this small eats the height the letters + * need to stay apart. */ +function monogram(text: string): string { + return `${text}`; +} + +/** The `` for an icon id, or `null` when the id has no mark. */ +export function toolIconSvg(tool: string): string | null { + const body = SVGS[tool]; + if (!body) return null; + const label = TOOL_LABELS[tool] ?? tool; + return `${label}${body}`; +} diff --git a/web/src/types.ts b/web/src/types.ts index 3e95848..c86ceb8 100644 --- a/web/src/types.ts +++ b/web/src/types.ts @@ -164,6 +164,42 @@ export interface InTheWay { uncommitted_files?: number; untracked_files?: number; unpushed_commits?: number; + /** Lines added across the tracked changes, staged and unstaged together. */ + insertions?: number; + /** Lines removed across the same tracked changes. */ + deletions?: number; +} + +/** One fragment of the sidebar's compact blocked-work stat (docs/SIDEBAR.md). + * `kind` picks the colour; `text` is what the client prints. */ +export interface BlockerStat { + kind: "add" | "del" | "files" | "untracked" | "unpushed"; + text: string; +} + +/** The blocked-work breakdown as diff notation: `+124 -18 ?3 ↑2`. + * + * The line counts cover tracked changes only, so a tree whose only tracked + * change carries no line delta — a mode change, a rename — falls back to the + * file count (`~2`) rather than claiming `+0 -0`. Both clients render exactly + * these fragments; the spelled-out {@link formatInTheWay} survives in the + * tooltip and in the kill refusal. */ +export function blockerStats(w: InTheWay): BlockerStat[] { + const stats: BlockerStat[] = []; + const added = w.insertions ?? 0; + const removed = w.deletions ?? 0; + const changed = w.uncommitted_files ?? 0; + if (added > 0 || removed > 0) { + if (added > 0) stats.push({ kind: "add", text: `+${added}` }); + if (removed > 0) stats.push({ kind: "del", text: `-${removed}` }); + } else if (changed > 0) { + stats.push({ kind: "files", text: `~${changed}` }); + } + if ((w.untracked_files ?? 0) > 0) + stats.push({ kind: "untracked", text: `?${w.untracked_files}` }); + if ((w.unpushed_commits ?? 0) > 0) + stats.push({ kind: "unpushed", text: `\u2191${w.unpushed_commits}` }); + return stats; } /** The phrasing `maestro_protocol::InTheWay`'s Display impl uses — shared by