Skip to content

harvest carry sweeps operator dirt into its own commit when inflight recovery re-enters _defer #684

Description

@pbean

Summary

_carry_harvested_deferrals commits the main checkout's deferred-work.md through
verify.commit_paths, which runs git add -- :(literal)<ledger> and then a pathspec commit — it
takes whatever the working tree holds at that path, no matter who wrote it. On the merge path the
pre-flight added in #618 keeps that safe. There is one reachable path where no pre-flight runs at
all: _finish_inflight re-entering _defer. An operator's unstaged edit to a tracked ledger is
swept into the run's own commit, the tree is left clean, and nothing surfaces it.

Measured on pbean/merge-preflight-taxonomy at 4bc57629.

The path

An isolated defer never merges — the unit's code is discarded, not integrated — so merge_local,
and therefore clean_incoming_collisions, is never on this path.

  1. Engine._defer's isolated arm calls _carry_harvested_deferrals(task) (engine.py:5814) BEFORE
    advance(task, Phase.DEFERRED) (engine.py:5815). The comment at engine.py:5809-5813 says the
    pre-terminal phase is kept deliberately: "if its git commit fails, ordinary inflight recovery
    must re-enter this decision".
  2. _carry_harvested_deferrals sets task.harvest_carry_commit_pending = True and _save()s
    before appending a novel row, then commits. On a TRACKED ledger
    _harvest_carry_commit_may_degrade is False, so a GitError from the commit RE-RAISES: the
    latch stays True, the phase stays pre-terminal, and task.defer_reason is already persisted.
  3. Next run, _finish_inflight (engine.py:1280) matches isolated and task.defer_reason is not None on a nonterminal task, journals resume-defer, and calls _defer again (engine.py:1298).
  4. The second _defer calls _carry_harvested_deferrals again. The row is already in the ledger,
    so append_entry dedupes to carried == [] — but
    commit_needed = bool(carried) or task.harvest_carry_commit_pending is True from the latch, so
    commit_paths runs with nothing of its own to commit. Every byte it takes is somebody else's.

_replay_unlatched_ledger_carries does not intercept: its Phase.DEFERRED branch requires the
task to already be terminal-DEFERRED, and the advance to DEFERRED never completed.

Measured

Driven run against the sandbox fixture — real worktree, real sessions via the mock adapter, real
budget exhaustion, real crash propagation. The only injection was verify.commit_paths raising
GitError on its FIRST call, which is the documented non-degradable failure for a tracked ledger
(a rejecting pre-commit hook, an index lock, a signing failure all produce it).

Persisted state after run 1's crash:

task.phase                        = Phase.REVIEW_VERIFY
task.defer_reason                 = 'review did not converge within budget ...'
task.harvest_carry_commit_pending = True
task.worktree_path                = '.../worktrees/1-1-a'   (still mounted)

Between the runs, an unstaged marker was appended to the tracked ledger in the MAIN checkout
(git status --porcelain showed M ...deferred-work.md).

Run 2 journal: ... 'run-crash', 'resume-defer', 'harvest-carried', 'story-deferred', 'unit-closed', 'run-complete'. No new session was dispatched — pure state replay.

git log --format='%h %s'
  533ff58 chore(deferred-work): carry harvested findings from 1-1-a
  52c93f3 sprint
  4a9ec0b initial

Oracle — git rev-list HEAD -- <rel> then git show <sha>:<rel> for each:

52c93f3 (pre-run baseline)   marker present = False
533ff58 (the carry commit)   marker present = TRUE

git status --porcelain afterwards: empty. The tree looks pristine.

verify.clean_incoming_collisions call counter for run 2: 0. No merge-target-tolerated,
merge-target-cleaned or unit-merged entry for the story. No pre-flight anywhere on the path.

What is NOT affected (also measured)

  • The DONE fall-through carry leg (engine.py:1273) with a tracked ledger: NOT sweepable. The
    unit's ledger writes rode the merge, so append_entry dedupes and mark_done_many no-ops — all
    three base carries reach carried == [] and none of them commits. Measured: the operator's
    marker reached no commit and the tree was left dirty, with story-deferred-close-carried firing
    on dw_ids: [].
  • The DEFERRED replay leg (engine.py:1214): the sweep mechanism is real there, but the leg is
    unreachable from a driven run. It needs phase == DEFERRED AND the latch AND worktree_path
    simultaneously, and _defer always carries before it flips the phase — so any interruption
    either aborts before the flip or clears the latch before it. Reproducing it required hand-built
    StoryTask state, which is also the only way the existing suite reaches it
    (test_tracked_harvest_carry_commit_failure_retries_its_pending_commit, parametrized over
    Phase.DEFERRED).
  • The sprint board on the replay carry leg: guarded by 4bc57629.

On the fix

The board guard proves ownership by recomputing the intended content from HEAD's blob through
sprintstatus.advance itself and comparing bytes. The same shape looks tractable for this one
writer, and cheaper than it first appears:

  • append_entry mints ids as a pure function of the ledger text (DW_ID_RE scan, max+1) and
    stamps NO date. Measured: replaying a partial append set to completion is byte-identical to
    running it from clean, because the dedupe is open-row-and-provenance based.
  • So intended = HEAD's blob + this run's persisted harvested_deferrals is computable, and a
    crashed pass's row matches it byte for byte while an operator's edit does not.
  • The multi-writer difficulty that makes this hard in general does not arise here: per the section
    above, the composite carry chain does not commit a tracked ledger at all.

What does NOT work, measured: structural discrimination via parse_ledger. Its model is an
id -> (title, body-span) map built from ### DW-<n> headings, with no representation of text
outside an entry span and none of ordering. Free prose before the first heading, and a pure reorder
of two entries, both leave every parsed entry byte-identical while the file's bytes differ — so no
comparison built on that model can see them. A per-id content hash catches four of six plausible
operator edit shapes and is blind to those two.

Note for whoever takes this: the docstrings at engine.py:6091 and verify.py:1791 still describe the
merge pre-flight as preceding the ledger carries. That premise wants the same correction the board
carry's docstring got in 4bc57629.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions