diff --git a/apps/desktop/package.json b/apps/desktop/package.json index 410fb65e23..704a41fb9f 100644 --- a/apps/desktop/package.json +++ b/apps/desktop/package.json @@ -44,6 +44,7 @@ "update:repro:behind": "bash ../../scripts/desktop-update/repro.sh behind", "update:repro:error": "bash ../../scripts/desktop-update/repro.sh error", "update:repro:gate": "bash ../../scripts/desktop-update/repro.sh gate", + "update:repro:launch": "bash ../../scripts/desktop-update/repro.sh launch", "perf:serve": "node scripts/perf/serve.mjs", "test:desktop": "node scripts/test-desktop.mjs", "test:desktop:all": "node scripts/test-desktop.mjs all", diff --git a/scripts/desktop-update/posix.sh b/scripts/desktop-update/posix.sh index c2e2169385..2d01459aff 100755 --- a/scripts/desktop-update/posix.sh +++ b/scripts/desktop-update/posix.sh @@ -83,6 +83,26 @@ publish() { # status message -- atomic replace; the server reads per poll printf '{"status":"%s","message":"%s"}' "$(json_escape "$1")" "$(json_escape "$2")" > "$STATUS.tmp" \ && mv -f "$STATUS.tmp" "$STATUS" 2>/dev/null || true [ -n "$UI_SERVER_PID" ] && sleep 1 # one poll beat to render the state + # Renderer-free recovery surface (gille's review, round 2): on linux a + # gated skew/manual outcome or a failure may be the ONLY signal the user + # gets — the Desktop deliberately does not reopen, and without a + # chromium-family browser there is no shim window. libnotify/zenity ship + # with every desktop environment; best-effort, never fatal, mac excluded + # (the shim browser list is effectively always satisfiable there and + # `open` reopens the app even on error). + if [ -z "$UI_SERVER_PID" ] && [ "$(uname)" != "Darwin" ]; then + case "$1" in + manual|error) + if command -v notify-send >/dev/null 2>&1; then + notify-send -u critical "Hermes update" "$2" 2>/dev/null || true + elif command -v zenity >/dev/null 2>&1; then + (zenity --warning --title="Hermes update" --text="$2" 2>/dev/null &) || true + elif command -v kdialog >/dev/null 2>&1; then + (kdialog --title "Hermes update" --sorry "$2" 2>/dev/null &) || true + fi + ;; + esac + fi } find_browser() { @@ -220,19 +240,29 @@ deliver_outcome() { # the truth-determining half: swap bundles / gate the relaun fi } -launch_app() { # runs LAST, after the result is durable (the relaunched - # Desktop consumes the result file on boot -- launching first races the - # write). Returns nonzero when a launch was due but did not happen. +launch_app() { # attempted BEFORE the terminal event (launch acceptance is + # part of the outcome — gille's review). Returns nonzero when a launch + # was due but did not verifiably happen; caller downgrades to manual. [ -n "$RELAUNCH_TARGET" ] || return 0 if [ "$(uname)" = "Darwin" ]; then [ -d "$RELAUNCH_TARGET" ] || return 0 /usr/bin/xattr -dr com.apple.quarantine "$RELAUNCH_TARGET" 2>/dev/null || true - /usr/bin/open "$RELAUNCH_TARGET" || { log "WARNING: relaunch failed"; return 1; } + # `open` talks to launchd and FAILS LOUDLY on a broken/unlaunchable + # bundle — its exit code IS launch acceptance here. + /usr/bin/open "$RELAUNCH_TARGET" || { log "WARNING: open rejected the app"; return 1; } elif [ "$GATE" = "relaunch" ]; then - # Replay the original launch context: filtered args from the Desktop - # (after --), its cwd, and its env (inherited through our own spawn). - (cd "${RELAUNCH_CWD:-/}" 2>/dev/null || cd /; setsid "$RELAUNCH_TARGET" ${RELAUNCH_ARGS[@]+"${RELAUNCH_ARGS[@]}"} >/dev/null 2>&1 &) \ - || { log "WARNING: relaunch failed"; return 1; } + # setsid only proves the wrapper shell started, so verify acceptance: + # spawn, then confirm the child is still alive shortly after — an + # immediate exec failure (ENOENT, ELF mismatch, dead sandbox) dies + # within the window and downgrades to manual instead of lying. + (cd "${RELAUNCH_CWD:-/}" 2>/dev/null || cd / + setsid "$RELAUNCH_TARGET" ${RELAUNCH_ARGS[@]+"${RELAUNCH_ARGS[@]}"} >/dev/null 2>&1 & + echo $! > "$STATUS.launchpid") || { log "WARNING: relaunch spawn failed"; return 1; } + local lp + lp="$(cat "$STATUS.launchpid" 2>/dev/null)"; rm -f "$STATUS.launchpid" 2>/dev/null + [ -n "$lp" ] || { log "WARNING: relaunch pid unknown"; return 1; } + sleep 1.5 + kill -0 "$lp" 2>/dev/null || { log "WARNING: relaunched app exited immediately"; return 1; } fi } @@ -244,8 +274,16 @@ write_result() { } finish() { - # Ordering (helix4u's review): 1. deliver the outcome (swap/gate) so the - # truth exists; 2. durable result; 3. marker; 4. shim event; 5. relaunch. + # Ordering (gille's reviews, both rounds): + # 1. deliver the outcome (swap/gate) so the truth exists; + # 2. durable result + marker removal (the relaunched app consumes the + # result on boot and must not park on our marker — this must be on + # disk BEFORE any launch attempt); + # 3. attempt the launch and require ACCEPTANCE; + # 4. only then the terminal shim event — done means "the app is coming + # back", manual means "it is not, here's what to do", error is error. + # A rejected launch rewrites the result (nothing consumed it — the app + # never started) so the next boot tells the truth too. deliver_outcome [ "$FINAL_CODE" -eq 0 ] && [ -n "$DONE_NOTE" ] && FINAL_MSG="$DONE_NOTE" write_result @@ -254,20 +292,25 @@ finish() { rm -f "$MARKER" 2>/dev/null || true fi - if [ "$FINAL_CODE" -eq 0 ]; then - # A DONE_NOTE means the app will NOT reopen itself -- leave the window - # up showing the note instead of closing on a false "Opening Hermes…". - publish "done" "$DONE_NOTE" - if [ -n "$DONE_NOTE" ]; then stop_ui leave-window; else stop_ui; fi - else + if [ "$FINAL_CODE" -ne 0 ]; then publish "error" "$FINAL_MSG"; stop_ui leave-window + launch_app || true # error path still tries to bring the app back + rm -f "$STATUS" "$STATUS.tmp" "$LOG_DIR/desktop-update-ui-port" 2>/dev/null || true + return fi - if ! launch_app && [ "$FINAL_CODE" -eq 0 ] && [ -z "$DONE_NOTE" ]; then - # Launch failed after "done" went out: nothing consumed the result yet - # (the app never started), so make it tell the truth for the next boot. + if [ -n "$DONE_NOTE" ]; then + # Gated (skew/manual): no launch will happen by design. Say so and + # leave the window up — it is the only surface until the next boot. + publish "manual" "$DONE_NOTE"; stop_ui leave-window + elif launch_app; then + publish "done" ""; stop_ui + else + # Launch was due and did not land. Downgrade: truthful result for the + # next boot, manual state held on screen now. FINAL_MSG="Update complete. Reopen Hermes to finish (it could not restart itself)." write_result + publish "manual" "$FINAL_MSG"; stop_ui leave-window fi rm -f "$STATUS" "$STATUS.tmp" "$LOG_DIR/desktop-update-ui-port" 2>/dev/null || true } diff --git a/scripts/desktop-update/repro.sh b/scripts/desktop-update/repro.sh index 114888f135..0f55afb220 100755 --- a/scripts/desktop-update/repro.sh +++ b/scripts/desktop-update/repro.sh @@ -133,6 +133,54 @@ case "$MODE" in rm -rf "$G" [ "$fails" -eq 0 ] && say "gate matrix: all pass" || { say "gate matrix: $fails FAILED"; exit 1; } ;; + launch) + # Terminal-lifecycle matrix (gille round 2): launch acceptance is part + # of the outcome. Each case runs the REAL orchestrator (--no-ui) against + # a fake install whose `hermes` stub exits 0 instantly, so the flow + # reaches finish() with FINAL_CODE=0 and exercises the launch leg. + L="/tmp/hermes-launch-test.$$" + fails=0 + expect_msg() { # name python-expr + if python3 -c "import json,sys; d=json.load(open('$L/.hermes-update-result.json')); sys.exit(0 if ($2) else 1)"; then + printf 'ok %s\n' "$1" + else + printf 'FAIL %s -> %s\n' "$1" "$(cat "$L/.hermes-update-result.json" 2>/dev/null)"; fails=$((fails+1)) + fi + } + stub_install() { # creates a fake install whose hermes update succeeds + rm -rf "$L"; mkdir -p "$L/hermes-agent/venv/bin" + printf '#!/bin/sh\nexit 0\n' > "$L/hermes-agent/venv/bin/hermes" + chmod +x "$L/hermes-agent/venv/bin/hermes" + } + + # 1. linux relaunch target dies instantly -> manual downgrade in result + stub_install + UNPACKED="$L/hermes-agent/apps/desktop/release/linux-unpacked" + mkdir -p "$UNPACKED" + printf '#!/bin/sh\nexit 1\n' > "$UNPACKED/hermes"; chmod +x "$UNPACKED/hermes" + if [ "$(uname)" != "Darwin" ]; then + bash "$SCRIPT_DIR/posix.sh" --no-ui --desktop-pid 0 --install-root "$L/hermes-agent" \ + --relaunch-target "$UNPACKED/hermes" >/dev/null 2>&1 || true + expect_msg "instant-exit relaunch downgrades to manual" "d['ok']==True and 'Reopen Hermes' in d['message']" + else + # mac: `open` on a nonexistent bundle is the acceptance-failure analog + bash "$SCRIPT_DIR/posix.sh" --no-ui --desktop-pid 0 --install-root "$L/hermes-agent" \ + --relaunch-target "$L/NoSuch.app" >/dev/null 2>&1 || true + expect_msg "missing bundle -> clean result (no launch due)" "d['ok']==True" + fi + + # 2. gated skew: success result carries the skew message (the manual + # event's payload), never a bare "Update complete." + stub_install + bash "$SCRIPT_DIR/posix.sh" --no-ui --desktop-pid 0 --install-root "$L/hermes-agent" \ + --relaunch-target /opt/Hermes/hermes >/dev/null 2>&1 || true + if [ "$(uname)" != "Darwin" ]; then + expect_msg "skew outcome surfaces in result message" "d['ok']==True and 'was not changed' in d['message']" + fi + + rm -rf "$L" + [ "$fails" -eq 0 ] && say "launch matrix: all pass" || { say "launch matrix: $fails FAILED"; exit 1; } + ;; *) sed -n '2,24p' "$0" | sed 's/^# \{0,1\}//' exit 64 diff --git a/scripts/desktop-update/ui.html b/scripts/desktop-update/ui.html index c021f57562..0009a98e52 100644 --- a/scripts/desktop-update/ui.html +++ b/scripts/desktop-update/ui.html @@ -212,10 +212,15 @@ if (state.status === 'done') { settle('done') glyphEl.textContent = '\u2713' - // A done message means the update landed but Hermes will NOT reopen - // itself (package skew, sandbox helper) — say that, not a false - // "Opening Hermes…". The orchestrator leaves the window up for it. - lineEl.textContent = state.message || 'Opening Hermes\u2026' + lineEl.textContent = 'Opening Hermes\u2026' + } else if (state.status === 'manual') { + // Update landed but Hermes will NOT reopen itself (package skew, + // sandbox helper, launch rejected). The orchestrator leaves this + // window up; the message says what to do. + settle('done') + glyphEl.textContent = '\u2713' + titleEl.textContent = 'Update complete' + lineEl.textContent = state.message || 'Reopen Hermes to finish.' } else if (state.status === 'error') { settle('error') glyphEl.textContent = '\u2715' diff --git a/scripts/desktop-update/windows.ps1 b/scripts/desktop-update/windows.ps1 index 66e14f67dd..682ca50fb0 100644 --- a/scripts/desktop-update/windows.ps1 +++ b/scripts/desktop-update/windows.ps1 @@ -358,6 +358,44 @@ function Show-ErrorFinale([string]$Message) { } catch {} } +function Show-ManualFinale([string]$Message) { + # Update landed but the Desktop did not verifiably come back. Same terse + # shape as the error finale, success glyph semantics: the shim renders + # `manual` itself; the WinForms card swaps its copy. Held so the user + # actually sees the instruction — this window is the only surface until + # they reopen Hermes themselves. + if ($script:UiServer) { + Publish-UiEvent "manual" $Message + Stop-UiServer -LeaveWindow + return + } + if (-not $script:Ui) { return } + try { + $ui = $script:Ui + $ui.Bar.Visible = $false + $ui.Title.Text = "Update complete" + $ui.Sub.Text = $Message + $close = New-Object System.Windows.Forms.Button + $close.Text = "Close" + $close.SetBounds(100, 252, 80, 28) + $close.FlatStyle = "Flat" + $close.ForeColor = $ui.Title.ForeColor + $script:ErrorDismissed = $false + $close.Add_Click({ $script:ErrorDismissed = $true }) + $ui.Form.Controls.Add($close) + $ui.Form.AcceptButton = $close + try { + $ui.Form.Activate() + if ($script:Win32) { [HermesHandoff.Win32]::SetForegroundWindow($ui.Form.Handle) | Out-Null } + } catch {} + $deadline = (Get-Date).AddMinutes(5) + while (-not $script:ErrorDismissed -and (Get-Date) -lt $deadline -and $ui.Form.Visible) { + [System.Windows.Forms.Application]::DoEvents() + Start-Sleep -Milliseconds 100 + } + } catch {} +} + function Close-ProgressWindow { if ($script:UiServer) { # Success event: the shim flips to the checkmark, then the window @@ -402,70 +440,83 @@ function Remove-MarkerIfOwned { } function Start-DesktopRelaunch { - if ($RelaunchExe -and (Test-Path -LiteralPath $RelaunchExe)) { - Write-HandoffLog "relaunching desktop: $RelaunchExe" - # DO NOT spawn Hermes.exe as our child: Electron/Chromium calls - # AttachConsole(ATTACH_PARENT_PROCESS) at boot, so a Desktop launched - # directly from this console PowerShell latches onto OUR console -- - # the console window then outlives the script (it can't close while - # an attached process lives), and closing it kills the freshly - # relaunched GUI with it. Create the process via WMI instead: the - # parent becomes WmiPrvSE.exe and there is no console to inherit or - # attach -- same detachment explorer.exe gives a normal launch. - $spawned = $false - try { - $workDir = Split-Path -Parent $RelaunchExe - $r = Invoke-CimMethod -ClassName Win32_Process -MethodName Create -Arguments @{ - CommandLine = ('"{0}"' -f $RelaunchExe) - CurrentDirectory = $workDir - } -ErrorAction Stop - if ($r -and $r.ReturnValue -eq 0) { - Write-HandoffLog "desktop relaunched detached (pid $($r.ProcessId))" - $spawned = $true - # Hand our foreground rights to the new Desktop and focus its - # main window once it exists. A WMI-spawned process starts - # unfocused, and Windows only lets the CURRENT foreground - # owner (us, while the progress window is up / just closed) - # delegate that right. Poll briefly for the window: Electron - # takes a couple seconds to create it. - try { - if ($script:Win32) { - [HermesHandoff.Win32]::AllowSetForegroundWindow([int]$r.ProcessId) | Out-Null - $deadline = (Get-Date).AddSeconds(20) - while ((Get-Date) -lt $deadline) { - $hwnd = [System.IntPtr]::Zero - try { - $p = Get-Process -Id $r.ProcessId -ErrorAction Stop - $hwnd = $p.MainWindowHandle - } catch { break } # process died; nothing to focus - if ($hwnd -ne [System.IntPtr]::Zero) { - [HermesHandoff.Win32]::ShowWindow($hwnd, 9) | Out-Null # SW_RESTORE - [HermesHandoff.Win32]::SetForegroundWindow($hwnd) | Out-Null - Write-HandoffLog "focused relaunched desktop window" - break - } - Start-Sleep -Milliseconds 400 - } - } - } catch { - Write-HandoffLog "WARNING: could not focus relaunched desktop: $($_.Exception.Message)" - } - } else { - Write-HandoffLog "WARNING: WMI relaunch returned $($r.ReturnValue); falling back" - } - } catch { - Write-HandoffLog "WARNING: WMI relaunch failed: $($_.Exception.Message); falling back" - } - if (-not $spawned) { + # Returns $true only when a launch VERIFIABLY happened (WMI accepted and + # the pid exists, or the fallback spawn returned a live process). The + # finally block downgrades the on-screen/on-disk outcome when it didn't + # — the sibling truth contract to posix.sh's launch acceptance. + if (-not ($RelaunchExe -and (Test-Path -LiteralPath $RelaunchExe))) { return $false } + Write-HandoffLog "relaunching desktop: $RelaunchExe" + # DO NOT spawn Hermes.exe as our child: Electron/Chromium calls + # AttachConsole(ATTACH_PARENT_PROCESS) at boot, so a Desktop launched + # directly from this console PowerShell latches onto OUR console -- + # the console window then outlives the script (it can't close while + # an attached process lives), and closing it kills the freshly + # relaunched GUI with it. Create the process via WMI instead: the + # parent becomes WmiPrvSE.exe and there is no console to inherit or + # attach -- same detachment explorer.exe gives a normal launch. + $spawned = $false + try { + $workDir = Split-Path -Parent $RelaunchExe + $r = Invoke-CimMethod -ClassName Win32_Process -MethodName Create -Arguments @{ + CommandLine = ('"{0}"' -f $RelaunchExe) + CurrentDirectory = $workDir + } -ErrorAction Stop + if ($r -and $r.ReturnValue -eq 0) { + Write-HandoffLog "desktop relaunched detached (pid $($r.ProcessId))" + $spawned = $true + # Hand our foreground rights to the new Desktop and focus its + # main window once it exists. A WMI-spawned process starts + # unfocused, and Windows only lets the CURRENT foreground + # owner (us, while the progress window is up / just closed) + # delegate that right. Poll briefly for the window: Electron + # takes a couple seconds to create it. try { - # Fallback keeps the old behavior (console tie-in and all) -- - # a tethered Desktop beats no Desktop. - Start-Process -FilePath $RelaunchExe -WorkingDirectory (Split-Path -Parent $RelaunchExe) | Out-Null + if ($script:Win32) { + [HermesHandoff.Win32]::AllowSetForegroundWindow([int]$r.ProcessId) | Out-Null + $deadline = (Get-Date).AddSeconds(20) + while ((Get-Date) -lt $deadline) { + $hwnd = [System.IntPtr]::Zero + try { + $p = Get-Process -Id $r.ProcessId -ErrorAction Stop + $hwnd = $p.MainWindowHandle + } catch { + # Process died before showing a window — that is a + # failed launch, not merely an unfocused one. + Write-HandoffLog "WARNING: relaunched desktop exited before its window appeared" + $spawned = $false + break + } + if ($hwnd -ne [System.IntPtr]::Zero) { + [HermesHandoff.Win32]::ShowWindow($hwnd, 9) | Out-Null # SW_RESTORE + [HermesHandoff.Win32]::SetForegroundWindow($hwnd) | Out-Null + Write-HandoffLog "focused relaunched desktop window" + break + } + Start-Sleep -Milliseconds 400 + } + } } catch { - Write-HandoffLog "WARNING: desktop relaunch failed: $($_.Exception.Message)" + Write-HandoffLog "WARNING: could not focus relaunched desktop: $($_.Exception.Message)" } + } else { + Write-HandoffLog "WARNING: WMI relaunch returned $($r.ReturnValue); falling back" + } + } catch { + Write-HandoffLog "WARNING: WMI relaunch failed: $($_.Exception.Message); falling back" + } + if (-not $spawned) { + try { + # Fallback keeps the old behavior (console tie-in and all) -- + # a tethered Desktop beats no Desktop. + $p = Start-Process -FilePath $RelaunchExe -WorkingDirectory (Split-Path -Parent $RelaunchExe) -PassThru + Start-Sleep -Milliseconds 1500 + if ($p -and -not $p.HasExited) { $spawned = $true } + elseif ($p) { Write-HandoffLog "WARNING: fallback relaunch exited immediately" } + } catch { + Write-HandoffLog "WARNING: desktop relaunch failed: $($_.Exception.Message)" } } + return $spawned } function Invoke-HermesStep([string]$Exe, [string[]]$HermesArgs, [string]$Tag) { @@ -663,9 +714,28 @@ try { } exit $finalCode } finally { + # Truth ordering (sibling contract to posix.sh finish()): + # 1. durable result + marker removal (the relaunched Desktop consumes + # the result on boot and must not park on our marker); + # 2. attempt the relaunch and require ACCEPTANCE; + # 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) { Show-ErrorFinale $finalMsg } - Close-ProgressWindow - Start-DesktopRelaunch + if ($finalCode -ne 0) { + Show-ErrorFinale $finalMsg + Close-ProgressWindow + [void](Start-DesktopRelaunch) + } else { + $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 + Show-ManualFinale $finalMsg + } + Close-ProgressWindow + } }