Moe Qa Loop

---
# moe-generated: sha=289b623b6490
name: moe-qa-loop
description: Use when reviewing a task in REVIEW status as the QA agent. Provides the structured decision flow for moe.qa_approve vs moe.qa_reject, with rejectionDetails that drive a clean fix on the worker side.
when_to_use: QA agent claims a task in REVIEW status; replaces ad-hoc "looks fine to me" reviews.
allowed-tools: Read, Grep, Glob, Bash(git diff:*), Bash(git log:*), Bash(git show:*), Bash(git branch:*), Bash(git status:*), Bash(git rev-parse:*), Bash(git add:*), Bash(git commit:*), Bash(grep:*)
---

# Moe QA Loop

Your job: read the worker's diff and the task's plan, decide if it's done, and either `moe.qa_approve` or `moe.qa_reject` with actionable details.

## The decision flow

For each task in `REVIEW`:

1. **Read `task.implementationPlan` and `task.definitionOfDone`.** Know what was promised.
2. **Audit the verification evidence.** `moe.get_context` returns `task.verification` — the exact command the worker ran at completion, its exit code, and an output tail — plus `filesModified`, `commits` (the wrapper's recorded landings — sha, ref, kind, pushed), `landing.lastCompletion` and recent `rejectionHistory`. Re-run the command yourself. Missing evidence, a non-zero exit, output that contradicts the claim, or a command that isn't the one the plan named → reject, citing the evidence gap.
3. **Read the diff — the recorded one.** `git show <sha>` for each `task.commits[]` entry of kind `completion` (plus the same session's `checkpoint` entries when the completion is a follow-up), or `git diff <base>..HEAD -- <filesModified>`; `git branch --contains <sha>` confirms it is on the shared branch. Do not review the dirty shared working tree — other sessions' edits live there too. An empty `task.commits` at REVIEW is a bounded wait, not a blocker. Step 2 already re-ran `task.verification` and the tests, and that re-run normally outlasts the wrapper's landing window on its own — so re-poll `moe.get_context` straight after it rather than idling, up to ~2 minutes total. Never sleep-loop, and re-poll and decide before you stage anything. If a completion commit arrives, review that. If none has by then, verify the row on its merits on the working tree and land it yourself, then `moe.record_commit`, then approve — saying in the `qa_approve` summary that you self-landed after the bounded wait expired. That self-landing fallback is the one sanctioned exception to the dirty-tree rule above, and it is scoped to the paths *measured* to be this row's own by the recipe in `.moe/roles/qa.reference.md` (a path that also carries a peer's hunks is excluded whole). Read it adversarially — see the `adversarial-self-review` skill for the checklist. Count the size: **>400 net changed LOC is itself grounds to reject** (see "Oversized diffs" below).
4. **Verify each Definition-of-Done item.** Map every item to evidence in the diff. Missing evidence is a reject.
5. **Spot-check the tests.** Did the worker add tests for the new behavior? Are they mutation-resistant (`assertEquals('expected', actual)`, not `assert(actual)`)? Are edge cases covered or only the happy path? Any **deleted or weakened test** in the diff (loosened assertion, skipped case, removed file) that the plan didn't call for is a reject on sight.
6. **Run the regression suite if you can.** If the worker's `complete_step` summaries don't include test counts, run the suite yourself — the one the plan named, at the width the plan named (see below).

## Review depth follows the task's position in its epic

Plans deliberately concentrate the heavy verification at the **end of an epic**, not on every task. Review against what *this* task promised, not against the epic's finish line.

- **Mid-epic task** (sibling tasks come after it — check `moe.list_tasks {epicId}` and compare `order`): the bar is "this slice works and is proven by focused tests." Do **not** reject it for lacking end-to-end coverage, full-suite output, or docs the plan assigned to a later task. That work isn't missing; it's scheduled.
- **Epic-final / hardening task, or a task with no epic siblings**: this is the real gate. Full regression evidence, integration coverage of the epic's whole flow, docs, and a clean adversarial pass are all in scope — and their absence *is* a reject.
- **Any task touching shared types, schema, wire protocol, or migrations**: full regression evidence regardless of position.

Still reject at any position for: a DoD item with no code, tests that pass when the code does nothing, scope creep beyond the plan, an unhandled error path, or a `complete_step` claim that doesn't hold when re-run. Lean scope is not lower quality.

## Oversized diffs are a defect

More than **400 net changed LOC** is legitimate grounds for `moe.qa_reject` on its own, whatever the code quality: review defect-discovery collapses past ~400 changed lines, so a diff that size is unreviewable, and unreviewable means unverifiable. Don't line-edit it — reject with `rejectionDetails` telling the architect to split the task (`moe-epic-breakdown` / SPIDR) and land it as reviewable slices. Target diff size is ≤200 net LOC.

## Approve when

- `task.verification` is present, matches the plan's named command, and re-runs green.
- `task.commits` contains a completion commit recorded for this review round and reachable from the shared branch (with `autoCommit` on) — or, when the bounded wait expired with none, the commit you landed yourself and recorded via `moe.record_commit`.
- Every DoD item has clear evidence in the diff.
- Tests cover the new behavior (happy path + at least one edge case) at the depth the plan called for.
- No obvious adversarial-review red flags (concurrency, null-deref, missing cleanup).
- The diff scope matches the plan's scope. No drift, no surprise refactors.

Call `moe.qa_approve` with a `summary` naming what you verified — the commands you re-ran and the DoD items you checked. It is required (the daemon rejects approvals without it) and is persisted on the task as the review audit trail. A `warnings` entry (`NO-COMPLETION-COMMIT`) in the response means you approved without commit evidence — expected only with `autoCommit` off or when you approved inside the post-flight race; otherwise you should have landed it yourself first. `commitEvidence` in the response is what to cite in your summary.

## Reject when

- `verification` evidence is missing, its exit code isn't 0, or it doesn't reproduce when you re-run the command.
- `task.commits` has no completion commit after the bounded wait **and** the self-landing fallback cannot run — the recipe leaves nothing to stage (no owned paths, or every dirty path also carries a peer's hunks and is excluded whole), so the bytes genuinely are not there or cannot be attributed to this row. Cite the evidence: `lastCommitOutcome` is `refused`/`failed`, or the wrapper's `[OK] Committed` banner never came. A missing commit you *can* attribute and land yourself is not a reject — land it.
- A DoD item has no corresponding code change.
- Tests are missing or only check the happy path (for *this* task's behavior — see the depth section above before demanding system-wide coverage from a mid-epic task).
- The diff deletes or weakens existing tests without the plan calling for it.
- The diff exceeds ~400 net changed LOC — reject as oversized, route to a split.
- The diff does something the plan didn't promise (scope creep / surprise refactor).
- An adversarial-review red flag is present and ignored.
- A claim made in `complete_step` (e.g., "all tests pass") doesn't hold when re-run.

Call `moe.qa_reject` with `rejectionDetails` that are **specific and actionable**:

> ❌ "Tests are weak."
> ✅ "src/auth/login.ts:42 — `validateToken` is tested only with a valid token. Add cases for: expired token, malformed token, missing token, token signed with wrong key."

A good reject:
- Names the file and line.
- Says what's missing or wrong.
- Says what would make it pass — specific enough that the worker doesn't have to guess.

Every reject cites a **failed DoD item, a failing command, or a specific out-of-scope hunk**. Style-only preference is never grounds for rejection.

Bad rejects produce ping-pong. Good rejects produce one round-trip.

## What never to do

- **Never move a rejected task to `BACKLOG`.** That deprioritizes work the worker is mid-flow on. Use `moe.qa_reject` — it routes the task back to `WORKING` for the worker to fix.
- **Never approve "with notes."** Either it's done or it's not. If you have notes, reject and let the worker address them.
- **Never re-write the worker's code in your reject message.** Describe the gap, don't fix it for them — they need the practice.

## Second pass: complexity

Once the verdict is settled on correctness, run `ponytail-review` over the same diff — a complexity-only pass
(`delete:` / `stdlib:` / `native:` / `yagni:` / `shrink:`, one line per finding). Route the findings, don't just list them:

- Breaches a rail or a DoD item → a real `rejectionDetails` entry.
- Leaner but equally correct → the `qa_approve { summary }`, or a follow-up card via `moe.create_task`. Taste is never grounds for a reject (see above), and a needless reopen pushes the task toward the 3-reopen auto-flip back to PLANNING.
- A worker's `ponytail:` comment naming a ceiling and an upgrade path is declared intent. Flag it only if the ceiling is wrong or the corner breaks a DoD item.

## When you're not sure

If the diff is large or touches an unfamiliar subsystem, before deciding:

- `Read` the files the diff touches.
- `Grep` for callers of any new public function.
- Check `task.reopenCount` — if > 0, look at past `rejectionDetails` to see if the same issue is recurring.

If after that you still can't tell — `moe.add_comment` on the task asking the worker a specific clarifying question. Don't reject for ambiguity; reject for defect.