diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 45b721642..da7ccb93d 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -53,6 +53,8 @@ ### Fixed +- Wrapped `todo_write` operations in an `ops` object so Codex/OpenAI function schemas always use a JSON Schema object. + - Fixed JSON tree rendering for tool arguments by excluding injected internal keys from displayed root records - Printed assistant `errorMessage` text in print mode output to stderr so message-level errors are visible during non-interactive runs - Displayed assistant `errorMessage` text in the assistant message component for completed tool responses with non-terminal stop reasons diff --git a/packages/coding-agent/src/prompts/tools/todo-write.md b/packages/coding-agent/src/prompts/tools/todo-write.md index c29dd363d..cb73bcd0f 100644 --- a/packages/coding-agent/src/prompts/tools/todo-write.md +++ b/packages/coding-agent/src/prompts/tools/todo-write.md @@ -1,20 +1,22 @@ -Manages a phased task list through an ordered list of flat operations. +Manages a phased task list through an `ops` array of flat operations. The next pending task is auto-promoted to `in_progress` after completing the current one. ## Shape -Pass an array of operation objects: +Pass an object with an `ops` array: ```ts -[ - { op: "replace", phases: [...] }, - { op: "start", task: "task-3" }, - { op: "done", phase: "Implementation" }, - { op: "rm" }, - { op: "drop", task: "task-9" }, - { op: "append", phase: "Implementation", items: [{ id: "task-10", label: "Run tests" }] } -] +{ + ops: [ + { op: "replace", phases: [...] }, + { op: "start", task: "task-3" }, + { op: "done", phase: "Implementation" }, + { op: "rm" }, + { op: "drop", task: "task-9" }, + { op: "append", phase: "Implementation", items: [{ id: "task-10", label: "Run tests" }] }, + ], +} ``` ## Operation fields @@ -58,17 +60,17 @@ Create a todo list when: # Initial setup -`[{op: "replace", phases: [{name: "Investigation", tasks: [{content: "Read source"}, {content: "Map callsites"}]}, {name: "Implementation", tasks: [{content: "Apply fix"}, {content: "Run tests"}]}]}]` +`{"ops":[{"op":"replace","phases":[{"name":"Investigation","tasks":[{"content":"Read source"},{"content":"Map callsites"}]},{"name":"Implementation","tasks":[{"content":"Apply fix"},{"content":"Run tests"}]}]}]}` # Complete one task -`[{op: "done", task: "task-2"}]` +`{"ops":[{"op":"done","task":"task-2"}]}` # Complete a whole phase -`[{op: "done", phase: "Implementation"}]` +`{"ops":[{"op":"done","phase":"Implementation"}]}` # Remove all tasks -`[{op: "rm"}]` +`{"ops":[{"op":"rm"}]}` # Drop one task -`[{op: "drop", task: "task-7"}]` +`{"ops":[{"op":"drop","task":"task-7"}]}` # Append tasks to a phase -`[{op: "append", phase: "Implementation", items: [{id: "task-8", label: "Handle retries"}, {id: "task-9", label: "Run tests"}]}]` +`{"ops":[{"op":"append","phase":"Implementation","items":[{"id":"task-8","label":"Handle retries"},{"id":"task-9","label":"Run tests"}]}]}` diff --git a/packages/coding-agent/src/tools/todo-write.ts b/packages/coding-agent/src/tools/todo-write.ts index 1be20ae73..ba7673bb3 100644 --- a/packages/coding-agent/src/tools/todo-write.ts +++ b/packages/coding-agent/src/tools/todo-write.ts @@ -73,13 +73,18 @@ const TodoOpEntry = Type.Object({ items: Type.Optional(Type.Array(AppendItem, { minItems: 1, description: "items to append for op=append" })), }); -const todoWriteSchema = Type.Array(TodoOpEntry, { - minItems: 1, - description: "ordered todo operations", -}); +const todoWriteSchema = Type.Object( + { + ops: Type.Array(TodoOpEntry, { + minItems: 1, + description: "ordered todo operations", + }), + }, + { description: "Apply ordered todo operations" }, +); type TodoWriteParams = Static; -type TodoOpEntryValue = TodoWriteParams[number]; +type TodoOpEntryValue = TodoWriteParams["ops"][number]; // ============================================================================= // File format @@ -332,7 +337,7 @@ function applyEntry(file: TodoFile, entry: TodoOpEntryValue, errors: string[]): function applyParams(file: TodoFile, params: TodoWriteParams): { file: TodoFile; errors: string[] } { const errors: string[] = []; - for (const entry of params) { + for (const entry of params.ops) { file = applyEntry(file, entry, errors); } normalizeInProgressTask(file.phases); @@ -428,12 +433,14 @@ export class TodoWriteTool implements AgentTool; -}>; +type TodoWriteRenderArgs = { + ops?: Array<{ + op?: string; + task?: string; + phase?: string; + items?: Array<{ id?: string; label?: string }>; + }>; +}; function formatTodoLine(item: TodoItem, uiTheme: Theme, prefix: string): string { const checkbox = uiTheme.checkbox; @@ -451,7 +458,7 @@ function formatTodoLine(item: TodoItem, uiTheme: Theme, prefix: string): string export const todoWriteToolRenderer = { renderCall(args: TodoWriteRenderArgs, _options: RenderResultOptions, uiTheme: Theme): Component { - const ops = args?.map(entry => { + const ops = args?.ops?.map(entry => { const parts = [entry.op ?? "update"]; if (entry.task) parts.push(entry.task); if (entry.phase) parts.push(entry.phase); diff --git a/packages/coding-agent/test/agent-session-eager-todo.test.ts b/packages/coding-agent/test/agent-session-eager-todo.test.ts index 4dfb8029f..1e5cc4432 100644 --- a/packages/coding-agent/test/agent-session-eager-todo.test.ts +++ b/packages/coding-agent/test/agent-session-eager-todo.test.ts @@ -208,17 +208,19 @@ describe("AgentSession eager todo enforcement", () => { it("initializes todos once, then continues within the same user turn", async () => { scriptedResponses = [ - createToolCallAssistantMessage("todo_write", [ - { - op: "replace", - phases: [ - { - name: "List worktrees", - tasks: [{ content: "List all git worktrees in the current repository", status: "in_progress" }], - }, - ], - }, - ]), + createToolCallAssistantMessage("todo_write", { + ops: [ + { + op: "replace", + phases: [ + { + name: "List worktrees", + tasks: [{ content: "List all git worktrees in the current repository", status: "in_progress" }], + }, + ], + }, + ], + }), createAssistantMessage("real user turn handled"), ]; diff --git a/packages/coding-agent/test/tools/todo-write.test.ts b/packages/coding-agent/test/tools/todo-write.test.ts index 6f93b7c91..66b060dba 100644 --- a/packages/coding-agent/test/tools/todo-write.test.ts +++ b/packages/coding-agent/test/tools/todo-write.test.ts @@ -21,17 +21,19 @@ function createSession(initialPhases: TodoPhase[] = []): ToolSession { describe("TodoWriteTool auto-start behavior", () => { it("auto-starts the first task after replace", async () => { const tool = new TodoWriteTool(createSession()); - const result = await tool.execute("call-1", [ - { - op: "replace", - phases: [ - { - name: "Execution", - tasks: [{ content: "status" }, { content: "diagnostics" }], - }, - ], - }, - ]); + const result = await tool.execute("call-1", { + ops: [ + { + op: "replace", + phases: [ + { + name: "Execution", + tasks: [{ content: "status" }, { content: "diagnostics" }], + }, + ], + }, + ], + }); const tasks = result.details?.phases[0]?.tasks ?? []; expect(tasks.map(task => task.status)).toEqual(["in_progress", "pending"]); @@ -44,19 +46,21 @@ describe("TodoWriteTool auto-start behavior", () => { it("auto-promotes the next pending task when current task is completed", async () => { const tool = new TodoWriteTool(createSession()); - await tool.execute("call-1", [ - { - op: "replace", - phases: [ - { - name: "Execution", - tasks: [{ content: "status" }, { content: "diagnostics" }], - }, - ], - }, - ]); + await tool.execute("call-1", { + ops: [ + { + op: "replace", + phases: [ + { + name: "Execution", + tasks: [{ content: "status" }, { content: "diagnostics" }], + }, + ], + }, + ], + }); - const result = await tool.execute("call-2", [{ op: "done", task: "task-1" }]); + const result = await tool.execute("call-2", { ops: [{ op: "done", task: "task-1" }] }); const tasks = result.details?.phases[0]?.tasks ?? []; expect(tasks.map(task => task.status)).toEqual(["completed", "in_progress"]); @@ -65,7 +69,7 @@ describe("TodoWriteTool auto-start behavior", () => { expect(summary.text).toContain("Remaining items (1):"); expect(summary.text).toContain("task-2 diagnostics [in_progress] (Execution)"); - const completedResult = await tool.execute("call-3", [{ op: "done", task: "task-2" }]); + const completedResult = await tool.execute("call-3", { ops: [{ op: "done", task: "task-2" }] }); const completedSummary = completedResult.content.find(part => part.type === "text"); if (!completedSummary || completedSummary.type !== "text") { throw new Error("Expected text summary from todo_write"); @@ -75,42 +79,46 @@ describe("TodoWriteTool auto-start behavior", () => { it("keeps only one in_progress task when replace input contains multiples", async () => { const tool = new TodoWriteTool(createSession()); - const result = await tool.execute("call-1", [ - { - op: "replace", - phases: [ - { - name: "Execution", - tasks: [ - { content: "status", status: "in_progress" }, - { content: "diagnostics", status: "in_progress" }, - ], - }, - ], - }, - ]); + const result = await tool.execute("call-1", { + ops: [ + { + op: "replace", + phases: [ + { + name: "Execution", + tasks: [ + { content: "status", status: "in_progress" }, + { content: "diagnostics", status: "in_progress" }, + ], + }, + ], + }, + ], + }); const tasks = result.details?.phases[0]?.tasks ?? []; expect(tasks.map(task => task.status)).toEqual(["in_progress", "pending"]); }); }); -describe("TodoWriteTool array operations", () => { +describe("TodoWriteTool ops operations", () => { it("jumps to a specific task out of order", async () => { const tool = new TodoWriteTool(createSession()); - await tool.execute("call-1", [ - { - op: "replace", - phases: [ - { - name: "Phase A", - tasks: [{ content: "first" }, { content: "second" }, { content: "third" }], - }, - ], - }, - ]); + await tool.execute("call-1", { + ops: [ + { + op: "replace", + phases: [ + { + name: "Phase A", + tasks: [{ content: "first" }, { content: "second" }, { content: "third" }], + }, + ], + }, + ], + }); - const result = await tool.execute("call-2", [{ op: "start", task: "task-3" }]); + const result = await tool.execute("call-2", { ops: [{ op: "start", task: "task-3" }] }); const tasks = result.details?.phases[0]?.tasks ?? []; expect(tasks.map(task => task.status)).toEqual(["pending", "pending", "in_progress"]); @@ -118,17 +126,19 @@ describe("TodoWriteTool array operations", () => { it("demotes the current in_progress task when starting another", async () => { const tool = new TodoWriteTool(createSession()); - await tool.execute("call-1", [ - { - op: "replace", - phases: [ - { name: "A", tasks: [{ content: "a1" }, { content: "a2" }] }, - { name: "B", tasks: [{ content: "b1" }] }, - ], - }, - ]); + await tool.execute("call-1", { + ops: [ + { + op: "replace", + phases: [ + { name: "A", tasks: [{ content: "a1" }, { content: "a2" }] }, + { name: "B", tasks: [{ content: "b1" }] }, + ], + }, + ], + }); - const result = await tool.execute("call-2", [{ op: "start", task: "task-3" }]); + const result = await tool.execute("call-2", { ops: [{ op: "start", task: "task-3" }] }); const allTasks = result.details?.phases.flatMap(phase => phase.tasks) ?? []; expect(allTasks.map(task => task.status)).toEqual(["pending", "pending", "in_progress"]); @@ -136,15 +146,19 @@ describe("TodoWriteTool array operations", () => { it("appends items to an existing phase", async () => { const tool = new TodoWriteTool(createSession()); - await tool.execute("call-1", [{ op: "replace", phases: [{ name: "Work", tasks: [{ content: "First" }] }] }]); + await tool.execute("call-1", { + ops: [{ op: "replace", phases: [{ name: "Work", tasks: [{ content: "First" }] }] }], + }); - const result = await tool.execute("call-2", [ - { - op: "append", - phase: "phase-1", - items: [{ id: "task-9", label: "Second" }], - }, - ]); + const result = await tool.execute("call-2", { + ops: [ + { + op: "append", + phase: "phase-1", + items: [{ id: "task-9", label: "Second" }], + }, + ], + }); const tasks = result.details?.phases[0]?.tasks ?? []; expect(tasks.map(task => ({ id: task.id, content: task.content, status: task.status }))).toEqual([ @@ -155,15 +169,19 @@ describe("TodoWriteTool array operations", () => { it("creates a phase when append targets a missing phase", async () => { const tool = new TodoWriteTool(createSession()); - await tool.execute("call-1", [{ op: "replace", phases: [{ name: "Work", tasks: [{ content: "First" }] }] }]); + await tool.execute("call-1", { + ops: [{ op: "replace", phases: [{ name: "Work", tasks: [{ content: "First" }] }] }], + }); - const result = await tool.execute("call-2", [ - { - op: "append", - phase: "Cleanup", - items: [{ id: "task-10", label: "Remove dead code" }], - }, - ]); + const result = await tool.execute("call-2", { + ops: [ + { + op: "append", + phase: "Cleanup", + items: [{ id: "task-10", label: "Remove dead code" }], + }, + ], + }); expect(result.details?.phases.map(phase => ({ id: phase.id, name: phase.name }))).toEqual([ { id: "phase-1", name: "Work" }, @@ -174,31 +192,35 @@ describe("TodoWriteTool array operations", () => { it("marks all tasks in a phase done", async () => { const tool = new TodoWriteTool(createSession()); - await tool.execute("call-1", [ - { - op: "replace", - phases: [ - { name: "Work", tasks: [{ content: "First" }, { content: "Second" }] }, - { name: "Later", tasks: [{ content: "Third" }] }, - ], - }, - ]); + await tool.execute("call-1", { + ops: [ + { + op: "replace", + phases: [ + { name: "Work", tasks: [{ content: "First" }, { content: "Second" }] }, + { name: "Later", tasks: [{ content: "Third" }] }, + ], + }, + ], + }); - const result = await tool.execute("call-2", [{ op: "done", phase: "phase-1" }]); + const result = await tool.execute("call-2", { ops: [{ op: "done", phase: "phase-1" }] }); const allTasks = result.details?.phases.flatMap(phase => phase.tasks) ?? []; expect(allTasks.map(task => task.status)).toEqual(["completed", "completed", "in_progress"]); }); it("removes all tasks when rm omits task and phase", async () => { const tool = new TodoWriteTool(createSession()); - await tool.execute("call-1", [ - { - op: "replace", - phases: [{ name: "Work", tasks: [{ content: "First" }, { content: "Second" }] }], - }, - ]); + await tool.execute("call-1", { + ops: [ + { + op: "replace", + phases: [{ name: "Work", tasks: [{ content: "First" }, { content: "Second" }] }], + }, + ], + }); - const result = await tool.execute("call-2", [{ op: "rm" }]); + const result = await tool.execute("call-2", { ops: [{ op: "rm" }] }); expect(result.details?.phases[0]?.tasks).toEqual([]); const summary = result.content.find(part => part.type === "text"); if (!summary || summary.type !== "text") throw new Error("Expected text summary"); @@ -207,14 +229,16 @@ describe("TodoWriteTool array operations", () => { it("drops all tasks in a phase", async () => { const tool = new TodoWriteTool(createSession()); - await tool.execute("call-1", [ - { - op: "replace", - phases: [{ name: "Work", tasks: [{ content: "First" }, { content: "Second" }] }], - }, - ]); + await tool.execute("call-1", { + ops: [ + { + op: "replace", + phases: [{ name: "Work", tasks: [{ content: "First" }, { content: "Second" }] }], + }, + ], + }); - const result = await tool.execute("call-2", [{ op: "drop", phase: "phase-1" }]); + const result = await tool.execute("call-2", { ops: [{ op: "drop", phase: "phase-1" }] }); const tasks = result.details?.phases[0]?.tasks ?? []; expect(tasks.map(task => task.status)).toEqual(["abandoned", "abandoned"]); });