cli._skill_trees gates the review skill tree even when review.enabled = false and nothing can dispatch a review session.
What happens today
_skill_trees (src/bmad_loop/cli.py) is scoped to install.DEV_PRIMITIVE_ROLES = ("dev", "review") and never consults pol.review.enabled. _require_base_skills then aborts on any Finding with no severity filter, from cmd_run, cmd_sweep and cmd_resume/cmd_resolve. So a project that has deliberately turned review off — and whose review adapter therefore points at a CLI whose skill tree was never populated — still fails preflight on skills no session in that run would read.
Why the obvious narrowing is wrong
Raised by an automated review on PR #406 (review.enabled = false ⇒ drop the review tree) and declined there, because review.enabled = false does not retire the review adapter:
_skip_review_and_commit ends at Engine._commit, which opens with an unconditional _run_workflows("pre_commit_gate", …)._run_workflows dispatches role=wf.role, and WORKFLOW_ROLES = {"dev", "review"} (plugins/model.py), so a role = "review" workflow runs on self.adapters["review"] with review disabled.- The in-wheel TEA plugin ships three such workflows at
pre_commit_gate (data/plugins/tea/plugin.toml: trace, nfr, review).
The suggested variant — narrowing Engine._worktree_profiles to match — is worse than the finding it fixes: that list drives per-CLI Stop-signal hook registration in install.provision_worktree as well as skill seeding, plus the Unity MCP agent ids. A worktree provisioned without the review profile leaves those sessions with no completion signal, i.e. a stall rather than a missing skill.
The residue that is real
The preflight could be narrowed when both hold:
review.enabled = false, and- no active plugin binds a
role = "review" workflow at any stage.
Cost
Condition (2) is why this is not a one-liner: the CLI preflight would have to build the plugin registry — trust gates and [python] module exec — before the engine exists. That is a real coupling for a preflight whose whole job is to be cheap and side-effect-free, and it is the coupling install.DEV_PRIMITIVE_ROLES' own comment declines to take on. Any fix should also keep _skill_trees and Engine._worktree_profiles in agreement, since the docstring's contract is that what is gated and what is seeded stay one decision.
Pinned meanwhile
tests/test_cli.py::test_skill_trees_covers_review_even_when_review_is_disabled pins the current behavior, so the collapse cannot be reintroduced silently while this stays open. _skill_trees' docstring carries the reason.
cli._skill_treesgates the review skill tree even whenreview.enabled = falseand nothing can dispatch a review session.What happens today
_skill_trees(src/bmad_loop/cli.py) is scoped toinstall.DEV_PRIMITIVE_ROLES = ("dev", "review")and never consultspol.review.enabled._require_base_skillsthen aborts on anyFindingwith no severity filter, fromcmd_run,cmd_sweepandcmd_resume/cmd_resolve. So a project that has deliberately turned review off — and whose review adapter therefore points at a CLI whose skill tree was never populated — still fails preflight on skills no session in that run would read.Why the obvious narrowing is wrong
Raised by an automated review on PR #406 (
review.enabled = false⇒ drop the review tree) and declined there, becausereview.enabled = falsedoes not retire the review adapter:_skip_review_and_commitends atEngine._commit, which opens with an unconditional_run_workflows("pre_commit_gate", …)._run_workflowsdispatchesrole=wf.role, andWORKFLOW_ROLES = {"dev", "review"}(plugins/model.py), so arole = "review"workflow runs onself.adapters["review"]with review disabled.pre_commit_gate(data/plugins/tea/plugin.toml:trace,nfr,review).The suggested variant — narrowing
Engine._worktree_profilesto match — is worse than the finding it fixes: that list drives per-CLI Stop-signal hook registration ininstall.provision_worktreeas well as skill seeding, plus the Unity MCP agent ids. A worktree provisioned without the review profile leaves those sessions with no completion signal, i.e. a stall rather than a missing skill.The residue that is real
The preflight could be narrowed when both hold:
review.enabled = false, androle = "review"workflow at any stage.Cost
Condition (2) is why this is not a one-liner: the CLI preflight would have to build the plugin registry — trust gates and
[python]module exec — before the engine exists. That is a real coupling for a preflight whose whole job is to be cheap and side-effect-free, and it is the couplinginstall.DEV_PRIMITIVE_ROLES' own comment declines to take on. Any fix should also keep_skill_treesandEngine._worktree_profilesin agreement, since the docstring's contract is that what is gated and what is seeded stay one decision.Pinned meanwhile
tests/test_cli.py::test_skill_trees_covers_review_even_when_review_is_disabledpins the current behavior, so the collapse cannot be reintroduced silently while this stays open._skill_trees' docstring carries the reason.