feat(stories): engine, adapter + HITL checkpoints for folder+id dispatch (Phase 2)#77
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
🤖 Augment PR SummarySummary: Adds “stories mode” (folder+id dispatch) to bmad-loop so runs can be driven from a typed Changes:
Tests: Adds coverage for StoriesEngine scheduling/pause/resume seams, adapter deterministic read-back, CLI/policy/state round-trips, install probing, and verify gates for stories mode. 🤖 Was this summary useful? React with 👍 or 👎 |
| base = folder if folder.is_absolute() else Path(spec.cwd) / folder | ||
| deadline = time.monotonic() + RESULT_GRACE_S | ||
| while True: | ||
| state = stories.resolve_story_spec(base, story_key) |
There was a problem hiding this comment.
src/bmad_loop/adapters/generic.py:510: _stories_result_json() no longer uses handle.launched_ns (unlike the mtime-scan path), so an already-terminal story spec from a previous step (notably the pre-existing done spec when starting a follow-up review session) could be misread as this session’s completion even if the session produced no new output.
Severity: high
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| result = devcontract.synthesize_result(state.path, story_key=story_key or None) | ||
| if result.result_json is not None: | ||
| return result.result_json | ||
| if not wait or time.monotonic() >= deadline: |
There was a problem hiding this comment.
src/bmad_loop/adapters/generic.py:515: If resolve_story_spec() yields KIND_AMBIGUOUS (multiple <id>-*.md matches), this currently just waits and then returns None, which will look like a generic stall rather than an actionable “ambiguous story file” error requiring operator intervention.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| print(e, file=sys.stderr) | ||
| return 1 | ||
| if stories_on: | ||
| problem = _validate_stories_folder(paths, spec_folder) |
There was a problem hiding this comment.
src/bmad_loop/cli.py:323: In stories mode, a bad --story id isn’t validated here (only in _dry_run_stories), so the run can start, create state, and then crash later when StoriesEngine calls stories.schedule(..., selector=...) and gets StoriesError for an unknown id.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| help="force stories mode: dispatch the epic spec folder's stories.yaml by " | ||
| "folder+id (overrides [stories].source)", | ||
| ) | ||
| run_p.add_argument("--epic", type=int, help="only stories from this epic (sprint mode)") |
There was a problem hiding this comment.
…tch (Phase 2)
Wire "stories mode" (BMAD-METHOD #2549) into the loop as an opt-in engine
variant alongside the default sprint-status flow — building on the Phase 1
contract layer. Covers the folder+id engine/adapter plus the human-in-the-loop
plan/story checkpoints and sentinel recovery.
Engine + adapter:
- StoriesEngine(Engine): a thin override layer like SweepEngine. `_pick_next`
runs the linear schedule from stories.py (re-validated fresh every pick,
within-run skip set mirroring sprint's base_skip, blocked/sentinel/ambiguous
→ pause for resolve). `_dev_prompt` emits the folder+id dispatch
(`/bmad-dev-auto Spec folder: <rel>. Story id: <id>.` + verbatim
`invoke_dev_with`). `_post_dev_state_sync` is a no-op (no sprint board),
`_verify_dev_artifacts`→verify_dev_stories, and `_verify_review`→
verify_review_stories (drops the sprint-status gate).
- Deterministic adapter read-back: GenericDevAdapter resolves the id-keyed story
spec via stories.resolve_story_spec when BMAD_LOOP_SPEC_FOLDER is set (new env
seam on Engine._run_session), skipping the mtime scan; a relative folder is
rebased against spec.cwd for worktree isolation.
- [stories] policy (source = sprint-status|stories, spec_folder; no
continue_independent) mirroring [review] — dataclass, validation, core.toml
schema section, template. RunState pins source + spec_folder so resume/resolve
rebuild the right engine without re-reading policy.
- Preflight content-probe (install.missing_stories_support): stories mode needs
a bmad-dev-auto whose step-01 carries folder+id dispatch; fail loud, not at
dispatch time. `run --spec <folder>` forces stories mode; `--dry-run` prints
the linear schedule (checkpoints, live on-disk state); `--story <id>` filters.
- verify_dev_stories proof-of-work also excludes the spec folder's stories/ +
stories.yaml so a spec-only story never reads as implementation work.
HITL checkpoints (per-story, independent — a story may set both and pause twice):
- spec_checkpoint (two-leg plan-halt): `_plan_halt_leg` reads on-disk state —
leg 1 dispatches `Halt after planning.` + BMAD_LOOP_PLAN_HALT (adapter
synthesizes ready-for-dev as a `plan_halt` terminal), verify_dev_stories(
plan_halt=True) gates the plan (ready-for-dev, no proof-of-work, no build/test
via the `_run_verify_commands_after_dev` seam), then StoriesEngine pauses at
PAUSE_PLAN_CHECKPOINT. Resume re-drives leg 2 (plain folder+id → implement) via
`_resume_after_dev_verify`, keyed off StoryTask.plan_checkpoint_pending; the
on-disk status (not a flag) keeps prompt + env in lock-step.
- done_checkpoint: after a story commits, `_after_story` pauses at
PAUSE_STORY_CHECKPOINT — skipped when the story was the last to dispatch. Fires
from both _loop and _finish_inflight, always after worktree integration, so a
committed unit is merged before the run stops.
- Blocked/sentinel/ambiguous wedge (_pause_wedged) records an ESCALATED task
(spec path attached), so `resolve`/rearm_escalation and the resolved re-drive
flow through the same machinery as an in-run escalation — no defer-and-continue.
- Sentinel recovery in runs.rearm_escalation: a fixed-slug <id>-unresolved.md /
<id>-ambiguous.md is preserved under {run_dir}/sentinels/, journaled
`sentinel-cleared` with its blocking condition, then deleted so the re-dispatch
starts clean (PENDING → re-plan).
- New pause consts PAUSE_PLAN_CHECKPOINT / PAUSE_STORY_CHECKPOINT + StoryTask
.plan_checkpoint_pending (serialized). Journal events: plan-halt,
checkpoint-pause, sentinel-cleared, stories-validated. Base engine seams are
no-ops for sprint/sweep; the TUI keeps reusing the CLI resume/resolve paths.
Tests: StoriesEngine happy path / scheduling / prompt seams / resume round-trip,
plan-checkpoint pause/resume round-trip, story-checkpoint pause incl. skip-if-last,
additive spec+done double-pause, sentinel re-arm (preserved copy + journal + clean
re-dispatch), adapter id-keyed + plan_halt read-back, [stories] policy matrix,
install probe, CLI dry-run/validate, RunState + plan_checkpoint_pending round-trip,
verify_dev_stories plan_halt gate, verify_review_stories. Full sandbox E2E matrix
is deferred to Phase 4.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
e94b265 to
3171264
Compare
Phase 2 of the spec-to-loop contract (BMAD-METHOD #2549, merged), now also carrying the Phase 3 human-in-the-loop checkpoints (folded into this branch). Stacked on #65 (Phase 1) — base is
feat/spec-to-loop-stories-phase1, notmain, because this builds directly on Phase 1'sstories.py/verify_dev_stories/ devcontract plan-halt seam. Review/merge #65 first.Re-pinned to the merged #2549 text before coding: field names, the seven blocking-condition strings, and the
Halt after planning.phrasing all match verbatim — no drift.Engine + adapter
StoriesEngine(Engine)— a thin override layer, same patternSweepEngineproves:_pick_next: linear schedule fromstories.py, re-validated fresh every pick (id-stability rule F), a within-run skip set mirroring the sprint engine'sbase_skip, and a blocked/sentinel/ambiguous story → pause for resolve (PAUSE_ESCALATION, cannot leapfrog)._dev_prompt: folder+id dispatch —/bmad-dev-auto Spec folder: <rel>. Story id: <id>.+ verbatiminvoke_dev_with, folder always project-relative._post_dev_state_syncno-op (no sprint board);_verify_dev_artifacts→verify_dev_stories;_verify_review→verify_review_stories(drops the sprint-status gate).GenericDevAdapterresolves the id-keyed story spec viastories.resolve_story_specwhenBMAD_LOOP_SPEC_FOLDERis set (new env seam onEngine._run_session), skipping the mtime scan; a relative folder is rebased againstspec.cwdfor worktree isolation.[stories]policy (source = sprint-status|stories,spec_folder; nocontinue_independent) mirroring[review]— dataclass + validation +core.tomlschema section + template.RunStatepinssource+spec_folderso resume/resolve rebuild the right engine without re-reading policy.install.missing_stories_supportcontent-probes bmad-dev-auto for folder+id dispatch (fail loud, not at dispatch time);run --spec <folder>forces stories mode;--dry-runprints the linear schedule (checkpoint badges, live on-disk state);--story <id>filters.verify_dev_storiesproof-of-work also excludes the spec folder'sstories/+stories.yaml, so a spec-only story never reads as implementation work.HITL checkpoints
Per-story and independent — a story may set both flags and pause twice.
spec_checkpoint(two-leg plan-halt) —_plan_halt_legreads on-disk state: leg 1 dispatchesHalt after planning.+BMAD_LOOP_PLAN_HALT(the adapter synthesizesready-for-devas aplan_haltterminal),verify_dev_stories(plan_halt=True)gates the plan (ready-for-dev, no proof-of-work / build-test via the_run_verify_commands_after_devseam), then the run pauses atPAUSE_PLAN_CHECKPOINT. Resume re-drives leg 2 (a plain folder+id → implement dispatch) via_resume_after_dev_verify, keyed offStoryTask.plan_checkpoint_pending. Reading on-disk status (not a flag) keeps the prompt'sHalt after planning.leg and the env in lock-step.done_checkpoint— after a story commits,_after_storypauses atPAUSE_STORY_CHECKPOINT, skipped when the story was the last to dispatch. Fires from both_loopand_finish_inflight, always after worktree integration, so a committed unit is merged before the run stops._pause_wedgedrecords an ESCALATED task (spec path attached), soresolve/rearm_escalationand the resolved re-drive flow through the same machinery as an in-run escalation. No defer-and-continue.runs.rearm_escalation) — a fixed-slug<id>-unresolved.md/<id>-ambiguous.mdis preserved under{run_dir}/sentinels/, journaledsentinel-clearedwith its blocking condition, then deleted so the re-dispatch starts clean (PENDING→ re-plan).PAUSE_PLAN_CHECKPOINT/PAUSE_STORY_CHECKPOINT+StoryTask.plan_checkpoint_pending(serialized). Journal events:plan-halt,checkpoint-pause,sentinel-cleared,stories-validated. The base-engine seams are no-ops for sprint/sweep; the TUI needs no changes — it already shells out to the CLIresume/resolvepaths.Tests
StoriesEngine happy path / scheduling (skip-done, blocked-pause, sentinel-pause, selector) / prompt seams / crash-resume round-trip; plan-checkpoint pause/resume round-trip, story-checkpoint pause incl. skip-if-last, additive spec+done double-pause, sentinel re-arm (preserved copy + journal + clean re-dispatch); adapter id-keyed +
plan_haltread-back;[stories]policy matrix; install probe; CLI dry-run/validate/mode-detection;RunState+plan_checkpoint_pendinground-trip;verify_dev_storiesplan_halt gate;verify_review_stories. Full suite green;trunk checkclean.The full sandbox E2E matrix (two-story happy path, blocked→resolve→re-dispatch, worktree isolation, sprint-mode regression) + TUI polish + docs/CHANGELOG are Phase 4.
🤖 Generated with Claude Code