diff --git a/src/tui/commands/built-in.ts b/src/tui/commands/built-in.ts index 4bad7a493..9717576d1 100644 --- a/src/tui/commands/built-in.ts +++ b/src/tui/commands/built-in.ts @@ -254,7 +254,7 @@ export function registerBuiltInCommands(): void { if (snap === null) { return { type: "message", text: "No paused or budget-limited goal to resume." }; } - api.kickoff?.(snap.brief || snap.condition, "resume"); + api.kickoff(snap.brief || snap.condition, "resume"); return { type: "message", text: `Goal resumed.\n${formatGoalStatus(snap)}` }; } if (parsed.sub === "clear") { @@ -283,7 +283,7 @@ export function registerBuiltInCommands(): void { }; } api.set(condition, parsed.opts); - api.kickoff?.(condition, "set"); + api.kickoff(condition, "set"); // 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 1c2ee865d..60e0a7e31 100644 --- a/src/tui/commands/goal.test.ts +++ b/src/tui/commands/goal.test.ts @@ -157,6 +157,7 @@ describe("/goal command", () => { pause: () => null, resume: () => null, clear: () => {}, + kickoff: () => {}, }, }; const result = getCommand("goal")!.handler("", ctx); diff --git a/src/tui/commands/registry.ts b/src/tui/commands/registry.ts index 451cdd57c..4084c9cb9 100644 --- a/src/tui/commands/registry.ts +++ b/src/tui/commands/registry.ts @@ -15,9 +15,8 @@ export type CommandContext = { pause: () => GoalSnapshot | null; resume: (opts?: GoalResumeOpts) => GoalSnapshot | null; clear: () => void; - /** Kick off a turn after set/resume so the agent starts working immediately. */ - /** Kick the agent after set/resume. phase defaults to set. */ - kickoff?: (condition: string, phase?: "set" | "resume") => void; + /** Kick the agent after set/resume so a turn actually starts. Phase defaults to "set". */ + kickoff: (condition: string, phase?: "set" | "resume") => void; }; }; diff --git a/src/tui/goal-kickoff.test.ts b/src/tui/goal-kickoff.test.ts new file mode 100644 index 000000000..8a885c6c9 --- /dev/null +++ b/src/tui/goal-kickoff.test.ts @@ -0,0 +1,70 @@ +import { describe, expect, test } from "bun:test"; +import { goalKickoffUserMessage } from "../agent/goal.js"; +import { createGoalKickoff, type GoalKickoffDeps } from "./goal-kickoff.js"; + +function harness(): { deps: GoalKickoffDeps; sent: string[] } { + const sent: string[] = []; + const deps: GoalKickoffDeps = { + send: (text) => { + sent.push(text); + return Promise.resolve(); + }, + onSendFailure: () => {}, + }; + return { deps, sent }; +} + +describe("createGoalKickoff", () => { + test("set phase delivers the kickoff message down the send path", async () => { + const { deps, sent } = harness(); + const kickoff = createGoalKickoff(deps); + + kickoff("ship the feature", "set"); + // send() is fire-and-forget from kickoff's perspective; flush microtasks. + await Promise.resolve(); + + // This is the assertion the bug report calls out as missing: the message + // must reach the send path, not merely mutate governor state. + expect(sent).toEqual([goalKickoffUserMessage("ship the feature", "set")]); + expect(sent[0]).toContain("manage_goal"); + expect(sent[0]).toContain("manage_tasks"); + }); + + test("resume phase uses the same send path with phase: resume", async () => { + const { deps, sent } = harness(); + const kickoff = createGoalKickoff(deps); + + kickoff("ship the feature", "resume"); + await Promise.resolve(); + + expect(sent).toEqual([goalKickoffUserMessage("ship the feature", "resume")]); + expect(sent[0]).toContain("Goal resumed."); + }); + + test("phase defaults to set", async () => { + const { deps, sent } = harness(); + const kickoff = createGoalKickoff(deps); + + kickoff("ship the feature"); + await Promise.resolve(); + + expect(sent).toEqual([goalKickoffUserMessage("ship the feature", "set")]); + }); + + test("send failures are routed to onSendFailure, not thrown", async () => { + let failure: unknown; + const deps: GoalKickoffDeps = { + send: () => Promise.reject(new Error("boom")), + onSendFailure: (err) => { + failure = err; + }, + }; + const kickoff = createGoalKickoff(deps); + + kickoff("ship the feature", "set"); + await Promise.resolve(); + await Promise.resolve(); + + expect(failure).toBeInstanceOf(Error); + }); +}); diff --git a/src/tui/goal-kickoff.ts b/src/tui/goal-kickoff.ts new file mode 100644 index 000000000..3df4edd98 --- /dev/null +++ b/src/tui/goal-kickoff.ts @@ -0,0 +1,26 @@ +import { goalKickoffUserMessage } from "../agent/goal.js"; + +export type GoalKickoffDeps = { + /** + * Deliver the kickoff message down the same path a typed prompt takes. + * Must serialize behind any in-flight turn so a goal set mid-run cannot + * corrupt it. The ordinary send path already echoes the sent message into + * the transcript in full (the same way any operator prompt does), so + * kickoff needs no separate echo of its own — a second copy would only + * duplicate it. + */ + send: (text: string) => Promise; + onSendFailure: (err: unknown) => void; +}; + +/** + * Builds the `/goal` kickoff handler: turns a set/resume into the lifecycle + * message the agent needs to actually start working, and sends it through + * the ordinary send path. + */ +export function createGoalKickoff(deps: GoalKickoffDeps): (condition: string, phase?: "set" | "resume") => void { + return (condition, phase = "set") => { + const message = goalKickoffUserMessage(condition, phase); + void deps.send(message).catch(deps.onSendFailure); + }; +} diff --git a/src/tui/runner.ts b/src/tui/runner.ts index 49a5d0ed8..e1edd5376 100644 --- a/src/tui/runner.ts +++ b/src/tui/runner.ts @@ -113,6 +113,7 @@ import { createSessionOperationQueue } from "./session-operation-queue.js"; import { setAgentSourceUnlessClosed } from "./agent-source-sync.js"; import { createChatDirector } from "../agent/director.js"; import { createGoalGovernor } from "../agent/goal.js"; +import { createGoalKickoff } from "./goal-kickoff.js"; import { createGoalEvaluator } from "../agent/goal-evaluator.js"; import { loadGoalState, saveGoalState } from "../session/goal-state.js"; import { loadAgentProfiles, type AgentProfile } from "../agent/profiles.js"; @@ -1756,6 +1757,24 @@ export async function runTUI(initialConfig: Config): Promise { // session can show them. } + const systemRow = (text: string): void => { + appendStreamRow(host.shell, { role: "system", text, meta: "command" }); + }; + + /** Settle the shell after a rejected send so the run does not look live. */ + const handleSendFailure = (err: unknown): void => { + const kind = classifyAgentSendFailure( + err, + sendAborted, + isCodexAuthError, + isXaiAuthError, + ); + if (!shouldSettleUiAfterSendFailure(kind)) return; + recordRunError(err); + systemRow(err instanceof Error ? err.message : String(err)); + setShellRunState(host.shell, "idle"); + }; + const commandContext: CommandContext = { signalClear: newSession, getCostSummary: (): CostSummary => { @@ -1800,27 +1819,17 @@ export async function runTUI(initialConfig: Config): Promise { pause: () => goalGovernor.pause(), resume: (opts) => goalGovernor.resume(opts), clear: () => goalGovernor.clear(), + // Routed through agentProxy.send, the same queue-safe path every typed + // prompt and command "send" result uses: it awaits any in-flight + // turn's tail first, so a goal set mid-run cannot corrupt it — the + // kickoff simply starts once the turn settles. + kickoff: createGoalKickoff({ + send: (text) => agentProxy.send(text), + onSendFailure: handleSendFailure, + }), }, }; - const systemRow = (text: string): void => { - appendStreamRow(host.shell, { role: "system", text, meta: "command" }); - }; - - /** Settle the shell after a rejected send so the run does not look live. */ - const handleSendFailure = (err: unknown): void => { - const kind = classifyAgentSendFailure( - err, - sendAborted, - isCodexAuthError, - isXaiAuthError, - ); - if (!shouldSettleUiAfterSendFailure(kind)) return; - recordRunError(err); - systemRow(err instanceof Error ? err.message : String(err)); - setShellRunState(host.shell, "idle"); - }; - // The permissions surface addresses grants by their position in the last // listing, so revoke resolves against the same snapshot the operator saw. let listedGrants: readonly ScopedApproval[] = [];