From 9faec69107d2aeb0c16730d4a892595ab856bf92 Mon Sep 17 00:00:00 2001 From: Brian Madison Date: Sun, 21 Jun 2026 21:49:06 -0500 Subject: [PATCH] Fix review findings: resolve_personas crash + doc accuracy - resolve_parties: resolve token once + coerce non-strings (fixes KeyError on mixed-case members and TypeError on unhashable members) - Guard malformed party-mode config shapes so discovery stays best-effort - _brief: pass through model/capabilities; register: no name-lookup hijack - 5 new regression tests (case-insensitivity, malformed input, rename collision) - Docs: correct bmad-review-adversarial-general name, bmad-prfaq handoff, recursive resume glob, broken table row, and Produces filenames across forge-idea/brainstorming/product-brief/architecture rows --- docs/explanation/forge-idea.md | 2 +- docs/reference/core-tools.md | 2 +- docs/reference/workflow-map.md | 8 ++--- docs/tutorials/getting-started.md | 2 +- src/core-skills/bmad-forge-idea/SKILL.md | 4 +-- .../scripts/resolve_personas.py | 29 ++++++++++++----- .../scripts/tests/test_resolve_personas.py | 31 +++++++++++++++++++ 7 files changed, 61 insertions(+), 17 deletions(-) diff --git a/docs/explanation/forge-idea.md b/docs/explanation/forge-idea.md index 53f77e323..cb621650e 100644 --- a/docs/explanation/forge-idea.md +++ b/docs/explanation/forge-idea.md @@ -57,7 +57,7 @@ Reach for the forge when you already hold an idea and want it hardened or killed | `bmad-prfaq` | You've committed to a product and want it proven customer-first | A Working Backwards coach | | `bmad-brainstorming` | You have no idea yet and need to generate options | A facilitation coach | | `bmad-party-mode` | You want your agents to discuss or decide together | Your whole roster in one conversation | -| `bmad-adversarial-review` | You have an artifact and need its flaws found | A reviewer who must find issues | +| `bmad-review-adversarial-general` | You have an artifact and need its flaws found | A reviewer who must find issues | ## Example diff --git a/docs/reference/core-tools.md b/docs/reference/core-tools.md index 767def4d6..f8e61c171 100644 --- a/docs/reference/core-tools.md +++ b/docs/reference/core-tools.md @@ -71,7 +71,7 @@ Run any core tool by typing its skill name (e.g., `bmad-help`) in your IDE. No a **Input:** Brainstorming topic or problem statement, optional context file -**Output:** `brainstorming-session-{date}.md` with all generated ideas and an HTML bespoke report of your session +**Output:** a self-contained `brainstorm.html` keepsake of the session, an optional `brainstorm-intent.md` for downstream skills, and a `.memlog.md` session record :::note[Quantity Target] The magic happens in ideas 50–100. The workflow encourages generating 100+ ideas before organization. diff --git a/docs/reference/workflow-map.md b/docs/reference/workflow-map.md index 6dccb0b78..50784e771 100644 --- a/docs/reference/workflow-map.md +++ b/docs/reference/workflow-map.md @@ -35,10 +35,10 @@ it**](../explanation/analysis-phase.md). | Workflow | Purpose | Produces | |---------------------------------------------------------------------------|----------------------------------------------------------------------------|---------------------------| -| `bmad-brainstorming` | Brainstorm Project Ideas with guided facilitation of a brainstorming coach | `brainstorming-report.md` and a bespoke HTML | -| `bmad-forge-idea` | Pressure-test an idea until it hardens, proves out, or dies cheaply | `forged-idea.md` and a bespoke HTML | +| `bmad-brainstorming` | Brainstorm Project Ideas with guided facilitation of a brainstorming coach | `brainstorm.html` keepsake plus an optional `brainstorm-intent.md` | +| `bmad-forge-idea` | Pressure-test an idea until it hardens, proves out, or dies cheaply | `forge-report.html` every run; `forged-idea.md` when an idea hardens | | `bmad-domain-research`, `bmad-market-research`, `bmad-technical-research` | Validate market, technical, or domain assumptions | Research findings | -| `bmad-product-brief` | Capture strategic vision — best when your concept is clear | `product-brief.md` and a bespoke HTML along with any desired presentation output | +| `bmad-product-brief` | Capture strategic vision — best when your concept is clear | `brief.md` + `addendum.md`, plus any desired HTML or presentation output | | `bmad-prfaq` | Working Backwards — stress-test your product concept customer-first | `prfaq-{project}.md` | ## Phase 2: Planning @@ -68,7 +68,7 @@ Decide how to build it and break work into stories. | Workflow | Purpose | Produces | |---------------------------------------|--------------------------------------------|-----------------------------| -| `bmad-architecture` | Make technical decisions explicit | `architecture.md` is a spine by default but can hydrate to your desired output or presentation needs also | +| `bmad-architecture` | Make technical decisions explicit | `ARCHITECTURE-SPINE.md` is the spine by default but can hydrate to your desired output or presentation needs also | | `bmad-create-epics-and-stories` | Break requirements into implementable work | Epic files with stories | | `bmad-check-implementation-readiness` | Gate check before implementation | PASS/CONCERNS/FAIL decision | diff --git a/docs/tutorials/getting-started.md b/docs/tutorials/getting-started.md index 71b57c22f..02f470d49 100644 --- a/docs/tutorials/getting-started.md +++ b/docs/tutorials/getting-started.md @@ -71,7 +71,7 @@ BMad helps you build software through guided workflows with specialized AI agent | Phase | Name | What Happens | | ----- | -------------- | ------------------------------------------------------------ | | 1 | Analysis | Brainstorming, research, forge idea, product brief or PRFAQ _(optional)_ | -| 2 | Planning | Create requirements and design PRD, UX, SPEC +| 2 | Planning | Create requirements and design PRD, UX, SPEC | | 3 | Solutioning | Design architecture spine or detailed project or system architectures | | 4 | Implementation | Build epic by epic, story by story with quick dev or automated epic delivery | diff --git a/src/core-skills/bmad-forge-idea/SKILL.md b/src/core-skills/bmad-forge-idea/SKILL.md index 26702d2ae..e4d30940e 100644 --- a/src/core-skills/bmad-forge-idea/SKILL.md +++ b/src/core-skills/bmad-forge-idea/SKILL.md @@ -26,7 +26,7 @@ Act as an exacting interrogator who would rather find the crack than spare the f 2. Run each `{workflow.activation_steps_prepend}` entry; treat each `{workflow.persistent_facts}` entry as foundational context (`file:` entries load their contents, `skill:` names a skill to consult, others are facts verbatim). 3. Load `{project-root}/_bmad/core/config.yaml` (and `config.user.yaml` if present); resolve `{user_name}`, `{communication_language}`, `{output_folder}`. Missing → neutral defaults; never block. Greet `{user_name}` in `{communication_language}` and stay in it. 4. Note whether a BMad persona is already active in this conversation — the user loaded one (e.g. the analyst, the storyteller) and invoked the forge from within it. If so, that persona leads the session, in voice, throughout. -5. Resume: glob `{workflow.forge_output_path}/*/.memlog.md` and read only each match's frontmatter to find any whose `status` is not `complete`. Offer to resume one — then read its full memlog once to rebuild state and continue append-only — or to start fresh. +5. Resume: glob `{workflow.forge_output_path}/**/.memlog.md` (recursive, so it still finds sessions when `run_folder_pattern` is overridden to nest paths) and read only each match's frontmatter to find any whose `status` is not `complete`. Offer to resume one — then read its full memlog once to rebuild state and continue append-only — or to start fresh. 6. Run each `{workflow.activation_steps_append}` entry. ## Open the session @@ -69,7 +69,7 @@ They hammer the branch in character; you synthesize their hits into your next qu The session ends however the thinking lands, and every landing is a real outcome: -- **Hardened** — the idea survived. Distill the memlog into `{workspace}/forged-idea.md`: super succinct — the locked items and what was killed and why, in the user's meaning. Not a prose retelling, not a template, not the conversation replayed — the load-bearing residue, nothing else. If it reads like a document, it's too long. Note it can feed `bmad-spec`, `bmad-prd`, or `bmad-help`. +- **Hardened** — the idea survived. Distill the memlog into `{workspace}/forged-idea.md`: super succinct — the locked items and what was killed and why, in the user's meaning. Not a prose retelling, not a template, not the conversation replayed — the load-bearing residue, nothing else. If it reads like a document, it's too long. Note it can feed `bmad-spec`, `bmad-prd`, or `bmad-prfaq`. - **Killed** — the idea did not survive. Say so plainly and record why. Finding this cheaply is a win, not a failure. - **Clearer** — the user simply thinks straighter now. The memlog stands on its own; no `forged-idea.md` needed (the report below still renders). diff --git a/src/core-skills/bmad-forge-idea/scripts/resolve_personas.py b/src/core-skills/bmad-forge-idea/scripts/resolve_personas.py index d9352814f..f1d5d5cd6 100644 --- a/src/core-skills/bmad-forge-idea/scripts/resolve_personas.py +++ b/src/core-skills/bmad-forge-idea/scripts/resolve_personas.py @@ -109,10 +109,11 @@ def load_party_workflow(project_root: Path, party_skill: Path): """Merged [workflow] table for bmad-party-mode (base + user overrides).""" resolver = project_root / "_bmad" / "scripts" / "resolve_customization.py" data = _run_json([sys.executable, str(resolver), "--skill", str(party_skill), "--key", "workflow"]) - if data is not None and "workflow" in data: + if data is not None and isinstance(data.get("workflow"), dict): return data["workflow"] # Fallback: base customize.toml directly, no override merge. - return _load_toml(party_skill / "customize.toml").get("workflow", {}) + wf = _load_toml(party_skill / "customize.toml").get("workflow", {}) + return wf if isinstance(wf, dict) else {} def load_party_overrides(project_root: Path): @@ -124,6 +125,8 @@ def load_party_overrides(project_root: Path): custom = project_root / "_bmad" / "custom" team = _load_toml(custom / f"{PARTY_SKILL}.toml").get("workflow", {}) user = _load_toml(custom / f"{PARTY_SKILL}.user.toml").get("workflow", {}) + team = team if isinstance(team, dict) else {} + user = user if isinstance(user, dict) else {} merged = dict(team) for key, val in user.items(): if isinstance(val, list) and isinstance(merged.get(key), list): @@ -157,8 +160,12 @@ def build_pool(agents: dict, party_members: list): index[code] = code index[code.lower()] = code index[_alias(code).lower()] = code - if entry.get("name"): - index[entry["name"].lower()] = code + name = entry.get("name") + if name: + key = name.lower() + # A custom rename must not hijack another agent's name lookup. + if index.get(key, code) == code: + index[key] = code for code, info in (agents or {}).items(): register(code, { @@ -171,7 +178,9 @@ def build_pool(agents: dict, party_members: list): }) installed_codes.append(code) - for m in party_members or []: + for m in (party_members if isinstance(party_members, list) else []): + if not isinstance(m, dict): + continue code = m.get("code") if not code: continue @@ -192,7 +201,7 @@ def build_pool(agents: dict, party_members: list): def _brief(entry): """The slim card the orchestrator needs to cast a persona.""" out = {k: entry[k] for k in ("code", "name", "icon", "title", "source") if entry.get(k)} - for k in ("description", "persona"): + for k in ("description", "persona", "capabilities", "model"): if entry.get(k): out[k] = entry[k] return out @@ -204,8 +213,12 @@ def resolve_parties(groups, pool, index): if not isinstance(g, dict) or not g.get("id"): continue raw = g.get("members", []) or [] - members = [_brief(pool[index[t]]) for t in raw - if (index.get(t) or index.get(str(t).lower())) in pool] + members = [] + for t in raw: + key = t if isinstance(t, str) else str(t) + code = index.get(key) or index.get(key.lower()) + if code in pool: + members.append(_brief(pool[code])) party = {"id": g["id"], "name": g.get("name", g["id"]), "members": members} if g.get("scene"): party["scene"] = g["scene"] diff --git a/src/core-skills/bmad-forge-idea/scripts/tests/test_resolve_personas.py b/src/core-skills/bmad-forge-idea/scripts/tests/test_resolve_personas.py index 4fbf65175..4867b0dca 100644 --- a/src/core-skills/bmad-forge-idea/scripts/tests/test_resolve_personas.py +++ b/src/core-skills/bmad-forge-idea/scripts/tests/test_resolve_personas.py @@ -55,6 +55,24 @@ class TestBuildPool(unittest.TestCase): self.assertEqual(custom, []) self.assertEqual(set(pool), {"bmad-agent-analyst", "bmad-agent-pm"}) + def test_custom_rename_does_not_hijack_another_agents_name(self): + # Override the analyst slot, renaming it to "John" — the PM's name. + # The PM's name lookup must survive (last-writer-wins would corrupt it). + _, idx, _, _ = rp.build_pool(AGENTS, [{"code": "analyst", "name": "John"}]) + self.assertEqual(idx["john"], "bmad-agent-pm") + + def test_brief_carries_model_and_capabilities(self): + pool, _, _, _ = rp.build_pool( + AGENTS, [{"code": "neo", "name": "Neo", "model": "opus", "capabilities": ["x"]}]) + brief = rp._brief(pool["neo"]) + self.assertEqual(brief["model"], "opus") + self.assertEqual(brief["capabilities"], ["x"]) + + def test_non_list_party_members_is_safe(self): + pool, _, installed, custom = rp.build_pool(AGENTS, "not-a-list") + self.assertEqual(custom, []) + self.assertEqual(set(pool), {"bmad-agent-analyst", "bmad-agent-pm"}) + class TestResolveParties(unittest.TestCase): def setUp(self): @@ -74,6 +92,19 @@ class TestResolveParties(unittest.TestCase): [{"id": "g", "members": ["analyst", "ghost"]}], self.pool, self.idx) self.assertEqual([m["name"] for m in parties[0]["members"]], ["Mary"]) + def test_member_resolution_is_case_insensitive(self): + # A TOML author naturally writes "Analyst"/"Shark"; the filter accepts + # them via the lowercase index, so resolution must too (no KeyError). + parties = rp.resolve_parties( + [{"id": "g", "members": ["Analyst", "Shark"]}], self.pool, self.idx) + self.assertEqual([m["name"] for m in parties[0]["members"]], ["Mary", "Marcus"]) + + def test_non_string_member_does_not_crash(self): + # Malformed members (int, list) must drop silently, never raise. + parties = rp.resolve_parties( + [{"id": "g", "members": [123, ["x"], "analyst"]}], self.pool, self.idx) + self.assertEqual([m["name"] for m in parties[0]["members"]], ["Mary"]) + def test_open_cast_group_flagged(self): parties = rp.resolve_parties( [{"id": "rebels", "name": "Rebels", "scene": "the Ghost"}], self.pool, self.idx)