fix(update): Windows progress server hands out its URL only once it is serving
`Start-UiServer` printed the -SelfTestUi URL (and opened the browser window) as soon as the TcpListener was bound, but the runspace that answers /progress starts asynchronously — BeginInvoke returns before the pipeline is open and the script block is JIT'd, which is seconds on a loaded runner. The kernel accepted connections into the backlog during that gap and nobody answered them. The self-test hit it three times (#90371 and two follow-ups each widened a timeout instead of removing the race) and it just failed an unrelated hermes_state.py PR (run 33591547099, two 5s stale-backlog timeouts = red). - windows.ps1: readiness handshake after BeginInvoke — one /progress round-trip must succeed (≤15s) before the server is returned; on failure tear the listener down and continue without UI. The URL now means "serving", not "bound". Also fixes the browser opening to a page that never loads on a slow machine. - test: 1s per-attempt probe timeout so a single dead backlog socket cannot consume half the readiness budget. - CI: new `desktop_updater` classifier lane. tests/test_desktop_update_windows_*.py spawn the real PowerShell script; the Windows-only job now runs them only when scripts/desktop-update/**, the Electron updater launcher, conftest, pyproject, or those tests change (push/dispatch fail open). A PR that never touched that surface cannot be failed by its process timing.
This commit is contained in:
@@ -48,6 +48,9 @@ outputs:
|
||||
installer:
|
||||
description: Run the PowerShell installer tests on a Windows runner.
|
||||
value: ${{ steps.classify.outputs.installer }}
|
||||
desktop_updater:
|
||||
description: Run the Windows desktop-update hand-off (windows.ps1) integration tests.
|
||||
value: ${{ steps.classify.outputs.desktop_updater }}
|
||||
rust:
|
||||
description: Run `cargo test` for the Tauri bootstrap installer.
|
||||
value: ${{ steps.classify.outputs.rust }}
|
||||
|
||||
@@ -49,6 +49,7 @@ jobs:
|
||||
uv_lock: ${{ steps.classify.outputs.uv_lock }}
|
||||
npm_lock: ${{ steps.classify.outputs.npm_lock }}
|
||||
installer: ${{ steps.classify.outputs.installer }}
|
||||
desktop_updater: ${{ steps.classify.outputs.desktop_updater }}
|
||||
rust: ${{ steps.classify.outputs.rust }}
|
||||
docker_meta: ${{ steps.classify.outputs.docker_meta }}
|
||||
mcp_catalog: ${{ steps.classify.outputs.mcp_catalog }}
|
||||
@@ -84,6 +85,11 @@ jobs:
|
||||
needs: detect
|
||||
if: needs.detect.outputs.python == 'true'
|
||||
uses: ./.github/workflows/tests-os.yml
|
||||
with:
|
||||
# The Windows lane spawns the real desktop-update hand-off script
|
||||
# (tests/test_desktop_update_windows_*.py) only when that surface
|
||||
# changed; unit-level windows_only tests always run.
|
||||
desktop_updater: ${{ needs.detect.outputs.desktop_updater == 'true' }}
|
||||
|
||||
lint:
|
||||
name: Python lints
|
||||
|
||||
@@ -27,6 +27,19 @@ name: OS-specific tests
|
||||
|
||||
on:
|
||||
workflow_call:
|
||||
inputs:
|
||||
desktop_updater:
|
||||
description: >-
|
||||
Run the Windows desktop-update hand-off integration tests
|
||||
(tests/test_desktop_update_windows_*.py). These spawn the real
|
||||
scripts/desktop-update/windows.ps1 and poll its loopback server, so
|
||||
they carry process-timing noise a shared runner amplifies; the
|
||||
caller gates them on the classifier's desktop_updater lane so a PR
|
||||
that never touched that surface cannot be failed by it. Push /
|
||||
dispatch runs fail open (classifier sets every lane true).
|
||||
type: boolean
|
||||
required: false
|
||||
default: true
|
||||
|
||||
permissions:
|
||||
contents: read
|
||||
@@ -134,9 +147,23 @@ jobs:
|
||||
# would therefore abort the script on any non-zero exit and the
|
||||
# exit-5 branch below would be unreachable dead code — the job
|
||||
# would still fail red, but the diagnostic would never print.
|
||||
# Desktop-update hand-off integration tests spawn the real
|
||||
# windows.ps1; deselect them unless the PR touched that surface
|
||||
# (see the workflow_call input). ``--ignore-glob`` keeps the file
|
||||
# list above intact, so a renamed test file still trips the
|
||||
# zero-tests guard rather than silently vanishing.
|
||||
# (bash 3.2 on the macOS runner: an empty array under ``set -u`` is
|
||||
# an unbound-variable error, hence the ``${arr[@]+...}`` idiom.)
|
||||
EXTRA_ARGS=()
|
||||
if [ "${{ inputs.desktop_updater }}" != "true" ]; then
|
||||
echo "desktop_updater lane off: skipping tests/test_desktop_update_windows_*.py"
|
||||
EXTRA_ARGS+=(--ignore-glob='*test_desktop_update_windows_*.py')
|
||||
fi
|
||||
|
||||
status=0
|
||||
uv run --no-sync python -m pytest \
|
||||
"$@" \
|
||||
${EXTRA_ARGS[@]+"${EXTRA_ARGS[@]}"} \
|
||||
-m "${{ matrix.marker }} and not integration" \
|
||||
-v --tb=short || status=$?
|
||||
if [ "$status" -eq 5 ]; then
|
||||
|
||||
@@ -25,6 +25,12 @@ Lanes:
|
||||
must not run it.
|
||||
* ``npm_lock`` — semantic package-lock.json diff PR comment.
|
||||
* ``installer`` — PowerShell installer tests (Windows runner).
|
||||
* ``desktop_updater`` — the Windows desktop-update hand-off script and the
|
||||
tests that drive the REAL ``windows.ps1`` (``-SelfTestUi`` / pipe drain /
|
||||
retry policy). These are integration tests of a PowerShell process on a
|
||||
shared runner; running them on every Python PR made their timing noise
|
||||
everyone's problem. They still run on push (fail-open) and whenever the
|
||||
script, its siblings, or their tests change.
|
||||
* ``rust`` — ``cargo test`` for the Tauri bootstrap installer. ``.rs``
|
||||
lives under ``apps/``, so without this lane a Rust change matched ``frontend``
|
||||
and only the TypeScript matrix ran.
|
||||
@@ -110,6 +116,17 @@ _MCP_CATALOG_FILES = {"hermes_cli/mcp_catalog.py"}
|
||||
_INSTALLER_PATHS = ("scripts/tests/",)
|
||||
_INSTALLER_FILES = {"scripts/install.ps1", "scripts/install.cmd"}
|
||||
|
||||
# Windows desktop-update hand-off (scripts/desktop-update/windows.ps1 + the
|
||||
# Electron side that launches it) and the pytest files that spawn it.
|
||||
_DESKTOP_UPDATER_PATHS = ("scripts/desktop-update/",)
|
||||
_DESKTOP_UPDATER_TEST_PREFIX = "tests/test_desktop_update_"
|
||||
_DESKTOP_UPDATER_FILES = {
|
||||
"apps/desktop/electron/updater-process.ts",
|
||||
"apps/desktop/electron/managed-ssh-update.ts",
|
||||
"tests/conftest.py",
|
||||
"pyproject.toml",
|
||||
}
|
||||
|
||||
# Rust crates — currently just the Tauri bootstrap installer (Hermes-Setup).
|
||||
# These live under ``apps/``, so before this lane existed a ``.rs`` edit matched
|
||||
# ``frontend`` and nothing more: the TypeScript matrix built, cargo never ran,
|
||||
@@ -163,6 +180,14 @@ def _is_installer(p: str) -> bool:
|
||||
return p.startswith(_INSTALLER_PATHS) or p in _INSTALLER_FILES
|
||||
|
||||
|
||||
def _is_desktop_updater(p: str) -> bool:
|
||||
return (
|
||||
p.startswith(_DESKTOP_UPDATER_PATHS)
|
||||
or p.startswith(_DESKTOP_UPDATER_TEST_PREFIX)
|
||||
or p in _DESKTOP_UPDATER_FILES
|
||||
)
|
||||
|
||||
|
||||
def _is_rust(p: str) -> bool:
|
||||
return (
|
||||
p.endswith(".rs")
|
||||
@@ -206,6 +231,7 @@ def classify(files: list[str]) -> dict[str, bool]:
|
||||
"uv_lock": any(f in ("pyproject.toml", "uv.lock") for f in files),
|
||||
"npm_lock": npm_lock,
|
||||
"installer": any(_is_installer(f) for f in files),
|
||||
"desktop_updater": any(_is_desktop_updater(f) for f in files),
|
||||
"rust": any(_is_rust(f) for f in files),
|
||||
"mcp_catalog": any(_is_mcp_catalog(f) for f in files),
|
||||
"ci_review": any(_is_ci_review(f) for f in files),
|
||||
@@ -223,6 +249,7 @@ def classify(files: list[str]) -> dict[str, bool]:
|
||||
ret["uv_lock"] = True
|
||||
ret["npm_lock"] = True
|
||||
ret["installer"] = True
|
||||
ret["desktop_updater"] = True
|
||||
ret["rust"] = True
|
||||
ret["nix"] = True
|
||||
ret["ci_review"] = True
|
||||
|
||||
@@ -212,6 +212,36 @@ function Start-UiServer([string]$HtmlPath) {
|
||||
})
|
||||
[void]$ps.BeginInvoke()
|
||||
|
||||
# Readiness handshake. BeginInvoke returns before the runspace has
|
||||
# opened its pipeline and JIT'd the script block — on a loaded machine
|
||||
# that is seconds, during which the kernel ACCEPTS connections into
|
||||
# the listener's backlog and nobody answers them. Anything that
|
||||
# trusted "listener bound" as "server serving" (the browser window
|
||||
# opening to a page that never loads; the -SelfTestUi URL that CI
|
||||
# polls) raced that gap. Prove one /progress round-trip before
|
||||
# handing the port out, so the URL means "serving", not "bound".
|
||||
$ready = $false
|
||||
$readyDeadline = [DateTime]::UtcNow.AddSeconds(15)
|
||||
while (-not $ready -and [DateTime]::UtcNow -lt $readyDeadline) {
|
||||
try {
|
||||
$probe = [System.Net.HttpWebRequest]::Create("http://127.0.0.1:$port/progress")
|
||||
$probe.Timeout = 1000
|
||||
$probe.ReadWriteTimeout = 1000
|
||||
$probe.KeepAlive = $false
|
||||
$resp = $probe.GetResponse()
|
||||
try { $ready = ([int]$resp.StatusCode -eq 200) } finally { $resp.Close() }
|
||||
} catch {
|
||||
Start-Sleep -Milliseconds 100
|
||||
}
|
||||
}
|
||||
if (-not $ready) {
|
||||
Write-HandoffLog "progress server did not answer /progress within 15s; continuing without UI"
|
||||
try { $listener.Stop() } catch {}
|
||||
try { $ps.Stop() } catch {}
|
||||
try { $rs.Close() } catch {}
|
||||
return $null
|
||||
}
|
||||
|
||||
return @{ Listener = $listener; Runspace = $rs; PowerShell = $ps; Port = $port; BrowserProc = $null; Profile = $null }
|
||||
} catch {
|
||||
try { if ($listener) { $listener.Stop() } } catch {}
|
||||
|
||||
@@ -41,13 +41,14 @@ DEFAULT = {
|
||||
"uv_lock": True,
|
||||
"npm_lock": True,
|
||||
"installer": True,
|
||||
"desktop_updater": True,
|
||||
"rust": True,
|
||||
"mcp_catalog": False,
|
||||
"ci_review": True,
|
||||
}
|
||||
|
||||
|
||||
def _lanes(python=False, frontend=False, site=False, scan=False, deps=False, uv_lock=False, npm_lock=False, installer=False, rust=False, mcp_catalog=False, docker_meta=False, ci_review=False, python_prod=None, nix=None, docker=None) -> dict[str, bool]:
|
||||
def _lanes(python=False, frontend=False, site=False, scan=False, deps=False, uv_lock=False, npm_lock=False, installer=False, desktop_updater=False, rust=False, mcp_catalog=False, docker_meta=False, ci_review=False, python_prod=None, nix=None, docker=None) -> dict[str, bool]:
|
||||
# python_prod tracks python except for tests-only diffs; default it to
|
||||
# python so the majority of cases don't need to spell it out.
|
||||
#
|
||||
@@ -69,6 +70,7 @@ def _lanes(python=False, frontend=False, site=False, scan=False, deps=False, uv_
|
||||
"uv_lock": uv_lock,
|
||||
"npm_lock": npm_lock,
|
||||
"installer": installer,
|
||||
"desktop_updater": desktop_updater,
|
||||
"rust": rust,
|
||||
"mcp_catalog": mcp_catalog,
|
||||
"ci_review": ci_review,
|
||||
@@ -78,7 +80,9 @@ def _lanes(python=False, frontend=False, site=False, scan=False, deps=False, uv_
|
||||
CASES = {
|
||||
"docs-only → nothing heavy": (["README.md", "docs/guide.md"], _lanes()),
|
||||
"python source → python": (["run_agent.py"], _lanes(python=True, scan=True)),
|
||||
"dep manifest → python": (["pyproject.toml"], _lanes(python=True, scan=True, deps=True, uv_lock=True)),
|
||||
# pyproject.toml declares the pytest markers the OS lanes select on, so it
|
||||
# also re-arms the desktop_updater integration tests (fail-open).
|
||||
"dep manifest → python": (["pyproject.toml"], _lanes(python=True, scan=True, deps=True, uv_lock=True, desktop_updater=True)),
|
||||
"uv.lock → python": (["uv.lock"], _lanes(python=True, uv_lock=True)),
|
||||
"ts package → frontend": (["apps/desktop/src/app.tsx"], _lanes(frontend=True)),
|
||||
"ui-tui → frontend": (["ui-tui/src/entry.ts"], _lanes(frontend=True)),
|
||||
@@ -141,6 +145,23 @@ CASES = {
|
||||
_lanes(python=True, installer=True),
|
||||
),
|
||||
"python source alone → no installer lane": (["run_agent.py"], _lanes(python=True, scan=True)),
|
||||
# The Windows desktop-update hand-off is a PowerShell integration surface:
|
||||
# its tests spawn the real script and poll its loopback server. They run
|
||||
# when the script, the Electron side that launches it, or their own test
|
||||
# files change — not on every hermes_state.py PR.
|
||||
"windows.ps1 → desktop_updater": (
|
||||
["scripts/desktop-update/windows.ps1"],
|
||||
_lanes(python=True, desktop_updater=True),
|
||||
),
|
||||
"desktop-update test → desktop_updater": (
|
||||
["tests/test_desktop_update_windows_progress.py"],
|
||||
_lanes(python=True, python_prod=False, scan=True, desktop_updater=True),
|
||||
),
|
||||
"updater-process.ts → desktop_updater": (
|
||||
["apps/desktop/electron/updater-process.ts"],
|
||||
_lanes(frontend=True, desktop_updater=True),
|
||||
),
|
||||
"python source alone → no desktop_updater lane": (["hermes_state.py"], _lanes(python=True, scan=True)),
|
||||
# `.rs` lives under apps/, so it matches `frontend` too. That lane builds
|
||||
# TypeScript and cannot notice a Rust error — before `rust` existed it was
|
||||
# the ONLY lane a Rust change ran, and the crate's tests never executed.
|
||||
@@ -168,9 +189,15 @@ CASES = {
|
||||
# tests-only diffs: pytest lanes stay ON, product jobs (Desktop E2E,
|
||||
# Docker) gate on python_prod and skip.
|
||||
"tests-only → python without python_prod": (
|
||||
["tests/agent/test_foo.py", "tests/conftest.py"],
|
||||
["tests/agent/test_foo.py"],
|
||||
_lanes(python=True, python_prod=False, scan=True),
|
||||
),
|
||||
# conftest.py owns the _OS_MARKS skip logic, so it re-arms the
|
||||
# desktop_updater integration tests too (fail-open).
|
||||
"conftest → python + desktop_updater": (
|
||||
["tests/conftest.py"],
|
||||
_lanes(python=True, python_prod=False, scan=True, desktop_updater=True),
|
||||
),
|
||||
"tests + prod source → both lanes": (
|
||||
["tests/agent/test_foo.py", "agent/x.py"],
|
||||
_lanes(python=True, scan=True),
|
||||
|
||||
@@ -35,17 +35,24 @@ def _read_progress(url: str, deadline: float) -> dict[str, object]:
|
||||
``urlopen(timeout=5)`` propagating TimeoutError was exactly the Aug 2026
|
||||
flake (run 32440286339). Only a listener that stays unresponsive until
|
||||
the deadline fails the test.
|
||||
|
||||
Per-attempt timeout is 1s, not 5s: a connection the kernel accepted into
|
||||
the backlog before the runspace was serving never gets answered, and a 5s
|
||||
wait on it burned half the readiness budget per attempt (two stale
|
||||
attempts = red, run 33591547099). The script's own readiness handshake
|
||||
now keeps that gap from reaching us, but the probe should not be able to
|
||||
lose the whole budget to one dead socket either way.
|
||||
"""
|
||||
last_exc: Exception | None = None
|
||||
attempted = False
|
||||
while not attempted or time.monotonic() < deadline:
|
||||
attempted = True
|
||||
try:
|
||||
with urlopen(f"{url}progress", timeout=5) as response:
|
||||
with urlopen(f"{url}progress", timeout=1) as response:
|
||||
return json.loads(response.read().decode("utf-8"))
|
||||
except (TimeoutError, OSError) as exc: # transient stall — retry
|
||||
last_exc = exc
|
||||
time.sleep(0.2)
|
||||
time.sleep(0.1)
|
||||
raise AssertionError(
|
||||
f"/progress unresponsive until deadline (last error: {last_exc!r})"
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user