From 683dfc2a96c6744a9e08f7e5de5207f6f760c0c4 Mon Sep 17 00:00:00 2001 From: Sergei Malykh <2181517+xronos-i-am@users.noreply.github.com> Date: Fri, 30 Oct 2026 11:26:20 +0300 Subject: [PATCH 1/2] fix(install): keep files deployed to other Claude/Hermes profiles Claude (CLAUDE_CONFIG_DIR) and Hermes (HERMES_HOME) profiles share one user lockfile. A global install into one profile treated every unrefreshed row of the active target as stale, regardless of its root, and the lockfile phase deleted the default profile's files. A row of a target whose user root an environment variable selects is now stale only when it lies under the selected root. Rows of other profiles stay in the lockfile and are cleaned up when their own profile is installed into. The fix is deliberately bounded. Unlike #2884 it does not isolate profiles through APM_HOME: there is still one lockfile and no schema change. Unlike #3238 it adds no symlink normalization: the root check is a lexical prefix match that neither resolves nor follows symlinks, it only narrows what counts as stale, and every deletion still goes through validate_deploy_path. Uninstall across profiles is out of scope. Refs #3423 --- CHANGELOG.md | 4 ++ docs/src/content/docs/integrations/hermes.md | 2 +- .../src/content/docs/reference/cli/install.md | 2 +- src/apm_cli/core/deployment_state.py | 11 +++- src/apm_cli/integration/targets.py | 29 ++++++++- .../test_claude_config_profile_lifecycle.py | 61 +++++++++++++++++++ tests/unit/core/test_deployment_state.py | 24 ++++++++ 7 files changed, 127 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 82960dc779..9058e0e135 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- `apm install -g` into one Claude profile (`CLAUDE_CONFIG_DIR`) or Hermes home (`HERMES_HOME`) no longer deletes files deployed to another profile through the shared user lockfile; stale-file cleanup is limited to the selected root. (#3423) + ## [0.35.0] - 2026-10-22 ### Added diff --git a/docs/src/content/docs/integrations/hermes.md b/docs/src/content/docs/integrations/hermes.md index 8b9603f196..46597370f9 100644 --- a/docs/src/content/docs/integrations/hermes.md +++ b/docs/src/content/docs/integrations/hermes.md @@ -47,7 +47,7 @@ export HERMES_HOME="$HOME/.config/hermes" apm install --target hermes --global ``` -When `HERMES_HOME` lives under `$HOME`, APM keeps the deploy root home-relative; otherwise it uses the absolute path. The directory does not need to exist yet. +When `HERMES_HOME` lives under `$HOME`, APM keeps the deploy root home-relative; otherwise it uses the absolute path. The directory does not need to exist yet. Hermes homes share one user lockfile; a global install removes stale skills only under the selected `HERMES_HOME`, leaving skills deployed to other homes in place. ## MCP servers diff --git a/docs/src/content/docs/reference/cli/install.md b/docs/src/content/docs/reference/cli/install.md index 909e7d016e..ab2253c402 100644 --- a/docs/src/content/docs/reference/cli/install.md +++ b/docs/src/content/docs/reference/cli/install.md @@ -208,7 +208,7 @@ still cleans up its managed files, subject to the usual user-edit protections. - **Direct registry lookup fails closed.** Registry-form MCP entries (`apm install --mcp NAME` with no `--url` and no post-`--` command) resolve one unique registry identity before writing the manifest or user config. An unreachable registry, missing identity, or ambiguous bare server name exits non-zero without changing state. - **Diff-aware.** Packages whose ref or version changed in `apm.yml` are re-downloaded automatically. MCP servers with matching config are skipped; unchanged APM-managed connections can be updated when their declaration changes. - **Claude/Codex MCP collisions.** A differing self-defined connection with the same name is preserved unless its native configuration still matches the recorded APM baseline. Install identifies the conflicting target and exits nonzero; successful writes to other targets remain recorded. An identical foreign entry is not adopted. Neither `--force` nor `--frozen` takes over a user connection. Removal also preserves entries with a differing or unverifiable baseline. See [MCP conflict recovery](../../../consumer/install-mcp-servers/#resolve-a-connection-name-conflict). -- **Claude Code profiles.** For user-scope MCP installs, an absolute `CLAUDE_CONFIG_DIR` selects that profile's `.claude.json` for both installation and stale-server cleanup. Removing an APM-managed server in the selected profile preserves other profiles and user-owned entries. Cleanup refuses symlinked config files or profile directories; replace the link with a regular file or directory before retrying. +- **Claude Code profiles.** For user-scope MCP installs, an absolute `CLAUDE_CONFIG_DIR` selects that profile's `.claude.json` for both installation and stale-server cleanup. Removing an APM-managed server in the selected profile preserves other profiles and user-owned entries. Cleanup refuses symlinked config files or profile directories; replace the link with a regular file or directory before retrying. Skills, agents, and other deployed files follow the same rule: profiles share one user lockfile, and a global install removes stale files only under the selected `CLAUDE_CONFIG_DIR` (or `~/.claude`), leaving files deployed to other profiles in place. Files of a profile you no longer install into stay on disk and in the lockfile; remove them manually. - **Transactional replacement.** `--update` and `--refresh` download package replacements to isolated staging paths and validate them before publication. Failed downloads or validation leave the previous package active. Failed diff --git a/src/apm_cli/core/deployment_state.py b/src/apm_cli/core/deployment_state.py index 6333f18e0f..c6bf761379 100644 --- a/src/apm_cli/core/deployment_state.py +++ b/src/apm_cli/core/deployment_state.py @@ -525,11 +525,20 @@ def _filter_absent_owners( ) def _is_stale(self, record: DeploymentRecord, intent: DeploymentIntent) -> bool: + """Decide whether an unrefreshed row lost its target this run. + + A row of an active target is stale unless it lies under another + profile root of that target (``CLAUDE_CONFIG_DIR``, ``HERMES_HOME``). + A row of a known target dropped from the declared set is stale too. + """ if not intent.authoritative_targets: return False target = record.locator.target if target in intent.active_targets: - return True + from apm_cli.integration.targets import is_other_profile_path + + profile = self.target_profiles.get(target) + return profile is None or not is_other_profile_path(profile, record.locator.value) return ( intent.declared_targets is not None and target in self.target_profiles diff --git a/src/apm_cli/integration/targets.py b/src/apm_cli/integration/targets.py index 1c4fa98d18..39eaa4df4e 100644 --- a/src/apm_cli/integration/targets.py +++ b/src/apm_cli/integration/targets.py @@ -53,6 +53,16 @@ on the mapping, and add a ``_convert_to_*`` branch in ``_render_instruction``. """ +USER_ROOT_ENV_VARS: dict[str, str] = { + "claude": "CLAUDE_CONFIG_DIR", + "hermes": "HERMES_HOME", +} +"""Targets whose user-scope root an environment variable selects. + +Each value of the variable is a separate profile of the same runtime, and +all of them share one user lockfile. +""" + @dataclass(frozen=True) class PrimitiveMapping: @@ -475,12 +485,11 @@ def for_scope(self, user_scope: bool = False) -> TargetProfile | None: # Claude Code honors CLAUDE_CONFIG_DIR (default ~/.claude) and Hermes # honors HERMES_HOME (default ~/.hermes); mirror that at user scope so # `apm install -g` lands where the tool reads. - if self.name in ("claude", "hermes"): + if self.name in USER_ROOT_ENV_VARS: import os from pathlib import Path - env_var = "CLAUDE_CONFIG_DIR" if self.name == "claude" else "HERMES_HOME" - env = os.environ.get(env_var, "").strip() + env = os.environ.get(USER_ROOT_ENV_VARS[self.name], "").strip() if env: # ``resolve`` collapses ``..`` so traversal segments cannot # leak into ``root_dir`` and escape ``project_root / root_dir``. @@ -1272,6 +1281,20 @@ def get_integration_prefixes(targets=None, *, user_scope: bool = False) -> tuple return tuple(prefixes) +def is_other_profile_path(profile: TargetProfile, value: str) -> bool: + """Return True when *value* was deployed to another profile of *profile*. + + Profiles of a target listed in :data:`USER_ROOT_ENV_VARS` share one user + lockfile, so its ``deployed_files`` can list paths under the roots of + profiles other than the selected one. Such a path is not stale for an + install into the selected profile. The check is a lexical prefix match + against the selected root; it neither resolves nor follows symlinks. + """ + if profile.name not in USER_ROOT_ENV_VARS or "://" in value: + return False + return not value.startswith(get_integration_prefixes(targets=[profile])) + + def active_targets_user_scope( explicit_target: str | list[str] | None = None, *, diff --git a/tests/integration/test_claude_config_profile_lifecycle.py b/tests/integration/test_claude_config_profile_lifecycle.py index 54583a33eb..a305ed648d 100644 --- a/tests/integration/test_claude_config_profile_lifecycle.py +++ b/tests/integration/test_claude_config_profile_lifecycle.py @@ -165,3 +165,64 @@ def test_claude_profile_cleanup_refuses_unsafe_paths_without_changing_state( ) assert config.read_bytes() == config_before assert lock_path.read_bytes() == lock_before + + +def _skills(root: Path) -> set[str]: + """Return the names of skills deployed under one Claude profile root.""" + return {path.parent.name for path in (root / "skills").glob("*/SKILL.md")} + + +@pytest.mark.parametrize("location", ["home", "outside-home"]) +def test_claude_profile_install_keeps_files_deployed_to_other_profiles( + tmp_path: Path, location: str +) -> None: + """A -> B -> A -> B keeps both profiles; stale cleanup stays in the selected one (#3423).""" + isolated, cwd, runner = _scenario(tmp_path) + dump_yaml({"name": "skills-home", "version": "0.1.0"}, isolated.config_root / "apm.yml") + factory = LocalPackageFactory(isolated.package_root) + package = factory.create("profile-skills", targets=("claude",)) + factory.add_skill(package, "keep", "---\nname: keep\ndescription: Kept.\n---\nKeep.\n") + renamed = factory.add_skill( + package, "renamed", "---\nname: renamed\ndescription: Removed.\n---\nRemoved.\n" + ) + default = isolated.home / ".claude" + profile_b = (isolated.home if location == "home" else isolated.root) / "profiles" / "b" + lock_path = isolated.config_root / "apm.lock.yaml" + all_skills = {"keep", "renamed"} + + def install(root: Path) -> None: + """Install the local package into the profile rooted at *root*.""" + result = runner.run( + ("install", "--global", str(package.root), "--target", "claude", "--no-policy"), + scenario_id="claude-profile-files", + cwd=cwd, + env={ + **isolated.subprocess_env(), + "CLAUDE_CONFIG_DIR": "" if root == default else str(root), + }, + ) + assert result.returncode == 0, result.stdout + result.stderr + + install(default) + assert _skills(default) == all_skills + for root in (profile_b, default, profile_b): + install(root) + assert _skills(default) == all_skills + assert _skills(profile_b) == all_skills + + lock_before = lock_path.read_bytes() + install(profile_b) + assert lock_path.read_bytes() == lock_before + assert _skills(default) == all_skills + assert _skills(profile_b) == all_skills + + # A skill dropped from the package is removed only from the selected + # profile; the other profile is cleaned when it is installed into. + renamed.unlink() + renamed.parent.rmdir() + install(profile_b) + assert _skills(profile_b) == {"keep"} + assert _skills(default) == all_skills + install(default) + assert _skills(default) == {"keep"} + assert _skills(profile_b) == {"keep"} diff --git a/tests/unit/core/test_deployment_state.py b/tests/unit/core/test_deployment_state.py index 79f750f79f..1312516a46 100644 --- a/tests/unit/core/test_deployment_state.py +++ b/tests/unit/core/test_deployment_state.py @@ -420,6 +420,30 @@ def test_same_target_stale_and_undeclared_target_are_removed(tmp_path: Path) -> assert set(reconciled.removed) == {active, ghost} +@pytest.mark.parametrize("selected_root", ["profiles/b", "/outside/profiles/b"]) +@pytest.mark.parametrize(("target", "default_root"), [("claude", ".claude"), ("hermes", ".hermes")]) +def test_other_profile_rows_are_not_stale( + tmp_path: Path, selected_root: str, target: str, default_root: str +) -> None: + """Rows under another CLAUDE_CONFIG_DIR/HERMES_HOME root survive an install into the selected one.""" + selected = _locator(f"{selected_root}/skills/demo/SKILL.md", target=target) + default = _locator(f"{default_root}/skills/demo/SKILL.md", target=target) + sibling = _locator("profiles/c/skills/demo/SKILL.md", target=target) + prior = DeploymentLedger( + records={locator.key: _record(locator) for locator in (selected, default, sibling)} + ) + reconciler = DeploymentReconciler( + tmp_path, + {target: _target(target, selected_root)}, + diagnostics=DiagnosticCollector(), + ) + + reconciled = reconciler.reconcile(prior, [], _intent(active=frozenset({target}))) + + assert reconciled.removed == (selected,) + assert set(reconciled.ledger.records) == {default.key, sibling.key} + + @pytest.mark.parametrize( ("dropped_target", "surviving_target"), ( From ca1fb372a3888a43aa62ac1d1f07e2df9678a690 Mon Sep 17 00:00:00 2001 From: Sergei Malykh <2181517+xronos-i-am@users.noreply.github.com> Date: Fri, 30 Oct 2026 11:49:22 +0300 Subject: [PATCH 2/2] fix(install): compare profile roots with POSIX separators An absolute profile root keeps native separators (D:\profiles\b on Windows) while lockfile values use forward slashes, so rows of the selected profile outside $HOME looked like another profile's and were never treated as stale. Compare both sides in POSIX form; the check stays lexical and does not resolve symlinks. Refs #3423 --- src/apm_cli/integration/targets.py | 7 ++++++- tests/unit/core/test_deployment_state.py | 14 +++++++++++--- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/src/apm_cli/integration/targets.py b/src/apm_cli/integration/targets.py index 39eaa4df4e..3a20c28b74 100644 --- a/src/apm_cli/integration/targets.py +++ b/src/apm_cli/integration/targets.py @@ -1289,10 +1289,15 @@ def is_other_profile_path(profile: TargetProfile, value: str) -> bool: profiles other than the selected one. Such a path is not stale for an install into the selected profile. The check is a lexical prefix match against the selected root; it neither resolves nor follows symlinks. + Separators are compared in POSIX form: an absolute root keeps native + separators (``D:\\profiles\\b``) while lockfile values use ``/``. """ if profile.name not in USER_ROOT_ENV_VARS or "://" in value: return False - return not value.startswith(get_integration_prefixes(targets=[profile])) + prefixes = tuple( + prefix.replace("\\", "/") for prefix in get_integration_prefixes(targets=[profile]) + ) + return not value.replace("\\", "/").startswith(prefixes) def active_targets_user_scope( diff --git a/tests/unit/core/test_deployment_state.py b/tests/unit/core/test_deployment_state.py index 1312516a46..4641337c49 100644 --- a/tests/unit/core/test_deployment_state.py +++ b/tests/unit/core/test_deployment_state.py @@ -420,13 +420,21 @@ def test_same_target_stale_and_undeclared_target_are_removed(tmp_path: Path) -> assert set(reconciled.removed) == {active, ghost} -@pytest.mark.parametrize("selected_root", ["profiles/b", "/outside/profiles/b"]) +@pytest.mark.parametrize( + ("selected_root", "selected_value"), + [ + ("profiles/b", "profiles/b"), + ("/outside/profiles/b", "/outside/profiles/b"), + ("D:\\profiles\\b", "D:/profiles/b"), + ], + ids=["home-relative", "absolute", "windows-absolute"], +) @pytest.mark.parametrize(("target", "default_root"), [("claude", ".claude"), ("hermes", ".hermes")]) def test_other_profile_rows_are_not_stale( - tmp_path: Path, selected_root: str, target: str, default_root: str + tmp_path: Path, selected_root: str, selected_value: str, target: str, default_root: str ) -> None: """Rows under another CLAUDE_CONFIG_DIR/HERMES_HOME root survive an install into the selected one.""" - selected = _locator(f"{selected_root}/skills/demo/SKILL.md", target=target) + selected = _locator(f"{selected_value}/skills/demo/SKILL.md", target=target) default = _locator(f"{default_root}/skills/demo/SKILL.md", target=target) sibling = _locator("profiles/c/skills/demo/SKILL.md", target=target) prior = DeploymentLedger(