fix(install): anchor transitive local_path deps on declaring package (#857)

[danielmeppiel]

TL;DR

Re-implementation of #940 (closes #857). apm install now anchors a package's local_path references on that package's own directory — npm/pip/cargo parity for transitive locals — and rejects any remote-cloned package that declares a local_path dep. Supersedes #940; original investigation, fix architecture, and tests preserved via Co-authored-by trailer to @JahanzaibTayyab. All seven REQUIRED panel findings from #940 (review) plus three follow-up panel nits (python-architect, supply-chain, test-coverage) addressed in a single commit.

Problem (WHY)

• A transitive ../sibling declared inside packages/specialized/apm.yml was anchored on the consumer's project root, not on packages/specialized/. The same apm.yml worked or broke depending on where the consumer placed it in their tree — making shared workspace layouts (mono-repos, side-by-side helper packages) impossible to express portably.
• Every other package manager (npm, pip, cargo) anchors on the declaring package. APM violated reasonable user expectation here.
• The APMPackage._cache was keyed on apm_yml_path alone, so two checkouts of the same apm.yml reached from different anchors collided on the first one cached.
• Remote packages could declare a local-filesystem local_path dependency, opening a prompt-injection / pseudo-supply-chain surface (a downloaded package can ask you to read arbitrary local files into your install).

Approach (WHAT)

Decision	Choice	Rejected alternative
Anchor for transitive deps	Declaring package's source_path (npm/pip/cargo parity)	Consumer project root (broken status quo)
Cross-phase contract	New ctx.dep_base_dirs: dict[dep_key, Path] populated by resolve, consumed by integrate	Re-walking the dependency tree from integrate (couples phases tighter)
Cache-key correctness	Tuple (apm_yml_path, source_path)	Mutating cached entries (poisons future lookups)
Remote-parent local_path	Dual-reject ABSOLUTE + RELATIVE at resolve time, ERROR severity, fail-closed	Warn + allow (preserves attack surface)
Containment of legitimate sibling layouts	Allow — boundary lives upstream in resolver dual-reject	Strict ensure_path_within(project_root) (caught by CI: would have broken apm install ../pkg-a workflows)

Implementation (HOW)

File	What changed
models/apm_package.py	Tuple cache key (Path, Path | None); new source_path field; from_apm_yml(apm_yml_path, source_path=None).
install/phases/resolve.py	download_callback gains parent_pkg=None; populates ctx.dep_base_dirs by walking dependency_tree.nodes; narrow (AttributeError, KeyError) fallback to empty map (fail-safe to legacy anchoring). Root from_apm_yml threads project_root as source_path.
install/sources.py	LocalDependencySource.acquire() reads ctx.dep_base_dirs for base_dir (defensive getattr for legacy ctx). _materialize_local reuses base_dir (NOT ctx.project_root) when stamping source_path — fixes architect-flagged latent cache-key bug.
install/phases/local_content.py	_copy_local_package(dep_ref, install_path, base_dir, *, project_root, logger). base_dir is the anchor, project_root is retained on signature for symmetry/future strict-mode hook. Required logger (was silent fallback masking transitive logger-threading bugs).
deps/apm_resolver.py	_signature_accepts_parent_pkg (False fallback for callbacks that don't accept the kwarg yet); _is_remote_parent heuristic (excludes _local/ prefix); dual-reject ABSOLUTE+RELATIVE local from remote-parent at ERROR; consistent error messages recommending "publish as standalone package"; _logger.debug replaces bare except: pass.
CHANGELOG.md, docs/.../dependencies.md, packages/apm-guide/.apm/skills/apm-usage/dependencies.md	Migration guidance + Before/After YAML for the anchor rule.

Diagrams

Sequence (execution flow). How dep_base_dirs bridges resolve → integrate so the integrate phase knows which anchor to use per local dep. New behaviour is highlighted in soft-yellow blocks.

sequenceDiagram
    participant U as User
    participant CLI as apm install
    participant R as ResolvePhase
    participant T as DependencyTree
    participant Ctx as InstallContext
    participant I as IntegratePhase
    participant LDS as LocalDependencySource
    participant CP as _copy_local_package
    participant APk as APMPackage

    U->>CLI: apm install
    CLI->>R: run(deps_to_install)
    R->>T: build & walk
    rect rgb(255, 247, 200)
        Note over T,Ctx: NEW: anchor map populated
        T-->>R: nodes (parent->child)
        R->>R: for node: anchor = parent.source_path or project_root
        R->>Ctx: ctx.dep_base_dirs[dep_key] = anchor
    end
    CLI->>I: run(deps_to_install)
    loop per local dep
        I->>LDS: acquire(dep_ref, ctx)
        rect rgb(255, 247, 200)
            Note over LDS: NEW: anchor lookup
            LDS->>Ctx: dep_base_dirs.get(dep_key) or project_root
            Ctx-->>LDS: base_dir
        end
        LDS->>CP: (dep_ref, install_path, base_dir, project_root)
        CP-->>LDS: install_path
        rect rgb(255, 247, 200)
            Note over LDS,APk: NEW: source_path stamped on package
            LDS->>APk: from_apm_yml(yml, source_path=base_dir/local)
            APk-->>LDS: APMPackage(source_path=...)
        end
    end

Loading

Class (data model). Modules touched by this PR and the data they exchange. Notes call out the three correctness invariants the reviewers should verify hold post-merge.

classDiagram
    class APMPackage {
        +str name
        +str version
        +Path package_path
        +Path source_path
        +from_apm_yml(yml, source_path) APMPackage
    }
    class APMPackageCache {
        -dict~tuple~ _cache
        +get(yml_path, source_path) APMPackage
    }
    class DependencyReference {
        +str source
        +str local_path
        +bool is_local
        +get_unique_key() str
    }
    class InstallContext {
        +Path project_root
        +dict dep_base_dirs
    }
    class DownloadCallback {
        <<Protocol>>
        +__call__(dep_ref, target, parent_pkg) bool
    }
    class APMDependencyResolver {
        -_signature_accepts_parent_pkg(cb) bool
        -_is_remote_parent(parent_pkg) bool
        -_try_load_dependency_package(dep_ref, parent_pkg) APMPackage
    }
    class LocalDependencySource {
        +acquire(dep_ref, ctx) ResolvedDep
        -_materialize_local(...) APMPackage
    }
    class CopyLocalPackage {
        +_copy_local_package(dep_ref, install_path, base_dir, project_root, logger) Path
    }

    APMPackage ..> APMPackageCache : keyed by tuple
    APMDependencyResolver ..> DownloadCallback : invokes
    APMDependencyResolver ..> APMPackage : threads source_path
    LocalDependencySource ..> InstallContext : reads dep_base_dirs
    LocalDependencySource ..> CopyLocalPackage : delegates
    LocalDependencySource ..> APMPackage : stamps source_path
    CopyLocalPackage ..> InstallContext : reads project_root

    note for APMPackage "Cache key tuple (yml_path, source_path) prevents collision across anchors"
    note for InstallContext "dep_base_dirs is the cross-phase bridge from resolve to integrate"
    note for APMDependencyResolver "Dual-rejects local_path declared by remote parent (ERROR, fail-closed)"

Loading

Trade-offs

• Soft dep_base_dirs vs typed field on InstallContext. Used getattr(ctx, "dep_base_dirs", {}) for forward-compat with any legacy callers constructing InstallContext directly. If more phase-bridged maps accumulate, promote to a typed dataclass field (architect observation apm install <my-apm-package-repo> #5 — accepted as follow-up).
• _is_remote_parent heuristic is coupled to the _local/ prefix convention. Defense-in-depth only — the security boundary is the upstream dual-reject; misclassification still fails closed because remote-parent local_paths are blocked at resolve. Documented in the SECURITY note on the helper.
• No unconditional ensure_path_within(project_root) in _copy_local_package. First v2 draft enforced it; existing 2 sibling-layout e2e tests in test_transitive_chain_e2e.py failed against it (legitimate apm install ../pkg-a was blocked). The actual untrusted-source threat (remote-parent local_path) is already blocked upstream in apm_resolver._try_load_dependency_package, so the redundant check was overreach. Signature retained for a future opt-in strict mode hook.
• Backward-compat shim _signature_accepts_parent_pkg in resolver. Detects callbacks that don't yet accept the new parent_pkg=None kwarg and falls back. Costs one introspection call per dep; alternative was a hard breaking change to every callback site, which would have widened the blast radius.

Benefits

1. npm/pip/cargo parity for transitive local_path deps — the same workspace layout works regardless of consumer location.
2. Cache correctness — two checkouts of the same apm.yml from different anchors no longer collide.
3. Closed prompt-injection surface — remote-cloned packages cannot pull arbitrary local files into the consumer's install via local_path.
4. Existing sibling/monorepo workflows preserved — apm install ../pkg-a continues to work; no migration needed for consumers.
5. Logger threading bugs surfaced — required logger argument removes a class of silent-progress-output regressions in transitive call paths.

Validation

Note

All gates run on commit d57cc291 in the worktree (not in CI yet — CI will run on push).

• uv run --extra dev ruff check src/ tests/ — silent
• uv run --extra dev ruff format --check src/ tests/ — silent
• uv run --extra dev pytest tests/unit tests/test_console.py tests/test_apm_resolver.py — 7239 passed, 1 warning, 30 subtests passed in 69.47s
• pytest tests/integration/test_transitive_chain_e2e.py — 3/3 pass, including the new test_asymmetric_layout_anchors_on_declaring_pkg (asymmetric layout that only passes with the fix; the existing symmetric-sibling test is layout-degenerate and would pass either way)
• End-to-end smoke repro at /tmp/apm940-repro/ — both ./packages/specialized and transitive ../base install correctly, integrating into .agents/skills/.

Smoke repro output

[>] Installing dependencies from apm.yml...
  [+] ./packages/specialized (local)
  |-- 1 skill(s) integrated -> .agents/skills/
  [+] ../base (local)
  |-- 1 skill(s) integrated -> .agents/skills/

[*] Installed 2 APM dependencies.

Scenario evidence

User-promise scenario (APM principle)	Test that proves it
Transitive ../sibling resolves against the declaring package, not the consumer (npm/pip/cargo parity — deterministic install)	tests/integration/test_transitive_chain_e2e.py::test_asymmetric_layout_anchors_on_declaring_pkg
Resolve phase populates the cross-phase anchor map for every transitive node (phase contract)	tests/unit/test_local_deps.py::TestDepBaseDirsCrossPhase::test_dep_base_dirs_anchors_transitive_on_parent_source_path
APMPackage._cache distinguishes same-apm.yml loaded from different anchors (cache correctness)	tests/unit/test_local_deps.py::TestSourcePathField (3 cases)
Remote-parent declaring local_path is rejected at resolve, both relative and absolute (supply-chain boundary)	tests/test_apm_resolver.py::TestIsRemoteParentHeuristic (3 cases)
Callbacks that don't yet accept parent_pkg= keep working (backward compat)	tests/test_apm_resolver.py::TestSignatureFallback (3 cases)
Sibling layout outside project_root (../pkg-a) still installs (legitimate workflow preserved)	tests/unit/test_local_deps.py::TestCopyLocalPackageContainmentBoundary::test_allows_sibling_outside_project_root + e2e symmetric chain

Local panel review

Panelist	Verdict	Action
python-architect	needs_minor (1 bug, 2 nits)	Bug #1 fixed (sources.py:190 ctx.project_root→base_dir for source_path stamping); narrowed except Exception→(AttributeError, KeyError); error-message symmetry.
supply-chain-security	ship (1 medium nit)	Contradictory error hint fixed — both branches now consistently recommend "publish as standalone package".
test-coverage	needs_more_tests (5 gaps)	2 highest-signal gaps closed (asymmetric e2e + cross-phase contract). 3 lower-signal text-pin tests deferred — non-blocking.

How to test

• Pull this branch: gh pr checkout 1111
• Build a 3-package asymmetric repro: consumer at /tmp/foo/consumer/, declaring ./packages/specialized; specialized/apm.yml declaring ../base; base/ as a sibling of specialized/ under consumer/packages/. Run apm install from consumer/ — both packages should appear under apm_modules/_local/.
• Try a sibling layout: consumer/apm.yml declaring ../pkg-a from outside the consumer dir; should still install (legitimate workflow preserved).
• Construct an apm.yml from a remote-cloned package that declares local_path: ../something — expect [x] ERROR with hint about publishing.
• Run uv run --extra dev pytest tests/integration/test_transitive_chain_e2e.py -v — all 3 e2e tests pass.

---

Closes #857. Supersedes #940.

Co-authored-by: Jahanzaib Tayyab noreply@github.com
Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com

[Copilot]

Pull request overview

This PR fixes how apm install resolves transitive local_path dependencies by anchoring relative paths to the declaring package's directory (instead of the consumer project root), and adds a security rule intended to prevent remote-cloned packages from declaring local_path dependencies.

Changes:

• Add APMPackage.source_path and include it in the apm.yml parse cache key to avoid cross-anchor cache collisions.
• Thread parent-package context through dependency resolution and use it to compute per-dependency base dirs (ctx.dep_base_dirs) for correct transitive local anchoring.
• Add docs/tests + update local package copy plumbing to take an explicit base_dir for relative-path resolution.

Show a summary per file

File	Description
src/apm_cli/models/apm_package.py	Adds source_path and updates from_apm_yml cache keying to include it.
src/apm_cli/deps/apm_resolver.py	Threads parent_pkg through callbacks; computes source_path; attempts to reject remote-parent local_path.
src/apm_cli/install/phases/resolve.py	Updates download callback signature and builds ctx.dep_base_dirs from the dependency tree.
src/apm_cli/install/phases/local_content.py	Changes _copy_local_package to resolve relative paths against base_dir (declaring package anchor).
src/apm_cli/install/sources.py	Uses ctx.dep_base_dirs when copying locals and stamps source_path on loaded packages.
tests/unit/test_local_deps.py	Updates existing local-copy tests and adds regression tests around anchoring/caching.
tests/unit/test_install_command.py	Updates callback signature in a unit test to accept parent_pkg.
tests/test_apm_resolver.py	Adds unit coverage for _is_remote_parent and callback signature fallback.
tests/integration/test_transitive_chain_e2e.py	Adds an asymmetric-layout e2e regression test validating the new anchor rule.
packages/apm-guide/.apm/skills/apm-usage/dependencies.md	Documents the new anchor behavior (but currently claims containment).
docs/src/content/docs/guides/dependencies.md	Documents the new anchor behavior (but currently claims containment).
CHANGELOG.md	Adds an Unreleased entry describing the fix and security tightening.

Copilot's findings

• Files reviewed: 12/12 changed files
• Comments generated: 6