refactor(cowork): DRY internal hot-path cleanup (closes #922)

src/apm_cli/integration/skill_integrator.py

@@ -9,6 +9,7 @@
 from pathlib import Path
 
 from apm_cli.integration.base_integrator import BaseIntegrator
+from apm_cli.integration.targets import TargetProfile
 from apm_cli.utils.atomic_io import write_text_lf
 
 
@@ -440,6 +441,20 @@ def __init__(self) -> None:
         # map so that same-manifest collisions are detected before the lockfile is written.
         self._native_skill_session_owners: dict[str, str] = {}
 
+    @staticmethod
+    def _target_skills_root(target: TargetProfile, project_root: Path) -> Path:
+        """Return the target skills root for static and dynamic-root targets."""
+        if target.resolved_deploy_root is not None:
+            return target.deploy_path(project_root)
+        skills_mapping = target.primitives["skills"]
+        effective_root = skills_mapping.deploy_root or target.root_dir
+        return project_root / effective_root / "skills"
+
+    @staticmethod
+    def _target_skill_dir(target: TargetProfile, project_root: Path, skill_name: str) -> Path:
+        """Return the concrete directory for a deployed skill."""
+        return SkillIntegrator._target_skills_root(target, project_root) / skill_name
+
     def find_instruction_files(self, package_path: Path) -> list[Path]:
         """Find all instruction files in a package.
 
@@ -832,13 +847,7 @@ def _promote_sub_skills_standalone(
                 continue
 
             is_primary = idx == 0  # first active target owns diagnostics
-            skills_mapping = target.primitives["skills"]
-            # Dynamic-root targets (cowork): use resolved_deploy_root.
-            if target.resolved_deploy_root is not None:
-                target_skills_root = target.resolved_deploy_root
-            else:
-                effective_root = skills_mapping.deploy_root or target.root_dir
-                target_skills_root = project_root / effective_root / "skills"
+            target_skills_root = self._target_skills_root(target, project_root)
 
             # Dedup: skip if same resolved skills root already processed.
             resolved_root = target_skills_root.resolve()
@@ -974,12 +983,9 @@ def _integrate_native_skill(
 
             is_primary = idx == 0  # first active target owns diagnostics
             skills_mapping = target.primitives["skills"]
-            # Dynamic-root targets (cowork): use resolved_deploy_root.
-            if target.resolved_deploy_root is not None:
-                target_skill_dir = target.resolved_deploy_root / skill_name
-            else:
-                effective_root = skills_mapping.deploy_root or target.root_dir
-                target_skill_dir = project_root / effective_root / "skills" / skill_name
+            # Static targets still need the effective root for the containment guard below.
+            effective_root = skills_mapping.deploy_root or target.root_dir
+            target_skill_dir = self._target_skill_dir(target, project_root, skill_name)
 
             # Security: validate name + containment + symlink rejection.
             from apm_cli.utils.path_security import (
@@ -1072,11 +1078,8 @@ def _ignore_non_content_and_apm(directory, contents):
             if is_primary:
                 files_copied = sum(1 for _ in target_skill_dir.rglob("*") if _.is_file())
 
-            # Promote sub-skills for this target
-            if target.resolved_deploy_root is not None:
-                target_skills_root = target.resolved_deploy_root
-            else:
-                target_skills_root = project_root / effective_root / "skills"
+            # Promote sub-skills for this target.
+            target_skills_root = self._target_skills_root(target, project_root)
             _, sub_deployed = self._promote_sub_skills(
                 sub_skills_dir,
                 target_skills_root,

tests/unit/integration/test_skill_integrator_hermetic.py

@@ -63,6 +63,11 @@ def _make_target(
     target.root_dir = root_dir
     target.auto_create = auto_create
     target.resolved_deploy_root = resolved_deploy_root
+    target.deploy_path = MagicMock(
+        side_effect=lambda project_root, *parts: (
+            resolved_deploy_root if resolved_deploy_root is not None else project_root / root_dir
+        ).joinpath(*parts)
+    )
     prim = MagicMock()
     mapping = MagicMock()
     mapping.deploy_root = deploy_root

tests/unit/integration/test_skill_integrator_phase3w4.py

@@ -63,6 +63,11 @@ def _make_target(
     target.root_dir = root_dir
     target.auto_create = auto_create
     target.resolved_deploy_root = resolved_deploy_root
+    target.deploy_path = MagicMock(
+        side_effect=lambda project_root, *parts: (
+            resolved_deploy_root if resolved_deploy_root is not None else project_root / root_dir
+        ).joinpath(*parts)
+    )
     prim = MagicMock()
     mapping = MagicMock()
     mapping.deploy_root = deploy_root