mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-30 19:09:28 +00:00
fix(cli): sweep aged venv.stale.runtime-* backups on hermes update
Follow-up to the salvaged success-path removal: installs that already repaired (or predate the cleanup) still carry leaked ~1 GB parked venvs. When the runtime probes safe, reclaim aged (>1h) stale markers next to the live venv — age-gated to avoid racing an in-flight sibling repair, boundary-checked via _remove_tree so symlinked names can't escape the checkout. Also drop the now-stale 'before removing the parked venv' user guidance in update_cmd. Tests: success-path removal, safe-path sweep (aged removed, fresh kept).
This commit is contained in:
parent
819357d741
commit
adf217b584
3 changed files with 123 additions and 8 deletions
|
|
@ -1012,6 +1012,45 @@ def _default_live_venv(root: Path) -> Path:
|
|||
return primary
|
||||
|
||||
|
||||
def _sweep_stale_runtime_backups(
|
||||
live: Path,
|
||||
*,
|
||||
root: Path,
|
||||
keep: Path | None = None,
|
||||
min_age_seconds: float = 3600.0,
|
||||
) -> None:
|
||||
"""Remove leftover ``venv.stale.runtime-*`` backups next to *live*.
|
||||
|
||||
A successful runtime repair parks the previous venv as
|
||||
``<live>.stale.runtime-<token>``; historically nothing ever reclaimed
|
||||
those, so each repair leaked a full venv (~1 GB) at the project root
|
||||
forever (issue #73109). On POSIX, deleting the tree is safe even while
|
||||
an older process still maps files from it — open FDs and mmaps keep
|
||||
their inodes alive; the directory entry is what goes away.
|
||||
|
||||
``min_age_seconds`` guards against racing a concurrent repair in
|
||||
another process: a backup parked seconds ago may still be that
|
||||
repair's rollback path, so only clearly-old markers are swept.
|
||||
``keep`` exempts the backup the current repair just created.
|
||||
Best-effort: never raises.
|
||||
"""
|
||||
try:
|
||||
candidates = list(live.parent.glob(f"{live.name}.stale.runtime-*"))
|
||||
except OSError:
|
||||
return
|
||||
now = time.time()
|
||||
for candidate in candidates:
|
||||
if keep is not None and candidate == keep:
|
||||
continue
|
||||
try:
|
||||
age = now - candidate.stat().st_mtime
|
||||
except OSError:
|
||||
continue
|
||||
if age < min_age_seconds:
|
||||
continue
|
||||
_remove_tree(candidate, boundary=root)
|
||||
|
||||
|
||||
def repair_vulnerable_runtime(
|
||||
uv_bin: str,
|
||||
*,
|
||||
|
|
@ -1036,6 +1075,13 @@ def repair_vulnerable_runtime(
|
|||
f"could not probe live interpreter {live_python}",
|
||||
)
|
||||
if not current.wal_reset_vulnerable:
|
||||
# The runtime is already fixed — any venv.stale.runtime-* markers
|
||||
# next to the live venv are leftovers from a past repair (or from
|
||||
# a build predating the post-repair cleanup) and will never be
|
||||
# rolled back to. Sweep them so they don't leak ~1 GB each
|
||||
# forever (issue #73109). Age-gated to avoid racing an in-flight
|
||||
# repair in a sibling process.
|
||||
_sweep_stale_runtime_backups(live, root=root)
|
||||
return RuntimeRepairResult(
|
||||
"safe",
|
||||
sqlite_before=current.sqlite_version_string,
|
||||
|
|
|
|||
|
|
@ -3398,14 +3398,7 @@ def _cmd_update_impl(args, gateway_mode: bool):
|
|||
" Any running Hermes gateways, Desktop backends, or other "
|
||||
"long-lived processes still use the previous runtime."
|
||||
)
|
||||
print(
|
||||
" Restart each of them before removing the parked venv"
|
||||
+ (
|
||||
f": {runtime_repaired.backup_venv}"
|
||||
if runtime_repaired.backup_venv is not None
|
||||
else "."
|
||||
)
|
||||
)
|
||||
print(" Restart each of them to pick up the repaired runtime.")
|
||||
_m()._resume_windows_gateways_after_update(_windows_gateway_resume)
|
||||
return
|
||||
|
||||
|
|
|
|||
|
|
@ -430,6 +430,82 @@ class TestRuntimeRepair:
|
|||
assert reacquired is not None
|
||||
_release_repair_lock(reacquired)
|
||||
|
||||
def test_safe_runtime_sweeps_old_stale_backups(self, tmp_path):
|
||||
"""A fixed runtime reclaims aged venv.stale.runtime-* leftovers
|
||||
(issue #73109) but leaves fresh ones (possible in-flight repair)."""
|
||||
import os
|
||||
import time as _time
|
||||
|
||||
from hermes_cli.managed_uv import repair_vulnerable_runtime
|
||||
|
||||
root, live, sentinel = _make_runtime_install(tmp_path)
|
||||
old_backup = root / f"{live.name}.stale.runtime-1-2-aaaa"
|
||||
(old_backup / "bin").mkdir(parents=True)
|
||||
(old_backup / "bin" / "python").write_text("old", encoding="utf-8")
|
||||
stale_mtime = _time.time() - 7200
|
||||
os.utime(old_backup, (stale_mtime, stale_mtime))
|
||||
|
||||
fresh_backup = root / f"{live.name}.stale.runtime-9-9-bbbb"
|
||||
(fresh_backup / "bin").mkdir(parents=True)
|
||||
|
||||
current = _runtime_info(live / "bin" / "python", (3, 53, 1))
|
||||
with patch("hermes_cli.managed_uv.platform.system", return_value="Linux"), \
|
||||
patch(
|
||||
"hermes_cli.managed_uv.probe_sqlite_runtime",
|
||||
return_value=current,
|
||||
):
|
||||
result = repair_vulnerable_runtime("uv", project_root=root)
|
||||
|
||||
assert result.status == "safe"
|
||||
assert not old_backup.exists(), "aged stale backup must be reclaimed"
|
||||
assert fresh_backup.exists(), "fresh backup may be an in-flight repair"
|
||||
assert sentinel.read_text(encoding="utf-8") == "live"
|
||||
|
||||
def test_successful_repair_removes_parked_backup(self, tmp_path):
|
||||
"""After a successful cutover the parked venv is removed instead of
|
||||
leaking ~1 GB at the project root forever (issue #73109)."""
|
||||
from hermes_cli.managed_uv import repair_vulnerable_runtime
|
||||
|
||||
root, live, sentinel = _make_runtime_install(tmp_path)
|
||||
current = _runtime_info(live / "bin" / "python", (3, 50, 4))
|
||||
generation = root / ".hermes-runtime" / "python" / "generation-test"
|
||||
candidate_python = generation / "bin" / "python"
|
||||
candidate_python.parent.mkdir(parents=True)
|
||||
candidate_python.write_text("candidate interpreter", encoding="utf-8")
|
||||
fixed = _runtime_info(candidate_python, (3, 53, 1))
|
||||
candidate_venv = root / ".hermes-runtime" / "venv-candidate"
|
||||
(candidate_venv / "bin").mkdir(parents=True)
|
||||
(candidate_venv / "bin" / "python").write_text(
|
||||
"candidate venv interpreter", encoding="utf-8"
|
||||
)
|
||||
|
||||
with patch("hermes_cli.managed_uv.platform.system", return_value="Linux"), \
|
||||
patch(
|
||||
"hermes_cli.managed_uv.probe_sqlite_runtime",
|
||||
side_effect=[current, current],
|
||||
), \
|
||||
patch(
|
||||
"hermes_cli.managed_uv._install_safe_python_generation",
|
||||
return_value=(generation, candidate_python, fixed),
|
||||
), \
|
||||
patch(
|
||||
"hermes_cli.managed_uv._stage_candidate_venv",
|
||||
return_value=candidate_venv,
|
||||
), \
|
||||
patch(
|
||||
"hermes_cli.managed_uv._smoke_candidate_venv",
|
||||
return_value=(True, "", fixed),
|
||||
):
|
||||
result = repair_vulnerable_runtime("uv", project_root=root)
|
||||
|
||||
assert result.status == "repaired"
|
||||
assert result.backup_venv is not None
|
||||
assert not result.backup_venv.exists(), (
|
||||
"parked venv must be removed after a successful repair"
|
||||
)
|
||||
leftovers = list(root.glob(f"{live.name}.stale.runtime-*"))
|
||||
assert leftovers == [], f"no stale markers may remain: {leftovers}"
|
||||
|
||||
|
||||
class TestRuntimeCutover:
|
||||
def test_os_lock_blocks_concurrent_repair_and_releases(self, tmp_path):
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue