From af2dc685fc244522188470dc20bfd59d25f7ea66 Mon Sep 17 00:00:00 2001 From: EndeavorYen Date: Thu, 20 Aug 2026 01:11:31 +0800 Subject: [PATCH] fix(profiles): honor tombstones in exists/backfill and tighten named-home detection Treat tombstoned leftover dirs as gone for exists/-p/use, skip them in env backfill, replace only empty shells on recreate, and stop treating a default home that merely contains a profiles path segment as named. --- hermes_cli/profiles.py | 13 +++- hermes_constants.py | 13 +++- .../test_deleted_profile_tombstone.py | 73 +++++++++++++++++++ 3 files changed, 95 insertions(+), 4 deletions(-) diff --git a/hermes_cli/profiles.py b/hermes_cli/profiles.py index d439f57b27..76d3f9c36e 100644 --- a/hermes_cli/profiles.py +++ b/hermes_cli/profiles.py @@ -385,11 +385,12 @@ def get_profile_dir(name: str) -> Path: def profile_exists(name: str) -> bool: - """Check whether a profile directory exists.""" + """Check whether a live (non-tombstoned) profile directory exists.""" canon = normalize_profile_name(name) if canon == "default": return True - return get_profile_dir(canon).is_dir() + profile_dir = get_profile_dir(canon) + return profile_dir.is_dir() and not named_profile_is_deleted(profile_dir) def profile_matches_home(name: str, home: "Path | None" = None) -> bool: @@ -1183,6 +1184,10 @@ def create_profile( profile_dir = get_profile_dir(canon) if profile_dir.exists() and named_profile_is_deleted(profile_dir): + # Empty shells left by post-delete mkdir may be replaced. Identity + # files mean the leftover is not a shell — fail closed, no rmtree. + if (profile_dir / "config.yaml").exists() or (profile_dir / ".env").exists(): + raise FileExistsError(f"Profile '{canon}' already exists at {profile_dir}") shutil.rmtree(profile_dir) if profile_dir.exists(): raise FileExistsError(f"Profile '{canon}' already exists at {profile_dir}") @@ -1397,6 +1402,8 @@ def backfill_profile_envs(quiet: bool = False) -> List[str]: continue if entry.name == "default": continue + if named_profile_is_deleted(entry): + continue env_path = entry / ".env" if env_path.exists(): continue @@ -2544,7 +2551,7 @@ def resolve_profile_env(profile_name: str) -> str: return str(root) profile_dir = root / "profiles" / canon - if not profile_dir.is_dir(): + if not profile_dir.is_dir() or named_profile_is_deleted(profile_dir): raise FileNotFoundError( f"Profile '{canon}' does not exist. " f"Create it with: hermes profile create {canon}" diff --git a/hermes_constants.py b/hermes_constants.py index c70e251fb0..0e21ecd809 100644 --- a/hermes_constants.py +++ b/hermes_constants.py @@ -234,11 +234,22 @@ _DELETED_PROFILES_DIR = ".deleted" def named_profile_home(path: str | Path) -> Path | None: - """Return ``/profiles/`` when *path* is that home or under it.""" + """Return ``/profiles/`` when *path* is that home or under it. + + A named profile home is only ``.../profiles/`` where ```` does + not start with ``.``. A default Hermes home whose path merely contains a + ``profiles`` segment (e.g. ``/tmp/foo/profiles/notahome/.hermes``) is not + a named profile. ``.../profiles/worker/logs`` still resolves to + ``.../profiles/worker``. + """ current = Path(path) for candidate in (current, *current.parents): if candidate.parent.name == "profiles" and not candidate.name.startswith("."): return candidate + # Stop at a default Hermes home so a coincidental ``profiles/`` + # ancestor is not treated as a named-profile root. + if candidate.name == ".hermes": + return None return None diff --git a/tests/hermes_cli/test_deleted_profile_tombstone.py b/tests/hermes_cli/test_deleted_profile_tombstone.py index b30bd15dd1..b0fca479ad 100644 --- a/tests/hermes_cli/test_deleted_profile_tombstone.py +++ b/tests/hermes_cli/test_deleted_profile_tombstone.py @@ -15,11 +15,16 @@ import pytest from hermes_cli.config import ensure_hermes_home from hermes_cli.profiles import ( + backfill_profile_envs, create_profile, delete_profile, list_profiles, + profile_exists, profiles_to_serve, + resolve_profile_env, + set_active_profile, ) +from hermes_constants import named_profile_home from hermes_logging import setup_logging @@ -36,6 +41,13 @@ def _named_homes(tmp_path: Path) -> list[str]: return [info.name for info in list_profiles() if not info.is_default] +def _delete(name: str) -> None: + with patch("hermes_cli.profiles._cleanup_gateway_service"), patch( + "hermes_cli.profiles._stop_profile_backends" + ): + delete_profile(name, yes=True) + + class TestDeletedProfileTombstone: def test_delete_then_logging_setup_does_not_recreate_home(self, profile_env, monkeypatch): profile_dir = create_profile("worker", no_alias=True, no_skills=True) @@ -93,3 +105,64 @@ class TestDeletedProfileTombstone: recreated = create_profile("worker", no_alias=True, no_skills=True) assert recreated.is_dir() assert "worker" in _named_homes(profile_env) + + def test_profile_exists_is_false_for_tombstoned_shell(self, profile_env): + profile_dir = create_profile("worker", no_alias=True, no_skills=True) + assert profile_exists("worker") is True + _delete("worker") + profile_dir.mkdir(parents=True) + + assert profile_exists("worker") is False + with pytest.raises(FileNotFoundError, match="does not exist"): + set_active_profile("worker") + with pytest.raises(FileNotFoundError, match="does not exist"): + resolve_profile_env("worker") + + def test_backfill_skips_tombstoned_directory(self, profile_env): + profile_dir = create_profile("worker", no_alias=True, no_skills=True) + _delete("worker") + profile_dir.mkdir(parents=True) + (profile_env / ".hermes" / ".env").write_text("OPENROUTER_API_KEY=root-key\n") + + backfilled = backfill_profile_envs(quiet=True) + + assert "worker" not in backfilled + assert not (profile_dir / ".env").exists() + + def test_create_after_delete_replaces_empty_shell(self, profile_env): + profile_dir = create_profile("worker", no_alias=True, no_skills=True) + _delete("worker") + profile_dir.mkdir(parents=True) + (profile_dir / "logs").mkdir() + + recreated = create_profile("worker", no_alias=True, no_skills=True) + assert recreated.is_dir() + assert "worker" in _named_homes(profile_env) + + @pytest.mark.parametrize("leftover", ["config.yaml", ".env"]) + def test_create_after_delete_refuses_when_identity_files_remain( + self, profile_env, leftover + ): + profile_dir = create_profile("worker", no_alias=True, no_skills=True) + _delete("worker") + profile_dir.mkdir(parents=True) + leftover_path = profile_dir / leftover + leftover_path.write_text("keep-me\n", encoding="utf-8") + + with pytest.raises(FileExistsError, match="already exists"): + create_profile("worker", no_alias=True, no_skills=True) + + assert leftover_path.read_text(encoding="utf-8") == "keep-me\n" + assert leftover_path.exists() + + +class TestNamedProfileHome: + def test_logs_under_named_profile_resolve_to_profile_home(self, tmp_path): + worker = tmp_path / "profiles" / "worker" + assert named_profile_home(worker / "logs") == worker + assert named_profile_home(worker) == worker + + def test_default_home_with_profiles_in_path_is_not_named(self, tmp_path): + default_home = tmp_path / "foo" / "profiles" / "notahome" / ".hermes" + assert named_profile_home(default_home) is None + assert named_profile_home(default_home / "logs") is None