Skip to content

[BUG] Review session deadlocks when the orchestrator-owned deferred-work ledger is dirty at finalization #772

Description

@ben-damian

Description

A review session HALTs with a CRITICAL escalation even though it found nothing wrong with the story. The cause is two rules that cannot both be satisfied at once. First, the review session must never modify, revert, or commit deferred-work.md — the orchestrator owns that file. Second, the finalization step checks that the working copy is fully clean, and HALTs with status blocked and blocking condition finalization left repository dirty if not. When the orchestrator has already written an uncommitted change to the ledger before the review session's finalization step runs, the session cannot satisfy both rules: it cannot commit the ledger edit (forbidden) and it cannot end clean (required). It escalates every time this coincidence occurs, and the same story can hit this identical HALT on several separate review passes, each needing a human to resolve it by hand.

Steps to reproduce

  1. Run a story with review.trigger set so a fresh review session runs after implementation.
  2. Before the review session reaches finalization, have the orchestrator write an entry to deferred-work.md in that story's working copy (for example, from a concurrent sweep or a prior sign-off), so the file sits modified-but-uncommitted.
  3. Let the review session run to completion with no other outstanding findings.
  4. Watch it reach the finalization step: it commits its own reviewed-diff files, then checks the working copy for cleanliness.

Expected behavior

The review session finishes with status done, leaving the pre-existing ledger edit exactly as it found it — untouched and uncommitted — since it does not own that file and the diff did not come from this session.

Actual behavior

The session HALTs with status blocked, blocking condition finalization left repository dirty. The only uncommitted change is the orchestrator-owned ledger edit, which the session is explicitly told never to touch. There is no exemption for it in the clean-tree check, so the HALT reproduces on every subsequent pass until a human manually approves treating that one file as exempt.

Observed 5 times across 2 stories in the attached run's journal (story-escalated × 8 total, several tied to this exact blocking condition).

Related issue: #437 fixed the same class of problem for sprint-status.yaml — a review session was reverting the orchestrator's own write to that file because the review prompt had no "hands off" instruction for it. That fix stopped the session from touching the file, but did not extend to the separate "the tree must end clean or HALT" rule, and did not cover deferred-work.md's finalization check. This report is that remaining gap.

Suggested fix

Treat orchestrator-owned artifacts (deferred-work.md, and any equivalent to what _protected_carry_paths already protects) as exempt from a review session's "working copy is clean" gate — clean should mean "clean apart from pre-existing orchestrator-owned diffs," not byte-for-byte clean.

Which area is this for?

Orchestrator / control loop

bmad-loop Version

0.10.0

Which coding CLI are you using?

Claude (claude)

Operating System

macOS

Diagnostic dump (bmad-loop diagnose)

bmad-loop diagnose also flagged a routing gap while producing this dump — a journal kind value carried a story identifier baked into the string itself (e.g. <alias>-result), which the per-field aliasing does not catch; the fail-closed backstop caught and pseudonymized it instead (see "Backstop repairs" below). Filing per the tool's own request.

# bmad-loop diagnostic dump (sanitized)

_Identifiers are pseudonymized; code, prompts, paths and free text are redacted. Safe to share._

## Environment
- **bmad-loop version:** 0.10.0
- **python:** 3.11.15
- **os:** Darwin 25.5.0
- **sys.platform:** darwin
- **win32 on WSL distro path:** no
- **multiplexer:** TmuxMultiplexer
- **tmux:** tmux 3.7c
- **schema / generated:** v1 @ 2026-09-07T19:13:16Z

## Run `20260905-224232-e0a7` (sweep)
- **project:** `project-1cff24c89a18`
- **started:** 2026-09-05
- **state:** finished=False stopped=False paused=False
- **epic / sweep_cycle:** None / 3
- **tasks:** 12
- **phase histogram:** done=10, escalated=1, review-running=1
- **token totals:** cache_creation_tokens=827627, cache_read_tokens=200080855, input_tokens=4150636, output_tokens=690037, total=205749155, weighted=25676387
- **sessions by status:** completed=37
- **sessions by role:** dev=16, review=18, triage=3

### Tasks
| alias | epic | phase | att | rev | committed | spec | dw | sessions | weighted | raw |
|---|---|---|---|---|---|---|---|---|---|---|
| `s0-ea2fd81503aa` | 0 | done | 1 | 0 | False | False | 0 | 1 | 110688 | 479558 |
| `s0-91f07d59ebf3` | 0 | done | 1 | 2 | True | True | 1 | 3 | 1717395 | 13504889 |
| `s0-f49a402174bc` | 0 | done | 1 | 0 | True | True | 1 | 1 | 529664 | 3960320 |
| `s0-40025665c07c` | 0 | done | 1 | 2 | True | True | 1 | 3 | 1812236 | 14185292 |
| `s0-4264d19b74dd` | 0 | done | 1 | 0 | False | False | 0 | 1 | 206364 | 1198466 |
| `s0-9dc3e927f611` | 0 | escalated | 1 | 0 | False | True | 1 | 2 | 540211 | 3443481 |
| `s0-ba95ba085d21` | 0 | done | 1 | 2 | True | True | 1 | 6 | 3073909 | 23885711 |
| `s0-5b107e3595dc` | 0 | done | 2 | 2 | True | True | 1 | 3 | 3279336 | 23726953 |
| `s0-ccf763ed6a4e` | 0 | done | 1 | 2 | True | True | 3 | 12 | 11613440 | 98684710 |
| `s0-08bab1d15c70` | 0 | done | 1 | 0 | False | False | 0 | 1 | 150159 | 675471 |
| `s0-7aa6ea01547a` | 0 | done | 2 | 0 | True | True | 17 | 1 | 1158917 | 10288978 |
| `s0-bffc534e02d0` | 0 | review-running | 1 | 1 | False | True | 1 | 3 | 1484068 | 11715326 |

### Journal
- **entries:** 443
- **duration (s):** 196113.906
- **escalations / defers / plugin-errors:** 8 / 0 / 0
- **kind histogram:** bundle-start=9, decision-preanswered=15, decision-preanswers-pruned=1, decision-skipped-unattended=75, dev-decision=16, harvest-carried=1, plugin-hook=20, plugins-active=14, resume-restart=11, review-followup-carried=5, review-followup-damped=5, review-not-recommended=2, review-result=12, review-skipped=2, run-paused=8, run-resume=13, run-start=1, run-stop=5, session-end=41, session-start=42, spec-deferrals-harvested=3, story-done=7, story-escalated=8, story-escalation-resolved=7, sweep-bundle-close-carried=7, sweep-bundle-closed=14, sweep-bundle-skipped=6, sweep-bundles-truncated=8, sweep-cycle=7, sweep-inflight-redrive=11, sweep-ledger-commit=1, s0-ea2fd81503aa-result=3, target-branch=1, token-budget-exceeded=3, triage-decision=3, unit-merge-started=7, unit-merged=7, worktree-kept=1, worktree-opened=20, worktree-seed-skipped=20, worktree-teardown-degraded=1

### Run-dir files (counts only)
- **logs:** 27 files, 135963721 bytes, 427984 lines
- **tasks:** 81 files, 81800 bytes
- **bundles:** 9 files, 25503 bytes
- **worktrees:** 70595 files, 2945619534 bytes
- **events:** 86 files, 36001 bytes

### Backstop repairs

_The leak self-check caught stray occurrences of pseudonymized identifiers that the per-field routing missed, and substituted their aliases — a bmad-loop routing gap; please report it._

- `story:s0-ea2fd81503aa`: 4 stray occurrence(s) pseudonymized

Confirm

  • I've searched for existing issues
  • I reviewed the diagnose dump and confirmed it contains no PII, secrets, or project-specific information
  • I'm using the latest version

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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