diff --git a/packages/runtime/src/__tests__/session-manager.test.ts b/packages/runtime/src/__tests__/session-manager.test.ts index 504faa7065..08bb7810cd 100644 --- a/packages/runtime/src/__tests__/session-manager.test.ts +++ b/packages/runtime/src/__tests__/session-manager.test.ts @@ -2838,6 +2838,43 @@ describe('SessionManager child-session runtime primitive', () => { while (!(await parentTurn.next()).done) {} }); + test('tells callers to retry when a subagent preset profile changes during spawn', async () => { + const manager = new SessionManager({ + store: new MemorySessionStore(), + backends: new BackendRegistry(), + subagentCatalog: { + list: async () => [], + resolve: async (id) => ({ + connectionId: '33333333-3333-4333-8333-333333333333', + id, + name: 'Changed preset', + description: 'Changed while spawning', + profile: IMPLEMENTATION_AGENT_DEFINITION.profile, + connectionSlug: 'worker-connection', + model: 'worker-model', + thinkingLevel: 'low', + enabled: true, + }), + }, + newId: nextId(), + now: nextNow(150), + }); + + await expectRejects( + manager.spawnChildSession('parent-session', { + spawnedBy: { + parentRunId: 'parent-run', + parentTurnId: 'parent-turn', + toolCallId: 'tool-call-preset-race', + }, + agentProfile: LOCAL_READ_AGENT_PROFILE, + subagentId: 'changed-preset', + prompt: 'inspect cheaply', + }), + /profile changed during spawn\. Retry the same agent_spawn call\./, + ); + }); + test('child sessions preserve an explicit no-project association', async () => { const store = new MemorySessionStore(); const runStore = new MemoryAgentRunStore(); diff --git a/packages/runtime/src/__tests__/subagent-tools.test.ts b/packages/runtime/src/__tests__/subagent-tools.test.ts index 08434de0f7..0c5a7e4af3 100644 --- a/packages/runtime/src/__tests__/subagent-tools.test.ts +++ b/packages/runtime/src/__tests__/subagent-tools.test.ts @@ -60,7 +60,7 @@ import { buildSubagentOutputTool, buildSubagentSpawnTool, } from '../subagent-tools.js'; -import { ToolRuntime, type MakaTool } from '../tool-runtime.js'; +import { ToolRuntime, type MakaTool, type MakaToolContext } from '../tool-runtime.js'; describe('subagent tools', () => { test('parent-facing agent tools declare permission hints and names', () => { @@ -109,8 +109,10 @@ describe('subagent tools', () => { }; assert.deepStrictEqual(Object.keys(await advertisedProperties(buildSubagentSpawnTool())), [ + 'target_kind', 'profile', 'subagent_id', + 'executor_mode', 'executor_id', 'task', 'write_back', @@ -169,6 +171,233 @@ describe('subagent tools', () => { ); }); + for (const { targetKind, selector, value, inactive } of [ + { + targetKind: 'profile', + selector: 'profile', + value: LOCAL_READ_AGENT_PROFILE, + inactive: { subagent_id: 'fast-reader' }, + }, + { + targetKind: 'preset', + selector: 'subagent_id', + value: 'fast-reader', + inactive: { profile: LOCAL_READ_AGENT_PROFILE }, + }, + ] as const) { + test(`agent_spawn identifies the missing ${selector} in ${targetKind} mode and accepts the correction`, () => { + const schema = buildSubagentSpawnTool().parameters as { + safeParse(input: unknown): { + success: boolean; + data?: Record; + error?: { issues: Array<{ message: string; path: PropertyKey[] }> }; + }; + }; + const input = { target_kind: targetKind, task: 'Inspect the repo.' }; + for (const args of [input, { ...input, ...inactive }]) { + const rejected = schema.safeParse(args); + assert.strictEqual(rejected.success, false); + assert.strictEqual(rejected.error?.issues.length, 1); + const issue = rejected.error!.issues[0]!; + assert.deepStrictEqual(issue.path, [selector]); + assert.match(issue.message, new RegExp(`target_kind=${targetKind} requires ${selector}`)); + assert.match(issue.message, /spawn_args/); + // Follow the reported field while keeping the mode and inactive field unchanged. + const corrected = schema.safeParse({ ...args, [String(issue.path[0])]: value }); + assert.strictEqual(corrected.success, true); + assert.deepStrictEqual(corrected.data, { ...input, [selector]: value }); + } + }); + } + + test('explicit spawn modes discard provider-filled inactive fields and validate active fields', () => { + const schema = buildSubagentSpawnTool().parameters as { + safeParse(input: unknown): { success: boolean; data?: Record }; + }; + const inherited = { + target_kind: 'profile', + profile: 'implementation', + subagent_id: { placeholder: true }, + executor_mode: 'inherit', + executor_id: '', + task: 'Read the fixture.', + }; + assert.deepStrictEqual(schema.safeParse(inherited).data, { + target_kind: 'profile', + profile: 'implementation', + executor_mode: 'inherit', + task: 'Read the fixture.', + }); + assert.strictEqual(schema.safeParse({ ...inherited, profile: 'invented' }).success, false); + assert.strictEqual(schema.safeParse({ ...inherited, executor_mode: 'plugin' }).success, false); + assert.strictEqual( + schema.safeParse({ ...inherited, executor_mode: 'plugin', executor_id: undefined }).success, + false, + ); + assert.strictEqual( + schema.safeParse({ ...inherited, executor_mode: 'plugin', executor_id: 'codex' }).data + ?.executor_id, + 'codex', + ); + assert.strictEqual( + schema.safeParse({ ...inherited, executor_mode: undefined, executor_id: 'default' }).data + ?.executor_id, + 'default', + ); + assert.strictEqual(schema.safeParse({ ...inherited, target_kind: 'preset' }).success, false); + }); + + test('empty presets still expose a callable implementation profile and recover a mistaken preset selector', async () => { + const calls: Array>[0]> = []; + const ctx: MakaToolContext = { + sessionId: 'session-1', + turnId: 'parent-turn', + cwd: '/tmp/cwd', + toolCallId: 'tool-spawn', + abortSignal: new AbortController().signal, + emitOutput: () => {}, + listChildAgents: async () => ({ + definitions: [ + { ...IMPLEMENTATION_AGENT_DEFINITION, availability: { status: 'available' } }, + ], + presets: [], + }), + spawnChildSession: async (input) => { + calls.push(input); + return { + childSessionId: 'child-session', + agentId: IMPLEMENTATION_AGENT_ID, + agentName: 'Implementation', + turnId: 'child-turn', + runId: 'child-run', + status: 'completed', + permissionMode: 'ask', + summary: 'done', + artifactIds: [], + }; + }, + }; + const spawn = buildSubagentSpawnTool({ definitions: [IMPLEMENTATION_AGENT_DEFINITION] }); + const schema = spawn.parameters as { + parse(input: unknown): Parameters[0]; + }; + // The real failure: three tasks reused the same mistaken selector and invented executor. + await Promise.all( + ['Shared validation', 'Storage lifecycle', 'Desktop UI'].map((task) => + assert.rejects( + async () => + spawn.impl( + schema.parse({ + subagent_id: 'implementation', + executor_id: 'default', + isolation: 'worktree', + write_back: 'patch', + task, + }), + ctx, + ), + (error: Error) => { + assert.match(error.message, /No child was started/); + assert.match(error.message, /"target_kind":"profile"/); + assert.match(error.message, /"profile":"implementation"/); + assert.match(error.message, /"executor_mode":"inherit"/); + assert.match(error.message, /"default" is not a default selector/); + return true; + }, + ), + ), + ); + assert.strictEqual(calls.length, 0); + + const catalog = (await buildSubagentListTool().impl({}, ctx)) as { + presets: unknown[]; + legacy_profiles: Array<{ spawn_args: Record }>; + }; + assert.deepStrictEqual(catalog.presets, []); + await spawn.impl( + schema.parse({ + ...catalog.legacy_profiles[0]!.spawn_args, + subagent_id: 'unused', + executor_id: '', + task: 'Shared validation', + }), + ctx, + ); + assert.strictEqual(calls.length, 1); + assert.strictEqual(calls[0]!.agentProfile, IMPLEMENTATION_AGENT_PROFILE); + assert.strictEqual(calls[0]!.prompt, 'Shared validation'); + assert.strictEqual('subagentId' in calls[0]!, false); + assert.strictEqual('executorId' in calls[0]!, false); + }); + + test('preset selectors remain authoritative even when their id matches a built-in profile', async () => { + const calls: unknown[] = []; + let availability = { status: 'available' }; + const ctx: MakaToolContext = { + sessionId: 'session-1', + turnId: 'parent-turn', + cwd: '/tmp/cwd', + toolCallId: 'tool-spawn', + abortSignal: new AbortController().signal, + emitOutput: () => {}, + listChildAgents: async () => ({ + definitions: [{ ...LOCAL_READ_AGENT_DEFINITION, availability: { status: 'available' } }], + presets: [ + { + id: 'implementation', + name: 'Custom reader', + description: 'Read-only preset with a name shared by a built-in profile.', + profile: LOCAL_READ_AGENT_PROFILE, + model: 'mock-model', + availability, + }, + ], + }), + spawnChildSession: async (input) => { + calls.push(input); + return { + childSessionId: 'child-session', + agentId: LOCAL_READ_AGENT_ID, + agentName: 'Custom reader', + turnId: 'child-turn', + runId: 'child-run', + status: 'completed', + permissionMode: 'explore', + summary: 'done', + artifactIds: [], + }; + }, + }; + const spawn = buildSubagentSpawnTool(); + const schema = spawn.parameters as { + parse(input: unknown): Parameters[0]; + }; + const catalog = (await buildSubagentListTool().impl({}, ctx)) as { + presets: Array<{ spawn_args: Record }>; + }; + const input = schema.parse({ + ...catalog.presets[0]!.spawn_args, + profile: IMPLEMENTATION_AGENT_PROFILE, + task: 'Inspect files', + }); + await spawn.impl(input, ctx); + assert.partialDeepStrictEqual(calls[0], { + agentProfile: LOCAL_READ_AGENT_PROFILE, + subagentId: 'implementation', + }); + availability = { status: 'unavailable' }; + await assert.rejects( + async () => spawn.impl(input, ctx), + /Subagent preset "implementation" is unavailable/, + ); + assert.strictEqual(calls.length, 1); + await assert.rejects( + async () => spawn.impl({ subagent_id: 'invented-preset', task: 'Inspect files' }, ctx), + /copy an available choice's spawn_args/, + ); + assert.strictEqual(calls.length, 1); + }); + test('built-in catalog exposes local-read without shell, web, nested, or write tools', () => { const definitions = listBuiltinAgentDefinitions({ tools: [ @@ -836,6 +1065,11 @@ describe('subagent tools', () => { presets: [ { subagent_id: 'fast-reader', + spawn_args: { + target_kind: 'preset', + subagent_id: 'fast-reader', + executor_mode: 'inherit', + }, name: 'Fast reader', description: 'Cheap repository inspection.', profile: LOCAL_READ_AGENT_PROFILE, @@ -849,6 +1083,11 @@ describe('subagent tools', () => { agent_id: LOCAL_READ_AGENT_ID, profile: LOCAL_READ_AGENT_PROFILE, name: 'Local Read', + spawn_args: { + target_kind: 'profile', + profile: LOCAL_READ_AGENT_PROFILE, + executor_mode: 'inherit', + }, description: 'Read-only repository exploration.', workspace: 'same_workspace', write_back: 'summary', @@ -899,6 +1138,14 @@ describe('subagent tools', () => { contract: { workspace: 'same_workspace', defaultWriteBack: 'summary' }, availability: { status: 'available' }, }, + { + id: 'web-research', + profile: 'web_research', + name: 'Web Research', + description: 'Read-only web research.', + contract: { workspace: 'same_workspace', defaultWriteBack: 'summary' }, + availability: { status: 'unavailable', reason: 'missing_tools' }, + }, ], presets: Array.from({ length: 11 }, (_, index) => ({ id: `reader-${index}`, @@ -947,6 +1194,16 @@ describe('subagent tools', () => { status: 'unavailable', reason: 'connection_disabled', }); + assert.strictEqual( + 'spawn_args' in (diagnosticTail.presets as Array>)[2]!, + false, + ); + const unavailableProfile = ( + diagnosticTail.legacy_profiles as Array> + ).find((profile) => profile.status === 'unavailable'); + assert.ok(unavailableProfile); + assert.strictEqual(unavailableProfile.reason, 'missing_tools'); + assert.strictEqual('spawn_args' in unavailableProfile, false); const worstCase = await call( {}, diff --git a/packages/runtime/src/session-manager.ts b/packages/runtime/src/session-manager.ts index 6146073f02..91dca00b51 100644 --- a/packages/runtime/src/session-manager.ts +++ b/packages/runtime/src/session-manager.ts @@ -2643,7 +2643,9 @@ export class SessionManager { } const resolvedPreset = await this.deps.subagentCatalog.resolve(input.subagentId); if (resolvedPreset.profile !== input.agentProfile) { - throw new Error(`Subagent preset "${input.subagentId}" profile changed during spawn`); + throw new Error( + `Subagent preset "${input.subagentId}" profile changed during spawn. Retry the same agent_spawn call.`, + ); } return { ...input, resolvedPreset }; } diff --git a/packages/runtime/src/subagent-tools.ts b/packages/runtime/src/subagent-tools.ts index 29982e8dba..45a56bb838 100644 --- a/packages/runtime/src/subagent-tools.ts +++ b/packages/runtime/src/subagent-tools.ts @@ -60,6 +60,8 @@ const AGENT_LIST_PAGE_SIZE = 8; const AGENT_LIST_MAX_RESPONSE_CHARS = 7_000; const AGENT_LIST_DESCRIPTION_MAX_CHARS = 240; const AGENT_LIST_MODEL_MAX_CHARS = 160; +const CHILD_EXECUTOR_SELECTION_GUIDANCE = + 'Use executor_mode=inherit for the preset or inherited execution route; executor_id is then ignored. Use executor_mode=plugin only for an explicitly selected registered plugin executor. Without executor_mode, omit executor_id to inherit; "default" is not a default selector.'; /** * Which schema fields each `agent_output` locator needs. A rejection that only @@ -92,8 +94,10 @@ export function buildSubagentSpawnTool( deps: { definitions?: readonly AgentDefinition[] } = {}, ): MakaTool< { + target_kind?: 'profile' | 'preset'; profile?: string; subagent_id?: string; + executor_mode?: 'inherit' | 'plugin'; executor_id?: string; task: string; write_back?: string; @@ -107,24 +111,40 @@ export function buildSubagentSpawnTool( name: AGENT_SPAWN_TOOL_NAME, displayName: 'Agent', description: - 'Run one bounded foreground child task. Prefer agent_list, then select the user-approved subagent_id whose description fits the task; profile is retained for legacy callers. If both selectors are present, subagent_id wins and profile is ignored.', + "Run one bounded foreground child task. Call agent_list and copy an available choice's spawn_args, then add task. target_kind=profile uses profile; target_kind=preset uses subagent_id. The unused selector is ignored. Empty presets does not disable built-in profiles. Without target_kind, subagent_id takes precedence for legacy callers. Independent child tasks may be called together in one parallel batch, including corrected retries after selector errors. " + + CHILD_EXECUTOR_SELECTION_GUIDANCE, parameters: z.preprocess( cleanSubagentSpawnInput, z .object({ - profile: z.enum(profiles).optional().describe('Legacy child capability profile.'), + target_kind: z + .enum(['profile', 'preset']) + .optional() + .describe( + 'Choose profile for legacy_profiles or preset for presets. Only the selected identity field is used; the other is ignored even if populated.', + ), + profile: z + .enum(profiles) + .optional() + .describe('Built-in child capability: copy legacy_profiles[].profile from agent_list.'), subagent_id: z .string() .min(1) .max(128) .refine(isSafeSubagentPresetId) .optional() - .describe('User-approved subagent preset id from agent_list.'), + .describe( + 'For target_kind=preset, copy presets[].subagent_id from agent_list. Ignored for target_kind=profile.', + ), + executor_mode: z + .enum(['inherit', 'plugin']) + .optional() + .describe(CHILD_EXECUTOR_SELECTION_GUIDANCE), executor_id: z .string() .refine(isExecutorId) .optional() - .describe('Plugin executor id for this child task.'), + .describe(CHILD_EXECUTOR_SELECTION_GUIDANCE), task: z .string() .min(1) @@ -145,13 +165,32 @@ export function buildSubagentSpawnTool( }) .strip() .superRefine((input, ctx) => { - if (!input.profile && !input.subagent_id) { + if (input.executor_mode === 'plugin' && !input.executor_id) { ctx.addIssue({ code: z.ZodIssueCode.custom, + path: ['executor_id'], message: - 'No child selector was provided. Call agent_list and pass a returned subagent_id to agent_spawn, ' + - `or pass one legacy profile: ${profiles.join(', ')}.`, + 'executor_mode=plugin requires a registered executor_id. Use executor_mode=inherit for normal child execution.', }); + } + if (!input.profile && !input.subagent_id) { + if (input.target_kind) { + const selector = input.target_kind === 'profile' ? 'profile' : 'subagent_id'; + const catalogSection = + input.target_kind === 'profile' ? 'legacy_profiles' : 'presets'; + ctx.addIssue({ + code: z.ZodIssueCode.custom, + path: [selector], + message: `target_kind=${input.target_kind} requires ${selector}. Call agent_list and copy an available ${catalogSection} choice's spawn_args, then add task.`, + }); + } else { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: + 'No child selector was provided. Call agent_list and pass a returned subagent_id to agent_spawn, ' + + `or pass one legacy profile: ${profiles.join(', ')}.`, + }); + } return; } if (input.subagent_id) return; @@ -233,7 +272,10 @@ export function buildSubagentSpawnTool( function cleanSubagentSpawnInput(input: unknown): unknown { if (!input || typeof input !== 'object' || Array.isArray(input)) return input; const cleaned = { ...(input as Record) }; - if (cleaned.subagent_id !== undefined) delete cleaned.profile; + if (cleaned.target_kind === 'profile') delete cleaned.subagent_id; + else if (cleaned.target_kind === 'preset' || cleaned.subagent_id !== undefined) + delete cleaned.profile; + if (cleaned.executor_mode === 'inherit') delete cleaned.executor_id; return cleaned; } @@ -259,7 +301,17 @@ async function resolvePresetDefinition( (candidate as { id?: unknown }).id === subagentId && typeof (candidate as { profile?: unknown }).profile === 'string', ); - if (!preset) throw new Error(`Unknown subagent_id "${subagentId}". Call agent_list first.`); + if (!preset) { + const legacy = definitions.find( + (definition) => definition.profile === subagentId || definition.id === subagentId, + ); + const recovery = legacy + ? `"${subagentId}" identifies a built-in agent, not a configured preset. Use ${JSON.stringify({ target_kind: 'profile', profile: legacy.profile, executor_mode: 'inherit' })}, keeping your task. The unused subagent_id and executor_id fields are ignored in these modes.` + : "Call agent_list and copy an available choice's spawn_args, then add task. presets use subagent_id; legacy_profiles use profile."; + throw new Error( + `Unknown subagent_id "${subagentId}". No child was started. ${recovery} ${CHILD_EXECUTOR_SELECTION_GUIDANCE}`, + ); + } if (preset.availability?.status !== 'available') { throw new Error(`Subagent preset "${subagentId}" is unavailable.`); } @@ -308,7 +360,7 @@ export function buildSubagentListTool(): MakaTool< name: AGENT_LIST_TOOL_NAME, displayName: 'Agent List', description: - 'List a compact page of subagents to select. The default selection view returns runnable user-approved subagent_id values first, followed by legacy choices with separate agent_id (Graph) and profile (agent_spawn) selectors. Use view=catalog only to diagnose unavailable routes. Child execution history is intentionally excluded; use refs returned by agent_spawn or asynchronous graph work with agent_output.', + "List a compact page of subagents to select. Copy an available choice's spawn_args into agent_spawn and add task. presets use subagent_id; legacy_profiles use profile, including when presets is empty. agent_id is for Graph only. Use view=catalog only to diagnose unavailable routes. Child execution history is intentionally excluded; use refs returned by agent_spawn or asynchronous graph work with agent_output.", parameters: z .object({ view: z @@ -381,6 +433,15 @@ function projectAgentList( return [ { subagent_id: preset.id, + ...(availability.status === 'available' + ? { + spawn_args: { + target_kind: 'preset', + subagent_id: preset.id, + executor_mode: 'inherit', + }, + } + : {}), name: boundedCatalogText(preset.name, 128), description: boundedCatalogText(preset.description, AGENT_LIST_DESCRIPTION_MAX_CHARS), profile: preset.profile, @@ -419,6 +480,15 @@ function projectAgentList( { agent_id: definition.id, profile: definition.profile, + ...(availability.status === 'available' + ? { + spawn_args: { + target_kind: 'profile', + profile: definition.profile, + executor_mode: 'inherit', + }, + } + : {}), name: boundedCatalogText(definition.name, 128), description: boundedCatalogText(definition.description, AGENT_LIST_DESCRIPTION_MAX_CHARS), ...(typeof contract?.workspace === 'string'