Skip to content

fix(pack): select plugin sources by .apm presence#2122

Open
danielmeppiel wants to merge 11 commits into
mainfrom
fix/2054-pack-auto-includes
Open

fix(pack): select plugin sources by .apm presence#2122
danielmeppiel wants to merge 11 commits into
mainfrom
fix/2054-pack-auto-includes

Conversation

@danielmeppiel

@danielmeppiel danielmeppiel commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

fix(pack): select plugin sources by .apm presence

TL;DR

apm pack now treats .apm/ presence as the local source-authority switch instead of using includes: as a layout discriminator. Existing Claude-style root layouts remain packable after apm init writes includes: auto, while mixed layouts use .apm/ and emit actionable warnings. Explicit path lists remain exhaustive and now fail closed on missing, unsafe, symlinked, invalid-hook, or unpackable paths.

Note

Closes #2054. The approved design is captured in docs/superpowers/specs/2026-07-10-plugin-pack-source-layout-design.md.

Problem (WHY)

  • PR fix(pack): select plugin sources by .apm presence #2122 originally treated includes: auto as a signal to ignore root plugin convention folders, even though apm init writes that value without creating .apm/.
  • That made the common apm init -> apm pack journey stop packing an existing root skills/ directory.
  • Using publication consent to select a source layout conflated two independent contracts.
  • [!] Mixed .apm/ plus root layouts needed deterministic authority and visible migration guidance rather than silent merging.
  • [!] Explicit path lists needed to fail instead of falling back when a listed path was invalid.

The approved design defines the behavioral authority: .apm/ is exclusive when present; otherwise official plugin convention folders remain sources. Issue #2054 supplies the original accidental-publication reproduction that the authority rule must prevent.

Approach (WHAT)

# Fix
1 Make explicit includes: lists exhaustive and validate every listed source.
2 For auto or omitted includes, select .apm/ when present and root conventions otherwise.
3 Warn once per skipped mixed-layout source and explain how to resolve it.
4 Preserve dependency discovery independently from local source selection.
5 Keep root hooks under the same authority rule and leave root .mcp.json behavior unchanged.
6 Warn during apm init that existing plugin-native roots remain packable.

Implementation (HOW)

  • src/apm_cli/bundle/plugin_layout.py -- centralizes supported plugin-native root directories and excludes symlinked roots from discovery.
  • src/apm_cli/bundle/plugin_exporter.py -- implements the three collection branches, strict explicit-path validation, mixed-layout warnings, empty-.apm/ guidance, hook authority, and source-directory symlink hardening.
  • src/apm_cli/commands/init.py -- detects an existing native plugin layout after creating apm.yml and explains that apm pack still includes it.
  • tests/unit/test_plugin_exporter.py -- covers the layout matrix, strict lists, warnings, hooks, dependencies, malformed inputs, and direct or nested symlinks.
  • tests/unit/test_init_command.py -- proves init writes includes: auto, does not create .apm/, and emits the native-layout guidance.
  • tests/integration/test_pack_root_skills_e2e.py -- executes real apm init and apm pack subprocesses for both .apm/ authority and native Claude root layouts.
  • docs/src/content/docs/{quickstart.mdx,getting-started/first-package.md,concepts/package-anatomy.md,producer/pack-a-bundle.md,producer/repo-shapes.md,reference/cli/init.md,reference/cli/pack.md,reference/manifest-schema.md} -- documents consent, source authority, warnings, install discovery, and the init-to-pack journey.
  • packages/apm-guide/.apm/skills/apm-usage/package-authoring.md -- mirrors the package-authoring contract in the bundled usage guide.
  • CHANGELOG.md -- records the user-visible authority and compatibility fix.
  • docs/superpowers/{specs,plans}/2026-07-10-plugin-pack-source-layout*.md -- preserves the approved design and executable implementation plan.

Diagrams

Legend: the dashed nodes are the selection and validation behavior introduced by this PR; dependency collection remains outside this local-source decision.

flowchart LR
    M[apm.yml] --> E[export_plugin_bundle]
    E --> I{"includes is a list"}
    I -->|yes| X[validate listed paths]
    I -->|no| A{".apm directory exists"}
    A -->|yes| N[pack .apm sources]
    A -->|no| P[pack plugin root sources]
    X --> B[plugin bundle]
    N --> B
    P --> B
    D[lockfile dependencies] --> B
    classDef new stroke-dasharray: 5 5;
    class I,X,A,N,P new;
Loading

Trade-offs

  • Presence over declaration. Chose .apm/ existence as the authority switch; rejected includes: because consent must not silently change a repository's source layout.
  • Warn instead of fail on mixed layouts. Chose successful packing from .apm/ with actionable warnings; rejected a hard error to keep migration incremental.
  • Fail closed for explicit lists. Chose validation errors for missing, unsafe, symlinked, invalid, or unpackable entries; rejected implicit fallback because it would violate an exhaustive publication boundary.
  • Preserve native compatibility. Chose root convention discovery when .apm/ is absent; rejected forcing existing Claude plugin authors to restructure before adopting APM.
  • Keep dependency semantics stable. Local authority does not alter lockfile-attested dependency packing.

Benefits

  1. A native Claude project still packs its root skills/ after apm init writes includes: auto.
  2. A mixed project packs only .apm/ local content and names every skipped root source.
  3. An explicit list packs exactly its listed paths and produces a configuration error for invalid entries.
  4. Root hooks follow the same authority decision as other local primitives.
  5. Symlinked convention directories and nested explicit symlinks cannot enter a bundle.

Validation

Focused regression suite:

169 passed in 5.39s
Canonical full integration suite
=== 10223 passed, 172 skipped, 16 deselected, 2 xfailed in 377.19s (0:06:17) ===
Integration test suite passed (collected and ran via pytest discovery)
All integration tests completed successfully!
Canonical lint contract
All checks passed!
1424 files already formatted
YAML_FILE_LENGTH_RELATIVE_PATH_GUARDS_PASSED
Your code has been rated at 10.00/10
[+] auth-signal lint clean
ALL_LINT_GATES_PASSED

Mutation-break proof: restoring the old includes discriminator, disabling .apm/ authority, relaxing explicit-list validation, suppressing warnings, removing dependency merging, or allowing direct/nested symlinks caused the corresponding regression tests to fail before each mutation was restored.

Scenario Evidence

# Scenario (user promise) Principle(s) Test(s) proving it Type
1 apm init followed by apm pack preserves an existing Claude root skill Portability, DevX tests/integration/test_pack_root_skills_e2e.py::test_init_then_pack_preserves_native_claude_skill e2e
2 .apm/ wins in a mixed layout and root sources are excluded with warnings Governed by policy, DevX tests/integration/test_pack_root_skills_e2e.py::test_pack_auto_includes_only_apm_authored_skills; tests/unit/test_plugin_exporter.py::test_mixed_layout_emits_actionable_warnings e2e + unit
3 Explicit lists are exhaustive and invalid entries fail Governed by policy, Secure by default tests/unit/test_plugin_exporter.py::test_explicit_includes_are_exhaustive; test_missing_explicit_include_fails_pack; test_explicit_include_rejects_non_packable_path unit
4 Direct and nested symlinks never enter local package content Secure by default tests/unit/test_plugin_exporter.py::test_explicit_include_rejects_symlink; test_explicit_include_rejects_nested_directory_symlink; test_native_root_symlink_is_not_packed unit
5 Local source authority does not remove installed dependency primitives Portability by manifest tests/unit/test_plugin_exporter.py::test_apm_authority_preserves_dependency_components unit
6 Root hooks obey the same authority rule Governed by policy tests/unit/test_plugin_exporter.py::test_apm_dir_excludes_root_hook_config unit

How to test

  • Run uv run --extra dev pytest -q tests/unit/test_plugin_exporter.py tests/unit/test_init_command.py tests/integration/test_pack_root_skills_e2e.py; expect 169 passed.
  • Run bash scripts/test-integration.sh; expect 10223 passed and no failures.
  • Run the canonical lint chain; expect ruff, all three guards, pylint R0801, and auth-signals to pass.
  • In a project with root skills/demo/SKILL.md, run apm init -y then apm pack; expect the skill in the generated bundle.
  • Add .apm/skills/published/SKILL.md beside that root layout and pack again; expect only the .apm/ skill plus a skipped-root warning.

Closes #2054

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

Copilot AI review requested due to automatic review settings July 10, 2026 13:47

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes apm pack so that once a manifest declares includes: (e.g., includes: auto), packing no longer “discovers” and publishes plugin-shaped root directories like skills/; root convention discovery is preserved only for legacy manifests that omit includes:. It also adds an end-to-end regression test and updates docs/changelog to describe the clarified source-layout boundary.

Changes:

  • Gate root plugin convention directory collection in the plugin exporter on package.includes is None (legacy-only behavior).
  • Add an integration e2e regression test that packs a temp project containing both .apm/skills/ (publishable) and root skills/ (should be excluded when includes is declared).
  • Update CLI/docs guidance and add a changelog entry describing the behavior change.
Show a summary per file
File Description
src/apm_cli/bundle/plugin_exporter.py Stops collecting root plugin convention directories when includes: is declared, keeping root discovery only for legacy manifests.
tests/integration/test_pack_root_skills_e2e.py New hermetic e2e test asserting .apm/skills/ is packed while root skills/ is excluded under includes: auto.
tests/integration/test_intra_package_cleanup.py Updates an ownership precondition assertion to match current last-writer ownership semantics.
docs/src/content/docs/reference/cli/pack.md Documents the includes-vs-legacy root discovery boundary for apm pack plugin bundles.
packages/apm-guide/.apm/skills/apm-usage/package-authoring.md Aligns the bundled usage guide with the same .apm/<type>/ guidance and legacy behavior notes.
CHANGELOG.md Adds an Unreleased “Fixed” entry for the packing behavior change.

Review details

  • Files reviewed: 6/6 changed files
  • Comments generated: 4
  • Review effort level: Low

Comment on lines 283 to 285
assert [dep["name"] for dep in shared_claims] == ["pkg-b"], (
"The last package to deploy the shared prompt must own it before cleanup"
)
Comment thread CHANGELOG.md Outdated
Comment on lines +12 to +14
- `apm pack` with a declared `includes:` value no longer publishes local
root-level plugin convention directories such as `skills/`. (closes #2054,
#2122)
Comment on lines +5 to +7
import subprocess
import sys
from pathlib import Path
Comment on lines +10 to +34
def test_pack_auto_includes_only_apm_authored_skills(tmp_path: Path) -> None:
"""The real pack command must not treat a root skills directory as publishable."""
project = tmp_path / "project"
project.mkdir()
(project / "apm.yml").write_text(
"name: root-skills-test\n"
"version: 1.0.0\n"
"description: Root skills regression\n"
"license: MIT\n"
"target: claude\n"
"includes: auto\n"
"dependencies:\n"
" apm: []\n",
encoding="utf-8",
)

authored_skill = project / ".apm" / "skills" / "published"
authored_skill.mkdir(parents=True)
(authored_skill / "SKILL.md").write_text("# Published\n", encoding="utf-8")

local_skill = project / "skills" / "work-in-progress"
local_skill.mkdir(parents=True)
(local_skill / "SKILL.md").write_text("# Work in progress\n", encoding="utf-8")

apm_executable = Path(sys.executable).with_name("apm")
@danielmeppiel

Copy link
Copy Markdown
Collaborator Author

APM Review Panel: ship_with_followups

apm pack now treats declared includes as explicit consent to publish .apm content, closing a silent-publish footgun while preserving legacy behavior for manifests that omit it.

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

Panel signals converge cleanly: the behavioral change is narrow, tested through the real CLI with mutation evidence, and tightens the publication boundary. The architecture and test lenses found the guard surgical and adequately defended. The documentation lens found three canonical pages whose omitted-versus-declared wording still needed alignment.

Aligned with: Portable by manifest: includes is authoritative when declared. Secure by default: explicit scope prevents accidental root-directory publication. Pragmatic as npm: legacy behavior remains for omitted includes.

Panel summary

Persona B R N Takeaway
Python architect 0 0 2 Surgical guard is idiomatic and tested.
CLI logging expert 0 0 1 No default CLI output surface changed.
DevX UX expert 0 0 0 Persona return failed schema validation after retries.
Supply chain security expert 0 0 0 Prose confirmed a tighter publication boundary; structured return failed validation.
OSS growth hacker 0 0 0 Persona return failed schema validation after retries.
Doc writer 0 3 1 Canonical guides need omitted-versus-declared semantics.
Test coverage expert 0 0 1 Real CLI e2e proves the regression; direct unit coverage would add defense in depth.

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

Top 5 follow-ups

  1. [Doc writer] Update pack-a-bundle.md to clarify root plugin dirs are discovered only for legacy omitted-includes manifests.
  2. [Doc writer] Update repo-shapes.md with the same declared-includes boundary.
  3. [Doc writer] Update manifest-schema.md because omitted and auto now differ during packing.
  4. [Test coverage expert] Add a direct unit test for declared includes excluding root dirs.
  5. [Python architect] Encapsulate the declaration check on APMPackage.

Architecture

flowchart TD
    A[apm pack] --> B[Collect .apm primitives]
    B --> C{includes declared?}
    C -->|yes| D[Skip root convention dirs]
    C -->|no| E[Collect legacy root convention dirs]
    D --> F[Write plugin bundle]
    E --> F
Loading

Recommendation

Fold the five small, in-scope follow-ups, then re-run the panel against the converged diff.


Full per-persona findings

Python architect

  • [nit] Consider a semantic APMPackage helper for the declaration check.
  • [nit] Consider explaining the cleanup-test ownership precondition.

CLI logging expert

  • [nit] Consider a verbose breadcrumb when declared includes skips root convention dirs.

DevX UX expert

No structured findings survived schema validation.

Supply chain security expert

No structured findings survived schema validation; prose consistently described the change as security-positive.

OSS growth hacker

No structured findings survived schema validation.

Auth expert -- inactive

No auth, token, credential, or host-classification surface changed.

Doc writer

  • [recommended] Correct pack-a-bundle.md root-discovery wording.
  • [recommended] Correct repo-shapes.md root-discovery wording.
  • [recommended] Distinguish omitted and auto in manifest-schema.md.
  • [nit] Cross-link the pack reference to the canonical includes definition.

Test coverage expert

  • [nit] Add direct unit coverage for the inverse branch; the real CLI e2e already passes at the required tier.

Performance expert -- inactive

No package-manager performance hot path or performance claim changed.

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

@danielmeppiel danielmeppiel force-pushed the fix/2054-pack-auto-includes branch from d2ab380 to aac876b Compare July 10, 2026 17:27
@danielmeppiel

Copy link
Copy Markdown
Collaborator Author

APM Review Panel: ship_now

Pack includes gate ships clean: every active specialist returned zero findings, the real CLI regression mutation-breaks, and all local and hosted checks are green.

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

All active specialist signals converge. The declared-includes publication boundary is narrow and structurally sound, tightens what can enter a distributed artifact, preserves omitted-includes compatibility, and now has consistent documentation across every producer surface. The e2e test proves the user-visible command contract; direct unit coverage defends the branch locally.

Aligned with: Portable by manifest: declared includes determines what ships. Secure by default: undeclared root content cannot leak into the bundle. Pragmatic as npm: legacy omitted-includes behavior remains compatible.

Panel summary

Persona B R N Takeaway
Python architect 0 0 0 The semantic manifest helper and exporter guard are structurally sound.
CLI logging expert 0 0 0 No CLI logging or output regression.
DevX UX expert 0 0 0 The boundary matches the documented package-author mental model.
Supply chain security expert 0 0 0 The change tightens the publication boundary.
OSS growth hacker 0 0 0 No adoption or conversion concern remains.
Doc writer 0 0 0 Canonical docs and cross-links are consistent.
Test coverage expert 0 0 0 Real CLI e2e and direct unit coverage defend the contract.

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

Architecture

flowchart TD
    A[apm pack] --> B[Collect .apm primitives]
    B --> C{includes declared?}
    C -->|yes| D[Skip root convention dirs]
    C -->|no| E[Collect legacy root convention dirs]
    D --> F[Write plugin bundle]
    E --> F
Loading

Recommendation

All panelists are clear, all in-scope follow-ups are folded, and all evidence is green. Ship.

Folded in this run

  • (panel) Corrected canonical pack guide semantics -- resolved in 1685027.
  • (panel) Corrected repo-shapes omitted-versus-declared semantics -- resolved in aac876b.
  • (panel) Corrected manifest-schema omitted-versus-auto semantics -- resolved in 1685027.
  • (panel) Added direct unit coverage for declared includes excluding root dirs -- resolved in 1685027.
  • (panel) Encapsulated the manifest declaration check on APMPackage -- resolved in 1685027.
  • (panel) Split dense pack reference guidance and added canonical cross-links -- resolved in aac876b.

Copilot signals reviewed

Copilot produced no inline comments across two fetch rounds.

Regression-trap evidence (mutation-break gate)

  • tests/integration/test_pack_root_skills_e2e.py::test_pack_auto_includes_only_apm_authored_skills -- deleted if not package.has_declared_includes; test FAILED as expected; guard restored and test passed.

Lint contract

The full canonical lint chain passed post-rebase: ruff check, ruff format check, YAML I/O, 2100-line, portable-relpath, pylint R0801, and auth-signal guards.

CI

CI run 29111061959 and every PR check completed successfully after 0 CI fix iterations.

Mergeability status

PR head SHA CEO stance iters folds defers Copilot rounds CI mergeable mergeStateStatus notes
#2122 aac876b ship_now 2 6 0 2 green MERGEABLE BLOCKED awaiting required review

Convergence

2 outer iterations; 2 Copilot rounds. Final panel stance: ship_now.

Ready for maintainer review.


Full per-persona findings

All active panelists returned no findings. Auth and performance were inactive because this PR does not touch their surfaces.

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

danielmeppiel and others added 6 commits July 10, 2026 21:37
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Fold the panel follow-ups by documenting omitted-versus-declared includes semantics and adding direct unit coverage for the exporter branch. This also names the manifest intent through APMPackage instead of repeating a storage-level None check. Addresses panel doc-writer, test-coverage, and python-architect follow-ups.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Remove the remaining unconditional root-layout claim and link the packing guidance to the canonical includes contract. Addresses the final doc-writer and growth panel findings.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Separate source-layout detection from publication consent so native plugin repositories can adopt APM without silently dropping convention-folder content.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@danielmeppiel

Copy link
Copy Markdown
Collaborator Author

APM Review Panel: needs_rework

PR #2122 fixes pack scope leak (#2054), but the implementation must use the approved .apm/-presence authority model rather than an includes-field discriminator.

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

The panel converges on the approved design: .apm/ presence selects the APM-native source layout; its absence preserves official Claude plugin root conventions, including after apm init writes includes: auto. The current has_declared_includes guard conflates publication consent with source-layout selection and must be replaced.

The DevX concern about native Claude plugins is valid but already resolved by the design: root skills/, agents/, commands/, and hooks remain sources whenever .apm/ is absent. When .apm/ exists, it is exclusively authoritative; mixed layouts succeed with actionable warnings. Explicit include lists remain exhaustive, invalid paths fail, dependency discovery remains independent, and root hook sources follow the same authority rule.

Dissent. DevX questioned .apm/ as pack authority. The approved presence-based boundary wins because it preserves native layouts before adoption while preventing silent mixed-tree publication after adoption.

Aligned with: portable manifests through separate consent and layout concepts; secure defaults through deterministic source authority; pragmatic compatibility with official plugin-native layouts; actionable migration UX.

Growth signal. The release story is simple: apm init does not break an existing Claude plugin, while creating .apm/ deliberately opts into a safer publication boundary.

Panel summary

Persona B R N Takeaway
Python Architect 0 0 2 The presence-based design is deterministic; implementation must replace the old guard.
CLI Logging Expert 1 3 1 Mixed and empty APM-native layouts need actionable warnings through the canonical logger path.
DevX UX Expert 0 5 1 Preserve native Claude init-to-pack behavior and make source authority explainable.
Supply Chain Security Expert 0 2 1 Separate publication consent from source authority and validate explicit paths.
OSS Growth Hacker 0 2 2 Lead docs and changelog with the user outcome and progressive adoption path.
Doc Writer 0 5 0 Correct includes-based claims and document hooks, dependencies, and strict explicit lists.
Test Coverage Expert 2 3 0 Add user-visible init-to-pack and .apm/-authority regression traps.

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] (blocking-severity) Add an e2e test that executes apm init and then apm pack on a native Claude root skills/ layout, proving includes: auto preserves the skill while .apm/ remains absent.
  2. [CLI Logging Expert] (blocking-severity) Emit one actionable warning per skipped root source when .apm/ is present, plus the empty-APM-source warning.
  3. [Supply Chain Security Expert] Replace has_declared_includes with .apm/-presence authority; keep explicit lists exhaustive and fail invalid paths.
  4. [Test Coverage Expert] Add complementary .apm/-authoritative, root hooks, dependency preservation, explicit-list, invalid-path, and warning tests.
  5. [Doc Writer] Update the maintained docs and agent guide to describe layout authority independently from publication consent.

Recommendation

Implement the approved presence-based model before shipping. Re-run the panel after the discriminator, warnings, strict explicit-list handling, e2e proof, unit matrix, docs, changelog, mutation-break evidence, full integration suite, and canonical lint contract are complete.


Full per-persona findings

Python Architect

  • [nit] The current diagrams and has_declared_includes property describe the superseded implementation, not the approved authority model.
  • [nit] Keep the e2e binary invocation consistent with repository integration conventions.

CLI Logging Expert

  • [blocking] Root convention sources are currently skipped silently.
  • [recommended] Correct the changelog discriminator.
  • [recommended] Warn when .apm/ is authoritative but contains no local primitives.
  • [recommended] Route new warnings through the existing logger-or-rich-warning path.
  • [nit] Rename tests and docstrings around .apm/ presence rather than declared includes.

DevX UX Expert

  • [recommended] Preserve plugin-native roots when .apm/ is absent, including after apm init writes includes: auto.
  • [recommended] Make mixed-layout migration succeed with precise warnings.
  • [recommended] Keep explicit lists exhaustive and invalid paths actionable.
  • [recommended] Avoid any implicit file moves during init or pack.
  • [recommended] Prove the user journey through the real CLI.

Supply Chain Security Expert

  • [recommended] Replace the includes discriminator with .apm/ presence.
  • [recommended] Warn on every skipped mixed-layout source.
  • [nit] Fail explicit include paths that do not exist.

OSS Growth Hacker

  • [recommended] Frame the changelog around preserving existing Claude plugins and preventing accidental publication.
  • [recommended] Ensure the pack guide metadata does not imply .apm/ is mandatory.
  • [nit] Include the remove-.apm/ escape hatch in the empty-source warning.
  • [nit] Add a concise native-plugin onboarding callout.

Auth Expert -- inactive

No authentication, token, credential, or host-classification surface is involved.

Doc Writer

  • [recommended] Replace every includes-based source-layout claim.
  • [recommended] Document explicit-list strictness, deliberate root paths, and invalid-path failure.
  • [recommended] Cover root hook authority and dependency independence.
  • [recommended] Correct first-package onboarding for apm init plus native roots.
  • [recommended] Keep canonical ownership by topic and link rather than duplicate prose.

Test Coverage Expert

  • [blocking] The user-visible native Claude apm init then apm pack regression trap is missing.
  • [blocking] .apm/-presence authority lacks direct regression coverage.
  • [recommended] Mixed-layout warning assertions are missing.
  • [recommended] Invalid explicit include failure coverage is missing.
  • [recommended] Root hook authority coverage is missing.

Performance Expert -- inactive

No dependency transport, cache, materialization, parallelism, or performance-claim surface is involved.

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

danielmeppiel and others added 5 commits July 10, 2026 21:52
Separates publication consent from local source authority and folds the design panel source-selection, warning, hooks, dependency, and explicit-list follow-ups.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Proves through the real CLI that apm init writes includes: auto without changing a Claude-native root layout, and explains that migration path during init.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@danielmeppiel danielmeppiel force-pushed the fix/2054-pack-auto-includes branch from aac876b to 5f07a42 Compare July 10, 2026 20:50
@danielmeppiel danielmeppiel changed the title fix: respect includes during plugin packing (closes #2054) fix(pack): select plugin sources by .apm presence Jul 10, 2026
@danielmeppiel

Copy link
Copy Markdown
Collaborator Author

APM Review Panel: ship_now

Establishes presence-based source authority for plugin convention folders, enabling zero-migration Claude plugin pack journeys while preserving .apm/ as the authoritative override.

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

All panelists converge without dissent. The architecture review confirms that source authority is isolated cleanly and explicit validation fails closed. Security review verified that implicit and explicit pack paths reject symlinks, traversal, invalid hooks, and unpackable sources. Test review verified every critical authority boundary with passing regression traps and mutation-break evidence: 169 focused tests and 10,223 integration tests passed.

This change honors the official Claude plugin convention rather than forcing a migration: root folders remain sources when .apm/ is absent, including after apm init writes includes: auto. .apm/ presence remains the authoritative override, explicit lists remain exhaustive, mixed layouts warn, dependencies remain independent, and root hooks follow the same authority rule.

Aligned with: Portability by manifest: native plugin roots remain portable. Secure by default: explicit paths fail closed. Governed by policy: explicit lists are exhaustive and mixed layouts do not silently merge. Pragmatic as npm: existing projects work without restructuring.

Growth signal. Existing Claude plugin authors can adopt APM without moving files before their first successful pack.

Panel summary

Persona B R N Takeaway
Python Architect 0 0 0 Presence-based authority is isolated cleanly, explicit validation fails closed, and shared conventions avoid drift.
CLI Logging Expert 0 0 0 Warnings and errors are actionable, ASCII-safe, and routed through CommandLogger.
DevX UX Expert 0 0 0 The init-to-pack journey, recovery wording, and documentation are clear.
Supply Chain Security Expert 0 0 0 Explicit and implicit paths fail closed against unsafe or invalid sources.
OSS Growth Hacker 0 0 0 The zero-migration Claude plugin journey supports adoption.
Doc Writer 0 0 0 Docs distinguish install consent from exhaustive plugin-pack selection.
Test Coverage Expert 0 0 0 Critical behavior has passing regression traps with mutation-break proof.

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

Architecture

classDiagram
  APMPackage --> PluginExporter : includes
  PluginExporter --> PluginLayout : root conventions
  PluginExporter --> PathSecurity : validates explicit paths
  InitCommand --> PluginLayout : detects native sources
Loading
flowchart TD
  Pack[apm pack] --> Includes{explicit list?}
  Includes -->|yes| Explicit[validate and pack listed paths]
  Includes -->|no| Apm{.apm present?}
  Apm -->|yes| Native[pack .apm and warn on root sources]
  Apm -->|no| Plugin[pack plugin-native root sources]
Loading

Recommendation

All panelists report zero findings. Test evidence is comprehensive and mutation-proven. The implementation honors existing conventions, fails closed at security boundaries, and preserves explicit-list governance. Ship now with no follow-ups.


Full per-persona findings

Python Architect

No findings.

CLI Logging Expert

No findings.

DevX UX Expert

No findings.

Supply Chain Security Expert

No findings.

OSS Growth Hacker

No findings.

Auth Expert -- inactive

Pack and init behavior only; no authentication surface is touched.

Doc Writer

No findings.

Test Coverage Expert

No findings.

Performance Expert -- inactive

Local pack-time source selection only; no performance-critical surface is touched.

This panel is advisory. Re-apply the panel-review label for another pass.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] apm pack automatically includes a local root-level skills/ directory in the plugin bundle when includes is set to auto in apm.yml

2 participants