-
Notifications
You must be signed in to change notification settings - Fork 2.5k
fix: resume parent task when subtask gets interrupted #7781
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
Conversation
- Add handleInterruptedSubtask method to properly handle interrupted subtasks - Check and unpause parent tasks in resumeTaskFromHistory when they were waiting for an interrupted subtask - Add proper logging and conversation history updates when subtask is interrupted Fixes #7780
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.
Reviewing my own code is like debugging in production - technically possible but morally questionable.
| * Handle the case when a subtask was interrupted (e.g., due to 5-hour limit) | ||
| * and the parent task needs to be resumed without the subtask result. | ||
| */ | ||
| public async handleInterruptedSubtask() { |
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.
I notice this method appears to be duplicated in the diff. Is this intentional, or did the same method get added twice? We should have only one definition of handleInterruptedSubtask().
| * Handle the case when a subtask was interrupted (e.g., due to 5-hour limit) | ||
| * and the parent task needs to be resumed without the subtask result. | ||
| */ | ||
| public async handleInterruptedSubtask() { |
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.
This critical functionality lacks test coverage. Could we add unit tests to verify that:
- The method correctly clears the paused state
- The pause interval is properly cleared
- The correct events are emitted
- The conversation history is updated appropriately?
|
|
||
| // Check if this task was paused waiting for a subtask that got interrupted | ||
| // If so, handle the interrupted subtask properly | ||
| if (this.isPaused && this.childTaskId) { |
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.
Could there be a race condition here? What happens if the child task completes between checking this.isPaused && this.childTaskId and calling handleInterruptedSubtask()? Should we consider using a lock or atomic operation?
|
|
||
| // Add a message to the conversation history indicating the subtask was interrupted | ||
| try { | ||
| await this.say("subtask_result", `[Subtask ${interruptedChildId} was interrupted and could not complete]`) |
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.
Consider extracting this message format to a constant or configuration for better maintainability. We might want to reuse this format elsewhere or make it configurable.
| // Set skipPrevResponseIdOnce to ensure continuity | ||
| this.skipPrevResponseIdOnce = true | ||
| } catch (error) { | ||
| provider?.log(`Error handling interrupted subtask: ${error}`) |
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.
The error is only logged but not propagated. Is silent failure the intended behavior here? Should we consider rethrowing the error or at least emitting an error event?
Description
This PR fixes an issue where parent tasks would not resume after a subtask gets interrupted (e.g., due to Claude Code's 5-hour limit).
Problem
When a subtask gets interrupted, the parent task remains in a paused state (
isPaused = true) and never resumes, leaving the task stuck indefinitely.Solution
handleInterruptedSubtask()method to properly handle interrupted subtasksresumeTaskFromHistory()to check if a task was paused waiting for an interrupted subtaskChanges
resumeTaskFromHistory()to detect and handle interrupted subtaskshandleInterruptedSubtask()method that:Testing
Fixes #7780
Important
Fixes issue in
Task.tswhere parent tasks remain paused if a subtask is interrupted by addinghandleInterruptedSubtask()and modifying task resumption logic.Task.ts.handleInterruptedSubtask()to clear paused state, remove child task reference, and update conversation history.resumeTaskFromHistory()to check for interrupted subtasks and resume parent task.handleInterruptedSubtask().This description was created by
for b6005bf. You can customize this summary. It will automatically update as commits are pushed.