Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,15 @@ breaking changes may land in a minor release.

### Fixed

- **Resume no longer discards a story that already passed its pre-commit gates.** A host death in
the COMMITTING window (phase persisted before `finalize_commit` ran and the DONE save stamped
`commit_sha`) matched no resume arm — there is no COMMITTING-keyed session record to replay — and
fell through to resume-restart, rolling back or pausing over fully-verified work. Resume now
finishes the commit in place: the `pre_commit_gate` workflows are not re-charged (the persisted
phase is durable proof they passed), the `pre_commit` hook re-fires (message regeneration and
pause veto honored), and `finalize_commit`'s content-idempotence covers both the pre- and
post-squash crash states. Sweep bundles get the same recovery in `_recover_inflight_bundle` (#115).

- **Resume no longer asks for a rollback of a completed session's committed work.** A host death
in the post-verify decision window left the task persisted at `DEV_VERIFY`/`REVIEW_VERIFY`,
where the resume replay matcher (which only knew the `*_RUNNING` phases) missed the
Expand Down
46 changes: 46 additions & 0 deletions src/bmad_loop/engine.py
Original file line number Diff line number Diff line change
Expand Up @@ -1184,6 +1184,27 @@ def _finish_inflight(self) -> None:
self._integrate_unit(task, unit)
else:
continuation()
elif task.phase == Phase.COMMITTING:
# the host died in the commit window: the gate+advance save
# landed (pre_commit_gate ran clean) but the DONE save that
# stamps commit_sha did not. Finish the commit instead of
# rolling verified work back — the gates are deliberately NOT
# re-run (see _finalize_commit_phase), the pre_commit hook IS
# re-emitted (message regenerated; pause veto still honored),
# and finalize_commit tolerates both the pre- and post-squash
# crash states (#115).
self.journal.append("resume-commit", story_key=task.story_key)
if isolated:
unit = self._reopen_unit(task)
prev = self.workspace
self.workspace = unit.workspace
try:
self._finalize_commit_phase(task)
finally:
self.workspace = prev
self._integrate_unit(task, unit)
else:
self._finalize_commit_phase(task)
else:
self.journal.append(
"resume-restart", story_key=task.story_key, phase=str(task.phase)
Expand Down Expand Up @@ -1784,6 +1805,31 @@ def _commit(self, task: StoryTask) -> None:
return
advance(task, Phase.COMMITTING)
self._save()
self._finalize_commit_phase(task)

def _finalize_commit_phase(self, task: StoryTask) -> None:
"""Drive an already-COMMITTING task to DONE: regenerate the message,
emit ``pre_commit`` (rewrite honored; a pause veto escalates —
COMMITTING→ESCALATED is legal), squash via ``finalize_commit``, stamp
``commit_sha``, advance to DONE.

Precondition: ``task.phase == COMMITTING`` and that phase is PERSISTED
(the gate+advance+save in ``_commit``, or a resume that found it on
disk). The persisted phase is durable proof the ``pre_commit_gate``
workflows already ran and passed — the COMMITTING save lands only
after the gate loop returns clean — which is why the resume arm calls
this WITHOUT re-running them: a re-run would double-charge the session
budget, and a blocking failure would need an illegal
COMMITTING→DEFERRED move (#115).

Re-drive contract: safe to call again after a host death anywhere
inside it. ``finalize_commit`` is content-idempotent across both crash
states — pre-squash (skill commit chain above baseline) squashes
normally; post-squash (squashed commit at HEAD, clean tree)
re-squashes to an identical-content commit, orphaning the pre-crash
squash (harmless). ``commit_sha`` is stamped only here and is
write-only (never routing), so the empty persisted value is
harmless."""
message = self._commit_message(task)
# pre_commit: a plugin may rewrite the commit message or escalate (pause).
# A defer/skip veto would have to unwind a COMMITTING task (no legal move
Expand Down
23 changes: 21 additions & 2 deletions src/bmad_loop/sweep.py
Original file line number Diff line number Diff line change
Expand Up @@ -604,9 +604,28 @@ def _recover_inflight_bundle(self, task: StoryTask) -> bool:
Deliberately narrower than the base _finish_inflight: no
`_resumable_session` arm, so a bundle whose host died in the
post-session window still restarts rather than replaying its recorded
result. Lifting that is a resume-fidelity change of its own."""
self.journal.append("resume-restart", story_key=task.story_key, phase=str(task.phase))
result. Lifting that is a resume-fidelity change of its own. The
COMMITTING window IS recovered, though — same as the base engine's
resume-commit arm (#115)."""
isolated = self._isolated and task.worktree_path
if task.phase == Phase.COMMITTING:
# the gate+advance save landed pre-death; finish the commit
# instead of rolling verified bundle work back (see
# Engine._finalize_commit_phase for the re-drive contract).
self.journal.append("resume-commit", story_key=task.story_key)
if isolated:
unit = self._reopen_unit(task)
prev = self.workspace
self.workspace = unit.workspace
try:
self._finalize_commit_phase(task)
finally:
self.workspace = prev
self._integrate_unit(task, unit)
else:
self._finalize_commit_phase(task)
return True
self.journal.append("resume-restart", story_key=task.story_key, phase=str(task.phase))
if task.phase == Phase.DEV_VERIFY and task.spec_file:
self._save()
if isolated:
Expand Down
49 changes: 48 additions & 1 deletion tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
from bmad_loop.bmadconfig import ProjectPaths
from bmad_loop.journal import save_state
from bmad_loop.model import PAUSE_ESCALATION, Phase, RunState, SessionRecord, StoryTask
from bmad_loop.verify import rev_parse_head
from bmad_loop.verify import finalize_commit, rev_parse_head

# The suite reads/writes UTF-8 files (specs, journals, JSON, reports). Windows'
# default text encoding is cp1252, so a plain read_text()/open() throws
Expand Down Expand Up @@ -256,6 +256,53 @@ def spec_path(paths: ProjectPaths, story_key: str) -> Path:
return paths.implementation_artifacts / f"spec-{story_key}.md"


def committing_crash_state(paths: ProjectPaths, engine, *, post_squash: bool = False) -> str:
"""Persist the exact state.json shape from issue #115: a task at COMMITTING
(the save right after advance(COMMITTING), before finalize_commit / the DONE
save that stamps commit_sha). Fully verified on disk: attempt work committed
above baseline (only the work file — sweeping the still-untracked sprint
board into the commit would make a later baseline reset delete it), spec at
done, sprint synced at DEV time. review_cycle stays 0 — the
_skip_review_and_commit path reaches COMMITTING with zero review sessions.
With post_squash, finalize_commit already ran before the death (squashed
commit at HEAD, clean tree) but commit_sha was never persisted. Returns the
baseline sha."""
baseline = rev_parse_head(paths.project)
src = paths.project / "src.txt"
src.write_text(src.read_text() + "change for 1-1-a\n")
git(paths.project, "add", "src.txt")
git(paths.project, "commit", "-q", "-m", "attempt work for 1-1-a")
sp = spec_path(paths, "1-1-a")
write_spec(sp, "done", baseline)
write_sprint(paths, {"1-1-a": "done"})
if post_squash:
finalize_commit(paths.project, baseline, "pre-crash squash")

task = StoryTask(story_key="1-1-a", epic=1, phase=Phase.COMMITTING, attempt=1)
task.review_cycle = 0
task.baseline_commit = baseline
task.baseline_untracked = []
task.spec_file = str(sp)
task.record_session(
SessionRecord(
task_id="1-1-a-dev-1",
role="dev",
status="completed",
result_json={
"workflow": "auto-dev",
"story_key": "1-1-a",
"spec_file": str(sp),
"baseline_commit": baseline,
"escalations": [],
"followup_review_recommended": False,
},
)
)
engine.state.tasks[task.story_key] = task
engine._save()
return baseline


def dev_effect(
paths: ProjectPaths,
story_key: str,
Expand Down
88 changes: 88 additions & 0 deletions tests/test_engine.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import pytest
from conftest import (
_file_exists_cmd,
committing_crash_state,
dev_effect,
fault_read_text,
generic_dev_effect,
Expand Down Expand Up @@ -585,6 +586,93 @@ def crashing_emit(stage, *args, **kwargs):
assert "rollback-manual-required" not in kinds


def test_resume_committing_finishes_commit_to_done(project):
"""#115: the host died after _commit persisted COMMITTING but before the
DONE save stamped commit_sha. That phase matched no resume arm and fell
through to resume-restart, rolling back (or pausing over) fully-verified
work. Resume must finish the commit in place — without re-charging the
pre_commit_gate workflows (the persisted phase is durable proof they
passed) and without any fresh session."""
policy = Policy(
gates=GatesPolicy(mode="none"),
notify=QUIET,
# the issue's environment: the production default, where the old
# resume-restart arm paused with the manual-recovery notice
scm=ScmPolicy(rollback_on_failure=False),
)
engine, _ = make_engine(project, [], policy=policy)
baseline = committing_crash_state(project, engine)

resumed, adapter = resume_engine(project, engine, [])
summary = resumed.run()

assert summary.done == 1 and not summary.paused and not summary.crashed
final = load_state(resumed.run_dir).tasks["1-1-a"]
assert final.phase == Phase.DONE
assert final.attempt == 1 and final.review_cycle == 0 # no budget burned
assert final.commit_sha == rev_parse_head(project.project) != baseline
assert adapter.sessions == [] # no session re-run — gates included
assert len(final.sessions) == 1 # only the pre-crash dev record
# the whole attempt squashed into exactly one story commit above baseline
log = git(project.project, "log", "--format=%s", f"{baseline}..HEAD")
assert len(log.splitlines()) == 1
assert "change for 1-1-a" in (project.project / "src.txt").read_text()
assert worktree_clean(project.project) # sprint board swept into the squash
kinds = [e["kind"] for e in resumed.journal.entries()]
assert "resume-commit" in kinds and "story-done" in kinds
assert "resume-restart" not in kinds
assert "rollback-manual-required" not in kinds


def test_resume_committing_post_squash_still_one_commit(project):
"""The other #115 crash state: finalize_commit completed just before the
death (squashed commit at HEAD, clean tree) but the DONE save never landed.
The re-drive must converge on exactly ONE commit above baseline — not stack
a second squash — and stamp commit_sha at HEAD."""
engine, _ = make_engine(project, [])
baseline = committing_crash_state(project, engine, post_squash=True)

resumed, adapter = resume_engine(project, engine, [])
summary = resumed.run()

assert summary.done == 1 and not summary.crashed
final = load_state(resumed.run_dir).tasks["1-1-a"]
assert final.phase == Phase.DONE
assert final.commit_sha == rev_parse_head(project.project)
assert adapter.sessions == []
log = git(project.project, "log", "--format=%s", f"{baseline}..HEAD")
assert len(log.splitlines()) == 1 # re-squash, not a stacked second commit
assert worktree_clean(project.project)
kinds = [e["kind"] for e in resumed.journal.entries()]
assert "resume-commit" in kinds
assert "resume-restart" not in kinds


def test_resume_committing_git_error_escalates(project):
"""A commit failure during the re-drive escalates (COMMITTING→ESCALATED is
the legal failure move) with the attempt chain intact at HEAD — never a
silent rollback through resume-restart."""
engine, _ = make_engine(project, [])
committing_crash_state(project, engine)
head_before = rev_parse_head(project.project)
# a rejecting pre-commit hook makes finalize_commit's commit step fail
hook = project.project / ".git" / "hooks" / "pre-commit"
hook.write_text("#!/bin/sh\nexit 1\n")
hook.chmod(0o755)

resumed, _ = resume_engine(project, engine, [])
summary = resumed.run()

assert summary.paused and summary.escalated == 1
final = load_state(resumed.run_dir).tasks["1-1-a"]
assert final.phase == Phase.ESCALATED
# finalize's HEAD-restore preserved the attempt chain on the branch
assert rev_parse_head(project.project) == head_before
kinds = [e["kind"] for e in resumed.journal.entries()]
assert "resume-commit" in kinds
assert "resume-restart" not in kinds


def test_reconcile_early_return_heals_stale_resumed_dict(project):
"""On the idempotent early-return path (frontmatter already at the success
status), the reconcile still syncs a stale *resumed* result dict from the
Expand Down
70 changes: 69 additions & 1 deletion tests/test_engine_worktree.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
from bmad_loop.adapters.mock import MockAdapter
from bmad_loop.engine import Engine
from bmad_loop.journal import Journal, load_state
from bmad_loop.model import Phase, RunState, StoryTask, TokenUsage
from bmad_loop.model import Phase, RunState, SessionRecord, StoryTask, TokenUsage
from bmad_loop.policy import GatesPolicy, NotifyPolicy, Policy, ScmPolicy
from bmad_loop.verify import (
branch_exists,
Expand Down Expand Up @@ -476,6 +476,74 @@ def test_worktree_crash_restart_discards_stale_worktree(project):
assert [p.resolve() for p in worktree_list(project.project)] == [project.project.resolve()]


def test_worktree_resume_committing_finishes_and_merges(project):
"""#115, isolated flavor: a unit persisted at COMMITTING (gate+advance save
landed, DONE save did not) is finished inside its still-mounted worktree
and merged back — not discarded as a stale worktree by resume-restart."""
commit_sprint(project, {"1-1-a": "ready-for-dev"})
engine, _ = make_engine(project, [])
from bmad_loop.workspace import open_unit_workspace

unit = open_unit_workspace(
project.project, project, "test-run", "1-1-a", "main", "story", engine.run_dir
)
# the attempt committed its work inside the unit (only the work file —
# the sprint board is the orchestrator's dev-time write, still uncommitted)
src = unit.path / "src.txt"
src.write_text(src.read_text() + "change for 1-1-a\n")
git(unit.path, "add", "src.txt")
git(unit.path, "commit", "-q", "-m", "attempt work for 1-1-a")
wt = project.rebased(unit.path)
sp = wt.implementation_artifacts / "spec-1-1-a.md"
write_spec(sp, "done", unit.baseline)
set_sprint(wt, "1-1-a", "done")

task = StoryTask("1-1-a", 1, phase=Phase.COMMITTING, attempt=1)
task.worktree_path = str(unit.path)
task.branch = unit.branch
task.baseline_commit = unit.baseline
task.spec_file = str(sp)
task.record_session(
SessionRecord(
task_id="1-1-a-dev-1",
role="dev",
status="completed",
result_json={
"workflow": "auto-dev",
"story_key": "1-1-a",
"spec_file": str(sp),
"baseline_commit": unit.baseline,
"escalations": [],
"followup_review_recommended": False,
},
)
)
engine.state.tasks["1-1-a"] = task
engine._save()

state = load_state(engine.run_dir)
state.clear_pause()
adapter = MockAdapter([])
resumed = Engine(
paths=project,
policy=wt_policy(),
adapter=adapter,
run_dir=engine.run_dir,
journal=engine.journal,
state=state,
)
summary = resumed.run()

assert summary.done == 1 and not summary.crashed
assert adapter.sessions == [] # commit finished from persisted state alone
assert "change for 1-1-a" in (project.project / "src.txt").read_text()
assert [p.resolve() for p in worktree_list(project.project)] == [project.project.resolve()]
assert worktree_clean(project.project)
kinds = journal_kinds(resumed)
assert "resume-commit" in kinds and "unit-merged" in kinds
assert "resume-restart" not in kinds


# ----------------------------------------------------------------- regression guard


Expand Down
Loading
Loading