diff --git a/.github/actions/detect-changes/action.yml b/.github/actions/detect-changes/action.yml index ade05ba124..6ce85c6b7b 100644 --- a/.github/actions/detect-changes/action.yml +++ b/.github/actions/detect-changes/action.yml @@ -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 }} diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index ea5127a3cd..aaa8da0386 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -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 diff --git a/.github/workflows/tests-os.yml b/.github/workflows/tests-os.yml index 12719a853c..2477c839f4 100644 --- a/.github/workflows/tests-os.yml +++ b/.github/workflows/tests-os.yml @@ -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 diff --git a/scripts/ci/classify_changes.py b/scripts/ci/classify_changes.py index 935703c870..71afcda686 100644 --- a/scripts/ci/classify_changes.py +++ b/scripts/ci/classify_changes.py @@ -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 diff --git a/scripts/desktop-update/windows.ps1 b/scripts/desktop-update/windows.ps1 index 7067cff335..19c9d8615a 100644 --- a/scripts/desktop-update/windows.ps1 +++ b/scripts/desktop-update/windows.ps1 @@ -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 {} diff --git a/tests/ci/test_classify_changes.py b/tests/ci/test_classify_changes.py index 81e93d6809..43e33dd620 100644 --- a/tests/ci/test_classify_changes.py +++ b/tests/ci/test_classify_changes.py @@ -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), diff --git a/tests/test_desktop_update_windows_progress.py b/tests/test_desktop_update_windows_progress.py index 031971570e..70708d754b 100644 --- a/tests/test_desktop_update_windows_progress.py +++ b/tests/test_desktop_update_windows_progress.py @@ -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})" )