Package the Windows client in CI, on the runner that could not

The build job published a win-x64 tree and stopped there, so the half of a
release that fails in ways a compile cannot see was proved by nobody until a
person was midway through cutting one. It now packs as well: vpk opens the
published binaries and verifies the main executable really calls
VelopackApp.Build().Run(), which is the check worth having — a refactor that
drops that call compiles, tests green, and produces an application that silently
never updates itself.

The file said this was impossible on Linux, and also said it was fine, in
comments forty lines apart. The claim that vpk needs Windows tooling to stamp the
Setup.exe stub is the one that was wrong: vpk cross-compiles when told to, and
the telling is a bracketed directive before the verb rather than a flag. Plain
`vpk pack --runtime win-x64` on a Linux host refuses outright and says so in the
message that names the fix. `[win]` must be quoted, or the shell reads it as a
glob matching any one of w, i and n. Only signing needs Windows, and nothing here
is signed yet.

Fixing that does not move ADR 0013 rule 3 an inch, which is why the two reasons
were recorded separately in the first place. What may not live on a runner is the
token, not the build: Velopack clients apply what their feed serves without
verifying a signature, so whoever can write a release can ship an update every
install runs. The packages go to RUNNER_TEMP and die with the job. They are not
offered as workflow artefacts either — an installer nobody has run should not sit
somewhere that invites passing it on.

Written out as shell rather than by calling scripts/release-windows.ps1. That
script is a person's procedure and holds things a runner must not have and must
not skip: it refuses a dirty tree, insists HEAD is tagged, downloads the previous
release for deltas, and asks for the forge token. Calling it would mean either
weakening it with CI switches or having CI satisfy conditions that only make
sense at a desk. The constants the two now share — pack id, title, authors,
channel, icon — are a contract with VelopackUpdateChannel and with every
installed client, and both sides say so.

Two things fell out of running the steps rather than reading them, and both were
in code nothing had ever executed:

dotnet msbuild -getProperty:Version answers 1.0.0. Without a target named it
evaluates the project and runs nothing, and MinVer computes inside a target — so
the read comes back as the SDK default on a full checkout with every tag present.
That line is the tag check in this file, which is `if:` a tag ref, and there are
no tags yet: the first release ever cut would have been refused by its own guard,
which would then have blamed fetch-depth. release-windows.ps1 had the same line
and would have demanded HEAD be tagged v1.0.0. Both now pass -t:MinVer.

And MinVer answers 0.0.0-alpha.0.N until that first tag exists, which vpk rejects
outright as below 0.0.1 — so packing the true version could not have worked on
any build made today. The patch digit is lifted for the throwaway package only.
The release script gets no such floor and must not: its version is the one users
compare against, and there the refusal is the right outcome.

Verified by extracting both steps from this file and running them against a real
clone in a dotnet SDK container: Setup.exe, the portable zip, the .nupkg and
releases.win.json, from a machine that is not Windows.

.github/workflows/ci.yml

@@ -147,13 +147,25 @@ jobs:
       # green, the package is cut, and the symptom arrives weeks later as clients that never update.
       #
       # -getProperty evaluates without building, so this costs a second and runs before the build.
+      #
+      # ◆ -t:MinVer IS NOT OPTIONAL, AND WITHOUT IT THIS STEP FAILED EVERY TAG BUILD.
+      #
+      # -getProperty on its own evaluates the project and runs no targets, and MinVer sets Version
+      # from inside a target — so what came back was the SDK's default 1.0.0, for any tag, on any
+      # commit, in a clean checkout with every tag present. The step would have failed the first
+      # release ever cut and blamed fetch-depth for it, which is the one explanation that is not it.
+      # Naming a target makes -getProperty report the value after that target has run.
+      #
+      # Never exercised, because no v* tag exists yet: this whole step is `if:` a tag ref. It is the
+      # shape of bug a guard is prone to — the guard is the thing nothing tests, and it only speaks
+      # on the one build nobody is watching, which is the reason this step exists at all.
       - name: the tag and the version agree
         if: startsWith(github.ref, 'refs/tags/v')
         run: |
           set -euo pipefail
           tag="${GITHUB_REF#refs/tags/v}"
           declared="$(dotnet msbuild src/DodoSSH.Client.App/DodoSSH.Client.App.csproj \
-                        -getProperty:Version -nologo | tr -d '[:space:]')"
+                        -getProperty:Version -t:MinVer -nologo | tr -d '[:space:]')"
 
           if [ "$tag" != "$declared" ]; then
             echo "The tag says v$tag and MinVer computed $declared." >&2
@@ -214,8 +226,7 @@ jobs:
       # The one build shape nothing else here exercises: a self-contained RID-specific publish. Its
       # failure mode is a restore graph or a native asset that resolves for net10.0 and not for
       # net10.0/win-x64, which nobody would see until a person was halfway through cutting a release
-      # on a Windows machine. vpk can pack a Windows package from Linux; only signing needs Windows,
+      # on a Windows machine.
-      # and this repository signs nothing yet, so proving the publish here is worth the minutes.
       #
       # RestoreLockedMode=false for this command only, and it is not a loosened gate. The committed
       # lock files are deliberately RID-free: declaring win-x64 on the desktop head writes a
@@ -224,20 +235,144 @@ jobs:
       # under locked mode, so the image job would fail NU1004. The gate is the locked solution
       # restore at the top of this job, which is unchanged.
       #
-      # It rewrites the lock files as it goes; nothing after this step reads them, and the runner's
+      # It rewrites the lock files as it goes; nothing after it reads them, and the runner's
       # checkout is thrown away. The release script does the same thing and puts them back, because
       # there the tree is somebody's working copy.
       #
       # After the tests rather than before them, so a red suite does not first spend a hundred
       # megabytes pulling a win-x64 runtime pack. main and tags only, for the same reason: a break
       # found by the person about to release is found early enough.
-      - name: the windows publish still resolves
+      #
+      # The output directory is read by the packaging step below, so the two names have to agree.
+      - name: publish the windows desktop client
         if: github.event_name != 'pull_request'
         run: >
           dotnet publish src/DodoSSH.Client.App/DodoSSH.Client.App.csproj
           --configuration Release --runtime win-x64 --self-contained true
           -p:RestoreLockedMode=false
-          --output "$RUNNER_TEMP/win-x64-check"
+          --output "$RUNNER_TEMP/win-x64"
+
+      # ◆ THE WINDOWS INSTALLER IS BUILT HERE, ON LINUX, AND IS DELIBERATELY THROWN AWAY.
+      #
+      # Packaging rather than only publishing, for the same reason the android job packages an APK
+      # nobody installs: the failures a release is most exposed to are the ones only the packager
+      # finds. vpk opens the published binaries and verifies that the main executable really calls
+      # VelopackApp.Build().Run() — so a refactor that drops that call, which compiles, tests green
+      # and produces an application that silently never updates itself, fails here instead of
+      # shipping. A publish alone cannot see that, and the person who would otherwise see it first is
+      # the one midway through a release.
+      #
+      # ◆ NOT WITH scripts/release-windows.ps1. That script is a person's release procedure and holds
+      # things a runner must not have and must not skip — it refuses a dirty tree, insists HEAD is
+      # tagged, downloads the previous release to build deltas against, and in its second phase asks
+      # for the forge token. Calling it from here would mean either weakening it with CI switches or
+      # having CI meet conditions that only make sense at a desk. So the packaging arguments below
+      # are written out again in shell, and the duplication is accepted and stated: the pack id, the
+      # title, the authors, the channel and the icon are a contract with VelopackUpdateChannel and
+      # with every installed client. Change one, change the other.
+      #
+      # ◆ AND CROSS-COMPILING NEEDS THE OS DIRECTIVE, WHICH IS EASY TO MISS.
+      #
+      # vpk picks its target from the host: on Linux, plain `vpk pack` builds a Linux release and
+      # refuses a Windows RID outright — "the target rid must be Linux (actually was Windows)". The
+      # bracketed [win] before the verb is what turns it into a cross-compile, and it must be quoted
+      # or the shell reads it as a glob matching any one of w, i and n. With it, the runner writes
+      # DodoSSH.Desktop-win-Setup.exe, the portable zip, the full .nupkg and releases.win.json,
+      # exactly as a Windows machine does. Only signing needs Windows tooling, and nothing here is
+      # signed — see ADR 0013 rule 8 for the trigger that changes.
+      #
+      # ◆ NOTHING IS UPLOADED, AND NOTHING IS EVEN OFFERED FOR DOWNLOAD.
+      #
+      # The output goes to RUNNER_TEMP and dies with the job. That is the whole point rather than an
+      # omission: ADR 0013 rule 3 puts the capability to ship somebody a build on a machine which is
+      # not a runner, because Velopack clients apply what their feed serves without verifying a
+      # signature. Publishing a CI-built package as a workflow artefact would not cross that line by
+      # itself, and it would put an installer nobody has run in a place that invites passing it on —
+      # so it is not done either. What reaches users is built, installed and walked through Phase 16
+      # of docs/manual-checks.md by a person first.
+      - name: package the windows desktop client
+        if: github.event_name != 'pull_request'
+        run: |
+          set -euo pipefail
+
+          # vpk comes from the tool manifest, at the version committed there, which is the same one
+          # the release script uses. Restored here rather than in a step of its own because this is
+          # the only job that needs a tool at all.
+          dotnet tool restore
+
+          # The same value the release script packs with, read the same way. A version vpk disagrees
+          # with the assemblies about is a client that compares wrongly, so it is read from MSBuild
+          # rather than reconstructed from the ref — a main build has no tag to reconstruct from
+          # anyway, and MinVer's answer there carries the prerelease height that tells two of them
+          # apart.
+          #
+          # -t:MinVer for the reason spelled out on the tag check above: without a target named,
+          # -getProperty answers 1.0.0 and every package would carry the same version forever, which
+          # Velopack reads as "no update available".
+          version="$(dotnet msbuild src/DodoSSH.Client.App/DodoSSH.Client.App.csproj \
+                       -getProperty:Version -t:MinVer -nologo | tr -d '[:space:]')"
+
+          if [ -z "$version" ]; then
+            echo "Could not read the version from MSBuild." >&2
+            exit 1
+          fi
+
+          # ◆ AND A FLOOR UNDER IT, BECAUSE MINVER'S PRE-FIRST-TAG ANSWER IS ONE vpk REFUSES.
+          #
+          # Until a v* tag exists MinVer answers 0.0.0-alpha.0.N — correct, documented, and rejected
+          # outright by vpk: "Invalid package version '0.0.0-alpha.0.143', it must be >= 0.0.1". So
+          # today, and on every main build until the first release, packing the true version cannot
+          # work at all.
+          #
+          # The patch digit is lifted and the prerelease part kept, so the stand-in still sorts below
+          # anything real and still differs between commits. Sound only because this package is
+          # thrown away: what is being proved here is that the desktop client packages, and no client
+          # will ever see this number. The release script is not given a floor and must not be — its
+          # version is the one users compare against, and if MinVer ever answered 0.0.0 there the
+          # right outcome is the refusal.
+          packVersion="$version"
+          case "$packVersion" in
+            0.0.0*)
+              packVersion="0.0.1${packVersion#0.0.0}"
+              echo "MinVer says $version, which vpk will not pack; packaging as $packVersion."
+              echo "This disappears the moment a v* tag exists, and never reaches a release."
+              ;;
+          esac
+
+          releases="$RUNNER_TEMP/win-releases"
+
+          # --skip-updates because vpk otherwise asks nuget.org whether a newer vpk exists on every
+          # run, which is a network call whose answer this build must not act on: the version that
+          # packs is the one in the tool manifest.
+          #
+          # No `vpk download`, where the release script has one. That fetches the previous release so
+          # deltas can be built against it, which needs the feed and produces an artefact nobody
+          # applies. A full package proves the packaging.
+          dotnet vpk '[win]' pack \
+            --skip-updates \
+            --packId DodoSSH.Desktop \
+            --packVersion "$packVersion" \
+            --packDir "$RUNNER_TEMP/win-x64" \
+            --packTitle DodoSSH \
+            --packAuthors DodoTech \
+            --mainExe DodoSSH.exe \
+            --icon src/DodoSSH.Client.App/Assets/dodossh.ico \
+            --runtime win-x64 \
+            --channel win \
+            --outputDir "$releases"
+
+          # Asked for rather than assumed. vpk fails loudly and this costs a line, but the artefact
+          # whose existence is the entire question — a Windows setup stub built on a machine that is
+          # not Windows — is worth naming rather than inferring from an exit code.
+          setup="$releases/DodoSSH.Desktop-win-Setup.exe"
+          if [ ! -s "$setup" ]; then
+            echo "vpk reported success and there is no setup executable at $setup." >&2
+            ls -la "$releases" >&2 || true
+            exit 1
+          fi
+
+          ls -la "$releases"
+          echo "Packaged DodoSSH $packVersion for win-x64, from a build MinVer calls $version."
 
       # This includes the end-to-end suite, which starts PostgreSQL, Keycloak and an OpenSSH
       # server through Testcontainers and runs the API as a child process — so it needs a
@@ -892,18 +1027,27 @@ jobs:
         if: always() && github.event_name != 'pull_request'
         run: docker logout registry-docker.dodotech.cloud
 
-# There is no job here that publishes the desktop client, and there is not going to be one. Two
+# There is no job here that publishes the desktop client, and there is not going to be one. It is built
-# independent reasons, and both need saying because someone will fix one and think they are done.
+# and packaged here — the two steps at the end of the build job — and what is withheld is only the
+# upload.
 #
-# The smaller one is mechanical: vpk stamps and embeds the Setup.exe and Update.exe stubs with Windows
+# ◆ ONE REASON, WHERE THIS ONCE CLAIMED TWO, AND THE SECOND WAS NOT TRUE.
-# tooling, and every job in this file is runs-on: [linux]. A Windows runner would answer that.
 #
-# The larger one is that a Windows runner would not answer the other. Velopack clients fetch from the
+# It used to say that vpk stamps and embeds the Setup.exe and Update.exe stubs with Windows tooling and
-# release feed and do not verify a package signature when they apply it, so whoever can write a release
+# so could not run on a Linux runner, and offered that as the smaller of two reasons. It was wrong, and
-# on this repository can publish an update that every installed client downloads and runs. That is the
+# worth recording as wrong because it is the kind of claim that discourages anybody from trying: vpk
-# same capability as the signing key, reached through a different door — and docs/adr/0011 rule 1 puts
+# cross-compiles when told to, `vpk [win] pack` on this runner builds the setup stub, the portable zip
-# that capability on a machine which is not a runner, because a workflow secret is held by everyone who
+# and the .nupkg, and only signing needs Windows — which this repository does not do yet. That is what
-# can change a workflow file. See docs/adr/0013-desktop-distribution-and-updates.md.
+# the packaging step now does on every main and tag build.
+#
+# The reason that stands is the one that was never mechanical. Velopack clients fetch from the release
+# feed and do not verify a package signature when they apply it, so whoever can write a release on this
+# repository can publish an update that every installed client downloads and runs. That is the same
+# capability as the signing key, reached through a different door — and docs/adr/0011 rule 1 puts that
+# capability on a machine which is not a runner, because a workflow secret is held by everyone who can
+# change a workflow file. A Windows runner would never have answered it, which is why fixing the
+# mechanical half changes nothing about the division of labour.
+# See docs/adr/0013-desktop-distribution-and-updates.md.
 #
 # What cuts a release is scripts/release-windows.ps1, run by a person. What this file does is prove the
 # thing still builds and packages, which is the same division of labour the android job above already

docs/adr/0013-desktop-distribution-and-updates.md

@@ -85,13 +85,38 @@ packs and stops. Phase two, a separate invocation, asks for the token and upload
 what reaches users has been installed and started by a human first, and so that the credential is in memory
 only for the minutes that need it.
 
-There is a second, smaller reason the release could not be a CI job here anyway: `vpk` stamps and embeds
+A second, smaller reason was recorded here — that `vpk` stamps and embeds the `Setup.exe` and
-the `Setup.exe` and `Update.exe` stubs with Windows tooling, and every job in `ci.yml` is
+`Update.exe` stubs with Windows tooling, and every job in `ci.yml` is `runs-on: [linux]` — on the
-`runs-on: [linux]`. Both reasons are recorded because somebody will fix one and believe they are done.
+reasoning that somebody will fix one reason and believe they are done.
 
-What CI does gain is the same thing the `android` job already does — it proves the artefact still builds.
+◆ **That second reason was not true, and the correction is worth more than the claim was.** `vpk`
-A `win-x64` publish runs on main and on tags, so a restore graph that resolves for `net10.0` and not for
+cross-compiles when it is told to: the bracketed OS directive, `vpk [win] pack`, builds the setup stub,
-`net10.0/win-x64` fails there rather than under a person midway through a release.
+the portable bundle and the `.nupkg` on Linux, and reports `Directive enabled for cross-compiling from
+Linux (current os) to Windows` while doing it. Only signing needs Windows tooling, and rule 8 above is
+why nothing is signed yet. Plain `vpk pack` on Linux targets Linux and rejects a Windows RID, which is
+the shape of the mistake: the tool does not fail because it cannot, it fails because it was not asked.
+
+The correction does not move this rule at all, and that is the point of separating the two. The reason
+that stands was never mechanical, so a Linux runner that *can* pack changes nothing: what may not be on
+a runner is the token, not the build.
+
+What CI gains is the same thing the `android` job already does — it proves the artefact still builds and
+still packages. On main and on tags the build job publishes `win-x64` and then packs it, so a restore
+graph that resolves for `net10.0` and not for `net10.0/win-x64` fails there rather than under a person
+midway through a release. Packaging is the more valuable half and adds a few seconds to a publish that
+has already happened — measured at two to seven, against the minutes the publish itself takes: `vpk` verifies
+that the main executable actually calls `VelopackApp.Build().Run()`, so the change that quietly produces
+an application which never updates itself — compiling, tested, and invisible until a release goes
+unreceived — fails in CI. The packages are written to `RUNNER_TEMP` and discarded, and are deliberately
+not offered as workflow artefacts: an installer nobody has run should not sit somewhere that invites
+passing it on.
+
+CI does not call `scripts/release-windows.ps1` to do any of this, and should not. That script is the
+person's procedure — it refuses a dirty tree, requires a tagged `HEAD`, fetches the previous release for
+deltas, and prompts for the token — so running it on a runner would mean either weakening it with CI
+switches or having CI satisfy conditions that only make sense at a desk. The workflow writes the same
+`vpk` arguments out in shell, and the constants they share (pack id, title, authors, channel, icon) are
+noted in both places as a contract with `VelopackUpdateChannel` and with every installed client.
 
 ### 4. The update check points at the project's forge, and the address is a constant
 

docs/platform-flags.md

@@ -568,12 +568,37 @@ Packaging the desktop client had broken the server's image build, and nothing bu
 have caught it. Found by running `docker build` locally rather than by reading the lock files.
 
 So the RID stays out of the committed state, and the two commands that need one — the release script's
-publish and the `windows publish still resolves` step in `ci.yml` — pass `-p:RestoreLockedMode=false` for
+publish and the `publish the windows desktop client` step in `ci.yml` — pass `-p:RestoreLockedMode=false`
-themselves alone. That restore rewrites the lock files as a side effect, which does not matter on a runner
+for themselves alone. That restore rewrites the lock files as a side effect, which does not matter on a runner
 whose checkout is discarded and does matter on a developer's machine, so the release script runs
 `git checkout -- '*packages.lock.json'` afterwards. `-p:RestorePackagesWithLockFile=false` is not an
 alternative: it fails NU1005 whenever a lock file already exists.
 
+**`vpk` picks its target from the host, and cross-compiling is a bracketed directive rather than a flag.**
+The CI job packages the Windows desktop client on a Linux runner, and plain `vpk pack --runtime win-x64`
+there refuses outright:
+
+```
+To build packages for Linux, the target rid must be Linux (actually was Windows). If your real intention
+was to cross-compile a release for Windows then you should provide an OS directive: eg. 'vpk [win] pack ...'
+```
+
+The directive goes before the verb — `dotnet vpk '[win]' pack …` — and must be quoted in a POSIX shell,
+where `[win]` is a glob matching any one of `w`, `i` and `n`. With it, a Linux runner logs
+`Directive enabled for cross-compiling from Linux (current os) to Windows` and writes
+`DodoSSH.Desktop-win-Setup.exe`, the portable zip, the `.nupkg` and `releases.win.json` — the same set a
+Windows machine produces. `--runtime win-x64` is still required: the directive says which OS is being
+targeted, not which RID. Only signing needs Windows tooling, which is why `ci.yml` can package and
+`scripts/release-windows.ps1` will still be the thing that signs when there is a certificate to sign with.
+
+**`dotnet msbuild -getProperty:Version` answers `1.0.0`, and MinVer is not to blame.** `-getProperty`
+without a target *evaluates* the project and runs nothing, while MinVer computes the version inside a
+target — so the read comes back as the SDK default on a full checkout with every tag present, which looks
+exactly like a version that was never configured. `-t:MinVer` makes `-getProperty` report the value after
+that target has run, and both readers of it — the tag check in `ci.yml` and `scripts/release-windows.ps1`
+— pass it. Neither had, and neither had ever run: the CI check is `if:` a tag ref and there are no tags
+yet, so the first release would have been refused by its own guard, which would have blamed `fetch-depth`.
+
 **The Android head's lock file is outside the solution, so nothing checks it until the android job runs
 — and the android job was broken for an unrelated reason for the whole of the release that went stale.**
 `DodoSSH.Client.Android` is deliberately not in `DodoSSH.slnx` (it needs a workload the other two jobs

scripts/release-windows.ps1

@@ -100,7 +100,12 @@ Push-Location $RepoRoot
 try {
     # ---- What is being released -----------------------------------------------------------------------
 
-    $version = (& dotnet msbuild $Project -getProperty:Version -nologo) -replace '\s', ''
+    # -t:MinVer, and it is load-bearing. -getProperty on its own evaluates the project and runs no
+    # targets, while MinVer sets Version from inside one — so this read answered the SDK's default
+    # 1.0.0 regardless of the tag, and the check below would then have refused to build anything not
+    # tagged v1.0.0. CI's tag check had the same line and the same fault; both are fixed, and both
+    # say so, because this is the version that ends up in the package a client compares against.
+    $version = (& dotnet msbuild $Project -getProperty:Version -t:MinVer -nologo) -replace '\s', ''
     if ([string]::IsNullOrWhiteSpace($version)) {
         Stop-With 'Could not read the version from MSBuild.'
     }