fix(deps): support meta-packages and apm.yml inside collections/ subpaths

[edenfunf]

Description

Fixes two related gaps that prevented a natural meta-package mental model
(a sub-path inside a monorepo whose apm.yml declares dependencies on other
skills/MCPs/packages):

1. /collections/ path segment hard-routed to the .collection.yml parser,
   shadowing any apm.yml that lived at <repo>/collections/<name>/apm.yml.
2. Pure dependency-only apm.yml (no .apm/ directory) failed validation
   even after transitive deps had been resolved and installed.

Both stem from the same architectural error: package type was inferred from
path segments rather than content. The fix removes the URL-level path
heuristic and adds META_PACKAGE as a first-class PackageType.

Fixes #1094

Type of change

• Bug fix
• New feature
• Documentation
• Maintenance / refactor

What changed

models/dependency/reference.py

• virtual_type is now extension-only: a path ending in .collection.yml
  /.collection.yaml is COLLECTION; otherwise (no recognised file
  extension) it is SUBDIRECTORY, and the actual on-disk shape is resolved
  at fetch time. Path segments like /collections/ no longer carry semantic
  weight.
• Parser-level acceptance check unified to use VIRTUAL_FILE_EXTENSIONS + VIRTUAL_COLLECTION_EXTENSIONS, so explicit .collection.yml URLs are
  accepted regardless of whether the path is under /collections/.

deps/github_downloader_validation.py

• Probe order in the SUBDIRECTORY branch puts apm.yml before the legacy
  <vpath>.collection.yml fallback so collections/<name>/apm.yml is
  recognised as an APM package, not as a missing collection manifest.
• Fixed a latent double-extension bug in the COLLECTION probe
  ({vpath}.collection.yml.collection.yml) exposed by the new
  extension-only classification.

deps/github_downloader.py

• New _is_legacy_collection_fallback helper detects the legacy
  /collections/<name> URL form whose actual content is a sibling
  <name>.collection.yml, and routes the install to
  download_collection_package so existing consumers keep working without
  the path heuristic.
• download_collection_package precondition relaxed to accept
  SUBDIRECTORY-typed refs (the legacy fallback path).

models/validation.py

• New PackageType.META_PACKAGE for apm.yml that declares dependencies
  but contributes no own primitives. detect_package_type cascade
  recognises this shape (apm.yml exists, no .apm/, no nested skills,
  declared deps in dependencies or devDependencies). Validation parses
  apm.yml and returns cleanly; transitive resolution and integration
  happen on the declared dependencies as before.
• An apm.yml with no deps and no .apm/ still resolves to INVALID so
  the existing helpful error wording is preserved.

install/sources.py

• Label table updated for META_PACKAGE.

Why

The fix removes a class of bugs rather than adding a special case. Path
segments and package shape were two orthogonal concerns conflated by the
old /collections/ heuristic; once virtual_type is extension-only the
two gaps disappear together. A first-class META_PACKAGE type avoids the
empty-.apm/.gitkeep workaround the issue described as a wart.

The legacy implicit /collections/<name> URL form (where the repo only
contains a sibling <name>.collection.yml) is preserved via the
_is_legacy_collection_fallback dispatcher: classification stays
content-based, but a path-shape hint (/collections/ segment) narrows
the search space so SUBDIRECTORY refs outside that convention skip 2-3
otherwise-wasted HTTP probes per install.

How (testing)

Unit tests: 7040 passed locally, including:

• tests/unit/test_meta_package.py (new) -- 12 tests covering the
  META_PACKAGE detection cascade and validation, including dev-only
  aggregators, conflicts with SKILL_BUNDLE / HYBRID, and the negative
  case (apm.yml with no deps and no .apm/ -> INVALID).
• tests/unit/deps/test_github_downloader_validation.py -- new
  regression tests for probe order (apm.yml priority over
  .collection.yml), the _is_legacy_collection_fallback dispatcher
  (path-hint short-circuit, both .yml/.yaml extensions), and the
  double-extension bug.
• 6 existing tests updated to reflect the extension-only classification
  (URLs that asserted is_virtual_collection() is True for
  collections/<name> without extension now use the explicit form, with
  separate tests added for the new SUBDIRECTORY semantics).

Manual end-to-end: against a fixture repo, all five package shapes
under /collections/<name>/ install correctly (APM_PACKAGE,
META_PACKAGE, SKILL_BUNDLE, HYBRID, MARKETPLACE_PLUGIN). The
conflict case (sibling apm.yml and .collection.yml both present)
correctly resolves to the apm.yml-defined package. Backwards-compat
flows verified: explicit .collection.yml URL form, legacy implicit
/collections/<name> form (CLI and via apm.yml), pinned refs (#main,
commit SHA), and lockfile round-trip stability.

Testing

• Tested locally
• All existing tests pass
• Added tests for new functionality (if applicable)

[Copilot]

Pull request overview

This PR fixes dependency resolution and validation for monorepo subpaths by removing the /collections/ path-segment heuristic, making virtual-package classification extension-based, and introducing a first-class META_PACKAGE type for dependency-only apm.yml packages.

Changes:

• Make virtual package type detection extension-only (explicit .collection.yml/.yaml => COLLECTION; otherwise SUBDIRECTORY) and adjust naming/install-path expectations accordingly.
• Add PackageType.META_PACKAGE and validation support for dependency-only apm.yml packages with no .apm/.
• Update GitHub downloader validation/probing and add a legacy fallback dispatcher so implicit collections/<name> can still resolve to a sibling <name>.collection.yml.

Reviewed changes

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

Show a summary per file

File	Description
tests/unit/test_script_runner.py	Updates virtual collection ref in script-runner tests to explicit .collection.yml.
tests/unit/test_package_identity.py	Aligns install-path expectations with new extension-only classification; adds SUBDIRECTORY natural-layout test.
tests/unit/test_meta_package.py	Adds unit coverage for META_PACKAGE detection and validation behavior.
tests/unit/test_generic_git_urls.py	Updates Bitbucket virtual collection parsing test to explicit .collection.yml.
tests/unit/test_artifactory_support.py	Updates Artifactory virtual collection parsing test to explicit .collection.yml.
tests/unit/test_ado_path_structure.py	Updates ADO virtual collection parsing tests to explicit .collection.yml and adjusts expected virtual_path.
tests/unit/deps/test_github_downloader_validation.py	Adds regression tests for probe order, explicit extension handling, and legacy collection fallback routing.
tests/test_apm_package_models.py	Updates model parsing tests for explicit collection refs and new SUBDIRECTORY semantics for implicit /collections/<name>.
tests/integration/test_collection_install.py	Adjusts integration parsing expectations for implicit vs explicit collection refs under the new rules.
src/apm_cli/models/validation.py	Introduces META_PACKAGE, detection helper, and meta-package validation path.
src/apm_cli/models/dependency/reference.py	Implements extension-only virtual type classification and updates virtual-path acceptance and naming.
src/apm_cli/install/sources.py	Adds user-facing label for META_PACKAGE.
src/apm_cli/deps/github_downloader_validation.py	Fixes COLLECTION probe double-extension and reorders SUBDIRECTORY probes to prioritize apm.yml.
src/apm_cli/deps/github_downloader.py	Adds _is_legacy_collection_fallback dispatcher and relaxes collection downloader preconditions to support legacy SUBDIRECTORY refs.

Comments suppressed due to low confidence (1)

src/apm_cli/models/dependency/reference.py:662

• The InvalidVirtualPackageExtensionError message only lists VIRTUAL_FILE_EXTENSIONS, but this code path now also recognizes explicit collection manifests (.collection.yml / .collection.yaml). If a user typos a collection URL (e.g., .collection.ym), the error currently suggests only the file extensions, which is misleading. Please update the message to mention the recognized collection-manifest extensions (or list the combined recognized extensions) so users get an actionable hint.

            # Accept any path ending in a recognised virtual extension
            # (file or collection-manifest). Reject other dotted final
            # segments so typos like `prompts/file.txt` fail fast instead
            # of silently mis-classifying as a subdirectory.
            recognised_exts = cls.VIRTUAL_FILE_EXTENSIONS + cls.VIRTUAL_COLLECTION_EXTENSIONS
            if any(virtual_path.endswith(ext) for ext in recognised_exts):
                pass
            else:
                last_segment = virtual_path.split("/")[-1]
                if "." in last_segment:
                    raise InvalidVirtualPackageExtensionError(
                        f"Invalid virtual package path '{virtual_path}'. "
                        f"Individual files must end with one of: {', '.join(cls.VIRTUAL_FILE_EXTENSIONS)}. "
                        f"For subdirectory packages, the path should not have a file extension."
                    )

[danielmeppiel]

Collection is legacy. This should be solved by transitive steps (even local) which APM already resolves. We should remove collections support

[edenfunf]

Collection is legacy. This should be solved by transitive steps (even local) which APM already resolves. We should remove collections support

Got it — happy to go this direction. One scope question before I rework:

drop the new collection-related code I added (the _is_legacy_collection_fallback dispatcher, the .collection.yml probes in validate_virtual_package_exists, the relaxed download_collection_package precondition) and leave the existing download_collection_package / collection_parser.py inert for now? Or actually delete those modules in this PR?

The latter would be a breaking change for anyone with .collection.yml deps in their apm.yml today, so I'd lean toward splitting into a separate issue — but happy either way, just want to scope correctly.

[danielmeppiel]

Rework: drop collection-virtual-package, accept dep-only apm.yml

Note

This commit reworks the PR to align with the apm-review-panel synthesis on #1094. Original commits credited via Co-authored-by: xuyuanhao trailer. Thanks @edenfunf for surfacing the issue and the original PR.

TL;DR

The conceptual fix for #1094 is a one-line guard relaxation in _validate_apm_package_with_yml: a curated dependency aggregator (apm.yml with declared deps and no .apm/) is a valid APM_PACKAGE. The original PR also introduced a parallel .collection.yml / VirtualPackageType.COLLECTION codepath that overlaps with the existing SUBDIRECTORY virtual-package handling and added a latent bug. The panel's recommendation: delete the collection-virtual-package machinery wholesale -- the relaxed validation makes it redundant, no lockfile is affected (the enum was never persisted), and the smaller surface is easier to maintain.

Problem (WHY)

• _validate_apm_package_with_yml rejected apm.yml without .apm/, forcing dep-only aggregators to commit an empty .apm/.gitkeep placeholder. This is the user-visible bug in apm.yml inside collections/<name>/ is ignored — Collection detector preempts package resolution #1094.
• The original PR's META_PACKAGE enum + .collection.yml virtual-package codepath solved the symptom by adding parallel infrastructure rather than fixing the guard.
• script_runner.py referenced download_virtual_collection_package, which never existed on GitHubPackageDownloader -- a latent AttributeError waiting for any user to hit the auto-install path with an explicit .collection.yml URL.
• is_virtual_collection() and SUBDIRECTORY overlapped: a collections/foo path with apm.yml already worked correctly via SUBDIRECTORY, making the explicit .collection.yml form redundant.

Approach (WHAT)

Layer	Change
Validation cascade	apm.yml present -> APM classification. With .apm/ OR declared deps, APM_PACKAGE. Otherwise INVALID (preserves the standard "missing .apm/" diagnostic).
Dep-list detection	_apm_yml_declares_dependencies requires at least one parseable str/dict entry under dependencies.{apm,mcp} or devDependencies.{apm,mcp} -- rejects malformed shapes like apm: [123].
Parser	DependencyReference.parse() raises ValueError at parse time for any URL ending in .collection.yml / .collection.yaml, with a migration message.
Surface deletion	VirtualPackageType.COLLECTION, is_virtual_collection(), download_collection_package, normalize_collection_path, _is_legacy_collection_fallback, the collection probe in github_downloader_validation, collection_parser.py, and the META_PACKAGE install-source label all gone.

Implementation (HOW)

• src/apm_cli/models/validation.py -- cascade collapsed to 7 steps (was 8). New _apm_yml_declares_dependencies() helper. _validate_apm_package_with_yml .apm/ guard relaxed to accept dep-only.
• src/apm_cli/models/dependency/reference.py -- new REMOVED_COLLECTION_EXTENSIONS constant, parse-time ValueError, is_virtual_collection() deleted. The has_collection = "collections" in path_segments heuristic is preserved on purpose: it lets gitlab.com/owner/repo/collections/foo (no extension) parse as a SUBDIRECTORY virtual package on generic hosts.
• src/apm_cli/models/dependency/types.py -- VirtualPackageType.COLLECTION enum value removed.
• src/apm_cli/deps/github_downloader.py -- normalize_collection_path, download_collection_package, _is_legacy_collection_fallback deleted; dispatcher collapsed to if is_virtual_file(): file else: subdirectory (with Artifactory modes).
• src/apm_cli/deps/github_downloader_validation.py -- is_virtual_collection() probe block deleted; marker_paths cleaned up (apm.yml first, then SKILL.md / plugin.json / README.md).
• src/apm_cli/core/script_runner.py -- collection branch deleted (also kills the latent download_virtual_collection_package AttributeError).
• src/apm_cli/install/sources.py -- META_PACKAGE label entry deleted.
• src/apm_cli/integration/skill_integrator.py -- comment cleanup.
• src/apm_cli/deps/collection_parser.py -- entire file deleted.
• Docs -- manifest-schema.md, dependencies.md, CHANGELOG.md updated with migration callouts.

Diagrams

Validation cascade after the rework -- the dep-only branch is the single new edge:

flowchart TD
    A[detect_package_type] --> B{plugin.json or .claude-plugin/?}
    B -- yes --> P[MARKETPLACE_PLUGIN]
    B -- no --> C{root SKILL.md + apm.yml?}
    C -- yes --> H[HYBRID]
    C -- no --> D{root SKILL.md only?}
    D -- yes --> CS[CLAUDE_SKILL]
    D -- no --> E{nested skills/x/SKILL.md?}
    E -- yes --> SB[SKILL_BUNDLE]
    E -- no --> F{apm.yml present?}
    F -- yes --> G{.apm/ OR declared deps?}
    G -- yes --> AP[APM_PACKAGE]
    G -- no --> I1[INVALID]
    F -- no --> J{hooks/*.json?}
    J -- yes --> HP[HOOK_PACKAGE]
    J -- no --> I2[INVALID]

Loading

Trade-offs

• Breaking change to URL surface. Any user who pinned .collection.yml URLs in apm.yml will hit the migration ValueError. Mitigation: the error names the offending path and links to the dependencies guide. Lockfile is unaffected -- the COLLECTION enum was never persisted, so existing apm.lock.yaml files keep working.
• Two-step migration for collection authors. They convert .collection.yml to an apm.yml with dependencies: AND change the URL form. The accepted alternative -- silent translation of .collection.yml -> apm.yml -- was rejected because it would resurrect the parallel codepath this PR removes.
• The has_collection = "collections" in path_segments heuristic survives. It is parsing convenience for generic-host SUBDIRECTORY URLs, not a type classification. Removing it would break gitlab.com/owner/repo/collections/foo-style URLs.

Validation

$ uv run --extra dev ruff check src/ tests/ && uv run --extra dev ruff format --check src/ tests/
All checks passed!
620 files already formatted

$ uv run --extra dev pytest tests/unit tests/test_*.py --ignore=tests/unit/test_audit_report.py
3 failed, 7513 passed, 2 skipped

Note

The 3 failures (test_resolve_git_reference_commit, test_clone_fallback_respects_enterprise_host, test_credential_fill_used_when_no_env_token) reproduce on origin/main as well -- they are environment-specific git-auth tests, unrelated to this diff.

Scenario evidence

User promise	Test
Dep-only apm.yml (no .apm/) installs without .gitkeep workaround	tests/unit/test_dep_only_package.py::TestDepOnlyPackageDetection (12 tests)
Validation still rejects malformed apm.yml (e.g. apm: [123])	tests/unit/test_dep_only_package.py::test_apm_list_with_only_non_parseable_entries_is_invalid (and mcp / devDependencies siblings)
.collection.yml URLs surface a clear migration error at parse time	tests/unit/test_collection_migration_error.py (6 tests)
Implicit collections/<name> paths still resolve correctly as SUBDIRECTORY	tests/unit/test_package_identity.py::test_collections_path_subdirectory_uses_natural_layout, test_ado_virtual_collection_subdirectory
Existing FILE virtual packages (prompts/*.prompt.md, agents/*.agent.md) unaffected	tests/test_github_downloader.py::test_yaml_with_colon_in_*

How to test

1. git checkout fix/issue-1094-collections-meta-package from edenfunf's fork.
2. uv run --extra dev ruff check src/ tests/ && uv run --extra dev ruff format --check src/ tests/ -- both silent.
3. uv run --extra dev pytest tests/unit tests/test_*.py --ignore=tests/unit/test_audit_report.py -- expect 7513 pass plus the 3 pre-existing env failures noted above.
4. Author a tmp/apm.yml with only dependencies.apm: [microsoft/apm-sample-package] and no .apm/; run apm install -- expect success (was failing on main).
5. Add a dep entry like owner/repo/collections/foo.collection.yml to any apm.yml and run apm install -- expect a clear ValueError pointing at the dependencies guide.

[danielmeppiel]

Follow-up: test coverage gaps closed

Pushed 65e8a796 to address two gaps surfaced in feedback.

Why the trailer reads xuyuanhao and not edenfunf

That's the local git config name on the verified email aa9736195201@gmail.com, which is registered to GitHub user @edenfunf. GitHub resolves the trailer email to the account, so the "Co-authored-by" pill on commit f1b8109a links to @edenfunf's avatar -- credit lands on the correct person, only the displayed name is whatever Eden has in their git config locally. (Confirmed via gh pr view 1097 --json commits -> authors: ['', 'edenfunf', 'Copilot'].)

Test coverage gaps closed by 65e8a796

The original push had unit-test-only coverage. A test-coverage review identified three integration / seam gaps where the unit tests would not catch a regression. All three are now closed:

Gap	Test added	Why it matters
Dep-only apm.yml end-to-end (no .apm/ on root)	tests/integration/test_apm_dependencies.py::test_dep_only_project_installs_dependencies_without_dot_apm	Real install against microsoft/apm-sample-package proves the entire #1094 user story works through the install pipeline -- not just the detection layer. Picked up by scripts/test-integration.sh in CI.
.collection.yml migration error survives APMPackage.from_apm_yml re-wrap	tests/unit/test_collection_migration_error.py::TestCollectionMigrationErrorPropagation (2 tests, dependencies.apm + devDependencies.apm)	The parse-time ValueError is re-raised with a "Invalid APM dependency '...': " prefix. Without this trap, a future refactor could strip the actionable migration text and no test would notice.
apm install owner/repo/.../foo.collection.yml CLI-argument path	tests/unit/test_install_command.py::test_install_collection_yml_argument_surfaces_migration_message	The CLI surfaces the migration message via _resolve_package_references -> invalid_outcomes -> "All packages failed validation". Without this trap, that catch-and-surface chain could be silently swallowed by a future refactor.

Verification

$ uv run --extra dev ruff check src/ tests/ && uv run --extra dev ruff format --check src/ tests/
All checks passed!
620 files already formatted

$ uv run --extra dev pytest tests/unit tests/test_*.py --ignore=tests/unit/test_audit_report.py
3 failed, 7516 passed, 2 skipped (3 pre-existing env failures, unrelated)

$ GITHUB_APM_PAT=$(gh auth token) uv run --extra dev pytest \
    tests/integration/test_apm_dependencies.py::TestAPMDependenciesIntegration::test_dep_only_project_installs_dependencies_without_dot_apm -m integration
1 passed in 1.79s

The integration test fetches microsoft/apm-sample-package over real network to a temp dir, asserts not (test_dir / ".apm").exists() as a precondition, then runs the actual APMPackage.from_apm_yml + GitHubPackageDownloader.download_package chain. That's the closest we can get to "it works for users" without spinning up a fixture repo.

[danielmeppiel]

Cascade boundary coverage closed + UX fix surfaced

Following on from the test-coverage audit (previous comment), I ran the test-coverage-expert persona against all 9 leaves of the validation cascade diagram. The persona's ruthless prioritization identified one remaining high-value gap: the INVALID negative boundary (apm.yml present, no .apm/, no declared deps) — the fence of this entire rework, with no integration coverage.

Closing it surfaced a real UX bug.

What landed in 56688ff2

1. New boundary integration test — test_apm_yml_without_deps_or_apm_dir_rejects_as_invalid in tests/integration/test_apm_dependencies.py. Asserts the user-facing error message at the cascade fence (a) explicitly mentions .apm/ and (b) surfaces the dep-only escape hatch as a recovery path.

2. UX fix in validation.py:368-373 — the original cascade-exit message only listed two recovery paths:

Add .apm/ with primitives (instructions, skills, etc.) or add skills//SKILL.md for a skill bundle.

A user hitting the fence had no signal that declaring dependencies: in apm.yml is now the third valid path (the headline #1094 fix). The validator-level message at validation.py:747-752 already mentioned it; the cascade-exit message did not. Now aligned:

Add .apm/ with primitives (instructions, skills, etc.), declare dependencies in apm.yml (curated aggregator), or add skills//SKILL.md for a skill bundle.

Audit result for the other 8 leaves

Leaf	Outcome
MARKETPLACE_PLUGIN	Covered by units (deferred)
HYBRID	Silent-drift candidate — follow-up issue
CLAUDE_SKILL	Covered by units (deferred)
SKILL_BUNDLE	Real-GH live test exists
APM_PACKAGE w/ .apm/	Real-GH integration covered
APM_PACKAGE dep-only	Just added in 65e8a796
INVALID (apm.yml + no .apm/ + no deps)	Just added in 56688ff2 (this commit)
HOOK_PACKAGE	Covered by units — follow-up
INVALID (empty)	Covered by units (deferred)

Verification

• New integration test: PASSED against real microsoft/apm-sample-package (1.81s)
• Existing test_dep_only_project_installs_dependencies_without_dot_apm: PASSED
• Lint contract: clean
• Full unit sweep: 7516 passed (3 pre-existing env failures unrelated)

Co-authored with @edenfunf via xuyuanhao <aa9736195201@gmail.com> trailer.