Merge branch 'claude/android-release'

.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,
-      # and this repository signs nothing yet, so proving the publish here is worth the minutes.
+      # on a Windows machine.
       #
       # 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
@@ -932,18 +1067,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
-# independent reasons, and both need saying because someone will fix one and think they are done.
+# There is no job here that publishes the desktop client, and there is not going to be one. It is built
+# 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
-# tooling, and every job in this file is runs-on: [linux]. A Windows runner would answer that.
+# ◆ ONE REASON, WHERE THIS ONCE CLAIMED TWO, AND THE SECOND WAS NOT TRUE.
 #
-# The larger one is that a Windows runner would not answer the other. 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. See docs/adr/0013-desktop-distribution-and-updates.md.
+# It used to say that vpk stamps and embeds the Setup.exe and Update.exe stubs with Windows tooling and
+# so could not run on a Linux runner, and offered that as the smaller of two reasons. It was wrong, and
+# worth recording as wrong because it is the kind of claim that discourages anybody from trying: vpk
+# cross-compiles when told to, `vpk [win] pack` on this runner builds the setup stub, the portable zip
+# and the .nupkg, and only signing needs Windows — which this repository does not do yet. That is what
+# 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
-the `Setup.exe` and `Update.exe` stubs with Windows tooling, and every job in `ci.yml` is
-`runs-on: [linux]`. Both reasons are recorded because somebody will fix one and believe they are done.
+A second, smaller reason was recorded here — that `vpk` stamps and embeds the `Setup.exe` and
+`Update.exe` stubs with Windows tooling, and every job in `ci.yml` is `runs-on: [linux]` — on the
+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.
-A `win-x64` publish runs on main and on tags, 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.
+◆ **That second reason was not true, and the correction is worth more than the claim was.** `vpk`
+cross-compiles when it is told to: the bracketed OS directive, `vpk [win] pack`, builds the setup stub,
+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
-themselves alone. That restore rewrites the lock files as a side effect, which does not matter on a runner
+publish and the `publish the windows desktop client` step in `ci.yml` — pass `-p:RestoreLockedMode=false`
+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.'
     }