fix(hooks): stabilize root .apm hook source-ids across renames/worktrees (supersedes #1330, closes #1329) (#1392)

* fix: stabilize root hook source ids

* test: remove project-specific hook fixture names

* fix(hooks): bound dependency hook source discovery

* fix(hooks): reject symlinked dependency markers

* test(hooks): subprocess-tier coverage + debug logs for #1329

- Add tests/integration/test_hook_root_source_drift_e2e.py: subprocess
  test that seeds stale .claude/settings.json + apm-hooks.json sidecar
  with old source-id, runs apm install, asserts entries heal to
  _local/<manifest-name> and user-owned hook entry survives.
- Add two _log.debug lines in hook_integrator.py:
  - manifest-parse fallback in _get_root_local_package_name
  - heal count when stale same-content merged entries are removed
- Update affected unit tests to read _apm_source from Claude sidecar
  (post-#1359 schema-strict layout).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(hooks): act on review-panel recommendations for #1329 follow-up

Addresses the top recommended-tier follow-ups from the APM review panel
on #1392 (microsoft/apm#1392 (comment)).

In-PR:

* CHANGELOG: add [Unreleased] > Fixed entry with user-outcome framing,
  enumerating all five affected harnesses (Claude / Codex / Cursor /
  Gemini / Windsurf) and the explicit parenthetical that Copilot is
  unaffected because its hooks live in per-file namespaces rather than
  a shared merged config. (doc-writer + oss-growth-hacker convergence)

* Integration coverage: parametrize the e2e heal regression test across
  all five merged-hook harnesses instead of Claude only. Uses an
  install-then-corrupt-then-reinstall pattern so the seeded entries
  match each target's on-disk shape (sidecar for Claude; in-file
  _apm_source for Codex / Cursor / Gemini / Windsurf), closing the
  silent-drift gap the test-coverage-expert flagged as highest-signal.

* _safe_source_name hardening: collapse runs of 2+ dots to a single
  dot before stripping edges. No traversal risk today (the marker is
  JSON metadata), but guards any future caller that passes the name
  into a Path join. (supply-chain-security recommendation.)

* Nits: include exception message (not just class name) in the
  manifest-unreadable debug log; replace O(n*m) list membership in
  the heal counter with an id-set; document the project_root parameter
  on _get_package_name; promote _get_root_local_package_name,
  _get_hook_source_marker, and _should_remove_prior_merged_entry to
  @staticmethod (they touch no instance state).

The DependencySourceScanner extraction the python-architect proposed is
explicitly out of scope for this PR per the panel's own guidance
("does not need to land in this PR but should not slip past the next
meaningful touch to hook_integrator.py").

Validation:

* uv run --extra dev ruff check src/ tests/ -- clean
* uv run --extra dev ruff format --check src/ tests/ -- clean
* uv run --extra dev pytest tests/unit/integration/test_hook_integrator.py -- 156 passed
* uv run --extra dev pytest tests/integration/test_hook_root_source_drift_e2e.py -- 5 passed (claude, codex, cursor, gemini, windsurf)
* uv run --extra dev pytest tests/unit/integration/ tests/integration/test_hook_root_source_drift_e2e.py -- 1364 passed

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(ci): resolve R0801 duplication; expand lint contract to all 7 CI gates

CI lint was failing the PR with R0801 (pylint duplicate-code). The
duplication was introduced when main was merged into this branch: the
recent MCP-client work on main left an identical
`_select_best_package` method in both CodexClientAdapter and
CopilotClientAdapter, each ~22 lines long. pylint --fail-on=R0801
catches this at the merge commit CI tests, so the PR cannot pass
until the dup is resolved.

Fix: hoist `_select_best_package` to the `MCPClientAdapter` base class
(alongside the existing `_infer_registry_name` it already calls) and
delete the duplicates from codex.py and copilot.py. Both call sites
use `self._select_best_package(packages)`, so inheritance resolves
the lookup transparently. Mirrors the in-flight refactor on
`daf99e0f refactor(adapters): hoist _select_remote_with_url and
_select_best_package to base` (not yet on main).

Also: expand the canonical lint contract at
`.apm/instructions/linting.instructions.md` to cover all seven CI
Lint steps (ruff check, ruff format, YAML I/O guard, file-length
guard, no raw `str(relative_to)`, pylint R0801, auth-signal lint)
instead of only ruff. Adds the full `uv run ... && ...` mirror chain
and calls out the merge-commit semantics (CI tests HEAD merged with
main, so duplication on main can fail a clean PR diff). Apm compile
flows the change into the generated AGENTS.md (gitignored at root)
and into `.github/copilot-instructions.md`.

Validation:

* uv run --extra dev ruff check src/ tests/ -- clean
* uv run --extra dev ruff format --check src/ tests/ -- clean
* uv run --extra dev python -m pylint --disable=all --enable=R0801 \
  --min-similarity-lines=10 --fail-on=R0801 src/apm_cli/ -- 10.00/10, exit 0
* bash scripts/lint-auth-signals.sh -- clean
* uv run --extra dev pytest tests/unit/integration/test_hook_integrator.py \
  tests/integration/test_hook_root_source_drift_e2e.py -- 161 passed

Pre-existing main-side test failures in
`tests/unit/test_codex_adapter_compatibility.py` (introduced by
#1262 / #1277 on main and tracked by the in-flight
`32991fb6 fix(adapters): restore _resolve_env_placeholders shim and
reject SSE remotes in tests` branch) are out of scope for this PR
and will resolve when that follow-up lands on main.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(adapters): restore _resolve_env_placeholders shim and reject SSE remotes in tests

PR #1277 removed the _resolve_env_placeholders legacy wrapper from
adapters and softened Codex remote-server rejection (https URLs are
now accepted as streamable-http), but did not update the phase-3
and compatibility test suites. The result was 6 pre-existing test
failures on Build & Test that this PR's Windows-targeted changes
inherited.

- Restore _resolve_env_placeholders on MCPClientAdapter as a thin
  delegate to _resolve_variable_placeholders (empty runtime_vars).
  External callers and 4 test cases rely on the legacy name.
- Update test_returns_false_for_remote_only_server in both codex
  test files to use an SSE-transport remote, which is still the
  rejection contract post-#1277 (streamable-http is now accepted).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Koichi Takahashi <tkou15.hi@gmail.com>
Co-authored-by: Koichi Takahashi <work@k1t.space>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Daniel Meppiel <copilot-rework@github.com>

Author: 4 people

.apm/instructions/linting.instructions.md

@@ -10,20 +10,41 @@ report) that claims CI is green.
 
 ## CI-mirror commands
 
-The `Lint` job runs:
+The `Lint` job runs (see `.github/workflows/ci.yml`):
 
-- `uv run --extra dev ruff check src/ tests/`
-- `uv run --extra dev ruff format --check src/ tests/`
+1. `uv run --extra dev ruff check src/ tests/`
+2. `uv run --extra dev ruff format --check src/ tests/`
+3. YAML I/O safety guard (rejects raw `yaml.dump(..., handle)` outside
+   `utils/yaml_io.py`; mark approved exceptions with `# yaml-io-exempt`).
+4. File length guardrail (no `src/**/*.py` may exceed **2450 lines**).
+5. No raw `str(path.relative_to(...))` patterns -- use
+   `portable_relpath()` from `apm_cli.utils.paths`.
+6. **Code duplication guardrail (pylint R0801):**
+   `uv run --extra dev python -m pylint --disable=all --enable=R0801 \
+   --min-similarity-lines=10 --fail-on=R0801 src/apm_cli/`
+7. Auth-protocol boundary check: `bash scripts/lint-auth-signals.sh`
 
-Both must be silent.
+All seven must succeed. CI evaluates these on the **PR merge commit**
+(HEAD merged with current `main`), so duplication introduced by a
+recent main commit can fail your PR even if your own diff is clean.
+Always merge `main` locally before running the mirror.
 
 ## Local workflow
 
 - **Auto-fix style+imports:** `uv run --extra dev ruff check src/ tests/ --fix`
 - **Apply formatter:** `uv run --extra dev ruff format src/ tests/`
-- **Verify (must be silent):** `uv run --extra dev ruff check src/ tests/ && uv run --extra dev ruff format --check src/ tests/`
+- **Verify the full Lint job (must all be silent / exit 0):**
+  ```bash
+  uv run --extra dev ruff check src/ tests/ \
+    && uv run --extra dev ruff format --check src/ tests/ \
+    && uv run --extra dev python -m pylint --disable=all --enable=R0801 \
+       --min-similarity-lines=10 --fail-on=R0801 src/apm_cli/ \
+    && bash scripts/lint-auth-signals.sh
+  ```
+  (The YAML, file-length, and `relative_to` guards are pure-grep one-liners
+  from `ci.yml`; run them directly if you have touched those surfaces.)
 
-Always run the verify pair before `git push` -- the CI Lint job
+Always run the verify chain before `git push` -- the CI Lint job
 fails on any remaining diagnostic.
 
 ## Common surprises
@@ -36,11 +57,13 @@ fails on any remaining diagnostic.
 - `F401` / `F841` -- remove unused imports / unused locals.
 - `SIM103` -- inline negated returns where the body is one line.
 - `I001` -- import sort order (auto-fixable).
+- `R0801` -- 10+ identical lines across two files. Extract the shared
+  block into a base class / helper module instead of disabling.
 
 ## Lifecycle binding
 
 This is the canonical lint contract for the repo. Skills that
 produce artifacts asserting green CI -- notably `pr-description-skill`
 (whose "Validation evidence" row covers CI checks) -- inherit this
-gate transitively. Do NOT redefine ruff commands inside individual
-skills; honor this instruction before invoking them.
+gate transitively. Do NOT redefine ruff or pylint commands inside
+individual skills; honor this instruction before invoking them.

.github/copilot-instructions.md

@@ -1,5 +1,5 @@
 <!-- Generated by APM CLI from .apm/ primitives -->
-<!-- Build ID: a3144f4613b2 -->
+<!-- Build ID: 9a566a78e962 -->
 <!-- APM Version: 0.14.1 -->
 
 <!-- Source: .apm/instructions/linting.instructions.md -->
@@ -11,20 +11,41 @@ report) that claims CI is green.
 
 ## CI-mirror commands
 
-The `Lint` job runs:
+The `Lint` job runs (see `.github/workflows/ci.yml`):
 
-- `uv run --extra dev ruff check src/ tests/`
-- `uv run --extra dev ruff format --check src/ tests/`
+1. `uv run --extra dev ruff check src/ tests/`
+2. `uv run --extra dev ruff format --check src/ tests/`
+3. YAML I/O safety guard (rejects raw `yaml.dump(..., handle)` outside
+   `utils/yaml_io.py`; mark approved exceptions with `# yaml-io-exempt`).
+4. File length guardrail (no `src/**/*.py` may exceed **2450 lines**).
+5. No raw `str(path.relative_to(...))` patterns -- use
+   `portable_relpath()` from `apm_cli.utils.paths`.
+6. **Code duplication guardrail (pylint R0801):**
+   `uv run --extra dev python -m pylint --disable=all --enable=R0801 \
+   --min-similarity-lines=10 --fail-on=R0801 src/apm_cli/`
+7. Auth-protocol boundary check: `bash scripts/lint-auth-signals.sh`
 
-Both must be silent.
+All seven must succeed. CI evaluates these on the **PR merge commit**
+(HEAD merged with current `main`), so duplication introduced by a
+recent main commit can fail your PR even if your own diff is clean.
+Always merge `main` locally before running the mirror.
 
 ## Local workflow
 
 - **Auto-fix style+imports:** `uv run --extra dev ruff check src/ tests/ --fix`
 - **Apply formatter:** `uv run --extra dev ruff format src/ tests/`
-- **Verify (must be silent):** `uv run --extra dev ruff check src/ tests/ && uv run --extra dev ruff format --check src/ tests/`
+- **Verify the full Lint job (must all be silent / exit 0):**
+  ```bash
+  uv run --extra dev ruff check src/ tests/ \
+    && uv run --extra dev ruff format --check src/ tests/ \
+    && uv run --extra dev python -m pylint --disable=all --enable=R0801 \
+       --min-similarity-lines=10 --fail-on=R0801 src/apm_cli/ \
+    && bash scripts/lint-auth-signals.sh
+  ```
+  (The YAML, file-length, and `relative_to` guards are pure-grep one-liners
+  from `ci.yml`; run them directly if you have touched those surfaces.)
 
-Always run the verify pair before `git push` -- the CI Lint job
+Always run the verify chain before `git push` -- the CI Lint job
 fails on any remaining diagnostic.
 
 ## Common surprises
@@ -37,14 +58,16 @@ fails on any remaining diagnostic.
 - `F401` / `F841` -- remove unused imports / unused locals.
 - `SIM103` -- inline negated returns where the body is one line.
 - `I001` -- import sort order (auto-fixable).
+- `R0801` -- 10+ identical lines across two files. Extract the shared
+  block into a base class / helper module instead of disabling.
 
 ## Lifecycle binding
 
 This is the canonical lint contract for the repo. Skills that
 produce artifacts asserting green CI -- notably `pr-description-skill`
 (whose "Validation evidence" row covers CI checks) -- inherit this
-gate transitively. Do NOT redefine ruff commands inside individual
-skills; honor this instruction before invoking them.
+gate transitively. Do NOT redefine ruff or pylint commands inside
+individual skills; honor this instruction before invoking them.
 <!-- End source: .apm/instructions/linting.instructions.md -->
 
 ---

CHANGELOG.md

@@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - Unit test coverage raised to 88% (gate: `fail_under = 80`); integration test coverage raised to 71% with first CI gate at 55%. (#1402)
 
+### Fixed
+
+- Root `.apm` hooks no longer duplicate after renaming the project directory or using git worktrees; Claude, Codex, Cursor, Gemini, and Windsurf hook configs stay idempotent across checkouts. The hook source-id is now derived from `apm.yml`'s `name` field instead of `install_path.name`, and `apm install` silently heals stale same-content entries from prior checkout basenames. Copilot is unaffected (its hooks live in per-file namespaces under `.github/hooks/`, not a shared merged config). (#1392, closes #1329)
+
 ## [0.14.1] - 2026-05-20
 
 ### Added

src/apm_cli/adapters/client/base.py

@@ -248,6 +248,30 @@ def _infer_registry_name(package):
 
         return ""
 
+    @classmethod
+    def _select_best_package(cls, packages):
+        """Select the best package for installation from available packages.
+
+        Prioritizes packages in order: npm, docker, pypi, homebrew, others.
+        Uses ``_infer_registry_name`` so selection works even when the
+        registry API returns empty ``registry_name``.
+
+        Args:
+            packages (list): List of package dictionaries.
+
+        Returns:
+            dict: Best package to use, or None if no suitable package found.
+        """
+        priority_order = ["npm", "docker", "pypi", "homebrew"]
+
+        for target in priority_order:
+            for package in packages:
+                if cls._infer_registry_name(package) == target:
+                    return package
+
+        # If no priority package found, return the first one
+        return packages[0] if packages else None
+
     @staticmethod
     def _warn_input_variables(mapping, server_name, runtime_label):
         """Emit a warning for each ``${input:...}`` reference found in *mapping*.
@@ -542,6 +566,16 @@ def _replace_runtime(match):
 
         return processed
 
+    def _resolve_env_placeholders(self, value, resolved_env):
+        """Legacy thin wrapper for backward compatibility.
+
+        Kept because external callers and the phase-3 test suite invoke
+        the pre-#1277 name. Delegates to ``_resolve_variable_placeholders``
+        with an empty ``runtime_vars`` map. New code should call
+        ``_resolve_variable_placeholders`` directly.
+        """
+        return self._resolve_variable_placeholders(value, resolved_env, {})
+
     # ------------------------------------------------------------------
     # Shared server-info helpers (used by all adapter subclasses)
     # ------------------------------------------------------------------

src/apm_cli/adapters/client/codex.py

@@ -548,26 +548,3 @@ def _select_remote_with_url(remotes):
             if url:
                 return remote
         return None
-
-    def _select_best_package(self, packages):
-        """Select the best package for installation from available packages.
-
-        Prioritizes packages in order: npm, docker, pypi, homebrew, others.
-        Uses ``_infer_registry_name`` so selection works even when the
-        registry API returns empty ``registry_name``.
-
-        Args:
-            packages (list): List of package dictionaries.
-
-        Returns:
-            dict: Best package to use, or None if no suitable package found.
-        """
-        priority_order = ["npm", "docker", "pypi", "homebrew"]
-
-        for target in priority_order:
-            for package in packages:
-                if self._infer_registry_name(package) == target:
-                    return package
-
-        # If no priority package found, return the first one
-        return packages[0] if packages else None

src/apm_cli/adapters/client/copilot.py

@@ -1007,29 +1007,6 @@ def _select_remote_with_url(remotes):
                 return remote
         return None
 
-    def _select_best_package(self, packages):
-        """Select the best package for installation from available packages.
-
-        Prioritizes packages in order: npm, docker, pypi, homebrew, others.
-        Uses ``_infer_registry_name`` so selection works even when the
-        registry API returns empty ``registry_name``.
-
-        Args:
-            packages (list): List of package dictionaries.
-
-        Returns:
-            dict: Best package to use, or None if no suitable package found.
-        """
-        priority_order = ["npm", "docker", "pypi", "homebrew"]
-
-        for target in priority_order:
-            for package in packages:
-                if self._infer_registry_name(package) == target:
-                    return package
-
-        # If no priority package found, return the first one
-        return packages[0] if packages else None
-
     def _is_github_server(self, server_name, url):
         """Securely determine if a server is a GitHub MCP server.