From cb4fa18915cc2b19666e85d30179099f59f363a0 Mon Sep 17 00:00:00 2001 From: ethernet Date: Fri, 11 Sep 2026 20:27:21 -0400 Subject: [PATCH] fix: preserve source channel in desktop update handoffs --- scripts/desktop-update/posix.sh | 31 ++- scripts/desktop-update/windows.ps1 | 17 +- tests/test_desktop_update_shim_progress.py | 1 + tests/test_desktop_update_target.py | 227 +++++++++++++++++++++ 4 files changed, 262 insertions(+), 14 deletions(-) create mode 100644 tests/test_desktop_update_target.py diff --git a/scripts/desktop-update/posix.sh b/scripts/desktop-update/posix.sh index d9ff73ce23..03883ffaa4 100755 --- a/scripts/desktop-update/posix.sh +++ b/scripts/desktop-update/posix.sh @@ -11,7 +11,7 @@ # CONTRACT (keep in sync with apps/desktop/electron/main.ts): # bash scripts/desktop-update/posix.sh # --install-root repo checkout (HERMES_HOME/hermes-agent) -# --branch branch to update against +# [--branch | --channel stable|canary|main] default: branch main # --desktop-pid the Electron main process to wait out # [--relaunch-target

] mac: running .app to swap+reopen; # linux: running binary (omit = no relaunch) @@ -36,7 +36,8 @@ set -u ORIGINAL_ARGS=("$@") -INSTALL_ROOT="" BRANCH="main" DESKTOP_PID=0 RELAUNCH_TARGET="" +INSTALL_ROOT="" BRANCH="main" CHANNEL="" DESKTOP_PID=0 RELAUNCH_TARGET="" +BRANCH_EXPLICIT=0 RELAUNCH_CWD="" SANDBOX_FALLBACK=0 RELAUNCH_ARGS=() NO_UI=0 NO_MARKER_CLEANUP=0 SELF_TEST_UI=0 SELF_TEST_GATE=0 SELF_TEST_MARKER=0 SELF_TEST_TCC_HEAL=0 @@ -44,7 +45,13 @@ HANDOFF_DAEMONIZED=0 while [ $# -gt 0 ]; do case "$1" in --install-root) INSTALL_ROOT="$2"; shift 2 ;; - --branch) BRANCH="$2"; shift 2 ;; + --branch) BRANCH="$2"; BRANCH_EXPLICIT=1; shift 2 ;; + --channel) + case "${2:-}" in + stable|canary|main) CHANNEL="$2" ;; + *) echo "--channel must be stable, canary, or main" >&2; exit 64 ;; + esac + shift 2 ;; --desktop-pid) DESKTOP_PID="$2"; shift 2 ;; --relaunch-target) RELAUNCH_TARGET="$2"; shift 2 ;; --relaunch-cwd) RELAUNCH_CWD="$2"; shift 2 ;; @@ -61,10 +68,14 @@ while [ $# -gt 0 ]; do esac done [ "$SELF_TEST_UI" -eq 1 ] || [ -n "$INSTALL_ROOT" ] || { echo "--install-root is required" >&2; exit 64; } +[ "$BRANCH_EXPLICIT" -eq 0 ] || [ -z "$CHANNEL" ] || { echo "--branch and --channel are mutually exclusive" >&2; exit 64; } +TARGET_ARGS=(--branch "$BRANCH") +[ -z "$CHANNEL" ] || TARGET_ARGS=(--channel "$CHANNEL") SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" -HERMES_HOME="${INSTALL_ROOT:+$(dirname "$INSTALL_ROOT")}" +HERMES_HOME="${HERMES_HOME:-${INSTALL_ROOT:+$(dirname "$INSTALL_ROOT")}}" HERMES_HOME="${HERMES_HOME:-${TMPDIR:-/tmp}}" +export HERMES_HOME MARKER="$HERMES_HOME/.hermes-update-in-progress" LOG_DIR="$HERMES_HOME/logs"; mkdir -p "$LOG_DIR" 2>/dev/null || true LOG="$LOG_DIR/desktop-update-handoff.log" @@ -422,10 +433,10 @@ launch_app() { # attempted BEFORE the terminal event (launch acceptance is MANUAL=0 # 1 = update landed but the user must act (result protocol field) write_result() { - printf '{"ok":%s,"exit_code":%s,"manual":%s,"message":"%s","branch":"%s","finished_at":%s}' \ + printf '{"ok":%s,"exit_code":%s,"manual":%s,"message":"%s","branch":"%s","channel":"%s","finished_at":%s}' \ "$([ "$FINAL_CODE" -eq 0 ] && echo true || echo false)" "$FINAL_CODE" \ "$([ "$MANUAL" -eq 1 ] && echo true || echo false)" \ - "$(json_escape "$FINAL_MSG")" "$(json_escape "$BRANCH")" "$(date +%s)" \ + "$(json_escape "$FINAL_MSG")" "$(json_escape "$BRANCH")" "$(json_escape "$CHANNEL")" "$(date +%s)" \ > "$RESULT.tmp" 2>/dev/null && mv -f "$RESULT.tmp" "$RESULT" 2>/dev/null || true } @@ -681,7 +692,7 @@ fi # command has returned; the already-running server keeps the inherited setting # until normal cleanup closes it. trap '' TERM -log "hand-off start: root=$INSTALL_ROOT branch=$BRANCH desktopPid=$DESKTOP_PID pid=$$" +log "hand-off start: root=$INSTALL_ROOT branch=$BRANCH channel=$CHANNEL desktopPid=$DESKTOP_PID pid=$$" rm -f "$RESULT" 2>/dev/null || true # Marker claim: same cross-process lock contract as windows.ps1 / @@ -761,9 +772,9 @@ if "${UPDATE_INVOKE[@]}" update --help 2>/dev/null | grep -q -- '--keep-stash'; else log "installed hermes predates --keep-stash; running without it" fi -log "running: ${UPDATE_INVOKE[*]} update --yes --gateway $KEEP_STASH --branch $BRANCH" +log "running: ${UPDATE_INVOKE[*]} update --yes --gateway $KEEP_STASH ${TARGET_ARGS[*]}" publish_stage "Updating code and dependencies" -OUT="$("${UPDATE_INVOKE[@]}" update --yes --gateway $KEEP_STASH --branch "$BRANCH" 2>&1)"; CODE=$? +OUT="$("${UPDATE_INVOKE[@]}" update --yes --gateway $KEEP_STASH "${TARGET_ARGS[@]}" 2>&1)"; CODE=$? printf '%s\n' "$OUT" >> "$LOG" 2>/dev/null log "hermes update exit code: $CODE" @@ -785,7 +796,7 @@ if [ "$CODE" -ne 0 ] && [ "$CODE" -ne 2 ]; then fi log "retrying once (freshly pulled fix loads on the second run)" publish_stage "Retrying update" - OUT="$("${UPDATE_INVOKE[@]}" update --yes --gateway $KEEP_STASH --branch "$BRANCH" 2>&1)"; CODE=$? + OUT="$("${UPDATE_INVOKE[@]}" update --yes --gateway $KEEP_STASH "${TARGET_ARGS[@]}" 2>&1)"; CODE=$? printf '%s\n' "$OUT" >> "$LOG" 2>/dev/null log "retry exit code: $CODE" fi diff --git a/scripts/desktop-update/windows.ps1 b/scripts/desktop-update/windows.ps1 index d2dd7dc045..120ba82fd1 100644 --- a/scripts/desktop-update/windows.ps1 +++ b/scripts/desktop-update/windows.ps1 @@ -20,7 +20,7 @@ # cmd /d /s /c start "" /min powershell -NoProfile -ExecutionPolicy Bypass # -File scripts\desktop-update\windows.ps1 # -InstallRoot repo checkout (HERMES_HOME\hermes-agent) -# -Branch branch to update against +# [-Branch | -Channel stable|canary|main] default: branch main # -DesktopPid the Electron main process to wait out # [-RelaunchExe ] Hermes.exe to start when done (omit = no relaunch) # [-NoUi] headless (tests); default shows a progress window @@ -44,6 +44,8 @@ param( [string]$InstallRoot, [string]$Branch = "main", + [ValidateSet("stable", "canary", "main")] + [string]$Channel, [int]$DesktopPid = 0, [string]$RelaunchExe = "", [switch]$NoUi, @@ -54,6 +56,11 @@ param( [switch]$SelfTestWorkingDirectory ) +if ($PSBoundParameters.ContainsKey("Branch") -and $PSBoundParameters.ContainsKey("Channel")) { + throw "-Branch and -Channel are mutually exclusive" +} +$targetArgs = if ($Channel) { @("--channel", $Channel.ToLowerInvariant()) } else { @("--branch", $Branch) } + if (-not $SelfTestUi -and -not $SelfTestPipeDrain -and -not $InstallRoot) { # Mandatory in spirit; relaxed in the signature only so the self-test # switches can drive the UI / the pipe drain without a checkout. @@ -81,7 +88,8 @@ try { $OutputEncoding = [System.Text.Encoding]::UTF8 } catch {} $TempDir = if ($env:TEMP) { $env:TEMP } else { [System.IO.Path]::GetTempPath() } -$HermesHome = if ($InstallRoot) { Split-Path -Parent $InstallRoot } else { $TempDir } +$HermesHome = if ($env:HERMES_HOME) { $env:HERMES_HOME } elseif ($InstallRoot) { Split-Path -Parent $InstallRoot } else { $TempDir } +$env:HERMES_HOME = $HermesHome $MarkerPath = Join-Path $HermesHome ".hermes-update-in-progress" $LogDir = Join-Path $HermesHome "logs" $LogPath = Join-Path $LogDir "desktop-update-handoff.log" @@ -576,6 +584,7 @@ function Write-Result([bool]$Ok, [int]$Code, [string]$Message, [bool]$ManualActi manual = $ManualAction message = $Message branch = $Branch + channel = $Channel finished_at = [DateTimeOffset]::UtcNow.ToUnixTimeSeconds() } | ConvertTo-Json -Compress [System.IO.File]::WriteAllText($ResultPath, $obj) @@ -1436,7 +1445,7 @@ try { New-Item -ItemType Directory -Path $LogDir -Force -ErrorAction SilentlyContinue | Out-Null Remove-Item -LiteralPath $ResultPath -Force -ErrorAction SilentlyContinue Show-ProgressWindow - Write-HandoffLog "hand-off start: root=$InstallRoot branch=$Branch desktopPid=$DesktopPid pid=$PID" + Write-HandoffLog "hand-off start: root=$InstallRoot branch=$Branch channel=$Channel desktopPid=$DesktopPid pid=$PID" # -- 0. Claim the update marker with OUR pid --------------------------- try { @@ -1593,7 +1602,7 @@ try { Write-HandoffLog $finalMsg exit $finalCode } - $updateArgs = @("-m", "hermes_cli.main", "update", "--yes", "--gateway", "--force", "--branch", $Branch) + $updateArgs = @("-m", "hermes_cli.main", "update", "--yes", "--gateway", "--force") + $targetArgs # --keep-stash: never re-apply local source edits after the update (they # stay parked in git stash). Probe --help first: the flag ships with newer # backends and an unknown flag would abort argparse with exit 2, which diff --git a/tests/test_desktop_update_shim_progress.py b/tests/test_desktop_update_shim_progress.py index fc34774e64..fc63fd5d48 100644 --- a/tests/test_desktop_update_shim_progress.py +++ b/tests/test_desktop_update_shim_progress.py @@ -150,6 +150,7 @@ def _run_handoff(tmp_path, exits: dict[int, int]) -> list[dict]: "HERMES_TEST_CALLS": str(calls), "HERMES_TEST_EXITS": str(tmp_path / "exits"), } + env.pop("HERMES_HOME", None) # Exercise the legacy install-parent fallback. # The hand-off daemonizes and the launcher exits immediately; the result # file is the orchestrator's own completion signal. subprocess.run( diff --git a/tests/test_desktop_update_target.py b/tests/test_desktop_update_target.py new file mode 100644 index 0000000000..49ea913e75 --- /dev/null +++ b/tests/test_desktop_update_target.py @@ -0,0 +1,227 @@ +"""Run the real handoffs against a disposable CLI, never an installed updater.""" + +from __future__ import annotations + +import json +import os +from pathlib import Path +import shlex +import subprocess +import sys + +import pytest + + +SCRIPTS = Path(__file__).resolve().parents[1] / "scripts" / "desktop-update" +FAKE_CLI = """ +import json +import os +from pathlib import Path +import sys + +if __name__ == '__main__': + if '--help' in sys.argv: + print('update options') + sys.exit(0) + receipt = Path(os.environ['HANDOFF_CAPTURE']) + previous = receipt.read_text(encoding='utf-8') if receipt.exists() else '' + with receipt.open('a', encoding='utf-8') as stream: + stream.write(json.dumps({'argv': sys.argv[1:], 'home': os.environ.get('HERMES_HOME'), + 'install_root': os.environ.get('HERMES_INSTALL_ROOT'), + 'cwd': os.getcwd()}) + '\\n') + sys.exit(1 if not previous else 0) +""" + + +def _run_handoff(tmp_path, target, *, windows=False, inherited_home=True): + install = tmp_path / "checkout with spaces" + if windows: + subprocess.run( + [sys.executable, "-m", "venv", "--without-pip", str(install / "venv")], + check=True, + capture_output=True, + timeout=60, + ) + package = ( + install / "venv" / "Lib" / "site-packages" if windows else install + ) / "hermes_cli" + package.mkdir(parents=True) + (package / "__init__.py").touch() + (package / "main.py").write_text(FAKE_CLI, encoding="utf-8") + capture = tmp_path / "calls.jsonl" + home = tmp_path / "profile home" if inherited_home else tmp_path + home.mkdir(exist_ok=True) + env = { + **os.environ, + "HOME": str(tmp_path), + "TMPDIR": str(tmp_path), + "HERMES_INSTALL_ROOT": str(install), + "HANDOFF_CAPTURE": str(capture), + } + env.pop("PYTHONPATH", None) + env.pop("PYTHONHOME", None) + env.pop("HERMES_HOME", None) + if inherited_home: + env["HERMES_HOME"] = str(home) + if windows: + # The disposable runtime has only the fixture CLI; verification is outside + # this transport contract and runs its own harmless fixture implementation. + (package / "desktop_update_verify.py").write_text( + "def verify_windows_desktop_update(): pass\n", + encoding="utf-8", + ) + command = [ + "powershell", + "-NoProfile", + "-ExecutionPolicy", + "Bypass", + "-File", + str(SCRIPTS / "windows.ps1"), + "-InstallRoot", + str(install), + "-NoUi", + ] + else: + bin_dir = install / "venv" / "bin" + bin_dir.mkdir(parents=True) + (bin_dir / "python3").symlink_to(sys.executable) + hermes = bin_dir / "hermes" + hermes.write_text( + f'#!/usr/bin/env bash\nexec {shlex.quote(sys.executable)} -m hermes_cli.main "$@"\n', + encoding="utf-8", + ) + hermes.chmod(0o755) + command = [ + "bash", + str(SCRIPTS / "posix.sh"), + "--install-root", + str(install), + "--daemonized", + "--no-ui", + ] + result = subprocess.run( + [*command, *target], + env=env, + cwd=tmp_path, + capture_output=True, + text=True, + encoding="utf-8", + timeout=90, + ) + calls = ( + [json.loads(line) for line in capture.read_text(encoding="utf-8").splitlines()] + if capture.exists() + else [] + ) + return result, calls, home, install + + +def _assert_forwarded( + tmp_path, target, expected, *, windows=False, inherited_home=True +): + result, calls, home, install = _run_handoff( + tmp_path, + target, + windows=windows, + inherited_home=inherited_home, + ) + assert result.returncode == 0, result.stdout + result.stderr + assert len(calls) == 2, calls + expected_args = ["update", "--yes", "--gateway"] + if windows: + expected_args += ["--force"] + expected_args += expected + assert ( + calls + == [ + { + "argv": expected_args, + "home": str(home), + "cwd": str(install), + "install_root": str(install), + } + ] + * 2 + ) + receipt = json.loads( + (home / ".hermes-update-result.json").read_text(encoding="utf-8-sig") + ) + assert receipt["ok"] + assert receipt["channel"] == (expected[1] if expected[0] == "--channel" else "") + assert not (home / ".hermes-update-in-progress").exists() + + +@pytest.mark.platforms("posix") +@pytest.mark.parametrize("channel", ["stable", "canary", "main"]) +def test_posix_channel_survives_retry_in_active_profile(tmp_path, channel): + _assert_forwarded(tmp_path, ["--channel", channel], ["--channel", channel]) + + +@pytest.mark.platforms("windows") +@pytest.mark.parametrize("channel", ["stable", "canary", "main"]) +def test_windows_channel_survives_retry_in_active_profile(tmp_path, channel): + _assert_forwarded( + tmp_path, ["-Channel", channel], ["--channel", channel], windows=True + ) + + +@pytest.mark.platforms("posix") +@pytest.mark.parametrize( + "target, expected", + [ + ([], ["--branch", "main"]), + (["--branch", "feature/target"], ["--branch", "feature/target"]), + ], +) +def test_posix_legacy_branch_and_default_home(tmp_path, target, expected): + _assert_forwarded(tmp_path, target, expected, inherited_home=False) + + +@pytest.mark.platforms("windows") +@pytest.mark.parametrize( + "target, expected", + [ + ([], ["--branch", "main"]), + (["-Branch", "feature/target"], ["--branch", "feature/target"]), + ], +) +def test_windows_legacy_branch_and_default_home(tmp_path, target, expected): + _assert_forwarded(tmp_path, target, expected, windows=True, inherited_home=False) + + +def _assert_rejected(tmp_path, target, *, windows=False): + result, calls, home, _ = _run_handoff(tmp_path, target, windows=windows) + assert result.returncode != 0, result.stdout + result.stderr + assert not calls + assert not (home / ".hermes-update-in-progress").exists() + assert not (home / ".hermes-update-result.json").exists() + + +@pytest.mark.platforms("posix") +@pytest.mark.parametrize( + "target", + [ + ["--channel", "nightly"], + ["--channel", ""], + ["--channel"], + ["--branch", "main", "--channel", "stable"], + ["--channel", "canary", "--branch", "main"], + ], +) +def test_posix_rejects_invalid_or_conflicting_target_before_update(tmp_path, target): + _assert_rejected(tmp_path, target) + + +@pytest.mark.platforms("windows") +@pytest.mark.parametrize( + "target", + [ + ["-Channel", "nightly"], + ["-Channel", ""], + ["-Channel"], + ["-Branch", "main", "-Channel", "stable"], + ["-Channel", "canary", "-Branch", "main"], + ], +) +def test_windows_rejects_invalid_or_conflicting_target_before_update(tmp_path, target): + _assert_rejected(tmp_path, target, windows=True)