Skip to content

Escalate deliberate sprint-status regressions at review-verify instead of burning review cycles #334

Description

@pbean

When a review session deliberately reverts a story's sprint-status because the work is not actually done, the engine has no way to hear it. It re-runs the review until max_review_cycles is exhausted, then defers the story and rolls the work back. The disagreement is structural — no number of retries can resolve it — so every cycle is wasted, and the outcome is the worst available one.

The livelock chain (validated at 3c68153)

  1. The orchestrator flips sprint-status to done at dev time, not the dev session — engine.py:2072-2080 (_dev_review_enabled is hardwired False for bmad-dev-auto → target="done"). engine.py:2080 is the only sprint_advance call site in the codebase. The dev skill sets spec frontmatter status: done unconditionally (step-04-review.md:93) with no acceptance-criteria gate anywhere.
  2. The review session reads the board, sees work that cannot be done, and reverts the status to in-progress — the correct call.
  3. verify_review literally checks status != "done" for both spec and sprint → VerifyOutcome.retry(fixable=False)verify.py:1565-1581.
  4. engine.py:1479-1492: no escalate, no fix, bare continue.
  5. The review loop never re-advances sprint-status, so the condition is unrecoverable. Bounded only by max_review_cycles (reporter's run: 3 wasted passes).
  6. engine.py:1551 → defer + rollback. The review was right and the run treats it as a failure.

review-verify-failed is a journal event kind (engine.py:1473-1478); nothing routes on it.

Why it is deterministically detectable

Three facts make "sprint-status regressed below done" a reliable signal of a deliberate session write, distinguishable from "never got there":

  • engine.py:2080 is the sole sprint_advance site, and it runs at dev time.
  • verify_dev already asserted the story reached done (verify.py:1194-1199).
  • sprintstatus.advance() never regresses.

So at review-verify, STATUS_ORDER.index(sprint) < STATUS_ORDER.index("done") plus the durable knowledge that it got to done ⇒ a session wrote it backwards on purpose. No snapshot needed; the invariant is structural and survives crash-resume for free.

There is no structured review→engine verdict channel today: the review session is a bmad-dev-auto re-invocation with no result.json, and the spec template's warnings: [] frontmatter (spec-template.md:8) is declared but never read.

Fix outline

  • Detection lands in verify_review (verify.py:1565-1581), not the engine loop: new kw-only sprint_reached_done: bool, passed by Engine._verify_review (engine.py:2378-2379) as not self._dev_review_enabled(). All four _verify_review call sites inherit routing through the existing not-retryable→escalate path; only the rescue site (engine.py:1528-1531, bare .ok) needs an edit — capture the outcome, escalate on contradiction instead of misleading-deferring.
  • Keys on sprint status only (orchestrator-owned truth), never spec status (skill-owned, legitimately cycles). Guard: the fresh status must be in STATUS_ORDER with index < index("done"); None/unknown token → conservative retry. Spec status: blocked remains the sanctioned reviewer channel and does not overlap (it pauses before verify).
  • New knob review.on_status_contradiction = "escalate" (default) | "retry" (legacy opt-out), following the review.on_timeout template from [BUG] Review session timeout always RETRY until max_review_cycles; no salvage/converge exit #271 verbatim: policy.py constant ~:31, ReviewPolicy field ~:206, parse/validation ~:781-791, POLICY_TEMPLATE ~:1050, core.toml [review] block after :54, OPTIONS_ENUM in tests/test_settings_schema.py:75-85. Default escalate is justified: the released behaviour is a defect — it burns cycles and then rolls the work back.
  • VerifyOutcome.contradiction: bool = False field + escalate() kwarg (env_fault precedent, model.py:474-499). Journal keeps the review-verify-failed kind and adds a contradiction= field at both emit sites (:1473-1478, :1667-1672); the TUI colorizer is substring-based, no change.
  • Escalation reason text names both sides of the contradiction and the resolution options; it travels via paused_reasonresolve.build_context (resolve.py:110-121).
  • Prompt hardening is explicitly out. Prohibiting the revert in the review prompt would make a correct reviewer comply, and the story would silently commit without sign-off — the contradiction becomes invisible, which is strictly worse.
  • Re-drives rearm budgets; an unresolved contradiction re-escalates with the same reason (each pauses, so no ping-pong). No escalation.py / resolve.py / stories / sweep / TUI changes.

Known limitation (documented, out of scope)

A review that regresses both spec and sprint status never reaches _verify_review (gated at engine.py:1462) and still lands in the non-terminal-status exhaustion-defer path.

Doc fix to fold in here

gates.on_escalation is a dead reserved field — declared at policy.py:64, parsed at :670, with zero readers; pause is hardcoded. The umbrella reporter had set on_escalation = pause and it was inert. Document it as reserved (or read it) as part of this change.

Tests

test_policy.py (default / parse / invalid / template); test_verify.py four cases (escalates — this one is the ablation detector; retry-mode legacy; no-launch-flag; missing/unknown token); test_engine.py (sign-off regression escalates within 2 scripted sessions — with the gate ablated the engine requests a nonexistent 3rd session and fails hard; retry-mode 3-cycle legacy defer; non-regression verify failure still retries; rescue-site contradiction escalates rather than defers, with max_followup_reviews=5 so damping never trips; keep the :5140 fixable-path test green). Run both ablations before trusting the suite (repo rule).

Part of the 0.9.0 field-report umbrella (linked parent).

Metadata

Metadata

Assignees

No one assigned

    Labels

    area:engineOrchestrator engine and run lifecyclebugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions