diff --git a/coworker/tools/todo.py b/coworker/tools/todo.py index 0bd148dd..f35fb832 100644 --- a/coworker/tools/todo.py +++ b/coworker/tools/todo.py @@ -14,15 +14,20 @@ _STATUSES = {"pending", "in_progress", "done"} # Explicit schema — the array-of-objects shape can't be auto-generated reliably, and # providers reject a bare `list` annotation. Registered via `__coworker_schema__`. +# +# The parameter is `todos`, NOT `items`: a top-level argument key named "items" shadows +# minijinja's `.items()` map method in at least one hosted chat template (Together's +# GLM-5.2, 2026-07-21 — "object is not callable"), 400-ing every request that replays +# the call. Any key name that isn't a minijinja map method is safe; never rename back. _TODO_SCHEMA = { "type": "function", "function": { "name": "todo_write", - "description": "Replace the task list. Provide the full list of items each call.", + "description": "Replace the task list. Provide the full list of todos each call.", "parameters": { "type": "object", "properties": { - "items": { + "todos": { "type": "array", "items": { "type": "object", @@ -37,7 +42,7 @@ _TODO_SCHEMA = { }, } }, - "required": ["items"], + "required": ["todos"], }, }, } @@ -49,11 +54,12 @@ class TodoList: def todo_tools(todo: TodoList) -> list: - def todo_write(items: list) -> dict: - """Replace the task list. Each item is an object with `content` and a `status` + def todo_write(todos: list = None, items: list = None) -> dict: + """Replace the task list. Each todo is an object with `content` and a `status` of pending, in_progress, or done.""" + # `items` stays accepted (models that free-style the old name; queued replays). normalized = [] - for entry in items or []: + for entry in (todos if todos is not None else items) or []: if isinstance(entry, dict): status = entry.get("status", "pending") if status == "completed": # common model alias for our "done" @@ -67,7 +73,7 @@ def todo_tools(todo: TodoList) -> list: else: normalized.append({"content": str(entry), "status": "pending"}) todo.items = normalized - return {"count": len(normalized), "items": normalized} + return {"count": len(normalized), "todos": normalized} wrapped = ai.tool( todo_write, diff --git a/surfaces/gui/src/App.tsx b/surfaces/gui/src/App.tsx index 5f5d5b1b..70357fbe 100644 --- a/surfaces/gui/src/App.tsx +++ b/surfaces/gui/src/App.tsx @@ -554,7 +554,8 @@ export function App() { setStreaming(""); // finalized into items (or empty tool-only turn) break; case "tool_proposed": - if (d.name === "todo_write" && d.arguments?.items) setTodo(normalizeTodos(d.arguments.items)); + if (d.name === "todo_write" && (d.arguments?.todos || d.arguments?.items)) + setTodo(normalizeTodos(d.arguments.todos ?? d.arguments.items)); setItems((p) => [ ...p, { kind: "tool", id: newId(), name: d.name, args: d.arguments, status: "…" }, diff --git a/surfaces/gui/src/components/Transcript.test.tsx b/surfaces/gui/src/components/Transcript.test.tsx index a6ed35d5..5975ca13 100644 --- a/surfaces/gui/src/components/Transcript.test.tsx +++ b/surfaces/gui/src/components/Transcript.test.tsx @@ -195,9 +195,14 @@ describe("humanizeTool", () => { }); it("summarizes todo_write by its single item and status", () => { - const line = humanizeTool("todo_write", { items: [{ content: "Post the digest", status: "in_progress" }] }); + const line = humanizeTool("todo_write", { todos: [{ content: "Post the digest", status: "in_progress" }] }); expect(line.pre).toBe("Updated the plan — "); expect(line.obj).toContain("Post the digest"); expect(line.post).toBe(" → in progress"); }); + + it("still renders pre-rename todo_write histories (legacy `items` key)", () => { + const line = humanizeTool("todo_write", { items: [{ content: "Old plan", status: "pending" }] }); + expect(line.obj).toContain("Old plan"); + }); }); diff --git a/surfaces/gui/src/humanize.ts b/surfaces/gui/src/humanize.ts index eb1832fe..ad4c751c 100644 --- a/surfaces/gui/src/humanize.ts +++ b/surfaces/gui/src/humanize.ts @@ -56,7 +56,9 @@ export function humanizeTool(name: string, args: any): HumanLine { case "git_log": return { pre: "Looked through recent git history" }; case "todo_write": { - const items = Array.isArray(a.items) ? a.items : []; + // `todos` is current; `items` renders histories from before the rename (the old + // key breaks Together's GLM-5.2 chat template — see coworker/tools/todo.py). + const items = Array.isArray(a.todos) ? a.todos : Array.isArray(a.items) ? a.items : []; if (items.length === 1) { const it = items[0] || {}; const status = String(it.status || "").replace(/_/g, " "); diff --git a/tests/test_todo_tool.py b/tests/test_todo_tool.py new file mode 100644 index 00000000..9dae87fb --- /dev/null +++ b/tests/test_todo_tool.py @@ -0,0 +1,34 @@ +"""todo_write's wire contract. + +The parameter is `todos` — a top-level arguments key named "items" shadows minijinja's +`.items()` map method in hosted chat templates (Together GLM-5.2, 2026-07-21) and 400s +every request that replays the call. The old key stays accepted at execution time for +models that free-style it, but must never reappear in the schema. +""" + +from coworker.tools.todo import _TODO_SCHEMA, TodoList, todo_tools + + +def _write(**kwargs): + todo = TodoList() + (spec,) = todo_tools(todo) + return spec(**kwargs), todo + + +def test_schema_param_is_todos_not_items(): + props = _TODO_SCHEMA["function"]["parameters"]["properties"] + assert "todos" in props + assert "items" not in props # regression guard: see module docstring + assert _TODO_SCHEMA["function"]["parameters"]["required"] == ["todos"] + + +def test_todos_key_writes_the_list(): + result, todo = _write(todos=[{"content": "a", "status": "in_progress"}]) + assert todo.items == [{"content": "a", "status": "in_progress"}] + assert result == {"count": 1, "todos": [{"content": "a", "status": "in_progress"}]} + + +def test_legacy_items_key_still_executes(): + result, todo = _write(items=[{"content": "b", "status": "done"}]) + assert todo.items == [{"content": "b", "status": "done"}] + assert result["count"] == 1