Skip to content

Commit b06492d

Browse files
committed
Merge branch 'main' into cl-6902-repetition-detector-misses-short-phrase-loops-and-zero
2 parents ad6252d + c0c8abe commit b06492d

2 files changed

Lines changed: 90 additions & 7 deletions

File tree

src/subagent/task-tool.ts

Lines changed: 55 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,51 @@ function taskToolResult(callId: string, content: string): ToolResult {
195195
return { callId, content, ...(isError ? { isError: true } : {}) };
196196
}
197197

198+
type RequiredTaskField = "description" | "prompt";
199+
200+
const REQUIRED_TASK_FIELD_HINTS: Record<RequiredTaskField, string> = {
201+
description: "a short label for the sub-agent job",
202+
prompt: "the actionable goal for the worker",
203+
};
204+
205+
/** Truncated echo of a received value so the rejection shows what arrived. */
206+
function receivedFieldPreview(value: string): string {
207+
const trimmed = value.trim();
208+
return JSON.stringify(trimmed.length > 80 ? `${trimmed.slice(0, 77)}...` : trimmed);
209+
}
210+
211+
/**
212+
* Rejection naming only the actually-bad required fields, echoing the valid
213+
* one back. A generic "requires description and prompt" hid which field was
214+
* missing, so models retried the identical call verbatim (CL-6901).
215+
*/
216+
function requiredTaskFieldsError(
217+
args: Record<string, unknown>,
218+
bad: readonly RequiredTaskField[],
219+
): string {
220+
const parts = bad.map((name) => {
221+
const value = args[name];
222+
const hint = REQUIRED_TASK_FIELD_HINTS[name];
223+
if (value === undefined) return `is missing ${name} (string): ${hint}`;
224+
if (typeof value !== "string") return `has invalid ${name} (must be a string): ${hint}`;
225+
return `requires a non-empty ${name}: ${hint}`;
226+
});
227+
let message = `Error: task ${parts.join(" and ")}.`;
228+
const good = (Object.keys(REQUIRED_TASK_FIELD_HINTS) as RequiredTaskField[]).filter(
229+
(name) =>
230+
!bad.includes(name) &&
231+
typeof args[name] === "string" &&
232+
(args[name] as string).trim().length > 0,
233+
);
234+
if (good.length > 0) {
235+
const echo = good
236+
.map((name) => `${name} ${receivedFieldPreview(args[name] as string)}`)
237+
.join(" and ");
238+
message += ` Received ${echo} — keep it and add ${bad.join(" and ")}.`;
239+
}
240+
return message;
241+
}
242+
198243
export function createTaskTool(deps: TaskToolDeps): AgentTool {
199244
const run = deps.run;
200245
const telemetry = deps.telemetry ?? NOOP_TELEMETRY;
@@ -206,10 +251,13 @@ export function createTaskTool(deps: TaskToolDeps): AgentTool {
206251
const args = call.arguments;
207252
const parsed = TaskToolArgs(args);
208253
if (parsed instanceof type.errors) {
209-
return taskToolResult(
210-
call.id,
211-
"Error: task requires description (string) and prompt (string).",
254+
const bad = (["description", "prompt"] as const).filter(
255+
(name) => typeof args[name] !== "string",
212256
);
257+
if (bad.length === 0) {
258+
return taskToolResult(call.id, `Error: task arguments invalid: ${parsed.summary}`);
259+
}
260+
return taskToolResult(call.id, requiredTaskFieldsError(args, bad));
213261
}
214262
const {
215263
description: rawDesc,
@@ -233,7 +281,10 @@ export function createTaskTool(deps: TaskToolDeps): AgentTool {
233281
const doNot = rawDoNot?.map((d) => d.trim()).filter((d) => d.length > 0) ?? [];
234282
const reportFocus = rawReportFocus?.trim();
235283
if (description.length === 0 || prompt.length === 0) {
236-
return taskToolResult(call.id, "Error: task requires a non-empty description and prompt.");
284+
const empty = (["description", "prompt"] as const).filter(
285+
(name) => (name === "description" ? description : prompt).length === 0,
286+
);
287+
return taskToolResult(call.id, requiredTaskFieldsError(args, empty));
237288
}
238289

239290
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)