From e1ec6e7b1ae6b3ac01c4962d0d3d316497eedaa3 Mon Sep 17 00:00:00 2001 From: Kevin Brown Date: Fri, 11 Sep 2026 16:51:49 -0600 Subject: [PATCH 1/2] Add a verified deploy script for the worker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Deploying was eight manual commands in an elevated shell, with three traps: the supervisor has to be stopped before the worker or it resurrects it, a running exe cannot be overwritten, and Start-ScheduledTask is a silent no-op while the task is still marked running. Deploy-FileMill.ps1 avoids all three by leaving the supervisor alone. Windows will not overwrite a running executable but will rename one, so the running binary is renamed aside (becoming the rollback copy), the new build moves into its place, and stopping the worker is enough — the supervisor relaunches it into the new build. Downtime is one restart and Task Scheduler is untouched. Nothing is assumed from an exit code: the new binary must report its version, the old worker must be gone, a new one must appear, and the new version must reach filemill.log. Otherwise the previous binary goes back, the log tail is printed, and it exits non-zero. Worker processes are matched by full path, so a second checkout is never touched. Test-DeployFileMill.ps1 runs it against a stub worker and stub supervisor in a temp directory: one good build, and one that exits at startup to prove the rollback. It needs no elevation and no modules. Build-FileMill.ps1 gains -Output so the deploy can build to a staging name. Co-Authored-By: Claude Opus 5 (1M context) --- README.md | 26 +++ scripts/Build-FileMill.ps1 | 19 ++- scripts/Deploy-FileMill.ps1 | 283 ++++++++++++++++++++++++++++++++ scripts/Test-DeployFileMill.ps1 | 196 ++++++++++++++++++++++ 4 files changed, 520 insertions(+), 4 deletions(-) create mode 100644 scripts/Deploy-FileMill.ps1 create mode 100644 scripts/Test-DeployFileMill.ps1 diff --git a/README.md b/README.md index 08664ca..e9758a6 100644 --- a/README.md +++ b/README.md @@ -175,6 +175,32 @@ The task also overrides two Task Scheduler defaults that are wrong for a laptop: without them Windows refuses to start the task on battery and stops it the moment you unplug. +### Deploy a new build + +With the worker running under the supervisor, deploy from an **elevated** +PowerShell: + +```powershell +.\scripts\Deploy-FileMill.ps1 +``` + +It builds and tests the current checkout, keeps the running binary as +`bin\filemill..exe`, moves the new one into place, and stops the +worker — the supervisor relaunches it into the new build, so the downtime is a +single restart and Task Scheduler is not involved. Nothing is taken on trust: +the new binary has to report its version, a new worker has to appear, and the +new version has to show up in `filemill.log`. If it doesn't, the previous +binary goes back and the script exits non-zero. + +Elevation is required for the same reason as above: the worker runs in session +0. Stopping it interrupts any job it is running — the restarted worker marks +that job interrupted, and its sender's reply asks them to send the file again. + +`scripts\Test-DeployFileMill.ps1` exercises the deploy against a stub worker in +a temporary directory (including a build that refuses to start, to prove the +rollback), so the script can be changed without experimenting on the installed +service. + To remove the automatic start later (this also stops the running supervisor and worker): diff --git a/scripts/Build-FileMill.ps1 b/scripts/Build-FileMill.ps1 index 5035c62..47dd400 100644 --- a/scripts/Build-FileMill.ps1 +++ b/scripts/Build-FileMill.ps1 @@ -1,19 +1,30 @@ [CmdletBinding()] -param() +param( + # Where to write the binary. Defaults to bin\filemill.exe. Deploy-FileMill.ps1 + # passes a staging name instead, because Windows will not let a running + # executable be overwritten. + [string]$Output +) $ErrorActionPreference = 'Stop' $repositoryRoot = Split-Path -Parent $PSScriptRoot +if (-not $Output) { + $Output = 'bin\filemill.exe' +} Push-Location -LiteralPath $repositoryRoot try { $describe = git describe --tags --dirty --always 2>$null if ($LASTEXITCODE -ne 0 -or [string]::IsNullOrWhiteSpace($describe)) { Write-Warning 'git describe failed (no commits or tags reachable?) - building without a stamped version' - go build -o bin\filemill.exe .\cmd\filemill + go build -o $Output .\cmd\filemill } else { - Write-Host "Building filemill $describe" - go build -ldflags "-X main.version=$describe" -o bin\filemill.exe .\cmd\filemill + Write-Host "Building filemill $describe -> $Output" + go build -ldflags "-X main.version=$describe" -o $Output .\cmd\filemill + } + if ($LASTEXITCODE -ne 0) { + throw "go build failed with exit code $LASTEXITCODE" } } finally { Pop-Location diff --git a/scripts/Deploy-FileMill.ps1 b/scripts/Deploy-FileMill.ps1 new file mode 100644 index 0000000..5680e46 --- /dev/null +++ b/scripts/Deploy-FileMill.ps1 @@ -0,0 +1,283 @@ +<# +.SYNOPSIS + Deploys a fresh build of the FileMill worker and verifies it is serving, + rolling back if it is not. + +.DESCRIPTION + The supervisor is left alone. Windows will not let a running executable be + overwritten, but it will let one be renamed, so a deploy is: + + rename bin\filemill.exe -> bin\filemill..exe (the rollback copy) + move the new build -> bin\filemill.exe + kill the worker, and the supervisor relaunches it into the new binary + + That is one worker restart of downtime, no Task Scheduler interaction (and + so no way to trip over MultipleInstances IgnoreNew, which makes + Start-ScheduledTask a silent no-op while a task is already running), and the + previous binary is left beside the new one. + + Nothing is assumed from an exit code alone: the new binary must run and + report its version, the old worker must be gone, a new one must appear, and + the worker's own log must show the new version serving. If any of that + fails, the previous binary goes back and the script exits non-zero. + + Requires elevation: the worker runs in session 0 (see + Install-FileMillScheduledTask.ps1) and an unelevated Stop-Process on it + fails with "Access is denied". + + Killing the worker interrupts a job it is running. The restarted worker + marks that job interrupted, and the sender's reply asks them to send the + file again. + +.EXAMPLE + .\scripts\Deploy-FileMill.ps1 + Builds from the current checkout, runs the tests, and deploys. + +.EXAMPLE + .\scripts\Deploy-FileMill.ps1 -NewBinary .\bin\filemill.new.exe -Yes + Deploys a binary you built beforehand (handy to keep `go build` unelevated). +#> +[CmdletBinding()] +param( + # The FileMill checkout to deploy. Defaults to this script's repository. + [string]$RepositoryRoot, + # Deploy this build instead of building one. + [string]$NewBinary, + [switch]$SkipBuild, + [switch]$SkipTests, + # Deploy even though the working tree has uncommitted changes. + [switch]$AllowDirty, + # Don't wait for the new version to appear in filemill.log. That line is + # written by the Mailgun adapter, so a worker with no Mailgun configuration + # never prints it. + [switch]$SkipLogCheck, + # Don't ask before restarting the worker. + [switch]$Yes, + [int]$TimeoutSeconds = 90, + # Testing only: the harness drives this against a fake worker in its own + # session, which needs no elevation. + [switch]$NoElevationCheck +) + +$ErrorActionPreference = 'Stop' + +function Write-Step { param([string]$Message) Write-Host "==> $Message" } + +function Fail { + param([string]$Message) + Write-Host "DEPLOY FAILED: $Message" -ForegroundColor Red + exit 1 +} + +# Wait-Until polls Condition until it returns true or Seconds elapse. Every +# check in this script is a wait-for-a-fact, never a fixed sleep: on this +# machine a command returning cleanly has more than once meant nothing happened. +function Wait-Until { + param([scriptblock]$Condition, [int]$Seconds) + $deadline = (Get-Date).AddSeconds($Seconds) + while ((Get-Date) -lt $deadline) { + if (& $Condition) { return $true } + Start-Sleep -Milliseconds 500 + } + return $false +} + +# Get-Workers finds worker processes running one specific binary. Matching on +# the path, not just the name, keeps a second checkout's worker (or a test +# harness's fake) out of it. +function Get-Workers { + param([string]$Path) + # Not $matches: that is a PowerShell automatic variable, written by -match. + $found = Get-CimInstance Win32_Process -Filter "Name='filemill.exe'" -ErrorAction SilentlyContinue | + Where-Object { $_.ExecutablePath -and ($_.ExecutablePath -ieq $Path) } + return @($found) +} + +# Get-Version runs a binary's --version. It is also the proof that the file is +# a working executable before anything is swapped. +function Get-Version { + param([string]$Path) + $output = & $Path --version 2>&1 + if ($LASTEXITCODE -ne 0) { return $null } + $text = ($output | Select-Object -First 1) + if ([string]::IsNullOrWhiteSpace($text)) { return $null } + return ($text.Trim() -split '\s+')[-1] +} + +# Read-LogSince reads what a log file has gained since an offset, sharing the +# file with the worker that is writing it. +function Read-LogSince { + param([string]$Path, [long]$Offset) + if (-not (Test-Path -LiteralPath $Path)) { return '' } + $stream = [System.IO.File]::Open($Path, [System.IO.FileMode]::Open, [System.IO.FileAccess]::Read, [System.IO.FileShare]::ReadWrite) + try { + if ($Offset -gt $stream.Length) { $Offset = 0 } # the log was rotated or truncated + $stream.Seek($Offset, [System.IO.SeekOrigin]::Begin) | Out-Null + $reader = New-Object System.IO.StreamReader($stream) + return $reader.ReadToEnd() + } finally { + $stream.Dispose() + } +} + +function Get-LogLength { + param([string]$Path) + if (Test-Path -LiteralPath $Path) { return (Get-Item -LiteralPath $Path).Length } + return 0 +} + +if (-not $NoElevationCheck) { + $identity = [Security.Principal.WindowsPrincipal][Security.Principal.WindowsIdentity]::GetCurrent() + if (-not $identity.IsInRole([Security.Principal.WindowsBuiltInRole]::Administrator)) { + Fail 'Run this from an elevated PowerShell (Run as administrator). The worker runs in session 0, and stopping it needs elevation.' + } +} + +if (-not $RepositoryRoot) { $RepositoryRoot = Split-Path -Parent $PSScriptRoot } +$RepositoryRoot = (Resolve-Path -LiteralPath $RepositoryRoot).Path +$binary = Join-Path $RepositoryRoot 'bin\filemill.exe' +$logPath = Join-Path $RepositoryRoot 'data\logs\filemill.log' + +# --- preflight: everything that can fail before anything is touched --------- + +if (-not (Test-Path -LiteralPath $binary -PathType Leaf)) { + Fail "No worker binary at $binary. Build one first with .\scripts\Build-FileMill.ps1" +} +$oldVersion = Get-Version $binary +if (-not $oldVersion) { Fail "The current binary at $binary did not report a version." } + +if ($NewBinary) { + $SkipBuild = $true +} else { + $NewBinary = Join-Path $RepositoryRoot 'bin\filemill.new.exe' +} + +if (-not $SkipBuild) { + Push-Location -LiteralPath $RepositoryRoot + try { + $dirty = git status --porcelain + if ($LASTEXITCODE -ne 0) { Fail 'git status failed; is this a checkout?' } + if ($dirty -and -not $AllowDirty) { + Fail "The working tree has uncommitted changes, so the build would not match any commit. Commit them, or pass -AllowDirty.`n$($dirty -join "`n")" + } + if (-not $SkipTests) { + Write-Step 'Running go test ./...' + go test ./... + if ($LASTEXITCODE -ne 0) { Fail 'Tests failed; nothing was deployed.' } + } + Write-Step 'Building' + & (Join-Path $PSScriptRoot 'Build-FileMill.ps1') -Output $NewBinary + } finally { + Pop-Location + } +} + +if (-not (Test-Path -LiteralPath $NewBinary -PathType Leaf)) { + Fail "No new build at $NewBinary." +} +$NewBinary = (Resolve-Path -LiteralPath $NewBinary).Path +$newVersion = Get-Version $NewBinary +if (-not $newVersion) { Fail "The new build at $NewBinary did not report a version, so it is not a working executable." } + +$workers = Get-Workers $binary +Write-Host '' +Write-Host " repository: $RepositoryRoot" +Write-Host " running: $oldVersion (worker PID(s): $(if ($workers.Count) { ($workers | ForEach-Object { $_.ProcessId }) -join ', ' } else { 'none' }))" +Write-Host " deploying: $newVersion" +Write-Host '' + +if (-not $Yes) { + $answer = Read-Host 'Restart the worker into the new build? Any job it is running will be interrupted. [y/N]' + if ($answer -notmatch '^(y|yes)$') { Write-Host 'Nothing was changed.'; exit 0 } +} + +# --- swap: rename the running binary aside, move the new one in ------------- + +$backup = Join-Path $RepositoryRoot ('bin\filemill.{0}.exe' -f ($oldVersion -replace '[^\w\.\-]', '_')) +Write-Step "Keeping the current binary as $(Split-Path -Leaf $backup)" +Move-Item -LiteralPath $binary -Destination $backup -Force +try { + Move-Item -LiteralPath $NewBinary -Destination $binary -Force +} catch { + Move-Item -LiteralPath $backup -Destination $binary -Force + Fail "Could not put the new build in place, so the old one stayed: $_" +} + +# Restore-Previous puts the old binary back and restarts the worker into it. +# Called when the new build does not come up. +function Restore-Previous { + param([string]$Reason) + Write-Host "Rolling back: $Reason" -ForegroundColor Yellow + # What the worker managed to say before it failed is the most useful thing + # on the screen at this point. + $tail = Read-LogSince $logPath $logOffset + if ($tail) { + Write-Host 'filemill.log since the restart:' -ForegroundColor Yellow + $tail -split "`r?`n" | Where-Object { $_ } | Select-Object -Last 8 | ForEach-Object { Write-Host " $_" } + } + $failed = Join-Path $RepositoryRoot 'bin\filemill.failed.exe' + Move-Item -LiteralPath $binary -Destination $failed -Force + Move-Item -LiteralPath $backup -Destination $binary -Force + foreach ($worker in (Get-Workers $binary)) { + Stop-Process -Id $worker.ProcessId -Force -ErrorAction SilentlyContinue + } + $back = Wait-Until -Seconds $TimeoutSeconds -Condition { + $running = Get-Workers $binary + ($running.Count -gt 0) -and ((Get-Version $binary) -eq $oldVersion) + } + if ($back) { + Fail "$Reason. Rolled back to $oldVersion; the build that failed is at $failed." + } + Fail "$Reason. Rolled back to $oldVersion, but no worker came back — start it with: Start-ScheduledTask -TaskName 'FileMill Worker'. The build that failed is at $failed." +} + +# --- restart: kill the worker and let the supervisor relaunch it ------------ + +if ($workers.Count -eq 0) { + Write-Host '' + Write-Host "Deployed $newVersion. No worker was running, so nothing was restarted." -ForegroundColor Green + Write-Host "Start it with: Start-ScheduledTask -TaskName 'FileMill Worker'" + exit 0 +} + +$logOffset = Get-LogLength $logPath +$oldPids = @($workers | ForEach-Object { $_.ProcessId }) +Write-Step "Stopping the worker (PID $($oldPids -join ', ')); the supervisor will relaunch it" +foreach ($worker in $workers) { + Stop-Process -Id $worker.ProcessId -Force +} +if (-not (Wait-Until -Seconds $TimeoutSeconds -Condition { (Get-Workers $binary | Where-Object { $oldPids -contains $_.ProcessId }).Count -eq 0 })) { + Restore-Previous 'The old worker did not stop' +} + +Write-Step 'Waiting for the supervisor to start the new worker' +if (-not (Wait-Until -Seconds $TimeoutSeconds -Condition { (Get-Workers $binary | Where-Object { $oldPids -notcontains $_.ProcessId }).Count -gt 0 })) { + # Two causes look identical from here, and a build that crashes at startup + # can exit before it is ever seen in the process list, so name both rather + # than guessing: the log tail printed below usually settles it. + Restore-Previous 'No new worker stayed up. Either the new build exits at startup, or no supervisor is running to relaunch it (Start-ScheduledTask -TaskName ''FileMill Worker'')' +} +$newWorkers = Get-Workers $binary | Where-Object { $oldPids -notcontains $_.ProcessId } + +# --- verify: the new version is the one serving ----------------------------- + +if (-not $SkipLogCheck) { + Write-Step "Waiting for $newVersion to appear in filemill.log" + if (-not (Wait-Until -Seconds $TimeoutSeconds -Condition { (Read-LogSince $logPath $logOffset) -match [regex]::Escape($newVersion) })) { + Restore-Previous "The new worker did not report $newVersion in $logPath within $TimeoutSeconds seconds" + } +} + +# A worker that started and then died leaves a PID that no longer exists, so +# this is checked after the log line, not instead of it. +if ((Get-Workers $binary).Count -eq 0) { + Restore-Previous 'The new worker started and then exited' +} + +Write-Host '' +Write-Host "Deployed $oldVersion -> $newVersion" -ForegroundColor Green +Write-Host " worker PID(s): $(($newWorkers | ForEach-Object { $_.ProcessId }) -join ', ')" +Write-Host " previous binary kept at: $backup" +$startup = (Read-LogSince $logPath $logOffset) -split "`r?`n" | Where-Object { $_ -match [regex]::Escape($newVersion) } | Select-Object -First 1 +if ($startup) { Write-Host " $($startup.Trim())" } +exit 0 diff --git a/scripts/Test-DeployFileMill.ps1 b/scripts/Test-DeployFileMill.ps1 new file mode 100644 index 0000000..3f1cbf1 --- /dev/null +++ b/scripts/Test-DeployFileMill.ps1 @@ -0,0 +1,196 @@ +<# +.SYNOPSIS + Exercises Deploy-FileMill.ps1 against a fake worker in a temporary + directory, so the real deploy is not the first time it runs. + +.DESCRIPTION + Sets up a throwaway "repository": a bin\ holding a stub worker, a + data\logs\ for it to write to, and a stub supervisor that relaunches the + worker whenever it exits — the same shape as the real install, with none of + its parts. The stub worker is a tiny Go program built three times with + different stamped versions, one of which fails to start on purpose. + + Two cases are covered: + + 1. A good build is deployed: the binary is swapped, the worker restarts + into it, the new version reaches the log, and the previous binary is + kept. + 2. A build that exits immediately is deployed: the deploy must notice + that the new version never comes up, roll back, and exit non-zero, + leaving the previous worker serving again. + + It needs no elevation (the stub worker runs in this session) and no modules. + Exits 0 if every check passes, 1 otherwise. +#> +[CmdletBinding()] +param( + # Where to build the throwaway repository. Removed afterwards unless -Keep. + [string]$WorkRoot, + [switch]$Keep +) + +$ErrorActionPreference = 'Stop' + +$script:failures = @() +function Check { + param([string]$What, [bool]$Ok, [string]$Detail) + if ($Ok) { + Write-Host " ok $What" -ForegroundColor Green + } else { + Write-Host " FAIL $What" -ForegroundColor Red + if ($Detail) { Write-Host " $Detail" } + $script:failures += $What + } +} + +function Wait-Until { + param([scriptblock]$Condition, [int]$Seconds = 30) + $deadline = (Get-Date).AddSeconds($Seconds) + while ((Get-Date) -lt $deadline) { + if (& $Condition) { return $true } + Start-Sleep -Milliseconds 200 + } + return $false +} + +function Get-FakeWorkers { + param([string]$Path) + return @(Get-CimInstance Win32_Process -Filter "Name='filemill.exe'" -ErrorAction SilentlyContinue | + Where-Object { $_.ExecutablePath -and ($_.ExecutablePath -ieq $Path) }) +} + +$deployScript = Join-Path $PSScriptRoot 'Deploy-FileMill.ps1' +if (-not (Test-Path -LiteralPath $deployScript)) { throw "Deploy-FileMill.ps1 not found next to this script." } + +if (-not $WorkRoot) { $WorkRoot = Join-Path $env:TEMP ("filemill-deploy-test-" + [guid]::NewGuid().ToString('N').Substring(0, 8)) } +$binDir = Join-Path $WorkRoot 'bin' +$logDir = Join-Path $WorkRoot 'data\logs' +$srcDir = Join-Path $WorkRoot 'src' +$stage = Join-Path $WorkRoot 'stage' +$binary = Join-Path $binDir 'filemill.exe' +$logPath = Join-Path $logDir 'filemill.log' +foreach ($dir in @($binDir, $logDir, $srcDir, $stage)) { New-Item -ItemType Directory -Force -Path $dir | Out-Null } + +# The stub worker: prints its version like the real one, appends the same +# startup line the Mailgun adapter logs, then idles until it is killed. Built +# with version "bad" it exits at once, which is the failure this has to catch. +$fakeSource = @' +package main + +import ( + "fmt" + "os" + "path/filepath" + "time" +) + +var version = "dev" + +func logLine(text string) { + path := filepath.Join("data", "logs", "filemill.log") + f, err := os.OpenFile(path, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0644) + if err != nil { + return + } + defer f.Close() + fmt.Fprintf(f, "%s %s\n", time.Now().UTC().Format("2006/01/02 15:04:05"), text) +} + +func main() { + if len(os.Args) > 1 && os.Args[1] == "--version" { + fmt.Println("filemill " + version) + return + } + if version == "bad" { + // A build that dies at startup, as a bad config or a failed bind + // would: it says why in the log, then exits. + logLine("filemill: startup failed: stub worker refusing to run") + os.Exit(3) + } + logLine(fmt.Sprintf("mailgun FileMill %s - webhook listening on :8080; delivery loop started", version)) + for { + time.Sleep(time.Second) + } +} +'@ +Set-Content -Path (Join-Path $srcDir 'main.go') -Value $fakeSource -Encoding ascii +Set-Content -Path (Join-Path $srcDir 'go.mod') -Value "module fakemill`n`ngo 1.26.0`n" -Encoding ascii + +Write-Host "Building stub workers in $WorkRoot" +Push-Location -LiteralPath $srcDir +try { + foreach ($build in @( + @{ Version = 'v-old'; Path = $binary }, + @{ Version = 'v-new'; Path = (Join-Path $stage 'filemill.new.exe') }, + @{ Version = 'bad'; Path = (Join-Path $stage 'filemill.bad.exe') })) { + go build -ldflags "-X main.version=$($build.Version)" -o $build.Path . + if ($LASTEXITCODE -ne 0) { throw "building the stub worker ($($build.Version)) failed" } + } +} finally { + Pop-Location +} + +# The stub supervisor: relaunches the worker whenever it exits, like +# Supervise-FileMill.ps1, minus the backoff and logging. +$supervisorPath = Join-Path $WorkRoot 'stub-supervisor.ps1' +Set-Content -Path $supervisorPath -Encoding ascii -Value @' +param([string]$Executable, [string]$Root) +Set-Location -LiteralPath $Root +while ($true) { + & $Executable run | Out-Null + Start-Sleep -Milliseconds 300 +} +'@ + +$supervisor = Start-Process -FilePath 'powershell.exe' ` + -ArgumentList '-NoProfile', '-ExecutionPolicy', 'Bypass', '-WindowStyle', 'Hidden', '-File', "`"$supervisorPath`"", '-Executable', "`"$binary`"", '-Root', "`"$WorkRoot`"" ` + -PassThru -WindowStyle Hidden + +try { + if (-not (Wait-Until { (Get-FakeWorkers $binary).Count -gt 0 })) { throw 'the stub worker never started' } + if (-not (Wait-Until { (Test-Path $logPath) -and ((Get-Content $logPath -Raw) -match 'v-old') })) { throw 'the stub worker never logged' } + $originalPid = (Get-FakeWorkers $binary)[0].ProcessId + + Write-Host '' + Write-Host 'Case 1: a good build deploys and the worker comes back on it' + & $deployScript -RepositoryRoot $WorkRoot -NewBinary (Join-Path $stage 'filemill.new.exe') ` + -Yes -NoElevationCheck -TimeoutSeconds 30 | Out-Null + $exit = $LASTEXITCODE + Check 'deploy exits 0' ($exit -eq 0) "exit code $exit" + Check 'bin\filemill.exe is now the new build' ((& $binary --version) -match 'v-new') + Check 'the previous binary was kept' (Test-Path (Join-Path $binDir 'filemill.v-old.exe')) + Check 'the worker was restarted' (Wait-Until { $running = Get-FakeWorkers $binary; ($running.Count -gt 0) -and ($running[0].ProcessId -ne $originalPid) }) + Check 'the new version reached the log' ((Get-Content $logPath -Raw) -match 'FileMill v-new') + + Write-Host '' + Write-Host 'Case 2: a build that will not start is rolled back' + $beforePid = (Get-FakeWorkers $binary)[0].ProcessId + # 6>&1 captures Write-Host output (the information stream), so the failure + # report itself can be checked: a rolled-back deploy has to say why. + $output = & $deployScript -RepositoryRoot $WorkRoot -NewBinary (Join-Path $stage 'filemill.bad.exe') ` + -Yes -NoElevationCheck -TimeoutSeconds 10 6>&1 | Out-String + $exit = $LASTEXITCODE + Check 'deploy exits non-zero' ($exit -ne 0) "exit code $exit" + Check 'the failure report shows what the worker logged' ($output -match 'startup failed') $output + Check 'bin\filemill.exe is back to the previous build' ((& $binary --version) -match 'v-new') + Check 'the build that failed was kept for inspection' (Test-Path (Join-Path $binDir 'filemill.failed.exe')) + Check 'a worker is serving again' (Wait-Until { $running = Get-FakeWorkers $binary; ($running.Count -gt 0) -and ($running[0].ProcessId -ne $beforePid) }) + Check 'it is the previous version' (Wait-Until { ((Get-Content $logPath -Raw) -split "`r?`n" | Where-Object { $_ -match 'FileMill v-new' }).Count -ge 2 }) +} finally { + Stop-Process -Id $supervisor.Id -Force -ErrorAction SilentlyContinue + foreach ($worker in (Get-FakeWorkers $binary)) { Stop-Process -Id $worker.ProcessId -Force -ErrorAction SilentlyContinue } + Start-Sleep -Milliseconds 500 + if (-not $Keep) { + Remove-Item -LiteralPath $WorkRoot -Recurse -Force -ErrorAction SilentlyContinue + } else { + Write-Host "Kept $WorkRoot" + } +} + +Write-Host '' +if ($script:failures.Count -gt 0) { + Write-Host "$($script:failures.Count) check(s) failed." -ForegroundColor Red + exit 1 +} +Write-Host 'All checks passed.' -ForegroundColor Green +exit 0 From 5fc4aff5948bfaa007142e797a974f76e213ed78 Mon Sep 17 00:00:00 2001 From: Kevin Brown Date: Fri, 11 Sep 2026 17:28:25 -0600 Subject: [PATCH 2/2] Fix two Windows PowerShell 5.1 bugs in the deploy script The deploy script failed on its first real run, in the elevated 5.1 window it is meant for, while passing the harness under pwsh 7. Two 5.1 differences, both of which the harness now guards: - A string containing an em dash. 5.1 reads a file with no byte-order mark as ANSI, so the em dash decodes to three characters, one of which is a smart quote - and PowerShell accepts smart quotes as string delimiters. It opened a string that never closed and the whole file stopped parsing. The scripts are ASCII-only now, and the harness enforces it. - .Count on the result of Get-Workers. PowerShell unwraps a one-element array on return, and 5.1 answers $null when a lone CimInstance is asked for .Count (it looks for a CIM property of that name). Every worker check therefore read as "no worker running", which in the deploy would mean swapping the binary and never restarting the worker. Every call site wraps in @() now. The harness parses both scripts with both shells before running anything, and its waits report what they saw when they give up - polls made, the path queried, and the process list - which is what finally identified the second bug. It must be run under both shells; only 5.1 showed either of these. Co-Authored-By: Claude Opus 5 (1M context) --- scripts/Deploy-FileMill.ps1 | 59 ++++++--- scripts/Install-FileMillScheduledTask.ps1 | 6 +- scripts/Test-DeployFileMill.ps1 | 144 ++++++++++++++++++++-- 3 files changed, 178 insertions(+), 31 deletions(-) diff --git a/scripts/Deploy-FileMill.ps1 b/scripts/Deploy-FileMill.ps1 index 5680e46..85b134c 100644 --- a/scripts/Deploy-FileMill.ps1 +++ b/scripts/Deploy-FileMill.ps1 @@ -77,7 +77,9 @@ function Wait-Until { $deadline = (Get-Date).AddSeconds($Seconds) while ((Get-Date) -lt $deadline) { if (& $Condition) { return $true } - Start-Sleep -Milliseconds 500 + # A second between polls: these conditions query Win32_Process, which + # starts failing when it is asked several times a second. + Start-Sleep -Seconds 1 } return $false } @@ -87,10 +89,23 @@ function Wait-Until { # harness's fake) out of it. function Get-Workers { param([string]$Path) - # Not $matches: that is a PowerShell automatic variable, written by -match. - $found = Get-CimInstance Win32_Process -Filter "Name='filemill.exe'" -ErrorAction SilentlyContinue | - Where-Object { $_.ExecutablePath -and ($_.ExecutablePath -ieq $Path) } - return @($found) + # Win32_Process fails transiently when it is queried several times a + # second, and treating that as "no worker" would be dangerous here: it + # reads as the worker having vanished, which rolls a healthy deploy back. + # So retry, and say so rather than silently reporting nothing. + for ($attempt = 1; $attempt -le 4; $attempt++) { + try { + # Not $matches: that is a PowerShell automatic variable, written by -match. + $found = Get-CimInstance Win32_Process -Filter "Name='filemill.exe'" -ErrorAction Stop | + Where-Object { $_.ExecutablePath -and ($_.ExecutablePath -ieq $Path) } + return @($found) + } catch { + $lastError = $_.Exception.Message + Start-Sleep -Milliseconds 250 + } + } + Write-Host " (could not read the process list: $lastError)" -ForegroundColor Yellow + return @() } # Get-Version runs a binary's --version. It is also the proof that the file is @@ -158,7 +173,11 @@ if (-not $SkipBuild) { $dirty = git status --porcelain if ($LASTEXITCODE -ne 0) { Fail 'git status failed; is this a checkout?' } if ($dirty -and -not $AllowDirty) { - Fail "The working tree has uncommitted changes, so the build would not match any commit. Commit them, or pass -AllowDirty.`n$($dirty -join "`n")" + # Concatenated, not interpolated: Windows PowerShell 5.1 cannot + # parse a double-quoted string inside $() inside another one, and + # this script is run by hand in an elevated 5.1 shell. + $changed = $dirty -join [Environment]::NewLine + Fail ('The working tree has uncommitted changes, so the build would not match any commit. Commit them, or pass -AllowDirty.' + [Environment]::NewLine + $changed) } if (-not $SkipTests) { Write-Step 'Running go test ./...' @@ -179,10 +198,16 @@ $NewBinary = (Resolve-Path -LiteralPath $NewBinary).Path $newVersion = Get-Version $NewBinary if (-not $newVersion) { Fail "The new build at $NewBinary did not report a version, so it is not a working executable." } -$workers = Get-Workers $binary +# @() around every Get-Workers call: PowerShell unwraps a one-element array on +# return, and asking a lone CimInstance for .Count gets $null in Windows +# PowerShell 5.1, which reads as "no worker running" - here that would mean +# swapping the binary and never restarting the worker. +$workers = @(Get-Workers $binary) +$runningPids = 'none' +if ($workers.Count -gt 0) { $runningPids = ($workers | ForEach-Object { $_.ProcessId }) -join ', ' } Write-Host '' Write-Host " repository: $RepositoryRoot" -Write-Host " running: $oldVersion (worker PID(s): $(if ($workers.Count) { ($workers | ForEach-Object { $_.ProcessId }) -join ', ' } else { 'none' }))" +Write-Host " running: $oldVersion (worker PID(s): $runningPids)" Write-Host " deploying: $newVersion" Write-Host '' @@ -222,13 +247,13 @@ function Restore-Previous { Stop-Process -Id $worker.ProcessId -Force -ErrorAction SilentlyContinue } $back = Wait-Until -Seconds $TimeoutSeconds -Condition { - $running = Get-Workers $binary + $running = @(Get-Workers $binary) ($running.Count -gt 0) -and ((Get-Version $binary) -eq $oldVersion) } if ($back) { Fail "$Reason. Rolled back to $oldVersion; the build that failed is at $failed." } - Fail "$Reason. Rolled back to $oldVersion, but no worker came back — start it with: Start-ScheduledTask -TaskName 'FileMill Worker'. The build that failed is at $failed." + Fail "$Reason. Rolled back to $oldVersion, but no worker came back - start it with: Start-ScheduledTask -TaskName 'FileMill Worker'. The build that failed is at $failed." } # --- restart: kill the worker and let the supervisor relaunch it ------------ @@ -242,22 +267,23 @@ if ($workers.Count -eq 0) { $logOffset = Get-LogLength $logPath $oldPids = @($workers | ForEach-Object { $_.ProcessId }) -Write-Step "Stopping the worker (PID $($oldPids -join ', ')); the supervisor will relaunch it" +$oldPidList = $oldPids -join ', ' +Write-Step "Stopping the worker (PID $oldPidList); the supervisor will relaunch it" foreach ($worker in $workers) { Stop-Process -Id $worker.ProcessId -Force } -if (-not (Wait-Until -Seconds $TimeoutSeconds -Condition { (Get-Workers $binary | Where-Object { $oldPids -contains $_.ProcessId }).Count -eq 0 })) { +if (-not (Wait-Until -Seconds $TimeoutSeconds -Condition { @(Get-Workers $binary | Where-Object { $oldPids -contains $_.ProcessId }).Count -eq 0 })) { Restore-Previous 'The old worker did not stop' } Write-Step 'Waiting for the supervisor to start the new worker' -if (-not (Wait-Until -Seconds $TimeoutSeconds -Condition { (Get-Workers $binary | Where-Object { $oldPids -notcontains $_.ProcessId }).Count -gt 0 })) { +if (-not (Wait-Until -Seconds $TimeoutSeconds -Condition { @(Get-Workers $binary | Where-Object { $oldPids -notcontains $_.ProcessId }).Count -gt 0 })) { # Two causes look identical from here, and a build that crashes at startup # can exit before it is ever seen in the process list, so name both rather # than guessing: the log tail printed below usually settles it. Restore-Previous 'No new worker stayed up. Either the new build exits at startup, or no supervisor is running to relaunch it (Start-ScheduledTask -TaskName ''FileMill Worker'')' } -$newWorkers = Get-Workers $binary | Where-Object { $oldPids -notcontains $_.ProcessId } +$newWorkers = @(Get-Workers $binary | Where-Object { $oldPids -notcontains $_.ProcessId }) # --- verify: the new version is the one serving ----------------------------- @@ -270,13 +296,14 @@ if (-not $SkipLogCheck) { # A worker that started and then died leaves a PID that no longer exists, so # this is checked after the log line, not instead of it. -if ((Get-Workers $binary).Count -eq 0) { +if (@(Get-Workers $binary).Count -eq 0) { Restore-Previous 'The new worker started and then exited' } Write-Host '' Write-Host "Deployed $oldVersion -> $newVersion" -ForegroundColor Green -Write-Host " worker PID(s): $(($newWorkers | ForEach-Object { $_.ProcessId }) -join ', ')" +$newPidList = ($newWorkers | ForEach-Object { $_.ProcessId }) -join ', ' +Write-Host " worker PID(s): $newPidList" Write-Host " previous binary kept at: $backup" $startup = (Read-LogSince $logPath $logOffset) -split "`r?`n" | Where-Object { $_ -match [regex]::Escape($newVersion) } | Select-Object -First 1 if ($startup) { Write-Host " $($startup.Trim())" } diff --git a/scripts/Install-FileMillScheduledTask.ps1 b/scripts/Install-FileMillScheduledTask.ps1 index e48eb7f..0655e7e 100644 --- a/scripts/Install-FileMillScheduledTask.ps1 +++ b/scripts/Install-FileMillScheduledTask.ps1 @@ -31,7 +31,7 @@ $action = New-ScheduledTaskAction -Execute $powershell -Argument "-NoProfile -Ex # Two triggers. At boot is the one that matters: a machine that reboots # overnight for updates used to sit idle until someone signed in, with the # Cloudflare tunnel (a real service) up and nothing listening behind it. The -# logon trigger stays as a backstop — MultipleInstances IgnoreNew makes a second +# logon trigger stays as a backstop - MultipleInstances IgnoreNew makes a second # fire while the task is already running a no-op. # # The boot trigger waits 30 seconds. Nothing is waiting on FileMill at second @@ -44,8 +44,8 @@ $atLogon = New-ScheduledTaskTrigger -AtLogOn -User $user # and without storing a password. It is what makes the boot trigger useful, and # it is also why this script needs elevation. # -# It costs the task a network identity — an S4U process cannot reach SMB shares -# as the user — which FileMill does not need: it makes outbound HTTPS calls +# It costs the task a network identity - an S4U process cannot reach SMB shares +# as the user - which FileMill does not need: it makes outbound HTTPS calls # authenticated by API keys and listens on a local port. Its secrets must be # machine-scope environment variables, though, because a task running before # logon has no user registry hive to read user-scope ones from. diff --git a/scripts/Test-DeployFileMill.ps1 b/scripts/Test-DeployFileMill.ps1 index 3f1cbf1..4471b5c 100644 --- a/scripts/Test-DeployFileMill.ps1 +++ b/scripts/Test-DeployFileMill.ps1 @@ -6,7 +6,7 @@ .DESCRIPTION Sets up a throwaway "repository": a bin\ holding a stub worker, a data\logs\ for it to write to, and a stub supervisor that relaunches the - worker whenever it exits — the same shape as the real install, with none of + worker whenever it exits - the same shape as the real install, with none of its parts. The stub worker is a tiny Go program built three times with different stamped versions, one of which fails to start on purpose. @@ -21,6 +21,13 @@ It needs no elevation (the stub worker runs in this session) and no modules. Exits 0 if every check passes, 1 otherwise. + + Run it under BOTH shells - powershell.exe (5.1, which is what an elevated + window gives you, and what the deploy is run in) and pwsh.exe (7). They + differ in ways that decide whether the deploy works at all: 5.1 reads a + BOM-less file as ANSI, and it returns $null for .Count on a single + CimInstance that PowerShell unwrapped out of an array. Both of those + shipped in this script's first version and only 5.1 showed them. #> [CmdletBinding()] param( @@ -33,7 +40,10 @@ $ErrorActionPreference = 'Stop' $script:failures = @() function Check { - param([string]$What, [bool]$Ok, [string]$Detail) + # $Ok is deliberately untyped: a [bool] parameter throws when a helper + # accidentally returns a collection, which hides the real failure behind a + # binding error. + param([string]$What, $Ok, [string]$Detail) if ($Ok) { Write-Host " ok $What" -ForegroundColor Green } else { @@ -43,25 +53,131 @@ function Check { } } +$script:polls = 0 +$script:lastPollValue = '(never evaluated)' + function Wait-Until { param([scriptblock]$Condition, [int]$Seconds = 30) + $script:polls = 0 $deadline = (Get-Date).AddSeconds($Seconds) while ((Get-Date) -lt $deadline) { - if (& $Condition) { return $true } - Start-Sleep -Milliseconds 200 + $script:polls++ + # Kept as a value rather than tested inline, so a wait that gives up can + # report what it was actually seeing instead of only that it failed. + $value = & $Condition + $script:lastPollValue = "[$value]" + if ($value) { return $true } + # 750ms, not 200: these conditions query Win32_Process, which starts + # failing when it is asked several times a second. + Start-Sleep -Milliseconds 750 } return $false } +$script:lastQueryError = $null + function Get-FakeWorkers { param([string]$Path) - return @(Get-CimInstance Win32_Process -Filter "Name='filemill.exe'" -ErrorAction SilentlyContinue | - Where-Object { $_.ExecutablePath -and ($_.ExecutablePath -ieq $Path) }) + # Records what it saw on the way through: a wait that gives up has to be + # able to say whether the query failed, matched nothing, or was handed the + # wrong path. + $script:lastQueryPath = $Path + for ($attempt = 1; $attempt -le 4; $attempt++) { + try { + $all = @(Get-CimInstance Win32_Process -Filter "Name='filemill.exe'" -ErrorAction Stop) + $found = @($all | Where-Object { $_.ExecutablePath -and ($_.ExecutablePath -ieq $Path) }) + $script:lastQueryError = $null + $script:lastQueryCounts = "unfiltered=$($all.Count) filtered=$($found.Count)" + return $found + } catch { + $script:lastQueryError = $_.Exception.Message + Start-Sleep -Milliseconds 250 + } + } + return @() +} + +# Show-WorkerState prints what the process list actually held when a wait gave +# up. A bare "never started" says nothing about whether the process was missing, +# somewhere else, or simply unreadable from this shell. +function Show-WorkerState { + param([string]$Path) + Write-Host " expected worker path: [$Path]" + Write-Host " polls made: $script:polls, last condition value: $script:lastPollValue" + Write-Host " last query path: [$script:lastQueryPath]" + Write-Host " last query counts: $script:lastQueryCounts" + if ($script:lastQueryError) { Write-Host " last process-query error: $script:lastQueryError" } + $all = @(Get-CimInstance Win32_Process -Filter "Name='filemill.exe'" -ErrorAction SilentlyContinue) + Write-Host " filemill.exe processes visible: $($all.Count)" + foreach ($process in $all) { + Write-Host (" pid={0} path=[{1}]" -f $process.ProcessId, $process.ExecutablePath) + } } $deployScript = Join-Path $PSScriptRoot 'Deploy-FileMill.ps1' if (-not (Test-Path -LiteralPath $deployScript)) { throw "Deploy-FileMill.ps1 not found next to this script." } +# Test-Parses asks one PowerShell to parse a script and report syntax errors. +# The path travels in an environment variable so the command needs no quoting +# of its own. +function Test-Parses { + param([string]$Shell, [string]$Path) + $env:FILEMILL_PARSE_TARGET = $Path + $code = '$e = $null; [void][System.Management.Automation.Language.Parser]::ParseFile($env:FILEMILL_PARSE_TARGET, [ref]$null, [ref]$e); if ($e) { $e | ForEach-Object { "line {0}: {1}" -f $_.Extent.StartLineNumber, $_.Message }; exit 1 }; exit 0' + # Capture the child's output instead of letting it join this function's + # return value, which would make the result an array rather than a boolean. + $output = & $Shell -NoProfile -Command $code 2>&1 + $parsed = ($LASTEXITCODE -eq 0) + if (-not $parsed) { + $output | ForEach-Object { Write-Host " $_" } + } + return $parsed +} + +# These scripts are run by hand in an elevated Windows PowerShell, which is 5.1 +# and parses more strictly than pwsh 7 - it rejects a double-quoted string +# inside $() inside another one, for instance. A harness that only ever ran +# under pwsh once let exactly that reach the operator, so both shells parse +# both scripts before anything else happens. +Write-Host 'Parsing the scripts in each installed PowerShell' +foreach ($shell in @('powershell.exe', 'pwsh.exe')) { + if (-not (Get-Command $shell -ErrorAction SilentlyContinue)) { + Write-Host " skip $shell (not installed)" + continue + } + foreach ($target in @($deployScript, $PSCommandPath)) { + Check "$([System.IO.Path]::GetFileName($target)) parses in $shell" (Test-Parses $shell $target) + } +} +# Windows PowerShell 5.1 reads a .ps1 with no byte-order mark as ANSI rather +# than UTF-8, so a character like an em dash arrives as three characters, one of +# which is a smart quote - and PowerShell accepts smart quotes as string +# delimiters. In a comment that is only mojibake. In a string it opens a string +# that never closes and the file stops parsing, which is exactly how a broken +# deploy script reached the operator once. Keeping these scripts ASCII-only +# sidesteps the encoding question rather than relying on remembering it. +Write-Host 'Checking the scripts are ASCII-only' +foreach ($script in (Get-ChildItem -LiteralPath $PSScriptRoot -Filter '*.ps1')) { + $offenders = @() + $number = 0 + foreach ($line in (Get-Content -LiteralPath $script.FullName -Encoding UTF8)) { + $number++ + foreach ($char in $line.ToCharArray()) { + if ([int]$char -gt 127) { + $offenders += ('line {0}: U+{1:X4}' -f $number, [int]$char) + break + } + } + } + Check "$($script.Name) is ASCII-only" ($offenders.Count -eq 0) ($offenders -join '; ') +} + +if ($script:failures.Count -gt 0) { + Write-Host '' + Write-Host 'Fix the syntax or encoding problems above; not running the deploy cases.' -ForegroundColor Red + exit 1 +} + if (-not $WorkRoot) { $WorkRoot = Join-Path $env:TEMP ("filemill-deploy-test-" + [guid]::NewGuid().ToString('N').Substring(0, 8)) } $binDir = Join-Path $WorkRoot 'bin' $logDir = Join-Path $WorkRoot 'data\logs' @@ -147,9 +263,13 @@ $supervisor = Start-Process -FilePath 'powershell.exe' ` -PassThru -WindowStyle Hidden try { - if (-not (Wait-Until { (Get-FakeWorkers $binary).Count -gt 0 })) { throw 'the stub worker never started' } - if (-not (Wait-Until { (Test-Path $logPath) -and ((Get-Content $logPath -Raw) -match 'v-old') })) { throw 'the stub worker never logged' } - $originalPid = (Get-FakeWorkers $binary)[0].ProcessId + # @() around every call: PowerShell unwraps a one-element array on return, + # and asking a lone CimInstance for .Count gets $null in Windows PowerShell + # 5.1 (it looks for a CIM property by that name), so an unwrapped count + # silently reads as "nothing running". + if (-not (Wait-Until { @(Get-FakeWorkers $binary).Count -gt 0 })) { Show-WorkerState $binary; throw 'the stub worker never started' } + if (-not (Wait-Until { (Test-Path $logPath) -and ((Get-Content $logPath -Raw) -match 'v-old') })) { Show-WorkerState $binary; throw 'the stub worker never logged' } + $originalPid = @(Get-FakeWorkers $binary)[0].ProcessId Write-Host '' Write-Host 'Case 1: a good build deploys and the worker comes back on it' @@ -159,12 +279,12 @@ try { Check 'deploy exits 0' ($exit -eq 0) "exit code $exit" Check 'bin\filemill.exe is now the new build' ((& $binary --version) -match 'v-new') Check 'the previous binary was kept' (Test-Path (Join-Path $binDir 'filemill.v-old.exe')) - Check 'the worker was restarted' (Wait-Until { $running = Get-FakeWorkers $binary; ($running.Count -gt 0) -and ($running[0].ProcessId -ne $originalPid) }) + Check 'the worker was restarted' (Wait-Until { $running = @(Get-FakeWorkers $binary); ($running.Count -gt 0) -and ($running[0].ProcessId -ne $originalPid) }) Check 'the new version reached the log' ((Get-Content $logPath -Raw) -match 'FileMill v-new') Write-Host '' Write-Host 'Case 2: a build that will not start is rolled back' - $beforePid = (Get-FakeWorkers $binary)[0].ProcessId + $beforePid = @(Get-FakeWorkers $binary)[0].ProcessId # 6>&1 captures Write-Host output (the information stream), so the failure # report itself can be checked: a rolled-back deploy has to say why. $output = & $deployScript -RepositoryRoot $WorkRoot -NewBinary (Join-Path $stage 'filemill.bad.exe') ` @@ -174,7 +294,7 @@ try { Check 'the failure report shows what the worker logged' ($output -match 'startup failed') $output Check 'bin\filemill.exe is back to the previous build' ((& $binary --version) -match 'v-new') Check 'the build that failed was kept for inspection' (Test-Path (Join-Path $binDir 'filemill.failed.exe')) - Check 'a worker is serving again' (Wait-Until { $running = Get-FakeWorkers $binary; ($running.Count -gt 0) -and ($running[0].ProcessId -ne $beforePid) }) + Check 'a worker is serving again' (Wait-Until { $running = @(Get-FakeWorkers $binary); ($running.Count -gt 0) -and ($running[0].ProcessId -ne $beforePid) }) Check 'it is the previous version' (Wait-Until { ((Get-Content $logPath -Raw) -split "`r?`n" | Where-Object { $_ -match 'FileMill v-new' }).Count -ge 2 }) } finally { Stop-Process -Id $supervisor.Id -Force -ErrorAction SilentlyContinue