feat: story manifest breakdown and folder+id dispatch (spec-to-loop contract)#2549
Conversation
🤖 Augment PR SummarySummary: This PR tightens the contract between spec creation and unattended execution by introducing a deterministic Changes:
Technical Notes: Folder+id dispatch relies on 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
|
|
||
| | Field | Type | Required | Meaning | | ||
| |---|---|---|---| | ||
| | `id` | string | yes | Stable identity, unique in the manifest, machine-opaque — compared for equality and prefix-matched in filenames, never parsed. Convention: unpadded integers (`"1"`), optionally epic-qualified composites (`"3-2"`) when the epic sits inside a larger project spec. No zero-padding — execution order comes from the manifest, not filename sort. Never reused or renumbered once assigned. | |
There was a problem hiding this comment.
In src/core-skills/bmad-spec/assets/story-manifest-schema.md line 9, id is treated as an opaque string but is also used for equality checks and as a filename prefix; if YAML parses id: 1 as a number (or if an id contains path/glob characters like /, .., *), folder+id dispatch can fail to match or write/read outside the intended stories/ directory. Consider explicitly requiring ids to parse as strings (e.g., quoted) and restricting them to a safe filename-segment character set in the schema validity rules.
Severity: high
Other Locations
src/bmm-skills/4-implementation/bmad-dev-auto/step-01-clarify-and-route.md:29src/bmm-skills/4-implementation/bmad-dev-auto/SKILL.md:18
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
There was a problem hiding this comment.
Taken. Validity rule 4 now requires ids to be YAML strings, always quoted, containing only letters, digits, and dashes — that covers both the id: 1 number coercion and path/glob characters. In 174887e (the schema file is now assets/stories-schema.md).
| - `blocked` → HALT with status `blocked` and blocking condition `blocked spec supplied`. | ||
| - `done` → set `review_loop_iteration` to `0` in the frontmatter, then **EARLY EXIT** to `./step-04-review.md` for a fresh review pass. (A `done` spec is a completed run, so this starts a follow-up review, not a resumption.) | ||
|
|
||
| If the invocation prompt instead supplies a spec folder and a story id, with no specific spec file path, this is a **folder+id dispatch**: set `spec_folder` (a `{project-root}`-relative or absolute path) and `story_id` from the prompt. Any further prompt text (e.g. `invoke_dev_with` guidance the caller appended) is additional planning context to carry into step-02 — not a competing description of what to implement. |
There was a problem hiding this comment.
In src/bmm-skills/4-implementation/bmad-dev-auto/step-01-clarify-and-route.md line 27, allowing {spec_folder} to be an absolute path means the skill may read/write outside {project-root}, which can break portability and is easy to misuse in unattended runs. Consider constraining {spec_folder} to be {project-root}-relative (or explicitly validating it’s inside {project-root}) before using it for filesystem reads/writes.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
There was a problem hiding this comment.
Not taken. An absolute spec folder is deliberate: the caller is the trust boundary, and real layouts keep the spec folder and the code checkout under different roots — this PR was itself developed that way. Restricting the path to {project-root}-relative would break that layout without adding safety the caller needs.
|
Full spec this branch was derived from (kernel, companions, story manifest, append-only memlog, and all five story records including the end-to-end demonstration): https://gist.github.com/alexeyv/0d80e3b4cd79bb4423c22846fe36f610 The memlog is the interesting read — it carries the complete decision trail from the original Discord thread through every design reversal (status home, slug demotion, handoff collapse, fixed-name discovery) to the final contract. |
📝 WalkthroughWalkthroughThis PR adds a "folder+id dispatch" mode to bmad-dev-auto, letting invocations supply a spec folder plus story id instead of a spec path, with new routing, HALT write-back, and ready-for-dev halt logic. It also adds an optional Story Breakdown workflow and stories.yaml schema to bmad-spec, updating both skills' reference documentation. Changesbmad-dev-auto folder+id dispatch and HALT handling
Story manifest schema and Story Breakdown workflow
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Invoker
participant Step01 as step-01-clarify-and-route.md
participant Manifest as stories.yaml
participant StorySpec as story spec file
participant SkillHalt as SKILL.md HALT
Invoker->>Step01: invoke with spec_folder + story_id
Step01->>Manifest: read and validate manifest
Manifest-->>Step01: matching entry by id
Step01->>StorySpec: locate spec_folder/stories/story_id-*.md
StorySpec-->>Step01: status (draft or other)
Step01->>Step01: derive slug from title/description, set spec_file
alt draft or early routing
Step01-->>Invoker: route to step-02-plan.md
else blocking condition
Step01->>SkillHalt: trigger HALT
SkillHalt->>StorySpec: update or create story spec, append Auto Run Result
SkillHalt-->>Invoker: report blocking condition
end
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/bmm-skills/4-implementation/bmad-dev-auto/SKILL.md`:
- Around line 16-34: The folder+id write-back in the Bmad auto flow is placing
HALT sentinel files in the same canonical stories glob as real story specs,
which causes later resolution in the Route/step-01 path to pick the sentinel
instead of the intended story. Update the logic in the folder+id dispatch flow
so `{spec_file}` for unresolved or ambiguous halts is generated outside the
`stories/{story_id}-*.md` namespace, while keeping the existing resolved
`{story_id}-{slug}.md` path for normal cases. Make sure the change is applied in
the write-back path that decides `{spec_file}` and the skeletal story
creation/update behavior remains unchanged for the real story file.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d7ec3706-87c7-4f4d-b994-0a136c9df25a
📒 Files selected for processing (6)
src/bmm-skills/4-implementation/bmad-dev-auto/SKILL.mdsrc/bmm-skills/4-implementation/bmad-dev-auto/step-01-clarify-and-route.mdsrc/bmm-skills/4-implementation/bmad-dev-auto/step-02-plan.mdsrc/core-skills/bmad-spec/SKILL.mdsrc/core-skills/bmad-spec/assets/story-manifest-schema.mdsrc/core-skills/bmad-spec/customize.toml
alexeyv
force-pushed
the
feat/spec-to-loop-contract
branch
3 times, most recently
from
0de7eed to
844a092
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
| - **If more than one matches**, HALT with status `blocked` and blocking condition `ambiguous story file match`. | ||
| - **If exactly one matches**, set `spec_file` to that path. | ||
| - `draft` (planning was interrupted mid-flight): accumulate cross-story context before resuming — load every other file matching `{spec_folder}/stories/*.md` (every match except `{spec_file}` itself), regardless of `status`, and carry forward each one's **Code Map**, **Design Notes**, **Spec Change Log**, **Tasks & Acceptance** checklist state, and **Auto Run Result** details, where present, as additional planning context for step-02. Then **EARLY EXIT** to `./step-02-plan.md`. | ||
| - Any other recognized `status`: **EARLY EXIT** using the same routing as above. One difference: a `blocked` story HALTs with blocking condition `story already blocked`, not `blocked spec supplied` — the caller did not supply this file; dev-auto found it by id. |
There was a problem hiding this comment.
In src/bmm-skills/4-implementation/bmad-dev-auto/step-01-clarify-and-route.md line 35, the folder+id resume path says it uses the same routing as explicit spec-file resumes, but the done route above also resets review_loop_iteration to 0; it’s unclear whether that reset is intended/required here too when a done story is discovered by id. If the review loop logic relies on review_loop_iteration, this ambiguity could lead to an incorrect follow-up review pass.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
There was a problem hiding this comment.
Taken. The folder+id resume routing now explicitly includes the review_loop_iteration reset for done. In 174887e.
|
|
||
| | Field | Type | Required | Meaning | | ||
| |---|---|---|---| | ||
| | `id` | string | yes | Identity, unique within the file. Compared for equality and used as a filename prefix; carries no other meaning. Use unpadded integers (`"1"`), or composites (`"3-2"`) when the epic sits inside a larger project spec. No zero-padding — execution order is the list order, not filename sort. Pinned once work on the story has started (see Update semantics); until then it may be renumbered. | |
There was a problem hiding this comment.
In src/core-skills/bmad-spec/assets/stories-schema.md line 9, id is specified as a string but YAML will parse unquoted scalars like id: 1 as a number, which can break equality matching against a string {story_id} and any filename-prefix logic downstream. Since id is also used as a filename prefix, allowing path/glob characters (e.g. /, .., *, [ ) can cause unintended matches or writes outside the intended stories/ namespace unless constrained/validated.
Severity: medium
Other Locations
docs/reference/core-tools.md:158src/bmm-skills/4-implementation/bmad-dev-auto/step-01-clarify-and-route.md:31
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
There was a problem hiding this comment.
Taken — same fix as the earlier copy of this finding: validity rule 4 in assets/stories-schema.md (quoted YAML strings, letters/digits/dashes only). In 174887e.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/core-skills/bmad-spec/assets/stories-schema.md`:
- Line 9: The pinning rule in the story schema description is inconsistent with
Update semantics. Update the `id` field guidance so it says a story is pinned
once its story spec file exists, and before that point it may still be
renumbered; keep the wording aligned with the Update semantics reference and
remove the current “work on the story has started” phrasing.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 316335fb-984d-4cda-a8fb-812ffb079599
📒 Files selected for processing (7)
docs/reference/core-tools.mddocs/reference/dev-auto.mdsrc/bmm-skills/4-implementation/bmad-dev-auto/SKILL.mdsrc/bmm-skills/4-implementation/bmad-dev-auto/step-01-clarify-and-route.mdsrc/bmm-skills/4-implementation/bmad-dev-auto/step-02-plan.mdsrc/core-skills/bmad-spec/SKILL.mdsrc/core-skills/bmad-spec/assets/stories-schema.md
🚧 Files skipped from review as they are similar to previous changes (1)
- src/bmm-skills/4-implementation/bmad-dev-auto/step-01-clarify-and-route.md
alexeyv
force-pushed
the
feat/spec-to-loop-contract
branch
from
844a092 to
174887e
Compare
bmad-spec gains an optional, interactive-only Story Breakdown step that derives stories.yaml from the memlog: a fixed-name sibling of SPEC.md listing stories as a simple sequence (list order = execution order), each with id, title, description, and orchestration fields spec_checkpoint, done_checkpoint, and invoke_dev_with. Field definitions and validity rules live in assets/stories-schema.md. Ids are pinned only once a story's spec file exists; un-started stories may be renumbered on re-derive. stories.yaml never carries status. bmad-dev-auto becomes dispatchable per stories.yaml entry: invoked with a spec folder and story id, it reads only that entry's title and description, derives the slug, and creates or resumes the story spec at stories/<id>-<slug>.md just in time. All HALT write-back lands at that id-keyed path (skeletal spec for pre-planning halts). An invocation-prompt directive halts ready-for-dev after planning; re-dispatching the same folder+id resumes via existing status routing. Planning accumulates context from all prior story records in the folder. Reference docs updated to cover both halves.
alexeyv
force-pushed
the
feat/spec-to-loop-contract
branch
from
91ae2eb to
814e3fc
Compare
1 participant
What
Machine contract between planning and unattended execution, in two halves:
stories.yaml(fixed-name sibling of SPEC.md, deliberately not a companion) from the memlog: a simple sequence — list order is execution order — with per-entryid,title,description,spec_checkpoint,done_checkpoint,invoke_dev_with; neverstatus. Ids are pinned once a story's spec file exists; un-started stories may be renumbered on re-derive. Schema + validity rules inassets/stories-schema.md.<spec-folder>/stories/<id>-<slug>.md. All HALT write-back lands at that path (skeletal spec for pre-planning halts — early failures can't masquerade as pending). An invocation-prompt directive haltsready-for-devafter planning for human plan review; re-dispatching the same folder+id resumes via existing status routing. Planning accumulates context from all prior story records in the folder. Reference docs (docs/reference/dev-auto.md,docs/reference/core-tools.md) updated to match.Story lifecycle lives only in story-spec frontmatter (the existing HALT shape, unchanged — downstream tooling already consumes it);
stories.yamlnever records status. Dependency graphs deliberately don't exist at this level — they belong to epics within a larger project, outside this contract.Why
A deterministic orchestrator (bmad-loop) needs dispatchable, machine-readable inputs: identity, ordering, display, and a planner→dev token channel — with story specs created just-in-time by the run that implements them, so plans are never written against repo state that doesn't exist yet.
Validation
npm ci && npm run qualitygreen on HEAD in this exact checkout;validate:skillsclean.donewrite-back, and cross-story context accumulation all observed working (the dispatched story landed a real pytest surface, 133 tests green). The stories.yaml fields were simplified in review hardening after that demo; dev-auto's read surface (title/description by id) is unchanged by it.🤖 Generated with Claude Code