diff --git a/README.md b/README.md index 49de699..66f1a90 100644 --- a/README.md +++ b/README.md @@ -8,7 +8,7 @@ Hierarchical, session-local task tracking for [Pi](https://pi.dev), with atomic ## Features -- Five focused tools: `set_tasks`, `update_task`, `rm_task`, `list_task`, and `get_task` +- Five focused tools: `set_tasks`, `patch_tasks`, `rm_task`, `list_task`, and `get_task` - Parent/child task trees with generated integer IDs and atomic append/replace modes - Atomic batch mutations: every requested change succeeds or none of it is applied - Bounded task input and widget output to keep sessions and terminals responsive @@ -57,7 +57,7 @@ After automatic or manual context compaction, the extension sends one short hidd | Tool | Parameters | Behavior | | --- | --- | --- | | `set_tasks` | `{ mode, tasks, force }` | `append` extends the tree with `force: false`; `replace` atomically installs a new tree or clears it with `[]`. Replacing unfinished work requires `force: true`. | -| `update_task` | `{ tasks: [...] }` | Atomically patches multiple tasks. Use `null` for unchanged text/status, `parentId: -1` for unchanged or `0` to move to root, and `position: -1` to keep the current order. | +| `patch_tasks` | `{ tasks: [...] }` | Atomically patches multiple tasks. Use `null` for unchanged text/status, `parentId: -1` for unchanged or `0` to move to root, and `position: -1` to keep the current order. Completed trees may auto-clear; do not recreate cleared completed tasks. | | `rm_task` | `{ ids: [...], reason }` | Removes only mistaken, duplicate, or canceled tasks. Removing a parent cascades through its descendants. | | `list_task` | `{}` | Returns the full tree as compact `id/status/name` lines. | | `get_task` | `{ ids: [...] }` | Returns complete task details, including parent/child IDs and timing for tasks that have started. | diff --git a/src/prompt.ts b/src/prompt.ts index 5d6393b..524ebda 100644 --- a/src/prompt.ts +++ b/src/prompt.ts @@ -7,7 +7,7 @@ export const COMPACTION_REMINDER = export const TASK_PROMPT_GUIDELINES = [ "For work with 3+ distinct steps, explore enough to understand the scope, then call set_tasks before implementation; otherwise skip tracking. Start one task in-progress.", - "Use update_task when tasks start, block, or finish; keep statuses accurate before the final response.", + "Use patch_tasks when tasks start, block, or finish; keep statuses accurate before the final response.", ]; export function formatCompactTaskTree(tasks: Task[]): string { diff --git a/src/state.ts b/src/state.ts index b7640da..d3a7c61 100644 --- a/src/state.ts +++ b/src/state.ts @@ -20,7 +20,7 @@ import type { } from "./types.ts"; import { TaskValidationError } from "./types.ts"; -const MUTATION_TOOLS = new Set(["add_task", "set_tasks", "update_task", "rm_task"]); +const MUTATION_TOOLS = new Set(["add_task", "set_tasks", "update_task", "patch_tasks", "rm_task"]); export const TASK_STATE_ENTRY_TYPE = "pi-todo-herdr-state"; const VALID_STATUSES = new Set(["pending", "in-progress", "blocked", "done"]); const MAX_DATE_MS = 8_640_000_000_000_000; diff --git a/src/tools.ts b/src/tools.ts index 2e460a9..f71d448 100644 --- a/src/tools.ts +++ b/src/tools.ts @@ -169,7 +169,7 @@ const TaskPatchSchema = Type.Object( { additionalProperties: false }, ); -export const UpdateTaskParams = Type.Object( +export const PatchTasksParams = Type.Object( { tasks: Type.Array(TaskPatchSchema, { minItems: 1, @@ -255,9 +255,9 @@ function prepareTaskPatch(value: unknown): unknown { }; } -function prepareUpdateTaskArguments(value: unknown): Static { +function preparePatchTasksArguments(value: unknown): Static { if (!value || typeof value !== "object" || Array.isArray(value)) { - return value as Static; + return value as Static; } const input = value as Record; return { @@ -265,7 +265,7 @@ function prepareUpdateTaskArguments(value: unknown): Static; + } as Static; } interface SetTaskInput { @@ -274,7 +274,7 @@ interface SetTaskInput { force?: boolean; } -interface UpdateTaskInput { +interface PatchTasksInput { tasks: TaskPatch[]; } @@ -361,14 +361,19 @@ export function registerTaskTools(pi: ExtensionAPI, state: TaskState, onChange: }); pi.registerTool({ - name: "update_task", - label: "Update Tasks", + name: "patch_tasks", + label: "Patch Tasks", description: - "Atomically patch tasks. Use null for unchanged text/status, parentId -1 for unchanged or 0 for root, and position -1 for unchanged.", - parameters: UpdateTaskParams, - prepareArguments: prepareUpdateTaskArguments, - async execute(_toolCallId, params: UpdateTaskInput, _signal, _onUpdate, ctx) { + "Atomically patch tasks. Use null for unchanged text/status, parentId -1 for unchanged or 0 for root, and position -1 for unchanged. Completed task trees may auto-clear; do not recreate cleared completed tasks.", + parameters: PatchTasksParams, + prepareArguments: preparePatchTasksArguments, + async execute(_toolCallId, params: PatchTasksInput, _signal, _onUpdate, ctx) { try { + if (state.list().length === 0) { + throw new TaskValidationError([ + "task tree is empty and may have auto-cleared after completion; do not recreate completed tasks. Use set_tasks only for genuinely new or remaining work", + ]); + } const affected = state.update(params.tasks); notifyChange(onChange, ctx); return { @@ -376,11 +381,11 @@ export function registerTaskTools(pi: ExtensionAPI, state: TaskState, onChange: details: details("update", state, affected), }; } catch (error) { - throwToolError("Update", error); + throwToolError("Patch", error); } }, - renderCall(args: UpdateTaskInput, theme: Theme) { - return renderCall("Update Tasks", args.tasks?.length, theme); + renderCall(args: PatchTasksInput, theme: Theme) { + return renderCall("Patch Tasks", args.tasks?.length, theme); }, renderResult(result, { expanded }, theme) { return renderToolResult(result, expanded, theme); diff --git a/tests/prompt.test.ts b/tests/prompt.test.ts index 6584063..9301a73 100644 --- a/tests/prompt.test.ts +++ b/tests/prompt.test.ts @@ -100,7 +100,7 @@ test("keeps task guidance short with planning after exploration", () => { assert.match(prompt, /then call set_tasks before implementation/); assert.match(prompt, /otherwise skip tracking/); assert.match(prompt, /Start one task in-progress/); - assert.match(prompt, /update_task when tasks start, block, or finish/); + assert.match(prompt, /patch_tasks when tasks start, block, or finish/); assert.match(prompt, /keep statuses accurate before the final response/); assert.ok(prompt.split(/\s+/).length < 40); }); @@ -179,8 +179,8 @@ test("session_compact sends one hidden reminder only for unfinished tasks", asyn }, ]); - await tools.get("update_task").execute( - "update", + await tools.get("patch_tasks").execute( + "patch", { tasks: [{ id: 1, status: "done" }] }, undefined, undefined, diff --git a/tests/state.test.ts b/tests/state.test.ts index e282396..26cdfbe 100644 --- a/tests/state.test.ts +++ b/tests/state.test.ts @@ -353,7 +353,7 @@ test("loads the latest valid mutation snapshot from a session branch", () => { type: "message", message: { role: "toolResult", - toolName: index === 0 ? "add_task" : "update_task", + toolName: index === 0 ? "add_task" : "patch_tasks", details: { action: "add", snapshot, affected: [] }, }, })); @@ -362,6 +362,25 @@ test("loads the latest valid mutation snapshot from a session branch", () => { assert.deepEqual(state.list().map((task) => task.name), ["First", "Second"]); }); +test("loads legacy update_task mutation snapshots", () => { + const completed = new TaskState(); + completed.add([{ name: "Legacy completed task", status: "done" }]); + const branch = [ + { + type: "message", + message: { + role: "toolResult", + toolName: "update_task", + details: { action: "update", snapshot: completed.snapshot(), affected: [] }, + }, + }, + ]; + + const state = new TaskState(); + state.loadFromSession({ sessionManager: { getBranch: () => branch } } as never); + assert.deepEqual(state.list().map((task) => task.name), ["Legacy completed task"]); +}); + test("loads a persisted automatic clear after task tool snapshots", () => { const completed = new TaskState(); completed.add([{ name: "Completed", status: "done" }]); diff --git a/tests/tools.test.ts b/tests/tools.test.ts index ede2c41..f3eb7e0 100644 --- a/tests/tools.test.ts +++ b/tests/tools.test.ts @@ -28,12 +28,13 @@ async function execute(tool: any, params: unknown) { test("registers five focused tools without the old add_task alias", () => { const { tools } = registeredTools(); - assert.deepEqual([...tools.keys()], ["set_tasks", "update_task", "rm_task", "list_task", "get_task"]); + assert.deepEqual([...tools.keys()], ["set_tasks", "patch_tasks", "rm_task", "list_task", "get_task"]); assert.deepEqual(Object.keys(tools.get("set_tasks").parameters.properties), ["mode", "tasks", "force"]); - assert.deepEqual(Object.keys(tools.get("update_task").parameters.properties), ["tasks"]); + assert.deepEqual(Object.keys(tools.get("patch_tasks").parameters.properties), ["tasks"]); assert.deepEqual(Object.keys(tools.get("rm_task").parameters.properties), ["ids", "reason"]); assert.deepEqual(Object.keys(tools.get("list_task").parameters.properties), []); assert.deepEqual(Object.keys(tools.get("get_task").parameters.properties), ["ids"]); + assert.match(tools.get("patch_tasks").description, /do not recreate cleared completed tasks/); }); test("set_tasks schema hoists $defs to the root with #/$defs/ refs", () => { @@ -57,7 +58,7 @@ test("task schemas publish text and batch limits", () => { const { tools } = registeredTools(); const setParams = tools.get("set_tasks").parameters; const addTask = setParams.$defs.AddTask; - const updateTask = tools.get("update_task").parameters.properties.tasks.items; + const updateTask = tools.get("patch_tasks").parameters.properties.tasks.items; assert.equal(setParams.properties.tasks.maxItems, 50); assert.equal(addTask.properties.children.maxItems, 50); @@ -65,7 +66,7 @@ test("task schemas publish text and batch limits", () => { assert.match(addTask.properties.name.pattern, /061C/i); assert.equal(addTask.properties.description.anyOf[0].maxLength, 4_000); assert.match(addTask.properties.description.anyOf[0].pattern, /061C/i); - assert.equal(tools.get("update_task").parameters.properties.tasks.maxItems, 50); + assert.equal(tools.get("patch_tasks").parameters.properties.tasks.maxItems, 50); assert.equal(updateTask.properties.name.anyOf[0].maxLength, 200); assert.equal(updateTask.properties.description.anyOf[0].maxLength, 4_000); assert.equal(tools.get("rm_task").parameters.properties.ids.maxItems, 50); @@ -108,7 +109,7 @@ test("set_tasks preparation bounds hostile recursive input before validation", ( /validation failed/i, ); - const updateTool = tools.get("update_task"); + const updateTool = tools.get("patch_tasks"); const oversizedUpdate = updateTool.prepareArguments({ tasks: Array.from({ length: 1_000 }, (_, index) => ({ id: index + 1 })), }); @@ -122,7 +123,7 @@ test("set_tasks preparation bounds hostile recursive input before validation", ( test("strict task schemas normalize legacy optional arguments", () => { const { tools } = registeredTools(); const setTool = tools.get("set_tasks"); - const updateTool = tools.get("update_task"); + const updateTool = tools.get("patch_tasks"); assert.deepEqual(updateTool.parameters.properties.tasks.items.required, [ "id", @@ -171,7 +172,7 @@ test("strict task schemas normalize legacy optional arguments", () => { assert.equal(updateTool.prepareArguments({ tasks: [{ id: 1, parentId: null }] }).tasks[0].parentId, 0); const validated = validateToolArguments(updateTool, { - name: "update_task", + name: "patch_tasks", arguments: updateTool.prepareArguments({ tasks: [{ id: 1, status: "done" }] }), } as never); assert.equal(validated.tasks[0].parentId, -1); @@ -188,16 +189,27 @@ test("validated update sentinels preserve order and map legacy null parent to ro ], }); - await execute(harness.tools.get("update_task"), { tasks: [{ id: 3, status: "blocked" }] }); + await execute(harness.tools.get("patch_tasks"), { tasks: [{ id: 3, status: "blocked" }] }); assert.deepEqual( harness.state.list().filter((task) => task.parentId === null).map((task) => task.id), [1, 3], ); - await execute(harness.tools.get("update_task"), { tasks: [{ id: 2, parentId: null }] }); + await execute(harness.tools.get("patch_tasks"), { tasks: [{ id: 2, parentId: null }] }); assert.equal(harness.state.get([2])[0]?.parentId, null); }); +test("patch_tasks explains an auto-cleared empty tree without recreating tasks", async () => { + const harness = registeredTools(); + + await assert.rejects( + execute(harness.tools.get("patch_tasks"), { tasks: [{ id: 1, status: "done" }] }), + /Patch failed · 1 validation error\n- task tree is empty and may have auto-cleared after completion; do not recreate completed tasks\. Use set_tasks only for genuinely new or remaining work/, + ); + assert.deepEqual(harness.state.list(), []); + assert.equal(harness.changes(), 0); +}); + test("set_tasks appends optional-description tasks and persists a complete snapshot", async () => { const harness = registeredTools(); const result = await execute(harness.tools.get("set_tasks"), { @@ -247,8 +259,8 @@ test("tool validation errors preserve atomic state", async () => { const before = harness.state.snapshot(); await assert.rejects( - execute(harness.tools.get("update_task"), { tasks: [{ id: 1, parentId: 2 }] }), - /Update failed · 1 validation error\n- task [12]: parentId creates a cycle/, + execute(harness.tools.get("patch_tasks"), { tasks: [{ id: 1, parentId: 2 }] }), + /Patch failed · 1 validation error\n- task [12]: parentId creates a cycle/, ); assert.deepEqual(harness.state.snapshot(), before); assert.equal(harness.changes(), 1); @@ -310,7 +322,7 @@ test("get_task exposes cumulative timing while list output stays compact", async }); now += 568; - const updated = await execute(harness.tools.get("update_task"), { + const updated = await execute(harness.tools.get("patch_tasks"), { tasks: [{ id: 1, status: "blocked" }], }); assert.equal(updated.content[0].text, "Updated 1 task: 1. 0/1 done.");