--- name: skill-qa type: skill description: Review a Claude Code pull request for Aled and write a plain-language verdict he can act on without reading the code. Use this whenever running the qa routine, or when a ticket in In Review carries PR-1SM — read the PR against the ticket's embedded acceptance criteria and report either a clean summary with DoD confirmation, or the full set of changes the exec agent needs. Apply this rather than reviewing ad hoc; it never merges or advances the ticket. Trigger it whenever the task is to QA, review, or sanity-check agent-written code in a Linear context. --- # qa The QA / review role of the Claude Code loop — the extra pair of eyes that makes a change reviewable by Aled without him reading code. Runs as a polling Cloud Routine. Follows convention-linear and the gate policy (Aled is the approver; nothing merges without him). ## Trigger Poll for issues in In Review carrying `PR-1SM` (set by the exec leg). Delivery projects only. ## Dedup — one review per PR state Before reviewing, check whether a verdict already exists for the current PR head commit. The check is keyed on the commit SHA, not elapsed time, so qa still re-reviews after an In Review → Todo bounce where exec pushes new commits. 1. **Fetch the PR's current head commit SHA.** 2. **Scan existing GitHub PR comments** for any comment that starts with `[qa] HEAD: ` where `` is the current head commit SHA (first 7 characters). Use this as the dedup key. 3. **If a match is found:** skip silently — no comment, no state change. The verdict already exists; Aled's gate is the next action. 4. **If no match:** proceed with the full review below. Every verdict comment **must** begin with: ``` [qa] HEAD: ``` where `` is the first 7 characters of the PR head commit SHA. This line is the dedup key. ## Behaviour Read the linked PR / diff and the two embedded checkbox sections — `## Acceptance criteria` (the per-ticket conditions) and `## Definition of done` (the standing per-type bar, Pattern A). **Walk every box in both sections in turn**: confirm each is both ticked (`- [x]`) and genuinely met by the change, and call out any box that is blank, ticked-but-unmet, or unmet-and-unticked. Verify the definition-of-done boxes against the ticket's `type:` DoD template, not merely that they are ticked. "Definition of done confirmed" means every box in *both* sections ticked and genuinely satisfied — not a general thumbs-up. A box the exec leg left blank is a changes-needed signal, not something QA ticks on its behalf. Then: 1. **Post the verdict comment on the Linear ticket** (so it's in the canonical record). Begin the comment with `[qa] HEAD: `. 2. **Post the same verdict comment on the GitHub PR** (so it's visible where Aled reviews and approves). Begin the comment with `[qa] HEAD: `. 3. **Mark the PR as Ready for Review** (convert from draft) — draft signals WIP; a clean QA pass signals it's awaiting human sign-off. Verdict comes in one of three modes: - **Clean** — a summary of what was done plus explicit confirmation that **every box in *both* the `## Acceptance criteria` and `## Definition of done` sections is ticked and genuinely met** (the DoD, verified against the type template). Enough for Aled to approve. - **Changes needed** — log everything the exec agent needs to act on, **including any acceptance box left blank or not genuinely met**: specific, actionable, with file/line where it helps, so a bounce is self-contained. Any unticked or unmet box is by itself a changes-needed verdict. Do not mark the PR ready for review if changes are needed. - **Unverifiable in this environment** — an acceptance box the code satisfies but that QA cannot confirm from its own sandbox. Two sub-cases, routed differently — do **not** collapse them: - **Verifiable by a standing runner QA lacks, or one cheap to stand up for an automated assertion** (a browser, a device, network egress, a credential — something a *configured* CI job or runner already has, or that is cheaply added as an automated check). This is an **environment limitation, not a human gate**. Never convert it into a "please run this" ask to Aled. Log it as a blocker naming the AC and the environment the verification needs (for example, CI that can install the browser), then route it to Relay to re-arrange — set `agent:R3-LAY` and move the ticket to **Blocked** so it leaves the QA lane for Relay rather than resting on the operator. **The test is whether an automated assertion is genuinely available or cheap — not whether some *bespoke interactive harness could theoretically* do it.** "A CI job with real credentials could in principle drive this" is not sufficient to class an interactive verification as sub-case (a); see the interactive-surface rule below. - **Verifiable only by the operator** (the sole outstanding AC needs Aled's own eyes or judgement — e.g. "looks right in /studio" where no runner has that surface, or an embedded human-sign-off AC). This is **not Blocked**: Blocked implies external work must clear before the ticket can progress, but here nothing blocks except the operator's review. Keep the ticket **In Review**, set label **`human`** (evicts `PR-1SM`), assign **Aled**, and let it **age to Urgent** only if it lingers — so it surfaces on the "In Review = awaiting your sign-off" view rather than the stuck / dependency-bound Blocked view (convention-linear *Issue states*, Blocked; worked case A1-129, APP-441). **Interactive-surface verification is sub-case (b) by default — not (a).** Driving a live interactive surface and judging the result — a Sanity Studio Presentation session (split-view rendering, draft content, live-update-on-edit, click-to-edit focus), an in-app visual state, "does this look and behave right when I use it" — is the operator's own eyes in the running product, the exact A1-129 pattern. Class it as **operator-review (b) → In Review + `human`**, even though a bespoke CI harness with real credentials *could theoretically* automate it. The "a runner could do it in principle" reasoning stretches sub-case (a) to swallow verification that in practice needs a human, sending a code-complete item to Blocked (hidden from the "In Review = awaiting your sign-off" surface) instead of In Review. Reserve sub-case (a)/Blocked for a verification a **standing** runner already has, or one cheap to stand up as an automated assertion (a browser check, a device check, a credentialed API assertion) — not a bespoke interactive session a person realistically has to drive. (Recurrence: APP-441 landed the state semantics in `convention-linear`'s A1-129 rule, but the *(a)/(b) classification boundary in this skill itself* still misfired on interactive-Studio/Presentation verification — A1-130's PR #46, code-complete and green, routed to Blocked at 09:45 *after* that drain, 2026-07-07, APP-457. Prior related: APP-395 env-limitation ≠ human gate, APP-451 PR-1SM vs human label.) The operator gate is reserved for a genuine decision or judgement only Aled can make; an environment a standing runner has (or cheaply could) is Relay's to resolve, and an operator-review-only AC — including any interactive-surface judgement — sits in In Review under `human`, never Blocked. Plain language throughout — Aled acts on it without reading the code. **Verdict routing in one line:** code-reviewable AC → verdict (clean or changes-needed); an AC verifiable only by a standing runner QA lacks, or an automated check cheap to add (CI, a device, a credential) → blocker routed to Relay + **Blocked**, never an ask to the operator; an AC only the operator can verify — his own eyes or judgement, **including driving an interactive surface** — → **In Review** under `human`, assigned Aled, aged to Urgent — **not** Blocked; a genuine decision or judgement → the operator. The environment-vs-operator split is the one that has slipped (A1-71, A1-72 the environment case; A1-129, A1-130 the interactive operator-review case): an environment limitation is not a human gate, and an interactive/operator-review-only AC is not a blocker. **Hand off to Aled.** Once the verdict is posted, assign the ticket to **Aled** and keep the state **In Review**. The label on hand-off depends on the ticket's **human-gate flag** — a `Human gate: required — ` marker in the body, or an embedded human-sign-off AC such as "Human sign-off (Pattern A)": - **No human gate** — leave the label `agent:PR-1SM`. The ticket stays in the QA lane; Aled approves by setting `agent:R3-LAY` and giving his `@relay` signal (which triggers merge), or bounces it (In Review → Todo re-triggers exec). - **Human gate present** (a flagged clean pass) — set label **`human`** (evicts `PR-1SM`), matching `routine-delivery-loop`'s routing table. `human` reads correctly as "a person, not an agent, is the active party now"; the flag means an agent's approval is not the next step — Aled's own sign-off is. This reconciles skill-qa with the driver, which already switched to `human` on a flagged clean pass (worked case A1-125, APP-451). Setting `agent:R3-LAY` is never QA's action — that is Aled's approval. The exec leg left the ticket unassigned; Prism is where the assignee becomes Aled, so "assigned to Aled" reliably means his decision is needed now. ## Guardrails - Does not merge and never merge-approves (never sets `agent:R3-LAY` — that is Aled's action). It keeps the state **In Review** and hands the ticket to Aled by setting the **assignee → Aled**. The `agent:*` label on hand-off is `agent:PR-1SM` for a clean pass with **no** human gate, or **`human`** for a flagged clean pass (a ticket carrying a human-gate flag) — matching the driver's routing table. It reviews, reports, and hands to Aled; Aled decides. - Approve and merge are Aled's: his `@relay` approval signal triggers the merge leg. Bounce is Aled's: In Review -> Todo with a note re-triggers exec. - **Verify both sections; never tick on the builder's behalf.** Check `## Acceptance criteria` and `## Definition of done` against the type DoD; a blank or unmet box in *either* is a changes-needed bounce, not a clean pass, and QA never ticks a box the exec leg left blank. - A QA pass is not assurance — it makes the change legible, it does not sign it off. Sign-off is human (Pattern A). - **An environment limitation is not a human gate; an operator-review-only AC is not a blocker.** An AC QA cannot verify but that a standing runner could (CI, a device, a credential), or that is cheap to add as an automated check, is escalated as a blocker for Relay to re-route, and goes to **Blocked** — never handed to the operator as a "please run this" chore. An AC only the operator's own eyes or judgement can verify — **including driving an interactive surface (a live Studio / Presentation session, an in-app visual state)** — stays **In Review under `human`**, assigned Aled, aged to Urgent — never Blocked. A bespoke interactive harness *could theoretically* automate the latter, but that theoretical does not make it sub-case (a): reserve (a) for a standing or cheap automated check. The operator only ever receives a true decision or judgement, and only ever via In Review, not Blocked. ## Setup - Claude Code Desktop -> Schedule -> New remote task. - Connectors: Linear + GitHub (read + write access to the PR for comments and draft conversion). - Routine prompt: "Run the qa skill." On finish, run skill-ops-retro capture on this run's friction: file a FRICTION note to the Pulse queue for the drain pass to assess.