Skip to content

Ledger snapshot disarm is skipped when _rollback_or_pause raises anything but RunPaused #420

Description

@pbean

Found while closing out PR #406 (the pre-harvest ledger snapshot work, 0a8088f). Recorded rather than fixed there because it is unreachable from the policy every test in that family uses.

What

_dev_phase's non-fixable RETRY leg latches the pause so its finally can tell that leg apart (engine.py:2078-2090):

paused = False
try:
    self._rollback_or_pause(task)
except RunPaused:
    paused = True
    raise
finally:
    ...

The except catches only RunPaused. _rollback_or_pause (engine.py:1140) does not always pause — with scm.rollback_on_failure = on, and unconditionally inside a mounted unit worktree, it performs a git reset --hard to the attempt baseline. That reset can raise verify.GitError, and the preserve-ref parking around it can raise OSError.

On that path the exception is neither RunPaused nor caught here, so:

  1. paused stays False, so the finally reads the leg as a plain rollback rather than a pause; and
  2. the disarm of pre_harvest_ledger / pre_harvest_ledger_captured never runs, while run()'s finally: self._save() (engine.py:420) still persists the task — leaving a still-armed snapshot on disk for a task whose attempt died.

Why it is not already caught

Every test in the ledger-snapshot family runs on the default rollback_on_failure = off, where _rollback_or_pause raises RunPaused by construction and the latch fires correctly. The uncovered combination is rollback_on_failure = on (or worktree isolation, which always auto-recovers) plus a git/IO failure during the reset — two conditions, neither of which any current fixture supplies together.

Severity

Bounded. The residue is state in state.json on a task that is going terminal, and nothing reads pre_harvest_ledger on a terminal task — the restore path gates on pre_harvest_ledger_captured only when a replay reaches the same attempt. The concrete risk is a resume that finds an armed snapshot from an attempt that never completed, and restores ledger text captured before a harvest that may since have been reverted by other means.

Shape of a fix

Either widen the latch to the exception types _rollback_or_pause can actually raise, or — probably better — move the disarm out of the exception-shaped bookkeeping entirely, so it is unconditional on leaving the attempt rather than conditional on which exception left it. Worth checking the same question at the other except RunPaused (engine.py:307).

Whichever shape, it needs a fixture that sets rollback_on_failure = on and makes the reset fail — the ledger family has no such fixture today, which is the actual gap.

Metadata

Metadata

Assignees

No one assigned

    Labels

    area:engineOrchestrator engine and run lifecyclebugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions