From 0f643d4bcd1acc6bb703eaa5b613284f8ca063bc Mon Sep 17 00:00:00 2001 From: fangliquanflq Date: Wed, 26 Aug 2026 22:38:17 +0800 Subject: [PATCH 1/9] fix(update): recover stalled Windows desktop handoffs --- scripts/desktop-update/windows.ps1 | 73 ++++++++++++++++++- .../test_desktop_update_windows_pipe_drain.py | 27 +++++-- 2 files changed, 88 insertions(+), 12 deletions(-) diff --git a/scripts/desktop-update/windows.ps1 b/scripts/desktop-update/windows.ps1 index 88718a2fa52e3..fe3e73ff39c46 100644 --- a/scripts/desktop-update/windows.ps1 +++ b/scripts/desktop-update/windows.ps1 @@ -707,6 +707,21 @@ if ($env:HERMES_UPDATE_PIPE_DRAIN_SECONDS) { } } +# A live step also needs a ceiling. The pipe-drain bound above only starts +# after the child exits, so it cannot recover a child that completed its visible +# work and then parks forever inside finalization (#95589). Treat a prolonged +# absence of stdout/stderr as a stalled step, terminate the direct child, and +# let the existing retry + finally path restore the Desktop. The retry is +# load-bearing at an update boundary: newly-pulled updater code is only loaded +# by the second process. +$script:StepIdleTimeoutSeconds = 300 +if ($env:HERMES_UPDATE_STEP_IDLE_SECONDS) { + $parsedIdle = 0 + if ([int]::TryParse($env:HERMES_UPDATE_STEP_IDLE_SECONDS, [ref]$parsedIdle) -and $parsedIdle -gt 0) { + $script:StepIdleTimeoutSeconds = $parsedIdle + } +} + function Step-PipeDrain($Reader, [ref]$Task, $Buffer, $Sink, [ref]$Moved) { # Advance one redirected pipe by whatever has already arrived, without # ever blocking. Returns $true once the pipe has reached EOF (or its read @@ -770,6 +785,10 @@ function Invoke-HermesStep([string]$Exe, [string[]]$HermesArgs, [string]$Tag) { # encoding from the console codepage when attached to one. $psi.EnvironmentVariables["PYTHONIOENCODING"] = "utf-8" $psi.EnvironmentVariables["PYTHONUTF8"] = "1" + # The idle watchdog below is only sound when Python progress is observable + # promptly. A redirected Python stdout is block-buffered by default, which + # otherwise makes an active update look silent until the buffer fills. + $psi.EnvironmentVariables["PYTHONUNBUFFERED"] = "1" $psi.CreateNoWindow = $true $proc = [System.Diagnostics.Process]::Start($psi) $outSink = New-Object System.Text.StringBuilder @@ -780,10 +799,13 @@ function Invoke-HermesStep([string]$Exe, [string[]]$HermesArgs, [string]$Tag) { $errTask = $proc.StandardError.ReadAsync($errBuffer, 0, $errBuffer.Length) $abandonAt = $null $abandoned = $false + $lastProgressAt = Get-Date + $stalled = $false while ($true) { $moved = $false $outDone = Step-PipeDrain $proc.StandardOutput ([ref]$outTask) $outBuffer $outSink ([ref]$moved) $errDone = Step-PipeDrain $proc.StandardError ([ref]$errTask) $errBuffer $errSink ([ref]$moved) + if ($moved) { $lastProgressAt = Get-Date } if ($proc.HasExited) { if ($outDone -and $errDone) { break } # Clock starts at the step's exit, not at its start: a slow step is @@ -794,6 +816,15 @@ function Invoke-HermesStep([string]$Exe, [string[]]$HermesArgs, [string]$Tag) { $abandoned = $true break } + } elseif (-not $stalled -and ((Get-Date) - $lastProgressAt).TotalSeconds -ge $script:StepIdleTimeoutSeconds) { + # The child is alive but has produced no observable progress for + # the whole bound. Kill only the direct update/rebuild process; a + # surviving descendant is handled by the post-exit pipe-drain + # grace above. Returning 124 enters the existing one-retry path, + # while the outer finally still restores Desktop if both runs stall. + $stalled = $true + Write-HandoffLog ("{0}!| step stalled: no stdout/stderr for {1}s while pid {2} remained alive; terminating it so the hand-off can recover." -f $Tag, $script:StepIdleTimeoutSeconds, $proc.Id) + try { $proc.Kill() } catch {} } # Only idle when both pipes came up empty this pass, and idle on the # reads themselves rather than on the clock. @@ -838,7 +869,8 @@ function Invoke-HermesStep([string]$Exe, [string[]]$HermesArgs, [string]$Tag) { } $all = $outText if ($errText) { $all += "`n" + $errText } - return @{ Code = $proc.ExitCode; Output = $all } + $code = if ($stalled) { 124 } else { $proc.ExitCode } + return @{ Code = $code; Output = $all } } $finalCode = 1 @@ -883,7 +915,7 @@ if ($SelfTestUi) { # before any marker/desktop machinery, same as -SelfTestUi; touches nothing # but its own temp files. # -# Two arms, because the bound and the drain rate fail in opposite directions: +# Three arms cover the independent wait modes: # # leak -- a step whose grandchild outlives it. Guards the #90455 deadlock: # the drain must abandon rather than wait out the descendant. @@ -891,6 +923,9 @@ if ($SelfTestUi) { # that idles after every chunk it reads is metered at one buffer per # tick, which backpressures the running step. Waiting for EOF and # trickling toward it are both ways to make a fast step slow. +# stall -- a step that remains alive after its visible work and emits no more +# output. Guards #95589: the hand-off must terminate it and reach its +# retry/finally recovery rather than strand the Desktop. if ($SelfTestPipeDrain) { New-Item -ItemType Directory -Path $LogDir -Force -ErrorAction SilentlyContinue | Out-Null $hold = 60 @@ -903,6 +938,8 @@ if ($SelfTestPipeDrain) { $childPs1 = Join-Path $TempDir "hermes-pipe-drain-$stamp.ps1" $floodPs1 = Join-Path $TempDir "hermes-pipe-flood-$stamp.ps1" $pidFile = Join-Path $TempDir "hermes-pipe-drain-$stamp.pid" + $stallPs1 = Join-Path $TempDir "hermes-step-stall-$stamp.ps1" + $stallPidFile = Join-Path $TempDir "hermes-step-stall-$stamp.pid" # UseShellExecute=$false with no redirection is what makes the grandchild # inherit our stdout/stderr -- the whole point of the fixture. Anything # that redirects (Start-Process, subprocess with stdout=DEVNULL) would @@ -932,9 +969,18 @@ $chunk = "x" * (131072 - 1) for ($i = 0; $i -lt [Math]::Ceiling($Kb / 128); $i++) { [Console]::Out.Write($chunk + "`n") } [Console]::Out.Flush() exit 5 +'@ + $stallSource = @' +param([int]$Hold, [string]$PidFile) +[System.IO.File]::WriteAllText($PidFile, [string]$PID) +Write-Output "step entered silent finalization" +[Console]::Out.Flush() +Start-Sleep -Seconds $Hold +exit 0 '@ [System.IO.File]::WriteAllText($childPs1, $childSource) [System.IO.File]::WriteAllText($floodPs1, $floodSource) + [System.IO.File]::WriteAllText($stallPs1, $stallSource) $sw = [System.Diagnostics.Stopwatch]::StartNew() $res = Invoke-HermesStep $powershell @( "-NoProfile", "-ExecutionPolicy", "Bypass", "-File", $childPs1, @@ -962,7 +1008,21 @@ exit 5 $floodElapsed = [Math]::Round($floodSw.Elapsed.TotalSeconds, 2) $floodBytes = $flood.Output.Length - Remove-Item -LiteralPath $childPs1, $floodPs1, $pidFile -Force -ErrorAction SilentlyContinue + $stallSw = [System.Diagnostics.Stopwatch]::StartNew() + $stall = Invoke-HermesStep $powershell @( + "-NoProfile", "-ExecutionPolicy", "Bypass", "-File", $stallPs1, + "-Hold", [string]$hold, "-PidFile", $stallPidFile + ) "stepstall" + $stallSw.Stop() + $stallElapsed = [Math]::Round($stallSw.Elapsed.TotalSeconds, 2) + $stallPid = 0 + if (Test-Path -LiteralPath $stallPidFile) { + [void][int]::TryParse((Get-Content -LiteralPath $stallPidFile -Raw).Trim(), [ref]$stallPid) + } + $stallAlive = $stallPid -gt 0 -and [bool](Get-Process -Id $stallPid -ErrorAction SilentlyContinue) + if ($stallAlive) { Stop-Process -Id $stallPid -Force -ErrorAction SilentlyContinue } + + Remove-Item -LiteralPath $childPs1, $floodPs1, $stallPs1, $pidFile, $stallPidFile -Force -ErrorAction SilentlyContinue # The grandchild still being alive at return is what makes this a proof # rather than a timing coincidence: the pipe was demonstrably still open. @@ -978,8 +1038,13 @@ exit 5 if ($floodElapsed -ge $floodBudget) { $problems += "flood arm returned in ${floodElapsed}s, over the ${floodBudget}s budget -- the drain is metering itself, which backpressures the step" } if ($flood.Code -ne 5) { $problems += "flood arm exit code $($flood.Code), expected 5" } if ($floodBytes -lt ($floodKb * 1024)) { $problems += "flood arm captured $floodBytes bytes of $($floodKb * 1024)" } + $stallBudget = $script:StepIdleTimeoutSeconds + 30 + if ($stallElapsed -ge $stallBudget) { $problems += "stall arm returned in ${stallElapsed}s, over the ${stallBudget}s budget" } + if ($stall.Code -ne 124) { $problems += "stall arm exit code $($stall.Code), expected 124" } + if ($stall.Output -notmatch "step entered silent finalization") { $problems += "stall arm step output was lost" } + if ($stallAlive) { $problems += "stalled child pid $stallPid remained alive after Invoke-HermesStep returned" } - $detail = "leak: elapsed=${elapsed}s budget=${budget}s code=$($res.Code) grandchildAlive=$leakAlive | flood: ${floodKb}KB in ${floodElapsed}s budget=${floodBudget}s bytes=$floodBytes code=$($flood.Code)" + $detail = "leak: elapsed=${elapsed}s budget=${budget}s code=$($res.Code) grandchildAlive=$leakAlive | flood: ${floodKb}KB in ${floodElapsed}s budget=${floodBudget}s bytes=$floodBytes code=$($flood.Code) | stall: elapsed=${stallElapsed}s budget=${stallBudget}s code=$($stall.Code) childAlive=$stallAlive" if ($problems.Count -gt 0) { Write-Host "PIPE-DRAIN SELF-TEST: FAIL $detail -- $($problems -join '; ')" exit 1 diff --git a/tests/test_desktop_update_windows_pipe_drain.py b/tests/test_desktop_update_windows_pipe_drain.py index e8f344f53e73e..16b0e35fe1215 100644 --- a/tests/test_desktop_update_windows_pipe_drain.py +++ b/tests/test_desktop_update_windows_pipe_drain.py @@ -1,4 +1,4 @@ -"""Regression: the Windows Desktop update hand-off must not meter its step pipes. +"""Regression: Windows Desktop update steps must drain and terminate reliably. ``scripts/desktop-update/windows.ps1`` runs each update step through ``Invoke-HermesStep``, which starts the step with ``RedirectStandardOutput`` / @@ -31,8 +31,14 @@ step writing to both pipes took 18.3s vs 0.29s. ``hermes update`` is exactly this shape; the Electron/vite build alone is megabytes. -So the contract is: bounded when a descendant holds the pipe open, and never -slower than the step can write. Both arms live in the script's own +**Live child stall (#95589).** A step can also finish its visible update work +but remain alive and silent in finalization. A post-exit pipe bound cannot help +that case. The hand-off must terminate the silent child so its existing retry +and finally paths can write the result, remove the marker, and relaunch Desktop. + +So the contract is: bounded when a descendant holds the pipe open, never slower +than the step can write, and bounded when the step itself remains alive without +observable progress. All arms live in the script's own ``-SelfTestPipeDrain`` fixture, which is ``windows_only`` because Linux CI cannot execute the PowerShell hand-off. """ @@ -51,12 +57,12 @@ @pytest.mark.windows_only -def test_pipe_drain_survives_a_leak_without_metering_a_chatty_step( +def test_update_step_survives_pipe_leak_flood_and_live_child_stall( tmp_path: Path, ) -> None: - """Execute the real drain against both shapes of step. + """Execute the real hand-off runner against all three step shapes. - ``-SelfTestPipeDrain`` runs two steps through the real + ``-SelfTestPipeDrain`` runs three steps through the real ``Invoke-HermesStep``: *leak* -- a step that spawns a grandchild with ``UseShellExecute = $false`` @@ -70,8 +76,12 @@ def test_pipe_drain_survives_a_leak_without_metering_a_chatty_step( must complete in wall-clock far under what a sleep-per-chunk drain would take, and every byte must arrive. - Measured on Windows 11 / PowerShell 5.1: leak 4.3s (vs 47.4s waiting out the - grandchild), flood 8 MiB in ~1s (vs ~76s metered). + *stall* -- a step that emits one progress line and then remains alive and + silent. It must be terminated with the timeout sentinel (124), preserve its + output, and leave no child process behind. + + The existing leak/flood arms retain their measured Windows 11 / PowerShell + 5.1 budgets; the stall arm uses the same real runner and process table. """ system_root = Path(os.environ.get("SystemRoot", r"C:\Windows")) powershell = ( @@ -90,6 +100,7 @@ def test_pipe_drain_survives_a_leak_without_metering_a_chatty_step( # how long the leaking grandchild lives. hold >> grace is what makes a # regression measurable rather than lucky. "HERMES_UPDATE_PIPE_DRAIN_SECONDS": "3", + "HERMES_UPDATE_STEP_IDLE_SECONDS": "3", "HERMES_SELFTEST_HOLD_SECONDS": "45", } From c4da8c12a53efa4c9fc74c5c5fa06e5f2c120185 Mon Sep 17 00:00:00 2001 From: fangliquanflq Date: Wed, 26 Aug 2026 23:04:19 +0800 Subject: [PATCH 2/9] fix(update): quiesce stalled Windows updater trees --- scripts/desktop-update/windows.ps1 | 177 +++++++++++++++--- .../test_desktop_update_windows_pipe_drain.py | 7 +- 2 files changed, 150 insertions(+), 34 deletions(-) diff --git a/scripts/desktop-update/windows.ps1 b/scripts/desktop-update/windows.ps1 index fe3e73ff39c46..0138bc4056a52 100644 --- a/scripts/desktop-update/windows.ps1 +++ b/scripts/desktop-update/windows.ps1 @@ -709,11 +709,10 @@ if ($env:HERMES_UPDATE_PIPE_DRAIN_SECONDS) { # A live step also needs a ceiling. The pipe-drain bound above only starts # after the child exits, so it cannot recover a child that completed its visible -# work and then parks forever inside finalization (#95589). Treat a prolonged -# absence of stdout/stderr as a stalled step, terminate the direct child, and -# let the existing retry + finally path restore the Desktop. The retry is -# load-bearing at an update boundary: newly-pulled updater code is only loaded -# by the second process. +# work and then parks forever inside finalization (#95589). Silence is only the +# cancellation trigger, never evidence that the process tree is safe to overlap: +# every step is assigned to a private, non-breakaway Windows job and a timed-out +# step is retryable only after that job reports zero active processes. $script:StepIdleTimeoutSeconds = 300 if ($env:HERMES_UPDATE_STEP_IDLE_SECONDS) { $parsedIdle = 0 @@ -722,6 +721,77 @@ if ($env:HERMES_UPDATE_STEP_IDLE_SECONDS) { } } +if (-not ("HermesUpdateJob" -as [type])) { + Add-Type -TypeDefinition @' +using System; +using System.Diagnostics; +using System.Runtime.InteropServices; +using System.Threading; + +public static class HermesUpdateJob { + [StructLayout(LayoutKind.Sequential)] + private struct BasicAccountingInformation { + public long TotalUserTime; + public long TotalKernelTime; + public long ThisPeriodTotalUserTime; + public long ThisPeriodTotalKernelTime; + public uint TotalPageFaultCount; + public uint TotalProcesses; + public uint ActiveProcesses; + public uint TotalTerminatedProcesses; + } + + [DllImport("kernel32.dll", CharSet = CharSet.Unicode, SetLastError = true)] + private static extern IntPtr CreateJobObject(IntPtr attributes, string name); + + [DllImport("kernel32.dll", SetLastError = true)] + private static extern bool AssignProcessToJobObject(IntPtr job, IntPtr process); + + [DllImport("kernel32.dll", SetLastError = true)] + private static extern bool TerminateJobObject(IntPtr job, uint exitCode); + + [DllImport("kernel32.dll", SetLastError = true)] + private static extern bool QueryInformationJobObject( + IntPtr job, + int informationClass, + out BasicAccountingInformation information, + uint informationLength, + IntPtr returnLength + ); + + [DllImport("kernel32.dll")] + private static extern bool CloseHandle(IntPtr handle); + + public static IntPtr CreateAndAssign(IntPtr process) { + IntPtr job = CreateJobObject(IntPtr.Zero, null); + if (job == IntPtr.Zero) return IntPtr.Zero; + if (AssignProcessToJobObject(job, process)) return job; + CloseHandle(job); + return IntPtr.Zero; + } + + public static bool TerminateAndWait(IntPtr job, uint exitCode, int timeoutMs) { + if (job == IntPtr.Zero || !TerminateJobObject(job, exitCode)) return false; + Stopwatch clock = Stopwatch.StartNew(); + BasicAccountingInformation information; + do { + if (!QueryInformationJobObject( + job, 1, out information, + (uint)Marshal.SizeOf(typeof(BasicAccountingInformation)), + IntPtr.Zero)) return false; + if (information.ActiveProcesses == 0) return true; + Thread.Sleep(50); + } while (clock.ElapsedMilliseconds < timeoutMs); + return false; + } + + public static void Close(IntPtr job) { + if (job != IntPtr.Zero) CloseHandle(job); + } +} +'@ +} + function Step-PipeDrain($Reader, [ref]$Task, $Buffer, $Sink, [ref]$Moved) { # Advance one redirected pipe by whatever has already arrived, without # ever blocking. Returns $true once the pipe has reached EOF (or its read @@ -791,6 +861,15 @@ function Invoke-HermesStep([string]$Exe, [string[]]$HermesArgs, [string]$Tag) { $psi.EnvironmentVariables["PYTHONUNBUFFERED"] = "1" $psi.CreateNoWindow = $true $proc = [System.Diagnostics.Process]::Start($psi) + # A job gives cancellation a kernel-enforced tree boundary. We deliberately + # do NOT set KILL_ON_JOB_CLOSE: successful updates may start detached + # services that are meant to outlive this pipe reader. Descendants cannot + # break away from a default job, but survive when its handle is closed after + # a normal step. + $job = [HermesUpdateJob]::CreateAndAssign($proc.Handle) + if ($job -eq [IntPtr]::Zero) { + Write-HandoffLog ("{0}!| could not assign pid {1} to a cancellation job; live-step timeout is disabled to prevent an unsafe partial-tree retry." -f $Tag, $proc.Id) + } $outSink = New-Object System.Text.StringBuilder $errSink = New-Object System.Text.StringBuilder $outBuffer = New-Object char[] 16384 @@ -816,15 +895,19 @@ function Invoke-HermesStep([string]$Exe, [string[]]$HermesArgs, [string]$Tag) { $abandoned = $true break } - } elseif (-not $stalled -and ((Get-Date) - $lastProgressAt).TotalSeconds -ge $script:StepIdleTimeoutSeconds) { + } elseif (-not $stalled -and $job -ne [IntPtr]::Zero -and ((Get-Date) - $lastProgressAt).TotalSeconds -ge $script:StepIdleTimeoutSeconds) { # The child is alive but has produced no observable progress for - # the whole bound. Kill only the direct update/rebuild process; a - # surviving descendant is handled by the post-exit pipe-drain - # grace above. Returning 124 enters the existing one-retry path, - # while the outer finally still restores Desktop if both runs stall. - $stalled = $true - Write-HandoffLog ("{0}!| step stalled: no stdout/stderr for {1}s while pid {2} remained alive; terminating it so the hand-off can recover." -f $Tag, $script:StepIdleTimeoutSeconds, $proc.Id) - try { $proc.Kill() } catch {} + # the whole bound. Terminate the job, not just its direct process: + # retrying while a descendant still mutates the checkout, venv, or + # release tree can overlap two installers and corrupt the install. + Write-HandoffLog ("{0}!| step stalled: no stdout/stderr for {1}s while pid {2} remained alive; cancelling its process tree." -f $Tag, $script:StepIdleTimeoutSeconds, $proc.Id) + $stalled = [HermesUpdateJob]::TerminateAndWait($job, 124, 10000) + if (-not $stalled) { + Write-HandoffLog ("{0}!| process-tree cancellation could not prove quiescence; refusing the timeout retry." -f $Tag) + $script:TreeSafeToFinalize = $false + [HermesUpdateJob]::Close($job) + throw "Unable to quiesce stalled update process tree" + } } # Only idle when both pipes came up empty this pass, and idle on the # reads themselves rather than on the clock. @@ -870,11 +953,13 @@ function Invoke-HermesStep([string]$Exe, [string[]]$HermesArgs, [string]$Tag) { $all = $outText if ($errText) { $all += "`n" + $errText } $code = if ($stalled) { 124 } else { $proc.ExitCode } - return @{ Code = $code; Output = $all } + [HermesUpdateJob]::Close($job) + return @{ Code = $code; Output = $all; TreeQuiesced = (-not $stalled -or $proc.HasExited) } } $finalCode = 1 $finalMsg = "update did not complete" +$script:TreeSafeToFinalize = $true # ── -SelfTestUi: drive the shim to both terminal states, no update ───────── # Manual QA for the Edge shell without a checkout or a real update. Exits @@ -940,6 +1025,7 @@ if ($SelfTestPipeDrain) { $pidFile = Join-Path $TempDir "hermes-pipe-drain-$stamp.pid" $stallPs1 = Join-Path $TempDir "hermes-step-stall-$stamp.ps1" $stallPidFile = Join-Path $TempDir "hermes-step-stall-$stamp.pid" + $stallGrandchildPidFile = Join-Path $TempDir "hermes-step-stall-grandchild-$stamp.pid" # UseShellExecute=$false with no redirection is what makes the grandchild # inherit our stdout/stderr -- the whole point of the fixture. Anything # that redirects (Start-Process, subprocess with stdout=DEVNULL) would @@ -971,8 +1057,15 @@ for ($i = 0; $i -lt [Math]::Ceiling($Kb / 128); $i++) { [Console]::Out.Write($ch exit 5 '@ $stallSource = @' -param([int]$Hold, [string]$PidFile) +param([int]$Hold, [string]$PidFile, [string]$GrandchildPidFile) [System.IO.File]::WriteAllText($PidFile, [string]$PID) +$psi = New-Object System.Diagnostics.ProcessStartInfo +$psi.FileName = Join-Path $PSHOME "powershell.exe" +$psi.Arguments = "-NoProfile -Command Start-Sleep -Seconds $Hold" +$psi.UseShellExecute = $false +$psi.CreateNoWindow = $true +$grandchild = [System.Diagnostics.Process]::Start($psi) +[System.IO.File]::WriteAllText($GrandchildPidFile, [string]$grandchild.Id) Write-Output "step entered silent finalization" [Console]::Out.Flush() Start-Sleep -Seconds $Hold @@ -1011,7 +1104,8 @@ exit 0 $stallSw = [System.Diagnostics.Stopwatch]::StartNew() $stall = Invoke-HermesStep $powershell @( "-NoProfile", "-ExecutionPolicy", "Bypass", "-File", $stallPs1, - "-Hold", [string]$hold, "-PidFile", $stallPidFile + "-Hold", [string]$hold, "-PidFile", $stallPidFile, + "-GrandchildPidFile", $stallGrandchildPidFile ) "stepstall" $stallSw.Stop() $stallElapsed = [Math]::Round($stallSw.Elapsed.TotalSeconds, 2) @@ -1021,8 +1115,14 @@ exit 0 } $stallAlive = $stallPid -gt 0 -and [bool](Get-Process -Id $stallPid -ErrorAction SilentlyContinue) if ($stallAlive) { Stop-Process -Id $stallPid -Force -ErrorAction SilentlyContinue } + $stallGrandchildPid = 0 + if (Test-Path -LiteralPath $stallGrandchildPidFile) { + [void][int]::TryParse((Get-Content -LiteralPath $stallGrandchildPidFile -Raw).Trim(), [ref]$stallGrandchildPid) + } + $stallGrandchildAlive = $stallGrandchildPid -gt 0 -and [bool](Get-Process -Id $stallGrandchildPid -ErrorAction SilentlyContinue) + if ($stallGrandchildAlive) { Stop-Process -Id $stallGrandchildPid -Force -ErrorAction SilentlyContinue } - Remove-Item -LiteralPath $childPs1, $floodPs1, $stallPs1, $pidFile, $stallPidFile -Force -ErrorAction SilentlyContinue + Remove-Item -LiteralPath $childPs1, $floodPs1, $stallPs1, $pidFile, $stallPidFile, $stallGrandchildPidFile -Force -ErrorAction SilentlyContinue # The grandchild still being alive at return is what makes this a proof # rather than a timing coincidence: the pipe was demonstrably still open. @@ -1043,8 +1143,10 @@ exit 0 if ($stall.Code -ne 124) { $problems += "stall arm exit code $($stall.Code), expected 124" } if ($stall.Output -notmatch "step entered silent finalization") { $problems += "stall arm step output was lost" } if ($stallAlive) { $problems += "stalled child pid $stallPid remained alive after Invoke-HermesStep returned" } + if ($stallGrandchildAlive) { $problems += "stalled descendant pid $stallGrandchildPid remained alive after Invoke-HermesStep returned" } + if (-not $stall.TreeQuiesced) { $problems += "stall arm returned without proving its process tree quiescent" } - $detail = "leak: elapsed=${elapsed}s budget=${budget}s code=$($res.Code) grandchildAlive=$leakAlive | flood: ${floodKb}KB in ${floodElapsed}s budget=${floodBudget}s bytes=$floodBytes code=$($flood.Code) | stall: elapsed=${stallElapsed}s budget=${stallBudget}s code=$($stall.Code) childAlive=$stallAlive" + $detail = "leak: elapsed=${elapsed}s budget=${budget}s code=$($res.Code) grandchildAlive=$leakAlive | flood: ${floodKb}KB in ${floodElapsed}s budget=${floodBudget}s bytes=$floodBytes code=$($flood.Code) | stall: elapsed=${stallElapsed}s budget=${stallBudget}s code=$($stall.Code) childAlive=$stallAlive descendantAlive=$stallGrandchildAlive quiesced=$($stall.TreeQuiesced)" if ($problems.Count -gt 0) { Write-Host "PIPE-DRAIN SELF-TEST: FAIL $detail -- $($problems -join '; ')" exit 1 @@ -1236,22 +1338,35 @@ try { # 3. only then the terminal UI state — done means "Hermes is back", # manual means "it is not, reopen it", error is error (and still # tries to bring the app back after showing itself). - Write-Result ($finalCode -eq 0) $finalCode $finalMsg - Remove-MarkerIfOwned - if ($finalCode -ne 0) { + if (-not $script:TreeSafeToFinalize) { + # A failed job termination means a mutating descendant may still own + # checkout/install files. Preserve the marker and do not relaunch into + # that unknown state. This is intentionally fail-closed; the marker's + # dead-owner recovery remains the next-start escape hatch. + $finalCode = 7 + $finalMsg = "Update recovery could not stop every updater process. Hermes was not restarted to avoid overlapping the active install. Wait for it to finish or restart Windows, then reopen Hermes." + Write-Result $false $finalCode $finalMsg + Write-HandoffLog $finalMsg Show-ErrorFinale $finalMsg Close-ProgressWindow - [void](Start-DesktopRelaunch) } else { - Publish-UiProgress "Opening Hermes" - $cameBack = Start-DesktopRelaunch - if (-not $cameBack -and $RelaunchExe) { - # Launch was due and did not verifiably land: truthful result - # for the next boot, manual state held on screen now. - $finalMsg = "Update complete. Reopen Hermes to finish (it could not restart itself)." - Write-Result $true 0 $finalMsg $true - Show-ManualFinale $finalMsg + Write-Result ($finalCode -eq 0) $finalCode $finalMsg + Remove-MarkerIfOwned + if ($finalCode -ne 0) { + Show-ErrorFinale $finalMsg + Close-ProgressWindow + [void](Start-DesktopRelaunch) + } else { + Publish-UiProgress "Opening Hermes" + $cameBack = Start-DesktopRelaunch + if (-not $cameBack -and $RelaunchExe) { + # Launch was due and did not verifiably land: truthful result + # for the next boot, manual state held on screen now. + $finalMsg = "Update complete. Reopen Hermes to finish (it could not restart itself)." + Write-Result $true 0 $finalMsg $true + Show-ManualFinale $finalMsg + } + Close-ProgressWindow } - Close-ProgressWindow } } diff --git a/tests/test_desktop_update_windows_pipe_drain.py b/tests/test_desktop_update_windows_pipe_drain.py index 16b0e35fe1215..5fe7e4f417d2e 100644 --- a/tests/test_desktop_update_windows_pipe_drain.py +++ b/tests/test_desktop_update_windows_pipe_drain.py @@ -76,9 +76,10 @@ def test_update_step_survives_pipe_leak_flood_and_live_child_stall( must complete in wall-clock far under what a sleep-per-chunk drain would take, and every byte must arrive. - *stall* -- a step that emits one progress line and then remains alive and - silent. It must be terminated with the timeout sentinel (124), preserve its - output, and leave no child process behind. + *stall* -- a step that emits one progress line, starts a descendant, and + then remains alive and silent. Its private Windows job must be terminated + with the timeout sentinel (124), preserve output, and report quiescence only + after both processes are gone. This is the invariant that permits retry. The existing leak/flood arms retain their measured Windows 11 / PowerShell 5.1 budgets; the stall arm uses the same real runner and process table. From bc678fd5bd8da9c299cef486f7eee2fab08bc576 Mon Sep 17 00:00:00 2001 From: fangliquanflq Date: Wed, 26 Aug 2026 23:43:52 +0800 Subject: [PATCH 3/9] fix(update): assign Windows steps before execution --- scripts/desktop-update/windows.ps1 | 197 ++++++++++++++---- .../test_desktop_update_windows_pipe_drain.py | 10 +- 2 files changed, 165 insertions(+), 42 deletions(-) diff --git a/scripts/desktop-update/windows.ps1 b/scripts/desktop-update/windows.ps1 index 0138bc4056a52..587b7b7471015 100644 --- a/scripts/desktop-update/windows.ps1 +++ b/scripts/desktop-update/windows.ps1 @@ -725,10 +725,57 @@ if (-not ("HermesUpdateJob" -as [type])) { Add-Type -TypeDefinition @' using System; using System.Diagnostics; +using System.IO; using System.Runtime.InteropServices; +using System.Text; using System.Threading; +using Microsoft.Win32.SafeHandles; public static class HermesUpdateJob { + public sealed class StartedProcess { + public Process Process; + public StreamReader StandardOutput; + public StreamReader StandardError; + public IntPtr Job; + } + + [StructLayout(LayoutKind.Sequential)] + private struct SecurityAttributes { + public int Length; + public IntPtr SecurityDescriptor; + public bool InheritHandle; + } + + [StructLayout(LayoutKind.Sequential, CharSet = CharSet.Unicode)] + private struct StartupInfo { + public int Size; + public string Reserved; + public string Desktop; + public string Title; + public int X; + public int Y; + public int XSize; + public int YSize; + public int XCountChars; + public int YCountChars; + public int FillAttribute; + public int Flags; + public short ShowWindow; + public short Reserved2; + public IntPtr Reserved2Ptr; + public IntPtr StdInput; + public IntPtr StdOutput; + public IntPtr StdError; + } + + [StructLayout(LayoutKind.Sequential)] + private struct ProcessInformation { + public IntPtr Process; + public IntPtr Thread; + public int ProcessId; + public int ThreadId; + } + [StructLayout(LayoutKind.Sequential)] private struct BasicAccountingInformation { public long TotalUserTime; @@ -747,6 +794,29 @@ public static class HermesUpdateJob { [DllImport("kernel32.dll", SetLastError = true)] private static extern bool AssignProcessToJobObject(IntPtr job, IntPtr process); + [DllImport("kernel32.dll", SetLastError = true)] + private static extern bool CreatePipe(out IntPtr read, out IntPtr write, ref SecurityAttributes attributes, int size); + + [DllImport("kernel32.dll", SetLastError = true)] + private static extern bool SetHandleInformation(IntPtr handle, int mask, int flags); + + [DllImport("kernel32.dll", CharSet = CharSet.Unicode, SetLastError = true)] + private static extern bool CreateProcess( + string applicationName, StringBuilder commandLine, + IntPtr processAttributes, IntPtr threadAttributes, bool inheritHandles, + int creationFlags, IntPtr environment, string currentDirectory, + ref StartupInfo startupInfo, out ProcessInformation processInformation + ); + + [DllImport("kernel32.dll", SetLastError = true)] + private static extern uint ResumeThread(IntPtr thread); + + [DllImport("kernel32.dll", SetLastError = true)] + private static extern bool TerminateProcess(IntPtr process, uint exitCode); + + [DllImport("kernel32.dll")] + private static extern IntPtr GetStdHandle(int standardHandle); + [DllImport("kernel32.dll", SetLastError = true)] private static extern bool TerminateJobObject(IntPtr job, uint exitCode); @@ -762,12 +832,66 @@ public static class HermesUpdateJob { [DllImport("kernel32.dll")] private static extern bool CloseHandle(IntPtr handle); - public static IntPtr CreateAndAssign(IntPtr process) { - IntPtr job = CreateJobObject(IntPtr.Zero, null); - if (job == IntPtr.Zero) return IntPtr.Zero; - if (AssignProcessToJobObject(job, process)) return job; - CloseHandle(job); - return IntPtr.Zero; + public static StartedProcess StartAssigned(string executable, string arguments) { + IntPtr job = IntPtr.Zero; + IntPtr outRead = IntPtr.Zero, outWrite = IntPtr.Zero; + IntPtr errRead = IntPtr.Zero, errWrite = IntPtr.Zero; + ProcessInformation pi = new ProcessInformation(); + try { + job = CreateJobObject(IntPtr.Zero, null); + if (job == IntPtr.Zero) throw new InvalidOperationException("CreateJobObject failed"); + SecurityAttributes sa = new SecurityAttributes(); + sa.Length = Marshal.SizeOf(typeof(SecurityAttributes)); + sa.InheritHandle = true; + if (!CreatePipe(out outRead, out outWrite, ref sa, 0) || + !CreatePipe(out errRead, out errWrite, ref sa, 0)) + throw new InvalidOperationException("CreatePipe failed"); + if (!SetHandleInformation(outRead, 1, 0) || !SetHandleInformation(errRead, 1, 0)) + throw new InvalidOperationException("SetHandleInformation failed"); + + StartupInfo si = new StartupInfo(); + si.Size = Marshal.SizeOf(typeof(StartupInfo)); + si.Flags = 0x00000100; // STARTF_USESTDHANDLES + si.StdInput = GetStdHandle(-10); + si.StdOutput = outWrite; + si.StdError = errWrite; + StringBuilder commandLine = new StringBuilder("\"" + executable + "\" " + arguments); + if (!CreateProcess(executable, commandLine, IntPtr.Zero, IntPtr.Zero, true, + 0x00000004 | 0x08000000, IntPtr.Zero, null, ref si, out pi)) + throw new InvalidOperationException("CreateProcess failed"); + if (!AssignProcessToJobObject(job, pi.Process)) { + TerminateProcess(pi.Process, 1); + throw new InvalidOperationException("AssignProcessToJobObject failed"); + } + + Process process = Process.GetProcessById(pi.ProcessId); + // Force Process to open its own stable query handle before the raw + // CreateProcess handle is closed; PS 5.1 otherwise reports a null + // ExitCode after fast children have already disappeared. + IntPtr stableProcessHandle = process.Handle; + StreamReader stdout = new StreamReader(new FileStream( + new SafeFileHandle(outRead, true), FileAccess.Read, 4096, false), Encoding.UTF8); + StreamReader stderr = new StreamReader(new FileStream( + new SafeFileHandle(errRead, true), FileAccess.Read, 4096, false), Encoding.UTF8); + outRead = IntPtr.Zero; + errRead = IntPtr.Zero; + CloseHandle(outWrite); outWrite = IntPtr.Zero; + CloseHandle(errWrite); errWrite = IntPtr.Zero; + if (ResumeThread(pi.Thread) == 0xffffffff) + throw new InvalidOperationException("ResumeThread failed"); + return new StartedProcess { Process = process, StandardOutput = stdout, StandardError = stderr, Job = job }; + } catch { + if (pi.Process != IntPtr.Zero) TerminateProcess(pi.Process, 1); + if (job != IntPtr.Zero) CloseHandle(job); + throw; + } finally { + if (pi.Thread != IntPtr.Zero) CloseHandle(pi.Thread); + if (pi.Process != IntPtr.Zero) CloseHandle(pi.Process); + if (outRead != IntPtr.Zero) CloseHandle(outRead); + if (outWrite != IntPtr.Zero) CloseHandle(outWrite); + if (errRead != IntPtr.Zero) CloseHandle(errRead); + if (errWrite != IntPtr.Zero) CloseHandle(errWrite); + } } public static bool TerminateAndWait(IntPtr job, uint exitCode, int timeoutMs) { @@ -838,52 +962,48 @@ function Invoke-HermesStep([string]$Exe, [string[]]$HermesArgs, [string]$Tag) { # log is the strictly better failure. # System.Diagnostics.Process directly: Start-Process's .ExitCode is # unreliably $null under PS 5.1 even with the Handle-touch workaround. - $psi = New-Object System.Diagnostics.ProcessStartInfo - $psi.FileName = $Exe - # .Arguments string (PS 5.1 / .NET Framework has no ArgumentList). - # Args here are fixed flags + a branch ref; quote each defensively. - $psi.Arguments = ($HermesArgs | ForEach-Object { '"{0}"' -f ($_ -replace '"', '\"') }) -join ' ' - $psi.UseShellExecute = $false - $psi.RedirectStandardOutput = $true - $psi.RedirectStandardError = $true - # hermes update prints UTF-8 (checkmarks, arrows, box glyphs). PS 5.1 - # defaults these readers to the OEM codepage, which mangles every - # multi-byte glyph into mojibake in the log. - $psi.StandardOutputEncoding = [System.Text.Encoding]::UTF8 - $psi.StandardErrorEncoding = [System.Text.Encoding]::UTF8 - # And ask the child to actually EMIT UTF-8: Python decides its stdio - # encoding from the console codepage when attached to one. - $psi.EnvironmentVariables["PYTHONIOENCODING"] = "utf-8" - $psi.EnvironmentVariables["PYTHONUTF8"] = "1" - # The idle watchdog below is only sound when Python progress is observable - # promptly. A redirected Python stdout is block-buffered by default, which - # otherwise makes an active update look silent until the buffer fills. - $psi.EnvironmentVariables["PYTHONUNBUFFERED"] = "1" - $psi.CreateNoWindow = $true - $proc = [System.Diagnostics.Process]::Start($psi) + # CREATE_SUSPENDED closes the startup race: no updater instruction can run + # before the process is assigned to its private job and resumed. + $arguments = ($HermesArgs | ForEach-Object { '"{0}"' -f ($_ -replace '"', '\"') }) -join ' ' + # CreateProcess inherits this process's environment. Set Python's encoding + # and buffering only for the atomic launch, then restore the hand-off host. + $savedPythonIoEncoding = $env:PYTHONIOENCODING + $savedPythonUtf8 = $env:PYTHONUTF8 + $savedPythonUnbuffered = $env:PYTHONUNBUFFERED + try { + $env:PYTHONIOENCODING = "utf-8" + $env:PYTHONUTF8 = "1" + $env:PYTHONUNBUFFERED = "1" + $started = [HermesUpdateJob]::StartAssigned($Exe, $arguments) + } finally { + if ($null -eq $savedPythonIoEncoding) { Remove-Item Env:PYTHONIOENCODING -ErrorAction SilentlyContinue } else { $env:PYTHONIOENCODING = $savedPythonIoEncoding } + if ($null -eq $savedPythonUtf8) { Remove-Item Env:PYTHONUTF8 -ErrorAction SilentlyContinue } else { $env:PYTHONUTF8 = $savedPythonUtf8 } + if ($null -eq $savedPythonUnbuffered) { Remove-Item Env:PYTHONUNBUFFERED -ErrorAction SilentlyContinue } else { $env:PYTHONUNBUFFERED = $savedPythonUnbuffered } + } + $proc = $started.Process + $stdoutReader = $started.StandardOutput + $stderrReader = $started.StandardError + $job = $started.Job # A job gives cancellation a kernel-enforced tree boundary. We deliberately # do NOT set KILL_ON_JOB_CLOSE: successful updates may start detached # services that are meant to outlive this pipe reader. Descendants cannot # break away from a default job, but survive when its handle is closed after # a normal step. - $job = [HermesUpdateJob]::CreateAndAssign($proc.Handle) - if ($job -eq [IntPtr]::Zero) { - Write-HandoffLog ("{0}!| could not assign pid {1} to a cancellation job; live-step timeout is disabled to prevent an unsafe partial-tree retry." -f $Tag, $proc.Id) - } + $outSink = New-Object System.Text.StringBuilder $errSink = New-Object System.Text.StringBuilder $outBuffer = New-Object char[] 16384 $errBuffer = New-Object char[] 16384 - $outTask = $proc.StandardOutput.ReadAsync($outBuffer, 0, $outBuffer.Length) - $errTask = $proc.StandardError.ReadAsync($errBuffer, 0, $errBuffer.Length) + $outTask = $stdoutReader.ReadAsync($outBuffer, 0, $outBuffer.Length) + $errTask = $stderrReader.ReadAsync($errBuffer, 0, $errBuffer.Length) $abandonAt = $null $abandoned = $false $lastProgressAt = Get-Date $stalled = $false while ($true) { $moved = $false - $outDone = Step-PipeDrain $proc.StandardOutput ([ref]$outTask) $outBuffer $outSink ([ref]$moved) - $errDone = Step-PipeDrain $proc.StandardError ([ref]$errTask) $errBuffer $errSink ([ref]$moved) + $outDone = Step-PipeDrain $stdoutReader ([ref]$outTask) $outBuffer $outSink ([ref]$moved) + $errDone = Step-PipeDrain $stderrReader ([ref]$errTask) $errBuffer $errSink ([ref]$moved) if ($moved) { $lastProgressAt = Get-Date } if ($proc.HasExited) { if ($outDone -and $errDone) { break } @@ -954,7 +1074,7 @@ function Invoke-HermesStep([string]$Exe, [string[]]$HermesArgs, [string]$Tag) { if ($errText) { $all += "`n" + $errText } $code = if ($stalled) { 124 } else { $proc.ExitCode } [HermesUpdateJob]::Close($job) - return @{ Code = $code; Output = $all; TreeQuiesced = (-not $stalled -or $proc.HasExited) } + return @{ Code = $code; Output = $all; TreeQuiesced = (-not $stalled -or $proc.HasExited); StartedAfterJobAssignment = $true } } $finalCode = 1 @@ -1145,6 +1265,7 @@ exit 0 if ($stallAlive) { $problems += "stalled child pid $stallPid remained alive after Invoke-HermesStep returned" } if ($stallGrandchildAlive) { $problems += "stalled descendant pid $stallGrandchildPid remained alive after Invoke-HermesStep returned" } if (-not $stall.TreeQuiesced) { $problems += "stall arm returned without proving its process tree quiescent" } + if (-not $stall.StartedAfterJobAssignment) { $problems += "stall arm started before cancellation-job assignment" } $detail = "leak: elapsed=${elapsed}s budget=${budget}s code=$($res.Code) grandchildAlive=$leakAlive | flood: ${floodKb}KB in ${floodElapsed}s budget=${floodBudget}s bytes=$floodBytes code=$($flood.Code) | stall: elapsed=${stallElapsed}s budget=${stallBudget}s code=$($stall.Code) childAlive=$stallAlive descendantAlive=$stallGrandchildAlive quiesced=$($stall.TreeQuiesced)" if ($problems.Count -gt 0) { diff --git a/tests/test_desktop_update_windows_pipe_drain.py b/tests/test_desktop_update_windows_pipe_drain.py index 5fe7e4f417d2e..a77948f598760 100644 --- a/tests/test_desktop_update_windows_pipe_drain.py +++ b/tests/test_desktop_update_windows_pipe_drain.py @@ -76,10 +76,12 @@ def test_update_step_survives_pipe_leak_flood_and_live_child_stall( must complete in wall-clock far under what a sleep-per-chunk drain would take, and every byte must arrive. - *stall* -- a step that emits one progress line, starts a descendant, and - then remains alive and silent. Its private Windows job must be terminated - with the timeout sentinel (124), preserve output, and report quiescence only - after both processes are gone. This is the invariant that permits retry. + *stall* -- a step that immediately starts a descendant, emits one progress + line, and then remains alive and silent. A suspended start must prevent that + first descendant from predating cancellation-job assignment. The private job + must then terminate the whole tree with timeout sentinel (124), preserve + output, and report quiescence only after both processes are gone. This is the + invariant that permits retry. The existing leak/flood arms retain their measured Windows 11 / PowerShell 5.1 budgets; the stall arm uses the same real runner and process table. From d9b655ab963999ffc245da681a9b30d71b8b249c Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 26 Aug 2026 15:35:58 -0700 Subject: [PATCH 4/9] fix(update): count logs/update.log growth as watchdog progress The #95625 watchdog cancels a step after StepIdleTimeoutSeconds (300s) with no stdout/stderr. But a real `hermes update` is stdout-silent for 40+ minutes by design: the Electron/vite build streams to logs/update.log, not the child's pipes (hermes_cli/update_cmd.py's update-log tee). An output-only ceiling would therefore kill every healthy large update at 5 minutes and mark it exit 124. The drain now fingerprints logs/update.log (size + mtime) and, when the idle ceiling is otherwise reached, treats growth of that file as progress: reset the clock instead of terminating the tree. The stat runs only once the ceiling fires, so the hot drain path never touches the filesystem. HERMES_UPDATE_STEP_IDLE_SECONDS remains the override; HERMES_UPDATE_PROGRESS_LOG points the self-test at its own file. TDD proof: -SelfTestPipeDrain gains a fourth arm, logstall -- a step that is silent on its pipes but appends to the progress log every second and must reach its natural exit 3, never 124. Linux CI pins the same contract at source level (TestIdleWatchdogCountsUpdateLogGrowth); sabotage-verified: making the log-growth consult inert fails test_stall_branch_consults_log_growth_before_terminating. --- scripts/desktop-update/windows.ps1 | 110 +++++++++++++++--- .../test_desktop_update_windows_pipe_drain.py | 75 ++++++++++++ 2 files changed, 172 insertions(+), 13 deletions(-) diff --git a/scripts/desktop-update/windows.ps1 b/scripts/desktop-update/windows.ps1 index 587b7b7471015..5ed91c952ebdc 100644 --- a/scripts/desktop-update/windows.ps1 +++ b/scripts/desktop-update/windows.ps1 @@ -721,6 +721,33 @@ if ($env:HERMES_UPDATE_STEP_IDLE_SECONDS) { } } +# Silence on the pipes is NOT silence in the update. `hermes update` captures +# the (very loud) Electron/vite build into logs/update.log instead of its own +# stdout (hermes_cli/update_cmd.py, the update-log tee), so a real update is +# routinely stdout-silent for 40+ minutes while demonstrably progressing. An +# idle ceiling that watched only stdout/stderr would cancel every healthy +# large update at StepIdleTimeoutSeconds. The drain therefore also counts +# growth of this file (size or mtime) as progress before declaring a stall. +# Overridable so the pipe-drain self-test can point it at its own file; not +# documented as a user knob. +$script:StepProgressLogPath = Join-Path $LogDir "update.log" +if ($env:HERMES_UPDATE_PROGRESS_LOG) { + $script:StepProgressLogPath = $env:HERMES_UPDATE_PROGRESS_LOG +} + +function Get-StepProgressLogStamp { + # Size + mtime fingerprint of the update log; $null when absent or + # unreadable. Comparing fingerprints between passes is how the idle + # watchdog sees a build that streams to update.log instead of stdout. + try { + $fi = New-Object System.IO.FileInfo($script:StepProgressLogPath) + if (-not $fi.Exists) { return $null } + return ('{0}:{1}' -f $fi.Length, $fi.LastWriteTimeUtc.Ticks) + } catch { + return $null + } +} + if (-not ("HermesUpdateJob" -as [type])) { Add-Type -TypeDefinition @' using System; @@ -999,6 +1026,7 @@ function Invoke-HermesStep([string]$Exe, [string[]]$HermesArgs, [string]$Tag) { $abandonAt = $null $abandoned = $false $lastProgressAt = Get-Date + $progressLogStamp = Get-StepProgressLogStamp $stalled = $false while ($true) { $moved = $false @@ -1016,17 +1044,31 @@ function Invoke-HermesStep([string]$Exe, [string[]]$HermesArgs, [string]$Tag) { break } } elseif (-not $stalled -and $job -ne [IntPtr]::Zero -and ((Get-Date) - $lastProgressAt).TotalSeconds -ge $script:StepIdleTimeoutSeconds) { - # The child is alive but has produced no observable progress for - # the whole bound. Terminate the job, not just its direct process: - # retrying while a descendant still mutates the checkout, venv, or - # release tree can overlap two installers and corrupt the install. - Write-HandoffLog ("{0}!| step stalled: no stdout/stderr for {1}s while pid {2} remained alive; cancelling its process tree." -f $Tag, $script:StepIdleTimeoutSeconds, $proc.Id) - $stalled = [HermesUpdateJob]::TerminateAndWait($job, 124, 10000) - if (-not $stalled) { - Write-HandoffLog ("{0}!| process-tree cancellation could not prove quiescence; refusing the timeout retry." -f $Tag) - $script:TreeSafeToFinalize = $false - [HermesUpdateJob]::Close($job) - throw "Unable to quiesce stalled update process tree" + # Quiet pipes are how a healthy `hermes update` looks for 40+ + # minutes: its build output streams to logs/update.log, not the + # child's stdout. Growth of that file is progress -- reset the + # clock instead of cancelling. Stat'd only once the ceiling is + # otherwise reached (at most once per 150ms pass after that), so + # the hot drain path never touches the filesystem. + $currentLogStamp = Get-StepProgressLogStamp + if ($currentLogStamp -ne $progressLogStamp) { + $progressLogStamp = $currentLogStamp + $lastProgressAt = Get-Date + } else { + # The child is alive but has produced no observable progress + # -- neither on its pipes nor in the update log -- for the + # whole bound. Terminate the job, not just its direct process: + # retrying while a descendant still mutates the checkout, + # venv, or release tree can overlap two installers and + # corrupt the install. + Write-HandoffLog ("{0}!| step stalled: no stdout/stderr for {1}s and no update.log growth while pid {2} remained alive; cancelling its process tree." -f $Tag, $script:StepIdleTimeoutSeconds, $proc.Id) + $stalled = [HermesUpdateJob]::TerminateAndWait($job, 124, 10000) + if (-not $stalled) { + Write-HandoffLog ("{0}!| process-tree cancellation could not prove quiescence; refusing the timeout retry." -f $Tag) + $script:TreeSafeToFinalize = $false + [HermesUpdateJob]::Close($job) + throw "Unable to quiesce stalled update process tree" + } } } # Only idle when both pipes came up empty this pass, and idle on the @@ -1131,6 +1173,11 @@ if ($SelfTestUi) { # stall -- a step that remains alive after its visible work and emits no more # output. Guards #95589: the hand-off must terminate it and reach its # retry/finally recovery rather than strand the Desktop. +# logstall -- a step that is silent on its pipes but keeps growing the +# update log, the shape of every real `hermes update` build (output +# goes to logs/update.log, not stdout, for 40+ minutes). Guards the +# watchdog's other cliff: the idle ceiling must count update.log +# growth as progress and must NOT kill the healthy step. if ($SelfTestPipeDrain) { New-Item -ItemType Directory -Path $LogDir -Force -ErrorAction SilentlyContinue | Out-Null $hold = 60 @@ -1146,6 +1193,8 @@ if ($SelfTestPipeDrain) { $stallPs1 = Join-Path $TempDir "hermes-step-stall-$stamp.ps1" $stallPidFile = Join-Path $TempDir "hermes-step-stall-$stamp.pid" $stallGrandchildPidFile = Join-Path $TempDir "hermes-step-stall-grandchild-$stamp.pid" + $logStallPs1 = Join-Path $TempDir "hermes-step-logstall-$stamp.ps1" + $logStallProgress = Join-Path $TempDir "hermes-step-logstall-$stamp.update.log" # UseShellExecute=$false with no redirection is what makes the grandchild # inherit our stdout/stderr -- the whole point of the fixture. Anything # that redirects (Start-Process, subprocess with stdout=DEVNULL) would @@ -1190,10 +1239,24 @@ Write-Output "step entered silent finalization" [Console]::Out.Flush() Start-Sleep -Seconds $Hold exit 0 +'@ + # Pipe-silent but log-writing: one stdout line, then only Add-Content to + # the progress log every second. With Hold far above the idle ceiling, + # surviving to exit 3 proves the watchdog counted the log growth. + $logStallSource = @' +param([int]$Hold, [string]$ProgressLog) +Write-Output "silent but logging" +[Console]::Out.Flush() +for ($i = 0; $i -lt $Hold; $i++) { + Add-Content -LiteralPath $ProgressLog -Value ("build tick {0}" -f $i) + Start-Sleep -Seconds 1 +} +exit 3 '@ [System.IO.File]::WriteAllText($childPs1, $childSource) [System.IO.File]::WriteAllText($floodPs1, $floodSource) [System.IO.File]::WriteAllText($stallPs1, $stallSource) + [System.IO.File]::WriteAllText($logStallPs1, $logStallSource) $sw = [System.Diagnostics.Stopwatch]::StartNew() $res = Invoke-HermesStep $powershell @( "-NoProfile", "-ExecutionPolicy", "Bypass", "-File", $childPs1, @@ -1242,7 +1305,24 @@ exit 0 $stallGrandchildAlive = $stallGrandchildPid -gt 0 -and [bool](Get-Process -Id $stallGrandchildPid -ErrorAction SilentlyContinue) if ($stallGrandchildAlive) { Stop-Process -Id $stallGrandchildPid -Force -ErrorAction SilentlyContinue } - Remove-Item -LiteralPath $childPs1, $floodPs1, $stallPs1, $pidFile, $stallPidFile, $stallGrandchildPidFile -Force -ErrorAction SilentlyContinue + # logstall arm: point the watchdog's progress log at the fixture's file + # for exactly this step, restore afterwards so the other arms' contract + # (no update.log in play) is untouched. + $savedProgressLogPath = $script:StepProgressLogPath + $script:StepProgressLogPath = $logStallProgress + $logStallSw = [System.Diagnostics.Stopwatch]::StartNew() + try { + $logstall = Invoke-HermesStep $powershell @( + "-NoProfile", "-ExecutionPolicy", "Bypass", "-File", $logStallPs1, + "-Hold", [string]$hold, "-ProgressLog", $logStallProgress + ) "logstall" + } finally { + $script:StepProgressLogPath = $savedProgressLogPath + } + $logStallSw.Stop() + $logStallElapsed = [Math]::Round($logStallSw.Elapsed.TotalSeconds, 2) + + Remove-Item -LiteralPath $childPs1, $floodPs1, $stallPs1, $logStallPs1, $pidFile, $stallPidFile, $stallGrandchildPidFile, $logStallProgress -Force -ErrorAction SilentlyContinue # The grandchild still being alive at return is what makes this a proof # rather than a timing coincidence: the pipe was demonstrably still open. @@ -1266,8 +1346,12 @@ exit 0 if ($stallGrandchildAlive) { $problems += "stalled descendant pid $stallGrandchildPid remained alive after Invoke-HermesStep returned" } if (-not $stall.TreeQuiesced) { $problems += "stall arm returned without proving its process tree quiescent" } if (-not $stall.StartedAfterJobAssignment) { $problems += "stall arm started before cancellation-job assignment" } + $logStallBudget = $hold + 60 + if ($logstall.Code -ne 3) { $problems += "logstall arm exit code $($logstall.Code), expected 3 -- the idle watchdog killed a pipe-silent step whose progress was visible as update.log growth (the shape of every real 40+ min build)" } + if ($logstall.Output -notmatch "silent but logging") { $problems += "logstall arm step output was lost" } + if ($logStallElapsed -ge $logStallBudget) { $problems += "logstall arm returned in ${logStallElapsed}s, over the ${logStallBudget}s budget" } - $detail = "leak: elapsed=${elapsed}s budget=${budget}s code=$($res.Code) grandchildAlive=$leakAlive | flood: ${floodKb}KB in ${floodElapsed}s budget=${floodBudget}s bytes=$floodBytes code=$($flood.Code) | stall: elapsed=${stallElapsed}s budget=${stallBudget}s code=$($stall.Code) childAlive=$stallAlive descendantAlive=$stallGrandchildAlive quiesced=$($stall.TreeQuiesced)" + $detail = "leak: elapsed=${elapsed}s budget=${budget}s code=$($res.Code) grandchildAlive=$leakAlive | flood: ${floodKb}KB in ${floodElapsed}s budget=${floodBudget}s bytes=$floodBytes code=$($flood.Code) | stall: elapsed=${stallElapsed}s budget=${stallBudget}s code=$($stall.Code) childAlive=$stallAlive descendantAlive=$stallGrandchildAlive quiesced=$($stall.TreeQuiesced) | logstall: elapsed=${logStallElapsed}s budget=${logStallBudget}s code=$($logstall.Code)" if ($problems.Count -gt 0) { Write-Host "PIPE-DRAIN SELF-TEST: FAIL $detail -- $($problems -join '; ')" exit 1 diff --git a/tests/test_desktop_update_windows_pipe_drain.py b/tests/test_desktop_update_windows_pipe_drain.py index a77948f598760..d3cd1e8ee8098 100644 --- a/tests/test_desktop_update_windows_pipe_drain.py +++ b/tests/test_desktop_update_windows_pipe_drain.py @@ -56,6 +56,74 @@ WINDOWS_PS1 = REPO_ROOT / "scripts" / "desktop-update" / "windows.ps1" +class TestIdleWatchdogCountsUpdateLogGrowth: + """The idle watchdog must count logs/update.log growth as progress. + + Real updates are stdout-silent for 40+ minutes: ``hermes update`` captures + the (very loud) Electron/vite build into ``logs/update.log`` — NOT the + child's stdout (``hermes_cli/update_cmd.py``, the update-log tee) — so the + step's pipes go quiet for the whole build while the update is demonstrably + progressing. A no-output ceiling that watches only stdout/stderr would + kill every healthy large update at ``StepIdleTimeoutSeconds`` and mark it + exit 124. + + These are source-contract assertions (the executable proof is the + ``logstall`` arm of ``-SelfTestPipeDrain``, ``windows_only`` below): + Linux CI cannot run the PowerShell hand-off, but it CAN pin that the + drain loop consults update-log growth before terminating the tree. + Sabotage-proof: removing the ``Get-StepProgressLogStamp`` consult from + the stall branch, dropping the ``logstall`` self-test arm, or dropping + the ``HERMES_UPDATE_STEP_IDLE_SECONDS`` override each fails a test here. + """ + + def _src(self) -> str: + return WINDOWS_PS1.read_text(encoding="utf-8") + + def test_progress_log_default_is_update_log(self): + src = self._src() + assert '$script:StepProgressLogPath = Join-Path $LogDir "update.log"' in src + + def test_progress_log_overridable_for_self_test(self): + assert "HERMES_UPDATE_PROGRESS_LOG" in self._src() + + def test_idle_override_env_retained(self): + # The user/test-facing idle override must survive the amendment. + assert "HERMES_UPDATE_STEP_IDLE_SECONDS" in self._src() + + def test_stall_branch_consults_log_growth_before_terminating(self): + src = self._src() + assert "function Get-StepProgressLogStamp" in src + # The consult must sit inside the idle-ceiling branch, upstream of + # TerminateAndWait: growth resets the progress clock instead of + # cancelling the tree. Pin the exact consult + compare + reset shape + # so an inert consult (or a removed one) fails here. + msg = ( + "the idle watchdog no longer checks logs/update.log growth " + "before declaring a stall -- a healthy 40+ min build whose " + "output goes to update.log would be killed at the idle ceiling" + ) + assert "$currentLogStamp = Get-StepProgressLogStamp" in src, msg + assert "if ($currentLogStamp -ne $progressLogStamp)" in src, msg + # The growth check must gate the termination: compare-and-reset + # appears before the 124 tree-termination inside the drain loop. + consult = src.index("if ($currentLogStamp -ne $progressLogStamp)") + terminate = src.index("TerminateAndWait($job, 124") + assert consult < terminate, msg + # And the clock actually resets on growth. + growth_block = src[consult:terminate] + assert "$progressLogStamp = $currentLogStamp" in growth_block, msg + assert "$lastProgressAt = Get-Date" in growth_block, msg + + def test_self_test_has_silent_but_logging_arm(self): + src = self._src() + assert "logstall" in src, ( + "-SelfTestPipeDrain lost its silent-but-logging arm: the fixture " + "no longer proves that a step which is quiet on its pipes but " + "growing update.log is NOT killed by the idle watchdog" + ) + assert "silent but logging" in src + + @pytest.mark.windows_only def test_update_step_survives_pipe_leak_flood_and_live_child_stall( tmp_path: Path, @@ -83,6 +151,13 @@ def test_update_step_survives_pipe_leak_flood_and_live_child_stall( output, and report quiescence only after both processes are gone. This is the invariant that permits retry. + *logstall* -- a step that is silent on its pipes but appends to the + progress log (pointed at the fixture's own file) every second, exiting 3. + This is the shape of every real ``hermes update`` build: output streams to + ``logs/update.log``, not stdout, for 40+ minutes. The idle watchdog must + count that growth as progress and let the step run to its natural exit + instead of killing it at the ceiling with 124. + The existing leak/flood arms retain their measured Windows 11 / PowerShell 5.1 budgets; the stall arm uses the same real runner and process table. """ From e28c8b27626f5ff0d6abd007822d5603d3582341 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 26 Aug 2026 15:35:58 -0700 Subject: [PATCH 5/9] fix(update): stop counting the Windows resume token as a fleet runtime Fixes #93406 (residual). _fleet_probe_expected_runtimes counted the _windows_gateway_resume pause/resume token (profiles/unmapped entries) as an 'expected fleet rows' signal. The token is pause/resume bookkeeping, not a runtime inventory, and its entries have no rows collect_fleet_versions() can return: unmapped Scheduled-Task gateways never publish gateway_state.json, and a resumed profile gateway relaunches detached and may not republish within the probe window. So every Windows update that paused a gateway set _fleet_rows_expected, the verification loop silently waited out its polling window (~14 min wall clock with the retry loop on user reports), printed 'Fleet version check returned no rows', and exited 1 for an update that succeeded. Expected-runtimes now keys only on row-capable signals: restart-phase bookkeeping, the pre-restart PID snapshot, and the pre-update plan inventory -- which already cover any genuinely live pre-update Windows gateway. Counterfactual proof: tests/hermes_cli/test_update_fleet_probe_resume_token.py fails on the pre-fix predicate (token-only => True) and passes with the fix; the row-capable signals are pinned unchanged. --- hermes_cli/update_cmd.py | 40 ++++---- .../test_update_fleet_check_fail_closed.py | 37 ++++---- .../test_update_fleet_probe_resume_token.py | 94 +++++++++++++++++++ 3 files changed, 138 insertions(+), 33 deletions(-) create mode 100644 tests/hermes_cli/test_update_fleet_probe_resume_token.py diff --git a/hermes_cli/update_cmd.py b/hermes_cli/update_cmd.py index 49c46a5fae9cd..a83bd8cc5a636 100644 --- a/hermes_cli/update_cmd.py +++ b/hermes_cli/update_cmd.py @@ -9656,21 +9656,36 @@ def _fleet_probe_expected_runtimes( cannot prove nothing was running; same contract as ``_restart_phase_failure_is_incomplete``, #78574). * the pre-update plan inventoried ≥1 runtime. - * the Windows pause/resume token carries paused ``profiles`` or - ``unmapped`` entries — ``_pause_windows_gateways_for_update`` / - ``_resume_windows_gateways_after_update`` populate NEITHER - ``restarted_services`` NOR ``killed_pids``, which is exactly why the - original ``(restarted_services or killed_pids)`` guard never fires on - Windows. - The same condition gates the 2.0s settle sleep: a freshly resumed Windows - gateway needs the settle window to rewrite ``gateway_state.json`` just - like a systemd-restarted one. + ``windows_resume_token`` is deliberately EXCLUDED (#93406 residual). The + pause/resume token is bookkeeping for ``_pause_windows_gateways_for_update`` + / ``_resume_windows_gateways_after_update`` — it is not a runtime + inventory, and its entries do not correspond to rows + ``collect_fleet_versions()`` is capable of returning: + + * ``unmapped`` entries (Scheduled-Task gateways) never publish + ``gateway_state.json`` rows at all, and + * a paused profile gateway is resumed as a DETACHED relaunch that may not + republish its identity within the probe window. + + Counting the token therefore made ``_fleet_rows_expected`` True on every + Windows update that had paused a gateway, the probe's polling window ran + out with zero rows on a perfectly healthy update, and verification + reported "no rows … verification incomplete" and exited 1 after a long + silent wait. Expected-runtimes must key only on signals that map to rows + the probe can actually see; a genuinely live pre-update Windows gateway + is already covered by ``pre_restart_pids`` and the plan inventory. The + parameter stays in the signature so the call site keeps passing the token + (cheap, explicit, and the docstring is where the exclusion is explained). + + The same condition gates the 2.0s settle sleep: a freshly restarted + gateway needs the settle window to rewrite ``gateway_state.json``. Note this keys ONLY on zero-rows-despite-expected-runtimes. A non-empty snapshot — including rows in ``unknown`` state — is still judged solely by ``print_fleet_version_matrix``. """ + del windows_resume_token # excluded on purpose — see docstring (#93406) if restarted_services or killed_pids: return True if pre_restart_pids is None or pre_restart_pids: @@ -9680,13 +9695,6 @@ def _fleet_probe_expected_runtimes( return True except Exception: pass - if isinstance(windows_resume_token, dict) and ( - windows_resume_token.get("profiles") - or windows_resume_token.get("unmapped") - or windows_resume_token.get("services") - or windows_resume_token.get("expected_services") - ): - return True return False diff --git a/tests/hermes_cli/test_update_fleet_check_fail_closed.py b/tests/hermes_cli/test_update_fleet_check_fail_closed.py index ab71d5e896655..6e51e8c33f36c 100644 --- a/tests/hermes_cli/test_update_fleet_check_fail_closed.py +++ b/tests/hermes_cli/test_update_fleet_check_fail_closed.py @@ -8,13 +8,15 @@ The first guard (PR #93410) keyed on ``(restarted_services or killed_pids)``, which never fires on Windows: ``_pause_windows_gateways_for_update`` / -``_resume_windows_gateways_after_update`` populate neither list, so a healthy -resumed Windows gateway still yielded zero rows and exit 0. The fix hoists -the "should the probe have produced rows?" decision into -``_fleet_probe_expected_runtimes`` and keys it on every pre-update liveness -signal: restart-phase bookkeeping, the pre-restart PID snapshot, the -pre-update plan inventory, and the Windows pause/resume token. The same -condition gates the 2.0s settle sleep. +``_resume_windows_gateways_after_update`` populate neither list. The fix +hoists the "should the probe have produced rows?" decision into +``_fleet_probe_expected_runtimes`` and keys it on the ROW-CAPABLE pre-update +liveness signals: restart-phase bookkeeping, the pre-restart PID snapshot, +and the pre-update plan inventory. The Windows pause/resume token is +deliberately NOT a signal — it is bookkeeping, not a runtime inventory, and +its entries have no corresponding ``collect_fleet_versions()`` rows (see +``test_update_fleet_probe_resume_token.py``). The same condition gates the +2.0s settle sleep. """ from __future__ import annotations @@ -47,21 +49,22 @@ def test_incomplete_when_pre_update_plan_saw_runtimes(self): is True ) - def test_incomplete_when_windows_resume_token_has_profiles(self): - # (c) The Windows pause/resume path: restarted_services and - # killed_pids stay empty by construction, so the resume token is the - # ONLY signal that a gateway was live. This is exactly the case the - # original (restarted_services or killed_pids) guard missed. + def test_windows_resume_token_alone_is_not_expected(self): + # (c) The Windows pause/resume token is EXCLUDED from the expectation + # (#93406 residual): it is pause/resume bookkeeping, not a runtime + # inventory, and collect_fleet_versions() cannot return rows for its + # entries (unmapped Scheduled-Task gateways never publish + # gateway_state.json; a resumed profile gateway relaunches detached). + # Counting it made a healthy Windows update wait out the probe window + # and exit 1 on zero rows. Full coverage lives in + # test_update_fleet_probe_resume_token.py. token = {"resume_needed": False, "profiles": {"default": 4321}} assert ( - _fleet_probe_expected_runtimes(None, [], token, [], set()) is True + _fleet_probe_expected_runtimes(None, [], token, [], set()) is False ) - - def test_incomplete_when_windows_resume_token_has_unmapped(self): - # Scheduled-Task gateways land in token["unmapped"], not profiles. token = {"resume_needed": False, "unmapped": [{"pid": 99, "argv": ["x"]}]} assert ( - _fleet_probe_expected_runtimes(None, [], token, [], set()) is True + _fleet_probe_expected_runtimes(None, [], token, [], set()) is False ) def test_incomplete_when_windows_resume_token_has_services(self): diff --git a/tests/hermes_cli/test_update_fleet_probe_resume_token.py b/tests/hermes_cli/test_update_fleet_probe_resume_token.py new file mode 100644 index 0000000000000..cc916593850d1 --- /dev/null +++ b/tests/hermes_cli/test_update_fleet_probe_resume_token.py @@ -0,0 +1,94 @@ +"""Regression for #93406 (residual) — the Windows pause/resume token is NOT a +fleet runtime and must not be counted by ``_fleet_probe_expected_runtimes``. + +The first #93406 guard counted the ``_windows_gateway_resume`` token +(``profiles`` / ``unmapped`` entries) as an "expected fleet rows" signal. But +the token is pause/resume *bookkeeping*, not a runtime inventory: + +* ``unmapped`` entries (Scheduled-Task gateways) never publish + ``gateway_state.json`` rows at all, and +* a paused-then-resumed profile gateway relaunches DETACHED and may not + republish its identity within the probe's window, + +so ``collect_fleet_versions()`` can legitimately return zero rows for a +perfectly healthy Windows update. With the token counted as an expected +runtime, ``_fleet_rows_expected`` is True, the verification loop silently +waits out its polling window (~14 min wall clock on an end-user report with +the retry loop), prints "Fleet version check returned no rows", and ``hermes +update`` exits 1 — for an update that succeeded. + +The invariant this file pins: ``_fleet_probe_expected_runtimes`` may only +return True for signals that correspond to rows ``collect_fleet_versions()`` +is actually capable of returning (restart-phase bookkeeping, the pre-restart +PID snapshot, the pre-update plan inventory). A genuinely live pre-update +Windows gateway is already covered by ``pre_restart_pids`` and the plan +inventory — the token adds no row-capable information on top. + +Counterfactual: every test in ``TestResumeTokenIsNotARuntime`` FAILS on the +pre-fix ``_fleet_probe_expected_runtimes`` (which returns True for a +token-only signal). +""" + +from __future__ import annotations + +import types + +from hermes_cli.main import _fleet_probe_expected_runtimes + + +def _plan(runtimes): + return types.SimpleNamespace(runtimes=runtimes) + + +class TestResumeTokenIsNotARuntime: + """Token-only signals must NOT mark fleet rows as expected (#93406).""" + + def test_token_profiles_alone_do_not_expect_rows(self): + # A paused/resumed profile gateway relaunches detached; its row is + # not guaranteed within the probe window. Token-only == no rows + # expected, so zero rows stays exit 0 instead of a false failure. + token = {"resume_needed": False, "profiles": {"default": 4321}} + assert ( + _fleet_probe_expected_runtimes(None, [], token, [], set()) is False + ) + + def test_token_unmapped_alone_does_not_expect_rows(self): + # Scheduled-Task gateways (token["unmapped"]) never publish + # gateway_state.json rows — collect_fleet_versions() CANNOT return a + # row for them, so they must not be counted as expected rows. + token = {"resume_needed": False, "unmapped": [{"pid": 99, "argv": ["x"]}]} + assert ( + _fleet_probe_expected_runtimes(None, [], token, [], set()) is False + ) + + def test_token_with_empty_pid_snapshot_is_still_not_expected(self): + # Even alongside an affirmatively-empty PID snapshot and an empty + # plan, the token alone must not flip the expectation. + token = {"resume_needed": True, "profiles": {"work": 777}, "unmapped": []} + assert ( + _fleet_probe_expected_runtimes(_plan([]), [], token, [], set()) + is False + ) + + +class TestRowCapableSignalsStillCount: + """The row-capable liveness signals are unaffected by the exclusion.""" + + def test_pre_restart_pids_still_expect_rows_alongside_token(self): + # A live pre-update gateway is covered by the PID snapshot — the + # row-capable signal — regardless of the token riding along. + token = {"resume_needed": False, "profiles": {"default": 4321}} + assert ( + _fleet_probe_expected_runtimes(None, [4321], token, [], set()) + is True + ) + + def test_plan_inventory_still_expects_rows_alongside_token(self): + token = {"resume_needed": False, "unmapped": [{"pid": 99, "argv": ["x"]}]} + assert ( + _fleet_probe_expected_runtimes(_plan([object()]), [], token, [], set()) + is True + ) + + def test_unreadable_pre_state_still_expects_rows(self): + assert _fleet_probe_expected_runtimes(None, None, None, [], set()) is True From 1b8fb312dc6027ed4d2238b45acc4eb451c45ac3 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 26 Aug 2026 15:55:27 -0700 Subject: [PATCH 6/9] ci: retrigger after lost webhook (outage tail) From 4efeadf7519ba0714cced12517fab7cd306c045f Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 26 Aug 2026 16:07:18 -0700 Subject: [PATCH 7/9] fix(update): normalize windows.ps1 to LF and keep fixture here-string braces off column 0 The cherry-pick landed the file with CRLF endings and a fixture here-string whose col-0 brace prematurely terminated the handoff test's SelfTest-block strip, tripping the drive-python-not-the-shim guard on fixture code. LF restored (matching main), child-script loop inlined. --- scripts/desktop-update/windows.ps1 | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/scripts/desktop-update/windows.ps1 b/scripts/desktop-update/windows.ps1 index 5ed91c952ebdc..cb8d5da4e5d6d 100644 --- a/scripts/desktop-update/windows.ps1 +++ b/scripts/desktop-update/windows.ps1 @@ -1247,10 +1247,7 @@ exit 0 param([int]$Hold, [string]$ProgressLog) Write-Output "silent but logging" [Console]::Out.Flush() -for ($i = 0; $i -lt $Hold; $i++) { - Add-Content -LiteralPath $ProgressLog -Value ("build tick {0}" -f $i) - Start-Sleep -Seconds 1 -} +for ($i = 0; $i -lt $Hold; $i++) { Add-Content -LiteralPath $ProgressLog -Value ("build tick {0}" -f $i); Start-Sleep -Seconds 1 } exit 3 '@ [System.IO.File]::WriteAllText($childPs1, $childSource) From 5ec952b0a185a42928b1ff7c98b9ea621b0439c2 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 26 Aug 2026 16:07:54 -0700 Subject: [PATCH 8/9] test: normalize CRLF in the windows.ps1 handoff guard reader .gitattributes forces eol=crlf for *.ps1, so CI checkouts hand the test CRLF content and the SelfTest-strip regex anchors never matched; it went unnoticed while the stripped fixture blocks contained no offenders. --- tests/test_desktop_update_windows_python_handoff.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/test_desktop_update_windows_python_handoff.py b/tests/test_desktop_update_windows_python_handoff.py index 08ae08dde3c5e..5a2739641f631 100644 --- a/tests/test_desktop_update_windows_python_handoff.py +++ b/tests/test_desktop_update_windows_python_handoff.py @@ -37,7 +37,10 @@ def _read() -> str: - return WINDOWS_PS1.read_text(encoding="utf-8") + # windows.ps1 is eol=crlf in .gitattributes, so checkouts materialize + # CRLF on disk (CI included). Normalize so the SelfTest-block strip's + # `\n}\n` anchors match regardless of the working-copy line endings. + return WINDOWS_PS1.read_text(encoding="utf-8").replace("\r\n", "\n") def _handoff_source() -> str: From 5c90ba17949dcf3d953c86b4e31694e33a4ffdb7 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 26 Aug 2026 17:48:28 -0700 Subject: [PATCH 9/9] test: invert the resume-token fleet-probe pin to the new contract Main's pin (added after this branch was cut) froze the #93406 bug as the contract: resume-token services demanded probe rows that SCM-paused services can never produce, stalling every healthy Windows desktop update (#95589). The pin now asserts the exclusion; restart-phase and pre-restart-pid signals keep failing closed. --- .../hermes_cli/test_update_fleet_check_fail_closed.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/hermes_cli/test_update_fleet_check_fail_closed.py b/tests/hermes_cli/test_update_fleet_check_fail_closed.py index 6e51e8c33f36c..f299e13ec1572 100644 --- a/tests/hermes_cli/test_update_fleet_check_fail_closed.py +++ b/tests/hermes_cli/test_update_fleet_check_fail_closed.py @@ -67,7 +67,13 @@ def test_windows_resume_token_alone_is_not_expected(self): _fleet_probe_expected_runtimes(None, [], token, [], set()) is False ) - def test_incomplete_when_windows_resume_token_has_services(self): + def test_windows_resume_token_services_do_not_demand_rows(self): + # Deliberately inverted from the original pin (#93406/#95589): SCM + # services the updater itself paused/resumed produce NO probe rows — + # counting them as "expected runtimes" made every healthy Windows + # desktop update stall ~14min in fleet verification and exit 1. + # The token is excluded wholesale; restart-phase and pre-restart + # signals below still fail closed. token = { "resume_needed": False, "profiles": {}, @@ -75,7 +81,7 @@ def test_incomplete_when_windows_resume_token_has_services(self): "services": ["HermesGateway"], } assert ( - _fleet_probe_expected_runtimes(None, [], token, [], set()) is True + _fleet_probe_expected_runtimes(None, [], token, [], set()) is False ) def test_incomplete_when_restart_phase_touched_gateways(self):