fix(deps): preserve transitive dependencies in marketplace and remote subdir installs (closes #1666)

[sergio-sisternes-epam]

TL;DR

Plugin normalisation was silently overwriting apm.yml with identity-only fields, stripping all dependencies from dual-format packages. This broke transitive resolution for marketplace and remote subdirectory installs.

Problem (WHY)

When installing a package that has both plugin.json (or .claude-plugin/) and apm.yml, the package type detection cascade classifies it as MARKETPLACE_PLUGIN and triggers synthesize_apm_yml_from_plugin(). This function unconditionally overwrites the existing apm.yml with a synthesized version containing only identity fields (name, version, description, author, license, type), discarding:

• dependencies.apm and dependencies.mcp -- breaking transitive resolution
• devDependencies -- breaking dev install flows
• registries, targets, includes, scripts -- losing resolution-critical metadata

This manifests as two user-facing symptoms reported in #1666:

1. Marketplace install says "no transitive" and the lockfile only contains the direct dep
2. Remote subdir install stops at depth 1 for git: parent monorepo sibling chains

Both are the same root cause: the resolver reads the stripped manifest, finds no sub-dependencies, and stops.

Approach (WHAT)

Make plugin normalisation non-destructive for dual-format packages:

1. Before overwriting, load the existing apm.yml via load_yaml()
2. Generate plugin-derived fields (identity, MCP/LSP deps from mcpServers)
3. Merge existing resolution-critical blocks into the synthesized manifest
4. Union dependency lists without duplicates

Plugin identity fields override (name, version, etc.). Dependency lists are unioned. Resolution-critical blocks (devDependencies, registries, targets, includes, scripts) are preserved from the original manifest.

Implementation (HOW)

src/apm_cli/deps/plugin_parser.py

• synthesize_apm_yml_from_plugin() -- reads existing apm.yml before overwriting and passes it to _generate_apm_yml() as existing_manifest
• _generate_apm_yml() -- new existing_manifest parameter; starts from existing deps as base, then layers plugin-derived deps via _union_dep_list()
• _union_dep_list() -- new helper that appends entries to a dep list without duplicates, handling both string and dict entries (e.g. {git: parent, path: ...})

tests/unit/test_plugin_parser.py

8 new regression tests in TestSynthesizePreservesExistingManifest:

1. Preserves existing dependencies.apm and dependencies.mcp
2. Synthesised manifest is loadable by APMPackage.from_apm_yml() with non-empty deps
3. Merges plugin MCP deps with existing apm.yml deps (union, no duplicates)
4. No-existing-apm.yml case unchanged (no regression)
5. Preserves devDependencies
6. Preserves registries, targets, scripts, includes
7. .claude-plugin/ directory without plugin.json preserves deps
8. Full validate_apm_package() chain on dual-format package preserves deps

Trade-offs

• Merge semantics: existing apm.yml entries take priority in the union. Plugin-derived deps that duplicate existing entries are skipped. This is the safe default -- the apm.yml is the author's intent.
• Dict-format plugin deps: plugin.json dependencies as a dict ({"name": "^version"}) is preserved as-is under dependencies.apm (matching pre-existing behaviour), while list-format deps go through the union path.

Validation evidence

Check	Result
ruff check src/ tests/	Clean
ruff format --check src/ tests/	Clean
pylint R0801 (duplication)	10.00/10
lint-auth-signals.sh	Clean
New tests (8)	All pass
Full unit suite	16,638 passed, 2 skipped

How to test

# Run the new regression tests
uv run --extra dev pytest tests/unit/test_plugin_parser.py::TestSynthesizePreservesExistingManifest -v

# Run the full plugin parser suite (84 tests)
uv run --extra dev pytest tests/unit/test_plugin_parser.py -v

# Reproduce with the reporter's repo (requires network)
apm marketplace add cisco-genai/ai-native-maturity-leaderboard --ref setup-apm-packages --name repro-marketplace
apm install --target claude --trust-transitive-mcp --refresh "stack-react@repro-marketplace"
# Should now resolve transitive deps instead of "no transitive"

Closes #1666

[Copilot]

Pull request overview

This PR fixes a dependency-resolution regression for dual-format packages (those shipping both plugin.json/.claude-plugin/ signals and an apm.yml) by making plugin normalization non-destructive: instead of overwriting the existing apm.yml with identity-only fields, it now loads the existing manifest and merges/retains resolution-critical blocks so transitive APM/MCP deps continue to resolve for marketplace and remote-subdir installs.

Changes:

• Load any pre-existing apm.yml during synthesize_apm_yml_from_plugin() and merge its dependency blocks into the synthesized output.
• Add _union_dep_list() to union dependency lists (supporting both string and dict dependency entries) without duplicates.
• Add regression tests ensuring dependencies and other resolution-critical manifest blocks survive synthesis/normalization.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
src/apm_cli/deps/plugin_parser.py	Loads existing apm.yml and merges dependencies + preserves key blocks during plugin->apm.yml synthesis to avoid stripping transitive deps.
tests/unit/test_plugin_parser.py	Adds regression tests covering preservation/merging behavior across synthesize/normalize/validate flows for dual-format packages.

[github-actions]

APM Review Panel: ship_with_followups

Fix restores the README headline promise of transitive dependency resolution for dual-format packages by making synthesize_apm_yml_from_plugin() non-destructive; every marketplace and remote-subdir install colocating plugin.json with apm.yml now correctly preserves its transitive deps.

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

Panel converges strongly on two pre-ship items and three follow-up concerns. The fix is correct and materially important: the README central promise -- transitive dependency resolution, just like npm or pip -- was silently broken for every dual-format package until this PR. The code change is targeted and the 8 new tests in TestSynthesizePreservesExistingManifest provide a solid regression trap for the core #1666 scenario, even though the sandbox could not run-certify them (outcome: unknown per test-coverage-expert S7 contract; treated as structured opinion, not load-bearing evidence).

The single strongest signal across the panel is the bare except Exception: pass at line 224, flagged independently by four active panelists (python-architect, cli-logging-expert, devx-ux-expert, supply-chain-security-expert) and confirmed as an untested fallback gap by test-coverage-expert (outcome: missing on a portability-by-manifest surface, near-blocking weight). This line silently re-introduces the exact symptom of #1666 under a YAML corruption trigger: the user loses all transitive deps with zero diagnostic output, indistinguishable from pre-fix behavior. Every other error path in this file that discards data calls _surface_warning() at lines 404, 424, and 568; this path breaks that established contract. The fix is one line: except Exception as exc: _surface_warning(f'Could not load existing apm.yml for merge; transitive deps may be lost: {exc}', _logger). A regression-trap test for the malformed-YAML fallback must accompany it. The missing CHANGELOG.md entry is the second pre-ship item: CONTRIBUTING.md requires it for behavior changes, the [Unreleased] section currently has entries for #1662 and #1664 but nothing for #1666, and two panelists flag this gap independently.

Supply-chain raises two forward-looking concerns -- scripts blocks preserved verbatim with no lifecycle isolation and registries blocks carrying potentially attacker-controlled URLs on disk -- but neither is exploitable today: APM does not auto-execute scripts and plugin registries are isolated from root resolution. Stripping these fields would introduce new data loss for dual-format authors, which is the exact failure mode this PR fixes. The correct path is a tracked follow-up issue designing an explicit allowlist for scripts lifecycle keys and a registry URL authority check. The python-architect SRP concern and the dict-form dep silencing gap are real but scope-bounded; handle in a targeted follow-up.

Dissent. supply-chain-security-expert recommends stripping scripts and registries from the preserved-blocks tuple entirely; no other active panelist supports stripping. The CEO sides against: both risks are non-exploitable today, stripping would introduce new data loss, and a threat model with no active exploitation path does not justify regressing the fix's data-fidelity goal. test-coverage-expert returned outcome: unknown for the 8 new tests (sandbox S7 contract, no run-certify capability); treated as structured opinion rather than load-bearing evidence.

Aligned with: Portable by manifest (apm.yml is now authoritative for transitive resolution in dual-format packages; dict-form dep sub-key gap remains); Secure by default (scripts and registries now on disk without lifecycle isolation or URL authority checks -- forward risk, not current exploit); Pragmatic as npm (install now works for dual-format packages; YAML-corrupt silent-fallback is the last deviation); OSS community driven (EPAM contributor closed a headline regression with a well-structured 210-line test suite).

Growth signal. The README hero copy -- transitive dependency resolution, just like npm or pip -- was silently broken for every dual-format package until this PR. Critically, apm plugin init generates dual-format packages on day one, meaning every user who followed the official quickstart was exposed to the #1666 regression from their first install. The CHANGELOG entry should name this explicitly to signal APM's responsiveness. EPAM contributor is an enterprise signal. Post-release, targeted outreach to marketplace package authors who declared transitive deps in apm.yml alongside plugin.json and saw silent drops would demonstrate that APM hears and closes community-reported regressions at speed.

Panel summary

Persona	B	R	N	Takeaway
Python Architect	0	3	2	Correct fix for #1666; 3 recommended: bare-except with no log (line 224), dict-form dep silencing (line 835), SRP leak in _generate_apm_yml (line 792).
CLI Logging Expert	0	2	1	Silent except-pass on corrupt apm.yml re-silences the #1666 regression with no [!] warning; verbose merge path is also invisible to agents.
DevX UX Expert	0	2	1	Fix restores correct transitive resolution mental model; one silent-failure surface remains where malformed apm.yml still drops deps without any user-visible warning.
Supply Chain Security Expert	0	3	2	Fix is correct; three forward-risk surfaces elevated by preservation of registries, scripts, and unaudited dep injection via existing apm.yml -- none exploitable today but warrant hardening.
OSS Growth Hacker	0	1	1	Critical silent data-loss fix restoring the README's core promise (transitive resolution). Excellent community contribution pattern. CHANGELOG entry missing; dual-format authoring story untold.
Doc Writer	0	2	1	CHANGELOG.md has no entry for #1666; reference/package-types.md omits dual-format (plugin.json + apm.yml) behavior now fixed by this PR. No other doc updates needed.
Test Coverage Expert	0	4	1	8 new tests meet integration-fixture tier for core #1666 regression; 3 edge-case gaps: malformed-YAML fallback silently strips deps (no trap), dict-form existing deps skipped without test, and LSP preservation from existing manifest untested.

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

Top 5 follow-ups

1. [CLI Logging Expert + Python Architect + DevX UX Expert + Supply Chain Security Expert + Test Coverage Expert] (blocking-severity) Replace bare except Exception: pass at line 224 with _surface_warning() emitting a human-readable warning that transitive deps may not be preserved, and add a regression-trap test for the malformed-YAML fallback path -- Four panelists flagged the silent except independently; test-coverage-expert confirmed the fallback path has no regression trap (outcome: missing on a portability-by-manifest surface). The fix re-creates the Bug: marketplace and remote subdir installs drop transitive dependencies #1666 symptom under YAML corruption with zero user diagnostic. One-line code change matching the pattern already established at lines 404, 424, and 568; one test case. Pre-ship.
2. [OSS Growth Hacker + Doc Writer] (blocking-severity) Add CHANGELOG.md [Unreleased] ### Fixed entry crediting @sergio-sisternes-epam and closing Bug: marketplace and remote subdir installs drop transitive dependencies #1666 -- CONTRIBUTING.md requires a CHANGELOG entry for behavior changes. The [Unreleased] section currently carries entries for apm install never updates existing .claude/rules/*.md from local .apm/instructions/ (deployed rule files missing from lockfile local_deployed_files) #1662 and refactor(auth): debug-log all exception handlers in credential cascade (closes #935) #1664 but nothing for the fix that restores the project headline promise. Two panelists flag this independently. Pre-ship.
3. [Python Architect + Test Coverage Expert] Fix dict-form existing dep blocks silently skipped at line 835 by removing or guarding the isinstance(val, list) check, and add a regression-trap test for this dep shape -- The isinstance guard silently discards dict-form dependency sub-keys from existing apm.yml -- the same data-loss pattern this PR fixes for list-form deps. test-coverage-expert confirms no test for this shape (outcome: missing). Two panelists flagged this independently. One-line fix; one test.
4. [Supply Chain Security Expert] Open a tracked issue to design lifecycle-key allowlisting for scripts blocks and root-project registry URL authority checking before any lifecycle-hook or registry-federation feature is introduced -- scripts and registries are now preserved from existing apm.yml without content validation or URL authority checks. Neither surface is exploitable today, but both become active attack vectors when auto-execution or cross-plugin registry propagation lands.
5. [Doc Writer + OSS Growth Hacker] Add dual-format package authoring documentation to reference/package-types.md describing the plugin.json + apm.yml merge contract -- Dual-format is now a supported, non-destructive pattern but no documentation tells a producer this combination works or what the merge semantics are. apm plugin init generates dual-format packages on day one, making this the default authoring path for new contributors.

Architecture

classDiagram
    direction LR
    class plugin_parser {
      <<module>>
      +normalize_plugin_directory(plugin_path, plugin_json_path) Path
      +synthesize_apm_yml_from_plugin(plugin_path, manifest) Path
      -_generate_apm_yml(manifest, existing_manifest) str
      -_union_dep_list(merged, key, new_entries) None
    }
    class yaml_io {
      <<IOBoundary>>
      +load_yaml(path) dict
      +yaml_to_str(data) str
    }
    class validation {
      <<module>>
      +validate_apm_package(package_path) ValidationResult
      -_validate_marketplace_plugin(package_path, plugin_json_path, result) ValidationResult
    }
    class APMPackage {
      <<ValueObject>>
      +name str
      +version str
      +dependencies dict
      +targets list
      +from_apm_yml(apm_yml_path) APMPackage
      -_parse_dependency_dict(raw_deps) dict
    }
    class ValidationResult {
      <<ValueObject>>
      +is_valid bool
      +package APMPackage
      +package_type PackageType
    }
    validation ..> plugin_parser : calls normalize_plugin_directory
    plugin_parser ..> yaml_io : load_yaml reads existing apm.yml
    plugin_parser ..> yaml_io : yaml_to_str writes merged content
    validation ..> APMPackage : from_apm_yml consumes synthesized file
    ValidationResult *-- APMPackage : contains
    class plugin_parser:::touched
    classDef touched fill:#fff3b0,stroke:#d47600

Loading

flowchart TD
    A["validate_apm_package\nvalidation.py:305"] --> B["detect_package_type MARKETPLACE_PLUGIN\nvalidation.py:335"]
    B --> C["_validate_marketplace_plugin\nvalidation.py:681"]
    C --> D["normalize_plugin_directory\nplugin_parser.py:135"]
    D --> E["synthesize_apm_yml_from_plugin\nplugin_parser.py:168"]
    E --> F{"apm.yml exists?\nplugin_parser.py:217"}
    F -- yes --> G["load_yaml(apm_yml_path)\nexisting_manifest populated"]
    F -- no --> H["existing_manifest = None\noriginal behavior preserved"]
    G --> I["_generate_apm_yml(manifest, existing_manifest)\nplugin_parser.py:792"]
    H --> I
    I --> J{"existing_manifest present?\nplugin_parser.py:831"}
    J -- yes --> K["copy list-typed dep keys only\nif isinstance val list: merged_deps key\nplugin_parser.py:834-836\nWARN: dict-form vals silently dropped"]
    J -- no --> L["merged_deps = {}"]
    K --> M["_union_dep_list: layer plugin apm/mcp/lsp deps\nplugin_parser.py:841-855"]
    L --> M
    M --> N["preserve devDeps, registries, target/targets, includes, scripts\nplugin_parser.py:863-873"]
    N --> O["write merged apm.yml\nplugin_parser.py:230"]
    O --> P["APMPackage.from_apm_yml\napm_package.py:312"]
    P --> Q["_parse_dependency_dict: parse full dep tree"]
    Q --> R["ValidationResult: transitive deps intact"]

Loading

Recommendation

Ship after two pre-ship items are addressed in this PR: (1) replace the bare except Exception: pass at line 224 with a _surface_warning() call matching the pattern at lines 404, 424, and 568, and add a malformed-YAML regression-trap test -- one-line code change, one test, unanimously flagged by four panelists and confirmed by a missing-evidence gap on a portability-by-manifest surface; (2) add the CHANGELOG.md [Unreleased] ### Fixed entry as required by CONTRIBUTING.md. The remaining three follow-ups -- scripts and registries hardening design, dict-form dep gap with regression trap, and dual-format authoring docs -- are correctly scoped as post-merge issues and do not block the regression fix from landing. The supply-chain recommendation to strip scripts and registries from preserved-blocks is rejected in favor of a tracked forward-risk issue; stripping would regress the data-fidelity goal this fix establishes. This is a high-quality community contribution from an EPAM engineer that closes a silent data-loss regression affecting every dual-format marketplace package; it should merge promptly once the two pre-ship items are in.

---

Full per-persona findings

Python Architect

• [recommended] Dict-form existing deps silently dropped; inconsistent with plugin-derived dict deps which ARE preserved at src/apm_cli/deps/plugin_parser.py:835
  Lines 834-836: the loop only copies list-typed dep sub-keys from the existing manifest. A dep sub-key with a non-list value (e.g. a dict like {pkg: ^1.0}) is silently discarded with no warning. Asymmetric with plugin-derived deps (lines 843-845) where a non-list value IS preserved via setdefault.
  Suggested: Either (a) drop the isinstance guard and use merged_deps[key] = val to faithfully round-trip any shape; or (b) keep the guard but add an else branch calling _surface_warning() to surface the skipped key.

• [recommended] Bare except Exception: pass on apm.yml load silently swallows YAML corruption, logging nothing at src/apm_cli/deps/plugin_parser.py:224
  If load_yaml() raises yaml.YAMLError or PermissionError, the code silently falls back to plugin-only synthesis, stripping all deps -- exactly the regression this PR is fixing. _surface_warning() is already imported and used throughout this module for best-effort diagnostics.
  Suggested: Replace except Exception: pass with except Exception as exc: _surface_warning(f'Could not read existing apm.yml for merge; transitive deps may be lost: {exc}', _logger).

• [recommended] _generate_apm_yml() now mixes generation and merge, violating single responsibility at src/apm_cli/deps/plugin_parser.py:792
_generate_apm_yml() was a pure transformation: (plugin manifest dict) -> YAML string. Adding existing_manifest couples a second concern into the same function, making the merge logic harder to test in isolation (now 90+ lines).
  Suggested: Extract merge logic into _merge_existing_into(plugin_manifest: dict, existing: dict) -> dict (pure, no I/O). Call from synthesize_apm_yml_from_plugin(): merged = _merge_existing_into(manifest, existing_manifest), then _generate_apm_yml(merged) with the original single-param signature.

• [nit] Docstring mentions only 'targets' but code also preserves singular 'target' (legacy form) at src/apm_cli/deps/plugin_parser.py:177
  The preservation loop includes both 'target' (singular legacy) and 'targets' (canonical plural) but only 'targets' is mentioned in the docstring.

• [nit] _union_dep_list() type hints use bare unparameterized list at src/apm_cli/deps/plugin_parser.py:884
  With from typing import Any already imported, dict[str, list[Any]] and list[Any] are more precise.
  Suggested: Change signature to _union_dep_list(merged: dict[str, list[Any]], key: str, new_entries: list[Any]) -> None:.

CLI Logging Expert

• [recommended] Silent except Exception: pass swallows load failure with no user-visible warning at src/apm_cli/deps/plugin_parser.py:224
  When an existing apm.yml fails to parse, existing_manifest stays None and _generate_apm_yml silently strips deps -- reproducing the exact symptom of Bug: marketplace and remote subdir installs drop transitive dependencies #1666 with no signal. Every other error path in this file that discards data uses _surface_warning() (lines 404, 424, 568). This path breaks that contract.
  Suggested: Change except Exception: pass to except Exception as exc: _surface_warning(f"Could not load existing apm.yml for '{plugin_path.name}'; transitive dependencies may not be preserved: {exc}", _logger).

• [recommended] No debug/verbose signal on the successful dep-preservation path at src/apm_cli/deps/plugin_parser.py:222
  When existing_manifest loads cleanly and deps are merged, the output to agents running with --verbose is zero. Resolution decisions should be traceable.
  Suggested: After existing_manifest = data, add: _logger.debug("Merging existing apm.yml for '%s' into synthesized manifest (preserving %d dep blocks)", plugin_path.name, len(existing_manifest.get('dependencies') or {})).

• [nit] _generate_apm_yml preserves 6 resolution-critical keys with no per-key trace at src/apm_cli/deps/plugin_parser.py:863
  A single _logger.debug listing which keys were preserved would let operators and agents confirm the full merge without reading source code.

DevX UX Expert

• [recommended] Malformed apm.yml silently drops all transitive deps with no user-visible warning at src/apm_cli/deps/plugin_parser.py:224
  The bare except Exception: pass means a user whose apm.yml has a YAML syntax error will still hit the exact symptom described in Bug: marketplace and remote subdir installs drop transitive dependencies #1666 with zero diagnostic output. npm, pip, and cargo all treat a corrupt manifest as a hard, loud failure. This file already uses _surface_warning() and imports _rich_warning for similar situations.
  Suggested: Replace pass # Best-effort; fall back to plugin-only metadata with _surface_warning(f'Could not read existing apm.yml at {apm_yml_path}: dependencies may be incomplete. Run apm install --verbose for details.', _logger).

• [recommended] No positive signal when transitive deps are loaded from existing apm.yml during marketplace install at src/apm_cli/deps/plugin_parser.py:223
  The bug was silent; the fix is also silent. A user who hit Bug: marketplace and remote subdir installs drop transitive dependencies #1666 and upgrades APM has no way to confirm the fix is working without inspecting the lockfile manually. npm prints each package it resolves; cargo announces downloading N crates.
  Suggested: After existing_manifest = data, emit: _logger.debug('Loaded existing apm.yml from %s; %d top-level keys preserved', apm_yml_path, len(data)).

• [nit] except Exception is too broad and masks I/O errors distinct from YAML parse errors at src/apm_cli/deps/plugin_parser.py:224
  A PermissionError or OSError is not a YAML parse failure; conflating them makes triage harder.
  Suggested: Split into except (yaml.YAMLError, ValueError) for parse errors and a separate except OSError for I/O failures, each with its own warning text.

Supply Chain Security Expert

• [recommended] scripts block from existing apm.yml preserved verbatim with no lifecycle isolation at src/apm_cli/deps/plugin_parser.py:870
  APM's install pipeline does not execute scripts today (ScriptRunner requires explicit apm run), but there is no structural guard preventing a future lifecycle hook from auto-executing them. Unlike registries (validated by _parse_registries_block), scripts values undergo zero content validation in this code path. A compromised plugin maintainer can embed any shell payload.
  Suggested: Drop scripts from the preserved-blocks tuple, or add an explicit allowlist of safe script names and strip known lifecycle keys (postinstall, prepare, install, preinstall) before writing.

• [recommended] registries block emitted to synthesized manifest with no URL authority check at src/apm_cli/deps/plugin_parser.py:866
_parse_registries_block validates format when the file is loaded, but does NOT check URLs against any project-level allowlist. The plugin's registries are currently isolated from root-level resolution, but if any future code treats the plugin's APMPackage.registries as a resolution authority, those URLs activate silently.
  Suggested: Strip registries from the preserved-blocks tuple. The synthesized manifest does not need registry configuration -- the root project governs registry resolution.

• [recommended] Existing apm.yml dependencies are now an active dep-injection surface, previously inert due to bug Bug: marketplace and remote subdir installs drop transitive dependencies #1666 at src/apm_cli/deps/plugin_parser.py:831
  Before this PR, normalization discarded all apm.yml deps (the bug). After this PR, merged_deps are faithfully written and resolved transitively. This is the correct fix, but it elevates apm.yml to the same injection authority as plugin.json with no corresponding visibility -- no log or warning is emitted when deps are merged from existing_manifest.
  Suggested: Emit a _logger.info or _rich_warning when non-empty deps are merged from an existing apm.yml, e.g. 'Merging N deps from existing apm.yml in plugin-name'.

• [nit] _union_dep_list dedup uses full == equality; dict deps differing only in git URL both survive dedup at src/apm_cli/deps/plugin_parser.py:895
  Document in the _union_dep_list docstring that dedup is structural (==) not semantic. If the same logical package appears with different URLs, both are retained -- this is intentional but callers should be aware.

• [nit] TOCTOU window between apm_yml_path.exists() and open() write is technically present but safe at src/apm_cli/deps/plugin_parser.py:217
  Practical risk is negligible (filesystem access to install directory + sub-millisecond race window required). No action needed today.

OSS Growth Hacker

• [recommended] CHANGELOG [Unreleased] has no entry for this fix despite it restoring the project's headline promise at CHANGELOG.md:8
  The README hero copy reads 'transitive dependency resolution, just like npm or pip' -- that promise was silently broken for every dual-format package until this PR. The [Unreleased] section lists lower-visibility fixes but omits this one.
  Suggested: Add under [Unreleased] ### Fixed: 'Marketplace and remote subdir installs now preserve transitive dependencies for dual-format packages (plugin.json + apm.yml); previously, synthesizing apm.yml from plugin.json silently discarded existing dependencies, devDependencies, registries, targets, includes, and scripts blocks. (Bug: marketplace and remote subdir installs drop transitive dependencies #1666, by @sergio-sisternes-epam)'

• [nit] Dual-format package authoring is now a supported, non-destructive pattern but docs don't say so at docs/src/content/docs/getting-started/first-package.md:289
  A one-sentence callout would turn a silent fix into a documentable authoring pattern that closes the trust loop for authors who were burned by the old behavior.

Auth Expert -- inactive

No fast-path files changed; dep-merging fix does not alter token resolution, host classification, or AuthResolver call sites.

Doc Writer

• [recommended] CHANGELOG.md [Unreleased] section missing a Fixed entry for Bug: marketplace and remote subdir installs drop transitive dependencies #1666 at CHANGELOG.md
  CONTRIBUTING.md requires a CHANGELOG.md entry for behavior changes. The [Unreleased] ### Fixed section has an entry for apm install never updates existing .claude/rules/*.md from local .apm/instructions/ (deployed rule files missing from lockfile local_deployed_files) #1662 but none for this fix. The bug caused silent data loss (dependencies stripped from dual-format packages on install) -- a high-impact regression that should be recorded.
  Suggested: Add under [Unreleased] ### Fixed: 'apm install now preserves dependencies, devDependencies, registries, targets, and scripts from apm.yml when a package also has plugin.json or .claude-plugin/, fixing transitive resolution for marketplace and remote subdirectory installs. (by @sergio-sisternes-epam, closes Bug: marketplace and remote subdir installs drop transitive dependencies #1666)'

• [recommended] reference/package-types.md: Plugin collection section does not document dual-format (plugin.json + apm.yml) packages or their transitive-dep semantics at docs/src/content/docs/reference/package-types.md
  With this fix, the dual-format pattern now works correctly. A producer authoring a Claude plugin with sub-dependencies has no documentation telling them this combination is supported or what the merge contract is.
  Suggested: In the 'Plugin collection (plugin.json)' section, add a paragraph: 'If the package also has an apm.yml, APM treats this as a dual-format package. Plugin identity fields (name, version, description, author) are sourced from the plugin artifact. All other blocks -- dependencies, devDependencies, registries, targets, scripts, includes -- are read from apm.yml and resolved transitively on install.'

• [nit] PR body uses 'identity-only fields' without defining the term in the TL;DR
  Minor: 'identity fields' could be parenthetically explained in the TL;DR as '(name, version, description)' for reviewers who skim.

Test Coverage Expert

• [recommended] Malformed existing apm.yml triggers except Exception: pass fallback -- deps silently stripped, no regression trap at tests/unit/test_plugin_parser.py
  When existing apm.yml has a YAML syntax error, existing_manifest stays None. The entire dep-merging block is skipped. Result is identical to pre-fix behavior: all deps silently discarded. Grep of tests/unit/test_plugin_parser.py confirmed zero tests for malformed/corrupt YAML in the context of synthesize_apm_yml_from_plugin.
  Suggested: Add test_malformed_existing_apm_yml_falls_back_to_plugin_only_metadata in TestSynthesizePreservesExistingManifest: write a malformed apm.yml, call synthesize_apm_yml_from_plugin, assert the call succeeds and the synthesized apm.yml contains the plugin identity fields.
  Proof (missing at integration-with-fixtures): tests/unit/test_plugin_parser.py::TestSynthesizePreservesExistingManifest::test_malformed_existing_apm_yml_falls_back_to_plugin_only_metadata -- proves: A malformed existing apm.yml does not crash synthesis and produces a working (if incomplete) manifest [portability-by-manifest, devx]

• [recommended] Dict-form existing dependencies silently skipped -- no test for this shape at tests/unit/test_plugin_parser.py
  Lines 833-836 gate on isinstance(val, list). If the existing manifest carries dependencies in dict form (a format some tooling generates), the isinstance check fails and the dep block is silently excluded. Grep confirmed no test where the existing manifest has dict-form dependencies.
  Suggested: Add test_existing_apm_deps_dict_form_preserved_or_warned in TestSynthesizePreservesExistingManifest: write an apm.yml with dependencies: {apm: {my-package: '>=1.0.0'}} (dict, not list), call synthesize_apm_yml_from_plugin, assert either the dict is preserved or a warning is emitted.
  Proof (missing at integration-with-fixtures): tests/unit/test_plugin_parser.py::TestSynthesizePreservesExistingManifest::test_existing_apm_deps_dict_form_preserved_or_warned -- proves: When existing apm.yml uses dict-format dependencies, user gets predictable behavior rather than silent data loss [portability-by-manifest, devx]

• [recommended] 8 new integration-fixture-tier tests verified present and correctly structured; cannot run-certify due to sandbox at tests/unit/test_plugin_parser.py
  8 tests in TestSynthesizePreservesExistingManifest use real filesystem operations (tmp_path, real YAML read/write, no mocks at dep-merging layer) -- integration-with-fixtures tier. Cannot produce irrefutable run evidence per S7 contract; author reports 16,638 passed in PR body.
  Suggested: Maintainer can verify with: uv run --extra dev pytest tests/unit/test_plugin_parser.py::TestSynthesizePreservesExistingManifest -v
  Proof (unknown at integration-with-fixtures): tests/unit/test_plugin_parser.py::TestSynthesizePreservesExistingManifest -- proves: Plugin normalisation does not strip transitive deps from dual-format packages (regression trap for Bug: marketplace and remote subdir installs drop transitive dependencies #1666) [portability-by-manifest, devx]

• [recommended] PR body lacks Scenario Evidence table mapping changed behavior to APM principles at PR body
  Per the PR description rubric, behavior-change PRs should include a scenario-to-principle mapping table. The PR body has a Validation Evidence table and strong prose, but no row-by-row mapping of the 8 new scenarios to APM principles (Portability, DevX, etc.).
  Suggested: Add a Scenario Evidence table with one row per new test, naming the APM principle each scenario defends.

• [nit] LSP dep preservation from existing manifest has no explicit regression-trap test at tests/unit/test_plugin_parser.py
  The dep-merging loop handles 'lsp' via the same isinstance gate, but no test puts dependencies.lsp in an existing manifest and verifies it survives synthesis.
  Suggested: Parametrize test_preserves_existing_apm_dependencies with dep_type 'apm', 'mcp', 'lsp', or add a standalone test.
  Proof (missing at integration-with-fixtures): tests/unit/test_plugin_parser.py::TestSynthesizePreservesExistingManifest::test_preserves_existing_lsp_dependencies -- proves: When an existing apm.yml carries LSP server dependencies, they survive plugin normalisation [portability-by-manifest]

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

Warning

Firewall blocked 2 domains

The following domains were blocked by the firewall during workflow execution:

• astral.sh
• pypi.org

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "astral.sh"
    - "pypi.org"

See Network Configuration for more information.

Generated by PR Review Panel for issue #1687 · sonnet46 16M · ◷