Enable package validation - #83
Merged
Merged
Conversation
Package validation is enabled only on the core package and has no baseline anywhere, so nothing distinguishes the 0.7.0 breaking changes that break a 0.6.0 consumer from the ones that only break someone tracking main. Validation also hangs off Pack, which no workflow but release-on-nuget runs, so a shape break would surface in the publish job of an already-tagged release rather than during review. Measured against a 0.6.0 baseline, the whole solution produces exactly two consumer-visible breaks: the deliberate MetadataKind.Array and MetadataKind.Object renumbering. The rest of the breaking-change bullets are either behavioral, which ApiCompat cannot see, or describe removals of types that never shipped in 0.6.0. That makes the pre-release audit cheap enough to be worth doing now, when it is most useful. The blocker is strong naming: published packages are signed with a release-only secret, so an ordinary build fails the baseline with CP0003 on identity alone. Defaulting SignAssembly, PublicSign and the committed public key in src/Directory.Build.props resolves it and keeps the gate reachable as a bare dotnet pack, which is what makes it a feedback loop for coding agents rather than a CI-only check. The release workflow passes AssemblyOriginatorKeyFile as a global property, so the fallback disables itself and private-key signing is untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018Hxg7Ci3BXPMS8Q8K9J49J
The plan claimed the inner build and test loop was unaffected while also noting the baseline download, which cannot both be true. The SDK injects the PackageDownload from an evaluation-time ItemGroup in Microsoft.NET.ApiCompat.targets rather than from a target, so it participates in restore whether or not Pack runs: a plain dotnet restore fetches all seven 0.6.0 packages. Only validation execution hangs off Pack, so the accurate claim is that the loop is untouched after restore. Gating the baseline on packing was considered and rejected. It would depend on the undocumented _IsPacking property and would leave the CI restore without the baselines the --no-build pack step needs, for one cold download of seven small packages that NuGet and the CI cache keep. The opt-out is documented for restore instead. The same ItemGroup is conditioned on DisablePackageBaselineValidation, so passing it as a global property suppresses the download and the baseline check together; with the baselines purged from the cache, restore then fetches nothing and build and the full suite still pass. The lock file findings move here as well: no Download entry appears even after a forced restore, and locked mode is unaffected. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018Hxg7Ci3BXPMS8Q8K9J49J
The plan told AGENTS.md to document a clean run as printing only Successfully-created-package lines. Pack is incremental, so that is wrong in both directions: a second run with no source changes prints no package lines at all, and a --no-build repeat on an up-to-date tree prints nothing whatsoever and exits 0. An agent looking for those lines would read a silent, correct run as a failure. Success is now described as exit code zero and no CP diagnostics. The non-packable sample warning is removed rather than documented around. Microsoft.NET.Sdk.Web.ProjectSystem.props turns WarnOnPackingNonPackableProject on for web projects, which the sample is, so it alone warns on a solution pack; setting the property to false silences it. That warning is in the release workflow's pack output today, and removing it is what leaves a correct --no-build run silent instead of showing a lone warning as its only output. Incremental packing was checked separately and does not weaken the gate: altering a baseline member is caught on the next pack and the next --no-build pack. The plan records that most MetadataKind members are new in 0.7.0, so an injected regression has to target one of the seven the baseline actually contains. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018Hxg7Ci3BXPMS8Q8K9J49J
The plan claimed the existing CI cache already absorbs the baseline
download. It does not, and the gap does not close on its own. The
cache-nuget action keys on hashFiles('**/packages.lock.json'), and the
baseline PackageDownload provably leaves those files untouched, so the
key is unchanged by this work. actions/cache skips its post-job save on
an exact key hit, so the pre-existing entry - which predates the
baseline and lacks the seven packages - would be restored on every run,
the baselines re-downloaded every time, and the entry never refreshed.
This affects every run rather than only fresh runners, and
release-on-nuget.yml shares the same composite action.
The key now hashes src/Directory.Build.props alongside the lock files.
That is the correct dependency set: lock files describe what
PackageReference resolves to, and the props file is where
PackageValidationBaselineVersion lives. It rotates once when this change
lands, so the refreshed entry is saved with the baselines in it, and
again when the baseline moves to 0.7.0 at release. Hard-coding the
version into the key was rejected as duplication that goes stale the
first time someone bumps one without the other.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018Hxg7Ci3BXPMS8Q8K9J49J
The acceptance criterion still claimed dotnet build and dotnet test cost is unchanged. A cold run of either performs an implicit restore and therefore pays for the baseline packages, so only compilation and test execution are unaffected. The Technical Details already drew that line; the criterion now matches it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018Hxg7Ci3BXPMS8Q8K9J49J
DisablePackageBaselineValidation suppresses the baseline download and the baseline comparison, but RunPackageValidation still executes and still performs the compatible-framework checks. The plan described it as disabling validation at pack, which both overstates it and hides a desirable property: verified that a netstandard2.0-only public type is still reported as CP0001 with the baseline disabled, so the multi-targeted packages keep their asset-compatibility gate offline. Testing that also surfaced a defect in the escape hatch. With baseline validation off the two committed baseline suppressions are never matched, and unnecessary suppressions are an error by default, so dotnet pack -p:DisablePackageBaselineValidation=true failed with "Unnecessary suppressions found" on an otherwise correct tree. The permission is now tied to the escape hatch rather than granted globally, which keeps offline work to a single property while preserving the stale-suppression detection the release handover relies on. Both directions verified: offline pack succeeds, and an ordinary pack still fails on a stale suppression entry. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018Hxg7Ci3BXPMS8Q8K9J49J
The plan is roughly half its previous length. The condensation is a net improvement, and it corrects one claim of mine: 0.6.0 ships only a netstandard2.0 asset, so the net10.0 asset genuinely has no baseline counterpart, which is why both suppression entries name netstandard2.0. Four rationales were restored because each explains a decision that would otherwise be re-litigated or quietly undone: - Why the baseline is set before 0.7.0 rather than after. Otherwise the release with the largest breaking-change surface is the only one that never gets audited. - Why the local loop is a design constraint. A check living only in CI is one that agents and developers discover after pushing. - Why the signing properties are defaulted in the props file instead of passed on the command line. A gate reachable only through an invocation nobody remembers is not a local gate. - What is at stake in the -getProperty:PublicSign check. It is the one place where a mistake ships strong-named but unsigned packages. Also records a trap found while testing the injected regression that criterion asks for: an injected public API without XML doc comments fails Release with CS1591 before validation runs, and a grep for CP diagnostics hides that failure. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018Hxg7Ci3BXPMS8Q8K9J49J
All seven packable projects under src/ now validate against their published 0.6.0 package, so an undeclared change to the public API shape fails the build instead of reaching review as prose. Validation previously ran only on Light.PortableResults, without a baseline, and only when release-on-nuget.yml packed an already-tagged release. The gate is a bare `dotnet pack ./Light.PortableResults.slnx -c Release` with no extra arguments, properties or scripts, and CI runs the same thing with --no-build after the tests so a shape break cannot mask a test failure. Three things about the MSBuild setup are worth recording: - IsPackable has to be set explicitly in src/Directory.Build.props. Microsoft.NET.ApiCompat.targets is imported before the NuGet pack targets that default it to true, and the baseline PackageDownload is an evaluation-time item, so leaving IsPackable implicit silently acquires no baseline and validates nothing. Verified per project via -getItem:PackageDownload: seven declare [0.6.0], the non-packable source generator declares none. - Ordinary builds are public-signed with the committed public key, because the 0.6.0 packages are strong-named and unsigned builds would report CP0003 against every baseline. The guard reads AssemblyOriginatorKeyFile rather than SignAssembly, which the release also sets. Verified with -getProperty for both invocation shapes: ordinary builds get PublicSign=true, and the release invocation leaves both UsePublicSigningKey and PublicSign empty, so released packages stay genuinely private-signed. - The CI cache key now also hashes src/Directory.Build.props. PackageDownload never writes to packages.lock.json, so a lock-file-only key would not rotate on a baseline bump and an exact hit on the immutable cache could never acquire the new packages. The suppression file holds exactly the two intentional MetadataKind renumberings the release notes already describe; the generator produced no third entry. Unnecessary suppressions remain an error, which is what forces the file to be deleted when the baseline moves to 0.7.0. Verified beyond the criteria: an injected MetadataKind.String change fails clean packs, incremental packs, and the CI build-then-no-build- pack sequence, each with CP0011 and a non-zero exit; a stale suppression fails an ordinary pack; with all seven baselines removed from the NuGet cache, restore/build/test/pack succeed under -p:DisablePackageBaselineValidation=true without downloading any of them, while a netstandard2.0-only type still fails with CP0001, so the compatible-framework gate survives the escape hatch; locked restore is green with no lock-file diff; the suite is 2791/2791 and the Native AOT publish is clean under strong naming. The plan's suggested probe value does not compile - MetadataKind.String = 15 collides with an existing kind and yields CS0152 and CS8510 instead of a CP diagnostic, which is the masked-failure mode the plan warns about one sentence later. Recorded as a correction on the plan. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HQQpp9JcvNV1N8xfPpiWq9
10 tasks
Minimum allowed line rate is |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #82