Don't reap executions that are waiting on a child - #867
Merged
Conversation
An execution's updated_at stops moving while it waits for a child to return, so the staleness sweep read 'blocked on a child' as 'stuck'. An agent doing many minutes of work inside a single request had its whole ancestor chain marked timed out mid-flight: the caller was told 'execution timed out (no activity)' while the work was still running and went on to finish. Rows with a non-terminal child are now skipped. There is deliberately no recency test on the child, so the chain still drains when work genuinely stops — the leaf goes stale first, which makes its parent childless and eligible on the next sweep, and so on up. One sweep per level, and nothing is stranded in running. Applies to both the execution and workflow-execution sweeps, which had the same shape. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
📊 Coverage gateThresholds from
✅ Gate passedNo surface regressed past the allowed threshold and the aggregate stayed above the floor. |
Contributor
📐 Patch coverage gateThreshold: 80% on lines this PR touches vs
✅ Patch gate passedEvery surface whose lines were touched by this PR has patch coverage at or above the threshold. |
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.
An execution's
updated_atstops moving while it waits for a child to return, so the staleness sweep read "blocked on a child" as "stuck". Any agent that does many minutes of work inside a single request had its whole ancestor chain marked timed out mid-flight — the caller gotexecution timed out (no activity)while the work was still running, and the agent went on to finish normally against a run already reported as failed.Observed on a real build: the root execution was marked failed at
20:15:46, and the agent doing the work kept emitting progress until20:46and beyond. A comparable build whose work was split across many short reasoner calls succeeded in the same window — each call bumpedupdated_at, so it never looked idle.This is the invariant the code already documents ("callers must ensure updated_at is bumped on every meaningful execution activity") meeting a caller that legitimately cannot bump it, because it is blocked.
Change
Rows with a non-terminal child are skipped by both sweeps (
MarkStaleExecutionsandMarkStaleWorkflowExecutionshad the same shape).parent_execution_idalready exists and is indexed on both tables.There is deliberately no recency test on the child, which is what makes the rule safe for arbitrary nesting depth: the chain drains bottom-up. If work genuinely stops, the leaf goes stale and is reaped first, which makes its parent childless and eligible on the next sweep, and so on up to the root. One sweep per level, and nothing is stranded in
running. Requiring the child to be recently active instead would only protect one level and would still reap grandparents.Tests
internal/storage/stale_execution_parent_test.gocovers:Both new behavioural tests were confirmed to fail with the fix reverted, so they pin the behaviour rather than the implementation.
go build ./...and the full./internal/storage/...suite pass, existing reaper tests included.🤖 Generated with Claude Code