-
Notifications
You must be signed in to change notification settings - Fork 2.9k
feat: rewind-on-interrupt — clean conversation state on abort/resume #11284
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
- Add rewindToLastAskPoint() to truncate clineMessages and sanitize API histories - Add two-phase completion flow (completion_result → resume_completed_task) - Extract normalizeContentBlocks() utility to src/shared/messages.ts - Replace 'as' casts with type predicates and 'satisfies' checks - Remove synthetic 'interrupted' tool_result injection - Neutral fallback message in validateToolResultIds - Comprehensive test coverage: 65 tests across 5 files - Shared test fixtures in rewind-test-setup.ts
All previously flagged issues have been addressed. No new issues found in the latest changes.
Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues. |
| const toolUse = content.find( | ||
| (block): block is Anthropic.Messages.ToolUseBlockParam => | ||
| block.type === "tool_use" && block.name === "attempt_completion", | ||
| ) | ||
|
|
||
| return toolUse?.id |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the last assistant message contains attempt_completion alongside other tool_use blocks (e.g., read_file + attempt_completion) and the process crashes before any tool_result is saved, this method returns the attempt_completion ID, which causes resumeTaskFromHistory to bypass the rewind and treat the task as completed. The other tool_uses never executed, so validateAndFixToolResultIds patches in synthetic "No result available" fallbacks, producing misleading context for the model. This is unlikely in practice (models rarely combine attempt_completion with other tools), but the guard is straightforward:
| const toolUse = content.find( | |
| (block): block is Anthropic.Messages.ToolUseBlockParam => | |
| block.type === "tool_use" && block.name === "attempt_completion", | |
| ) | |
| return toolUse?.id | |
| const toolUse = content.find( | |
| (block): block is Anthropic.Messages.ToolUseBlockParam => | |
| block.type === "tool_use" && block.name === "attempt_completion", | |
| ) | |
| // Only treat as pending completion if there are no other unanswered | |
| // tool_use blocks — otherwise the rewind path is more appropriate. | |
| if (toolUse) { | |
| const hasOtherToolUse = content.some( | |
| (block) => block.type === "tool_use" && block !== toolUse, | |
| ) | |
| if (hasOtherToolUse) return undefined | |
| } | |
| return toolUse?.id |
Fix it with Roo Code or mention @roomote and request a fix.
…e messages When the last assistant message contains attempt_completion alongside other tool_use blocks (e.g. read_file + attempt_completion) and the process crashes before any tool_result is saved, the method previously returned the attempt_completion ID. This caused resumeTaskFromHistory to bypass the rewind and treat the task as completed, while the other tool_uses never executed. Now returns undefined when other tool_use blocks are present alongside attempt_completion, allowing the rewind path to handle the cleanup correctly. Addresses review feedback from @roomote.
Summary
When a user interrupts a task (abort or close VS Code) and later resumes it, the conversation history could contain corrupted state: orphaned tool_use blocks without matching tool_results, synthetic "interrupted" messages that confused the LLM, and attempt_completion calls that never received a response. This PR introduces a rewind mechanism that truncates conversation state to the last valid checkpoint on both abort and resume, ensuring the LLM always sees a clean, API-compliant conversation.
Changes
Core Implementation
rewindToLastAskPoint()inTask.ts— truncatesclineMessagesto the last qualifying ask point, then sanitizes trailing API history (removes orphaned assistant tool_use blocks, incomplete user tool_result messages)AttemptCompletionTool.ts—completion_resultask followed byresume_completed_taskask holds execution context alive, ensuringattempt_completionalways gets atool_resultgetPendingAttemptCompletionToolUseId()inTask.ts— detects pending attempt_completion tool_use for routing follow-up text as tool_result on resumevalidateToolResultIds.ts— "No result available for this tool call." replaces the old "Tool execution was interrupted" synthetic messageCode Quality
normalizeContentBlocks()tosrc/shared/messages.ts— replaces 5 inlineArray.isArray(content)patternsascasts onArray.filter()/Array.find()resultssatisfieschecks replace 2as Anthropic.ToolResultBlockParamcasts on conforming object literals!non-null assertions — direct truthiness checks onpendingAttemptCompletionToolUseIdlet TypeScript narrow naturallyTests (65 total)
rewindToLastAskPoint()covering truncation, ask-type filtering, API history cleanup, error handling, idempotencyAttemptCompletionToolcovering accept, feedback, and abort pathsnormalizeContentBlocks()overwriteClineMessages/overwriteApiConversationHistoryreceive correct truncated datarewind-test-setup.tsreduce ~110 lines of duplicated mock setupImportant
Introduces a rewind mechanism to clean conversation state on task abort/resume, ensuring consistent conversation history by truncating to the last valid checkpoint.
rewindToLastAskPoint()inTask.tstruncatesclineMessagesto the last valid ask point, sanitizing API history by removing orphaned tool_use blocks and incomplete tool_result messages.AttemptCompletionTool.tsensuresattempt_completionalways gets atool_resultby askingcompletion_resultfollowed byresume_completed_task.getPendingAttemptCompletionToolUseId()inTask.tsdetects pendingattempt_completiontool_use for routing follow-up text as tool_result on resume.validateToolResultIds.tsreplaces synthetic "interrupted" messages with "No result available for this tool call."normalizeContentBlocks()tomessages.tsto replace inlineArray.isArray(content)patterns.ascasts onArray.filter()/Array.find()results.satisfieschecks replaceas Anthropic.ToolResultBlockParamcasts on conforming object literals.!non-null assertions by using direct truthiness checks.rewindToLastAskPoint()covering truncation, ask-type filtering, API history cleanup, error handling, idempotency.AttemptCompletionToolcovering accept, feedback, and abort paths.normalizeContentBlocks().overwriteClineMessages/overwriteApiConversationHistoryreceive correct truncated data.rewind-test-setup.tsreduce ~110 lines of duplicated mock setup.This description was created by
for 1f04bab. You can customize this summary. It will automatically update as commits are pushed.