fix(todo-list): accept 'completed' status and normalize to 'done'#1229
fix(todo-list): accept 'completed' status and normalize to 'done'#1229bj456736 wants to merge 1 commit into
Conversation
Closes MoonshotAI#1016 The LLM sometimes passes 'completed' as the status for TodoList items, but the schema only accepted 'pending' | 'in_progress' | 'done'. This produced two problems: 1. Validation failed when the model used 'completed'. 2. Even if validation passed, statusMarker() had no case for 'completed' and fell through to the unreachable default branch. Changes: - Extend TodoStatus union to include 'completed' so it is accepted at the type level. - Map 'completed' -> 'done' in setTodos() so persisted state stays clean. - Handle 'completed' in statusMarker() so it renders as '[done]'. - Update the markdown description to explicitly warn against using 'completed'. - Add a test confirming 'completed' is accepted and mapped to 'done'.
|
commit: |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8d4dd8bed3
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| title: z.string().min(1).describe('Short, actionable title for the todo.'), | ||
| status: z.enum(['pending', 'in_progress', 'done']).describe('Current status of the todo.'), | ||
| status: z | ||
| .preprocess((val) => (val === 'completed' ? 'done' : val), z.enum(['pending', 'in_progress', 'done'])) |
There was a problem hiding this comment.
In the live tool-call path, tool-call.ts only compiles tool.parameters with AJV and passes the original parsedArgs.data into resolveExecution, so this zod preprocess is not run during execution. Because toInputJsonSchema renders the input side of a preprocess as an unconstrained transform input, the generated JSON Schema drops the pending | in_progress | done enum for status; calls such as { todos: [{ title: 'x', status: 'wip' }] } can now validate and be persisted/rendered instead of being rejected. The new test calls executeTool directly, so it misses this validator path.
Useful? React with 👍 / 👎.
1 participant
Closes #1016
Problem
The LLM sometimes passes
completedas the status for TodoList items, but the schema only acceptedpending|in_progress|done. This produced two problems:completed.statusMarker()had no case forcompletedand fell through to the unreachable default branch.Changes
TodoStatusunion to includecompletedso it is accepted at the type level.completed→doneinsetTodos()so persisted state stays clean.completedinstatusMarker()so it renders as[done].completed.completedis accepted and mapped todone.Testing
pnpm -C packages/agent-core test tools/todo-list.test.ts✅