diff --git a/extensions/shared/activity-status.test.ts b/extensions/shared/activity-status.test.ts index 4fc7adae..44179dd1 100644 --- a/extensions/shared/activity-status.test.ts +++ b/extensions/shared/activity-status.test.ts @@ -60,7 +60,7 @@ test("status text names its own view command", () => { done: 2, failed: 0, }), - "subagents: ■ 1 running · ■ 2 done · /subagents to view", + "subagents: ● 1 running · ✓ 2 done · /subagents to view", ); assert.equal( formatActivityStatus(identityTheme, "workflows", { @@ -68,6 +68,28 @@ test("status text names its own view command", () => { done: 0, failed: 3, }), - "workflows: ■ 3 failed · /workflows to view", + "workflows: ✗ 3 failed · /workflows to view", ); }); + +test("the pushed footer status uses a still marker; strip visibility owns the hint", () => { + const counts = { running: 1, done: 0, failed: 0 }; + // setStatus stores a finished string, so this line cannot animate: a spinner + // frame would freeze on whatever event happened to write it last. + const visible = formatActivityStatus( + identityTheme, + "subagents", + counts, + true, + ); + assert.equal(visible, "subagents: ● 1 running"); + assert.doesNotMatch(visible, /to view/); + + const hidden = formatActivityStatus( + identityTheme, + "subagents", + counts, + false, + ); + assert.equal(hidden, "subagents: ● 1 running · /subagents to view"); +}); diff --git a/extensions/shared/activity-status.ts b/extensions/shared/activity-status.ts index 1baf1048..f9ee86b4 100644 --- a/extensions/shared/activity-status.ts +++ b/extensions/shared/activity-status.ts @@ -1,5 +1,12 @@ import type { ExtensionContext } from "@earendil-works/pi-coding-agent"; +/** + * Still marker for the push-based footer status. It deliberately does not + * import the subagent spinner: `shared/` must not depend on a single + * extension's internals, and a pushed string cannot animate anyway. + */ +const RUNNING_MARK = "●"; + type Theme = ExtensionContext["ui"]["theme"]; export interface ActivityCounts { @@ -8,8 +15,6 @@ export interface ActivityCounts { failed: number; } -const SQUARE = "■"; - /** * Settled work is an unread notice, not a session tally: `done`/`failed` stay * visible until the user's next explicit request acknowledges them, while @@ -44,22 +49,34 @@ export function hasActivity(counts: ActivityCounts) { return counts.running + counts.done + counts.failed > 0; } +/** + * `ui.setStatus` stores a finished string and is only called when the watched + * work changes, so this line is NOT re-evaluated per frame. A spinner here + * would freeze on whatever frame the last event happened to land on, which + * reads as a hung UI. The animated glyph belongs to the strip, which owns a + * render loop; the footer states the count with a still marker. + */ export function formatActivityStatus( theme: Theme, label: "subagents" | "workflows", counts: ActivityCounts, + stripVisible = false, ) { const parts: string[] = []; if (counts.running > 0) { - parts.push(theme.fg("warning", `${SQUARE} ${counts.running} running`)); + parts.push( + theme.fg("warning", `${RUNNING_MARK} ${counts.running} running`), + ); } if (counts.done > 0) { - parts.push(theme.fg("success", `${SQUARE} ${counts.done} done`)); + parts.push(theme.fg("success", `✓ ${counts.done} done`)); } if (counts.failed > 0) { - parts.push(theme.fg("error", `${SQUARE} ${counts.failed} failed`)); + parts.push(theme.fg("error", `✗ ${counts.failed} failed`)); + } + if (!stripVisible) { + parts.push(theme.fg("accent", `/${label}`) + theme.fg("dim", " to view")); } - parts.push(theme.fg("accent", `/${label}`) + theme.fg("dim", " to view")); return `${theme.fg("muted", `${label}:`)} ${parts.join(theme.fg("dim", " · "))}`; } diff --git a/extensions/subagents/index.ts b/extensions/subagents/index.ts index 59d8fd3a..52179a1f 100644 --- a/extensions/subagents/index.ts +++ b/extensions/subagents/index.ts @@ -383,6 +383,9 @@ export default function (pi: ExtensionAPI) { const updateStatus = (manager: SubagentManagerShape) => { if (!ui) return; + // Refresh the strip first: when it is on screen it already carries the + // "↓ to manage" affordance, so the footer must not repeat the command. + updateSubagentWidget(); const counts = unreadActivityCounts( manager.view.list(), settledAcknowledgedAt, @@ -390,10 +393,9 @@ export default function (pi: ExtensionAPI) { ui.setStatus( "subagents", hasActivity(counts) - ? formatActivityStatus(ui.theme, "subagents", counts) + ? formatActivityStatus(ui.theme, "subagents", counts, widgetVisible) : undefined, ); - updateSubagentWidget(); }; const openDashboard = async (ctx: ExtensionContext, initialId?: string) => { diff --git a/extensions/subagents/navigation.test.ts b/extensions/subagents/navigation.test.ts index 240f08ee..1e7a80f4 100644 --- a/extensions/subagents/navigation.test.ts +++ b/extensions/subagents/navigation.test.ts @@ -6,6 +6,7 @@ import { BelowEditorStripState } from "../shared/below-editor-navigation.ts"; import { normalizeSubagentTitle, selectSubagentStripEntry, + type SubagentStripEntry, SubagentStripWidget, } from "./navigation.ts"; import type { SubagentSnapshot } from "./src/domain.ts"; @@ -41,6 +42,19 @@ const theme = { bold: (text: string) => text, } as unknown as Theme; +const SPINNERS = /[⠋⠙⠹⠸⠼⠴⠦⠧⠇⠏]/; + +function stripWidget(entry: SubagentStripEntry | undefined) { + const strip = new BelowEditorStripState(); + const widget = new SubagentStripWidget( + { requestRender() {} } as unknown as TUI, + theme, + strip, + () => entry, + ); + return { strip, widget }; +} + test("subagent titles are sanitized and bounded at ingress", () => { assert.equal( normalizeSubagentTitle(" review\u001b]52;c;payload\u0007\nnow "), @@ -74,17 +88,11 @@ test("strip selection prefers newest running, then newest unread settled", () => }); test("subagent strip matches Workflow's bounded one-line affordance", () => { - const strip = new BelowEditorStripState(); const entry = selectSubagentStripEntry( [snapshot("sa-1", "running", Date.now() - 2_000)], 0, ); - const widget = new SubagentStripWidget( - { requestRender() {} } as unknown as TUI, - theme, - strip, - () => entry, - ); + const { strip, widget } = stripWidget(entry); try { const idle = widget.render(100); assert.equal(idle.length, 1); @@ -98,7 +106,7 @@ test("subagent strip matches Workflow's bounded one-line affordance", () => { assert.ok(visibleWidth(focused[0]!) <= 54); assert.match(focused[0]!, /enter open/); - for (const width of [1, 8, 20]) { + for (const width of [1, 8, 20, 40]) { const narrow = widget.render(width); assert.equal(narrow.length, 1); assert.ok(visibleWidth(narrow[0]!) <= width); @@ -107,3 +115,101 @@ test("subagent strip matches Workflow's bounded one-line affordance", () => { widget.dispose(); } }); + +test("unfocused strip shows exactly one leading glyph: spinner or settle mark", () => { + const running = stripWidget( + selectSubagentStripEntry([snapshot("sa-1", "running", Date.now())], 0), + ); + try { + const line = running.widget.render(100)[0]!; + assert.match(line, /^ [⠋⠙⠹⠸⠼⠴⠦⠧⠇⠏] /); + assert.doesNotMatch(line, /■|○|❯|✓|✗/); + } finally { + running.widget.dispose(); + } + + const done = stripWidget( + selectSubagentStripEntry([snapshot("sa-2", "done", 1, 2)], 0), + ); + try { + const line = done.widget.render(100)[0]!; + assert.match(line, /✓/); + assert.doesNotMatch(line, SPINNERS); + assert.doesNotMatch(line, /■|○|❯|✗/); + } finally { + done.widget.dispose(); + } + + const failed = stripWidget( + selectSubagentStripEntry([snapshot("sa-3", "error", 1, 2)], 0), + ); + try { + const line = failed.widget.render(100)[0]!; + assert.match(line, /✗/); + assert.doesNotMatch(line, SPINNERS); + assert.doesNotMatch(line, /■|○|❯|✓/); + } finally { + failed.widget.dispose(); + } +}); + +test("focused strip shows the marker and no status glyph", () => { + const { strip, widget } = stripWidget( + selectSubagentStripEntry([snapshot("sa-1", "running", Date.now())], 0), + ); + strip.focused = true; + try { + const line = widget.render(100)[0]!; + assert.match(line, /❯/); + assert.doesNotMatch(line, SPINNERS); + assert.doesNotMatch(line, /■|○|✓|✗/); + } finally { + widget.dispose(); + } +}); + +test("strip carries no model label and no context percentage", () => { + const { widget } = stripWidget( + selectSubagentStripEntry( + [snapshot("sa-1", "running", Date.now(), undefined)], + 0, + ), + ); + try { + const line = widget.render(100)[0]!; + assert.doesNotMatch(line, /gpt-5\.6-sol/); + assert.doesNotMatch(line, /%/); + } finally { + widget.dispose(); + } +}); + +test("a single subagent shows no count; several show a readable one", () => { + const single = stripWidget( + selectSubagentStripEntry([snapshot("solo", "running", 3)], 0), + ); + try { + const line = single.widget.render(100)[0]!; + assert.doesNotMatch(line, /agents/); + assert.doesNotMatch(line, /\d\/\d/); + } finally { + single.widget.dispose(); + } + + const entry = selectSubagentStripEntry( + [ + snapshot("done-1", "done", 1, 5), + snapshot("done-2", "done", 2, 6), + snapshot("run", "running", 3), + ], + 0, + ); + const several = stripWidget(entry); + try { + const line = several.widget.render(100)[0]!; + assert.match(line, /2\/3 done/); + assert.doesNotMatch(line, /agents/); + } finally { + several.widget.dispose(); + } +}); diff --git a/extensions/subagents/navigation.ts b/extensions/subagents/navigation.ts index 499b21d6..70467469 100644 --- a/extensions/subagents/navigation.ts +++ b/extensions/subagents/navigation.ts @@ -10,7 +10,8 @@ import { } from "../shared/activity-status.ts"; import { sanitizeTerminalText } from "../shared/terminal-text.ts"; import { formatElapsed, type SubagentSnapshot } from "./src/domain.ts"; -import { formatContextUtilization } from "./src/format.ts"; +import { statusGlyph } from "./src/ui/takeover.ts"; +import { SPINNER_INTERVAL_MS } from "./src/ui/transcript.ts"; export interface SubagentStripEntry { snapshot: SubagentSnapshot; @@ -58,13 +59,10 @@ function statusColor(status: SubagentSnapshot["status"]) { return "error" as const; } -function statusSquare(snapshot: SubagentSnapshot, theme: Theme) { - return theme.fg(statusColor(snapshot.status), "■"); -} - /** One-line subagent manager entry with the same affordance as Workflow. */ export class SubagentStripWidget { - private readonly timer: ReturnType; + private timer?: ReturnType; + private timerInterval = 0; private readonly tui: TUI; private readonly theme: Theme; private readonly strip: BelowEditorStripState; @@ -80,12 +78,23 @@ export class SubagentStripWidget { this.theme = theme; this.strip = strip; this.getEntry = getEntry; - this.timer = setInterval(() => this.tui.requestRender(), 500); + this.refreshTimer(false); + } + + /** A running subagent animates a spinner, so repaint at its cadence. */ + private refreshTimer(running: boolean) { + const interval = running ? SPINNER_INTERVAL_MS : 500; + if (interval === this.timerInterval) return; + if (this.timer) clearInterval(this.timer); + this.timerInterval = interval; + this.timer = setInterval(() => this.tui.requestRender(), interval); this.timer.unref?.(); } dispose() { - clearInterval(this.timer); + if (this.timer) clearInterval(this.timer); + this.timer = undefined; + this.timerInterval = 0; } invalidate() {} @@ -94,23 +103,23 @@ export class SubagentStripWidget { const entry = this.getEntry(); if (!entry || width <= 0) return []; const { snapshot, counts } = entry; - const marker = this.strip.focused + this.refreshTimer(snapshot.status === "running"); + // One glyph column: the focus marker when selected, the status glyph — + // spinner, ✓, ✗ — otherwise. Model and context stay in Pi's footer and + // the dashboard; this strip owns navigation and progress. + const glyph = this.strip.focused ? this.theme.fg("accent", "❯") - : this.theme.fg("dim", "○"); + : statusGlyph(snapshot, this.theme); const titleText = normalizeSubagentTitle(snapshot.title, snapshot.id); const title = this.strip.focused ? this.theme.bold(this.theme.fg("accent", titleText)) : this.theme.fg("text", titleText); - const model = snapshot.meta.modelLabel - ? cleanLine(snapshot.meta.modelLabel) - : undefined; - const left = ` ${marker} ${statusSquare(snapshot, this.theme)} ${title}${model ? this.theme.fg("dim", ` · ${model}`) : ""}`; + const left = ` ${glyph} ${title}`; const settled = counts.done + counts.failed; const total = counts.running + settled; const metrics = [ - `${settled}/${total} agents`, + total > 1 ? `${settled}/${total} done` : "", formatElapsed(snapshot), - formatContextUtilization(snapshot.usage), this.strip.focused ? "enter open · ↑ back" : "↓ to manage", ] .filter((part): part is string => Boolean(part)) diff --git a/extensions/subagents/src/ui/takeover.ts b/extensions/subagents/src/ui/takeover.ts index 77dbf24c..ec412010 100644 --- a/extensions/subagents/src/ui/takeover.ts +++ b/extensions/subagents/src/ui/takeover.ts @@ -37,10 +37,11 @@ function configuredKeys( /** * One spinner definition for the whole subagent UI: the dashboard glyph, the - * takeover header, and the transcript's live tools must animate in step, so the - * frames and their cadence live in `transcript.ts` and are imported here. + * takeover header, the below-editor strip, and the transcript's live tools + * must animate in step, so the frames and their cadence live in + * `transcript.ts` and are imported here. */ -function statusGlyph( +export function statusGlyph( snap: SubagentSnapshot, theme: Theme, now = Date.now(), @@ -284,12 +285,17 @@ export class SubagentDashboard implements Component { // One timestamp per frame so every row's spinner shows the same frame. const now = Date.now(); const rows = this.tui.terminal.rows || 30; - const maxBodyHeight = Math.max(1, rows - 5); + // Three breathing rows (above, before the hints, below) join the border and + // hints as fixed chrome, so the body clamp shrinks by the same three rows + // and the overlay never exceeds its old rows - 2 maximum. + const maxBodyHeight = Math.max(1, rows - 8); const bodyHeight = subs.length > maxBodyHeight ? maxBodyHeight : Math.max(1, subs.length); const innerWidth = Math.max(0, width - 2); const lines: string[] = []; + // Breathing room: one blank row above the box. + lines.push(""); const running = subs.filter((snap) => snap.status === "running").length; const done = subs.filter((snap) => snap.status === "done").length; const failed = subs.filter((snap) => snap.status === "error").length; @@ -320,7 +326,8 @@ export class SubagentDashboard implements Component { theme.fg("border", "╯"), ); - // Hints + // Hints, with one blank row on each side. + lines.push(""); lines.push( truncateToWidth( theme.fg( @@ -330,6 +337,7 @@ export class SubagentDashboard implements Component { width, ), ); + lines.push(""); return lines; } diff --git a/extensions/subagents/takeover.test.ts b/extensions/subagents/takeover.test.ts index 64b7f4df..459b3a8b 100644 --- a/extensions/subagents/takeover.test.ts +++ b/extensions/subagents/takeover.test.ts @@ -117,7 +117,8 @@ test("dashboard box height follows its subagents and retains the old maximum", ( const one = dashboard([snap("one")], 10); try { const lines = one.render(100); - assert.equal(lines.length, 4); // border, one agent, border, hints + // blank, border, one agent, border, blank, hints, blank + assert.equal(lines.length, 7); assert.equal(lines.filter((line) => line.includes("agent one")).length, 1); } finally { one.dispose(); @@ -129,7 +130,10 @@ test("dashboard box height follows its subagents and retains the old maximum", ( ); try { const lines = many.render(100); - assert.equal(lines.filter((line) => line.startsWith("│")).length, 5); + assert.equal(lines.filter((line) => line.startsWith("│")).length, 2); + // The three breathing rows are paid for by the body clamp: the overlay + // still never exceeds its old rows - 2 maximum. + assert.ok(lines.length <= 8, `height ${lines.length} > 8`); } finally { many.dispose(); } @@ -138,14 +142,65 @@ test("dashboard box height follows its subagents and retains the old maximum", ( test("dashboard reserves a more row and shows each visible subagent", () => { const view = dashboard( Array.from({ length: 8 }, (_, i) => snap(`${i}`)), - 10, + 12, ); try { const output = view.render(100).join("\n"); - for (const id of ["0", "1", "2", "3"]) { + for (const id of ["0", "1", "2"]) { assert.match(output, new RegExp(`agent ${id}`)); } - assert.match(output, /… 4 more/); + assert.match(output, /… 5 more/); + } finally { + view.dispose(); + } +}); + +test("dashboard breathes: blank rows above, around the hints, and below", () => { + const view = dashboard([snap("one")]); + try { + const lines = view.render(100); + assert.equal(lines[0], ""); + assert.equal(lines.at(-1), ""); + const hints = lines.findIndex((line) => line.includes("select")); + assert.ok(hints > 0); + assert.equal(lines[hints - 1], ""); + // The box's bottom border sits directly above the pre-hints blank row. + assert.ok(lines[hints - 2]!.startsWith("╰")); + } finally { + view.dispose(); + } +}); + +test("dashboard total height never exceeds its old rows - 2 maximum", () => { + for (const rows of [10, 15, 24]) { + const view = dashboard( + Array.from({ length: 40 }, (_, i) => snap(`${i}`)), + rows, + ); + try { + const lines = view.render(100); + assert.ok( + lines.length <= rows - 2, + `rows=${rows} height=${lines.length}`, + ); + } finally { + view.dispose(); + } + } +}); + +test("takeover view keeps its full-screen rows - 1 height budget", () => { + const running = snap("run", "running"); + const view = new TakeoverView( + tui(20), + theme, + keys, + "run", + model([running]), + () => {}, + ); + try { + assert.equal(view.render(80).length, 19); } finally { view.dispose(); }