Summary
pending is in neither REHYDRATABLE_RUN_STATUSES nor TERMINAL_RUN_STATUSES. So a run record left in that state is invisible to both halves of the machinery that would deal with it:
- rehydration only adopts
waiting / running / retry_scheduled, so nothing picks it up
workflow cleanup and the store's record-cap eviction both filter on TERMINAL_RUN_STATUSES (store.py:460, :619, :641), so nothing retires it
It leaks, permanently, and LocalWorkflowStore.list_runs() is linear in accumulated records (#380), so leaked ones keep costing.
Reproduction
store = create_workflow_store(backend="local", root=root)
rec = store.create_run(run_id="p1", graph_id="p1", workflow_name="w", execution_kind="workflow")
rec.status -> 'pending'
store.list_rehydratable_runs() -> []
rec.status in TERMINAL_RUN_STATUSES -> False
Reachability
run_workflow calls create_run, then claim_run, then sets the record running and saves. A process that dies inside that window leaves the record at pending. The window is small, which is why this is severity:low rather than higher — but nothing narrows it and nothing recovers from it.
How it surfaced
models.py says of the status vocabulary: "Partitioned deliberately: every status is terminal or not, and the two sets below must stay disjoint. tests/test_run_status_vocabulary.py drives off this tuple, so a ninth status fails the suite until it is classified."
Three things there were untrue, and they held each other up:
tests/test_run_status_vocabulary.py does not exist. The real file is tests/test_run_cancellation.py.
- Its partition test could not fail.
test_every_status_is_terminal_or_not_and_never_both iterated RUN_STATUSES asserting status in set(RUN_STATUSES) — true by construction. The disjointness half was real; the coverage half was a tautology.
- So "a ninth status fails the suite until it is classified" was false. Verified: adding a ninth status classified into neither set fails only the count assertion beside it, which a maintainer resolves by bumping the count — and the suite is then green with the status still unclassified.
Fixing the tautology is what surfaced pending, which has been violating the documented partition all along.
Fix direction
Decide which half pending belongs to, rather than adding a third category:
- terminal — cleanup retires an abandoned
pending run on its normal retention schedule. Simple, and matches the fact that nothing can continue a run that never started.
- rehydratable — adoption picks it up and starts it. Better if a created-but-unstarted run should still run, but adoption currently means "register the graph so someone can carry it on", and there is no graph state for a run that never began.
Terminal looks right, and is the smaller change.
The partition test is corrected in the #733 branch and currently records pending as a known exception pointing here, so the guard is honest in the meantime and a tenth status cannot hide behind it.
Affected versions
v5.9.0 (current), and every version since the vocabulary was consolidated.
Summary
pendingis in neitherREHYDRATABLE_RUN_STATUSESnorTERMINAL_RUN_STATUSES. So a run record left in that state is invisible to both halves of the machinery that would deal with it:waiting/running/retry_scheduled, so nothing picks it upworkflow cleanupand the store's record-cap eviction both filter onTERMINAL_RUN_STATUSES(store.py:460,:619,:641), so nothing retires itIt leaks, permanently, and
LocalWorkflowStore.list_runs()is linear in accumulated records (#380), so leaked ones keep costing.Reproduction
Reachability
run_workflowcallscreate_run, thenclaim_run, then sets the recordrunningand saves. A process that dies inside that window leaves the record atpending. The window is small, which is why this isseverity:lowrather than higher — but nothing narrows it and nothing recovers from it.How it surfaced
models.pysays of the status vocabulary: "Partitioned deliberately: every status is terminal or not, and the two sets below must stay disjoint.tests/test_run_status_vocabulary.pydrives off this tuple, so a ninth status fails the suite until it is classified."Three things there were untrue, and they held each other up:
tests/test_run_status_vocabulary.pydoes not exist. The real file istests/test_run_cancellation.py.test_every_status_is_terminal_or_not_and_never_bothiteratedRUN_STATUSESassertingstatus in set(RUN_STATUSES)— true by construction. The disjointness half was real; the coverage half was a tautology.Fixing the tautology is what surfaced
pending, which has been violating the documented partition all along.Fix direction
Decide which half
pendingbelongs to, rather than adding a third category:pendingrun on its normal retention schedule. Simple, and matches the fact that nothing can continue a run that never started.Terminal looks right, and is the smaller change.
The partition test is corrected in the #733 branch and currently records
pendingas a known exception pointing here, so the guard is honest in the meantime and a tenth status cannot hide behind it.Affected versions
v5.9.0 (current), and every version since the vocabulary was consolidated.