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
This commit is contained in:
Brian Madison 2026-06-21 21:49:06 -05:00
parent ebee3d2afc
commit 9faec69107
7 changed files with 61 additions and 17 deletions

View File

@ -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-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-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-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 ## Example

View File

@ -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 **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] :::note[Quantity Target]
The magic happens in ideas 50100. The workflow encourages generating 100+ ideas before organization. The magic happens in ideas 50100. The workflow encourages generating 100+ ideas before organization.

View File

@ -35,10 +35,10 @@ it**](../explanation/analysis-phase.md).
| Workflow | Purpose | Produces | | Workflow | Purpose | Produces |
|---------------------------------------------------------------------------|----------------------------------------------------------------------------|---------------------------| |---------------------------------------------------------------------------|----------------------------------------------------------------------------|---------------------------|
| `bmad-brainstorming` | Brainstorm Project Ideas with guided facilitation of a brainstorming coach | `brainstorming-report.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 | `forged-idea.md` and a bespoke HTML | | `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-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` | | `bmad-prfaq` | Working Backwards — stress-test your product concept customer-first | `prfaq-{project}.md` |
## Phase 2: Planning ## Phase 2: Planning
@ -68,7 +68,7 @@ Decide how to build it and break work into stories.
| Workflow | Purpose | Produces | | 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-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 | | `bmad-check-implementation-readiness` | Gate check before implementation | PASS/CONCERNS/FAIL decision |

View File

@ -71,7 +71,7 @@ BMad helps you build software through guided workflows with specialized AI agent
| Phase | Name | What Happens | | Phase | Name | What Happens |
| ----- | -------------- | ------------------------------------------------------------ | | ----- | -------------- | ------------------------------------------------------------ |
| 1 | Analysis | Brainstorming, research, forge idea, product brief or PRFAQ _(optional)_ | | 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 | | 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 | | 4 | Implementation | Build epic by epic, story by story with quick dev or automated epic delivery |

View File

@ -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). 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. 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. 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. 6. Run each `{workflow.activation_steps_append}` entry.
## Open the session ## 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: 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. - **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). - **Clearer** — the user simply thinks straighter now. The memlog stands on its own; no `forged-idea.md` needed (the report below still renders).

View File

@ -109,10 +109,11 @@ def load_party_workflow(project_root: Path, party_skill: Path):
"""Merged [workflow] table for bmad-party-mode (base + user overrides).""" """Merged [workflow] table for bmad-party-mode (base + user overrides)."""
resolver = project_root / "_bmad" / "scripts" / "resolve_customization.py" resolver = project_root / "_bmad" / "scripts" / "resolve_customization.py"
data = _run_json([sys.executable, str(resolver), "--skill", str(party_skill), "--key", "workflow"]) 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"] return data["workflow"]
# Fallback: base customize.toml directly, no override merge. # 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): def load_party_overrides(project_root: Path):
@ -124,6 +125,8 @@ def load_party_overrides(project_root: Path):
custom = project_root / "_bmad" / "custom" custom = project_root / "_bmad" / "custom"
team = _load_toml(custom / f"{PARTY_SKILL}.toml").get("workflow", {}) team = _load_toml(custom / f"{PARTY_SKILL}.toml").get("workflow", {})
user = _load_toml(custom / f"{PARTY_SKILL}.user.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) merged = dict(team)
for key, val in user.items(): for key, val in user.items():
if isinstance(val, list) and isinstance(merged.get(key), list): 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] = code
index[code.lower()] = code index[code.lower()] = code
index[_alias(code).lower()] = code index[_alias(code).lower()] = code
if entry.get("name"): name = entry.get("name")
index[entry["name"].lower()] = code 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(): for code, info in (agents or {}).items():
register(code, { register(code, {
@ -171,7 +178,9 @@ def build_pool(agents: dict, party_members: list):
}) })
installed_codes.append(code) 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") code = m.get("code")
if not code: if not code:
continue continue
@ -192,7 +201,7 @@ def build_pool(agents: dict, party_members: list):
def _brief(entry): def _brief(entry):
"""The slim card the orchestrator needs to cast a persona.""" """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)} 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): if entry.get(k):
out[k] = entry[k] out[k] = entry[k]
return out return out
@ -204,8 +213,12 @@ def resolve_parties(groups, pool, index):
if not isinstance(g, dict) or not g.get("id"): if not isinstance(g, dict) or not g.get("id"):
continue continue
raw = g.get("members", []) or [] raw = g.get("members", []) or []
members = [_brief(pool[index[t]]) for t in raw members = []
if (index.get(t) or index.get(str(t).lower())) in pool] 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} party = {"id": g["id"], "name": g.get("name", g["id"]), "members": members}
if g.get("scene"): if g.get("scene"):
party["scene"] = g["scene"] party["scene"] = g["scene"]

View File

@ -55,6 +55,24 @@ class TestBuildPool(unittest.TestCase):
self.assertEqual(custom, []) self.assertEqual(custom, [])
self.assertEqual(set(pool), {"bmad-agent-analyst", "bmad-agent-pm"}) 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): class TestResolveParties(unittest.TestCase):
def setUp(self): def setUp(self):
@ -74,6 +92,19 @@ class TestResolveParties(unittest.TestCase):
[{"id": "g", "members": ["analyst", "ghost"]}], self.pool, self.idx) [{"id": "g", "members": ["analyst", "ghost"]}], self.pool, self.idx)
self.assertEqual([m["name"] for m in parties[0]["members"]], ["Mary"]) 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): def test_open_cast_group_flagged(self):
parties = rp.resolve_parties( parties = rp.resolve_parties(
[{"id": "rebels", "name": "Rebels", "scene": "the Ghost"}], self.pool, self.idx) [{"id": "rebels", "name": "Rebels", "scene": "the Ghost"}], self.pool, self.idx)