From 9671a3af1239a59915950e65c33717a8b1e7d80e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Wed, 21 Oct 2026 07:37:08 +0900 Subject: [PATCH 01/18] chore(agents): make test skills language-neutral The test skills stay one skill per job rather than gaining a copy per language, because the method is identical in Rust and C# and only the mechanics differ; each language's tool mechanics and test forms sit in a reference loaded only for that language. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .agents/skills/test-oracle-reviewer/SKILL.md | 2 +- .agents/skills/test-pruner/SKILL.md | 36 +++++---- .../{mutants.md => cargo-mutants.md} | 41 +--------- .../test-pruner/references/domain-mutants.md | 37 +++++++++ .../skills/test-pruner/references/stryker.md | 75 +++++++++++++++++++ .agents/skills/test-reviewer/SKILL.md | 10 +-- .agents/skills/test-writer/SKILL.md | 5 +- .../skills/test-writer/references/csharp.md | 15 ++++ 8 files changed, 162 insertions(+), 59 deletions(-) rename .agents/skills/test-pruner/references/{mutants.md => cargo-mutants.md} (78%) create mode 100644 .agents/skills/test-pruner/references/domain-mutants.md create mode 100644 .agents/skills/test-pruner/references/stryker.md create mode 100644 .agents/skills/test-writer/references/csharp.md diff --git a/.agents/skills/test-oracle-reviewer/SKILL.md b/.agents/skills/test-oracle-reviewer/SKILL.md index d159c865b2..0072dd7659 100644 --- a/.agents/skills/test-oracle-reviewer/SKILL.md +++ b/.agents/skills/test-oracle-reviewer/SKILL.md @@ -23,4 +23,4 @@ Unknown is an outcome, not a guess in either direction. In a pull request review, comment on each self-derived test with the independent source to use and leave the fix to the author. When that source needs a small non-test change, such as an injectable deadline the test can choose, request the change. -For mutation evidence on the same pull request, use `test-pruner` in pull request mode. +For mutation evidence on the same pull request, use `test-pruner` in review mode. diff --git a/.agents/skills/test-pruner/SKILL.md b/.agents/skills/test-pruner/SKILL.md index 6601597d58..981ffe8e93 100644 --- a/.agents/skills/test-pruner/SKILL.md +++ b/.agents/skills/test-pruner/SKILL.md @@ -1,31 +1,38 @@ --- name: test-pruner -description: Prune, strengthen, and consolidate tests using oracle classification and cargo-mutants evidence, for one Rust crate or module or for a pull request's diff. Use when the user asks to remove redundant tests, dedupe or consolidate tests with rstest or proptest, audit test value, clean up AI-written tests, or get a mutation-backed review of a pull request's tests. For a quick oracle-only check of a pull request's tests, use `test-oracle-reviewer` instead. +description: Prune, strengthen, and consolidate tests using oracle classification and mutation evidence, cargo-mutants for Rust and Stryker.NET for C#, for one crate, project, or module or for one change's diff. Use when asked to remove redundant tests, consolidate tests into cases or properties, audit test value, clean up AI-written tests, finalize a change's tests before its pull request opens, or get a mutation-backed review of a pull request's tests. For a quick oracle-only check, use `test-oracle-reviewer` instead. --- # Test pruner -Scope each run to one crate or module, or to one pull request's diff, so mutation runs stay affordable and results stay reviewable. +Scope each run to one crate, project, or module, or to one change's diff, so mutation runs stay affordable and results stay reviewable. Decide from two independent measurements: whether the expected value is justified independently of the code, and which faults only this test catches. Mutation testing alone cannot tell a test that matches itself from a correct one, and oracle review alone cannot tell which tests are the sole guard for a fault. +Tool mechanics and case forms: `references/cargo-mutants.md` for Rust, `references/stryker.md` for C#; load only the language in scope. + +The skill has two modes. +In editing mode, run on tests you own, such as a change's diff before its pull request opens, you commit the deletions, consolidations and strengthenings yourself and hand the evidence tables to whoever posts the pull request comment. +In review mode, run on another's pull request, you post a review and edit nothing. +Run mutants in a clone of their own, with absolute paths for every file operation, since an applied mutant left in a shared checkout corrupts every other run there. ## Measure Invoke `test-oracle-reviewer` on every test in scope and record independent, self-derived, or unknown with evidence. -Run cargo-mutants on the scoped source as described in `references/mutants.md`, adding domain mutants from its catalogs because generic mutants miss most domain faults. -Build the kill matrix of mutant by test from the cargo-mutants logs; it needs no per-test reruns. +Run the mutation tool as the reference describes, adding domain mutants from `references/domain-mutants.md` because generic mutants miss most domain faults. +Build the kill matrix of mutant by test from the tool's outputs; it needs no per-test reruns. For tests with several assertions, record which assertion fails for each kill. Run the scoped suite repeatedly on the clean tree, for example 15 times, to find flaky tests before trusting any kill. ## Decide -Keep independent tests; merge them into rstest cases only when they check the same property and differ only in data. +Keep independent tests; merge them into cases of one parametrized test only when they check the same property and differ only in data. Never delete an independent test on mutant evidence: the mutant sample is tiny next to the real fault space, and a spec vector costs nothing to keep. -Strengthen a self-derived test with unique kills by replacing its expected value with an independent one; if none exists, keep it and mark it `// oracle: self-derived; sole guard for `. +Strengthen a self-derived test with unique kills by replacing its expected value with an independent one; if none exists, keep it and mark it `// oracle: self-derived; sole guard for `, naming each fault in words, since mutant ids renumber with every run. A strengthened test that fails is a finding, never a task to make it pass. +Report it to whoever owns the decision, a worker to its lead and a lead to its coordinator or the human, with the test, the independent expected value and its source, and the code's actual output; change neither test nor code to make it pass until that owner decides, because whichever is edited first becomes the specification. Delete a self-derived test with no unique kills. A test of unknown oracle with unique kills has already passed the challenge; strengthen it. -Challenge a test of unknown oracle without unique kills. +Challenge a test of unknown oracle without unique kills, unless it is protected below, since its fate does not depend on the answer. Deletion is a set decision, not a per-test verdict. Two tests that both catch mutant M each have zero unique kills, and deleting both loses M. @@ -46,20 +53,21 @@ Without one, delete the test, or hand a protected test to the file's CODEOWNERS. ## Consolidate -Keep rstest case names readable, with `#[case::name]` or enum-valued `#[values]`. -Use proptest only for a real property; an assertion that re-implements the unit is a self-derived test with random inputs. +Use a property test only for a real property; an assertion that re-implements the unit is a self-derived test with random inputs. Round-trip passes symmetric bugs, so every codec module keeps spec or capture vectors. -Do not add a never-panics property where a fuzz target already covers it. +Do not add a never-panics or never-throws property where a fuzz target already covers it. ## Invariant and output The caught-mutant set never shrinks except through deletions a human approves in a pull request. -Rerun the caught set after every deletion set and every consolidation, verify by rerunning rather than reasoning, and revert on any loss. -Triage surviving mutants as equivalent, coverage gap, or bug lead; a mutant that survives because its code is unreachable is a bug lead. +Rerun the caught set after every deletion set and every consolidation, verify by rerunning rather than reasoning, and revert on any loss; compare runs made with identical tool settings over the same scope. +Triage surviving mutants as equivalent, unconstrained (it changes only text no requirement fixes), coverage gap, or bug lead; a mutant that survives because its code is unreachable is a bug lead. +Close each coverage gap with a test whose expected value is independent, written with `test-writer`; a gap no independent source decides is reported instead. +A suite of independent tests yields few deletions, so expect most of a pass's change to be consolidations and gap tests. -In crate mode, open separate pull requests for deletions, consolidations, and strengthenings, or separate commits when amending an open pull request; they are different reviews. +In editing mode, make separate commits for deletions, consolidations, and strengthenings, each as soon as it is verified, so an interrupted run loses little; they are different reviews. Put the oracle table, kill matrix, challenge results, survivors, and found bugs in a pull request comment; the pull request body is the squash commit. Change only tests and source comments; never modify non-test source otherwise. -In pull request mode, post one review instead: inline comments on the affected tests, with the tables in `
` blocks. +In review mode, post one review: inline comments on the affected tests, with the tables in `
` blocks. Deletions, consolidations, strengthenings, and small non-test changes that enable an independent oracle become requests to the author, with the current caught set as the bar. diff --git a/.agents/skills/test-pruner/references/mutants.md b/.agents/skills/test-pruner/references/cargo-mutants.md similarity index 78% rename from .agents/skills/test-pruner/references/mutants.md rename to .agents/skills/test-pruner/references/cargo-mutants.md index 884e2ba2c8..f7bea9c079 100644 --- a/.agents/skills/test-pruner/references/mutants.md +++ b/.agents/skills/test-pruner/references/cargo-mutants.md @@ -1,7 +1,7 @@ # cargo-mutants mechanics cargo-mutants reports whether the suite caught each mutant. -It has no user-defined mutation operators and keeps per-test results only in its logs, so both are built on its outputs. +It has no user-defined mutation operators and keeps per-test results only in its logs, so the kill matrix is parsed from the logs and domain mutants are patches as `domain-mutants.md` describes. These notes target cargo-mutants 27.1. ## Run @@ -65,40 +65,7 @@ Rerun with the same scope and flags after each change. Run the scoped tests on the clean tree first; with `--baseline skip`, a suite broken by a test edit marks every mutant caught. Any mutant that moved from caught to missed without an approved deletion explaining it means revert. -## Domain mutants +## Consolidation forms -Write each as a small patch; apply it, run the scoped tests, revert it with `git apply -R`; caught means any test failed. -Patches are pull request evidence, not repository files. - -Codec catalog: - -- Swap endianness on one field read or write. -- Length prefix off by one in either direction. -- Remove one bounds check or remaining-length test. -- Wrong padding count, or padding read but not skipped. -- Swap two adjacent fields of the same type. -- Flip one bit in a flags field. -- Write one byte fewer than `size()` reports. -- Return `Ok` on a malformed-input branch that returns `Err`. -- Accept a reserved or out-of-range enum value. - -Async and resource catalog: - -- Remove or shorten a deadline. -- Detach a task instead of aborting it, or hold a `JoinHandle` where an abort-on-drop handle is required. -- Return early before a drain or flush. -- Swallow an error as `Ok`. -- Skip recording a state flag. -- Swap two status or close codes. - -Decision and validation catalog, for each security or policy decision: remove it, weaken it, key it on the wrong identity, or apply it to the wrong object. - -- Always return `Ok` from the check. -- Accept any candidate by dropping a filter. -- Look up by the wrong key, such as a name hash instead of a thumbprint. -- Stop after the first candidate. -- Ignore one input source, such as caller-provided roots. -- Skip verification of a candidate's chain. -- Swap error precedence or report the wrong error. - -Add every mutant the challenger produces that the rest of the suite misses. +Cases are `rstest` with `#[case::name]` or enum-valued `#[values]`, so each case's harness name, which the kill matrix keys on, says what it checks. +Properties are `proptest`, reusing the crate's strategies as `test-writer` says. diff --git a/.agents/skills/test-pruner/references/domain-mutants.md b/.agents/skills/test-pruner/references/domain-mutants.md new file mode 100644 index 0000000000..e960629442 --- /dev/null +++ b/.agents/skills/test-pruner/references/domain-mutants.md @@ -0,0 +1,37 @@ +# Domain mutants + +Neither mutation tool takes user-defined operators, so write each domain mutant as a patch: take it with `git diff` from an edit you then revert rather than by hand, since a hand-written hunk header is easily wrong; `git apply` it, run the scoped tests, `git apply -R` it; caught means any test failed. +Patches are pull request evidence, not repository files. + +Codec catalog: + +- Swap endianness on one field read or write. +- Length prefix off by one in either direction. +- Remove one bounds check or remaining-length test. +- Wrong padding count, or padding read but not skipped; accept non-zero pad bits. +- Swap two adjacent fields of the same type. +- Flip one bit in a flags field. +- Write one byte fewer than the size computation reports. +- Return success (`Ok`, `true`) on a malformed-input branch that returns an error. +- Accept a reserved or out-of-range enum value. + +Async and resource catalog: + +- Remove or shorten a deadline. +- Detach a task instead of aborting it, or hold a handle that neither aborts on drop nor cancels on dispose where one is required. +- Return early before a drain or flush. +- Swallow an error as success. +- Skip recording a state flag. +- Swap two status or close codes. + +Decision and validation catalog, for each security or policy decision: remove it, weaken it, key it on the wrong identity, or apply it to the wrong object. + +- Always return success from the check. +- Accept any candidate by dropping a filter. +- Look up by the wrong key, such as a name hash instead of a thumbprint. +- Stop after the first candidate. +- Ignore one input source, such as caller-provided roots. +- Skip verification of a candidate's chain. +- Swap error precedence or report the wrong error. + +Add every mutant the challenger produces that the rest of the suite misses. diff --git a/.agents/skills/test-pruner/references/stryker.md b/.agents/skills/test-pruner/references/stryker.md new file mode 100644 index 0000000000..8451a29e9a --- /dev/null +++ b/.agents/skills/test-pruner/references/stryker.md @@ -0,0 +1,75 @@ +# Stryker.NET mechanics + +Stryker.NET lists the tests that killed each mutant in its JSON report, but only when bail is disabled, so the kill matrix is read from the report; it has no user-defined mutation operators, so domain mutants are patches as `domain-mutants.md` describes. +These notes target Stryker.NET 5.0.0 with xUnit v2 on the VsTest runner. + +## Install + +In the solution directory, `dotnet new tool-manifest` then `dotnet tool install dotnet-stryker`; the manifest it writes is a repository file, not output, so leave it untracked unless the pull request adopts it. + +## Run + +Run the clean suite first with a TRX log, because the report names tests by id and the TRX carries the id with the display name: + +```powershell +dotnet test .csproj --logger "trx;LogFileName=clean.trx" --results-directory /trx +``` + +Then, from the test project directory: + +```powershell +dotnet stryker --mutate "**//*.cs" --disable-bail --break-on-initial-test-failure --reporter json --reporter cleartext --output --skip-version-check +``` + +`--mutate` takes several globs, `!` excludes, and `.cs{..}` limits to a character span; Stryker mutates the whole project in memory before filtering, so compile-error warnings about files outside the scope are noise. +`--disable-bail` runs every covering test to completion; by default Stryker aborts a mutant's run at the first failure, and `killedBy` then holds only the tests that had failed by then, a partial list that makes kills look unique. +`--break-on-initial-test-failure` stops the run when the clean suite fails, which a test edit can cause, because a matrix built over failing tests proves nothing. +For a change's diff, add `--since:`; a revision expression such as `HEAD~1` or an abbreviated SHA is rejected. +A test-only diff still yields mutants under `--since`: every mutant covered by a test in a changed test file counts as changed. +Pass `--project .csproj` only when the test project references more than one project. +Coverage analysis, `perTest` by default, runs only the tests covering each mutant, so the matrix spans the whole test project without scoping tests and a unique kill is unique across the project; restricting to a test class needs `test-case-filter` in `stryker-config.json`, since the 5.0.0 command line rejects it as a flag. +Stryker builds the test project itself and writes a `.gitignore` of `*` into `--output`, so the output stays untracked wherever it is. +Build and the initial run take a few minutes before the first mutant, so scope first; timeouts are computed per mutant from the initial run, and `additional-timeout` and `timeout-ratio` are config-file options. + +## Kill matrix + +`/reports/mutation-report.json` holds `files[].mutants[]` with `id`, `mutatorName`, `replacement`, `location`, `status` (`Killed`, `Survived`, `NoCoverage`, `Timeout`, `CompileError`, `Ignored`), and `coveredBy` and `killedBy` lists of test ids, and `testFiles[].tests[]` with `id` and `name`. +`name` is the method's qualified name without arguments, so every row of a `[Theory]` shares one name; `id` is the VsTest test case id, which the TRX carries as `UnitTest/@id` with the display name and arguments. +Key on the id throughout: display names collide when xUnit truncates long string arguments. + +```powershell +$report = Get-Content -Raw /reports/mutation-report.json | ConvertFrom-Json +[xml]$trx = Get-Content -Raw /trx/clean.trx +$name = @{} +foreach ($t in $trx.TestRun.TestDefinitions.UnitTest) { $name[$t.id] = $t.name } +$report.files.PSObject.Properties | Where-Object Name -like '**' | ForEach-Object { + foreach ($m in $_.Value.mutants | Where-Object status -eq 'Killed') { + foreach ($id in $m.killedBy) { "$($m.id)`tL$($m.location.start.line) $($m.mutatorName): $($m.replacement)`t$($name[$id])" } + } +} +``` + +A unique kill is a mutant whose `killedBy` has one entry. +A test in `coveredBy` of mutants it never kills catches none of them. +Rows of a `[MemberData]` theory whose arguments xUnit cannot serialize share one id, so the matrix cannot tell them apart; give such rows a string key. +Stryker keeps no per-test output, so for assertion-level evidence apply the mutant by hand from `location` and `replacement`, run the killing test with `dotnet test --filter "FullyQualifiedName~."`, and read the failing assertion from the stack trace. +Mutant ids are assigned in order over the whole project, so an edit anywhere can renumber them, and lines shift with edits above; across runs match mutants by file, `mutatorName`, `replacement`, and the text of the mutated line in `files[].source`. + +## Rerun + +Refresh the clean run and its TRX, then rerun with the same `--mutate` globs and flags after each change. +Do not use `--with-baseline`; it reuses results for mutants it considers unchanged, the opposite of a rerun. +Keep `mutation-level`, `--mutate` and any `test-case-filter` identical between the runs you compare: a run at another level or over a filtered test set reports a different mutant population, not a different suite. + +## Pitfalls + +Verify a surprising status by applying the mutant by hand: Stryker can report as survived a mutant in a static initializer, whose value is computed before the mutant switches on, or a replacement that would not compile on its own, and a machine at full load turns slow kills into timeouts. +A whole library of about 6,000 lines gave 1,724 mutants and ran in 48 minutes at `--concurrency 12`, so a pull request's pass scopes with `--since` or `--mutate` first. +A domain mutant that makes a member public must add an XML doc comment where documentation warnings are errors, or it fails to compile and proves nothing. +Patches written on Windows may carry CRLF; convert them to LF before `git apply` in a repository whose `.gitattributes` sets `eol=lf`. + +## Consolidation forms + +Cases are `[Theory]` with `[InlineData]` rows for literal data, whose display name carries the arguments, or `[MemberData(nameof())]` keyed by a string per row for other data, so a failure names the row. +Properties are CsCheck: compose `Gen` with LINQ, keep each type's generator in one static class of the test project so it is written once, and `Sample` reports a seed on failure to replay through `seed:` or `CsCheck_Seed`. +CsCheck over FsCheck because its generators shrink without per-type shrinkers, so a composed generator costs one declaration like a proptest strategy, and because it adds no FSharp.Core to a C# test project. diff --git a/.agents/skills/test-reviewer/SKILL.md b/.agents/skills/test-reviewer/SKILL.md index ef09b4f529..91501b23d7 100644 --- a/.agents/skills/test-reviewer/SKILL.md +++ b/.agents/skills/test-reviewer/SKILL.md @@ -1,15 +1,15 @@ --- name: test-reviewer -description: Review changed Rust tests against the `test-writer` rules, including a named bug, an independent oracle, non-branching `rstest` bodies, checked snapshots, and properties under `proptest`. Use when a diff or pull request adds or changes Rust tests, or when asked to review test organization or parametrization. +description: Review changed Rust or C# tests against the `test-writer` rules, including a named bug, an independent oracle, non-branching parametrized bodies, checked snapshots, and real properties. Use when a diff or pull request adds or changes tests, or when asked to review test organization or parametrization. --- # Test reviewer -Invoke `test-writer` and use its rules as review criteria without following its generation workflow. +Invoke `test-writer` and use its rules as review criteria without following its generation workflow; for C# tests, judge the forms by its `references/csharp.md`. Review only changed tests unless the user requests a broader audit. Run `test-oracle-reviewer` on each changed test; this skill covers a test's shape, not where its expected values come from. When a test's name and assertions do not say which bug it catches, ask the author rather than guess. -When the change touches an invariant from an applicable intent or contract file that no test checks, ask what pins it and whether `proptest` could exercise it across generated inputs. -A new or changed `expect-test` snapshot is a behavior claim; check that the pull request intends it and that the value is right. +When the change touches an invariant from an applicable intent or contract file that no test checks, ask what pins it and whether a property test could exercise it across generated inputs. +A new or changed snapshot is a behavior claim; check that the pull request intends it and that the value is right. Report each violation with its exact location and a concrete correction, and group repeated instances that share one correction. -Return no findings when the changed tests already comply. \ No newline at end of file +Return no findings when the changed tests already comply. diff --git a/.agents/skills/test-writer/SKILL.md b/.agents/skills/test-writer/SKILL.md index b255c7953c..455652659c 100644 --- a/.agents/skills/test-writer/SKILL.md +++ b/.agents/skills/test-writer/SKILL.md @@ -1,6 +1,6 @@ --- name: test-writer -description: Design Rust tests that catch bugs, choosing between an example test, `rstest` cases, an `expect-test` snapshot, a `proptest` property, and a fuzz target. Use whenever adding or changing Rust unit, integration, snapshot, or property tests. +description: Design tests that catch bugs, choosing between an example test, parametrized cases, a snapshot, a property, and a fuzz target; Rust forms (`rstest`, `expect-test`, `proptest`) in the body, C# forms (xUnit `[Theory]`, CsCheck) in `references/csharp.md`. Use whenever adding or changing unit, integration, snapshot, or property tests in Rust or C#. --- # Test writer @@ -8,6 +8,7 @@ description: Design Rust tests that catch bugs, choosing between an example test First name the bug the test catches that existing tests miss; without one it costs runtime and maintenance and guards nothing. Take the expected value from a source independent of the code under test, as `test-oracle-reviewer` defines; a value derived from the implementation detects change, not bugs. Reach nothing beyond loopback, because an external host makes a failure unreproducible. +The rules below name the Rust forms; for a C# test, read `references/csharp.md` for the form each rule takes there. ## Properties @@ -28,4 +29,4 @@ Move a snapshot or blob longer than a screen into a sibling file through `expect Parametrize with `rstest` only when every case checks the same behavior with different data, and name each case with `#[case::name]` so a failure names the case, not an index. A body that branches on its parameters, through an `if`, a `match`, or an optional expected value, is several tests behind one name, so a failure reports a case instead of the behavior that broke; write one test per behavior and share setup through a helper. A `#[values]` cross product is right only when every combination is a real scenario; one that needs an `if` to skip or adjust a combination is that branching. -Keep each regression test standalone and named for its issue, because its setup reproduces the bug's exact conditions and a case table would generalize them away. \ No newline at end of file +Keep each regression test standalone and named for its issue, because its setup reproduces the bug's exact conditions and a case table would generalize them away. diff --git a/.agents/skills/test-writer/references/csharp.md b/.agents/skills/test-writer/references/csharp.md new file mode 100644 index 0000000000..3fc244f267 --- /dev/null +++ b/.agents/skills/test-writer/references/csharp.md @@ -0,0 +1,15 @@ +# C# forms + +The repository's C# tests are xUnit v2; each rule in `SKILL.md` takes these forms there. + +Properties use CsCheck: compose `Gen` with LINQ and `Sample` it; keep each type's generator in one static class of the test project so it is written once, as a crate keeps its strategies. +A failing `Sample` prints a seed; replay it with `seed:` or the `CsCheck_Seed` environment variable, and pin a regression by keeping the shrunk value as an `[InlineData]` row rather than the seed, since a generator change moves the seed's meaning. +No fuzz harness exists for the repository's C#, so a parser of untrusted input gets a CsCheck property over malformed inputs, and the report says it is not fuzzed. + +No snapshot library is in use; write the expected value inline and, past a screen, in a sibling file copied into the test output. +Nothing rewrites a stale value, so each is edited by hand after reading it. + +Cases are `[Theory]` with `[InlineData]` rows for literal data, whose display name carries the arguments, so a failure names the row as `#[case::name]` does. +Other data goes through `[MemberData(nameof())]` with a string key per row, because rows whose arguments xUnit cannot serialize share one test case, which no filter or kill matrix can tell apart. +A cross product is a `[MemberData]` member that yields it, so the `#[values]` rule applies to that member. +A regression test is a `[Fact]` named for its issue. From 9d46c40aab428397796e840cf33d5580ec79bc70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Wed, 21 Oct 2026 07:58:52 +0900 Subject: [PATCH 02/18] chore(agents): tighten language-neutral test skills Point consolidation forms at `test-writer` instead of restating them in each mechanics reference, and state the failed-strengthening rule once on top of `test-oracle-reviewer`. Exempting protected tests from the challenge leaves no protected test to hand to CODEOWNERS, so that branch goes, and "protected" is now defined where the tests are listed. xUnit v2 turns a whole theory into one test case when any row holds a value it cannot serialize, so `[MemberData]` rows hold only serializable values. Stryker reruns compare `--mutate` runs, because a test edit changes which mutants `--since` selects. Domain mutant patches are taken with `git diff --output`, since a Windows PowerShell redirect writes UTF-16 with CRLF that `git apply` rejects, and a patch that does not build proves nothing. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .agents/skills/test-pruner/SKILL.md | 27 +++++++++---------- .../test-pruner/references/cargo-mutants.md | 8 +----- .../test-pruner/references/domain-mutants.md | 8 +++--- .../skills/test-pruner/references/stryker.md | 26 +++++++----------- .agents/skills/test-reviewer/SKILL.md | 2 +- .agents/skills/test-writer/SKILL.md | 2 +- .../skills/test-writer/references/csharp.md | 5 ++-- 7 files changed, 32 insertions(+), 46 deletions(-) diff --git a/.agents/skills/test-pruner/SKILL.md b/.agents/skills/test-pruner/SKILL.md index 981ffe8e93..86310edff0 100644 --- a/.agents/skills/test-pruner/SKILL.md +++ b/.agents/skills/test-pruner/SKILL.md @@ -8,12 +8,11 @@ description: Prune, strengthen, and consolidate tests using oracle classificatio Scope each run to one crate, project, or module, or to one change's diff, so mutation runs stay affordable and results stay reviewable. Decide from two independent measurements: whether the expected value is justified independently of the code, and which faults only this test catches. Mutation testing alone cannot tell a test that matches itself from a correct one, and oracle review alone cannot tell which tests are the sole guard for a fault. -Tool mechanics and case forms: `references/cargo-mutants.md` for Rust, `references/stryker.md` for C#; load only the language in scope. +Tool mechanics: `references/cargo-mutants.md` for Rust, `references/stryker.md` for C#; load only the language in scope. -The skill has two modes. -In editing mode, run on tests you own, such as a change's diff before its pull request opens, you commit the deletions, consolidations and strengthenings yourself and hand the evidence tables to whoever posts the pull request comment. -In review mode, run on another's pull request, you post a review and edit nothing. -Run mutants in a clone of their own, with absolute paths for every file operation, since an applied mutant left in a shared checkout corrupts every other run there. +Editing mode is for tests you own, such as a change's diff before its pull request opens: commit the changes yourself and hand the evidence tables to whoever posts the pull request comment. +Review mode is for another's pull request: post a review and edit nothing. +Run mutants in a clone of their own and address its files by absolute path, since a relative path resolves against whichever checkout the shell starts in, and an applied mutant left in a shared checkout corrupts every other run there. ## Measure @@ -27,9 +26,8 @@ Run the scoped suite repeatedly on the clean tree, for example 15 times, to find Keep independent tests; merge them into cases of one parametrized test only when they check the same property and differ only in data. Never delete an independent test on mutant evidence: the mutant sample is tiny next to the real fault space, and a spec vector costs nothing to keep. -Strengthen a self-derived test with unique kills by replacing its expected value with an independent one; if none exists, keep it and mark it `// oracle: self-derived; sole guard for `, naming each fault in words, since mutant ids renumber with every run. -A strengthened test that fails is a finding, never a task to make it pass. -Report it to whoever owns the decision, a worker to its lead and a lead to its coordinator or the human, with the test, the independent expected value and its source, and the code's actual output; change neither test nor code to make it pass until that owner decides, because whichever is edited first becomes the specification. +Strengthen a self-derived test with unique kills by replacing its expected value with an independent one; if none exists, keep it and mark it `// oracle: self-derived; sole guard for `, naming each fault in words, since mutant ids and line numbers change between runs. +A strengthened test that fails is a finding, never a task to make it pass: report the test, the independent expected value and its source, and the code's actual output to whoever owns the decision, and change neither test nor code meanwhile, as `test-oracle-reviewer` requires. Delete a self-derived test with no unique kills. A test of unknown oracle with unique kills has already passed the challenge; strengthen it. Challenge a test of unknown oracle without unique kills, unless it is protected below, since its fate does not depend on the answer. @@ -39,7 +37,7 @@ Two tests that both catch mutant M each have zero unique kills, and deleting bot Choose the deletion set so the caught mutants afterward equal the caught mutants before. The same rule applies to assertions: a self-derived one that never fails across its test's kills is a deletion candidate. -Never propose deleting a regression test tied to an issue or fuzz crash, a test of untrusted-input parsing or bounds checks, or a doctest; strengthen or merge those only. +Protected tests are never proposed for deletion, only strengthened or merged: a regression test tied to an issue or fuzz crash, a test of untrusted-input parsing or bounds checks, and a doctest. Treat a suite that deliberately uses snapshots as a change detector: dedupe it, do not delete it. Report flaky tests separately; deleting one hides a bug. @@ -49,10 +47,11 @@ Give a fresh-context agent the test, the code under test, and the mutants the re Apply and run the patch yourself; arguments do not count. Add any mutant it finds to the domain catalog regardless of the verdict. If it finds none, ask the human who wrote or prompted the test for the source of its expected value, and accept a source you can check, never a confirmation. -Without one, delete the test, or hand a protected test to the file's CODEOWNERS. +Without one, delete the test. ## Consolidate +Consolidate into the case and property forms `test-writer` gives for the language, naming every case, since the kill matrix reports each case by its name. Use a property test only for a real property; an assertion that re-implements the unit is a self-derived test with random inputs. Round-trip passes symmetric bugs, so every codec module keeps spec or capture vectors. Do not add a never-panics or never-throws property where a fuzz target already covers it. @@ -61,13 +60,13 @@ Do not add a never-panics or never-throws property where a fuzz target already c The caught-mutant set never shrinks except through deletions a human approves in a pull request. Rerun the caught set after every deletion set and every consolidation, verify by rerunning rather than reasoning, and revert on any loss; compare runs made with identical tool settings over the same scope. -Triage surviving mutants as equivalent, unconstrained (it changes only text no requirement fixes), coverage gap, or bug lead; a mutant that survives because its code is unreachable is a bug lead. +Triage surviving mutants as equivalent, unconstrained (it changes only text that no requirement pins, such as a log or error message), coverage gap, or bug lead; a mutant that survives because its code is unreachable is a bug lead. Close each coverage gap with a test whose expected value is independent, written with `test-writer`; a gap no independent source decides is reported instead. A suite of independent tests yields few deletions, so expect most of a pass's change to be consolidations and gap tests. -In editing mode, make separate commits for deletions, consolidations, and strengthenings, each as soon as it is verified, so an interrupted run loses little; they are different reviews. -Put the oracle table, kill matrix, challenge results, survivors, and found bugs in a pull request comment; the pull request body is the squash commit. +In editing mode, commit deletions, consolidations, strengthenings, and gap tests separately, each as soon as it is verified, so an interrupted run loses little; they are different reviews. +The evidence tables are the oracle table, kill matrix, challenge results, survivors, and found bugs; they go in a pull request comment, since the pull request body is the squash commit. Change only tests and source comments; never modify non-test source otherwise. In review mode, post one review: inline comments on the affected tests, with the tables in `
` blocks. -Deletions, consolidations, strengthenings, and small non-test changes that enable an independent oracle become requests to the author, with the current caught set as the bar. +Deletions, consolidations, strengthenings, gap tests, and small non-test changes that enable an independent oracle become requests to the author, with the current caught set as the bar. diff --git a/.agents/skills/test-pruner/references/cargo-mutants.md b/.agents/skills/test-pruner/references/cargo-mutants.md index f7bea9c079..72beed53a8 100644 --- a/.agents/skills/test-pruner/references/cargo-mutants.md +++ b/.agents/skills/test-pruner/references/cargo-mutants.md @@ -1,7 +1,7 @@ # cargo-mutants mechanics cargo-mutants reports whether the suite caught each mutant. -It has no user-defined mutation operators and keeps per-test results only in its logs, so the kill matrix is parsed from the logs and domain mutants are patches as `domain-mutants.md` describes. +It keeps per-test results only in its logs, so the kill matrix is parsed from them. These notes target cargo-mutants 27.1. ## Run @@ -61,11 +61,5 @@ For assertion-level evidence, read the `panicked at :` location of e ## Rerun -Rerun with the same scope and flags after each change. Run the scoped tests on the clean tree first; with `--baseline skip`, a suite broken by a test edit marks every mutant caught. Any mutant that moved from caught to missed without an approved deletion explaining it means revert. - -## Consolidation forms - -Cases are `rstest` with `#[case::name]` or enum-valued `#[values]`, so each case's harness name, which the kill matrix keys on, says what it checks. -Properties are `proptest`, reusing the crate's strategies as `test-writer` says. diff --git a/.agents/skills/test-pruner/references/domain-mutants.md b/.agents/skills/test-pruner/references/domain-mutants.md index e960629442..f2c67759a6 100644 --- a/.agents/skills/test-pruner/references/domain-mutants.md +++ b/.agents/skills/test-pruner/references/domain-mutants.md @@ -1,6 +1,8 @@ # Domain mutants -Neither mutation tool takes user-defined operators, so write each domain mutant as a patch: take it with `git diff` from an edit you then revert rather than by hand, since a hand-written hunk header is easily wrong; `git apply` it, run the scoped tests, `git apply -R` it; caught means any test failed. +Neither mutation tool takes user-defined operators, so write each domain mutant as a patch. +Take it with `git diff --output=` from an edit you then revert, since a hand-written hunk header is easily wrong and a Windows PowerShell redirect re-encodes the patch as UTF-16 with CRLF, which `git apply` rejects. +`git apply` it, run the scoped tests, and `git apply -R` it; caught means a test failed, and a patch that does not build proves nothing until it does. Patches are pull request evidence, not repository files. Codec catalog: @@ -12,7 +14,7 @@ Codec catalog: - Swap two adjacent fields of the same type. - Flip one bit in a flags field. - Write one byte fewer than the size computation reports. -- Return success (`Ok`, `true`) on a malformed-input branch that returns an error. +- Return success (`Ok`, `true`) on a malformed-input branch that returns an error or throws. - Accept a reserved or out-of-range enum value. Async and resource catalog: @@ -33,5 +35,3 @@ Decision and validation catalog, for each security or policy decision: remove it - Ignore one input source, such as caller-provided roots. - Skip verification of a candidate's chain. - Swap error precedence or report the wrong error. - -Add every mutant the challenger produces that the rest of the suite misses. diff --git a/.agents/skills/test-pruner/references/stryker.md b/.agents/skills/test-pruner/references/stryker.md index 8451a29e9a..67cde9ded0 100644 --- a/.agents/skills/test-pruner/references/stryker.md +++ b/.agents/skills/test-pruner/references/stryker.md @@ -1,11 +1,11 @@ # Stryker.NET mechanics -Stryker.NET lists the tests that killed each mutant in its JSON report, but only when bail is disabled, so the kill matrix is read from the report; it has no user-defined mutation operators, so domain mutants are patches as `domain-mutants.md` describes. +Stryker.NET lists the tests that killed each mutant in its JSON report, but only when bail is disabled, so the kill matrix is read from the report. These notes target Stryker.NET 5.0.0 with xUnit v2 on the VsTest runner. ## Install -In the solution directory, `dotnet new tool-manifest` then `dotnet tool install dotnet-stryker`; the manifest it writes is a repository file, not output, so leave it untracked unless the pull request adopts it. +In the solution directory, `dotnet new tool-manifest` then `dotnet tool install dotnet-stryker --version 5.0.0`; the manifest it writes is a repository file, not output, so leave it untracked unless the pull request adopts it. ## Run @@ -24,12 +24,12 @@ dotnet stryker --mutate "**//*.cs" --disable-bail --break-on-initial-tes `--mutate` takes several globs, `!` excludes, and `.cs{..}` limits to a character span; Stryker mutates the whole project in memory before filtering, so compile-error warnings about files outside the scope are noise. `--disable-bail` runs every covering test to completion; by default Stryker aborts a mutant's run at the first failure, and `killedBy` then holds only the tests that had failed by then, a partial list that makes kills look unique. `--break-on-initial-test-failure` stops the run when the clean suite fails, which a test edit can cause, because a matrix built over failing tests proves nothing. -For a change's diff, add `--since:`; a revision expression such as `HEAD~1` or an abbreviated SHA is rejected. +For a change's diff, `--since:` selects its mutants; a revision expression such as `HEAD~1` or an abbreviated SHA is rejected. A test-only diff still yields mutants under `--since`: every mutant covered by a test in a changed test file counts as changed. Pass `--project .csproj` only when the test project references more than one project. Coverage analysis, `perTest` by default, runs only the tests covering each mutant, so the matrix spans the whole test project without scoping tests and a unique kill is unique across the project; restricting to a test class needs `test-case-filter` in `stryker-config.json`, since the 5.0.0 command line rejects it as a flag. Stryker builds the test project itself and writes a `.gitignore` of `*` into `--output`, so the output stays untracked wherever it is. -Build and the initial run take a few minutes before the first mutant, so scope first; timeouts are computed per mutant from the initial run, and `additional-timeout` and `timeout-ratio` are config-file options. +Build and the initial run take a few minutes before the first mutant, and a whole library of a few thousand lines can run for most of an hour, so scope first; timeouts are computed per mutant from the initial run, and `additional-timeout` and `timeout-ratio` are config-file options. ## Kill matrix @@ -50,26 +50,18 @@ $report.files.PSObject.Properties | Where-Object Name -like '**' | ForEa ``` A unique kill is a mutant whose `killedBy` has one entry. -A test in `coveredBy` of mutants it never kills catches none of them. -Rows of a `[MemberData]` theory whose arguments xUnit cannot serialize share one id, so the matrix cannot tell them apart; give such rows a string key. +Read kills only from `killedBy`; `coveredBy` lists the tests that ran against a mutant, not those that failed. +The script lists `Killed` mutants only; attribute a `Timeout` by rerunning its `coveredBy` tests one at a time before counting it caught. +One row with an argument xUnit cannot serialize makes the whole theory one test id, so the matrix cannot tell its rows apart; `test-writer`'s C# forms avoid this. Stryker keeps no per-test output, so for assertion-level evidence apply the mutant by hand from `location` and `replacement`, run the killing test with `dotnet test --filter "FullyQualifiedName~."`, and read the failing assertion from the stack trace. Mutant ids are assigned in order over the whole project, so an edit anywhere can renumber them, and lines shift with edits above; across runs match mutants by file, `mutatorName`, `replacement`, and the text of the mutated line in `files[].source`. ## Rerun -Refresh the clean run and its TRX, then rerun with the same `--mutate` globs and flags after each change. +Refresh the clean run and its TRX before each rerun, and compare only runs scoped by `--mutate` over the files a `--since` run mutated: a test edit changes which mutants `--since` selects, so a lost kill drops out instead of surviving. Do not use `--with-baseline`; it reuses results for mutants it considers unchanged, the opposite of a rerun. -Keep `mutation-level`, `--mutate` and any `test-case-filter` identical between the runs you compare: a run at another level or over a filtered test set reports a different mutant population, not a different suite. +Identical settings include `mutation-level` and any `test-case-filter`, since either changes the mutant population or the tests that can kill it. ## Pitfalls Verify a surprising status by applying the mutant by hand: Stryker can report as survived a mutant in a static initializer, whose value is computed before the mutant switches on, or a replacement that would not compile on its own, and a machine at full load turns slow kills into timeouts. -A whole library of about 6,000 lines gave 1,724 mutants and ran in 48 minutes at `--concurrency 12`, so a pull request's pass scopes with `--since` or `--mutate` first. -A domain mutant that makes a member public must add an XML doc comment where documentation warnings are errors, or it fails to compile and proves nothing. -Patches written on Windows may carry CRLF; convert them to LF before `git apply` in a repository whose `.gitattributes` sets `eol=lf`. - -## Consolidation forms - -Cases are `[Theory]` with `[InlineData]` rows for literal data, whose display name carries the arguments, or `[MemberData(nameof())]` keyed by a string per row for other data, so a failure names the row. -Properties are CsCheck: compose `Gen` with LINQ, keep each type's generator in one static class of the test project so it is written once, and `Sample` reports a seed on failure to replay through `seed:` or `CsCheck_Seed`. -CsCheck over FsCheck because its generators shrink without per-type shrinkers, so a composed generator costs one declaration like a proptest strategy, and because it adds no FSharp.Core to a C# test project. diff --git a/.agents/skills/test-reviewer/SKILL.md b/.agents/skills/test-reviewer/SKILL.md index 91501b23d7..3b45616e5b 100644 --- a/.agents/skills/test-reviewer/SKILL.md +++ b/.agents/skills/test-reviewer/SKILL.md @@ -5,7 +5,7 @@ description: Review changed Rust or C# tests against the `test-writer` rules, in # Test reviewer -Invoke `test-writer` and use its rules as review criteria without following its generation workflow; for C# tests, judge the forms by its `references/csharp.md`. +Invoke `test-writer` and use its rules as review criteria without following its generation workflow. Review only changed tests unless the user requests a broader audit. Run `test-oracle-reviewer` on each changed test; this skill covers a test's shape, not where its expected values come from. When a test's name and assertions do not say which bug it catches, ask the author rather than guess. diff --git a/.agents/skills/test-writer/SKILL.md b/.agents/skills/test-writer/SKILL.md index 455652659c..fa70849e34 100644 --- a/.agents/skills/test-writer/SKILL.md +++ b/.agents/skills/test-writer/SKILL.md @@ -1,6 +1,6 @@ --- name: test-writer -description: Design tests that catch bugs, choosing between an example test, parametrized cases, a snapshot, a property, and a fuzz target; Rust forms (`rstest`, `expect-test`, `proptest`) in the body, C# forms (xUnit `[Theory]`, CsCheck) in `references/csharp.md`. Use whenever adding or changing unit, integration, snapshot, or property tests in Rust or C#. +description: Design tests that catch bugs, choosing between an example test, parametrized cases, a snapshot, a property, and a fuzz target, in Rust (`rstest`, `expect-test`, `proptest`) or C# (xUnit `[Theory]`, CsCheck). Use whenever adding or changing unit, integration, snapshot, or property tests in Rust or C#. --- # Test writer diff --git a/.agents/skills/test-writer/references/csharp.md b/.agents/skills/test-writer/references/csharp.md index 3fc244f267..e101adca5d 100644 --- a/.agents/skills/test-writer/references/csharp.md +++ b/.agents/skills/test-writer/references/csharp.md @@ -3,13 +3,14 @@ The repository's C# tests are xUnit v2; each rule in `SKILL.md` takes these forms there. Properties use CsCheck: compose `Gen` with LINQ and `Sample` it; keep each type's generator in one static class of the test project so it is written once, as a crate keeps its strategies. +CsCheck rather than FsCheck, because its generators shrink without per-type shrinkers, so a composed generator costs one declaration like a `proptest` strategy, and it adds no FSharp.Core to a C# test project. A failing `Sample` prints a seed; replay it with `seed:` or the `CsCheck_Seed` environment variable, and pin a regression by keeping the shrunk value as an `[InlineData]` row rather than the seed, since a generator change moves the seed's meaning. -No fuzz harness exists for the repository's C#, so a parser of untrusted input gets a CsCheck property over malformed inputs, and the report says it is not fuzzed. +Where the C# code under test has no fuzz harness, a parser of untrusted input gets a CsCheck property over malformed inputs, and the pull request says it is not fuzzed. No snapshot library is in use; write the expected value inline and, past a screen, in a sibling file copied into the test output. Nothing rewrites a stale value, so each is edited by hand after reading it. Cases are `[Theory]` with `[InlineData]` rows for literal data, whose display name carries the arguments, so a failure names the row as `#[case::name]` does. -Other data goes through `[MemberData(nameof())]` with a string key per row, because rows whose arguments xUnit cannot serialize share one test case, which no filter or kill matrix can tell apart. +Other data goes through `[MemberData(nameof())]` whose rows hold only values xUnit can serialize, such as a string key the test resolves to its data, because one row it cannot serialize turns the whole theory into one test case that no filter or mutation report can split. A cross product is a `[MemberData]` member that yields it, so the `#[values]` rule applies to that member. A regression test is a `[Fact]` named for its issue. From 3be625cd4601ca7cde3d30481a7e5ad25ce69802 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Wed, 21 Oct 2026 08:14:45 +0900 Subject: [PATCH 03/18] chore(agents): close gaps in the test-pruner fixes A failing gap test is a finding like a failing strengthened test, and the rule keeps its reason because it is the one most tempting to break. Protected tests are no longer offered for merging, since issue regression tests stay standalone under `test-writer`. The evidence tables are defined where editing mode first names them, Stryker reruns for a diff are measured with `--mutate` from the start, and a challenger's hand-written patch is rebuilt with `git diff --output`. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .agents/skills/test-pruner/SKILL.md | 9 ++++----- .agents/skills/test-pruner/references/domain-mutants.md | 2 +- .agents/skills/test-pruner/references/stryker.md | 5 +++-- .agents/skills/test-writer/references/csharp.md | 2 +- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/.agents/skills/test-pruner/SKILL.md b/.agents/skills/test-pruner/SKILL.md index 86310edff0..d52e6b1675 100644 --- a/.agents/skills/test-pruner/SKILL.md +++ b/.agents/skills/test-pruner/SKILL.md @@ -10,7 +10,7 @@ Decide from two independent measurements: whether the expected value is justifie Mutation testing alone cannot tell a test that matches itself from a correct one, and oracle review alone cannot tell which tests are the sole guard for a fault. Tool mechanics: `references/cargo-mutants.md` for Rust, `references/stryker.md` for C#; load only the language in scope. -Editing mode is for tests you own, such as a change's diff before its pull request opens: commit the changes yourself and hand the evidence tables to whoever posts the pull request comment. +Editing mode is for tests you own, such as a change's diff before its pull request opens: commit the changes yourself and hand the evidence tables (oracle table, kill matrix, challenge results, survivors, and found bugs) to whoever posts them as a pull request comment. Review mode is for another's pull request: post a review and edit nothing. Run mutants in a clone of their own and address its files by absolute path, since a relative path resolves against whichever checkout the shell starts in, and an applied mutant left in a shared checkout corrupts every other run there. @@ -18,7 +18,7 @@ Run mutants in a clone of their own and address its files by absolute path, sinc Invoke `test-oracle-reviewer` on every test in scope and record independent, self-derived, or unknown with evidence. Run the mutation tool as the reference describes, adding domain mutants from `references/domain-mutants.md` because generic mutants miss most domain faults. -Build the kill matrix of mutant by test from the tool's outputs; it needs no per-test reruns. +Build the kill matrix of mutant by test from the tool's outputs; only the cases the reference names need per-test reruns. For tests with several assertions, record which assertion fails for each kill. Run the scoped suite repeatedly on the clean tree, for example 15 times, to find flaky tests before trusting any kill. @@ -27,7 +27,7 @@ Run the scoped suite repeatedly on the clean tree, for example 15 times, to find Keep independent tests; merge them into cases of one parametrized test only when they check the same property and differ only in data. Never delete an independent test on mutant evidence: the mutant sample is tiny next to the real fault space, and a spec vector costs nothing to keep. Strengthen a self-derived test with unique kills by replacing its expected value with an independent one; if none exists, keep it and mark it `// oracle: self-derived; sole guard for `, naming each fault in words, since mutant ids and line numbers change between runs. -A strengthened test that fails is a finding, never a task to make it pass: report the test, the independent expected value and its source, and the code's actual output to whoever owns the decision, and change neither test nor code meanwhile, as `test-oracle-reviewer` requires. +A strengthened or gap test that fails is a finding, never a task to make it pass: report the test, the independent expected value and its source, and the code's actual output to whoever owns the decision, and change neither test nor code meanwhile, since whichever is edited first becomes the specification. Delete a self-derived test with no unique kills. A test of unknown oracle with unique kills has already passed the challenge; strengthen it. Challenge a test of unknown oracle without unique kills, unless it is protected below, since its fate does not depend on the answer. @@ -37,7 +37,7 @@ Two tests that both catch mutant M each have zero unique kills, and deleting bot Choose the deletion set so the caught mutants afterward equal the caught mutants before. The same rule applies to assertions: a self-derived one that never fails across its test's kills is a deletion candidate. -Protected tests are never proposed for deletion, only strengthened or merged: a regression test tied to an issue or fuzz crash, a test of untrusted-input parsing or bounds checks, and a doctest. +Protected tests are never proposed for deletion: a regression test tied to an issue or fuzz crash, a test of untrusted-input parsing or bounds checks, and a doctest. Treat a suite that deliberately uses snapshots as a change detector: dedupe it, do not delete it. Report flaky tests separately; deleting one hides a bug. @@ -65,7 +65,6 @@ Close each coverage gap with a test whose expected value is independent, written A suite of independent tests yields few deletions, so expect most of a pass's change to be consolidations and gap tests. In editing mode, commit deletions, consolidations, strengthenings, and gap tests separately, each as soon as it is verified, so an interrupted run loses little; they are different reviews. -The evidence tables are the oracle table, kill matrix, challenge results, survivors, and found bugs; they go in a pull request comment, since the pull request body is the squash commit. Change only tests and source comments; never modify non-test source otherwise. In review mode, post one review: inline comments on the affected tests, with the tables in `
` blocks. diff --git a/.agents/skills/test-pruner/references/domain-mutants.md b/.agents/skills/test-pruner/references/domain-mutants.md index f2c67759a6..63a6abf1d1 100644 --- a/.agents/skills/test-pruner/references/domain-mutants.md +++ b/.agents/skills/test-pruner/references/domain-mutants.md @@ -1,7 +1,7 @@ # Domain mutants Neither mutation tool takes user-defined operators, so write each domain mutant as a patch. -Take it with `git diff --output=` from an edit you then revert, since a hand-written hunk header is easily wrong and a Windows PowerShell redirect re-encodes the patch as UTF-16 with CRLF, which `git apply` rejects. +Take it, or rebuild a challenger's patch, with `git diff --output=` from an edit you then revert, since a hand-written hunk header is easily wrong and a Windows PowerShell redirect re-encodes the patch as UTF-16 with CRLF, which `git apply` rejects. `git apply` it, run the scoped tests, and `git apply -R` it; caught means a test failed, and a patch that does not build proves nothing until it does. Patches are pull request evidence, not repository files. diff --git a/.agents/skills/test-pruner/references/stryker.md b/.agents/skills/test-pruner/references/stryker.md index 67cde9ded0..4e0a3fa631 100644 --- a/.agents/skills/test-pruner/references/stryker.md +++ b/.agents/skills/test-pruner/references/stryker.md @@ -51,14 +51,15 @@ $report.files.PSObject.Properties | Where-Object Name -like '**' | ForEa A unique kill is a mutant whose `killedBy` has one entry. Read kills only from `killedBy`; `coveredBy` lists the tests that ran against a mutant, not those that failed. -The script lists `Killed` mutants only; attribute a `Timeout` by rerunning its `coveredBy` tests one at a time before counting it caught. +The script lists `Killed` mutants only; attribute a `Timeout` by applying it by hand, as below, and running its `coveredBy` tests one at a time before counting it caught. One row with an argument xUnit cannot serialize makes the whole theory one test id, so the matrix cannot tell its rows apart; `test-writer`'s C# forms avoid this. Stryker keeps no per-test output, so for assertion-level evidence apply the mutant by hand from `location` and `replacement`, run the killing test with `dotnet test --filter "FullyQualifiedName~."`, and read the failing assertion from the stack trace. Mutant ids are assigned in order over the whole project, so an edit anywhere can renumber them, and lines shift with edits above; across runs match mutants by file, `mutatorName`, `replacement`, and the text of the mutated line in `files[].source`. ## Rerun -Refresh the clean run and its TRX before each rerun, and compare only runs scoped by `--mutate` over the files a `--since` run mutated: a test edit changes which mutants `--since` selects, so a lost kill drops out instead of surviving. +Refresh the clean run and its TRX before each rerun. +For a change's diff, measure and rerun with `--mutate` over the files a `--since` run mutated, never comparing `--since` runs: a test edit changes which mutants `--since` selects, so a lost kill drops out instead of surviving. Do not use `--with-baseline`; it reuses results for mutants it considers unchanged, the opposite of a rerun. Identical settings include `mutation-level` and any `test-case-filter`, since either changes the mutant population or the tests that can kill it. diff --git a/.agents/skills/test-writer/references/csharp.md b/.agents/skills/test-writer/references/csharp.md index e101adca5d..7ed62b3176 100644 --- a/.agents/skills/test-writer/references/csharp.md +++ b/.agents/skills/test-writer/references/csharp.md @@ -5,7 +5,7 @@ The repository's C# tests are xUnit v2; each rule in `SKILL.md` takes these form Properties use CsCheck: compose `Gen` with LINQ and `Sample` it; keep each type's generator in one static class of the test project so it is written once, as a crate keeps its strategies. CsCheck rather than FsCheck, because its generators shrink without per-type shrinkers, so a composed generator costs one declaration like a `proptest` strategy, and it adds no FSharp.Core to a C# test project. A failing `Sample` prints a seed; replay it with `seed:` or the `CsCheck_Seed` environment variable, and pin a regression by keeping the shrunk value as an `[InlineData]` row rather than the seed, since a generator change moves the seed's meaning. -Where the C# code under test has no fuzz harness, a parser of untrusted input gets a CsCheck property over malformed inputs, and the pull request says it is not fuzzed. +Where the C# code under test has no fuzz target, a parser of untrusted input gets a CsCheck property over malformed inputs, and a pull request comment says it is not fuzzed. No snapshot library is in use; write the expected value inline and, past a screen, in a sibling file copied into the test output. Nothing rewrites a stale value, so each is edited by hand after reading it. From e065236565e7965bc404f57cf40f1711f04c59f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Wed, 21 Oct 2026 08:43:58 +0900 Subject: [PATCH 04/18] chore(agents): key Stryker kills by test id The Stryker kill matrix now prints each killing test's id beside its display name, since truncated display names collide. Mutants that share file, mutator, replacement, and line text are matched by their order in the file, which test and comment edits leave unchanged. A CsCheck counterexample is pinned in a regression test rather than an `[InlineData]` row, so regression tests stay standalone. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .agents/skills/test-pruner/references/stryker.md | 4 ++-- .agents/skills/test-writer/references/csharp.md | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.agents/skills/test-pruner/references/stryker.md b/.agents/skills/test-pruner/references/stryker.md index 4e0a3fa631..b2de1b07aa 100644 --- a/.agents/skills/test-pruner/references/stryker.md +++ b/.agents/skills/test-pruner/references/stryker.md @@ -44,7 +44,7 @@ $name = @{} foreach ($t in $trx.TestRun.TestDefinitions.UnitTest) { $name[$t.id] = $t.name } $report.files.PSObject.Properties | Where-Object Name -like '**' | ForEach-Object { foreach ($m in $_.Value.mutants | Where-Object status -eq 'Killed') { - foreach ($id in $m.killedBy) { "$($m.id)`tL$($m.location.start.line) $($m.mutatorName): $($m.replacement)`t$($name[$id])" } + foreach ($id in $m.killedBy) { "$($m.id)`tL$($m.location.start.line) $($m.mutatorName): $($m.replacement)`t$id`t$($name[$id])" } } } ``` @@ -54,7 +54,7 @@ Read kills only from `killedBy`; `coveredBy` lists the tests that ran against a The script lists `Killed` mutants only; attribute a `Timeout` by applying it by hand, as below, and running its `coveredBy` tests one at a time before counting it caught. One row with an argument xUnit cannot serialize makes the whole theory one test id, so the matrix cannot tell its rows apart; `test-writer`'s C# forms avoid this. Stryker keeps no per-test output, so for assertion-level evidence apply the mutant by hand from `location` and `replacement`, run the killing test with `dotnet test --filter "FullyQualifiedName~."`, and read the failing assertion from the stack trace. -Mutant ids are assigned in order over the whole project, so an edit anywhere can renumber them, and lines shift with edits above; across runs match mutants by file, `mutatorName`, `replacement`, and the text of the mutated line in `files[].source`. +Mutant ids are assigned in order over the whole project, so an edit anywhere can renumber them, and lines shift with edits above; across runs match mutants by file, `mutatorName`, `replacement`, and the text of the mutated line in `files[].source`, and mutants sharing all four by their order in the file, which test and comment edits leave unchanged. ## Rerun diff --git a/.agents/skills/test-writer/references/csharp.md b/.agents/skills/test-writer/references/csharp.md index 7ed62b3176..29dbf031f6 100644 --- a/.agents/skills/test-writer/references/csharp.md +++ b/.agents/skills/test-writer/references/csharp.md @@ -4,7 +4,7 @@ The repository's C# tests are xUnit v2; each rule in `SKILL.md` takes these form Properties use CsCheck: compose `Gen` with LINQ and `Sample` it; keep each type's generator in one static class of the test project so it is written once, as a crate keeps its strategies. CsCheck rather than FsCheck, because its generators shrink without per-type shrinkers, so a composed generator costs one declaration like a `proptest` strategy, and it adds no FSharp.Core to a C# test project. -A failing `Sample` prints a seed; replay it with `seed:` or the `CsCheck_Seed` environment variable, and pin a regression by keeping the shrunk value as an `[InlineData]` row rather than the seed, since a generator change moves the seed's meaning. +A failing `Sample` prints a seed; replay it with `seed:` or the `CsCheck_Seed` environment variable, and pin the failure by keeping the shrunk value in a regression test rather than the seed, since a generator change moves the seed's meaning. Where the C# code under test has no fuzz target, a parser of untrusted input gets a CsCheck property over malformed inputs, and a pull request comment says it is not fuzzed. No snapshot library is in use; write the expected value inline and, past a screen, in a sibling file copied into the test output. From 645d4988a98e4ba98b25d2c2ffcd082315f8eef7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Wed, 21 Oct 2026 09:14:14 +0900 Subject: [PATCH 05/18] chore(agents): pass the solution for .NET Framework Stryker runs Stryker.NET cannot run on a .NET Framework test project without a solution path, and the Agent installer tests target net48. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .agents/skills/test-pruner/references/stryker.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.agents/skills/test-pruner/references/stryker.md b/.agents/skills/test-pruner/references/stryker.md index b2de1b07aa..31b9c44ed9 100644 --- a/.agents/skills/test-pruner/references/stryker.md +++ b/.agents/skills/test-pruner/references/stryker.md @@ -26,7 +26,7 @@ dotnet stryker --mutate "**//*.cs" --disable-bail --break-on-initial-tes `--break-on-initial-test-failure` stops the run when the clean suite fails, which a test edit can cause, because a matrix built over failing tests proves nothing. For a change's diff, `--since:` selects its mutants; a revision expression such as `HEAD~1` or an abbreviated SHA is rejected. A test-only diff still yields mutants under `--since`: every mutant covered by a test in a changed test file counts as changed. -Pass `--project .csproj` only when the test project references more than one project. +Pass `--project .csproj` only when the test project references more than one project, and `--solution .sln` when it targets .NET Framework, where Stryker requires it. Coverage analysis, `perTest` by default, runs only the tests covering each mutant, so the matrix spans the whole test project without scoping tests and a unique kill is unique across the project; restricting to a test class needs `test-case-filter` in `stryker-config.json`, since the 5.0.0 command line rejects it as a flag. Stryker builds the test project itself and writes a `.gitignore` of `*` into `--output`, so the output stays untracked wherever it is. Build and the initial run take a few minutes before the first mutant, and a whole library of a few thousand lines can run for most of an hour, so scope first; timeouts are computed per mutant from the initial run, and `additional-timeout` and `timeout-ratio` are config-file options. From eab5674039b5702dba9c66f62bf90252c891f7c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Wed, 21 Oct 2026 12:56:21 +0900 Subject: [PATCH 06/18] chore(agents): move Rust test forms into a reference `test-writer` now states each rule once without naming a library, and the Rust forms sit in `references/rust.md` beside the C# ones, so neither language reads the other's forms and each reference stands alone. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .agents/skills/test-writer/SKILL.md | 20 +++++++++---------- .../skills/test-writer/references/csharp.md | 8 ++++---- .agents/skills/test-writer/references/rust.md | 13 ++++++++++++ 3 files changed, 27 insertions(+), 14 deletions(-) create mode 100644 .agents/skills/test-writer/references/rust.md diff --git a/.agents/skills/test-writer/SKILL.md b/.agents/skills/test-writer/SKILL.md index fa70849e34..9d2baaf164 100644 --- a/.agents/skills/test-writer/SKILL.md +++ b/.agents/skills/test-writer/SKILL.md @@ -8,25 +8,25 @@ description: Design tests that catch bugs, choosing between an example test, par First name the bug the test catches that existing tests miss; without one it costs runtime and maintenance and guards nothing. Take the expected value from a source independent of the code under test, as `test-oracle-reviewer` defines; a value derived from the implementation detects change, not bugs. Reach nothing beyond loopback, because an external host makes a failure unreproducible. -The rules below name the Rust forms; for a C# test, read `references/csharp.md` for the form each rule takes there. +Each rule below takes a form per language: `references/rust.md` for Rust, `references/csharp.md` for C#; load only the language in scope. ## Properties Look for a property before writing example tests, because a property holds across generated inputs while examples show only the cases someone thought of. Candidates are a round-trip through encode and decode, a size computation matching the bytes written, and an invariant from the applicable intent or contract file that speaks of inputs or state, such as one that must survive any sequence of operations. -Test a property with `proptest`, reusing the crate's existing strategies and adding new ones beside them, so each type's generator is written once. -With no property that must hold across many inputs, write example tests; a `proptest` whose assertion re-implements the code under test checks the code against itself. -A parser of untrusted input gets a fuzz target in `fuzz/` rather than a never-panics `proptest`, because the fuzzer is coverage-guided and keeps a corpus, which a few hundred random cases are not. +Reuse the existing generator for each type and add new ones beside it, so each type's generator is written once. +With no property that must hold across many inputs, write example tests; a property whose assertion re-implements the code under test checks the code against itself. +A parser of untrusted input gets a fuzz target rather than a never-panics property, because the fuzzer is coverage-guided and keeps a corpus, which a few hundred random cases are not. ## Snapshots -Snapshot structured values such as a parsed config, a decoded message, or a CLI error with `expect-test`; the expected value sits beside the assertion and `UPDATE_EXPECT=1` rewrites every stale one after a representation change. -Taking or updating a snapshot is the oracle, so read and check it, because `UPDATE_EXPECT=1` accepts whatever the code produced; when a spec or capture defines the value, assert that instead. -Move a snapshot or blob longer than a screen into a sibling file through `expect_file!` or `include_bytes!`, so the source shows the test rather than its data; keep short byte vectors inline so each field can carry its comment. +Snapshot structured values such as a parsed config, a decoded message, or a CLI error, with the expected value beside the assertion. +Taking or updating a snapshot is the oracle, so read and check it, because its value is whatever the code produced; when a spec or capture defines the value, assert that instead. +Move a snapshot or blob longer than a screen into a sibling file, so the source shows the test rather than its data; keep short byte vectors inline so each field can carry its comment. ## Examples and cases -Parametrize with `rstest` only when every case checks the same behavior with different data, and name each case with `#[case::name]` so a failure names the case, not an index. -A body that branches on its parameters, through an `if`, a `match`, or an optional expected value, is several tests behind one name, so a failure reports a case instead of the behavior that broke; write one test per behavior and share setup through a helper. -A `#[values]` cross product is right only when every combination is a real scenario; one that needs an `if` to skip or adjust a combination is that branching. +Parametrize only when every case checks the same behavior with different data, and name each case so a failure names the case, not an index. +A body that branches on its parameters, through a conditional or an optional expected value, is several tests behind one name, so a failure reports a case instead of the behavior that broke; write one test per behavior and share setup through a helper. +A cross product of parameters is right only when every combination is a real scenario; one that needs a conditional to skip or adjust a combination is that branching. Keep each regression test standalone and named for its issue, because its setup reproduces the bug's exact conditions and a case table would generalize them away. diff --git a/.agents/skills/test-writer/references/csharp.md b/.agents/skills/test-writer/references/csharp.md index 29dbf031f6..e8d5eb6cf9 100644 --- a/.agents/skills/test-writer/references/csharp.md +++ b/.agents/skills/test-writer/references/csharp.md @@ -2,15 +2,15 @@ The repository's C# tests are xUnit v2; each rule in `SKILL.md` takes these forms there. -Properties use CsCheck: compose `Gen` with LINQ and `Sample` it; keep each type's generator in one static class of the test project so it is written once, as a crate keeps its strategies. -CsCheck rather than FsCheck, because its generators shrink without per-type shrinkers, so a composed generator costs one declaration like a `proptest` strategy, and it adds no FSharp.Core to a C# test project. +Properties use CsCheck: compose `Gen` with LINQ and `Sample` it; each type's generator lives in one static class of the test project. +CsCheck rather than FsCheck, because its generators shrink without per-type shrinkers, so a composed generator costs one declaration, and it adds no FSharp.Core to a C# test project. A failing `Sample` prints a seed; replay it with `seed:` or the `CsCheck_Seed` environment variable, and pin the failure by keeping the shrunk value in a regression test rather than the seed, since a generator change moves the seed's meaning. Where the C# code under test has no fuzz target, a parser of untrusted input gets a CsCheck property over malformed inputs, and a pull request comment says it is not fuzzed. No snapshot library is in use; write the expected value inline and, past a screen, in a sibling file copied into the test output. Nothing rewrites a stale value, so each is edited by hand after reading it. -Cases are `[Theory]` with `[InlineData]` rows for literal data, whose display name carries the arguments, so a failure names the row as `#[case::name]` does. +Cases are `[Theory]` with `[InlineData]` rows for literal data, whose display name carries the arguments, so a failure names the row. Other data goes through `[MemberData(nameof())]` whose rows hold only values xUnit can serialize, such as a string key the test resolves to its data, because one row it cannot serialize turns the whole theory into one test case that no filter or mutation report can split. -A cross product is a `[MemberData]` member that yields it, so the `#[values]` rule applies to that member. +A cross product is a `[MemberData]` member that yields it. A regression test is a `[Fact]` named for its issue. diff --git a/.agents/skills/test-writer/references/rust.md b/.agents/skills/test-writer/references/rust.md new file mode 100644 index 0000000000..bbab8a80d6 --- /dev/null +++ b/.agents/skills/test-writer/references/rust.md @@ -0,0 +1,13 @@ +# Rust forms + +Each rule in `SKILL.md` takes these forms in Rust. + +Properties use `proptest`; the crate's existing strategies are the generators to reuse. +Fuzz targets live in `fuzz/`. + +Snapshots use `expect-test`, whose `UPDATE_EXPECT=1` rewrites every stale value after a representation change. +Past a screen, a snapshot moves to a sibling file through `expect_file!`, and a blob through `include_bytes!`. + +Cases are `rstest`, each named with `#[case::name]`. +A cross product is enum-valued `#[values]`, so each combination's name says what it checks. +A regression test is its own test function, not an `rstest` case, named for its issue. From 7e593a56bbe1a4fc41b1ac59e35d19cb36681ab5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Wed, 21 Oct 2026 13:06:05 +0900 Subject: [PATCH 07/18] chore(agents): pin CsCheck for .NET Framework test projects CsCheck releases after 2.14.1 need .NET 6 or later, and the Agent installer tests target net48. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .agents/skills/test-writer/references/csharp.md | 1 + 1 file changed, 1 insertion(+) diff --git a/.agents/skills/test-writer/references/csharp.md b/.agents/skills/test-writer/references/csharp.md index e8d5eb6cf9..475d324533 100644 --- a/.agents/skills/test-writer/references/csharp.md +++ b/.agents/skills/test-writer/references/csharp.md @@ -4,6 +4,7 @@ The repository's C# tests are xUnit v2; each rule in `SKILL.md` takes these form Properties use CsCheck: compose `Gen` with LINQ and `Sample` it; each type's generator lives in one static class of the test project. CsCheck rather than FsCheck, because its generators shrink without per-type shrinkers, so a composed generator costs one declaration, and it adds no FSharp.Core to a C# test project. +Releases after CsCheck 2.14.1 need .NET 6 or later, so a .NET Framework test project pins 2.14.1, the last on .NET Standard 2.0. A failing `Sample` prints a seed; replay it with `seed:` or the `CsCheck_Seed` environment variable, and pin the failure by keeping the shrunk value in a regression test rather than the seed, since a generator change moves the seed's meaning. Where the C# code under test has no fuzz target, a parser of untrusted input gets a CsCheck property over malformed inputs, and a pull request comment says it is not fuzzed. From 814d1ef3ae2ca86463567d0c5f7c5c80318f9631 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Wed, 21 Oct 2026 13:13:26 +0900 Subject: [PATCH 08/18] chore(agents): guard C# pruning against an empty mutant sample The rule against deciding deletions on an empty or near-empty viable sample applies to every mutation tool, so it moves from the cargo-mutants reference into the shared measurement step. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .agents/skills/test-pruner/SKILL.md | 1 + .agents/skills/test-pruner/references/cargo-mutants.md | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/.agents/skills/test-pruner/SKILL.md b/.agents/skills/test-pruner/SKILL.md index d52e6b1675..783c212c43 100644 --- a/.agents/skills/test-pruner/SKILL.md +++ b/.agents/skills/test-pruner/SKILL.md @@ -19,6 +19,7 @@ Run mutants in a clone of their own and address its files by absolute path, sinc Invoke `test-oracle-reviewer` on every test in scope and record independent, self-derived, or unknown with evidence. Run the mutation tool as the reference describes, adding domain mutants from `references/domain-mutants.md` because generic mutants miss most domain faults. Build the kill matrix of mutant by test from the tool's outputs; only the cases the reference names need per-test reruns. +Make no deletion decision on an empty or near-empty viable sample; with nothing to catch, every test has zero unique kills and the invariant passes trivially. For tests with several assertions, record which assertion fails for each kill. Run the scoped suite repeatedly on the clean tree, for example 15 times, to find flaky tests before trusting any kill. diff --git a/.agents/skills/test-pruner/references/cargo-mutants.md b/.agents/skills/test-pruner/references/cargo-mutants.md index 72beed53a8..ef586ca583 100644 --- a/.agents/skills/test-pruner/references/cargo-mutants.md +++ b/.agents/skills/test-pruner/references/cargo-mutants.md @@ -8,7 +8,6 @@ These notes target cargo-mutants 27.1. Scope with `--file '/src//**'` for a crate or module, `--re '|'` for a pull request's changed functions, or `--in-diff ` for its whole diff; build time dominates, so scope first. A test-only diff yields no mutants under `--in-diff`, so scope with `--re` to the production functions the changed tests call, including unchanged ones. -Make no deletion decision on an empty or near-empty viable sample; with nothing to catch, every test has zero unique kills and the invariant passes trivially. Package directories vary; take the path from `cargo metadata` rather than assuming `crates/`. ```sh From 3a69f31d7045c38d77b20fe3d1b92b4245459d00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Wed, 21 Oct 2026 13:53:46 +0900 Subject: [PATCH 09/18] chore(agents): settle test-pruner modes, guards, and triage Review mode changes nothing in the reviewed branch but still applies mutants in its own clone, and a failing strengthened or gap test goes to whoever assigned the run, or to the author in review mode. Deletions wait until some mutant of the exercised code is caught, because zero unique kills over nothing caught measures nothing however many mutants survived. Survivor classes are defined by what an independent requirement says about the change, after scoping errors and tool artifacts are ruled out, and challenger mutants are proposed for the catalog separately because neither mode edits skills. `code-review` routes C# test changes to `test-reviewer`, the fuzz rule defers to the language reference where no harness exists, and the Stryker notes confirm the clean run discovered tests. Restated definitions and discoverable lines are dropped from the references. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .agents/skills/test-pruner/SKILL.md | 15 ++++++++------- .../test-pruner/references/cargo-mutants.md | 4 +--- .agents/skills/test-pruner/references/stryker.md | 3 ++- .agents/skills/test-writer/SKILL.md | 2 +- .agents/skills/test-writer/references/csharp.md | 8 ++++---- .agents/skills/test-writer/references/rust.md | 7 +++---- .github/skills/code-review/SKILL.md | 2 +- 7 files changed, 20 insertions(+), 21 deletions(-) diff --git a/.agents/skills/test-pruner/SKILL.md b/.agents/skills/test-pruner/SKILL.md index 783c212c43..cbefd7cc14 100644 --- a/.agents/skills/test-pruner/SKILL.md +++ b/.agents/skills/test-pruner/SKILL.md @@ -11,7 +11,7 @@ Mutation testing alone cannot tell a test that matches itself from a correct one Tool mechanics: `references/cargo-mutants.md` for Rust, `references/stryker.md` for C#; load only the language in scope. Editing mode is for tests you own, such as a change's diff before its pull request opens: commit the changes yourself and hand the evidence tables (oracle table, kill matrix, challenge results, survivors, and found bugs) to whoever posts them as a pull request comment. -Review mode is for another's pull request: post a review and edit nothing. +Review mode is for another's pull request: post a review and change nothing in its branch, applying mutants and challenge patches only in the clone below. Run mutants in a clone of their own and address its files by absolute path, since a relative path resolves against whichever checkout the shell starts in, and an applied mutant left in a shared checkout corrupts every other run there. ## Measure @@ -19,7 +19,8 @@ Run mutants in a clone of their own and address its files by absolute path, sinc Invoke `test-oracle-reviewer` on every test in scope and record independent, self-derived, or unknown with evidence. Run the mutation tool as the reference describes, adding domain mutants from `references/domain-mutants.md` because generic mutants miss most domain faults. Build the kill matrix of mutant by test from the tool's outputs; only the cases the reference names need per-test reruns. -Make no deletion decision on an empty or near-empty viable sample; with nothing to catch, every test has zero unique kills and the invariant passes trivially. +A unique kill is a mutant caught by exactly one test in scope. +Make no deletion decision about a test until some mutant of the code it exercises is caught, by it or another test, since zero unique kills over nothing caught measures nothing and the invariant then passes trivially; challenge such a test instead. For tests with several assertions, record which assertion fails for each kill. Run the scoped suite repeatedly on the clean tree, for example 15 times, to find flaky tests before trusting any kill. @@ -28,7 +29,7 @@ Run the scoped suite repeatedly on the clean tree, for example 15 times, to find Keep independent tests; merge them into cases of one parametrized test only when they check the same property and differ only in data. Never delete an independent test on mutant evidence: the mutant sample is tiny next to the real fault space, and a spec vector costs nothing to keep. Strengthen a self-derived test with unique kills by replacing its expected value with an independent one; if none exists, keep it and mark it `// oracle: self-derived; sole guard for `, naming each fault in words, since mutant ids and line numbers change between runs. -A strengthened or gap test that fails is a finding, never a task to make it pass: report the test, the independent expected value and its source, and the code's actual output to whoever owns the decision, and change neither test nor code meanwhile, since whichever is edited first becomes the specification. +A strengthened or gap test that fails is a finding, never a task to make it pass: report the test, the independent expected value and its source, and the code's actual output to whoever assigned the run in editing mode, or to the author in review mode, and change neither test nor code meanwhile, since whichever is edited first becomes the specification. Delete a self-derived test with no unique kills. A test of unknown oracle with unique kills has already passed the challenge; strengthen it. Challenge a test of unknown oracle without unique kills, unless it is protected below, since its fate does not depend on the answer. @@ -46,7 +47,7 @@ Report flaky tests separately; deleting one hides a bug. Give a fresh-context agent the test, the code under test, and the mutants the rest of the suite already catches, and ask for one mutant that this test catches and the others miss, as a patch only. Apply and run the patch yourself; arguments do not count. -Add any mutant it finds to the domain catalog regardless of the verdict. +Record any mutant it finds in the evidence regardless of the verdict, and propose a reusable one for `references/domain-mutants.md` in a separate pull request, since neither mode edits skills. If it finds none, ask the human who wrote or prompted the test for the source of its expected value, and accept a source you can check, never a confirmation. Without one, delete the test. @@ -61,9 +62,9 @@ Do not add a never-panics or never-throws property where a fuzz target already c The caught-mutant set never shrinks except through deletions a human approves in a pull request. Rerun the caught set after every deletion set and every consolidation, verify by rerunning rather than reasoning, and revert on any loss; compare runs made with identical tool settings over the same scope. -Triage surviving mutants as equivalent, unconstrained (it changes only text that no requirement pins, such as a log or error message), coverage gap, or bug lead; a mutant that survives because its code is unreachable is a bug lead. -Close each coverage gap with a test whose expected value is independent, written with `test-writer`; a gap no independent source decides is reported instead. -A suite of independent tests yields few deletions, so expect most of a pass's change to be consolidations and gap tests. +After ruling out the scoping errors and tool artifacts the reference names, triage each survivor as equivalent (no observable change), unconstrained (an observable change no independent requirement decides, such as a log message's wording), coverage gap (a change an independent requirement rejects), or bug lead (the original code looks wrong, or is unreachable in every supported configuration). +Close each coverage gap with a test whose expected value comes from that requirement, written with `test-writer`. +A suite of independent tests yields few deletions; a pass may end in consolidations and gap tests only, or in no change. In editing mode, commit deletions, consolidations, strengthenings, and gap tests separately, each as soon as it is verified, so an interrupted run loses little; they are different reviews. Change only tests and source comments; never modify non-test source otherwise. diff --git a/.agents/skills/test-pruner/references/cargo-mutants.md b/.agents/skills/test-pruner/references/cargo-mutants.md index ef586ca583..bde8e62eb3 100644 --- a/.agents/skills/test-pruner/references/cargo-mutants.md +++ b/.agents/skills/test-pruner/references/cargo-mutants.md @@ -1,7 +1,6 @@ # cargo-mutants mechanics -cargo-mutants reports whether the suite caught each mutant. -It keeps per-test results only in its logs, so the kill matrix is parsed from them. +cargo-mutants keeps per-test results only in its logs, so the kill matrix is parsed from them. These notes target cargo-mutants 27.1. ## Run @@ -55,7 +54,6 @@ $expected = Select-String -Path -Pattern '^test (\S+)(?: - shoul } ``` -A unique kill is a mutant caught by exactly one test in scope. For assertion-level evidence, read the `panicked at :` location of each failure; an assertion whose line never appears across its test's kills catches nothing the others miss. ## Rerun diff --git a/.agents/skills/test-pruner/references/stryker.md b/.agents/skills/test-pruner/references/stryker.md index 31b9c44ed9..13bb6ef2c3 100644 --- a/.agents/skills/test-pruner/references/stryker.md +++ b/.agents/skills/test-pruner/references/stryker.md @@ -15,6 +15,8 @@ Run the clean suite first with a TRX log, because the report names tests by id a dotnet test .csproj --logger "trx;LogFileName=clean.trx" --results-directory /trx ``` +Confirm the TRX lists the tests you expect before trusting any result, because `dotnet test` passes when it discovers none, as with a wrong project or filter. + Then, from the test project directory: ```powershell @@ -49,7 +51,6 @@ $report.files.PSObject.Properties | Where-Object Name -like '**' | ForEa } ``` -A unique kill is a mutant whose `killedBy` has one entry. Read kills only from `killedBy`; `coveredBy` lists the tests that ran against a mutant, not those that failed. The script lists `Killed` mutants only; attribute a `Timeout` by applying it by hand, as below, and running its `coveredBy` tests one at a time before counting it caught. One row with an argument xUnit cannot serialize makes the whole theory one test id, so the matrix cannot tell its rows apart; `test-writer`'s C# forms avoid this. diff --git a/.agents/skills/test-writer/SKILL.md b/.agents/skills/test-writer/SKILL.md index 9d2baaf164..32f6852f40 100644 --- a/.agents/skills/test-writer/SKILL.md +++ b/.agents/skills/test-writer/SKILL.md @@ -16,7 +16,7 @@ Look for a property before writing example tests, because a property holds acros Candidates are a round-trip through encode and decode, a size computation matching the bytes written, and an invariant from the applicable intent or contract file that speaks of inputs or state, such as one that must survive any sequence of operations. Reuse the existing generator for each type and add new ones beside it, so each type's generator is written once. With no property that must hold across many inputs, write example tests; a property whose assertion re-implements the code under test checks the code against itself. -A parser of untrusted input gets a fuzz target rather than a never-panics property, because the fuzzer is coverage-guided and keeps a corpus, which a few hundred random cases are not. +A parser of untrusted input gets a fuzz target rather than a never-panics property, because the fuzzer is coverage-guided and keeps a corpus, which a few hundred random cases are not; where no fuzz harness covers the code, the language reference gives the fallback. ## Snapshots diff --git a/.agents/skills/test-writer/references/csharp.md b/.agents/skills/test-writer/references/csharp.md index 475d324533..fe2bea4db8 100644 --- a/.agents/skills/test-writer/references/csharp.md +++ b/.agents/skills/test-writer/references/csharp.md @@ -6,12 +6,12 @@ Properties use CsCheck: compose `Gen` with LINQ and `Sample` it; each type's CsCheck rather than FsCheck, because its generators shrink without per-type shrinkers, so a composed generator costs one declaration, and it adds no FSharp.Core to a C# test project. Releases after CsCheck 2.14.1 need .NET 6 or later, so a .NET Framework test project pins 2.14.1, the last on .NET Standard 2.0. A failing `Sample` prints a seed; replay it with `seed:` or the `CsCheck_Seed` environment variable, and pin the failure by keeping the shrunk value in a regression test rather than the seed, since a generator change moves the seed's meaning. -Where the C# code under test has no fuzz target, a parser of untrusted input gets a CsCheck property over malformed inputs, and a pull request comment says it is not fuzzed. +Where no fuzz harness covers the C# code under test, a parser of untrusted input gets a CsCheck property over malformed inputs, and a pull request comment says it is not fuzzed. -No snapshot library is in use; write the expected value inline and, past a screen, in a sibling file copied into the test output. -Nothing rewrites a stale value, so each is edited by hand after reading it. +Snapshots are written by hand: the expected value inline or, past a screen, in a sibling file copied into the test output. +Nothing rewrites a stale value, so edit each by hand after reading it. Cases are `[Theory]` with `[InlineData]` rows for literal data, whose display name carries the arguments, so a failure names the row. Other data goes through `[MemberData(nameof())]` whose rows hold only values xUnit can serialize, such as a string key the test resolves to its data, because one row it cannot serialize turns the whole theory into one test case that no filter or mutation report can split. A cross product is a `[MemberData]` member that yields it. -A regression test is a `[Fact]` named for its issue. +A regression test is a separate `[Fact]`. diff --git a/.agents/skills/test-writer/references/rust.md b/.agents/skills/test-writer/references/rust.md index bbab8a80d6..c10b870305 100644 --- a/.agents/skills/test-writer/references/rust.md +++ b/.agents/skills/test-writer/references/rust.md @@ -2,12 +2,11 @@ Each rule in `SKILL.md` takes these forms in Rust. -Properties use `proptest`; the crate's existing strategies are the generators to reuse. -Fuzz targets live in `fuzz/`. +Properties use `proptest`, whose generators are the crate's strategies. Snapshots use `expect-test`, whose `UPDATE_EXPECT=1` rewrites every stale value after a representation change. -Past a screen, a snapshot moves to a sibling file through `expect_file!`, and a blob through `include_bytes!`. +Use `expect_file!` for a snapshot in a sibling file and `include_bytes!` for a blob. Cases are `rstest`, each named with `#[case::name]`. A cross product is enum-valued `#[values]`, so each combination's name says what it checks. -A regression test is its own test function, not an `rstest` case, named for its issue. +A regression test is not an `rstest` case. diff --git a/.github/skills/code-review/SKILL.md b/.github/skills/code-review/SKILL.md index 378b76ceb2..1f3234b23d 100644 --- a/.github/skills/code-review/SKILL.md +++ b/.github/skills/code-review/SKILL.md @@ -13,7 +13,7 @@ Inspect the diff, its stated goal, and repository guidance before selecting cond - Run `contract-review` when the diff changes a `CONTRACT.md`. - Run `code-compressor` for every diff, limiting it to changed code. - Run `rust-style-reviewer` when Rust code changes, limiting it to changed lines. -- Run `test-reviewer` when Rust tests change; it runs `test-oracle-reviewer` itself, so do not add that pass separately. +- Run `test-reviewer` when Rust or C# tests change; it runs `test-oracle-reviewer` itself, so do not add that pass separately. - Run `docs-compressor` when documentation, rustdoc, explanatory comments, or instructional prose changes. - Run `prose-verifier` when hand-maintained prose changes in a format where source line breaks are stylistic. - Run `skeptical-reviewer` last for every diff. From 852cffecf527cef3e21120dc875d2dd992984056 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Wed, 21 Oct 2026 14:05:08 +0900 Subject: [PATCH 10/18] chore(agents): state the fuzz fallback once for every language A parser with no fuzz harness to add a target to falls back to a property over malformed inputs in any language, so the rule sits in the `test-writer` body instead of only in the C# reference, and a Rust reader is not sent to a reference with nothing for it. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .agents/skills/test-writer/SKILL.md | 2 +- .agents/skills/test-writer/references/csharp.md | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/.agents/skills/test-writer/SKILL.md b/.agents/skills/test-writer/SKILL.md index 32f6852f40..74b31f03f1 100644 --- a/.agents/skills/test-writer/SKILL.md +++ b/.agents/skills/test-writer/SKILL.md @@ -16,7 +16,7 @@ Look for a property before writing example tests, because a property holds acros Candidates are a round-trip through encode and decode, a size computation matching the bytes written, and an invariant from the applicable intent or contract file that speaks of inputs or state, such as one that must survive any sequence of operations. Reuse the existing generator for each type and add new ones beside it, so each type's generator is written once. With no property that must hold across many inputs, write example tests; a property whose assertion re-implements the code under test checks the code against itself. -A parser of untrusted input gets a fuzz target rather than a never-panics property, because the fuzzer is coverage-guided and keeps a corpus, which a few hundred random cases are not; where no fuzz harness covers the code, the language reference gives the fallback. +A parser of untrusted input gets a fuzz target rather than a never-panics property, because the fuzzer is coverage-guided and keeps a corpus, which a few hundred random cases are not; where no fuzz harness exists to add one to, a property over malformed inputs stands in, and a pull request comment says the parser is not fuzzed. ## Snapshots diff --git a/.agents/skills/test-writer/references/csharp.md b/.agents/skills/test-writer/references/csharp.md index fe2bea4db8..3d19522d8a 100644 --- a/.agents/skills/test-writer/references/csharp.md +++ b/.agents/skills/test-writer/references/csharp.md @@ -6,7 +6,6 @@ Properties use CsCheck: compose `Gen` with LINQ and `Sample` it; each type's CsCheck rather than FsCheck, because its generators shrink without per-type shrinkers, so a composed generator costs one declaration, and it adds no FSharp.Core to a C# test project. Releases after CsCheck 2.14.1 need .NET 6 or later, so a .NET Framework test project pins 2.14.1, the last on .NET Standard 2.0. A failing `Sample` prints a seed; replay it with `seed:` or the `CsCheck_Seed` environment variable, and pin the failure by keeping the shrunk value in a regression test rather than the seed, since a generator change moves the seed's meaning. -Where no fuzz harness covers the C# code under test, a parser of untrusted input gets a CsCheck property over malformed inputs, and a pull request comment says it is not fuzzed. Snapshots are written by hand: the expected value inline or, past a screen, in a sibling file copied into the test output. Nothing rewrites a stale value, so edit each by hand after reading it. From 2ce6efb275e88afcaed41e56b69cf0e639aeb3f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Wed, 21 Oct 2026 16:11:18 +0900 Subject: [PATCH 11/18] chore(agents): mark where each test's expected value comes from Coverage driven by mutants rewards tests that pin how the code works today, and nothing in a test said whether a source still required it. The test skills now have each test, case, or assertion carry an `// oracle:` marker naming its source. Spec, contract, intent, regression, reference, and property markers are strong; `mutant` and `scaffold` markers are weak, and any intentional change may delete what they mark without justification, except tests `test-pruner` protects. Tests sit at the seam and assert only what their source decides, and one that fails while the code still meets its source is narrowed or deleted. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .agents/skills/test-oracle-reviewer/SKILL.md | 31 +++++++++++++++++-- .agents/skills/test-pruner/SKILL.md | 8 +++-- .agents/skills/test-writer/SKILL.md | 7 +++-- .agents/skills/test-writer/references/rust.md | 2 ++ 4 files changed, 40 insertions(+), 8 deletions(-) diff --git a/.agents/skills/test-oracle-reviewer/SKILL.md b/.agents/skills/test-oracle-reviewer/SKILL.md index 0072dd7659..0068ef661c 100644 --- a/.agents/skills/test-oracle-reviewer/SKILL.md +++ b/.agents/skills/test-oracle-reviewer/SKILL.md @@ -1,6 +1,6 @@ --- name: test-oracle-reviewer -description: Judge whether a test's expected value is justified independently of the code under test. Use when reviewing tests added or changed in a pull request, auditing a module's tests, or whenever expected bytes, snapshots, or mock values may have been copied from the implementation's own output. +description: Judge whether a test's expected value is justified independently of the code under test, and define the `// oracle:` marker that records its source. Use when reviewing tests added or changed in a pull request, auditing a module's tests, marking a test's source, or whenever expected bytes, snapshots, or mock values may have been copied from the implementation's own output. --- # Test oracle reviewer @@ -17,10 +17,35 @@ A value pasted from the code's output is independent once it checks out against A search-engine or AI summary is not a source; cite the primary document. Classify each test as independent, self-derived, or unknown, with evidence, and report per assertion when a test mixes classes. -When a value checks out, add a one-line citation comment to the test, such as `// §
example` or `// hand-derived; see PR #N`, so the next review is cheap. +When a value checks out, add its marker, as below, so the next review is cheap. When a value is wrong, report a bug; do not adjust the value or the code. Unknown is an outcome, not a guess in either direction. -In a pull request review, comment on each self-derived test with the independent source to use and leave the fix to the author. +## Markers + +Mark where an expected value comes from with `// oracle: ` on the test, on a case row, or on one assertion when a test mixes sources, so one search finds every test that rests on a source. +Strong kinds name an independent source, and a test, case, or assertion marked with one is deleted only when its source changes or is shown not to decide what it asserts: + +- `spec §
`: normative text other than a contract or intent file, such as an RFC, a Microsoft open specification, or `docs/JMUX-spec.md`. +- `contract §
`: a `CONTRACT.md`. +- `intent §
`: a statement in an intent file. +- `regression `: a bug report or fuzz crash. +- `reference `: what another implementation or a reference model does, recorded or compared live. +- `property `: a property no document states, such as a round-trip or an input the test chose being honored. + +A hand-derived value or a checked snapshot takes the kind of the source it was checked against. + +Weak kinds name a self-derived source, which detects change rather than bugs, so any intentional change, such as a refactor or a new feature, may delete a test, case, or assertion marked with one without justification unless `test-pruner` protects it: + +- `mutant `: a test or assertion kept to catch mutants that no strong source decides. +- `scaffold`: a test or assertion written for feedback while implementing that no strong source decides. + +Mark a strong source whenever one decides the value, so a test found through a mutant but decided by a spec is `spec`, because whether a test may go depends on whether a source still requires it, not on how it was found. +An unmarked test starts as unknown: verify it as above, and turn a citation in another form into the marker it supports. +A marker is a claim: verify it like any other origin. + +## Review + +In a pull request review, comment on each self-derived test that an independent source decides, naming that source, and on each changed test without a marker; leave the fix to the author. When that source needs a small non-test change, such as an injectable deadline the test can choose, request the change. For mutation evidence on the same pull request, use `test-pruner` in review mode. diff --git a/.agents/skills/test-pruner/SKILL.md b/.agents/skills/test-pruner/SKILL.md index cbefd7cc14..c74d4ebb5a 100644 --- a/.agents/skills/test-pruner/SKILL.md +++ b/.agents/skills/test-pruner/SKILL.md @@ -28,11 +28,12 @@ Run the scoped suite repeatedly on the clean tree, for example 15 times, to find Keep independent tests; merge them into cases of one parametrized test only when they check the same property and differ only in data. Never delete an independent test on mutant evidence: the mutant sample is tiny next to the real fault space, and a spec vector costs nothing to keep. -Strengthen a self-derived test with unique kills by replacing its expected value with an independent one; if none exists, keep it and mark it `// oracle: self-derived; sole guard for `, naming each fault in words, since mutant ids and line numbers change between runs. +Strengthen a self-derived test with unique kills by replacing its expected value with an independent one; if none exists, keep it with a `mutant` marker, as `test-oracle-reviewer` defines, naming each fault it alone catches in words, since mutant ids and line numbers change between runs. A strengthened or gap test that fails is a finding, never a task to make it pass: report the test, the independent expected value and its source, and the code's actual output to whoever assigned the run in editing mode, or to the author in review mode, and change neither test nor code meanwhile, since whichever is edited first becomes the specification. Delete a self-derived test with no unique kills. A test of unknown oracle with unique kills has already passed the challenge; strengthen it. Challenge a test of unknown oracle without unique kills, unless it is protected below, since its fate does not depend on the answer. +No `scaffold` marker outlives the pass: these rules replace it with the verified source or `mutant`, or delete what it marks, and a protected test they leave loses the marker and keeps its classification. Deletion is a set decision, not a per-test verdict. Two tests that both catch mutant M each have zero unique kills, and deleting both loses M. @@ -40,7 +41,8 @@ Choose the deletion set so the caught mutants afterward equal the caught mutants The same rule applies to assertions: a self-derived one that never fails across its test's kills is a deletion candidate. Protected tests are never proposed for deletion: a regression test tied to an issue or fuzz crash, a test of untrusted-input parsing or bounds checks, and a doctest. -Treat a suite that deliberately uses snapshots as a change detector: dedupe it, do not delete it. +That holds whatever their marker, because a refactor of a parser is where a dropped bounds check goes unnoticed. +Treat a suite that deliberately uses snapshots as a change detector as protected: dedupe it, do not delete it. Report flaky tests separately; deleting one hides a bug. ## Challenge @@ -63,7 +65,7 @@ Do not add a never-panics or never-throws property where a fuzz target already c The caught-mutant set never shrinks except through deletions a human approves in a pull request. Rerun the caught set after every deletion set and every consolidation, verify by rerunning rather than reasoning, and revert on any loss; compare runs made with identical tool settings over the same scope. After ruling out the scoping errors and tool artifacts the reference names, triage each survivor as equivalent (no observable change), unconstrained (an observable change no independent requirement decides, such as a log message's wording), coverage gap (a change an independent requirement rejects), or bug lead (the original code looks wrong, or is unreachable in every supported configuration). -Close each coverage gap with a test whose expected value comes from that requirement, written with `test-writer`. +Close each coverage gap with a test at the seam `tdd` defines, marked with the requirement that rejects the change and written with `test-writer`; a survivor only a test of internals could catch is not a gap but equivalent or a bug lead, as above. A suite of independent tests yields few deletions; a pass may end in consolidations and gap tests only, or in no change. In editing mode, commit deletions, consolidations, strengthenings, and gap tests separately, each as soon as it is verified, so an interrupted run loses little; they are different reviews. diff --git a/.agents/skills/test-writer/SKILL.md b/.agents/skills/test-writer/SKILL.md index 74b31f03f1..5d068d2f88 100644 --- a/.agents/skills/test-writer/SKILL.md +++ b/.agents/skills/test-writer/SKILL.md @@ -7,8 +7,11 @@ description: Design tests that catch bugs, choosing between an example test, par First name the bug the test catches that existing tests miss; without one it costs runtime and maintenance and guards nothing. Take the expected value from a source independent of the code under test, as `test-oracle-reviewer` defines; a value derived from the implementation detects change, not bugs. +Mark that source as `test-oracle-reviewer` defines, and assert only what it decides, because an assertion past it pins how the code works today; add one only for feedback while implementing, marked `scaffold`, and never in place of a decision `tdd` says to ask for. +Test at the seam `tdd` defines, the public boundary where the behavior is observed. +A test that fails while the code still does what its marked source requires asserts past that source: narrow it to what the source decides, or delete it unless `test-pruner` protects it; a snapshot that fails only on representation is re-read and updated instead. Reach nothing beyond loopback, because an external host makes a failure unreproducible. -Each rule below takes a form per language: `references/rust.md` for Rust, `references/csharp.md` for C#; load only the language in scope. +Each rule here takes a form per language: `references/rust.md` for Rust, `references/csharp.md` for C#; load only the language in scope. ## Properties @@ -20,7 +23,7 @@ A parser of untrusted input gets a fuzz target rather than a never-panics proper ## Snapshots -Snapshot structured values such as a parsed config, a decoded message, or a CLI error, with the expected value beside the assertion. +Snapshot structured values such as a parsed config or a decoded message, with the expected value beside the assertion. Taking or updating a snapshot is the oracle, so read and check it, because its value is whatever the code produced; when a spec or capture defines the value, assert that instead. Move a snapshot or blob longer than a screen into a sibling file, so the source shows the test rather than its data; keep short byte vectors inline so each field can carry its comment. diff --git a/.agents/skills/test-writer/references/rust.md b/.agents/skills/test-writer/references/rust.md index c10b870305..d259c41880 100644 --- a/.agents/skills/test-writer/references/rust.md +++ b/.agents/skills/test-writer/references/rust.md @@ -2,6 +2,8 @@ Each rule in `SKILL.md` takes these forms in Rust. +In a doctest, write the marker as a hidden line, `# // oracle: ...`, so it stays out of the rendered documentation. + Properties use `proptest`, whose generators are the crate's strategies. Snapshots use `expect-test`, whose `UPDATE_EXPECT=1` rewrites every stale value after a representation change. From 34a68c9919390fba0ec7def371196f5e692d467e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Wed, 21 Oct 2026 16:39:40 +0900 Subject: [PATCH 12/18] chore(agents): keep a protected test's scaffold marker A protected test the pruner cannot resolve would otherwise lose its only marker and turn into an unmarked test every review flags, so it keeps the marker and is listed as unresolved in the oracle table. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .agents/skills/test-pruner/SKILL.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.agents/skills/test-pruner/SKILL.md b/.agents/skills/test-pruner/SKILL.md index c74d4ebb5a..a7cb9de24b 100644 --- a/.agents/skills/test-pruner/SKILL.md +++ b/.agents/skills/test-pruner/SKILL.md @@ -33,7 +33,7 @@ A strengthened or gap test that fails is a finding, never a task to make it pass Delete a self-derived test with no unique kills. A test of unknown oracle with unique kills has already passed the challenge; strengthen it. Challenge a test of unknown oracle without unique kills, unless it is protected below, since its fate does not depend on the answer. -No `scaffold` marker outlives the pass: these rules replace it with the verified source or `mutant`, or delete what it marks, and a protected test they leave loses the marker and keeps its classification. +No `scaffold` marker outlives the pass unreported: these rules replace it with the verified source or `mutant`, or delete what it marks, and a protected test they leave keeps it and is listed as unresolved in the oracle table. Deletion is a set decision, not a per-test verdict. Two tests that both catch mutant M each have zero unique kills, and deleting both loses M. From 85846c93324328cbbf03f9eb6137b7fdae904bcd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Wed, 21 Oct 2026 17:03:18 +0900 Subject: [PATCH 13/18] chore(agents): cite scoped contract files in oracle markers Contracts may now be named `CONTRACT..md`, so a `contract` marker accepts any `CONTRACT*.md` rather than only `CONTRACT.md`. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .agents/skills/test-oracle-reviewer/SKILL.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.agents/skills/test-oracle-reviewer/SKILL.md b/.agents/skills/test-oracle-reviewer/SKILL.md index 0068ef661c..fa09fe235f 100644 --- a/.agents/skills/test-oracle-reviewer/SKILL.md +++ b/.agents/skills/test-oracle-reviewer/SKILL.md @@ -27,7 +27,7 @@ Mark where an expected value comes from with `// oracle: ` on Strong kinds name an independent source, and a test, case, or assertion marked with one is deleted only when its source changes or is shown not to decide what it asserts: - `spec §
`: normative text other than a contract or intent file, such as an RFC, a Microsoft open specification, or `docs/JMUX-spec.md`. -- `contract §
`: a `CONTRACT.md`. +- `contract §
`: a `CONTRACT*.md`. - `intent §
`: a statement in an intent file. - `regression `: a bug report or fuzz crash. - `reference `: what another implementation or a reference model does, recorded or compared live. From e9bb9d8a1619f54e4d5c42b8e828ef031feae613 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Wed, 21 Oct 2026 17:15:27 +0900 Subject: [PATCH 14/18] chore(agents): merge no scaffold test A scaffold test now gains a strong source, becomes `mutant` when it alone catches a fault, or is deleted before its pull request merges. One that checks a property or intention no document states prompts a question about where to specify it, and is marked with that source once it exists, because a confirmation is not a source. Protected tests take that path instead of keeping their scaffold marker, which a merge check could not tell apart from a forgotten one. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .agents/skills/test-oracle-reviewer/SKILL.md | 3 +++ .agents/skills/test-pruner/SKILL.md | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/.agents/skills/test-oracle-reviewer/SKILL.md b/.agents/skills/test-oracle-reviewer/SKILL.md index fa09fe235f..38b19f959e 100644 --- a/.agents/skills/test-oracle-reviewer/SKILL.md +++ b/.agents/skills/test-oracle-reviewer/SKILL.md @@ -40,6 +40,9 @@ Weak kinds name a self-derived source, which detects change rather than bugs, so - `mutant `: a test or assertion kept to catch mutants that no strong source decides. - `scaffold`: a test or assertion written for feedback while implementing that no strong source decides. +No `scaffold` marker merges: before the pull request does, give what it marks a strong source, keep it as `mutant` when `test-pruner` finds it alone catches a fault, or delete it. +When it checks a property or intention no document states, ask whether and where to specify it, and mark it with that source once specified, since a confirmation alone is not a source; a protected test takes this path rather than deletion. + Mark a strong source whenever one decides the value, so a test found through a mutant but decided by a spec is `spec`, because whether a test may go depends on whether a source still requires it, not on how it was found. An unmarked test starts as unknown: verify it as above, and turn a citation in another form into the marker it supports. A marker is a claim: verify it like any other origin. diff --git a/.agents/skills/test-pruner/SKILL.md b/.agents/skills/test-pruner/SKILL.md index a7cb9de24b..ac9c7dc5f3 100644 --- a/.agents/skills/test-pruner/SKILL.md +++ b/.agents/skills/test-pruner/SKILL.md @@ -33,7 +33,7 @@ A strengthened or gap test that fails is a finding, never a task to make it pass Delete a self-derived test with no unique kills. A test of unknown oracle with unique kills has already passed the challenge; strengthen it. Challenge a test of unknown oracle without unique kills, unless it is protected below, since its fate does not depend on the answer. -No `scaffold` marker outlives the pass unreported: these rules replace it with the verified source or `mutant`, or delete what it marks, and a protected test they leave keeps it and is listed as unresolved in the oracle table. +No `scaffold` marker outlives the pass: resolve each as `test-oracle-reviewer` requires, sending its questions to whoever assigned the run, or to the author in review mode. Deletion is a set decision, not a per-test verdict. Two tests that both catch mutant M each have zero unique kills, and deleting both loses M. From ad19e7b4eb3b782b583df6284ee6fba6adf8d612 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Wed, 21 Oct 2026 17:46:22 +0900 Subject: [PATCH 15/18] chore(agents): give oracle marker paths from the repository root Markers sit in test files at every depth, so a path relative to the test would read differently from each and could not be resolved without knowing where the marker lives. Marker paths are now repository-root relative with forward slashes and exact case, a `spec` outside `docs/` is named rather than given as a path, a crash file sits under `fuzz/`, and a section names a heading or the number it starts with. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .agents/skills/test-oracle-reviewer/SKILL.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.agents/skills/test-oracle-reviewer/SKILL.md b/.agents/skills/test-oracle-reviewer/SKILL.md index 38b19f959e..ba5b5eecb2 100644 --- a/.agents/skills/test-oracle-reviewer/SKILL.md +++ b/.agents/skills/test-oracle-reviewer/SKILL.md @@ -33,6 +33,10 @@ Strong kinds name an independent source, and a test, case, or assertion marked w - `reference `: what another implementation or a reference model does, recorded or compared live. - `property `: a property no document states, such as a round-trip or an input the test chose being honored. +Write every path from the repository root, with forward slashes and exact case, such as `crates/gateway-db/INTENT.md`, because markers sit at every depth and a root-relative path reads and resolves the same from each. +A `spec` document outside `docs/`, such as `RFC 6455` or `MS-RDPBCGR`, is named rather than given as a path, and a `regression` crash file is a path under `fuzz/`. +`§
` names a heading in the file, in any case, or the number the heading starts with, so `§3.2` matches `3.2 Framing`. + A hand-derived value or a checked snapshot takes the kind of the source it was checked against. Weak kinds name a self-derived source, which detects change rather than bugs, so any intentional change, such as a refactor or a new feature, may delete a test, case, or assertion marked with one without justification unless `test-pruner` protects it: From e402c4cd3fd9102810befc2492055d5ddd120d50 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Wed, 21 Oct 2026 17:48:34 +0900 Subject: [PATCH 16/18] chore(agents): spell out regression marker references A `regression` marker names a Jira key, a GitHub issue as `#123` or `owner/repo#123`, or a crash file under `fuzz/`, the forms a marker check can resolve without guessing. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .agents/skills/test-oracle-reviewer/SKILL.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.agents/skills/test-oracle-reviewer/SKILL.md b/.agents/skills/test-oracle-reviewer/SKILL.md index ba5b5eecb2..dde9f2bd23 100644 --- a/.agents/skills/test-oracle-reviewer/SKILL.md +++ b/.agents/skills/test-oracle-reviewer/SKILL.md @@ -29,12 +29,12 @@ Strong kinds name an independent source, and a test, case, or assertion marked w - `spec §
`: normative text other than a contract or intent file, such as an RFC, a Microsoft open specification, or `docs/JMUX-spec.md`. - `contract §
`: a `CONTRACT*.md`. - `intent §
`: a statement in an intent file. -- `regression `: a bug report or fuzz crash. +- `regression `: a bug report, as a Jira key such as `DGW-123` or a GitHub issue such as `#123` or `owner/repo#123`, or a fuzz crash file under `fuzz/`. - `reference `: what another implementation or a reference model does, recorded or compared live. - `property `: a property no document states, such as a round-trip or an input the test chose being honored. Write every path from the repository root, with forward slashes and exact case, such as `crates/gateway-db/INTENT.md`, because markers sit at every depth and a root-relative path reads and resolves the same from each. -A `spec` document outside `docs/`, such as `RFC 6455` or `MS-RDPBCGR`, is named rather than given as a path, and a `regression` crash file is a path under `fuzz/`. +A `spec` document outside `docs/`, such as `RFC 6455` or `MS-RDPBCGR`, is named rather than given as a path. `§
` names a heading in the file, in any case, or the number the heading starts with, so `§3.2` matches `3.2 Framing`. A hand-derived value or a checked snapshot takes the kind of the source it was checked against. From d0e9d2fe47dfdb719689bfda24bfde69af59939a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Wed, 21 Oct 2026 18:17:27 +0900 Subject: [PATCH 17/18] chore(agents): restrict property markers to named laws A `property` marker now names one of six well-known laws, because a named law is its own definition while any other claimed property needs a written source, and stays `scaffold` until it has one. An assertion whose value comes from running the code is never a property, even over generated inputs, and a round-trip never replaces a codec's vectors. A section reference also matches an `` anchor placed before a heading, and markers cite the anchor when the file has one, since it survives a renamed heading or a renumbered section. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .agents/skills/test-oracle-reviewer/SKILL.md | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/.agents/skills/test-oracle-reviewer/SKILL.md b/.agents/skills/test-oracle-reviewer/SKILL.md index dde9f2bd23..8bce230ce3 100644 --- a/.agents/skills/test-oracle-reviewer/SKILL.md +++ b/.agents/skills/test-oracle-reviewer/SKILL.md @@ -8,7 +8,7 @@ description: Judge whether a test's expected value is justified independently of A test whose expected value came from the code under test detects change, not bugs. For each test, ask whether the expected value can be justified without running the code. -Independent sources: a spec example, an external capture, a hand-derived value, an input the test chose itself and asserts was honored, a real property such as round-trip or `encode` writing exactly `size()` bytes, or a reference model. +Independent sources: a spec example, an external capture, a hand-derived value, an input the test chose itself and asserts was honored, a named law such as round-trip, a property a document states, such as `encode` writing exactly `size()` bytes, or a reference model. Self-derived sources: a snapshot of current output, a value computed through the code or a shared helper, a production constant or message copied into the test, a mock that returns what is then asserted, `is_ok()` or `!is_empty()` on an input that no requirement says must succeed, or a restatement of what a derive or the type system guarantees. Verify instead of guessing. @@ -31,13 +31,17 @@ Strong kinds name an independent source, and a test, case, or assertion marked w - `intent §
`: a statement in an intent file. - `regression `: a bug report, as a Jira key such as `DGW-123` or a GitHub issue such as `#123` or `owner/repo#123`, or a fuzz crash file under `fuzz/`. - `reference `: what another implementation or a reference model does, recorded or compared live. -- `property `: a property no document states, such as a round-trip or an input the test chose being honored. +- `property `: a law of the code under test named by one of `round-trip`, `idempotent`, `involution`, `commutative`, `associative`, or `monotonic`. Write every path from the repository root, with forward slashes and exact case, such as `crates/gateway-db/INTENT.md`, because markers sit at every depth and a root-relative path reads and resolves the same from each. A `spec` document outside `docs/`, such as `RFC 6455` or `MS-RDPBCGR`, is named rather than given as a path. -`§
` names a heading in the file, in any case, or the number the heading starts with, so `§3.2` matches `3.2 Framing`. +`§
` names the `id` of an anchor the file places before a heading as ``, or else a heading, in any case, or the number a heading starts with, so `§3.2` matches `3.2 Framing`. +Cite the anchor when the file has one, because it survives a renamed heading or a renumbered section. -A hand-derived value or a checked snapshot takes the kind of the source it was checked against. +A `property` law is accepted only by its well-known name, because the name is its definition, while any other property needs a written source: it takes the kind of the document that states it, and is `scaffold` until one does. +An assertion whose expected value comes from running the code under test is self-derived even over generated inputs, so it is `mutant` or `scaffold`, never `property`. +A codec's `round-trip` test never replaces its spec or capture vectors, because a round-trip passes symmetric bugs. +A hand-derived value, a checked snapshot, or an input the test chose and asserts was honored takes the kind of the source that requires it. Weak kinds name a self-derived source, which detects change rather than bugs, so any intentional change, such as a refactor or a new feature, may delete a test, case, or assertion marked with one without justification unless `test-pruner` protects it: From 2e4303c220ee139b5281b29520fa951b6a4c1624 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Wed, 21 Oct 2026 20:08:50 +0900 Subject: [PATCH 18/18] chore(agents): cite files in other repositories by commit A marker can now cite a file in another repository as `/@:`, pinned to a commit of at least 12 hex digits rather than a branch or tag, because a pinned commit never changes and resolves the same every time. When the pinned section changes upstream, the test is re-checked against the new text and its pin or the test changes, never the expected value alone, since a value changed without its pin cites text that decides the old one. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .agents/skills/test-oracle-reviewer/SKILL.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.agents/skills/test-oracle-reviewer/SKILL.md b/.agents/skills/test-oracle-reviewer/SKILL.md index 8bce230ce3..793f611472 100644 --- a/.agents/skills/test-oracle-reviewer/SKILL.md +++ b/.agents/skills/test-oracle-reviewer/SKILL.md @@ -34,9 +34,11 @@ Strong kinds name an independent source, and a test, case, or assertion marked w - `property `: a law of the code under test named by one of `round-trip`, `idempotent`, `involution`, `commutative`, `associative`, or `monotonic`. Write every path from the repository root, with forward slashes and exact case, such as `crates/gateway-db/INTENT.md`, because markers sit at every depth and a root-relative path reads and resolves the same from each. +A file in another repository is cited as `/@:`, with `` from that repository's root, such as `Devolutions/devolutions-gateway@43958682a1b2:docs/agent-identity/CONTRACT.agent-identity.md`, and `` a commit of at least 12 hex digits, never a branch or tag, because a pinned commit never changes and so resolves the same every time. A `spec` document outside `docs/`, such as `RFC 6455` or `MS-RDPBCGR`, is named rather than given as a path. `§
` names the `id` of an anchor the file places before a heading as ``, or else a heading, in any case, or the number a heading starts with, so `§3.2` matches `3.2 Framing`. Cite the anchor when the file has one, because it survives a renamed heading or a renumbered section. +When a pinned section changes upstream, re-check the test against the new text, then bump the pin or change the test, never the expected value alone, because a value changed without its pin cites text that decides the old one. A `property` law is accepted only by its well-known name, because the name is its definition, while any other property needs a written source: it takes the kind of the document that states it, and is `scaffold` until one does. An assertion whose expected value comes from running the code under test is self-derived even over generated inputs, so it is `mutant` or `scaffold`, never `property`.