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..3a20c28b74 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,25 @@ 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. + 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 + prefixes = tuple( + prefix.replace("\\", "/") for prefix in get_integration_prefixes(targets=[profile]) + ) + return not value.replace("\\", "/").startswith(prefixes) + + 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..4641337c49 100644 --- a/tests/unit/core/test_deployment_state.py +++ b/tests/unit/core/test_deployment_state.py @@ -420,6 +420,38 @@ def test_same_target_stale_and_undeclared_target_are_removed(tmp_path: Path) -> assert set(reconciled.removed) == {active, ghost} +@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, 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_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( + 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"), (