From c055eee1fee03a510ed797c35bfbf20c074d992a Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Wed, 16 Sep 2026 09:57:11 -0700 Subject: [PATCH] refactor(checkpoints): slim the profile-rename rekey and name it as a checkpoint_manager sibling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the cherry-picked fix from #112724 (@poijygfdyy): - tools/checkpoint_profile_migration.py -> tools/checkpoint_manager_profile_rename.py, the repo's `_.py` sibling convention for code that extends checkpoint_manager. - Replace the fail-closed target-collision check plus temp-file/rollback choreography with an idempotent rekey: every step overwrites and the old project metadata is removed last, so a mid-way failure is repaired by `hermes profile migrate-identity` redoing the same writes. A genuine collision cannot occur — `profiles/` must not exist for the rename to run. 192 -> 98 lines. - Metadata/ledger writes go through the same idiom as checkpoint_manager itself (`_register_project` plain write, `_save_ledger`), dropping the private temp-file helpers. - Keep the git-present precondition as a single early check: without git the ref cannot move and rekeying only the metadata would orphan the history. - Test: `create_profile` now seeds `workspace/`, so the fixture uses `project/`; add the control assertion that a workdir outside the profile dir keeps its history unchanged. - Docs: the profile rename / migrate-identity reference notes that checkpoint history is preserved. Fixes #112973 --- hermes_cli/profile_identity.py | 2 +- .../test_profile_rename_checkpoints.py | 14 +- tools/checkpoint_manager_profile_rename.py | 97 +++++++++ tools/checkpoint_profile_migration.py | 192 ------------------ website/docs/reference/profile-commands.md | 5 +- 5 files changed, 112 insertions(+), 198 deletions(-) create mode 100644 tools/checkpoint_manager_profile_rename.py delete mode 100644 tools/checkpoint_profile_migration.py diff --git a/hermes_cli/profile_identity.py b/hermes_cli/profile_identity.py index c53db1b228..4844acd22f 100644 --- a/hermes_cli/profile_identity.py +++ b/hermes_cli/profile_identity.py @@ -164,7 +164,7 @@ def _gateway_accepts_profile_identity_verb(root: Path) -> bool: def _migrate_checkpoint_identity(old_canon: str, new_canon: str) -> bool: """Rekey checkpoint projects whose absolute workdirs moved with the profile directory.""" from hermes_cli.profiles import get_profile_dir - from tools.checkpoint_profile_migration import migrate_profile_checkpoint_projects + from tools.checkpoint_manager_profile_rename import migrate_profile_checkpoint_projects old_dir = get_profile_dir(old_canon) new_dir = get_profile_dir(new_canon) diff --git a/tests/hermes_cli/test_profile_rename_checkpoints.py b/tests/hermes_cli/test_profile_rename_checkpoints.py index a887746fb5..888663bab6 100644 --- a/tests/hermes_cli/test_profile_rename_checkpoints.py +++ b/tests/hermes_cli/test_profile_rename_checkpoints.py @@ -1,4 +1,4 @@ -"""Profile rename must preserve profile-local checkpoint history.""" +"""Profile rename must preserve profile-local checkpoint history (#112973).""" from pathlib import Path from unittest.mock import patch @@ -20,7 +20,7 @@ def profile_env(tmp_path, monkeypatch): return default_home -def test_rename_preserves_profile_local_checkpoint_history(profile_env): +def test_rename_preserves_profile_local_checkpoint_history(profile_env, tmp_path): """A moved profile keeps rollback history under the moved workspace path. Checkpoint refs, project metadata, and the safe-restore ledger are keyed by the @@ -28,8 +28,11 @@ def test_rename_preserves_profile_local_checkpoint_history(profile_env): checkpoint store itself, but it also changes every profile-local workdir key. """ old_dir = create_profile("oldname", no_alias=True) - workdir = old_dir / "workspace" + workdir = old_dir / "project" workdir.mkdir() + outside = tmp_path / "outside-project" # control: not under the profile dir, same store + outside.mkdir() + (outside / "keep.txt").write_text("v1\n", encoding="utf-8") (workdir / "pyproject.toml").write_text("[project]\nname = 'rename-checkpoint'\n", encoding="utf-8") tracked = workdir / "note.txt" tracked.write_text("before\n", encoding="utf-8") @@ -39,6 +42,8 @@ def test_rename_preserves_profile_local_checkpoint_history(profile_env): manager = CheckpointManager(enabled=True, max_snapshots=5) assert manager.ensure_checkpoint(str(workdir), "before profile rename") is True checkpoint_hash = manager.list_checkpoints(str(workdir))[0]["hash"] + assert manager.ensure_checkpoint(str(outside), "outside profile") is True + outside_hash = manager.list_checkpoints(str(outside))[0]["hash"] tracked.write_text("after\n", encoding="utf-8") manager.record_agent_write(str(tracked)) finally: @@ -48,13 +53,14 @@ def test_rename_preserves_profile_local_checkpoint_history(profile_env): patch("hermes_cli.profiles._live_default_multiplexer", return_value=False): new_dir = rename_profile("oldname", "newname") - new_workdir = new_dir / "workspace" + new_workdir = new_dir / "project" new_tracked = new_workdir / "note.txt" token = set_hermes_home_override(new_dir) try: manager = CheckpointManager(enabled=True, max_snapshots=5) checkpoints = manager.list_checkpoints(str(new_workdir)) assert [entry["hash"] for entry in checkpoints] == [checkpoint_hash] + assert [entry["hash"] for entry in manager.list_checkpoints(str(outside))] == [outside_hash] project_paths = {entry["workdir"] for entry in manager.list_all_checkpoints()} assert str(new_workdir.resolve()) in project_paths diff --git a/tools/checkpoint_manager_profile_rename.py b/tools/checkpoint_manager_profile_rename.py new file mode 100644 index 0000000000..57c2ffbaa9 --- /dev/null +++ b/tools/checkpoint_manager_profile_rename.py @@ -0,0 +1,97 @@ +"""Rekey profile-local checkpoint projects after a named profile directory moves (#112973). + +The checkpoint store lives under ``HERMES_HOME/checkpoints`` and therefore moves with a renamed +profile, but project identity inside it (ref, ``projects/.json``, agent-write ledger) is a +hash of the absolute workdir. Every workdir beneath the profile home gets a new hash after the +rename, so its history stays on disk yet unreachable from the new path until it is rekeyed here. +""" + +from __future__ import annotations + +import json +import logging +import shutil +from pathlib import Path +from typing import Dict + +from tools import checkpoint_manager as cm + +logger = logging.getLogger(__name__) + + +def _rebase_ledger_paths(ledger: Dict, old_workdir: Path, new_workdir: Path) -> Dict: + """Move absolute ledger keys under ``old_workdir`` to the corresponding new path.""" + rebased = {} + for raw_path, entry in ledger.items(): + try: + relative = Path(raw_path).relative_to(old_workdir) + except (TypeError, ValueError): + rebased[raw_path] = entry + else: + rebased[str(new_workdir / relative)] = entry + return rebased + + +def _rekey_project(store: Path, meta: Dict, old_workdir: Path, new_workdir: Path) -> None: + """Install the project under its new hash, then drop the old identity. + + Every step overwrites, so a retry (``hermes profile migrate-identity``) after a mid-way failure + simply redoes the same writes; the old metadata goes last because it is what keeps the project + visible to that retry. The per-project git index is a rebuildable cache and is discarded. + """ + old_hash, new_hash = meta["_hash"], cm._project_hash(str(new_workdir)) + new_meta = {k: v for k, v in meta.items() if k != "_hash"} + new_meta.update({"workdir": str(new_workdir), **cm._volume_evidence(new_workdir)}) + meta_path = cm._project_meta_path(store, new_hash) + meta_path.parent.mkdir(parents=True, exist_ok=True) + meta_path.write_text(json.dumps(new_meta), encoding="utf-8") + old_ledger_path = cm._ledger_path(store, old_hash) + if old_ledger_path.exists(): + cm._save_ledger(store, new_hash, _rebase_ledger_paths(cm._load_ledger(store, old_hash), old_workdir, new_workdir)) + old_ref, new_ref = cm._ref_name(old_hash), cm._ref_name(new_hash) + old_tip = cm._ref_tip(store, str(new_workdir), old_ref) + if old_tip: + ok, _, err = cm._run_git(["update-ref", new_ref, old_tip], store, str(new_workdir)) + if not ok: + raise OSError(f"could not create {new_ref}: {err}") + if not cm._delete_ref(store, old_ref): + raise OSError(f"could not delete {old_ref}") + cm._unlink_quiet(cm._project_meta_path(store, old_hash)) + cm._unlink_quiet(old_ledger_path) + cm._unlink_quiet(cm._index_path(store, old_hash)) + + +def migrate_profile_checkpoint_projects(old_profile_dir: Path, new_profile_dir: Path) -> Dict[str, int]: + """Rekey checkpoint projects whose workdirs moved with a profile rename. + + Only workdirs beneath ``old_profile_dir`` are affected: an external workdir keeps its absolute + path (and hash) even though the store holding its history moved. A workdir that no longer + exists under the new profile dir did not move and keeps its (orphaned) identity. + """ + old_root = cm._normalize_path(str(old_profile_dir)) + new_root = cm._normalize_path(str(new_profile_dir)) + store = cm._store_path(new_root / "checkpoints") + result = {"scanned": 0, "migrated": 0, "errors": 0} + if not cm._store_has_head(store): + return result + if shutil.which("git") is None: + # Without git the ref cannot move; rekeying only the metadata would orphan the history. + logger.warning("Cannot migrate checkpoint projects after profile rename: git not found") + result["errors"] = 1 + return result + for meta in cm._list_projects(store): + result["scanned"] += 1 + old_workdir = cm._normalize_path(str(meta.get("workdir") or "")) + if not meta.get("workdir") or not old_workdir.is_relative_to(old_root): + continue + new_workdir = new_root / old_workdir.relative_to(old_root) + if not new_workdir.is_dir(): + continue + try: + _rekey_project(store, meta, old_workdir, new_workdir) + result["migrated"] += 1 + except OSError as exc: + result["errors"] += 1 + logger.warning("Cannot migrate checkpoint project %s -> %s after profile rename: %s", + old_workdir, new_workdir, exc) + return result diff --git a/tools/checkpoint_profile_migration.py b/tools/checkpoint_profile_migration.py deleted file mode 100644 index 107c0e3a18..0000000000 --- a/tools/checkpoint_profile_migration.py +++ /dev/null @@ -1,192 +0,0 @@ -"""Rekey profile-local checkpoint projects after a named profile directory moves. - -The checkpoint store itself lives under ``HERMES_HOME/checkpoints`` and therefore moves with a -renamed profile. Project identity inside that store is different: refs, metadata and the -agent-write ledger are keyed by a hash of the absolute workdir. A profile rename changes that -absolute path for every project beneath the profile home, so those entries must follow the move. -""" - -from __future__ import annotations - -import json -import logging -import os -import shutil -from pathlib import Path -from typing import Dict - -from tools import checkpoint_manager as cm - -logger = logging.getLogger(__name__) - - -def _rebase_ledger_paths(ledger: Dict, old_workdir: Path, new_workdir: Path) -> Dict: - """Move absolute ledger keys under ``old_workdir`` to the corresponding new path.""" - rebased = {} - for raw_path, entry in ledger.items(): - try: - relative = Path(raw_path).relative_to(old_workdir) - except (TypeError, ValueError): - rebased[raw_path] = entry - else: - rebased[str(new_workdir / relative)] = entry - return rebased - - -def _temp_json_path(path: Path) -> Path: - return path.with_name(f".{path.name}.profile-rename-{os.getpid()}.tmp") - - -def _write_json_temp(path: Path, data: Dict) -> Path: - """Write a same-directory temporary JSON file ready for atomic ``replace``.""" - path.parent.mkdir(parents=True, exist_ok=True) - tmp = _temp_json_path(path) - tmp.write_text(json.dumps(data), encoding="utf-8") - return tmp - - -def migrate_profile_checkpoint_projects(old_profile_dir: Path, new_profile_dir: Path) -> Dict[str, int]: - """Rekey live checkpoint projects whose workdirs moved with a profile rename. - - Only workdirs beneath ``old_profile_dir`` are affected. External workdirs keep the same - absolute path even though their profile-owned checkpoint store moved, so their hash/ref stays - valid. Target collisions fail closed rather than merging two checkpoint histories. - - The per-project git index is a rebuildable cache: after the new ref and durable metadata are - installed, the old index is discarded and the next checkpoint seeds a fresh index from the - migrated ref. The agent-write ledger is durable behavior state and is rebased to the moved - absolute file paths so safe restore continues to recognize Hermes-authored writes. - """ - old_root = cm._normalize_path(str(old_profile_dir)) - new_root = cm._normalize_path(str(new_profile_dir)) - base = new_root / "checkpoints" - store = cm._store_path(base) - result = {"scanned": 0, "migrated": 0, "errors": 0} - if not cm._store_has_head(store): - return result - - git_available = shutil.which("git") is not None - for meta in cm._list_projects(store): - result["scanned"] += 1 - raw_workdir = meta.get("workdir") - old_hash = meta.get("_hash") - if not raw_workdir or not old_hash: - continue - old_workdir = cm._normalize_path(str(raw_workdir)) - try: - relative = old_workdir.relative_to(old_root) - except ValueError: - continue - - new_workdir = cm._normalize_path(str(new_root / relative)) - # A project that was already absent before the profile rename stays stale/orphaned; the - # rename should not retarget its retention evidence to a path that was never moved. - if not new_workdir.is_dir(): - continue - if not git_available: - result["errors"] += 1 - logger.warning( - "Cannot migrate checkpoint project %s after profile rename: git not found", - old_workdir, - ) - continue - - new_hash = cm._project_hash(str(new_workdir)) - if new_hash == old_hash: # defensive (cryptographic/path identity coincidence) - continue - - old_ref, new_ref = cm._ref_name(old_hash), cm._ref_name(new_hash) - old_meta = cm._project_meta_path(store, old_hash) - new_meta = cm._project_meta_path(store, new_hash) - old_ledger = cm._ledger_path(store, old_hash) - new_ledger = cm._ledger_path(store, new_hash) - old_index = cm._index_path(store, old_hash) - - old_tip = cm._ref_tip(store, str(new_workdir), old_ref) - new_tip = cm._ref_tip(store, str(new_workdir), new_ref) - if new_tip or new_meta.exists() or new_ledger.exists(): - result["errors"] += 1 - logger.warning( - "Cannot migrate checkpoint project %s -> %s after profile rename: " - "target identity %s already exists", - old_workdir, - new_workdir, - new_hash, - ) - continue - - serialized_meta = {key: value for key, value in meta.items() if key != "_hash"} - serialized_meta["workdir"] = str(new_workdir) - evidence = cm._volume_evidence(new_workdir) - if evidence: - serialized_meta.update(evidence) - else: - serialized_meta.pop("workdir_parent_dev", None) - serialized_meta.pop("workdir_parent_ino", None) - - ledger = cm._read_json_dict(old_ledger) if old_ledger.exists() else None - rebased_ledger = ( - _rebase_ledger_paths(ledger, old_workdir, new_workdir) - if ledger is not None - else None - ) - - meta_tmp = ledger_tmp = None - new_ref_created = False - installed_meta = installed_ledger = False - try: - meta_tmp = _write_json_temp(new_meta, serialized_meta) - if rebased_ledger is not None: - ledger_tmp = _write_json_temp(new_ledger, rebased_ledger) - - if old_tip: - ok, _, err = cm._run_git(["update-ref", new_ref, old_tip], store, str(new_workdir)) - if not ok: - raise OSError(f"could not create target checkpoint ref: {err}") - new_ref_created = True - - meta_tmp.replace(new_meta) - meta_tmp = None - installed_meta = True - if ledger_tmp is not None: - ledger_tmp.replace(new_ledger) - ledger_tmp = None - installed_ledger = True - except Exception as exc: - result["errors"] += 1 - logger.warning( - "Cannot migrate checkpoint project %s -> %s after profile rename: %s", - old_workdir, - new_workdir, - exc, - ) - if meta_tmp is not None: - cm._unlink_quiet(meta_tmp) - if ledger_tmp is not None: - cm._unlink_quiet(ledger_tmp) - if installed_meta: - cm._unlink_quiet(new_meta) - if installed_ledger: - cm._unlink_quiet(new_ledger) - if new_ref_created: - cm._delete_ref(store, new_ref) - continue - - # Delete the source ref only after the complete target identity exists. If that destructive - # step fails, roll the target back and leave the source metadata/ledger/index untouched so - # the standalone migrate-identity command can retry without merging two histories. - if old_tip and not cm._delete_ref(store, old_ref): - result["errors"] += 1 - logger.warning("Could not delete source checkpoint ref %s after creating %s", old_ref, new_ref) - cm._unlink_quiet(new_meta) - cm._unlink_quiet(new_ledger) - if new_ref_created: - cm._delete_ref(store, new_ref) - continue - - cm._unlink_quiet(old_meta) - cm._unlink_quiet(old_ledger) - cm._unlink_quiet(old_index) - result["migrated"] += 1 - - return result diff --git a/website/docs/reference/profile-commands.md b/website/docs/reference/profile-commands.md index df8bef7274..0f7a83ca30 100644 --- a/website/docs/reference/profile-commands.md +++ b/website/docs/reference/profile-commands.md @@ -245,7 +245,10 @@ hermes profile rename mybot assistant The rename also migrates the profile's persisted session/routing identity — session keys (`agent::*`), `sessions.profile_name`, heartbeats, and routing/delivery rows — to the new name. A live multiplexed gateway owns that migration (it holds the routing index in memory), so -when it is running the CLI delegates to it. +when it is running the CLI delegates to it. Checkpoint (`/rollback`) history of workspaces that +live inside the profile directory is rekeyed to their new path as well, so it stays reachable +after the rename; `hermes profile migrate-identity` retries that step too if it was reported as +failed. ## `hermes profile migrate-identity`