You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Split out of the #284 round-7 review, which recommended tracking it separately. Not introduced by that PR — #284 only made it observable, by adding a unit_merged flag that names the state.
The window
Under scm.isolation = "worktree" a story is committed on its unit branch and only later merged into the target branch by WorktreeFlow.integrate_unit. Engine._finalize_commit_phase advances the task to DONE and calls self._save()before integration runs — the commit is what stamps the phase, not the merge.
A crash in between (src/bmad_loop/engine.py:846-851, 1842-1853) therefore leaves a task that is:
persisted as phase = DONE
carrying a real commit_sha on a unit branch
never merged into the target branch
On resume, _finish_inflight skips terminal tasks by design, and nothing else re-integrates. The committed work sits on the unit branch and is never merged — silently, because from the state file's point of view the story is done.
Why resume cannot currently repair it
_finish_inflight's loop opens with if task.terminal: continue. That is deliberate: terminal tasks are not supposed to need driving. The gap is that "DONE" is stamped by the commit while the durable landing under isolation is the merge, so DONE is not proof the work reached the target branch.
#284 added StoryTask.unit_merged, saved the statement after verify.merge_branch returns, precisely so its own bookkeeping could tell the two apart — and its reconcile pass reports (deferred-close-abandoned) rather than repairs, because repairing is out of that PR's scope. So the discriminator a fix would need already exists and is already persisted.
Impact
A story's work is committed but absent from the target branch, with the run reporting it done.
Any later story branching from the target baseline does not see it.
A resume-time reconcile for DONE-but-unmerged isolated tasks: re-open the unit worktree and re-run integrate_unit, or — if the worktree is gone — merge the recorded commit_sha directly. merge_local is already idempotent-ish across re-drives, and unit_merged gates it so a task that did merge is not merged twice.
Worth checking against the squash strategy, which rewrites the commit, so the unit's sha is not on the target branch under every strategy and ancestry cannot be used as the test after the fact.
Split out of the #284 round-7 review, which recommended tracking it separately. Not introduced by that PR — #284 only made it observable, by adding a
unit_mergedflag that names the state.The window
Under
scm.isolation = "worktree"a story is committed on its unit branch and only later merged into the target branch byWorktreeFlow.integrate_unit.Engine._finalize_commit_phaseadvances the task to DONE and callsself._save()before integration runs — the commit is what stamps the phase, not the merge.A crash in between (
src/bmad_loop/engine.py:846-851,1842-1853) therefore leaves a task that is:phase = DONEcommit_shaon a unit branchOn resume,
_finish_inflightskips terminal tasks by design, and nothing else re-integrates. The committed work sits on the unit branch and is never merged — silently, because from the state file's point of view the story is done.Why resume cannot currently repair it
_finish_inflight's loop opens withif task.terminal: continue. That is deliberate: terminal tasks are not supposed to need driving. The gap is that "DONE" is stamped by the commit while the durable landing under isolation is the merge, so DONE is not proof the work reached the target branch.#284 added
StoryTask.unit_merged, saved the statement afterverify.merge_branchreturns, precisely so its own bookkeeping could tell the two apart — and its reconcile pass reports (deferred-close-abandoned) rather than repairs, because repairing is out of that PR's scope. So the discriminator a fix would need already exists and is already persisted.Impact
open(correct, and feat: auto-resolve deferred-work entries a story declares (closes #234) #284 journalsdeferred-close-abandonednaming them) — but the underlying lost merge is not reported as such.Suggested direction
A resume-time reconcile for DONE-but-unmerged isolated tasks: re-open the unit worktree and re-run
integrate_unit, or — if the worktree is gone — merge the recordedcommit_shadirectly.merge_localis already idempotent-ish across re-drives, andunit_mergedgates it so a task that did merge is not merged twice.Worth checking against the
squashstrategy, which rewrites the commit, so the unit's sha is not on the target branch under every strategy and ancestry cannot be used as the test after the fact.Related
merge_local's tail skippingdone_checkpoint; adjacent window, different cause (that one merged, this one did not)