mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-31 19:16:29 +00:00
fix(installer): never let a stale --commit pin roll an 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. The repository stage checked that SHA out unconditionally, so an installer built months earlier rewound a current managed checkout to its build commit -- 9,160 commits in the reported case -- leaving ancient source against a current venv. npm then failed on workspaces that did not exist yet at that commit, and every later update ran against the wrong tree. Skip the pin when its target is already an ancestor of HEAD. Fresh clones have no such ancestry so reproducible/CI pinning is unchanged, and --force-commit / -ForceCommit still rolls back on purpose.
This commit is contained in:
Brooklyn Nicholson
parent
fc551f9691
commit
b3daf1609c
3 changed files with 199 additions and 4 deletions
|
|
@ -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"
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
|
|
|
|||
138
tests/test_install_commit_pin_rollback.py
Normal file
138
tests/test_install_commit_pin_rollback.py
Normal file
|
|
@ -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
|
||||
Loading…
Add table
Add a link
Reference in a new issue