diff --git a/.agents/skills/process/refactor-protocol/SKILL.md b/.agents/skills/process/refactor-protocol/SKILL.md index 930c6dac..c2e4e6d4 100644 --- a/.agents/skills/process/refactor-protocol/SKILL.md +++ b/.agents/skills/process/refactor-protocol/SKILL.md @@ -52,13 +52,15 @@ All work in the worktree; never the primary checkout. Conventional commits in lo - **After a rebase or merge that brings in commits you did not write, lint the *whole tree*** — `ruff check .`, not the files you touched. A rename is a whole-tree fact: your branch renames a symbol, somebody else's branch adds a *new* use of the old name, and git merges both without a conflict because they are different lines. Re-running the tests you edited proves nothing either when the surviving use is a type annotation, which is never evaluated. #339 renamed a test double; #281 landed mid-flight with a fixture annotated on the old name; every targeted pytest module passed and CI answered `F821`. — #339 - **A test double must not encode invisible-order or frozen-state semantics.** Put defaults in the *unmatched-request fallback* so an explicit stub always wins whichever order it was registered in, and derive stub responses from the state the test walks rather than from frozen literals. Both failure modes make a test assert against the fixture instead of the code, and both are silent. — 2026-08 run, T6/T7/T10 - **A test double is constructed against the real signature it doubles, and an absence assertion requires its positive path proven in the same test file.** The two halves are one rule because they fail together: a double built from a remembered signature does not run the code under test at all, and the assertion that then passes is almost always an absence — *nothing was written*, *no download started*, *the field did not change* — which a double that raises on entry satisfies vacuously. So: read the real callable or model before writing the fake (field names included — a fake `IntegrityReport` spelled one field differently made a check that never ran look like a check that found nothing), and never let a "nothing happened" assertion stand alone. Somewhere in the same file, the same double must be shown making something happen; if no test in the file exercises the positive path, the absence proves the fake is broken and not that the code is right. — #491, #496 -- **A new rule is verified by breaking it — and the harness that breaks it lies in three ways unless you hold it to these.** A test that passed the moment it was written has not been shown to fail; deliberately violating the rule it guards is the only thing that tells a test from a description. Every one of the three below has already cost a run: +- **A new rule is verified by breaking it — and the harness that breaks it lies in four ways unless you hold it to these.** A test that passed the moment it was written has not been shown to fail; deliberately violating the rule it guards is the only thing that tells a test from a description. Every one of the four below has already cost a run: - - **Commit the work before the first mutation.** To a directory-wide revert, your uncommitted implementation and the mutation are the same edit. `git checkout -- frontend` after one mutation reverted ~20 files of finished work — and left the mutation in place, because it lived in a file git was not yet tracking. Three more then stacked on that same file and the next run came back as unrelated-looking red spread across the suite, which reads as a broken implementation rather than as a broken harness. + - **Commit the work before the first mutation.** To a directory-wide revert, your uncommitted implementation and the mutation are the same edit. `git checkout -- frontend` after one mutation reverted ~20 files of finished work — and left the mutation in place, because it lived in a file git was not yet tracking. Three more then stacked on that same file and the next run came back as unrelated-looking red spread across the suite, which reads as a broken implementation rather than as a broken harness. It is also the whole of the recovery when the trap below fires: with the finished work on a commit, a tree carrying six stacked mutations costs one `git reset --hard HEAD` and nothing else. + - **The harness must not share a failure path with the tests it runs.** A mutation is *expected* to make a command fail, so a harness that chains its steps on success discards its own cleanup at exactly the moment the cleanup matters. One battery ran `mutate && run && revert` with the test output piped through `head`: `head` closes the pipe, the runner takes SIGPIPE, `pipefail` makes the whole pipeline non-zero, and the `&&` short-circuits before the revert. Four of eight reverts silently never executed, the mutations stacked, and the next run's red read like a broken implementation. So: every step is its own unconditional statement rather than a link in an `&&` chain, the harness asserts a **clean tree before each case** and refuses to continue on one that is dirty, an empty recorded patch is a loud failure rather than a no-op, and test output goes to a file you grep afterwards instead of through anything that can close a pipe underneath the runner. - **Revert each mutation by its exact diff** — `git apply -R` on the recorded patch, or a stash of the single hunk — never by checking out a path. The revert must name what the mutation changed, so that a file it created and a file it edited are both undone, and nothing beside them is. - **Assert the mutation's anchor, before applying it and after.** Before: the text you are about to replace is present, exactly once. After: the replacement is in the file. A mutation that silently patched nothing produces a fully green suite that reads as coverage, and one such run reported a guard verified while no code had changed at all. - — earned #360/#362 + — earned #360/#362 and #514 +- **A green mutation is a claim about one spelling, not about the rule.** A guard enforced at more than one site survives any single-site mutation with the suite still green, and the conclusion that reads as honest — *this rule is unverifiable*, or *that test is redundant* — is then exactly wrong. A cool-down scrub's cutoff scoping lived at two sites, a `grep` deciding whether to rewrite the file at all and an `awk` rule deciding which line to remove, both matching the same cutoff; two single-site mutations came back green before mutating both together went red, and stopping at the first green would have reported a guard verified that no test could see. Before declaring a rule unverifiable or a test redundant, iterate spellings and mutate **every site that enforces the rule** — a multi-site guard needs a multi-site mutation. — 2026-08 run, T2; #507 ## PR & CI