Skip to content

Commit 431075e

Browse files
committed
Name only the actually-missing task fields in rejections
The task tool rejected a call missing prompt with "requires description (string) and prompt (string)" even when description was provided, so models could not tell which field was missing and retried the identical call. Rejections now name only the missing, invalid, or empty fields and echo the valid one back with a hint to keep it and add the other.
1 parent bebe563 commit 431075e

2 files changed

Lines changed: 91 additions & 5 deletions

File tree

src/subagent/task-tool.ts

Lines changed: 56 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,51 @@ function taskToolResult(callId: string, content: string): ToolResult {
201201
return { callId, content, ...(isError ? { isError: true } : {}) };
202202
}
203203

204+
type RequiredTaskField = "description" | "prompt";
205+
206+
const REQUIRED_TASK_FIELD_HINTS: Record<RequiredTaskField, string> = {
207+
description: "a short label for the sub-agent job",
208+
prompt: "the actionable goal for the worker",
209+
};
210+
211+
/** Truncated echo of a received value so the rejection shows what arrived. */
212+
function receivedFieldPreview(value: string): string {
213+
const trimmed = value.trim();
214+
return JSON.stringify(trimmed.length > 80 ? `${trimmed.slice(0, 77)}...` : trimmed);
215+
}
216+
217+
/**
218+
* Rejection naming only the actually-bad required fields, echoing the valid
219+
* one back. A generic "requires description and prompt" hid which field was
220+
* missing, so models retried the identical call verbatim (CL-6901).
221+
*/
222+
function requiredTaskFieldsError(
223+
args: Record<string, unknown>,
224+
bad: readonly RequiredTaskField[],
225+
): string {
226+
const parts = bad.map((name) => {
227+
const value = args[name];
228+
const hint = REQUIRED_TASK_FIELD_HINTS[name];
229+
if (value === undefined) return `is missing ${name} (string): ${hint}`;
230+
if (typeof value !== "string") return `has invalid ${name} (must be a string): ${hint}`;
231+
return `requires a non-empty ${name}: ${hint}`;
232+
});
233+
let message = `Error: task ${parts.join(" and ")}.`;
234+
const good = (Object.keys(REQUIRED_TASK_FIELD_HINTS) as RequiredTaskField[]).filter(
235+
(name) =>
236+
!bad.includes(name) &&
237+
typeof args[name] === "string" &&
238+
(args[name] as string).trim().length > 0,
239+
);
240+
if (good.length > 0) {
241+
const echo = good
242+
.map((name) => `${name} ${receivedFieldPreview(args[name] as string)}`)
243+
.join(" and ");
244+
message += ` Received ${echo} — keep it and add ${bad.join(" and ")}.`;
245+
}
246+
return message;
247+
}
248+
204249
export function createTaskTool(deps: TaskToolDeps): AgentTool {
205250
const run = deps.run;
206251
const telemetry = deps.telemetry ?? NOOP_TELEMETRY;
@@ -212,7 +257,13 @@ export function createTaskTool(deps: TaskToolDeps): AgentTool {
212257
const args = call.arguments;
213258
const parsed = TaskToolArgs(args);
214259
if (parsed instanceof type.errors) {
215-
return taskToolResult(call.id, "Error: task requires description (string) and prompt (string).");
260+
const bad = (["description", "prompt"] as const).filter(
261+
(name) => typeof args[name] !== "string",
262+
);
263+
if (bad.length === 0) {
264+
return taskToolResult(call.id, `Error: task arguments invalid: ${parsed.summary}`);
265+
}
266+
return taskToolResult(call.id, requiredTaskFieldsError(args, bad));
216267
}
217268
const {
218269
description: rawDesc,
@@ -244,7 +295,10 @@ export function createTaskTool(deps: TaskToolDeps): AgentTool {
244295
.filter((d) => d.length > 0) ?? [];
245296
const reportFocus = rawReportFocus?.trim();
246297
if (description.length === 0 || prompt.length === 0) {
247-
return taskToolResult(call.id, "Error: task requires a non-empty description and prompt.");
298+
const empty = (["description", "prompt"] as const).filter(
299+
(name) => (name === "description" ? description : prompt).length === 0,
300+
);
301+
return taskToolResult(call.id, requiredTaskFieldsError(args, empty));
248302
}
249303

250304
let provider: SubAgentProvider =

tests/unit/subagent.test.ts

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,16 +48,48 @@ test("task tool definition requires description and prompt", () => {
4848
expect(taskToolDefinition.inputSchema.required).toEqual(["description", "prompt"]);
4949
});
5050

51-
test("handler rejects empty description or prompt", async () => {
51+
test("handler rejects empty description or prompt, naming only the empty field", async () => {
5252
const tool = createTaskTool({
5353
permissionGate: testPermissionGate,
5454
cwd: "/repo",
5555
getWorkdirBase: () => "/repo/.ctx",
5656
provider,
5757
run: async () => "should not run",
5858
});
59-
expect(await callHandler(tool, { description: "", prompt: "do it" })).toContain("Error:");
60-
expect(await callHandler(tool, { description: "label", prompt: " " })).toContain("Error:");
59+
const emptyDesc = await callHandler(tool, { description: "", prompt: "do it" });
60+
expect(emptyDesc).toContain("Error: task requires a non-empty description");
61+
expect(emptyDesc).toContain('Received prompt "do it"');
62+
expect(emptyDesc).not.toContain("non-empty prompt");
63+
const emptyPrompt = await callHandler(tool, { description: "label", prompt: " " });
64+
expect(emptyPrompt).toContain("Error: task requires a non-empty prompt");
65+
expect(emptyPrompt).toContain('Received description "label" — keep it and add prompt.');
66+
expect(emptyPrompt).not.toContain("non-empty description");
67+
});
68+
69+
test("handler rejects missing required fields, naming only the missing ones", async () => {
70+
const tool = createTaskTool({
71+
permissionGate: testPermissionGate,
72+
cwd: "/repo",
73+
getWorkdirBase: () => "/repo/.ctx",
74+
provider,
75+
run: async () => "should not run",
76+
});
77+
const missingPrompt = await callHandler(tool, { description: "Add GET /health route" });
78+
expect(missingPrompt).toContain(
79+
"Error: task is missing prompt (string): the actionable goal for the worker.",
80+
);
81+
expect(missingPrompt).toContain(
82+
'Received description "Add GET /health route" — keep it and add prompt.',
83+
);
84+
expect(missingPrompt).not.toContain("missing description");
85+
const missingDesc = await callHandler(tool, { prompt: "do it" });
86+
expect(missingDesc).toContain("Error: task is missing description (string)");
87+
expect(missingDesc).toContain('Received prompt "do it" — keep it and add description.');
88+
expect(missingDesc).not.toContain("missing prompt");
89+
const missingBoth = await callHandler(tool, {});
90+
expect(missingBoth).toContain("Error: task is missing description (string)");
91+
expect(missingBoth).toContain("is missing prompt (string)");
92+
expect(missingBoth).not.toContain("Received");
6193
});
6294

6395
test("generic leaf gets role-default medium even when parent effort is high", async () => {

0 commit comments

Comments
 (0)