diff --git a/apps/desktop/scripts/desktop-update-ui.test.mjs b/apps/desktop/scripts/desktop-update-ui.test.mjs new file mode 100644 index 0000000000..bc69320a4a --- /dev/null +++ b/apps/desktop/scripts/desktop-update-ui.test.mjs @@ -0,0 +1,100 @@ +import assert from 'node:assert/strict' +import fs from 'node:fs' +import { JSDOM } from 'jsdom' +import { afterEach, test, vi } from 'vitest' + +// Execute the shipped page, including its inline script, rather than matching +// source strings or testing a second implementation of the progress client. +const html = fs.readFileSync(new URL('../../../scripts/desktop-update/ui.html', import.meta.url), 'utf8') +const windows = [] + +function openPage(fetch) { + vi.useFakeTimers() + const dom = new JSDOM(html, { + url: 'http://127.0.0.1:12345/', + runScripts: 'dangerously', + beforeParse(window) { + window.fetch = fetch + window.AbortController = AbortController + window.setTimeout = setTimeout + window.clearTimeout = clearTimeout + window.requestAnimationFrame = () => 1 + window.cancelAnimationFrame = () => {} + } + }) + windows.push(dom.window) + return dom.window.document +} + +afterEach(() => { + windows.splice(0).forEach(window => window.close()) + vi.useRealTimers() +}) + +test.each(['done', 'manual', 'error'])('renders %s before acknowledging terminal delivery', async status => { + let document + const requests = [] + const receipt = '550e8400-e29b-41d4-a716-446655440000' + const fetch = vi.fn(async (url, options) => { + requests.push(url) + if (url.startsWith('/ack/')) { + assert.equal(options.method, 'POST') + assert.equal(document.body.className, status === 'error' ? 'error' : 'done') + assert.notEqual(document.getElementById('title').textContent, 'Updating Hermes') + return { ok: true } + } + return { ok: true, json: async () => ({ status, receipt, message: 'The updater result' }) } + }) + document = openPage(fetch) + await vi.advanceTimersByTimeAsync(1000) + assert.equal(document.body.className, status === 'error' ? 'error' : 'done') + assert.notEqual(document.getElementById('title').textContent, 'Updating Hermes') + assert.deepEqual(requests, ['/progress', `/ack/${receipt}`]) +}) + +test.each(['disconnect', 'hung', 'hung-body', 'http', 'invalid'])('bounds %s progress failures without inventing an update outcome', async failure => { + let attempts = 0 + const fetch = vi.fn((_url, options) => { + attempts++ + if (attempts === 1) { + return Promise.resolve({ ok: true, json: async () => ({ status: 'running', message: 'Installing dependencies' }) }) + } + if (failure === 'hung' || failure === 'hung-body') { + const pending = () => new Promise((_resolve, reject) => { + options.signal?.addEventListener('abort', () => reject(new Error('timeout')), { once: true }) + }) + return failure === 'hung' ? pending() : Promise.resolve({ ok: true, json: pending }) + } + if (failure === 'http') return Promise.resolve({ ok: false }) + if (failure === 'invalid') return Promise.resolve({ ok: true, json: async () => ({}) }) + return Promise.reject(new Error('connection refused')) + }) + const document = openPage(fetch) + await vi.advanceTimersByTimeAsync(20_000) + assert.equal(document.body.className, 'disconnected') + assert.equal(document.getElementById('title').textContent, 'Update status unavailable') + assert.match(document.getElementById('line').textContent, /Check Hermes/) + assert.ok(attempts <= 4, `unbounded retry loop: ${attempts}`) +}) + +test.each(['legacy', 'transient', 'ack-failure'])('preserves terminal truth with %s servers', async mode => { + let attempts = 0 + const fetch = vi.fn(async url => { + if (url.startsWith('/ack/')) throw new Error('server already stopped') + if (++attempts === 1 && mode === 'transient') throw new Error('temporary disconnect') + return { ok: true, json: async () => ({ status: 'done', ...(mode === 'ack-failure' ? { receipt: 'test-receipt' } : {}) }) } + }) + const document = openPage(fetch) + await vi.advanceTimersByTimeAsync(20_000) + assert.equal(document.body.className, 'done') + assert.equal(document.getElementById('title').textContent, 'Update complete') +}) + +test('continues displaying a healthy long update while progress remains reachable', async () => { + const fetch = vi.fn(async () => ({ ok: true, json: async () => ({ status: 'running', message: 'Building Desktop' }) })) + const document = openPage(fetch) + await vi.advanceTimersByTimeAsync(60_000) + assert.equal(document.body.className, '') + assert.equal(document.getElementById('title').textContent, 'Updating Hermes') + assert.equal(document.getElementById('line').textContent, 'Building Desktop') +}) diff --git a/scripts/desktop-update/ui.html b/scripts/desktop-update/ui.html index 6e55cb694c..3b0646b260 100644 --- a/scripts/desktop-update/ui.html +++ b/scripts/desktop-update/ui.html @@ -99,8 +99,8 @@ font-size: 11px; color: var(--foreground); } - body.done #loader, body.error #loader { display: none; } - body.done #glyph, body.error #glyph { display: flex; } + body.done #loader, body.error #loader, body.disconnected #loader { display: none; } + body.done #glyph, body.error #glyph, body.disconnected #glyph { display: flex; } @@ -205,6 +205,7 @@ const glyphEl = document.getElementById('glyph') const defaultLine = lineEl.textContent /* what a stage-less run says */ let settled = false + let failures = 0 const elapsedText = s => s < 60 ? `${s}s elapsed` : `${Math.floor(s / 60)}m ${s % 60}s elapsed` @@ -226,7 +227,8 @@ } else if (state.status === 'done') { settle('done') glyphEl.textContent = '\u2713' - lineEl.textContent = 'Opening Hermes\u2026' + titleEl.textContent = 'Update complete' + lineEl.textContent = 'Opening Hermes\u2026\nYou can close this window.' } else if (state.status === 'manual') { // Update landed but Hermes will NOT reopen itself (package skew, // sandbox helper, launch rejected). The orchestrator leaves this @@ -243,13 +245,43 @@ } } + async function request(url, options = {}) { + const controller = new AbortController() + const timeout = setTimeout(() => controller.abort(), 5000) + try { + const response = await fetch(url, { ...options, cache: 'no-store', signal: controller.signal }) + if (!response.ok) throw new Error('Progress request failed') + // Keep the deadline active while reading the body too: receiving + // headers alone does not prove that the progress server is responsive. + return options.method === 'POST' ? null : await response.json() + } finally { + clearTimeout(timeout) + } + } + async function poll() { try { - const res = await fetch('/progress', { cache: 'no-store' }) - if (res.ok) apply(await res.json()) + const state = await request('/progress') + if (!state || !['running', 'done', 'manual', 'error'].includes(state.status)) { + throw new Error('Invalid progress response') + } + failures = 0 + apply(state) + // The Windows server can now wait for delivery instead of guessing + // that one polling interval was enough. Older/POSIX servers omit this + // receipt. Apply first so a failed acknowledgement cannot hide a result. + if (settled && typeof state.receipt === 'string' && state.receipt) { + await request(`/ack/${encodeURIComponent(state.receipt)}`, { method: 'POST' }) + } } catch { - // Server gone: hold the last known state. The orchestrator owns - // closing this window; the relaunched Desktop owns the result. + if (!settled && ++failures >= 3) { + // A vanished server is not evidence of success OR failure. Leave an + // honest, finite state even if the browser refuses the close request. + settle('disconnected') + glyphEl.textContent = '!' + titleEl.textContent = 'Update status unavailable' + lineEl.textContent = 'The progress connection was lost.\nCheck Hermes for the update result. You can close this window.' + } } if (!settled) setTimeout(poll, 400) } diff --git a/scripts/desktop-update/windows.ps1 b/scripts/desktop-update/windows.ps1 index 19c9d8615a..320db0d845 100644 --- a/scripts/desktop-update/windows.ps1 +++ b/scripts/desktop-update/windows.ps1 @@ -108,6 +108,8 @@ $script:UiState = [hashtable]::Synchronized(@{ status = "running" # running | done | manual | error message = $script:UiStage clock = $script:UiStopwatch + receipt = $null + acknowledged_receipt = $null }) $script:UiServer = $null # @{ Listener; Runspace; PowerShell; Port; BrowserProc; Profile } @@ -190,15 +192,26 @@ function Start-UiServer([string]$HtmlPath) { $request = $reader.ReadLine() # Drain headers so the client doesn't see a reset mid-send. while ($true) { $h = $reader.ReadLine(); if ($null -eq $h -or $h -eq "") { break } } - if ($request -match "^GET /progress") { + if ($request -match "^GET /progress HTTP/1\.[01]$") { $elapsed = [Math]::Floor($State.clock.Elapsed.TotalSeconds) $snapshot = @{ status = $State.status message = $State.message elapsed_seconds = $elapsed + receipt = $State.receipt } | ConvertTo-Json -Compress Send-Response $stream "200 OK" "application/json; charset=utf-8" ([System.Text.Encoding]::UTF8.GetBytes($snapshot)) - } elseif ($request -match "^GET / ") { + } elseif ($request -match "^POST /ack/([^ /?]+) HTTP/1\.[01]$") { + $receipt = $Matches[1] + if ($State.status -in @("done", "manual", "error") -and $State.receipt -and $receipt -ceq $State.receipt) { + # Flush acceptance before waking the owner that will + # close the listener. No request body is needed. + Send-Response $stream "204 No Content" "text/plain" ([byte[]]@()) + $State.acknowledged_receipt = $receipt + } else { + Send-Response $stream "409 Conflict" "text/plain" ([System.Text.Encoding]::ASCII.GetBytes("unknown terminal receipt")) + } + } elseif ($request -match "^GET / HTTP/1\.[01]$") { Send-Response $stream "200 OK" "text/html; charset=utf-8" $HtmlBytes } else { Send-Response $stream "404 Not Found" "text/plain" ([System.Text.Encoding]::ASCII.GetBytes("not found")) @@ -283,11 +296,26 @@ function Stop-UiServer([switch]$LeaveWindow) { } function Publish-UiEvent([string]$Status, [string]$Message) { - # The event the shim listens for. One beat of poll latency (400ms) before - # teardown so the page actually renders the terminal state. + # A background browser can miss a fixed 900ms delivery window. Retain the + # terminal event until the page acknowledges applying this exact receipt. + # Older/headless clients cannot acknowledge, so teardown remains bounded. + $receipt = [Guid]::NewGuid().ToString('N') + $script:UiState.receipt = $receipt + $script:UiState.acknowledged_receipt = $null $script:UiState.message = $Message $script:UiState.status = $Status - if ($script:UiServer) { Start-Sleep -Milliseconds 900 } + if ($script:UiServer) { + $deliveryWait = [System.Diagnostics.Stopwatch]::StartNew() + while ($script:UiState.acknowledged_receipt -cne $receipt -and $deliveryWait.Elapsed.TotalSeconds -lt 10) { + Start-Sleep -Milliseconds 50 + if ($script:Ui) { [System.Windows.Forms.Application]::DoEvents() } + } + if ($script:UiState.acknowledged_receipt -ceq $receipt) { + Write-HandoffLog "shim: terminal state '$Status' acknowledged by the window" + } else { + Write-HandoffLog "shim: terminal state '$Status' was not acknowledged within 10s; closing the progress server" + } + } } function Get-UiElapsedText { @@ -309,6 +337,8 @@ function Publish-UiProgress([string]$Message) { $script:UiStage = $Message $script:UiState.message = $Message $script:UiState.status = "running" + $script:UiState.receipt = $null + $script:UiState.acknowledged_receipt = $null if ($script:Ui) { try { $script:Ui.Sub.Text = Get-UiProgressLine diff --git a/tests/test_desktop_update_windows_ui_delivery.py b/tests/test_desktop_update_windows_ui_delivery.py new file mode 100644 index 0000000000..dcd7b17c96 --- /dev/null +++ b/tests/test_desktop_update_windows_ui_delivery.py @@ -0,0 +1,92 @@ +"""The real Windows update server retains terminal events until acknowledged.""" + +from __future__ import annotations + +import json +import os +import re +import shutil +import subprocess +import time +from contextlib import contextmanager +from pathlib import Path +from urllib.error import HTTPError +from urllib.request import Request, urlopen + +import pytest + +pytestmark = pytest.mark.windows_only +SCRIPT = Path(__file__).resolve().parents[1] / "scripts/desktop-update/windows.ps1" + + +@contextmanager +def _server(tmp_path: Path, *, failed: bool = False): + powershell = shutil.which("powershell.exe") + assert powershell, "Windows updater tests require Windows PowerShell." + env = os.environ.copy() + env.update(TEMP=str(tmp_path), TMP=str(tmp_path), HERMES_SELFTEST_HOLD_SECONDS="0") + env.pop("HERMES_SELFTEST_FAIL", None) + if failed: + env["HERMES_SELFTEST_FAIL"] = "1" + output_path = tmp_path / "ui-delivery.log" + with output_path.open("wb") as output: + process = subprocess.Popen( + [powershell, "-NoProfile", "-ExecutionPolicy", "Bypass", "-File", str(SCRIPT), "-SelfTestUi", "-NoUi"], + env=env, + stdout=output, + stderr=subprocess.STDOUT, + creationflags=subprocess.CREATE_NO_WINDOW, + ) + try: + deadline = time.monotonic() + 30 + while time.monotonic() < deadline: + text = output_path.read_text(encoding="utf-8", errors="replace") + match = re.search(r"SELF-TEST: shim at (http://127\.0\.0\.1:\d+/)", text) + if match: + yield process, match.group(1) + return + if process.poll() is not None: + break + time.sleep(0.05) + pytest.fail(f"Update server did not publish a serving URL: {text}") + finally: + if process.poll() is None: + process.kill() + process.wait(timeout=5) + + +def _request(url: str, *, post: bool = False): + request = Request(url, data=b"" if post else None, method="POST" if post else "GET") + return urlopen(request, timeout=3) + + +@pytest.mark.parametrize("failed", [False, True], ids=["complete", "failed"]) +def test_delayed_client_receives_terminal_state_before_acknowledging(tmp_path: Path, failed: bool) -> None: + with _server(tmp_path, failed=failed) as (process, url): + # A background browser can miss the former 900ms terminal-state window. + time.sleep(2) + assert process.poll() is None, "the updater discarded its final state before the delayed client received it" + with _request(url + "progress") as response: + state = json.load(response) + assert state["status"] == ("error" if failed else "done") + receipt = state["receipt"] + assert isinstance(receipt, str) and receipt + + with pytest.raises(HTTPError) as wrong: + _request(url + "ack/not-the-published-receipt", post=True) + assert wrong.value.code == 409 + assert process.poll() is None, "an unrelated acknowledgement must not dispose the window's state" + + for route, post in [("progress-other", False), ("ack/" + receipt, False), ("ack/" + receipt + "/extra", True)]: + with pytest.raises(HTTPError) as invalid: + _request(url + route, post=post) + assert invalid.value.code == 404 + + with _request(url + "ack/" + receipt, post=True) as response: + assert response.status == 204 + assert process.wait(timeout=15) == 0 + + +def test_no_client_does_not_keep_the_update_process_alive_forever(tmp_path: Path) -> None: + with _server(tmp_path) as (process, _url): + assert process.wait(timeout=25) == 0