fix(engine): tell dispatched sessions the sprint board is orchestrator-owned

[didostap]

The orchestrator advances sprint-status.yaml as soon as dev verifies, but
the story's single commit lands only after the review loop — so every
session dispatched in between opens on an uncommitted, unattributed board
change with nothing in the repo naming its author. A review session read
that write as a spec violation, reverted it, and the #334 contradiction
gate escalated a story both sessions agreed was finished.

Story dev prompts and the review prompts of sprint and sweep runs now
carry one shared prohibition: never write the board, never revert it, and
a row at done or awaiting-operator is the orchestrator's own bookkeeping
rather than a defect. Review prompts alone add the way out — hand back
through status: blocked. Dev prompts deliberately get no such invitation,
since blocked halts the whole run and a repair session would gain a
sanctioned early exit out of one that otherwise keeps going.

Supersedes the #334 note that review prompts were unchanged by design:
verify_review reads the spec's frontmatter status before it ever reads
the board, so withholding sign-off through the spec already blocks the
commit and the board revert was never the load-bearing channel. Both
entries are unreleased, so #334 is amended in place rather than rebutted.

Also corrects two stale claims in docs/FEATURES.md that described the
board as owned by BMAD skills with the orchestrator only reading it.

Closes #437

Summary by CodeRabbit

• Documentation

  • Clarified sprint-board ownership and update rules across development, review, story, and sweep workflows.
  • Documented that unfinished review work should be marked as blocked, while board reverts remain prohibited.

• Improvements

  • Prompts now provide consistent sprint-board and operator-parking guidance.
  • Stories workflows omit sprint-board instructions when no board is present.

• Tests

  • Added coverage for prompt content, ordering, and workflow-specific behavior.

[coderabbitai]

Walkthrough

The change adds orchestrator-owned sprint-board guidance to development and review prompts. Review prompts can redirect unfinished work to status: blocked. Stories mode omits board guidance. Documentation and tests cover these rules.

Changes

Sprint-board prompt ownership

Layer / File(s)	Summary
Prompt composition and board guidance src/bmad_loop/engine.py	Development and review prompts add board ownership, mutation prohibitions, review-only blocked handback, deferred-work wording, and consistent operator-park spacing.
Workflow-mode overrides src/bmad_loop/stories_engine.py, tests/test_stories_engine.py, tests/test_sweep.py	Stories review prompts omit sprint-board guidance. Bundle development and review prompts retain their mode-specific behavior.
Prompt regressions and documentation tests/test_engine.py, docs/FEATURES.md, CHANGELOG.md	Tests verify prompt content, ordering, punctuation, and disabled parking. Documentation records board ownership and status rules.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related issues

• Review session can revert the orchestrator's own sprint-status write and dead-end the run #437: This change adds sprint-board ownership and no-revert guidance to dispatched review prompts.

Possibly related PRs

• bmad-code-org/bmad-loop#173: Both modify prompt-generation logic in src/bmad_loop/engine.py.
• bmad-code-org/bmad-loop#348: Both modify prompt generation and StoriesEngine overrides for sprint-board handling.

Poem

A rabbit guards the sprint-board gate,
No stray writes may change its state.
Unfinished review work turns blocked,
Stories hop where no board is stocked.
The orchestrator keeps the slate.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: informing dispatched sessions that the sprint board is owned by the orchestrator.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@CHANGELOG.md`:
- Around line 266-280: Replace the verbose Unreleased changelog paragraph with
one concise, imperative summary of the orchestrator-owned sprint board behavior
and shared prompt prohibition. Remove the detailed mode-specific exceptions and
follow-up nuances from CHANGELOG.md; retain those details in the existing
documentation at docs/FEATURES.md.

In `@docs/FEATURES.md`:
- Line 38: Update the sprint-status.yaml ownership description in the feature
documentation to limit orchestrator ownership to writes made during its runtime,
while explicitly preserving write access for external bmad-sprint-planning and
bmad-correct-course skill runs.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 170e0204-c340-4ef3-81bb-e2adb600203c

📥 Commits

Reviewing files that changed from the base of the PR and between 90915aa and e313d59.

📒 Files selected for processing (7)

• CHANGELOG.md
• docs/FEATURES.md
• src/bmad_loop/engine.py
• src/bmad_loop/stories_engine.py
• tests/test_engine.py
• tests/test_stories_engine.py
• tests/test_sweep.py

[greptile-apps]

Greptile Summary

This PR fixes a race condition where the orchestrator writes sprint-status.yaml immediately after dev verification but before the story's single commit lands. A review session dispatched in that window sees an uncommitted, unattributed board change and incorrectly treats it as a spec violation — triggering the #334 contradiction gate on a story both sessions agreed was done.

The fix injects a shared board-ownership prohibition into story dev prompts and all review prompts, with an additional status: blocked hand-back redirect on review prompts only (kept away from dev prompts because blocked halts the entire run). StoriesEngine overrides the new method to "" since it has no sprint board, which also gates out the redirect.

• New _sprint_board_instruction() and _board_handback_redirect() methods on Engine; StoriesEngine overrides the former to empty, dropping both halves from that mode's review prompt via the if board else [] guard in _review_prompt.
• _operator_park_instruction() drops its leading — separator; _generic_dev_prompt now owns all separators and joins board + park clauses with a plain space, using an em dash only after a bare story key.
• Sweep bundle dev prompts carry neither clause (SweepEngine overrides _dev_prompt); sweep review prompts inherit both. The gap for plugin workflow sessions dispatched between the board write and the commit is acknowledged and left as follow-up.

Confidence Score: 5/5

Safe to merge. The change adds two pure string-returning methods and refactors how existing methods are composed; it touches no state machines, no I/O paths, and no verification logic.

The implementation is a narrow, well-contained prompt-text change. The separator refactoring is applied consistently at all three call sites in _generic_dev_prompt. The StoriesEngine override correctly short-circuits both the prohibition and the redirect through a single empty return. Seven new tests pin every behavioral contract: ordering, punctuation seams, backtick-free invariant, per-mode inclusion/exclusion, and ablation guards. No logic outside prompt assembly is touched.

Files Needing Attention: No files require special attention.

Important Files Changed

Filename	Overview
src/bmad_loop/engine.py	Core change: adds _sprint_board_instruction() and _board_handback_redirect() methods; refactors _generic_dev_prompt and _review_prompt to compose board + park clauses with correct separator logic.
src/bmad_loop/stories_engine.py	Adds _sprint_board_instruction() override returning empty string to suppress both board clauses from the review prompt in a mode that has no sprint board.
tests/test_engine.py	Adds 7 new tests covering: clause contract, both halves in the review prompt, correct ordering, punctuation-noise prevention, all dev-prompt branches, no blocked invite on dev prompts, and standalone behavior when parking is disabled.
tests/test_stories_engine.py	Adds one test verifying the stories mode review prompt carries neither the board clause nor the status: blocked hand-back, with an ablation guard.
tests/test_sweep.py	Adds one test confirming sweep bundle dev prompts carry neither board clause half while the inherited sweep review prompt carries both.
CHANGELOG.md	Amends the stale #334 note in place and adds a new Fixed entry describing the board-ownership injection.
docs/FEATURES.md	Corrects two stale claims that described the sprint board as BMAD-skills-owned; both now accurately state the orchestrator owns every in-flight write.

_{Reviews (5): Last reviewed commit: "fix(engine): tell dispatched sessions th..." | Re-trigger Greptile}

[didostap]

Skipping this one. The 80% threshold is not met anywhere in this codebase: engine.py is at 53%, stories_engine.py at 52%, sweep.py at 38%. The functions this PR adds are at 11 of 12, about 92%, so the diff raises coverage rather than lowering it.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.

[pbean]

Thank you for this, @didostap — both for the report and for coming back with a fix. The diagnosis is right and the design is right: naming the board's owner in the prompt is the only channel that reaches the reviewer's actual decision surface, and you found the two halves that matter (a prohibition on every dev leg, a hand-back that rides the review prompt alone because blocked halts the run). I've reconstructed it on a maintainer branch as #491, with you as Co-authored-by.

Closing this one rather than asking you to rebase, because the branch has drifted in a way a rebase would resolve silently and wrongly:

1. The base predates 4d90532 (#433), which rewrote _review_prompt. That commit deleted the affirmative "append them to the deferred-work ledger as NEW entries only" instruction. Post-BMAD-METHOD#2640 the dev primitive records its own defer findings in the spec frontmatter and _harvest_spec_deferrals files them — so the affirmative form files every finding twice. Your _review_prompt hunk restores that sentence, and your LEDGER_SENTENCE test constant pins the deleted wording verbatim, which is the part that would have made the revert look intentional. main now guards it with assert "append" not in prompt.lower() over the whole prompt (tests/test_engine.py), so a verbatim carry is a hard red — but only after the rebase conflict had already been "resolved" in favour of your side.

2. The CHANGELOG hunk amends a sentence that no longer exists. 6615ad5 condensed the #334 entry; the surviving line already reads "status: blocked stays the hand-back channel" — which is exactly what your redirect implements, so the amendment has no purpose left.

3. This repo's CI never ran on it. on: pull_request plus a first-time contributor means every workflow was held for manual approval. Only CodeRabbit and Greptile reported, and both have a track record of green-on-nothing here — so the branch never had a real signal.

The functions themselves are otherwise unchanged between your base and main, so the design transferred cleanly. Three deliberate departures in #491, all argued in its body:

• Narrower redirect trigger. "If you judge the story genuinely unfinished" → "If the story cannot be finished without a human decision". A review pass is itself a dev-primitive run whose job is to fix or defer what it finds; the broader trigger hands it a run-halting early exit on cycle 1 of 3.
• A second clause in the prohibition: the row is "not a defect to fix, and not proof that the work is verified". The board is written before _verify_dev_artifacts runs, and a repair session is dispatched precisely when that verification failed — so it opens on a red tree under a done row, and "not a defect" alone would leave it free to read the row as evidence.
• The empty-when-no-board gate moved inside _board_handback_redirect rather than sitting in _review_prompt, so the invariant is directly assertable and _review_prompt stays structurally identical to the dev seam.

Your nine tests carried over as the starting point; four were rewritten, mostly to replace assertions built from the builder's own return value with test-local literals ("" in s is True and s.index("") is 0, so several of them went vacuous under exactly the ablation they existed to catch). The full A1–A17 ablation table and its results are in #491's description.

The plugin-workflow gap you flagged is tracked and is phase 3 of #437. Thanks again — this was a good catch and a good fix.