Description
The review prompt that bmad-loop dispatches tells the session to keep its hands off
deferred-work.md, but there is no equivalent sentence for sprint-status.yaml. I hit that gap.
Here is the review prompt from my run's journal.jsonl, verbatim:
/bmad-dev-auto - If this review defers new findings, append them to
the deferred-work ledger as NEW entries only; do NOT modify, re-open, or rewrite
existing ledger entries - the orchestrator owns their status and resolution.
So the loop already knows it has to say "hands off, the orchestrator owns this" for one of
its two ledgers. There is nothing like it for the sprint board.
What happened: my story spec's Boundaries section had a line saying "Never edit epics.md,
the PRD, the spine, or sprint-status.yaml." The dev session finished, the orchestrator
flipped the row from backlog to done as its sign-off. The review session then read the
spec, saw the uncommitted done in git status, filed it as a [high] finding for "directly
violating the intent-contract's Never edit ... sprint-status.yaml", reverted the row to
backlog, and committed the revert.
From the reviewer's side that is a defensible read. It had no way to know that write came
from the orchestrator and not from a misbehaving dev session. My spec prohibited writing
the file, and the reviewer turned that into a mandate to revert someone else's write.
The contradiction detection worked exactly as advertised. on_status_contradiction =
"escalate" caught it and paused with a very clear message instead of burning the remaining
review cycles. No complaints there. The escalation is the symptom being caught correctly,
the bug is that it was reachable at all.
Two things I think are worth fixing:
-
Extend the review prompt appendix to cover sprint-status.yaml, same shape as the
deferred-work sentence.
-
Optionally, commit the orchestrator's sprint-status write together with the dev pass
rather than leaving it uncommitted. What the reviewer actually saw was a loose
modification in git status with no author attached to it, which is what made it look
like a violation.
Two things I found while digging that might matter to whoever picks this up:
The CHANGELOG entry for #334 says "Review prompts are unchanged by design: forbidding the
revert would make a correct reviewer comply and the story would commit without sign-off."
I do not think that holds any more. verify_review checks the spec's frontmatter status
first and returns retry there, before it ever reads the board. So a reviewer that withholds
sign-off through the spec already blocks the commit, and the board revert was never the
channel doing the work. Worth noting that #334 is still under Unreleased, so if this gets
fixed the two entries would want to agree.
Also, docs/FEATURES.md line 38 currently says sprint-status.yaml is "owned by BMAD skills;
orchestrator only reads it". That contradicts AGENTS.md and the code, and line 43 of the
same file says the opposite. I suspect that line is part of why I wrote the prohibition
into my spec in the first place, since nothing told me who actually owns the file.
Secondary annoyance from the same run: the review session ran git commit, and earlier in
the reflog, git reset --hard HEAD~1 on main. So the revert is in my history, not just my
worktree. I know scm.isolation = "worktree" is the answer and I am switching, just flagging
that with the default isolation = "none" a review session has unrestricted write access to
the branch it is reviewing.
I have a fix on a local branch already but I do not have push access to this repo,
so I am filing this first. Happy to open a PR if someone can grant access, or someone can
take the approach and run with it.
Steps to reproduce
- Author a story spec whose Boundaries/Constraints section tells the agent not to edit
sprint-status.yaml. This is a pretty natural thing for a spec author to write, since the
file gets described in project-context.md without ever saying who owns it.
- Set review.enabled = true, review.trigger = "recommended", scm.isolation = "none".
- bmad-loop run --epic 1
- Dev session finishes with status: done and followup_review_recommended: true. The
orchestrator flips the sprint-status row to done.
- Review session starts, reads the spec, and finds the modified sprint-status.yaml sitting
in git status with nothing naming its author.
- It files the orchestrator's own write as a finding and reverts it.
Expected behavior
The review session should never touch sprint-status.yaml, and specifically should never
revert a change the orchestrator made to it. The dispatched review prompt already carries
this exact instruction for deferred-work.md, so the sprint board should be getting the same
protection.
The story should have gone to done and the run should have continued to 1-2.
Actual behavior
Review returned status: done with escalations: [] and followup_review_recommended: false.
As far as it was concerned the story passed. But it had also written the sprint-status row
back to backlog and committed that.
The orchestrator then found spec frontmatter at status: done and the board row at backlog,
flagged a status contradiction, and paused the run:
paused [escalation] - review revoked the sprint sign-off for 1-1-...: the orchestrator
advanced the board to 'done' after dev verified, and the review session wrote it back to
'backlog' while leaving the spec frontmatter at 'done'. [...]
Run summary: 1 task, 0 done, 0 deferred, 1 escalated, 3.8M weighted tokens (24.7M raw)
spent on a story where both sessions actually agreed the work was finished.
Screenshots
No response
Which area is this for?
Orchestrator / control loop
bmad-loop Version
0.9.0
Which coding CLI are you using?
Claude (claude)
Operating System
macOS
Relevant log output
{"kind": "session-start", "task_id": "1-1-...-review-1", "role": "review", "adapter": "claude", "model": "sonnet", "prompt": "/bmad-dev-auto <spec-path> - If this review defers new findings, append them to the deferred-work ledger as NEW entries only; do NOT modify, re-open, or rewrite existing ledger entries - the orchestrator owns their status and resolution."}
{"kind": "review-result", "cycle": 1, "status": "done", "followup_review_recommended": false}
{"kind": "review-verify-failed", "reason": "review revoked the sprint sign-off for 1-1-...: the orchestrator advanced the board to 'done' after dev verified, and the review session wrote it back to 'backlog' while leaving the spec frontmatter at 'done'. [...]", "env_fault": false, "contradiction": true}
{"kind": "story-escalated", "story_key": "1-1-..."}
{"kind": "run-paused", "stage": "escalation"}
The reviewer's own writeup, from the spec's Review Triage Log:
- `[high]` `[patch]` `sprint-status.yaml` had been flipped to `done` for this story,
directly violating the intent-contract's `Never edit ... sprint-status.yaml`.
Reverted to `backlog`.
Relevant policy.toml:
[review]
enabled = true
trigger = "recommended"
on_timeout = "retry"
on_status_contradiction = "escalate"
[adapter.review]
name = "claude"
model = "sonnet"
[scm]
isolation = "none"Diagnostic dump (bmad-loop diagnose)
No response
Confirm
Description
The review prompt that bmad-loop dispatches tells the session to keep its hands off
deferred-work.md, but there is no equivalent sentence for sprint-status.yaml. I hit that gap.
Here is the review prompt from my run's journal.jsonl, verbatim:
/bmad-dev-auto - If this review defers new findings, append them to
the deferred-work ledger as NEW entries only; do NOT modify, re-open, or rewrite
existing ledger entries - the orchestrator owns their status and resolution.
So the loop already knows it has to say "hands off, the orchestrator owns this" for one of
its two ledgers. There is nothing like it for the sprint board.
What happened: my story spec's Boundaries section had a line saying "Never edit epics.md,
the PRD, the spine, or sprint-status.yaml." The dev session finished, the orchestrator
flipped the row from backlog to done as its sign-off. The review session then read the
spec, saw the uncommitted done in git status, filed it as a [high] finding for "directly
violating the intent-contract's Never edit ... sprint-status.yaml", reverted the row to
backlog, and committed the revert.
From the reviewer's side that is a defensible read. It had no way to know that write came
from the orchestrator and not from a misbehaving dev session. My spec prohibited writing
the file, and the reviewer turned that into a mandate to revert someone else's write.
The contradiction detection worked exactly as advertised. on_status_contradiction =
"escalate" caught it and paused with a very clear message instead of burning the remaining
review cycles. No complaints there. The escalation is the symptom being caught correctly,
the bug is that it was reachable at all.
Two things I think are worth fixing:
Extend the review prompt appendix to cover sprint-status.yaml, same shape as the
deferred-work sentence.
Optionally, commit the orchestrator's sprint-status write together with the dev pass
rather than leaving it uncommitted. What the reviewer actually saw was a loose
modification in git status with no author attached to it, which is what made it look
like a violation.
Two things I found while digging that might matter to whoever picks this up:
The CHANGELOG entry for #334 says "Review prompts are unchanged by design: forbidding the
revert would make a correct reviewer comply and the story would commit without sign-off."
I do not think that holds any more. verify_review checks the spec's frontmatter status
first and returns retry there, before it ever reads the board. So a reviewer that withholds
sign-off through the spec already blocks the commit, and the board revert was never the
channel doing the work. Worth noting that #334 is still under Unreleased, so if this gets
fixed the two entries would want to agree.
Also, docs/FEATURES.md line 38 currently says sprint-status.yaml is "owned by BMAD skills;
orchestrator only reads it". That contradicts AGENTS.md and the code, and line 43 of the
same file says the opposite. I suspect that line is part of why I wrote the prohibition
into my spec in the first place, since nothing told me who actually owns the file.
Secondary annoyance from the same run: the review session ran git commit, and earlier in
the reflog, git reset --hard HEAD~1 on main. So the revert is in my history, not just my
worktree. I know scm.isolation = "worktree" is the answer and I am switching, just flagging
that with the default isolation = "none" a review session has unrestricted write access to
the branch it is reviewing.
I have a fix on a local branch already but I do not have push access to this repo,
so I am filing this first. Happy to open a PR if someone can grant access, or someone can
take the approach and run with it.
Steps to reproduce
sprint-status.yaml. This is a pretty natural thing for a spec author to write, since the
file gets described in project-context.md without ever saying who owns it.
orchestrator flips the sprint-status row to done.
in git status with nothing naming its author.
Expected behavior
The review session should never touch sprint-status.yaml, and specifically should never
revert a change the orchestrator made to it. The dispatched review prompt already carries
this exact instruction for deferred-work.md, so the sprint board should be getting the same
protection.
The story should have gone to done and the run should have continued to 1-2.
Actual behavior
Review returned status: done with escalations: [] and followup_review_recommended: false.
As far as it was concerned the story passed. But it had also written the sprint-status row
back to backlog and committed that.
The orchestrator then found spec frontmatter at status: done and the board row at backlog,
flagged a status contradiction, and paused the run:
paused [escalation] - review revoked the sprint sign-off for 1-1-...: the orchestrator
advanced the board to 'done' after dev verified, and the review session wrote it back to
'backlog' while leaving the spec frontmatter at 'done'. [...]
Run summary: 1 task, 0 done, 0 deferred, 1 escalated, 3.8M weighted tokens (24.7M raw)
spent on a story where both sessions actually agreed the work was finished.
Screenshots
No response
Which area is this for?
Orchestrator / control loop
bmad-loop Version
0.9.0
Which coding CLI are you using?
Claude (claude)
Operating System
macOS
Relevant log output
{"kind": "session-start", "task_id": "1-1-...-review-1", "role": "review", "adapter": "claude", "model": "sonnet", "prompt": "/bmad-dev-auto <spec-path> - If this review defers new findings, append them to the deferred-work ledger as NEW entries only; do NOT modify, re-open, or rewrite existing ledger entries - the orchestrator owns their status and resolution."} {"kind": "review-result", "cycle": 1, "status": "done", "followup_review_recommended": false} {"kind": "review-verify-failed", "reason": "review revoked the sprint sign-off for 1-1-...: the orchestrator advanced the board to 'done' after dev verified, and the review session wrote it back to 'backlog' while leaving the spec frontmatter at 'done'. [...]", "env_fault": false, "contradiction": true} {"kind": "story-escalated", "story_key": "1-1-..."} {"kind": "run-paused", "stage": "escalation"} The reviewer's own writeup, from the spec's Review Triage Log: - `[high]` `[patch]` `sprint-status.yaml` had been flipped to `done` for this story, directly violating the intent-contract's `Never edit ... sprint-status.yaml`. Reverted to `backlog`. Relevant policy.toml: [review] enabled = true trigger = "recommended" on_timeout = "retry" on_status_contradiction = "escalate" [adapter.review] name = "claude" model = "sonnet" [scm] isolation = "none"Diagnostic dump (bmad-loop diagnose)
No response
Confirm