Imported from WealthTechPros/sge-public (skills/qa-audit/SKILL.md). Install upstream with npx skills add WealthTechPros/sge-public --skill qa-audit. Copyright stays with the author.

QA Audit

Role

Verify a pull request's behaviour at runtime — start the app, exercise each acceptance criterion, capture evidence, and post a structured report — without owning the merge gate.

Out of scope

• Reviewing the diff or managing merge-gate labels (that is /sge:pr-review)
• Implementing fixes (hand findings back to the PR author)
• Inferring a pass from code review — all passes require observed runtime behaviour

Runtime/behavioural QA of a pull request: check out the PR in an isolated worktree, start the app, exercise each acceptance criterion against the running build, capture evidence, and post a structured report comment on the PR.

Division of labour with /sge:pr-review — differentiated by execution model, not depth:

/sge:pr-review Phase 4.3 consumes this skill's report comment as runtime evidence, so the report format in Step 6 — including the head-SHA-pinned qa-audit-verdict block — is a stable contract — keep its structure and the qa-audit-report marker intact.

Runs as context: fork: the audit is self-contained (own worktree, own server, advisory output only), so it is safe to run forked and in the background. Because a forked context cannot itself spawn further subagents, Step 5's default execution is inline-sequential, not per-criterion fan-out — see Step 5.

Usage

/sge:qa-audit <pr-number> [--adversarial]

--adversarial (issue #2211) switches Steps 1 and 4–6 from "does the stated behaviour work" to "does the control actually stop what it claims to stop." Dispatched by /sge:pr-review Phase 2 whenever rl_diff_control_bearing classifies the diff as touching an enforcement mechanism (a security check, policy gate, or static audit script) — see Adversarial mode below for the full contract. Every other step (worktree isolation, health-check-before-exercise, head-SHA pinning, cleanup) is unchanged; only what gets exercised and how a criterion is judged pass differs.

Target repo — cross-repo / control-session invocation. Apply the shared repo-targeting convention — gh-repo — before anything else: these steps act on the repo in the current working directory. Resolve the plugin root once (used throughout this skill) via SGE_ROOT="$(bash ./scripts/resolve-sge-root.sh 2>/dev/null || bash "${CLAUDE_PLUGIN_ROOT}/scripts/resolve-sge-root.sh")" || exit 1. From a directory that is not the target repo (a control/orchestrator session, or before cd-ing into the PR's worktree) resolve + cd via the shared helper — cd "$("$SGE_ROOT/scripts/with-repo-cwd.sh" resolve owner/repo)" || exit 1 (fail-loud, never falls through to the ambient hub cwd) — and run the startup echo it defines. Because QA runs the target repo's build and dev-server from cwd, the cd (not a bare export GH_REPO) is required. Same-repo: leave GH_REPO unset; cwd detection is used.

Context

• PR snapshot: !gh pr view "$ARGUMENTS" --json number,title,body,url,headRefName,baseRefName,state,isDraft,labels,changedFiles 2>/dev/null || echo "NO_PR_SNAPSHOT — pass a PR number; from another repo export GH_REPO=owner/repo or cd into the target repo first."
• Changed files: !gh pr diff "$ARGUMENTS" --name-only 2>/dev/null || echo "(no diff — set GH_REPO=owner/repo or cd into the target repo)"

The preload above is advisory: under subagent dispatch $ARGUMENTS may not be threaded into the forked context, so it can read NO_PR_SNAPSHOT even though the dispatch prompt named the PR. When the invocation itself carries the PR number, resolve it from there and fetch the snapshot/diff yourself — never stop on an empty preload alone (sge issue #1764).

Step 1: Derive the QA criteria

1. Extract the linked issue number from the PR body (patterns: Closes #N, Fixes #N, Resolves #N, Part of #N, bare #N):

   gh issue view "$ISSUE_NUMBER" --json title,body,labels

2. SGE spec linkage. If the issue (or PR body) references a SPEC-NNN, locate that spec in the repo (spec location per the repo's CLAUDE.md) and extract its Gherkin acceptance scenarios. Each scenario becomes a QA criterion that must be verified at runtime, and each gets a traceability line in the report (scenario → evidence).

3. No linked issue — graceful degradation. Derive the acceptance criteria from the PR description and changed files instead, and say so explicitly in the report: "Criteria derived from PR description — no linked issue found." Do not silently invent requirements; if the PR body is too thin to derive any criterion, report that as a finding.

The output of this step is a numbered list of criteria. Every criterion must end the audit as exactly one of pass, fail, or blocked.

Step 2: Check out the PR in an isolated worktree

Never QA in the shared checkout. Create a detached worktree per the shared worktrees convention — the canonical sibling ../<repo>-worktrees/qa-<PR> layout (purpose token qa, <id> = the PR number).

Claim before create (issue #2214). Export an agent-unique SGE_AGENT_ID and route through resume-or-create.sh with purpose qa, so a concurrent QA lane on the same PR backs off instead of racing this checkout — mechanics: worktrees — PR-scoped lanes.

REPO_ROOT=$(git rev-parse --show-toplevel)
export SGE_AGENT_ID="${SGE_AGENT_ID:-qa-$1}"
while IFS= read -r _line; do case "$_line" in
  verdict:*)  roc_verdict="${_line#verdict:}" ;;
  worktree:*) roc_worktree="${_line#worktree:}" ;;
esac; done < <(bash "$SGE_ROOT/skills/worktrees/resume-or-create.sh" decide "$1" "$REPO_ROOT" "" qa)
[ "$roc_verdict" = "backoff" ] && { echo "PR #$1: qa worktree claimed by a live agent — back off"; exit 3; }
WT="${roc_worktree:-$REPO_ROOT/../$(basename "$REPO_ROOT")-worktrees/qa-$1}"
[ -d "$WT" ] || git worktree add "$WT" --detach
bash "$SGE_ROOT/skills/worktrees/resume-or-create.sh" claim "$WT"
cd "$WT"
gh pr checkout $1
HEAD_SHA=$(git rev-parse HEAD)

Then install dependencies / prepare the environment per the repo's CLAUDE.md. All subsequent steps run inside $WT.

Pin $HEAD_SHA now. This is the commit every criterion below is actually exercised against — it is what makes the Step 6 report verifiable evidence rather than a claim. If the PR gets new commits after this checkout, this report describes the old head; carry $HEAD_SHA through to Step 6 unchanged (re-running Step 2 to re-pin is the only way to cover new commits, exactly as /sge:pr-review re-pins before its own verdict, issue #397).

Step 3: Start the environment and wait for health

Start the dev environment in the background using the repo's documented command (refer to the repo's CLAUDE.md — do not guess stack-specific commands), then wait until it is actually healthy before exercising anything. This is the wait-for-condition loop: wait on the health condition, not the clock.

Primary mechanism — Monitor with an until-healthy condition on the background server task: it wakes you when curl -fsS "http://localhost:${PORT}/" first succeeds, with an explicit timeout. No foreground sleep-polling.

Fallback only when Monitor is unavailable — a bounded curl loop:

# Health-check fallback — adapt URL/port to the repo's documented dev server
for i in $(seq 1 60); do
  curl -fsS "http://localhost:${PORT}/" >/dev/null 2>&1 && { echo "healthy"; break; }
  sleep 2
done

• Do not start exercising criteria until the health check passes.
• If the server never becomes healthy, mark every criterion blocked, capture the server log tail as evidence, and still post the report — a broken boot is itself a QA finding.

Step 4: Exercise the feature (stack-agnostic evidence ladder)

Choose the highest rung available; never hardcode a specific test framework:

1. chrome-devtools MCP tools (when the chrome-devtools MCP server is present) — the standard path for UI/web changes. Navigate to the affected pages, drive the user flow (click/fill/submit), and capture evidence with screenshots, console messages, and network requests. A clean console (no new errors) on the exercised pages is part of the evidence.

2. The repo's own e2e suite — when no browser MCP is available, run the repo's end-to-end/integration suite per its CLAUDE.md ("the repo's quality suite"), scoped to the affected area where the runner supports it, and capture the runner output.

3. Manual curl/CLI evidence — last resort, and the natural path for API/CLI changes regardless:

   curl -fsS -X POST "http://localhost:${PORT}/api/endpoint" \
     -H "Content-Type: application/json" \
     -d '{"key": "value"}'

   Capture the exact command and its output for the report.

Whatever the rung: test edge cases and failure paths, not just the happy path, and exercise each Gherkin scenario from Step 1 as written (Given → When → Then).

Step 5: Verify each criterion

Execution doctrine (fork-safe by default). This skill declares context: fork (line 4) — it is normally invoked as a forked subagent, and a forked subagent cannot itself spawn further subagents (no nested spawning; the same constraint /sge:pr-review documents at its own line 27). So the default path here is inline-sequential verification, not fan-out:

• Work through the criteria list from Step 1 one at a time (still only exercising runtime behaviour via the Step 4 ladder).
• This is the default for every invocation, not just "trivial" PRs — it is what actually executes under context: fork.

Subagent fan-out — non-fork invocation only. If qa-audit is ever run as the top-level agent (invoked directly rather than dispatched via Agent(subagent_type: "fork")), and there are 3+ independent criteria, fan-out is available and faster: dispatch one verifier subagent per criterion (parallel where independent). Do not attempt this while already running as a forked context — the spawn call has no subagent to nest into.

Each verifier (fanned-out or inline) receives the worktree path, the running server's base URL, and one criterion (for Gherkin criteria: the full scenario text), exercises only that criterion, and returns a single structured result:

{ "requirement": "<criterion text>", "status": "pass | fail | blocked", "evidence": "<command + output excerpt, console state, or screenshot description>" }

Rules:

• pass requires observed runtime behaviour — never infer a pass from reading code.
• blocked means the criterion could not be exercised (missing fixture, dead dependency, unreachable route); the evidence states what blocked it.
• The parent aggregates the results verbatim into the Step 6 report. Whichever path produced a result, the result objects keep the same shape.

Adversarial mode (--adversarial, issue #2211)

The gap this closes. The same PR was reviewed twice. A read-the-diff pass approved it with minor observations. An execute-it pass built a 30-case bypass corpus and ran it against the checker under review: 26 of 30 succeeded, including os.system/os.popen/os.exec*/pty.spawn entirely uncovered, a dynamic-import spelling that walked past the scan, and a dependency-manifest field that passed the allowlist having read nothing at all. A third reviewer reproduced a fail-open by constructing the triggering condition natively: an exact-string comparison silently stopped matching, and the script named the PR author as the independent reviewer while exiting 0. For a change that is a control, reading it is not reviewing it — the failure modes live in what the control does not catch, and that is invisible in a diff.

When this mode runs. /sge:pr-review Phase 2 dispatches /sge:qa-audit <pr> --adversarial when rl_diff_control_bearing classifies the diff as touching an enforcement mechanism — a security check, policy gate, or static audit/scanner script. It can also be invoked directly for a manual behavioural audit of any control. This is a selected tier, not a default — a doc-only or prose PR never reaches this mode; ordinary feature/bug-fix PRs use the normal Steps 1–6 above.

Step 1 (adversarial variant) — derive the control's claimed guarantee, not acceptance criteria. Instead of "what should this feature do", ask "what is this control supposed to catch, and how would a violation get past it if the control were broken." Read the control's own logic (the diff, or the full script on a fresh introduction) to enumerate its detection surface — every distinct code path, pattern, or condition it claims to catch — and its known escape hatches to check (anything it explicitly does NOT claim to cover, which is a scope note, not a defect, but must be stated). This list becomes the adversarial criteria; there is no acceptance-criteria/Gherkin substitute here — a control with no stated detection surface is itself a Step 1 finding ("no enumerable claim to test against").

Step 4 (adversarial variant) — construct the adversarial condition, never reason about it (ask 2). For each item in the detection surface:

1. Plant a violation and observe the failure. Write the actual violating input/file/command the control claims to catch, run it through the control exactly as CI would invoke it, and capture the real exit code and output. "This pattern looks like it would match" is not evidence — only an observed non-zero exit (or whatever failure signal the control defines) counts.
2. Neuter the checker and confirm the suite goes red. Temporarily comment out or short-circuit the control's own enforcement logic (in the isolated worktree only — never the shared checkout) and re-run its test suite. A suite that stays green with the control disabled is the tests-passing-for-the-wrong-reason failure mode this mode exists to catch (see ask 4 below); a suite that goes red confirms the tests are actually exercising the control's logic, not merely running past it.
3. Build the bypass corpus systematically, not opportunistically: every distinct evasion technique applicable to the control's implementation language/mechanism — encoding/obfuscation variants, alternate syntax for the same operation (e.g. os.system vs subprocess vs os.popen vs pty.spawn for "shell execution"), boundary conditions (empty input, exact-match vs prefix-match confusion, case sensitivity), and any manifest/config field the control reads but does not validate. Aim for breadth over depth — the measured incident's corpus was 30 cases; there is no fixed minimum, but a single happy-path bypass attempt is not adversarial testing.

Step 5 (adversarial variant) — a negative control is mandatory, and its absence is itself a finding (ask 3). At least one constructed case in the corpus MUST be a genuine, unambiguous violation with no legitimate reason to pass. Run it. If the control does not catch it, that is not "the control has a gap" phrased as advisory — it is a BLOCKER finding, full stop, because it means the auditor cannot demonstrate the control works AT ALL, which is a stronger and more urgent problem than any individual bypass. Each result keeps the Step 5 JSON shape, requirement naming the specific bypass attempted:

{ "requirement": "os.system(\"rm -rf /\") is caught by the shell-exec scanner", "status": "fail", "evidence": "Ran the exact string through scripts/dependency-audit.sh; exit code 0 (expected non-zero); the pattern list at line 42 covers subprocess.* but not os.system/os.popen" }

status: fail here means the bypass succeeded (the control did not catch the violation) — read the corpus results as "did the control catch this", not "did my test pass"; a pass status means the control correctly rejected the constructed violation.

Step 5 (adversarial variant) — vacuous-test detection (ask 4). Separately from the corpus above, inspect the control's own test suite (if it has one) for the specific failure mode measured in the incident: a check with one failure exit path and many failure reasons, tested only by asserting exit != 0. For each existing test case:

1. Identify what specific violation the test's name/comment claims to catch.
2. Trigger a different violation that also causes the same generic non-zero exit.
3. If the test would pass regardless of which violation triggered the exit (i.e. it asserts only the exit code, never the specific error message/reason/matched-pattern), that test is vacuous — it proves the script fails sometimes, not that it fails for the stated reason. Report it as a finding: {"requirement": "test '<name>' asserts only exit code", "status": "fail", "evidence": "swapped the trigger from <original violation> to <unrelated violation>; test still passes because it only checks $? != 0, never the error message"}.

The measured incident: the pre-fix suite scored 5 passed / 3 failed against exit-code-only assertions; adding diagnostic (message/reason) assertions and re-running against the unfixed script (see next) scored 0 passed / 9 failed — every one of the original 5 "passes" was vacuous.

Step 4 (adversarial variant) — pre-change-revision regression run, when the PR claims to fix a defect (ask 5). If the PR body or linked issue claims to fix a bug/bypass/vulnerability, git stash or git worktree add --detach <base-sha> a second checkout at the PR's base (pre-fix) commit, and run the SAME adversarial corpus (including the specific case the issue reports) against that pre-change revision. A regression suite that does not fail against the unfixed code is not a regression suite — if every corpus case passes against the OLD code too, the "fix" demonstrably fixed nothing observable, regardless of what the diff appears to change. Record both results side by side: {"requirement": "<case>", "pre_fix_status": "pass|fail", "post_fix_status": "pass|fail"} — the interesting signal is any case where pre_fix_status == post_fix_status == "pass" (bypass succeeded both before and after) or pre_fix_status == "fail" on the corpus's OWN reproduction case (the fix's regression test doesn't even fail on the bug it claims to fix).

Step 6 (adversarial variant) — report structure. Same qa-audit-report/qa-audit-verdict contract as the standard report (Step 6 below), with these additions:

• A Detection Surface section listing every claimed-catch enumerated in Step 1, each mapped to a corpus case and its result.
• A Negative Control line stating explicitly whether the mandatory negative control passed (control caught the deliberate violation) — a missing or failing negative control is called out before the recommendation, not buried in the table.
• A Vacuous Test Findings section (or "None found") from the Step 5 pass above.
• A Pre-Fix Regression section (or "N/A — PR does not claim to fix a defect") from the Step 4 pre-change-revision run.
• recommendation: APPROVED requires: negative control passed, no vacuous tests found (or all fixed), and — when applicable — the pre-fix run shows the regression case genuinely failing on the old code and passing on the new. Any one of those missing/failing is CHANGES REQUESTED, regardless of how many corpus cases the control did catch.

Step 6: Post the report

Evidence delivery — be honest about the medium. gh pr comment cannot attach arbitrary files, so:

• Include terminal/console/test-runner output inline in fenced blocks (trimmed to the relevant excerpt).
• Describe visual evidence precisely (page, state, what was observed) rather than pretending to attach screenshots.
• If the repo has an evidence convention (an artifacts branch or evidence directory documented in its CLAUDE.md), commit screenshots there and link them.
• Otherwise, note plainly that screenshots were captured in the QA session and are summarised, not attached.

Build the report as a file and post with --body-file (a quoted heredoc would suppress $BRANCH/variable expansion — do not use one):

BRANCH=$(git branch --show-current)
HEAD_SHA=$(git rev-parse HEAD)
cat > /tmp/qa-report-$1.md <<EOF
<!-- qa-audit-report -->
## QA Audit Report

**PR:** #$1 · **Branch:** \`$BRANCH\` · **Head SHA:** \`$HEAD_SHA\`
**Audited by:** Claude (Automated QA — /sge:qa-audit)
**Criteria source:** [linked issue #N / SPEC-NNN Gherkin scenarios / PR description (no linked issue)]

### Requirements Verification

| Requirement | Status | Evidence |
|---|---|---|
| [criterion 1] | pass | [evidence excerpt or description] |
| [criterion 2] | fail | [evidence excerpt or description] |

### Spec Traceability
<!-- One line per Gherkin scenario when a SPEC-NNN applies; omit section otherwise -->
- SPEC-NNN / Scenario: [name] → [evidence reference from the table above]

### Test Summary
[How the app was started, which evidence rung was used, what was exercised]

### Issues Found
[Numbered list, or "None found"]

### Recommendation
**[APPROVED / CHANGES REQUESTED]**
[Brief justification. Advisory only — this audit applies no gate labels.]

\`\`\`qa-audit-verdict
pr: $1
head_sha: $HEAD_SHA
criteria_source: linked-issue | spec-gherkin | pr-description
criteria:
  - requirement: "[criterion 1]"
    status: pass
  - requirement: "[criterion 2]"
    status: fail
recommendation: APPROVED | CHANGES_REQUESTED
audited_at: $(date -u +%Y-%m-%dT%H:%M:%SZ)
\`\`\`
EOF
gh pr comment $1 --body-file /tmp/qa-report-$1.md

Fill the bracketed placeholders with real content before posting — the table rows and the qa-audit-verdict block's criteria list come directly from the Step 5 result objects (requirement/status), not retyped independently. head_sha is $HEAD_SHA from Step 2/above — the commit actually exercised in $WT, never the PR's current tip if new commits landed mid-audit.

Format stability: the leading <!-- qa-audit-report --> marker, the section headings above, and the fenced ```qa-audit-verdict YAML block (mirroring /sge:pr-review's own sge-verdict block and its commit: field) are how /sge:pr-review Phase 4.3 finds and parses this comment. Keep all three stable; add new sections only after ### Recommendation, and only after the qa-audit-verdict block.

Why head_sha matters. /sge:pr-review Phase 4.3 compares this head_sha against the PR's current headRefOid before trusting this report as evidence. A QA pass recorded against an old head must not vouch for commits pushed afterward — same stale-head class pr-review already guards against for its own verdict (issue #397). If the PR gets new commits after this report posts, re-run /sge:qa-audit against the new head; don't assume the old report still applies.

Step 7: Clean up

# stop the background dev server, then:
git worktree remove "$WT" --force

Guidelines

• No gate labels. Never add or remove pr-reviewing/pr-reviewed — /sge:pr-review owns that state machine.
• Test edge cases, not just the happy path; reference exact acceptance criteria wording.
• A criterion verified only by reading the diff is not verified — this skill's whole value is runtime evidence.
• Be honest: if something doesn't work, mark it fail with the evidence; if it couldn't be exercised, mark it blocked and say why. Never soften a fail into the prose.
• Stay stack-agnostic: dev-server commands, ports, e2e runners, and evidence conventions all come from the target repo's CLAUDE.md, never from this skill.