Put the nightly stamp in the binaries, not just around them (#3455)

[erikdarlingdata]

Put the nightly stamp in the binaries, not just around them: pass the derived version to every nightly publish (#3455)

Closes #3455.

The nightly derives its version string once per run (#3390), asserts its shape at the source, hands it to every job — and then spends it entirely on NAMES. The zip filenames carry it, the release title carries it, vpk pack -v carries it, and not one of the five dotnet publish steps ever passed it to msbuild. So every assembly inside those correctly-named artifacts stamped whatever the version property resolves to at build time, which is the clean release string Directory.Build.props declares: a zip named PerformanceMonitorDarling-3.7.0-nightly.20260915.438.zip shipped a service whose AssemblyInformationalVersionAttribute read bare 3.7.0. The first live reading of #3453's service block — built precisely to answer "is the running service the build that carries fix X" — came back bare on all three stores that installed that nightly, which is the question answered with silence at the read built to answer it.

The MCP read needed no change and got none: DarlingCliCommands.ProductVersion() faithfully surfaces the attribute, prerelease kept, + build metadata stripped, and #3453's pins already say out loud that the nightly stamp lives in the prerelease. The attribute simply never received it. The #3390 stamp rationale claimed FileVersion / ProductVersion / InformationalVersion all carry the string — that sentence was aspirational the day it was written, the binaries never carried it, and it is corrected in place rather than left describing intent.

The property that matters, and why the fix is a flag and not a rewrite

-p:Version=<the derived nightly string> on each publish, nothing else. The SDK derives the informational version from the version property VERBATIM, and that attribute is the one surface a running build answers version questions from: the CLI's --version prints it and #3453's service.version serves the same method's result, so stamping the property stamps both consumers at once and they cannot disagree. AssemblyVersion and FileVersion derive as the numeric prefix (3.7.0.0) — identical to a release build, so nothing assembly-identity-shaped differs between a nightly and its release. A command-line global property outranks the Directory.Build.props declaration, which is why the flag works, and why an ambient environment variable would not (project-file properties outrank the environment).

Rewriting the props in the workspace instead was considered and refused: the nightly's own test step runs AFTER the version step, so a rewritten declaration would feed every source-parsing pin a prerelease string mid-run, and the linux job checks out its own tree and would need the rewrite repeated — two copies of a mutation versus five copies of a flag, and the flag is inert everywhere it isn't passed.

No +sha joins the stamp, deliberately. Directory.Build.props suppresses SourceLink's source-revision append tree-wide (#3222), so the informational version is exactly the nightly string and nothing more. That stays right on three counts: the stamp's run_number already identifies the build uniquely (the whole point of #3390); a sha would sort as an alphanumeric SemVer identifier and outrank every numeric one; and ProductVersion() strips a + suffix from display anyway, so a sha there would be invisible at precisely the surfaces this fix feeds. Had the suppression NOT existed, the attribute would have read 3.7.0+<sha> and the strip would have yielded the same bare 3.7.0 by a second route — the reproduction below confirms the suppression is live.

The container is the same defect one artifact over. The image is tagged :3.7.0-nightly.<date>.<run>, but its service compiles INSIDE docker build, on the other side of a boundary the workflow's flag cannot cross. The Dockerfile now declares ARG VERSION= — empty by default — and expands ${VERSION:+-p:Version=$VERSION} on its publish, so the nightly's --build-arg threads the stamp through while build.yml's release image and any hand-run docker build produce byte-for-byte the publish command they produced before the ARG existed (verified by running the expansion with the variable empty, set, and unset).

Release builds keep their clean string by construction

build.yml is untouched. Its publishes pass no version property, so release binaries keep stamping the one declaration the release process bumps and check-version-bump.yml gates — and that is now pinned rather than trusted: NightlyVersionInjectionTests.NoReleasePublish_PassesAVersionProperty fails the suite if a version property ever leaks into a release publish.

Tests

Darling.Tests/NightlyVersionInjectionTests (new), three facts: every dotnet publish in nightly.yml carries -p:Version=${{ steps.version.outputs.VERSION }} — the version step's own output, not a re-derivation — with a non-vacuity floor of five publish lines; no publish in build.yml passes a version property; and the container threading holds on BOTH sides of the image boundary (the workflow passes the build-arg, the Dockerfile declares the ARG and carries the conditional expansion), because each half alone reads as complete at its own file. The pin exists because this defect was a comment claiming a property the mechanics never had: prose describing intent outlives mechanics, and the local precedent is #2384's checksum fix landing in one workflow while the other drifted for months. nightly.yml and Darling/** are already in the darling change filter, so an edit to either file runs the job that runs these pins.

#3453's ServiceBuildSurfaceTests pins were checked against this change and constrain the read's MECHANICS, not the string's shape — prerelease kept, + stripped, leading component parses — and a nightly-stamped informational version satisfies all of them; the leading component of 3.7.0-nightly.20260915.438 is 3.7.0, which parses. ProductVersionDeclarationTests was re-run in spirit against the edited workflow: the two version reads nightly.yml holds are unchanged and still guarded, and none of the new comment prose spells the property in the form the read detector scores.

Reproduction, on the SDK the workflow uses (10.0.302)

The workflow cannot be run from a lane, so the property behavior was reproduced locally with the exact flag the workflow now passes, on the service project — the assembly ProductVersion() reflects over:

$ dotnet build Darling/.../PerformanceMonitor.Darling.Service.csproj -c Release -p:Version=3.7.0-nightly.20260915.438
  0 Warning(s), 0 Error(s)
$ grep InformationalVersion obj/Release/net10.0/PerformanceMonitor.Darling.Service.AssemblyInfo.cs
[assembly: System.Reflection.AssemblyInformationalVersionAttribute("3.7.0-nightly.20260915.438")]
$ dotnet PerformanceMonitor.Darling.Service.dll --version
3.7.0-nightly.20260915.438

AssemblyVersion and FileVersion both derived as 3.7.0.0, no compiler warnings, no +sha (the #3222 suppression held). The control build without the flag stamps 3.7.0 — the release path, unchanged. So the acceptance chain closes end to end: the nightly's service.version equals the artifact stamp, two same-day nightlies differ in the run_number component the stamp already carries, a release build keeps its clean string, and the CLI stays consistent with the MCP read because they were always the same attribute — that half was never broken, which is what made the miss purely upstream.

Downstream consumers of the now-prerelease informational version were audited: VersionText.Normalize (server-list display) strips prerelease by design, SingleInstanceDecision.ParseProductVersion parses the numeric core and documents the two-nightlies case by name, and UpdateCheckService compares the numeric AssemblyVersion, which this change keeps numeric. Nothing downstream parses the informational version expecting a bare semver.

[claude]

Minor robustness nit: ${VERSION:+-p:Version=$VERSION} expands unquoted on the RUN command line. With the only current caller (nightly.yml's derived date.run_number stamp) this is safe, but it relies on that invariant — a future caller passing --build-arg VERSION with a space or shell metacharacter (e.g. someone hand-testing with a branch name or free-form string) would get word-split/glob-expanded in the /bin/sh -c that runs this RUN. Quoting the expansion is a no-op for the current value and closes that off:

Suggested change

	-c Release -o /app/publish ${VERSION:+-p:Version=$VERSION}
	RUN dotnet publish Darling/PerformanceMonitor.Darling.Service/PerformanceMonitor.Darling.Service.csproj \
	-c Release -o /app/publish "${VERSION:+-p:Version=$VERSION}"

[erikdarlingdata]

Declining as theoretical for now: the sole caller passes a token that cannot contain whitespace (date.run_number), and the suggested fix — quoting the whole expansion — is itself wrong, since an empty VERSION would then pass a literal empty-string argument and break the byte-identical-command property this file's own comment pins. The correct form when this line is next touched is the inner-quoted ${VERSION:+"-p:Version=$VERSION"} (expansion unquoted, payload quoted), which passes zero args when unset and one arg when set. Recording it here so the next editor inherits the right fix instead of the plausible one.

[claude]

Reviewed. This PR is entirely CI/CD (nightly.yml, Darling/Dockerfile, new NightlyVersionInjectionTests) — no T-SQL, no Lite/Darling app-code parity concerns, so most of the usual SQL-style/parity checklist doesn't apply here.

Verified logically:

• Every nightly dotnet publish (4 Windows + 1 Linux) now carries -p:Version=${{ steps.version.outputs.VERSION }}, sourced from the one version step's output rather than re-derived — matches the "one authority" rule already used for artifact naming.
• build.yml's 6 release publishes correctly pass no version property, so release binaries keep stamping the Directory.Build.props version by construction — the new NoReleasePublish_PassesAVersionProperty test pins this both ways.
• Docker: nightly.yml passes --build-arg VERSION="${version}" (properly quoted), the Dockerfile declares ARG VERSION= (empty default) and conditionally expands ${VERSION:+-p:Version=$VERSION}, and build.yml's two docker build calls (PR smoke build + release image) pass no --build-arg, so they're unaffected — verified byte-identical publish command when the arg is absent.
• The new NightlyVersionInjectionTests correctly excludes comment lines before scanning for dotnet publish, has a non-vacuity floor (≥5 lines) so a broken scan can't vacuously pass, and is added to RepoFileAdoptionTests's tracked-file list.
• No +sha leaks into the informational version — IncludeSourceRevisionInInformationalVersion stays false in Directory.Build.props, consistent with the PR's claim.

Left one minor inline nit on Darling/Dockerfile (unquoted $VERSION expansion in the RUN line — safe today given the only caller's value shape, but worth quoting defensively).

No correctness bugs, no parity drift, no security issues beyond the minor quoting nit found.