# Workflow Review Changes > [Workflow] Use when activating the Review Current Changes workflow for review, fix, and re-review recursively until all issues resolved. - Skill: `duc01226/workflow-review-changes` (Agent Skill) - Install (CLI): `npx skillmds@latest add duc01226/workflow-review-changes` - Raw SKILL.md: https://api.skillmd.com/api/skills/duc01226/workflow-review-changes/raw - Safety review: pending - Works with: Claude Code, Claude.ai, OpenAI Codex - Category: Productivity - Author: duc01226 (https://skillmd.com/u/duc01226) - Updated: 2026-09-09 - Page: https://skillmd.com/skills/duc01226/workflow-review-changes --- > **[BLOCKING]** Execute skill steps in declared order. NEVER skip, reorder, or merge steps without explicit user approval. > **[BLOCKING]** Before each step or sub-skill call, update task tracking: set `in_progress` when step starts, set `completed` when step ends. > **[BLOCKING]** Every completed/skipped step MUST include brief evidence or explicit skip reason. > **[BLOCKING]** If Task tools are unavailable, create and maintain an equivalent step-by-step plan tracker with the same status transitions. ## Quick Summary **Goal:** Ensure changed work reaches clean review through an initial whole-target adversarial pass run in parallel with dimensional review, validated findings, verified fixes, full re-review, and synchronized docs/tests — review all uncommitted changes, fix only validated findings, then repeat the plan→plan-execute→changes-review loop until a complete pass is clean. **Summary:** - **Step 0 (FIRST ACTION, pre-sequence):** bind the self-recursive review loop — an always-on protocol loop you self-drive (the BINDING mechanism, hook/command-independent) PLUS, when available, a `/goal` Stop-hook gate as an optional accelerator — so the review→self-fix→whole-diff-re-review loop is unabandonable until it converges to a clean pass at that round's bar (zero findings in rounds 1-2; zero CRITICAL/HIGH/MEDIUM from round 3) — why: a soft "loop until clean" directive gets rationalized away after one fix cycle, and the protocol loop holds even where `/goal` is absent. Session-level wrapper, NOT one of the 20 canonical steps. ALWAYS runs — including as a step inside a parent workflow — because this workflow always runs INLINE in the main session (never a sub-agent), so it owns the loop directly in every case. - **Initial parallel phase (steps 1–2, all-return barrier):** launch step 2 `/why-review --target=whole-review-target` as a fresh read-only `code-reviewer` sub-agent, then immediately run step 1 `/changes-review` INLINE while it is active. Step 1 owns the dimensional baseline (surface analysis, integration-test/translation/spec-drift gaps, internal UI review); step 2 independently reviews the WHOLE review target + current changes in FULL mode. Neither consumes the other's output. Advance only after BOTH return, then consolidate both reports. - Step 3 `/why-review` validates the step-1 `/changes-review` findings to drop false positives BEFORE the specialist batch fires. The initial whole-target step 2 already validates its own findings through `/why-review`'s full-mode closing gate. - Steps 4–10 (`/architecture-review`, `/domain-entities-review` [if entity files], `/performance-review`, `/integration-test-review`, `/security-review`, `/production-readiness-review`, `/ui-review` [if frontend files]) are read-only sub-agents: spawn ALL in ONE message and advance ONLY after every member returns (all-return barrier); the mutating `/code-simplifier` (step 11) waits until the barrier clears and self-reviews its own changes via `/code-review`. (`/ui-review` runs here as a DEDICATED conditional batch member AND still runs internally inside step 1's `/changes-review` — both by design; see the UI-review note below.) - Fix cycle (steps 12–15 `/plan`→`/plan-review`→`/plan-execute`→`/changes-review`) runs ONLY when validated findings exist; the step-15 re-review runs ONLY if `/plan-execute` changed files, re-reading the full diff from scratch INLINE to counter orchestrator confirmation bias, and loops until the round's exit bar is clear — **zero findings in rounds 1-2, zero CRITICAL/HIGH/MEDIUM from round 3 (a LOW-only round ENDS the loop, deferred not fixed)** — bounded at **3 rounds MAX** (escalate via `AskUserQuestion` at whichever trips first: 2 no-progress repeats of the same blocker, or round 3 completing with CRITICAL/HIGH/MEDIUM still open — cap exhaustion escalates, never PASSes). - **Step 16 `/why-review` (ALWAYS runs, FULL mode, standalone)** — near-final HOLISTIC review of the settled WHOLE target + current changes as ONE artifact. It remains mandatory even though step 2 uses the same lens at startup: step 2 finds whole-package risks early; step 16 proves the final post-fix state. On findings → re-enter `/plan`→`/plan-execute`→`/changes-review`, then re-run step 16 until a full-mode pass finds zero new findings (bounded by `/why-review`'s own review loop: max 2 re-dos / 3-repeat-blocker → escalate). - `/docs-update` (step 18) ALWAYS runs and triages internally; SPEC-STALE drift verdicts from step 1 flow here to update the Feature Spec first — the workflow is NOT clean while any behavior-vs-spec divergence stays unadjudicated (green tests do not normalize drift). **Sequence:** _(Step 0 pre-sequence: bind self-recursive review loop — protocol loop always, `/goal` accelerator when available)_ → **[initial parallel phase]** /changes-review (INLINE; owns dimensional/UI baseline) + `/why-review --target=whole-review-target` (fresh read-only sub-agent; FULL mode over the whole target) → /why-review (validate step-1 findings) → **[specialist parallel batch]** /architecture-review + /domain-entities-review (if entity changes) + /performance-review + /integration-test-review + /security-review + /production-readiness-review + /ui-review (if frontend changes) → /code-simplifier → /plan → /plan-review → /plan-execute → **`/changes-review` (conditional inline re-review)** → **`/why-review` (final HOLISTIC full-mode review of the settled WHOLE target)** → /scan --target=domain-entities → /docs-update → /workflow-end → /watzup **Key Rules:** - MUST ATTENTION define success criteria before execution and loop until observable verification passes. - MUST ATTENTION when creating/reviewing specs or tests, name `Business Intent / Invariant Guarded` or the protected business intent/invariant and ensure the test would fail if that intent breaks. - MUST ATTENTION carry every unresolved finding or unaccepted risk into validation/fix planning; do not close until fixed or explicitly accepted. - MUST ATTENTION include unresolved risk register, generated mirror drift, and spec/test/docs drift in the fresh review prompt when relevant. - MUST ATTENTION launch the whole-target full-mode `/why-review` sub-agent before starting step 1 inline, then advance only after BOTH initial-phase members return; run step 3 `/why-review --validate-findings` on the step-1 findings before spawning the specialist reviewers. - After `/plan-execute` applies validated fixes (and ONLY if `/plan-execute` changed files) → re-run `/changes-review` INLINE over the current full diff from the first phase; re-read the diff from scratch to counter orchestrator confirmation bias - Main-agent re-review (with knowledge of its own fixes) is NOT sufficient — orchestrator-level confirmation bias - PASS = one complete review pass finds zero blocking issues after all validated fixes and verification are included - Repeated blockers are tracked in conversation context; stop after 2 no-progress full invocations of the same blocker --- ## First Principle — Easy to Change > **The success metric of every coding decision is _future change cost_.** > DRY, SRP, abstraction, design patterns, naming, layering, tests — every > technique exists to serve one goal: **making the next change cheaper**. When evaluating code, a refactor, a test, or an abstraction, ask: **does this make the next change cheaper or more expensive?** - Reject "best practices" that raise change cost (premature abstraction, speculative generality, leaky indirection, ceremony without payoff). - Name the real enemies in findings: **coupling, hidden state, duplicated knowledge, unclear intent, irreversible decisions exposed too early**. - A simpler design that is easy to change beats a sophisticated design that isn't. Apply this lens **before** invoking any specific rule, pattern, or checklist below — if a downstream rule would raise change cost, this principle wins. --- ## Step 0 — Bind the Self-Recursive Review Loop (FIRST ACTION — pre-sequence; protocol-first, `/goal` optional) > **MUST ATTENTION:** Before creating the 20 step tasks below, the VERY FIRST action is to BIND the self-recursive review loop so the session cannot end until the whole workflow loop converges to a clean zero-finding pass. Binding has TWO layers: (1) an **always-on protocol loop** you self-drive — the BINDING mechanism, hook/command-independent, in force on every host; and (2) an **optional `/goal` accelerator** — a mechanical Stop-hook block installed only WHEN the command is available. This is a session-level enforcement WRAPPER — NOT one of the 20 canonical `workflows.json` sequence steps, so it does NOT change the step count or the sequence; it makes the existing loop unabandonable. **Entry gate:** - **ALWAYS run** — whether this workflow is the top-level invocation (user ran `/start-workflow workflow-review-changes` or `/changes-review` routed here directly) OR a step inside a parent workflow (e.g. `workflow-feature`, `workflow-bugfix`, `workflow-refactor`). Because this workflow always runs INLINE in the main session (never as a sub-agent — see the WORKFLOW-IN-WORKFLOW note), it owns the session Stop hook directly in every case, so the loop binds and enforces identically. There is no "deferred to parent" case. **Procedure:** **1. Protocol loop — ALWAYS binding (hook/command-independent).** This is the mechanism that actually holds the loop shut; it binds Claude, Codex, and Copilot equally, whether or not `/goal` exists. You yourself MUST NOT stop until the condition below holds: > Run the initial parallel phase (`/changes-review` INLINE + `/why-review --target=whole-review-target` in a fresh sub-agent) to its all-return barrier → validate step-1 findings → run the specialist parallel reviewers + `/code-simplifier` → if validated findings exist, `/plan` → `/plan-execute` SELF-FIXES them → re-run `/changes-review` INLINE over the WHOLE current diff → loop until one complete pass clears the round's bar (rounds 1-2: zero findings; round 3+: zero CRITICAL/HIGH/MEDIUM, LOW-only ENDS the loop with the LOWs deferred) → run the final full-mode `/why-review` over the settled whole target → only then `/docs-update` → `/workflow-end`. Stop only when all required review passes are clean (or the same blocker repeats 3× with no progress → escalate via `AskUserQuestion`). **2. `/goal` command — invoke as an accelerator WHEN AVAILABLE.** If `/goal` is registered and permitted on this host, invoke it (the actual built-in command) with the same condition to add a mechanical Stop-hook block on top of the protocol loop: ``` /goal workflow-review-changes self-recursive loop: run the initial parallel phase (/changes-review INLINE + /why-review --target=whole-review-target in a fresh sub-agent) to its all-return barrier → validate step-1 findings → run the specialist parallel reviewers + /code-simplifier → if validated findings exist, /plan → /plan-execute SELF-FIXES them → re-run /changes-review INLINE over the WHOLE current diff → loop until one complete pass clears the round's bar (rounds 1-2: zero findings; round 3+: zero CRITICAL/HIGH/MEDIUM, a LOW-only round ENDS the loop with the LOWs recorded as deferred) → run the final full-mode /why-review over the settled whole target → only then /docs-update → /workflow-end. Stop only when all required review passes are clean (or the same blocker repeats 3× with no progress → escalate via AskUserQuestion). ``` The `/goal` Stop hook then blocks stopping until the condition holds and auto-clears when met — do not tell the user to clear it. **If `/goal` is unavailable, unregistered, or not permitted** — record ONE line and proceed under the protocol loop (step 1), which is already binding; NEVER error, block, or fake a gate on its absence: ``` /goal accelerator unavailable — review loop bound by protocol (Step 0 step 1) ``` 3. Then proceed to create the 20 step tasks below and run the sequence. > **Why bind the loop on top of the loop prose:** the conditional re-review (step 15) and the "loop until clean" rules are soft directives an orchestrator can rationalize away after one fix cycle. The protocol loop converts them into a self-enforced invariant on every host; the optional `/goal` Stop hook adds a mechanical block, but correctness never depends on it. ## Mandatory Task Creation (ZERO TOLERANCE) > **Step 0 first:** bind the Step 0 self-recursive review loop (above — protocol loop always, `/goal` accelerator when available) BEFORE creating these tasks — always, including when this workflow is a step inside a parent workflow, since it always runs inline in the main session and owns the loop directly. Create one task per row in the table below — source of truth is `workflows.json` → `workflow-review-changes.sequence` (currently 20 steps; verify count matches if you suspect drift). The Step 0 loop binding is a pre-sequence wrapper and is NOT counted among these 20: | # | Task Subject | Conditional? | | --- | -------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | 1 | `[Workflow] /changes-review — Inline dimensional baseline, UI dimension, integration/translation/spec-drift checks` ⚡ **INITIAL PARALLEL PHASE** | No — run INLINE while step 2's sub-agent is active | | 2 | `[Workflow] /why-review --target=whole-review-target — FULL-mode adversarial review of the WHOLE target + current changes` ⚡ **INITIAL PARALLEL PHASE** | No — launch first as a fresh read-only `code-reviewer` sub-agent; independent of step 1; barrier waits for both | | 3 | `[Workflow] /why-review — Validate the step-1 /changes-review findings before specialist reviewers run` | No — FINDINGS-VALIDATION gate over step-1 findings; step 2 self-validates its own findings | | 4 | `[Workflow] /architecture-review — Architecture compliance review` ⚡ **SPECIALIST PARALLEL BATCH** | No — run as sub-agent in parallel with steps 5/6/7/8/9/10 | | 5 | `[Workflow] /domain-entities-review — DDD quality review of changed domain entity files` ⚡ **SPECIALIST PARALLEL BATCH** | Yes — skip if no domain entity files in git diff | | 6 | `[Workflow] /performance-review — Performance analysis` ⚡ **SPECIALIST PARALLEL BATCH** | No — run as sub-agent in parallel with steps 4/5/7/8/9/10 | | 7 | `[Workflow] /integration-test-review — Test quality + change-coverage review` ⚡ **SPECIALIST PARALLEL BATCH** | No — run as sub-agent in parallel with steps 4/5/6/8/9/10 | | 8 | `[Workflow] /security-review — Security vulnerability review` ⚡ **SPECIALIST PARALLEL BATCH** | No — run as sub-agent in parallel with steps 4/5/6/7/9/10 | | 9 | `[Workflow] /production-readiness-review — Read-only SRE readiness review` ⚡ **SPECIALIST PARALLEL BATCH** | No — run as sub-agent in parallel with steps 4/5/6/7/8/10 | | 10 | `[Workflow] /ui-review — UI/frontend quality review` ⚡ **SPECIALIST PARALLEL BATCH** | Yes — skip if no frontend files; dedicated batch member and still runs internally inside step 1 | | 11 | `[Workflow] /code-simplifier — Simplify and refine code, then self-review its changes` | No — runs after the specialist barrier | | 12 | `[Workflow] /plan — Consolidate validated review findings into fix plan` | Conditional — only when validated findings exist | | 13 | `[Workflow] /plan-review — Review the fix plan and its rationale` | Conditional — only when a fix plan exists | | 14 | `[Workflow] /plan-execute — Implement validated fixes from plan` | Conditional — only when validated findings exist | | 15 | `[Workflow] /changes-review — Conditional inline re-review after /plan-execute; loop until clean` | Skip if all reviews pass or `/plan-execute` changed no files | | 16 | `[Workflow] /why-review — Final HOLISTIC full-mode review of the settled WHOLE target + changes` | Always run — preserves post-fix convergence; on findings re-enter steps 12–15, then re-run step 16 | | 17 | `[Workflow] /scan --target=domain-entities — Refresh the domain-entity reference catalog` | Conditional — run only when the final diff changes an entity/model, DTO/data contract, persistence schema/migration, or entity-sync evidence; otherwise complete with a cited skip reason | | 18 | `[Workflow] /docs-update — Update impacted documentation` | Always run — triages internally | | 19 | `[Workflow] /workflow-end — End workflow state` | No | | 20 | `[Workflow] /watzup — Post-workflow summary and final /understand handoff` | No | > **UI review runs in TWO places by design (keep both).** `/ui-review` runs BOTH (a) INTERNALLY inside step 1 (`/changes-review` invokes it as its UI dimension) AND (b) as a DEDICATED conditional specialist-batch member (step 10, `ui-ux-designer` sub-agent). Both are gated on the same trigger — frontend/UI files in the diff — so both are skipped when no frontend files changed. Create the step-10 `[Workflow] /ui-review` task (conditional) AND keep step 1's internal UI dimension; do NOT collapse them into one. NEVER consolidate, rename, or omit steps. If reviews PASS, mark conditional tasks `completed` with note "Skipped — all reviews passed". > **Integration Test Sync:** The `/changes-review` skill (task #1) includes a **mandatory** integration test coverage check for changed command/query/handler files. When gaps are found, the skill uses `AskUserQuestion` to surface them — NOT purely advisory. The user must explicitly choose to run `/integration-test` or confirm tests are already written. No silent skip. > **Translation Sync:** The `/changes-review` skill (task #1) includes a **mandatory** multilingual UI translation-sync check. When UI text changes in multilingual projects without locale updates, the skill uses `AskUserQuestion` for an explicit user decision — NOT purely advisory. > **Docs Update:** `/docs-update` (step 18) MUST run after EVERY review — it performs Phase 0 triage and fast-exits automatically when only non-business-code files changed (`.claude/**`, config). When business code is in the changeset, it WILL invoke: Phase 2 `/spec` (business feature doc update), Phase 2.5 `/spec-index [mode=index]` (derived bucket INDEX/ERD refresh — if `docs/specs/` bucket maintains a derived index; note: dirs may be app buckets or flat system folders — probe `ls docs/specs/{name}/` to find a specific service), Phase 2.6 `/tech-spec` when the derived technical view is affected, Phase 3 `/spec [mode=tests]` (test spec sync), Phase 4 `/spec [mode=sync]` (§8 TCs ↔ executing test code). Never skip based on review PASS status alone. > **Spec Drift Adjudication:** The `/changes-review` skill (task #1) runs a **mandatory** spec-drift adjudication (`SYNC:spec-drift-adjudication`, per `shared/sdd-artifact-contract.md` → Drift Gates) for every behavior-changing file: it classifies each divergence between changed behavior and the canonical Feature Spec as **CODE-WRONG** (BLOCKING — fix the code/test against intended behavior), **SPEC-STALE** (the change is the new intent — the spec documents the old behavior), or **AMBIGUOUS** (escalate). The reviewer never silently picks a side. A **SPEC-STALE** verdict flows downstream: `/docs-update` (step 18) updates the Feature Spec FIRST via `/spec [update]`, then re-syncs `/spec [mode=tests]`. The workflow is NOT clean while any behavior-vs-spec divergence remains unadjudicated — green tests do not normalize drift (green can encode the drift itself). > **Spec enrichment per cycle (MANDATORY — closes the feedback loop):** Every confirmed finding fixed in the loop (steps 12–15) that changed observable behavior MUST produce a new or updated §8 regression/preservation TC via `/spec [mode=tests]` before the workflow is clean — a code-only fix with no covering §8 TC is an INCOMPLETE cycle, not a clean pass. This applies to EVERY confirmed behavior-changing fix, not only SPEC-STALE drift verdicts or bugfix-workflow paths: a CODE-WRONG fix owes a regression TC describing the now-correct behavior; a behavior change owes a preservation/regression TC guarding the new behavior. So each recursive cycle ENRICHES the spec rather than only mutating code — the inline re-review (step 15) and the `/workflow-end` spec ↔ TDD-test sync gate both treat a behavior-changing fix that left no §8 TC as an open finding. --- ## Initial Parallel Phase (Steps 1–2) — EXECUTION PROTOCOL Steps 1 and 2 are independent, read-only review lanes over the same starting state: 1. Launch `/why-review --target=whole-review-target` as a **fresh `code-reviewer` sub-agent** in FULL mode. Its target is the whole review target combined with the current changes — the complete changeset plus surrounding code/spec/docs — and it writes its report incrementally under `plans/reports/`. 2. Immediately run `/changes-review` **INLINE in the main session** while that sub-agent is active. It owns surface detection, dimensional review, the internal UI dimension, and integration/translation/spec-drift gates. 3. Treat the pair as one declared all-return barrier. Neither lane consumes or waits on the other's partial output. Advance only after BOTH return; then mark both tasks complete and consolidate both reports. 4. Run step 3 `/why-review --validate-findings` against the step-1 findings. Do not revalidate step 2's findings here because full-mode `/why-review` already owns its closing findings-validation gate. This phase intentionally mixes one inline member with one sub-agent member: dispatch the sub-agent first, start the inline member immediately, and do not advance past the phase until both complete. The unique sequence token `why-review --target=whole-review-target` prevents the initial occurrence from being confused with the final plain `why-review` occurrence by workflow barrier renderers. ## Specialist Parallel Review Phase (Steps 4–10) — EXECUTION PROTOCOL > **Note:** Steps 4–10 are the specialist reviewers — architecture compliance, DDD entities, > performance, integration test quality, security vulnerabilities, production readiness, and > UI/frontend quality (`/ui-review`, conditional on frontend files). They run as workflow-level > parallel sub-agents, separate from the DIMENSIONAL review (BE/FE/SCSS/Synthesis + UI dimension) > that runs INSIDE Step 1 (`/changes-review`). > **`/ui-review` runs in TWO places by design (keep both):** (a) INTERNALLY inside Step 1 as > `/changes-review`'s UI dimension, AND (b) here as the DEDICATED step-10 batch member > (`ui-ux-designer` sub-agent). Both fire only when the diff has files matching the project's > configured frontend/UI file patterns; both are skipped otherwise. Steps 4–10 (`/architecture-review`, `/domain-entities-review`, `/performance-review`, `/integration-test-review`, `/security-review`, `/production-readiness-review`, `/ui-review`) are **read-only** and **independent** — no shared mutable state, no ordering dependency between them. Run them as parallel sub-agents to preserve main session context budget and reduce wall-clock time. ### Why parallel? Each reviewer reads the git diff independently and analyzes one concern. Sequential execution would burn 50K+ tokens in the main session absorbing all seven inline. The `stepMeta` in `workflows.json` marks all seven as `executionMode: subagent, contextBudget: high` — dispatch each as a sub-agent per the model-driven advancement rule (no hook emits a `💡 [SUB-AGENT RECOMMENDED]` hint). > **UI review runs in TWO places by design (keep both).** `/changes-review` (step 1) invokes `/ui-review` internally as its dimensional-batch UI dimension when frontend files changed, AND step 10 spawns `/ui-review` again as a DEDICATED conditional member of THIS specialist phase. Both fire only when frontend/UI files are in the diff; both are skipped otherwise. ### Execution: spawn in one message After the initial steps 1–2 barrier clears and step 3 validates the step-1 findings, spawn all active specialist reviewers in **a single response** with multiple `Agent` tool calls: ``` Agent(architecture-review, subagent_type="architect", ...) ← all in ONE message Agent(domain-entities-review, subagent_type="code-reviewer", ...) ← only if entity files in diff Agent(performance-review, subagent_type="performance-optimizer", ...) Agent(integration-test-review, subagent_type="integration-tester", ...) Agent(security-review, subagent_type="security-auditor", ...) Agent(production-readiness-review, subagent_type="code-reviewer", ...) ← read-only SRE findings/score mode Agent(ui-review, subagent_type="ui-ux-designer", ...) ← only if frontend/UI files in diff ``` Each sub-agent receives: - The baseline summary from step 1 (what changed, integration test gaps found) - Instruction to write report to `plans/reports/{skill}-{date}-{slug}.md` - Full review protocols per `SYNC:review-protocol-injection` (verbatim in prompt — never by file reference) ### State advancement after parallel batch (model-driven — PRIMARY) Advancement here is **model-driven** — your responsibility against the task list, NOT a hook/tool signal. This is the same rule the universal context files carry ("Workflow Step Advancement & Parallel Phases" in CLAUDE.md / AGENTS.md), so the batch advances identically under Claude and Codex. The shared kernel is the canonical **`SYNC:parallel-phase-advancement`** block consolidated at the end of this skill — its barrier rule governs this batch: declare the group up-front; spawn ALL members in ONE message; advance ONLY after EVERY member returns (a skipped conditional member counts as "returned"); a sub-agent return advances a step IDENTICALLY to an inline call; defer the mutating `/code-simplifier` step until the barrier clears; hooks are accelerators only. **Applied to this workflow's specialist batch** — after ALL parallel reviewers (steps 4–10) have returned: 1. `TaskUpdate` step 4 → `completed` 2. `TaskUpdate` step 5 → `completed` (or "Skipped — no entity files" if the conditional `domain-entities-review` member did not run — a skipped conditional counts as "returned") 3. `TaskUpdate` step 6 → `completed` 4. `TaskUpdate` step 7 → `completed` 5. `TaskUpdate` step 8 → `completed` 6. `TaskUpdate` step 9 (`/production-readiness-review`) → `completed` 7. `TaskUpdate` step 10 (`/ui-review`) → `completed` (or "Skipped — no frontend/UI files" if the conditional `ui-review` member did not run — a skipped conditional counts as "returned") 8. Read all sub-agent report files; synthesize findings into a combined review summary 9. Proceed to step 11 (`/code-simplifier`) sequentially — only after the barrier above (it is a code-mutating step and must see the complete review snapshot) > **Advancement here is model-driven.** This sub-agent batch advances only after every member returns (the all-return barrier) — no step-tracking hook advances it. Claude and Codex both rely entirely on this rule. ### Consolidation before /code-simplifier Before running `/code-simplifier`, synthesize all parallel sub-agent findings: - List all Critical/High/Medium/Low findings across all 7 reports (plus the UI-dimension findings folded into step 1's report when frontend files changed) - Note any conflicts between reviewers (same file, different concerns) - Pass this summary to `/code-simplifier` as context so simplification is informed by review findings **Surface Analysis from Step 1:** Step 1 (`/changes-review`) now emits a surface analysis summary in its report: ``` ## Change Surface Analysis BE files: {N} FE-Logic files: {M} SCSS files: {P} Review Mode: [DIMENSIONAL | BE-ONLY | FE-ONLY | FE-SPLIT | TOOLING] ``` Include this surface analysis in the consolidation summary passed to `/code-simplifier`. This lets the simplifier focus attention on the dominant surface without re-analyzing the diff. Dimensional agent reports (if mode = DIMENSIONAL): - `plans/reports/review-be-{date}.md` — BE findings - `plans/reports/review-fe-logic-{date}.md` — FE-Logic findings - `plans/reports/review-scss-{date}.md` — SCSS findings (if spawned) - `plans/reports/synthesis-review-{date}.md` — Cross-boundary findings All four (plus the UI-dimension `/ui-review` findings when frontend files changed) feed into the consolidation summary alongside the step-2 whole-target report and steps 4–10 specialist findings (including the dedicated step-10 `/ui-review` pass). ### What runs sequentially (never parallelize) | Step | Why sequential | | ------------------------------------------------ | -------------------------------------------------------------------------------------------------------------------------------------------- | | `why-review` (#3) | Validates the step-1 findings after the initial all-return barrier and before the specialist batch | | `code-simplifier` (#11) | Modifies code — specialist batch reviews pre-simplification state; self-reviews its own output via `/code-review` before returning | | `plan` → `plan-review` → `plan-execute` (#12–14) | Ordered validated fix-plan cycle — `/plan` consumes already-validated findings and `/plan-review` reviews the fix plan before implementation | | `why-review` (#16) | Final HOLISTIC standalone review — runs in FULL mode over the settled WHOLE target after the step-15 loop converges | --- ## Conditional Inline Re-Review Protocol (CRITICAL) ### Decision Logic ``` Reviews (steps 1-11) → ALL PASS (no findings)? YES → skip steps 12-15 (/plan → /plan-review → /plan-execute → /changes-review), proceed to final /why-review HOLISTIC full-mode pass (step 16) → /scan --target=domain-entities (step 17, conditional — run on entity/DTO/schema changes, else complete with a cited skip reason) → /docs-update (step 18) → /workflow-end → /watzup → DONE NO (findings exist) → /plan → /plan-review → /plan-execute → (if /plan-execute changed files) /changes-review INLINE re-review (step 15) → loop until clean at the round's bar (rounds 1-2: zero findings; round 3+: zero CRITICAL/HIGH/MEDIUM) → final /why-review HOLISTIC full-mode pass (step 16) Step 16 (ALWAYS): /why-review FULL mode over the settled WHOLE target + changes. If it finds new BLOCKING findings → re-enter /plan → /plan-execute → /changes-review, then re-run step 16; loop until clean at the round's bar (round 3+ ignores LOW-only) → /scan --target=domain-entities (step 17, conditional) → /docs-update (step 18). Note: /code-simplifier (step 11) self-reviews the code it changes via /code-review before returning. Note: /why-review has three workflow occurrences: step 2 FULL mode on the whole starting target in parallel with step 1; step 3 `--validate-findings` mode over step-1 findings; step 16 FULL mode over the settled post-fix target. Steps 2 and 16 share a lens but observe different states, while step 3 is a terminal findings-validation gate. ``` ### Conditional Inline Re-Review Gate (Step 15) — After `/plan-execute` Applies Fixes 1. **CONDITION (run only if /plan-execute changed files):** Step 15 runs ONLY when `/plan-execute` actually modified files. If `/plan-execute` made no file changes, SKIP step 15 and proceed to the step-16 holistic `/why-review`, then `/docs-update`. 2. **DO** re-run the `/changes-review` protocol **INLINE in the main session** over the current full diff. Create a fresh task breakdown, rerun blast radius, risk detection, surface categorization, diff collection, dimensional reviews, synthesis, and validation gates. (Inline by design for this workflow — cheaper than spawning a fresh sub-agent; accept the mild orchestrator-confirmation-bias tradeoff, and counter it by re-reading the diff from scratch.) 3. **DO** track re-review invocation count and repeated blockers in conversation context 4. **DO** integrate the inline `/changes-review` findings — MUST NOT filter, reinterpret, or override 5. **IF** the inline re-review clears the round's bar — PASS with zero findings (rounds 1-2), or zero CRITICAL/HIGH/MEDIUM with only LOW findings left (round 3+, recorded as deferred) → confirm every behavior-changing fix has its required §8 regression/preservation TC, then proceed to the step-16 holistic `/why-review`; after it is clean, continue to `/docs-update` → `/workflow-end` → `/watzup`. 6. **IF** the inline re-review returns FAIL on a BLOCKING finding (any severity in rounds 1-2; CRITICAL/HIGH/MEDIUM from round 3) and the same blocker has not repeated 3 times → validate findings, run `/plan` + `/plan-execute` again, then re-run `/changes-review` (step 15) 7. **IF** the same validated blocker repeats across 3 invocations with no observable progress → STOP and escalate via `AskUserQuestion` — do NOT silently loop or fall back to any prior protocol > **Loop-binding tie-in:** the Step 0 protocol loop (and the `/goal` gate when available) stays OPEN until this loop reaches a clean zero-finding pass (or a 3-repeat blocker escalates). The session cannot stop with a validated finding still unfixed by `/plan-execute` or a non-clean re-review outstanding — this binds on every host, whether or not `/goal` is installed. Each re-review reviews the WHOLE current diff from the first phase combined with ALL prior fixes — never just the previous cycle's fix in isolation. (This applies in every case — including when this workflow is a step inside a parent workflow — because it always runs inline in the main session and owns the loop directly.) > **Specialist finding re-entry (steps 4–10) — DEFAULT is re-run the raising specialist, NOT holistic-only.** The step-15 inline `/changes-review` re-runs only its own BE/FE/SCSS/UI dimensions; it does NOT re-run the specialist reviewers that ran once at steps 4–10. A specialist finding fixed in this cycle MUST therefore receive a scoped re-run of the specialist that raised it. The final step-16 `/why-review` alone may substitute only with explicit written justification. A step-10 UI finding may close on step 15's internal UI dimension because that lens is intentionally duplicated. ### Final Holistic Why-Review Gate (Step 16) — Standalone Full-Mode Review of the Settled Whole Target (ALWAYS RUNS) > **Why this step remains after the new initial whole-target pass:** step 2 reviews the starting state early, in parallel with dimensional review. Any simplification or fix invalidates that verdict. Step 16 applies the same holistic adversarial lens to the settled final state, so parallelizing discovery does not weaken post-fix convergence. 1. **CONDITION:** ALWAYS run after the step-15 `/changes-review` loop converges or is skipped because no fixes were needed. 2. **MODE:** Invoke `/why-review` in **FULL mode** — pass the review target + the current changes as the target (e.g. `/why-review the whole combined with the current changes`). MUST NOT use `--validate-findings` here (that mode is terminal and only re-checks an existing findings list — it would NOT perform the holistic review this step requires). 3. **SCOPE:** "the whole review target combined with current changes" = the complete changeset AND the surrounding code/spec/docs it touches, reviewed as ONE artifact — not a per-file or per-finding pass. `/why-review` runs its full Validation Checklist + both Adversarial Rounds + Easy-to-Change gate, then validates its own findings via its internal closing gate. 4. **LOOP (run → fix → run until no findings):** If step 16 surfaces ANY new finding, validate it, re-enter `/plan` → `/plan-execute` → `/changes-review` (step 15 loop), …(truncated)