Skip to content

fix(loops): repair a run claimed but never closed after a mid-advance fault (#3316) - #3334

Merged
vybe merged 1 commit into
devfrom
feature/3316-loop-claim-close-atomic
Oct 7, 2026
Merged

vybe merged 1 commit into
devfrom
feature/3316-loop-claim-close-atomic

Conversation

@trinity-ability

Copy link
Copy Markdown
Contributor

Summary

loop_service.advance_on_terminal claims run N on the loop row, then closes run N's row in a second write. A fault between the two (database is locked, a process kill) left the loop claimed with the run still running. After that, every redelivered terminal and reconcile_after_restart lost the claim CAS, so the loop showed "running" forever.

Fix (detect and repair):

  • A delivery that loses the claim re-reads the loop. If the loop is not finished, runs_completed == N and run N is still running, it closes run N itself and carries on.
  • finalize_loop_run is now a second CAS on status = 'running' and returns bool. Only the caller that wins the close runs the rest of the advance, so a repair racing a live advance still dispatches run N+1 exactly once.
  • No schema change, so no migrations.

Why not one transaction: it would need a new combined DB method, and it would leave the issue's test with no gap to inject the fault into. The repair also covers faults earlier in the close (the execution read, the gate lookup).

Tests

  • test_m56_fault_after_claim_is_recoverable_on_restart: xfail marker removed.
  • New test_m56_repair_that_loses_the_close_does_not_dispatch: a repair beaten to the close does not dispatch.
  • Fake finalize_loop_run in four existing test files updated to the new return contract.
  • Every unit file referencing the loop service or these DB methods: 433 passed, 1 xfailed (test_m61, bug: reconcile_after_restart racing a live loop advance runs the same iteration twice #3317, still strict).
  • Mutation check:
    • whole fix reverted → test_m56_fault… fails;
    • only the status == 'running' condition removed → test_m56_repair… fails.

Docs: feature-flows/run-agent-loop.md describes the repair path.

Fixes #3316

🤖 Generated with Claude Code

… fault (#3316)

advance_on_terminal claims run N (CAS runs_completed N-1 -> N) and then closes
run N's row in a second write. A fault between the two (database is locked, a
process kill) left the loop claimed with the run still `running`, and every
redelivered terminal and reconcile_after_restart then lost the claim CAS, so
the loop showed "running" forever.

A delivery that loses the claim now recognises that state (loop non-terminal,
runs_completed == N, run N still running) and closes the run itself.
finalize_loop_run becomes a second CAS on status='running' and returns bool;
only the caller that wins the close runs the tail, so a repair racing a live
advance still dispatches run N+1 exactly once. No schema change.

Red before the fix: test_m56_fault_after_claim_is_recoverable_on_restart
(xfail marker removed). Red with the close CAS removed:
test_m56_repair_that_loses_the_close_does_not_dispatch.

Fixes #3316

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@vybe vybe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merge-train: batch validated on train/20261007-1609 (#3349)

@vybe
vybe merged commit bec4854 into dev Oct 7, 2026
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants