fix: resolve transitive local paths against declaring package (#857)

src/apm_cli/deps/apm_resolver.py

@@ -11,17 +11,21 @@
 )
 
 # Type alias for the download callback.
-# Takes (dep_ref, apm_modules_dir, parent_chain) and returns the install path
-# if successful.  ``parent_chain`` is a human-readable breadcrumb string like
-# "root-pkg > mid-pkg > this-pkg" showing the full dependency path including
-# the current node, or just the node's display name for direct (depth-1) deps.
+# Takes (dep_ref, apm_modules_dir, parent_chain, parent_pkg) and returns the
+# install path if successful. ``parent_chain`` is a human-readable breadcrumb
+# string like "root-pkg > mid-pkg > this-pkg" showing the full dependency path
+# including the current node, or just the node's display name for direct
+# (depth-1) deps. ``parent_pkg`` is the APMPackage that declared this
+# dependency (None for direct deps from the root); callers use its
+# ``source_path`` to anchor relative ``local_path`` resolution (#857).
 @runtime_checkable
 class DownloadCallback(Protocol):
     def __call__(
         self,
         dep_ref: 'DependencyReference',
         apm_modules_dir: Path,
         parent_chain: str = "",
+        parent_pkg: Optional['APMPackage'] = None,
     ) -> Optional[Path]: ...
 
 
@@ -79,6 +83,8 @@ def resolve_dependencies(self, project_root: Path) -> DependencyGraph:
 
         try:
             root_package = APMPackage.from_apm_yml(apm_yml_path)
+            # Anchor for relative local_path dependencies declared at the root.
+            root_package.source_path = project_root.resolve()
         except (ValueError, FileNotFoundError) as e:
             # Create error graph
             empty_package = APMPackage(name="error", version="0.0.0", package_path=project_root)
@@ -213,7 +219,9 @@ def build_dependency_tree(self, root_apm_yml: Path) -> DependencyTree:
                 parent_chain = node.get_ancestor_chain()
 
                 loaded_package = self._try_load_dependency_package(
-                    dep_ref, parent_chain=parent_chain
+                    dep_ref,
+                    parent_chain=parent_chain,
+                    parent_pkg=parent_node.package if parent_node else None,
                 )
                 if loaded_package:
                     # Update the node with the actual loaded package
@@ -358,35 +366,43 @@ def _validate_dependency_reference(self, dep_ref: DependencyReference) -> bool:
         return True
 
     def _try_load_dependency_package(
-        self, dep_ref: DependencyReference, parent_chain: str = ""
+        self,
+        dep_ref: DependencyReference,
+        parent_chain: str = "",
+        parent_pkg: Optional[APMPackage] = None,
     ) -> Optional[APMPackage]:
         """
         Try to load a dependency package from apm_modules/.
-        
+
         This method scans apm_modules/ to find installed packages and loads their
         apm.yml to enable transitive dependency resolution. If a package is not
         installed and a download_callback is available, it will attempt to fetch
         the package first.
-        
+
         Args:
             dep_ref: Reference to the dependency to load
             parent_chain: Human-readable breadcrumb of the dependency path
                 that led here (e.g. "root-pkg > mid-pkg").  Forwarded to the
                 download callback for contextual error messages.
-
+            parent_pkg: APMPackage that declared *dep_ref* (None for direct
+                deps from the root project). Its ``source_path`` is forwarded
+                to the download callback so relative ``local_path`` deps
+                resolve against the directory containing the declaring
+                apm.yml rather than the root consumer (#857).
+
         Returns:
             APMPackage: Loaded package if found, None otherwise
-            
+
         Raises:
             ValueError: If package exists but has invalid format
             FileNotFoundError: If package cannot be found
         """
         if self._apm_modules_dir is None:
             return None
-        
+
         # Get the canonical install path for this dependency
         install_path = dep_ref.get_install_path(self._apm_modules_dir)
-        
+
         # If package doesn't exist locally, try to download it
         if not install_path.exists():
             if self._download_callback is not None:
@@ -395,19 +411,29 @@ def _try_load_dependency_package(
                 if unique_key not in self._downloaded_packages:
                     try:
                         downloaded_path = self._download_callback(
-                            dep_ref, self._apm_modules_dir, parent_chain
+                            dep_ref,
+                            self._apm_modules_dir,
+                            parent_chain,
+                            parent_pkg,
                         )
                         if downloaded_path and downloaded_path.exists():
                             self._downloaded_packages.add(unique_key)
                             install_path = downloaded_path
                     except Exception:
                         # Download failed - continue without this dependency's sub-deps
                         pass
-            
+
             # Still doesn't exist after download attempt
             if not install_path.exists():
                 return None
-
+
+        # Anchor for resolving this package's own relative local_path deps:
+        # the *original* source dir for local deps (not the apm_modules copy),
+        # otherwise the install_path itself.
+        resolved_source_path = self._compute_dep_source_path(
+            dep_ref, install_path, parent_pkg
+        )
+
         # Look for apm.yml in the install path
         apm_yml_path = install_path / "apm.yml"
         if not apm_yml_path.exists():
@@ -420,22 +446,56 @@ def _try_load_dependency_package(
                     name=dep_ref.get_display_name(),
                     version="1.0.0",
                     source=dep_ref.repo_url,
-                    package_path=install_path
+                    package_path=install_path,
+                    source_path=resolved_source_path,
                 )
             # No manifest found
             return None
-        
+
         # Load and return the package
         try:
             package = APMPackage.from_apm_yml(apm_yml_path)
             # Ensure source is set for tracking
             if not package.source:
                 package.source = dep_ref.repo_url
+            # Set source_path so transitive resolution of THIS package's
+            # local-path deps anchors on the right directory.
+            package.source_path = resolved_source_path
             return package
         except (ValueError, FileNotFoundError) as e:
             # Package has invalid apm.yml - log warning but continue
             # In production, we might want to surface this to the user
             return None
+
+    def _compute_dep_source_path(
+        self,
+        dep_ref: DependencyReference,
+        install_path: Path,
+        parent_pkg: Optional[APMPackage],
+    ) -> Path:
+        """Return the on-disk anchor for resolving *dep_ref*'s own local deps.
+
+        For local dependencies this is the *original* directory the user
+        referenced (so relative paths inside the package's apm.yml mean what
+        a developer reading the file would expect). For remote deps it is
+        ``install_path`` (the clone location), which is also where the
+        package's apm.yml lives.
+        """
+        if dep_ref.is_local and dep_ref.local_path:
+            local = Path(dep_ref.local_path).expanduser()
+            if local.is_absolute():
+                return local.resolve()
+            base_dir = (
+                parent_pkg.source_path
+                if parent_pkg is not None and parent_pkg.source_path is not None
+                else self._project_root
+            )
+            if base_dir is None:
+                # Fallback: best-effort relative-to-cwd. Rare; only hit if
+                # the resolver was driven without a project root.
+                return local.resolve()
+            return (base_dir / local).resolve()
+        return install_path.resolve()
 
     def _create_resolution_summary(self, graph: DependencyGraph) -> str:
         """

src/apm_cli/install/phases/local_content.py

@@ -75,13 +75,16 @@ def _has_local_apm_content(project_root):
 # ---------------------------------------------------------------------------
 
 
-def _copy_local_package(dep_ref, install_path, project_root, logger=None):
+def _copy_local_package(dep_ref, install_path, base_dir, logger=None):
     """Copy a local package to apm_modules/.
 
     Args:
         dep_ref: DependencyReference with is_local=True
         install_path: Target path under apm_modules/
-        project_root: Project root for resolving relative paths
+        base_dir: Directory used to resolve a relative ``dep_ref.local_path``.
+            For direct deps from the root project this is the project root;
+            for transitive deps it is the source directory of the package
+            whose apm.yml declared *dep_ref* (#857).
         logger: Optional CommandLogger for structured output
 
     Returns:
@@ -91,7 +94,7 @@ def _copy_local_package(dep_ref, install_path, project_root, logger=None):
 
     local = Path(dep_ref.local_path).expanduser()
     if not local.is_absolute():
-        local = (project_root / local).resolve()
+        local = (base_dir / local).resolve()
     else:
         local = local.resolve()
 

src/apm_cli/install/phases/resolve.py

@@ -119,14 +119,21 @@ def run(ctx: "InstallContext") -> None:
     logger = ctx.logger
     verbose = ctx.verbose
 
-    def download_callback(dep_ref, modules_dir, parent_chain=""):
+    def download_callback(dep_ref, modules_dir, parent_chain="", parent_pkg=None):
         """Download a package during dependency resolution.
 
         Args:
             dep_ref: The dependency to download.
             modules_dir: Target apm_modules directory.
             parent_chain: Human-readable breadcrumb (e.g. "root > mid")
                 showing which dependency path led to this transitive dep.
+            parent_pkg: The APMPackage that declared *dep_ref* (None for
+                direct deps from the root project). For local deps, the
+                parent's ``source_path`` becomes the base directory for
+                resolving the relative path -- so a transitive dep like
+                ``../sibling`` declared inside a package nested several
+                directories deep resolves against the package's own
+                location, not the root consumer (#857).
         """
         install_path = dep_ref.get_install_path(modules_dir)
         if install_path.exists():
@@ -140,8 +147,17 @@ def download_callback(dep_ref, modules_dir, parent_chain=""):
                     # so use .add() rather than dict-style assignment.
                     callback_failures.add(dep_ref.get_unique_key())
                     return None
+                # Anchor relative paths on the *declaring* package's source
+                # directory when available (#857). Falls back to
+                # ``project_root`` for direct deps and for parents that
+                # predate the source_path field.
+                base_dir = (
+                    parent_pkg.source_path
+                    if parent_pkg is not None and parent_pkg.source_path is not None
+                    else project_root
+                )
                 result_path = _copy_local_package(
-                    dep_ref, install_path, project_root, logger=logger
+                    dep_ref, install_path, base_dir, logger=logger
                 )
                 if result_path:
                     callback_downloaded[dep_ref.get_unique_key()] = None

src/apm_cli/models/apm_package.py

@@ -73,6 +73,12 @@ class APMPackage:
     dev_dependencies: Optional[Dict[str, List[Union[DependencyReference, str, dict]]]] = None
     scripts: Optional[Dict[str, str]] = None
     package_path: Optional[Path] = None  # Local path to package
+    # Absolute on-disk directory holding this package's apm.yml. Used to
+    # resolve relative ``local_path`` dependencies declared in this package's
+    # apm.yml (#857). For local deps this is the *original* source directory,
+    # not the apm_modules/_local/ copy. For remote deps and the root project
+    # this matches package_path.
+    source_path: Optional[Path] = None
     target: Optional[Union[str, List[str]]] = None  # Target agent(s): single string or list (applies to compile and install)
     type: Optional[PackageContentType] = None  # Package content type: instructions, skill, hybrid, or prompts
     includes: Optional[Union[str, List[str]]] = None  # Include-only manifest: 'auto' or list of repo paths

tests/test_apm_resolver.py

@@ -493,5 +493,120 @@ def test_dependency_graph_error_handling(self):
         assert not summary["is_valid"]
 
 
+class TestSourcePathField(unittest.TestCase):
+    """APMPackage.source_path field exists and is independent of package_path."""
+
+    def test_source_path_default_is_none(self):
+        pkg = APMPackage(name="x", version="1.0.0")
+        assert pkg.source_path is None
+
+    def test_source_path_can_be_set(self):
+        pkg = APMPackage(name="x", version="1.0.0", source_path=Path("/some/dir"))
+        assert pkg.source_path == Path("/some/dir")
+
+
+class TestComputeDepSourcePath(unittest.TestCase):
+    """``_compute_dep_source_path`` anchors local-relative deps correctly (#857)."""
+
+    def setUp(self):
+        self.resolver = APMDependencyResolver()
+        self.resolver._project_root = Path("/proj")
+
+    def _local_ref(self, local_path: str):
+        return DependencyReference.parse(local_path)
+
+    def test_remote_dep_returns_install_path(self):
+        ref = DependencyReference(repo_url="user/repo", is_local=False)
+        install = Path("/proj/apm_modules/user/repo")
+        result = self.resolver._compute_dep_source_path(ref, install, parent_pkg=None)
+        assert result == install.resolve()
+
+    def test_local_absolute_path_returns_resolved(self):
+        ref = self._local_ref("/abs/path/pkg")
+        install = Path("/proj/apm_modules/_local/pkg")
+        result = self.resolver._compute_dep_source_path(ref, install, parent_pkg=None)
+        assert result == Path("/abs/path/pkg").resolve()
+
+    def test_local_relative_path_uses_parent_source_path(self):
+        # The bug: ``../editorial-pipeline`` declared inside a transitive
+        # package must resolve against that package's directory, not the
+        # root consumer.
+        ref = self._local_ref("../editorial-pipeline")
+        install = Path("/proj/apm_modules/_local/editorial-pipeline")
+        parent = APMPackage(
+            name="handbook-agents",
+            version="1.0.0",
+            source_path=Path("/proj/packages/handbook-agents"),
+        )
+        result = self.resolver._compute_dep_source_path(ref, install, parent_pkg=parent)
+        assert result == Path("/proj/packages/editorial-pipeline").resolve()
+
+    def test_local_relative_path_falls_back_to_project_root_when_no_parent(self):
+        # Direct deps from root: parent_pkg is None, anchor is project_root.
+        ref = self._local_ref("./packages/foo")
+        install = Path("/proj/apm_modules/_local/foo")
+        result = self.resolver._compute_dep_source_path(ref, install, parent_pkg=None)
+        assert result == Path("/proj/packages/foo").resolve()
+
+    def test_local_relative_path_falls_back_to_project_root_when_parent_has_no_source_path(self):
+        # Backwards compat: a parent package created before source_path
+        # was added (still None) shouldn't break resolution -- fall back
+        # to the project root rather than crashing.
+        ref = self._local_ref("./packages/foo")
+        install = Path("/proj/apm_modules/_local/foo")
+        legacy_parent = APMPackage(name="legacy", version="1.0.0")
+        result = self.resolver._compute_dep_source_path(
+            ref, install, parent_pkg=legacy_parent
+        )
+        assert result == Path("/proj/packages/foo").resolve()
+
+
+class TestResolverSetsRootSourcePath(unittest.TestCase):
+    """``resolve_dependencies`` populates ``source_path`` on the root package."""
+
+    def test_root_package_has_source_path_after_resolve(self):
+        with TemporaryDirectory() as temp_dir:
+            project_root = Path(temp_dir)
+            (project_root / "apm.yml").write_text(
+                "name: root-pkg\nversion: 1.0.0\n"
+            )
+            graph = APMDependencyResolver().resolve_dependencies(project_root)
+            assert graph.root_package.source_path == project_root.resolve()
+
+
+class TestDownloadCallbackReceivesParent(unittest.TestCase):
+    """The download callback is invoked with ``parent_pkg`` for transitive deps."""
+
+    def test_callback_called_with_parent_package(self):
+        with TemporaryDirectory() as temp_dir:
+            project_root = Path(temp_dir)
+            apm_modules = project_root / "apm_modules"
+            apm_modules.mkdir()
+            (project_root / "apm.yml").write_text(
+                "name: root-pkg\nversion: 1.0.0\n"
+                "dependencies:\n  apm:\n    - user/dep1\n"
+            )
+
+            received_calls = []
+
+            def fake_download(dep_ref, modules_dir, parent_chain="", parent_pkg=None):
+                received_calls.append((dep_ref.get_unique_key(), parent_pkg))
+                return None  # Simulate download miss; we only care about args
+
+            resolver = APMDependencyResolver(
+                apm_modules_dir=apm_modules,
+                download_callback=fake_download,
+            )
+            resolver.resolve_dependencies(project_root)
+
+            # The first (and only) download attempt is for the direct dep.
+            # parent_pkg is None for direct deps -- the assert below pins
+            # the contract that the callback receives the parameter (and
+            # would receive a real parent for any transitive deps).
+            assert received_calls, "download_callback was never invoked"
+            _, parent_pkg = received_calls[0]
+            assert parent_pkg is None  # direct dep from root
+
+
 if __name__ == '__main__':
     unittest.main()