Skip to content

fix(toolchain): make the documented C# gates execute truthfully - #540

Merged
drmoisan merged 2 commits into
epic/build-ci-coverage-gate-fidelity-integrationfrom
bug/csharp-toolchain-gate-fidelity-512
Aug 11, 2026
Merged

drmoisan merged 2 commits into
epic/build-ci-coverage-gate-fidelity-integrationfrom
bug/csharp-toolchain-gate-fidelity-512

Conversation

@drmoisan

Copy link
Copy Markdown
Owner

fix(toolchain): make the documented C# gates execute truthfully

Summary

  • Corrects the documented C# toolchain commands so that each one actually runs, actually compiles, and can actually both pass and fail. Four issues (Bug: nullable-gate-cannot-fail-incremental-build #512, Bug: nullable-gate-masked-by-incremental-build #492, Bug: csharpier-documented-command-incompatible-with-pinned-version #509, Bug: claudemd-nullable-gate-diverges-from-ci #522) are four symptoms of the same root problem: the commands in CLAUDE.md, .claude/rules/csharp.md, and .claude/skills/csharp-qa-gate/SKILL.md had drifted from .github/workflows/ci.yml and from the pinned tool versions.
  • Format gate could not run. dotnet tool run csharpier . is CSharpier v0 syntax; dotnet-tools.json pins 1.2.6, which requires a subcommand. Measured: EXIT_CODE: 1, Required command was not provided. Corrected to csharpier format . with csharpier check . for read-only CI parity.
  • Analyzer and type-check gates could not fail. /t:Build lets MSBuild's incremental up-to-date check skip CoreCompile, because that check compares timestamps and does not invalidate on a command-line /p: change. Measured on a warm tree: EXIT_CODE: 0 with Skipping target "CoreCompile" on 18 of 18 projects. Corrected to /t:Rebuild /m.
  • Type-check gate could not pass. /p:Nullable=enable is deliberately absent from ci.yml because nullable analysis here is per-file opt-in via #nullable enable. Forcing it solution-wide conscripts every file that never adopted the pragma. Measured: 195 errors in UtilitiesCS.csproj against 0 without it. Removed, making the documented command character-for-character ci.yml's.
  • Adds an executable carrier for the corrected behavior: scripts/vscode/Invoke-VSBuild.ps1 gains a -Target parameter and the VS Code lint: and type-check: tasks pass -Target Rebuild.
  • Adds a non-vacuity requirement to the QA-gate skill: a step must produce an MSBuild /fl log with zero occurrences of Skipping target "CoreCompile". Exit 0 with a non-zero skip count is now explicitly unverified, not passed.

Why

Two deliveries on 2026-08-08 (#507, #508) required a human override because the documented nullable gate manufactured blocking findings that were red on a clean main. Separately, every agent that ran the documented analyzer or type-check command on a warm working tree received a green result from a build that compiled nothing. The governance documents were the defect: they described gates that were structurally incapable of doing their job.

This change corrects those documents. It is a strengthening, not a relaxation, and that distinction is load-bearing:

  • Removing /p:Nullable=enable loses no enforcement over any file that has opted in. No project in this repository carries a <Nullable> element and there is no Directory.Build.props, so the flag was a solution-wide opt-in, not a per-file one. /p:TreatWarningsAsErrors=true still promotes every CS86xx diagnostic in every #nullable enable file to a build error. A negative-path control confirms this: a deliberately introduced nullable violation in a pragma-carrying production file still fails the corrected gate.
  • Replacing /t:Build with /t:Rebuild strictly increases what the gate examines, from nothing (warm) to all 18 projects.

The corrected sites carry in-line rationale so that a future agent does not "restore" the removed flag, which has already happened once in the record.

What Changed

Governance documents (authorized sites only)

File Sites
CLAUDE.md § C#1 items 1-3 (format, analyzer, type-check command blocks); § CUT3 items 1-3; § "C# Toolchain (run in this exact order)" items 1-3
.claude/rules/csharp.md § Toolchain items 1-3; § "Severity-first ordering invariant" (embedded command string only, one changed line)
.claude/skills/csharp-qa-gate/SKILL.md § "Toolchain Execution Sequence" steps 1-3; § "Evidence Storage" (appended non-vacuity bullet)

CLAUDE.md § UT2, .claude/rules/general-unit-test.md, and .claude/rules/quality-tiers.md are byte-unchanged and were verified by a zero-line diff and matching SHA-256 against the merge base. AGENTS.md, .agents/**, .github/instructions/**, .github/agents/**, .codex/**, and .github/workflows/ci.yml are absent from the diff.

Executable carrier

  • scripts/vscode/Invoke-VSBuild.ps1: new -Target parameter, [ValidateSet('Build','Rebuild')], defaulting to Build so the existing build: task is unchanged. Get-MSBuildBuildArguments emits "/t:$Target" in the same array position. The -EnableNullable switch is retained for compatibility but now emits a deprecation Write-Warning instead of the Nullable=enable property.
  • .vscode/tasks.json: the lint: and type-check: tasks pass -Target Rebuild; the type-check: task drops -EnableNullable and retains -TreatWarningsAsErrors.

Tests

  • tests/scripts/vscode/Invoke-VSBuild.Tests.ps1: one new It asserting -Target Rebuild emits /t:Rebuild, and the nullable-mapping It rewritten to assert -EnableNullable emits no MSBuild property. Both were run red before the fix and green after.

Documentation and evidence

  • 50 evidence artifacts under docs/features/active/2026-08-10-csharp-toolchain-gate-fidelity-512/evidence/{baseline,qa-gates,regression-testing,issue-updates}/, each recording Timestamp:, Command:, EXIT_CODE: and Output Summary:.
  • Three audit artifacts: policy-audit, code-review, feature-audit.

Architecture / How It Fits Together

There are two carriers of the same command, and both had to move together.

The normative carrier is prose: CLAUDE.md and .claude/rules/csharp.md state the commands that every agent is required to run, and .claude/skills/csharp-qa-gate/SKILL.md is the gate procedure agents invoke. The executable carrier is scripts/vscode/Invoke-VSBuild.ps1, reached through .vscode/tasks.json, which is what a developer actually runs from the editor. Correcting only the prose would have left the tasks emitting /t:Build; correcting only the script would have left agents reading a defective instruction. Both are corrected, and the Pester suite pins the script's argument construction so the executable side cannot silently drift back.

.github/workflows/ci.yml is the third carrier and was already correct. It is the reference the other two are now reconciled against, and it is untouched by this PR.

One deliberate asymmetry remains and is documented in-line: CI's analyzer step uses /t:Build /m while the documented local command uses /t:Rebuild /m. A runner checkout is always cold, so /t:Build is non-vacuous there; a local working tree is not.

Verification

Completed

Language Step Command Result
C# format dotnet tool run csharpier format . EXIT_CODE: 0 — Formatted 1517 files
C# verify dotnet tool run csharpier check . EXIT_CODE: 0 — Checked 1517 files
C# analyze msbuild TaskMaster.sln /t:Rebuild /m /p:Configuration=Debug "/p:Platform=Any CPU" /p:EnableNETAnalyzers=true /p:EnforceCodeStyleInBuild=true EXIT_CODE: 0 — 0 Error(s), 0 CoreCompile skips
C# type-check msbuild TaskMaster.sln /t:Rebuild /m /p:Configuration=Debug "/p:Platform=Any CPU" /p:TreatWarningsAsErrors=true EXIT_CODE: 0 — 0 Error(s), 0 CoreCompile skips
PowerShell format PoshQC format (scripts/vscode, tests/scripts/vscode) EXIT_CODE: 0 — zero files rewritten
PowerShell analyze PoshQC analyze (same folders) EXIT_CODE: 1 — 16 findings, multiset identical to the merge-base baseline, zero new
PowerShell test PoshQC test (tests/scripts/vscode) EXIT_CODE: 0
PowerShell test + coverage Pester 5.6.1 over scripts/vscode/Invoke-VSBuild.ps1 41 passed / 0 failed, 85.71% line coverage, 100% changed-line coverage

Controls, both recorded as evidence:

  • Positive control. The corrected type-check command returns EXIT_CODE: 0 against an unperturbed clean checkout. The gate is passable.
  • Negative control. A deliberately introduced nullable violation in a #nullable enable production file (UtilitiesCS/Extensions/QueueExtensions.cs) causes the corrected gate to return non-zero with the expected CS86xx diagnostic. The perturbation was reverted; no *.cs file appears in this diff. The proof is non-vacuous — the log confirms the perturbed file's project was genuinely recompiled with zero CoreCompile skips.

All 13 acceptance criteria pass. Feature review reports 0 blocking findings, with each executor check-off independently re-verified against evidence rather than accepted.

Recommended

dotnet tool restore
dotnet tool run csharpier check .
msbuild TaskMaster.sln /t:Rebuild /m /p:Configuration=Debug "/p:Platform=Any CPU" /p:EnableNETAnalyzers=true /p:EnforceCodeStyleInBuild=true
msbuild TaskMaster.sln /t:Rebuild /m /p:Configuration=Debug "/p:Platform=Any CPU" /p:TreatWarningsAsErrors=true

Deliberate scope deviations, recorded rather than silent

  • No MSTest/vstest run. This PR changes no *.cs, *.csproj, *.props or *.targets file, and Run MSTest suite with coverage is failing on main for reasons unrelated to this feature. Recorded at plan task [P6-T9].
  • PoshQC analyze exits 1. 16 PSScriptAnalyzer findings pre-exist at the merge base, three of them inside Invoke-VSBuild.ps1 itself. The acceptance condition is a finding multiset identical to the baseline, which holds; EXIT_CODE: 0 was never asserted for that step.
  • AC2 substitution. The criterion's parenthetical suggested a csc.exe invocation count as the non-vacuity proof, but that count is zero at verbosity=normal even for genuine compiles. The zero-Skipping target "CoreCompile" assertion is used instead and is strictly more discriminating. Recorded as a formal deviation in spec.md; no criterion text was changed.

Backward Compatibility / Migration Notes

  • No breaking change to Invoke-VSBuild.ps1's interface. -Target defaults to Build, so the existing build: task and every unqualified caller behave exactly as before. -EnableNullable still binds; it now warns and emits nothing rather than failing.
  • Agents must run dotnet tool restore once per clone or worktree before the first CSharpier invocation. This is now stated at the documentation site.
  • The Nullable=enable property is no longer emitted by any documented command or VS Code task. Anything that depended on the forced solution-wide behavior will see the 195 latent diagnostics stop appearing — which was the intended state, since they were never enforceable.

Risks and Mitigations

Risk Mitigation
A future agent "restores" /p:Nullable=enable or /t:Build, reading the removal as a relaxation. This has already happened once. In-line rationale is carried at every corrected site, phrased as an explicit prohibition with the measured figures. The "strengthening, not a relaxation" argument is recorded in spec.md and above.
/t:Rebuild is slower than a warm /t:Build. This is the intended cost: the warm /t:Build was fast because it compiled nothing. /m is added to parallelize. Cold and warm elapsed times are recorded in the baseline evidence.
The corrected gate now surfaces 195 previously-hidden nullable diagnostics. It does not: those 195 appear only under /p:Nullable=enable, which the corrected command does not pass. The corrected gate is green on this branch. The 195 are measured and attributed for a follow-on epic.
Mirror instruction trees still carry the defective commands. Deliberate SD1 exclusion, enumerated with rationale; follow-up filed as #535.

Review Guide

Suggested order:

  1. CLAUDE.md, .claude/rules/csharp.md, .claude/skills/csharp-qa-gate/SKILL.md — the substantive change. Confirm each edit sits in the C# toolchain command block and that § UT2 is untouched.
  2. scripts/vscode/Invoke-VSBuild.ps1 and .vscode/tasks.json — the executable carrier, 16 and 4 changed lines respectively.
  3. tests/scripts/vscode/Invoke-VSBuild.Tests.ps1 — the pinning tests.
  4. evidence/qa-gates/typecheck-positive-control.* and typecheck-negative-control.* — the two controls that establish the corrected gate both passes and fails correctly. These are the load-bearing artifacts.
  5. evidence/qa-gates/no-relaxation-review.* and protected-files-zero-diff.* — the AC8/AC9 guards.

The bulk of the diff by line count is evidence artifacts, which are append-only records and can be skimmed.

Follow-ups

  • Nullable debt burn-down. The corrected gate under an explicit /p:Nullable=enable probe reports 195 errors, all attributed to UtilitiesCS.csproj: CS8766 x130, CS8618 x23, CS8625 x12, CS8600 x9, CS8601 x8, CS8604 x7, CS8602 x3, CS8603 x2, CS8714 x1. This figure is a lower bound — the build aborted after 22 of 73 CoreCompile executions, so UtilitiesCS's dependents never compiled. Sizing that epic should begin by measuring the solution-wide total. Explicitly out of scope here (Bug: nullable-gate-masked-by-incremental-build #492).
  • Mirror instruction trees. #535 tracks the .codex/**, .agents/** and .github/instructions/** copies that still document the defective commands.
  • Analyzer-step rationale. The /t:Rebuild versus CI's /t:Build rationale is carried at the normative sites but omitted at three condensed documentation sites; a reviewer noted spec.md SD2's prose and its replacement table are internally inconsistent on this point. Recommended to fold into Codex/Copilot instruction mirrors still document the CSharpier v0 command and the unpassable nullable command #535.

GitHub Auto-close

drmoisan and others added 2 commits August 10, 2026 23:41
The documented C# toolchain commands could not do what they claim. Three
independent defects made the format, analyzer and type-check gates either
unrunnable or unable to fail:

- #509: `dotnet tool run csharpier .` is CSharpier v0 syntax. dotnet-tools.json
  pins 1.2.6, which requires a `format`/`check` subcommand, so the documented
  form exits 1 with "Required command was not provided."
- #512/#492: `/t:Build` lets MSBuild's incremental up-to-date check skip
  CoreCompile on all 18 projects, because the check does not invalidate on a
  command-line `/p:` change. A warm run returned exit 0 having compiled
  nothing. Measured: 18 of 18 projects skipped.
- #522: `/p:Nullable=enable` is deliberately absent from ci.yml because
  nullable analysis here is per-file opt-in via `#nullable enable`. Forcing it
  solution-wide conscripts every un-annotated file and produces 195 errors in
  UtilitiesCS.csproj that are red on a clean main, so the documented gate could
  never pass.

Corrections, at the enumerated sites only:

- Format: `dotnet tool run csharpier format .` (verify with `check .`), always
  through `dotnet tool run` so the pinned version is used.
- Analyze and type-check: `/t:Rebuild /m`, with the type-check command now
  character-for-character ci.yml's.
- Rationale prose carried in-line at every corrected site so a future agent
  does not "restore" the removed flag.
- csharp-qa-gate SKILL.md now requires a `/fl` log with zero occurrences of
  `Skipping target "CoreCompile"`; exit 0 with a non-zero skip count is
  unverified, not passed.

Executable carrier: Invoke-VSBuild.ps1 gains a `-Target` parameter
(ValidateSet Build/Rebuild, default Build so the `build:` task is unchanged);
`-EnableNullable` is retained but now emits a deprecation warning instead of
the property. The lint and type-check VS Code tasks pass `-Target Rebuild`.

No policy requirement is relaxed. The change is a strengthening: gates that
returned exit 0 without compiling now compile, and a gate that could never
pass now can. The 195 nullable diagnostics the corrected gate exposes are
measured and attributed for a follow-on burn-down epic, not fixed here.

Files: CLAUDE.md (C# toolchain command block only; § UT2 byte-unchanged),
.claude/rules/csharp.md, .claude/skills/csharp-qa-gate/SKILL.md,
scripts/vscode/Invoke-VSBuild.ps1, .vscode/tasks.json, and the Pester tests.

Closes #512
Closes #492
Closes #509
Closes #522

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Feature review of bug/csharp-toolchain-gate-fidelity-512 against the
epic integration branch. Zero blocking findings; all 13 acceptance
criteria verified against evidence rather than accepted from the
executor's check-off.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@drmoisan
drmoisan merged commit 22eaee8 into epic/build-ci-coverage-gate-fidelity-integration Aug 11, 2026
drmoisan added a commit that referenced this pull request Aug 11, 2026
…g the final gate

512 merged via PR #540 (22eaee8), worktree removed. Wave 0 complete.

Records that the integrated-tree workflow_dispatch run failed on a single
intermittent test that also fails on main at this branch's base commit, with
the same 6435 total, and characterizes its wall-clock race mechanism.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
drmoisan added a commit that referenced this pull request Aug 15, 2026
Three updates following the delivery of issue #512 (PR #540):

- Mark the "CLAUDE.md nullable command diverges from ci.yml" memory
  RESOLVED. The documented C# commands now match ci.yml, so the old
  advice ("reproduce ci.yml's command before accepting a nullable
  blocker") no longer describes a live divergence. A future appearance
  of `/p:Nullable=enable` or `/t:Build` in a documented command is now a
  regression of #512/#522, and the memory says so. Also records the
  measured 195-error UtilitiesCS figure, with its lower-bound
  qualification, for the #492 burn-down.
- Add a memory for the untracked `coverage.xml` that PoshQC test runs
  drop at the repository root. It is in neither .gitignore nor
  .csharpierignore, so it inflates the CSharpier file count between two
  otherwise-identical runs and can be swept into a diff by `git add -A`.
- Correct the analyzer-vacuity memory: the non-vacuity assertion must be
  a zero `Skipping target "CoreCompile"` count, not a csc.exe count.
  csc.exe occurrences are zero at verbosity=normal even for genuine
  compiles, so the previously recorded csc-count acceptance would have
  been unsatisfiable.

The index was compacted concurrently by a sibling; this commit takes the
sibling's compaction as the base and applies only the three deltas above.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
drmoisan added a commit that referenced this pull request Aug 15, 2026
…g the final gate

512 merged via PR #540 (22eaee8), worktree removed. Wave 0 complete.

Records that the integrated-tree workflow_dispatch run failed on a single
intermittent test that also fails on main at this branch's base commit, with
the same 6435 total, and characterizes its wall-clock race mechanism.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@drmoisan
drmoisan deleted the bug/csharp-toolchain-gate-fidelity-512 branch August 15, 2026 02:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant