fix(installer,tools,ci): review findings: other-session upgrade, stale WindUA, temp-dir elevation, CI installer build (#498)

.github/workflows/build.yml

@@ -15,27 +15,11 @@ jobs:
       - name: Build app (compile + link + manifest)
         shell: cmd
         run: build.bat
-      - name: Build and run unit tests
-        shell: cmd
-        run: build.bat test
-      # Everything release.yml builds must also break the PR, not main (review 2026-10-09 M7): the
-      # settings host and UI (build.bat config), the signed-variant compile (uiaccess) and the
-      # settings UI's Playwright suite. Node comes with the runner image, as in release.yml.
-      - name: Install UI dependencies
-        shell: cmd
-        working-directory: ui
-        run: npm ci
-      - name: Keybind rules, JS half (node --test)
-        shell: cmd
-        working-directory: ui
-        run: npm run test:rules
-      - name: Build settings host and UI (build/WindConfig.exe, ui/dist)
-        shell: cmd
-        run: build.bat config
-      - name: Build the UIAccess variant
-        shell: cmd
-        run: build.bat uiaccess
-      - name: UI tests (Playwright, headless)
-        shell: cmd
-        working-directory: ui
-        run: npx playwright install chromium && npx playwright test --reporter=line
+      # Everything release.yml builds must also break the PR, not main (review 2026-10-09 M7, and
+      # 2026-10-16: the installer itself). So the PR runs the same shared action release.yml and
+      # alpha.yml publish from: the unit tests, the keybind JS test, the Playwright UI suite and tools/release.ps1,
+      # which builds the UIAccess variant, the standard build, the settings host and UI, compiles
+      # the installer (makensis /WX) and runs tools/installer_check.ps1. The runner is
+      # administrator, so the silent install/uninstall round-trip runs too. Nothing is published.
+      - name: Build installer (shared with release.yml and alpha.yml)
+        uses: ./.github/actions/build-installer

.github/workflows/release.yml

@@ -19,18 +19,23 @@ name: release
 on:
   push:
     branches: [ main ]
-    # Docs cannot change the installer, so they do not need to republish it.
-    paths-ignore:
-      - '**.md'
-      - 'docs/**'
-      - '.github/ISSUE_TEMPLATE/**'
-      # Licence text cannot change the binary, and republishing would hand the current
-      # release a new asset hash for no reason, resetting what little SmartScreen
-      # reputation the existing download has earned.
+    # Docs cannot change the installer, so they do not need to republish it: a republish gives
+    # the current release a new asset hash for nothing, resetting what little SmartScreen
+    # reputation the existing download has earned. A `paths` list rather than paths-ignore,
+    # because only it can take a file back after a broader pattern dropped it (later lines win).
+    paths:
+      - '**'
+      - '!**.md'
+      - '!docs/**'
+      - '!.github/ISSUE_TEMPLATE/**'
+      # Test harness only. It cannot change the shipped binary.
+      - '!tools/testenv/**'
+      # Text that ships IN the installer, so a change to it must republish: the licence it
+      # shows and installs (wind.nsi, LICENSE.txt), the notices it installs, and the release
+      # notes this workflow publishes with it.
       - 'LICENSE'
-      # Test harness only. It cannot change the shipped binary, and a republish would give
-      # the current release a new asset hash for nothing.
-      - 'tools/testenv/**'
+      - 'THIRD-PARTY-NOTICES.md'
+      - '.github/release-notes.md'
   workflow_dispatch:
 
 permissions:

build.bat

@@ -68,6 +68,11 @@ rem --- Wind.exe itself, shared by the app and uiaccess targets. %1 = extra defi
 rem    %2 = the manifest to embed, %3 = its object dir under build\obj. Everything else (sources,
 rem    flags, libraries) is the same. Output: build\Wind.exe and build\Wind.pdb.
 :core_exe
+rem A new Wind.exe makes any build\WindUA.exe stale: the installer packs WindUA.exe as the
+rem Wind.exe it installs whenever the file exists (installer\wind.nsi, LOCAL_SIGN), so a leftover
+rem one would ship old code. tools\release.ps1 sets WIND_KEEP_UA for the one build that must keep
+rem the WindUA.exe it has just made from the same tree.
+if not "%WIND_KEEP_UA%"=="1" if exist "%BUILD%\WindUA.exe" del /q "%BUILD%\WindUA.exe"
 set "CORE_OBJ=%OBJ%\%~3"
 if not exist "%CORE_OBJ%" mkdir "%CORE_OBJ%"
 rem Compile the app-icon resource (rc.exe ships with the Windows SDK, on PATH via vcvars).

docs/architecture/11-build-test-release.md

@@ -87,11 +87,12 @@ headlessly, so the unit tests are the only verification loop that runs everywher
 the host's messages and records them in `window.__msgs`. When you add a bridge message to
 `HandleWebMessage`, extend the mocks of the specs that use it in the same change. Details in [09](09-settings-ui.md).
 
-**CI** (`.github/workflows/build.yml`, on pull requests and pushes to `main`) builds the app and
-runs the doctest suite, the JS node tests (`npm run test:rules`: keybind rules, config defaults and
-global keys) and the Playwright UI suite. Run the Playwright suite locally too when anything under
-`ui/` changes. The release and alpha workflows run the same gates before they publish (see Release
-automation).
+**CI** (`.github/workflows/build.yml`, on pull requests and pushes to `main`) builds the app, then
+runs the same `.github/actions/build-installer` the release and alpha workflows publish from: the
+doctest suite, the JS node tests (`npm run test:rules`: keybind rules, config defaults and global
+keys), the Playwright UI suite and `tools/release.ps1`, so an NSIS compile error or a failing
+`installer_check.ps1` breaks the PR, not main. Nothing is published from it. Run the Playwright suite
+locally too when anything under `ui/` changes.
 
 ## Local UIAccess deploy
 
@@ -100,7 +101,8 @@ elevated windows and the opt-in band-16 overlay. Windows grants it only to a bin
 locally trusted certificate and run from a secure location, in practice `C:\Program Files\Wind`.
 Without it, Wind reads `TokenUIAccess` at init and Auto keeps the desktop on Render.
 
-`tools/uiaccess_setup.ps1` (elevated) stops Wind, builds `uiaccess` and `config`, creates or reuses
+`tools/uiaccess_setup.ps1` (elevated) stops this session's Wind the way setup does (quit event,
+wait, kill only as a fallback; another session's Wind is left alone), builds `uiaccess` and `config`, creates or reuses
 a self-signed "Wind Dev Test Cert", trusts it, signs all three exes in `build\` (unsigned fresh
 builds trip a Defender false positive) and copies them with `ui\dist` to Program Files. It does not deploy an
 ini. It logs to `tools\uiaccess_setup.log`; a good deploy ends with `status=Valid` and `DONE`.
@@ -126,11 +128,15 @@ install-location chooser, because UIAccess is granted only in a secure location.
 3. **Stop Wind by asking, not killing.** `WIND_QUIT_RUNNING` sets `Local\Wind_QuitRequest` and waits
    for the process to exit; only a clean exit restores the OS cursor and releases `ClipCursor`. The
    single-instance mutex frees ~3 ms into teardown, long before the process is gone, so the macro
-   polls for the process before falling back to `taskkill`.
+   polls for the process before falling back to `taskkill`. That quit reaches this session only; a
+   Wind in another user's session would keep the exes locked, so the macro first asks to force-close
+   it (a silent run does), before touching this session's Wind, and stops setup without changes on a No.
 
 `tools/installer_check.ps1` checks what a compile cannot: every `File` source exists, every
 hit-tested rectangle was generated into `over.nsh`, and a silent install/uninstall round-trips
-(needs elevation; skipped from a normal shell, runs in CI).
+(needs elevation; skipped from a normal shell, runs in CI). It installs into
+`%ProgramFiles%\Wind-installer-check` (admin-only, because setup runs a staged script there as admin)
+and saves and restores a real install's registry values, signing roots and all-users shortcuts.
 
 ## Release automation
 
@@ -141,7 +147,7 @@ mechanical.
 
 ```mermaid
 flowchart TD
-    P[push to main] --> F{only docs, LICENSE or tools/testenv?}
+    P[push to main] --> F{only docs or tools/testenv?}
     F -- yes --> S[skip]
     F -- no --> V[read version from src/version.h]
     V --> T[build.bat test]
@@ -159,7 +165,8 @@ flowchart TD
 - Each release carries the installer twice: `Wind-Setup-x64-<ver>.exe` and the stable name
   `Wind-Setup-x64.exe`, so `releases/latest/download/Wind-Setup-x64.exe` always serves the newest.
 - Release notes come from `.github/release-notes.md`; the workflow fills `__VERSION__`,
-  `__SHA256__` and `__COMMIT__`.
+  `__SHA256__` and `__COMMIT__`. The trigger skips `*.md`, `docs/**` and `tools/testenv/**`, but takes
+  back what ships with the installer: `LICENSE`, `THIRD-PARTY-NOTICES.md` and the release notes.
 - Tests gate the build: the doctest suite, the JS keybind-rule test and the Playwright UI suite
   must all pass before anything is published. A concurrency group serializes runs.
 - The build itself (version read, test gates, `tools/release.ps1`) is the composite action
@@ -174,6 +181,9 @@ environment-driven: `WIND_SIGN_THUMBPRINT`, or `WIND_SIGN_PFX` plus `WIND_SIGN_P
 - Without one (CI today) it packs both variants: the ordinary build and the UIAccess build as
   `build\WindUA.exe` (the `uiaccess` build's `build\Wind.exe`, renamed). Setup signs `WindUA.exe` on each PC with a per-machine certificate whose private key
   it deletes at once (`installer/local-sign.ps1`). If that fails, setup keeps the ordinary build, says signing failed and does not start Wind: without UIAccess, Wind refuses to start ("Wind can't start", `src/core/uiaccess_gate.h`).
+  `WindUA.exe` is never left stale: every `Wind.exe` build deletes it (`build.bat :core_exe`) except
+  the one `release.ps1` makes from the same tree (`WIND_KEEP_UA=1`), and a certificate run deletes it
+  even with `-SkipBuild`.
 - **Never put the self-signed dev certificate in CI**: it is trusted by nobody and looks worse than
   no signature.
 

installer/README.md

@@ -56,7 +56,9 @@ which makes `wind.nsi` define `LOCAL_SIGN`. Setup then runs `local-sign.ps1`:
   over only after its signature verifies. An unsigned UIAccess `Wind.exe` does not start at all, so
   any failure leaves the ordinary build. Wind refuses to start without UIAccess (issue #460), so
   setup then shows an error and does not launch it.
-- A release signed with a real certificate has no `WindUA.exe` and skips this.
+- A release signed with a real certificate has no `WindUA.exe` and skips this. Any `Wind.exe` build
+  deletes `WindUA.exe` (`build.bat :core_exe`) unless `release.ps1` just made both, so `build.bat
+  installer` never packs a stale one.
 
 ## Licence screen
 

installer/app.nsh

@@ -43,12 +43,40 @@ Var LicenceDir   ; where "Read the full licence" put its copy, empty until then
   DetailPrint "Closing Wind..."
 
   ; Every Wind object here is session-local (the quit event, the mutex), and an HKLM Run entry
-  ; starts one Wind per logged-on user. So the process checks and the force-kill fallback below
-  ; are limited to this session; another user's Wind keeps running.
+  ; starts one Wind per logged-on user. So the clean quit and its force-kill fallback below are
+  ; limited to this session; another user's Wind is handled first, by asking.
   System::Call 'kernel32::GetCurrentProcessId() i .r0'
   System::Call 'kernel32::ProcessIdToSessionId(i r0, *i .r1)'
   StrCpy $WindSession $1
 
+  ; Another signed-in user's Wind runs the same exes from the same folder (the Run entry is
+  ; HKLM), so it keeps them locked: the copy or delete below would fail and leave a broken
+  ; install. Its quit event is in that session's namespace, out of reach from here, so the only
+  ; way to free the files is a forced close, and that is the user's call. A silent run closes it.
+  ; Asked FIRST, before this session's Wind is touched, so a No (or giving up) really does leave
+  ; everything as it was. Three tries, then stop: a close that keeps failing must not loop a
+  ; silent run forever.
+  StrCpy $3 0
+  ${Do}
+    nsExec::Exec 'cmd /c tasklist /FI "SESSION ne $WindSession" /NH | findstr /I /B "Wind.exe WindTray.exe WindConfig.exe"'
+    Pop $4
+    ${If} $4 != 0
+      ${Break}                 ; nothing running in any other session
+    ${EndIf}
+    ${If} $3 >= 3
+      MessageBox MB_ICONSTOP|MB_OK "Setup couldn't close Wind for another user signed in to this PC. Ask them to quit Wind, then run Setup again." /SD IDOK
+      Abort "Wind is still running for another user."
+    ${EndIf}
+    IntOp $3 $3 + 1
+    MessageBox MB_YESNO|MB_ICONEXCLAMATION "Wind is also running for another user signed in to this PC, and Setup can't update or remove it while it runs.$\r$\n$\r$\nClose it for them now? If you choose No, Setup stops without changing anything." /SD IDYES IDYES windCloseOthers
+    Abort "Wind is running for another user."
+    windCloseOthers:
+    DetailPrint "Closing Wind in other sessions..."
+    nsExec::Exec 'taskkill /IM WindConfig.exe /IM Wind.exe /IM WindTray.exe /FI "SESSION ne $WindSession" /F'
+    Pop $0
+    Sleep 500
+  ${Loop}
+
   ; WindConfig first, so it cannot relaunch Wind while we are stopping it. Force, because
   ; a WM_CLOSE with unsaved settings opens a confirm dialog that would hang setup
   ; (src\config_ui\main.cpp:490). It holds no OS state, so there is nothing to lose.
@@ -122,6 +150,7 @@ Var LicenceDir   ; where "Read the full licence" put its copy, empty until then
     nsExec::Exec 'taskkill /IM WindTray.exe /FI "SESSION eq $WindSession" /F'
     Pop $0
   ${EndIf}
+
   Sleep 300
 !macroend
 

tools/installer_check.ps1

@@ -138,9 +138,22 @@ if (-not (Test-Path $setup)) {
     $wasRunning = @(Get-Process -Name Wind, WindTray -ErrorAction SilentlyContinue |
         ForEach-Object { $_.Path } | Where-Object { $_ })
 
-    $scratch = Join-Path $env:TEMP 'wind-installer-check'
+    # Not under %TEMP%: with local signing, setup stages local-sign.ps1 and the uiAccess build in
+    # $INSTDIR\.stage and runs the script as admin, which is only safe in an admin-only folder
+    # (wind.nsi). A new folder in Program Files inherits its ACL, so nothing unelevated can write
+    # there, or create it in advance.
+    $scratch = Join-Path $env:ProgramFiles 'Wind-installer-check'
     if (Test-Path $scratch) { Remove-Item -LiteralPath $scratch -Recurse -Force }
 
+    # The smoke install points the all-users Start menu shortcut at $scratch, and the smoke
+    # uninstall deletes it and the all-users desktop one, so a real install's shortcuts are saved
+    # and put back like its registry values. (Bytes in memory: no file to restore from.)
+    $links = @(foreach ($f in 'CommonPrograms', 'CommonDesktopDirectory') {
+        $lnk = Join-Path ([Environment]::GetFolderPath($f)) 'Wind.lnk'
+        $raw = if (Test-Path -LiteralPath $lnk) { [IO.File]::ReadAllBytes($lnk) } else { $null }
+        [pscustomobject]@{ Path = $lnk; Raw = $raw }
+    })
+
     # The uninstaller must KEEP the user's settings unless they say otherwise, and a silent
     # uninstall answers its prompt with /SD IDNO. Testing that needs something to preserve: on a
     # machine that has never run Wind (every CI runner) the folder does not exist, and asserting
@@ -221,6 +234,12 @@ if (-not (Test-Path $setup)) {
 
         if (Test-Path $scratch) { Remove-Item -LiteralPath $scratch -Recurse -Force -ErrorAction SilentlyContinue }
 
+        # A shortcut that was here is put back; one that was not came from the smoke install.
+        foreach ($l in $links) {
+            if ($null -ne $l.Raw) { [IO.File]::WriteAllBytes($l.Path, [byte[]]$l.Raw) }
+            elseif (Test-Path -LiteralPath $l.Path) { Remove-Item -LiteralPath $l.Path -Force -ErrorAction SilentlyContinue }
+        }
+
         # Put back whatever was here before the smoke test.
         if ($savedArp) {
             New-Item -Path $ARP -Force | Out-Null

tools/release.ps1

@@ -89,19 +89,25 @@ if ($cert) {
 # build.bat puts every binary in build\ (never the repo root); the installer packs them from there.
 $bin = "$root\build"
 $ua = "$bin\WindUA.exe"
+# With a certificate the signed uiAccess build IS Wind.exe, so a WindUA.exe left over from an
+# earlier run must never reach the installer, -SkipBuild or not: its presence turns local
+# signing on, and setup would install it over the properly signed Wind.exe.
+if ($cert -and (Test-Path $ua)) { Remove-Item $ua -Force }
 if (-not $SkipBuild) {
     # No certificate: build the uiAccess variant as well, as WindUA.exe. Setup signs it on the
     # PC it installs to (installer\local-sign.ps1, issue #261) and keeps the standard build as
-    # the fallback. With a certificate the signed uiAccess build IS Wind.exe, so a WindUA.exe
-    # left over from an earlier run must not reach the installer.
+    # the fallback.
     if (Test-Path $ua) { Remove-Item $ua -Force }
     if (-not $cert) {
         Write-Host "=== building WindUA.exe (uiaccess, signed at install) ==="
         Invoke-Build 'uiaccess'
         Move-Item "$bin\Wind.exe" $ua -Force
     }
     Write-Host "=== building Wind.exe ($(if ($variant) { $variant } else { 'standard' })) ==="
-    Invoke-Build $variant
+    # Every Wind.exe build deletes build\WindUA.exe as stale (build.bat :core_exe); this one is
+    # from the same tree as the WindUA.exe just made, so it keeps it.
+    $env:WIND_KEEP_UA = '1'
+    try { Invoke-Build $variant } finally { Remove-Item Env:WIND_KEEP_UA -ErrorAction SilentlyContinue }
     Write-Host "=== building WindConfig.exe + ui\dist ==="
     Invoke-Build 'config'
 }