From 890158febadff3e48965b313910ffc4ef3abc412 Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Fri, 26 Jun 2026 00:35:45 +0200 Subject: [PATCH 1/2] refactor(cowork): route skill deploy roots through target helper Consolidate the cowork dynamic-root branches in SkillIntegrator so the hot path reuses TargetProfile.deploy_path for resolved deploy roots while preserving static deploy_root handling. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/apm_cli/integration/skill_integrator.py | 38 ++++++++++--------- .../test_skill_integrator_hermetic.py | 5 +++ .../test_skill_integrator_phase3w4.py | 5 +++ 3 files changed, 30 insertions(+), 18 deletions(-) diff --git a/src/apm_cli/integration/skill_integrator.py b/src/apm_cli/integration/skill_integrator.py index f98fb873e..88b9aeea3 100644 --- a/src/apm_cli/integration/skill_integrator.py +++ b/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 def _build_copy_ignore( @@ -439,6 +440,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. @@ -831,13 +846,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() @@ -973,12 +982,8 @@ 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 + 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 ( @@ -1071,11 +1076,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, diff --git a/tests/unit/integration/test_skill_integrator_hermetic.py b/tests/unit/integration/test_skill_integrator_hermetic.py index d83f868e4..86554d18e 100644 --- a/tests/unit/integration/test_skill_integrator_hermetic.py +++ b/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 diff --git a/tests/unit/integration/test_skill_integrator_phase3w4.py b/tests/unit/integration/test_skill_integrator_phase3w4.py index f7030d94f..639270fed 100644 --- a/tests/unit/integration/test_skill_integrator_phase3w4.py +++ b/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 From ff0600b22091402772451dbdc915c20940e973f9 Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Fri, 26 Jun 2026 00:48:44 +0200 Subject: [PATCH 2/2] docs(cowork): clarify skill root containment guard Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/apm_cli/integration/skill_integrator.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/apm_cli/integration/skill_integrator.py b/src/apm_cli/integration/skill_integrator.py index 3663c1d32..f158961af 100644 --- a/src/apm_cli/integration/skill_integrator.py +++ b/src/apm_cli/integration/skill_integrator.py @@ -983,6 +983,7 @@ def _integrate_native_skill( is_primary = idx == 0 # first active target owns diagnostics skills_mapping = target.primitives["skills"] + # 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)