Skip to content

Commit 5c949d7

Browse files
committed
fix(agent-core): harden collaboration tool inputs
1 parent d539a21 commit 5c949d7

7 files changed

Lines changed: 81 additions & 16 deletions

File tree

‎docs/reference/tools.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ Plan mode is a constrained working state: once entered, `Write` and `Edit` are r
7676
| --- | --- | --- |
7777
| `TodoList` | Auto-allow | Manage a task to-do list |
7878

79-
**`TodoList`** maintains a visible subtask list across multi-step operations; state is stored within the Agent session. The `todos` parameter accepts an array where each item has a `title` and `status` (`pending` / `in_progress` / `done`). Omitting `todos` queries the current list; passing an empty array clears it.
79+
**`TodoList`** maintains a visible subtask list across multi-step operations; state is stored within the Agent session. The `todos` parameter accepts an array where each item has a `title` and `status` (`pending` / `in_progress` / `done`). TodoWrite-compatible items can use `content`, `activeForm`, and `completed`; `done` and `completed` are equivalent with either item shape. Omitting `todos` queries the current list; passing an empty array clears it.
8080

8181
## Collaboration Tools
8282

@@ -97,7 +97,7 @@ Each entry in `items` launches one new subagent: without `prompt_template`, ever
9797

9898
Pass `subagent_type` to choose the profile used by every spawned subagent, or omit it to use `coder`. Pass `model` and `effort` to run this workflow's subagents on a different model than the agent orchestrating them — a cheaper or faster model for mechanical work, for example; both apply to every subagent in the call, and omitting them falls back to the subagent profile's own settings and then to the calling agent's. A `model` the provider cannot resolve falls back to the calling agent's model rather than failing the run.
9999

100-
Without `resume_agent_ids`, the tool requires at least 2 items; with `resume_agent_ids`, it can resume one or more existing subagents. The tool supports up to 128 total subagents, waits for all of them to finish, and returns an aggregated report. Workflow subagents have no automatic timeout; they run until completion, failure, or user cancellation.
100+
Without `resume_agent_ids`, the tool requires at least 2 items; with `resume_agent_ids`, it can resume one or more existing subagents. Calls that fail these plan checks are rejected before an approval request is shown. The tool supports up to 128 total subagents, waits for all of them to finish, and returns an aggregated report. Workflow subagents have no automatic timeout; they run until completion, failure, or user cancellation.
101101

102102
If a model response calls `DynamicWorkflow`, that call must be the only tool call in the response; to run several workflows, call one `DynamicWorkflow`, wait for its result, then call the next, or combine the work into a single workflow.
103103

‎packages/agent-core/src/tools/builtin/collaboration/dynamic-workflow.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
Launch multiple subagents from complete prompts, one prompt template, existing agent resumes, or a combination.
22

3-
Use DynamicWorkflow when several independent subagents should run in parallel. Without `prompt_template`, every `items` entry is a complete subagent prompt. When many subagents should run the same kind of task over different inputs, provide a template whose placeholder is exactly `{{item}}`. For example, with `prompt_template` set to `Review {{item}} for likely regressions.` and `items` set to `["src/a.ts", "src/b.ts"]`, DynamicWorkflow launches two new subagents with those two concrete prompts.
3+
Use DynamicWorkflow when several independent subagents should run in parallel. Without `resume_agent_ids`, provide at least two non-empty items. Without `prompt_template`, every `items` entry is a complete subagent prompt. When many subagents should run the same kind of task over different inputs, provide a template whose placeholder is exactly `{{item}}`. For example, with `prompt_template` set to `Review {{item}} for likely regressions.` and `items` set to `["src/a.ts", "src/b.ts"]`, DynamicWorkflow launches two new subagents with those two concrete prompts.
44

55
Use `resume_agent_ids` to continue subagents that already exist from earlier work, such as ones that failed: map each agent id to the prompt for that resumed subagent (usually `continue` if no extra information is needed). You may combine `resume_agent_ids` with `items` in the same call to resume existing subagents and launch new ones. Do not duplicate resumed work in `items`.
66

‎packages/agent-core/src/tools/builtin/collaboration/dynamic-workflow.ts‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ export const DynamicWorkflowToolInputSchema = z
8888
.array(z.string())
8989
.optional()
9090
.describe(
91-
`Each item launches one new subagent. Items fill ${PROMPT_TEMPLATE_PLACEHOLDER} when prompt_template is provided; otherwise they are complete prompts.`,
91+
`Each item launches one new subagent. Without resume_agent_ids, provide at least two non-empty items. Items fill ${PROMPT_TEMPLATE_PLACEHOLDER} when prompt_template is provided; otherwise they are complete prompts.`,
9292
),
9393
resume_agent_ids: z
9494
.record(z.string().trim().min(1), z.string().trim().min(1))
@@ -168,6 +168,13 @@ export class DynamicWorkflowTool implements BuiltinTool<DynamicWorkflowToolInput
168168
}
169169

170170
resolveExecution(args: DynamicWorkflowToolInput): ToolExecution {
171+
try {
172+
createDynamicWorkflowSpecs(args, (agentId) =>
173+
this.subagentHost.getDynamicWorkflowItem(agentId),
174+
);
175+
} catch (error) {
176+
return { isError: true, output: error instanceof Error ? error.message : String(error) };
177+
}
171178
const workflow = dynamicWorkflowPreview(args);
172179
const approvalSubject = dynamicWorkflowApprovalSubject(args, workflow);
173180
return {
@@ -358,9 +365,8 @@ function hasMinimumDynamicWorkflowInputs(itemCount: number, resumeCount: number)
358365
* What the call is about to launch, for the approval panel — the counted
359366
* subagents, the work each one gets, and how much prompt that adds up to.
360367
*
361-
* Built from the arguments alone and deliberately total: `resolveExecution`
362-
* runs before `createDynamicWorkflowSpecs` validates anything, so a call this
363-
* function threw on would never reach the panel that exists to refuse it.
368+
* Built from the arguments alone after `createDynamicWorkflowSpecs` validates
369+
* the plan, so an invalid workflow never reaches the approval panel.
364370
* `prompt_tokens` is the summed prompt estimate, which is a real input size —
365371
* not a guess at what the run will finally cost.
366372
*/

‎packages/agent-core/src/tools/builtin/state/todo-list.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ Use this tool to maintain a structured TODO list as you work through a multi-ste
2020

2121
**How to use:**
2222
- Call with `todos: [...]` to replace the full list. Use `title` plus an optional present-continuous `activeForm`; statuses are pending / in_progress / done.
23-
- TodoWrite-compatible `content`, required `activeForm`, and completed status are also accepted.
23+
- TodoWrite-compatible `content`, required `activeForm`, and completed status are also accepted. `done` and `completed` are equivalent with either item shape.
2424
- Call with no arguments to retrieve the current list without changing it.
2525
- Call with `todos: []` to clear the list.
2626
- Keep titles short and actionable (e.g. "Read session-control.ts", "Add planMode flag to TurnManager").

‎packages/agent-core/src/tools/builtin/state/todo-list.ts‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,18 +48,22 @@ declare module '../../store' {
4848

4949
// ── Schema ───────────────────────────────────────────────────────────
5050

51+
const TodoInputStatusSchema = z
52+
.enum(['pending', 'in_progress', 'done', 'completed'])
53+
.describe('Current status of the todo. done and completed are equivalent.');
54+
5155
const NativeTodoItemSchema = z.object({
5256
title: z.string().min(1).describe('Short, actionable title for the todo.'),
5357
activeForm: z.string().min(1).optional().describe('Present-continuous activity label.'),
54-
status: z.enum(['pending', 'in_progress', 'done']).describe('Current status of the todo.'),
58+
status: TodoInputStatusSchema,
5559
});
5660

5761
// Accept the older TodoWrite-style shape for compatibility, then normalize it
5862
// into the native TodoList store/render contract below.
5963
const TodoWriteItemSchema = z.object({
6064
content: z.string().min(1).describe('Imperative description of the task.'),
6165
activeForm: z.string().min(1).describe('Present-continuous activity label.'),
62-
status: z.enum(['pending', 'in_progress', 'completed']).describe('Current status of the todo.'),
66+
status: TodoInputStatusSchema,
6367
});
6468

6569
type TodoListInputItem =
@@ -172,16 +176,17 @@ export class TodoListTool implements BuiltinTool<TodoListInput> {
172176
}
173177

174178
function normalizeTodoItem(todo: TodoListInputItem): TodoItem {
179+
const status = todo.status === 'completed' ? 'done' : todo.status;
175180
if ('title' in todo) {
176181
return {
177182
title: todo.title,
178183
activeForm: todo.activeForm,
179-
status: todo.status,
184+
status,
180185
};
181186
}
182187
return {
183188
title: todo.content,
184189
activeForm: todo.activeForm,
185-
status: todo.status === 'completed' ? 'done' : todo.status,
190+
status,
186191
};
187192
}

‎packages/agent-core/test/tools/builtin-current.test.ts‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1068,15 +1068,15 @@ describe('current builtin collaboration tools', () => {
10681068
},
10691069
output: 'prompt_template must include the {{item}} placeholder.',
10701070
},
1071-
])('DynamicWorkflow rejects $name at execution time', async ({ input, output }) => {
1071+
])('DynamicWorkflow rejects $name before approval', ({ input, output }) => {
10721072
const host = mockSubagentHost({ runQueued: vi.fn() });
10731073
const dynamicWorkflowMode = mockDynamicWorkflowMode();
10741074
const tool = new DynamicWorkflowTool(host, dynamicWorkflowMode);
10751075

1076-
const result = await executeTool(tool, context(input));
1076+
const result = tool.resolveExecution(input);
10771077

1078-
expect(result.output).toBe(output);
1079-
expect(result.isError).toBe(true);
1078+
expect(result).toEqual({ isError: true, output });
1079+
expect(dynamicWorkflowMode.enter).not.toHaveBeenCalled();
10801080
expect(host.runQueued).not.toHaveBeenCalled();
10811081
});
10821082

@@ -1598,6 +1598,7 @@ describe('workflowSizeGuideline', () => {
15981598

15991599
const unrestrictedTool = new DynamicWorkflowTool(host, mockDynamicWorkflowMode(), 'unrestricted');
16001600
expect(unrestrictedTool.description).toContain('DynamicWorkflow supports up to 128 subagents');
1601+
expect(unrestrictedTool.description).toContain('at least two non-empty items');
16011602
expect(unrestrictedTool.description).not.toContain('Workflow size guideline:');
16021603
});
16031604
});

‎packages/agent-core/test/tools/todo-list.test.ts‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {
1515
TodoListTool,
1616
type TodoItem,
1717
} from '../../src/tools/builtin/state/todo-list';
18+
import { compileToolArgsValidator, validateToolArgs } from '../../src/tools/args-validator';
1819
import type { ToolStore } from '../../src/tools/store';
1920
import { executeTool } from './fixtures/execute-tool';
2021

@@ -64,6 +65,18 @@ describe('TodoListTool', () => {
6465
todos: { type: 'array' },
6566
},
6667
});
68+
const validator = compileToolArgsValidator(tool.parameters);
69+
expect(
70+
validateToolArgs(validator, {
71+
todos: [
72+
{
73+
title: 'Map logical groups',
74+
activeForm: 'Mapping logical groups',
75+
status: 'completed',
76+
},
77+
],
78+
}),
79+
).toBeNull();
6780
});
6881

6982
it('description includes an Avoid churn section with the anti-spin guardrails', () => {
@@ -178,6 +191,46 @@ describe('TodoListTool', () => {
178191
]);
179192
});
180193

194+
it('normalizes done and completed with either item shape', async () => {
195+
const { tool, getTodos } = makeTool();
196+
const parsed = TodoListInputSchema.parse({
197+
todos: [
198+
{
199+
title: 'Inspect the implementation',
200+
activeForm: 'Inspecting the implementation',
201+
status: 'completed',
202+
},
203+
{
204+
content: 'Run focused tests',
205+
activeForm: 'Running focused tests',
206+
status: 'done',
207+
},
208+
{ title: 'Continue review', status: 'pending' },
209+
],
210+
});
211+
212+
await executeTool(tool, {
213+
turnId: 't1',
214+
toolCallId: 'call_1',
215+
args: parsed,
216+
signal,
217+
});
218+
219+
expect(getTodos()).toEqual([
220+
{
221+
title: 'Inspect the implementation',
222+
activeForm: 'Inspecting the implementation',
223+
status: 'done',
224+
},
225+
{
226+
title: 'Run focused tests',
227+
activeForm: 'Running focused tests',
228+
status: 'done',
229+
},
230+
{ title: 'Continue review', status: 'pending' },
231+
]);
232+
});
233+
181234
it('renders a done todo with a marker matching the status enum value', async () => {
182235
const { tool } = makeTool([{ title: 'shipped', status: 'done' }]);
183236

0 commit comments

Comments
 (0)