diff --git a/src/bmad_loop/devcontract.py b/src/bmad_loop/devcontract.py index cf83f9c..66fe1cb 100644 --- a/src/bmad_loop/devcontract.py +++ b/src/bmad_loop/devcontract.py @@ -43,6 +43,15 @@ DONE = "done" BLOCKED = "blocked" +# The status a plan-halt dispatch leaves behind: under folder+id dispatch a +# `Halt after planning.` directive makes the skill HALT right after the +# Ready-for-Development gate, at `ready-for-dev`. This is a *successful* terminal +# outcome for that leg (the plan is done, awaiting human review / implementation) +# — but ONLY when the caller asked for the halt. Without the directive the same +# status is a died-mid-flight non-terminal (it stays in RECONCILABLE_FROM), so +# the seam is gated on `plan_halt`, never on the status alone. +PLAN_HALT_STATUS = "ready-for-dev" + # Frontmatter statuses a half-finalized generic spec may be reconciled FROM when # its prose terminal `## Auto Run Result` Status is `done`. Deliberately an # allowlist: anything else (already-`done`, `blocked`, or an unknown custom token) @@ -167,6 +176,7 @@ def synthesize_result( *, story_key: str | None, dw_ids: list[str] | None = None, + plan_halt: bool = False, ) -> SynthResult: """Build the legacy result dict from the generic skill's on-disk spec. @@ -178,6 +188,19 @@ def synthesize_result( outcome is rendered as a single CRITICAL escalation so ``decide_dev`` PAUSEs unchanged — the generic skill has no severity tiers, and per the integration decision every block maps to PAUSE. + + ``plan_halt`` is the stories-mode expected-terminal seam: on the first leg of + a ``spec_checkpoint`` dispatch the caller sends ``Halt after planning.`` and + the skill HALTs at ``ready-for-dev``. Passing ``plan_halt=True`` treats that + status as a *successful* terminal — the returned dict carries + ``status="ready-for-dev"``, no escalation, and a ``plan_halt=True`` marker so + verify/engine expect a planned (not implemented) spec. Without ``plan_halt`` + the default is unchanged: ``ready-for-dev`` is non-terminal (died mid-flight) + and returns ``SynthResult(None, True)``. This composes with the engine's + ``_reconcile_generic_terminal_status`` — that path only reconciles a spec + whose prose ``## Auto Run Result`` says ``done`` while the frontmatter lags, + so a plan-halt ``ready-for-dev`` (no such prose) is never reconciled to + ``done`` and this leg's success outcome is not clobbered. """ fm = read_frontmatter(spec_path) fm_status = str(fm.get("status", "")).strip().lower() @@ -185,8 +208,9 @@ def synthesize_result( spec_path.read_text(encoding="utf-8") if spec_path.is_file() else "" ) + terminal = (DONE, BLOCKED, PLAN_HALT_STATUS) if plan_halt else (DONE, BLOCKED) # Not terminal yet: no result section AND frontmatter not at a terminal state. - if not arr.present and fm_status not in (DONE, BLOCKED): + if not arr.present and fm_status not in terminal: return SynthResult(result_json=None, status_consistent=True) # Authoritative status = frontmatter (read off disk). Prose status only @@ -218,6 +242,11 @@ def synthesize_result( # on a blocked exit, so only carry it through on `done`. if status == DONE: result["followup_review_recommended"] = bool(fm.get("followup_review_recommended", False)) + # Mark the clean plan-halt success so verify/engine expect a planned spec + # (status ready-for-dev, no implementation work). Never marked when a block + # escalation is present — that routes to PAUSE, not a plan-review pause. + if plan_halt and status == PLAN_HALT_STATUS and not escalations: + result["plan_halt"] = True return SynthResult(result_json=result, status_consistent=consistent) diff --git a/src/bmad_loop/stories.py b/src/bmad_loop/stories.py new file mode 100644 index 0000000..571d3df --- /dev/null +++ b/src/bmad_loop/stories.py @@ -0,0 +1,361 @@ +"""Model of stories.yaml — the Story Breakdown manifest for "stories mode". + +The optional Story Breakdown step of `bmad-spec` emits ``stories.yaml``, a +fixed-name sibling of ``SPEC.md`` in the spec folder (discovered by name, never +referenced from frontmatter). It is a flat list, one entry per story, in strict +execution order — **there is no ``depends_on`` field**, so the schedule is a +single left-to-right scan, not a DAG. Each entry pins a stable, prefix-free, +machine-opaque ``id`` plus ``title``/``description`` and the caller-only knobs +``spec_checkpoint`` / ``done_checkpoint`` / ``invoke_dev_with``. ``status`` is +deliberately absent: bmad-spec is the sole writer of ``stories.yaml`` and +bmad-dev-auto is the sole writer of each story spec's status — the orchestrator +writes neither. + +This module is the strict, typed parser the orchestrator reads it through. The +upstream schema (validity rule 4) already says ids are quoted strings of +letters/digits/dashes, but an LLM-authored file may still emit an unquoted +``id: 1`` (PyYAML -> int) or ``id: 3.5`` (-> float); we ``str()``-normalize then +charset-validate as defense-in-depth. Everything here is pure contract: no +engine or sprint-mode coupling (only :mod:`verify`'s frontmatter readers, for +the id-keyed disk resolution). +""" + +from __future__ import annotations + +import re +from dataclasses import dataclass +from pathlib import Path + +import yaml + +from .verify import read_frontmatter, status_of + +# Fixed-name discovery, like SPEC.md / .memlog.md — never listed in companions. +STORIES_FILENAME = "stories.yaml" +# Story specs live under /stories/, keyed -.md. +STORIES_SUBDIR = "stories" + +# Schema validity rule 4: ids are letters, digits, and dashes only. Matches the +# upstream authoring rule exactly — ids become filename segments and task keys, +# so a stray character must fail loud, not slip into a path. +ID_RE = re.compile(r"^[A-Za-z0-9]+(-[A-Za-z0-9]+)*$") + +REQUIRED_FIELDS = ("id", "title", "description") + +# The dispatch-protocol read model. Non-terminal statuses a re-dispatch resumes +# from (the session died mid-flight); `done` is terminal-skip; `blocked` stops +# the run. A story spec that is absent reads as PENDING (never dispatched). +RESUMABLE_STATUSES = frozenset({"draft", "ready-for-dev", "in-progress", "in-review"}) +DONE = "done" +BLOCKED = "blocked" + +# resolve_story_spec state kinds. +KIND_PENDING = "pending" # no story spec on disk yet +KIND_PRESENT = "present" # exactly one real story spec; carries .status +KIND_AMBIGUOUS = "ambiguous" # >1 matching file — an anomaly, refuse to pick +KIND_SENTINEL = "sentinel" # the single match is a fixed-slug skeletal sentinel + +# Fixed-slug skeletal specs the skill writes on a pre-planning HALT, kept inside +# the -*.md glob so "no file = pending" holds; recoverable by deletion. +SENTINEL_UNRESOLVED = "unresolved" +SENTINEL_AMBIGUOUS = "ambiguous" +SENTINEL_SLUGS = (SENTINEL_UNRESOLVED, SENTINEL_AMBIGUOUS) + +# schedule() outcomes. +SCHEDULE_NEXT = "next" # .entry is the next story to dispatch +SCHEDULE_COMPLETE = "complete" # every story is done — the run is finished +SCHEDULE_WEDGED = "wedged" # scan stopped on a blocked/sentinel/ambiguous entry + + +class StoriesError(Exception): + pass + + +@dataclass(frozen=True) +class StoryEntry: + """One story in the breakdown. ``id`` is stable once its spec file exists; + the checkpoint flags are independent (a story may set both and pause twice). + ``invoke_dev_with`` is free text appended verbatim to the dispatch prompt — + the single planner->dev channel, never interpreted here.""" + + id: str + title: str + description: str + spec_checkpoint: bool = False + done_checkpoint: bool = False + invoke_dev_with: str = "" + + +@dataclass(frozen=True) +class Stories: + path: Path + entries: tuple[StoryEntry, ...] + + def get(self, story_id: str) -> StoryEntry | None: + sid = str(story_id).strip() + return next((e for e in self.entries if e.id == sid), None) + + +def load_stories(spec_folder: Path | str) -> Stories: + """Parse + validate ``/stories.yaml`` into a typed :class:`Stories`. + + Validates: required fields present, ids unique and prefix-free, no ``status`` + key, id charset. Ids are ``str()``-normalized before validation (int/float + coercion defense). Raises :class:`StoriesError` with the pinned + ``no stories.yaml found`` message when the file is absent. **No DAG / cycle + validation** — the list is strictly linear. + """ + path = Path(spec_folder) / STORIES_FILENAME + if not path.is_file(): + raise StoriesError("no stories.yaml found") + try: + doc = yaml.safe_load(path.read_text(encoding="utf-8")) + except yaml.YAMLError as e: + raise StoriesError(f"stories.yaml is not valid YAML: {path}: {e}") from e + if doc is None or (isinstance(doc, list) and not doc): + raise StoriesError("stories.yaml has no story entries") + if not isinstance(doc, list): + raise StoriesError("stories.yaml must be a top-level list of story entries") + + entries: list[StoryEntry] = [] + seen: set[str] = set() + seen_folded: dict[str, str] = {} # casefolded id -> first id that used it + for index, raw in enumerate(doc): + entry = _parse_entry(raw, index) + if entry.id in seen: + raise StoriesError(f"stories.yaml has a duplicate id {entry.id!r}") + # Story specs resolve by the `-*.md` glob, which is case-insensitive on + # Windows/macOS filesystems (both in the CI matrix), so two ids that differ + # only by case would cross-match the same files. Reject them up front rather + # than let resolution become filesystem-dependent. + folded = entry.id.casefold() + if folded in seen_folded: + raise StoriesError( + f"stories.yaml ids {seen_folded[folded]!r} and {entry.id!r} differ only " + "by case — story specs resolve by the case-insensitive glob -*.md, so " + "on a case-insensitive filesystem (Windows/macOS) they would cross-match" + ) + seen.add(entry.id) + seen_folded[folded] = entry.id + entries.append(entry) + _validate_prefix_free([e.id for e in entries]) + return Stories(path=path, entries=tuple(entries)) + + +def _parse_entry(raw: object, index: int) -> StoryEntry: + if not isinstance(raw, dict): + raise StoriesError(f"stories.yaml entry {index} is not a mapping") + if "status" in raw: + raise StoriesError( + f"stories.yaml entry {index} has a forbidden 'status' key — a story's " + "status lives in its story spec, never in stories.yaml" + ) + story_id = _parse_id(raw, index) + return StoryEntry( + id=story_id, + title=_require_text(raw, "title", story_id), + description=_require_text(raw, "description", story_id), + spec_checkpoint=_bool_field(raw, "spec_checkpoint", story_id), + done_checkpoint=_bool_field(raw, "done_checkpoint", story_id), + invoke_dev_with=_text_field(raw, "invoke_dev_with", story_id), + ) + + +def _parse_id(raw: dict, index: int) -> str: + if raw.get("id") is None: + raise StoriesError(f"stories.yaml entry {index} is missing required field 'id'") + # str()-normalize: schema rule 4 says ids are quoted strings, but an + # LLM-authored file may still emit an unquoted `id: 1` (PyYAML -> int) or + # `id: 3.5` (-> float). Coerce, then charset-validate — a float's `.` fails. + story_id = str(raw["id"]).strip() + if not ID_RE.match(story_id): + raise StoriesError( + f"stories.yaml entry {index} has invalid id {story_id!r}: ids must be " + "letters, digits, and dashes (^[A-Za-z0-9]+(-[A-Za-z0-9]+)*$)" + ) + return story_id + + +def _require_text(raw: dict, key: str, story_id: str) -> str: + value = raw.get(key) + if value is None: + raise StoriesError(f"stories.yaml story {story_id!r} is missing required field {key!r}") + if not isinstance(value, str): + raise StoriesError(f"stories.yaml story {story_id!r} field {key!r} must be a string") + value = value.strip() + if not value: + raise StoriesError(f"stories.yaml story {story_id!r} field {key!r} is empty") + return value + + +def _bool_field(raw: dict, key: str, story_id: str) -> bool: + """A checkpoint flag: bool, defaulting False when missing/null. Strict — a + non-bool (`1`, `"true"`) is a schema error, not a silent truthy coercion. In + Python ``bool`` is an ``int`` subclass, but ``isinstance(1, bool)`` is False, + so an integer 1 is correctly rejected.""" + value = raw.get(key) + if value is None: + return False + if not isinstance(value, bool): + raise StoriesError( + f"stories.yaml story {story_id!r} field {key!r} must be a boolean " + f"(got {type(value).__name__})" + ) + return value + + +def _text_field(raw: dict, key: str, story_id: str) -> str: + """Optional free-text field, defaulting "" when missing/null. Not stripped: + ``invoke_dev_with`` is appended to the dispatch prompt verbatim.""" + value = raw.get(key) + if value is None: + return "" + if not isinstance(value, str): + raise StoriesError(f"stories.yaml story {story_id!r} field {key!r} must be a string") + return value + + +def _validate_prefix_free(ids: list[str]) -> None: + """No id may equal another id plus a ``-suffix`` (schema validity rule 2). + + Story specs are discovered by the ``-*.md`` glob, so if ``3`` and + ``3-2`` were both ids the ``3-*.md`` glob for story ``3`` would also match + ``3-2-slug.md`` — the id would no longer resolve to a single file. ``3`` vs + ``31`` is fine: ``3-*.md`` never matches ``31-slug.md``. + + The check is case-insensitive for the same reason ``load_stories`` rejects + case-only duplicates: on a case-insensitive filesystem ``Auth-*.md`` also + matches ``auth-2-slug.md``, so ``Auth`` and ``auth-2`` collide there too. + Case-only duplicates (equal casefold) are caught earlier in ``load_stories``; + by here every id has a distinct casefold, so this map is unambiguous. + """ + by_fold = {i.casefold(): i for i in ids} + for story_id in ids: + parts = story_id.casefold().split("-") + for k in range(1, len(parts)): + prefix = "-".join(parts[:k]) + other = by_fold.get(prefix) + if other is not None: + raise StoriesError( + f"stories.yaml id {story_id!r} is not prefix-free: {other!r} is " + f"also an id, so the {other}-*.md glob would match both" + ) + + +def find_entry(stories: Stories, story_id: str) -> StoryEntry: + """The entry for ``story_id`` or a :class:`StoriesError` with the pinned + ``story id not found in stories.yaml`` message.""" + entry = stories.get(story_id) + if entry is None: + raise StoriesError("story id not found in stories.yaml") + return entry + + +@dataclass(frozen=True) +class StoryState: + """The resolved on-disk state of one story (see :func:`resolve_story_spec`). + + ``status`` is set only for :data:`KIND_PRESENT`; ``path`` for PRESENT / + SENTINEL; ``paths`` for AMBIGUOUS; ``sentinel_kind`` for SENTINEL. + """ + + kind: str + status: str = "" + path: Path | None = None + paths: tuple[Path, ...] = () + sentinel_kind: str = "" + + +def resolve_story_spec(spec_folder: Path | str, story_id: str) -> StoryState: + """Deterministic on-disk state of one story, keyed by id. + + Globs ``/stories/-*.md``. Ids are prefix-free, so a + conforming tree yields at most one match: no match = :data:`KIND_PENDING` + (never dispatched); a fixed-slug ``-unresolved.md`` / ``-ambiguous.md`` + = :data:`KIND_SENTINEL`; any other single file = :data:`KIND_PRESENT` with + its frontmatter status read off disk. More than one match = + :data:`KIND_AMBIGUOUS` (an anomaly the dispatcher must refuse rather than + silently pick one). + """ + sid = str(story_id).strip() + stories_dir = Path(spec_folder) / STORIES_SUBDIR + matches = sorted(stories_dir.glob(f"{sid}-*.md")) if stories_dir.is_dir() else [] + if not matches: + return StoryState(kind=KIND_PENDING) + if len(matches) > 1: + return StoryState(kind=KIND_AMBIGUOUS, paths=tuple(matches)) + path = matches[0] + for sentinel_kind in SENTINEL_SLUGS: + if path.name == f"{sid}-{sentinel_kind}.md": + return StoryState(kind=KIND_SENTINEL, path=path, sentinel_kind=sentinel_kind) + return StoryState(kind=KIND_PRESENT, status=status_of(read_frontmatter(path)), path=path) + + +@dataclass(frozen=True) +class Schedule: + """The scheduler's verdict. ``outcome`` is one of :data:`SCHEDULE_NEXT` + (``entry`` is the next story to dispatch), :data:`SCHEDULE_COMPLETE` (all + done), or :data:`SCHEDULE_WEDGED` (``entry``/``state`` name the + blocked/sentinel/ambiguous story that stopped the scan).""" + + outcome: str + entry: StoryEntry | None = None + state: StoryState | None = None + + @property + def is_complete(self) -> bool: + return self.outcome == SCHEDULE_COMPLETE + + @property + def is_wedged(self) -> bool: + return self.outcome == SCHEDULE_WEDGED + + +def schedule( + stories: Stories, + states: dict[str, StoryState], + selector: str | None = None, +) -> Schedule: + """Linear scheduler: the first list entry ready to (re)dispatch. + + The manifest is a flat list in strict execution order (no ``depends_on``), + so scheduling is a single left-to-right scan. An entry is actionable when + its state is PENDING or a resumable non-terminal (``draft`` / ``ready-for-dev`` + / ``in-progress`` / ``in-review`` = died mid-flight, re-dispatch resumes); a + ``done`` entry is skipped (never re-dispatch done); a ``blocked``, sentinel, + ambiguous, or unknown-status entry STOPS the scan (:data:`SCHEDULE_WEDGED` — + the run pauses for resolve; a blocked story cannot be leapfrogged to later + work). Falling off the end with everything done is :data:`SCHEDULE_COMPLETE`. + + ``selector`` restricts the scan to a single story id (raises when the id is + unknown), for ``--story`` runs. A state missing from ``states`` is treated + as PENDING (no spec on disk). + """ + entries: tuple[StoryEntry, ...] + entries = (find_entry(stories, selector),) if selector is not None else stories.entries + for entry in entries: + state = states.get(entry.id) + if state is None: + state = StoryState(kind=KIND_PENDING) + disposition = _classify(state) + if disposition == "actionable": + return Schedule(SCHEDULE_NEXT, entry=entry, state=state) + if disposition == "done": + continue + return Schedule(SCHEDULE_WEDGED, entry=entry, state=state) + return Schedule(SCHEDULE_COMPLETE) + + +def _classify(state: StoryState) -> str: + """One of ``'actionable'`` | ``'done'`` | ``'wedged'`` for a resolved state.""" + if state.kind == KIND_PENDING: + return "actionable" + if state.kind == KIND_PRESENT: + if state.status == DONE: + return "done" + if state.status in RESUMABLE_STATUSES: + return "actionable" + # blocked, or a status the skill itself would HALT on as unrecognized. + return "wedged" + # AMBIGUOUS or SENTINEL — not actionable without dispatcher/human recovery. + return "wedged" diff --git a/src/bmad_loop/verify.py b/src/bmad_loop/verify.py index ca0e8af..02aaedb 100644 --- a/src/bmad_loop/verify.py +++ b/src/bmad_loop/verify.py @@ -1132,6 +1132,84 @@ def verify_dev_bundle( return VerifyOutcome.passed() +def verify_dev_stories( + task: StoryTask, + paths: ProjectPaths, + result_json: dict[str, Any] | None, + *, + spec_folder: Path, + review_enabled: bool = True, +) -> VerifyOutcome: + """verify_dev for stories mode: the story spec lives at the id-keyed path + ``/stories/-.md`` and there is no sprint-status entry. + + Same gates as :func:`verify_dev` — workflow tag, expected frontmatter status, + baseline match, proof-of-work since baseline — with two differences: the spec + is resolved **deterministically by id** (``task.story_key``) via + ``stories.resolve_story_spec`` rather than trusting the session-claimed path, + and the sprint-status gate is dropped (stories mode has no sprint board). + A resolution that is pending / ambiguous / a sentinel is a retryable failure, + and the resolved filename's id prefix is asserted to equal the task id. + """ + from . import stories + + rj = result_json or {} + story_id = task.story_key + state = stories.resolve_story_spec(spec_folder, story_id) + if state.kind == stories.KIND_PENDING: + return VerifyOutcome.retry(f"no story spec found for id {story_id!r} under {spec_folder}") + if state.kind == stories.KIND_AMBIGUOUS: + names = ", ".join(p.name for p in state.paths) + return VerifyOutcome.retry(f"ambiguous story file match for id {story_id!r}: {names}") + if state.kind == stories.KIND_SENTINEL: + return VerifyOutcome.retry( + f"story {story_id!r} resolved to a {state.sentinel_kind} sentinel: {state.path}" + ) + spec_path = state.path + # The glob is `-*.md`, so this holds by construction — assert it anyway as + # a defensive gate against a future resolver change silently widening the match. + if spec_path is None or not spec_path.name.startswith(f"{story_id}-"): + return VerifyOutcome.retry( + f"resolved story spec {spec_path} does not match id {story_id!r}" + ) + if not spec_path.is_file(): + return VerifyOutcome.retry(f"claimed spec file does not exist: {spec_path}") + + workflow = rj.get("workflow") + if workflow != DEV_WORKFLOW: + return VerifyOutcome.retry( + f"dev result.json workflow is {workflow!r}, expected {DEV_WORKFLOW!r}" + ) + + # Generic path always self-finalizes to done (no in-review handoff); the + # review_enabled arm mirrors verify_dev for symmetry. + expected = "in-review" if review_enabled else "done" + fm = read_frontmatter(spec_path) + status = status_of(fm) + if status != expected: + return VerifyOutcome.retry(f"spec status is {status!r}, expected {expected!r}: {spec_path}") + + claimed_baseline = str(fm.get("baseline_commit", "")).strip() + if task.baseline_commit and claimed_baseline not in ("", "NO_VCS"): + if not same_commit(claimed_baseline, task.baseline_commit): + return VerifyOutcome.retry( + f"spec baseline_commit {claimed_baseline[:12]} does not match " + f"orchestrator-recorded baseline {task.baseline_commit[:12]}" + ) + + if task.baseline_commit: + try: + if not has_changes_since( + paths.project, task.baseline_commit, exclude=artifact_relpaths(paths) + ): + return VerifyOutcome.retry("no changes in worktree since baseline commit") + except GitError as e: + return VerifyOutcome.escalate(str(e)) + + task.spec_file = str(spec_path) + return VerifyOutcome.passed() + + @dataclass(frozen=True) class CommandResult: command: str diff --git a/tests/fixtures/stories.yaml b/tests/fixtures/stories.yaml new file mode 100644 index 0000000..e07034d --- /dev/null +++ b/tests/fixtures/stories.yaml @@ -0,0 +1,48 @@ +# Dogfooded Story Breakdown from BMAD-METHOD PR #2549's gist +# (https://gist.github.com/alexeyv/0d80e3b4cd79bb4423c22846fe36f610). +# Quoted ids, independent spec_checkpoint/done_checkpoint bools, verbatim +# invoke_dev_with, and NO depends_on — the linear stories-mode contract. +- id: "1" + title: Add story-breakdown step to bmad-spec + description: >- + Add the optional interactive breakdown step to the bmad-spec skill, + emitting stories.yaml per stories-schema.md with its validity + rules as authoring checks; the file is discovered by fixed name, + not referenced from frontmatter. + spec_checkpoint: true + +- id: "2" + title: Dispatch dev-auto by spec folder and story id + description: >- + Extend dev-auto step-01 routing to accept spec folder + story id, + read the entry's title and description from stories.yaml, derive the + slug, and create or resume the story spec at the id-keyed path. + Existing free-form and spec-file invocations keep working. + spec_checkpoint: true + invoke_dev_with: >- + The artifact under edit is bmad-dev-auto itself; keep its + deliberately-minimal-change tenet — routing additions only, + no new step files. + +- id: "3" + title: Pin write-back to the id-keyed path and honor the plan halt + description: >- + Under folder+id dispatch, every HALT writes the id-keyed story spec + (skeletal if pre-planning) and never the implementation_artifacts + fallback. Honor an invocation-prompt directive to HALT ready-for-dev + after planning. + +- id: "4" + title: Accumulate context from prior story records + description: >- + Under folder+id dispatch, the planning step reads all stories/*.md + records present regardless of status, superseding the epic-numbered + continuity scan for this dispatch mode. + +- id: "5" + title: End-to-end contract demonstration + description: >- + Run the breakdown on a real epic and hand-dispatch dev-auto per + entry, human standing in for the orchestrator, as the success-signal + demonstration; record outcomes in the spec folder. + done_checkpoint: true diff --git a/tests/test_devcontract.py b/tests/test_devcontract.py index 564bbaf..4f3fe95 100644 --- a/tests/test_devcontract.py +++ b/tests/test_devcontract.py @@ -216,6 +216,60 @@ def test_synth_followup_review_recommended_omitted_on_blocked(tmp_path): assert "followup_review_recommended" not in out.result_json +# --------------------------------------------------- plan-halt expected-terminal + + +def test_synth_ready_for_dev_non_terminal_by_default(tmp_path): + # Without the plan-halt directive, ready-for-dev is a died-mid-flight + # non-terminal (still in RECONCILABLE_FROM) — nothing to translate yet. + sp = _spec(tmp_path / "s.md", status="ready-for-dev", auto_run=None) + out = devcontract.synthesize_result(sp, story_key="1") + assert out.result_json is None and out.status_consistent + + +def test_synth_plan_halt_ready_for_dev_is_success_terminal(tmp_path): + sp = _spec(tmp_path / "s.md", status="ready-for-dev", auto_run=None) + out = devcontract.synthesize_result(sp, story_key="1", plan_halt=True) + rj = out.result_json + assert rj is not None and rj["status"] == "ready-for-dev" + assert rj["plan_halt"] is True + assert rj["escalations"] == [] + assert "followup_review_recommended" not in rj # only carried on a done exit + assert out.status_consistent + + +def test_synth_plan_halt_overrun_to_done_is_plain_done(tmp_path): + # Plan-halt requested but the session ran on to done: treat as a normal done + # (no plan_halt marker), carrying the followup flag as usual. + sp = _spec(tmp_path / "s.md", status="done", auto_run="done", followup=True) + out = devcontract.synthesize_result(sp, story_key="1", plan_halt=True) + rj = out.result_json + assert rj["status"] == "done" and "plan_halt" not in rj + assert rj["followup_review_recommended"] is True + + +def test_synth_plan_halt_blocked_still_escalates(tmp_path): + # A block during planning routes to PAUSE, not a plan-review pause — no marker. + sp = _spec(tmp_path / "s.md", status="blocked", auto_run="blocked") + out = devcontract.synthesize_result(sp, story_key="1", plan_halt=True) + rj = out.result_json + assert "plan_halt" not in rj + assert any(e["severity"] == "CRITICAL" for e in rj["escalations"]) + + +def test_plan_halt_composes_with_reconcile_guard(tmp_path): + # The engine's _reconcile_generic_terminal_status only rewrites a spec whose + # prose `## Auto Run Result` says done while the frontmatter lags. A plan-halt + # ready-for-dev spec carries no such prose, so the reconcile guard no-ops and + # this leg's ready-for-dev success outcome is never clobbered to done — + # even though ready-for-dev is (for the died-mid-flight case) reconcilable-from. + assert "ready-for-dev" in devcontract.RECONCILABLE_FROM + sp = _spec(tmp_path / "s.md", status="ready-for-dev", auto_run=None) + arr = devcontract.parse_auto_run_result(sp.read_text(encoding="utf-8")) + reconcile_would_noop = not arr.present or arr.status != devcontract.DONE + assert reconcile_would_noop + + # ------------------------------------------------------------ find_result_artifact diff --git a/tests/test_stories.py b/tests/test_stories.py new file mode 100644 index 0000000..3bb5224 --- /dev/null +++ b/tests/test_stories.py @@ -0,0 +1,379 @@ +"""Tests for the stories.yaml contract layer (parse/validate + linear schedule).""" + +from pathlib import Path + +import pytest + +from bmad_loop import stories + +FIXTURES = Path(__file__).parent / "fixtures" + + +def write_stories(spec_folder: Path, text: str) -> Path: + spec_folder.mkdir(parents=True, exist_ok=True) + path = spec_folder / stories.STORIES_FILENAME + path.write_text(text, encoding="utf-8") + return path + + +def write_story_spec(spec_folder: Path, filename: str, *, status: str = "") -> Path: + d = spec_folder / stories.STORIES_SUBDIR + d.mkdir(parents=True, exist_ok=True) + fm = ( + f"---\ntitle: x\nstatus: {status}\n---\n\nbody\n" + if status + else "---\ntitle: x\n---\n\nbody\n" + ) + path = d / filename + path.write_text(fm, encoding="utf-8") + return path + + +# --------------------------------------------------------------- fixture / parse + + +def test_load_dogfooded_fixture(): + s = stories.load_stories(FIXTURES) + assert [e.id for e in s.entries] == ["1", "2", "3", "4", "5"] + by_id = {e.id: e for e in s.entries} + # independent checkpoint bools, verbatim invoke_dev_with, no depends_on field + assert by_id["1"].spec_checkpoint is True and by_id["1"].done_checkpoint is False + assert by_id["2"].spec_checkpoint is True + assert "deliberately-minimal-change" in by_id["2"].invoke_dev_with + assert by_id["3"].spec_checkpoint is False and by_id["3"].invoke_dev_with == "" + assert by_id["5"].done_checkpoint is True and by_id["5"].spec_checkpoint is False + assert not hasattr(by_id["1"], "depends_on") + + +def test_load_missing_file_raises_pinned_message(tmp_path): + with pytest.raises(stories.StoriesError, match="no stories.yaml found"): + stories.load_stories(tmp_path) + + +def test_id_unquoted_int_normalized(tmp_path): + # An LLM-authored file may emit `id: 1` unquoted (PyYAML -> int); we str()-normalize. + write_stories(tmp_path, "- id: 1\n title: t\n description: d\n") + s = stories.load_stories(tmp_path) + assert s.entries[0].id == "1" + + +def test_id_unquoted_composite_is_string(tmp_path): + # `3-2` is not a YAML number, so it parses as the string "3-2" — a valid id. + write_stories(tmp_path, "- id: 3-2\n title: t\n description: d\n") + s = stories.load_stories(tmp_path) + assert s.entries[0].id == "3-2" + + +def test_id_float_rejected(tmp_path): + # `id: 3.5` -> float -> str "3.5"; the `.` fails the charset (fail loud). + write_stories(tmp_path, "- id: 3.5\n title: t\n description: d\n") + with pytest.raises(stories.StoriesError, match="invalid id"): + stories.load_stories(tmp_path) + + +def test_id_bad_charset_rejected(tmp_path): + write_stories(tmp_path, '- id: "a_b"\n title: t\n description: d\n') + with pytest.raises(stories.StoriesError, match="invalid id"): + stories.load_stories(tmp_path) + + +def test_id_leading_dash_rejected(tmp_path): + write_stories(tmp_path, '- id: "-1"\n title: t\n description: d\n') + with pytest.raises(stories.StoriesError, match="invalid id"): + stories.load_stories(tmp_path) + + +def test_duplicate_ids_rejected(tmp_path): + # int 1 and str "1" both normalize to "1" — a duplicate. + write_stories( + tmp_path, + '- id: 1\n title: a\n description: d\n- id: "1"\n title: b\n description: d\n', + ) + with pytest.raises(stories.StoriesError, match="duplicate id"): + stories.load_stories(tmp_path) + + +def test_prefix_free_violation_rejected(tmp_path): + # ids "3" and "3-2": the `3-*.md` glob for story 3 would also match `3-2-*.md`. + write_stories( + tmp_path, + '- id: "3"\n title: a\n description: d\n' '- id: "3-2"\n title: b\n description: d\n', + ) + with pytest.raises(stories.StoriesError, match="not prefix-free"): + stories.load_stories(tmp_path) + + +def test_prefix_free_allows_numeric_neighbor(tmp_path): + # "3" and "31" do NOT collide: `3-*.md` never matches `31-*.md`. + write_stories( + tmp_path, + '- id: "3"\n title: a\n description: d\n' '- id: "31"\n title: b\n description: d\n', + ) + s = stories.load_stories(tmp_path) + assert [e.id for e in s.entries] == ["3", "31"] + + +def test_case_only_duplicate_ids_rejected(tmp_path): + # "Auth" and "auth" are distinct strings but identical under casefold — on a + # case-insensitive filesystem the `Auth-*.md` glob also matches `auth-*.md`. + write_stories( + tmp_path, + '- id: "Auth"\n title: a\n description: d\n' + '- id: "auth"\n title: b\n description: d\n', + ) + with pytest.raises(stories.StoriesError, match="differ only by case"): + stories.load_stories(tmp_path) + + +def test_case_insensitive_prefix_collision_rejected(tmp_path): + # "Auth" and "auth-2": on a case-insensitive filesystem the `Auth-*.md` glob + # for story "Auth" also matches `auth-2-*.md`, so this is a prefix collision + # even though the two ids never collide byte-for-byte. + write_stories( + tmp_path, + '- id: "Auth"\n title: a\n description: d\n' + '- id: "auth-2"\n title: b\n description: d\n', + ) + with pytest.raises(stories.StoriesError, match="not prefix-free"): + stories.load_stories(tmp_path) + + +def test_status_key_forbidden(tmp_path): + write_stories(tmp_path, '- id: "1"\n title: t\n description: d\n status: draft\n') + with pytest.raises(stories.StoriesError, match="forbidden 'status' key") as exc: + stories.load_stories(tmp_path) + # The user-facing vocabulary is "stories.yaml", not the plugin-layer word + # "manifest" (which names plugin.toml elsewhere in the codebase). + assert "manifest" not in str(exc.value) + assert "stories.yaml" in str(exc.value) + + +def test_missing_required_field_rejected(tmp_path): + write_stories(tmp_path, '- id: "1"\n description: d\n') # no title + with pytest.raises(stories.StoriesError, match="missing required field 'title'"): + stories.load_stories(tmp_path) + + +def test_missing_id_rejected(tmp_path): + write_stories(tmp_path, "- title: t\n description: d\n") + with pytest.raises(stories.StoriesError, match="missing required field 'id'"): + stories.load_stories(tmp_path) + + +def test_empty_title_rejected(tmp_path): + write_stories(tmp_path, '- id: "1"\n title: " "\n description: d\n') + with pytest.raises(stories.StoriesError, match="field 'title' is empty"): + stories.load_stories(tmp_path) + + +def test_checkpoint_bool_defaults_false(tmp_path): + write_stories(tmp_path, '- id: "1"\n title: t\n description: d\n') + e = stories.load_stories(tmp_path).entries[0] + assert e.spec_checkpoint is False and e.done_checkpoint is False + + +def test_both_checkpoints_settable(tmp_path): + write_stories( + tmp_path, + '- id: "1"\n title: t\n description: d\n' + " spec_checkpoint: true\n done_checkpoint: true\n", + ) + e = stories.load_stories(tmp_path).entries[0] + assert e.spec_checkpoint is True and e.done_checkpoint is True + + +def test_checkpoint_non_bool_rejected(tmp_path): + # `spec_checkpoint: 1` is an int, not a bool — strict, no truthy coercion. + write_stories(tmp_path, '- id: "1"\n title: t\n description: d\n spec_checkpoint: 1\n') + with pytest.raises(stories.StoriesError, match="must be a boolean"): + stories.load_stories(tmp_path) + + +def test_invoke_dev_with_defaults_empty(tmp_path): + write_stories(tmp_path, '- id: "1"\n title: t\n description: d\n') + assert stories.load_stories(tmp_path).entries[0].invoke_dev_with == "" + + +def test_invoke_dev_with_non_string_rejected(tmp_path): + write_stories(tmp_path, '- id: "1"\n title: t\n description: d\n invoke_dev_with: [a, b]\n') + with pytest.raises(stories.StoriesError, match="must be a string"): + stories.load_stories(tmp_path) + + +def test_top_level_must_be_list(tmp_path): + write_stories(tmp_path, "development_status:\n a: b\n") + with pytest.raises(stories.StoriesError, match="top-level list"): + stories.load_stories(tmp_path) + + +def test_empty_list_rejected(tmp_path): + write_stories(tmp_path, "[]\n") + with pytest.raises(stories.StoriesError, match="no story entries"): + stories.load_stories(tmp_path) + + +def test_empty_file_rejected(tmp_path): + write_stories(tmp_path, "") + with pytest.raises(stories.StoriesError, match="no story entries"): + stories.load_stories(tmp_path) + + +def test_entry_not_mapping_rejected(tmp_path): + write_stories(tmp_path, "- just a string\n") + with pytest.raises(stories.StoriesError, match="is not a mapping"): + stories.load_stories(tmp_path) + + +def test_invalid_yaml_rejected(tmp_path): + write_stories(tmp_path, "- id: '1'\n title: [unterminated\n") + with pytest.raises(stories.StoriesError, match="not valid YAML"): + stories.load_stories(tmp_path) + + +# --------------------------------------------------------------- find_entry + + +def test_find_entry(): + s = stories.load_stories(FIXTURES) + assert stories.find_entry(s, "3").title.startswith("Pin write-back") + + +def test_find_entry_unknown_raises_pinned_message(): + s = stories.load_stories(FIXTURES) + with pytest.raises(stories.StoriesError, match="story id not found in stories.yaml"): + stories.find_entry(s, "99") + + +# --------------------------------------------------------------- schedule (linear) + + +def _stories(*ids: str) -> stories.Stories: + entries = tuple(stories.StoryEntry(id=i, title=f"t{i}", description="d") for i in ids) + return stories.Stories(path=Path("stories.yaml"), entries=entries) + + +def _present(status: str) -> stories.StoryState: + return stories.StoryState(kind=stories.KIND_PRESENT, status=status, path=Path("x.md")) + + +def test_schedule_first_pending_is_next(): + s = _stories("1", "2", "3") + sched = stories.schedule(s, {}) # all missing -> pending + assert sched.outcome == stories.SCHEDULE_NEXT and sched.entry.id == "1" + + +def test_schedule_skips_done(): + s = _stories("1", "2", "3") + states = {"1": _present("done"), "2": stories.StoryState(kind=stories.KIND_PENDING)} + sched = stories.schedule(s, states) + assert sched.entry.id == "2" + + +@pytest.mark.parametrize("status", sorted(stories.RESUMABLE_STATUSES)) +def test_schedule_resumes_non_terminal(status): + # A died-mid-flight story (draft/ready-for-dev/in-progress/in-review) is + # actionable — re-dispatch resumes it. + s = _stories("1", "2") + sched = stories.schedule(s, {"1": _present(status)}) + assert sched.outcome == stories.SCHEDULE_NEXT and sched.entry.id == "1" + + +def test_schedule_all_done_is_complete(): + s = _stories("1", "2") + sched = stories.schedule(s, {"1": _present("done"), "2": _present("done")}) + assert sched.is_complete and sched.entry is None + + +def test_schedule_blocked_stops_scan_before_later_pending(): + # A blocked story earlier in the list wedges the run — the linear contract + # forbids leapfrogging it to the later pending story. + s = _stories("1", "2", "3") + states = {"1": _present("done"), "2": _present("blocked")} # 3 is pending + sched = stories.schedule(s, states) + assert sched.is_wedged and sched.entry.id == "2" + + +def test_schedule_sentinel_wedges(): + s = _stories("1", "2") + sentinel = stories.StoryState( + kind=stories.KIND_SENTINEL, sentinel_kind=stories.SENTINEL_UNRESOLVED, path=Path("1-u.md") + ) + sched = stories.schedule(s, {"1": sentinel}) + assert sched.is_wedged and sched.entry.id == "1" + + +def test_schedule_ambiguous_wedges(): + s = _stories("1") + amb = stories.StoryState(kind=stories.KIND_AMBIGUOUS, paths=(Path("1-a.md"), Path("1-b.md"))) + sched = stories.schedule(s, {"1": amb}) + assert sched.is_wedged and sched.entry.id == "1" + + +def test_schedule_unknown_status_wedges(): + # A frontmatter status the skill would itself HALT on as unrecognized. + s = _stories("1") + sched = stories.schedule(s, {"1": _present("weird-custom")}) + assert sched.is_wedged + + +def test_schedule_missing_state_is_pending(): + s = _stories("1", "2") + sched = stories.schedule(s, {"1": _present("done")}) # "2" absent from states + assert sched.outcome == stories.SCHEDULE_NEXT and sched.entry.id == "2" + + +def test_schedule_selector_targets_one_story(): + s = _stories("1", "2", "3") + sched = stories.schedule(s, {}, selector="2") + assert sched.entry.id == "2" + + +def test_schedule_selector_done_is_complete(): + s = _stories("1", "2") + sched = stories.schedule(s, {"2": _present("done")}, selector="2") + assert sched.is_complete + + +def test_schedule_selector_unknown_raises(): + s = _stories("1") + with pytest.raises(stories.StoriesError, match="story id not found in stories.yaml"): + stories.schedule(s, {}, selector="99") + + +# --------------------------------------------------------------- resolve_story_spec + + +def test_resolve_pending_no_dir(tmp_path): + assert stories.resolve_story_spec(tmp_path, "1").kind == stories.KIND_PENDING + + +def test_resolve_present_reads_status(tmp_path): + write_story_spec(tmp_path, "1-user-auth.md", status="in-review") + st = stories.resolve_story_spec(tmp_path, "1") + assert st.kind == stories.KIND_PRESENT and st.status == "in-review" + assert st.path.name == "1-user-auth.md" + + +def test_resolve_ambiguous(tmp_path): + write_story_spec(tmp_path, "1-one.md") + write_story_spec(tmp_path, "1-two.md") + st = stories.resolve_story_spec(tmp_path, "1") + assert st.kind == stories.KIND_AMBIGUOUS and len(st.paths) == 2 + + +def test_resolve_sentinel_unresolved(tmp_path): + write_story_spec(tmp_path, "1-unresolved.md", status="blocked") + st = stories.resolve_story_spec(tmp_path, "1") + assert st.kind == stories.KIND_SENTINEL and st.sentinel_kind == stories.SENTINEL_UNRESOLVED + + +def test_resolve_sentinel_ambiguous(tmp_path): + write_story_spec(tmp_path, "1-ambiguous.md", status="blocked") + st = stories.resolve_story_spec(tmp_path, "1") + assert st.kind == stories.KIND_SENTINEL and st.sentinel_kind == stories.SENTINEL_AMBIGUOUS + + +def test_resolve_prefix_isolation(tmp_path): + # id "3" must not resolve a file for id "31" — `3-*.md` doesn't match `31-*.md`. + write_story_spec(tmp_path, "31-other.md", status="done") + assert stories.resolve_story_spec(tmp_path, "3").kind == stories.KIND_PENDING diff --git a/tests/test_verify.py b/tests/test_verify.py index ab803a9..fa89198 100644 --- a/tests/test_verify.py +++ b/tests/test_verify.py @@ -342,6 +342,136 @@ def test_verify_dev_bundle_ledger_only_counts_as_real_work(project): assert out.ok +# ------------------------------------------------------------ verify_dev_stories + + +def make_stories_task(paths, story_key="1"): + task = StoryTask(story_key=story_key, epic=0) + task.baseline_commit = verify.rev_parse_head(paths.project) + return task + + +def write_story(spec_folder, story_id, slug, status, baseline): + d = spec_folder / "stories" + d.mkdir(parents=True, exist_ok=True) + sp = d / f"{story_id}-{slug}.md" + write_spec(sp, status, baseline) + return sp + + +def test_verify_dev_stories_happy_no_sprint_gate(project): + # No sprint-status file exists at all — stories mode has no sprint board, so + # the sprint-status gate that verify_dev enforces is dropped here. + assert not project.sprint_status.is_file() + spec_folder = project.planning_artifacts / "epic-a" + task = make_stories_task(project, "1") + sp = write_story(spec_folder, "1", "user-auth", "done", task.baseline_commit) + (project.project / "src.txt").write_text("changed\n") + out = verify.verify_dev_stories( + task, project, dev_result(sp), spec_folder=spec_folder, review_enabled=False + ) + assert out.ok + assert task.spec_file == str(sp) # set to the id-keyed resolution + + +def test_verify_dev_stories_composite_id(project): + spec_folder = project.planning_artifacts / "epic-a" + task = make_stories_task(project, "3-2") + sp = write_story(spec_folder, "3-2", "user-auth", "done", task.baseline_commit) + (project.project / "src.txt").write_text("changed\n") + out = verify.verify_dev_stories( + task, project, dev_result(sp), spec_folder=spec_folder, review_enabled=False + ) + assert out.ok and task.spec_file == str(sp) + + +def test_verify_dev_stories_pending_retry(project): + spec_folder = project.planning_artifacts / "epic-a" + task = make_stories_task(project, "1") + out = verify.verify_dev_stories( + task, project, dev_result(spec_folder / "ghost.md"), spec_folder=spec_folder + ) + assert not out.ok and out.retryable and "no story spec found" in out.reason + + +def test_verify_dev_stories_ambiguous_retry(project): + spec_folder = project.planning_artifacts / "epic-a" + task = make_stories_task(project, "1") + write_story(spec_folder, "1", "one", "done", task.baseline_commit) + write_story(spec_folder, "1", "two", "done", task.baseline_commit) + out = verify.verify_dev_stories( + task, project, {"workflow": "auto-dev"}, spec_folder=spec_folder, review_enabled=False + ) + assert not out.ok and "ambiguous story file match" in out.reason + + +def test_verify_dev_stories_sentinel_retry(project): + spec_folder = project.planning_artifacts / "epic-a" + task = make_stories_task(project, "1") + write_story(spec_folder, "1", "unresolved", "blocked", task.baseline_commit) + out = verify.verify_dev_stories( + task, project, {"workflow": "auto-dev"}, spec_folder=spec_folder, review_enabled=False + ) + assert not out.ok and "sentinel" in out.reason + + +def test_verify_dev_stories_wrong_status(project): + spec_folder = project.planning_artifacts / "epic-a" + task = make_stories_task(project, "1") + sp = write_story(spec_folder, "1", "x", "draft", task.baseline_commit) + out = verify.verify_dev_stories( + task, project, dev_result(sp), spec_folder=spec_folder, review_enabled=False + ) + assert not out.ok and "expected 'done'" in out.reason + + +def test_verify_dev_stories_review_enabled_expects_in_review(project): + spec_folder = project.planning_artifacts / "epic-a" + task = make_stories_task(project, "1") + sp = write_story(spec_folder, "1", "x", "done", task.baseline_commit) + (project.project / "src.txt").write_text("changed\n") + out = verify.verify_dev_stories( + task, project, dev_result(sp), spec_folder=spec_folder, review_enabled=True + ) + assert not out.ok and "expected 'in-review'" in out.reason + + +def test_verify_dev_stories_wrong_workflow(project): + spec_folder = project.planning_artifacts / "epic-a" + task = make_stories_task(project, "1") + sp = write_story(spec_folder, "1", "x", "done", task.baseline_commit) + (project.project / "src.txt").write_text("changed\n") + rj = {"workflow": "quick-dev", "spec_file": str(sp)} + out = verify.verify_dev_stories( + task, project, rj, spec_folder=spec_folder, review_enabled=False + ) + assert not out.ok and "auto-dev" in out.reason + + +def test_verify_dev_stories_lying_baseline(project): + spec_folder = project.planning_artifacts / "epic-a" + task = make_stories_task(project, "1") + sp = write_story(spec_folder, "1", "x", "done", "deadbeef" * 5) + out = verify.verify_dev_stories( + task, project, dev_result(sp), spec_folder=spec_folder, review_enabled=False + ) + assert not out.ok and "does not match" in out.reason + + +def test_verify_dev_stories_no_changes(project): + # NO_VCS baseline skips the mismatch check; everything committed -> proof-of- + # work fails since there are no changes vs the orchestrator baseline. + spec_folder = project.planning_artifacts / "epic-a" + sp = write_story(spec_folder, "1", "x", "done", "NO_VCS") + git(project.project, "add", "-A") + git(project.project, "commit", "-q", "-m", "artifacts") + task = make_stories_task(project, "1") + out = verify.verify_dev_stories( + task, project, dev_result(sp), spec_folder=spec_folder, review_enabled=False + ) + assert not out.ok and "no changes" in out.reason + + def test_verify_review_bundle_ledger_gate(project): task = make_bundle_task(project) sp = project.implementation_artifacts / "spec-dw-test-bundle.md"