Team Review — Add a plan-review Agent Team skill to plan-agent

The plan is architecturally sound — it cleanly adapts the proven product-plans Agent Team topology for technical implementation plans, with thoughtful conditional UX/a11y reviewer scaling. The team verdict is sound with revisions: the core design is correct and the implementation was already shipped as v1.9.0 (commit 1352236), making this a retrospective review.

Three high-priority follow-up items stand out: (1) the hero smoke test references fixture files that were never created; (2) the shipped agent files use allowed-tools: where the agent runtime reads tools: — a functional correctness gap; (3) the plan mutates the source file in place with no backup or atomicity guarantee. The 5-vs-7 reviewer count drift (Files note, Diagram, Step 9) has been corrected inline. A new AC12 for team cleanup completeness was added.

Role-by-Role Findings

Architecture

Fit: Architecturally sound — clean extension of the product-plans topology with HTML-aware integration layered on cleanly.

Synthesis data flow (prose → table → HTML selectors) has no schema between hops — the weakest seam high
HTML mutation via CSS-class string replace; skeleton drift silently breaks integration high
Reviewer dual-role contract (team teammate vs standalone subagent) under-specified medium
UI-signal heuristic duplicated, not shared with implementation-plan medium
No minimum-quorum gate — partial reviewer failure masquerades as full review medium
Re-running appends another Team Review block — no idempotency guard low

Completeness

Completeness: Highly specific and well-sequenced; path from scaffold to validate is unbroken, but fixture gap and count drift weaken it.

Fixture files tests/fixtures/sample-plan-backend.html / sample-plan-ui.html referenced but no step creates them high
5-vs-7 reviewer count drift in Files note, Diagram, Step 9 (fixed inline) medium
Two Unresolved Questions gate concrete step decisions (activation mode, large-plan handling) medium
SendUserFile in allowed-tools but its role in the workflow is unanchored low
No AC or verify asserting no orphaned teammates after a run (AC12 now added) low

Testability

Test coverage: Hero smoke test is strong conceptually but cannot run — fixtures are missing; per-step verify lines check file structure, not behavior.

Hero test fixtures never created; primary verification is currently unrunnable critical
Hero test gated on interactive Agent Teams run, no CI fallback high
AC6/AC11 (reviewer counts) have no machine-assertable signal — human observation only high
UI-signal detection heuristic has no boundary-case tests (false positives/negatives) high
HTML-escaping and <style>/<script> non-mutation have no automated assertion medium

Risk

Risk level: Medium overall.

Plan already shipped as v1.9.0 — re-running would create version collision high
Self-mutating file: N sequential Edits with no backup, no transaction, no rollback on partial failure high
LLM-synthesised HTML edits risk injection or wrong-selector corruption medium
Bash grant on reviewer agents is a latent write vector medium
Missing test fixtures leave primary verification path unrunnable medium

Conventions

Fit: Well-aligned overall; one functional correctness issue in the shipped files.

Shipped agent files use allowed-tools: — correct key is tools: (every other agent in repo uses tools:) high
Agent descriptions omit the "teammate-only" clause present in product-plans sibling agents medium
AC ID sequence: ac11 inserted mid-list, breaking sequential order low
Step 9 "five agents" typo — should be seven (fixed inline) low

User fit: Happy path is clear; multi-agent run is opaque; in-place mutation lacks consent moment.

No live progress feedback while 5–7 teammates run — operation appears hung high
In-place mutation with no diff preview or undo path before editing the source plan high
Disabled-state error message content not specified — first-run experience is undefined high
Empty/no-findings outcome undefined — may appear as silent failure medium
Partial-failure recovery path undefined — a mid-run crash could leave a half-edited plan medium

Accessibility

A11y compliance: Plan document has measurable contrast failures; feature under-specifies the a11y reviewer's rubric.

--subtle (#9ca3af) at 2.54:1 fails WCAG AA for normal text high
State colors #16a34a (3.3:1) and #d97706 (3.19:1) fail AA when used as small text labels high
plan-reviewer-accessibility agent lacks a concrete WCAG rubric — will produce shallow findings high
Copy buttons lack aria-live confirmation (reuse existing #criteria-status pattern) medium
Copy buttons and checkboxes below 44×44px touch target minimum medium
Pulse-dot animation not gated by prefers-reduced-motion low

Cross-Reviewer Agreements

Missing test fixtures — flagged independently by Completeness, Testability, and Risk. Highest-priority follow-up: create tests/fixtures/sample-plan-backend.html and sample-plan-ui.html before any verification run.

Activation mode: command-only recommended — UX and Risk both independently recommend disable-model-invocation given cost and file mutation. Resolve in v1.10.0.

Highest-Risk Issues for v1.10.0

1. Agent frontmatter key — Change allowed-tools: to tools: in all 7 plan-reviewer-*.md agent files. Functional correctness: the agent runtime reads tools: — the restriction may be silently ignored as-is. high

2. Create test fixtures — Create tests/fixtures/sample-plan-backend.html (no UI signals) and sample-plan-ui.html (body references .tsx, not inside <style>/<script>). Hero test is unrunnable without them. critical

3. Backup before mutation — Copy plan to <stem>.bak (or assert clean git state) before the first Edit call. Treat any failed Edit as abort-and-report rather than continue. high

4. Raise CSS contrast tokens — Darken --subtle to ≥4.5:1; darken --green and --amber text variants for small label use. high

Target	Action	Change
`.file-note` (role-prompts.md)	edit	"5 lens-specific spawn prompts" → "7 spawn prompts (5 core + 2 UI-conditional)"
Diagram collect `.pipeline-sub`	edit	"lead waits for all 5" → "lead waits for all spawned teammates (5 core or 7 with UI)"
Step 9 `.step-why`	edit	"five agents" → "all seven agents (five core + two UI-conditional)"
Tests hero fixture note	edit	Added explicit prerequisite: fixtures must be created (copy from `docs/plans/`) before running the test
`#criteria-list`	append	Added AC12: no orphaned teammates after a completed run
Before `<footer>`	insert	Appended this Team Review `<details>` block

Team Review — Add a plan-review Agent Team skill

Executive Summary

Role-by-Role Findings

Cross-Reviewer Agreements

Highest-Risk Issues for v1.10.0

Inline Edits Applied to Source Plan