From ae88bc7ba689b46018b8fa67ec5a14b16251eb00 Mon Sep 17 00:00:00 2001 From: Guillermo Montero Date: Thu, 17 Sep 2026 19:34:01 +0200 Subject: [PATCH 1/2] fix(policy): warn on legacy 'retry' on_status_contradiction mode --- CHANGELOG.md | 2 ++ src/bmad_loop/data/settings/core.toml | 2 +- src/bmad_loop/policy.py | 11 ++++++++++- tests/test_policy.py | 13 +++++++++++++ 4 files changed, 26 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a55a12d0d..347208100 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -394,6 +394,8 @@ breaking changes may land in a minor release. cannot be read at a graceful stop journals `sweep-remaining-estimate-unreadable` next to the `run-stop` row, so a withheld estimate says why it was withheld. +- bmad-loop now warns when the legacy `review.on_status_contradiction = "retry"` mode is configured. + - **`bmad-loop diagnose --json` reports `schema_version: 4`.** Journal `path` values become `path_present`; stale-restore and merge filename lists become counts. diff --git a/src/bmad_loop/data/settings/core.toml b/src/bmad_loop/data/settings/core.toml index 23363de67..920a6a656 100644 --- a/src/bmad_loop/data/settings/core.toml +++ b/src/bmad_loop/data/settings/core.toml @@ -58,7 +58,7 @@ 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" +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; slated for removal in 0.12.0 (#813)" [[section]] name = "stories" diff --git a/src/bmad_loop/policy.py b/src/bmad_loop/policy.py index 319af2467..a95cc638d 100644 --- a/src/bmad_loop/policy.py +++ b/src/bmad_loop/policy.py @@ -251,7 +251,8 @@ class ReviewPolicy: # 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. + # review cycles to limits.max_review_cycles, then defer. Slated for + # removal in 0.12.0 (#813); loading it now emits a DeprecationWarning. # 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. @@ -1047,6 +1048,14 @@ def loads(text: str, plugin_schemas: dict[str, Any] | None = None) -> Policy: f"{sorted(REVIEW_ON_STATUS_CONTRADICTION_MODES)}:" f" got {review.on_status_contradiction!r}" ) + if review.on_status_contradiction == "retry": + warnings.warn( + 'review.on_status_contradiction = "retry" is legacy (superseded by ' + '"escalate" per #334) and will be removed in 0.12.0 (#813). Switch to ' + '"escalate" (the default).', + DeprecationWarning, + stacklevel=3, + ) stories = StoriesPolicy( source=_typed_str(stories_d, "stories", "source", StoriesPolicy.source).strip(), spec_folder=_typed_str( diff --git a/tests/test_policy.py b/tests/test_policy.py index 1cadd597c..f18d1d5a2 100644 --- a/tests/test_policy.py +++ b/tests/test_policy.py @@ -1,6 +1,7 @@ import json import re import sys +import warnings import pytest @@ -73,6 +74,18 @@ def test_review_on_status_contradiction_invalid(): policy.loads('[review]\non_status_contradiction = "defer"\n') +def test_review_on_status_contradiction_retry_warns_escalate_does_not(): + # "retry" is legacy (superseded by "escalate" per #334) and slated for + # removal in 0.12.0 (#813); loading it must warn. The "escalate" default + # must stay silent. + with pytest.warns(DeprecationWarning, match="retry"): + policy.loads('[review]\non_status_contradiction = "retry"\n') + with warnings.catch_warnings(): + warnings.simplefilter("error") + policy.loads('[review]\non_status_contradiction = "escalate"\n') + policy.loads("") + + def test_stories_defaults(): pol = policy.loads("") assert pol.stories.source == "sprint-status" From 9080f630e2e1742fd8fe251e83a0c98170297384 Mon Sep 17 00:00:00 2001 From: Guillermo Montero Date: Wed, 23 Sep 2026 14:59:25 +0200 Subject: [PATCH 2/2] fix(policy): target 0.13.0 for legacy retry-mode removal, fix CHANGELOG placement MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Same bug already fixed in #810/#811: 0.12.0 (tag v0.12.0, released 2026-09-20) shipped without removing review.on_status_contradiction = "retry", so pinning the removal notice to 0.12.0 is false the moment this merges. Retargeted the comment, the runtime DeprecationWarning, core.toml's field description, and the test comment to 0.13.0 (this repo's minor-bump convention for deprecation removals). Also moved the CHANGELOG entry from the already-published ## [0.12.0] section into ## [Unreleased] / ### Changed, and reworded it as a terse imperative per this repo's own CHANGELOG guideline. Left the DeprecationWarning-visibility question (CodeRabbit: default Python filters suppress DeprecationWarning outside __main__, so this warning may not reach a user in normal CLI use) unresolved here — it's a pre-existing pattern shared with _fold_deprecated_engine, not specific to this PR, and fixing it well means a CLI-wide decision about warning categories/filters that's out of scope for a CHANGELOG/version fix; opening a separate issue for it. --- CHANGELOG.md | 4 ++-- src/bmad_loop/data/settings/core.toml | 2 +- src/bmad_loop/policy.py | 4 ++-- tests/test_policy.py | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 347208100..b6907416e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,8 @@ breaking changes may land in a minor release. ### Changed +- Warn when the legacy `review.on_status_contradiction = "retry"` mode is + configured (#813). - Register hooks through the installed `bmad-loop relay ` command. Upgrading invalidates Codex hook trust: Codex re-prompts at the next launch, and hooks silently do not fire until the new commands are accepted. Re-run `bmad-loop init` to migrate @@ -394,8 +396,6 @@ breaking changes may land in a minor release. cannot be read at a graceful stop journals `sweep-remaining-estimate-unreadable` next to the `run-stop` row, so a withheld estimate says why it was withheld. -- bmad-loop now warns when the legacy `review.on_status_contradiction = "retry"` mode is configured. - - **`bmad-loop diagnose --json` reports `schema_version: 4`.** Journal `path` values become `path_present`; stale-restore and merge filename lists become counts. diff --git a/src/bmad_loop/data/settings/core.toml b/src/bmad_loop/data/settings/core.toml index 920a6a656..bbd6cc296 100644 --- a/src/bmad_loop/data/settings/core.toml +++ b/src/bmad_loop/data/settings/core.toml @@ -58,7 +58,7 @@ 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; slated for removal in 0.12.0 (#813)" +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; slated for removal in 0.13.0 (#813)" [[section]] name = "stories" diff --git a/src/bmad_loop/policy.py b/src/bmad_loop/policy.py index a95cc638d..101ed1da5 100644 --- a/src/bmad_loop/policy.py +++ b/src/bmad_loop/policy.py @@ -252,7 +252,7 @@ class ReviewPolicy: # down onto a rollback. # "retry" — legacy behavior: treat it as an ordinary verify failure, burn # review cycles to limits.max_review_cycles, then defer. Slated for - # removal in 0.12.0 (#813); loading it now emits a DeprecationWarning. + # removal in 0.13.0 (#813); loading it now emits a DeprecationWarning. # 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. @@ -1051,7 +1051,7 @@ def loads(text: str, plugin_schemas: dict[str, Any] | None = None) -> Policy: if review.on_status_contradiction == "retry": warnings.warn( 'review.on_status_contradiction = "retry" is legacy (superseded by ' - '"escalate" per #334) and will be removed in 0.12.0 (#813). Switch to ' + '"escalate" per #334) and will be removed in 0.13.0 (#813). Switch to ' '"escalate" (the default).', DeprecationWarning, stacklevel=3, diff --git a/tests/test_policy.py b/tests/test_policy.py index f18d1d5a2..46b00c2f0 100644 --- a/tests/test_policy.py +++ b/tests/test_policy.py @@ -76,7 +76,7 @@ def test_review_on_status_contradiction_invalid(): def test_review_on_status_contradiction_retry_warns_escalate_does_not(): # "retry" is legacy (superseded by "escalate" per #334) and slated for - # removal in 0.12.0 (#813); loading it must warn. The "escalate" default + # removal in 0.13.0 (#813); loading it must warn. The "escalate" default # must stay silent. with pytest.warns(DeprecationWarning, match="retry"): policy.loads('[review]\non_status_contradiction = "retry"\n')