fix(claude): map TodoWrite updates to plan events#1387
fix(claude): map TodoWrite updates to plan events#1387samjandris wants to merge 4 commits intopingdotgg:mainfrom
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
af1bfde to
606fbe6
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| method: "claude/user", | ||
| payload: message, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
TodoWrite tools misclassified as file_change in lifecycle events
Medium Severity
The new code emits turn.plan.updated for todowrite tools, but classifyToolItemType still classifies "todowrite" as "file_change" because the name contains "write" (matching at line 422). This means the item.started and item.completed lifecycle events report itemType: "file_change" and title: "File change" for what is actually a plan/todo tool. A check for "todowrite" returning "plan" needs to be added to classifyToolItemType before the "write" check so the full item lifecycle is consistent with the new plan event.
Additional Locations (1)
There was a problem hiding this comment.
This review is valid. TodoWrite already emits turn.plan.updated, but its lifecycle events are still classified as file_change. So, the work log currently shows both a file_change tool event and a separate Plan updated event for the same action.
I can fix that if desired, though it slightly widens the scope because it affects lifecycle classification and not just event emission. If so, should TodoWrite still appear as its own tool event, or should we only show Plan updated?
- Classify TodoWrite tool calls as plan items - Emit turn.plan.updated events from successful TodoWrite results
- Emit plan updates for TodoWrite tool results regardless of casing - Add coverage for lowercase todowrite tool events
- Treat any tool name containing `todowrite` as a completed todo-write action - Preserve turn state updates for renamed Claude tool variants
- Treat `todowrite` as a normal tool call instead of a plan item - Update the Claude adapter test expectations accordingly
38e14ce to
b4bbd92
Compare
|
Hey @samjandris - I was working on this same thing and found your PR. Nice catch on the core problem. I ran into one concern while testing a similar approach locally: replacing the I also took a slightly different approach on timing. Instead of intercepting at tool result time, I emit the plan event during input streaming (in the I opened an alternative PR in #1541 that takes this approach. Not trying to step on your work, just figured it's worth giving the maintainers both options. Happy to chat about it. |
|
Hey @TimCrooker , I like the approach you took and am excited to properly try it out tonight. Happy to have both PRs available for the maintainers to work off of. |
|
@samjandris I came up with an idea and implemented it in my branch that made it even easier, more seamless, and simpler then my previous implementation. I would really appreciate it if you could test it out locally and tell me if this solves your particular issue as well, because it completely solved mine. I was actually finding myself going back to Claude code instead of T3 code because of the invisibility of tasks, and this completely solved it. Let me know what you think, please, if you get a chance. |


What Changed
Maps successful Claude
TodoWriteresults toturn.plan.updatedevents so Claude todo updates flow through the existing plan event path.Validated with
bun fmt,bun lint, andbun typecheck.Why
Without this mapping, successful Claude
TodoWriteupdates do not flow through the existing plan event path, which can leave plan todos missing.UI Changes
Shows how TodoWrite was previously ignored by the plan sidebar panel vs now.
Before:
After:
Checklist
Note
Medium Risk
Changes the runtime event stream for successful Claude
TodoWritetool results by emittingturn.plan.updated(and skipping prioritem.updated/content.deltaemissions), which may affect downstream consumers that relied on the old item update events.Overview
Successful Claude tool results whose
toolNamecontainstodowrite(case-insensitive) are now converted intoturn.plan.updatedevents, with the plan derived fromtool.input.todosand statuses normalized tocompleted/inProgress/pending.Adds a regression test ensuring lowercase/variant
todowritetool names still produce the expectedturn.plan.updatedpayload andturnIdlinkage.Written by Cursor Bugbot for commit b4bbd92. This will update automatically on new commits. Configure here.
Note
[!NOTE]
Map
TodoWritetool results toturn.plan.updatedevents inClaudeAdapterclassifyToolItemTypein ClaudeAdapter.ts now returns'plan'for any tool whose normalized name includes'todowrite'.todowritetool result is received, the adapter emits aturn.plan.updatedevent built fromtool.input.todos, mapping each item to a{step, status}pair with status normalized tocompleted,inProgress, orpending.item.completedemission is preserved; the prioritem.updated/content.deltasequence is skipped fortodowritetools.todowritetool results no longer emititem.updatedorcontent.deltaevents.Macroscope summarized b4bbd92.