diff --git a/.changeset/fix-goal-outcome-summaries.md b/.changeset/fix-goal-outcome-summaries.md new file mode 100644 index 000000000..02872c198 --- /dev/null +++ b/.changeset/fix-goal-outcome-summaries.md @@ -0,0 +1,5 @@ +--- +"@moonshot-ai/kimi-code": patch +--- + +Fix goal completion and blocked updates to produce one final user-facing outcome summary from the tool result. diff --git a/.changeset/fix-goal-queue-starts.md b/.changeset/fix-goal-queue-starts.md new file mode 100644 index 000000000..e9f6713a1 --- /dev/null +++ b/.changeset/fix-goal-queue-starts.md @@ -0,0 +1,5 @@ +--- +"@moonshot-ai/kimi-code": patch +--- + +Fix goal startup and queue handling so failed starts restore permission mode and queued goals wait behind new user messages. diff --git a/.changeset/fix-goal-token-budget.md b/.changeset/fix-goal-token-budget.md new file mode 100644 index 000000000..806efc329 --- /dev/null +++ b/.changeset/fix-goal-token-budget.md @@ -0,0 +1,5 @@ +--- +"@moonshot-ai/kimi-code": patch +--- + +Fix goal token budgets to count model completion tokens and stop without extra continuation steps when the budget is exhausted. diff --git a/.changeset/keep-goal-tools-visible.md b/.changeset/keep-goal-tools-visible.md new file mode 100644 index 000000000..1a0beb393 --- /dev/null +++ b/.changeset/keep-goal-tools-visible.md @@ -0,0 +1,5 @@ +--- +"@moonshot-ai/kimi-code": patch +--- + +Keep goal tools available to the main agent and return clear messages for invalid goal-control calls. diff --git a/.changeset/show-goal-hours.md b/.changeset/show-goal-hours.md new file mode 100644 index 000000000..b9ebe1cf2 --- /dev/null +++ b/.changeset/show-goal-hours.md @@ -0,0 +1,5 @@ +--- +"@moonshot-ai/kimi-code": patch +--- + +Show long-running goal wall-clock budget reminders in hours. diff --git a/.changeset/tighten-goal-mode-guidance.md b/.changeset/tighten-goal-mode-guidance.md new file mode 100644 index 000000000..6129ebcd0 --- /dev/null +++ b/.changeset/tighten-goal-mode-guidance.md @@ -0,0 +1,5 @@ +--- +"@moonshot-ai/kimi-code": patch +--- + +Tighten goal-mode guidance so agents continue reasonable work across turns instead of ending goals prematurely. diff --git a/apps/kimi-code/src/tui/commands/goal.ts b/apps/kimi-code/src/tui/commands/goal.ts index 881e1a40b..de790b906 100644 --- a/apps/kimi-code/src/tui/commands/goal.ts +++ b/apps/kimi-code/src/tui/commands/goal.ts @@ -372,10 +372,19 @@ async function startGoalWithPermission( choice: GoalStartPermissionChoice, options: GoalStartOptions, ): Promise { - if (choice !== host.state.appState.permissionMode && (choice === 'auto' || choice === 'yolo')) { + const previousMode = host.state.appState.permissionMode; + const switched = + choice !== previousMode && (choice === 'auto' || choice === 'yolo'); + if (switched) { if (!(await setPermissionForGoal(host, choice))) return; } - await startGoal(host, parsed, options); + const started = await startGoal(host, parsed, options); + // The permission switch only exists to run this goal. If creation fails + // (e.g. a goal already exists and `replace` was not given), restore the + // previous mode so the session is not left more permissive than before. + if (!started && switched) { + await setPermissionForGoal(host, previousMode); + } } async function setPermissionForGoal(host: GoalCommandHost, mode: PermissionMode): Promise { diff --git a/apps/kimi-code/src/tui/controllers/session-event-handler.ts b/apps/kimi-code/src/tui/controllers/session-event-handler.ts index e389e1cbf..2052c38c0 100644 --- a/apps/kimi-code/src/tui/controllers/session-event-handler.ts +++ b/apps/kimi-code/src/tui/controllers/session-event-handler.ts @@ -738,7 +738,8 @@ export class SessionEventHandler { (session === undefined || this.host.session === session) && !this.host.aborted && this.host.state.appState.streamingPhase === 'idle' && - this.host.state.queuedMessages.length === 0 + this.host.state.queuedMessages.length === 0 && + !this.host.state.queuedMessageDispatchPending ); } @@ -1003,14 +1004,18 @@ export class SessionEventHandler { private finishCompaction(sendQueued: (item: QueuedMessage) => void): void { const hasActiveTurn = this.host.streamingUI.hasActiveTurn(); if (!hasActiveTurn) { + const next = this.host.shiftQueuedMessage(); + if (next !== undefined) { + this.host.state.queuedMessageDispatchPending = true; + } this.host.setAppState({ isCompacting: false, streamingPhase: 'idle', }); this.host.resetLivePane(); - const next = this.host.shiftQueuedMessage(); if (next !== undefined) { setTimeout(() => { + this.host.state.queuedMessageDispatchPending = false; sendQueued(next); }, 0); } diff --git a/apps/kimi-code/src/tui/controllers/streaming-ui.ts b/apps/kimi-code/src/tui/controllers/streaming-ui.ts index bbfe25fdc..ae5eac768 100644 --- a/apps/kimi-code/src/tui/controllers/streaming-ui.ts +++ b/apps/kimi-code/src/tui/controllers/streaming-ui.ts @@ -560,9 +560,15 @@ export class StreamingUIController { const next = this.host.shiftQueuedMessage(); if (next !== undefined) { + // The message is out of the queue but not yet sent. Mark the dispatch + // pending *before* setAppState — that call synchronously retries + // queued-goal promotion, which would otherwise see an empty queue and an + // idle phase and start a goal ahead of this message. + state.queuedMessageDispatchPending = true; this.host.setAppState({ streamingPhase: 'idle' }); this.host.resetLivePane(); setTimeout(() => { + state.queuedMessageDispatchPending = false; sendQueued(next); }, 0); return; diff --git a/apps/kimi-code/src/tui/tui-state.ts b/apps/kimi-code/src/tui/tui-state.ts index 54eaa0e8a..d9665c9e3 100644 --- a/apps/kimi-code/src/tui/tui-state.ts +++ b/apps/kimi-code/src/tui/tui-state.ts @@ -52,6 +52,13 @@ export interface TUIState { tasksBrowser: TasksBrowserState | undefined; externalEditorRunning: boolean; queuedMessages: QueuedMessage[]; + /** + * True while a queued user message has been shifted out of + * {@link queuedMessages} but its deferred send has not run yet. The queue + * looks empty during this window, so queued-goal promotion must also check + * this flag to avoid starting a goal ahead of the user's earlier message. + */ + queuedMessageDispatchPending: boolean; swarmModeEntry: 'manual' | 'task' | undefined; } @@ -103,6 +110,7 @@ export function createTUIState(options: KimiTUIOptions): TUIState { tasksBrowser: undefined, externalEditorRunning: false, queuedMessages: [], + queuedMessageDispatchPending: false, swarmModeEntry: undefined, }; } diff --git a/apps/kimi-code/test/tui/commands/goal.test.ts b/apps/kimi-code/test/tui/commands/goal.test.ts index 1560f3d9a..ea59d4fae 100644 --- a/apps/kimi-code/test/tui/commands/goal.test.ts +++ b/apps/kimi-code/test/tui/commands/goal.test.ts @@ -330,6 +330,25 @@ describe('handleGoalCommand', () => { expect(manualHost.setAppState).toHaveBeenCalledWith({ permissionMode: 'yolo' }); }); + it('restores the previous permission mode when the goal fails to start', async () => { + const { host: manualHost, session: s } = makeHost({ permissionMode: 'manual' }); + s.createGoal = vi.fn(async () => { + throw new KimiError(ErrorCodes.GOAL_ALREADY_EXISTS, 'A goal already exists'); + }); + + await handleGoalCommand(manualHost, 'Ship feature X'); + const picker = mountedPicker(manualHost); + picker.handleInput(DOWN); + picker.handleInput(ENTER); + + await vi.waitFor(() => { + // Switched to YOLO to run the goal, then restored to Manual on failure. + expect(s.setPermission).toHaveBeenLastCalledWith('manual'); + }); + expect(s.setPermission).toHaveBeenCalledWith('yolo'); + expect(manualHost.setAppState).toHaveBeenLastCalledWith({ permissionMode: 'manual' }); + }); + it('returns the command to the input box when a Manual-mode goal start is cancelled', async () => { const { host: manualHost, session: s } = makeHost({ permissionMode: 'manual' }); diff --git a/apps/kimi-code/test/tui/controllers/session-event-handler-goal-queue.test.ts b/apps/kimi-code/test/tui/controllers/session-event-handler-goal-queue.test.ts index d02578f30..8b9d5fbdf 100644 --- a/apps/kimi-code/test/tui/controllers/session-event-handler-goal-queue.test.ts +++ b/apps/kimi-code/test/tui/controllers/session-event-handler-goal-queue.test.ts @@ -54,6 +54,7 @@ function makeHost(options: { createGoalRejects?: boolean } = {}) { permissionMode: 'auto', }, queuedMessages: [], + queuedMessageDispatchPending: false, theme: { palette: getBuiltInPalette('dark') }, toolOutputExpanded: false, todoPanel: { getTodos: vi.fn(() => []) }, @@ -68,10 +69,14 @@ function makeHost(options: { createGoalRejects?: boolean } = {}) { flushNow: vi.fn(), resetToolUi: vi.fn(), finalizeTurn: vi.fn(), + hasActiveTurn: vi.fn(() => false), hasThinkingDraft: vi.fn(() => false), flushThinkingToTranscript: vi.fn(), appendAssistantDelta: vi.fn(), scheduleFlush: vi.fn(), + beginCompaction: vi.fn(), + endCompaction: vi.fn(), + cancelCompaction: vi.fn(), }, requireSession: vi.fn(() => session), setAppState: vi.fn(), @@ -139,6 +144,20 @@ function turnEndedEvent() { } as const; } +function compactionCompletedEvent() { + return { + type: 'compaction.completed', + sessionId: 's1', + agentId: 'main', + result: { + summary: 'summary', + tokensBefore: 100, + tokensAfter: 10, + compactedCount: 1, + }, + } as const; +} + function modelBlockedEvent() { return { type: 'goal.updated', @@ -234,6 +253,76 @@ describe('SessionEventHandler goal queue promotion', () => { expect(host.sendQueuedMessage).toHaveBeenLastCalledWith(session, { text: 'Ship queued goal' }); }); + it('defers queued-goal promotion while a queued message is mid-dispatch', async () => { + const { host, session } = makeHost(); + host.state.appState.streamingPhase = 'idle'; + host.state.queuedMessages = []; + // The queue looks empty and the phase is idle, but a shifted queued message + // is still awaiting its deferred send. Promotion must not jump ahead of it. + host.state.queuedMessageDispatchPending = true; + const handler = new SessionEventHandler(host); + + handler.requestQueuedGoalPromotion(); + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(session.createGoal).not.toHaveBeenCalled(); + + // Once the queued message has been dispatched, the flag clears and the + // promotion proceeds on the next retry. + host.state.queuedMessageDispatchPending = false; + handler.retryQueuedGoalPromotion(); + await vi.waitFor(() => { + expect(session.createGoal).toHaveBeenCalledWith({ + objective: 'Ship queued goal', + replace: false, + }); + }); + }); + + it('waits for a queued user input drained after compaction before promoting the next queued goal', async () => { + const { host, session } = makeHost(); + host.state.appState.isCompacting = true; + host.state.queuedMessages = [{ text: 'queued user turn' }]; + host.shiftQueuedMessage.mockImplementation(() => host.state.queuedMessages.shift()); + const handler = new SessionEventHandler(host); + host.setAppState.mockImplementation((patch: Record) => { + const busyChanged = 'streamingPhase' in patch || 'isCompacting' in patch; + Object.assign(host.state.appState, patch); + if (busyChanged) handler.retryQueuedGoalPromotion(); + }); + host.sendQueuedMessage.mockImplementation((_session: unknown, item: { text: string }) => { + if (item.text === 'queued user turn') { + host.setAppState({ streamingPhase: 'waiting' }); + } + }); + const sendQueued = sendQueuedViaHost(host, session); + + handler.requestQueuedGoalPromotion(); + handler.handleEvent(compactionCompletedEvent(), sendQueued); + + await vi.waitFor(() => { + expect(host.sendQueuedMessage).toHaveBeenCalledWith(session, { text: 'queued user turn' }); + }); + expect(session.createGoal).not.toHaveBeenCalled(); + + handler.handleEvent(turnEndedEvent(), sendQueued); + + await vi.waitFor(() => { + expect(session.createGoal).toHaveBeenCalledWith({ + objective: 'Ship queued goal', + replace: false, + }); + }); + const sendQueuedCalls = host.sendQueuedMessage.mock.calls as Array<[unknown, { text?: string }]>; + const userMessageIndex = sendQueuedCalls.findIndex( + ([, item]) => item.text === 'queued user turn', + ); + expect(userMessageIndex).toBeGreaterThanOrEqual(0); + expect(host.sendQueuedMessage).toHaveBeenLastCalledWith(session, { text: 'Ship queued goal' }); + const userMessageOrder = host.sendQueuedMessage.mock.invocationCallOrder[userMessageIndex]!; + const goalCreateOrder = session.createGoal.mock.invocationCallOrder[0]!; + expect(userMessageOrder).toBeLessThan(goalCreateOrder); + }); + it('leaves the queued goal in place when the next goal cannot start', async () => { const { host, session } = makeHost({ createGoalRejects: true }); const handler = new SessionEventHandler(host); diff --git a/packages/agent-core/src/agent/goal/index.ts b/packages/agent-core/src/agent/goal/index.ts index d8f8bafd3..7fb2e2946 100644 --- a/packages/agent-core/src/agent/goal/index.ts +++ b/packages/agent-core/src/agent/goal/index.ts @@ -19,6 +19,15 @@ import { /** Maximum objective length in characters. */ const MAX_GOAL_OBJECTIVE_LENGTH = 4000; +/** + * Maximum completion-criterion length in characters. The criterion is repeated + * in every active/paused/blocked goal reminder, so an unbounded one would bloat + * both `state.json` and every continuation prompt. Unlike the objective (which + * is rejected when too long), this supplementary field is truncated so an + * over-long criterion never fails goal creation outright. + */ +const MAX_GOAL_COMPLETION_CRITERION_LENGTH = MAX_GOAL_OBJECTIVE_LENGTH; + const GOAL_CANCELLED_REMINDER = [ 'The user cancelled the current goal.', 'Ignore earlier active-goal reminders for that goal.', @@ -760,5 +769,8 @@ function budgetTelemetryProperties(limits: GoalBudgetLimits): TelemetryPropertie function normalizeCompletionCriterion(value: string | undefined): string | undefined { const trimmed = value?.trim(); - return trimmed?.length ? trimmed : undefined; + if (!trimmed?.length) return undefined; + return trimmed.length > MAX_GOAL_COMPLETION_CRITERION_LENGTH + ? trimmed.slice(0, MAX_GOAL_COMPLETION_CRITERION_LENGTH) + : trimmed; } diff --git a/packages/agent-core/src/agent/injection/goal.ts b/packages/agent-core/src/agent/injection/goal.ts index 7f3886bd1..75f008bcf 100644 --- a/packages/agent-core/src/agent/injection/goal.ts +++ b/packages/agent-core/src/agent/injection/goal.ts @@ -141,16 +141,20 @@ function buildGoalReminder(goal: GoalSnapshot): string { 'Goal mode is iterative. Keep the self-audit brief each turn. Do not explore unrelated ' + 'interpretations once the goal can be decided. If the objective is simple, already answered, ' + 'impossible, unsafe, or contradictory, do not run another goal turn. Explain briefly if useful, ' + - 'then call UpdateGoal with `complete` or `blocked` in the same turn. Otherwise, self-audit ' + - 'against the objective and any completion criteria above, then do one coherent slice of work ' + - 'toward the objective. Use multiple turns when the task naturally has multiple phases. Call ' + - 'UpdateGoal with `complete` only when all required work is done, any stated validation has ' + - 'passed, and there is no useful next action. Do not mark complete after only producing a plan, ' + - 'summary, first pass, or partial result. If an external condition or required user input ' + - 'prevents progress, or the objective cannot be completed as stated, call UpdateGoal with ' + - '`blocked`. Otherwise keep working — after your turn ends you will be prompted to continue. ' + - "Call UpdateGoal as soon as the goal is genuinely done or cannot proceed; don't keep going " + - 'once there is nothing left to do.', + 'then call UpdateGoal with `complete` or `blocked` in the same turn. Otherwise, choose one ' + + 'bounded, useful slice of work toward the objective. Do not try to finish a broad goal in one ' + + 'turn unless the whole goal is genuinely small. Most goal turns should not call UpdateGoal: ' + + 'after completing a useful slice, if material work remains, end the turn normally without ' + + 'calling UpdateGoal so the runtime can continue the goal in the next turn. Call UpdateGoal ' + + 'with `complete` only when all required work is done, any stated validation has passed, and ' + + 'there is no useful next action. Do not mark complete after only producing a plan, summary, ' + + 'first pass, or partial result. Before calling `complete`, check that every required part of ' + + 'the objective is done, completion criteria are satisfied, requested or expected validation ' + + 'passed or has been reported as impossible, and no known material task remains. Call ' + + 'UpdateGoal with `blocked` only for a genuine impasse: an external condition, required user ' + + 'input, missing credentials or permissions, a persistent technical failure, or an impossible, ' + + 'unsafe, or contradictory objective. Do not use `blocked` because the work is large, hard, ' + + 'slow, uncertain, partial, still needs validation, or needs more goal turns.', ); return lines.join('\n'); } @@ -195,5 +199,7 @@ function formatElapsed(ms: number): string { if (totalSeconds < 60) return `${totalSeconds}s`; const minutes = Math.floor(totalSeconds / 60); const seconds = totalSeconds % 60; - return `${minutes}m${seconds.toString().padStart(2, '0')}s`; + if (minutes < 60) return `${minutes}m${seconds.toString().padStart(2, '0')}s`; + const hours = Math.floor(minutes / 60); + return `${hours}h${(minutes % 60).toString().padStart(2, '0')}m`; } diff --git a/packages/agent-core/src/agent/tool/index.ts b/packages/agent-core/src/agent/tool/index.ts index d0a6d6b26..5ddd0dc3c 100644 --- a/packages/agent-core/src/agent/tool/index.ts +++ b/packages/agent-core/src/agent/tool/index.ts @@ -829,14 +829,8 @@ export class ToolManager { ? enabledMcpNames : enabledMcpNames.filter((name) => loadedSet.has(name)); const selectToolsName = disclosure ? [b.SELECT_TOOLS_TOOL_NAME] : []; - // Mutation goal tools are only offered to the model while a goal exists. - const hideGoalMutationTools = this.agent.goal.getGoal().goal === null; return uniq([...this.enabledTools, ...selectToolsName, ...mcpNames]) .toSorted((a, b) => a.localeCompare(b)) - .filter( - (name) => - !(hideGoalMutationTools && (name === 'SetGoalBudget' || name === 'UpdateGoal')), - ) // select_tools is exposed exclusively through the disclosure gate — a // profile or setActiveTools listing the name explicitly must not // surface it in inline mode (it was silently dropped back when diff --git a/packages/agent-core/src/agent/turn/index.ts b/packages/agent-core/src/agent/turn/index.ts index 9d722ea0e..1492fa557 100644 --- a/packages/agent-core/src/agent/turn/index.ts +++ b/packages/agent-core/src/agent/turn/index.ts @@ -7,7 +7,6 @@ import { APIEmptyResponseError, APIStatusError, APITimeoutError, - grandTotal, inputTotal, isContextOverflowStatusError, type ContentPart, @@ -69,8 +68,6 @@ const LLM_NOT_SET_MESSAGE = 'LLM not set, send "/login" to login'; /** Origin tag for the synthetic "continue" prompt that drives each goal turn. */ const GOAL_CONTINUATION_ORIGIN: PromptOrigin = { kind: 'system_trigger', name: 'goal_continuation' }; -export const GOAL_COMPLETION_REMINDER_NAME = 'goal_completion'; -export const GOAL_BLOCKED_REMINDER_NAME = 'goal_blocked'; const GOAL_RATE_LIMIT_PAUSE_REASON = 'Paused after provider rate limit'; const GOAL_PROVIDER_CONNECTION_PAUSE_PREFIX = 'Paused after provider connection error'; const GOAL_PROVIDER_AUTH_PAUSE_PREFIX = 'Paused after provider authentication error'; @@ -90,13 +87,21 @@ const GOAL_CONTINUATION_PROMPT = [ 'decided. If the objective is simple, already answered, impossible, unsafe, or contradictory,', 'do not run another goal turn. Explain briefly if useful, then call UpdateGoal with `complete`', 'or `blocked` in the same turn. Otherwise, weigh the objective and any completion criteria', - 'against the work done so far. Goal mode is iterative: do one coherent slice of work, then', - 'reassess. Call UpdateGoal with `complete` only when all required work is done, any stated', - 'validation has passed, and there is no useful next action. Do not mark complete after only', - 'producing a plan, summary, first pass, or partial result. If an external condition or required', - 'user input prevents progress, or the objective cannot be completed as stated, call UpdateGoal', - 'with `blocked`. Otherwise keep going — use the existing conversation context and your tools,', - 'and do not ask the user for input unless a real blocker prevents progress.', + 'against the work done so far, choose one bounded, useful slice of work, and use the existing', + 'conversation context and your tools. Do not try to finish a broad goal in one turn unless the', + 'whole goal is genuinely small. Most goal turns should not call UpdateGoal: after completing a', + 'useful slice, if material work remains, end the turn normally without calling UpdateGoal so', + 'the runtime can continue the goal in the next turn. Call UpdateGoal with `complete` only when', + 'all required work is done, any stated validation has passed, and there is no useful next', + 'action. Do not mark complete after only producing a plan, summary, first pass, or partial', + 'result. Before calling `complete`, check that every required part of the objective is done,', + 'completion criteria are satisfied, requested or expected validation passed or has been', + 'reported as impossible, and no known material task remains. Call UpdateGoal with `blocked`', + 'only for a genuine impasse: an external condition, required user input, missing credentials', + 'or permissions, a persistent technical failure, or an impossible, unsafe, or contradictory', + 'objective. Do not use `blocked` because the work is large, hard, slow, uncertain, partial,', + 'still needs validation, or needs more goal turns. Do not ask the user for input unless a real', + 'blocker prevents progress.', ].join(' '); export class TurnFlow { @@ -680,6 +685,7 @@ export class TurnFlow { private async runStepLoop(turnId: number, signal: AbortSignal): Promise { let stopHookContinuationUsed = false; let goalOutcomeMessageContinuationUsed = false; + let goalOutcomeToolResultPending = false; const deduper = new ToolCallDeduplicator({ telemetry: this.agent.telemetry }); await this.agent.mcp?.waitForInitialLoad(signal); // Surface the active goal at the start of the turn (append-only; no-op when @@ -712,7 +718,7 @@ export class TurnFlow { maxRetryAttempts: loopControl?.maxRetriesPerStep, recordStepUsage: async (usage) => { try { - const snapshot = await this.agent.goal.recordTokenUsage(grandTotal(usage)); + const snapshot = await this.agent.goal.recordTokenUsage(usage.output); stopForGoalBudget = snapshot?.budget.overBudget === true; } catch (error) { this.agent.log.warn('goal token accounting failed', { error }); @@ -742,8 +748,21 @@ export class TurnFlow { // oxlint-disable-next-line no-loop-func -- stop hook continuation state is scoped to this turn. shouldContinueAfterStop: async (ctx) => { const { signal } = ctx; - // 1. Flush any steered user messages. - if (this.flushSteerBuffer()) return { continue: true }; + const flushedSteeredMessages = this.flushSteerBuffer(); + // 0. A reached hard goal budget is a deterministic ceiling. While + // the goal is still active, never extend the turn — neither a + // steered message nor a Stop-hook continuation — past it; end + // the turn so the goal driver blocks the goal at the boundary. + // Buffered steers are still flushed above so real-time user + // input is preserved in context even when the budget stops the + // turn. A goal the model just marked terminal is no longer + // active, so its final outcome message (step 2 below) still runs. + if (stopForGoalBudget && this.agent.goal.getActiveGoal() !== null) { + return { continue: false }; + } + // 1. If steered user messages were flushed and no active-goal + // budget stopped the turn, let the model react to them. + if (flushedSteeredMessages) return { continue: true }; signal.throwIfAborted(); // Print-mode drain: when `kimi -p` ends a turn while background @@ -769,15 +788,16 @@ export class TurnFlow { } } - // 2. After UpdateGoal marks a goal terminal, ask the model for one - // final user-facing outcome message before the turn ends. + // 2. After UpdateGoal marks a goal terminal, its tool result carries + // the final-message reminder. Let the model read that result and + // produce one user-facing outcome message before the turn ends. if ( !goalOutcomeMessageContinuationUsed && - isGoalOutcomeReminderOrigin(this.agent.context.history.at(-1)?.origin) + goalOutcomeToolResultPending ) { goalOutcomeMessageContinuationUsed = true; + goalOutcomeToolResultPending = false; if (!hasStepBudgetRemaining(loopControl?.maxStepsPerTurn, ctx.stepNumber)) { - this.agent.context.popMatchedMessage(isGoalOutcomeReminderOrigin); return { continue: false }; } return { continue: true }; @@ -843,12 +863,16 @@ export class TurnFlow { toolOutput: isError === true ? undefined : toolOutputText(output).slice(0, 2000), }, }); - return budgetToolResultForModel({ + const modelResult = await budgetToolResultForModel({ homedir: this.agent.homedir, toolName: ctx.toolCall.name, toolCallId: ctx.toolCall.id, result: finalResult, }); + if (isTerminalUpdateGoalResult(ctx.toolCall.name, ctx.args, finalResult)) { + goalOutcomeToolResultPending = true; + } + return modelResult; }, }, }); @@ -1085,18 +1109,23 @@ export class TurnFlow { } } -function isGoalOutcomeReminderOrigin(origin: PromptOrigin | undefined): boolean { - return ( - origin?.kind === 'system_trigger' && - (origin.name === GOAL_COMPLETION_REMINDER_NAME || - origin.name === GOAL_BLOCKED_REMINDER_NAME) - ); -} - function hasStepBudgetRemaining(maxSteps: number | undefined, currentStep: number): boolean { return maxSteps === undefined || maxSteps <= 0 || currentStep < maxSteps; } +function isTerminalUpdateGoalResult( + toolName: string, + args: unknown, + result: ExecutableToolResult, +): boolean { + if (toolName !== 'UpdateGoal' || result.isError === true || result.stopTurn !== true) { + return false; + } + if (!isPlainRecord(args)) return false; + const status = args['status']; + return status === 'complete' || status === 'blocked'; +} + function mapLoopEvent(event: LoopEvent, turnId: number): AgentEvent | undefined { switch (event.type) { case 'step.begin': diff --git a/packages/agent-core/src/loop/run-turn.ts b/packages/agent-core/src/loop/run-turn.ts index 06a59c1d5..cb0f328e4 100644 --- a/packages/agent-core/src/loop/run-turn.ts +++ b/packages/agent-core/src/loop/run-turn.ts @@ -46,8 +46,8 @@ export interface RunTurnInput { /** * Per-step tool table builder. When present it wins over `tools` and is * re-invoked before every step, so a tool loaded mid-turn (select_tools - * schema injection) is dispatchable on the very next step and state-driven - * visibility (e.g. goal mutation tools) stays fresh. `tools` remains as the + * schema injection) is dispatchable on the very next step and runtime tool + * visibility stays fresh. `tools` remains as the * static per-turn snapshot for hosts without dynamic tool tables. */ readonly buildTools?: (() => readonly ExecutableTool[]) | undefined; diff --git a/packages/agent-core/src/skill/builtin/write-goal.md b/packages/agent-core/src/skill/builtin/write-goal.md index b258722d7..49e5e9997 100644 --- a/packages/agent-core/src/skill/builtin/write-goal.md +++ b/packages/agent-core/src/skill/builtin/write-goal.md @@ -13,7 +13,7 @@ This skill is about authoring the objective text together with the user. Draftin **This is the most important rule in this skill. Every decision you put to the user goes through `AskUserQuestion`. No exceptions except one (below).** -Goal authoring is a chain of choices — what to scope, which phrasing, whether to add a budget and how large, which permission mode to start under. For every one of them: **stop and call `AskUserQuestion`.** Do not write a paragraph that lists options and asks the user to reply in prose. Do not say "let me know if you'd prefer A or B." Do not bundle three questions into a wall of text. If you catch yourself typing out choices for the user to answer in free text, delete it and call `AskUserQuestion` instead. +Goal authoring is a chain of choices — what to scope, which phrasing, whether to add a budget and how large, which permission mode to start under. For every one of them: **stop and call `AskUserQuestion`.** Batch related choices in the same `AskUserQuestion` call when that helps the user decide. Do not write a paragraph that lists options and asks the user to reply in prose. Do not say "let me know if you'd prefer A or B." Do not bundle three questions into a wall of text. If you catch yourself typing out choices for the user to answer in free text, delete it and call `AskUserQuestion` instead. A prose menu is a defect, not a style choice: it is slower for the user, easy to skim past, and usually gets a vague answer that forces another round. The only time you may ask in plain text is when `AskUserQuestion` is genuinely unavailable — auto mode, or a host that does not support it — and only then do you fall back to a short message with clearly labelled options and wait. Plain prose for *open-ended* input ("what would prove this is done?") is fine; the rule is about **choices between options**, which always use the tool. diff --git a/packages/agent-core/src/tools/builtin/goal/set-goal-budget.ts b/packages/agent-core/src/tools/builtin/goal/set-goal-budget.ts index 1078ea8e3..56ea0fb58 100644 --- a/packages/agent-core/src/tools/builtin/goal/set-goal-budget.ts +++ b/packages/agent-core/src/tools/builtin/goal/set-goal-budget.ts @@ -39,14 +39,25 @@ export class SetGoalBudgetTool implements BuiltinTool { const goal = this.agent.goal; const normalizedArgs = normalizeBudgetInput(args); + const budget = budgetLimitsFromInput(normalizedArgs); + // Recording a budget the goal has already spent (e.g. the user says "one + // turn" after a turn was already used) leaves the goal immediately over + // budget. The goal driver only enforces budgets at turn boundaries, so + // without a stop here the rest of this tool batch and the next model step + // would run past the ceiling before the goal is blocked. Predict that case + // and stop the batch; the driver then blocks the goal at the boundary. + const overBudgetAfterSet = budget !== null && this.wouldExceedBudget(budget); return { description: `Setting goal budget: ${formatBudget( normalizedArgs.value, normalizedArgs.unit, )}`, + stopBatchAfterThis: overBudgetAfterSet, approvalRule: this.name, execute: async () => { - const budget = budgetLimitsFromInput(normalizedArgs); + if (goal.getGoal().goal === null) { + return { output: 'Goal budget not set: no current goal.' }; + } if (budget === null) { return { output: @@ -54,13 +65,42 @@ export class SetGoalBudgetTool implements BuiltinTool { 'reasonable goal budget.', }; } - await goal.setBudgetLimits({ budgetLimits: budget }, 'model'); + const snapshot = await goal.setBudgetLimits({ budgetLimits: budget }, 'model'); + if (snapshot.budget.overBudget) { + return { + output: + `Goal budget set: ${formatBudget(normalizedArgs.value, normalizedArgs.unit)}. ` + + 'The goal has already reached this budget and will stop now.', + stopTurn: true, + }; + } return { output: `Goal budget set: ${formatBudget(normalizedArgs.value, normalizedArgs.unit)}.`, }; }, }; } + + /** + * Predicts whether merging {@link newLimits} into the current goal's budget + * would already be at or over budget, mirroring the reached-budget math in + * `computeBudgetReport`. Used to stop the tool batch synchronously when a + * just-set budget is exhausted. Returns false when there is no current goal + * (the set itself will reject with GOAL_NOT_FOUND). + */ + private wouldExceedBudget(newLimits: GoalBudgetLimits): boolean { + const goal = this.agent.goal.getGoal().goal; + if (goal === null) return false; + const current = goal.budget; + const turnBudget = newLimits.turnBudget ?? current.turnBudget; + const tokenBudget = newLimits.tokenBudget ?? current.tokenBudget; + const wallClockBudgetMs = newLimits.wallClockBudgetMs ?? current.wallClockBudgetMs; + return ( + (turnBudget !== null && goal.turnsUsed >= turnBudget) || + (tokenBudget !== null && goal.tokensUsed >= tokenBudget) || + (wallClockBudgetMs !== null && goal.wallClockMs >= wallClockBudgetMs) + ); + } } function normalizeBudgetInput(input: SetGoalBudgetToolInput): SetGoalBudgetToolInput { diff --git a/packages/agent-core/src/tools/builtin/goal/update-goal.md b/packages/agent-core/src/tools/builtin/goal/update-goal.md index 29dc0b978..614a9c03a 100644 --- a/packages/agent-core/src/tools/builtin/goal/update-goal.md +++ b/packages/agent-core/src/tools/builtin/goal/update-goal.md @@ -2,7 +2,7 @@ Set the status of the current goal. This is how you resume, end, or yield an aut - `active` — resume a paused or blocked goal when the user explicitly asks you to work on that goal. - `complete` — the objective is satisfied and any stated validation has passed. The goal ends and a completion summary is recorded. -- `blocked` — an external condition or required user input prevents progress, or the objective cannot be completed as stated. The goal stops but can be resumed later. Do not use `blocked` merely because the work is hard, slow, uncertain, or incomplete — reserve it for a genuine impasse. +- `blocked` — a genuine impasse prevents useful progress: an external condition, required user input, missing credentials or permissions, a persistent technical failure, or an impossible, unsafe, or contradictory objective. The goal stops but can be resumed later. Do not use `blocked` because the work is large, hard, slow, uncertain, partial, still needs validation, or needs more goal turns. - `paused` — set the goal aside for now (e.g. to hand control back to the user). It can be resumed later. -If the goal is active and you do not call this, the goal keeps running: after your turn ends you will be prompted to continue. Call `complete` only when all required work is done, any stated validation has passed, and there is no useful next action. Do not call `complete` after only producing a plan, summary, first pass, or partial result. If you call `blocked`, you will be prompted to explain the blocker in your next message. Setting the status is the machine-readable signal; the completion summary or blocker explanation is yours to write in the following message. +Most active goal turns should not call this tool. If you complete one useful slice of work and material work remains, end the turn normally without calling UpdateGoal; the runtime will prompt you to continue in the next goal turn. Call `complete` only when all required work is done, any stated validation has passed, and there is no useful next action. Before calling `complete`, check that every required part of the objective is done, completion criteria are satisfied, requested or expected validation passed or has been reported as impossible, and no known material task remains. Do not call `complete` after only producing a plan, summary, first pass, or partial result. If you call `blocked`, you will be prompted to explain the blocker in your next message. Setting the status is the machine-readable signal; the completion summary or blocker explanation is yours to write in the following message. diff --git a/packages/agent-core/src/tools/builtin/goal/update-goal.ts b/packages/agent-core/src/tools/builtin/goal/update-goal.ts index f1bab7317..2a1f0eb34 100644 --- a/packages/agent-core/src/tools/builtin/goal/update-goal.ts +++ b/packages/agent-core/src/tools/builtin/goal/update-goal.ts @@ -6,17 +6,13 @@ * * The argument is intentionally just a status enum — no reason or evidence. The * model explains itself in its own reply; the status is the machine-readable - * signal. The tool is only offered to the model while a goal exists (see the - * `loopTools` filter in the tool manager). + * signal. The tool stays visible to the main agent even when no goal is active; + * goal-store operations decide whether a requested transition is valid. */ import type { Agent } from '#/agent'; import { z } from 'zod'; -import { - GOAL_BLOCKED_REMINDER_NAME, - GOAL_COMPLETION_REMINDER_NAME, -} from '../../../agent/turn'; import { buildGoalBlockedReasonPrompt, buildGoalCompletionSummaryPrompt, @@ -45,40 +41,41 @@ export class UpdateGoalTool implements BuiltinTool { resolveExecution(args: UpdateGoalToolInput): ToolExecution { const goal = this.agent.goal; + const currentGoal = goal.getGoal().goal; + const goalIsActive = currentGoal?.status === 'active'; return { description: `Setting goal status: ${args.status}`, - stopBatchAfterThis: args.status !== 'active', + stopBatchAfterThis: args.status !== 'active' && goalIsActive, approvalRule: this.name, execute: async () => { if (args.status === 'active') { + if (currentGoal === null) { + return { output: 'Goal not resumed: no current goal.' }; + } await goal.resumeGoal({}, 'model'); return { output: 'Goal resumed.' }; } if (args.status === 'complete') { const completed = await goal.markComplete({}, 'model'); - // `complete` is transient: markComplete announces then clears the - // record. Store the summary request as a system reminder, so the next - // provider request ends with a user message after the UpdateGoal tool - // result. Anthropic-compatible providers reject trailing assistant - // messages as unsupported prefill. - if (completed !== null) { - this.agent.context.appendSystemReminder(buildGoalCompletionSummaryPrompt(completed), { - kind: 'system_trigger', - name: GOAL_COMPLETION_REMINDER_NAME, - }); + if (completed === null) { + return { output: 'Goal not completed: no active goal.' }; } - return { output: 'Goal marked complete.', stopTurn: true }; + const output = + buildGoalCompletionSummaryPrompt(completed); + return { output, stopTurn: true }; } if (args.status === 'blocked') { const blocked = await goal.markBlocked({}, 'model'); - if (blocked !== null) { - this.agent.context.appendSystemReminder(buildGoalBlockedReasonPrompt(blocked), { - kind: 'system_trigger', - name: GOAL_BLOCKED_REMINDER_NAME, - }); + if (blocked === null) { + return { output: 'Goal not blocked: no active goal.' }; } - return { output: 'Goal marked blocked.', stopTurn: true }; + const output = + buildGoalBlockedReasonPrompt(blocked); + return { output, stopTurn: true }; + } + if (currentGoal === null) { + return { output: 'Goal not paused: no current goal.' }; } await goal.pauseGoal({}, 'model'); return { output: 'Goal paused.', stopTurn: true }; diff --git a/packages/agent-core/test/agent/goal.test.ts b/packages/agent-core/test/agent/goal.test.ts index 408c5de7c..4cb2cb692 100644 --- a/packages/agent-core/test/agent/goal.test.ts +++ b/packages/agent-core/test/agent/goal.test.ts @@ -81,6 +81,17 @@ describe('GoalMode creation', () => { expect(goals.getGoal().goal?.completionCriterion).toBe('tests pass'); }); + it('truncates an over-long completion criterion instead of failing', async () => { + const { goals } = makeGoalMode(); + + const snapshot = await goals.createGoal({ + objective: 'Ship feature X', + completionCriterion: 'c'.repeat(4001), + }); + + expect(snapshot.completionCriterion).toBe('c'.repeat(4000)); + }); + it('sets no default work caps when none is provided', async () => { const { goals } = makeGoalMode(); diff --git a/packages/agent-core/test/agent/injection/goal.test.ts b/packages/agent-core/test/agent/injection/goal.test.ts index 53e0c924c..ff02efb03 100644 --- a/packages/agent-core/test/agent/injection/goal.test.ts +++ b/packages/agent-core/test/agent/injection/goal.test.ts @@ -117,6 +117,18 @@ describe('GoalInjector content', () => { expect(text).toContain('turns 0/5'); }); + it('formats wall-clock budgets of an hour or more with an hours unit', async () => { + const store = makeStore(); + await store.createGoal({ objective: 'work' }); + await store.setBudgetLimits( + { budgetLimits: { wallClockBudgetMs: 2 * 60 * 60 * 1000 } }, + 'model', + ); + const text = (await injectOnce(store))!; + expect(text).toContain('2h00m'); + expect(text).not.toContain('120m00s'); + }); + it('uses the within-budget band below 75 percent', async () => { const store = makeStore(); await store.createGoal({ objective: 'work' }); @@ -162,10 +174,20 @@ describe('GoalInjector content', () => { await store.createGoal({ objective: 'fix the bugs' }); const text = (await injectOnce(store))!; expect(text).toContain('Goal mode is iterative'); - expect(text).toContain('one coherent slice of work'); + expect(text).toContain('one bounded, useful slice of work'); + expect(text).toContain('end the turn normally without calling UpdateGoal'); expect(text).toContain('Do not mark complete after only producing a plan'); }); + it('reserves blocked for genuine impasses rather than ordinary unfinished work', async () => { + const store = makeStore(); + await store.createGoal({ objective: 'finish the migration' }); + const text = (await injectOnce(store))!; + expect(text).toContain('only for a genuine impasse'); + expect(text).toContain('missing credentials or permissions'); + expect(text).toContain('needs more goal turns'); + }); + it('tells the model to decide simple or impossible goals in the same turn', async () => { const store = makeStore(); await store.createGoal({ objective: 'prove 1+1=3' }); diff --git a/packages/agent-core/test/agent/tool-select.e2e.test.ts b/packages/agent-core/test/agent/tool-select.e2e.test.ts index 86fe42162..9ef5ec207 100644 --- a/packages/agent-core/test/agent/tool-select.e2e.test.ts +++ b/packages/agent-core/test/agent/tool-select.e2e.test.ts @@ -453,19 +453,19 @@ describe('disclosure mode — model switch projection', () => { }); describe('disclosure mode — executable table freshness', () => { - it('reflects goal-state tool visibility without waiting for a new turn snapshot', async () => { - // The loop re-reads loopTools per step (buildTools); the same mechanism - // that makes a selected tool dispatchable mid-turn also makes goal-gated - // mutation tools appear as soon as a goal exists. + it('keeps goal tools visible without waiting for goal state changes', async () => { const ctx = await disclosureAgent(); ctx.configure({ tools: ['Read', 'UpdateGoal', 'SetGoalBudget', 'mcp__*'], provider: DISCLOSURE_PROVIDER, modelCapabilities: DISCLOSURE_CAPABILITIES, }); - expect(ctx.agent.tools.loopTools.map((t) => t.name)).not.toContain('UpdateGoal'); + expect(ctx.agent.tools.loopTools.map((t) => t.name)).toContain('UpdateGoal'); + expect(ctx.agent.tools.loopTools.map((t) => t.name)).toContain('SetGoalBudget'); + await ctx.agent.goal.createGoal({ objective: 'ship the feature' }); expect(ctx.agent.tools.loopTools.map((t) => t.name)).toContain('UpdateGoal'); + expect(ctx.agent.tools.loopTools.map((t) => t.name)).toContain('SetGoalBudget'); }); it('rebuilds the ledger from a replayed history with no in-memory state (resume path)', () => { diff --git a/packages/agent-core/test/harness/goal-session.test.ts b/packages/agent-core/test/harness/goal-session.test.ts index 5c1e45b20..d2d5c0d14 100644 --- a/packages/agent-core/test/harness/goal-session.test.ts +++ b/packages/agent-core/test/harness/goal-session.test.ts @@ -2,7 +2,12 @@ import { mkdtemp, readFile, rm } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { join } from 'pathe'; -import { APIConnectionError, APIStatusError, type ProviderConfig } from '@moonshot-ai/kosong'; +import { + APIConnectionError, + APIStatusError, + type GenerateResult, + type ProviderConfig, +} from '@moonshot-ai/kosong'; import { afterEach, describe, expect, it, vi } from 'vitest'; import { ProviderManager } from '../../src/session/provider-manager'; @@ -146,9 +151,16 @@ describe('goal session end-to-end', () => { const continuationHistory = JSON.stringify(scripted.calls[1]?.history ?? []); expect(continuationHistory).toContain('Keep the self-audit brief'); expect(continuationHistory).toContain('do not run another goal turn'); + expect(continuationHistory).toContain('end the turn normally without calling UpdateGoal'); + expect(continuationHistory).toContain('only for a genuine impasse'); // Terminal UpdateGoal asks the model for one final user-facing summary. expect(scripted.calls).toHaveLength(3); + const completionToolResult = events.find( + (event) => event['type'] === 'tool.result' && event['toolCallId'] === 'c1', + ); + expect(String(completionToolResult?.['output'])).toContain('Goal completed successfully.'); + expect(String(completionToolResult?.['output'])).toContain('Write a concise final message for the user'); const summaryHistory = JSON.stringify(scripted.calls[2]?.history ?? []); expect(summaryHistory).toContain('Goal completed successfully.'); expect(summaryHistory).toContain('Write a concise final message for the user'); @@ -171,6 +183,7 @@ describe('goal session end-to-end', () => { for (const t of ['goal.create', 'goal.update', 'goal.clear']) { expect(types.has(t)).toBe(true); } + expect(JSON.stringify(records)).not.toContain('goal_completion'); expect(types.has('goal.evaluate')).toBe(false); expect(types.has('goal.account_usage')).toBe(false); expect(types.has('goal.continuation')).toBe(false); @@ -349,6 +362,11 @@ describe('goal session end-to-end', () => { await agent.turn.waitForCurrentTurn(); expect(scripted.calls).toHaveLength(2); + const blockedToolResult = events.find( + (event) => event['type'] === 'tool.result' && event['toolCallId'] === 'blocked', + ); + expect(String(blockedToolResult?.['output'])).toContain('Goal blocked.'); + expect(String(blockedToolResult?.['output'])).toContain('concrete blocker'); const reasonHistory = JSON.stringify(scripted.calls[1]?.history ?? []); expect(reasonHistory).toContain('Goal blocked.'); expect(reasonHistory).toContain('State that the goal is blocked'); @@ -402,7 +420,9 @@ describe('goal session end-to-end', () => { }), ); expect((await api.getGoal({ agentId: 'main' })).goal).toBeNull(); - expect(JSON.stringify(agent.context.history)).not.toContain('Write a concise final message'); + expect(JSON.stringify(agent.context.history)).toContain('Write a concise final message'); + expect(JSON.stringify(agent.context.history)).not.toContain('This summary should not run.'); + expect(agent.context.history.at(-1)?.role).toBe('tool'); }); it('pauses the goal on provider rate limits', async () => { @@ -589,6 +609,199 @@ describe('goal session end-to-end', () => { expect(goal?.tokensUsed).toBeGreaterThan(1); }); + it('counts only model output tokens against the goal token budget', async () => { + const sessionDir = await makeTempDir(); + const events: Array> = []; + const largeInputSmallOutput = { + inputOther: 10_000, + inputCacheRead: 20_000, + inputCacheCreation: 30_000, + output: 1, + }; + let calls = 0; + const responses: GenerateResult[] = [ + { + id: 'usage-1', + message: { + role: 'assistant', + content: [{ type: 'text', text: 'working' }], + toolCalls: [], + }, + usage: largeInputSmallOutput, + finishReason: 'completed', + rawFinishReason: 'stop', + }, + { + id: 'usage-2', + message: { + role: 'assistant', + content: [], + toolCalls: [ + { + type: 'function', + id: 'complete', + name: 'UpdateGoal', + arguments: JSON.stringify({ status: 'complete' }), + }, + ], + }, + usage: largeInputSmallOutput, + finishReason: 'tool_calls', + rawFinishReason: 'tool_calls', + }, + { + id: 'usage-3', + message: { + role: 'assistant', + content: [{ type: 'text', text: 'done' }], + toolCalls: [], + }, + usage: largeInputSmallOutput, + finishReason: 'completed', + rawFinishReason: 'stop', + }, + ]; + const generate: NonNullable = async ( + _provider, + _systemPrompt, + _tools, + _history, + callbacks, + options, + ) => { + options?.signal?.throwIfAborted(); + options?.onRequestStart?.(); + const response = responses[calls]; + calls += 1; + if (response === undefined) { + throw new Error(`Unexpected generate call #${String(calls)}`); + } + for (const part of response.message.content) { + await callbacks?.onMessagePart?.(part); + } + for (const toolCall of response.message.toolCalls) { + await callbacks?.onMessagePart?.(toolCall); + } + options?.onStreamEnd?.(); + return response; + }; + const { session, agent } = await setupSession(sessionDir, events, ['UpdateGoal'], generate); + const api = new SessionAPIImpl(session); + await api.createGoal({ agentId: 'main', objective: 'work' }); + await agent.goal.setBudgetLimits({ budgetLimits: { tokenBudget: 3 } }, 'model'); + + agent.turn.prompt([{ type: 'text', text: 'work' }]); + await agent.turn.waitForCurrentTurn(); + await session.flushMetadata(); + + expect(calls).toBe(3); + expect((await api.getGoal({ agentId: 'main' })).goal).toBeNull(); + const records = await readWireRecords(sessionDir); + const usageRecords = records + .filter((record) => record['type'] === 'goal.update' && typeof record['tokensUsed'] === 'number') + .map((record) => record['tokensUsed']); + expect(usageRecords).toEqual([1, 2]); + const completionEvent = events.find((event) => { + const change = event['change'] as Record | undefined; + return event['type'] === 'goal.updated' && change?.['kind'] === 'completion'; + }); + expect(completionEvent?.['change']).toMatchObject({ + kind: 'completion', + status: 'complete', + stats: expect.objectContaining({ tokensUsed: 2 }), + }); + }); + + it('does not let a Stop hook continue past a reached goal budget', async () => { + const sessionDir = await makeTempDir(); + const events: Array> = []; + // A Stop hook that always asks to keep going. Without the budget guard it + // would append its reason and drive another model step past the ceiling. + const stopHook: HookDef = { + event: 'Stop', + command: + 'echo \'{"hookSpecificOutput":{"permissionDecision":"deny","permissionDecisionReason":"keep going"}}\'', + timeout: 5, + }; + const { session, agent, scripted } = await setupSession( + sessionDir, + events, + ['GetGoal'], + undefined, + [stopHook], + ); + const api = new SessionAPIImpl(session); + await api.createGoal({ agentId: 'main', objective: 'work' }); + await agent.goal.setBudgetLimits({ budgetLimits: { tokenBudget: 1 } }, 'model'); + + scripted.mockNextResponse({ + type: 'function', + id: 'g1', + name: 'GetGoal', + arguments: JSON.stringify({}), + }); + scripted.mockNextResponse({ type: 'text', text: 'should not run' }); + + agent.turn.prompt([{ type: 'text', text: 'work' }]); + await agent.turn.waitForCurrentTurn(); + + const goal = (await api.getGoal({ agentId: 'main' })).goal; + // Only the first step ran; the Stop-hook continuation was suppressed. + expect(scripted.calls).toHaveLength(1); + expect(goal?.status).toBe('blocked'); + }); + + it('preserves buffered steered input when the goal budget is reached', async () => { + const sessionDir = await makeTempDir(); + const events: Array> = []; + let agentRef: Awaited>['agent'] | undefined; + let calls = 0; + const generate: NonNullable = async ( + _provider, + _systemPrompt, + _tools, + _history, + callbacks, + options, + ) => { + calls += 1; + if (calls > 1) throw new Error('Budget should stop before another model step'); + options?.signal?.throwIfAborted(); + options?.onRequestStart?.(); + await callbacks?.onMessagePart?.({ type: 'text', text: 'working' }); + agentRef?.turn.steer([{ type: 'text', text: 'urgent user steer' }]); + options?.onStreamEnd?.(); + return { + id: 'budget-steer', + message: { + role: 'assistant', + content: [{ type: 'text', text: 'working' }], + toolCalls: [], + }, + usage: { + inputOther: 0, + inputCacheRead: 0, + inputCacheCreation: 0, + output: 1, + }, + finishReason: 'completed', + rawFinishReason: 'stop', + }; + }; + const { session, agent } = await setupSession(sessionDir, events, ['GetGoal'], generate); + agentRef = agent; + const api = new SessionAPIImpl(session); + await api.createGoal({ agentId: 'main', objective: 'work' }); + await agent.goal.setBudgetLimits({ budgetLimits: { tokenBudget: 1 } }, 'model'); + + agent.turn.prompt([{ type: 'text', text: 'work' }]); + await agent.turn.waitForCurrentTurn(); + + expect(calls).toBe(1); + expect((await api.getGoal({ agentId: 'main' })).goal?.status).toBe('blocked'); + expect(JSON.stringify(agent.context.history)).toContain('urgent user steer'); + }); + it('preserves terminal status and demotes active goals across resume', async () => { const sessionDir = await makeTempDir(); const events: Array> = []; diff --git a/packages/agent-core/test/profile/default-agent-profiles.test.ts b/packages/agent-core/test/profile/default-agent-profiles.test.ts index 42fcf2e5b..17dc6f681 100644 --- a/packages/agent-core/test/profile/default-agent-profiles.test.ts +++ b/packages/agent-core/test/profile/default-agent-profiles.test.ts @@ -42,11 +42,15 @@ describe('default agent profiles', () => { it('lists the goal tools on the agent profile but not on subagent profiles', () => { const agentTools = DEFAULT_AGENT_PROFILES['agent']?.tools ?? []; - expect(agentTools).toEqual(expect.arrayContaining(['CreateGoal', 'GetGoal'])); + expect(agentTools).toEqual( + expect.arrayContaining(['CreateGoal', 'GetGoal', 'SetGoalBudget', 'UpdateGoal']), + ); for (const name of ['coder', 'explore', 'plan']) { const tools = DEFAULT_AGENT_PROFILES[name]?.tools ?? []; expect(tools).not.toContain('CreateGoal'); expect(tools).not.toContain('GetGoal'); + expect(tools).not.toContain('SetGoalBudget'); + expect(tools).not.toContain('UpdateGoal'); } }); diff --git a/packages/agent-core/test/tools/goal.test.ts b/packages/agent-core/test/tools/goal.test.ts index 736e11bee..25855ef0c 100644 --- a/packages/agent-core/test/tools/goal.test.ts +++ b/packages/agent-core/test/tools/goal.test.ts @@ -216,6 +216,15 @@ describe('SetGoalBudgetTool', () => { expect(store.getGoal().goal?.budget.wallClockBudgetMs).toBe(30 * 60 * 1000); }); + it('reports no current goal instead of throwing when no goal exists', async () => { + const tool = new SetGoalBudgetTool(fakeAgent()); + + const result = await executeTool(tool, ctx({ value: 20, unit: 'turns' })); + + expect(result.isError).toBeFalsy(); + expect(result.output).toBe('Goal budget not set: no current goal.'); + }); + it('rounds fractional turn and token budgets before setting them', async () => { const store = makeStore(); await store.createGoal({ objective: 'work' }); @@ -247,20 +256,60 @@ describe('SetGoalBudgetTool', () => { expect(huge.output).toContain('not a reasonable goal budget'); expect(store.getGoal().goal?.budget.wallClockBudgetMs).toBeNull(); }); + + it('stops the batch and turn when the new budget is already exhausted', async () => { + const store = makeStore(); + await store.createGoal({ objective: 'work' }); + await store.incrementTurn(); // turnsUsed = 1 + const tool = new SetGoalBudgetTool(fakeAgent({ goal: store })); + + const execution = tool.resolveExecution({ value: 1, unit: 'turns' }); + if (execution.isError === true) throw new Error('execution should not be an error'); + expect(execution.stopBatchAfterThis).toBe(true); + + const result = await execution.execute({ turnId: '0', toolCallId: 'call_1', signal }); + expect(result.stopTurn).toBe(true); + expect(result.output).toContain('will stop now'); + expect(store.getGoal().goal?.budget.overBudget).toBe(true); + }); + + it('does not stop when the new budget leaves room', async () => { + const store = makeStore(); + await store.createGoal({ objective: 'work' }); + await store.incrementTurn(); // turnsUsed = 1 + const tool = new SetGoalBudgetTool(fakeAgent({ goal: store })); + + const execution = tool.resolveExecution({ value: 5, unit: 'turns' }); + if (execution.isError === true) throw new Error('execution should not be an error'); + expect(execution.stopBatchAfterThis).toBeFalsy(); + + const result = await execution.execute({ turnId: '0', toolCallId: 'call_1', signal }); + expect(result.stopTurn).toBeFalsy(); + expect(result.output).toBe('Goal budget set: 5 turns.'); + }); }); describe('UpdateGoalTool', () => { it('guards against premature blocked status', () => { const description = new UpdateGoalTool(fakeAgent()).description.toLowerCase(); - // Reference spec wording (without the 3-turn machinery kimi lacks). + // Reserve blocked for genuine impasses, not ordinary unfinished work. + expect(description).toContain('genuine impasse'); expect(description).toContain('hard, slow'); + expect(description).toContain('needs more goal turns'); // UpdateGoal also injects the completion/blocked outcome prompt, so it does // more than "only record the status". expect(description).not.toContain('only records the status'); }); - // Terminal paths append follow-up reminders, so the agent needs a context - // exposing appendSystemReminder. + it('discourages calling UpdateGoal after a non-terminal work slice', () => { + const description = new UpdateGoalTool(fakeAgent()).description; + expect(description).toContain('Most active goal turns should not call this tool'); + expect(description).toContain('end the turn normally without calling UpdateGoal'); + expect(description).toContain('every required part of the objective is done'); + }); + + // Keep a capturing context here to prove terminal paths no longer append a + // separate reminder; the outcome prompt is returned as the tool result. function agentWithContext( store: GoalMode, reminders: Array<{ readonly content: string; readonly origin: unknown }> = [], @@ -296,11 +345,10 @@ describe('UpdateGoalTool', () => { ); expect(result.isError).toBeFalsy(); expect(result.stopTurn).toBe(true); + expect(result.output).toContain('Goal completed successfully.'); + expect(result.output).toContain('Write a concise final message for the user'); expect(store.getGoal().goal).toBeNull(); - expect(reminders).toHaveLength(1); - expect(reminders[0]?.origin).toEqual({ kind: 'system_trigger', name: 'goal_completion' }); - expect(reminders[0]?.content).toContain('Goal completed successfully.'); - expect(reminders[0]?.content).toContain('Write a concise final message for the user'); + expect(reminders).toHaveLength(0); }); it('`blocked` marks the goal blocked (resumable) and asks for a blocker reason', async () => { @@ -312,12 +360,11 @@ describe('UpdateGoalTool', () => { ctx({ status: 'blocked' }), ); expect(result.stopTurn).toBe(true); + expect(result.output).toContain('Goal blocked.'); + expect(result.output).toContain('concrete blocker'); expect(store.getGoal().goal?.status).toBe('blocked'); expect(store.getGoal().goal?.terminalReason).toBeUndefined(); - expect(reminders).toHaveLength(1); - expect(reminders[0]?.origin).toEqual({ kind: 'system_trigger', name: 'goal_blocked' }); - expect(reminders[0]?.content).toContain('Goal blocked.'); - expect(reminders[0]?.content).toContain('concrete blocker'); + expect(reminders).toHaveLength(0); }); it('`paused` marks the goal paused', async () => { @@ -340,6 +387,24 @@ describe('UpdateGoalTool', () => { expect(result.output).toBe('Goal resumed.'); expect(store.getGoal().goal?.status).toBe('active'); }); + + it.each([ + ['active', 'Goal not resumed: no current goal.'], + ['complete', 'Goal not completed: no active goal.'], + ['blocked', 'Goal not blocked: no active goal.'], + ['paused', 'Goal not paused: no current goal.'], + ] as const)('reports a no-goal result for `%s` without stopping the turn', async (status, output) => { + const tool = new UpdateGoalTool(agentWithContext(makeStore())); + const execution = tool.resolveExecution({ status }); + if (execution.isError === true) throw new Error('execution should not be an error'); + + expect(execution.stopBatchAfterThis).toBeFalsy(); + const result = await execution.execute({ turnId: '0', toolCallId: 'call_1', signal }); + + expect(result.isError).toBeFalsy(); + expect(result.stopTurn).toBeFalsy(); + expect(result.output).toBe(output); + }); }); describe('ToolManager goal tool registration', () => { @@ -348,7 +413,7 @@ describe('ToolManager goal tool registration', () => { type, }); // configure() gives the agent a provider so builtin tools can initialize. - ctxAgent.configure({ tools: ['Read', 'CreateGoal', 'GetGoal', 'SetGoalBudget'] }); + ctxAgent.configure({ tools: ['Read', 'CreateGoal', 'GetGoal', 'SetGoalBudget', 'UpdateGoal'] }); // Re-run registration so the gate reads the scoped flag resolver state. ctxAgent.agent.tools.initializeBuiltinTools(); return ctxAgent.agent.tools.loopTools.map((tool) => tool.name); @@ -356,8 +421,9 @@ describe('ToolManager goal tool registration', () => { it('exposes goal tools to the main agent', () => { const names = loopToolNames('main'); - expect(names).toEqual(expect.arrayContaining(['CreateGoal', 'GetGoal'])); - expect(names).not.toContain('SetGoalBudget'); + expect(names).toEqual( + expect.arrayContaining(['CreateGoal', 'GetGoal', 'SetGoalBudget', 'UpdateGoal']), + ); }); it('does not expose goal tools to subagents even when enabled', () => { @@ -365,9 +431,10 @@ describe('ToolManager goal tool registration', () => { expect(names).not.toContain('CreateGoal'); expect(names).not.toContain('GetGoal'); expect(names).not.toContain('SetGoalBudget'); + expect(names).not.toContain('UpdateGoal'); }); - it('hides goal mutation tools until a goal exists, then exposes them', async () => { + it('keeps goal mutation tools visible across goal lifecycle states', async () => { const store = makeStore(); const ctxAgent = testAgent({ type: 'main', @@ -375,17 +442,16 @@ describe('ToolManager goal tool registration', () => { }); ctxAgent.configure({ tools: ['Read', 'CreateGoal', 'GetGoal', 'SetGoalBudget', 'UpdateGoal'] }); ctxAgent.agent.tools.initializeBuiltinTools(); - // No goal yet -> mutation tools are filtered out of the model's tool list. - expect(ctxAgent.agent.tools.loopTools.map((t) => t.name)).not.toContain('UpdateGoal'); - expect(ctxAgent.agent.tools.loopTools.map((t) => t.name)).not.toContain('SetGoalBudget'); - // Once a goal exists, it appears. + expect(ctxAgent.agent.tools.loopTools.map((t) => t.name)).toContain('UpdateGoal'); + expect(ctxAgent.agent.tools.loopTools.map((t) => t.name)).toContain('SetGoalBudget'); + await store.createGoal({ objective: 'work' }); expect(ctxAgent.agent.tools.loopTools.map((t) => t.name)).toContain('UpdateGoal'); expect(ctxAgent.agent.tools.loopTools.map((t) => t.name)).toContain('SetGoalBudget'); await store.markComplete({}, 'model'); - expect(ctxAgent.agent.tools.loopTools.map((t) => t.name)).not.toContain('UpdateGoal'); - expect(ctxAgent.agent.tools.loopTools.map((t) => t.name)).not.toContain('SetGoalBudget'); + expect(ctxAgent.agent.tools.loopTools.map((t) => t.name)).toContain('UpdateGoal'); + expect(ctxAgent.agent.tools.loopTools.map((t) => t.name)).toContain('SetGoalBudget'); }); });