Clarify review commit-readiness objective and blocker substantiation

_shared/recipes/self-review-gate.md

@@ -1,6 +1,6 @@
 ---
 name: self-review-gate
-version: 0.1.0
+version: 0.1.1
 ---
 # Recipe: Self-Review Gate
 
@@ -48,7 +48,12 @@ workflows with decision points), skip it and continue to Step 2.
 2. If the tool reports findings, assess each on value (using the criteria in
    `../review-protocol.md`) and fix issues that add real value
 3. Re-run the tool to confirm fixes pass
-4. Report results and return to the calling workflow
+4. Report results using the Step 5 format below and return to the calling
+   workflow. The tool path must evaluate the same review-protocol criteria
+   as the subagent path — a tool that only checks a subset (e.g., lint
+   without correctness or design review) satisfies its subset, but the
+   gate must still perform Steps 2-5 for uncovered criteria before
+   reporting PASS.
 
 **If no suitable review tool is found**, continue to Step 2.
 
@@ -171,6 +176,17 @@ Report a brief summary to the calling workflow:
 them. The calling workflow decides whether to proceed or stop.}
 ```
 
+**What PASS means.** PASS indicates that the gate's evaluation of the
+reviewed diff against all review-protocol criteria found no remaining
+substantiated blockers. It does not certify mandatory quality gate
+status (lint, tests, CI) or full branch readiness — those are the
+calling workflow's responsibility at the applicable commit or
+publication boundary. Early-phase callers (e.g., per-task code review)
+should not treat PASS as final branch approval; later-phase callers
+at the commit/publish boundary must ensure applicable mandatory checks
+pass and validation is current for the final diff including any
+self-review fixes before proceeding.
+
 The calling workflow is responsible for any git operations needed after the
 gate (staging, committing review fixes, etc.). The gate only makes code
 changes — it does not commit or push.

_shared/review-protocol.md

@@ -1,13 +1,41 @@
 ---
 name: review-protocol
-version: 0.2.0
+version: 0.3.0
 ---
 # Code Review Protocol
 
 Shared evaluation criteria, finding format, and assessment model used by all
 code review activities in this repository — both the standalone `code-review`
 workflow and the self-review gates embedded in other workflows.
 
+## Review Objective and Stopping Rule
+
+The purpose of a code review is to judge whether the change is ready to
+commit — not to find something to comment on. The review evaluates all
+applicable criteria below and checks that mandatory quality gates
+(lint, tests, required CI) have been satisfied.
+
+- **Zero actionable findings with APPROVED is a successful review.** When
+  the required evaluation and checks are complete and no substantiated
+  blockers remain, approving without findings is a valid and expected
+  outcome. Do not invent low-value comments merely to fill the findings
+  section.
+- **No minimum findings quota.** There is no duty to supply feedback.
+  A clean change deserves a clean approval.
+- **Mandatory gates still apply.** Do not issue an unqualified APPROVED
+  while required verification (lint, tests, security checks, or other
+  mandatory quality gates discovered from the project) is incomplete or
+  failing. An approval means the review is complete *and* the change
+  meets the project's commit-readiness bar.
+- **Optional suggestions are welcome but nonblocking.** Clearly valuable
+  improvements (better naming, a small safety margin, a useful test case)
+  may be reported as findings with MEDIUM or LOW severity, but they
+  are explicitly nonblocking. They cannot alone require CHANGES_REQUESTED
+  or trigger another review round. If a human chooses to implement an
+  optional suggestion, that is their prerogative — do not prohibit it,
+  but do not force another review cycle solely because nonblocking
+  advice exists.
+
 ## Core Principles
 
 - **Challenge decisions, not just implementation.** Don't just verify that
@@ -87,7 +115,17 @@ Each finding must include:
 - **Severity:** CRITICAL | HIGH | MEDIUM | LOW
 - **Category:** One of the evaluation criteria above
 - **Issue:** What the problem is
+- **Evidence:** What in the code, tests, or documented requirements
+  substantiates the claim — including whether existing safeguards
+  (error handling, validation, tests, type constraints) already address
+  the concern. For a blocker (CRITICAL/HIGH), cite the concrete problem
+  scenario or the specific requirement that is violated. For a
+  nonblocking suggestion (MEDIUM/LOW), state why the change would be
+  valuable but is not required before commit.
 - **Suggestion:** Concrete, actionable fix
+- **Commit-readiness impact:** `blocking` for CRITICAL/HIGH findings
+  that must be resolved before commit, or `nonblocking` for MEDIUM/LOW
+  suggestions that improve quality but do not prevent approval.
 
 Findings that cannot cite a specific file and location in the actual diff must
 be discarded — they indicate hallucinated references.
@@ -96,11 +134,102 @@ be discarded — they indicate hallucinated references.
 
 | Severity | Meaning | Action |
 |----------|---------|--------|
-| CRITICAL | Would cause bugs, security vulnerabilities, or data loss in production | Must fix before merge |
-| HIGH | Missing error handling, race conditions, incomplete coverage that could cause failures | Should fix before merge |
+| CRITICAL | Would cause bugs, security vulnerabilities, or data loss in production | Must fix before commit |
+| HIGH | Missing error handling, race conditions, incomplete coverage that could cause failures | Must fix before commit |
 | MEDIUM | Naming improvements, minor edge cases, convention violations | Fix if it adds value |
 | LOW | Readability suggestions, minor refactoring opportunities | Fix only if clearly valuable |
 
+## Blocker Substantiation
+
+A finding marked as a blocker (CRITICAL or HIGH) must meet all of these
+criteria:
+
+1. **Concrete problem.** Identify a specific, triggerable failure
+   scenario or a specific violated requirement — not a hypothetical
+   that requires unsupported assumptions about how the code might be
+   used.
+2. **Evidence from the code.** Cite the relevant code path, test gap,
+   or documented requirement. Account for existing safeguards — if
+   the code already handles the concern (error checks, type
+   constraints, upstream validation, tests), the finding is not a
+   blocker.
+3. **Necessity before commit.** Explain why the problem must be fixed
+   before the change lands, not as a follow-up. A desirable
+   alternative implementation, a discretionary polish item, or an
+   unsupported hypothetical use case is insufficient to block.
+
+**Disposition of unsubstantiated blockers.** A CRITICAL/HIGH finding
+that fails the criteria above is not automatically converted to a
+suggestion. If the finding identifies a validated, concrete, clearly
+useful concern that simply does not rise to blocking severity,
+downgrade it to MEDIUM/LOW (nonblocking). If the finding is
+unsupported, already handled by existing safeguards, or based on
+hypothetical scenarios without evidence, discard it — do not preserve
+a finding merely to populate the feedback section. Substantive
+design, testing, and maintainability concerns remain eligible as
+blockers when all three criteria are met.
+
+## Verdict Rules
+
+Before routing on any verdict, apply the **commit-readiness check**:
+APPROVED is valid only when **all three conditions hold**:
+
+1. The required evaluation across all applicable criteria is complete.
+2. All applicable mandatory quality gates (lint, tests, required CI,
+   security checks discovered from the project) are passing — not
+   merely unrun or incomplete.
+3. No substantiated blockers (CRITICAL/HIGH meeting the Blocker
+   Substantiation criteria) remain.
+
+If any condition fails, the verdict is CHANGES_REQUESTED regardless
+of the reviewer's label or the absence of code-level findings. An
+unrun or incomplete mandatory gate is not "passing."
+
+- **CHANGES_REQUESTED** requires at least one substantiated blocker
+  (CRITICAL/HIGH meeting the criteria above) or at least one unmet,
+  failing, or incomplete applicable mandatory quality gate.
+- **APPROVED** is correct only when the commit-readiness check above
+  is satisfied — regardless of whether nonblocking suggestions exist.
+  Zero findings is a valid approved state.
+- When issuing APPROVED with nonblocking suggestions, list them in the
+  findings section. They inform the developer but do not prevent the
+  approval and do not require another review round.
+
+## Criteria Coverage
+
+Structured review reports that issue an APPROVED or CHANGES_REQUESTED
+verdict — such as the standalone `code-review` workflow's report
+template — must include a **Criteria Coverage** record listing every
+evaluation criterion from this protocol with its coverage status.
+This requirement applies to formal review reports with a verdict, not
+to the self-review gate's compact PASS/FLAG summary (which is
+criteria-scoped but uses a different output format without a coverage
+table).
+
+The coverage record is review metadata — a record of what was
+evaluated — not mandatory findings or feedback. Zero findings with
+all criteria covered is a valid APPROVED outcome.
+
+Each row states one of:
+
+- **evaluated** — the criterion was assessed against the changes in
+  scope. Briefly note what was checked (e.g., "no new error paths
+  introduced", "naming consistent with existing module conventions").
+- **not applicable** — the criterion does not apply to this change,
+  with a brief justification (e.g., "no performance-sensitive paths
+  changed"). Applicability must itself be assessed; an unjustified
+  "N/A" is incomplete coverage.
+- **incomplete** — the criterion could not be fully evaluated (e.g.,
+  "test coverage not assessed — no test runner available"). State what
+  is missing.
+
+**Completeness** means every applicable criterion is `evaluated` and
+every non-applicable criterion has a justified `not applicable`.
+Any `incomplete` row or unjustified `not applicable` cannot support
+an APPROVED verdict, even with zero findings and passing quality
+gates. A coverage record is a report of review activity, not proof
+that all defects have been found.
+
 ## Validation Rules
 
 After obtaining a review (whether from a subagent, external tool, or inline
@@ -115,3 +244,12 @@ review), validate every finding:
 4. **Read surrounding context.** The diff shows what changed; the surrounding
    code reveals whether the change fits. Read full files around changed
    sections to understand context.
+5. **Verify applicability.** Confirm the finding applies to the actual
+   code path and is not already addressed by existing guards, checks,
+   type constraints, or tests. A finding about an error condition that
+   is already handled is not valid.
+6. **Verify blocker criteria.** For any CRITICAL or HIGH finding, confirm
+   it meets the Blocker Substantiation requirements above. Downgrade to
+   MEDIUM/LOW only if a validated, concrete, clearly useful nonblocking
+   concern remains; discard if the finding is unsupported or already
+   handled.

bugfix/SKILL.md

@@ -1,6 +1,6 @@
 ---
 name: bugfix
-version: 0.9.4
+version: 0.10.0
 description: >-
   Diagnostic and repair workflow that analyzes error logs, traces root causes,
   implements fixes, and verifies with regression tests.