Implement structured run events and cooperative cancellation (#29) - #59
Draft
4nass wants to merge 3 commits into
Draft
Implement structured run events and cooperative cancellation (#29)#594nass wants to merge 3 commits into
4nass wants to merge 3 commits into
Conversation
Owner
Author
StatusDelivered: durable versioned lifecycle events, cursor replay, full lifecycle coverage, idempotent cancellation, process-group termination and cleanup. Residual: authenticated REST/SSE consumption and provider-API interruption details remain in #47 and provider adapters. Closes #29 is present in the PR description. |
…ng a stop Three defects in the cancellation half of #29. The event stream half was fine; none of these were visible to tests that only checked the job row. The request was recorded as a failure of the work it cancelled. CancellationRequested subclassed RuntimeError, so the broad handlers that turn a stage's problems into a failed StageResult swallowed it: cancelling mid-stage produced "backend failed: CancellationRequested" and the DAG carried on to the next stage. It is a BaseException now, for the same reason KeyboardInterrupt and asyncio.CancelledError are — an instruction to unwind is not an error some layer handles and continues past. Worktrees survived. run() has no try/finally, and the only cleanup was a block placed after the executor, so a cancellation raised from verify, review or a correction attempt walked straight past it and left the integration worktree, its task worktrees and the branch on disk under a job reporting itself stopped. Cleanup now lives with the `with` block that already owns the run's target-level state, so it fires wherever the unwind starts, and a cancelled stage takes its own worktree with it. The row said `cancelled` while an agent was still holding a subprocess. That is the same lie this queue refused to tell when it declined to cancel a running job at all. A running job is now moved to `cancelling` by whoever asks, and only the worker that actually stopped writes `cancelled`; a run that finished before it noticed resolves to the outcome it really reached. Both regression tests fail on the previous code — one with DID NOT RAISE, the other naming the worktree that survived. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…eady does The contract's stable state vocabulary listed `cancel_requested` before an implementation existed; the state added with the cancellation fix should carry that name rather than invent a second one for the same thing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
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.
Delivered
Validation
Closes #29