diff --git a/src/subagent/task-tool.ts b/src/subagent/task-tool.ts index 832f0e248..3426428ed 100644 --- a/src/subagent/task-tool.ts +++ b/src/subagent/task-tool.ts @@ -195,6 +195,51 @@ function taskToolResult(callId: string, content: string): ToolResult { return { callId, content, ...(isError ? { isError: true } : {}) }; } +type RequiredTaskField = "description" | "prompt"; + +const REQUIRED_TASK_FIELD_HINTS: Record = { + description: "a short label for the sub-agent job", + prompt: "the actionable goal for the worker", +}; + +/** Truncated echo of a received value so the rejection shows what arrived. */ +function receivedFieldPreview(value: string): string { + const trimmed = value.trim(); + return JSON.stringify(trimmed.length > 80 ? `${trimmed.slice(0, 77)}...` : trimmed); +} + +/** + * Rejection naming only the actually-bad required fields, echoing the valid + * one back. A generic "requires description and prompt" hid which field was + * missing, so models retried the identical call verbatim (CL-6901). + */ +function requiredTaskFieldsError( + args: Record, + bad: readonly RequiredTaskField[], +): string { + const parts = bad.map((name) => { + const value = args[name]; + const hint = REQUIRED_TASK_FIELD_HINTS[name]; + if (value === undefined) return `is missing ${name} (string): ${hint}`; + if (typeof value !== "string") return `has invalid ${name} (must be a string): ${hint}`; + return `requires a non-empty ${name}: ${hint}`; + }); + let message = `Error: task ${parts.join(" and ")}.`; + const good = (Object.keys(REQUIRED_TASK_FIELD_HINTS) as RequiredTaskField[]).filter( + (name) => + !bad.includes(name) && + typeof args[name] === "string" && + (args[name] as string).trim().length > 0, + ); + if (good.length > 0) { + const echo = good + .map((name) => `${name} ${receivedFieldPreview(args[name] as string)}`) + .join(" and "); + message += ` Received ${echo} — keep it and add ${bad.join(" and ")}.`; + } + return message; +} + export function createTaskTool(deps: TaskToolDeps): AgentTool { const run = deps.run; const telemetry = deps.telemetry ?? NOOP_TELEMETRY; @@ -206,10 +251,13 @@ export function createTaskTool(deps: TaskToolDeps): AgentTool { const args = call.arguments; const parsed = TaskToolArgs(args); if (parsed instanceof type.errors) { - return taskToolResult( - call.id, - "Error: task requires description (string) and prompt (string).", + const bad = (["description", "prompt"] as const).filter( + (name) => typeof args[name] !== "string", ); + if (bad.length === 0) { + return taskToolResult(call.id, `Error: task arguments invalid: ${parsed.summary}`); + } + return taskToolResult(call.id, requiredTaskFieldsError(args, bad)); } const { description: rawDesc, @@ -233,7 +281,10 @@ export function createTaskTool(deps: TaskToolDeps): AgentTool { const doNot = rawDoNot?.map((d) => d.trim()).filter((d) => d.length > 0) ?? []; const reportFocus = rawReportFocus?.trim(); if (description.length === 0 || prompt.length === 0) { - return taskToolResult(call.id, "Error: task requires a non-empty description and prompt."); + const empty = (["description", "prompt"] as const).filter( + (name) => (name === "description" ? description : prompt).length === 0, + ); + return taskToolResult(call.id, requiredTaskFieldsError(args, empty)); } let provider: SubAgentProvider = diff --git a/tests/unit/subagent.test.ts b/tests/unit/subagent.test.ts index 02972200e..0904c5a3b 100644 --- a/tests/unit/subagent.test.ts +++ b/tests/unit/subagent.test.ts @@ -48,7 +48,7 @@ test("task tool definition requires description and prompt", () => { expect(taskToolDefinition.inputSchema.required).toEqual(["description", "prompt"]); }); -test("handler rejects empty description or prompt", async () => { +test("handler rejects empty description or prompt, naming only the empty field", async () => { const tool = createTaskTool({ permissionGate: testPermissionGate, cwd: "/repo", @@ -56,8 +56,40 @@ test("handler rejects empty description or prompt", async () => { provider, run: async () => "should not run", }); - expect(await callHandler(tool, { description: "", prompt: "do it" })).toContain("Error:"); - expect(await callHandler(tool, { description: "label", prompt: " " })).toContain("Error:"); + const emptyDesc = await callHandler(tool, { description: "", prompt: "do it" }); + expect(emptyDesc).toContain("Error: task requires a non-empty description"); + expect(emptyDesc).toContain('Received prompt "do it"'); + expect(emptyDesc).not.toContain("non-empty prompt"); + const emptyPrompt = await callHandler(tool, { description: "label", prompt: " " }); + expect(emptyPrompt).toContain("Error: task requires a non-empty prompt"); + expect(emptyPrompt).toContain('Received description "label" — keep it and add prompt.'); + expect(emptyPrompt).not.toContain("non-empty description"); +}); + +test("handler rejects missing required fields, naming only the missing ones", async () => { + const tool = createTaskTool({ + permissionGate: testPermissionGate, + cwd: "/repo", + getWorkdirBase: () => "/repo/.ctx", + provider, + run: async () => "should not run", + }); + const missingPrompt = await callHandler(tool, { description: "Add GET /health route" }); + expect(missingPrompt).toContain( + "Error: task is missing prompt (string): the actionable goal for the worker.", + ); + expect(missingPrompt).toContain( + 'Received description "Add GET /health route" — keep it and add prompt.', + ); + expect(missingPrompt).not.toContain("missing description"); + const missingDesc = await callHandler(tool, { prompt: "do it" }); + expect(missingDesc).toContain("Error: task is missing description (string)"); + expect(missingDesc).toContain('Received prompt "do it" — keep it and add description.'); + expect(missingDesc).not.toContain("missing prompt"); + const missingBoth = await callHandler(tool, {}); + expect(missingBoth).toContain("Error: task is missing description (string)"); + expect(missingBoth).toContain("is missing prompt (string)"); + expect(missingBoth).not.toContain("Received"); }); test("generic leaf gets role-default medium even when parent effort is high", async () => {