feat(qa): W29 guards — per-PR self-QA block, config-presence≠functional, read-findings (#33)

Three QA guards distilled from a week of Round-1 reviews where the same
failure modes recurred. All lessons are generalised — no environment-,
customer-, or ticket-specific detail.

## Q1 — Self-QA / self-resolve is a per-PR hard blocker

The `implementer ≠ reviewer` guard must hold for **each resolved
PR/change**, not once at the ticket level. When a ticket bundles several
PRs, a reviewer can legitimately QA the PRs they did not author while
still owing a second pair of eyes on their own. A PR resolved, approved,
or QA-closed by its own author is a blocking `(x)` that invalidates the
QA for that change — "only a quick one" and "already deployed, just
closing it out" are not exemptions. Self-*resolving* on the internal
path is the same violation as self-passing to QA2.

- `SKILL.md` "When it applies" — terse per-PR pointer to §E (kept under
the 500-word cap).
- `references/edge-cases.md` §E — new "per resolved PR/change, not per
ticket" paragraph; terminal-verdict block now covers the internal
Resolve path, not just QA2.
- `references/anti-patterns.md` #17 — per-PR clause + self-resolve =
blocking `(x)`.

Coordination: **PR #32** (`fix/self-review-team-queue-31`) tackles the
*detection* side of the same theme — inferring self-review from
authorship rather than the assignee on an unassigned team queue. This PR
is complementary (the *per-PR granularity* + self-resolve-as-blocker
angle) and touches the same §E / #17 regions, so the two should be
rebased together at merge time.

## Q2 — Config presence ≠ functionally proven

A set or grepped config flag, a present setting, or a green healthcheck
proves the setting *exists*, not that the feature *works*. Exercise the
feature end-to-end (fire the webhook with a real payload, render and
screenshot the output, confirm the role/flag actually takes effect) and
attach that evidence to the ticket. A shadowed or overridden config that
still "reports healthy" is its own trap — verify the *effective* running
configuration, not the health endpoint.

- `references/checklist.md` Pillar R — new **R7** (functional proof, not
config presence); severity note extended so R7 is `(x)` for a
behavioural change.
- `references/anti-patterns.md` #21 — mirror.

## Q3 — Two smaller anti-patterns

- **#22 QA findings treated as noise to clear** — a verdict with open
`(!)` / `(?)` items is not "passed". Findings are to be worked, not
clicked away; when four of five checks pass, the fifth is the point.
- **#23 Proposing a fix without checking the current state first** —
verify what already exists (a certificate, a router, a config block may
already be in place) before recommending an addition.

## Notes

- Anti-patterns appended as #21–#23 to keep the existing 1..20 citation
numbers stable (the catalog is cited by number; MD029 is intentionally
disabled for it).
- Lint: markdownlint-cli2, validate-skill (word cap), version-parity,
and the rest of the pre-commit set all pass.
- `CHANGELOG.md` Unreleased updated; no version bump (parity preserved).

Draft — not for merge; opened for review and coordination with #32.

Author: CybotTM

CHANGELOG.md

@@ -21,6 +21,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   path uncovered, so an implementer could self-*resolve* their own work with no
   second reviewer. §E, #17, and "-1 Claim" now forbid self-transitioning to
   `Resolve` *and* `QA passed` until a second set of eyes acknowledges.
+- **The self-review guard applies per resolved PR/change, not per ticket**
+  (`edge-cases.md` §E, anti-pattern #17). When a ticket bundles several PRs the
+  guard holds for each one; a PR the implementer resolved/approved/QA-closed
+  themselves is a blocking `(x)` that invalidates that change's QA. If a bundle
+  mixes your own PRs with a colleague's, split the verdict.
 - **`checklist.md` F1.5**: a description that *contradicts the delivered
   outcome* at resolve time (still frames the work as not-yet-done: in-progress
   status/phase table, unticked boxes for completed work, superseded version) is
@@ -30,6 +35,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - **Eval E5** (`evals/qa-discipline.md`): self-review on an unassigned queue,
   detect by authorship, do not self-resolve on the IT-internal path.
+- **`checklist.md` R7 — functional proof, not config presence.** A set/grepped
+  config flag or a green healthcheck is not proof the feature works: trigger it
+  end-to-end and attach the evidence. A shadowed/overridden config that still
+  "reports healthy" is its own trap — verify the *effective* running config,
+  not the health endpoint. Mirrored as anti-pattern #21.
+- **Anti-patterns #22–#23**: QA findings are to be worked, not clicked away (a
+  verdict with open `(!)` / `(?)` is not "passed"); check the current state
+  before proposing a fix (the certificate/router/config may already exist).
 
 ## [0.5.0] - 2026-06-18
 

skills/peer-qa-review/SKILL.md

@@ -21,7 +21,7 @@ detail in `references/`.
 
 Triggers: see description. **Skip** if: still In Progress (transition
 first); already QA2 (different scope); already closed (post-mortem only); or you
-are the implementer (no self-review).
+are the implementer (no self-review; per-PR, §E).
 
 A ticket-system skill is required (Jira: `jira-communication`) for Stage 0
 discovery. Consult maintenance skills for overrides.

skills/peer-qa-review/references/anti-patterns.md

@@ -29,7 +29,10 @@ Things to flag in the implementer's comments. Each item below is `(!)` unless no
 
 ## Process
 
-17. **Self-review** (see `edge-cases.md` §E). The reviewer can't be the implementer. Detect it from *authorship* (worklog author, `In Progress` transition author, implementation-comment author), **not the assignee**: on a team queue the ticket is worked while Unassigned, so the assignee never reveals the conflict. The block covers the *terminal verdict on both paths*: self-*resolving* on the IT-internal path is the same violation as self-passing to QA2, and closes the ticket with no second reviewer. If forced, document the constraint and flag for asynchronous sanity check; don't transition to Resolved/QA-passed until a second set of eyes acknowledges. `(x)`.
+17. **Self-review** (see `edge-cases.md` §E). The reviewer can't be the implementer. Detect it from *authorship* (worklog author, `In Progress` transition author, implementation-comment author), **not the assignee**: on a team queue the ticket is worked while Unassigned, so the assignee never reveals the conflict. The guard is **per resolved PR/change, not per ticket**: on a ticket bundling several PRs it must hold for each one, and a PR the implementer resolved, approved, or QA-closed themselves is a blocking `(x)` that invalidates that change's QA. The block covers the *terminal verdict on both paths*: self-*resolving* on the IT-internal path is the same violation as self-passing to QA2, and closes the ticket with no second reviewer. If forced, document the constraint and flag for asynchronous sanity check; don't transition to Resolved/QA-passed until a second set of eyes acknowledges. `(x)`.
 18. **QA2 routing skipped for customer-affecting work** — if the change touches customer-visible state, it must go to customer acceptance, not internal-resolve. Defaulting to QA2 when uncertain is the right call.
 19. **No follow-up ticket for structural `(!)` items** — if a should-fix is genuinely structural (architecture, naming, recurring pattern), document it AND file a follow-up. Otherwise it's lost.
 20. **Resolving without checking sibling tickets** — when this ticket is one of N parallel tickets, the others may have caught issues this one missed. A 30-second sibling spot-check is cheap insurance.
+21. **Config presence treated as functional proof** — a set or grepped config flag, a present setting, or a green healthcheck proves the setting *exists*, not that the feature *works*. Trigger the feature end-to-end (a webhook fired with a real payload, a render plus screenshot, the role/flag actually taking effect) and attach that evidence to the ticket. "Reports healthy" over a shadowed or overridden config is a false pass — verify the *effective* running configuration, not the health endpoint. `(x)` if only presence/health backs a behavioural change (see checklist R7).
+22. **QA findings treated as noise to clear** — a verdict with open `(!)` / `(?)` items is not "passed". Findings are there to be worked, not clicked away; don't close or resolve while flagged items are unaddressed. When four of five checks pass, the fifth is the point — read it and act on it before the verdict. `(x)` if closed over an unresolved `(x)`.
+23. **Proposing a fix without checking the current state first** — recommending an addition (a certificate, a router, a config block) before verifying what already exists. The thing may already be in place — a wildcard certificate already on the proxy, a route already defined — so check the live/inventory state before proposing to add it. Skipping this wastes work and adds noise to the ticket.

skills/peer-qa-review/references/checklist.md

@@ -47,8 +47,9 @@ The reviewer **re-runs** these. Copy-pasting the implementer's output does not s
 | R4 | **No collateral damage**: failed services, error-level logs since the change, container health, dependent services | `systemctl --failed`, `journalctl -p err --since '1h ago'`, etc. |
 | R5 | **Tag/release exists** where claimed (annotated vs lightweight is `(i)`; missing is `(x)`) | `git for-each-ref refs/tags/<v>` |
 | R6 | **CI pipeline** for the merged commit/tag was green | `glab api projects/<id>/pipelines?ref=<tag>` or equivalent |
+| R7 | **Functional proof, not config presence** — a set/grepped config flag, a present setting, or a green healthcheck is *not* proof the feature works. Exercise it end-to-end (fire the webhook with a real payload, render and screenshot the output, confirm the role/flag actually takes effect) and attach the evidence to the ticket. A shadowed/overridden config that still "reports healthy" is its own trap — verify the *effective* running configuration, not the health endpoint. | Trigger the feature; capture the evidence |
 
-Severity: each R-check failing on its own is `(x)` for R1–R4 (the fix doesn't actually work or breaks something else); `(!)` for R5–R6 (artefacts present but quality flag).
+Severity: each R-check failing on its own is `(x)` for R1–R4 and R7 (the fix doesn't actually work, isn't functionally proven, or breaks something else); `(!)` for R5–R6 (artefacts present but quality flag).
 
 ## Pillar G — Guardrails (what the change *wasn't* trying to touch)
 

skills/peer-qa-review/references/edge-cases.md

@@ -59,6 +59,8 @@ You cannot review your own work. The first-person bias is too strong; even with
 
 If your account appears in any of these, §E applies regardless of who the ticket is currently assigned to, and regardless of it being Unassigned.
 
+**Per resolved PR/change, not per ticket.** When a ticket bundles several changes or PRs, the guard holds for *each one separately*: apply implementer ≠ reviewer to every resolved PR/change, not once to the ticket as a whole. You may QA the PRs you did not author, but a PR you authored still needs a second reviewer. A PR resolved, approved, or QA-closed by its own author is a blocking `(x)` and invalidates the QA for that change — "only a quick one" and "already deployed, just closing it out" are not exemptions. If a bundle mixes your own PRs with a colleague's, split the verdict: pass the ones you can review, and leave your own in QA for a second pair of eyes.
+
 **Decision**: Hand off to another teammate. If genuinely no other reviewer is available right now:
 
 1. **Document the constraint** in the QA comment explicitly: *"Self-review by implementer due to no available reviewer at <time>. Requesting asynchronous sanity check from <colleague> when available."*