Move the tests to xunit.v3 4 and answer reviews with Conventional Comments

.claude/skills/respond-to-review/SKILL.md

@@ -0,0 +1,125 @@
+---
+name: respond-to-review
+description: As the author of a GitHub pull request, answer its review findings and questions with Conventional Comments reply labels — `done`, `ack`, `disagree`, `thanks`, plus `note` and `question` — fix what needs fixing, and push. Use when a pull request you wrote receives review comments, or when asked to address a review. Never posts `reviewed` or `decision`, never approves, never merges.
+---
+
+# Respond to a review
+
+You are the author of the pull request: you wrote its code, and you are answering its review.
+Every finding or question a reviewer raises gets an answer, in its own thread, and the answer
+says what you did about it. A reviewer's `reviewed:` comment is not one of them: it closes their
+review, raises no point and asks nothing, so it needs no reply.
+
+## 0. Language
+
+**Write every reply in the language the repository requires.** Look for it before posting:
+`CLAUDE.md`, `AGENTS.md`, `CONTRIBUTING.md`, or any other contribution guide the repository keeps.
+A rule there wins over everything else — including the language the reviewer wrote in, and the
+language of this skill. Without such a rule, answer in the language of the comment you reply to.
+
+The same rule applies to everything else you write on the pull request: its title, its
+description, and your commit messages, which a repository often constrains separately.
+
+Only the prose follows that language. Labels and identifiers (`done`, `disagree`, a function
+name) are never translated.
+
+## 1. Conventional Comments
+
+Follow https://conventionalcomments.org/ and the extensions of the Conventional Comments Toolkit
+(https://github.com/Reefact/conventional-comments-toolkit/blob/main/README.md).
+
+**Read the repository's configuration first**: `.conventional-comments.json` at the repository
+root, if it exists. A reply label it disables does not exist for this repository: answer with
+what it leaves enabled, or in plain text where replies need no label.
+
+**That file is not the whole configuration.** The labels in force come from three levels, merged
+in this order: the toolkit's defaults, then the **organisation's configuration**, then the
+repository's file, each overriding the previous one label by label; an **enterprise floor** can
+then force some labels on. The organisation's configuration and the floor reach the extension
+through the browser's enterprise policy, so you usually cannot see them. Therefore:
+
+- if the person you work for gives you the organisation's configuration, apply it before the
+  repository's file;
+- otherwise, use only labels you can confirm: those the repository's file declares or enables, and
+  the toolkit's defaults it does not disable — never one the file disables. Do not guess an
+  organisation's custom labels;
+- say in your report that the organisation's configuration was not visible to you, if it was not.
+
+Every reply starts with:
+
+`<label>: <subject>`
+
+The subject is at most **72 characters**. The discussion below it adds what the reviewer needs.
+
+## 2. The reply labels
+
+Answer each finding with the label that says what you did:
+
+- **`done:`** — the code changed because of the comment. Use it only once the change is
+  **pushed**, and name the commit: `done: fixed in a1b2c3d, null now returns an error`. A change
+  you have only planned, or only made locally, is not `done`. `fixed` is accepted for `done`.
+- **`ack:`** — received and accepted, the code has not changed at this stage: "noted, handled in
+  the follow-up PR", or "agreed, no change needed". It is an acknowledgement, never a polite
+  refusal. `acknowledged` is accepted for `ack`.
+- **`disagree:`** — you decline the remark, and the subject says why. Give a verifiable reason:
+  the code, a test, the specification, a recorded decision. `disagree` carries no authority and
+  resolves nothing: the reviewer decides whether your reason holds.
+- **`thanks:`** — you thank the reviewer without taking a position on the point or the code: the
+  natural answer to a `praise`. `thx` is accepted for `thanks`.
+
+Two more gestures in a thread already have their labels, and they are yours too (specification
+§3.2):
+
+- **`note:`** — you provide information: the answer to a reviewer's `question`, or a fact the
+  reviewer needs to settle a point.
+- **`question:`** — you ask back, when you need the reviewer to clarify before you can act.
+
+A reviewer's `question` gets the label that says what followed: `note:` when you answer it with
+information, `done:` when it led to a pushed change. `ack:` is not a neutral answer: keep it for a
+point you accept.
+
+## 3. What you never post
+
+- **`decision`**: it records that the team deliberately leaves a point unaddressed, and it is
+  reserved to the human members of the repository's `resolverOverrideGroup`. When you think a
+  blocking point should not be addressed, answer `disagree:` with your reason, and tell the
+  person you work for that a `decision` may be needed.
+- **`reviewed`**: it closes a reviewer's review, not yours.
+- The other labels that raise a point (`issue`, `suggestion`, `nitpick`, `praise`…): you are
+  answering, not reviewing. If you find a problem of your own while fixing, fix it and say so in the reply.
+
+Never approve or merge the pull request.
+
+## 4. Fixing
+
+For each comment that calls for a change:
+
+1. Check the claim before acting: trace it in the code, the tests or the specification. A
+   reviewer can be wrong; if so, answer `disagree:` with the evidence.
+2. Make the smallest change that addresses the point, without widening the pull request.
+3. Run the repository's checks and tests before pushing.
+4. Push, then answer `done:` with the commit.
+
+Answer every finding and question, including optional findings, `(non-blocking)` ones and
+`praise`. A standalone `reviewed:` closure needs no answer.
+
+## 5. Resolving threads
+
+- **A non-blocking thread**: you may resolve it yourself once your fix is pushed, or your answer
+  is final, and you have no doubt that it settles the point. When in doubt, leave it open for the
+  reviewer.
+- **A blocking thread**: leave it to its reviewer. Under the toolkit's governance rule
+  (specification §6.1), a blocking thread is resolved by the author of its comment, or through a
+  `decision`; a resolution by anyone else is refused by the toolkit's verifier, wherever one is
+  deployed.
+
+## 6. After pushing
+
+Ask the reviewer for a new pass. Any change you push reopens the reviewer's review: wait for
+their `reviewed:` again rather than assuming the earlier one still holds.
+
+Report to the person you work for: what you fixed (with commits), what you acknowledged without
+changing, what you declined and why, the threads you resolved, and anything that needs a human —
+a `decision`, or a disagreement you could not settle.
+
+Never claim to have pushed, run or verified something you have not.

.github/dependabot.yml

@@ -17,6 +17,12 @@ updates:
       # nuget config cannot scope the rule to one project, so these also freeze in the analyzer test project.
       - dependency-name: "Microsoft.CodeAnalysis.CSharp"
       - dependency-name: "Microsoft.CodeAnalysis.Common"
+      # Verify 33 added a build-time SponsorCheck (error SC021) that fails GenDoc.UnitTests unless the build
+      # declares a sponsorship, a licence or a time-bounded exemption — a licensing decision, not routine
+      # maintenance. Verify.XunitV3 32.x runs on xunit.v3 4 (its dependency is a 3.2.2 minimum), so it stays
+      # there until the maintainer settles that question.
+      - dependency-name: "Verify.XunitV3"
+        versions: [">= 33.0.0"]
 
   - package-ecosystem: "github-actions"
     directory: "/"

.github/pull_request_template.md

@@ -38,7 +38,7 @@
 <!-- Check only the commands/tests that were actually run. Add details if something was not run. -->
 
 * [ ] `dotnet build FirstClassErrors.sln`
-* [ ] `dotnet test FirstClassErrors.sln`
+* [ ] `dotnet test --solution FirstClassErrors.sln`
 * [ ] Analyzer tests pass (`FirstClassErrors.Analyzers.UnitTests`)
 
 ## Documentation

.github/workflows/canary.yml

@@ -156,6 +156,6 @@ jobs:
             # the preview major. The env is scoped inline to the test call so it never touches build/evaluation.
             dotnet build "$proj/$proj.csproj" -c Release
             DOTNET_ROLL_FORWARD=LatestMajor DOTNET_ROLL_FORWARD_TO_PRERELEASE=1 \
-              dotnet test "$proj/$proj.csproj" -c Release --no-build --logger "console;verbosity=normal"
+              dotnet test --project "$proj/$proj.csproj" -c Release --no-build --output Detailed
             echo "::endgroup::"
           done

.github/workflows/ci.yml

@@ -53,21 +53,22 @@ jobs:
         run: dotnet build FirstClassErrors.sln -c Release --no-restore
 
       - name: Test
-        # Collect OpenCover coverage via coverlet.collector (configured in coverage.runsettings).
-        # Reports land under artifacts/coverage/<guid>/coverage.opencover.xml, ready for a Sonar scan.
+        # Collect OpenCover coverage via coverlet.MTP (options in coverage.rsp). Reports land under
+        # artifacts/coverage/coverage.opencover.<timestamp>.xml, one per test project, ready for a Sonar scan.
+        # '@coverage.rsp' is quoted: unquoted, PowerShell (the windows-latest default shell) parses it as its
+        # splatting operator and fails before dotnet runs.
         run: >-
-          dotnet test FirstClassErrors.sln -c Release --no-build
-          --collect:"XPlat Code Coverage"
-          --settings coverage.runsettings
+          dotnet test --solution FirstClassErrors.sln -c Release --no-build
+          '@coverage.rsp'
           --results-directory artifacts/coverage
-          --logger "console;verbosity=normal"
+          --output Detailed
 
       - name: Upload coverage report
         uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7
         with:
           # Per-OS name so the two matrix legs do not clash when uploading.
           name: coverage-${{ matrix.os }}
-          path: artifacts/coverage/**/coverage.opencover.xml
+          path: artifacts/coverage/**/coverage.opencover.*.xml
           if-no-files-found: error
 
   # build-test above runs the suite on the LATEST .NET (10). This job proves the LIBRARY's OTHER supported
@@ -110,8 +111,8 @@ jobs:
             FirstClassErrors.PropertyTests \
             FirstClassErrors.RequestBinder.PropertyTests ; do
             echo "::group::$proj (net472)"
-            dotnet test "$proj/$proj.csproj" -c Release -f net472 -p:EnableNet472Floor=true \
-              --logger "console;verbosity=normal"
+            dotnet test --project "$proj/$proj.csproj" -c Release -f net472 -p:EnableNet472Floor=true \
+              --output Detailed
             echo "::endgroup::"
           done
 

.github/workflows/release.yml

@@ -130,7 +130,7 @@ jobs:
         run: dotnet build FirstClassErrors.sln -c Release --no-restore -p:Version="$VERSION"
 
       - name: Test
-        run: dotnet test FirstClassErrors.sln -c Release --no-build --logger "console;verbosity=normal"
+        run: dotnet test --solution FirstClassErrors.sln -c Release --no-build --output Detailed
 
       # GenDoc ships bundled inside the fce tool (no package of its own -- see pack.sh), so a breaking
       # change to its own error catalog is a breaking change of the cli train. doc/generated/gendoc/

.github/workflows/sonar.yml

@@ -93,7 +93,7 @@ jobs:
           /o:"reefact"
           /d:sonar.token="$SONAR_TOKEN"
           /d:sonar.host.url="https://sonarcloud.io"
-          /d:sonar.cs.opencover.reportsPaths="artifacts/coverage/**/coverage.opencover.xml"
+          /d:sonar.cs.opencover.reportsPaths="artifacts/coverage/**/coverage.opencover.*.xml"
           /d:sonar.exclusions="**/*.verified.*"
           /d:sonar.coverage.exclusions="FirstClassErrors.RequestBinder.Benchmarks/**"
           /d:sonar.issue.ignore.multicriteria="s7682,s7679"
@@ -112,13 +112,12 @@ jobs:
         run: dotnet build FirstClassErrors.sln -c Release -p:TreatWarningsAsErrors=false -p:MSBuildTreatWarningsAsErrors=false
 
       - name: Test with coverage
-        # Reuse coverage.runsettings so the OpenCover report matches the one the ci workflow produces.
+        # Reuse coverage.rsp so the OpenCover report matches the one the ci workflow produces.
         run: >-
-          dotnet test FirstClassErrors.sln -c Release --no-build
-          --collect:"XPlat Code Coverage"
-          --settings coverage.runsettings
+          dotnet test --solution FirstClassErrors.sln -c Release --no-build
+          '@coverage.rsp'
           --results-directory artifacts/coverage
-          --logger "console;verbosity=normal"
+          --output Detailed
 
       - name: End analysis
         env:

.gitignore

@@ -61,6 +61,8 @@ artifacts/
 *.pgc
 *.pgd
 *.rsp
+# ...except the shared coverage options of `dotnet test` (ci and sonar workflows).
+!/coverage.rsp
 *.sbr
 *.tlb
 *.tli

AGENTS.md

@@ -7,7 +7,7 @@ Two roles are covered: **writing code** and **reviewing pull requests**.
 
 - .NET Standard 2.0 library. Errors are first-class, documented, diagnosable concepts.
 - Build: `dotnet build FirstClassErrors.sln`
-- Test: `dotnet test FirstClassErrors.sln` (analyzer tests: `dotnet test FirstClassErrors.Analyzers.UnitTests`).
+- Test: `dotnet test --solution FirstClassErrors.sln` (analyzer tests: `dotnet test --project FirstClassErrors.Analyzers.UnitTests`).
 - Repository language is **English** (code, comments, commits, PRs, issues, and
   review comments). French lives only in `doc/handwritten/for-users/README.fr.md` and must stay in sync
   with the English README.
@@ -202,13 +202,24 @@ review (for example `@claude`), not the reviewer. The human maintainer `@reefact
 is the only authority that merges a pull request; no agent merges, and no agent
 enables auto-merge on its own pull request.
 
+Write every reply with the Conventional Comments reply labels (`done`, `ack`,
+`disagree`, `thanks`, `note`, `question`), as the
+[`respond-to-review`](.claude/skills/respond-to-review/SKILL.md) skill describes:
+it says which label fits which answer, which labels an agent never posts
+(`decision`, `reviewed`), and who resolves a blocking thread (its reviewer). The
+skill is vendored verbatim from
+[`Reefact/conventional-comments-toolkit`](https://github.com/Reefact/conventional-comments-toolkit/tree/main/skills/respond-to-review)
+(commit `996a6fd`); update it by copying a newer version over it, never by editing
+it here. Where it and this section overlap, this section decides who is mentioned
+and when to stop.
+
 For each review finding, take exactly one route:
 
 - **You agree and the fix is clear and local** — implement it, push, and reply on
-  the thread with `Resolved in <sha>`. You MAY ask the reviewer (`@codex`) for a
-  single confirming re-review; never open a back-and-forth.
-- **You believe the finding is wrong** — reply on the thread with the concrete
-  technical reason and mention `@reefact` to arbitrate. Do not ping `@codex` to
+  the thread with `done: fixed in <sha>, <what changed>`. You MAY ask the reviewer
+  (`@codex`) for a single confirming re-review; never open a back-and-forth.
+- **You believe the finding is wrong** — reply on the thread with `disagree:` and
+  the concrete technical reason, and mention `@reefact` to arbitrate. Do not ping `@codex` to
   argue: a peer reviewer has no authority to settle the disagreement.
 - **The finding needs a human judgement** — architecture, a product trade-off, an
   ambiguous requirement, a security or compatibility policy — mention `@reefact`

CLAUDE.md

@@ -19,9 +19,9 @@ errors should stay structured, documented, and close to the code.
 
 * Target framework: **.NET Standard 2.0**.
 * Build: `dotnet build FirstClassErrors.sln`
-* Test: `dotnet test FirstClassErrors.sln`
+* Test: `dotnet test --solution FirstClassErrors.sln`
 * Run the analyzer tests when touching analyzers:
-  `dotnet test FirstClassErrors.Analyzers.UnitTests`
+  `dotnet test --project FirstClassErrors.Analyzers.UnitTests`
 * Mutation testing measures every pull request on the files it changed, for every
   project whose code ships or runs
   (decisions: ADR-0043, and ADR-0046 which made the per-PR check **advisory** — it
@@ -145,10 +145,15 @@ When you act on review feedback on a pull request (for example a Codex review),
 follow the escalation rules in [`AGENTS.md`](AGENTS.md) ("Responding to review
 feedback"). The essentials, inlined so they hold even if `AGENTS.md` is not read:
 
+* Reply with Conventional Comments reply labels, per the
+  [`respond-to-review`](.claude/skills/respond-to-review/SKILL.md) skill: `done:`
+  only once the fix is pushed, `ack:`, `disagree:`, `thanks:`, `note:`, `question:`;
+  never `decision:` or `reviewed:`, and leave a blocking thread for its reviewer
+  to resolve.
 * If you agree and the fix is clear and local, implement it, push, and reply
-  `Resolved in <sha>`.
-* If you believe a finding is wrong, reply with the concrete technical reason and
-  mention `@reefact` to arbitrate — do not argue with the reviewer bot.
+  `done: fixed in <sha>, <what changed>`.
+* If you believe a finding is wrong, reply `disagree:` with the concrete technical
+  reason and mention `@reefact` to arbitrate — do not argue with the reviewer bot.
 * If a finding needs a human judgement (architecture, a trade-off, an ambiguous
   requirement, a security or compatibility policy), mention `@reefact` and wait.
 * Never mention both the reviewer bot and `@reefact` on the same thread; cap at

CONTRIBUTING.md

@@ -11,8 +11,8 @@ library produces. This guide defines how commits are written here.
 
 * Target framework: **.NET Standard 2.0**.
 * Build: `dotnet build FirstClassErrors.sln`
-* Test: `dotnet test FirstClassErrors.sln`
-* Analyzer tests, when touching analyzers: `dotnet test FirstClassErrors.Analyzers.UnitTests`
+* Test: `dotnet test --solution FirstClassErrors.sln`
+* Analyzer tests, when touching analyzers: `dotnet test --project FirstClassErrors.Analyzers.UnitTests`
 
 See [`CLAUDE.md`](CLAUDE.md) for the project layout and the broader change
 guidelines.

Directory.Packages.props

@@ -26,7 +26,7 @@
   <ItemGroup>
     <PackageVersion Include="AngleSharp" Version="1.8.5" />
     <PackageVersion Include="BenchmarkDotNet" Version="0.15.8" />
-    <PackageVersion Include="coverlet.collector" Version="10.1.0" />
+    <PackageVersion Include="coverlet.MTP" Version="10.1.0" />
     <PackageVersion Include="FsCheck" Version="3.5.0" />
     <PackageVersion Include="JetBrains.Annotations" Version="2026.2.0" />
     <PackageVersion Include="JustDummies" Version="1.0.0-preview.6" />
@@ -41,10 +41,10 @@
     <PackageVersion Include="NSubstitute" Version="6.2.0" />
     <PackageVersion Include="SonarAnalyzer.CSharp" Version="10.35.0.4138" />
     <PackageVersion Include="Spectre.Console.Cli" Version="0.55.0" />
-    <PackageVersion Include="Verify.XunitV3" Version="31.28.0" />
-    <PackageVersion Include="xunit.runner.visualstudio" Version="3.1.5" />
-    <PackageVersion Include="xunit.v3" Version="3.2.2" />
-    <PackageVersion Include="xunit.v3.extensibility.core" Version="3.2.2" />
+    <PackageVersion Include="Verify.XunitV3" Version="32.0.1" />
+    <PackageVersion Include="xunit.runner.visualstudio" Version="4.0.1" />
+    <PackageVersion Include="xunit.v3" Version="4.0.2" />
+    <PackageVersion Include="xunit.v3.extensibility.core" Version="4.0.2" />
   </ItemGroup>
 
 </Project>

FirstClassErrors.Analyzers.UnitTests/FirstClassErrors.Analyzers.UnitTests.csproj

@@ -8,9 +8,8 @@
   </PropertyGroup>
 
   <ItemGroup>
-    <PackageReference Include="coverlet.collector">
+    <PackageReference Include="coverlet.MTP">
       <PrivateAssets>all</PrivateAssets>
-      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
     </PackageReference>
     <PackageReference Include="Microsoft.CodeAnalysis.CSharp" />
     <PackageReference Include="Microsoft.NET.Test.Sdk" />

FirstClassErrors.Cli.UnitTests/FirstClassErrors.Cli.UnitTests.csproj

@@ -8,9 +8,8 @@
   </PropertyGroup>
 
   <ItemGroup>
-    <PackageReference Include="coverlet.collector">
+    <PackageReference Include="coverlet.MTP">
       <PrivateAssets>all</PrivateAssets>
-      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
     </PackageReference>
     <PackageReference Include="JetBrains.Annotations" />
     <PackageReference Include="Microsoft.NET.Test.Sdk" />