From 53af82d539533265a89b2b63d33f09e871dc987e Mon Sep 17 00:00:00 2001 From: can1357 Date: Sat, 27 Jun 2026 00:44:35 +0200 Subject: [PATCH] fix(coding-agent/tools): removed the enforced minimum length for the - Removed the enforced minimum length for the `items` array in the tool schema to prevent unnecessary validation errors on operations that ignore the field. - Added tests to verify schema acceptance of empty `items` arrays and confirmed that runtime errors are still correctly thrown for empty inputs during specific operations like `append`. --- packages/coding-agent/src/tools/todo.ts | 5 +++- packages/coding-agent/test/tools/todo.test.ts | 24 +++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/packages/coding-agent/src/tools/todo.ts b/packages/coding-agent/src/tools/todo.ts index b00489b88..b05329997 100644 --- a/packages/coding-agent/src/tools/todo.ts +++ b/packages/coding-agent/src/tools/todo.ts @@ -57,7 +57,10 @@ const todoSchema = type({ "list?": InitListEntry.array().describe("phased task list (init)"), "task?": type("string").describe("task content"), "phase?": type("string").describe("phase name"), - "items?": type("string").describe("task content").array().atLeastLength(1).describe("tasks to append"), + // No `atLeastLength(1)` here: `items` is only meaningful for `init`/`append`, + // and both enforce non-empty with op-specific errors. A stray `items: []` on + // an op that ignores it (e.g. `view`) must not be a hard schema rejection. + "items?": type("string").describe("task content").array().describe("tasks to append"), }).describe("apply a single todo operation"); type TodoParams = TodoSchema; diff --git a/packages/coding-agent/test/tools/todo.test.ts b/packages/coding-agent/test/tools/todo.test.ts index 2a6232426..01e1aa49f 100644 --- a/packages/coding-agent/test/tools/todo.test.ts +++ b/packages/coding-agent/test/tools/todo.test.ts @@ -15,6 +15,7 @@ import { todoToolRenderer, } from "@oh-my-pi/pi-coding-agent/tools"; import type { Component } from "@oh-my-pi/pi-tui"; +import { type } from "arktype"; function createSession(initialPhases: TodoPhase[] = []): ToolSession { let phases = initialPhases; @@ -284,6 +285,29 @@ describe("TodoTool lenient init shapes", () => { }); }); +describe("TodoTool empty items tolerance", () => { + // Regression: a stray `items: []` on an op that ignores items (here `view`) + // must not be a hard schema rejection. The top-level `items` array dropped + // its `atLeastLength(1)` so callers don't get "items must be tasks to append" + // for an irrelevant empty array; length is enforced per-op at runtime. + it("accepts op:view with an empty items array at the schema boundary", () => { + const schema = new TodoTool(createSession()).parameters; + expect(schema({ op: "view", items: [] }) instanceof type.errors).toBe(false); + }); + + it("defers empty append items to an op-specific runtime error", async () => { + const tool = new TodoTool(createSession()); + await tool.execute("call-1", { op: "init", list: [{ phase: "Work", items: ["First"] }] }); + + const result = await tool.execute("call-2", { op: "append", phase: "Work", items: [] }); + + expect(result.isError).toBe(true); + const summary = result.content.find(part => part.type === "text"); + if (summary?.type !== "text") throw new Error("Expected text summary"); + expect(summary.text).toContain("Missing items for append operation"); + }); +}); + describe("selectStickyTodoWindow", () => { const makeTasks = (statuses: TodoStatus[]): TodoItem[] => statuses.map((status, i) => ({ content: `task-${i + 1}`, status }));