From 5620879f152fd72b219ead0c89f39ae04f7d6da3 Mon Sep 17 00:00:00 2001 From: Pawel-N-pl Date: Sat, 4 Jul 2026 22:42:16 +0200 Subject: [PATCH 1/3] fix(runs): advance re-arm baseline so redrive keeps resolution commits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit rearm_escalation flipped the story back to PENDING but left baseline_commit at the escalated attempt's pre-attempt snapshot. When the resolve session had committed authorized work on the project branch (e.g. a baseline fixture the escalation was about), the redrive's reset-to-baseline in engine._rollback_or_pause parked those commits on an attempt-preserve ref and rebuilt against the unresolved tree — the re-driven dev session immediately hit the very gap the human had just resolved and re-escalated. Advance baseline_commit to the project's HEAD and refresh the untracked snapshot at re-arm time, so resolution work counts as the rebuild's starting point rather than attempt debris. Best-effort: a git failure keeps the old baseline. The story-escalation-resolved journal event now records the adopted baseline. --- src/bmad_loop/runs.py | 28 +++++++++++++++++++++++++--- tests/test_resolve.py | 31 +++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 3 deletions(-) diff --git a/src/bmad_loop/runs.py b/src/bmad_loop/runs.py index 5f89893..d395021 100644 --- a/src/bmad_loop/runs.py +++ b/src/bmad_loop/runs.py @@ -511,8 +511,11 @@ def rearm_escalation(run_dir: Path, story_key: str | None = None) -> str: Flips the escalated task out of its terminal ESCALATED phase back to PENDING — which makes `_finish_inflight` reset the tree to the story's baseline and re-run it (clean rebuild) against the now-corrected frozen - spec. Deterministically sets that spec's status to `ready-for-dev` so the - dev session routes straight to implement, and strips the escalated + spec. The baseline itself is advanced to the project's current HEAD (and + the untracked snapshot refreshed) so commits and files the resolve session + produced count as the rebuild's starting point, not as attempt debris to + roll back. Deterministically sets that spec's status to `ready-for-dev` so + the dev session routes straight to implement, and strips the escalated attempt's stale `## Auto Run Result` section so the re-drive cannot read as terminal from its first save. Does NOT clear the pause; the caller resumes the run separately. @@ -544,6 +547,23 @@ def rearm_escalation(run_dir: Path, story_key: str | None = None) -> str: task.rearmed = True # resume-time recovery notice describes a clean rebuild, # not a failed attempt (engine._finish_inflight clears it once the rebuild runs) + # Advance the attempt baseline to the project's current HEAD and refresh the + # untracked snapshot: whatever the human-driven resolve session left on the + # branch (a committed fixture, a corrected ledger, ...) is authorized input + # for the re-drive, not failed-attempt debris. Without this, the re-drive's + # reset-to-baseline in engine._rollback_or_pause parks the resolution + # commits on an attempt-preserve ref and rebuilds against a tree that + # contradicts the corrected spec — the re-driven dev session then hits the + # very gap the human just resolved. Best-effort: on a git failure the old + # baseline stands (the redrive rollback path tolerates a stale baseline; it + # just loses this protection). + try: + repo = Path(state.project) + task.baseline_commit = verify.rev_parse_head(repo) + task.baseline_untracked = sorted(verify.untracked_files(repo)) + except verify.GitError: + pass + if task.spec_file: # route /bmad-dev-auto to re-implement (decision table: ready-for-dev # -> step-03); independent of the resolve agent having set it. @@ -555,5 +575,7 @@ def rearm_escalation(run_dir: Path, story_key: str | None = None) -> str: devcontract.strip_auto_run_result(Path(task.spec_file)) save_state(run_dir, state) - Journal(run_dir).append("story-escalation-resolved", story_key=key) + Journal(run_dir).append( + "story-escalation-resolved", story_key=key, baseline=task.baseline_commit or "" + ) return key diff --git a/tests/test_resolve.py b/tests/test_resolve.py index cbcbb1a..ffb2bc0 100644 --- a/tests/test_resolve.py +++ b/tests/test_resolve.py @@ -3,6 +3,7 @@ import json import pytest +from conftest import git from bmad_loop import devcontract, resolve, runs, verify from bmad_loop.journal import load_state, save_state @@ -174,6 +175,36 @@ def test_rearm_journals_event(tmp_path): assert "story-escalation-resolved" in journal +def test_rearm_advances_baseline_to_resolved_head(project): + # The resolve session committed work on the project branch (e.g. a fixture + # the human authorized). Re-arm must adopt that state as the new attempt + # baseline, or the redrive's reset-to-baseline parks the resolution commit + # on an attempt-preserve ref and re-drives against the unresolved tree. + root = project.project + old_head = git(root, "rev-parse", "HEAD") + run_dir, _, _ = _escalated_run(root) + (root / "fixture.txt").write_text("captured baseline\n", encoding="utf-8") + git(root, "add", "fixture.txt") + git(root, "commit", "-q", "-m", "resolution: capture fixture") + # a file the resolve session (or the user) left untracked must enter the + # snapshot, so the redrive reset treats it as pre-existing, not run-created + (root / "leftover.txt").write_text("keep me\n", encoding="utf-8") + runs.rearm_escalation(run_dir) + task = load_state(run_dir).tasks["6-4-cli-list-command"] + assert task.baseline_commit == git(root, "rev-parse", "HEAD") + assert task.baseline_commit != old_head + assert "leftover.txt" in task.baseline_untracked + + +def test_rearm_keeps_stale_baseline_outside_a_repo(tmp_path): + # best-effort contract: a project dir that is not a git repo (or a broken + # one) must not make re-arm fail — the old baseline simply stands + run_dir, _, _ = _escalated_run(tmp_path) + runs.rearm_escalation(run_dir) + task = load_state(run_dir).tasks["6-4-cli-list-command"] + assert task.baseline_commit == "abc123" + + def test_rearm_rejects_non_escalation_stage(tmp_path): run_dir = tmp_path / ".bmad-loop" / "runs" / "r1" save_state( From 99d93e6090ef7ffc80ef851d77ff78ef89a4f018 Mon Sep 17 00:00:00 2001 From: Pawel-N-pl Date: Tue, 7 Jul 2026 01:14:59 +0200 Subject: [PATCH 2/3] fix(runs): keep baseline_commit/baseline_untracked as an atomic pair on partial git failure Both git reads in rearm_escalation's baseline refresh now land in locals before either task field is assigned, and the except widens from GitError to Exception (matching verify._prune_refs' best-effort contract for the whole subprocess surface: a timeout or missing-binary OSError must not fail re-arm either). Previously, rev_parse_head succeeding while untracked_files raised would advance baseline_commit alone, leaving baseline_untracked stale and the two fields describing different points in time - exactly the pairing attempt_dirty/safe_rollback rely on to tell pre-existing untracked files from run-created ones. Addresses review feedback from PR #78 (CodeRabbit + Augment, converging on the same finding). --- src/bmad_loop/runs.py | 13 ++++++++++--- tests/test_resolve.py | 18 ++++++++++++++++++ 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/src/bmad_loop/runs.py b/src/bmad_loop/runs.py index d395021..30208ca 100644 --- a/src/bmad_loop/runs.py +++ b/src/bmad_loop/runs.py @@ -559,9 +559,16 @@ def rearm_escalation(run_dir: Path, story_key: str | None = None) -> str: # just loses this protection). try: repo = Path(state.project) - task.baseline_commit = verify.rev_parse_head(repo) - task.baseline_untracked = sorted(verify.untracked_files(repo)) - except verify.GitError: + head = verify.rev_parse_head(repo) + untracked = sorted(verify.untracked_files(repo)) + task.baseline_commit = head + task.baseline_untracked = untracked + except Exception: # noqa: BLE001 - best-effort: a git timeout/OSError must not + # leave the pair half-updated any more than a GitError does. The two + # locals are computed before either field is assigned, so a failure on + # either call can't advance baseline_commit while baseline_untracked + # stays stale (same broad-except-for-best-effort contract as + # verify._prune_refs). pass if task.spec_file: diff --git a/tests/test_resolve.py b/tests/test_resolve.py index ffb2bc0..e4fb8e6 100644 --- a/tests/test_resolve.py +++ b/tests/test_resolve.py @@ -196,6 +196,24 @@ def test_rearm_advances_baseline_to_resolved_head(project): assert "leftover.txt" in task.baseline_untracked +def test_rearm_baseline_all_or_nothing_on_partial_git_failure(monkeypatch, project): + """rev_parse_head succeeding but untracked_files failing must not advance + baseline_commit while leaving baseline_untracked stale: both locals are + computed before either field is assigned, so a failure on the second call + leaves the pair exactly as it was, same as a failure on the first.""" + root = project.project + run_dir, _, _ = _escalated_run(root) + + def boom(repo): + raise verify.GitError("simulated failure") + + monkeypatch.setattr(runs.verify, "untracked_files", boom) + runs.rearm_escalation(run_dir) + task = load_state(run_dir).tasks["6-4-cli-list-command"] + assert task.baseline_commit == "abc123" + assert task.baseline_untracked is None + + def test_rearm_keeps_stale_baseline_outside_a_repo(tmp_path): # best-effort contract: a project dir that is not a git repo (or a broken # one) must not make re-arm fail — the old baseline simply stands From ccc7cb7f0548021872a352c4bed7525825c19cd9 Mon Sep 17 00:00:00 2001 From: Pawel-N-pl Date: Tue, 7 Jul 2026 01:27:51 +0200 Subject: [PATCH 3/3] fix(runs): suppress bandit B110 on the best-effort baseline-refresh except CI's trunk check (bandit) flagged the bare except/pass in rearm_escalation's baseline refresh as B110 (try/except/pass). It is deliberately best-effort (a git failure must not fail re-arm), matching the same pattern already used elsewhere in the codebase (e.g. verify._prune_refs, Engine's teardown handlers) - add the same noqa/nosec suppression with a short inline reason. Verified locally with bandit (via uvx): 0 issues, 1 potential issue specifically suppressed by the new nosec marker. --- src/bmad_loop/runs.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/bmad_loop/runs.py b/src/bmad_loop/runs.py index 30208ca..d1c794d 100644 --- a/src/bmad_loop/runs.py +++ b/src/bmad_loop/runs.py @@ -557,18 +557,16 @@ def rearm_escalation(run_dir: Path, story_key: str | None = None) -> str: # very gap the human just resolved. Best-effort: on a git failure the old # baseline stands (the redrive rollback path tolerates a stale baseline; it # just loses this protection). + # The two locals are computed before either task field is assigned, so a + # failure on either git call can't advance baseline_commit while + # baseline_untracked stays stale, or vice versa. try: repo = Path(state.project) head = verify.rev_parse_head(repo) untracked = sorted(verify.untracked_files(repo)) task.baseline_commit = head task.baseline_untracked = untracked - except Exception: # noqa: BLE001 - best-effort: a git timeout/OSError must not - # leave the pair half-updated any more than a GitError does. The two - # locals are computed before either field is assigned, so a failure on - # either call can't advance baseline_commit while baseline_untracked - # stays stale (same broad-except-for-best-effort contract as - # verify._prune_refs). + except Exception: # noqa: BLE001 # nosec B110 - best-effort git read, must not fail re-arm pass if task.spec_file: