Merge pull request #74436 from NousResearch/bb/update-mutex
fix(update): one updater at a time, and never roll an install backwards
This commit is contained in:
brooklyn!
GitHub
commit
4b7f709843
|
|
@ -66,6 +66,10 @@ windows-sys = { version = "0.59", features = [
|
|||
"Win32_UI_WindowsAndMessaging",
|
||||
] }
|
||||
|
||||
# Signal-0 liveness probe for the update-lock marker owner (update.rs).
|
||||
[target.'cfg(unix)'.dependencies]
|
||||
libc = "0.2"
|
||||
|
||||
[profile.release]
|
||||
# A 5-10MB signed installer is the goal. LTO + size-opt + single codegen unit.
|
||||
panic = "abort"
|
||||
|
|
|
|||
|
|
@ -107,16 +107,97 @@ pub async fn start_update(app: AppHandle) -> Result<(), String> {
|
|||
/// future desktop launches. The marker payload is `{pid}\n{started_at_unix}`
|
||||
/// so the desktop's launch gate can detect a stale marker (dead PID / past a
|
||||
/// hard ceiling) and self-heal rather than wait forever.
|
||||
///
|
||||
/// The marker is also the cross-process update lock: `hermes update` claims
|
||||
/// the same file (see `hermes_cli/update_lock.py`) so a dashboard-spawned
|
||||
/// update and this updater can't mutate one checkout at the same time.
|
||||
/// `acquire` therefore REFUSES when a live foreign owner holds it rather than
|
||||
/// overwriting — the pre-fix clobber is what let a dashboard `hermes update`
|
||||
/// keep running while install-mode bootstrap rewrote the tree underneath it.
|
||||
struct UpdateMarkerGuard {
|
||||
path: PathBuf,
|
||||
/// False when a live foreign updater already owns the marker: we hold no
|
||||
/// claim, so `Drop` must not delete their marker.
|
||||
owned: bool,
|
||||
}
|
||||
|
||||
/// Never treat a marker older than this as a live update. Mirrors
|
||||
/// UPDATE_MARKER_MAX_AGE_MS in apps/desktop/electron/update-marker.ts and
|
||||
/// UPDATE_MARKER_MAX_AGE_SECONDS in hermes_cli/update_lock.py — all three read
|
||||
/// this one file, so a shorter ceiling in any of them would steal a lock the
|
||||
/// others still consider live.
|
||||
const UPDATE_MARKER_MAX_AGE_SECS: u64 = 20 * 60;
|
||||
|
||||
/// The pid + age of a confirmed-live update holding the marker.
|
||||
struct MarkerOwner {
|
||||
pid: u32,
|
||||
age_secs: u64,
|
||||
}
|
||||
|
||||
/// Read the marker and report a live owner, if any. `None` for every "no live
|
||||
/// update" case — absent, unreadable, malformed, dead pid, past the ceiling —
|
||||
/// matching `readLiveUpdateMarker` in the Electron gate. Never panics.
|
||||
fn live_marker_owner(path: &Path) -> Option<MarkerOwner> {
|
||||
let raw = std::fs::read_to_string(path).ok()?;
|
||||
let mut lines = raw.lines();
|
||||
let pid: u32 = lines.next()?.trim().parse().ok()?;
|
||||
let started_at: u64 = lines.next().unwrap_or("").trim().parse().unwrap_or(0);
|
||||
let now = std::time::SystemTime::now()
|
||||
.duration_since(std::time::UNIX_EPOCH)
|
||||
.map(|d| d.as_secs())
|
||||
.unwrap_or(0);
|
||||
let age_secs = now.saturating_sub(started_at);
|
||||
if age_secs > UPDATE_MARKER_MAX_AGE_SECS || !pid_is_alive(pid) {
|
||||
return None;
|
||||
}
|
||||
Some(MarkerOwner { pid, age_secs })
|
||||
}
|
||||
|
||||
/// True when a process with `pid` currently exists.
|
||||
#[cfg(windows)]
|
||||
fn pid_is_alive(pid: u32) -> bool {
|
||||
use windows_sys::Win32::Foundation::{CloseHandle, STILL_ACTIVE};
|
||||
use windows_sys::Win32::System::Threading::{
|
||||
GetExitCodeProcess, OpenProcess, PROCESS_QUERY_LIMITED_INFORMATION,
|
||||
};
|
||||
|
||||
unsafe {
|
||||
let handle = OpenProcess(PROCESS_QUERY_LIMITED_INFORMATION, 0, pid);
|
||||
if handle.is_null() {
|
||||
// Either the pid is gone or we lack rights to open it. A pid we
|
||||
// can't inspect is treated as dead so an unopenable straggler
|
||||
// can't wedge every future update.
|
||||
return false;
|
||||
}
|
||||
let mut code: u32 = 0;
|
||||
let ok = GetExitCodeProcess(handle, &mut code);
|
||||
CloseHandle(handle);
|
||||
ok != 0 && code == STILL_ACTIVE as u32
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(not(windows))]
|
||||
fn pid_is_alive(pid: u32) -> bool {
|
||||
// signal 0 delivers nothing; it only probes existence/permission.
|
||||
// ESRCH => dead. EPERM => alive but owned by another user.
|
||||
let rc = unsafe { libc::kill(pid as libc::pid_t, 0) };
|
||||
if rc == 0 {
|
||||
return true;
|
||||
}
|
||||
std::io::Error::last_os_error().raw_os_error() == Some(libc::EPERM)
|
||||
}
|
||||
|
||||
impl UpdateMarkerGuard {
|
||||
/// Write the marker. Best-effort: a write failure must NOT abort the
|
||||
/// update (the gate degrades to "no marker => proceed", i.e. exactly the
|
||||
/// pre-fix behavior), so we log and carry on with a guard that still
|
||||
/// attempts cleanup of whatever may exist at the path.
|
||||
fn acquire(path: PathBuf) -> Self {
|
||||
/// Claim the marker, or report the live updater that already owns it.
|
||||
///
|
||||
/// Writing is best-effort: a write failure must NOT abort the update (the
|
||||
/// gate degrades to "no marker => proceed", i.e. exactly the pre-marker
|
||||
/// behavior), so we log and carry on with a guard that still attempts
|
||||
/// cleanup of whatever may exist at the path.
|
||||
fn acquire(path: PathBuf) -> Result<Self, MarkerOwner> {
|
||||
if let Some(owner) = live_marker_owner(&path) {
|
||||
return Err(owner);
|
||||
}
|
||||
let pid = std::process::id();
|
||||
let started_at = std::time::SystemTime::now()
|
||||
.duration_since(std::time::UNIX_EPOCH)
|
||||
|
|
@ -128,17 +209,32 @@ impl UpdateMarkerGuard {
|
|||
if let Err(err) = std::fs::write(&path, format!("{pid}\n{started_at}")) {
|
||||
tracing::warn!(?path, %err, "could not write update-in-progress marker");
|
||||
}
|
||||
Self { path }
|
||||
Ok(Self { path, owned: true })
|
||||
}
|
||||
|
||||
/// Release the marker as soon as every mutating stage has completed.
|
||||
///
|
||||
/// The updater still owns a Tauri/Cocoa event loop while it relaunches the
|
||||
/// desktop, and that loop can outlive `app.exit(0)`. Relying on `Drop`
|
||||
/// alone therefore leaves a *successful* update looking active — a live
|
||||
/// pid holding a fresh marker — which blocks desktop startup and every
|
||||
/// other updater for the full age ceiling. Idempotent: `Drop` still runs
|
||||
/// and tolerates an already-removed marker.
|
||||
fn complete(&self) {
|
||||
if !self.owned {
|
||||
return;
|
||||
}
|
||||
if let Err(err) = std::fs::remove_file(&self.path) {
|
||||
if err.kind() != std::io::ErrorKind::NotFound {
|
||||
tracing::warn!(path = ?self.path, %err, "could not remove completed update marker");
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl Drop for UpdateMarkerGuard {
|
||||
fn drop(&mut self) {
|
||||
if let Err(err) = std::fs::remove_file(&self.path) {
|
||||
if err.kind() != std::io::ErrorKind::NotFound {
|
||||
tracing::warn!(path = ?self.path, %err, "could not remove update-in-progress marker");
|
||||
}
|
||||
}
|
||||
self.complete();
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -152,7 +248,39 @@ async fn run_update(app: AppHandle) -> Result<()> {
|
|||
// it, that backend re-locks the venv shim, our `force_kill_other_hermes`
|
||||
// straggler-cleanup kills it, and the relaunch/kill cycle loops. The guard
|
||||
// removes the marker on every exit path (incl. early returns / panics).
|
||||
let _update_marker = UpdateMarkerGuard::acquire(crate::paths::update_in_progress_marker());
|
||||
//
|
||||
// The same marker is the cross-process update lock (hermes_cli/
|
||||
// update_lock.py claims it too), so a live foreign owner means another
|
||||
// updater — most often a dashboard-spawned `hermes update` — is already
|
||||
// mutating this checkout. Refuse instead of running a second one over it.
|
||||
let _update_marker = match UpdateMarkerGuard::acquire(
|
||||
crate::paths::update_in_progress_marker(),
|
||||
) {
|
||||
Ok(guard) => guard,
|
||||
Err(owner) => {
|
||||
let mins = owner.age_secs / 60;
|
||||
let secs = owner.age_secs % 60;
|
||||
let elapsed = if mins > 0 {
|
||||
format!("{mins}m {secs}s")
|
||||
} else {
|
||||
format!("{secs}s")
|
||||
};
|
||||
let msg = format!(
|
||||
"Another Hermes update is already running (PID {}, started {} ago). \
|
||||
Wait for it to finish, or close the window or dashboard tab that \
|
||||
started it, then try again.",
|
||||
owner.pid, elapsed
|
||||
);
|
||||
emit(
|
||||
&app,
|
||||
BootstrapEvent::Failed {
|
||||
stage: None,
|
||||
error: msg.clone(),
|
||||
},
|
||||
);
|
||||
return Err(anyhow!(msg));
|
||||
}
|
||||
};
|
||||
|
||||
let update_branch = update_branch_from_args(std::env::args().skip(1))
|
||||
.or_else(|| option_env_string("BUILD_PIN_BRANCH"))
|
||||
|
|
@ -453,6 +581,12 @@ async fn run_update(app: AppHandle) -> Result<()> {
|
|||
marker: None,
|
||||
},
|
||||
);
|
||||
// Every install-tree mutation is finished. Release the lock BEFORE the
|
||||
// relaunch: this process can stay wedged in its native event loop even
|
||||
// after a successful app.exit(), and a live pid on a fresh marker would
|
||||
// make a completed update look active — blocking desktop startup and
|
||||
// every other updater until the age ceiling expires.
|
||||
_update_marker.complete();
|
||||
|
||||
if let Some(target_app) = launch_target {
|
||||
if let Err(err) = launch_macos_app_and_exit(&app, &target_app).await {
|
||||
|
|
@ -477,9 +611,26 @@ async fn run_update(app: AppHandle) -> Result<()> {
|
|||
);
|
||||
}
|
||||
|
||||
// The launch helpers normally request exit themselves, but their failure
|
||||
// paths must still close a successful updater. A native event loop can
|
||||
// ignore that graceful request, so arm a process-exit fallback now that
|
||||
// all update state and the marker have been settled.
|
||||
exit_after_success(&app);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Ask the app to exit, with a hard `process::exit` fallback for a native
|
||||
/// event loop that ignores the graceful request. Without it a finished updater
|
||||
/// can linger as a live pid forever.
|
||||
fn exit_after_success(app: &AppHandle) {
|
||||
std::thread::spawn(|| {
|
||||
std::thread::sleep(std::time::Duration::from_secs(3));
|
||||
tracing::warn!("graceful updater exit timed out; forcing process exit");
|
||||
std::process::exit(0);
|
||||
});
|
||||
app.exit(0);
|
||||
}
|
||||
|
||||
/// Poll until the venv shim AND packaged desktop app bundle are no longer locked
|
||||
/// (Windows) or a bounded timeout elapses. On non-Windows this is a short fixed
|
||||
/// grace since file locking isn't the failure mode there.
|
||||
|
|
@ -1102,7 +1253,8 @@ mod tests {
|
|||
let marker = dir.join(".hermes-update-in-progress");
|
||||
|
||||
{
|
||||
let _g = UpdateMarkerGuard::acquire(marker.clone());
|
||||
let _g = UpdateMarkerGuard::acquire(marker.clone())
|
||||
.unwrap_or_else(|_| panic!("no live owner => acquire must succeed"));
|
||||
assert!(marker.exists(), "marker must exist while the guard is held");
|
||||
let body = std::fs::read_to_string(&marker).unwrap();
|
||||
let pid_line = body.lines().next().unwrap();
|
||||
|
|
@ -1127,7 +1279,8 @@ mod tests {
|
|||
std::fs::create_dir_all(&dir).unwrap();
|
||||
let marker = dir.join(".hermes-update-in-progress");
|
||||
|
||||
let guard = UpdateMarkerGuard::acquire(marker.clone());
|
||||
let guard = UpdateMarkerGuard::acquire(marker.clone())
|
||||
.unwrap_or_else(|_| panic!("no live owner => acquire must succeed"));
|
||||
// Simulate an external cleanup (e.g. the desktop pruned a marker it
|
||||
// judged stale) before our guard drops — Drop must not panic.
|
||||
std::fs::remove_file(&marker).unwrap();
|
||||
|
|
@ -1137,6 +1290,97 @@ mod tests {
|
|||
let _ = std::fs::remove_dir_all(&dir);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn acquire_refuses_while_a_live_updater_owns_the_marker() {
|
||||
let dir = unique_tmp_dir("marker-contended");
|
||||
std::fs::create_dir_all(&dir).unwrap();
|
||||
let marker = dir.join(".hermes-update-in-progress");
|
||||
|
||||
// A live updater (us) holds it. A second updater must NOT clobber the
|
||||
// marker and run concurrently over the same checkout — that race is
|
||||
// what let a dashboard `hermes update` and install-mode bootstrap
|
||||
// mutate one tree at once.
|
||||
let held = UpdateMarkerGuard::acquire(marker.clone())
|
||||
.unwrap_or_else(|_| panic!("first acquire must succeed"));
|
||||
let owner = UpdateMarkerGuard::acquire(marker.clone())
|
||||
.err()
|
||||
.expect("second acquire must be refused while the first is live");
|
||||
assert_eq!(owner.pid, std::process::id());
|
||||
|
||||
// The refused guard must not delete the live owner's marker.
|
||||
assert!(marker.exists(), "refused acquire must leave the marker intact");
|
||||
drop(held);
|
||||
assert!(!marker.exists(), "the real owner still cleans up on drop");
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn acquire_reclaims_a_marker_owned_by_a_dead_pid() {
|
||||
let dir = unique_tmp_dir("marker-dead-pid");
|
||||
std::fs::create_dir_all(&dir).unwrap();
|
||||
let marker = dir.join(".hermes-update-in-progress");
|
||||
|
||||
// pid 1 exists everywhere, so fabricate a dead one: a very large pid
|
||||
// that no live process owns. A crashed updater must never wedge every
|
||||
// future update.
|
||||
let started_at = std::time::SystemTime::now()
|
||||
.duration_since(std::time::UNIX_EPOCH)
|
||||
.map(|d| d.as_secs())
|
||||
.unwrap_or(0);
|
||||
std::fs::write(&marker, format!("4294967294\n{started_at}")).unwrap();
|
||||
|
||||
let guard = UpdateMarkerGuard::acquire(marker.clone())
|
||||
.unwrap_or_else(|_| panic!("a dead owner must not block acquisition"));
|
||||
let body = std::fs::read_to_string(&marker).unwrap();
|
||||
assert_eq!(
|
||||
body.lines().next().unwrap().trim().parse::<u32>().unwrap(),
|
||||
std::process::id(),
|
||||
"reclaiming rewrites the marker with our pid"
|
||||
);
|
||||
drop(guard);
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn acquire_reclaims_a_marker_past_the_age_ceiling() {
|
||||
let dir = unique_tmp_dir("marker-stale-age");
|
||||
std::fs::create_dir_all(&dir).unwrap();
|
||||
let marker = dir.join(".hermes-update-in-progress");
|
||||
|
||||
// Our own (live) pid, but started well past the ceiling: a wedged
|
||||
// updater must not hold the lock forever.
|
||||
let long_ago = std::time::SystemTime::now()
|
||||
.duration_since(std::time::UNIX_EPOCH)
|
||||
.map(|d| d.as_secs())
|
||||
.unwrap_or(0)
|
||||
.saturating_sub(UPDATE_MARKER_MAX_AGE_SECS + 60);
|
||||
std::fs::write(&marker, format!("{}\n{long_ago}", std::process::id())).unwrap();
|
||||
|
||||
let guard = UpdateMarkerGuard::acquire(marker.clone())
|
||||
.unwrap_or_else(|_| panic!("a marker past the ceiling must be reclaimable"));
|
||||
drop(guard);
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn completed_update_releases_marker_before_guard_drop() {
|
||||
let dir = unique_tmp_dir("marker-complete");
|
||||
std::fs::create_dir_all(&dir).unwrap();
|
||||
let marker = dir.join(".hermes-update-in-progress");
|
||||
|
||||
let guard = UpdateMarkerGuard::acquire(marker.clone())
|
||||
.unwrap_or_else(|_| panic!("no live owner => acquire must succeed"));
|
||||
guard.complete();
|
||||
|
||||
assert!(
|
||||
!marker.exists(),
|
||||
"a successful update must unblock desktop startup before relaunch/exit"
|
||||
);
|
||||
drop(guard);
|
||||
assert!(!marker.exists(), "Drop stays idempotent after completion");
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parses_update_branch_from_space_or_equals_args() {
|
||||
assert_eq!(
|
||||
|
|
|
|||
|
|
@ -8983,9 +8983,28 @@ def cmd_update(args):
|
|||
# writes to a closed stdout. No-op in gateway mode. See
|
||||
# _install_hangup_protection for rationale.
|
||||
_update_io_state = _install_hangup_protection(gateway_mode=gateway_mode)
|
||||
# Cross-process mutual exclusion. The dashboard's Update button spawns
|
||||
# this same command detached, and the desktop hands off to the Tauri
|
||||
# updater / install-mode bootstrap — all three mutate one checkout. Two of
|
||||
# them running together rewrite source under a live interpreter and strand
|
||||
# the tree half-updated. Share the marker the Tauri updater and Electron
|
||||
# already use rather than inventing a second lock.
|
||||
from hermes_cli.update_lock import (
|
||||
UPDATE_EXIT_CONCURRENT,
|
||||
UpdateLock,
|
||||
describe_holder,
|
||||
)
|
||||
|
||||
_update_lock = UpdateLock()
|
||||
if not _update_lock.acquire():
|
||||
print(describe_holder(_update_lock.holder))
|
||||
_finalize_update_output(_update_io_state)
|
||||
sys.exit(UPDATE_EXIT_CONCURRENT)
|
||||
|
||||
try:
|
||||
_cmd_update_impl(args, gateway_mode=gateway_mode)
|
||||
finally:
|
||||
_update_lock.release()
|
||||
_finalize_update_output(_update_io_state)
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -0,0 +1,214 @@
|
|||
"""Cross-process mutual exclusion for in-flight Hermes updates.
|
||||
|
||||
Three different surfaces can start an update of the same install tree:
|
||||
|
||||
* ``hermes update`` from a terminal,
|
||||
* the dashboard's Update button (``POST /api/hermes/update`` →
|
||||
``_spawn_hermes_action(["update"])``, detached),
|
||||
* the desktop's Update button, which hands off to the Tauri
|
||||
``hermes-setup --update`` and, on its failure screen, to install-mode
|
||||
bootstrap (``install.ps1`` / ``install.sh``).
|
||||
|
||||
Until now only the Tauri updater published an "update in progress" marker
|
||||
(``UpdateMarkerGuard`` in ``apps/bootstrap-installer/src-tauri/src/update.rs``),
|
||||
and only the Electron desktop consumed it (``electron/update-marker.ts``, to
|
||||
gate local backend startup). Nothing stopped two *updaters* from running at
|
||||
once — so a dashboard-spawned ``hermes update`` and an installer-driven
|
||||
``git checkout`` could mutate the same checkout concurrently, rewriting source
|
||||
under a live interpreter and leaving the tree half-updated.
|
||||
|
||||
This module makes that same marker the single lock for **all** update
|
||||
entrypoints instead of adding a fourth mechanism. Format and location are
|
||||
unchanged and remain byte-compatible with the Rust and Electron readers:
|
||||
|
||||
<HERMES_HOME>/.hermes-update-in-progress body: "<pid>\\n<started_at_unix>"
|
||||
|
||||
A marker only counts as a live update when its pid is alive AND it is younger
|
||||
than :data:`UPDATE_MARKER_MAX_AGE_MS` — mirroring ``readLiveUpdateMarker`` so a
|
||||
crashed updater self-heals instead of wedging every future update. A stale
|
||||
marker is removed on read by whoever notices it first.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import logging
|
||||
import os
|
||||
import time
|
||||
from dataclasses import dataclass
|
||||
from pathlib import Path
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
# Keep in sync with UPDATE_MARKER_MAX_AGE_MS in
|
||||
# apps/desktop/electron/update-marker.ts — the same marker is read by both, and
|
||||
# a shorter ceiling here would let Python steal a lock Electron still considers
|
||||
# live. A full update (git pull + uv sync + desktop rebuild) is minutes.
|
||||
UPDATE_MARKER_MAX_AGE_SECONDS = 20 * 60
|
||||
|
||||
MARKER_NAME = ".hermes-update-in-progress"
|
||||
|
||||
# Exit code meaning "another updater/instance owns this install right now".
|
||||
# Already the de-facto contract: the Windows shim + venv-holder guards in
|
||||
# _cmd_update_impl exit 2, and the Tauri updater matches on it
|
||||
# (UPDATE_EXIT_CONCURRENT in apps/bootstrap-installer/src-tauri/src/update.rs)
|
||||
# to show "Hermes is still running" instead of a generic failure. Naming it
|
||||
# here keeps the concurrent-update refusal on that same understood contract.
|
||||
UPDATE_EXIT_CONCURRENT = 2
|
||||
|
||||
|
||||
def update_marker_path() -> Path:
|
||||
"""Path of the shared update marker.
|
||||
|
||||
Uses the *process* Hermes home (never the context-local profile override):
|
||||
the Rust updater resolves ``$HERMES_HOME`` or the platform default, and the
|
||||
desktop pins that same value into the updater's env. A profile-scoped path
|
||||
here would put the lock somewhere the other two owners never look.
|
||||
"""
|
||||
from hermes_constants import get_process_hermes_home
|
||||
|
||||
return get_process_hermes_home() / MARKER_NAME
|
||||
|
||||
|
||||
def _pid_alive(pid: int) -> bool:
|
||||
"""True when a process with ``pid`` currently exists.
|
||||
|
||||
Delegates to :func:`gateway.status._pid_exists`, the project's existing
|
||||
no-kill probe. Do NOT hand-roll this with ``os.kill(pid, 0)``: on Windows
|
||||
that is not a no-op — CPython routes ``sig=0`` to
|
||||
``GenerateConsoleCtrlEvent``, which Ctrl+C's the target's whole console
|
||||
process group (bpo-14484). A liveness check that killed the updater it was
|
||||
asking about would be a spectacular way to fix a concurrency bug.
|
||||
|
||||
Any pid we cannot evaluate counts as dead: a corrupt marker must not wedge
|
||||
the lock forever.
|
||||
"""
|
||||
if pid <= 0:
|
||||
return False
|
||||
try:
|
||||
from gateway.status import _pid_exists
|
||||
|
||||
return bool(_pid_exists(pid))
|
||||
except Exception as exc:
|
||||
# Import failure or an unusable pid (e.g. larger than the platform's
|
||||
# pid_t). Treat the marker as stale rather than blocking updates.
|
||||
logger.debug("Could not probe pid %s: %s", pid, exc)
|
||||
return False
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class UpdateHolder:
|
||||
"""A confirmed-live update currently holding the lock."""
|
||||
|
||||
pid: int
|
||||
age_seconds: float
|
||||
|
||||
|
||||
def read_live_update(*, path: Path | None = None) -> UpdateHolder | None:
|
||||
"""Return the live update holding the lock, or ``None``.
|
||||
|
||||
Mirrors ``readLiveUpdateMarker`` in ``electron/update-marker.ts``: absent,
|
||||
unreadable, malformed, dead-pid, and past-the-ceiling all mean "no live
|
||||
update", and a stale marker file is deleted so it can't strand future runs.
|
||||
Never raises.
|
||||
"""
|
||||
marker = path or update_marker_path()
|
||||
try:
|
||||
raw = marker.read_text(encoding="utf-8")
|
||||
except OSError:
|
||||
return None # absent or unreadable => no live update
|
||||
|
||||
lines = raw.splitlines()
|
||||
try:
|
||||
pid = int(lines[0].strip())
|
||||
except (IndexError, ValueError):
|
||||
pid = -1
|
||||
try:
|
||||
started_at = float(lines[1].strip())
|
||||
except (IndexError, ValueError):
|
||||
started_at = float("-inf")
|
||||
|
||||
age = time.time() - started_at
|
||||
if not _pid_alive(pid) or age > UPDATE_MARKER_MAX_AGE_SECONDS:
|
||||
try:
|
||||
marker.unlink()
|
||||
except OSError:
|
||||
pass
|
||||
return None
|
||||
|
||||
return UpdateHolder(pid=pid, age_seconds=age)
|
||||
|
||||
|
||||
def describe_holder(holder: UpdateHolder) -> str:
|
||||
"""One-line, user-facing explanation of who holds the update lock."""
|
||||
minutes, seconds = divmod(int(max(holder.age_seconds, 0)), 60)
|
||||
elapsed = f"{minutes}m {seconds}s" if minutes else f"{seconds}s"
|
||||
return (
|
||||
f"✗ Another Hermes update is already running (PID {holder.pid}, "
|
||||
f"started {elapsed} ago).\n"
|
||||
"\n"
|
||||
" Two updates mutating the same checkout corrupt it: one rewrites\n"
|
||||
" source while the other is mid-install. Wait for it to finish, or\n"
|
||||
" close the window/dashboard tab that started it, then retry."
|
||||
)
|
||||
|
||||
|
||||
class UpdateLock:
|
||||
"""Context manager owning the shared update marker for this process.
|
||||
|
||||
``acquired`` is False when another live update already holds it — callers
|
||||
decide whether that's a hard refusal (CLI/dashboard) or a wait. Releasing
|
||||
only removes the marker when *we* still own it, so a marker rewritten by a
|
||||
handoff partner (the Tauri updater overwrites it with its own pid) is never
|
||||
deleted out from under its new owner.
|
||||
"""
|
||||
|
||||
def __init__(self, *, path: Path | None = None) -> None:
|
||||
self.path = path or update_marker_path()
|
||||
self.acquired = False
|
||||
self.holder: UpdateHolder | None = None
|
||||
|
||||
def acquire(self) -> bool:
|
||||
"""Claim the lock. Returns False (and sets ``holder``) if it's taken."""
|
||||
existing = read_live_update(path=self.path)
|
||||
if existing is not None:
|
||||
self.holder = existing
|
||||
return False
|
||||
try:
|
||||
self.path.parent.mkdir(parents=True, exist_ok=True)
|
||||
self.path.write_text(
|
||||
f"{os.getpid()}\n{int(time.time())}\n", encoding="utf-8"
|
||||
)
|
||||
except OSError as exc:
|
||||
# Best-effort, exactly like the Rust guard: an unwritable marker
|
||||
# must not block the update itself (that would be a worse failure
|
||||
# than the race it prevents). Degrade to the pre-lock behavior.
|
||||
logger.debug("Could not write update marker %s: %s", self.path, exc)
|
||||
return True
|
||||
self.acquired = True
|
||||
return True
|
||||
|
||||
def release(self) -> None:
|
||||
"""Drop the marker if this process still owns it. Never raises."""
|
||||
if not self.acquired:
|
||||
return
|
||||
self.acquired = False
|
||||
try:
|
||||
raw = self.path.read_text(encoding="utf-8")
|
||||
owner = int(raw.splitlines()[0].strip())
|
||||
except (OSError, IndexError, ValueError):
|
||||
return
|
||||
if owner != os.getpid():
|
||||
# A handoff partner took ownership (e.g. the Tauri updater wrote
|
||||
# its own pid). Leave it alone — it's still a live update.
|
||||
return
|
||||
try:
|
||||
self.path.unlink()
|
||||
except OSError:
|
||||
pass
|
||||
|
||||
def __enter__(self) -> "UpdateLock":
|
||||
self.acquire()
|
||||
return self
|
||||
|
||||
def __exit__(self, *_exc) -> None:
|
||||
self.release()
|
||||
|
|
@ -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"
|
||||
|
|
|
|||
|
|
@ -0,0 +1,177 @@
|
|||
"""Cross-process update mutual exclusion (``hermes_cli.update_lock``).
|
||||
|
||||
Three surfaces can start an update of one install tree: a terminal ``hermes
|
||||
update``, the dashboard's Update button (which spawns that same command
|
||||
detached), and the desktop's Update button (Tauri updater → install-mode
|
||||
bootstrap on its failure screen). Before the shared lock, two of them could run
|
||||
concurrently and rewrite source under a live interpreter — observed in the wild
|
||||
as an installer ``git checkout`` rewinding the checkout ~9k commits while a
|
||||
dashboard-spawned ``hermes update`` was mid-``npm install``, which then failed
|
||||
against the rewound tree's manifests.
|
||||
|
||||
These exercise the real marker file against a temp home — no mocks — because
|
||||
the contract that matters is what the Rust updater and the Electron gate see on
|
||||
disk.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import time
|
||||
|
||||
import pytest
|
||||
|
||||
from hermes_cli.update_lock import (
|
||||
UPDATE_MARKER_MAX_AGE_SECONDS,
|
||||
UpdateLock,
|
||||
describe_holder,
|
||||
read_live_update,
|
||||
update_marker_path,
|
||||
)
|
||||
|
||||
# A pid no live process owns. os.kill(pid, 0) must report it dead so a crashed
|
||||
# updater can never wedge every future update. Deliberately larger than any
|
||||
# platform's pid_t so it also covers the corrupt-marker path (OverflowError).
|
||||
DEAD_PID = 4294967294
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def marker(tmp_path):
|
||||
return tmp_path / ".hermes-update-in-progress"
|
||||
|
||||
|
||||
def test_marker_path_follows_process_hermes_home(tmp_path, monkeypatch):
|
||||
"""The lock must land where the Rust updater and Electron gate look.
|
||||
|
||||
All three resolve the *process* HERMES_HOME; a profile-scoped path would
|
||||
put the lock somewhere the other two owners never read.
|
||||
"""
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
|
||||
assert update_marker_path() == tmp_path / ".hermes-update-in-progress"
|
||||
|
||||
|
||||
def test_acquire_writes_pid_and_start_time(marker):
|
||||
lock = UpdateLock(path=marker)
|
||||
|
||||
assert lock.acquire() is True
|
||||
assert lock.acquired is True
|
||||
|
||||
lines = marker.read_text(encoding="utf-8").splitlines()
|
||||
assert int(lines[0]) == os.getpid(), "the Electron gate probes this pid for liveness"
|
||||
assert int(lines[1]) == pytest.approx(time.time(), abs=5)
|
||||
assert len(lines) == 2, "wire format is exactly pid + started_at"
|
||||
|
||||
|
||||
def test_second_acquire_is_refused_while_the_first_is_live(marker):
|
||||
"""The bug: two updaters mutating one checkout at the same time."""
|
||||
first = UpdateLock(path=marker)
|
||||
assert first.acquire() is True
|
||||
|
||||
second = UpdateLock(path=marker)
|
||||
assert second.acquire() is False
|
||||
assert second.holder is not None
|
||||
assert second.holder.pid == os.getpid()
|
||||
assert second.acquired is False
|
||||
|
||||
|
||||
def test_refused_lock_does_not_delete_the_live_owners_marker(marker):
|
||||
first = UpdateLock(path=marker)
|
||||
first.acquire()
|
||||
|
||||
second = UpdateLock(path=marker)
|
||||
second.acquire()
|
||||
second.release()
|
||||
|
||||
assert marker.exists(), "a refused claimant must never clear the live owner's lock"
|
||||
|
||||
first.release()
|
||||
assert not marker.exists()
|
||||
|
||||
|
||||
def test_release_leaves_a_marker_a_handoff_partner_now_owns(marker):
|
||||
"""The desktop writes the marker, then the Tauri updater takes ownership.
|
||||
|
||||
Releasing must not delete a marker whose pid is no longer ours — that would
|
||||
reopen the gate while the partner is still mid-update.
|
||||
"""
|
||||
lock = UpdateLock(path=marker)
|
||||
lock.acquire()
|
||||
|
||||
marker.write_text(f"{DEAD_PID}\n{int(time.time())}\n", encoding="utf-8")
|
||||
lock.release()
|
||||
|
||||
assert marker.exists(), "the partner's marker is not ours to remove"
|
||||
|
||||
|
||||
def test_dead_owner_is_reclaimed_not_honored(marker):
|
||||
marker.write_text(f"{DEAD_PID}\n{int(time.time())}\n", encoding="utf-8")
|
||||
|
||||
lock = UpdateLock(path=marker)
|
||||
assert lock.acquire() is True
|
||||
assert int(marker.read_text(encoding="utf-8").splitlines()[0]) == os.getpid()
|
||||
|
||||
|
||||
def test_owner_past_the_age_ceiling_is_reclaimed(marker):
|
||||
"""A live-but-wedged updater must not hold the lock forever."""
|
||||
long_ago = int(time.time()) - UPDATE_MARKER_MAX_AGE_SECONDS - 60
|
||||
marker.write_text(f"{os.getpid()}\n{long_ago}\n", encoding="utf-8")
|
||||
|
||||
lock = UpdateLock(path=marker)
|
||||
assert lock.acquire() is True
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"body",
|
||||
["", "not-a-pid\n123\n", "\n\n", "12345"],
|
||||
ids=["empty", "garbage-pid", "blank-lines", "no-start-time"],
|
||||
)
|
||||
def test_malformed_markers_never_block_an_update(marker, body):
|
||||
marker.write_text(body, encoding="utf-8")
|
||||
|
||||
assert read_live_update(path=marker) is None
|
||||
assert UpdateLock(path=marker).acquire() is True
|
||||
|
||||
|
||||
def test_stale_marker_is_removed_on_read(marker):
|
||||
marker.write_text(f"{DEAD_PID}\n{int(time.time())}\n", encoding="utf-8")
|
||||
|
||||
assert read_live_update(path=marker) is None
|
||||
assert not marker.exists(), "whoever notices a stale marker clears it"
|
||||
|
||||
|
||||
def test_absent_marker_reports_no_live_update(marker):
|
||||
assert read_live_update(path=marker) is None
|
||||
|
||||
|
||||
def test_context_manager_releases_even_on_exception(marker):
|
||||
with pytest.raises(RuntimeError):
|
||||
with UpdateLock(path=marker) as lock:
|
||||
assert lock.acquired is True
|
||||
raise RuntimeError("update blew up mid-flight")
|
||||
|
||||
assert not marker.exists(), "a crashed update must not strand the lock"
|
||||
|
||||
|
||||
def test_describe_holder_names_the_pid_and_elapsed_time(marker):
|
||||
lock = UpdateLock(path=marker)
|
||||
lock.acquire()
|
||||
|
||||
holder = read_live_update(path=marker)
|
||||
assert holder is not None
|
||||
message = describe_holder(holder)
|
||||
|
||||
assert str(os.getpid()) in message, "the user needs the pid to find the other update"
|
||||
assert "already running" in message
|
||||
|
||||
|
||||
def test_unwritable_marker_location_does_not_block_the_update(tmp_path):
|
||||
"""Degrade to pre-lock behavior rather than refusing to update at all.
|
||||
|
||||
An unwritable marker path is a worse reason to block an update than the
|
||||
race the lock prevents.
|
||||
"""
|
||||
lock = UpdateLock(path=tmp_path / "nonexistent-file" / "marker")
|
||||
(tmp_path / "nonexistent-file").write_text("i am a file, not a dir", encoding="utf-8")
|
||||
|
||||
assert lock.acquire() is True
|
||||
assert lock.acquired is False, "nothing was written, so there is nothing to release"
|
||||
|
|
@ -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…
Reference in New Issue