Skip to content

Commit 9cd7a1a

Browse files
Merge pull request #315 from corbitsdev/cl-5343-fix-goal-mode-layout-overflow-and-active-step-rendering
Fix goal mode layout overflow and active-step rendering
2 parents 9c03330 + d924838 commit 9cd7a1a

8 files changed

Lines changed: 366 additions & 117 deletions

File tree

src/tui/app.tsx

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -524,6 +524,14 @@ export function App({
524524
}
525525
wasWorkPrimary.current = workPrimary;
526526
}, [workPrimary]);
527+
// Drop the /goal one-shot once Goal chrome is live so it does not stack on
528+
// the brief / Work checklist (and blow the reserved chrome rows).
529+
useEffect(() => {
530+
if (!goalActive || commandMessage === null) return;
531+
if (commandMessage.startsWith("Goal set.")) {
532+
setCommandMessage(null);
533+
}
534+
}, [goalActive, commandMessage]);
527535
const workExpanded = tasksExpanded;
528536
const goalChromeRows = goalChromeRowCount({
529537
goalActive,
@@ -544,7 +552,8 @@ export function App({
544552

545553
const extraChromeRows = extraChromeRowCount({
546554
mcpNeedsAuthCount: mcpStatus.needsAuth.length,
547-
commandMessagePresent: commandMessage !== null,
555+
commandMessageRows:
556+
commandMessage === null ? 0 : Math.max(1, commandMessage.split("\n").length),
548557
goalChromeRows,
549558
taskChromeRows,
550559
pluginChromeRows,
@@ -1203,8 +1212,12 @@ export function App({
12031212
/>
12041213
{mcpStatus.needsAuth.length > 0 && <McpAuthPrompt servers={mcpStatus.needsAuth} />}
12051214
{commandMessage !== null && (
1206-
<Box paddingX={1}>
1207-
<Text color="cyan">{commandMessage}</Text>
1215+
<Box paddingX={1} width="100%" overflow="hidden" flexDirection="column">
1216+
{commandMessage.split("\n").map((line, i) => (
1217+
<Text key={i} color="cyan" wrap="truncate-end">
1218+
{line}
1219+
</Text>
1220+
))}
12081221
</Box>
12091222
)}
12101223
{!taskFullScreenOpen && (

src/tui/chrome-geometry.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,8 @@ export function pluginChromeRowCount(args: {
7070

7171
export function extraChromeRowCount(args: {
7272
mcpNeedsAuthCount: number;
73-
commandMessagePresent: boolean;
73+
/** Rows reserved for the command feedback banner (0 when absent). */
74+
commandMessageRows: number;
7475
goalChromeRows: number;
7576
taskChromeRows: number;
7677
pluginChromeRows: number;
@@ -85,7 +86,7 @@ export function extraChromeRowCount(args: {
8586
}): number {
8687
return (
8788
(args.mcpNeedsAuthCount > 0 ? 1 : 0) +
88-
(args.commandMessagePresent ? 1 : 0) +
89+
args.commandMessageRows +
8990
args.goalChromeRows +
9091
args.taskChromeRows +
9192
args.pluginChromeRows +

src/tui/commands/built-in.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -307,9 +307,11 @@ registerCommand({
307307
`Clear it first (/goal clear) or replace with /goal --replace <brief>.`,
308308
};
309309
}
310-
const snap = api.set(condition, parsed.opts);
310+
api.set(condition, parsed.opts);
311311
api.kickoff?.(condition, "set");
312-
return { type: "message", text: `Goal set.\nBrief: ${snap.brief}` };
312+
// One-shot banner only — brief lives in GoalView chrome (multi-line here
313+
// used to overflow chrome row accounting and collide with Work).
314+
return { type: "message", text: "Goal set." };
313315
},
314316
});
315317

src/tui/commands/goal.test.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -108,8 +108,9 @@ describe("/goal command", () => {
108108
const result = cmd!.handler("ship the feature", ctx);
109109
expect(result.type).toBe("message");
110110
if (result.type === "message") {
111-
expect(result.text).toContain("Goal set");
112-
expect(result.text).toContain("ship the feature");
111+
expect(result.text).toBe("Goal set.");
112+
// Brief is shown in GoalView chrome, not the one-shot banner.
113+
expect(result.text).not.toContain("ship the feature");
113114
expect(result.text).not.toContain("The agent will expand");
114115
expect(result.text).not.toContain("manage_goal");
115116
}
@@ -186,8 +187,8 @@ describe("/goal command", () => {
186187
const ok = getCommand("goal")!.handler("--replace new goal", ctx);
187188
expect(ok.type).toBe("message");
188189
if (ok.type === "message") {
189-
expect(ok.text).toContain("Goal set");
190-
expect(ok.text).toContain("new goal");
190+
expect(ok.text).toBe("Goal set.");
191+
expect(ok.text).not.toContain("new goal");
191192
}
192193
});
193194
});

src/tui/components/goal-view.tsx

Lines changed: 150 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
type GoalStatus,
1010
} from "../../agent/goal.js";
1111
import { color } from "../theme.js";
12+
import { useTerminalSize } from "../hooks/use-terminal-size.js";
1213

1314
export type GoalViewProps = {
1415
goal: GoalSnapshot;
@@ -39,67 +40,69 @@ const PHASE_ORDER: readonly GoalPhase[] = [
3940
"completed",
4041
];
4142

43+
/** Full trail `plan→impl→review→done` needs ~23 cols; below this show current only. */
44+
const PHASE_TRAIL_MIN_COLS = 48;
45+
4246
/**
4347
* Expanded acceptance checklist — primary goal surface.
4448
* Quiet styling (muted labels, no bright accent wash).
4549
* On achieve: freezes on "Goal completed in …" and stops looking like work-in-progress.
50+
* Width-constrained so long briefs/criteria truncate instead of colliding with Work/footer.
4651
*/
4752
export function GoalView({ goal, compact }: GoalViewProps) {
53+
// Hooks must run unconditionally — mount can flip inactive without unmount.
54+
const { columns } = useTerminalSize();
55+
4856
if (goal.status === "inactive" || goal.status === "cleared") return null;
4957

5058
const phase = goal.phase;
5159
const progress = goalCriteriaProgress(goal.criteria);
5260
const brief = goal.brief || goal.condition;
5361
const quiet = isQuietStatus(goal.status);
5462
const completed = formatGoalCompleted(goal);
63+
const narrow = columns < PHASE_TRAIL_MIN_COLS;
5564

5665
if (completed !== null) {
5766
return (
58-
<Box flexDirection="column" paddingX={1}>
59-
<Box gap={1}>
60-
<Text bold color={color("success")}>
61-
Goal
62-
</Text>
63-
<Text color={color("success")}>{completed}</Text>
64-
{progress.total > 0 && (
65-
<Text color={color("dim")} dimColor>
66-
{`${progress.done}/${progress.total}`}
67+
<Box flexDirection="column" width="100%" paddingX={1} overflow="hidden">
68+
<Box width="100%" gap={1} overflow="hidden">
69+
<Box flexShrink={0}>
70+
<Text bold color={color("success")}>
71+
Goal
72+
</Text>
73+
</Box>
74+
<Box flexGrow={1} flexShrink={1} minWidth={0} overflow="hidden">
75+
<Text color={color("success")} wrap="truncate-end">
76+
{completed}
6777
</Text>
78+
</Box>
79+
{progress.total > 0 && (
80+
<Box flexShrink={0}>
81+
<Text color={color("dim")} dimColor>
82+
{`${progress.done}/${progress.total}`}
83+
</Text>
84+
</Box>
6885
)}
6986
</Box>
70-
<Text wrap="truncate-end" color={color("dim")} dimColor>
71-
{brief}
72-
</Text>
87+
<BriefLine brief={brief} dim />
7388
{goal.criteria.length > 0 &&
74-
sortedCriteria(goal.criteria).map((c) => (
75-
<Box key={c.id} gap={1}>
76-
<Text color={criterionColor(c.status)}>{GLYPH[c.status]}</Text>
77-
<Text color={color("dim")} strikethrough={c.status === "done" || c.status === "cancelled"} wrap="truncate-end">
78-
{c.title}
79-
</Text>
80-
</Box>
81-
))}
89+
sortedCriteria(goal.criteria).map((c) => <CriterionRow key={c.id} criterion={c} />)}
8290
</Box>
8391
);
8492
}
8593

8694
if (compact || goal.criteria.length === 0) {
8795
return (
88-
<Box flexDirection="column" paddingX={1}>
89-
<Box gap={1}>
90-
<Text bold color={color("muted")}>
91-
Goal
92-
</Text>
93-
<PhaseTrail phase={phase} />
94-
{!quiet && (
95-
<Text color={statusColor(goal.status)} dimColor={quiet}>
96-
{goal.status}
97-
</Text>
98-
)}
99-
</Box>
100-
<Text wrap="truncate-end" dimColor={quiet}>
101-
{brief}
102-
</Text>
96+
<Box flexDirection="column" width="100%" paddingX={1} overflow="hidden">
97+
<HeaderRow
98+
label="Goal"
99+
phase={phase}
100+
narrow={narrow}
101+
progress={null}
102+
status={!quiet ? goal.status : null}
103+
quiet={quiet}
104+
/>
105+
<BriefLine brief={brief} dim={quiet} />
103106
{goal.criteria.length === 0 && phase === "planning" && (
104107
<Text color={color("dim")} dimColor>
105108
planning acceptance…
@@ -110,77 +113,136 @@ export function GoalView({ goal, compact }: GoalViewProps) {
110113
}
111114

112115
return (
113-
<Box flexDirection="column" paddingX={1}>
114-
<Box gap={1}>
116+
<Box flexDirection="column" width="100%" paddingX={1} overflow="hidden">
117+
<HeaderRow
118+
label="Acceptance"
119+
phase={phase}
120+
narrow={narrow}
121+
progress={progress.total > 0 ? `${progress.done}/${progress.total}` : null}
122+
status={!quiet ? goal.status : null}
123+
quiet={quiet}
124+
/>
125+
<BriefLine brief={brief} dim />
126+
{sortedCriteria(goal.criteria).map((c) => (
127+
<CriterionRow key={c.id} criterion={c} />
128+
))}
129+
{goal.lastReason !== undefined && goal.lastReason.length > 0 && (
130+
<Box width="100%" overflow="hidden">
131+
<Text color={color("dim")} dimColor wrap="truncate-end">
132+
{goal.lastReason}
133+
</Text>
134+
</Box>
135+
)}
136+
</Box>
137+
);
138+
}
139+
140+
function HeaderRow(props: {
141+
label: string;
142+
phase: GoalPhase;
143+
narrow: boolean;
144+
progress: string | null;
145+
status: GoalStatus | null;
146+
quiet: boolean;
147+
}) {
148+
const { label, phase, narrow, progress, status, quiet } = props;
149+
return (
150+
<Box width="100%" gap={1} overflow="hidden">
151+
<Box flexShrink={0}>
115152
<Text bold color={color("muted")}>
116-
Acceptance
153+
{label}
117154
</Text>
118-
<PhaseTrail phase={phase} />
119-
<Text color={color("dim")} dimColor>
120-
{`${progress.done}/${progress.total}`}
121-
</Text>
122-
{!quiet && (
123-
<Text color={statusColor(goal.status)} dimColor={quiet}>
124-
{goal.status}
125-
</Text>
126-
)}
127155
</Box>
128-
<Text wrap="truncate-end" color={color("dim")} dimColor>
129-
{brief}
130-
</Text>
131-
{sortedCriteria(goal.criteria).map((c) => (
132-
<Box key={c.id} gap={1}>
133-
<Text color={criterionColor(c.status)}>{GLYPH[c.status]}</Text>
134-
<Text
135-
{...(c.status === "done" || c.status === "cancelled"
136-
? { color: color("dim"), strikethrough: true }
137-
: {})}
138-
bold={c.status === "doing"}
139-
wrap="truncate-end"
140-
>
141-
{c.title}
156+
<Box flexShrink={0}>
157+
<PhaseTrail phase={phase} narrow={narrow} />
158+
</Box>
159+
{progress !== null && (
160+
<Box flexShrink={0}>
161+
<Text color={color("dim")} dimColor>
162+
{progress}
142163
</Text>
143-
{c.note !== undefined && c.note.length > 0 && (
144-
<Text color={color("dim")} dimColor wrap="truncate-end">
145-
{c.note}
146-
</Text>
147-
)}
148164
</Box>
149-
))}
150-
{goal.lastReason !== undefined && goal.lastReason.length > 0 && (
151-
<Text color={color("dim")} dimColor wrap="truncate-end">
152-
{goal.lastReason}
165+
)}
166+
{status !== null && (
167+
<Box flexShrink={1} minWidth={0} overflow="hidden">
168+
<Text color={statusColor(status)} dimColor={quiet} wrap="truncate-end">
169+
{status}
170+
</Text>
171+
</Box>
172+
)}
173+
</Box>
174+
);
175+
}
176+
177+
function BriefLine({ brief, dim }: { brief: string; dim?: boolean }) {
178+
return (
179+
<Box width="100%" overflow="hidden">
180+
{dim ? (
181+
<Text wrap="truncate-end" color={color("dim")} dimColor>
182+
{brief}
183+
</Text>
184+
) : (
185+
<Text wrap="truncate-end">{brief}</Text>
186+
)}
187+
</Box>
188+
);
189+
}
190+
191+
function CriterionRow({ criterion: c }: { criterion: GoalCriterion }) {
192+
const terminal = c.status === "done" || c.status === "cancelled";
193+
return (
194+
<Box width="100%" gap={1} overflow="hidden">
195+
<Box flexShrink={0}>
196+
<Text color={criterionColor(c.status)}>{GLYPH[c.status]}</Text>
197+
</Box>
198+
<Box flexGrow={1} flexShrink={1} minWidth={0} overflow="hidden">
199+
<Text
200+
{...(terminal ? { color: color("dim"), strikethrough: true } : {})}
201+
bold={c.status === "doing"}
202+
wrap="truncate-end"
203+
>
204+
{c.title}
153205
</Text>
206+
</Box>
207+
{c.note !== undefined && c.note.length > 0 && (
208+
<Box flexShrink={1} minWidth={0} overflow="hidden">
209+
<Text color={color("dim")} dimColor wrap="truncate-end">
210+
{c.note}
211+
</Text>
212+
</Box>
154213
)}
155214
</Box>
156215
);
157216
}
158217

159-
/** plan → impl → review → done with current phase emphasized. */
160-
function PhaseTrail({ phase }: { phase: GoalPhase }) {
218+
/** plan → impl → review → done; on narrow terminals show only the current phase. */
219+
function PhaseTrail({ phase, narrow }: { phase: GoalPhase; narrow: boolean }) {
220+
if (narrow) {
221+
return (
222+
<Text bold color={color("text")}>
223+
{PHASE_SHORT[phase]}
224+
</Text>
225+
);
226+
}
161227
const idx = PHASE_ORDER.indexOf(phase);
162228
return (
163-
<Box gap={0}>
229+
<Text>
164230
{PHASE_ORDER.map((p, i) => {
165231
const current = p === phase;
232+
const sep = i > 0 ? "→" : "";
166233
return (
167-
<Box key={p} gap={0}>
168-
{i > 0 && (
169-
<Text color={color("dim")} dimColor>
170-
171-
</Text>
172-
)}
173-
<Text
174-
bold={current}
175-
color={current ? color("text") : color("dim")}
176-
dimColor={!current || i < idx}
177-
>
178-
{PHASE_SHORT[p]}
179-
</Text>
180-
</Box>
234+
<Text
235+
key={p}
236+
bold={current}
237+
color={current ? color("text") : color("dim")}
238+
dimColor={!current || i < idx}
239+
>
240+
{sep}
241+
{PHASE_SHORT[p]}
242+
</Text>
181243
);
182244
})}
183-
</Box>
245+
</Text>
184246
);
185247
}
186248

0 commit comments

Comments
 (0)