Skip to content

Commit 6009e9a

Browse files
committed
Wire /goal kickoff so setting or resuming a goal actually starts a turn
api.kickoff was declared optional on CommandContext.goal and the only construction site (runner.ts) never provided it, so /goal's set and resume paths silently no-op'd: governor state changed but the agent was never told, leaving the operator staring at "Goal set." forever. Extract the wiring into createGoalKickoff (src/tui/goal-kickoff.ts): builds goalKickoffUserMessage and sends it through agentProxy.send, the same queue-safe path every typed prompt and command "send" result already uses, so a goal set mid-turn queues behind it instead of corrupting it. kickoff is now required on CommandContext.goal since runner.ts is confirmed the only constructor. Verified live: /goal set now drives the agent through the full planning -> implementing -> reviewing -> completed lifecycle unprompted.
1 parent d1d9c5a commit 6009e9a

6 files changed

Lines changed: 128 additions & 23 deletions

File tree

src/tui/commands/built-in.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,7 @@ export function registerBuiltInCommands(): void {
254254
if (snap === null) {
255255
return { type: "message", text: "No paused or budget-limited goal to resume." };
256256
}
257-
api.kickoff?.(snap.brief || snap.condition, "resume");
257+
api.kickoff(snap.brief || snap.condition, "resume");
258258
return { type: "message", text: `Goal resumed.\n${formatGoalStatus(snap)}` };
259259
}
260260
if (parsed.sub === "clear") {
@@ -283,7 +283,7 @@ export function registerBuiltInCommands(): void {
283283
};
284284
}
285285
api.set(condition, parsed.opts);
286-
api.kickoff?.(condition, "set");
286+
api.kickoff(condition, "set");
287287
// One-shot banner only — brief lives in GoalView chrome (multi-line here
288288
// used to overflow chrome row accounting and collide with Work).
289289
return { type: "message", text: "Goal set." };

src/tui/commands/goal.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,7 @@ describe("/goal command", () => {
157157
pause: () => null,
158158
resume: () => null,
159159
clear: () => {},
160+
kickoff: () => {},
160161
},
161162
};
162163
const result = getCommand("goal")!.handler("", ctx);

src/tui/commands/registry.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,8 @@ export type CommandContext = {
1515
pause: () => GoalSnapshot | null;
1616
resume: (opts?: GoalResumeOpts) => GoalSnapshot | null;
1717
clear: () => void;
18-
/** Kick off a turn after set/resume so the agent starts working immediately. */
19-
/** Kick the agent after set/resume. phase defaults to set. */
20-
kickoff?: (condition: string, phase?: "set" | "resume") => void;
18+
/** Kick the agent after set/resume so a turn actually starts. Phase defaults to "set". */
19+
kickoff: (condition: string, phase?: "set" | "resume") => void;
2120
};
2221
};
2322

src/tui/goal-kickoff.test.ts

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
import { describe, expect, test } from "bun:test";
2+
import { goalKickoffUserMessage } from "../agent/goal.js";
3+
import { createGoalKickoff, type GoalKickoffDeps } from "./goal-kickoff.js";
4+
5+
function harness(): { deps: GoalKickoffDeps; sent: string[] } {
6+
const sent: string[] = [];
7+
const deps: GoalKickoffDeps = {
8+
send: (text) => {
9+
sent.push(text);
10+
return Promise.resolve();
11+
},
12+
onSendFailure: () => {},
13+
};
14+
return { deps, sent };
15+
}
16+
17+
describe("createGoalKickoff", () => {
18+
test("set phase delivers the kickoff message down the send path", async () => {
19+
const { deps, sent } = harness();
20+
const kickoff = createGoalKickoff(deps);
21+
22+
kickoff("ship the feature", "set");
23+
// send() is fire-and-forget from kickoff's perspective; flush microtasks.
24+
await Promise.resolve();
25+
26+
// This is the assertion the bug report calls out as missing: the message
27+
// must reach the send path, not merely mutate governor state.
28+
expect(sent).toEqual([goalKickoffUserMessage("ship the feature", "set")]);
29+
expect(sent[0]).toContain("manage_goal");
30+
expect(sent[0]).toContain("manage_tasks");
31+
});
32+
33+
test("resume phase uses the same send path with phase: resume", async () => {
34+
const { deps, sent } = harness();
35+
const kickoff = createGoalKickoff(deps);
36+
37+
kickoff("ship the feature", "resume");
38+
await Promise.resolve();
39+
40+
expect(sent).toEqual([goalKickoffUserMessage("ship the feature", "resume")]);
41+
expect(sent[0]).toContain("Goal resumed.");
42+
});
43+
44+
test("phase defaults to set", async () => {
45+
const { deps, sent } = harness();
46+
const kickoff = createGoalKickoff(deps);
47+
48+
kickoff("ship the feature");
49+
await Promise.resolve();
50+
51+
expect(sent).toEqual([goalKickoffUserMessage("ship the feature", "set")]);
52+
});
53+
54+
test("send failures are routed to onSendFailure, not thrown", async () => {
55+
let failure: unknown;
56+
const deps: GoalKickoffDeps = {
57+
send: () => Promise.reject(new Error("boom")),
58+
onSendFailure: (err) => {
59+
failure = err;
60+
},
61+
};
62+
const kickoff = createGoalKickoff(deps);
63+
64+
kickoff("ship the feature", "set");
65+
await Promise.resolve();
66+
await Promise.resolve();
67+
68+
expect(failure).toBeInstanceOf(Error);
69+
});
70+
});

src/tui/goal-kickoff.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
import { goalKickoffUserMessage } from "../agent/goal.js";
2+
3+
export type GoalKickoffDeps = {
4+
/**
5+
* Deliver the kickoff message down the same path a typed prompt takes.
6+
* Must serialize behind any in-flight turn so a goal set mid-run cannot
7+
* corrupt it. The ordinary send path already echoes the sent message into
8+
* the transcript in full (the same way any operator prompt does), so
9+
* kickoff needs no separate echo of its own — a second copy would only
10+
* duplicate it.
11+
*/
12+
send: (text: string) => Promise<unknown>;
13+
onSendFailure: (err: unknown) => void;
14+
};
15+
16+
/**
17+
* Builds the `/goal` kickoff handler: turns a set/resume into the lifecycle
18+
* message the agent needs to actually start working, and sends it through
19+
* the ordinary send path.
20+
*/
21+
export function createGoalKickoff(deps: GoalKickoffDeps): (condition: string, phase?: "set" | "resume") => void {
22+
return (condition, phase = "set") => {
23+
const message = goalKickoffUserMessage(condition, phase);
24+
void deps.send(message).catch(deps.onSendFailure);
25+
};
26+
}

src/tui/runner.ts

Lines changed: 27 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,7 @@ import { createSessionOperationQueue } from "./session-operation-queue.js";
113113
import { setAgentSourceUnlessClosed } from "./agent-source-sync.js";
114114
import { createChatDirector } from "../agent/director.js";
115115
import { createGoalGovernor } from "../agent/goal.js";
116+
import { createGoalKickoff } from "./goal-kickoff.js";
116117
import { createGoalEvaluator } from "../agent/goal-evaluator.js";
117118
import { loadGoalState, saveGoalState } from "../session/goal-state.js";
118119
import { loadAgentProfiles, type AgentProfile } from "../agent/profiles.js";
@@ -1756,6 +1757,24 @@ export async function runTUI(initialConfig: Config): Promise<number> {
17561757
// session can show them.
17571758
}
17581759

1760+
const systemRow = (text: string): void => {
1761+
appendStreamRow(host.shell, { role: "system", text, meta: "command" });
1762+
};
1763+
1764+
/** Settle the shell after a rejected send so the run does not look live. */
1765+
const handleSendFailure = (err: unknown): void => {
1766+
const kind = classifyAgentSendFailure(
1767+
err,
1768+
sendAborted,
1769+
isCodexAuthError,
1770+
isXaiAuthError,
1771+
);
1772+
if (!shouldSettleUiAfterSendFailure(kind)) return;
1773+
recordRunError(err);
1774+
systemRow(err instanceof Error ? err.message : String(err));
1775+
setShellRunState(host.shell, "idle");
1776+
};
1777+
17591778
const commandContext: CommandContext = {
17601779
signalClear: newSession,
17611780
getCostSummary: (): CostSummary => {
@@ -1800,27 +1819,17 @@ export async function runTUI(initialConfig: Config): Promise<number> {
18001819
pause: () => goalGovernor.pause(),
18011820
resume: (opts) => goalGovernor.resume(opts),
18021821
clear: () => goalGovernor.clear(),
1822+
// Routed through agentProxy.send, the same queue-safe path every typed
1823+
// prompt and command "send" result uses: it awaits any in-flight
1824+
// turn's tail first, so a goal set mid-run cannot corrupt it — the
1825+
// kickoff simply starts once the turn settles.
1826+
kickoff: createGoalKickoff({
1827+
send: (text) => agentProxy.send(text),
1828+
onSendFailure: handleSendFailure,
1829+
}),
18031830
},
18041831
};
18051832

1806-
const systemRow = (text: string): void => {
1807-
appendStreamRow(host.shell, { role: "system", text, meta: "command" });
1808-
};
1809-
1810-
/** Settle the shell after a rejected send so the run does not look live. */
1811-
const handleSendFailure = (err: unknown): void => {
1812-
const kind = classifyAgentSendFailure(
1813-
err,
1814-
sendAborted,
1815-
isCodexAuthError,
1816-
isXaiAuthError,
1817-
);
1818-
if (!shouldSettleUiAfterSendFailure(kind)) return;
1819-
recordRunError(err);
1820-
systemRow(err instanceof Error ? err.message : String(err));
1821-
setShellRunState(host.shell, "idle");
1822-
};
1823-
18241833
// The permissions surface addresses grants by their position in the last
18251834
// listing, so revoke resolves against the same snapshot the operator saw.
18261835
let listedGrants: readonly ScopedApproval[] = [];

0 commit comments

Comments
 (0)