diff --git a/src/tui/app.tsx b/src/tui/app.tsx index 9e14aae27..19e1588ff 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 reserved chrome rows). + useEffect(() => { + if (!goalActive || commandMessage === null) return; + if (commandMessage.startsWith("Goal set.")) { + setCommandMessage(null); + } + }, [goalActive, commandMessage]); const workExpanded = tasksExpanded; const goalChromeRows = goalChromeRowCount({ goalActive, @@ -544,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, @@ -1203,8 +1212,12 @@ export function App({ /> {mcpStatus.needsAuth.length > 0 && } {commandMessage !== null && ( - - {commandMessage} + + {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 + 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..f74995afa 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,12 +40,19 @@ 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) { + // Hooks must run unconditionally — mount can flip inactive without unmount. + const { columns } = useTerminalSize(); + if (goal.status === "inactive" || goal.status === "cleared") return null; const phase = goal.phase; @@ -52,54 +60,49 @@ export function GoalView({ goal, compact }: GoalViewProps) { 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 +113,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 + {label} - - - {`${progress.done}/${progress.total}`} - - {!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} + )} + + ); +} + +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 with current phase emphasized. */ -function PhaseTrail({ phase }: { phase: GoalPhase }) { +/** 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); + }); });