Skip to content

A pause inside _defer permanently skips the story-deferred journal entry and notification #342

Description

@pbean

Split out of #340 / PR #341 review, which widens the reachability but does not cause it.

Current behavior

Engine._defer (engine.py) advances the task before it recovers the tree:

def _defer(self, task: StoryTask, reason: str) -> None:
    task.defer_reason = reason
    advance(task, Phase.DEFERRED)          # <- phase set first
    if self._isolated:
        ...
        return
    if task.baseline_commit:
        ...
        self._rollback_or_pause(task)      # <- can raise RunPaused
        ...
    self.journal.append("story-deferred", ...)   # never runs
    gates.notify(self.policy, self.run_dir, f"story deferred: {task.story_key}", ...)
    self._save()

_rollback_or_pause can raise RunPaused through preserve_attempt_commits
pause_for_manual_recovery when commits exist above baseline and the recovery ref cannot be
created. pause_for_manual_recovery calls self._save() before raising, so Phase.DEFERRED is
persisted — and statemachine.py declares Phase.DEFERRED: frozenset(), i.e. terminal.

Net: the story is durably DEFERRED, but the tail after the rollback never runs and can never run.
The story-deferred journal entry and the defer notification are skipped permanently, not deferred
until resume.

Impact — forensics only, as far as I can tell

Worth stating plainly so this is triaged at the right size:

  • No data loss. The pause leaves the working tree untouched, so the deferred_work snapshot
    captured just above has nothing to restore — the session's ledger edits are still on disk.
  • The operator is not left uninformed. They get the bold ACTION REQUIRED manual-recovery
    notice instead, which is louder than the defer notification would have been.
  • What is actually lost is the story-deferred journal entry (so a post-mortem reading
    journal.jsonl sees the story stop with no defer record) and the _defer_recovery_note pointer
    naming where the attempt survives (Name the attempt-preserve ref in the defer notification, status, and --json #333).

Reachability

Today: in-place defer (isolation="none") with commits above baseline whose attempt-preserve/*
ref fails to create. The isolated arm returns before the rollback, so worktree mode is unaffected.

After #341: a second path, a failed uncommitted-work snapshot over a tree that would still lose
work. Same shape, same consequence.

Possible shapes

  • Journal + notify before _rollback_or_pause, so the defer record exists regardless of what the
    recovery does. Changes the ordering guarantee — the notice would name a preserve_ref the
    rollback has not set yet, so _defer_recovery_note would need to move or be re-emitted.
  • Catch RunPaused around the rollback, emit the defer record, re-raise. Keeps the ordering but
    adds a catch of the control-flow exception, which the codebase otherwise treats as
    unwind-to-the-top.
  • Advance to DEFERRED only after the rollback returns, so a pause leaves the task in its prior
    phase and resume re-enters _defer. Most faithful, but changes what a resumed run sees and
    interacts with _finish_inflight's handling of the pre-defer phase.

No strong preference — the first is smallest, the third is most correct.

Acceptance

A decision plus, if behavior changes, a test at the Engine seam driving a defer whose rollback
pauses, asserting the defer record survives. Ablate it: the test must fail with the fix removed.

Metadata

Metadata

Assignees

No one assigned

    Labels

    area:engineOrchestrator engine and run lifecycle

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions