Skip to content

fix(engine): reconcile transient in-review frontmatter to done (0.7.10)#27

Merged
pbean merged 1 commit into
mainfrom
release/0.7.10
Jun 30, 2026
Merged

fix(engine): reconcile transient in-review frontmatter to done (0.7.10)#27
pbean merged 1 commit into
mainfrom
release/0.7.10

Conversation

@pbean

@pbean pbean commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator

Problem

A bmad-dev-auto session that dies in its step-04 Finalize tail leaves the spec frontmatter at the transient in-review marker (set at the start of step-04) while the ## Auto Run Result prose already says Status: done. This is the same stale-frontmatter gate bug fixed in 0.7.8, with a different stale value.

Since every gate reads only frontmatter, completed, tested work false-deferred → was rolled back → the open ledger entry got re-bundled on the next sweep → re-run and discarded again. Observed in ~/dev/Lights-Out: a repeating sweep loop burning ~47M tokens/cycle (e.g. DW-153, whose spec showed frontmatter in-review + prose Status: done + 1432/1432 EditMode & 222/222 PlayMode green).

The 0.7.8 reconcile (_reconcile_generic_terminal_status) could not catch it because in-review was deliberately excluded from devcontract.RECONCILABLE_FROM.

Fix

That exclusion only ever protected the legacy bmad-auto-dev fork's review-handoff, where in-review was a real terminal. That fork is retired; on the sole generic bmad-dev-auto path _dev_review_enabled() is always False, so the success status is always done and in-review is never a deliberate terminal.

  • Add "in-review" to devcontract.RECONCILABLE_FROM (one token) + rationale comment.
  • No logic change in engine.py (comment-only): the existing reconcile mutate-block already re-reads the frontmatter and re-attaches result_json["followup_review_recommended"] when advancing to done, so a followup_review_recommended: true spec still triggers the follow-up review pass (a second bmad-dev-auto run — there is no separate bmad-auto-review skill).
  • The prose Status: done requirement and every deterministic gate (worktree diff, dw-ids, verify commands, ledger) are unchanged — reconcile repairs bookkeeping only, it cannot pass uncompleted work.

Scope is intentionally minimal: no prose-absent ("Mode B") handling, no adapter nudge.

Tests

  • test_devcontract.py — doctrine test updated to assert in-review ∈ allowlist (and done/blocked still excluded); reset_spec_status parametrize gains in-review.
  • test_engine.pytest_generic_reconcile_advances_in_review_frontmatter_done, _preserves_followup_review_true, _followup_false_skips_review.
  • test_sweep.pytest_generic_bundle_reconcile_closes_ledger_on_in_review_frontmatter.

Full suite 1226 passed, trunk check clean. Version bumped 0.7.9 → 0.7.10 (sync_version.py) + CHANGELOG [0.7.10].

Recovery note

Rolled-back work left no code in the tree; the affected Lights-Out ledger entries stay open and will re-run + commit on the next post-fix sweep. No manual ledger edits.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Improved status reconciliation so items left in a transient in-review state can now progress to done correctly.
    • Preserved follow-up review behavior after reconciliation, including recommended reviews and related completion tracking.
    • Fixed sweep and bundle flows so open work is closed and ledger updates are applied as expected when in-review appears.

….10)

A `bmad-dev-auto` session that dies in its step-04 Finalize tail leaves the
spec frontmatter at the transient `in-review` marker while the
`## Auto Run Result` prose already says `Status: done` — the same stale-
frontmatter gate bug as 0.7.8 with a different value. The gates read only
frontmatter, so completed, tested work false-deferred and was rolled back,
then re-bundled on the next sweep (a ~47M-token/cycle loop).

The 0.7.8 reconcile excluded `in-review` only to protect the legacy
`bmad-auto-dev` review-handoff. That fork is retired; on the sole generic
path `in-review` is never a deliberate terminal, so add it to
`devcontract.RECONCILABLE_FROM`. The existing reconcile mutate-block already
re-reads the frontmatter and re-attaches `followup_review_recommended` when it
advances to done, so a `followup_review_recommended: true` spec still triggers
the follow-up review pass; the prose `Status: done` requirement and every
deterministic gate are unchanged.

Tests: doctrine + reset_spec_status param (devcontract), 3 engine cases
(advance-in-review, followup-true-runs-review, followup-false-skips), 1 sweep
case (bundle ledger closes). Version 0.7.9 -> 0.7.10 + CHANGELOG.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 30, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: eaf74c85-9a57-44af-8fb8-7c92866c4661

📥 Commits

Reviewing files that changed from the base of the PR and between bfe7562 and de13ed8.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (11)
  • .claude-plugin/marketplace.json
  • CHANGELOG.md
  • module.yaml
  • pyproject.toml
  • src/automator/__init__.py
  • src/automator/data/skills/bmad-auto-setup/assets/module.yaml
  • src/automator/devcontract.py
  • src/automator/engine.py
  • tests/test_devcontract.py
  • tests/test_engine.py
  • tests/test_sweep.py

Walkthrough

Adds "in-review" to the RECONCILABLE_FROM frozenset in devcontract.py, allowing the reconciler to rewrite transient in-review frontmatter status to done before gates run. Engine docstrings and an inline comment are updated accordingly. Three engine tests, one sweep test, and updated devcontract tests cover the new behavior. All version fields are bumped to 0.7.10.

Changes

in-review Frontmatter Reconciliation + v0.7.10

Layer / File(s) Summary
RECONCILABLE_FROM expansion and engine docstring
src/automator/devcontract.py, src/automator/engine.py
RECONCILABLE_FROM gains "in-review"; _reconcile_generic_terminal_status docstring and early-return comment updated to reflect the expanded allowlist.
Reconciliation tests
tests/test_devcontract.py, tests/test_engine.py, tests/test_sweep.py
Updated devcontract assertions include "in-review" as reconcilable; three new engine tests cover basic reconciliation, follow-up review preserved, and follow-up review skipped; one new sweep test covers bundle close with transient in-review frontmatter.
Version bump to 0.7.10
pyproject.toml, module.yaml, src/automator/__init__.py, src/automator/data/skills/bmad-auto-setup/assets/module.yaml, .claude-plugin/marketplace.json, CHANGELOG.md
All version fields updated from 0.7.9 to 0.7.10; changelog entry added describing the fix.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐇 The in-review ghost once haunted the gate,
Confusing the engine on matters of state.
Now RECONCILABLE_FROM knows the right path—
Transient no more, it escapes aftermath!
From in-review to done, the frontmatter hops,
Like a bunny at dusk that never quite stops. 🌙

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 76.92% 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
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is concise and accurately describes the main change: reconciling transient in-review frontmatter to done.
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.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch release/0.7.10

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

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

@augmentcode

augmentcode Bot commented Jun 30, 2026

Copy link
Copy Markdown
🤖 Augment PR Summary

Summary: Fixes stale in-review frontmatter left after generic bmad-dev-auto finalize crashes, preventing false deferrals/rollbacks.
Changes:

  • Add `in-review` to `devcontract.RECONCILABLE_FROM` so reconcile can advance it to `done` when prose status is done.
  • Clarify reconcile behavior/comments in `engine._reconcile_generic_terminal_status`.
  • Add regression tests covering reconcile from `in-review` and preserving follow-up review recommendation.
  • Add sweep/bundle test ensuring ledger closes when spec frontmatter is stuck at `in-review` but prose says done.
  • Bump version to 0.7.10 across packaging/skill metadata and update CHANGELOG.
Technical Notes: Reconcile is bookkeeping-only: it runs only on the generic dev path and still executes all verify gates afterward. Follow-up review behavior is preserved by re-reading frontmatter after reconciliation when setting `followup_review_recommended`.

🤖 Was this summary useful? React with 👍 or 👎

@augmentcode augmentcode Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review completed. No suggestions at this time.

Comment augment review to trigger a new review at any time.

@pbean pbean merged commit f32d922 into main Jun 30, 2026
7 checks passed
@pbean pbean deleted the release/0.7.10 branch June 30, 2026 05:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant