smashimo/oh-my-pi
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

refactor(coding-agent/tools): wrapped todo_write parameters in an ops object

Browse Source 
- Changed the todo-write parameter schema from a bare operations array to an object containing an `ops` array.
- Updated request parsing and renderer logic to consume `params.ops` and `args.ops` when processing todo operations.
- Updated session and unit tests to invoke todo_write with wrapped `{ ops: [...] }` payloads.
This commit is contained in:
can1357
2026-04-26 05:11:59 +02:00
parent 5d6f8a7951
commit 652f49442f
5 changed files with 178 additions and 141 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/coding-agent/CHANGELOG.md
+2
View File
@@ -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
packages/coding-agent/src/prompts/tools/todo-write.md
+18 -16
View File
@@ -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.
<protocol>
## 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:
<examples>
# 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"}]}]}`
</examples>
<avoid>
packages/coding-agent/src/tools/todo-write.ts
+20 -13
View File
@@ -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<typeof todoWriteSchema>;
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<typeof todoWriteSchema, TodoWrit
// TUI Renderer
// =============================================================================
type TodoWriteRenderArgs = Array<{
op?: string;
task?: string;
phase?: string;
items?: Array<{ id?: string; label?: string }>;
}>;
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);
packages/coding-agent/test/agent-session-eager-todo.test.ts
+13 -11
View File
@@ -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"),
];
packages/coding-agent/test/tools/todo-write.test.ts
+125 -101
View File
@@ -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"]);
});