From 9fc883224c0504d01ab1ce6800cc3ec68a114416 Mon Sep 17 00:00:00 2001 From: Sawyer Date: Tue, 4 Aug 2026 18:53:55 -0700 Subject: [PATCH 1/3] Fix goal mode layout overflow and active-step rendering Constrain GoalView and TaskView to terminal width so long briefs and Work step titles truncate instead of colliding with chrome. Collapse the phase trail on narrow terminals, clear the "Goal set." banner once goal UI is live, and keep Work progress as a single non-overprinting heading. Closes CL-5343 --- src/tui/app.tsx | 14 +- src/tui/commands/built-in.ts | 6 +- src/tui/commands/goal.test.ts | 9 +- src/tui/components/goal-view.tsx | 236 +++++++++++++++++++----------- src/tui/components/task-view.tsx | 65 +++++--- tests/unit/tui/goal-view.test.tsx | 98 +++++++++++++ tests/unit/tui/task-view.test.tsx | 43 ++++++ 7 files changed, 357 insertions(+), 114 deletions(-) create mode 100644 tests/unit/tui/goal-view.test.tsx diff --git a/src/tui/app.tsx b/src/tui/app.tsx index 9e14aae27..3b582e6d3 100644 --- a/src/tui/app.tsx +++ b/src/tui/app.tsx @@ -524,6 +524,14 @@ export function App({ } wasWorkPrimary.current = workPrimary; }, [workPrimary]); + // Drop the /goal one-shot once Goal chrome is live so it does not stack on + // the brief / Work checklist (and blow the single reserved chrome row). + useEffect(() => { + if (!goalActive || commandMessage === null) return; + if (commandMessage === "Goal set." || commandMessage.startsWith("Goal set.")) { + setCommandMessage(null); + } + }, [goalActive, commandMessage]); const workExpanded = tasksExpanded; const goalChromeRows = goalChromeRowCount({ goalActive, @@ -1203,8 +1211,10 @@ export function App({ /> {mcpStatus.needsAuth.length > 0 && } {commandMessage !== null && ( - - {commandMessage} + + + {commandMessage.split("\n")[0]} + )} {!taskFullScreenOpen && ( diff --git a/src/tui/commands/built-in.ts b/src/tui/commands/built-in.ts index a5ee7f470..1a82ba0e0 100644 --- a/src/tui/commands/built-in.ts +++ b/src/tui/commands/built-in.ts @@ -307,9 +307,11 @@ registerCommand({ `Clear it first (/goal clear) or replace with /goal --replace .`, }; } - const snap = api.set(condition, parsed.opts); + api.set(condition, parsed.opts); api.kickoff?.(condition, "set"); - return { type: "message", text: `Goal set.\nBrief: ${snap.brief}` }; + // One-shot banner only — brief lives in GoalView chrome (multi-line here + // used to overflow chrome row accounting and collide with Work). + return { type: "message", text: "Goal set." }; }, }); diff --git a/src/tui/commands/goal.test.ts b/src/tui/commands/goal.test.ts index ded63b3a9..139665d70 100644 --- a/src/tui/commands/goal.test.ts +++ b/src/tui/commands/goal.test.ts @@ -108,8 +108,9 @@ describe("/goal command", () => { const result = cmd!.handler("ship the feature", ctx); expect(result.type).toBe("message"); if (result.type === "message") { - expect(result.text).toContain("Goal set"); - expect(result.text).toContain("ship the feature"); + expect(result.text).toBe("Goal set."); + // Brief is shown in GoalView chrome, not the one-shot banner. + expect(result.text).not.toContain("ship the feature"); expect(result.text).not.toContain("The agent will expand"); expect(result.text).not.toContain("manage_goal"); } @@ -186,8 +187,8 @@ describe("/goal command", () => { const ok = getCommand("goal")!.handler("--replace new goal", ctx); expect(ok.type).toBe("message"); if (ok.type === "message") { - expect(ok.text).toContain("Goal set"); - expect(ok.text).toContain("new goal"); + expect(ok.text).toBe("Goal set."); + expect(ok.text).not.toContain("new goal"); } }); }); diff --git a/src/tui/components/goal-view.tsx b/src/tui/components/goal-view.tsx index a9de2b9ba..d18db3c25 100644 --- a/src/tui/components/goal-view.tsx +++ b/src/tui/components/goal-view.tsx @@ -9,6 +9,7 @@ import { type GoalStatus, } from "../../agent/goal.js"; import { color } from "../theme.js"; +import { useTerminalSize } from "../hooks/use-terminal-size.js"; export type GoalViewProps = { goal: GoalSnapshot; @@ -39,67 +40,67 @@ const PHASE_ORDER: readonly GoalPhase[] = [ "completed", ]; +/** Full trail `plan→impl→review→done` needs ~23 cols; below this show current only. */ +const PHASE_TRAIL_MIN_COLS = 48; + /** * Expanded acceptance checklist — primary goal surface. * Quiet styling (muted labels, no bright accent wash). * On achieve: freezes on "Goal completed in …" and stops looking like work-in-progress. + * Width-constrained so long briefs/criteria truncate instead of colliding with Work/footer. */ export function GoalView({ goal, compact }: GoalViewProps) { if (goal.status === "inactive" || goal.status === "cleared") return null; + const { columns } = useTerminalSize(); const phase = goal.phase; const progress = goalCriteriaProgress(goal.criteria); const brief = goal.brief || goal.condition; const quiet = isQuietStatus(goal.status); const completed = formatGoalCompleted(goal); + const narrow = columns < PHASE_TRAIL_MIN_COLS; if (completed !== null) { return ( - - - - Goal - - {completed} - {progress.total > 0 && ( - - {`${progress.done}/${progress.total}`} + + + + + Goal + + + + + {completed} + + {progress.total > 0 && ( + + + {`${progress.done}/${progress.total}`} + + )} - - {brief} - + {goal.criteria.length > 0 && - sortedCriteria(goal.criteria).map((c) => ( - - {GLYPH[c.status]} - - {c.title} - - - ))} + sortedCriteria(goal.criteria).map((c) => )} ); } if (compact || goal.criteria.length === 0) { return ( - - - - Goal - - - {!quiet && ( - - {goal.status} - - )} - - - {brief} - + + + {goal.criteria.length === 0 && phase === "planning" && ( planning acceptance… @@ -110,77 +111,136 @@ export function GoalView({ goal, compact }: GoalViewProps) { } return ( - - + + 0 ? `${progress.done}/${progress.total}` : null} + status={!quiet ? goal.status : null} + quiet={quiet} + /> + + {sortedCriteria(goal.criteria).map((c) => ( + + ))} + {goal.lastReason !== undefined && goal.lastReason.length > 0 && ( + + + {goal.lastReason} + + + )} + + ); +} + +function HeaderRow(props: { + label: string; + phase: GoalPhase; + narrow: boolean; + progress: string | null; + status: GoalStatus | null; + quiet: boolean; +}) { + const { label, phase, narrow, progress, status, quiet } = props; + return ( + + - Acceptance - - - - {`${progress.done}/${progress.total}`} + {label} - {!quiet && ( - - {goal.status} - - )} - - {brief} - - {sortedCriteria(goal.criteria).map((c) => ( - - {GLYPH[c.status]} - - {c.title} + + + + {progress !== null && ( + + + {progress} - {c.note !== undefined && c.note.length > 0 && ( - - {c.note} - - )} - ))} - {goal.lastReason !== undefined && goal.lastReason.length > 0 && ( - - {goal.lastReason} + )} + {status !== null && ( + + + {status} + + + )} + + ); +} + +function BriefLine({ brief, dim }: { brief: string; dim?: boolean }) { + return ( + + {dim ? ( + + {brief} + ) : ( + {brief} )} ); } -/** plan → impl → review → done with current phase emphasized. */ -function PhaseTrail({ phase }: { phase: GoalPhase }) { +function CriterionRow({ criterion: c }: { criterion: GoalCriterion }) { + const terminal = c.status === "done" || c.status === "cancelled"; + return ( + + + {GLYPH[c.status]} + + + + {c.title} + + + {c.note !== undefined && c.note.length > 0 && ( + + + {c.note} + + + )} + + ); +} + +/** plan → impl → review → done; on narrow terminals show only the current phase. */ +function PhaseTrail({ phase, narrow }: { phase: GoalPhase; narrow: boolean }) { + if (narrow) { + return ( + + {PHASE_SHORT[phase]} + + ); + } const idx = PHASE_ORDER.indexOf(phase); return ( - + {PHASE_ORDER.map((p, i) => { const current = p === phase; + const sep = i > 0 ? "→" : ""; return ( - - {i > 0 && ( - - → - - )} - - {PHASE_SHORT[p]} - - + + {sep} + {PHASE_SHORT[p]} + ); })} - + ); } diff --git a/src/tui/components/task-view.tsx b/src/tui/components/task-view.tsx index bdbe4fd7d..24aa792fa 100644 --- a/src/tui/components/task-view.tsx +++ b/src/tui/components/task-view.tsx @@ -15,6 +15,11 @@ const GLYPH: Record = { cancelled: "✗", }; +/** + * Work/Tasks checklist chrome. + * Rows are width-constrained so long titles truncate after the status glyph + * instead of colliding with the header (`Work 03/11`) or neighboring lines. + */ export function TaskView({ tasks, compact, title = "Tasks" }: TaskViewProps) { if (!hasActiveTasks(tasks)) return null; @@ -27,38 +32,62 @@ export function TaskView({ tasks, compact, title = "Tasks" }: TaskViewProps) { const remaining = active.length - 1; return ( - - {GLYPH[current.status]} - {current.title} - {remaining > 0 && {`+${remaining}`}} + + + {GLYPH[current.status]} + + + + {current.title} + + + {remaining > 0 && ( + + {`+${remaining}`} + + )} ); } const doneCount = sorted.filter((t) => t.status === "done").length; + // Single Text node for the heading so "Work" and "03/11" cannot overprint. + const heading = `${title} ${doneCount}/${sorted.length}`; return ( - - - {title} - {`${doneCount}/${sorted.length}`} + + + + {heading} + {sorted.map((task) => ( - - {GLYPH[task.status]} - - {task.title} - - + ))} ); } +function TaskRow({ task }: { task: Task }) { + const terminal = task.status === "done" || task.status === "cancelled"; + return ( + + + {GLYPH[task.status]} + + + + {task.title} + + + + ); +} + function byPriority(a: Task, b: Task): number { const rank: Record = { doing: 0, todo: 1, cancelled: 2, done: 3 }; return rank[a.status] - rank[b.status]; diff --git a/tests/unit/tui/goal-view.test.tsx b/tests/unit/tui/goal-view.test.tsx new file mode 100644 index 000000000..f6080c68f --- /dev/null +++ b/tests/unit/tui/goal-view.test.tsx @@ -0,0 +1,98 @@ +import { describe, expect, test } from "bun:test"; +import { Box } from "ink"; +import { render } from "ink-testing-library"; + +import { GoalView } from "../../../src/tui/components/goal-view.js"; +import type { GoalSnapshot } from "../../../src/agent/goal.js"; + +function snap(over: Partial = {}): GoalSnapshot { + return { + status: "active", + phase: "planning", + condition: "ship it", + brief: "ship it", + criteria: [], + turnsUsed: 0, + turnBudget: 0, + startedAt: Date.now(), + mainTokens: 0, + evalTokens: 0, + consecutiveEvalFailures: 0, + consecutiveEmptyYields: 0, + ...over, + }; +} + +describe("GoalView", () => { + test("compact strip shows phase trail and brief", () => { + const frame = + render( + , + ).lastFrame() ?? ""; + + expect(frame).toContain("Goal"); + expect(frame).toContain("impl"); + expect(frame).toContain("Fix goal mode layout overflow"); + }); + + test("long brief truncates inside a narrow container", () => { + const brief = + "A very long goal brief that would previously wrap into the Work checklist and footer chrome causing unreadable collisions on both full-width and split panes"; + const frame = + render( + + + , + ).lastFrame() ?? ""; + + expect(frame).toContain("Goal"); + const lines = frame.split("\n"); + for (const line of lines) { + // paddingX=1 eats 2 cols; allow a small slack for ink measurement + expect(line.length).toBeLessThanOrEqual(42); + } + // Full brief should not appear as a single unbroken line longer than width. + expect(frame.includes(brief)).toBe(false); + }); + + test("acceptance criteria keep glyph and title separated under width pressure", () => { + const frame = + render( + + + , + ).lastFrame() ?? ""; + + expect(frame).toContain("Acceptance"); + expect(frame).toContain("●"); + expect(frame).toMatch(/●\s/); + expect(frame).not.toMatch(/Acceptance[a-z]/); + }); + + test("phase trail remains readable (current phase visible)", () => { + const frame = + render( + , + ).lastFrame() ?? ""; + + expect(frame).toContain("review"); + }); +}); diff --git a/tests/unit/tui/task-view.test.tsx b/tests/unit/tui/task-view.test.tsx index f44f3da4f..0afa14f52 100644 --- a/tests/unit/tui/task-view.test.tsx +++ b/tests/unit/tui/task-view.test.tsx @@ -1,4 +1,5 @@ import { describe, expect, test } from "bun:test"; +import { Box } from "ink"; import { render } from "ink-testing-library"; import { TaskView } from "../../../src/tui/components/task-view.js"; @@ -37,4 +38,46 @@ describe("TaskView", () => { expect(render().lastFrame()).toBe(""); expect(render().lastFrame()).toBe(""); }); + + test("heading keeps a space between title and progress (no Work03/11 collision)", () => { + const many: Task[] = Array.from({ length: 11 }, (_, i) => ({ + id: `t${i}`, + title: i === 0 ? "Code for example long step title that should not collide" : `Step ${i}`, + // 0 doing, 1–3 done (3 done), rest todo + status: (i === 0 ? "doing" : i <= 3 ? "done" : "todo") as Task["status"], + })); + const frame = render().lastFrame() ?? ""; + + expect(frame).toContain("Work 3/11"); + expect(frame).not.toMatch(/Work3\/11/); + expect(frame).not.toMatch(/Work03\/11/); + // Active step stays on its own line after the heading, glyph preserved. + expect(frame).toMatch(/Work 3\/11[\s\S]*● Code for example/); + }); + + test("long step titles truncate within a narrow container without eating the glyph", () => { + const long: Task[] = [ + { + id: "doing", + title: "Implement the entire goal mode layout overflow fix with exhaustive edge cases", + status: "doing", + }, + { id: "todo", title: "Next", status: "todo" }, + ]; + const frame = + render( + + + , + ).lastFrame() ?? ""; + + expect(frame).toContain("Work 0/2"); + expect(frame).toContain("●"); + // Title is truncated; full string should not appear intact on a 36-col row + // after padding + glyph + gap. + const lines = frame.split("\n"); + const activeLine = lines.find((l) => l.includes("●")) ?? ""; + expect(activeLine.length).toBeLessThanOrEqual(36); + expect(activeLine.startsWith("●") || activeLine.includes("● ")).toBe(true); + }); }); From 524dd9fcf2e688a106bcca400e72f2728e5d97a9 Mon Sep 17 00:00:00 2001 From: Sawyer Date: Tue, 4 Aug 2026 20:44:15 -0700 Subject: [PATCH 2/3] Keep multi-line command banners and budget their rows The first-line clamp dropped /goal status, pause/resume, and already-active guidance. Render every line and reserve chrome rows to match. --- src/tui/app.tsx | 17 ++++++++++------- src/tui/chrome-geometry.ts | 5 +++-- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/src/tui/app.tsx b/src/tui/app.tsx index 3b582e6d3..19e1588ff 100644 --- a/src/tui/app.tsx +++ b/src/tui/app.tsx @@ -525,10 +525,10 @@ export function App({ wasWorkPrimary.current = workPrimary; }, [workPrimary]); // Drop the /goal one-shot once Goal chrome is live so it does not stack on - // the brief / Work checklist (and blow the single reserved chrome row). + // the brief / Work checklist (and blow the reserved chrome rows). useEffect(() => { if (!goalActive || commandMessage === null) return; - if (commandMessage === "Goal set." || commandMessage.startsWith("Goal set.")) { + if (commandMessage.startsWith("Goal set.")) { setCommandMessage(null); } }, [goalActive, commandMessage]); @@ -552,7 +552,8 @@ export function App({ const extraChromeRows = extraChromeRowCount({ mcpNeedsAuthCount: mcpStatus.needsAuth.length, - commandMessagePresent: commandMessage !== null, + commandMessageRows: + commandMessage === null ? 0 : Math.max(1, commandMessage.split("\n").length), goalChromeRows, taskChromeRows, pluginChromeRows, @@ -1211,10 +1212,12 @@ export function App({ /> {mcpStatus.needsAuth.length > 0 && } {commandMessage !== null && ( - - - {commandMessage.split("\n")[0]} - + + {commandMessage.split("\n").map((line, i) => ( + + {line} + + ))} )} {!taskFullScreenOpen && ( diff --git a/src/tui/chrome-geometry.ts b/src/tui/chrome-geometry.ts index 7a6552c83..02166dea8 100644 --- a/src/tui/chrome-geometry.ts +++ b/src/tui/chrome-geometry.ts @@ -70,7 +70,8 @@ export function pluginChromeRowCount(args: { export function extraChromeRowCount(args: { mcpNeedsAuthCount: number; - commandMessagePresent: boolean; + /** Rows reserved for the command feedback banner (0 when absent). */ + commandMessageRows: number; goalChromeRows: number; taskChromeRows: number; pluginChromeRows: number; @@ -85,7 +86,7 @@ export function extraChromeRowCount(args: { }): number { return ( (args.mcpNeedsAuthCount > 0 ? 1 : 0) + - (args.commandMessagePresent ? 1 : 0) + + args.commandMessageRows + args.goalChromeRows + args.taskChromeRows + args.pluginChromeRows + From d92483861af81f5b8a496951201f0d3747d798ae Mon Sep 17 00:00:00 2001 From: Sawyer Date: Tue, 4 Aug 2026 22:30:48 -0700 Subject: [PATCH 3/3] Call useTerminalSize before GoalView early return Rules of Hooks: the hook must run on every render even when status is inactive or cleared, so a later active mount keeps stable hook order. --- src/tui/components/goal-view.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/tui/components/goal-view.tsx b/src/tui/components/goal-view.tsx index d18db3c25..f74995afa 100644 --- a/src/tui/components/goal-view.tsx +++ b/src/tui/components/goal-view.tsx @@ -50,9 +50,11 @@ const PHASE_TRAIL_MIN_COLS = 48; * Width-constrained so long briefs/criteria truncate instead of colliding with Work/footer. */ export function GoalView({ goal, compact }: GoalViewProps) { + // Hooks must run unconditionally — mount can flip inactive without unmount. + const { columns } = useTerminalSize(); + if (goal.status === "inactive" || goal.status === "cleared") return null; - const { columns } = useTerminalSize(); const phase = goal.phase; const progress = goalCriteriaProgress(goal.criteria); const brief = goal.brief || goal.condition;