diff --git a/scripts/install.ps1 b/scripts/install.ps1 index aa434eed47f..b39464f42c2 100644 --- a/scripts/install.ps1 +++ b/scripts/install.ps1 @@ -22,6 +22,12 @@ param( # cloning the full default-branch history) and then `git checkout`s the # exact ref. Precedence: Commit > Tag > Branch. [string]$Commit = "", + # Apply -Commit even when it would roll an existing install BACKWARDS. + # Without this the repository stage skips a pin that is already an ancestor + # of HEAD, so a stale baked-in BUILD_PIN_COMMIT can't downgrade a current + # checkout. Reproducible/CI installs that genuinely want an older SHA on an + # existing tree pass -ForceCommit. + [switch]$ForceCommit, [string]$Tag = "", [string]$HermesHome = $(if ($env:HERMES_HOME) { $env:HERMES_HOME } else { "$env:LOCALAPPDATA\hermes" }), [string]$InstallDir = $(if ($env:HERMES_HOME) { "$env:HERMES_HOME\hermes-agent" } else { "$env:LOCALAPPDATA\hermes\hermes-agent" }), @@ -1505,8 +1511,32 @@ function Install-Repository { # Make sure we have the commit locally (a tag-less commit # SHA isn't always reachable from any one branch fetch). git -c windows.appendAtomically=false fetch origin $Commit - git -c windows.appendAtomically=false checkout --detach $Commit - if ($LASTEXITCODE -ne 0) { throw "git checkout $Commit failed (exit $LASTEXITCODE)" } + # A commit pin must never move an existing install + # BACKWARDS. hermes-setup.exe bakes its build-time commit + # into the binary (BUILD_PIN_COMMIT) and passes it as + # -Commit on every install-mode run -- including the retry + # the desktop's "Update didn't finish" screen kicks off. An + # installer built months ago would otherwise rewind a + # current checkout to its build commit, leaving ancient + # code against a current venv (npm workspaces and Python + # deps that no longer match: the #74xxx report). Skip the + # pin when the target is already an ancestor of HEAD; a + # fresh clone has no such ancestry and pins normally. + $skipRollback = $false + if (-not $ForceCommit) { + git -c windows.appendAtomically=false merge-base --is-ancestor $Commit HEAD 2>$null + $isAncestor = ($LASTEXITCODE -eq 0) + $pinnedSha = (& git -c windows.appendAtomically=false rev-parse "$Commit^{commit}" 2>$null) + $headSha = (& git -c windows.appendAtomically=false rev-parse HEAD 2>$null) + $skipRollback = $isAncestor -and ($pinnedSha -ne $headSha) + } + if ($skipRollback) { + Write-Warn "Ignoring -Commit $Commit`: the checkout is already newer." + Write-Warn "Pinning to it would roll this install back. Pass -ForceCommit to override." + } else { + git -c windows.appendAtomically=false checkout --detach $Commit + if ($LASTEXITCODE -ne 0) { throw "git checkout $Commit failed (exit $LASTEXITCODE)" } + } } elseif ($Tag) { git -c windows.appendAtomically=false fetch origin "refs/tags/${Tag}:refs/tags/${Tag}" git -c windows.appendAtomically=false checkout --detach "refs/tags/$Tag" diff --git a/scripts/install.sh b/scripts/install.sh index 119e99dc229..493b53e412c 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -73,6 +73,7 @@ SKIP_BROWSER=false NO_SKILLS=false BRANCH="main" INSTALL_COMMIT="" +FORCE_COMMIT=false ENSURE_DEPS="" MANIFEST_MODE=false @@ -117,6 +118,10 @@ while [[ $# -gt 0 ]]; do INSTALL_COMMIT="$2" shift 2 ;; + --force-commit|-ForceCommit) + FORCE_COMMIT=true + shift + ;; --manifest|-Manifest) MANIFEST_MODE=true shift @@ -165,6 +170,8 @@ while [[ $# -gt 0 ]]; do echo " 'hermes update' runs never inject bundled skills either" echo " --branch NAME Git branch to install (default: main)" echo " --commit SHA Pin checkout to a specific commit after clone/update" + echo " (ignored when it would roll an existing install back)" + echo " --force-commit Apply --commit even if it rolls the install backwards" echo " --manifest Print desktop bootstrap stage manifest as JSON" echo " --stage NAME Run one desktop bootstrap stage" echo " --json Print a JSON result frame for --stage" @@ -1330,11 +1337,31 @@ EOF cd "$INSTALL_DIR" if [ -n "$INSTALL_COMMIT" ]; then - log_info "Pinning checkout to commit $INSTALL_COMMIT..." + # A commit pin must never move an existing install BACKWARDS. The + # bootstrap installer bakes its build-time commit into the binary + # (BUILD_PIN_COMMIT) and passes it as --commit on every install-mode + # run -- including the one the desktop's failure screen retries. An + # installer built months ago would otherwise rewind a current checkout + # to its build commit, stranding the user on ancient code with a + # current venv. Only pin when the target is not already an ancestor of + # HEAD; a fresh clone has no such ancestry and pins normally. if ! git cat-file -e "$INSTALL_COMMIT^{commit}" 2>/dev/null; then git fetch origin "$INSTALL_COMMIT" || true fi - git checkout --detach "$INSTALL_COMMIT" + if git rev-parse --verify --quiet HEAD >/dev/null 2>&1 \ + && git merge-base --is-ancestor "$INSTALL_COMMIT" HEAD 2>/dev/null \ + && [ "$(git rev-parse "$INSTALL_COMMIT^{commit}" 2>/dev/null)" != "$(git rev-parse HEAD)" ]; then + if [ "$FORCE_COMMIT" = true ]; then + log_warn "--force-commit: rolling this install back to $INSTALL_COMMIT." + git checkout --detach "$INSTALL_COMMIT" + else + log_warn "Ignoring --commit $INSTALL_COMMIT: the checkout is already newer." + log_warn "Pinning to it would roll this install back. Pass --force-commit to override." + fi + else + log_info "Pinning checkout to commit $INSTALL_COMMIT..." + git checkout --detach "$INSTALL_COMMIT" + fi fi log_success "Repository ready" diff --git a/tests/test_install_commit_pin_rollback.py b/tests/test_install_commit_pin_rollback.py new file mode 100644 index 00000000000..4d6de7986cd --- /dev/null +++ b/tests/test_install_commit_pin_rollback.py @@ -0,0 +1,138 @@ +"""Regression: a stale ``--commit`` pin must not roll an install backwards. + +``hermes-setup.exe`` bakes its build-time commit into the binary +(``BUILD_PIN_COMMIT``) and passes it as ``-Commit`` / ``--commit`` on every +install-mode run — including the retry the desktop's "Update didn't finish" +screen kicks off. The repository stage used to ``git checkout --detach`` that +SHA unconditionally, so an installer built months earlier rewound a current +managed checkout to its build commit (observed: ~9k commits back), leaving +ancient source against a current venv — npm workspaces and Python deps that no +longer match, and every subsequent update failing against the wrong tree. + +The pin is skipped when its target is already an ancestor of HEAD, unless the +caller explicitly passes ``--force-commit`` / ``-ForceCommit``. A fresh clone +has no such ancestry, so reproducible/CI pinning is unaffected. + +``install.ps1`` carries the same guard (that is the path the Windows report +hit), but there is no PowerShell host in CI to execute it against a real repo, +and asserting on the script's *source text* would test its shape rather than +its behavior. These run the bash implementation of the same logic for real. +""" + +from __future__ import annotations + +import re +import shutil +import subprocess +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).resolve().parent.parent +INSTALL_SH = REPO_ROOT / "scripts" / "install.sh" + +pytestmark = pytest.mark.skipif( + shutil.which("git") is None or shutil.which("bash") is None, + reason="needs git and bash", +) + + +def _git(cwd: Path, *args: str) -> str: + return subprocess.run( + ["git", "-c", "user.email=t@t", "-c", "user.name=t", *args], + cwd=cwd, + check=True, + capture_output=True, + text=True, + ).stdout.strip() + + +def _extract_pin_block() -> str: + """Pull the commit-pin block out of install.sh's update_repo().""" + text = INSTALL_SH.read_text() + match = re.search( + r'if \[ -n "\$INSTALL_COMMIT" \]; then.*?\n fi\n', + text, + re.DOTALL, + ) + assert match is not None, "commit-pin block not found in install.sh" + return match.group(0) + + +@pytest.fixture +def repo(tmp_path): + """A checkout with three commits, HEAD at the newest.""" + origin = tmp_path / "origin" + origin.mkdir() + _git(origin, "init", "-q", "-b", "main") + shas = [] + for n in range(3): + (origin / "f.txt").write_text(f"rev{n}\n") + _git(origin, "add", "f.txt") + _git(origin, "commit", "-qm", f"rev{n}") + shas.append(_git(origin, "rev-parse", "HEAD")) + return origin, shas + + +def _run_pin_block(repo_dir: Path, commit: str, *, force: bool = False) -> str: + """Execute install.sh's pin block standalone against ``repo_dir``.""" + script = "\n".join( + [ + "set -e", + "log_info() { echo \"INFO $*\"; }", + "log_warn() { echo \"WARN $*\"; }", + f'INSTALL_COMMIT="{commit}"', + f'FORCE_COMMIT={"true" if force else "false"}', + f'cd "{repo_dir}"', + _extract_pin_block(), + ] + ) + return subprocess.run( + ["bash", "-c", script], + capture_output=True, + text=True, + check=True, + ).stdout + + +def test_stale_pin_does_not_rewind_a_newer_checkout(repo): + """The reported failure: an old baked-in pin downgrading a current tree.""" + repo_dir, shas = repo + head_before = _git(repo_dir, "rev-parse", "HEAD") + + out = _run_pin_block(repo_dir, shas[0]) + + assert _git(repo_dir, "rev-parse", "HEAD") == head_before, ( + "a pin older than HEAD must leave the checkout where it is" + ) + assert "already newer" in out + + +def test_force_commit_still_rolls_back(repo): + """Reproducible/CI installs that genuinely want an older SHA keep working.""" + repo_dir, shas = repo + + _run_pin_block(repo_dir, shas[0], force=True) + + assert _git(repo_dir, "rev-parse", "HEAD") == shas[0] + + +def test_pin_to_current_head_is_applied(repo): + """Pinning to HEAD itself is a no-op checkout, not a skipped one.""" + repo_dir, shas = repo + + out = _run_pin_block(repo_dir, shas[2]) + + assert _git(repo_dir, "rev-parse", "HEAD") == shas[2] + assert "already newer" not in out + + +def test_pin_to_a_newer_commit_is_applied(repo): + """Rolling FORWARD to a newer pin is the legitimate case — never blocked.""" + repo_dir, shas = repo + _git(repo_dir, "checkout", "-q", "--detach", shas[0]) + + out = _run_pin_block(repo_dir, shas[2]) + + assert _git(repo_dir, "rev-parse", "HEAD") == shas[2] + assert "already newer" not in out