diff --git a/docs/design/2026-08-25-goal-draft-skill.md b/docs/design/2026-08-25-goal-draft-skill.md index 15ae50ff384..6e00c2ca2ad 100644 --- a/docs/design/2026-08-25-goal-draft-skill.md +++ b/docs/design/2026-08-25-goal-draft-skill.md @@ -33,9 +33,17 @@ The objective is handed over on one line because `parseGoalCommand` splits on wh - `docs/users/features/goals.md` (commands, how a Goal is judged, writing a good objective, `/goal-draft`), rows in `commands.md`, a pointer from `headless.md`. - The web-shell Goals dialog placeholder now shows an objective with a check, a guardrail, and a budget in both locales. +## Phase 2: `propose_goal` + +A core tool, registered beside `get_goal` / `update_goal` (so never for subagents), only in interactive sessions (`resolveInteractionMode === 'interactive'`) and when `goals.modelProposed` is not `disabled`; ACP and stream-json remain excluded until they have an equivalent turn-boundary settlement path. It reuses the generic `info` confirmation: the objective is in both the invocation description (the one field every host forwards — the Web Shell drops an `info` prompt) and the plain-text prompt, and `requiresUserInteraction()` is `true` so no allow rule, YOLO, or AUTO_EDIT (which auto-approves `info` confirmations) can skip the dialog. Preconditions are checked before the dialog and again in `execute()`, because `/goal` can change the session while the dialog is open: plan mode, untrusted folder, no Goal persistence, and an active Goal all refuse with guidance (an active Goal is never replaced from the tool; the model hands over a `/goal edit` / `/goal set` line instead). A stopped Goal is replaced through `replace` with its expected version; no Goal creates. The tool does not dispatch at all: setting a Goal mid-turn would leave the rest of the proposing turn without a Goal permit (`client.ts` rejects a permit-less continuation while a Goal is active — the first end-to-end run showed exactly that error card). Instead the approval is parked on `Config` (`setPendingGoalProposal`), bound to the proposing turn's `prompt_id`, and the client applies it when the proposing turn truly ends, after queued steer, Stop-hook, and next-speaker continuations finish (`settlePendingGoalProposal` → `applyPendingGoalProposal`); only that turn's own terminal boundary may apply it, and any other frame that finds it parked drops it instead. The runtime's broadcast then renders the Goal card and starts the first Goal turn. An approval still parked when the next real user query starts belongs to a cancelled turn and is discarded; the tool result tells the model to acknowledge in one sentence and stop. + +`goals.modelProposed` (`alwaysAsk` | `disabled`, default `alwaysAsk`) sits in `WORKSPACE_RESTRICTED_SETTINGS`, so a repository cannot switch the tool on. An `auto` mode that lets the model skip the dialog (Claude Code's `ask_user: false`) is deliberately not offered. + +The skill's hand-off now prefers the tool when it is available and no Goal is active, and keeps the printed `/goal set` line for headless runs, the disabled setting, and the active-Goal case. + ## Later phases (not in this change) -- A `propose_goal` core tool with an approval dialog, mirroring Claude Code's `ProposeGoal` + `modelProposedGoals` (read from user/policy settings only), so the skill can offer "Set this goal" instead of a line to paste. `parseGoalCommand` keeps newlines for `set`/`edit`. +- `parseGoalCommand` keeps newlines for `set`/`edit`. - A deterministic lint on `/goal set` (rules 1–6 above) that warns and points at `/goal-draft`, and a "refine" entry in the web-shell Goals dialog. ## Verification diff --git a/docs/users/configuration/settings.md b/docs/users/configuration/settings.md index e83a77f0fce..5c8feb5d06c 100644 --- a/docs/users/configuration/settings.md +++ b/docs/users/configuration/settings.md @@ -377,6 +377,7 @@ If you are experiencing performance issues with file searching (e.g., with `@` c | `tools.useRipgrep` | boolean | Use ripgrep for file content search instead of the fallback implementation. Provides faster search performance. | `true` | | | `tools.useBuiltinRipgrep` | boolean | Use the bundled ripgrep binary. When set to `false`, the system-level `rg` command will be used instead. This setting is only effective when `tools.useRipgrep` is `true`. | `true` | | | `tools.workflowsEnabled` | boolean | Enable the Workflow tool, which lets the model author and run a script that orchestrates subagents in parallel. Off by default; a run can dispatch many subagents and spend tokens accordingly. | `false` | User, System, and SystemDefaults scopes only; workspace values are ignored. Requires restart: Yes. Env overrides: `QWEN_CODE_ENABLE_WORKFLOWS=1` forces on; `QWEN_CODE_DISABLE_WORKFLOWS=1` forces off (disable wins). | +| `goals.modelProposed` | enum | Controls the `propose_goal` tool, which lets the model propose a session Goal for you to approve: `alwaysAsk` shows every proposal in an approval dialog and nothing is set until you accept it; `"disabled"` removes the tool. A typed `/goal` is unaffected. | `alwaysAsk` | User, System, and SystemDefaults scopes only; workspace values are ignored. Requires restart: Yes. | | `tools.truncateToolOutputThreshold` | number | Truncate tool output if it is larger than this many characters. Applies to Shell, Grep, Glob, ReadFile and ReadManyFiles tools. | `25000` | Requires restart: Yes | | `tools.truncateToolOutputLines` | number | Maximum lines or entries kept when truncating tool output. Applies to Shell, Grep, Glob, ReadFile and ReadManyFiles tools. | `1000` | Requires restart: Yes | | `tools.toolSearch.enabled` | boolean | Load MCP tools on demand via ToolSearch to reduce prompt size. Disable this for models that rely on prefix-based KV caching (e.g. DeepSeek) to keep the prompt prefix stable and maximize cache hit rates. | `true` | Requires restart: Yes | diff --git a/docs/users/features/goals.md b/docs/users/features/goals.md index 8c2203bd553..52b82907454 100644 --- a/docs/users/features/goals.md +++ b/docs/users/features/goals.md @@ -51,8 +51,14 @@ Keep it to one objective and roughly under 1,200 characters. `/goal set` and `/g ## Let `/goal-draft` write it -`/goal-draft ` is a bundled skill that does the above for you. It checks whether the request is a Goal at all, reads the workspace for the real test and lint commands instead of guessing, asks at most one round of multiple-choice questions when the answer changes the check or the scope, drafts the objective in the format above, runs the self-check, and prints a `/goal set …` line you can run as-is. It never starts the work itself and never sets the Goal on your behalf. +`/goal-draft ` is a bundled skill that does the above for you. It checks whether the request is a Goal at all, reads the workspace for the real test and lint commands instead of guessing, asks at most one round of multiple-choice questions when the answer changes the check or the scope, drafts the objective in the format above, runs the self-check, and hands it over: in an interactive session it proposes the objective through the `propose_goal` approval dialog described below, otherwise it prints a `/goal set …` line you can run as-is. It never starts the work itself, and nothing is set without your approval. Pass an existing objective to tighten it: `/goal-draft all tests pass and the lint is clean`. +### Approve a Goal the model proposes + +In an interactive terminal session the model has a `propose_goal` tool. When `/goal-draft` finishes, or when you ask for an outcome that spans several turns, it can propose the objective instead of printing a `/goal set …` line for you to copy. The proposal appears as an approval dialog showing the full objective. Approving it sets the Goal exactly as `/goal set` would, the moment the current turn ends (the model acknowledges and stops; the first Goal turn then starts on its own), and declining sets nothing — the model is told only that the Goal was not set, and must not propose it again. The approval is bound to the turn that asked for it: if that turn is cancelled or otherwise never reaches its end, the approval is dropped rather than applied under a later message or an automated turn. No permission rule or approval mode (including YOLO) skips this dialog, and the tool refuses while another Goal is active, in plan mode, in subagents, and in untrusted folders. It is not available in headless runs, nor yet in Web Shell or other ACP-driven sessions (they do not pass through the turn boundary that applies the approval); there the printed `/goal set` line remains the hand-off. + +Turn it off with `goals.modelProposed: "disabled"` in your user settings. Because the setting decides whether the model may ask you to start an autonomous loop, it is honored only from user and system scope; a workspace `.qwen/settings.json` value is ignored with a warning. + The skill is instructed to be read-only, and only its non-mutating tools are auto-approved (`get_goal`, `read_file`, `glob`, `grep_search`). `ask_user_question` is deliberately not auto-approved, so its question dialog is shown before the skill drafts from your answers. Like other bundled skills, a project or personal skill named `goal-draft` overrides it, and `skills.disabled` can turn it off. See [Skills](./skills.md) for how bundled skills are discovered. diff --git a/packages/cli/src/config/config.test.ts b/packages/cli/src/config/config.test.ts index abcc8b3fe88..8ce7870cb8c 100644 --- a/packages/cli/src/config/config.test.ts +++ b/packages/cli/src/config/config.test.ts @@ -17,6 +17,7 @@ import { Storage, SessionIdCaseConflictError, } from '@qwen-code/qwen-code-core'; +import { normalizeModelProposedGoals } from './config.js'; import { isValidSessionId, loadCliConfig, @@ -5865,3 +5866,59 @@ describe('loadCliConfig skills.disabledLevels', () => { expect(config.getDisabledSkillLevels()).toEqual(new Set()); }); }); + +describe('loadCliConfig goals.modelProposed', () => { + const originalArgv = process.argv; + + beforeEach(() => { + vi.resetAllMocks(); + vi.mocked(os.homedir).mockReturnValue('/mock/home/user'); + vi.stubEnv('GEMINI_API_KEY', 'test-api-key'); + }); + + afterEach(() => { + process.argv = originalArgv; + vi.unstubAllEnvs(); + vi.restoreAllMocks(); + }); + + it('defaults to alwaysAsk when goals.modelProposed is not set', async () => { + process.argv = ['node', 'script.js']; + const argv = await parseArguments(); + const config = await loadCliConfig({}, argv, undefined, []); + expect(config.getModelProposedGoals()).toBe('alwaysAsk'); + }); + + // The wiring line in loadCliConfig is the only way the setting reaches + // core; without it "disabled" would be a dead switch and the model would + // keep the propose_goal tool. + it('passes goals.modelProposed = disabled through to core', async () => { + process.argv = ['node', 'script.js']; + const argv = await parseArguments(); + const settings: Settings = { goals: { modelProposed: 'disabled' } }; + const config = await loadCliConfig(settings, argv, undefined, []); + expect(config.getModelProposedGoals()).toBe('disabled'); + }); + + it('falls back to the core default for an unknown value', async () => { + process.argv = ['node', 'script.js']; + const argv = await parseArguments(); + const settings = { + goals: { modelProposed: 'auto' }, + } as unknown as Settings; + const config = await loadCliConfig(settings, argv, undefined, []); + expect(config.getModelProposedGoals()).toBe('alwaysAsk'); + }); +}); + +describe('normalizeModelProposedGoals', () => { + it('passes the two known modes through and drops anything else', () => { + expect(normalizeModelProposedGoals('alwaysAsk')).toBe('alwaysAsk'); + expect(normalizeModelProposedGoals('disabled')).toBe('disabled'); + // An unknown value must not reach core as a third mode; the core + // default (alwaysAsk) applies instead. + expect(normalizeModelProposedGoals('auto')).toBeUndefined(); + expect(normalizeModelProposedGoals(undefined)).toBeUndefined(); + expect(normalizeModelProposedGoals(true)).toBeUndefined(); + }); +}); diff --git a/packages/cli/src/config/config.ts b/packages/cli/src/config/config.ts index cea56783ca5..273177f61bc 100755 --- a/packages/cli/src/config/config.ts +++ b/packages/cli/src/config/config.ts @@ -5,6 +5,7 @@ */ import { + type ModelProposedGoalsMode, ApprovalMode, APPROVAL_MODES, type AuthType, @@ -1286,6 +1287,17 @@ export class SessionIdConflictError extends Error { } } +/** + * `goals.modelProposed` reaches core as a closed enum. Anything else in the + * settings file (a typo, an older value) falls back to the default rather + * than smuggling an unknown mode through. + */ +export function normalizeModelProposedGoals( + value: unknown, +): ModelProposedGoalsMode | undefined { + return value === 'alwaysAsk' || value === 'disabled' ? value : undefined; +} + /** * Resolves the output style for this session. `--output-style` wins over * `general.outputStyle`; an unset, empty, or `default` value means no style. @@ -2310,6 +2322,9 @@ export async function loadCliConfig( useRipgrep: settings.tools?.useRipgrep, useBuiltinRipgrep: settings.tools?.useBuiltinRipgrep, workflowsEnabled: settings.tools?.workflowsEnabled, + modelProposedGoals: normalizeModelProposedGoals( + settings.goals?.modelProposed, + ), shouldUseNodePtyShell: settings.tools?.shell?.enableInteractiveShell, shellDefaultTimeoutMs: settings.tools?.shell?.defaultTimeoutMs, shellHeartbeatIntervalMs: settings.tools?.shell?.heartbeatIntervalMs, diff --git a/packages/cli/src/config/settings.test.ts b/packages/cli/src/config/settings.test.ts index 87ba779e628..6b4752acbec 100644 --- a/packages/cli/src/config/settings.test.ts +++ b/packages/cli/src/config/settings.test.ts @@ -3719,6 +3719,51 @@ describe('Settings Loading and Merging', () => { }); }); + describe('goals.modelProposed scope handling', () => { + it('is listed as workspace-restricted', () => { + expect(WORKSPACE_RESTRICTED_SETTING_KEYS).toContain( + 'goals.modelProposed', + ); + }); + + it('honors goals.modelProposed from user scope', () => { + (mockFsExistsSync as Mock).mockReturnValue(true); + (fs.readFileSync as Mock).mockImplementation( + (p: fs.PathOrFileDescriptor) => { + if (p === USER_SETTINGS_PATH) + return JSON.stringify({ goals: { modelProposed: 'disabled' } }); + return '{}'; + }, + ); + + const settings = loadSettings(MOCK_WORKSPACE_DIR); + expect(settings.merged.goals?.modelProposed).toBe('disabled'); + }); + + it('strips goals.modelProposed from workspace scope and warns', () => { + // A repository must not be able to switch on a tool that asks the + // user to start an autonomous loop; the default is the user's call. + (mockFsExistsSync as Mock).mockReturnValue(true); + (fs.readFileSync as Mock).mockImplementation( + (p: fs.PathOrFileDescriptor) => { + if (p === MOCK_WORKSPACE_SETTINGS_PATH) + return JSON.stringify({ goals: { modelProposed: 'disabled' } }); + return '{}'; + }, + ); + + const settings = loadSettings(MOCK_WORKSPACE_DIR); + // The workspace key was dropped before merging (merged settings do not + // materialise schema defaults, so nothing set means undefined, and the + // core default of alwaysAsk applies downstream). + expect(settings.merged.goals?.modelProposed).toBeUndefined(); + const warnings = getSettingsWarnings(settings); + expect(warnings.some((w) => w.includes('goals.modelProposed'))).toBe( + true, + ); + }); + }); + describe('cross-session settings scope handling', () => { it('should honor the cross-session keys from user scope', () => { (mockFsExistsSync as Mock).mockReturnValue(true); diff --git a/packages/cli/src/config/settingsSchema.ts b/packages/cli/src/config/settingsSchema.ts index 1141372ef10..663768c0456 100644 --- a/packages/cli/src/config/settingsSchema.ts +++ b/packages/cli/src/config/settingsSchema.ts @@ -3279,6 +3279,31 @@ const SETTINGS_SCHEMA = { }, }, + goals: { + type: 'object', + label: 'Goals', + category: 'Advanced', + requiresRestart: true, + default: {}, + description: 'Settings for session Goals (/goal).', + showInDialog: false, + properties: { + modelProposed: { + type: 'enum', + label: 'Model-Proposed Goals', + category: 'Advanced', + requiresRestart: true, + default: 'alwaysAsk', + description: + 'Controls the propose_goal tool, which lets the model propose a session Goal for you to approve. "alwaysAsk" (default) shows every proposal in an approval dialog and nothing is set until you accept it; "disabled" removes the tool. A typed /goal is unaffected. Consent-affecting, so this setting is only honored from User, System, or SystemDefaults scope; workspace values are ignored.', + showInDialog: true, + options: [ + { value: 'alwaysAsk', label: 'Always ask' }, + { value: 'disabled', label: 'Disabled' }, + ], + }, + }, + }, agents: { type: 'object', label: 'Agents', diff --git a/packages/cli/src/config/settingsUtils.ts b/packages/cli/src/config/settingsUtils.ts index dc9f763d203..09e3fcb74ae 100644 --- a/packages/cli/src/config/settingsUtils.ts +++ b/packages/cli/src/config/settingsUtils.ts @@ -270,6 +270,7 @@ export const WORKSPACE_RESTRICTED_SETTINGS = [ { section: 'security', key: 'allowedInsecureVoiceBaseUrls' }, { section: 'agents', key: 'crossSessionMessaging' }, { section: 'agents', key: 'crossSessionInbound' }, + { section: 'goals', key: 'modelProposed' }, ] as const satisfies ReadonlyArray<{ readonly section: keyof Settings; readonly key: string; diff --git a/packages/cli/src/i18n/locales/en.js b/packages/cli/src/i18n/locales/en.js index 18243b98371..ebecbfb4b94 100644 --- a/packages/cli/src/i18n/locales/en.js +++ b/packages/cli/src/i18n/locales/en.js @@ -188,6 +188,7 @@ export default { 'toolDisplayName.TodoList': 'toolDisplayName.TodoList', 'toolDisplayName.Goal': 'toolDisplayName.Goal', 'toolDisplayName.UpdateGoal': 'toolDisplayName.UpdateGoal', + 'toolDisplayName.ProposeGoal': 'toolDisplayName.ProposeGoal', 'toolDisplayName.SaveMemory': 'toolDisplayName.SaveMemory', 'toolDisplayName.Agent': 'toolDisplayName.Agent', 'toolDisplayName.Artifact': 'toolDisplayName.Artifact', diff --git a/packages/cli/src/i18n/locales/zh-TW.js b/packages/cli/src/i18n/locales/zh-TW.js index 1dfcb1c6c98..0ac4272da2e 100644 --- a/packages/cli/src/i18n/locales/zh-TW.js +++ b/packages/cli/src/i18n/locales/zh-TW.js @@ -179,6 +179,7 @@ export default { 'toolDisplayName.TodoList': '任務清單', 'toolDisplayName.Goal': '目標', 'toolDisplayName.UpdateGoal': '更新目標', + 'toolDisplayName.ProposeGoal': '提議目標', 'toolDisplayName.SaveMemory': '儲存記憶', 'toolDisplayName.Agent': 'Agent', 'toolDisplayName.Artifact': '製品', diff --git a/packages/cli/src/i18n/locales/zh.js b/packages/cli/src/i18n/locales/zh.js index 36d926a0ce5..fd6eb292f6c 100644 --- a/packages/cli/src/i18n/locales/zh.js +++ b/packages/cli/src/i18n/locales/zh.js @@ -180,6 +180,7 @@ export default { 'toolDisplayName.TodoList': '任务清单', 'toolDisplayName.Goal': '目标', 'toolDisplayName.UpdateGoal': '更新目标', + 'toolDisplayName.ProposeGoal': '提议目标', 'toolDisplayName.SaveMemory': '保存记忆', 'toolDisplayName.Agent': 'Agent', 'toolDisplayName.Artifact': '制品', diff --git a/packages/cli/src/ui/hooks/use-llm-stream.test.tsx b/packages/cli/src/ui/hooks/use-llm-stream.test.tsx index 728032f369a..5a8fdd319e7 100644 --- a/packages/cli/src/ui/hooks/use-llm-stream.test.tsx +++ b/packages/cli/src/ui/hooks/use-llm-stream.test.tsx @@ -3527,6 +3527,9 @@ describe('useLlmStream', () => { await waitFor(() => { expect(mockSendMessageStream).toHaveBeenCalledTimes(2); }); + expect(mockSendMessageStream.mock.calls[1]?.[3]).toEqual( + expect.objectContaining({ isConcurrentSideQuery: true }), + ); // History scans, in call order: (1) the accept settlement captures // the pushed boundary entry for the debt fingerprint; (2) the diff --git a/packages/cli/src/ui/hooks/use-llm-stream.ts b/packages/cli/src/ui/hooks/use-llm-stream.ts index 487112206db..b2893c8339f 100644 --- a/packages/cli/src/ui/hooks/use-llm-stream.ts +++ b/packages/cli/src/ui/hooks/use-llm-stream.ts @@ -3914,6 +3914,9 @@ export const useLlmStream = ( todoWorkChainId: metadata?.todoWorkChainId, modelOverride: modelOverrideRef.current, steerInput: metadata?.steerInput, + ...(allowConcurrentBtwDuringResponse + ? { isConcurrentSideQuery: true } + : {}), ...(submittedPrompt !== undefined ? { submittedPrompt } : {}), ...(!allowConcurrentBtwDuringResponse && !isDetachedToolContinuation && diff --git a/packages/core/src/config/config.test.ts b/packages/core/src/config/config.test.ts index f476938df31..380404e5b93 100644 --- a/packages/core/src/config/config.test.ts +++ b/packages/core/src/config/config.test.ts @@ -3132,6 +3132,56 @@ describe('Server Config (config.ts)', () => { ); }); + it('parks one approved proposal and hands it to the client once', () => { + const config = new Config({ ...baseParams, chatRecording: true }); + + expect( + config.setPendingGoalProposal({ + objective: 'first', + turnKey: 'turn-1', + }), + ).toBe(true); + expect( + config.setPendingGoalProposal({ + objective: 'second', + turnKey: 'turn-1', + }), + ).toBe(false); + expect(config.hasPendingGoalProposal()).toBe(true); + expect(config.takePendingGoalProposal('turn-2')).toBeUndefined(); + expect(config.hasPendingGoalProposal()).toBe(true); + expect(config.takePendingGoalProposal('turn-1')).toEqual({ + objective: 'first', + turnKey: 'turn-1', + }); + expect(config.hasPendingGoalProposal()).toBe(false); + expect(config.takePendingGoalProposal()).toBeUndefined(); + + expect( + config.setPendingGoalProposal({ + objective: 'explicitly cleared', + turnKey: 'turn-3', + }), + ).toBe(true); + expect(config.takePendingGoalProposal()).toEqual({ + objective: 'explicitly cleared', + turnKey: 'turn-3', + }); + expect(config.hasPendingGoalProposal()).toBe(false); + }); + + it('clears a parked proposal when the session Goal runtime is replaced', () => { + const config = new Config({ ...baseParams, chatRecording: true }); + config.setPendingGoalProposal({ + objective: 'stale approval', + turnKey: 'turn-1', + }); + + config.startNewSession('replacement-session'); + + expect(config.takePendingGoalProposal()).toBeUndefined(); + }); + it('restores the complete resumed-session Goal before exposing readiness', async () => { const config = new Config({ ...baseParams, @@ -5071,6 +5121,68 @@ describe('Server Config (config.ts)', () => { expect(registeredNames).not.toContain(ToolNames.EXIT_PLAN_MODE); }); + it('registers propose_goal beside the Goal worker tools in interactive sessions', async () => { + const config = new Config({ ...baseParams, interactive: true }); + await config.initialize(); + + const registeredNames = ( + ToolRegistry.prototype.registerFactory as Mock + ).mock.calls.map((call) => call[0]); + expect(registeredNames).toContain(ToolNames.GET_GOAL); + expect(registeredNames).toContain(ToolNames.UPDATE_GOAL); + expect(registeredNames).toContain(ToolNames.PROPOSE_GOAL); + }); + it.each([ + ['ACP', { experimentalZedIntegration: true, interactive: true }], + [ + 'stream-json', + { inputFormat: InputFormat.STREAM_JSON, interactive: true }, + ], + ] as const)( + 'does not register propose_goal without a turn-boundary settlement path in %s sessions', + async (_mode, params) => { + const config = new Config({ ...baseParams, ...params }); + await config.initialize(); + + const registeredNames = ( + ToolRegistry.prototype.registerFactory as Mock + ).mock.calls.map((call) => call[0]); + expect(registeredNames).toContain(ToolNames.GET_GOAL); + expect(registeredNames).toContain(ToolNames.UPDATE_GOAL); + expect(registeredNames).not.toContain(ToolNames.PROPOSE_GOAL); + }, + ); + it('does not register propose_goal when goals.modelProposed is disabled', async () => { + const config = new Config({ + ...baseParams, + interactive: true, + modelProposedGoals: 'disabled', + }); + await config.initialize(); + + const registeredNames = ( + ToolRegistry.prototype.registerFactory as Mock + ).mock.calls.map((call) => call[0]); + expect(config.getModelProposedGoals()).toBe('disabled'); + expect(registeredNames).toContain(ToolNames.GET_GOAL); + expect(registeredNames).not.toContain(ToolNames.PROPOSE_GOAL); + }); + it('does not register propose_goal in plain headless sessions', async () => { + const config = new Config({ + ...baseParams, + interactive: false, + experimentalZedIntegration: false, + inputFormat: InputFormat.TEXT, + }); + await config.initialize(); + + const registeredNames = ( + ToolRegistry.prototype.registerFactory as Mock + ).mock.calls.map((call) => call[0]); + expect(config.getModelProposedGoals()).toBe('alwaysAsk'); + expect(registeredNames).toContain(ToolNames.GET_GOAL); + expect(registeredNames).not.toContain(ToolNames.PROPOSE_GOAL); + }); it('does not register user-interaction tools in plain headless sessions', async () => { const config = new Config({ ...baseParams, diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index 0f1724e10fa..1eced0f7d42 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -179,6 +179,7 @@ import { type GoalRuntime, type GoalTurnHost, } from '../goals/goal-runtime.js'; +import type { PendingGoalProposal } from '../goals/goal-tools.js'; import type { GoalRecoveryRecord } from '../goals/goal-persistence.js'; import { GOAL_DEFAULT_TOKEN_BUDGET } from '../goals/goal-protocol.js'; import { createGoalCheckpointVerifier } from '../goals/goal-checkpoint-verifier.js'; @@ -841,6 +842,9 @@ export interface AgentsCollabSettings { }; } +/** `goals.modelProposed`: whether the model may propose a Goal for approval. */ +export type ModelProposedGoalsMode = 'alwaysAsk' | 'disabled'; + export interface ConfigParameters { sessionId?: string; sessionData?: ResumedSessionData; @@ -1069,6 +1073,8 @@ export interface ConfigParameters { lsToolEnabled?: boolean; agentTeamEnabled?: boolean; workflowsEnabled?: boolean; + /** Consent gate for the propose_goal tool; see ProposeGoalTool. */ + modelProposedGoals?: ModelProposedGoalsMode; artifactEnabled?: boolean; artifactAutoOpen?: boolean; artifactPublisher?: 'local' | 'host' | 'oss'; @@ -2274,6 +2280,8 @@ export class Config { private chatRecordingService: ChatRecordingService | undefined = undefined; private goalRuntime: GoalRuntime | undefined; private goalRuntimeReady: Promise | undefined; + /** A `propose_goal` approval waiting for its turn to end; see PendingGoalProposal. */ + private pendingGoalProposal: PendingGoalProposal | undefined; /** * A Goal restore held back because the session writer is not accepting * writes yet. Settled by {@link startPendingGoalRestore} once the @@ -2342,6 +2350,7 @@ export class Config { private readonly artifactHost?: ArtifactHostConfig; private readonly artifactOss?: ArtifactOssConfig; private workflowsEnabled = false; + private readonly modelProposedGoals: ModelProposedGoalsMode; private readonly skipWorkflowUsageWarning: boolean = false; private readonly emitToolUseSummaries: boolean = true; private readonly chatRecordingEnabled: boolean; @@ -2657,6 +2666,7 @@ export class Config { this.artifactHost = params.artifactHost; this.artifactOss = params.artifactOss; this.workflowsEnabled = params.workflowsEnabled ?? false; + this.modelProposedGoals = params.modelProposedGoals ?? 'alwaysAsk'; this.skipWorkflowUsageWarning = params.skipWorkflowUsageWarning ?? false; this.emitToolUseSummaries = params.emitToolUseSummaries ?? true; this.listExtensions = params.listExtensions ?? false; @@ -7737,6 +7747,42 @@ export class Config { this.workflowsEnabled = enabled; } + /** + * Whether the model may propose a session Goal through `propose_goal`. + * Read from user/system settings only (see WORKSPACE_RESTRICTED_SETTINGS + * in the CLI): a workspace must not be able to switch on a tool that asks + * the user to start an autonomous loop. + */ + getModelProposedGoals(): ModelProposedGoalsMode { + return this.modelProposedGoals; + } + + hasPendingGoalProposal(): boolean { + return this.pendingGoalProposal !== undefined; + } + + /** Parks a `propose_goal` approval until the proposing turn ends. */ + setPendingGoalProposal(proposal: PendingGoalProposal): boolean { + if (this.pendingGoalProposal) return false; + this.pendingGoalProposal = proposal; + return true; + } + + /** Hands the parked approval to its owning turn, or clears it explicitly. */ + takePendingGoalProposal( + expectedTurnKey?: string, + ): PendingGoalProposal | undefined { + const proposal = this.pendingGoalProposal; + if ( + expectedTurnKey !== undefined && + proposal?.turnKey !== expectedTurnKey + ) { + return undefined; + } + this.pendingGoalProposal = undefined; + return proposal; + } + /** * P5 T7: read the `skipWorkflowUsageWarning` setting. When `true`, the * `Workflow` tool suppresses the one-time banner that announces the @@ -8558,6 +8604,8 @@ export class Config { 'Goal runtime was replaced before the session writer became available', ), ); + // An approval belongs to the session that produced it. + this.pendingGoalProposal = undefined; if (!this.chatRecordingService) { this.goalRuntime = undefined; this.goalRuntimeReady = undefined; @@ -9225,6 +9273,18 @@ export class Config { const { UpdateGoalTool } = await import('../goals/goal-tools.js'); return new UpdateGoalTool(this); }); + // propose_goal only exists where its approval dialog can be shown and + // the user has not switched model-proposed Goals off. Headless runs + // keep the text hand-off (`/goal set …`) that /goal-draft prints. + if ( + this.getModelProposedGoals() !== 'disabled' && + resolveInteractionMode(this) === 'interactive' + ) { + await registerLazy(ToolNames.PROPOSE_GOAL, async () => { + const { ProposeGoalTool } = await import('../goals/goal-tools.js'); + return new ProposeGoalTool(this); + }); + } }; if (this.getBareMode()) { diff --git a/packages/core/src/core/client-goal.test.ts b/packages/core/src/core/client-goal.test.ts index 4a2ee4cad76..b552872b1b5 100644 --- a/packages/core/src/core/client-goal.test.ts +++ b/packages/core/src/core/client-goal.test.ts @@ -19,26 +19,31 @@ import type { GoalStateRecordPayloadV2, GoalTurnPermit, } from '../goals/goal-protocol.js'; +import type { ChatRecord } from '../services/chatRecordingService.js'; +import { ApprovalMode } from '../config/config.js'; import { __resetActiveGoalStoreForTests, + clearActiveGoal, setActiveGoal, } from '../goals/activeGoalStore.js'; -import type { ChatRecord } from '../services/chatRecordingService.js'; -import { ApprovalMode } from '../config/config.js'; +import { GOAL_HOOK_ID_OUTPUT_KEY } from '../goals/goalHook.js'; +import type { PendingGoalProposal } from '../goals/goal-tools.js'; const turnMocks = vi.hoisted(() => ({ constructors: [] as unknown[][], + pendingToolCalls: [] as unknown[][], run: vi.fn(), })); vi.mock('./turn.js', async (importOriginal) => { const actual = await importOriginal(); class MockTurn { - pendingToolCalls: unknown[] = []; + pendingToolCalls: unknown[]; finishReason: undefined; constructor(...args: unknown[]) { turnMocks.constructors.push(args); + this.pendingToolCalls = turnMocks.pendingToolCalls.shift() ?? []; } run(...args: unknown[]) { @@ -90,6 +95,27 @@ async function collectOutcome(stream: AsyncGenerator) { } } +function pendingGoalProposalStore(initial?: PendingGoalProposal) { + let pending = initial; + return { + get: () => pending, + set: (proposal: PendingGoalProposal) => { + pending = proposal; + }, + take: vi.fn((expectedTurnKey?: string) => { + const proposal = pending; + if ( + expectedTurnKey !== undefined && + proposal?.turnKey !== expectedTurnKey + ) { + return undefined; + } + pending = undefined; + return proposal; + }), + }; +} + type GoalStateEvent = Extract< ServerLlmStreamEvent, { type: LlmEventType.GoalState } @@ -249,6 +275,7 @@ function setupGoalClient() { getUserContentPushCount: vi.fn(() => 0), getHistory: vi.fn(() => []), getHistoryLength: vi.fn(() => 0), + stripOrphanedUserEntriesFromHistory: vi.fn(() => []), } as unknown as LlmChat; client['drainPendingAddedMcpToolsReminder'] = vi.fn(); client['drainSkillAndCommandReminders'] = vi.fn(async () => undefined); @@ -258,13 +285,833 @@ function setupGoalClient() { describe('LlmClient Goal admission', () => { beforeEach(() => { + __resetActiveGoalStoreForTests(); turnMocks.constructors.length = 0; + turnMocks.pendingToolCalls.length = 0; turnMocks.run.mockReset().mockImplementation(emptyStream); nextSpeakerMocks.check.mockReset().mockResolvedValue({ next_speaker: 'model', }); }); + afterEach(() => { + __resetActiveGoalStoreForTests(); + }); + + it('sets an approved propose_goal proposal once the turn ends without tool calls', async () => { + const { client, config, runtime } = setupGoalClient(); + nextSpeakerMocks.check.mockResolvedValue({ next_speaker: 'user' }); + vi.mocked(runtime.getSnapshot).mockReturnValue({ + v: 2, + activity: 'idle', + goal: null, + }); + const takePendingGoalProposal = vi + .fn() + .mockReturnValueOnce(undefined) // the new-query discard + .mockReturnValueOnce({ objective: 'ship it', turnKey: 'real-user-key' }); + Object.assign(config, { + takePendingGoalProposal, + getUsageStatisticsEnabled: vi.fn(() => false), + }); + + await drain( + client.sendMessageStream( + [{ text: 'set a goal for this' }], + new AbortController().signal, + 'real-user-key', + { type: SendMessageType.UserQuery }, + ), + ); + + expect(takePendingGoalProposal).toHaveBeenCalledTimes(2); + expect(runtime.dispatch).toHaveBeenCalledWith({ + action: 'create', + objective: 'ship it', + }); + }); + + it('settles an approved proposal on the default skip-next-speaker exit', async () => { + const { client, config, runtime } = setupGoalClient(); + vi.mocked(runtime.getSnapshot).mockReturnValue({ + v: 2, + activity: 'idle', + goal: null, + }); + let pending: { objective: string; turnKey: string } | undefined; + const takePendingGoalProposal = vi.fn(() => { + const proposal = pending; + pending = undefined; + return proposal; + }); + Object.assign(config, { + takePendingGoalProposal, + getSkipNextSpeakerCheck: vi.fn(() => true), + getUsageStatisticsEnabled: vi.fn(() => false), + }); + turnMocks.run.mockImplementationOnce(() => { + pending = { objective: 'ship it', turnKey: 'default-exit-key' }; + return emptyStream(); + }); + + await drain( + client.sendMessageStream( + [{ text: 'set a goal for this' }], + new AbortController().signal, + 'default-exit-key', + { type: SendMessageType.UserQuery }, + ), + ); + + expect(nextSpeakerMocks.check).not.toHaveBeenCalled(); + expect(runtime.dispatch).toHaveBeenCalledWith({ + action: 'create', + objective: 'ship it', + }); + }); + + it('settles an approved proposal when a blocking Stop hook hits its cap', async () => { + const { client, config, runtime } = setupGoalClient(); + vi.mocked(runtime.getSnapshot).mockReturnValue({ + v: 2, + activity: 'idle', + goal: null, + }); + let pending: { objective: string; turnKey: string } | undefined; + const takePendingGoalProposal = vi.fn(() => { + const proposal = pending; + pending = undefined; + return proposal; + }); + Object.assign(config, { + takePendingGoalProposal, + getDisableAllHooks: vi.fn(() => false), + hasHooksForEvent: vi.fn((event) => event === 'Stop'), + getMessageBus: vi.fn(() => ({ + request: vi.fn(async () => ({ + output: { decision: 'block', reason: 'Keep working' }, + stopHookCount: 1, + })), + })), + getStopHookBlockingCap: vi.fn(() => 1), + getUsageStatisticsEnabled: vi.fn(() => false), + }); + turnMocks.run.mockImplementationOnce(() => { + pending = { objective: 'ship it', turnKey: 'stop-cap-key' }; + return emptyStream(); + }); + + await drain( + client.sendMessageStream( + [{ text: 'set a goal for this' }], + new AbortController().signal, + 'stop-cap-key', + { type: SendMessageType.UserQuery }, + ), + ); + + expect(runtime.dispatch).toHaveBeenCalledWith({ + action: 'create', + objective: 'ship it', + }); + }); + + it('settles an approved proposal when a cleared Goal removes the Stop continuation', async () => { + const { client, config, runtime } = setupGoalClient(); + vi.mocked(runtime.getSnapshot).mockReturnValue({ + v: 2, + activity: 'idle', + goal: null, + }); + setActiveGoal('goal-test-session', { + condition: 'finish the old goal', + iterations: 1, + setAt: 1, + tokensAtStart: 1, + hookId: 'old-goal-hook', + }); + let pending: { objective: string; turnKey: string } | undefined; + const takePendingGoalProposal = vi.fn(() => { + const proposal = pending; + pending = undefined; + return proposal; + }); + Object.assign(config, { + takePendingGoalProposal, + getDisableAllHooks: vi.fn(() => false), + hasHooksForEvent: vi.fn((event) => event === 'Stop'), + getMessageBus: vi.fn(() => ({ + request: vi.fn(async () => ({ + output: { + decision: 'block', + reason: 'Keep working', + hookSpecificOutput: { + [GOAL_HOOK_ID_OUTPUT_KEY]: 'old-goal-hook', + }, + }, + stopHookCount: 1, + hasNonGoalBlockingStopHook: false, + })), + })), + getMaxSessionTurns: vi.fn(() => 0), + getUsageStatisticsEnabled: vi.fn(() => false), + }); + turnMocks.run.mockImplementationOnce(() => { + pending = { objective: 'ship it', turnKey: 'stop-clear-key' }; + return emptyStream(); + }); + const getSteerInput = vi + .fn() + .mockResolvedValueOnce(undefined) + .mockImplementationOnce(async () => { + clearActiveGoal('goal-test-session'); + return undefined; + }); + + await drain( + client.sendMessageStream( + [{ text: 'set a goal for this' }], + new AbortController().signal, + 'stop-clear-key', + { type: SendMessageType.UserQuery, getSteerInput }, + ), + ); + + expect(turnMocks.run).toHaveBeenCalledOnce(); + expect(getSteerInput).toHaveBeenCalledTimes(2); + expect(takePendingGoalProposal).toHaveBeenCalledTimes(2); + expect(runtime.dispatch).toHaveBeenCalledWith({ + action: 'create', + objective: 'ship it', + }); + }); + + it('discards an approved proposal when settlement starts already aborted', async () => { + const { client, config, runtime } = setupGoalClient(); + const controller = new AbortController(); + controller.abort(); + let pending: { objective: string; turnKey: string } | undefined = { + objective: 'ship it', + turnKey: 'settle-key', + }; + const loadGoalRuntime = vi.fn(async () => runtime); + Object.assign(config, { + takePendingGoalProposal: vi.fn(() => { + const proposal = pending; + pending = undefined; + return proposal; + }), + }); + + await client['settlePendingGoalProposal']( + true, + controller.signal, + loadGoalRuntime, + 'settle-key', + ); + + expect(pending).toBeUndefined(); + expect(loadGoalRuntime).not.toHaveBeenCalled(); + expect(runtime.dispatch).not.toHaveBeenCalled(); + }); + + it('keeps an approved proposal parked until its ToolResult turn ends', async () => { + const { client, config, runtime } = setupGoalClient(); + nextSpeakerMocks.check.mockResolvedValue({ next_speaker: 'user' }); + vi.mocked(runtime.getSnapshot).mockReturnValue({ + v: 2, + activity: 'idle', + goal: null, + }); + let pending: { objective: string; turnKey: string } | undefined; + const takePendingGoalProposal = vi.fn(() => { + const proposal = pending; + pending = undefined; + return proposal; + }); + Object.assign(config, { + takePendingGoalProposal, + getMaxSessionTurns: vi.fn(() => 0), + getUsageStatisticsEnabled: vi.fn(() => false), + }); + turnMocks.pendingToolCalls.push([{ name: 'read_file' }], []); + turnMocks.run + .mockImplementationOnce(() => { + pending = { objective: 'ship it', turnKey: 'pending-tool-key' }; + return emptyStream(); + }) + .mockImplementation(emptyStream); + + await drain( + client.sendMessageStream( + [{ text: 'set a goal for this' }], + new AbortController().signal, + 'pending-tool-key', + { type: SendMessageType.UserQuery }, + ), + ); + + expect(takePendingGoalProposal).toHaveBeenCalledOnce(); + expect(runtime.dispatch).not.toHaveBeenCalled(); + expect(pending).toEqual({ + objective: 'ship it', + turnKey: 'pending-tool-key', + }); + + await drain( + client.sendMessageStream( + [ + { + functionResponse: { + name: 'read_file', + response: { output: 'ok' }, + }, + }, + ], + new AbortController().signal, + 'pending-tool-key', + { type: SendMessageType.ToolResult }, + ), + ); + + expect(takePendingGoalProposal).toHaveBeenCalledTimes(2); + expect(runtime.dispatch).toHaveBeenCalledWith({ + action: 'create', + objective: 'ship it', + }); + }); + + it('keeps an approved proposal parked through a queued steer continuation', async () => { + const { client, config, runtime } = setupGoalClient(); + vi.mocked(config.getMaxSessionTurns).mockReturnValue(0); + nextSpeakerMocks.check.mockResolvedValue({ next_speaker: 'user' }); + let pending: { objective: string; turnKey: string } | undefined; + const takePendingGoalProposal = vi.fn(() => { + const proposal = pending; + pending = undefined; + return proposal; + }); + Object.assign(config, { + takePendingGoalProposal, + getUsageStatisticsEnabled: vi.fn(() => false), + }); + let snapshot: GoalSnapshotV2 = { v: 2, activity: 'idle', goal: null }; + vi.mocked(runtime.getSnapshot).mockImplementation(() => + structuredClone(snapshot), + ); + vi.mocked(runtime.dispatch).mockImplementation(async (request) => { + if (request.action === 'create') { + snapshot = { + v: 2, + activity: 'idle', + goal: { + goalId: 'proposal-goal', + revision: 1, + objective: request.objective, + status: 'active', + evidenceCursor: { recordId: 'proposal-create' }, + turnCount: 0, + activeTimeMs: 0, + tokensUsed: 0, + createdAt: 1, + updatedAt: 1, + }, + }; + } + return { snapshot: structuredClone(snapshot) }; + }); + turnMocks.run + .mockImplementationOnce(() => { + pending = { objective: 'ship it', turnKey: 'real-user-key' }; + return emptyStream(); + }) + .mockImplementationOnce(() => { + expect(runtime.dispatch).not.toHaveBeenCalled(); + return emptyStream(); + }); + const getSteerInput = vi + .fn() + .mockResolvedValueOnce({ + parts: [{ text: 'queued user steering' }], + accept: vi.fn(), + restore: vi.fn(), + }) + .mockResolvedValue(undefined); + + await drain( + client.sendMessageStream( + [{ text: 'set a goal for this' }], + new AbortController().signal, + 'real-user-key', + { type: SendMessageType.UserQuery, getSteerInput }, + ), + ); + + expect(turnMocks.run).toHaveBeenCalledTimes(2); + expect(runtime.dispatch).toHaveBeenCalledTimes(1); + expect(runtime.dispatch).toHaveBeenCalledWith({ + action: 'create', + objective: 'ship it', + }); + }); + + it('drops a proposal when its turn exits with a provider error', async () => { + const { client, config, runtime } = setupGoalClient(); + vi.mocked(config.getMaxSessionTurns).mockReturnValue(0); + vi.mocked(runtime.getSnapshot).mockReturnValue({ + v: 2, + activity: 'idle', + goal: null, + }); + const store = pendingGoalProposalStore(); + Object.assign(config, { + takePendingGoalProposal: store.take, + getUsageStatisticsEnabled: vi.fn(() => false), + getSkipNextSpeakerCheck: vi.fn(() => true), + }); + turnMocks.run + .mockImplementationOnce(async function* () { + store.set({ + objective: 'stale proposal', + turnKey: 'failed-user-key', + }); + yield { + type: LlmEventType.Error, + value: { error: { status: 500 } }, + }; + }) + .mockImplementation(emptyStream); + + await drain( + client.sendMessageStream( + [{ text: 'set a goal for this' }], + new AbortController().signal, + 'failed-user-key', + { type: SendMessageType.UserQuery }, + ), + ); + expect(store.get()).toBeUndefined(); + await drain( + client.sendMessageStream( + [{ text: 'background notification' }], + new AbortController().signal, + 'notification-key', + { type: SendMessageType.Notification }, + ), + ); + + expect(runtime.dispatch).not.toHaveBeenCalled(); + }); + + it('drops a proposal when cancellation lands during runtime readiness', async () => { + const { client, config, runtime } = setupGoalClient(); + // No Goal at the boundary, as in production: the only thing standing + // between the approval and `create` is the post-loader abort guard. + vi.mocked(runtime.getSnapshot).mockReturnValue({ + v: 2, + activity: 'idle', + goal: null, + }); + const controller = new AbortController(); + let pending: { objective: string; turnKey: string } | undefined = { + objective: 'ship it', + turnKey: 'settle-key', + }; + const takePendingGoalProposal = vi.fn(() => { + const proposal = pending; + pending = undefined; + return proposal; + }); + Object.assign(config, { takePendingGoalProposal }); + + await client['settlePendingGoalProposal']( + true, + controller.signal, + async () => { + controller.abort(); + return runtime; + }, + 'settle-key', + ); + + // Dropped means taken and not applied: the slot is empty afterwards, so + // a later boundary cannot revive the cancelled approval. + expect(takePendingGoalProposal).toHaveBeenCalledTimes(1); + expect(pending).toBeUndefined(); + expect(runtime.dispatch).not.toHaveBeenCalled(); + }); + + it('pauses a proposal applied while cancellation is landing', async () => { + const { client, config, runtime } = setupGoalClient(); + const controller = new AbortController(); + Object.assign(config, { + takePendingGoalProposal: vi.fn(() => ({ + objective: 'ship it', + turnKey: 'settle-key', + })), + }); + const appliedGoal = { + goalId: 'proposal-goal', + revision: 1, + objective: 'ship it', + status: 'active' as const, + evidenceCursor: { recordId: 'proposal-create' }, + turnCount: 0, + activeTimeMs: 0, + tokensUsed: 0, + createdAt: 1, + updatedAt: 1, + }; + vi.mocked(runtime.getSnapshot).mockReturnValue({ + v: 2, + activity: 'idle', + goal: null, + }); + vi.mocked(runtime.dispatch) + .mockImplementationOnce(async () => { + controller.abort(); + return { + snapshot: { v: 2, activity: 'idle', goal: appliedGoal }, + }; + }) + .mockResolvedValueOnce({ + snapshot: { + v: 2, + activity: 'idle', + goal: { ...appliedGoal, status: 'paused' }, + }, + }); + + await client['settlePendingGoalProposal']( + true, + controller.signal, + async () => runtime, + 'settle-key', + ); + + expect(runtime.dispatch).toHaveBeenNthCalledWith(1, { + action: 'create', + objective: 'ship it', + }); + expect(runtime.dispatch).toHaveBeenNthCalledWith(2, { + action: 'pause', + expectedGoalId: appliedGoal.goalId, + expectedRevision: appliedGoal.revision, + }); + }); + + it.each(['terminal', 'aborted', 'throwing', 'side-query'] as const)( + 'leaves the owner approval parked through a foreign %s turn', + async (exit) => { + const { client, config, runtime } = setupGoalClient(); + nextSpeakerMocks.check.mockResolvedValue({ next_speaker: 'user' }); + vi.mocked(runtime.getSnapshot).mockReturnValue({ + v: 2, + activity: 'idle', + goal: null, + }); + const store = pendingGoalProposalStore({ + objective: 'approved earlier', + turnKey: 'owner-key', + }); + Object.assign(config, { + takePendingGoalProposal: store.take, + getMaxSessionTurns: vi.fn(() => 0), + getUsageStatisticsEnabled: vi.fn(() => false), + }); + + if (exit === 'aborted') { + const controller = new AbortController(); + controller.abort(); + await client['settlePendingGoalProposal']( + true, + controller.signal, + async () => runtime, + 'foreign-key', + ); + } else { + if (exit === 'throwing') { + turnMocks.run.mockImplementationOnce(() => { + throw new Error('provider exploded'); + }); + } + const foreignTurn = drain( + client.sendMessageStream( + [{ text: 'background task finished' }], + new AbortController().signal, + 'foreign-key', + exit === 'side-query' + ? { + type: SendMessageType.UserQuery, + isConcurrentSideQuery: true, + } + : { type: SendMessageType.Notification }, + ), + ); + if (exit === 'throwing') { + await expect(foreignTurn).rejects.toThrow('provider exploded'); + } else { + await foreignTurn; + } + } + + expect(store.take).toHaveBeenCalledWith('foreign-key'); + expect(store.get()).toEqual({ + objective: 'approved earlier', + turnKey: 'owner-key', + }); + expect(runtime.dispatch).not.toHaveBeenCalled(); + + await client['settlePendingGoalProposal']( + true, + new AbortController().signal, + async () => runtime, + 'owner-key', + ); + + expect(store.get()).toBeUndefined(); + expect(runtime.dispatch).toHaveBeenCalledWith({ + action: 'create', + objective: 'approved earlier', + }); + }, + ); + + it('clears a stale approval before a blocked user query', async () => { + const { client, config, runtime } = setupGoalClient(); + const store = pendingGoalProposalStore({ + objective: 'stale approval', + turnKey: 'cancelled-key', + }); + Object.assign(config, { takePendingGoalProposal: store.take }); + vi.mocked(config.getDisableAllHooks).mockReturnValue(false); + vi.mocked(config.hasHooksForEvent).mockImplementation( + (event) => event === 'UserPromptSubmit', + ); + vi.mocked(config.getMessageBus).mockReturnValue({ + request: vi.fn(async () => ({ + output: { decision: 'block', reason: 'policy denied' }, + })), + } as unknown as ReturnType); + + const events = await collect( + client.sendMessageStream( + [{ text: 'replacement query' }], + new AbortController().signal, + 'replacement-key', + { type: SendMessageType.UserQuery }, + ), + ); + + expect(store.take).toHaveBeenNthCalledWith(1); + expect(store.get()).toBeUndefined(); + expect(runtime.dispatch).not.toHaveBeenCalled(); + expect(events).toContainEqual({ + type: LlmEventType.UserPromptSubmitBlocked, + value: { + reason: 'policy denied', + originalPrompt: 'replacement query', + }, + }); + }); + + it('clears a stale approval before a retry chain', async () => { + const { client, config, runtime } = setupGoalClient(); + vi.mocked(runtime.getSnapshot).mockReturnValue({ + v: 2, + activity: 'idle', + goal: null, + }); + const store = pendingGoalProposalStore({ + objective: 'stale approval', + turnKey: 'cancelled-key', + }); + Object.assign(config, { + takePendingGoalProposal: store.take, + getSkipNextSpeakerCheck: vi.fn(() => true), + getUsageStatisticsEnabled: vi.fn(() => false), + }); + + await drain( + client.sendMessageStream( + [{ text: 'retry the interrupted request' }], + new AbortController().signal, + 'retry-key', + { type: SendMessageType.Retry }, + ), + ); + + expect(store.take).toHaveBeenNthCalledWith(1); + expect(store.get()).toBeUndefined(); + expect(runtime.dispatch).not.toHaveBeenCalled(); + }); + + it.each(['blocked', 'throwing'] as const)( + '%s owner ToolResult hook closes its parked approval', + async (hookExit) => { + const { client, config, runtime } = setupGoalClient(); + vi.mocked(runtime.getSnapshot).mockReturnValue({ + v: 2, + activity: 'idle', + goal: null, + }); + const store = pendingGoalProposalStore({ + objective: 'ship it', + turnKey: 'owner-key', + }); + Object.assign(config, { + takePendingGoalProposal: store.take, + getDisableAllHooks: vi.fn(() => false), + hasHooksForEvent: vi.fn((event) => event === 'UserPromptSubmit'), + getMessageBus: vi.fn( + () => + ({ + request: vi.fn(async () => { + if (hookExit === 'throwing') { + throw new Error('hook exploded'); + } + return { + output: { decision: 'block', reason: 'policy denied' }, + }; + }), + }) as unknown as ReturnType, + ), + }); + + const ownerStream = client.sendMessageStream( + [ + { + functionResponse: { + name: 'propose_goal', + response: { output: 'approved' }, + }, + }, + ], + new AbortController().signal, + 'owner-key', + { type: SendMessageType.ToolResult }, + ); + let events: unknown[] = []; + if (hookExit === 'throwing') { + await expect(drain(ownerStream)).rejects.toThrow('hook exploded'); + } else { + events = await collect(ownerStream); + } + + expect(store.get()).toBeUndefined(); + if (hookExit === 'blocked') { + expect(runtime.dispatch).toHaveBeenCalledWith({ + action: 'create', + objective: 'ship it', + }); + expect(eventIndex(events, LlmEventType.GoalState)).toBeLessThan( + eventIndex(events, LlmEventType.UserPromptSubmitBlocked), + ); + } else { + expect(runtime.dispatch).not.toHaveBeenCalled(); + } + }, + ); + + it.each(['Stop-hook', 'next-speaker'] as const)( + 'settles after a blocked %s continuation ends the owner turn', + async (continuation) => { + const { client, config, runtime } = setupGoalClient(); + vi.mocked(runtime.getSnapshot).mockReturnValue({ + v: 2, + activity: 'idle', + goal: null, + }); + const store = pendingGoalProposalStore(); + let userPromptSubmitCount = 0; + const messageBus = { + request: vi.fn(async (request: { eventName: string }) => { + if (request.eventName === 'Stop') { + return { + output: { decision: 'block', reason: 'Keep working' }, + stopHookCount: 1, + }; + } + userPromptSubmitCount += 1; + return userPromptSubmitCount === 1 + ? { output: {} } + : { output: { decision: 'block', reason: 'policy denied' } }; + }), + }; + Object.assign(config, { + takePendingGoalProposal: store.take, + getDisableAllHooks: vi.fn(() => false), + hasHooksForEvent: vi.fn( + (event) => + event === 'UserPromptSubmit' || + (continuation === 'Stop-hook' && event === 'Stop'), + ), + getMessageBus: vi.fn( + () => messageBus as unknown as ReturnType, + ), + getUsageStatisticsEnabled: vi.fn(() => false), + }); + if (continuation === 'next-speaker') { + nextSpeakerMocks.check.mockResolvedValue({ next_speaker: 'model' }); + } + turnMocks.run.mockImplementationOnce(() => { + store.set({ objective: 'ship it', turnKey: 'owner-key' }); + return emptyStream(); + }); + + await drain( + client.sendMessageStream( + [{ text: 'set a goal for this' }], + new AbortController().signal, + 'owner-key', + { type: SendMessageType.UserQuery }, + ), + ); + + expect(turnMocks.run).toHaveBeenCalledOnce(); + expect(store.get()).toBeUndefined(); + expect(runtime.dispatch).toHaveBeenCalledTimes(1); + expect(runtime.dispatch).toHaveBeenCalledWith({ + action: 'create', + objective: 'ship it', + }); + }, + ); + + it('discards a proposal still parked when the next user query starts', async () => { + // The proposing turn was cancelled before its boundary; the approval must + // not start a loop from under the user's next message. + const { client, config, runtime } = setupGoalClient(); + vi.mocked(runtime.getSnapshot).mockReturnValue({ + v: 2, + activity: 'idle', + goal: null, + }); + const takePendingGoalProposal = vi + .fn() + .mockReturnValueOnce({ + objective: 'stale', + turnKey: 'cancelled-turn-key', + }) + .mockReturnValue(undefined); + Object.assign(config, { + takePendingGoalProposal, + getUsageStatisticsEnabled: vi.fn(() => false), + }); + + await drain( + client.sendMessageStream( + [{ text: 'something else' }], + new AbortController().signal, + 'real-user-key', + { type: SendMessageType.UserQuery }, + ), + ); + + expect(takePendingGoalProposal).toHaveBeenCalled(); + expect(runtime.dispatch).not.toHaveBeenCalled(); + }); + it('exposes Goal as an explicit internal message type', () => { expect(SendMessageType.Goal).toBe('goal'); expect(LlmEventType.GoalState).toBe('goal_state'); diff --git a/packages/core/src/core/client.ts b/packages/core/src/core/client.ts index 84438ce826c..2630a262b14 100644 --- a/packages/core/src/core/client.ts +++ b/packages/core/src/core/client.ts @@ -49,6 +49,7 @@ import { getStopHookContinuationReason, GOAL_HOOK_ID_OUTPUT_KEY, } from '../goals/goalHook.js'; +import { applyPendingGoalProposal } from '../goals/goal-tools.js'; import { formatStopHookBlockingCapWarning } from '../hooks/stopHookCap.js'; import { buildContextUsage } from '../hooks/context-usage.js'; import { DEFAULT_TOKEN_LIMIT, tokenLimit } from './tokenLimits.js'; @@ -210,6 +211,8 @@ export interface SendMessageOptions { type: SendMessageType; /** User-submitted text captured before prompt expansion. */ submittedPrompt?: string; + /** A UserQuery running beside an active turn, without replacing its state. */ + isConcurrentSideQuery?: boolean; /** Returns user input waiting to steer the active turn at a model boundary. */ getSteerInput?: (signal: AbortSignal) => Promise; /** Steer lease already appended to this request, settled after history push. */ @@ -841,6 +844,58 @@ export class LlmClient { return chat.getHistoryLength?.() ?? chat.getHistory().length; } + /** + * Applies a `propose_goal` approval at the true end of the turn that made it. + * + * Only when the model has stopped calling tools, and only in the turn + * that parked it (matched by prompt id): a proposal made mid-turn stays + * parked through the tool-result continuations, because creating the Goal + * earlier would leave those continuations without a permit. Tail + * continuations keep the proposal parked until their final boundary. An + * aborted turn drops the approval instead of starting a loop the user just + * cancelled; an abort during dispatch pauses the new Goal. + */ + private async settlePendingGoalProposal( + turnEnded: boolean, + signal: AbortSignal, + loadGoalRuntime: (required: boolean) => Promise, + turnKey: string, + ): Promise { + const take = this.config.takePendingGoalProposal; + if (typeof take !== 'function') return; + if (!turnEnded && !signal.aborted) return; + const proposal = take.call(this.config, turnKey); + if (!proposal) return; + if (signal.aborted) return; + const runtime = await loadGoalRuntime(false); + if (!runtime) { + debugLogger.debug( + 'Dropping an approved Goal proposal: the Goal runtime is unavailable', + ); + return; + } + if (signal.aborted) return; + const result = await applyPendingGoalProposal(runtime, proposal); + if (signal.aborted && result.applied) { + try { + await runtime.dispatch({ + action: 'pause', + expectedGoalId: result.goal.goalId, + expectedRevision: result.goal.revision, + }); + } catch (error) { + debugLogger.warn( + 'Failed to pause a Goal applied during cancellation', + error, + ); + } + return; + } + if (!result.applied) { + debugLogger.debug(`Dropping an approved Goal proposal: ${result.reason}`); + } + } + private getLastModelMessageText(): string | undefined { const chat = this.getChat(); if (chat.getLastModelMessageText) { @@ -3021,6 +3076,17 @@ export class LlmClient { strippedRetryEntries = []; }; + if ( + (messageType === SendMessageType.UserQuery && + !options?.isConcurrentSideQuery) || + messageType === SendMessageType.Retry + ) { + // A propose_goal approval is applied when its own turn ends. One still + // parked when a new user/retry chain starts belongs to a turn that ended + // without settling, so clear it before the replacement chain can exit. + this.config.takePendingGoalProposal?.(); + } + if (messageType === SendMessageType.Retry) { strippedRetryEntries = this.stripOrphanedUserEntriesFromHistory() ?? []; // The matching dangling-`functionCall` repair runs inside @@ -3139,6 +3205,19 @@ export class LlmClient { } else { endCurrentInteraction('cancelled'); } + await this.settlePendingGoalProposal( + true, + signal, + async (required) => { + const runtime = await loadGoalRuntime(required); + if (runtime) bindGoalStateEvents(runtime); + return runtime; + }, + prompt_id, + ); + for (const goalEvent of takePendingGoalEvents()) { + yield goalEvent; + } yield { type: LlmEventType.UserPromptSubmitBlocked, value: { @@ -3182,6 +3261,7 @@ export class LlmClient { signal.aborted ? undefined : userPromptSubmitFailureMessage, signal.aborted ? undefined : getErrorType(error), ); + this.config.takePendingGoalProposal?.(prompt_id); for (const goalEvent of await finalizeInterruptedGoalTurn()) { yield goalEvent; } @@ -3260,6 +3340,7 @@ export class LlmClient { signal.aborted ? undefined : 'Goal turn admission failed', signal.aborted ? undefined : getErrorType(error), ); + this.config.takePendingGoalProposal?.(prompt_id); for (const goalEvent of await finalizeInterruptedGoalTurn()) { yield goalEvent; } @@ -4400,6 +4481,15 @@ export class LlmClient { value: warning, }; debugLogger.warn(warning); + await this.settlePendingGoalProposal( + true, + signal, + loadGoalRuntime, + prompt_id, + ); + for (const goalEvent of takePendingGoalEvents()) { + yield goalEvent; + } endCurrentInteraction('ok'); return turn; } @@ -4471,6 +4561,15 @@ export class LlmClient { ? response.nonGoalBlockingStopReason || 'No reason provided' : continueReason; if (!continuationReasonAfterSteer && !pendingSteer) { + await this.settlePendingGoalProposal( + true, + signal, + loadGoalRuntime, + prompt_id, + ); + for (const goalEvent of takePendingGoalEvents()) { + yield goalEvent; + } endCurrentInteraction('ok'); normalCompletion = true; return turn; @@ -4519,6 +4618,15 @@ export class LlmClient { if (!hasToolCalls) { endCurrentInteraction(signal.aborted ? 'cancelled' : 'ok'); } + await this.settlePendingGoalProposal( + !hasToolCalls, + signal, + loadGoalRuntime, + prompt_id, + ); + for (const goalEvent of takePendingGoalEvents()) { + yield goalEvent; + } // Preserve the pending prefetch: the inner Hook turn we just // yielded may have produced tool calls, and the caller's next // ToolResult turn still needs to consume the recall result. @@ -4590,6 +4698,15 @@ export class LlmClient { if (arenaAgentClient) { await arenaAgentClient.reportCompleted(); } + await this.settlePendingGoalProposal( + true, + signal, + loadGoalRuntime, + prompt_id, + ); + for (const goalEvent of takePendingGoalEvents()) { + yield goalEvent; + } endCurrentInteraction('ok'); return turn; } @@ -4638,6 +4755,15 @@ export class LlmClient { if (!hasToolCalls) { endCurrentInteraction(signal.aborted ? 'cancelled' : 'ok'); } + await this.settlePendingGoalProposal( + !hasToolCalls, + signal, + loadGoalRuntime, + prompt_id, + ); + for (const goalEvent of takePendingGoalEvents()) { + yield goalEvent; + } // Preserve the pending prefetch: same reasoning as the // `return hookTurn` site above — the recursive Hook turn may // have produced tool calls whose ToolResult turn still needs @@ -4672,6 +4798,12 @@ export class LlmClient { if (!hasToolCalls) { this.finishManagedAutoMemoryRecall(); } + await this.settlePendingGoalProposal( + turn.pendingToolCalls.length === 0, + signal, + loadGoalRuntime, + prompt_id, + ); for (const goalEvent of takePendingGoalEvents()) { yield goalEvent; } @@ -4731,6 +4863,7 @@ export class LlmClient { // `return turn`. Catches uncaught exceptions and guards against // future early-return sites that forget to call cancel. if (!normalCompletion) { + this.config.takePendingGoalProposal?.(prompt_id); this.cancelPendingMemoryPrefetch( signal?.aborted ? 'abort' : 'no_safe_delivery_point', ); diff --git a/packages/core/src/goals/goal-runtime.ts b/packages/core/src/goals/goal-runtime.ts index f253944b064..7fadc9d8390 100644 --- a/packages/core/src/goals/goal-runtime.ts +++ b/packages/core/src/goals/goal-runtime.ts @@ -47,6 +47,7 @@ import { } from './goal-protocol.js'; import { elapsedActiveTime, + GoalInvalidTransitionError, reduceGoalControl, reduceGoalTurnFinished, } from './goal-reducer.js'; @@ -176,7 +177,10 @@ export interface GoalRuntime { ): Promise; getPreparedRestore(): Promise; activateRestoredWork(): Promise; - dispatch(request: GoalControlRequest): Promise; + dispatch( + request: GoalControlRequest, + options?: { refuseIfActive?: boolean }, + ): Promise; bindHost(host: GoalTurnHost): () => void; beginTurn(turnKey: string): GoalTurnPermit | undefined; releaseTurn(turnKey: string): Promise; @@ -1730,9 +1734,22 @@ export function createGoalRuntime( pendingProposal = undefined; return proposal ? structuredClone(proposal) : undefined; }, - dispatch(request: GoalControlRequest): Promise { + dispatch( + request: GoalControlRequest, + dispatchOptions?: { refuseIfActive?: boolean }, + ): Promise { const execute = async (): Promise => { assertOperational(); + if ( + dispatchOptions?.refuseIfActive && + request.action === 'replace' && + snapshot.goal?.status === 'active' + ) { + throw new GoalInvalidTransitionError( + 'An active Goal cannot be replaced by an approved proposal', + getSnapshot(), + ); + } const recordUuid = randomUUID(); const nextGoal = reduceGoalControl(snapshot.goal, { request, diff --git a/packages/core/src/goals/goal-tools.test.ts b/packages/core/src/goals/goal-tools.test.ts index c838b13c2da..5a8f3c69f48 100644 --- a/packages/core/src/goals/goal-tools.test.ts +++ b/packages/core/src/goals/goal-tools.test.ts @@ -17,9 +17,24 @@ import { import { type GetGoalToolParams, GetGoalTool, + PROPOSE_GOAL_NOT_APPROVED_MESSAGE, + PROPOSE_GOAL_NO_TURN_MESSAGE, + PROPOSE_GOAL_OBJECTIVE_MAX_CHARACTERS, + PROPOSE_GOAL_PENDING_MESSAGE, + PROPOSE_GOAL_PLAN_MODE_MESSAGE, + PROPOSE_GOAL_UNAVAILABLE_MESSAGE, + PROPOSE_GOAL_UNTRUSTED_MESSAGE, + ProposeGoalTool, + type PendingGoalProposal, + type ProposeGoalToolConfig, + applyPendingGoalProposal, UpdateGoalTool, type GoalToolConfig, } from './goal-tools.js'; +import { ApprovalMode } from '../config/config.js'; +import { ToolConfirmationOutcome } from '../tools/tools.js'; +import { ToolErrorType } from '../tools/tool-error.js'; +import { promptIdContext } from '../utils/promptIdContext.js'; import { goalTurnContext } from './goal-turn-context.js'; import { emptyGoalSnapshot, @@ -1510,3 +1525,412 @@ describe('UpdateGoalTool', () => { expect(runtime.getSnapshot().goal?.status).toBe('active'); }); }); + +describe('ProposeGoalTool', () => { + const TURN_KEY = 'user-turn-key'; + /** Runs the tool inside the prompt-id context established by the scheduler. */ + const execute = (invocation: ReturnType) => + promptIdContext.run(TURN_KEY, () => + invocation.execute(new AbortController().signal), + ); + + const objective = + 'Outcome: auth tests pass. Done when: 1) `npm test` exits 0 (paste the summary line). Must not: edit test files. Budget: stop as blocked after 20 turns. On block: report the blocker.'; + + function proposeConfig( + runtime: Partial | (() => never), + overrides: Partial = {}, + ): ProposeGoalToolConfig & { + pending: () => PendingGoalProposal | undefined; + } { + let parked: PendingGoalProposal | undefined; + const setPendingGoalProposal = vi.fn((proposal: PendingGoalProposal) => { + if (parked) return false; + parked = proposal; + return true; + }); + const getGoalRuntime = + typeof runtime === 'function' + ? runtime + : vi.fn(() => runtime as GoalRuntime); + return { + getGoalRuntime, + getGoalRuntimeReady: async () => getGoalRuntime(), + isTrustedFolder: () => true, + getApprovalMode: () => ApprovalMode.DEFAULT, + hasPendingGoalProposal: () => parked !== undefined, + setPendingGoalProposal, + pending: () => parked, + ...overrides, + }; + } + + function idleRuntime() { + const runtime = createGoalRuntime({ journal: fakeGoalJournal() }); + const host = fakeHost(); + runtime.bindHost(host); + return { runtime, host }; + } + + async function confirm( + tool: ProposeGoalTool, + outcome: ToolConfirmationOutcome, + params = { objective }, + ) { + const invocation = tool.build(params); + const details = await invocation.getConfirmationDetails( + new AbortController().signal, + ); + await details.onConfirm(outcome); + return { invocation, details }; + } + + it('uses the canonical name, stays visible, and always goes through the dialog', async () => { + const tool = new ProposeGoalTool(proposeConfig(idleRuntime().runtime)); + expect(tool.name).toBe(ToolNames.PROPOSE_GOAL); + expect(tool.displayName).toBe(ToolDisplayNames.PROPOSE_GOAL); + expect(tool.shouldDefer).toBe(false); + + const invocation = tool.build({ objective }); + // Consent for an autonomous loop cannot come from a rule or an approval + // mode; YOLO and AUTO_EDIT would otherwise approve an `info` dialog. + expect(invocation.requiresUserInteraction?.()).toBe(true); + expect(await invocation.getDefaultPermission()).toBe('ask'); + expect(invocation.getDescription()).toContain(objective); + }); + + it('validates the objective', () => { + const tool = new ProposeGoalTool(proposeConfig(idleRuntime().runtime)); + expect(tool.validateToolParams({ objective: ' ' })).not.toBeNull(); + expect( + tool.validateToolParams({ + objective: 'x'.repeat(PROPOSE_GOAL_OBJECTIVE_MAX_CHARACTERS + 1), + }), + ).not.toBeNull(); + expect(tool.validateToolParams({ objective })).toBeNull(); + }); + + it('shows the objective in a plain-text info dialog and parks it on approval', async () => { + const { runtime, host } = idleRuntime(); + const config = proposeConfig(runtime); + const tool = new ProposeGoalTool(config); + + const { invocation, details } = await confirm( + tool, + ToolConfirmationOutcome.ProceedOnce, + ); + expect(details.type).toBe('info'); + if (details.type !== 'info') return; + expect(details.renderPromptAsPlainText).toBe(true); + expect(details.prompt).toContain('Set this as the session Goal?'); + expect(details.prompt).toContain(objective); + + const result = await execute(invocation); + expect(result.error).toBeUndefined(); + const payload = JSON.parse(result.llmContent as string); + expect(payload.approved).toBe(true); + expect(payload.objective).toBe(objective); + expect(payload.next).toContain('the moment this turn ends'); + expect(result.returnDisplay).toContain('Goal approved'); + + // Parked, not set: setting it mid-turn would strip the rest of the + // proposing turn of its Goal permit. The client applies it at the + // turn boundary (see applyPendingGoalProposal below). + expect(config.setPendingGoalProposal).toHaveBeenCalledTimes(1); + expect(config.pending()).toEqual({ objective, turnKey: 'user-turn-key' }); + expect(runtime.getSnapshot().goal).toBeNull(); + expect(host.started).toHaveLength(0); + + const applied = await applyPendingGoalProposal(runtime, config.pending()!); + expect(applied.applied).toBe(true); + if (!applied.applied) return; + const goal = runtime.getSnapshot().goal; + expect(goal?.goalId).toBe(applied.goal.goalId); + expect(goal?.status).toBe('active'); + expect(goal?.objective).toBe(objective); + // The runtime, not the tool, drives the first Goal turn. + expect(host.started).toHaveLength(1); + }); + + it('does not silently replace an already approved pending proposal', async () => { + const { runtime } = idleRuntime(); + const config = proposeConfig(runtime); + const tool = new ProposeGoalTool(config); + const firstObjective = 'Outcome: ship the first approved Goal.'; + const secondObjective = 'Outcome: ship a different Goal.'; + + const first = await confirm(tool, ToolConfirmationOutcome.ProceedOnce, { + objective: firstObjective, + }); + const second = await confirm(tool, ToolConfirmationOutcome.ProceedOnce, { + objective: secondObjective, + }); + + expect(await execute(first.invocation)).not.toHaveProperty('error'); + const secondResult = await execute(second.invocation); + + expect(secondResult.error?.type).toBe(ToolErrorType.EXECUTION_DENIED); + expect(config.pending()?.objective).toBe(firstObjective); + await expect( + tool + .build({ objective: 'Outcome: ask a third time.' }) + .getConfirmationDetails(new AbortController().signal), + ).rejects.toThrow(PROPOSE_GOAL_PENDING_MESSAGE); + }); + + it('refuses when the parking slot is taken between the re-check and the park', async () => { + // Two approved invocations in one turn can both pass the + // hasPendingGoalProposal() re-check before either parks; the set-once + // slot refuses the second, and execute() must surface that refusal + // instead of reporting "approved". + const { runtime, host } = idleRuntime(); + const config = proposeConfig(runtime, { + hasPendingGoalProposal: () => false, + setPendingGoalProposal: vi.fn(() => false), + }); + const tool = new ProposeGoalTool(config); + + const { invocation } = await confirm( + tool, + ToolConfirmationOutcome.ProceedOnce, + ); + const result = await execute(invocation); + + expect(result.error?.type).toBe(ToolErrorType.EXECUTION_DENIED); + expect(result.llmContent).toBe(PROPOSE_GOAL_PENDING_MESSAGE); + expect(config.setPendingGoalProposal).toHaveBeenCalledTimes(1); + expect(runtime.getSnapshot().goal).toBeNull(); + expect(host.started).toHaveLength(0); + }); + + it('refuses to park an approval it cannot bind to a turn', async () => { + const { runtime, host } = idleRuntime(); + const config = proposeConfig(runtime); + const tool = new ProposeGoalTool(config); + const { invocation } = await confirm( + tool, + ToolConfirmationOutcome.ProceedOnce, + ); + + // No scheduler prompt-id context: the settle boundary could not tell this + // approval apart from a stale one, so it is refused instead of parked. + const result = await invocation.execute(new AbortController().signal); + + expect(result.error?.type).toBe(ToolErrorType.EXECUTION_DENIED); + expect(result.llmContent).toBe(PROPOSE_GOAL_NO_TURN_MESSAGE); + expect(config.setPendingGoalProposal).not.toHaveBeenCalled(); + expect(runtime.getSnapshot().goal).toBeNull(); + expect(host.started).toHaveLength(0); + }); + + it('sets nothing when the dialog was cancelled', async () => { + const { runtime, host } = idleRuntime(); + const config = proposeConfig(runtime); + const tool = new ProposeGoalTool(config); + + const { invocation } = await confirm(tool, ToolConfirmationOutcome.Cancel); + const result = await execute(invocation); + expect(config.setPendingGoalProposal).not.toHaveBeenCalled(); + expect(config.pending()).toBeUndefined(); + + expect(result.error?.type).toBe(ToolErrorType.EXECUTION_DENIED); + expect(result.llmContent).toBe(PROPOSE_GOAL_NOT_APPROVED_MESSAGE); + expect(runtime.getSnapshot().goal).toBeNull(); + expect(host.started).toHaveLength(0); + }); + + it('refuses before the dialog in plan mode, in an untrusted folder, and without persistence', async () => { + const { runtime } = idleRuntime(); + const signal = new AbortController().signal; + + await expect( + new ProposeGoalTool( + proposeConfig(runtime, { getApprovalMode: () => ApprovalMode.PLAN }), + ) + .build({ objective }) + .getConfirmationDetails(signal), + ).rejects.toThrow(PROPOSE_GOAL_PLAN_MODE_MESSAGE); + + await expect( + new ProposeGoalTool( + proposeConfig(runtime, { isTrustedFolder: () => false }), + ) + .build({ objective }) + .getConfirmationDetails(signal), + ).rejects.toThrow(PROPOSE_GOAL_UNTRUSTED_MESSAGE); + + await expect( + new ProposeGoalTool( + proposeConfig(() => { + throw new Error('no persistence'); + }), + ) + .build({ objective }) + .getConfirmationDetails(signal), + ).rejects.toThrow(PROPOSE_GOAL_UNAVAILABLE_MESSAGE); + + expect(runtime.getSnapshot().goal).toBeNull(); + }); + + it('refuses before the dialog when Goal persistence failed to become ready', async () => { + const { runtime } = idleRuntime(); + const config = proposeConfig(runtime); + Object.assign(config, { + getGoalRuntimeReady: vi + .fn() + .mockRejectedValue(new Error('restore failed')), + }); + + await expect( + new ProposeGoalTool(config) + .build({ objective }) + .getConfirmationDetails(new AbortController().signal), + ).rejects.toThrow(PROPOSE_GOAL_UNAVAILABLE_MESSAGE); + }); + + it('refuses to replace an active Goal and points at /goal edit', async () => { + const { runtime } = await activeRuntime(); + const tool = new ProposeGoalTool(proposeConfig(runtime)); + + await expect( + tool + .build({ objective }) + .getConfirmationDetails(new AbortController().signal), + ).rejects.toThrow('/goal edit'); + expect(runtime.getSnapshot().goal?.objective).toBe('Ship Goal v3'); + }); + + it('rechecks the active Goal after the dialog before parking approval', async () => { + const { runtime } = idleRuntime(); + const config = proposeConfig(runtime); + const { invocation } = await confirm( + new ProposeGoalTool(config), + ToolConfirmationOutcome.ProceedOnce, + ); + await runtime.dispatch({ action: 'create', objective: 'Typed by hand' }); + + const result = await execute(invocation); + + expect(result.error?.type).toBe(ToolErrorType.EXECUTION_DENIED); + expect(config.pending()).toBeUndefined(); + expect(runtime.getSnapshot().goal?.objective).toBe('Typed by hand'); + }); + + it('replaces a stopped Goal when the parked approval is applied', async () => { + const { runtime, host } = idleRuntime(); + await runtime.dispatch({ action: 'create', objective: 'Ship Goal v3' }); + const paused = runtime.getSnapshot().goal!; + await runtime.dispatch({ + action: 'pause', + expectedGoalId: paused.goalId, + expectedRevision: paused.revision, + }); + expect(runtime.getSnapshot().goal?.status).toBe('paused'); + const startedBefore = host.started.length; + const config = proposeConfig(runtime); + const tool = new ProposeGoalTool(config); + + const { invocation, details } = await confirm( + tool, + ToolConfirmationOutcome.ProceedOnce, + ); + if (details.type !== 'info') throw new Error('expected info'); + expect(details.prompt).toContain('Replace the paused Goal'); + + const result = await execute(invocation); + const payload = JSON.parse(result.llmContent as string); + expect(payload.replacesGoalId).toBe(paused.goalId); + expect(runtime.getSnapshot().goal?.goalId).toBe(paused.goalId); + + const applied = await applyPendingGoalProposal(runtime, config.pending()!); + expect(applied).toMatchObject({ applied: true }); + const goal = runtime.getSnapshot().goal; + expect(goal?.goalId).not.toBe(paused.goalId); + expect(goal?.status).toBe('active'); + expect(goal?.objective).toBe(objective); + expect(host.started.length).toBe(startedBefore + 1); + }); + + it('does not set a parked approval over a Goal that became active meanwhile', async () => { + const { runtime } = idleRuntime(); + const config = proposeConfig(runtime); + const tool = new ProposeGoalTool(config); + const { invocation } = await confirm( + tool, + ToolConfirmationOutcome.ProceedOnce, + ); + await execute(invocation); + + // The user typed `/goal set …` before the proposing turn ended. + await runtime.dispatch({ action: 'create', objective: 'Typed by hand' }); + + const applied = await applyPendingGoalProposal(runtime, config.pending()!); + expect(applied.applied).toBe(false); + if (applied.applied) return; + expect(applied.reason).toContain('became active'); + expect(runtime.getSnapshot().goal?.objective).toBe('Typed by hand'); + }); + + it('does not replace a paused Goal resumed ahead of the proposal dispatch', async () => { + const { runtime } = idleRuntime(); + await runtime.dispatch({ action: 'create', objective: 'Paused by user' }); + const original = runtime.getSnapshot().goal!; + await runtime.dispatch({ + action: 'pause', + expectedGoalId: original.goalId, + expectedRevision: original.revision, + }); + + const resumed = runtime.dispatch({ + action: 'resume', + expectedGoalId: original.goalId, + expectedRevision: original.revision, + }); + const applied = applyPendingGoalProposal(runtime, { + objective, + turnKey: 'user-turn-key', + }); + + await expect(resumed).resolves.toMatchObject({ + snapshot: { + goal: { + goalId: original.goalId, + revision: original.revision, + status: 'active', + }, + }, + }); + await expect(applied).resolves.toMatchObject({ applied: false }); + expect(runtime.getSnapshot().goal).toMatchObject({ + goalId: original.goalId, + objective: 'Paused by user', + status: 'active', + }); + }); + + it('reports a conflict instead of throwing when the expected version moved', async () => { + const { runtime } = idleRuntime(); + await runtime.dispatch({ action: 'create', objective: 'Ship Goal v3' }); + const first = runtime.getSnapshot().goal!; + await runtime.dispatch({ + action: 'pause', + expectedGoalId: first.goalId, + expectedRevision: first.revision, + }); + const paused = runtime.getSnapshot().goal!; + const stale = { + getSnapshot: () => ({ + ...runtime.getSnapshot(), + goal: { ...paused, revision: paused.revision - 1 }, + }), + dispatch: runtime.dispatch.bind(runtime), + }; + + const applied = await applyPendingGoalProposal(stale, { + objective, + turnKey: 'user-turn-key', + }); + expect(applied.applied).toBe(false); + expect(runtime.getSnapshot().goal?.goalId).toBe(paused.goalId); + }); +}); diff --git a/packages/core/src/goals/goal-tools.ts b/packages/core/src/goals/goal-tools.ts index 1ba2194342f..0ccf49d07c7 100644 --- a/packages/core/src/goals/goal-tools.ts +++ b/packages/core/src/goals/goal-tools.ts @@ -5,7 +5,21 @@ */ import { ToolDisplayNames, ToolNames } from '../tools/tool-names.js'; -import type { ToolInvocation, ToolResult } from '../tools/tools.js'; +import type { + ToolCallConfirmationDetails, + ToolInvocation, + ToolResult, +} from '../tools/tools.js'; +import { ToolConfirmationOutcome } from '../tools/tools.js'; +import type { PermissionDecision } from '../permissions/types.js'; +import { ApprovalMode } from '../config/config.js'; +import { StructuredToolError } from '../tools/priorReadEnforcement.js'; +import { ToolErrorType } from '../tools/tool-error.js'; +import { promptIdContext } from '../utils/promptIdContext.js'; +import { + GoalConflictError, + GoalInvalidTransitionError, +} from './goal-reducer.js'; import { capPreviewBytes, GOAL_EVIDENCE_REFERENCE_LIMIT, @@ -17,6 +31,7 @@ import { } from '../tools/tools.js'; import { GOAL_RUNTIME_DISPOSED_MESSAGE, + GoalPersistenceUnavailableError, STALE_GOAL_TURN_MESSAGE, type GoalRuntime, type GoalWorkerView, @@ -24,6 +39,7 @@ import { import { goalTurnContext } from './goal-turn-context.js'; import { type GoalBlockerKind, + type GoalControlRequest, GOAL_PROPOSAL_REASON_MAX_CHARACTERS, type GoalRecord, type GoalSnapshotV2, @@ -578,3 +594,332 @@ function summarizeCatalog( ...(shortenedPreviews > 0 ? { shortenedPreviews } : {}), }; } + +// ── propose_goal ──────────────────────────────────────────────────────────── + +/** + * Upper bound on a proposed objective. The whole text is shown in the + * approval dialog, so it has to stay readable there; the /goal-draft contract + * (Outcome / Done when / Must not / Budget / On block / Context) fits in + * well under this. + */ +export const PROPOSE_GOAL_OBJECTIVE_MAX_CHARACTERS = 1500; + +export interface ProposeGoalToolParams { + objective: string; +} + +/** + * A Goal the user approved in the `propose_goal` dialog, waiting for the + * turn that proposed it to end. Setting it mid-turn would leave the rest of + * that turn without a Goal permit (see `client.ts`, "An active Goal requires + * an exact turn permit"), so the tool only parks it here and the client + * applies it at the same boundary a typed `/goal set` takes effect. + */ +export interface PendingGoalProposal { + objective: string; + /** + * The `prompt_id` of the turn whose dialog approved it. Only that turn's + * terminal boundary may set or discard the Goal; unrelated frames leave it + * parked for its owner. A new real user query clears any stale approval. + */ + turnKey: string; +} + +export interface ProposeGoalToolConfig extends GoalToolConfig { + getGoalRuntimeReady(): Promise; + isTrustedFolder(): boolean; + getApprovalMode(): ApprovalMode; + hasPendingGoalProposal(): boolean; + setPendingGoalProposal(proposal: PendingGoalProposal): boolean; +} + +type ProposeGoalRuntime = Pick; + +export type ApplyPendingGoalProposalResult = + | { applied: true; goal: GoalRecord } + | { applied: false; reason: string }; + +/** + * Sets an approved proposal as the session Goal. Called by the client once + * the proposing turn has ended; never from inside a turn. + * + * Re-reads the snapshot because `/goal` may have changed the session since + * the dialog: an active Goal is never replaced (someone is already running + * it), a stopped one is replaced through its expected version, and no Goal + * creates. + */ +export async function applyPendingGoalProposal( + runtime: ProposeGoalRuntime, + proposal: PendingGoalProposal, +): Promise { + const objective = proposal.objective.trim(); + const current = runtime.getSnapshot().goal; + if (current?.status === 'active') { + return { + applied: false, + reason: `A Goal became active (revision ${current.revision}) before the approved proposal could be set.`, + }; + } + const request: GoalControlRequest = current + ? { + action: 'replace', + objective, + expectedGoalId: current.goalId, + expectedRevision: current.revision, + } + : { action: 'create', objective }; + try { + const response = + request.action === 'replace' + ? await runtime.dispatch(request, { refuseIfActive: true }) + : await runtime.dispatch(request); + const goal = response.snapshot.goal; + if (!goal) { + return { + applied: false, + reason: 'The Goal runtime accepted the request but reported no Goal.', + }; + } + return { + applied: true, + goal, + }; + } catch (error) { + if ( + error instanceof GoalConflictError || + error instanceof GoalInvalidTransitionError || + error instanceof GoalPersistenceUnavailableError + ) { + return { applied: false, reason: error.message }; + } + throw error; + } +} + +export const PROPOSE_GOAL_PLAN_MODE_MESSAGE = + 'Keep planning; propose the Goal after the plan is approved.'; +export const PROPOSE_GOAL_UNTRUSTED_MESSAGE = + 'Goals can only be set in trusted workspaces. Tell the user to trust the folder with /trust and then run /goal set themselves.'; +export const PROPOSE_GOAL_UNAVAILABLE_MESSAGE = + 'This session cannot persist Goals, so no Goal can be set.'; +export const PROPOSE_GOAL_NOT_APPROVED_MESSAGE = + 'The Goal was not set: the user did not approve it. Do not ask why and do not propose the same or a reworded objective again.'; +export const PROPOSE_GOAL_NO_TURN_MESSAGE = + 'The Goal was not set: this call is not attributable to a turn, so its approval could not be bound to one. Hand the user a `/goal set ` line instead.'; +export const PROPOSE_GOAL_PENDING_MESSAGE = + 'Another approved Goal proposal is already waiting for this turn to end. Do not propose another one.'; + +function activeGoalMessage(revision: number): string { + return `A Goal is already active (revision ${revision}); this tool does not replace a running Goal. Hand the user a \`/goal edit \` line to tighten it or a \`/goal set \` line to replace it, and stop.`; +} + +function proposalPromptHeadline(current: GoalRecord | null): string { + if (current) { + return `Replace the ${current.status} Goal and start working toward this objective? Approving sets it like /goal set: after each turn an independent verifier checks the transcript, and Qwen Code keeps working until it is met.`; + } + return 'Set this as the session Goal? Approving sets it like /goal set: after each turn an independent verifier checks the transcript, and Qwen Code keeps working until it is met.'; +} + +class ProposeGoalInvocation extends BaseToolInvocation< + ProposeGoalToolParams, + GoalToolResult +> { + private approved = false; + + constructor( + params: ProposeGoalToolParams, + private readonly config: ProposeGoalToolConfig, + ) { + super(params); + } + + /** + * The description is the one piece of the confirmation every host shows + * (the Web Shell does not forward an `info` prompt), so the objective has + * to be in it. + */ + getDescription(): string { + return `Propose Goal: ${this.params.objective.trim()}`; + } + + /** + * Consent for an autonomous loop cannot come from a permission rule or an + * approval mode: a bare `propose_goal` allow rule, YOLO, or AUTO_EDIT + * (which auto-approves `info` confirmations) would otherwise set a Goal + * the user never saw. + */ + override requiresUserInteraction(): boolean { + return true; + } + + override async getDefaultPermission(): Promise { + return 'ask'; + } + + /** + * Why a proposal cannot be shown right now, or `undefined` when it can. + * Checked before the dialog so the user is never asked to approve a Goal + * that could not be set, and again in `execute()` because `/goal` can + * change the session while the dialog is open. + */ + private async blocker(): Promise< + { message: string; type: ToolErrorType } | undefined + > { + if (this.config.getApprovalMode() === ApprovalMode.PLAN) { + return { + message: PROPOSE_GOAL_PLAN_MODE_MESSAGE, + type: ToolErrorType.EXECUTION_DENIED, + }; + } + if (!this.config.isTrustedFolder()) { + return { + message: PROPOSE_GOAL_UNTRUSTED_MESSAGE, + type: ToolErrorType.EXECUTION_DENIED, + }; + } + if (this.config.hasPendingGoalProposal()) { + return { + message: PROPOSE_GOAL_PENDING_MESSAGE, + type: ToolErrorType.EXECUTION_DENIED, + }; + } + let runtime: ProposeGoalRuntime; + try { + runtime = await this.config.getGoalRuntimeReady(); + } catch { + return { + message: PROPOSE_GOAL_UNAVAILABLE_MESSAGE, + type: ToolErrorType.EXECUTION_DENIED, + }; + } + const current = runtime.getSnapshot().goal; + if (current?.status === 'active') { + return { + message: activeGoalMessage(current.revision), + type: ToolErrorType.EXECUTION_DENIED, + }; + } + return undefined; + } + + override async getConfirmationDetails( + _abortSignal: AbortSignal, + ): Promise { + const blocker = await this.blocker(); + if (blocker) { + throw new StructuredToolError(blocker.message, blocker.type); + } + const current = this.config.getGoalRuntime().getSnapshot().goal; + return { + type: 'info', + title: 'Set this as the session Goal?', + prompt: `${proposalPromptHeadline(current)}\n\n${this.params.objective.trim()}`, + renderPromptAsPlainText: true, + onConfirm: async (outcome: ToolConfirmationOutcome) => { + this.approved = outcome !== ToolConfirmationOutcome.Cancel; + }, + }; + } + + async execute(_signal: AbortSignal): Promise { + if (!this.approved) { + return this.errorResult( + PROPOSE_GOAL_NOT_APPROVED_MESSAGE, + ToolErrorType.EXECUTION_DENIED, + ); + } + const blocker = await this.blocker(); + if (blocker) return this.errorResult(blocker.message, blocker.type); + + const objective = this.params.objective.trim(); + const current = this.config.getGoalRuntime().getSnapshot().goal; + // Parked, not dispatched: the client sets it when this turn ends. Doing + // it here would strip the rest of the turn of its Goal permit. The + // approval is bound to this turn's prompt id so no other frame can + // apply it. + const turnKey = promptIdContext.getStore(); + if (!turnKey) { + return this.errorResult( + PROPOSE_GOAL_NO_TURN_MESSAGE, + ToolErrorType.EXECUTION_DENIED, + ); + } + if (!this.config.setPendingGoalProposal({ objective, turnKey })) { + return this.errorResult( + PROPOSE_GOAL_PENDING_MESSAGE, + ToolErrorType.EXECUTION_DENIED, + ); + } + const payload = { + approved: true, + objective, + ...(current ? { replacesGoalId: current.goalId } : {}), + next: 'The user approved the Goal. It is set the moment this turn ends: reply with one sentence acknowledging it and stop. Do not call more tools and do not begin the objective; the Goal runtime starts the first Goal turn on its own.', + }; + return { + llmContent: JSON.stringify(payload), + returnDisplay: `Goal approved · ${capDisplay(objective)}`, + }; + } + + private errorResult(message: string, type: ToolErrorType): GoalToolResult { + return { + llmContent: message, + returnDisplay: message, + error: { message, type }, + }; + } +} + +function capDisplay(objective: string): string { + const firstLine = objective.split('\n')[0] ?? objective; + return firstLine.length > 96 ? `${firstLine.slice(0, 95)}…` : firstLine; +} + +export class ProposeGoalTool extends BaseDeclarativeTool< + ProposeGoalToolParams, + GoalToolResult +> { + static readonly Name = ToolNames.PROPOSE_GOAL; + + constructor(private readonly config: ProposeGoalToolConfig) { + super( + ProposeGoalTool.Name, + ToolDisplayNames.PROPOSE_GOAL, + `Propose a session Goal for the user to approve. The user sees the objective in an approval dialog and decides; only their approval sets the Goal. This tool never sets one on its own, and no permission rule or approval mode skips the dialog. Propose only when the user asked for an outcome with a verifiable end state that spans multiple turns ("make the tests pass", "migrate every call site", or after /goal-draft produced an objective), and never to widen scope: the objective must follow from their request. Write the objective so an independent verifier can judge it from transcript evidence alone: one outcome; numbered binary "Done when" checks that name a command and ask to paste its output; what must not change; a budget; what to do when blocked. At most ${PROPOSE_GOAL_OBJECTIVE_MAX_CHARACTERS} characters, on one line. One Goal is active at a time: if a Goal is active this tool refuses and you must hand the user a \`/goal edit …\` or \`/goal set …\` line instead; a stopped Goal (paused, blocked, complete, usage-limited) is replaced on approval. If the user declines you will not be told why: do not ask about it and do not propose the same or a reworded objective again. After approval the Goal is set the moment the current turn ends: acknowledge it in one sentence and stop, without further tool calls; the Goal runtime starts the first Goal turn on its own. Unavailable in plan mode, in subagents, and in headless runs.`, + Kind.Other, + { + type: 'object', + properties: { + objective: { + type: 'string', + minLength: 1, + maxLength: PROPOSE_GOAL_OBJECTIVE_MAX_CHARACTERS, + description: `The objective to propose, written so the Goal verifier can judge it from the transcript (e.g. "Outcome: … Done when: 1) npm test exits 0 (paste the summary line) … Must not: … Budget: stop as blocked after 20 turns. On block: …"). At most ${PROPOSE_GOAL_OBJECTIVE_MAX_CHARACTERS} characters; the user reads all of it in the approval dialog.`, + }, + }, + required: ['objective'], + additionalProperties: false, + }, + ); + } + + protected override validateToolParamValues( + params: ProposeGoalToolParams, + ): string | null { + if (typeof params.objective !== 'string' || !params.objective.trim()) { + return 'objective must be a non-empty string.'; + } + if (params.objective.length > PROPOSE_GOAL_OBJECTIVE_MAX_CHARACTERS) { + return `objective must be at most ${PROPOSE_GOAL_OBJECTIVE_MAX_CHARACTERS} characters.`; + } + return null; + } + + protected createInvocation( + params: ProposeGoalToolParams, + ): ToolInvocation { + return new ProposeGoalInvocation(params, this.config); + } +} diff --git a/packages/core/src/permissions/rule-parser.ts b/packages/core/src/permissions/rule-parser.ts index dff9680b695..02f1e8812bc 100644 --- a/packages/core/src/permissions/rule-parser.ts +++ b/packages/core/src/permissions/rule-parser.ts @@ -168,6 +168,9 @@ export const TOOL_NAME_ALIASES: Readonly> = { update_goal: 'update_goal', UpdateGoal: 'update_goal', UpdateGoalTool: 'update_goal', + propose_goal: 'propose_goal', + ProposeGoal: 'propose_goal', + ProposeGoalTool: 'propose_goal', // Save Memory tool save_memory: 'save_memory', diff --git a/packages/core/src/skills/bundled/goal-draft/SKILL.md b/packages/core/src/skills/bundled/goal-draft/SKILL.md index 639cf20f7b7..f9f0de5b43c 100644 --- a/packages/core/src/skills/bundled/goal-draft/SKILL.md +++ b/packages/core/src/skills/bundled/goal-draft/SKILL.md @@ -100,7 +100,11 @@ Check every line before printing: 8. Under ~1200 characters. 9. Irreversible actions (push, delete, publish) are listed in Must not, or the user explicitly allowed them. -Then print, and nothing else: +Then hand off, and nothing else: + +**If the `propose_goal` tool is available and no Goal is active**, call it with the objective on one line. The user approves or declines it in a dialog; only their approval sets the Goal. If they decline you will not be told why: stop, do not ask about it, and do not propose the same or a reworded objective again. After approval, acknowledge it in one sentence and end the turn — the Goal runtime starts the first Goal turn on its own. + +**Otherwise** (headless, the tool is disabled, or a Goal is active), print: 1. The objective in a fenced code block. 2. One line the user can run as-is: `/goal set ` (or `/goal edit …` when tightening the active goal). Print it as plain text with no code markers, so it can be copied verbatim. diff --git a/packages/core/src/skills/bundled/goal-draft/SKILL.test.ts b/packages/core/src/skills/bundled/goal-draft/SKILL.test.ts index 2e018c1694d..00fc8dd7839 100644 --- a/packages/core/src/skills/bundled/goal-draft/SKILL.test.ts +++ b/packages/core/src/skills/bundled/goal-draft/SKILL.test.ts @@ -46,6 +46,9 @@ describe('bundled goal-draft skill', () => { expect(config.allowedTools).not.toContain('write_file'); expect(config.allowedTools).not.toContain('edit'); expect(config.allowedTools).not.toContain('update_goal'); + // propose_goal shows its approval dialog through the tool's own 'ask' + // default; a grant here would only mislead (see ask_user_question). + expect(config.allowedTools).not.toContain('propose_goal'); // ask_user_question must stay ungranted: a session-wide allow rule // overrides its 'ask' default and the scheduler then runs it without // showing the dialog, fabricating a declined-answer result (see the @@ -173,6 +176,23 @@ describe('bundled goal-draft skill', () => { expect(body).toContain('Print it as plain text with no code markers'); }); + it('hands off through propose_goal when it is available, and prints the /goal line otherwise', () => { + const { body } = loadGoalDraftSkill(); + + expect(body).toContain( + 'If the `propose_goal` tool is available and no Goal is active', + ); + expect(body).toContain('only their approval sets the Goal'); + expect(body).toContain( + 'do not propose the same or a reworded objective again', + ); + expect(body).toContain('acknowledge it in one sentence and end the turn'); + // The text hand-off survives for headless runs and disabled tools. + expect(body).toContain( + '**Otherwise** (headless, the tool is disabled, or a Goal is active)', + ); + }); + it('ends with the self-check list and an explicit stop', () => { const { body } = loadGoalDraftSkill(); diff --git a/packages/core/src/tools/tool-names.ts b/packages/core/src/tools/tool-names.ts index a53f9902058..4756e7d7672 100644 --- a/packages/core/src/tools/tool-names.ts +++ b/packages/core/src/tools/tool-names.ts @@ -65,6 +65,7 @@ export const ToolNames = { REPORT_FINDINGS: 'report_findings', GET_GOAL: 'get_goal', UPDATE_GOAL: 'update_goal', + PROPOSE_GOAL: 'propose_goal', DISPLAY_IMAGE: 'display_image', } as const; @@ -121,6 +122,7 @@ export const ToolDisplayNames = { REPORT_FINDINGS: 'ReportFindings', GET_GOAL: 'Goal', UPDATE_GOAL: 'UpdateGoal', + PROPOSE_GOAL: 'ProposeGoal', DISPLAY_IMAGE: 'DisplayImage', } as const; diff --git a/packages/vscode-ide-companion/schemas/settings.schema.json b/packages/vscode-ide-companion/schemas/settings.schema.json index 19ae43c0828..8285e58a119 100644 --- a/packages/vscode-ide-companion/schemas/settings.schema.json +++ b/packages/vscode-ide-companion/schemas/settings.schema.json @@ -1562,6 +1562,20 @@ } } }, + "goals": { + "description": "Settings for session Goals (/goal).", + "type": "object", + "properties": { + "modelProposed": { + "description": "Controls the propose_goal tool, which lets the model propose a session Goal for you to approve. \"alwaysAsk\" (default) shows every proposal in an approval dialog and nothing is set until you accept it; \"disabled\" removes the tool. A typed /goal is unaffected. Consent-affecting, so this setting is only honored from User, System, or SystemDefaults scope; workspace values are ignored. Options: alwaysAsk, disabled", + "enum": [ + "alwaysAsk", + "disabled" + ], + "default": "alwaysAsk" + } + } + }, "agents": { "description": "Settings for built-in agents and multi-agent collaboration features (Arena, Team, Swarm).", "type": "object", diff --git a/packages/web-shell/client/components/messages/toolFormatting.ts b/packages/web-shell/client/components/messages/toolFormatting.ts index da341c255ee..d6599a76bbb 100644 --- a/packages/web-shell/client/components/messages/toolFormatting.ts +++ b/packages/web-shell/client/components/messages/toolFormatting.ts @@ -23,6 +23,7 @@ export const TOOL_DISPLAY_NAMES: Record = { todo_write: 'TodoList', get_goal: 'Goal', update_goal: 'UpdateGoal', + propose_goal: 'ProposeGoal', save_memory: 'SaveMemory', agent: 'Agent', skill: 'Skill', diff --git a/packages/web-shell/client/i18n.tsx b/packages/web-shell/client/i18n.tsx index 32d307358ae..eb8e2a998a0 100644 --- a/packages/web-shell/client/i18n.tsx +++ b/packages/web-shell/client/i18n.tsx @@ -3403,6 +3403,7 @@ const ZH: Messages = { 'toolName.todo_write': '任务清单', 'toolName.get_goal': '目标', 'toolName.update_goal': '更新目标', + 'toolName.propose_goal': '提议目标', 'toolName.save_memory': '保存记忆', 'toolName.agent': '智能体', 'toolName.skill': '查看技能',