From 83a84408f4cb120d234780533f233d72ec4367b1 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 22 Sep 2026 17:06:57 +0000 Subject: [PATCH] chore(norms): forbid weakening checks to reach green (#75) --- skills/review/SKILL.md | 28 ++++++++++++++++++++++++++-- skills/verify/SKILL.md | 26 ++++++++++++++++++++++++-- 2 files changed, 50 insertions(+), 4 deletions(-) diff --git a/skills/review/SKILL.md b/skills/review/SKILL.md index 4d105a0..49553bc 100644 --- a/skills/review/SKILL.md +++ b/skills/review/SKILL.md @@ -31,10 +31,31 @@ whether they hold, not to guess them. Then review the diff against the author's stated intent and proof. +## Test-integrity gate (both lenses, before line-level review) + +A green gate proves the tests passed, not that the change is right, and the cheapest +path to green is to change the test. Before line-level review: + +- **Reviewer A reads the test changes before the code changes** — a test edit is the + fastest way to make a red change look green. Both lenses apply the same rule. +- Treat a diff that rewrites, deletes or skips **many** tests as a red flag: read each + one, compare the new assertion with the old, and require the stated reason. +- Never accept, as the way to a passing gate: a deleted or skipped test, a relaxed + lint/format/type/coverage or `tsconfig` setting, or an assertion edited to match the + new behaviour without a stated reason in the PR body. The signer oracles + (`AGENTS.md` "Hard constraints") are pinned; their expectations are never editable. +- Coverage is not this check: it proves a line ran, not that a test would notice if the + line were wrong. Mutation testing is the check coverage cannot give. + +A changeset that reaches green by weakening the check instead of fixing the behaviour +is `request-changes` (L1), citing this section and the exact rule broken — before +approving the behaviour change it hides. + ## Reviewer A — Correctness & regression - Does the change do what the task state says it should? -- Tests: do new tests fail without the change (bug) or cover the feature - surface (feature)? Are oracle/signature tests still green? +- Tests: read the test changes first (test-integrity gate); do new tests fail without + the change (bug) or cover the feature surface (feature)? Are oracle/signature tests + still green? - Edge cases: empty inputs, part boundary, resumable-upload checkpoint, non-browser transports, tree-shaking impact (no cross-provider references). @@ -66,6 +87,9 @@ Then review the diff against the author's stated intent and proof. ## Done when - The PR-contract gate passed: the body carried all four author sections (a missing one was a `request-changes` finding, never skipped in favour of line-level review). +- The test-integrity gate passed: the test diff was read before the code diff, and no + test was deleted/skipped and no assertion or gate config was weakened — or that was + the `request-changes` finding, with the rule cited in the review output. - Both reviewer runs issued a verdict into their own session and result file; all L1/L2 problems resolved or explicitly declined with reasons; `verify` gates green; verdict recorded in the run log. diff --git a/skills/verify/SKILL.md b/skills/verify/SKILL.md index f642186..31aa8ec 100644 --- a/skills/verify/SKILL.md +++ b/skills/verify/SKILL.md @@ -17,10 +17,10 @@ description: Run the repo's full check gates (lint, format, types, tests, build) ## Steps 1. Run each gate. Fix what it reports — formatting/lint/type issues are fixed - in the working tree, never suppressed. + in the working tree, never suppressed or configured away. 2. If a test fails because the code is mid-change, finish the change first; if it fails for an unrelated reason, stop and report (do not "fix" by - skipping). + skipping, deleting or editing the test). 3. Re-run all gates until green. Record outputs (trimmed) in the run log. 4. Never claim verification success without the final command outputs. 5. Collect the PR-contract evidence while the run is fresh (`docs/norms/ops.md`, @@ -29,6 +29,28 @@ description: Run the repo's full check gates (lint, format, types, tests, build) stage that opens a PR carries all four in the PR body; the host refuses a `pr-opened` whose body is missing any of them. +## Never make a gate pass by weakening it + +Green is evidence only if the check that produced it still measures the same thing. +When a gate is red, the fix is the behaviour, not the gate: + +- Do not delete, skip (`it.skip`/`describe.skip`/`todo`) or rename away a test to hide + a failure. +- Do not relax a gate's configuration — `.oxlintrc.json`, `.oxfmtrc.json`, + `tsconfig*.json`, the `vitest.config.ts` coverage block, or a `package.json` script — + to silence it. +- Do not edit an assertion, snapshot or fixture to match the new behaviour without + recording the reason in the PR body. The signer-oracle expectations + (`AGENTS.md` "Hard constraints") are pinned and never editable. +- If you believe a test itself is wrong, state that in the PR body and leave the test + alone; a reviewer decides, not the diff. + +A gate that fails because of the change you are making means the change is not +finished; a gate that fails for an unrelated reason is a reported blocker, not an edit. + +Coverage is not this rule: it proves a line ran, not that a test would notice if the +line were wrong. Mutation testing is the signal coverage cannot give. + ## Done when - All five gates pass with recorded output, or a gate failure is reported as a blocker with evidence.