Fix task cancellation bugs with orphaned blocks and panics #61
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit fixes several bugs that could occur when cancelling a task: 1. **Effect queue not cleared on cancel**: Pending effects remained in the queue after cancel, which could lead to effects being processed after the turn was already finished. 2. **Stage not cleared on cancel**: Blocks that were in the Stage awaiting approval remained orphaned after cancel. 3. **Tool executor not cancelled**: The cancel() function only cancelled the agent's streaming, not the tool executor's active pipelines. 4. **Panic in start_block after finish_turn**: If decide_pending_tool was called after the turn was finished (e.g., due to race conditions), start_block would panic with "No active turn". Changes: - Add EffectQueue::clear() method to drop all pending effects - Add Transcript::has_active_turn() method to check turn state - Add Transcript::try_start_block() method that returns false instead of panicking when no turn is active - Update App::cancel() to: - Cancel the tool executor - Clear the effect queue - Drain orphaned blocks from the stage - Update decide_pending_tool() to use try_start_block() instead of start_block() for defensive safety - Add comprehensive tests for cancellation scenarios https://claude.ai/code/session_0198d69SpWL6JAZo8nu3fpMN
When a user presses Esc to cancel a running tool, the shell process is now properly killed instead of being left running in the background. Changes: - Add CancellationToken and CancellationSource types to pipeline.rs for cooperative cancellation between the executor and handlers - Update EffectHandler trait to accept CancellationToken parameter - Update all handler implementations to accept the token (most ignore it) - Update Shell handler to pass the token to execute_shell - Update execute_shell in io.rs to use tokio::select! to monitor both the child process and the cancellation token, killing the process when cancellation is requested - Update ToolExecutor to create cancellation sources for each pipeline and trigger them in cancel() - Update README.md with new handler signature The cancellation flow: 1. User presses Esc → App::cancel() is called 2. App calls tool_executor.cancel() 3. Executor triggers cancel_source.cancel() for all active pipelines 4. Running handlers receive the signal via their CancellationToken 5. Shell handler's execute_shell uses select! to detect cancellation 6. Child process is killed with child.kill() https://claude.ai/code/session_0198d69SpWL6JAZo8nu3fpMN
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit fixes several bugs that could occur when cancelling a task:
Effect queue not cleared on cancel: Pending effects remained in the
queue after cancel, which could lead to effects being processed after
the turn was already finished.
Stage not cleared on cancel: Blocks that were in the Stage awaiting
approval remained orphaned after cancel.
Tool executor not cancelled: The cancel() function only cancelled
the agent's streaming, not the tool executor's active pipelines.
Panic in start_block after finish_turn: If decide_pending_tool was
called after the turn was finished (e.g., due to race conditions),
start_block would panic with "No active turn".
Changes:
panicking when no turn is active
start_block() for defensive safety
https://claude.ai/code/session_0198d69SpWL6JAZo8nu3fpMN