--- name: workflow-review-changes-loop description: '[Workflow] Use when you need to run /workflow-review-changes repeatedly in an outer convergence loop until a complete whole-workflow run applies zero fixes — recursive re-review of a fixed scope (branch-diff + current changes) until a clean no-op pass.' --- > Codex compatibility note: > > - Invoke repository skills with `$skill-name` in Codex; this mirrored copy rewrites legacy Claude `/skill-name` references. > - Task tracker mandate: BEFORE executing any workflow or skill step, create/update task tracking for all steps and keep it synchronized as progress changes. > - User-question prompts mean to ask the user directly in Codex. > - Ignore Claude-specific mode-switch instructions when they appear. > - Strict execution contract: when a user explicitly invokes a skill, execute that skill protocol as written. > - Subagent authorization: when a skill is user-invoked or AI-detected and its protocol requires subagents, that skill activation authorizes use of the required `spawn_agent` subagent(s) for that task. > - Do not skip, reorder, or merge protocol steps unless the user explicitly approves the deviation first. > - For workflow skills, execute each listed child-skill step explicitly and report step-by-step evidence. > - If a required step/tool cannot run in this environment, stop and ask the user before adapting. ## Codex Project-Reference Loading (No Hooks) Codex uses static project-reference loading instead of runtime-injected project docs. When coding, planning, debugging, testing, or reviewing, open project docs explicitly using this routing. **Always read:** - `docs/project-config.json` (project-specific paths, commands, modules, and workflow/test settings) - `docs/project-reference/docs-index-reference.md` (routes to the full `docs/project-reference/*` catalog) - `docs/project-reference/lessons.md` (always-on guardrails and anti-patterns) **Missing/stale context route:** If `docs/project-config.json`, the docs index, `lessons.md`, `CLAUDE.md`, `AGENTS.md`, or any task-required reference doc is missing or stale, auto-run `$project-init` or the narrow setup route (`$project-config`, `$docs-init`, `$scan-all`, `$scan --target=`, `$claude-md-init`) before ordinary project-specific work. If Codex mirrors or `AGENTS.md` are missing/stale, ask the user to run `$sync-codex`; do not auto-run it. **Situation-based docs:** - Project structure/architecture/tech-stack/deployment/setup (any layer — backend, frontend, or infra): `project-structure-reference.md` - Backend/CQRS/API/domain/entity changes: `backend-patterns-reference.md`, `domain-entities-reference.md` - Frontend/UI/styling/design-system: `frontend-patterns-reference.md`, `scss-styling-guide.md`, `design-system/README.md` - Spec authoring, `docs/specs/` pathing, or TC format: `feature-spec-reference.md`, `spec-system-reference.md`, `spec-principles.md` - Behavior/public-contract changes or spec-test-code sync: `workflow-spec-test-code-cycle-reference.md` plus the spec docs above - Derived spec indexes/ERDs/reimplementation guides: `spec-system-reference.md` and source Feature Specs under `docs/specs/` - Integration test implementation/review: `integration-test-reference.md` - E2E test implementation/review: `e2e-test-reference.md` - Code review/audit work: `code-review-rules.md` plus domain docs above based on changed files Do not read all docs blindly. Start from `docs-index-reference.md`, then open only relevant files for the task. > **[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:** Converge a review scope to a **clean no-op pass** by re-running the ENTIRE `$workflow-review-changes` workflow INLINE, round after round, over a fixed scope combined with the fixes accumulated so far — stopping when a complete round applies **zero fixes**. **From round 3 the severity floor applies: LOW findings are no longer fixed, so a round that surfaces only LOW findings is a zero-fix round and ENDS the loop.** **Summary:** - **Steps (in order):** (0) resolve scope + Goal Contract → (0b) bind the convergence loop (protocol loop primary + optional `/goal` accelerator) → (1) round loop { run `$workflow-review-changes` INLINE → detect fixes-applied → log iteration } → (2) converge on a zero-fix round OR escalate on non-progress → (3) recap. - **Convergence:** stop ONLY when a whole round applies **zero fixes** (its fix cycle skipped, working tree unchanged, reviews clean) — not merely one clean review. - **Inline invariant:** run `$workflow-review-changes` via the skill invocation, NEVER the `spawn_agent` tool — it self-binds its own review-loop obligation (owning the session Stop hook for its `/goal` gate WHEN available), which a sub-agent cannot own or carry back to this loop. - **Severity floor — from round 3, LOW stops blocking.** Rounds 1-2 fix every validated severity. **From round 3 tell the inner `$workflow-review-changes` to fix only CRITICAL/HIGH/MEDIUM and to defer LOW findings** — a round whose validated findings are ALL LOW therefore applies zero fixes and CONVERGES. Carry every deferred LOW into the recap and Goal Contract; NEVER re-tier a real CRITICAL/HIGH/MEDIUM down to LOW to reach the exit. Severity tiers per `SYNC:severity-rubric`. - **Bounded:** round cap default 3; blocking findings not shrinking across 2 rounds, or cap hit with CRITICAL/HIGH/MEDIUM fixes still landing → **STOP & escalate** by asking the user directly. **Why this skill exists (READ FIRST — it is the whole justification):** `$workflow-review-changes` already converges _internally_ to a clean pass, BUT its inner loop does **NOT** re-run the 7 specialist reviewers from scratch. Per the `Conditional Inline Re-Review Protocol` in `.claude/skills/workflow-review-changes/SKILL.md`, the step-15 inline re-review re-runs only `$changes-review`'s own BE/FE/SCSS/UI dimensions plus _scoped_ re-runs of the specific specialist that raised a finding — `$architecture-review`, `$performance-review`, `$security-review`, `$integration-test-review`, `$production-readiness-review`, `$domain-entities-review`, and `$ui-review` fire ONCE (steps 4–10). Only a **fresh full re-invocation** re-runs ALL specialists over the now-fixed code — catching **second-order defects the fixes themselves introduced** and killing whole-workflow confirmation bias. Without this outer loop those regressions ship unreviewed. **Workflow:** resolve scope + Goal Contract → bind the convergence loop (protocol loop + optional `/goal` accelerator) → **round loop** { run `$workflow-review-changes` INLINE → detect fixes-applied → log iteration } → converge when a round applies zero fixes → recap. **Key Rules:** - **MUST run INLINE in the main session — NEVER dispatch `$workflow-review-changes` as a sub-agent.** It self-binds its own review-loop obligation (owning the session Stop hook for its `/goal` gate when available); as a sub-agent that in-session guarantee is silently lost (see the `[WORKFLOW-IN-WORKFLOW]` execution block in `workflow-review-changes/SKILL.md`). This loop skill therefore also runs inline. - **Convergence = a whole round applied ZERO fixes** (its fix cycle, steps 12–15, was skipped because reviews passed clean). That, not "one clean review", ends the loop. - **Scope base is FIXED across rounds; the working tree grows.** Recompute the scope each round as `branch-diff base` ∪ current uncommitted changes — the diff base never moves, so convergence is measured against a stable target. - **Round cap (default 3)** and **findings-increasing → STOP & escalate** by asking the user directly. NEVER loop open-ended. Cap exhaustion escalates only when CRITICAL/HIGH/MEDIUM fixes are still landing — a LOW-only round converges via the severity floor. - **The severity floor bounds ITERATION, never the standard.** It ends the loop; it never authorizes shipping a known CRITICAL/HIGH/MEDIUM, and it never applies to a binary gate inside the inner workflow (a failing test is a failure, not a LOW finding). --- ## First Principle — Convergence, Not Motion > A round that changes files is progress **only if** the next round finds fewer things to fix. > The loop exists to reach a fixed point (zero fixes), not to keep churning the diff. > If findings stop shrinking, that is a signal to **escalate**, not to spin another round. --- ## Step 0 — Resolve Scope + Goal Contract (FIRST ACTION) 1. **Parse the review scope** from the user prompt into a stable, reusable scope string. It has two parts UNIONed: - **Branch-diff base** — a branch-to-branch or PR diff, e.g. `fix/timelog-org-business-timezone` into `develop`. Capture it as `git diff develop...HEAD` (three-dot: changes on the feature branch since it forked from `develop`) so the base is a **fixed merge-base**, not a moving target. - **Current changes** — the uncommitted working-tree changes (`git status --porcelain`, `git diff` + `git diff --staged`). - **Scope string (recompute each round):** `{branch-diff base} ∪ {current uncommitted changes}`. The base commit is fixed for the whole loop; the uncommitted set legitimately grows as fixes land. - If the prompt names no branch diff (pure "current changes" review), the scope is just the working-tree changes — the loop still applies. 2. **Resolve/create the Goal Contract** per `SYNC:goal-contract-satisfaction-loop` (`plans/goals/{YYMMDD-HHmm}-{slug}/goal.md`, template `.claude/templates/goal-contract-template.md`). Its single **required** Success Criterion: > _A complete `$workflow-review-changes` run over `{scope}` applies **zero fixes** (a clean no-op pass — no `$plan-execute` file changes; **rounds 1-2** no validated findings of any severity, **round 3+** no validated CRITICAL/HIGH/MEDIUM, with remaining LOW findings deferred rather than fixed)._ > Record the round cap (default 3), the severity floor (LOW non-blocking from round 3), and the scope string in **Constraints**. ## Step 0b — Bind the Convergence Loop (protocol-first; `/goal` is an optional accelerator) The convergence loop is bound by TWO layers. The **protocol loop (Steps 1–2) is the BINDING mechanism** and MUST be self-driven by you, the running agent, on every host — with or without any command or hook. The **`/goal` command is an OPTIONAL accelerator** layered on top; it is never the primary mechanism, and its absence NEVER weakens the loop. This mirrors the project rule that hooks/trackers are accelerators only — correctness must not depend on them. **1. Protocol loop — ALWAYS binding (hook/command-independent).** You are personally responsible for not stopping until the loop converges or bounded-escalates. This binds Claude, Codex, and Copilot equally, whether or not `/goal` exists: > Repeatedly run `$workflow-review-changes` INLINE over `{scope}` (branch-diff base ∪ current uncommitted changes, recomputed each round). After each round, detect whether it applied any fix. From round 3 on, instruct the inner workflow to fix only CRITICAL/HIGH/MEDIUM validated findings and to defer LOW ones. Do NOT stop while the last round still applied fixes. Converge when a full round applies ZERO fixes (reviews clean, `$plan-execute` changed no files) — which from round 3 includes a round whose only validated findings were LOW. Cap at `{N=5}` rounds; if blocking findings do not shrink across 2 consecutive rounds, or the cap is hit with CRITICAL/HIGH/MEDIUM fixes still landing → STOP and escalate by asking the user directly. Never loop open-ended. Treat this as a standing obligation you re-read at every Step 2 checkpoint — NOT a one-time note you can rationalize away after the first round. The Goal Contract's required Success Criterion (Step 0) is its durable, host-independent record. **2. `/goal` command — invoke as an accelerator WHEN AVAILABLE.** If a `/goal` command exists and you are permitted to run it in this environment, ALSO invoke it (a real tool/command call, NOT a paraphrase, NOT a Goal Contract file substituted for it) with the SAME condition, so a session Stop hook mechanically enforces the loop: ``` /goal review-changes convergence loop: repeatedly run $workflow-review-changes INLINE over {scope} (branch-diff base ∪ current uncommitted changes, recomputed each round). After each round, detect whether it applied any fix; if fixes>0 → run another round; if a full round applied ZERO fixes (reviews clean, $plan-execute changed no files) → CONVERGED, clear the gate. From round 3 on, the inner workflow fixes only CRITICAL/HIGH/MEDIUM and defers LOW, so a LOW-only round is a zero-fix round and CONVERGES. Do NOT stop while the last round still applied fixes. Cap at {N=5} rounds; if blocking findings do not shrink across 2 consecutive rounds, or the round cap is hit with CRITICAL/HIGH/MEDIUM fixes still landing → STOP and escalate by asking the user directly. Never loop open-ended. ``` The `/goal` Stop hook 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** (e.g. Codex/Copilot, or a Claude run without the command): DO NOT error, DO NOT block, and DO NOT invent a stand-in gate. Record ONE line in the Goal Contract — `/goal accelerator unavailable — loop bound by protocol (Steps 1–2) + this Goal Contract` — and proceed. The protocol loop above plus the Goal Contract are the same gate, enforced by discipline instead of a hook. > **Nested gates (by design, safe):** each inner `$workflow-review-changes` round self-binds its OWN Step 0 review-loop obligation (and installs its own `/goal` gate WHEN available) that clears when that round reaches its internal clean pass. This OUTER loop persists across rounds and **subsumes** the inner ones (a converged loop implies every inner round ended clean). All self-clear on satisfaction — no orphaned gate. Do NOT tell the user to clear either. ## Step 1 — Round Loop (run → detect → log) For each round `R` (starting at 1), do ALL of: 1. **Snapshot before:** record the working-tree fingerprint — `git status --porcelain` + `git diff --stat` (or `git rev-parse` of `git stash create` for an exact hash). This is the fixes-applied baseline for the round. 2. **Run the workflow INLINE:** invoke `$workflow-review-changes` via the skill invocation (NEVER the `spawn_agent` tool) with the recomputed `{scope}` as its prompt. Let it run its full 20-step sequence including its own internal fix→re-review loop. 3. **Detect fixes-applied (objective):** compare the working tree after the round to the before-snapshot AND read the workflow's own result: - **Fixes applied (>0)** if the working tree changed during the round OR the workflow reported its fix cycle (steps 12–15) ran / `$plan-execute` modified files. - **Zero fixes** if the working tree is byte-identical to the before-snapshot AND the workflow reported either no validated findings, or (round 3+) validated findings that are ALL LOW with the fix phase explicitly skipped. - If a claimed LOW-only exit has a changed fingerprint, re-review; it is not a no-op convergence pass. 4. **Append an Iteration Log entry** to the Goal Contract: round number, files changed this round (`file:line`), fixes-applied count, the review verdict, and remaining gaps. ## Step 2 — Convergence & Escalation Gate Evaluate after every round: | Condition | Action | | --------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | Round applied **ZERO fixes** (clean no-op pass) | **CONVERGED** → mark the required criterion PASS in the Goal Satisfaction matrix → clear the `/goal` gate → go to Step 3. | | Round applied fixes AND round `< N` AND findings shrank vs prior round | Recompute `{scope}`, run round `R+1`. | | Findings did **not shrink** across 2 consecutive rounds (same/increasing count) | **STOP & escalate** by asking the user directly — a non-converging loop is a signal, not a reason to spin. | | Round `R ≥ 3` whose validated findings are **ALL LOW** (zero CRITICAL/HIGH/MEDIUM, so zero fixes applied) | **CONVERGED on the severity floor** → do NOT run another round for LOW alone → record every remaining LOW as a deferred finding in the recap + Goal Contract → mark the required criterion PASS → go to Step 3. | | Round cap `N` hit with CRITICAL/HIGH/MEDIUM fixes still landing | **STOP & escalate** by asking the user directly — report the still-open findings; do not silently continue. (LOW-only at the cap converges via the severity-floor row above.) | > **Increasing findings = STOP.** If round `R` surfaces MORE findings than round `R-1`, the fixes are regressing the code — STOP and escalate immediately (mirrors the **Issue count increasing** rule under **Iteration Tracking (Conversation-Scoped)** in `workflow-review-changes/SKILL.md`). Never trade one fix for two new findings across rounds. ## Step 3 — Recap Emit a concise convergence recap: rounds run, total fixes applied per round (the shrinking sequence), the final clean-pass evidence, and the Goal Satisfaction matrix (required criterion PASS). Point to each round's report under `plans/reports/` and the Goal Contract Iteration Log. Do NOT commit or push unless the user explicitly asks. --- ## Convergence Detection — Why Two Conditions A round counts as converged ONLY when **both** hold: (a) the working tree is unchanged by the round, AND (b) the reviews reported clean at that round's bar — no validated findings in rounds 1-2, no validated CRITICAL/HIGH/MEDIUM from round 3 (deferred LOWs listed, not fixed). Both are required because: - Working-tree-unchanged alone is ambiguous — a round can make no changes because a finding was **unfixable/escalated**, not because it was clean. That is escalation, not convergence. - Reviews-clean alone is insufficient — an orchestrator can rationalize a "clean" verdict; the objective `git`-diff comparison is the backstop that proves no fix actually landed. When (a) is true but (b) is false → **escalate** (a real finding the loop cannot close). When (b) is true but (a) is false → the round DID fix things → run another round to re-prove clean. --- **IMPORTANT MANDATORY sequence:** Step 0 (scope + Goal Contract) → Step 0b (bind the convergence loop: protocol loop primary + opti