feat(review): escalate deliberate sprint-status regressions at review-verify

CHANGELOG.md

@@ -91,6 +91,20 @@ story <id>`, the same annotation a sweep bundle writes. Both sprint and stories
 
 ### Changed
 
+- **A review that revokes the sprint sign-off now escalates instead of burning review cycles
+  (#334).** The orchestrator advances `sprint-status` to `done` at dev time; a review session that
+  judges the story unfinished and writes the board back to an earlier stage contradicts that — and
+  nothing in the review loop re-advances it, so every remaining cycle re-read the same failure and
+  the story ended deferred with its work rolled back. That regression is now detected at the
+  review-verify gate and pauses the run with both sides named and the two ways out (finish the work
+  and re-arm, or accept the story and advance the board). Keys on `sprint-status` only — the spec's
+  own frontmatter legitimately cycles, and `status: blocked` stays the sanctioned hand-back channel
+  — and stays conservative: without the orchestrator's own launch-time sign-off, or on a status
+  outside the known lifecycle, it falls back to the old retry. New knob
+  `[review] on_status_contradiction`, default `escalate`; `retry` restores the previous
+  burn-cycles-then-defer behavior verbatim. Review prompts are unchanged by design: forbidding the
+  revert would make a correct reviewer comply and the story would commit without sign-off.
+
 - **`bmad-loop-setup` stops registering BMAD config; the installer owns it (#258).** The skill
   wrote `_bmad/config.yaml`, `_bmad/config.user.yaml` and a root `_bmad/module-help.csv` — the
   pre-v6.10 layout, which BMAD's own resolver never reads (it merges four TOML layers, and

README.md

@@ -381,6 +381,10 @@ enabled = true             # false = skip the separate review session; the dev p
 trigger = "recommended"    # when enabled: "recommended" runs the separate review only when
                            # bmad-dev-auto flags followup_review_recommended; "always" = every story
                            # (the loop is bounded by limits.max_review_cycles either way)
+on_status_contradiction = "escalate"
+                           # a review that writes sprint-status back off "done" (revoking the
+                           # sign-off the orchestrator recorded at dev time) pauses the run naming
+                           # both sides; "retry" = legacy — burn review cycles, then defer + roll back
 
 [adapter]
 name = "claude"            # CLI profile: claude | codex | gemini | copilot | antigravity | opencode-http (alias: opencode) | custom

src/bmad_loop/data/settings/core.toml

@@ -52,6 +52,13 @@ options_ref = "REVIEW_ON_TIMEOUT_MODES"
 default_ref = "ReviewPolicy.on_timeout"
 label = "review timeout policy"
 description = "retry: burn a review cycle per timeout (default) · salvage-if-done: commit the verified dev product and refile the follow-up · defer: give up on the first timeout"
+[[section.field]]
+key = "on_status_contradiction"
+kind = "select"
+options_ref = "REVIEW_ON_STATUS_CONTRADICTION_MODES"
+default_ref = "ReviewPolicy.on_status_contradiction"
+label = "review revokes sprint sign-off"
+description = "escalate: pause naming both sides when a review writes sprint-status back off done (default) · retry: legacy — burn review cycles, then defer + roll back"
 
 [[section]]
 name = "stories"

src/bmad_loop/engine.py

@@ -1475,11 +1475,13 @@ def _review_and_commit(
                     story_key=task.story_key,
                     reason=outcome.reason,
                     env_fault=outcome.env_fault,
+                    contradiction=outcome.contradiction,
                 )
                 if not outcome.retryable:
-                    # escalate-grade failure (environment fault, git error): a
-                    # repair session cannot fix it and another review cycle
-                    # would replay it — pause the run instead of burning budget
+                    # escalate-grade failure (environment fault, git error, a
+                    # review revoking the sprint sign-off): a repair session
+                    # cannot fix it and another review cycle would replay it —
+                    # pause the run instead of burning budget
                     self._escalate(task, outcome.reason)
                 if outcome.fixable and task.review_cycle < self.policy.limits.max_review_cycles:
                     # failing verify commands are dev work, not review work: a
@@ -1525,10 +1527,30 @@ def _review_and_commit(
             # the non-isolated path: in worktree isolation a defer already keeps the
             # unit's worktree + patch (no work is lost), so there is nothing to
             # rescue and committing into the main repo would be wrong.
-            if refileable_followup and not self._isolated and self._verify_review(task).ok:
-                self._record_review_budget_followup(task)
-                self._commit(task)
-                return
+            if refileable_followup and not self._isolated:
+                rescue = self._verify_review(task)
+                if rescue.ok:
+                    self._record_review_budget_followup(task)
+                    self._commit(task)
+                    return
+                if rescue.contradiction:
+                    # The rescue gate is the first place this story's sprint
+                    # sign-off was re-read (every in-loop cycle recommended its own
+                    # follow-up, so none of them verified). A defer here would roll
+                    # the work back under a "did not converge" reason that names
+                    # neither side of the disagreement — pause with both instead.
+                    # Journaled under the same kind as the two in-loop gates so a
+                    # consumer keying on `contradiction` sees all three escalating
+                    # paths. The non-contradiction arm keeps its existing silence:
+                    # its story is told by the defer reason below.
+                    self.journal.append(
+                        "review-verify-failed",
+                        story_key=task.story_key,
+                        reason=rescue.reason,
+                        env_fault=rescue.env_fault,
+                        contradiction=rescue.contradiction,
+                    )
+                    self._escalate(task, rescue.reason)
             # Name the last completed pass's real outcome (issue #160): the fixed
             # follow-up wording is only correct when a finalized pass actually left
             # a refileable recommendation. "did not converge" stays in every variant
@@ -1669,10 +1691,12 @@ def _skip_review_and_commit(self, task: StoryTask) -> None:
                 story_key=task.story_key,
                 reason=outcome.reason,
                 env_fault=outcome.env_fault,
+                contradiction=outcome.contradiction,
             )
             if not outcome.retryable:
-                # escalate-grade failure (environment fault, git error): a
-                # defer would just replay it on the next story — pause the run
+                # escalate-grade failure (environment fault, git error, a review
+                # revoking the sprint sign-off): a defer would just replay it on
+                # the next story — pause the run
                 self._escalate(task, outcome.reason)
             self._defer(task, f"verify failed with review disabled: {outcome.reason}")
             return
@@ -2376,7 +2400,16 @@ def _verify_dev_artifacts(self, task: StoryTask, result_json: dict | None):
         )
 
     def _verify_review(self, task: StoryTask):
-        return verify.verify_review(task, self.workspace.paths, self.policy)
+        # `not _dev_review_enabled()` is exactly the case where _post_dev_state_sync
+        # targeted "done" and verify_dev asserted the board got there, so a board
+        # now short of done is a review revoking that sign-off, not a stage never
+        # reached (#334).
+        return verify.verify_review(
+            task,
+            self.workspace.paths,
+            self.policy,
+            sprint_reached_done=not self._dev_review_enabled(),
+        )
 
     def _review_prompt(self, task: StoryTask) -> str:
         # Re-invoking bmad-dev-auto on a `done` spec resets review_loop_iteration

src/bmad_loop/model.py

@@ -483,6 +483,11 @@ class VerifyOutcome:
     # not found / not executable): no repair session can fix it and every
     # story shares the same commands, so it must never charge attempt budgets
     env_fault: bool = False
+    # a session deliberately contradicted a state the orchestrator had already
+    # established (a review revoking the sprint sign-off it advanced at dev
+    # time): no further session can reconcile it, so it routes to a pause with
+    # both sides named rather than to another cycle (#334)
+    contradiction: bool = False
 
     @classmethod
     def passed(cls) -> "VerifyOutcome":
@@ -494,9 +499,19 @@ def retry(cls, reason: str, fixable: bool = False) -> "VerifyOutcome":
 
     @classmethod
     def escalate(
-        cls, reason: str, severity: str = "CRITICAL", env_fault: bool = False
+        cls,
+        reason: str,
+        severity: str = "CRITICAL",
+        env_fault: bool = False,
+        contradiction: bool = False,
     ) -> "VerifyOutcome":
-        return cls(ok=False, reason=reason, severity=severity, env_fault=env_fault)
+        return cls(
+            ok=False,
+            reason=reason,
+            severity=severity,
+            env_fault=env_fault,
+            contradiction=contradiction,
+        )
 
     @property
     def retryable(self) -> bool:

src/bmad_loop/policy.py

@@ -29,6 +29,7 @@
 SWEEP_AUTO_MODES = {"never", "per-epic", "run-end"}
 REVIEW_TRIGGER_MODES = {"always", "recommended"}
 REVIEW_ON_TIMEOUT_MODES = {"retry", "salvage-if-done", "defer"}
+REVIEW_ON_STATUS_CONTRADICTION_MODES = {"escalate", "retry"}
 # Where the run gets its story queue. "sprint-status" (default) is the classic
 # flow — bmad-sprint-planning writes sprint-status.yaml from prose epics.
 # "stories" is the opt-in folder+id dispatch flow (BMAD-METHOD #2549): a typed,
@@ -204,6 +205,21 @@ class ReviewPolicy:
     #   "defer" — give up on the first timeout-like verdict (no retries).
     # `crashed` and env-fault (#194) verdicts keep their own routing in every mode.
     on_timeout: str = "retry"
+    # What a review that revokes the story's sprint sign-off costs (#334). The
+    # orchestrator advances sprint-status to `done` at dev time; a review session
+    # that judges the story unfinished and writes the board back to an earlier
+    # stage contradicts that — and nothing in the review loop re-advances the
+    # board, so every remaining cycle re-reads the same failure and the story
+    # ends deferred + rolled back.
+    #   "escalate" (default) — pause the run naming both sides of the
+    #       disagreement, so a human resolves it instead of the budget burning
+    #       down onto a rollback.
+    #   "retry" — legacy behavior: treat it as an ordinary verify failure, burn
+    #       review cycles to limits.max_review_cycles, then defer.
+    # Keys on sprint-status only: the spec's own frontmatter status legitimately
+    # cycles (in-review/in-progress) while a review patches, and `status: blocked`
+    # remains the sanctioned way for a review to hand a story back to a human.
+    on_status_contradiction: str = "escalate"
 
 
 @dataclass(frozen=True)
@@ -779,6 +795,9 @@ def loads(text: str, plugin_schemas: dict[str, Any] | None = None) -> Policy:
         enabled=bool(review_d.get("enabled", ReviewPolicy.enabled)),
         trigger=str(review_d.get("trigger", ReviewPolicy.trigger)).strip(),
         on_timeout=str(review_d.get("on_timeout", ReviewPolicy.on_timeout)).strip(),
+        on_status_contradiction=str(
+            review_d.get("on_status_contradiction", ReviewPolicy.on_status_contradiction)
+        ).strip(),
     )
     if review.trigger not in REVIEW_TRIGGER_MODES:
         raise PolicyError(
@@ -789,6 +808,12 @@ def loads(text: str, plugin_schemas: dict[str, Any] | None = None) -> Policy:
             f"review.on_timeout must be one of {sorted(REVIEW_ON_TIMEOUT_MODES)}:"
             f" got {review.on_timeout!r}"
         )
+    if review.on_status_contradiction not in REVIEW_ON_STATUS_CONTRADICTION_MODES:
+        raise PolicyError(
+            "review.on_status_contradiction must be one of "
+            f"{sorted(REVIEW_ON_STATUS_CONTRADICTION_MODES)}:"
+            f" got {review.on_status_contradiction!r}"
+        )
     stories = StoriesPolicy(
         source=str(stories_d.get("source", StoriesPolicy.source)).strip(),
         spec_folder=str(stories_d.get("spec_folder", StoriesPolicy.spec_folder)).strip(),
@@ -1048,6 +1073,12 @@ def _fold_deprecated_engine(
 #                        commit it and refile the outstanding follow-up to deferred work.
 #   "defer"           -> give up on the first timeout-like verdict.
 on_timeout = "retry"
+# What a review that writes sprint-status back off `done` (the sign-off the
+# orchestrator recorded at dev time) costs. Nothing in the review loop
+# re-advances the board, so every remaining cycle re-reads the same failure:
+#   "escalate" -> pause the run naming both sides of the disagreement (default).
+#   "retry"    -> legacy: burn review cycles to max_review_cycles, then defer.
+on_status_contradiction = "escalate"
 
 [stories]
 # Story-queue source. "sprint-status" (default) walks sprint-status.yaml written

src/bmad_loop/verify.py

@@ -24,7 +24,7 @@
 from .frontmatter import _split_frontmatter, read_frontmatter, status_of
 from .model import StoryTask, VerifyOutcome
 from .policy import POLICY_FILE, Policy
-from .sprintstatus import story_status
+from .sprintstatus import STATUS_ORDER, story_status
 
 GIT_TIMEOUT_S = 120
 COMMAND_TIMEOUT_S = 30 * 60
@@ -1562,7 +1562,26 @@ def verify_commands_outcome(policy: Policy, cwd: Path) -> VerifyOutcome:
     return VerifyOutcome.passed()
 
 
-def verify_review(task: StoryTask, paths: ProjectPaths, policy: Policy) -> VerifyOutcome:
+def verify_review(
+    task: StoryTask,
+    paths: ProjectPaths,
+    policy: Policy,
+    *,
+    sprint_reached_done: bool = False,
+) -> VerifyOutcome:
+    """Gate a completed review pass: spec at ``done``, sprint-status at ``done``,
+    deterministic verify commands green.
+
+    ``sprint_reached_done`` tells the gate that the orchestrator had already
+    advanced this story's sprint-status to ``done`` before the review ran (it is
+    the sole ``sprint_advance`` caller, ``verify_dev`` asserted the write landed,
+    and ``advance`` never regresses). A board now sitting *earlier* than ``done``
+    is therefore not a stage the story never reached — it is a review session
+    deliberately revoking the sign-off. Nothing in the review loop re-advances
+    the board, so retrying only replays the same failure until the budget runs
+    out and the work is rolled back; under
+    ``review.on_status_contradiction = "escalate"`` (the default) the gate
+    escalates instead, naming both sides. See #334."""
     if not task.spec_file:
         return VerifyOutcome.retry("no spec file recorded for task")
     fm = _gate_frontmatter(Path(task.spec_file))
@@ -1574,13 +1593,45 @@ def verify_review(task: StoryTask, paths: ProjectPaths, policy: Policy) -> Verif
 
     sprint = story_status(paths.sprint_status, task.story_key)
     if sprint != "done":
+        if _is_signoff_regression(sprint, sprint_reached_done, policy):
+            return VerifyOutcome.escalate(
+                f"review revoked the sprint sign-off for {task.story_key}: the "
+                f"orchestrator advanced the board to 'done' after dev verified, "
+                f"and the review session wrote it back to {sprint!r} while leaving "
+                f"the spec frontmatter at 'done'. The two sides disagree about "
+                f"whether the story is finished, and no further review cycle can "
+                f"reconcile them — the review loop never re-advances the board, so "
+                f"the remaining cycles would burn down onto a defer that rolls the "
+                f"work back. Resolve by either completing the outstanding work and "
+                f"re-arming the escalation (the attempt budget resets on re-arm), "
+                f"or accepting the story and advancing the board yourself; set "
+                f'review.on_status_contradiction = "retry" to restore the legacy '
+                f"retry-until-budget behavior.",
+                contradiction=True,
+            )
         return VerifyOutcome.retry(
             f"sprint-status for {task.story_key} is {sprint!r}, expected 'done'"
         )
 
     return verify_commands_outcome(policy, paths.project)
 
 
+def _is_signoff_regression(sprint: str | None, sprint_reached_done: bool, policy: Policy) -> bool:
+    """Whether a non-``done`` sprint status is a review deliberately walking the
+    board backward, as opposed to a stage the story simply never reached.
+
+    Conservative on every uncertainty: without the launch-time guarantee, with
+    the knob set to ``retry``, or when the fresh read yields no status at all
+    (missing story entry) or a token outside the known lifecycle (a hand-edited
+    or future board), the caller falls through to the ordinary retry — a wrong
+    escalation halts an otherwise healthy run."""
+    if not sprint_reached_done or policy.review.on_status_contradiction != "escalate":
+        return False
+    if sprint is None or sprint not in STATUS_ORDER:
+        return False
+    return STATUS_ORDER.index(sprint) < STATUS_ORDER.index("done")
+
+
 def verify_review_stories(task: StoryTask, paths: ProjectPaths, policy: Policy) -> VerifyOutcome:
     """verify_review for stories mode: same spec-done + verify-commands gates,
     minus the sprint-status gate (stories mode has no sprint board — the story