Skip to content

Commit f84ddd4

Browse files
authored
feat(core): fail impossible goals (#4230)
* feat(core): fail impossible goals * fix(core): refine impossible goal judgement * fix(core): include goal feedback when continuing * fix(core): clarify impossible goal terminal state * fix(core): harden impossible goal feedback * fix(core): log suppressed impossible verdicts * fix(goal): address review suggestions * test(goal): cover impossible parsing suggestions
1 parent c93d66c commit f84ddd4

13 files changed

Lines changed: 390 additions & 39 deletions

packages/cli/src/ui/commands/goalCommand.test.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -326,4 +326,27 @@ describe('goalCommand', () => {
326326
lastReason: 'Goal max iterations reached',
327327
});
328328
});
329+
330+
it('after impossible failure, empty /goal shows the failed summary', async () => {
331+
const ctx = createMockCommandContext({
332+
services: { config: makeConfig() as unknown as Config },
333+
});
334+
await goalCommand.action!(ctx, 'do x');
335+
clearActiveGoal('sess-1');
336+
notifyGoalTerminal('sess-1', {
337+
kind: 'failed',
338+
condition: 'do x',
339+
iterations: 2,
340+
durationMs: 12_000,
341+
lastReason: 'the required branch does not exist',
342+
});
343+
344+
const result = await goalCommand.action!(ctx, '');
345+
const content = (result as { content: string }).content;
346+
expect(content).toMatch(/Goal could not be achieved/);
347+
expect(content).toMatch(/2 turns/);
348+
expect(content).toMatch(/12s/);
349+
expect(content).toMatch(/Goal: do x/);
350+
expect(content).toMatch(/Last check: the required branch does not exist/);
351+
});
329352
});

packages/cli/src/ui/commands/goalCommand.ts

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -52,12 +52,29 @@ const goalInstructionPrompt = (condition: string): string =>
5252

5353
const formatTurns = (n: number) => `${n} ${n === 1 ? 'turn' : 'turns'}`;
5454

55+
function assertNeverGoalKind(kind: never): never {
56+
throw new Error(`Unexpected terminal goal kind: ${kind}`);
57+
}
58+
59+
function terminalGoalTitle(kind: GoalTerminalEvent['kind']): string {
60+
switch (kind) {
61+
case 'achieved':
62+
return 'Goal achieved';
63+
case 'failed':
64+
return 'Goal could not be achieved';
65+
case 'aborted':
66+
return 'Goal aborted';
67+
default:
68+
return assertNeverGoalKind(kind);
69+
}
70+
}
71+
5572
function formatTerminalSummary(event: GoalTerminalEvent): string {
5673
// Mirrors GoalStatusMessage: empty-`/goal` after completion surfaces the
5774
// most recent terminal event, including the judge's `lastReason` (when
58-
// present) so this view matches the inline `Goal achieved / aborted`
75+
// present) so this view matches the inline terminal
5976
// history card.
60-
const title = event.kind === 'achieved' ? 'Goal achieved' : 'Goal aborted';
77+
const title = terminalGoalTitle(event.kind);
6178
const stats: string[] = [];
6279
if (event.iterations > 0) stats.push(formatTurns(event.iterations));
6380
if (typeof event.durationMs === 'number')
@@ -110,9 +127,8 @@ export const goalCommand: SlashCommand = {
110127
`Goal active: ${active.condition} (${turns})${lastReason}`,
111128
);
112129
}
113-
// No active goal — surface a summary of the most recent terminal goal
114-
// for this session. Only achieved / aborted entries flow through
115-
// `getLastGoalTerminal`; user-initiated `/goal clear` does not
130+
// No active goal — surface a summary of the most recent automatic
131+
// terminal goal for this session. User-initiated `/goal clear` does not
116132
// populate it.
117133
const last = getLastGoalTerminal(sessionId);
118134
if (last) {
@@ -128,7 +144,7 @@ export const goalCommand: SlashCommand = {
128144
// When an active goal exists, drop the Stop hook and emit a `cleared`
129145
// history sentinel. When no active goal exists, this is a no-op that just
130146
// returns "No goal set." The cached terminal summary is left intact so a
131-
// later empty `/goal` can still show the latest achieved/aborted state.
147+
// later empty `/goal` can still show the latest automatic terminal state.
132148
if (CLEAR_KEYWORDS.has(q.toLowerCase())) {
133149
const cleared = unregisterGoalHook(config, sessionId);
134150
if (!cleared) {

packages/cli/src/ui/components/messages/GoalStatusMessage.test.tsx

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,12 @@ import { describe, expect, it } from 'vitest';
99
import { GoalStatusMessage } from './GoalStatusMessage.js';
1010

1111
describe('<GoalStatusMessage />', () => {
12+
it('is wrapped in React.memo to avoid unnecessary scrollback rerenders', () => {
13+
expect(
14+
(GoalStatusMessage as unknown as { $$typeof?: symbol }).$$typeof,
15+
).toBe(Symbol.for('react.memo'));
16+
});
17+
1218
it('shows the goal and judge reason on checking cards', () => {
1319
const { lastFrame } = render(
1420
<GoalStatusMessage
@@ -25,4 +31,23 @@ describe('<GoalStatusMessage />', () => {
2531
expect(output).toContain('Goal: finish the refactor');
2632
expect(output).toContain('Judge: tests are still failing');
2733
});
34+
35+
it('shows impossible goals as failed terminal cards', () => {
36+
const { lastFrame } = render(
37+
<GoalStatusMessage
38+
kind="failed"
39+
condition="merge a nonexistent branch"
40+
iterations={2}
41+
durationMs={12_000}
42+
lastReason="the remote branch does not exist"
43+
/>,
44+
);
45+
46+
const output = lastFrame();
47+
expect(output).toContain('✖');
48+
expect(output).toContain('Goal could not be achieved');
49+
expect(output).toContain('2 turns');
50+
expect(output).toContain('Goal: merge a nonexistent branch');
51+
expect(output).toContain('Last check: the remote branch does not exist');
52+
});
2853
});

packages/cli/src/ui/components/messages/GoalStatusMessage.tsx

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,11 @@
44
* SPDX-License-Identifier: Apache-2.0
55
*/
66

7-
import type React from 'react';
7+
import React from 'react';
88
import { Box, Text } from 'ink';
99
import { theme } from '../../semantic-colors.js';
1010
import { formatDuration } from '../../utils/formatters.js';
11-
import type { GoalStatusKind } from '../../types.js';
11+
import { isTerminalGoalStatusKind, type GoalStatusKind } from '../../types.js';
1212

1313
interface GoalStatusMessageProps {
1414
kind: GoalStatusKind;
@@ -20,7 +20,11 @@ interface GoalStatusMessageProps {
2020

2121
const pluralTurns = (n: number) => (n === 1 ? 'turn' : 'turns');
2222

23-
export const GoalStatusMessage: React.FC<GoalStatusMessageProps> = ({
23+
function assertNeverGoalStatusKind(kind: never): never {
24+
throw new Error(`Unexpected goal status kind: ${kind}`);
25+
}
26+
27+
const GoalStatusMessageInternal: React.FC<GoalStatusMessageProps> = ({
2428
kind,
2529
condition,
2630
iterations,
@@ -81,13 +85,20 @@ export const GoalStatusMessage: React.FC<GoalStatusMessageProps> = ({
8185
prefixColor: theme.text.secondary,
8286
title: 'Goal cleared',
8387
};
88+
case 'failed':
89+
return {
90+
prefix: '✖',
91+
prefixColor: theme.status.error,
92+
title: 'Goal could not be achieved',
93+
};
8494
case 'aborted':
85-
default:
8695
return {
8796
prefix: '!',
8897
prefixColor: theme.status.warning,
8998
title: 'Goal aborted',
9099
};
100+
default:
101+
return assertNeverGoalStatusKind(kind);
91102
}
92103
})();
93104

@@ -126,7 +137,8 @@ export const GoalStatusMessage: React.FC<GoalStatusMessageProps> = ({
126137
<Text wrap="wrap">{condition}</Text>
127138
</Box>
128139
</Box>
129-
{/* `lastReason` is shown on terminal cards (achieved / aborted) so
140+
{/* `lastReason` is shown on terminal cards (achieved / aborted /
141+
failed) so
130142
the final summary records *why* the judge ruled the goal complete
131143
or why the loop gave up. Skipped for `cleared` because user-driven
132144
clears don't carry a judge reason.
@@ -136,7 +148,7 @@ export const GoalStatusMessage: React.FC<GoalStatusMessageProps> = ({
136148
flex-row variant hangs the continuation at the value column's
137149
left edge (≈12 cols of empty space, easily mistaken for a blank
138150
line). One Text + natural wrap keeps the continuation flush. */}
139-
{(kind === 'achieved' || kind === 'aborted') && lastReason?.trim() ? (
151+
{isTerminalGoalStatusKind(kind) && lastReason?.trim() ? (
140152
<Text color={theme.text.secondary} wrap="wrap">
141153
Last check: {lastReason.trim()}
142154
</Text>
@@ -145,3 +157,5 @@ export const GoalStatusMessage: React.FC<GoalStatusMessageProps> = ({
145157
</Box>
146158
);
147159
};
160+
161+
export const GoalStatusMessage = React.memo(GoalStatusMessageInternal);

packages/cli/src/ui/types.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -505,9 +505,24 @@ export type GoalStatusKind =
505505
| 'set'
506506
| 'achieved'
507507
| 'cleared'
508+
| 'failed'
508509
| 'aborted'
509510
| 'checking';
510511

512+
export const TERMINAL_GOAL_STATUS_KINDS = [
513+
'achieved',
514+
'aborted',
515+
'failed',
516+
] as const satisfies readonly GoalStatusKind[];
517+
518+
export function isTerminalGoalStatusKind(
519+
kind: GoalStatusKind,
520+
): kind is (typeof TERMINAL_GOAL_STATUS_KINDS)[number] {
521+
return TERMINAL_GOAL_STATUS_KINDS.includes(
522+
kind as (typeof TERMINAL_GOAL_STATUS_KINDS)[number],
523+
);
524+
}
525+
511526
export type HistoryItemGoalStatus = HistoryItemBase & {
512527
type: 'goal_status';
513528
kind: GoalStatusKind;

packages/cli/src/ui/utils/restoreGoal.test.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,15 @@ describe('findGoalToRestore', () => {
9898
]),
9999
).toBeNull();
100100
});
101+
102+
it('returns null when last goal_status is failed', () => {
103+
expect(
104+
findGoalToRestore([
105+
goalItem({ kind: 'set', condition: 'do x' }),
106+
goalItem({ kind: 'failed', condition: 'do x' }),
107+
]),
108+
).toBeNull();
109+
});
101110
});
102111

103112
describe('restoreGoalFromHistory', () => {
@@ -270,4 +279,21 @@ describe('findLastTerminalGoal', () => {
270279
expect(result?.kind).toBe('aborted');
271280
expect(result?.condition).toBe('goal B');
272281
});
282+
283+
it('returns failed when it is the most recent terminal', () => {
284+
const result = findLastTerminalGoal([
285+
goalItem({ kind: 'achieved', condition: 'goal A' }),
286+
goalItem({ kind: 'set', condition: 'goal B' }),
287+
goalItem({
288+
kind: 'failed',
289+
condition: 'goal B',
290+
lastReason: 'external service unavailable',
291+
}),
292+
]);
293+
expect(result).toMatchObject({
294+
kind: 'failed',
295+
condition: 'goal B',
296+
lastReason: 'external service unavailable',
297+
});
298+
});
273299
});

packages/cli/src/ui/utils/restoreGoal.ts

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,18 @@ import {
1313
type GoalTerminalEvent,
1414
type GoalTerminalKind,
1515
} from '@qwen-code/qwen-code-core';
16-
import type { HistoryItem, HistoryItemGoalStatus } from '../types.js';
17-
import { MessageType } from '../types.js';
16+
import {
17+
isTerminalGoalStatusKind,
18+
MessageType,
19+
type HistoryItem,
20+
type HistoryItemGoalStatus,
21+
} from '../types.js';
1822

1923
/**
2024
* Finds the most recent `goal_status` history item. Returns the active
2125
* condition when the latest goal event is non-terminal (`set` or `checking`),
2226
* or `null` if the last goal_status was terminal/cancelled
23-
* (achieved / cleared / aborted) or none exists.
27+
* (achieved / failed / cleared / aborted) or none exists.
2428
*/
2529
export function findGoalToRestore(history: HistoryItem[]): string | null {
2630
for (let i = history.length - 1; i >= 0; i--) {
@@ -35,7 +39,7 @@ export function findGoalToRestore(history: HistoryItem[]): string | null {
3539
}
3640

3741
/**
38-
* Finds the most recent terminal (achieved / aborted) goal_status item in
42+
* Finds the most recent terminal (achieved / failed / aborted) goal_status item in
3943
* the transcript. Sentinel-style entries (`set`, `cleared`, `checking`) are
4044
* SKIPPED — `/goal clear` after an achievement is intentionally a no-op on
4145
* this scan, matching Claude Code's `yjK` behavior (`if (!K.met || K.sentinel)
@@ -49,7 +53,7 @@ export function findLastTerminalGoal(
4953
const item = history[i];
5054
if (item?.type !== MessageType.GOAL_STATUS) continue;
5155
const goal = item as HistoryItemGoalStatus;
52-
if (goal.kind !== 'achieved' && goal.kind !== 'aborted') continue;
56+
if (!isTerminalGoalStatusKind(goal.kind)) continue;
5357
return {
5458
kind: goal.kind as GoalTerminalKind,
5559
condition: goal.condition,

packages/core/src/goals/activeGoalStore.ts

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -62,15 +62,21 @@ export function __resetActiveGoalStoreForTests(): void {
6262
// Terminal-state observers
6363
//
6464
// The Stop hook callback that drives /goal runs inside core, but the UI cards
65-
// for "Goal achieved" / "Goal aborted" need to land in CLI history. We bridge
66-
// the two with a module-scoped observer table that the CLI command populates
67-
// when it registers the goal and clears when the goal is unregistered.
65+
// for terminal outcomes need to land in CLI history. We bridge the two with a
66+
// module-scoped observer table that the CLI command populates when it
67+
// registers the goal and clears when the goal is unregistered.
6868
//
6969
// Observers are fire-and-forget — they MUST NOT throw or block the hook
7070
// callback; any side effect (e.g. context.ui.addItem) should be guarded.
7171
// ───────────────────────────────────────────────────────────────────────────
7272

73-
export type GoalTerminalKind = 'achieved' | 'aborted';
73+
/**
74+
* Terminal outcomes for an automatic `/goal` loop:
75+
* - `achieved`: the judge found transcript evidence that satisfies the goal.
76+
* - `aborted`: the loop stopped at a system safety limit.
77+
* - `failed`: the judge found the goal is genuinely impossible this session.
78+
*/
79+
export type GoalTerminalKind = 'achieved' | 'aborted' | 'failed';
7480

7581
export interface GoalTerminalEvent {
7682
kind: GoalTerminalKind;
@@ -119,8 +125,8 @@ export function notifyGoalTerminal(
119125
// Last-completed-goal cache
120126
//
121127
// Empty `/goal` after the active goal is gone should show the most recent
122-
// actually-finished goal. Only `achieved` and `aborted` qualify (those are
123-
// the `GoalTerminalKind`s); the user-driven `/goal clear` path emits a
128+
// actually-finished goal. Automatic terminal states (`achieved`, `aborted`,
129+
// and `failed`) qualify; the user-driven `/goal clear` path emits a
124130
// `cleared` history card directly and never flows through this notifier.
125131
// ───────────────────────────────────────────────────────────────────────────
126132

0 commit comments

Comments
 (0)