Split out of #284 review round 6, finding 2. Not introduced by that PR — it only added one more way to reach it, which is fixed there (86844f6).
The window
WorktreeFlow.integrate_unit calls merge_local(task, unit) for a DONE task. The merge lands and task.unit_merged is persisted, then a tail runs: a unit-merged journal append, the post_merge plugin emit, and the worktree teardown. Any of those can raise — a full disk on the journal write, a plugin raising — and the raise escapes run_unit, _run_story and _loop into run()'s crash handler.
Engine._after_story(task) never fires. On resume, _finish_inflight skips terminal tasks by design, and _after_story is only called inside that loop. In stories mode _after_story is the done_checkpoint pause, so:
- a mandatory human review is skipped, permanently — no later pass re-offers it
- the next story dispatches on a resume as if the checkpoint had been honored
The run is visibly crashed, so this is not silent to an operator watching. But the checkpoint is silently gone: nothing in the journal says it was owed and skipped.
Why it isn't closed in #284
The current design relies on "terminal ⇒ _after_story already ran", which holds for every path except a crash inside integrate_unit's tail. Closing it properly needs a durable "post-story hook still owed" marker plus a resume pass that runs and discharges it for terminal tasks — real state and a real lifecycle change, disproportionate inside a PR about a ledger annotation. #284 instead made its own new callsite non-fatal, which removes the raiser it added without touching the general case.
Sketch
StoryTask.after_story_done: bool, set (and saved) when _after_story completes or raises RunPaused — the pause is the checkpoint being presented, so resuming past it is correct._finish_inflight runs _after_story for terminal DONE tasks where the flag is unset, after _reconcile_pending_deferred_closes.- Regression: crash inside
merge_local's tail (e.g. a raising post_merge plugin) on a story with done_checkpoint: true, then resume — the checkpoint must still pause, and story 2 must not dispatch.
Worth checking at the same time whether any other _after_story-shaped bookkeeping has the same terminal-task blind spot.
Split out of #284 review round 6, finding 2. Not introduced by that PR — it only added one more way to reach it, which is fixed there (
86844f6).The window
WorktreeFlow.integrate_unitcallsmerge_local(task, unit)for a DONE task. The merge lands andtask.unit_mergedis persisted, then a tail runs: aunit-mergedjournal append, thepost_mergeplugin emit, and the worktree teardown. Any of those can raise — a full disk on the journal write, a plugin raising — and the raise escapesrun_unit,_run_storyand_loopintorun()'s crash handler.Engine._after_story(task)never fires. On resume,_finish_inflightskips terminal tasks by design, and_after_storyis only called inside that loop. In stories mode_after_storyis thedone_checkpointpause, so:The run is visibly crashed, so this is not silent to an operator watching. But the checkpoint is silently gone: nothing in the journal says it was owed and skipped.
Why it isn't closed in #284
The current design relies on "terminal ⇒
_after_storyalready ran", which holds for every path except a crash insideintegrate_unit's tail. Closing it properly needs a durable "post-story hook still owed" marker plus a resume pass that runs and discharges it for terminal tasks — real state and a real lifecycle change, disproportionate inside a PR about a ledger annotation. #284 instead made its own new callsite non-fatal, which removes the raiser it added without touching the general case.Sketch
StoryTask.after_story_done: bool, set (and saved) when_after_storycompletes or raisesRunPaused— the pause is the checkpoint being presented, so resuming past it is correct._finish_inflightruns_after_storyfor terminal DONE tasks where the flag is unset, after_reconcile_pending_deferred_closes.merge_local's tail (e.g. a raisingpost_mergeplugin) on a story withdone_checkpoint: true, then resume — the checkpoint must still pause, and story 2 must not dispatch.Worth checking at the same time whether any other
_after_story-shaped bookkeeping has the same terminal-task blind spot.