From e9eebe8395f5c4acb02f5bb9b30384b26055a336 Mon Sep 17 00:00:00 2001 From: Brian Madison Date: Sun, 2 Aug 2026 14:25:45 -0500 Subject: [PATCH] Harden bmad-ticket from PR #2672 review; epic done becomes intentional-only Review round (Greptile + two deep review passes, 34 findings): - update_ticket: fix splice collision that could silently drop an inserted field; parse with the same frontmatter parser as ticket_tree (block-style depends_on now reaches the cycle gate, quoted ids resolve); refuse newlines in values; refuse duplicated ids; JSON-clean malformed-target errors; pass-through --transitions/--hitl-threshold documented as the override path - ticket_tree: duplicate ids, unparseable files, orphan leaves, scalar list fields, and unclosed inline lists are now detected instead of silently swallowed; iterative graph depth (no recursion limit); mermaid ids/labels sanitized; coverage --proposed separated from real coverage; fs errors keep the JSON contract; validate --path skips tree-wide cycles - Epic state (D-23 revised): computed state is only not-started or in-progress; done/dropped are stored intentionally (retrospective or user) and never calculated; only a stored done releases dependents - Autonomous bootstrap key (D-34): request > project_keys map > derived 3-5 uppercase letters, flagged unconfirmed in the completion report - Docs: planning_artifacts namespace, slice route scores epic risk, progressive-write scoped post-gate, refine gets no lens pass, bmad-spec handoff names the route; task/bug/spike templates gain References + Dev Notes; spike risk unhardcoded; bug discovered_from placeholder fixed - Tests: 18 new regressions incl. 1200-deep graph chains; test headers bumped to >=3.11 (tomllib) --- src/bmm-skills/plan/bmad-spec/SKILL.md | 2 +- src/bmm-skills/plan/bmad-ticket/SKILL.md | 48 ++--- .../plan/bmad-ticket/assets/bug-template.md | 10 +- .../plan/bmad-ticket/assets/spike-template.md | 10 +- .../plan/bmad-ticket/assets/task-template.md | 8 + .../plan/bmad-ticket/customize.toml | 12 +- .../bmad-ticket/evals/fixtures/tree/index.md | 2 +- .../bmad-ticket/references/slice-epics.md | 2 +- .../scripts/tests/test_ticket_tree.py | 128 ++++++++++- .../scripts/tests/test_update_ticket.py | 59 +++++- .../plan/bmad-ticket/scripts/ticket_tree.py | 198 ++++++++++++------ .../plan/bmad-ticket/scripts/update_ticket.py | 78 ++++--- src/scripts/tests/test_ticket_tree.py | 128 ++++++++++- src/scripts/tests/test_update_ticket.py | 59 +++++- src/scripts/ticket_tree.py | 198 ++++++++++++------ src/scripts/update_ticket.py | 78 ++++--- 16 files changed, 781 insertions(+), 239 deletions(-) diff --git a/src/bmm-skills/plan/bmad-spec/SKILL.md b/src/bmm-skills/plan/bmad-spec/SKILL.md index ca41be239..d454dd45d 100644 --- a/src/bmm-skills/plan/bmad-spec/SKILL.md +++ b/src/bmm-skills/plan/bmad-spec/SKILL.md @@ -131,7 +131,7 @@ When the user points the skill at an existing spec folder (or its SPEC.md) with Decomposition belongs to the `bmad-ticket` skill — it owns the ticket tree, the breakdown gates, and the coverage contract; bmad-spec never authors epics or stories itself. Requires `SPEC.md` on disk — run the normal Operation first if it doesn't exist yet. Headless runs never do this, even when the invocation text asks for it: if mode detection (On Activation, step 4) resolved headless, skip this section entirely and proceed with the normal headless response. In interactive mode, offer it at most once per run when the input reads as multiple independently shippable slices; a decline ends the offer for this run, not forever. Also run it on direct request ("break this into epics", "break this into stories") whenever `SPEC.md` exists. -On acceptance, invoke `bmad-ticket` with the spec folder as input — `SPEC.md` plus its `companions:` are the source documents. Its slice route produces the epic set with `covers:` carrying this spec's `CAP-N` ids verbatim; its incept route produces one epic's stories on request. That trace spine is why capability IDs must stay stable (Spec Law rule 6): ticket-side coverage validates against them. +On acceptance, invoke `bmad-ticket` with the spec folder as input — `SPEC.md` plus its `companions:` are the source documents — and name the route: **slice** to produce the epic set (with `covers:` carrying this spec's `CAP-N` ids verbatim), **incept** for one named epic's stories. "Break this into stories" on open scope means slice first, then offer incept per epic — say so, so epics-only output isn't a surprise. That trace spine is why capability IDs must stay stable (Spec Law rule 6): ticket-side coverage validates against them. When a spec update changes, adds, or retires capabilities and a ticket tree already covers this spec, say so — the tree may have drifted from the contract — and suggest re-running the ticket-side coverage check. diff --git a/src/bmm-skills/plan/bmad-ticket/SKILL.md b/src/bmm-skills/plan/bmad-ticket/SKILL.md index e8e7e3f57..60f6575cf 100644 --- a/src/bmm-skills/plan/bmad-ticket/SKILL.md +++ b/src/bmm-skills/plan/bmad-ticket/SKILL.md @@ -7,7 +7,7 @@ description: Turns any input into epics, stories, bugs, tasks, and spikes. Use w ## Overview -Act as the user's slicing partner: they hold the product knowledge; you hold the craft of shaping work into tickets an agent can build from. Take almost any input — a sentence, a bug report, a PRD, a spec, a brief, the current conversation — and produce well-formed tickets in the ticket tree at `{workflow.tickets_output_path}`. The consumer sets the bar: a fresh context must be able to build from a ticket using only what it carries and points at — behavior, acceptance criteria with verification, dependencies, trace ids, typed-document pointers. +Act as the user's slicing partner: they hold the product knowledge; you hold the craft of shaping work into tickets an agent can build from. Take almost any input — a sentence, a bug report, a PRD, a spec, the current conversation — and produce well-formed tickets in the tree at `{workflow.tickets_output_path}`. The consumer sets the bar: a fresh context must be able to build from a ticket using only what it carries and points at — behavior, acceptance criteria with verification, dependencies, trace ids, typed-document pointers. Three routes, cheap exit first. **Refine** writes one ticket with minimal ceremony. **Slice** decomposes open scope into the detailed epic set. **Incept** turns one epic into its stories. Never march a one-ticket request through inception altitude. @@ -15,16 +15,15 @@ Three routes, cheap exit first. **Refine** writes one ticket with minimal ceremo - Bare paths and `{skill-root}` (e.g. `assets/story-template.md`) resolve from this skill's installed directory. - `{project-root}` → the project working directory. -- `{skill-name}` → the skill directory's basename. - `{workflow.}` resolves to fields in `customize.toml`. ## On Activation 1. Resolve customization: `uv run {project-root}/_bmad/scripts/resolve_customization.py --skill {skill-root} --key workflow`. On failure — including when `{project-root}/_bmad/` does not exist (standalone installs) — read `{skill-root}/customize.toml` directly; don't hunt for the resolver elsewhere. 2. Run `{workflow.activation_steps_prepend}`; treat `{workflow.persistent_facts}` as foundational context (`file:` entries are loaded). -3. Resolve config: `uv run {project-root}/_bmad/scripts/resolve_config.py --project-root {project-root}`; from the merged JSON take `{communication_language}`, `{document_output_language}`, `{planning_artifacts}`, `{date}`. Converse in `{communication_language}`; write ticket content in `{document_output_language}`. On failure, ask the user where the ticket tree should live (a sensible project-relative default) and continue. -4. Open the floor before routing: invite everything the user has — planning docs, existing tickets, constraints, prior decisions, and their preferences for how the work should be sliced. Say the frame plainly: they are a coequal expert — you facilitate, they hold the product truth, and the more they bring the better the set. Validate what they point at (the docs exist and are the right ones, the tree resolves) and announce what you'll use. Guidance that seems wrong, or contradicts the source or itself, gets a conversation, not silent compliance. A bare request gets "tell me everything"; a supplied path gets "what should I focus on?". The dump replaces most ingest questioning. Then pick the route from intent — one ticket → refine; open scope or a planning doc → slice; an existing epic to break down → incept — and the mode — **guided** (the default), **quick**, or **autonomous** (also selectable by explicit invocation, e.g. "autonomous" in the request). Ambiguity costs one bundled question, not a quiz. -5. Tree check: if `{workflow.tickets_output_path}` has no `index.md`, bootstrap before any route runs — create the folder, ask the project key (one bundled question), write the index: `uv run {skill-root}/scripts/ticket_tree.py index --root {workflow.tickets_output_path} --key `. If the tree exists, read `index.md` once to rebuild the landscape before routing. +3. Resolve config: `uv run {project-root}/_bmad/scripts/resolve_config.py --project-root {project-root}`; from the merged JSON take `{communication_language}` and `{document_output_language}` (under `core`), `{planning_artifacts}` (under `modules.bmm`), `{date}`. Converse in `{communication_language}`; write ticket content in `{document_output_language}`. On failure, ask the user where the ticket tree should live (a sensible project-relative default) and continue. +4. Open the floor before routing: invite everything the user has — planning docs, existing tickets, constraints, prior decisions, slicing preferences. Say the frame plainly: they are a coequal expert — you facilitate, they hold the product truth, and the more they bring the better the set. Validate what they point at (right docs, tree resolves) and announce what you'll use. Guidance that seems wrong, or contradicts the source or itself, gets a conversation, not silent compliance. A bare request gets "tell me everything"; a supplied path gets "what should I focus on?" — the dump replaces most ingest questioning. Then pick the route from intent — one ticket → refine; open scope or a planning doc → slice; an existing epic to break down → incept — and the mode — **guided** (the default), **quick**, or **autonomous** (explicit invocation selects it). Ambiguity costs one bundled question, not a quiz. +5. Tree check: if `{workflow.tickets_output_path}` has no `index.md`, bootstrap before any route runs — create the folder, settle the project key, write the index: `uv run {skill-root}/scripts/ticket_tree.py index --root {workflow.tickets_output_path} --key `. Key precedence: the request, `{workflow.project_keys}`, then one bundled question — autonomous never asks: derive 3–5 uppercase letters from the project name and flag the derived key in the completion report. If the tree exists, read `index.md` once to rebuild the landscape before routing. 6. Run `{workflow.activation_steps_append}`. ## The ticket tree @@ -34,27 +33,27 @@ Three routes, cheap exit first. **Refine** writes one ticket with minimal ceremo ├── index.md # generated — identity only; carries the project key ├── alert-rules/ # epic = folder │ ├── ticket.md # envelope (id ALRT-3) -│ └── ALRT-12-rule-crud.md # leaf: story | bug | task | spike -└── ALRT-31-snooze-button.md # standalone leaf in the bin +│ └── ALRT-12-rule-crud.md # leaf (story|bug|task|spike) +└── ALRT-31-snooze-button.md # bin leaf ``` Rules that never bend: -- **Parent = containing folder.** No parent field; an envelope never lists or counts its children, so parallel work never collides on a shared file. An epic folder may nest sub-epic folders when decomposition genuinely needs a second level; most trees stay flat. -- **One stored fact.** A leaf stores exactly one state field, `status`. Blocked, frontier, next, rollups are derived by scanning frontmatter, never written down. No status ledger file, ever. -- **Index carries identity, not state.** Regenerate `index.md` after structural changes: project key in frontmatter, then `* [Title](path) - description` per entry. A status flip never touches it. -- **IDs are `KEY-n`.** The project key is asked once, at tree bootstrap — and recorded in the index. Ids come from `uv run {skill-root}/scripts/ticket_tree.py next-id --root {workflow.tickets_output_path}`; gaps are meaningless. Leaf filename: `KEY-n-slug.md`. -- **Lifecycle.** Leaves: `backlog → in-progress → review → done` (or `dropped`, kept on the record). `review` means complete on a branch; `done` means merged — a dependent is workable only when its dependencies are `done`. This skill writes `backlog` at creation; the build lane owns the rest. Epics store no status — their state is computed from children (`unsliced` when childless, then backlog / in-progress / done); `status: dropped` is the one storable epic fact. +- **Parent = containing folder.** No parent field; an envelope never lists or counts its children, so parallel work never collides on a shared file. Sub-epic folders are legal when decomposition genuinely needs a second level; most trees stay flat. +- **One stored fact.** A leaf stores exactly one state field, `status`; blocked, frontier, next, rollups are derived by scan, never written down. No status ledger file, ever. +- **Index carries identity, not state.** Regenerate `index.md` after structural changes: project key in frontmatter, then `* [Title](path)` per entry (epics carry `- description`). A status flip never touches it. +- **IDs are `KEY-n`.** The project key is settled once, at tree bootstrap, and recorded in the index. Ids come from `uv run {skill-root}/scripts/ticket_tree.py next-id --root {workflow.tickets_output_path}`; gaps are meaningless. Leaf filename: `KEY-n-slug.md`. +- **Lifecycle.** Leaves: `backlog → in-progress → review → done` (or `dropped`, kept on the record). `review` means complete on a branch; `done` means merged — a dependent is workable only when its dependencies are `done`. This skill writes `backlog` at creation; the build lane owns the rest. Epics compute their state from children — `not-started` (childless, or nothing past backlog) or `in-progress`; `done` and `dropped` are stored only intentionally (a retrospective or the user's call), **never calculated**, and only a stored `done` releases dependents. ## Writing a ticket -The type's template drives the shape: `{workflow.story_template}`, `{workflow.bug_template}`, `{workflow.task_template}`, `{workflow.spike_template}`, `{workflow.epic_template}`. Load the template for the type being written; fill every placeholder or cut the optional section — an unresolved placeholder never reaches disk. Write files progressively as content settles, never composed in memory and dumped at the end. +The type's template drives the shape: `{workflow.story_template}`, `{workflow.bug_template}`, `{workflow.task_template}`, `{workflow.spike_template}`, `{workflow.epic_template}`. Load the template for the type being written; fill every placeholder or cut the optional section — an unresolved placeholder never reaches disk. Write approved files progressively as generated, never composed in memory and dumped at the end; in gated routes nothing touches disk before the gate passes. The craft rules the templates cannot carry: - A story is a **vertical slice**: a narrow but complete path through every layer, demoable on its own. Its Behavior section is what the implementation plan gets built from. Its ACs are stable `#1..#n`, observable, atomic, bounded — outcomes, never engineer actions — each with a verify tail (a command, an endpoint, an observable). Given/When/Then is a per-criterion escalation when setup state genuinely matters, not the house style. When the story completes a user-visible flow, an e2e criterion says so. - `covers:` holds requirement ids verbatim from whatever scheme the input uses (CAP-4, FR-12, REQ-9) — never converted to a house scheme. -- No file paths or line numbers in bodies — stale before work starts. References are typed-document pointers ("architecture — data model"): document type plus section. +- No file paths or line numbers in bodies — stale before work starts; references are typed-document pointers: document type plus section. - Bugs carry a cause hypothesis, never a prescribed fix. Severity is proposed in conversation on `{workflow.severity_scale}`. - A story that reads like three stories is three stories — flag it and split. A steps checklist inside a body is working notes, not tickets. @@ -64,18 +63,19 @@ Frontmatter on an existing ticket is never hand-edited — every change goes thr ``` uv run {skill-root}/scripts/update_ticket.py --root {workflow.tickets_output_path} --id KEY-n \ - --set status=in-progress [--set risk=4 ...] + --set status=in-progress [--set risk=4 ...] \ + --transitions "<{workflow.lifecycle_transitions}, comma-joined>" --hitl-threshold {workflow.hitl_threshold} ``` -The script resolves `lifecycle_transitions` and `hitl_threshold` from its own config when the flags are omitted, refuses dependency edges that would close a cycle, and raises `hitl` to true when a newly set risk crosses the threshold (it never lowers it; an explicit `--set hitl=...` always wins). An off-graph status move is refused with the legal moves named — relay that to the user, and when they explicitly decide to make it anyway, re-run with `--force` (known states only; gibberish is always refused). Never work around the gate by editing the file directly. The body is never touched. +Pass the resolved values — overrides reach the gate only through those flags (omitted, the script uses its bundled defaults). The gate refuses dependency edges that would close a cycle and raises `hitl` to true when a newly set risk crosses the threshold (never lowers it; an explicit `--set hitl=...` always wins). An off-graph move is refused with the legal moves named — relay that, and on the user's explicit decision re-run with `--force` (known states only; gibberish is always refused). Never work around the gate by hand-editing. The body is never touched. ## Scoring -Propose `risk` (1–5) with a one-line rationale across six dimensions: blast radius, reversibility, data sensitivity, security surface, novelty, production exposure. Hard floors bind what you propose and what autonomous mode writes on its own: schema migrations, data deletion, auth, payments → risk 4 minimum and `hitl: true`. Otherwise `hitl` defaults to risk ≥ `{workflow.hitl_threshold}`. An explicit user decision overrides any of this — any allowed value, any combination; record their call and move on. Mine what the project already knows first — project context, architecture, org knowledge. The six-dimension proposal is slice/incept altitude; on refine, derive risk and `hitl` from the defaults and floors, state the result in one line, and move on unless a floor trips or the user pushes back. +Propose `risk` (1–5) with a one-line rationale across six dimensions: blast radius, reversibility, data sensitivity, security surface, novelty, production exposure. Hard floors bind what you propose and what autonomous mode writes on its own: schema migrations, data deletion, auth, payments → risk 4 minimum and `hitl: true`. Otherwise `hitl` defaults to risk ≥ `{workflow.hitl_threshold}`. An explicit user decision overrides any of this — any allowed value, any combination; record their call and move on. Mine what the project already knows first — project context, architecture, org knowledge. The six-dimension proposal is slice/incept altitude; refine derives from defaults and floors, states the result in one line, and moves on unless a floor trips or the user pushes back. ## Route 1 — Refine -"Make me a ticket for this fix." Elicit only what the template requires; propose the type from the input. Allocate the id and write one file — into the epic folder the user names or resolution finds, otherwise the bin. Zero setup beyond activation's tree check. +"Make me a ticket for this fix." Elicit only what the template requires; propose the type from the input. Allocate the id and write one file — into the epic folder the user names or resolution finds, otherwise the bin. Zero setup beyond activation's tree check; no review-lens pass — the user's confirm (or autonomous's floors plus `validate`) is refine's gate. ## Route 2 — Slice @@ -95,22 +95,22 @@ Routes say what gets made; modes say how collaboratively. | **Quick** | Propose the complete ticket, one confirm | Clarify from the dump, skeleton, full reviewed draft, one revision round, gate | | **Autonomous** | Write it, defaults and floors applied | No questions; reviews still run; gate quiz on yourself, self-check recorded in the envelope's Sequencing Notes | -Two rules shape every multi-artifact run in the interactive modes: +Two rules shape every multi-artifact run: -- **Skeleton before the expensive write.** When a run will produce several artifacts (an epic set, an epic's stories), present the skeleton first — each item as a title and a one-line summary (plus proposed `covers:`) — and let the user reshape the set. Full drafting, scoring, and review happen only after the skeleton stands; reshaping a title costs nothing, reshaping five finished stories costs everything. +- **Skeleton before the expensive write.** When a run will produce several artifacts (an epic set, an epic's stories), present the skeleton first — each item as a title and a one-line summary (plus proposed `covers:`) — and let the user reshape the set (autonomous builds the skeleton but skips the pause). Full drafting, scoring, and review happen only after the skeleton stands; reshaping a title costs nothing, reshaping five finished stories costs everything. - **The user never sees an unreviewed draft.** Once the full draft exists, run the review lenses in `{workflow.finalize_reviewers}` over it (parallel subagents where available), fold the findings in, and record material findings and their dispositions in Sequencing Notes or the item's Dev Notes. What gets presented — or what autonomous mode approves on its own — is the post-review draft. -All questioning is bounded: a handful of questions, each with a recommended answer, bundled through the harness question tool where available — never a mandated one-at-a-time drip. +All questioning is bounded: a handful, each with a recommended answer, bundled through the harness question tool where available — never a one-at-a-time drip. An autonomous run ends by reporting status — `complete`, or `blocked` with a one-line reason — plus the tree root and the ids/paths created, so a caller can chain (slice, then incept per epic) without re-scanning the tree. ## Finding a named ticket -A loosely named target ("add a defect to the alert rules thing") resolves in tiers: exact id/slug/title match (`ticket_tree.py list` gives the lookup table) → proceed; close matches → ranked candidates, the user picks; nothing → read the index and reason from it. Never guess ambiguity away. +A loosely named target resolves in tiers: exact id/slug/title match (`ticket_tree.py list` is the lookup table) → proceed; close matches → ranked candidates, the user picks; nothing → read the index and reason from it. Never guess ambiguity away. ## Tree queries -Derived state is never hand-computed: `uv run {skill-root}/scripts/ticket_tree.py --root {workflow.tickets_output_path}` — `next-id` before allocating, `index` after any structural write, `validate` after every write (schema, placeholders, dep resolution, cycles — fix what it names before presenting), `list` for the id/title/status/path inventory, `frontier` for "what's workable now," `board` for rollups (computed epic state included), `coverage --require ""` for the coverage check (`--proposed` pre-gate, before anything is on disk), `graph --mermaid` for the dependency graph, parallel lanes, and critical path. Status questions — "where are we?", "what's next?" — are answered from `board` and `frontier` output. When the user asks to optimize sequencing or dependencies — and as an offer after incept writes — render `graph --mermaid` and walk the lanes with them: false edges, over-serialized independents, the critical path. +Derived state is never hand-computed: `uv run {skill-root}/scripts/ticket_tree.py --root {workflow.tickets_output_path}` — `next-id` before allocating, `index` after any structural write, `validate` after every write (schema, placeholders, dep resolution, cycles — fix what it names before presenting), `list` for the id/title/status/path inventory, `frontier` for "what's workable now," `board` for rollups (computed epic state included), `coverage --require ""` for the coverage check (`--proposed` pre-gate, before anything is on disk), `graph --mermaid` for the dependency graph, parallel lanes, and critical path. When the user asks to optimize sequencing or dependencies — and as an offer after incept writes — render `graph --mermaid` and walk the lanes with them: false edges, over-serialized independents, the critical path. References: `slice-epics.md` (Route 2) · `incept-stories.md` (Route 3) · `greenfield-guidelines.md` (net-new project, at epic proposal) · type templates in `assets/` via `{workflow._template}`. -Run `{workflow.on_complete}` if set. +Run `{workflow.on_complete}` if set when we reach a terminal state. diff --git a/src/bmm-skills/plan/bmad-ticket/assets/bug-template.md b/src/bmm-skills/plan/bmad-ticket/assets/bug-template.md index 4a785242b..dc39acbc8 100644 --- a/src/bmm-skills/plan/bmad-ticket/assets/bug-template.md +++ b/src/bmm-skills/plan/bmad-ticket/assets/bug-template.md @@ -6,7 +6,7 @@ title: "[Symptom, not cause — what goes wrong]" status: backlog depends_on: [] covers: [] -discovered_from: "[KEY-n of the work that revealed this, if known]" +discovered_from: "" severity: [1-5] risk: [1-5] hitl: [true|false] @@ -30,3 +30,11 @@ created: [YYYY-MM-DD] ## Cause Hypothesis [Best current theory of the cause — a hypothesis, never a prescription. No "preferred fix," no solution steps.] + +## References + +- [Typed-document pointers relevant to this bug: document type + section, never file paths or line numbers. Set `discovered_from` in the frontmatter to the KEY-n that revealed this, when known. Cut the section if there are none.] + +## Dev Notes + +[Optional. Risk rationale when the score needs explaining (a hard floor tripped), constraints, gotchas.] diff --git a/src/bmm-skills/plan/bmad-ticket/assets/spike-template.md b/src/bmm-skills/plan/bmad-ticket/assets/spike-template.md index cc91e1ccb..b317e5681 100644 --- a/src/bmm-skills/plan/bmad-ticket/assets/spike-template.md +++ b/src/bmm-skills/plan/bmad-ticket/assets/spike-template.md @@ -7,7 +7,7 @@ status: backlog depends_on: [] covers: [] discovered_from: "" -risk: 1 +risk: [1-5] hitl: [true|false] created: [YYYY-MM-DD] --- @@ -26,3 +26,11 @@ created: [YYYY-MM-DD] ## Timebox [Bound the investigation — e.g. one session. Output is knowledge (recorded findings), not shipped code.] + +## References + +- [Typed-document pointers relevant to this spike: document type + section, never file paths or line numbers. Cut the section if there are none.] + +## Dev Notes + +[Optional. Risk rationale when the score needs explaining (a hard floor tripped — a spike into auth or migrations is not risk 1), constraints, gotchas.] diff --git a/src/bmm-skills/plan/bmad-ticket/assets/task-template.md b/src/bmm-skills/plan/bmad-ticket/assets/task-template.md index b92bcb002..9c8c8ca0e 100644 --- a/src/bmm-skills/plan/bmad-ticket/assets/task-template.md +++ b/src/bmm-skills/plan/bmad-ticket/assets/task-template.md @@ -22,3 +22,11 @@ created: [YYYY-MM-DD] - #1 [Verifiable state of the world when this is complete — infra up, tooling works, docs published] - #2 [...] + +## References + +- [Typed-document pointers relevant to this task: document type + section, never file paths or line numbers. Cut the section if there are none.] + +## Dev Notes + +[Optional. Risk rationale when the score needs explaining (a hard floor tripped), constraints, gotchas.] diff --git a/src/bmm-skills/plan/bmad-ticket/customize.toml b/src/bmm-skills/plan/bmad-ticket/customize.toml index b6f58a280..1c9e86f64 100644 --- a/src/bmm-skills/plan/bmad-ticket/customize.toml +++ b/src/bmm-skills/plan/bmad-ticket/customize.toml @@ -17,6 +17,11 @@ on_complete = "" # relocatable, so moving is a config change and a folder move. tickets_output_path = "{planning_artifacts}/tickets" +# Optional "project-name=KEY" entries consulted at tree bootstrap before +# asking (interactive) or deriving from the project name (autonomous — +# a derived key is flagged as unconfirmed in the completion report). +project_keys = [] + # Ticket templates, one per type. The templates drive the output shape, # so an org wanting a different AC rendering (Given/When/Then, EARS-style # shall-statements) swaps the template — the skill does not change. @@ -30,7 +35,9 @@ epic_template = "assets/epic-template.md" # any move not listed unless --force carries an explicit user decision; # states outside the graph's vocabulary are refused always. Overrides # append (widen the graph — e.g. add a custom state and its edges); -# removing a default edge means forking. +# removing a default edge means forking. Overrides reach the gate through +# the --transitions flag the skill passes; a direct script call without +# the flag falls back to the bundled defaults. # Defaults = the schema-1 lifecycle: backlog → in-progress → review → # done; dropped from any active state; review may bounce back to # in-progress. done is terminal. @@ -62,7 +69,8 @@ finalize_reviewers = [ # Bug severity scale (how bad the defect is — distinct from risk, which # scores the fix). Overrides append (add bands); renaming or removing a -# baked band means forking the skill. +# baked band means forking the skill. Advisory for the conversation: the +# scripts validate severity as an integer 1-5 regardless of added bands. severity_scale = [ "1 — cosmetic: annoyance, no functional impact", "2 — minor: workaround exists, limited users affected", diff --git a/src/bmm-skills/plan/bmad-ticket/evals/fixtures/tree/index.md b/src/bmm-skills/plan/bmad-ticket/evals/fixtures/tree/index.md index 3fb94ddc9..fa36ebdee 100644 --- a/src/bmm-skills/plan/bmad-ticket/evals/fixtures/tree/index.md +++ b/src/bmm-skills/plan/bmad-ticket/evals/fixtures/tree/index.md @@ -6,4 +6,4 @@ generated: true # Ticket Index * [Notification service](notifications/ticket.md) - Real-time alerts users configure themselves -* [Snooze button polish](NOTI-3-snooze-button.md) +* [Snooze button visual polish](NOTI-3-snooze-button.md) diff --git a/src/bmm-skills/plan/bmad-ticket/references/slice-epics.md b/src/bmm-skills/plan/bmad-ticket/references/slice-epics.md index 62eff5e18..564796514 100644 --- a/src/bmm-skills/plan/bmad-ticket/references/slice-epics.md +++ b/src/bmm-skills/plan/bmad-ticket/references/slice-epics.md @@ -10,7 +10,7 @@ A ticket-shaped aside raised mid-session (a defect, a follow-up, an out-of-scope 2. **Inventory.** Extract the requirement inventory in the input's own id scheme — CAP ids from a spec, FR/NFR/UX-DR from a PRD, REQ ids from an imported doc — whatever exists, verbatim. If the input has no stable ids, propose stamping them: generate the inventory in-session and offer (never silently apply) edits to the source doc. This inventory is the coverage contract. -3. **Propose the skeleton, then the set.** In the interactive modes, the skeleton comes first: each epic as a title, a one-line description, and proposed `covers:` — cheap to reshape, so the user reshapes it here, before any full envelope is drafted. Once the skeleton stands (autonomous proceeds without the pause), draft the full envelopes: goals, boundary rationale, `covers:`. Epic altitude follows the doctrine: **the epic is the single-person unit of work** — one dev drives it to completion with AI as the workforce. User-value-first; no technical-layer epics. Other boundaries (component split, team boundary) are legal and discussable, but single-person ownership is the recommendation that calibrates sizing. Greenfield project (nothing deployed, no CI) → Epic 1 is the scaffolding epic, calibrated to the project's what/who/purpose — `references/greenfield-guidelines.md` carries the doctrine. +3. **Propose the skeleton, then the set.** In the interactive modes, the skeleton comes first: each epic as a title, a one-line description, and proposed `covers:` — cheap to reshape, so the user reshapes it here, before any full envelope is drafted. Once the skeleton stands (autonomous proceeds without the pause), draft the full envelopes: goals, boundary rationale, `covers:`, and a scored `risk` (1–5, the ceiling hint for the epic's children) with a one-line rationale — hard floors bind here too (schema migrations, data deletion, auth, payments → 4 minimum). Epic altitude follows the doctrine: **the epic is the single-person unit of work** — one dev drives it to completion with AI as the workforce. User-value-first; no technical-layer epics. Other boundaries (component split, team boundary) are legal and discussable, but single-person ownership is the recommendation that calibrates sizing. Greenfield project (nothing deployed, no CI) → Epic 1 is the scaffolding epic, calibrated to the project's what/who/purpose — `references/greenfield-guidelines.md` carries the doctrine. 4. **Sequence.** The conversation happens in phase language ("scaffolding first, then the core flow"); encode it as epic-level `depends_on` edges and record the rationale in each envelope's Sequencing Notes — there is no phase field, and none is invented. diff --git a/src/bmm-skills/plan/bmad-ticket/scripts/tests/test_ticket_tree.py b/src/bmm-skills/plan/bmad-ticket/scripts/tests/test_ticket_tree.py index 51b2e187e..6c0bafe96 100644 --- a/src/bmm-skills/plan/bmad-ticket/scripts/tests/test_ticket_tree.py +++ b/src/bmm-skills/plan/bmad-ticket/scripts/tests/test_ticket_tree.py @@ -84,21 +84,55 @@ class TicketTreeTests(unittest.TestCase): self.assertNotIn("ALRT-31", ids) # dep is epic ALRT-3, not computed done self.assertNotIn("ALRT-12", ids) # already done - def test_epic_dep_releases_when_children_done(self): + def test_epic_dep_releases_only_when_marked_done(self): + # All children done — epic done is intentional, never computed. (self.root / "alert-rules" / "ALRT-13-rule-eval.md").write_text( ticket("ALRT-13", "story", "Rule eval", status="done", deps="[ALRT-12]")) code, out = run("frontier", "--root", str(self.root)) ids = [t["id"] for t in out["frontier"]] - self.assertIn("ALRT-31", ids) # epic now computed done + self.assertNotIn("ALRT-31", ids) # still gated: nobody marked the epic done + # Retrospective/user stores done on the envelope — now it releases. + (self.root / "alert-rules" / "ticket.md").write_text(ticket( + "ALRT-3", "epic", "Alert rules", covers="[CAP-4]", + extra='description: "Rules people manage"\nstatus: done\n')) + code, out = run("frontier", "--root", str(self.root)) + ids = [t["id"] for t in out["frontier"]] + self.assertIn("ALRT-31", ids) def test_board_states(self): code, out = run("board", "--root", str(self.root)) states = {e["id"]: e["state"] for e in out["epics"]} self.assertEqual(states["ALRT-3"], "in-progress") # one done, one backlog - self.assertEqual(states["ALRT-40"], "unsliced") + self.assertEqual(states["ALRT-40"], "not-started") # childless self.assertEqual(out["blocked"][0]["id"], "ALRT-31") self.assertEqual(out["leaf_totals"]["done"], 1) + def test_board_all_children_done_is_still_in_progress(self): + (self.root / "alert-rules" / "ALRT-13-rule-eval.md").write_text( + ticket("ALRT-13", "story", "Rule eval", status="done", deps="[ALRT-12]")) + code, out = run("board", "--root", str(self.root)) + states = {e["id"]: e["state"] for e in out["epics"]} + self.assertEqual(states["ALRT-3"], "in-progress") # done is never computed + + def test_board_all_dropped_epic_is_not_started(self): + for name, tid in (("ALRT-12-rule-crud.md", "ALRT-12"), + ("ALRT-13-rule-eval.md", "ALRT-13")): + (self.root / "alert-rules" / name).write_text( + ticket(tid, "story", "x", status="dropped")) + code, out = run("board", "--root", str(self.root)) + states = {e["id"]: e["state"] for e in out["epics"]} + self.assertEqual(states["ALRT-3"], "not-started") # abandoned != shipped + code, out = run("frontier", "--root", str(self.root)) + self.assertNotIn("ALRT-31", [t["id"] for t in out["frontier"]]) + + def test_board_stored_done_wins(self): + (self.root / "reporting" / "ticket.md").write_text(ticket( + "ALRT-40", "epic", "Reporting", + extra='description: "Reports"\nstatus: done\n')) + code, out = run("board", "--root", str(self.root)) + states = {e["id"]: e["state"] for e in out["epics"]} + self.assertEqual(states["ALRT-40"], "done") + def test_validate_clean_tree(self): code, out = run("validate", "--root", str(self.root)) self.assertEqual(code, 0, out) @@ -157,7 +191,93 @@ class TicketTreeTests(unittest.TestCase): code, out = run("coverage", "--root", str(self.root), "--require", "CAP-4,CAP-9", "--proposed", "CAP-9") self.assertEqual(out["uncovered"], []) - self.assertIn("(proposed)", out["covered"]["CAP-9"]) + self.assertEqual(out["proposed"], ["CAP-9"]) + self.assertNotIn("CAP-9", out["covered"]) # proposed is never real coverage + + def test_validate_flags_duplicate_ids(self): + (self.root / "ALRT-31-dupe.md").write_text( + ticket("ALRT-31", "task", "Snooze dupe")) + code, out = run("validate", "--root", str(self.root)) + self.assertEqual(code, 1) + msgs = " | ".join(e["error"] for e in out["errors"]) + self.assertIn("duplicate id ALRT-31", msgs) + + def test_malformed_file_is_an_error_not_invisible(self): + (self.root / "ALRT-50-broken.md").write_text( + "---\nschema: 1\nid: ALRT-50\ntype: task\n") # unterminated block + code, out = run("validate", "--root", str(self.root)) + self.assertEqual(code, 1) + msgs = " | ".join(e["error"] for e in out["errors"]) + self.assertIn("failed to parse", msgs) + code, out = run("next-id", "--root", str(self.root)) + self.assertEqual(out["id"], "ALRT-41") # broken file can't be counted... + self.assertTrue(out.get("warnings")) # ...so the caller is warned + + def test_validate_flags_unclosed_inline_list(self): + (self.root / "ALRT-51-badlist.md").write_text( + ticket("ALRT-51", "task", "Bad list").replace( + "depends_on: []", "depends_on: [ALRT-12, ALRT-13")) + code, out = run("validate", "--root", str(self.root)) + self.assertEqual(code, 1) + msgs = " | ".join(e["error"] for e in out["errors"]) + self.assertIn("failed to parse", msgs) + + def test_validate_flags_orphan_leaf_folder(self): + orphan = self.root / "alert-rules" / "notes" + orphan.mkdir() + (orphan / "ALRT-52-orphan.md").write_text(ticket("ALRT-52", "task", "Orphan")) + code, out = run("validate", "--root", str(self.root)) + self.assertEqual(code, 1) + msgs = " | ".join(e["error"] for e in out["errors"]) + self.assertIn("no epic ticket.md", msgs) + + def test_scalar_dep_is_not_iterated_charwise(self): + (self.root / "ALRT-53-scalar.md").write_text( + ticket("ALRT-53", "task", "Scalar dep").replace( + "depends_on: []", "depends_on: ALRT-13")) + code, out = run("board", "--root", str(self.root)) + blocked = {b["id"]: b["waiting_on"] for b in out["blocked"]} + self.assertEqual(blocked["ALRT-53"], ["ALRT-13"]) # one id, not 7 characters + code, out = run("validate", "--root", str(self.root)) + self.assertEqual(code, 1) # still a schema violation validate names + self.assertIn("must be a list", + " | ".join(e["error"] for e in out["errors"])) + + def test_validate_path_skips_treewide_cycle(self): + (self.root / "alert-rules" / "ALRT-13-rule-eval.md").write_text( + ticket("ALRT-13", "story", "Rule eval", deps="[ALRT-12]")) + (self.root / "alert-rules" / "ALRT-12-rule-crud.md").write_text( + ticket("ALRT-12", "story", "Rule CRUD", status="done", deps="[ALRT-13]")) + code, out = run("validate", "--root", str(self.root), + "--path", str(self.root / "ALRT-31-snooze.md")) + self.assertEqual(code, 0, out) # the cycle lives in two other files + + def test_graph_survives_deep_chains(self): + with tempfile.TemporaryDirectory() as d: + root = Path(d) + (root / "index.md").write_text("---\nkey: DEEP\n---\n") + n = 1200 # past the default recursion limit + for i in range(1, n + 1): + deps = f"[DEEP-{i - 1}]" if i > 1 else "[]" + (root / f"DEEP-{i}-t.md").write_text( + ticket(f"DEEP-{i}", "task", f"t{i}", deps=deps)) + code, out = run("graph", "--root", str(root)) + self.assertEqual(code, 0) + self.assertTrue(out["ok"]) + self.assertEqual(len(out["lanes"]), n) + self.assertEqual(len(out["critical_path"]), n) + + def test_mermaid_sanitizes_hostile_ids_and_titles(self): + (self.root / "KEY-n-template.md").write_text( + "---\nschema: 1\nid: [KEY-n]\ntype: story\n" + 'title: "[Outcome] with \\"quotes\\""\nstatus: backlog\n' + "depends_on: []\ncovers: []\nrisk: 2\nhitl: false\n" + "created: 2026-08-01\n---\n\n# x\n") + code, out = run("graph", "--root", str(self.root), "--mermaid") + self.assertEqual(code, 0) + for line in out["mermaid"].splitlines()[1:]: + if "[" in line: # node lines: id must be word-safe, label bracket-free + self.assertRegex(line.strip(), r'^\w+\["[^"\[\]]*"\]$') def test_coverage(self): code, out = run("coverage", "--root", str(self.root), diff --git a/src/bmm-skills/plan/bmad-ticket/scripts/tests/test_update_ticket.py b/src/bmm-skills/plan/bmad-ticket/scripts/tests/test_update_ticket.py index c44a7f6ff..066b11ed3 100644 --- a/src/bmm-skills/plan/bmad-ticket/scripts/tests/test_update_ticket.py +++ b/src/bmm-skills/plan/bmad-ticket/scripts/tests/test_update_ticket.py @@ -1,6 +1,6 @@ #!/usr/bin/env python3 # /// script -# requires-python = ">=3.9" +# requires-python = ">=3.11" # dependencies = [] # /// """Tests for update_ticket.py — run: uv run python -m unittest discover -s scripts/tests""" @@ -104,17 +104,72 @@ class UpdateTicketTests(unittest.TestCase): self.assertEqual(code, 1) self.assertIn("unknown status", out["error"]) - def test_epic_only_dropped(self): + def test_epic_status_intentional_only(self): code, out = run("--path", str(self.epic), "--set", "status=in-progress") self.assertEqual(code, 1) self.assertIn("dropped", out["error"]) # Structural, not a transition rule: --force does not open it. code, out = run("--path", str(self.epic), "--set", "status=in-progress", "--force") self.assertEqual(code, 1) + # done and dropped are both storable — intentional human calls. + code, out = run("--path", str(self.epic), "--set", "status=done") + self.assertEqual(code, 0) + self.assertIn("status: done", self.epic.read_text()) code, out = run("--path", str(self.epic), "--set", "status=dropped") self.assertEqual(code, 0) self.assertIn("status: dropped", self.epic.read_text()) + def test_insert_and_replace_adjacent_lines_both_land(self): + # status is absent on the epic (insert after type:) while title is the + # very next line (replace) — both must survive the splice. + code, out = run("--path", str(self.epic), "--set", "status=done", + "--set", "title=Renamed epic") + self.assertEqual(code, 0) + text = self.epic.read_text() + self.assertIn("status: done", text) + self.assertIn('title: "Renamed epic"', text) + self.assertEqual(text.count("title:"), 1) + self.assertEqual(text.count("description:"), 1) + + def test_newline_in_value_refused(self): + code, out = run("--path", str(self.story), + "--set", "title=Pwned\n---\njunk: yes") + self.assertEqual(code, 1) + self.assertIn("single line", out["error"]) + self.assertIn('title: "Rule CRUD"', self.story.read_text()) # untouched + + def test_cycle_rejected_through_block_style_dep(self): + # ALRT-13 declares its dep in block form; the gate must still see it. + (self.root / "alert-rules" / "ALRT-13-rule-eval.md").write_text( + STORY.replace("id: ALRT-12", "id: ALRT-13") + .replace("depends_on: []", "depends_on:\n - ALRT-12")) + code, out = run("--root", str(self.root), "--id", "ALRT-12", + "--set", "depends_on=ALRT-13") + self.assertEqual(code, 1) + self.assertIn("cycle", out["error"]) + + def test_quoted_id_resolves(self): + quoted = self.root / "ALRT-77-quoted.md" + quoted.write_text(STORY.replace("id: ALRT-12", 'id: "ALRT-77"')) + code, out = run("--root", str(self.root), "--id", "ALRT-77", + "--set", "status=in-progress") + self.assertEqual(code, 0) + self.assertTrue(out["file"].endswith("ALRT-77-quoted.md")) + + def test_duplicate_id_refused(self): + (self.root / "ALRT-12-clone.md").write_text(STORY) + code, out = run("--root", str(self.root), "--id", "ALRT-12", + "--set", "status=in-progress") + self.assertEqual(code, 1) + self.assertIn("duplicated", out["error"]) + + def test_malformed_target_fails_json_clean(self): + bad = self.root / "ALRT-88-bad.md" + bad.write_text("---\nschema: 1\nid: ALRT-88\ntype: task\n") # unterminated + code, out = run("--path", str(bad), "--set", "status=in-progress") + self.assertEqual(code, 1) + self.assertIn("unterminated", out["error"]) + def test_immutable_fields(self): for spec in ("id=ALRT-99", "type=task", "created=2026-01-01", "schema=2"): code, out = run("--path", str(self.story), "--set", spec) diff --git a/src/bmm-skills/plan/bmad-ticket/scripts/ticket_tree.py b/src/bmm-skills/plan/bmad-ticket/scripts/ticket_tree.py index 2d89f43c6..b60d0144f 100644 --- a/src/bmm-skills/plan/bmad-ticket/scripts/ticket_tree.py +++ b/src/bmm-skills/plan/bmad-ticket/scripts/ticket_tree.py @@ -82,7 +82,9 @@ def parse_frontmatter(text): continue fm[key] = "" elif rest.startswith("["): - inner = rest[1:-1].strip() if rest.endswith("]") else rest[1:].strip() + if not rest.endswith("]"): + return None # unclosed inline list — malformed frontmatter + inner = rest[1:-1].strip() fm[key] = [parse_scalar(p) for p in inner.split(",")] if inner else [] else: fm[key] = parse_scalar(rest) @@ -91,23 +93,40 @@ def parse_frontmatter(text): def scan(root): - """Return (tickets, by_id). Each ticket: frontmatter + _path (Path) + _rel (str).""" + """Return (tickets, by_id, problems). Each ticket: frontmatter + _path + _rel. + problems: files that claim to be tickets but can't be trusted — unreadable, + unparseable frontmatter, missing id/type, or a duplicate id (first file wins).""" tickets = [] by_id = {} + problems = [] for p in sorted(root.rglob("*.md")): if p.name == "index.md": continue + rel = p.relative_to(root).as_posix() try: - fm = parse_frontmatter(p.read_text(encoding="utf-8")) - except (OSError, UnicodeDecodeError): + text = p.read_text(encoding="utf-8") + except (OSError, UnicodeDecodeError) as e: + problems.append({"file": rel, "error": f"unreadable: {e.__class__.__name__}"}) continue - if not fm or "id" not in fm or "type" not in fm: + fm = parse_frontmatter(text) + if fm is None: + if text.lstrip().startswith("---"): + problems.append({"file": rel, "error": + "frontmatter failed to parse (unterminated block or malformed inline list)"}) + continue + if "id" not in fm or "type" not in fm: + problems.append({"file": rel, "error": "frontmatter lacks id or type"}) + continue + tid = str(fm["id"]) + if tid in by_id: + problems.append({"file": rel, "error": + f"duplicate id {tid} — also in {by_id[tid]['_rel']}"}) continue fm["_path"] = p - fm["_rel"] = p.relative_to(root).as_posix() + fm["_rel"] = rel tickets.append(fm) - by_id[str(fm["id"])] = fm - return tickets, by_id + by_id[tid] = fm + return tickets, by_id, problems def children_of(epic, tickets): @@ -125,24 +144,21 @@ def children_of(epic, tickets): def epic_state(epic, tickets, _seen=None): - if epic.get("status") == DROPPED: - return DROPPED + """done and dropped are stored, intentional facts (retrospective or user) — + never computed. Computed state is only: not-started (no children, or none + past backlog) or in-progress (any child underway or resolved done).""" + stored = epic.get("status") + if stored in (DONE, DROPPED): + return stored _seen = _seen or set() if str(epic["id"]) in _seen: return "in-progress" # cycle guard; graph lint owns real diagnosis _seen.add(str(epic["id"])) kids = children_of(epic, tickets) - if not kids: - return "unsliced" states = [epic_state(k, tickets, _seen) if k["type"] == "epic" else k.get("status") for k in kids] - active = [s for s in states if s not in (DONE, DROPPED)] - if not active: - return DONE started = any(s in (DONE, "in-progress", "review") for s in states) - if not started and all(s in ("backlog", "unsliced") for s in active): - return "backlog" - return "in-progress" + return "in-progress" if started else "not-started" def dep_done(dep_id, by_id, tickets): @@ -155,6 +171,15 @@ def dep_done(dep_id, by_id, tickets): return t.get("status") == DONE, t.get("status") +def as_list(ticket, field): + """List fields defensively: a scalar (schema violation validate will name) + reads as a one-entry list, never iterated character by character.""" + v = ticket.get(field) + if isinstance(v, list): + return v + return [v] if v not in (None, "") else [] + + def read_index_key(root): idx = root / "index.md" if not idx.is_file(): @@ -189,16 +214,19 @@ def id_num(ticket_id): def cmd_next_id(root, args): key = resolve_key(root, args.key) - tickets, _ = scan(root) + tickets, _, problems = scan(root) nums = [id_num(t["id"]) for t in tickets - if re.fullmatch(re.escape(key) + r"-\d+", str(t["id"]))] + if re.fullmatch(re.escape(key) + r"-\d+", str(t["id"]), re.IGNORECASE)] nxt = (max(nums) + 1) if nums else 1 - print(json.dumps({"ok": True, "key": key, "next": nxt, "id": f"{key}-{nxt}"})) + out = {"ok": True, "key": key, "next": nxt, "id": f"{key}-{nxt}"} + if problems: + out["warnings"] = problems # a broken file could be hiding an issued id + print(json.dumps(out)) def cmd_index(root, args): key = resolve_key(root, args.key) - tickets, _ = scan(root) + tickets, _, problems = scan(root) def entry_lines(t, depth): pad = " " * depth @@ -223,18 +251,20 @@ def cmd_index(root, args): body.extend(entry_lines(lf, 0)) body.append("") atomic_write(root / "index.md", "\n".join(body)) - print(json.dumps({"ok": True, "file": str(root / "index.md"), - "entries": len(tickets)})) + out = {"ok": True, "file": str(root / "index.md"), "entries": len(tickets)} + if problems: + out["warnings"] = problems + print(json.dumps(out)) def cmd_frontier(root, args): - tickets, by_id = scan(root) + tickets, by_id, _ = scan(root) out = [] for t in tickets: if t["type"] not in LEAF_TYPES or t.get("status") != "backlog": continue unmet = [] - for dep in t.get("depends_on") or []: + for dep in as_list(t, "depends_on"): ok, state = dep_done(dep, by_id, tickets) if not ok: unmet.append({"id": str(dep), "state": state}) @@ -245,7 +275,7 @@ def cmd_frontier(root, args): def cmd_board(root, args): - tickets, by_id = scan(root) + tickets, by_id, _ = scan(root) epics, blocked = [], [] totals = {} for t in tickets: @@ -259,7 +289,7 @@ def cmd_board(root, args): else: totals[t.get("status", "?")] = totals.get(t.get("status", "?"), 0) + 1 if t.get("status") == "backlog": - unmet = [str(d) for d in (t.get("depends_on") or []) + unmet = [str(d) for d in as_list(t, "depends_on") if not dep_done(d, by_id, tickets)[0]] if unmet: blocked.append({"id": str(t["id"]), "waiting_on": unmet}) @@ -272,27 +302,33 @@ def cmd_board(root, args): def cmd_graph(root, args): - tickets, by_id = scan(root) - deps_of = {str(t["id"]): [str(d) for d in (t.get("depends_on") or []) if str(d) in by_id] + tickets, by_id, _ = scan(root) + deps_of = {str(t["id"]): [str(d) for d in as_list(t, "depends_on") if str(d) in by_id] for t in tickets} edges = [[d, n] for n, ds in sorted(deps_of.items()) for d in ds] - memo, onstack = {}, set() - - def depth(n): - if n in memo: - return memo[n] - if n in onstack: - memo[n] = 0 # cycle guard; validate owns diagnosis - return 0 - onstack.add(n) - d = 0 if not deps_of[n] else 1 + max(depth(x) for x in deps_of[n]) - onstack.discard(n) - memo[n] = d - return d - - for n in deps_of: - depth(n) + # Longest-path depth, iterative (no recursion limit on deep chains). + # Cycle participants contribute depth 0; validate owns cycle diagnosis. + memo = {} + for start in deps_of: + if start in memo: + continue + stack, onstack = [start], {start} + while stack: + n = stack[-1] + if n in memo: + stack.pop() + onstack.discard(n) + continue + pending = [d for d in deps_of[n] if d not in memo and d not in onstack] + if pending: + stack.extend(pending) + onstack.update(pending) + continue + done_deps = [memo[d] for d in deps_of[n] if d in memo] + memo[n] = (1 + max(done_deps)) if done_deps else 0 + stack.pop() + onstack.discard(n) max_depth = max(memo.values(), default=0) lanes = [[n for n in sorted(deps_of, key=id_num) if memo[n] == i] for i in range(max_depth + 1)] @@ -309,13 +345,16 @@ def cmd_graph(root, args): "lanes": lanes, "critical_path": path} if getattr(args, "mermaid", False): def mid(tid): - return tid.replace("-", "_") + return re.sub(r"[^A-Za-z0-9_]", "_", str(tid)) + + def label(s): + return re.sub(r'[\[\]"\n]', "'", str(s)) lines = ["flowchart TD"] for t in tickets: tid = str(t["id"]) - title = str(t.get("title") or "").replace('"', "'") + title = label(t.get("title") or "") suffix = "" if t["type"] == "epic" else f" ({t.get('status')})" - lines.append(f' {mid(tid)}["{tid} {title}{suffix}"]') + lines.append(f' {mid(tid)}["{label(tid)} {title}{suffix}"]') for d, n in edges: lines.append(f" {mid(d)} --> {mid(n)}") out["mermaid"] = "\n".join(lines) @@ -323,23 +362,26 @@ def cmd_graph(root, args): def cmd_coverage(root, args): - tickets, _ = scan(root) + tickets, _, _ = scan(root) covered = {} for t in tickets: - for cid in t.get("covers") or []: + for cid in as_list(t, "covers"): covered.setdefault(str(cid), []).append(str(t["id"])) + proposed = [] if getattr(args, "proposed", None): - for cid in (p.strip() for p in args.proposed.split(",") if p.strip()): - covered.setdefault(cid, []).append("(proposed)") + proposed = [p.strip() for p in args.proposed.split(",") if p.strip()] result = {"ok": True, "covered": covered} + if proposed: + result["proposed"] = proposed if args.require: required = [r.strip() for r in args.require.split(",") if r.strip()] - result["uncovered"] = [r for r in required if r not in covered] + result["uncovered"] = [r for r in required + if r not in covered and r not in proposed] print(json.dumps(result)) def cmd_list(root, args): - tickets, _ = scan(root) + tickets, _, _ = scan(root) rows = [{"id": str(t["id"]), "type": t["type"], "title": t.get("title"), "status": t.get("status"), "path": t["_rel"]} for t in sorted(tickets, key=lambda x: id_num(x["id"]))] @@ -351,16 +393,23 @@ def _placeholderish(v): def cmd_validate(root, args): - tickets, by_id = scan(root) + tickets, by_id, problems = scan(root) errors = [] def err(t, msg): errors.append({"file": t["_rel"], "error": msg}) only = Path(args.path).resolve() if getattr(args, "path", None) else None - if only and not any(t["_path"].resolve() == only for t in tickets): - errors.append({"file": str(only), "error": - "not a parseable ticket — frontmatter with id and type required"}) + if only: + errors.extend(p for p in problems + if (root / p["file"]).resolve() == only) + if not any(t["_path"].resolve() == only for t in tickets) and not errors: + errors.append({"file": str(only), "error": + "not a parseable ticket — frontmatter with id and type required"}) + else: + errors.extend(problems) + + epic_dirs = {t["_path"].parent for t in tickets if t["type"] == "epic"} for t in tickets: if only and t["_path"].resolve() != only: @@ -372,8 +421,12 @@ def cmd_validate(root, args): if t["type"] == "epic": if t["_path"].name != "ticket.md": err(t, "epic envelope must be named ticket.md") - elif tid and not t["_path"].name.startswith(tid + "-"): - err(t, f"leaf filename must start with '{tid}-'") + else: + if tid and not t["_path"].name.startswith(tid + "-"): + err(t, f"leaf filename must start with '{tid}-'") + if t["_path"].parent != root and t["_path"].parent not in epic_dirs: + err(t, "leaf sits in a folder with no epic ticket.md — invisible to " + "index and board; move it to the bin or an epic folder") if not isinstance(t.get("schema"), int): err(t, "schema must be an integer") title = t.get("title") @@ -406,8 +459,9 @@ def cmd_validate(root, args): desc = t.get("description") if not isinstance(desc, str) or not desc.strip() or _placeholderish(desc): err(t, f"epic description is missing or a placeholder: {desc!r}") - if t.get("status") not in (None, DROPPED): - err(t, f"epics store no lifecycle — only 'dropped' is storable, got {t.get('status')!r}") + if t.get("status") not in (None, DONE, DROPPED): + err(t, "epics compute progress — only intentional 'done' or 'dropped' " + f"is storable, got {t.get('status')!r}") for banned in ("hitl", "severity"): if banned in t: err(t, f"{banned} is not an epic field") @@ -423,9 +477,10 @@ def cmd_validate(root, args): elif "severity" in t: err(t, "severity is a bug-only field") - # Tree-wide cycle check over valid ids. - graph = {str(t["id"]): [str(d) for d in (t.get("depends_on") or [])] - for t in tickets if isinstance(t.get("id"), str)} + # Tree-wide cycle check over valid ids (skipped for single-file runs — + # a per-file gate shouldn't fail on a cycle between two other files). + graph = {} if only else {str(t["id"]): [str(d) for d in as_list(t, "depends_on")] + for t in tickets if isinstance(t.get("id"), str)} seen_in_cycle = set() for start in graph: if start in seen_in_cycle: @@ -479,9 +534,14 @@ def main(): root = Path(args.root) if not root.is_dir(): fail(f"no such directory: {args.root}") - {"next-id": cmd_next_id, "index": cmd_index, "validate": cmd_validate, - "list": cmd_list, "frontier": cmd_frontier, "board": cmd_board, - "coverage": cmd_coverage, "graph": cmd_graph}[args.verb](root, args) + try: + {"next-id": cmd_next_id, "index": cmd_index, "validate": cmd_validate, + "list": cmd_list, "frontier": cmd_frontier, "board": cmd_board, + "coverage": cmd_coverage, "graph": cmd_graph}[args.verb](root, args) + except SystemExit: + raise + except Exception as e: # keep the JSON output contract on fs/encoding errors + fail(f"{e.__class__.__name__}: {e}") if __name__ == "__main__": diff --git a/src/bmm-skills/plan/bmad-ticket/scripts/update_ticket.py b/src/bmm-skills/plan/bmad-ticket/scripts/update_ticket.py index ffeae79a7..8e653a5d8 100644 --- a/src/bmm-skills/plan/bmad-ticket/scripts/update_ticket.py +++ b/src/bmm-skills/plan/bmad-ticket/scripts/update_ticket.py @@ -87,7 +87,10 @@ def parse_scalar(raw): def parse_inline_list(raw): - inner = raw.strip()[1:-1].strip() + raw = raw.strip() + if not raw.endswith("]"): + raise ValueError(f"unclosed inline list: {raw!r}") + inner = raw[1:-1].strip() if not inner: return [] return [parse_scalar(part) for part in inner.split(",")] @@ -95,16 +98,17 @@ def parse_inline_list(raw): def parse_frontmatter(lines): """Return (entries, close_idx). entries: list of dicts {key, value, start, end} - where start/end are line indexes [start, end) covering the entry.""" + where start/end are line indexes [start, end) covering the entry. + Raises ValueError on malformed frontmatter.""" if not lines or lines[0].strip() != "---": - fail("no frontmatter found (file does not start with ---)") + raise ValueError("no frontmatter found (file does not start with ---)") close = None for i in range(1, len(lines)): if lines[i].strip() == "---": close = i break if close is None: - fail("no frontmatter found (unterminated --- block)") + raise ValueError("no frontmatter found (unterminated --- block)") entries = [] i = 1 while i < close: @@ -150,6 +154,9 @@ def render(key, value): def coerce(field, raw, ticket_type): """Validate and convert a raw --set string for a field. Returns value or fails.""" + if any(c in raw for c in "\n\r"): + fail(f"{field} must be a single line — a newline in a value would " + "terminate the frontmatter block") if field in LIST_FIELDS: return [p.strip() for p in raw.split(",") if p.strip()] if field in INT_FIELDS: @@ -168,29 +175,29 @@ def coerce(field, raw, ticket_type): def collect_tree(root): - """Map every ticket in the tree: id -> path, and id -> depends_on list.""" - ids, deps = {}, {} + """Map every ticket in the tree: id -> path, id -> depends_on list, and + id -> list of duplicate paths. Uses the same parser as the target file so + quoted ids and block-style lists resolve identically everywhere.""" + ids, deps, dups = {}, {}, {} for p in sorted(root.rglob("*.md")): + if p.name == "index.md": + continue try: lines = p.read_text(encoding="utf-8").splitlines(keepends=True) - except (OSError, UnicodeDecodeError): + entries, _ = parse_frontmatter(lines) + except (OSError, UnicodeDecodeError, ValueError): continue - if not lines or lines[0].strip() != "---": + fm = {e["key"]: e["value"] for e in entries} + if "id" not in fm: continue - tid, tdeps = None, [] - for line in lines[1:60]: - if line.strip() == "---": - break - m = re.match(r"^id:\s*(\S+)\s*$", line) - if m: - tid = m.group(1) - m = re.match(r"^depends_on:\s*\[(.*)\]\s*$", line) - if m: - tdeps = [x.strip() for x in m.group(1).split(",") if x.strip()] - if tid: - ids[tid] = p - deps[tid] = tdeps - return ids, deps + tid = str(fm["id"]) + if tid in ids: + dups.setdefault(tid, [ids[tid]]).append(p) + continue + tdeps = fm.get("depends_on") + ids[tid] = p + deps[tid] = [str(d) for d in tdeps] if isinstance(tdeps, list) else [] + return ids, deps, dups def cycle_via(graph, start): @@ -254,7 +261,11 @@ def main(): root = Path(args.root) if not root.is_dir(): fail(f"no such directory: {args.root}") - tree_ids, tree_deps = collect_tree(root) + tree_ids, tree_deps, dups = collect_tree(root) + if args.id in dups: + files = ", ".join(str(p) for p in dups[args.id]) + fail(f"id '{args.id}' is duplicated across: {files} — fix the tree " + "before updating (run ticket_tree.py validate)") if args.id not in tree_ids: known = ", ".join(sorted(tree_ids)) or "none" fail(f"id '{args.id}' not found under {args.root} — known ids: {known}") @@ -263,14 +274,17 @@ def main(): fail("target the ticket with --path FILE, or --root DIR --id KEY-n") if args.root and tree_ids is None: - tree_ids, tree_deps = collect_tree(Path(args.root)) + tree_ids, tree_deps, _ = collect_tree(Path(args.root)) transitions = set(t.strip() for t in args.transitions.split(",") if t.strip()) known_states = {"backlog"} | {s for t in transitions for s in t.split(">")} text = path.read_text(encoding="utf-8") lines = text.splitlines(keepends=True) - entries, close = parse_frontmatter(lines) + try: + entries, close = parse_frontmatter(lines) + except ValueError as e: + fail(str(e)) fm = {e["key"]: e["value"] for e in entries} ticket_type = fm.get("type") if ticket_type not in ("epic", "story", "bug", "task", "spike"): @@ -298,9 +312,10 @@ def main(): new_status = updates["status"] current = fm.get("status") if ticket_type == "epic": - if new_status != "dropped": - fail("epics store no lifecycle — 'dropped' is the only storable epic status; " - "progress is computed from children") + if new_status not in ("done", "dropped"): + fail("epics compute progress from children — the storable epic statuses " + "are 'done' (an intentional call: retrospective or the user) and " + "'dropped'; neither is ever calculated") elif new_status != current: if new_status not in known_states: fail(f"unknown status '{new_status}' — known: {', '.join(sorted(known_states))}") @@ -355,7 +370,12 @@ def main(): if changes: new_lines = list(lines) - for start, end, new_line in sorted(replacements, key=lambda r: r[0], reverse=True): + # Apply bottom-up; at an equal start index the replacement (end > start) + # must land before the insertion (end == start), or the insertion is + # silently spliced away by the replacement of the line it sits on. + for start, end, new_line in sorted(replacements, + key=lambda r: (r[0], r[1] > r[0]), + reverse=True): new_lines[start:end] = [new_line] fd, tmp = tempfile.mkstemp(dir=str(path.parent), prefix=".update-", suffix=".tmp") try: diff --git a/src/scripts/tests/test_ticket_tree.py b/src/scripts/tests/test_ticket_tree.py index 51b2e187e..6c0bafe96 100644 --- a/src/scripts/tests/test_ticket_tree.py +++ b/src/scripts/tests/test_ticket_tree.py @@ -84,21 +84,55 @@ class TicketTreeTests(unittest.TestCase): self.assertNotIn("ALRT-31", ids) # dep is epic ALRT-3, not computed done self.assertNotIn("ALRT-12", ids) # already done - def test_epic_dep_releases_when_children_done(self): + def test_epic_dep_releases_only_when_marked_done(self): + # All children done — epic done is intentional, never computed. (self.root / "alert-rules" / "ALRT-13-rule-eval.md").write_text( ticket("ALRT-13", "story", "Rule eval", status="done", deps="[ALRT-12]")) code, out = run("frontier", "--root", str(self.root)) ids = [t["id"] for t in out["frontier"]] - self.assertIn("ALRT-31", ids) # epic now computed done + self.assertNotIn("ALRT-31", ids) # still gated: nobody marked the epic done + # Retrospective/user stores done on the envelope — now it releases. + (self.root / "alert-rules" / "ticket.md").write_text(ticket( + "ALRT-3", "epic", "Alert rules", covers="[CAP-4]", + extra='description: "Rules people manage"\nstatus: done\n')) + code, out = run("frontier", "--root", str(self.root)) + ids = [t["id"] for t in out["frontier"]] + self.assertIn("ALRT-31", ids) def test_board_states(self): code, out = run("board", "--root", str(self.root)) states = {e["id"]: e["state"] for e in out["epics"]} self.assertEqual(states["ALRT-3"], "in-progress") # one done, one backlog - self.assertEqual(states["ALRT-40"], "unsliced") + self.assertEqual(states["ALRT-40"], "not-started") # childless self.assertEqual(out["blocked"][0]["id"], "ALRT-31") self.assertEqual(out["leaf_totals"]["done"], 1) + def test_board_all_children_done_is_still_in_progress(self): + (self.root / "alert-rules" / "ALRT-13-rule-eval.md").write_text( + ticket("ALRT-13", "story", "Rule eval", status="done", deps="[ALRT-12]")) + code, out = run("board", "--root", str(self.root)) + states = {e["id"]: e["state"] for e in out["epics"]} + self.assertEqual(states["ALRT-3"], "in-progress") # done is never computed + + def test_board_all_dropped_epic_is_not_started(self): + for name, tid in (("ALRT-12-rule-crud.md", "ALRT-12"), + ("ALRT-13-rule-eval.md", "ALRT-13")): + (self.root / "alert-rules" / name).write_text( + ticket(tid, "story", "x", status="dropped")) + code, out = run("board", "--root", str(self.root)) + states = {e["id"]: e["state"] for e in out["epics"]} + self.assertEqual(states["ALRT-3"], "not-started") # abandoned != shipped + code, out = run("frontier", "--root", str(self.root)) + self.assertNotIn("ALRT-31", [t["id"] for t in out["frontier"]]) + + def test_board_stored_done_wins(self): + (self.root / "reporting" / "ticket.md").write_text(ticket( + "ALRT-40", "epic", "Reporting", + extra='description: "Reports"\nstatus: done\n')) + code, out = run("board", "--root", str(self.root)) + states = {e["id"]: e["state"] for e in out["epics"]} + self.assertEqual(states["ALRT-40"], "done") + def test_validate_clean_tree(self): code, out = run("validate", "--root", str(self.root)) self.assertEqual(code, 0, out) @@ -157,7 +191,93 @@ class TicketTreeTests(unittest.TestCase): code, out = run("coverage", "--root", str(self.root), "--require", "CAP-4,CAP-9", "--proposed", "CAP-9") self.assertEqual(out["uncovered"], []) - self.assertIn("(proposed)", out["covered"]["CAP-9"]) + self.assertEqual(out["proposed"], ["CAP-9"]) + self.assertNotIn("CAP-9", out["covered"]) # proposed is never real coverage + + def test_validate_flags_duplicate_ids(self): + (self.root / "ALRT-31-dupe.md").write_text( + ticket("ALRT-31", "task", "Snooze dupe")) + code, out = run("validate", "--root", str(self.root)) + self.assertEqual(code, 1) + msgs = " | ".join(e["error"] for e in out["errors"]) + self.assertIn("duplicate id ALRT-31", msgs) + + def test_malformed_file_is_an_error_not_invisible(self): + (self.root / "ALRT-50-broken.md").write_text( + "---\nschema: 1\nid: ALRT-50\ntype: task\n") # unterminated block + code, out = run("validate", "--root", str(self.root)) + self.assertEqual(code, 1) + msgs = " | ".join(e["error"] for e in out["errors"]) + self.assertIn("failed to parse", msgs) + code, out = run("next-id", "--root", str(self.root)) + self.assertEqual(out["id"], "ALRT-41") # broken file can't be counted... + self.assertTrue(out.get("warnings")) # ...so the caller is warned + + def test_validate_flags_unclosed_inline_list(self): + (self.root / "ALRT-51-badlist.md").write_text( + ticket("ALRT-51", "task", "Bad list").replace( + "depends_on: []", "depends_on: [ALRT-12, ALRT-13")) + code, out = run("validate", "--root", str(self.root)) + self.assertEqual(code, 1) + msgs = " | ".join(e["error"] for e in out["errors"]) + self.assertIn("failed to parse", msgs) + + def test_validate_flags_orphan_leaf_folder(self): + orphan = self.root / "alert-rules" / "notes" + orphan.mkdir() + (orphan / "ALRT-52-orphan.md").write_text(ticket("ALRT-52", "task", "Orphan")) + code, out = run("validate", "--root", str(self.root)) + self.assertEqual(code, 1) + msgs = " | ".join(e["error"] for e in out["errors"]) + self.assertIn("no epic ticket.md", msgs) + + def test_scalar_dep_is_not_iterated_charwise(self): + (self.root / "ALRT-53-scalar.md").write_text( + ticket("ALRT-53", "task", "Scalar dep").replace( + "depends_on: []", "depends_on: ALRT-13")) + code, out = run("board", "--root", str(self.root)) + blocked = {b["id"]: b["waiting_on"] for b in out["blocked"]} + self.assertEqual(blocked["ALRT-53"], ["ALRT-13"]) # one id, not 7 characters + code, out = run("validate", "--root", str(self.root)) + self.assertEqual(code, 1) # still a schema violation validate names + self.assertIn("must be a list", + " | ".join(e["error"] for e in out["errors"])) + + def test_validate_path_skips_treewide_cycle(self): + (self.root / "alert-rules" / "ALRT-13-rule-eval.md").write_text( + ticket("ALRT-13", "story", "Rule eval", deps="[ALRT-12]")) + (self.root / "alert-rules" / "ALRT-12-rule-crud.md").write_text( + ticket("ALRT-12", "story", "Rule CRUD", status="done", deps="[ALRT-13]")) + code, out = run("validate", "--root", str(self.root), + "--path", str(self.root / "ALRT-31-snooze.md")) + self.assertEqual(code, 0, out) # the cycle lives in two other files + + def test_graph_survives_deep_chains(self): + with tempfile.TemporaryDirectory() as d: + root = Path(d) + (root / "index.md").write_text("---\nkey: DEEP\n---\n") + n = 1200 # past the default recursion limit + for i in range(1, n + 1): + deps = f"[DEEP-{i - 1}]" if i > 1 else "[]" + (root / f"DEEP-{i}-t.md").write_text( + ticket(f"DEEP-{i}", "task", f"t{i}", deps=deps)) + code, out = run("graph", "--root", str(root)) + self.assertEqual(code, 0) + self.assertTrue(out["ok"]) + self.assertEqual(len(out["lanes"]), n) + self.assertEqual(len(out["critical_path"]), n) + + def test_mermaid_sanitizes_hostile_ids_and_titles(self): + (self.root / "KEY-n-template.md").write_text( + "---\nschema: 1\nid: [KEY-n]\ntype: story\n" + 'title: "[Outcome] with \\"quotes\\""\nstatus: backlog\n' + "depends_on: []\ncovers: []\nrisk: 2\nhitl: false\n" + "created: 2026-08-01\n---\n\n# x\n") + code, out = run("graph", "--root", str(self.root), "--mermaid") + self.assertEqual(code, 0) + for line in out["mermaid"].splitlines()[1:]: + if "[" in line: # node lines: id must be word-safe, label bracket-free + self.assertRegex(line.strip(), r'^\w+\["[^"\[\]]*"\]$') def test_coverage(self): code, out = run("coverage", "--root", str(self.root), diff --git a/src/scripts/tests/test_update_ticket.py b/src/scripts/tests/test_update_ticket.py index c44a7f6ff..066b11ed3 100644 --- a/src/scripts/tests/test_update_ticket.py +++ b/src/scripts/tests/test_update_ticket.py @@ -1,6 +1,6 @@ #!/usr/bin/env python3 # /// script -# requires-python = ">=3.9" +# requires-python = ">=3.11" # dependencies = [] # /// """Tests for update_ticket.py — run: uv run python -m unittest discover -s scripts/tests""" @@ -104,17 +104,72 @@ class UpdateTicketTests(unittest.TestCase): self.assertEqual(code, 1) self.assertIn("unknown status", out["error"]) - def test_epic_only_dropped(self): + def test_epic_status_intentional_only(self): code, out = run("--path", str(self.epic), "--set", "status=in-progress") self.assertEqual(code, 1) self.assertIn("dropped", out["error"]) # Structural, not a transition rule: --force does not open it. code, out = run("--path", str(self.epic), "--set", "status=in-progress", "--force") self.assertEqual(code, 1) + # done and dropped are both storable — intentional human calls. + code, out = run("--path", str(self.epic), "--set", "status=done") + self.assertEqual(code, 0) + self.assertIn("status: done", self.epic.read_text()) code, out = run("--path", str(self.epic), "--set", "status=dropped") self.assertEqual(code, 0) self.assertIn("status: dropped", self.epic.read_text()) + def test_insert_and_replace_adjacent_lines_both_land(self): + # status is absent on the epic (insert after type:) while title is the + # very next line (replace) — both must survive the splice. + code, out = run("--path", str(self.epic), "--set", "status=done", + "--set", "title=Renamed epic") + self.assertEqual(code, 0) + text = self.epic.read_text() + self.assertIn("status: done", text) + self.assertIn('title: "Renamed epic"', text) + self.assertEqual(text.count("title:"), 1) + self.assertEqual(text.count("description:"), 1) + + def test_newline_in_value_refused(self): + code, out = run("--path", str(self.story), + "--set", "title=Pwned\n---\njunk: yes") + self.assertEqual(code, 1) + self.assertIn("single line", out["error"]) + self.assertIn('title: "Rule CRUD"', self.story.read_text()) # untouched + + def test_cycle_rejected_through_block_style_dep(self): + # ALRT-13 declares its dep in block form; the gate must still see it. + (self.root / "alert-rules" / "ALRT-13-rule-eval.md").write_text( + STORY.replace("id: ALRT-12", "id: ALRT-13") + .replace("depends_on: []", "depends_on:\n - ALRT-12")) + code, out = run("--root", str(self.root), "--id", "ALRT-12", + "--set", "depends_on=ALRT-13") + self.assertEqual(code, 1) + self.assertIn("cycle", out["error"]) + + def test_quoted_id_resolves(self): + quoted = self.root / "ALRT-77-quoted.md" + quoted.write_text(STORY.replace("id: ALRT-12", 'id: "ALRT-77"')) + code, out = run("--root", str(self.root), "--id", "ALRT-77", + "--set", "status=in-progress") + self.assertEqual(code, 0) + self.assertTrue(out["file"].endswith("ALRT-77-quoted.md")) + + def test_duplicate_id_refused(self): + (self.root / "ALRT-12-clone.md").write_text(STORY) + code, out = run("--root", str(self.root), "--id", "ALRT-12", + "--set", "status=in-progress") + self.assertEqual(code, 1) + self.assertIn("duplicated", out["error"]) + + def test_malformed_target_fails_json_clean(self): + bad = self.root / "ALRT-88-bad.md" + bad.write_text("---\nschema: 1\nid: ALRT-88\ntype: task\n") # unterminated + code, out = run("--path", str(bad), "--set", "status=in-progress") + self.assertEqual(code, 1) + self.assertIn("unterminated", out["error"]) + def test_immutable_fields(self): for spec in ("id=ALRT-99", "type=task", "created=2026-01-01", "schema=2"): code, out = run("--path", str(self.story), "--set", spec) diff --git a/src/scripts/ticket_tree.py b/src/scripts/ticket_tree.py index 2d89f43c6..b60d0144f 100644 --- a/src/scripts/ticket_tree.py +++ b/src/scripts/ticket_tree.py @@ -82,7 +82,9 @@ def parse_frontmatter(text): continue fm[key] = "" elif rest.startswith("["): - inner = rest[1:-1].strip() if rest.endswith("]") else rest[1:].strip() + if not rest.endswith("]"): + return None # unclosed inline list — malformed frontmatter + inner = rest[1:-1].strip() fm[key] = [parse_scalar(p) for p in inner.split(",")] if inner else [] else: fm[key] = parse_scalar(rest) @@ -91,23 +93,40 @@ def parse_frontmatter(text): def scan(root): - """Return (tickets, by_id). Each ticket: frontmatter + _path (Path) + _rel (str).""" + """Return (tickets, by_id, problems). Each ticket: frontmatter + _path + _rel. + problems: files that claim to be tickets but can't be trusted — unreadable, + unparseable frontmatter, missing id/type, or a duplicate id (first file wins).""" tickets = [] by_id = {} + problems = [] for p in sorted(root.rglob("*.md")): if p.name == "index.md": continue + rel = p.relative_to(root).as_posix() try: - fm = parse_frontmatter(p.read_text(encoding="utf-8")) - except (OSError, UnicodeDecodeError): + text = p.read_text(encoding="utf-8") + except (OSError, UnicodeDecodeError) as e: + problems.append({"file": rel, "error": f"unreadable: {e.__class__.__name__}"}) continue - if not fm or "id" not in fm or "type" not in fm: + fm = parse_frontmatter(text) + if fm is None: + if text.lstrip().startswith("---"): + problems.append({"file": rel, "error": + "frontmatter failed to parse (unterminated block or malformed inline list)"}) + continue + if "id" not in fm or "type" not in fm: + problems.append({"file": rel, "error": "frontmatter lacks id or type"}) + continue + tid = str(fm["id"]) + if tid in by_id: + problems.append({"file": rel, "error": + f"duplicate id {tid} — also in {by_id[tid]['_rel']}"}) continue fm["_path"] = p - fm["_rel"] = p.relative_to(root).as_posix() + fm["_rel"] = rel tickets.append(fm) - by_id[str(fm["id"])] = fm - return tickets, by_id + by_id[tid] = fm + return tickets, by_id, problems def children_of(epic, tickets): @@ -125,24 +144,21 @@ def children_of(epic, tickets): def epic_state(epic, tickets, _seen=None): - if epic.get("status") == DROPPED: - return DROPPED + """done and dropped are stored, intentional facts (retrospective or user) — + never computed. Computed state is only: not-started (no children, or none + past backlog) or in-progress (any child underway or resolved done).""" + stored = epic.get("status") + if stored in (DONE, DROPPED): + return stored _seen = _seen or set() if str(epic["id"]) in _seen: return "in-progress" # cycle guard; graph lint owns real diagnosis _seen.add(str(epic["id"])) kids = children_of(epic, tickets) - if not kids: - return "unsliced" states = [epic_state(k, tickets, _seen) if k["type"] == "epic" else k.get("status") for k in kids] - active = [s for s in states if s not in (DONE, DROPPED)] - if not active: - return DONE started = any(s in (DONE, "in-progress", "review") for s in states) - if not started and all(s in ("backlog", "unsliced") for s in active): - return "backlog" - return "in-progress" + return "in-progress" if started else "not-started" def dep_done(dep_id, by_id, tickets): @@ -155,6 +171,15 @@ def dep_done(dep_id, by_id, tickets): return t.get("status") == DONE, t.get("status") +def as_list(ticket, field): + """List fields defensively: a scalar (schema violation validate will name) + reads as a one-entry list, never iterated character by character.""" + v = ticket.get(field) + if isinstance(v, list): + return v + return [v] if v not in (None, "") else [] + + def read_index_key(root): idx = root / "index.md" if not idx.is_file(): @@ -189,16 +214,19 @@ def id_num(ticket_id): def cmd_next_id(root, args): key = resolve_key(root, args.key) - tickets, _ = scan(root) + tickets, _, problems = scan(root) nums = [id_num(t["id"]) for t in tickets - if re.fullmatch(re.escape(key) + r"-\d+", str(t["id"]))] + if re.fullmatch(re.escape(key) + r"-\d+", str(t["id"]), re.IGNORECASE)] nxt = (max(nums) + 1) if nums else 1 - print(json.dumps({"ok": True, "key": key, "next": nxt, "id": f"{key}-{nxt}"})) + out = {"ok": True, "key": key, "next": nxt, "id": f"{key}-{nxt}"} + if problems: + out["warnings"] = problems # a broken file could be hiding an issued id + print(json.dumps(out)) def cmd_index(root, args): key = resolve_key(root, args.key) - tickets, _ = scan(root) + tickets, _, problems = scan(root) def entry_lines(t, depth): pad = " " * depth @@ -223,18 +251,20 @@ def cmd_index(root, args): body.extend(entry_lines(lf, 0)) body.append("") atomic_write(root / "index.md", "\n".join(body)) - print(json.dumps({"ok": True, "file": str(root / "index.md"), - "entries": len(tickets)})) + out = {"ok": True, "file": str(root / "index.md"), "entries": len(tickets)} + if problems: + out["warnings"] = problems + print(json.dumps(out)) def cmd_frontier(root, args): - tickets, by_id = scan(root) + tickets, by_id, _ = scan(root) out = [] for t in tickets: if t["type"] not in LEAF_TYPES or t.get("status") != "backlog": continue unmet = [] - for dep in t.get("depends_on") or []: + for dep in as_list(t, "depends_on"): ok, state = dep_done(dep, by_id, tickets) if not ok: unmet.append({"id": str(dep), "state": state}) @@ -245,7 +275,7 @@ def cmd_frontier(root, args): def cmd_board(root, args): - tickets, by_id = scan(root) + tickets, by_id, _ = scan(root) epics, blocked = [], [] totals = {} for t in tickets: @@ -259,7 +289,7 @@ def cmd_board(root, args): else: totals[t.get("status", "?")] = totals.get(t.get("status", "?"), 0) + 1 if t.get("status") == "backlog": - unmet = [str(d) for d in (t.get("depends_on") or []) + unmet = [str(d) for d in as_list(t, "depends_on") if not dep_done(d, by_id, tickets)[0]] if unmet: blocked.append({"id": str(t["id"]), "waiting_on": unmet}) @@ -272,27 +302,33 @@ def cmd_board(root, args): def cmd_graph(root, args): - tickets, by_id = scan(root) - deps_of = {str(t["id"]): [str(d) for d in (t.get("depends_on") or []) if str(d) in by_id] + tickets, by_id, _ = scan(root) + deps_of = {str(t["id"]): [str(d) for d in as_list(t, "depends_on") if str(d) in by_id] for t in tickets} edges = [[d, n] for n, ds in sorted(deps_of.items()) for d in ds] - memo, onstack = {}, set() - - def depth(n): - if n in memo: - return memo[n] - if n in onstack: - memo[n] = 0 # cycle guard; validate owns diagnosis - return 0 - onstack.add(n) - d = 0 if not deps_of[n] else 1 + max(depth(x) for x in deps_of[n]) - onstack.discard(n) - memo[n] = d - return d - - for n in deps_of: - depth(n) + # Longest-path depth, iterative (no recursion limit on deep chains). + # Cycle participants contribute depth 0; validate owns cycle diagnosis. + memo = {} + for start in deps_of: + if start in memo: + continue + stack, onstack = [start], {start} + while stack: + n = stack[-1] + if n in memo: + stack.pop() + onstack.discard(n) + continue + pending = [d for d in deps_of[n] if d not in memo and d not in onstack] + if pending: + stack.extend(pending) + onstack.update(pending) + continue + done_deps = [memo[d] for d in deps_of[n] if d in memo] + memo[n] = (1 + max(done_deps)) if done_deps else 0 + stack.pop() + onstack.discard(n) max_depth = max(memo.values(), default=0) lanes = [[n for n in sorted(deps_of, key=id_num) if memo[n] == i] for i in range(max_depth + 1)] @@ -309,13 +345,16 @@ def cmd_graph(root, args): "lanes": lanes, "critical_path": path} if getattr(args, "mermaid", False): def mid(tid): - return tid.replace("-", "_") + return re.sub(r"[^A-Za-z0-9_]", "_", str(tid)) + + def label(s): + return re.sub(r'[\[\]"\n]', "'", str(s)) lines = ["flowchart TD"] for t in tickets: tid = str(t["id"]) - title = str(t.get("title") or "").replace('"', "'") + title = label(t.get("title") or "") suffix = "" if t["type"] == "epic" else f" ({t.get('status')})" - lines.append(f' {mid(tid)}["{tid} {title}{suffix}"]') + lines.append(f' {mid(tid)}["{label(tid)} {title}{suffix}"]') for d, n in edges: lines.append(f" {mid(d)} --> {mid(n)}") out["mermaid"] = "\n".join(lines) @@ -323,23 +362,26 @@ def cmd_graph(root, args): def cmd_coverage(root, args): - tickets, _ = scan(root) + tickets, _, _ = scan(root) covered = {} for t in tickets: - for cid in t.get("covers") or []: + for cid in as_list(t, "covers"): covered.setdefault(str(cid), []).append(str(t["id"])) + proposed = [] if getattr(args, "proposed", None): - for cid in (p.strip() for p in args.proposed.split(",") if p.strip()): - covered.setdefault(cid, []).append("(proposed)") + proposed = [p.strip() for p in args.proposed.split(",") if p.strip()] result = {"ok": True, "covered": covered} + if proposed: + result["proposed"] = proposed if args.require: required = [r.strip() for r in args.require.split(",") if r.strip()] - result["uncovered"] = [r for r in required if r not in covered] + result["uncovered"] = [r for r in required + if r not in covered and r not in proposed] print(json.dumps(result)) def cmd_list(root, args): - tickets, _ = scan(root) + tickets, _, _ = scan(root) rows = [{"id": str(t["id"]), "type": t["type"], "title": t.get("title"), "status": t.get("status"), "path": t["_rel"]} for t in sorted(tickets, key=lambda x: id_num(x["id"]))] @@ -351,16 +393,23 @@ def _placeholderish(v): def cmd_validate(root, args): - tickets, by_id = scan(root) + tickets, by_id, problems = scan(root) errors = [] def err(t, msg): errors.append({"file": t["_rel"], "error": msg}) only = Path(args.path).resolve() if getattr(args, "path", None) else None - if only and not any(t["_path"].resolve() == only for t in tickets): - errors.append({"file": str(only), "error": - "not a parseable ticket — frontmatter with id and type required"}) + if only: + errors.extend(p for p in problems + if (root / p["file"]).resolve() == only) + if not any(t["_path"].resolve() == only for t in tickets) and not errors: + errors.append({"file": str(only), "error": + "not a parseable ticket — frontmatter with id and type required"}) + else: + errors.extend(problems) + + epic_dirs = {t["_path"].parent for t in tickets if t["type"] == "epic"} for t in tickets: if only and t["_path"].resolve() != only: @@ -372,8 +421,12 @@ def cmd_validate(root, args): if t["type"] == "epic": if t["_path"].name != "ticket.md": err(t, "epic envelope must be named ticket.md") - elif tid and not t["_path"].name.startswith(tid + "-"): - err(t, f"leaf filename must start with '{tid}-'") + else: + if tid and not t["_path"].name.startswith(tid + "-"): + err(t, f"leaf filename must start with '{tid}-'") + if t["_path"].parent != root and t["_path"].parent not in epic_dirs: + err(t, "leaf sits in a folder with no epic ticket.md — invisible to " + "index and board; move it to the bin or an epic folder") if not isinstance(t.get("schema"), int): err(t, "schema must be an integer") title = t.get("title") @@ -406,8 +459,9 @@ def cmd_validate(root, args): desc = t.get("description") if not isinstance(desc, str) or not desc.strip() or _placeholderish(desc): err(t, f"epic description is missing or a placeholder: {desc!r}") - if t.get("status") not in (None, DROPPED): - err(t, f"epics store no lifecycle — only 'dropped' is storable, got {t.get('status')!r}") + if t.get("status") not in (None, DONE, DROPPED): + err(t, "epics compute progress — only intentional 'done' or 'dropped' " + f"is storable, got {t.get('status')!r}") for banned in ("hitl", "severity"): if banned in t: err(t, f"{banned} is not an epic field") @@ -423,9 +477,10 @@ def cmd_validate(root, args): elif "severity" in t: err(t, "severity is a bug-only field") - # Tree-wide cycle check over valid ids. - graph = {str(t["id"]): [str(d) for d in (t.get("depends_on") or [])] - for t in tickets if isinstance(t.get("id"), str)} + # Tree-wide cycle check over valid ids (skipped for single-file runs — + # a per-file gate shouldn't fail on a cycle between two other files). + graph = {} if only else {str(t["id"]): [str(d) for d in as_list(t, "depends_on")] + for t in tickets if isinstance(t.get("id"), str)} seen_in_cycle = set() for start in graph: if start in seen_in_cycle: @@ -479,9 +534,14 @@ def main(): root = Path(args.root) if not root.is_dir(): fail(f"no such directory: {args.root}") - {"next-id": cmd_next_id, "index": cmd_index, "validate": cmd_validate, - "list": cmd_list, "frontier": cmd_frontier, "board": cmd_board, - "coverage": cmd_coverage, "graph": cmd_graph}[args.verb](root, args) + try: + {"next-id": cmd_next_id, "index": cmd_index, "validate": cmd_validate, + "list": cmd_list, "frontier": cmd_frontier, "board": cmd_board, + "coverage": cmd_coverage, "graph": cmd_graph}[args.verb](root, args) + except SystemExit: + raise + except Exception as e: # keep the JSON output contract on fs/encoding errors + fail(f"{e.__class__.__name__}: {e}") if __name__ == "__main__": diff --git a/src/scripts/update_ticket.py b/src/scripts/update_ticket.py index ffeae79a7..8e653a5d8 100644 --- a/src/scripts/update_ticket.py +++ b/src/scripts/update_ticket.py @@ -87,7 +87,10 @@ def parse_scalar(raw): def parse_inline_list(raw): - inner = raw.strip()[1:-1].strip() + raw = raw.strip() + if not raw.endswith("]"): + raise ValueError(f"unclosed inline list: {raw!r}") + inner = raw[1:-1].strip() if not inner: return [] return [parse_scalar(part) for part in inner.split(",")] @@ -95,16 +98,17 @@ def parse_inline_list(raw): def parse_frontmatter(lines): """Return (entries, close_idx). entries: list of dicts {key, value, start, end} - where start/end are line indexes [start, end) covering the entry.""" + where start/end are line indexes [start, end) covering the entry. + Raises ValueError on malformed frontmatter.""" if not lines or lines[0].strip() != "---": - fail("no frontmatter found (file does not start with ---)") + raise ValueError("no frontmatter found (file does not start with ---)") close = None for i in range(1, len(lines)): if lines[i].strip() == "---": close = i break if close is None: - fail("no frontmatter found (unterminated --- block)") + raise ValueError("no frontmatter found (unterminated --- block)") entries = [] i = 1 while i < close: @@ -150,6 +154,9 @@ def render(key, value): def coerce(field, raw, ticket_type): """Validate and convert a raw --set string for a field. Returns value or fails.""" + if any(c in raw for c in "\n\r"): + fail(f"{field} must be a single line — a newline in a value would " + "terminate the frontmatter block") if field in LIST_FIELDS: return [p.strip() for p in raw.split(",") if p.strip()] if field in INT_FIELDS: @@ -168,29 +175,29 @@ def coerce(field, raw, ticket_type): def collect_tree(root): - """Map every ticket in the tree: id -> path, and id -> depends_on list.""" - ids, deps = {}, {} + """Map every ticket in the tree: id -> path, id -> depends_on list, and + id -> list of duplicate paths. Uses the same parser as the target file so + quoted ids and block-style lists resolve identically everywhere.""" + ids, deps, dups = {}, {}, {} for p in sorted(root.rglob("*.md")): + if p.name == "index.md": + continue try: lines = p.read_text(encoding="utf-8").splitlines(keepends=True) - except (OSError, UnicodeDecodeError): + entries, _ = parse_frontmatter(lines) + except (OSError, UnicodeDecodeError, ValueError): continue - if not lines or lines[0].strip() != "---": + fm = {e["key"]: e["value"] for e in entries} + if "id" not in fm: continue - tid, tdeps = None, [] - for line in lines[1:60]: - if line.strip() == "---": - break - m = re.match(r"^id:\s*(\S+)\s*$", line) - if m: - tid = m.group(1) - m = re.match(r"^depends_on:\s*\[(.*)\]\s*$", line) - if m: - tdeps = [x.strip() for x in m.group(1).split(",") if x.strip()] - if tid: - ids[tid] = p - deps[tid] = tdeps - return ids, deps + tid = str(fm["id"]) + if tid in ids: + dups.setdefault(tid, [ids[tid]]).append(p) + continue + tdeps = fm.get("depends_on") + ids[tid] = p + deps[tid] = [str(d) for d in tdeps] if isinstance(tdeps, list) else [] + return ids, deps, dups def cycle_via(graph, start): @@ -254,7 +261,11 @@ def main(): root = Path(args.root) if not root.is_dir(): fail(f"no such directory: {args.root}") - tree_ids, tree_deps = collect_tree(root) + tree_ids, tree_deps, dups = collect_tree(root) + if args.id in dups: + files = ", ".join(str(p) for p in dups[args.id]) + fail(f"id '{args.id}' is duplicated across: {files} — fix the tree " + "before updating (run ticket_tree.py validate)") if args.id not in tree_ids: known = ", ".join(sorted(tree_ids)) or "none" fail(f"id '{args.id}' not found under {args.root} — known ids: {known}") @@ -263,14 +274,17 @@ def main(): fail("target the ticket with --path FILE, or --root DIR --id KEY-n") if args.root and tree_ids is None: - tree_ids, tree_deps = collect_tree(Path(args.root)) + tree_ids, tree_deps, _ = collect_tree(Path(args.root)) transitions = set(t.strip() for t in args.transitions.split(",") if t.strip()) known_states = {"backlog"} | {s for t in transitions for s in t.split(">")} text = path.read_text(encoding="utf-8") lines = text.splitlines(keepends=True) - entries, close = parse_frontmatter(lines) + try: + entries, close = parse_frontmatter(lines) + except ValueError as e: + fail(str(e)) fm = {e["key"]: e["value"] for e in entries} ticket_type = fm.get("type") if ticket_type not in ("epic", "story", "bug", "task", "spike"): @@ -298,9 +312,10 @@ def main(): new_status = updates["status"] current = fm.get("status") if ticket_type == "epic": - if new_status != "dropped": - fail("epics store no lifecycle — 'dropped' is the only storable epic status; " - "progress is computed from children") + if new_status not in ("done", "dropped"): + fail("epics compute progress from children — the storable epic statuses " + "are 'done' (an intentional call: retrospective or the user) and " + "'dropped'; neither is ever calculated") elif new_status != current: if new_status not in known_states: fail(f"unknown status '{new_status}' — known: {', '.join(sorted(known_states))}") @@ -355,7 +370,12 @@ def main(): if changes: new_lines = list(lines) - for start, end, new_line in sorted(replacements, key=lambda r: r[0], reverse=True): + # Apply bottom-up; at an equal start index the replacement (end > start) + # must land before the insertion (end == start), or the insertion is + # silently spliced away by the replacement of the line it sits on. + for start, end, new_line in sorted(replacements, + key=lambda r: (r[0], r[1] > r[0]), + reverse=True): new_lines[start:end] = [new_line] fd, tmp = tempfile.mkstemp(dir=str(path.parent), prefix=".update-", suffix=".tmp") try: