fix(deps): render logical local dep keys in deps list/tree; refresh stale v0.25.0 release tests

[danielmeppiel]

fix(deps): render logical local dep keys in deps list/tree; refresh stale v0.25.0 release tests

TL;DR

The v0.25.0 release integration suite (run 29184076146) went red with 13 failures across four independent root causes. Exactly one is a production bug: apm deps list and apm deps tree leak physical, parent-scoped install slots for transitive local dependencies instead of their logical lockfile identity. This PR fixes that display regression and regrounds three stale test fixtures that drifted behind intentional prior changes (#2058, #2155, and upstream awesome-copilot). No production behavior other than local-dep display changes; physical hashed storage, remote behavior, and lockfile keys are untouched.

Problem (WHY)

Thirteen failures, one production bug, three stale tests:

• Production regression (deps list/deps tree): for a local transitive chain, apm deps list printed _local/<12-char-hash>/pkg-depth-N and apm deps tree printed local:/abs/host/path/pkg-depth-N. Both leak the parent-scoped physical slot (introduced by refactor(core): consolidate red-team lifecycle fixes behind canonical owners #2155 to avoid sibling collisions) and, in the tree case, the absolute host filesystem path into user-facing output. The logical identity the user typed is ../pkg-depth-N (lockfile repo_url _local/pkg-depth-N).
• Stale policy test: test_emu_ssh_remote_routes_to_correct_org_policy_repo asserted the first policy candidate is contoso/.github, but Add .github-private as preferred policy repo candidate #2058 intentionally made .github-private first in the cascade.
• Stale pack fixture: two test_pack_apm_provenance_e2e cases built a lockfile shape no real install produces -- a skill directory in deployed_files with per-child hashes. refactor(core): consolidate red-team lifecycle fixes behind canonical owners #2155's deployment ledger rebuilds hashes only for paths present in deployed_files, so the fixture never exercised the intended path.
• Drifted plugin fixture: test_plugin_e2e installed github/awesome-copilot/plugins/context-engineering from default main, which upstream has since reduced to a thin stub whose manifest declares absent component paths. APM correctly fails closed ([BUG] Missing declared plugin components are silently ignored #2103/refactor(core): consolidate red-team lifecycle fixes behind canonical owners #2155); the assembled plugin now lives on the upstream marketplace branch.

The user-facing leak is the one that matters: the CLI's job is to show the logical graph the user declared, not internal storage. Grounding output in the declared identity keeps the surface honest, consistent with PROSE's "Grounding outputs in deterministic tool execution transforms probabilistic generation into verifiable action."

Approach (WHAT)

Root cause	Kind	Change
#4 deps list/tree leak	production	Map physical install slot -> logical lockfile key in _resolve_scope_deps; render local_path in _dep_display_name
#1 policy cascade	test	Assert .github-private first; update cascade comments
#2 pack ledger shape	test	List directory entry plus every contained file in deployed_files
#3 plugin ref	test	Point ref at ...context-engineering#marketplace; add ref-stripped path constant

Implementation (HOW)

src/apm_cli/commands/deps/cli.py (only production file). _resolve_scope_deps now builds a physical_to_logical map while reading the lockfile: for each source == "local" dep it records install_path.relative_to(apm_modules) -> dep.repo_url (logical). The scan loop resolves each scanned candidate's name through that map, so locked-membership, orphan detection, insecure lookup, and the reported name all key on the logical _local/pkg, never the hash slot. _dep_display_name renders a local dep's declared local_path (../pkg) instead of get_unique_key(), which for anchored transitive local deps is an absolute local:/... slot. Hashed physical storage, remote deps, and lockfile keys are unchanged.

tests/integration/test_dep_url_parsing_e2e.py -- assertion and cascade comments updated to .github-private -> .github -> .apm -> _apm. Production source untouched.

tests/integration/test_pack_apm_provenance_e2e.py -- both the tamper and directory-symlink-escape cases now set deployed_files=[skill_dir + "/", *file_rels], matching a real install. Fail-closed verification (verify_attested_file) is unchanged.

tests/integration/test_plugin_e2e.py -- PLUGIN_REF gains the #marketplace suffix; a new ref-stripped PLUGIN_PATH constant backs the five on-disk path assertions (the ref suffix is stripped from the install path by DependencyReference.get_install_path).

tests/integration/test_transitive_chain_e2e.py -- the post-prune assertion expected a flat _local/pkg/apm.yml for transitive deps; it now globs the hashed slot exactly as the sibling test_three_level_apm_chain_resolves_all_levels already does.

Diagram

Legend: how a scanned physical slot is resolved to the logical name that deps list reports, after this fix.

flowchart LR
    A["scan apm_modules/<br/>_local/&lt;hash&gt;/pkg-depth-2"] --> B{"source == local?<br/>(lockfile)"}
    B -->|yes| C["physical_to_logical<br/>&lt;hash&gt;/pkg-depth-2 → _local/pkg-depth-2"]
    B -->|no| D["canonical string<br/>owner/repo"]
    C --> E["report logical name<br/>_local/pkg-depth-2"]
    D --> E
    E --> F["orphan check · insecure lookup · display"]

Loading

Trade-offs

• Display-only fix, storage unchanged. I deliberately did not touch get_install_path's hashing. refactor(core): consolidate red-team lifecycle fixes behind canonical owners #2155 hashes physical slots to prevent sibling collisions; that is correct and the task scope preserves it. The logical/physical split is intentional -- the fix reconciles the view, not the storage.
• Scope crept from _resolve_scope_deps to _dep_display_name. The traced root cause named only deps list. Testing showed deps tree leaked the same class of value (absolute local:/...), so the one-line _dep_display_name change is included to make the tree logical too. Both live in the same file and the same conceptual fix.
• Test-only corrections for Why do we need a GitHub token? #1/Integrate copilot runtime #2 and the extra prune assertion in Add ARM64 Linux support to CI/CD pipeline #4. These reground expectations behind prior intentional changes; production source is not weakened (policy cascade, fail-closed pack verification, and component validation all preserved).

Benefits

1. apm deps list and apm deps tree show the logical dependency the user declared (_local/pkg, ../pkg) for local chains of any depth.
2. No host absolute path (local:/Users/...) leaks into CLI output.
3. The release integration suite's four root causes are resolved without relaxing any security or validation boundary.
4. Pack and plugin fixtures now exercise production-realistic shapes, so they will catch real regressions instead of drifting.

Validation

Note

Local main in this worktree is stale; the branch is exactly one commit on top of origin/main (3880677f, the v0.25.0 release commit). The PR diff is 5 files.

Focused failing tests (the four root causes) -- all green

$ APM_RUN_INTEGRATION_TESTS=1 APM_E2E_TESTS=1 pytest \
    test_dep_url_parsing_e2e.py::...::test_emu_ssh_remote_routes_to_correct_org_policy_repo \
    test_pack_apm_provenance_e2e.py::test_apm_pack_rejects_tampered_file_in_deployed_directory \
    test_pack_apm_provenance_e2e.py::test_apm_pack_directory_symlink_does_not_escape \
    test_transitive_chain_e2e.py::test_deps_commands_follow_full_lock_graph_and_ignore_embedded_manifests
4 passed

Full relevant test files + broader deps coverage

$ pytest test_dep_url_parsing_e2e.py test_pack_apm_provenance_e2e.py test_transitive_chain_e2e.py
23 passed

$ pytest tests/unit/deps tests/unit/commands tests/unit/test_deps*.py \
    tests/unit/test_local_deps.py tests/unit/test_transitive_deps.py tests/unit/test_prune_command.py ...
2510 passed

$ APM_RUN_INTEGRATION_TESTS=1 APM_E2E_TESTS=1 pytest \
    test_wave6_deps_cli_coverage.py test_apm_dependencies.py
89 passed, 15 skipped

Plugin ref -- real network install from #marketplace (token available)

$ GITHUB_APM_PAT=*** APM_RUN_INTEGRATION_TESTS=1 pytest \
    test_plugin_e2e.py::TestPluginNetworkE2E::test_install_real_plugin \
    test_plugin_e2e.py::TestPluginNetworkE2E::test_deps_tree_shows_plugin
2 passed

Branch/tree verified live before editing: marketplace exists (4ce0b479), plugins/context-engineering contains agents/ and skills/.

Lint mirror (canonical contract) -- all green

$ ruff check src/ tests/                                  # All checks passed!
$ ruff format --check src/ tests/                         # 1468 files already formatted
$ 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                       # [+] auth-signal lint clean
# relative_to guard: clean (uses .as_posix(), not str())
# file-length guard: deps/cli.py = 1060 lines (< 2100)
# yaml-io guard: no yaml.dump in changed file

Scenario Evidence

User-promise scenario	Proving test	APM principle
deps list shows the logical graph for a local transitive chain	test_transitive_chain_e2e.py::test_deps_commands_follow_full_lock_graph_and_ignore_embedded_manifests (list assertion)	Honest, legible operating surface
deps tree shows declared refs, no host path leak	same test (tree assertion)	Honest, legible operating surface
Transitive local deps materialise in their parent-scoped slot after prune	same test (glob assertion)	Collision-safe storage (#2155)
Policy discovery tries .github-private first	test_emu_ssh_remote_routes_to_correct_org_policy_repo	Org policy precedence (#2058)
Pack rejects a tampered file in a deployed directory	test_apm_pack_rejects_tampered_file_in_deployed_directory	Fail-closed provenance
Plugin install fails closed on absent components; succeeds on assembled ref	TestPluginNetworkE2E::test_install_real_plugin	Component validation (#2103/#2155)

How to test

• Build a local ../pkg-a -> ../pkg-b -> ../pkg-c chain and apm install ../pkg-a.
• Run apm deps list; confirm names read _local/pkg-b, _local/pkg-c (no hash prefix).
• Run apm deps tree; confirm rows read ../pkg-b, ../pkg-c (no local:/abs/path).
• Run the four focused tests above with APM_RUN_INTEGRATION_TESTS=1 APM_E2E_TESTS=1.
• Run the lint mirror; confirm all checks green.

Panel review follow-up (commit 7733c8ac3)

Folded four in-scope panel findings on this branch:

1. No hash leak in warnings - _resolve_scope_deps scan-loop exception
   warning emits the logical name (resolved before the try), so a package
   read failure cannot surface _local/<hash>/pkg.
2. Typed, safe display - _dep_display_name(dep: LockedDependency) drops
   getattr for direct attribute access; local deps prefer local_path, else
   the logical repo_url, never the absolute local:/ unique-key slot.
3. Focused unit coverage - test_deps_cli_helpers.py: local_path present
   renders the relative path; local_path absent falls back to repo_url. Both
   assert local:/ never appears and get_unique_key() is not consulted.
4. Negative integration guards - bounded-regex assertions that neither
   _local/<12-hex>/ nor local:/ appears in deps list/deps tree output.

$ pytest tests/unit/commands/test_deps_cli_helpers.py            # 18 passed
$ APM_RUN_INTEGRATION_TESTS=1 APM_E2E_TESTS=1 \
    pytest tests/integration/test_transitive_chain_e2e.py        # 4 passed
$ pytest tests/unit/commands/ -k deps                            # 166 passed
# lint mirror: ruff check + format clean, pylint R0801 10.00/10, auth-signals clean

The repeated network-E2E setup redesign is intentionally deferred; tracked
separately after the release.

Pre-merge review round 2 (commit b668d6141)

Three reachable residual leaks in deps list, each fixed TDD (RED first):

1. Absolute local_path - _dep_display_name rendered any truthy
   local_path, leaking /Users/..., ~/..., C:\.... New display-only
   _is_absolute_local_path (PurePosixPath + PureWindowsPath + ~) keeps
   relative declared paths, falls back to logical repo_url for absolute.
2. True orphan physical slot - a _local/<12hex>/pkg left after its
   lockfile entry is removed has no mapping. Orphan detection keeps the raw
   physical identity; new _logical_local_display strips the hash for the
   displayed name (_local/pkg).
3. Read-failure exception - the malformed-YAML ValueError embeds the
   absolute apm.yml path. The scan-loop warning no longer forwards the
   exception; it emits a stable public message keyed on the hash-free name.

Integration negative guard strengthened to reject any host-absolute path
(workspace root + bounded Windows-drive regex), not just local:/.

$ pytest tests/unit/commands/test_deps_cli_helpers.py            # 23 passed (5 new)
$ APM_RUN_INTEGRATION_TESTS=1 APM_E2E_TESTS=1 \
    pytest test_transitive_chain_e2e.py test_dep_url_parsing_e2e.py \
           test_pack_apm_provenance_e2e.py                        # 46 passed
$ pytest tests/unit/ -k "deps or lockfile or display"            # 2418 passed
$ pytest tests/integration/ -k "deps_list or deps_tree or transitive or local_install"
                                                                  # 93 passed, 4 skipped
# lint mirror: ruff check + format clean, pylint R0801 10.00/10, auth-signals clean

Remote behavior and physical hashed storage unchanged.

Important

Remote CI has not yet run on this branch; the "green" claims above are the local lint/test mirror only. Do not treat CI as green until the PR checks complete. Merge, tag moves, and release re-trigger are intentionally left to the maintainer.

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

[Copilot]

Pull request overview

Fixes a production UX regression where apm deps list / apm deps tree could surface physical on-disk local dependency slots (including hashed parent-scoped slots and, in the tree case, absolute local:/... paths) instead of the logical dependency identity the user declared / the lockfile represents. Also refreshes several integration tests that drifted behind intentional earlier behavior changes.

Changes:

• Map scanned local dependency install paths back to a logical lockfile identity in _resolve_scope_deps, so deps list/orphan detection keys on the logical name instead of the hashed slot.
• Render local dependencies in deps tree using their declared local_path (e.g. ../pkg) to avoid leaking host absolute paths.
• Update integration tests/fixtures to match current policy cascade behavior, pack provenance shapes, plugin ref selection, and hashed-slot materialization.

Show a summary per file

File	Description
src/apm_cli/commands/deps/cli.py	Adjusts deps list/tree naming for local dependencies by mapping physical slots to logical identities and displaying declared local paths.
tests/integration/test_dep_url_parsing_e2e.py	Updates policy discovery expectation to .github-private first in the cascade.
tests/integration/test_pack_apm_provenance_e2e.py	Regrounds pack provenance fixtures to include directory + contained files in deployed_files.
tests/integration/test_plugin_e2e.py	Pins the upstream plugin install to #marketplace and fixes on-disk path assertions accordingly.
tests/integration/test_transitive_chain_e2e.py	Updates post-prune assertions to locate transitive local deps in hashed parent-scoped slots via globbing.

Review details

• Files reviewed: 5/5 changed files
• Comments generated: 1
• Review effort level: Low

[danielmeppiel]

APM Review Panel: ship_with_followups

Fixes the local dependency identity leak in deps list and deps tree, restoring logical package names over physical hash slots, and regrounds stale v0.25.0 release tests.

cc @sergio-sisternes-epam -- a fresh advisory pass is ready for your review.

All active panelists converge: the production display fix is correct, scoped, and architecturally sound. The physical-to-logical mapping is internally consistent, uses constant-time lookup, and leaves collision-safe storage, lockfile identity, and fail-closed verification unchanged. The integration evidence is load-bearing and green.

Three bounded completions remain in the same defect class: the read-failure warning still uses the physical scan name, the local tree fallback can expose local:<absolute-path> for old lock entries, and the new display branch lacks focused unit coverage. The repeated large real-network plugin installs are a valid CI-cost concern, but they predate this fix and should be handled separately.

Dissent. None. The only variance is whether the three path-leak completions land now or as a fast follow; the CEO recommends folding them now because they are tightly bounded.

Aligned with: Portable by manifest: local deps render their declared identity, not a host-specific slot. Secure by default: hashed storage and provenance checks are preserved. Pragmatic as npm: users see the package they declared, not an implementation detail.

Growth signal. This is a trust-improving fix worth one future patch CHANGELOG line, not a standalone campaign.

Panel summary

Persona	B	R	N	Takeaway
Python Architect	0	0	2	The mapping is internally consistent and correctly scoped.
CLI Logging Expert	0	0	1	Logical identity is restored, but one warning still uses the physical slot.
DevX UX Expert	0	0	2	The mental model is restored; harden the old-lockfile fallback.
Supply Chain Security Expert	0	0	0	Hashed storage, lock identity, and fail-closed verification remain intact.
OSS Growth Hacker	0	0	2	Trust improves without requiring a conversion-surface change.
Test Coverage Expert	0	1	1	Integration coverage is strong; add focused branch and negative guards.
Performance Expert	0	1	2	Production complexity is unchanged; repeated network E2Es remain costly.

B = blocking-severity findings, R = recommended, N = nits.
Counts are signal strength, not gates. The maintainer ships.

Top 5 follow-ups

1. [Test Coverage Expert] Add unit tests for local display with and without local_path -- this cheaply guards the fallback that prevents path leakage.
2. [CLI Logging Expert] Use logical_name in the read-failure warning -- otherwise a failure can still expose _local/<hash>/pkg.
3. [DevX UX Expert] Use local_path or repo_url before get_unique_key() for local deps -- this prevents old lock entries from exposing local:<absolute-path>.
4. [Test Coverage Expert] Assert that hash slots and absolute local identities are absent from CLI output -- positive assertions alone would permit duplicate physical output.
5. [Performance Expert] Reduce repeated installs in the real-network plugin class -- retain one real lifecycle signal while addressing the pre-existing timeout risk separately.

Architecture

classDiagram
  class LockedDependency {
    +repo_url str
    +source str
    +local_path str
    +get_unique_key() str
    +to_dependency_ref() DependencyReference
  }
  class DependencyReference {
    +get_install_path(Path) Path
  }
  class LockFile {
    +dependencies dict
  }
  class DepsInventory {
    +physical_to_logical dict
    +resolve_scope_deps() list
    +display_name() str
  }
  LockFile *-- LockedDependency
  LockedDependency ..> DependencyReference
  DepsInventory ..> LockFile
  DepsInventory ..> DependencyReference

Loading

flowchart TD
  CLI["apm deps list/tree"] --> LF["Read lockfile"]
  LF --> MAP["Map physical local slot to logical repo_url"]
  MAP --> SCAN["Scan apm_modules"]
  SCAN --> VIEW["Render logical package identity"]
  LF --> TREE["Render declared local_path in tree"]

Loading

Recommendation

The release recovery is sound. Fold the three bounded path-leak and test completions, let CI certify the updated head, then merge. Track the heavyweight network-test redesign separately.

---

Full per-persona findings

Python Architect

• [nit] _dep_display_name uses defensive getattr on a typed domain object at src/apm_cli/commands/deps/cli.py:72.
  A LockedDependency type hint would improve static clarity, but runtime behavior is correct.
• [nit] No pattern extraction is warranted.
  The one-shot translation dictionary is the simplest correct design.

CLI Logging Expert

• [nit] Read-failure warning still interpolates the physical scan name at src/apm_cli/commands/deps/cli.py:317.
  Use logical_name so failures do not reintroduce the hash-slot leak.

DevX UX Expert

• [nit] deps list and deps tree use different logical representations.
  List shows _local/pkg, while tree shows the declared ../pkg; both are valid and better than physical identities.
• [nit] Local tree fallback can expose a raw unique key at src/apm_cli/commands/deps/cli.py:69.
  Prefer local_path or repo_url before get_unique_key().

Supply Chain Security Expert

No findings.

OSS Growth Hacker

• [nit] The identity-leak fix is too narrow for standalone launch communication.
• [nit] The PR body models strong release-recovery practice for future contributor guidance.

Auth Expert -- inactive

The touched dependency-display and integration-test files do not change authentication or credential flows.

Doc Writer -- inactive

Existing documentation uses remote dependency examples and does not assert the old physical local-path output.

Test Coverage Expert

• [recommended] No unit test directly exercises the new local branch in _dep_display_name at tests/unit/commands/test_deps_cli_helpers.py.
  Proof (missing at): tests/unit/commands/test_deps_cli_helpers.py::TestDepDisplayName::test_local_source_renders_local_path -- proves: tree output uses the declared relative path instead of a host path.
• [nit] The E2E has positive logical-name assertions but no explicit negative hash-slot assertion at tests/integration/test_transitive_chain_e2e.py.
  Proof (passed): tests/integration/test_transitive_chain_e2e.py::test_deps_commands_follow_full_lock_graph_and_ignore_embedded_manifests -- focused test passed in 3.34s.

Performance Expert

• [nit] The new dictionary is populated during the existing lockfile pass and adds constant-time lookup only.
• [nit] Two defensive attribute reads per displayed dependency are negligible.
• [recommended] Repeated installs from the assembled marketplace branch can make the full network class slow at tests/integration/test_plugin_e2e.py:601.
  Preserve the real-network signal but reduce redundant setup in a separate change.

_{This panel is advisory. It does not block merge. Re-apply the
panel-review label after addressing feedback to re-run.}

[danielmeppiel]

Docs sync advisory

Verdict: no_change * Pages affected: 0 * LLM calls: 1/15

No documentation impact detected at head b668d61. The synchronized diff only hardens path sanitization and regression coverage; it changes no documented command, flag, schema, or workflow.