diff --git a/QA-GUIDE.md b/QA-GUIDE.md index 2c1d2a9a..bdb7a539 100644 --- a/QA-GUIDE.md +++ b/QA-GUIDE.md @@ -31,8 +31,7 @@ every word. The convention answers the issue; that is a close, not a question. ## 1. Baseline -Run the suite green on the UNCHANGED code before touching anything. A red -baseline is its own bug to surface — never build on it, never mask it. +Green on UNCHANGED code first; a red baseline is its own bug to surface. ## 2. Discriminating tests — the core rule @@ -47,23 +46,30 @@ Every behavior the diff claims to fix or add gets a test that: sides of your change has pinned nothing (cyclo.site#398: three deploy-gate fixes, 15KB test file untouched, every test green before AND after). -## 3. Mutation-validate the new tests +## 3. Mutation-validate the new tests — with the bundled tool -For each new test, apply ONE targeted mutation to the line it claims to cover -(negate the guard, flip the comparison, drop the call), confirm the test fails, -restore. A test that survives its own mutation is decoration. +The route is the `adversarial-mutation-test` skill scoped to your change, whose +probe step authors ONE targeted mutation per behavior (its catalog) into a +`mutants.toml` and runs the bundled bin: + +```sh +nix run github:rainlanguage/adversarial-mutation-test#mutation-probe -- mutants.toml +``` + +`mutation-probe --help` is the manual: file format, verdicts, exit codes. Do NOT +hand-roll an edit-run-restore loop — the bin ENFORCES what a hand loop can only +assert. A red, silent or zero-test baseline aborts before any probe; the suite's +own tally proves it RAN, so a crash or compile error is NO-RUN and never a pass; +each target must occur EXACTLY once in its file; and every restore is verified +byte-exact before the next mutant. A test that survives its own mutation is +decoration. ## 4. Oracle discipline -- Expected values derive from the SPEC/ISSUE, never recomputed with the same - function the implementation uses (mirror tests enshrine bugs). -- Fixtures must exercise the case the fix exists for: a decimals-split fix with - all-18/18 fixtures makes every wrong usage an equivalent mutant - (cyclo.site#372); README literals pin the mirror, not the source - (erc4626#185). -- Symmetric properties cannot detect swaps — a*b == b*a whatever the order - (flare#196's reciprocity). Prefer ASYMMETRIC invariants that fail under the - exact confusion the issue names. +The skill's adversarial pass owns the method; these are the local precedents. +All-18/18 decimals fixtures make every wrong usage an equivalent mutant +(cyclo.site#372); README literals pin the mirror, not the source (erc4626#185); +symmetric properties cannot detect swaps (flare#196's reciprocity). ## 5. Guard strength @@ -101,20 +107,15 @@ don't hold. All four lines are required. A line your change cannot have takes `n/a` **with the reason** (a docs-only diff has no mutations to apply); an absent line is not -an option. - -That is enforced where the PR is opened, not only where it is judged. You open -PRs with the `open_pr` MCP tool, which reads the body file and REFUSES (exit 3) -before anything is created, so "a PR without its QA evidence does not get -opened" is literal: it costs one retry inside the run instead of a round trip -through the vetter's queue. The gate checks that the block is PRESENT; whether -its claims hold is still the vetter's call. - -Both gates are on PR-OPEN, so neither can reach a PR already open without the -block. `pr-review-report repair-qa-block --block-file ` -is the retrofit: it APPENDS the block to the body and leaves every other byte -identical, validated with the same predicate the open gate uses. It REFUSES a -body that already has a `## QA` section unless you pass `--replace` — a present -block whose claims don't hold is fixed by re-running the evidence, never by -rewriting the sentence. A body edit moves no commit, so push an `--allow-empty` -commit afterwards or the vetter will skip the PR as already vetted at that head. +an option. `Mutations applied` is TRANSCRIBED from the probe's verdicts (§3) — +the mutant, and the test that KILLED it — never recalled; a mutation nothing +scored is not evidence. + +That is enforced where the PR is opened, not only where it is judged: `open_pr` +reads the body file and REFUSES (exit 3) — PRESENCE only — before anything is +created, so "a PR without its QA evidence does not get opened" is literal. + +The gate cannot reach a PR already open without the block. The retrofit is +`pr-review-report repair-qa-block`, whose `--help` is the manual. A body edit +moves no commit, so push an `--allow-empty` commit afterwards or the vetter +skips the PR as already vetted at that head. diff --git a/campaign-prompt.txt b/campaign-prompt.txt index 5a0d9c32..e451be05 100644 --- a/campaign-prompt.txt +++ b/campaign-prompt.txt @@ -51,7 +51,7 @@ Each run: • PRE-EXISTING `main` RED (the failing check ALSO fails on the base branch, not introduced by this PR) is FIXABLE and is NOT a "leave it": a branch CAN carry ANY fix, INCLUDING the root-cause fix for a red `main`. Do NOT paper over it — a merge-main-only commit or an empty-commit retrigger CANNOT green a genuinely-broken test, and re-doing them every run is the churn this rule exists to kill (a PR with 4+ merge-main / empty-retrigger commits still red is the smell). Instead: READ the failing log, diagnose the ACTUAL failure on `main`, and FIX IT ON THE BRANCH (mutation-validated per (ii) if it touches a test — a real fix, NEVER a weakening) so the check goes green on the PR. Because the same breakage sits on `main`, ALSO open a dedicated minimal fix-`main` PR (one issue per PR) carrying that same root-cause fix — one such fix typically greens the whole cohort of PRs branched off that red `main`. • CANNOT-RESOLVE → FLICK TO DESIGN, never re-churn. If after a genuine diagnosis the red cannot be greened on the branch because doing so needs a HUMAN DECISION — the failing assertion encodes an ambiguous spec / design question, or the only available "fix" is a judgment call the producer must not make unilaterally (weakening an assertion, choosing between incompatible behaviours, a cross-repo contract change) — do NOT leave it to re-attempt every run. FLICK IT TO DESIGN via the transition: `pr-review-report flag-design ""` — the reason IS the question. A design-flagged PR is PARKED (its `ai:design` label surfaces it in `human-queue` and is the cross-run marker; skip it like any human-gated state, never re-attempt). Never claim a red fixed without reading its log. - (ii) Clone fresh into `{{WORK_DIR}}/-pr`, `gh pr checkout ` (if that dir already exists from a prior run, RE-SYNC to the PR branch with checkout-force + clean — `git -C fetch origin && git -C checkout -f -B origin/ && git -C clean -fdx` — NEVER `git reset --hard`, which is forbidden and hook-blocked). REPRODUCE the red locally on the UNCHANGED checkout FIRST: if the failing check is already GREEN locally before you change anything (you cannot reproduce it), it is CI-only — do NOT push a no-op code "fix"; handle it per (i) (empty-commit re-trigger if the log shows a transient flake, else LEAVE and note). Only once you reproduce the red: make the MINIMAL fix for exactly that failure. If the fix touches a test/assertion you MUST prove it is a correctness fix, NOT a weakening: mutation-validate per step 4 (break the covered line → the assertion FAILS → restore → PASSES); making the test pass by loosening/deleting an assertion is FORBIDDEN. A lint/format failure → run the repo's formatter. Then build + run the failing check locally to CONFIRM it goes green. + (ii) Clone fresh into `{{WORK_DIR}}/-pr`, `gh pr checkout ` (if that dir already exists from a prior run, RE-SYNC to the PR branch with checkout-force + clean — `git -C fetch origin && git -C checkout -f -B origin/ && git -C clean -fdx` — NEVER `git reset --hard`, which is forbidden and hook-blocked). REPRODUCE the red locally on the UNCHANGED checkout FIRST: if the failing check is already GREEN locally before you change anything (you cannot reproduce it), it is CI-only — do NOT push a no-op code "fix"; handle it per (i) (empty-commit re-trigger if the log shows a transient flake, else LEAVE and note). Only once you reproduce the red: make the MINIMAL fix for exactly that failure. If the fix touches a test/assertion you MUST prove it is a correctness fix, NOT a weakening: mutation-validate per step 4 (the skill's `mutation-probe` pass, never a hand loop); making the test pass by loosening/deleting an assertion is FORBIDDEN. A lint/format failure → run the repo's formatter. Then build + run the failing check locally to CONFIRM it goes green. (iii) Push the fix as a NEW fast-forward commit on top of the branch tip — with the `mcp__fsm__push` TOOL, `{"clone": ""}` (add `"branch": ""` only when the local branch is named something else). PUSHING IS A TOOL, never a bare `git push`: the tool's result is this run's TYPED RECORD that the rework happened — it reports the sha it created and the PR whose head that sha is, which is what `work-tokens` counts as a work item, and a `git push` in Bash records nothing any reader can join to a PR. It cannot spell a force-push at all. NEVER force-push in ANY form or spelling: not `--force`, `-f`, `--force-with-lease`, `--force-if-includes`, a leading-`+` refspec (`git push origin +branch`), a `HEAD:branch --force`, NOR any of these routed through `git -C push …`. If a plain push is rejected as non-fast-forward, STOP and leave the PR for a human — never reach for a force/lease/refspec variant to land it. Never `git reset --hard` (any form, incl. `git -C `), never rebase-force, never change the PR's scope or its linked issue. Only ever fix PRs authored by `{{ASSIGNEE}}`; never touch another author's PR or branch. (iv) MIGRATION — the deploy-shaped case, triggered by EITHER (a) a red prod-pin / `testProdDeploy*` / deploy-pinned-constant check (REPLACES (ii)/(iii) for it: there is no code fix to push), OR (b) a GREEN PR carrying a legacy `REQUIRES redeploy at land` marker in its body or a trusted note. Under the split deploy/abstract release lifecycle NO merge waits on a deploy (#162): deploy repos freeze per-tag records and publish on `rainix-tag-release`, the on-chain deploy is a separate manual workflow dispatch decoupled from publishing, and library repos never deploy — so either trigger firing is evidence of exactly ONE thing: THIS REPO HAS NOT MIGRATED to the split lifecycle. The PR is blocked on that migration, not on any deploy, and you deploy NOTHING, EVER (there is no producer deploy path; deploys are a human's decoupled manual dispatch). Route it: (1) find the repo's lifecycle-migration issue/PR (search the repo's open issues/PRs for the split-lifecycle / release-workflow migration); (2) if none exists, FILE it — `gh issue create` in that repo, assignee {{ASSIGNEE}}, shaped as the audit skill's lifecycle-split finding (the repo keeps a single "current" deployment pin that premerge CI tests against, so every bytecode-changing PR goes red until an out-of-band deploy; the fix is the split lifecycle: per-tag frozen deploy records + `rainix-tag-release` publishing, with the on-chain deploy a decoupled manual dispatch) — this is the ONE sanctioned `gh issue create` and the only exception to step 4's no-new-issues posture; (3) TRANSITION the PR: `pr-review-report flag-blocked-on "repo not migrated to the split release lifecycle: " --blocked-by ` — the typed dep makes clearance automatic: the vetter clears the flag the run after the migration lands and the PR re-enters vetting fresh. An APPROVED PR is human-sacred and cannot be flagged — report it as blocked-on-migration in the run summary instead (still do (1)/(2) so the dep exists). DEDUP: if the PR already carries `ai:blocked-on` naming the migration, it is parked — do not re-flag. There is no blocked-deploy state and no producer deploy any more (#162/#221) — a PR still wearing the string is an unmodelled label, so treat it as any other PR and route it by the rules above. Still NEVER merge; landing stays interactive on the human's explicit word. 3c. BACKFILL MISSING SCREENSHOTS on your OWN open UI PRs (still existing-work — after 3b's red fixes, before step 4 opens anything new): a UI PR with no screenshot is, like a red PR, in-flight work that is NOT review-ready (step 5), so finish it before starting new work. YOUR LIST IS STEP 2a's `worklist` ROWS WHOSE `nextAction` IS `screenshot-3c`, never a path list written here: `worklist`'s `is_ui_path` is the ONE definition of which paths fall under this requirement — raindex's `packages/webapp` and `packages/ui-components`, everything under a `site/` dashboard tree INCLUDING the `health.json` its panels draw, and every `.svelte`/`.css`/`.html` file wherever a repo keeps it (`cyclo.site` keeps its components in `src/lib/components/`, which is where both of #140's PRs lived) — and a second enumeration in this step is a second definition, free to drift from it while reading as though it agrees. The row's `markers.uiTouch` tells you which answer routed it: `yes` a path under the requirement is in the changed list, `unknown` the changed list could not be ruled out — BOTH route here, because `unknown` means nothing ruled UI out, not that there is none. For each, judge FROM THE DIFF whether it changes RENDERED OUTPUT a user sees (per step 5: `.svelte` markup, a visible component/element, user-visible styling, a non-`.svelte` service whose output is directly displayed — chart data / labels / formatters — OR a `site/` dashboard page's visible markup/section). A change that ONLY touches `