From 006daf338ae41541a6706d84cfe63631b9298eeb Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Fri, 10 Jul 2026 15:45:33 +0200 Subject: [PATCH 01/11] fix: respect includes during plugin packing Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- CHANGELOG.md | 5 ++ docs/src/content/docs/reference/cli/pack.md | 2 +- .../skills/apm-usage/package-authoring.md | 16 +++--- src/apm_cli/bundle/plugin_exporter.py | 5 +- .../integration/test_pack_root_skills_e2e.py | 49 +++++++++++++++++++ 5 files changed, 66 insertions(+), 11 deletions(-) create mode 100644 tests/integration/test_pack_root_skills_e2e.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 674411a76..6730e83a2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Azure DevOps marketplace checks now preserve suffix-free `/_git/` URLs and pass Azure CLI bearer authentication through to `git ls-remote`. (closes #2119) +### Fixed + +- `apm pack` with a declared `includes:` value no longer publishes local + root-level plugin convention directories such as `skills/`. (closes #2054) + ## [0.24.1] - 2026-07-10 ### Fixed diff --git a/docs/src/content/docs/reference/cli/pack.md b/docs/src/content/docs/reference/cli/pack.md index 4068dff5d..457904c2d 100644 --- a/docs/src/content/docs/reference/cli/pack.md +++ b/docs/src/content/docs/reference/cli/pack.md @@ -109,7 +109,7 @@ apm pack --archive --dry-run -v A Claude Code plugin directory under `--output`. Contains: - `plugin.json` -- schema-conformant manifest. Convention-dir keys are stripped because Claude Code auto-discovers them. -- Plugin-native subdirs populated from your `.apm/` content and from installed dependencies: `agents/`, `skills/`, `commands/`, `instructions/`, `hooks/`, `extensions/` (canvas extensions, when the `canvas` experimental flag is enabled). +- Plugin-native subdirs populated from your `.apm/` content and from installed dependencies: `agents/`, `skills/`, `commands/`, `instructions/`, `hooks/`, `extensions/` (canvas extensions, when the `canvas` experimental flag is enabled). With `includes: auto` or an explicit `includes:` list, root convention directories such as `skills/` are not package content; put publishable local primitives under `.apm/`. Manifests that omit `includes:` retain legacy root convention discovery for plugin-native projects. - Installed dependencies are packed exclusively from lockfile-attested `deployed_files`; the `apm_modules` cache is never packed (it has no provenance or integrity guarantee). Each attested file is verified against its `deployed_file_hashes` SHA-256 before inclusion. - If the dependency declares `skills:`, only the named skills are included; the cache cannot add extras. - If a dependency has cached primitives but no `deployed_files`, `apm pack` fails and tells you to run `apm install`. diff --git a/packages/apm-guide/.apm/skills/apm-usage/package-authoring.md b/packages/apm-guide/.apm/skills/apm-usage/package-authoring.md index 4272dc7b2..729d7952b 100644 --- a/packages/apm-guide/.apm/skills/apm-usage/package-authoring.md +++ b/packages/apm-guide/.apm/skills/apm-usage/package-authoring.md @@ -62,12 +62,12 @@ my-package/ ## Install-time discovery rules -`apm pack` (export) is liberal: it collects primitives from both -`.apm//` and root convention directories (`agents/`, `skills/`, -`instructions/`, etc.). `apm install` (integration) is per-primitive -and stricter. Authors who rely on root convention directories for -instructions or prompts will produce bundles that pack but install -silently incomplete. +`apm pack` collects declared local primitives from `.apm//`. +When `apm.yml` sets `includes: auto` or an explicit `includes:` list, +root convention directories (`agents/`, `skills/`, `instructions/`, +etc.) are not package content. Manifests that omit `includes:` retain +legacy root convention discovery for plugin-native projects. Prefer +`.apm//` so pack and install use the same source layout. Per-primitive scan paths for `apm install`: @@ -81,9 +81,7 @@ Per-primitive scan paths for `apm install`: **Recommendation for marketplace publishers:** use `.apm//` for every primitive. This is the only layout that is symmetric between -`apm pack` and `apm install`. Authoring `instructions/` at the plugin -root will pack cleanly but instructions will be silently dropped when -consumers run `apm install`. +`apm pack` and `apm install`. ## Hook files diff --git a/src/apm_cli/bundle/plugin_exporter.py b/src/apm_cli/bundle/plugin_exporter.py index 1c3b69ec2..4bba3866a 100644 --- a/src/apm_cli/bundle/plugin_exporter.py +++ b/src/apm_cli/bundle/plugin_exporter.py @@ -768,7 +768,10 @@ def export_plugin_bundle( # 6. Collect own components (.apm/ and root-level) own_apm_dir = project_root / ".apm" own_components = _collect_apm_components(own_apm_dir) - own_components.extend(_collect_root_plugin_components(project_root)) + # An includes declaration scopes local publication to .apm/. Preserve + # root convention discovery only for legacy plugin-native manifests. + if package.includes is None: + own_components.extend(_collect_root_plugin_components(project_root)) _merge_file_map(file_map, own_components, pkg_name, force, collisions) # Hooks -- root package wins on key collision diff --git a/tests/integration/test_pack_root_skills_e2e.py b/tests/integration/test_pack_root_skills_e2e.py new file mode 100644 index 000000000..b8cb56e65 --- /dev/null +++ b/tests/integration/test_pack_root_skills_e2e.py @@ -0,0 +1,49 @@ +"""Hermetic end-to-end coverage for root-level plugin component packing.""" + +from __future__ import annotations + +import subprocess +import sys +from pathlib import Path + + +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") + result = subprocess.run( + [str(apm_executable), "pack"], + cwd=project, + capture_output=True, + text=True, + timeout=60, + check=False, + ) + + assert result.returncode == 0, ( + f"apm pack failed:\nstdout:\n{result.stdout}\nstderr:\n{result.stderr}" + ) + bundle = project / "build" / "root-skills-test-1.0.0" + assert (bundle / "skills" / "published" / "SKILL.md").is_file() + assert not (bundle / "skills" / "work-in-progress").exists() From 34dc95ee105c50f5d2613dbb835f5c04921bb84a Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Fri, 10 Jul 2026 15:47:11 +0200 Subject: [PATCH 02/11] docs: link pack fix changelog entry Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6730e83a2..f8b19b27c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,7 +27,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - `apm pack` with a declared `includes:` value no longer publishes local - root-level plugin convention directories such as `skills/`. (closes #2054) + root-level plugin convention directories such as `skills/`. (closes #2054, + #2122) ## [0.24.1] - 2026-07-10 From 707e80e3aeb9275fe1e29a951ed9db24a97bb4f6 Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Fri, 10 Jul 2026 19:06:20 +0200 Subject: [PATCH 03/11] docs: align pack include boundaries 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> --- docs/src/content/docs/producer/pack-a-bundle.md | 6 ++++-- docs/src/content/docs/producer/repo-shapes.md | 10 ++++++---- docs/src/content/docs/reference/manifest-schema.md | 5 ++++- src/apm_cli/bundle/plugin_exporter.py | 2 +- src/apm_cli/models/apm_package.py | 5 +++++ tests/unit/test_plugin_exporter.py | 12 ++++++++++++ 6 files changed, 32 insertions(+), 8 deletions(-) diff --git a/docs/src/content/docs/producer/pack-a-bundle.md b/docs/src/content/docs/producer/pack-a-bundle.md index 1574fe181..5896b6a1c 100644 --- a/docs/src/content/docs/producer/pack-a-bundle.md +++ b/docs/src/content/docs/producer/pack-a-bundle.md @@ -180,8 +180,10 @@ Source: `src/apm_cli/integration/instruction_integrator.py`, ### Canonical layout for marketplace publishers :::caution[Silent install drops can remove intended guardrails] -`apm pack` accepts primitives from both `.apm//` and root convention -directories (for example, an `instructions/` folder at the plugin root). +`apm pack` always accepts primitives from `.apm//`. It also accepts root +convention directories (for example, an `instructions/` folder at the plugin +root) only for legacy manifests that omit `includes:`. Declaring +`includes: auto` or an explicit list scopes local package content to `.apm/`. `apm install` does NOT discover instructions, commands, or prompts placed in root convention directories. Packages that rely on these primitives for security guardrails or policy enforcement will install silently incomplete, diff --git a/docs/src/content/docs/producer/repo-shapes.md b/docs/src/content/docs/producer/repo-shapes.md index 2763aded8..c859d039a 100644 --- a/docs/src/content/docs/producer/repo-shapes.md +++ b/docs/src/content/docs/producer/repo-shapes.md @@ -134,10 +134,12 @@ my-monorepo/ ``` > **Important -- use `.apm//` for every primitive in each plugin.** -> `apm pack` accepts primitives from both `.apm//` and root -> convention directories (e.g. `instructions/` at the plugin root), but -> `apm install` only discovers instructions, commands, and prompts under -> `.apm//`. Authoring `packages/plugin-a/instructions/style.instructions.md` +> `apm pack` always accepts primitives from `.apm//`. Root convention +> directories (e.g. `instructions/` at the plugin root) are also accepted only +> for legacy manifests that omit `includes:`; declaring `includes: auto` or an +> explicit list scopes local package content to `.apm/`. `apm install` only +> discovers instructions, commands, and prompts under `.apm//`. Authoring +> `packages/plugin-a/instructions/style.instructions.md` > instead of `packages/plugin-a/.apm/instructions/style.instructions.md` > will produce a bundle that packs correctly but installs silently > incomplete. See [Pack a bundle -- source layout and install-time diff --git a/docs/src/content/docs/reference/manifest-schema.md b/docs/src/content/docs/reference/manifest-schema.md index 051a79cd6..0ac62956c 100644 --- a/docs/src/content/docs/reference/manifest-schema.md +++ b/docs/src/content/docs/reference/manifest-schema.md @@ -230,7 +230,10 @@ scripts: Declares which local `.apm/` content the project consents to publish when packing or deploying. Three forms are supported: -1. **Undeclared** (field omitted). Legacy behaviour: all local `.apm/` content is published as if `auto` were set. `apm audit` emits an `includes-consent` advisory whenever local content is deployed under this form. +1. **Undeclared** (field omitted). Legacy behaviour: all local `.apm/` content + is published and `apm pack` also discovers root plugin convention + directories. `apm audit` emits an `includes-consent` advisory whenever local + content is deployed under this form. 2. **`includes: auto`**. Explicit consent to publish all local `.apm/` content via the file scanner. No path enumeration required. Default for newly initialised projects. 3. **`includes: [, ...]`**. Explicit allow-list of paths the project consents to publish. Strongest governance form; changes are reviewable in PR diffs. diff --git a/src/apm_cli/bundle/plugin_exporter.py b/src/apm_cli/bundle/plugin_exporter.py index 4bba3866a..f4b912491 100644 --- a/src/apm_cli/bundle/plugin_exporter.py +++ b/src/apm_cli/bundle/plugin_exporter.py @@ -770,7 +770,7 @@ def export_plugin_bundle( own_components = _collect_apm_components(own_apm_dir) # An includes declaration scopes local publication to .apm/. Preserve # root convention discovery only for legacy plugin-native manifests. - if package.includes is None: + if not package.has_declared_includes: own_components.extend(_collect_root_plugin_components(project_root)) _merge_file_map(file_map, own_components, pkg_name, force, collisions) diff --git a/src/apm_cli/models/apm_package.py b/src/apm_cli/models/apm_package.py index 7a4e6adcf..9d02d45cc 100644 --- a/src/apm_cli/models/apm_package.py +++ b/src/apm_cli/models/apm_package.py @@ -275,6 +275,11 @@ class APMPackage: # to boolean (e.g. ``{"owner/repo#v1.0": {"hooks": true}}``). allow_executables: dict[str, dict[str, bool]] | None = None + @property + def has_declared_includes(self) -> bool: + """Return whether the manifest explicitly declares ``includes``.""" + return self.includes is not None + @classmethod def _parse_dependency_dict(cls, raw_deps: dict, label: str = "") -> dict: """Parse a dependencies or devDependencies dict from apm.yml. diff --git a/tests/unit/test_plugin_exporter.py b/tests/unit/test_plugin_exporter.py index 0a63b7c35..fa59b7ed2 100644 --- a/tests/unit/test_plugin_exporter.py +++ b/tests/unit/test_plugin_exporter.py @@ -1212,6 +1212,18 @@ def test_root_level_plugin_dirs_collected(self, tmp_path): result = export_plugin_bundle(project, out) assert (result.bundle_path / "agents" / "root-bot.agent.md").exists() + def test_declared_includes_excludes_root_level_plugin_dirs(self, tmp_path): + """Declared includes scopes plugin content to the .apm directory.""" + project = _setup_plugin_project(tmp_path) + _write_apm_yml(project, extra={"includes": "auto"}) + root_agents = project / "agents" + root_agents.mkdir() + (root_agents / "root-bot.agent.md").write_text("root bot") + + result = export_plugin_bundle(project, tmp_path / "build") + + assert not (result.bundle_path / "agents" / "root-bot.agent.md").exists() + class TestExportPluginBundleViaPackBundle: """Verify pack_bundle(fmt='plugin') delegates correctly.""" From c0cf87fbe633d677041a67aab02150e196dfe44c Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Fri, 10 Jul 2026 19:11:36 +0200 Subject: [PATCH 04/11] docs: clarify legacy pack behavior 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> --- docs/src/content/docs/producer/pack-a-bundle.md | 2 ++ docs/src/content/docs/producer/repo-shapes.md | 4 ++-- docs/src/content/docs/reference/cli/pack.md | 4 +++- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/docs/src/content/docs/producer/pack-a-bundle.md b/docs/src/content/docs/producer/pack-a-bundle.md index 5896b6a1c..f6db29bbf 100644 --- a/docs/src/content/docs/producer/pack-a-bundle.md +++ b/docs/src/content/docs/producer/pack-a-bundle.md @@ -184,6 +184,8 @@ Source: `src/apm_cli/integration/instruction_integrator.py`, convention directories (for example, an `instructions/` folder at the plugin root) only for legacy manifests that omit `includes:`. Declaring `includes: auto` or an explicit list scopes local package content to `.apm/`. +See the [`includes` manifest field](../../reference/manifest-schema/#39-includes) +for all three forms. `apm install` does NOT discover instructions, commands, or prompts placed in root convention directories. Packages that rely on these primitives for security guardrails or policy enforcement will install silently incomplete, diff --git a/docs/src/content/docs/producer/repo-shapes.md b/docs/src/content/docs/producer/repo-shapes.md index c859d039a..ffc95ec9c 100644 --- a/docs/src/content/docs/producer/repo-shapes.md +++ b/docs/src/content/docs/producer/repo-shapes.md @@ -141,8 +141,8 @@ my-monorepo/ > discovers instructions, commands, and prompts under `.apm//`. Authoring > `packages/plugin-a/instructions/style.instructions.md` > instead of `packages/plugin-a/.apm/instructions/style.instructions.md` -> will produce a bundle that packs correctly but installs silently -> incomplete. See [Pack a bundle -- source layout and install-time +> can produce a bundle only under the legacy omitted-`includes` behavior, and +> then installs silently incomplete. See [Pack a bundle -- source layout and install-time > discovery](./pack-a-bundle/#source-layout-and-install-time-discovery) > for the full per-primitive scan-path reference. diff --git a/docs/src/content/docs/reference/cli/pack.md b/docs/src/content/docs/reference/cli/pack.md index 457904c2d..bde62f068 100644 --- a/docs/src/content/docs/reference/cli/pack.md +++ b/docs/src/content/docs/reference/cli/pack.md @@ -109,7 +109,9 @@ apm pack --archive --dry-run -v A Claude Code plugin directory under `--output`. Contains: - `plugin.json` -- schema-conformant manifest. Convention-dir keys are stripped because Claude Code auto-discovers them. -- Plugin-native subdirs populated from your `.apm/` content and from installed dependencies: `agents/`, `skills/`, `commands/`, `instructions/`, `hooks/`, `extensions/` (canvas extensions, when the `canvas` experimental flag is enabled). With `includes: auto` or an explicit `includes:` list, root convention directories such as `skills/` are not package content; put publishable local primitives under `.apm/`. Manifests that omit `includes:` retain legacy root convention discovery for plugin-native projects. +- Plugin-native subdirs populated from your `.apm/` content and from installed dependencies: `agents/`, `skills/`, `commands/`, `instructions/`, `hooks/`, `extensions/` (canvas extensions, when the `canvas` experimental flag is enabled). + - With [`includes: auto` or an explicit `includes:` list](../manifest-schema/#39-includes), root convention directories such as `skills/` are not package content; put publishable local primitives under `.apm/`. + - Manifests that omit `includes:` retain legacy root convention discovery for plugin-native projects. - Installed dependencies are packed exclusively from lockfile-attested `deployed_files`; the `apm_modules` cache is never packed (it has no provenance or integrity guarantee). Each attested file is verified against its `deployed_file_hashes` SHA-256 before inclusion. - If the dependency declares `skills:`, only the named skills are included; the cache cannot add extras. - If a dependency has cached primitives but no `deployed_files`, `apm pack` fails and tells you to run `apm install`. From 2e252992988afa139f0e06ab2d60c418bb19cb5e Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Fri, 10 Jul 2026 21:23:04 +0200 Subject: [PATCH 05/11] docs: define plugin pack source authority 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> --- ...-07-10-plugin-pack-source-layout-design.md | 219 ++++++++++++++++++ 1 file changed, 219 insertions(+) create mode 100644 docs/superpowers/specs/2026-07-10-plugin-pack-source-layout-design.md diff --git a/docs/superpowers/specs/2026-07-10-plugin-pack-source-layout-design.md b/docs/superpowers/specs/2026-07-10-plugin-pack-source-layout-design.md new file mode 100644 index 000000000..a92064917 --- /dev/null +++ b/docs/superpowers/specs/2026-07-10-plugin-pack-source-layout-design.md @@ -0,0 +1,219 @@ +# Plugin Pack Source Layout Design + +Date: 2026-07-10 +Status: Approved +Related: issue #2054, PR #2122 + +## Decision + +`apm pack` uses the presence of the project-root `.apm/` directory as +the source-layout switch: + +- When `.apm/` exists, `.apm/` is the authoritative local primitive + source. Root convention directories are not packed implicitly. +- When `.apm/` does not exist, supported root convention directories + remain authoritative and are packed implicitly. +- An explicit `includes` list is exhaustive regardless of layout. +- `includes: auto` grants publication consent but does not select the + source layout. + +When both `.apm/` and supported root convention directories exist, +`.apm/` wins. Packing succeeds and emits an actionable warning for +each skipped root directory. + +## Problem + +Issue #2054 demonstrates a publication-boundary bug: an APM-authored +repository can contain an unrelated root `skills/` working directory, +and `apm pack` currently includes it in a plugin bundle. + +PR #2122 fixes that symptom by skipping root convention directories +whenever `includes` is declared. That discriminator is incorrect +because `includes` represents publication consent, not source layout. + +Official Claude plugin documentation defines `skills/`, `agents/`, +`commands/`, and `hooks/` at the plugin root as native authoring +locations: + +https://code.claude.com/docs/en/plugins + +A Claude plugin author can therefore have valid publishable root +content, run `apm init`, receive `includes: auto`, and then silently +lose that content from `apm pack`. APM must support progressive +adoption without making vendor-native content disappear. + +## Goals + +1. Prevent unrelated root convention directories from leaking into + bundles after a project adopts `.apm/`. +2. Preserve native Claude plugin behavior before `.apm/` adoption. +3. Keep `includes` semantics independent from source-layout detection. +4. Make mixed-layout behavior deterministic and explainable. +5. Support gradual migration from plugin-native layout to `.apm/`. + +## Non-goals + +- Merge `.apm/` and root convention trees implicitly. +- Add a `source_layout` field to `apm.yml`. +- Change dependency package discovery. +- Move files automatically during `apm init` or `apm pack`. +- Change README claims without explicit maintainer approval. + +## Behavioral Contract + +| Includes | `.apm/` | Root convention dirs | Implicit root packing | +|----------|---------|----------------------|-----------------------| +| omitted | absent | present | yes | +| `auto` | absent | present | yes | +| omitted | present | absent | no | +| `auto` | present | absent | no | +| omitted | present | present | no, with warning | +| `auto` | present | present | no, with warning | +| explicit list | either | either | no; only listed paths | + +Supported root convention directories remain the set already recognized +by plugin packing. This decision changes when that existing discovery +logic runs; it does not expand the recognized set. + +## Precedence + +Packing resolves local content in this order: + +1. If `includes` is an explicit path list, pack only those paths. +2. Otherwise, detect whether the project-root `.apm/` directory exists. +3. If `.apm/` exists, collect local primitives from `.apm/` and skip + implicit root convention discovery. +4. If `.apm/` does not exist, collect local primitives from supported + root convention directories. +5. Continue using existing dependency component discovery independently + of the root project's source-layout decision. + +Directory presence is intentional. An empty `.apm/` directory indicates +that the author has switched to APM-native layout. This prevents +half-migrated root content from being merged silently. + +## Mixed-layout UX + +When `.apm/` and one or more supported root convention directories both +exist, packing succeeds and writes one warning per skipped directory to +stderr: + +```text +[!] Skipping root-level skills/ because .apm/ is present. + Move publishable files to .apm/skills/ or remove skills/ to silence + this warning. +``` + +The warning names: + +1. the skipped directory, +2. the cause, +3. the next action. + +A mixed layout is not an error because migration can occur incrementally. +Implicitly merging both trees is rejected because collision provenance +would become difficult to predict and debug. + +If packing finds no local primitives after applying the source-layout +rule, it emits: + +```text +[!] No local primitives found. Expected content under .apm/. + Check the project layout or move plugin-native content into .apm/. +``` + +## `apm init` Onboarding + +`apm init` must not make an existing native Claude plugin stop packing. + +When root convention directories exist and `.apm/` does not: + +1. `apm init` may continue writing `includes: auto`. +2. It does not create `.apm/` implicitly. +3. It reports that native root directories remain pack sources. +4. It explains how to opt into `.apm/` by moving content. + +Suggested output: + +```text +[!] Found plugin-native directories at the project root: skills/. + They remain included by apm pack. Move them to .apm/skills/ when + adopting the APM source layout. +``` + +## Error Handling + +An explicit `includes` path that does not exist is a configuration +error, not a layout warning: + +```text +[x] includes path '.apm/skills/example' does not exist. + Fix the path in apm.yml or create it. +``` + +The error names the failure, cause, and next action. No implicit fallback +to root convention discovery occurs for an invalid explicit list. + +## Compatibility + +This design preserves: + +- plugin-native repositories with omitted `includes`, +- plugin-native repositories after `apm init` adds `includes: auto`, +- APM-native repositories that author under `.apm/`, +- explicit allow-list behavior, +- dependency package convention discovery. + +The intentional behavior change is for mixed repositories: once `.apm/` +exists, root convention directories stop being implicit pack sources and +produce warnings. Authors can still include a root path through an +explicit list when that is a deliberate publication decision. + +## Acceptance Tests + +1. `includes: auto`, `.apm/skills/published/`, and root `skills/wip/` + packs only the `.apm/` skill. +2. `includes: auto`, no `.apm/`, and root `skills/published/` packs the + root skill. +3. Omitted `includes`, `.apm/skills/published/`, and root `skills/wip/` + packs only the `.apm/` skill. +4. Omitted `includes`, no `.apm/`, and root `skills/published/` packs + the root skill. +5. An explicit list packs only listed paths with or without `.apm/`. +6. `.apm/` plus root `skills/` emits a warning naming `skills/`, the + `.apm/` cause, and a move-or-remove action. +7. `apm init` in a Claude-native plugin leaves root skills packable. +8. Creating `.apm/` after initialization switches authority and causes + root skills to be skipped with a warning. +9. An invalid explicit path fails with the path and a corrective action. +10. Dependency package root conventions continue to be discovered. + +The e2e regression trap must execute the user-visible `apm init` and +`apm pack` flow for a native Claude plugin. Mutation-break proof removes +the `.apm/` presence guard and confirms that the test fails. + +## Documentation Impact + +PR #2122 documentation must describe the conditional rule: + +> When `.apm/` exists, local primitive content is sourced from `.apm/`. +> Without `.apm/`, supported plugin-native root directories remain pack +> sources. + +The affected Starlight pages and APM usage guidance include: + +- producer pack guidance, +- repository shape guidance, +- pack CLI reference, +- manifest schema reference, +- first-package onboarding, +- package-authoring agent guidance. + +README drift, if any, requires separate explicit maintainer approval. + +## PR #2122 Disposition + +PR #2122 should be revised before merge. Its publication-safety goal is +correct, but `has_declared_includes` must not select the source layout. +The revised change should use `.apm/` presence, add mixed-layout +warnings, and cover the native-plugin-after-`apm init` journey. From a9622a8cc52ac2593f6988d385f3d24ee76f05dc Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Fri, 10 Jul 2026 21:34:46 +0200 Subject: [PATCH 06/11] docs: plan plugin pack source authority Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../2026-07-10-plugin-pack-source-layout.md | 729 ++++++++++++++++++ 1 file changed, 729 insertions(+) create mode 100644 docs/superpowers/plans/2026-07-10-plugin-pack-source-layout.md diff --git a/docs/superpowers/plans/2026-07-10-plugin-pack-source-layout.md b/docs/superpowers/plans/2026-07-10-plugin-pack-source-layout.md new file mode 100644 index 000000000..ef8301900 --- /dev/null +++ b/docs/superpowers/plans/2026-07-10-plugin-pack-source-layout.md @@ -0,0 +1,729 @@ +# Plugin Pack Source Layout Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use +> superpowers:subagent-driven-development (recommended) or +> superpowers:executing-plans to implement this plan task-by-task. Steps use +> checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Revise PR #2122 so `apm pack` prevents accidental root-folder +publication after `.apm/` adoption without breaking native Claude plugins that +author in root convention directories. + +**Architecture:** Separate source-layout detection from publication consent. +The project-root `.apm/` directory selects APM-native layout; its absence +selects plugin-native root discovery. Explicit `includes` lists remain the +strongest publication boundary. Mixed layouts pack from `.apm/` and emit +actionable warnings for skipped root directories. + +**Tech Stack:** Python 3.11+, Click, pytest, subprocess-based integration tests, +Starlight Markdown. + +**Design source:** Commit `c6c3c713d`, file +`docs/superpowers/specs/2026-07-10-plugin-pack-source-layout-design.md`. + +--- + +## File Map + +- Modify `src/apm_cli/bundle/plugin_exporter.py`: select local source layout, + warn on skipped mixed-layout directories, and retain dependency discovery. +- Create `src/apm_cli/bundle/plugin_layout.py`: hold the shared native-root + source names and detect which ones are present. +- Modify `src/apm_cli/commands/_helpers.py`: explain native-plugin discovery + during `apm init` without creating `.apm/`. +- Modify `tests/unit/test_plugin_exporter.py`: pin the layout matrix and warning + contract, including root hook configuration. +- Modify `tests/unit/test_init_command.py`: pin init messaging for native + convention directories. +- Modify `tests/integration/test_pack_root_skills_e2e.py`: cover native + Claude-plugin init-to-pack behavior and `.apm/` authority. +- Modify `docs/src/content/docs/producer/pack-a-bundle.md`: document the source + switch and mixed-layout warning. +- Modify `docs/src/content/docs/producer/repo-shapes.md`: document progressive + migration. +- Modify `docs/src/content/docs/reference/cli/pack.md`: document pack behavior. +- Modify `docs/src/content/docs/reference/manifest-schema.md`: keep `includes` + consent separate from layout. +- Modify `docs/src/content/docs/getting-started/first-package.md`: preserve the + plugin-native onboarding path. +- Modify `packages/apm-guide/.apm/skills/apm-usage/package-authoring.md`: align + agent-facing authoring guidance. +- Modify `CHANGELOG.md`: replace PR #2122's current discriminator claim with the + source-layout decision. + +### Task 1: Reground PR #2122 and run the design panel + +**Files:** +- Read: `docs/superpowers/specs/2026-07-10-plugin-pack-source-layout-design.md` +- Read: `src/apm_cli/bundle/plugin_exporter.py` +- Read: `src/apm_cli/commands/_helpers.py` +- Read: `tests/integration/test_pack_root_skills_e2e.py` + +- [ ] **Step 1: Check out and rebase the PR head** + +```bash +gh pr checkout 2122 --repo microsoft/apm +git fetch origin main +git rebase origin/main +``` + +Expected: the branch is based on current `main`; resolve only faithful conflicts +within PR #2122 files. + +- [ ] **Step 2: Load the approved design** + +```bash +git show c6c3c713d:docs/superpowers/specs/2026-07-10-plugin-pack-source-layout-design.md +``` + +Expected: the decision says `.apm/` presence selects APM-native layout, +plugin-native roots remain sources when `.apm/` is absent, and mixed layouts +warn. + +- [ ] **Step 3: Run `apm-review-panel` with the design as mandatory context** + +Require the panel to answer: + +1. Does `.apm/` presence provide a deterministic authority boundary? +2. Does the proposal preserve official Claude plugin-root conventions? +3. Are explicit `includes` paths still treated as the strongest publication + boundary? +4. Are warnings actionable and ASCII-safe? + +Expected: fold every in-scope panel finding into PR #2122 before terminal +`ship_now`. + +### Task 2: Pin source-layout selection with unit tests + +**Files:** +- Modify: `tests/unit/test_plugin_exporter.py` +- Test: `tests/unit/test_plugin_exporter.py` + +- [ ] **Step 1: Replace the PR's includes-based unit test** + +Replace `test_declared_includes_excludes_root_level_plugin_dirs` with tests that +make `.apm/` presence the only implicit-layout discriminator: + +```python +def test_apm_dir_excludes_root_level_plugin_dirs(self, tmp_path): + project = _setup_plugin_project(tmp_path) + _write_apm_yml(project, extra={"includes": "auto"}) + (project / ".apm").mkdir(exist_ok=True) + root_agents = project / "agents" + root_agents.mkdir() + (root_agents / "root-bot.agent.md").write_text("root bot", encoding="utf-8") + + result = export_plugin_bundle(project, tmp_path / "build") + + assert not (result.bundle_path / "agents" / "root-bot.agent.md").exists() + + +def test_auto_includes_preserves_native_root_without_apm_dir(self, tmp_path): + project = _setup_plugin_project(tmp_path) + _write_apm_yml(project, extra={"includes": "auto"}) + root_agents = project / "agents" + root_agents.mkdir() + (root_agents / "root-bot.agent.md").write_text("root bot", encoding="utf-8") + + result = export_plugin_bundle(project, tmp_path / "build") + + assert (result.bundle_path / "agents" / "root-bot.agent.md").is_file() +``` + +- [ ] **Step 2: Add omitted-includes matrix coverage** + +```python +def test_omitted_includes_with_apm_dir_skips_root_components(self, tmp_path): + project = _setup_plugin_project(tmp_path) + (project / ".apm").mkdir(exist_ok=True) + root_skills = project / "skills" / "root-skill" + root_skills.mkdir(parents=True) + (root_skills / "SKILL.md").write_text("# Root\n", encoding="utf-8") + + result = export_plugin_bundle(project, tmp_path / "build") + + assert not (result.bundle_path / "skills" / "root-skill").exists() +``` + +- [ ] **Step 3: Add explicit-includes and root-hooks coverage** + +```python +def test_explicit_includes_does_not_restore_root_components(self, tmp_path): + project = _setup_plugin_project(tmp_path) + _write_apm_yml(project, extra={"includes": [".apm/agents/published.agent.md"]}) + agents = project / ".apm" / "agents" + agents.mkdir(parents=True) + (agents / "published.agent.md").write_text("published", encoding="utf-8") + root_agents = project / "agents" + root_agents.mkdir() + (root_agents / "draft.agent.md").write_text("draft", encoding="utf-8") + + result = export_plugin_bundle(project, tmp_path / "build") + + assert (result.bundle_path / "agents" / "published.agent.md").is_file() + assert not (result.bundle_path / "agents" / "draft.agent.md").exists() + + +def test_explicit_includes_are_exhaustive(self, tmp_path): + project = _setup_plugin_project(tmp_path) + _write_apm_yml(project, extra={"includes": [".apm/agents/published.agent.md"]}) + agents = project / ".apm" / "agents" + agents.mkdir(parents=True) + (agents / "published.agent.md").write_text("published", encoding="utf-8") + (agents / "private.agent.md").write_text("private", encoding="utf-8") + + result = export_plugin_bundle(project, tmp_path / "build") + + assert (result.bundle_path / "agents" / "published.agent.md").is_file() + assert not (result.bundle_path / "agents" / "private.agent.md").exists() + + +def test_missing_explicit_include_fails_pack(self, tmp_path): + project = _setup_plugin_project(tmp_path) + _write_apm_yml(project, extra={"includes": [".apm/agents/missing.agent.md"]}) + + with pytest.raises( + ValueError, + match=r"includes path '\.apm/agents/missing\.agent\.md' does not exist", + ): + export_plugin_bundle(project, tmp_path / "build") + + +def test_apm_dir_excludes_root_hook_config(self, tmp_path): + project = _setup_plugin_project(tmp_path) + apm_hooks = project / ".apm" / "hooks" + apm_hooks.mkdir(parents=True) + (apm_hooks / "hooks.json").write_text( + json.dumps({"preCommit": ["published"]}), + encoding="utf-8", + ) + (project / "hooks.json").write_text( + json.dumps({"postPush": ["draft"]}), + encoding="utf-8", + ) + + result = export_plugin_bundle(project, tmp_path / "build") + hooks = json.loads((result.bundle_path / "hooks.json").read_text(encoding="utf-8")) + + assert hooks == {"preCommit": ["published"]} + + +def test_apm_authority_preserves_dependency_components(self, tmp_path): + project = _setup_plugin_project(tmp_path) + (project / ".apm").mkdir(exist_ok=True) + deployed = _write_deployed_agent(project, "dep-agent.agent.md", "dependency") + dep = LockedDependency( + repo_url="acme/tools", + depth=1, + deployed_files=deployed, + ) + _write_lockfile(project, [dep]) + + result = export_plugin_bundle(project, tmp_path / "build") + + assert (result.bundle_path / "agents" / "dep-agent.agent.md").is_file() + + +def test_empty_apm_dir_warns_when_no_local_primitives_exist(self, tmp_path): + project = _setup_plugin_project(tmp_path) + (project / ".apm").mkdir(exist_ok=True) + captured = [] + + class _StubLogger: + def warning(self, message): + captured.append(message) + + export_plugin_bundle(project, tmp_path / "build", logger=_StubLogger()) + + assert captured == [ + "No local primitives found. Expected content under .apm/. " + "Check the project layout or move plugin-native content into .apm/." + ] +``` + +- [ ] **Step 4: Add mixed-layout warning coverage** + +Use the `_StubLogger` pattern already present in this module. Assert exact +semantics rather than ANSI formatting: + +```python +captured = [] + + +class _StubLogger: + def warning(self, message): + captured.append(message) + + +result = export_plugin_bundle( + project, + tmp_path / "build", + logger=_StubLogger(), +) +assert result.bundle_path.is_dir() +assert captured == [ + "Skipping root-level agents/ because .apm/ is present. " + "Move publishable files to .apm/agents/ or remove agents/ " + "to silence this warning.", + "Skipping root-level hooks.json because .apm/ is present. " + "Move publishable hook configuration to .apm/hooks/ or remove hooks.json " + "to silence this warning.", +] +``` + +- [ ] **Step 5: Run the focused unit tests and confirm RED** + +```bash +uv run --extra dev pytest \ + tests/unit/test_plugin_exporter.py::TestExportPluginBundle::test_apm_dir_excludes_root_level_plugin_dirs \ + tests/unit/test_plugin_exporter.py::TestExportPluginBundle::test_auto_includes_preserves_native_root_without_apm_dir \ + tests/unit/test_plugin_exporter.py::TestExportPluginBundle::test_omitted_includes_with_apm_dir_skips_root_components \ + tests/unit/test_plugin_exporter.py::TestExportPluginBundle::test_apm_dir_excludes_root_hook_config \ + -xvs +``` + +Expected: at least the native-root and omitted-includes cases fail against PR +#2122's `has_declared_includes` gate. + +### Task 3: Implement deterministic local source selection + +**Files:** +- Create: `src/apm_cli/bundle/plugin_layout.py` +- Modify: `src/apm_cli/bundle/plugin_exporter.py:121-130` +- Modify: `src/apm_cli/bundle/plugin_exporter.py:768-778` +- Test: `tests/unit/test_plugin_exporter.py` + +- [ ] **Step 1: Add shared source detection** + +```python +"""Plugin-native source-layout conventions.""" + +from pathlib import Path + +PLUGIN_ROOT_DIRS = ("agents", "skills", "commands", "instructions", "extensions", "hooks") + + +def find_plugin_root_sources(project_root: Path) -> list[str]: + """Return plugin-native root sources that exist.""" + sources = [name for name in PLUGIN_ROOT_DIRS if (project_root / name).is_dir()] + if (project_root / "hooks.json").is_file(): + sources.append("hooks.json") + return sources +``` + +- [ ] **Step 2: Add a warning emitter** + +Add a focused helper next to `_collect_root_plugin_components`: + +```python +def _warn_skipped_root_components( + project_root: Path, + logger=None, +) -> None: + """Explain why plugin-native root directories are not packed.""" + for source in find_plugin_root_sources(project_root): + if source == "hooks.json": + message = ( + "Skipping root-level hooks.json because .apm/ is present. " + "Move publishable hook configuration to .apm/hooks/ or remove " + "hooks.json to silence this warning." + ) + else: + message = ( + f"Skipping root-level {source}/ because .apm/ is present. " + f"Move publishable files to .apm/{source}/ or remove {source}/ " + "to silence this warning." + ) + if logger: + logger.warning(message) + else: + _rich_warning(message) + + +def _warn_no_local_primitives(logger=None) -> None: + message = ( + "No local primitives found. Expected content under .apm/. " + "Check the project layout or move plugin-native content into .apm/." + ) + if logger: + logger.warning(message) + else: + _rich_warning(message) +``` + +Keep `logger` unannotated, matching `export_plugin_bundle`; do not introduce +`Any` or a new logging abstraction in this bug fix. + +- [ ] **Step 3: Add explicit include collection** + +Add a local collector that rejects unsafe or missing paths, expands declared +directories recursively, and maps each selected file through +`_plugin_rel_for_deployed_path`: + +```python +def _collect_explicit_local_components( + project_root: Path, + includes: list[str], +) -> tuple[list[tuple[Path, str]], dict]: + components: list[tuple[Path, str]] = [] + hooks: dict = {} + for declared_path in includes: + parts = _deployed_path_parts(declared_path) + source = ensure_path_within(project_root.joinpath(*parts), project_root) + if not source.exists(): + raise ValueError(f"includes path {declared_path!r} does not exist.") + files = ( + [source] + if source.is_file() + else sorted(path for path in source.rglob("*") if path.is_file()) + ) + for file_path in files: + if file_path.is_symlink(): + raise ValueError(f"Explicit include path is a symlink: {file_path}") + file_path = ensure_path_within(file_path, project_root) + repo_relative = portable_relpath(file_path, project_root) + plugin_relative = _plugin_rel_for_deployed_path(repo_relative, None) + if plugin_relative is None: + raise ValueError( + f"Explicit include path is not a packable primitive: {repo_relative}" + ) + if plugin_relative == "hooks.json" or plugin_relative.startswith("hooks/"): + try: + hook_data = json.loads(file_path.read_text(encoding="utf-8")) + except (OSError, ValueError, RecursionError) as exc: + raise ValueError( + f"Explicit hook include is not valid JSON: {repo_relative}" + ) from exc + if not isinstance(hook_data, dict): + raise ValueError( + f"Explicit hook include must contain a JSON object: {repo_relative}" + ) + _deep_merge(hooks, hook_data, overwrite=False) + else: + components.append((file_path, plugin_relative)) + return components, hooks +``` + +Import `portable_relpath` from `apm_cli.utils.paths`. + +- [ ] **Step 4: Replace the includes gate with the authority algorithm** + +```python +own_apm_dir = project_root / ".apm" +if isinstance(package.includes, list): + own_components, root_hooks = _collect_explicit_local_components( + project_root, + package.includes, + ) +else: + own_components = _collect_apm_components(own_apm_dir) + root_hooks = _collect_hooks_from_apm(own_apm_dir) + root_components = _collect_root_plugin_components(project_root) + if own_apm_dir.is_dir(): + _warn_skipped_root_components(project_root, logger) + else: + own_components.extend(root_components) + root_hooks_top = _collect_hooks_from_root(project_root) + _deep_merge(root_hooks, root_hooks_top, overwrite=False) + +if own_apm_dir.is_dir() and not own_components and not root_hooks: + _warn_no_local_primitives(logger) +_merge_file_map(file_map, own_components, pkg_name, force, collisions) +_deep_merge(merged_hooks, root_hooks, overwrite=True) +``` + +Add `.apm/hooks/published.json` and `.apm/hooks/private.json` to the +explicit-list unit fixture and assert only the selected hook key reaches +`hooks.json`. + +Do not alter `_collect_deployed_components`: dependency packages retain their +own source layouts. + +- [ ] **Step 5: Run focused tests and confirm GREEN** + +```bash +uv run --extra dev pytest tests/unit/test_plugin_exporter.py -q +``` + +Expected: all plugin exporter tests pass. + +- [ ] **Step 6: Prove the mutation break** + +Temporarily change `if own_apm_dir.is_dir():` to `if False:` and run: + +```bash +uv run --extra dev pytest \ + tests/unit/test_plugin_exporter.py::TestExportPluginBundle::test_apm_dir_excludes_root_level_plugin_dirs \ + -q +``` + +Expected: FAIL because the root agent is packed. Restore the guard and rerun to +PASS. + +- [ ] **Step 7: Commit the source-selection slice** + +```bash +git add src/apm_cli/bundle/plugin_layout.py src/apm_cli/bundle/plugin_exporter.py tests/unit/test_plugin_exporter.py +git commit -m "fix(pack): select source layout from .apm presence" \ + -m "Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>" +``` + +### Task 4: Add native-plugin init-to-pack e2e proof + +**Files:** +- Modify: `tests/integration/test_pack_root_skills_e2e.py` +- Test: `tests/integration/test_pack_root_skills_e2e.py` + +- [ ] **Step 1: Add a subprocess helper** + +```python +def _run_apm(project: Path, *args: str) -> subprocess.CompletedProcess[str]: + apm_executable = Path(sys.executable).with_name("apm") + return subprocess.run( + [str(apm_executable), *args], + cwd=project, + capture_output=True, + text=True, + timeout=60, + check=False, + ) +``` + +- [ ] **Step 2: Add the native Claude plugin journey** + +```python +def test_init_then_pack_preserves_native_claude_skill(tmp_path: Path) -> None: + project = tmp_path / "native-plugin" + skill = project / "skills" / "published" + skill.mkdir(parents=True) + (skill / "SKILL.md").write_text("# Published\n", encoding="utf-8") + + init_result = _run_apm(project, "init", "--yes") + assert init_result.returncode == 0, init_result.stderr + assert not (project / ".apm").exists() + + pack_result = _run_apm(project, "pack") + assert pack_result.returncode == 0, pack_result.stderr + bundles = [path for path in (project / "build").iterdir() if path.is_dir()] + assert len(bundles) == 1 + assert (bundles[0] / "skills" / "published" / "SKILL.md").is_file() +``` + +- [ ] **Step 3: Extend the existing `.apm/` authority e2e test** + +Keep `test_pack_auto_includes_only_apm_authored_skills` and assert the warning +contains the skipped `skills/` directory, `.apm/` cause, and move/remove action. + +- [ ] **Step 4: Run e2e tests RED then GREEN** + +```bash +uv run --extra dev pytest tests/integration/test_pack_root_skills_e2e.py -xvs +``` + +Expected before Task 3: native-plugin journey fails because root skill is +absent. Expected after Task 3: all tests pass. + +- [ ] **Step 5: Run mutation-break on the e2e trap** + +Temporarily restore PR #2122's `if not package.has_declared_includes` gate. +Run: + +```bash +uv run --extra dev pytest \ + tests/integration/test_pack_root_skills_e2e.py::test_init_then_pack_preserves_native_claude_skill \ + -q +``` + +Expected: FAIL. Restore the `.apm/` presence guard and confirm PASS. + +- [ ] **Step 6: Commit the e2e slice** + +```bash +git add tests/integration/test_pack_root_skills_e2e.py +git commit -m "test(pack): preserve native plugin init journey" \ + -m "Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>" +``` + +### Task 5: Add `apm init` migration guidance + +**Files:** +- Modify: `src/apm_cli/bundle/plugin_layout.py` +- Modify: `src/apm_cli/commands/_helpers.py:656-702` +- Modify: `tests/unit/test_init_command.py` + +- [ ] **Step 1: Add a root convention detector** + +Import the detector created in Task 3: + +```python +from ..bundle.plugin_layout import find_plugin_root_sources +``` + +- [ ] **Step 2: Warn after writing `apm.yml`** + +Use the existing console/logger helper in the init flow: + +```python +native_dirs = find_plugin_root_sources(out_file.parent) +if native_dirs and not (out_file.parent / ".apm").is_dir(): + rendered = ", ".join(f"{name}/" for name in native_dirs) + _rich_warning( + f"Found plugin-native directories at the project root: {rendered}. " + "They remain included by apm pack. Move publishable files under .apm/ " + "when adopting the APM source layout." + ) +``` + +Do not create `.apm/` and do not move files. + +- [ ] **Step 3: Add init warning coverage** + +```python +def test_init_preserves_plugin_native_layout(self): + with tempfile.TemporaryDirectory() as tmp_dir: + os.chdir(tmp_dir) + try: + Path("skills").mkdir() + + result = self.runner.invoke(cli, ["init", "--yes"]) + + assert result.exit_code == 0 + assert "Found plugin-native directories" in result.output + assert "skills/" in result.output + assert "remain included by apm pack" in result.output + assert not Path(".apm").exists() + finally: + os.chdir(self.original_dir) +``` + +- [ ] **Step 4: Run focused init tests** + +```bash +uv run --extra dev pytest tests/unit/test_init_command.py -q +``` + +Expected: all init tests pass. + +- [ ] **Step 5: Commit the onboarding slice** + +```bash +git add src/apm_cli/bundle/plugin_layout.py src/apm_cli/commands/_helpers.py tests/unit/test_init_command.py +git commit -m "feat(init): explain native plugin source layout" \ + -m "Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>" +``` + +### Task 6: Align documentation and changelog + +**Files:** +- Modify: `docs/src/content/docs/producer/pack-a-bundle.md` +- Modify: `docs/src/content/docs/producer/repo-shapes.md` +- Modify: `docs/src/content/docs/reference/cli/pack.md` +- Modify: `docs/src/content/docs/reference/manifest-schema.md` +- Modify: `docs/src/content/docs/getting-started/first-package.md` +- Modify: `packages/apm-guide/.apm/skills/apm-usage/package-authoring.md` +- Modify: `CHANGELOG.md` + +- [ ] **Step 1: Replace includes-based layout claims** + +Use this canonical sentence on each relevant documentation surface: + +```text +When `.apm/` exists, local primitive content is sourced from `.apm/`. +Without `.apm/`, supported plugin-native root directories remain pack +sources. An explicit `includes` list remains exhaustive. +``` + +- [ ] **Step 2: Document mixed-layout warnings** + +Explain that `.apm/` wins, root directories are skipped, pack succeeds, and the +warning tells the author to move or remove the directory. + +- [ ] **Step 3: Preserve first-package native onboarding** + +Keep the existing statement that a plugin can begin with no `.apm/` and +plugin-native convention directories. Add that `apm init` does not invalidate +that layout. + +- [ ] **Step 4: Correct the changelog** + +The entry must describe the user outcome: + +```text +- Fixed plugin packing so `.apm/` becomes authoritative when present while + native plugin convention directories remain packable before `.apm/` + adoption. +``` + +- [ ] **Step 5: Commit documentation** + +```bash +git add CHANGELOG.md docs/src/content/docs packages/apm-guide/.apm/skills/apm-usage/package-authoring.md +git commit -m "docs(pack): explain source layout authority" \ + -m "Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>" +``` + +### Task 7: Full validation and shepherd convergence + +**Files:** +- Verify all PR #2122 files. + +- [ ] **Step 1: Run focused suites** + +```bash +uv run --extra dev pytest tests/unit/test_plugin_exporter.py tests/unit/test_init_command.py -q +uv run --extra dev pytest tests/integration/test_pack_root_skills_e2e.py -q +``` + +Expected: all pass. + +- [ ] **Step 2: Run every integration test** + +```bash +bash scripts/test-integration.sh +``` + +Expected: exit 0 with no failures. + +- [ ] **Step 3: Run the canonical lint contract** + +```bash +uv run --extra dev ruff check src/ tests/ \ + && uv run --extra dev ruff format --check src/ tests/ \ + && uv run --extra dev python -m pylint --disable=all --enable=R0801 \ + --min-similarity-lines=10 --fail-on=R0801 src/apm_cli/ \ + && bash scripts/lint-auth-signals.sh +``` + +Expected: exit 0. + +- [ ] **Step 4: Re-run `apm-review-panel`** + +Fold all in-scope findings. Repeat implementation, focused tests, full +integration, lint, push, and CI recovery until the final posted panel verdict +is `ship_now`. + +- [ ] **Step 5: Push safely** + +```bash +git push --force-with-lease origin HEAD:fix/2054-pack-auto-includes +``` + +Expected: PR #2122 updates without overwriting unexpected remote work. + +- [ ] **Step 6: Watch CI** + +```bash +gh pr checks 2122 --repo microsoft/apm --watch +``` + +Expected: all required checks green on the final head SHA. + +- [ ] **Step 7: Verify mergeability and posted verdict** + +```bash +gh pr view 2122 --repo microsoft/apm \ + --json headRefOid,mergeable,mergeStateStatus,statusCheckRollup,comments +``` + +Expected: `mergeable=MERGEABLE`, CI green, and the final panel comment contains +`ship_now`. From 949fc809a4f0f85cff4c7add21e2dfdf830bc43d Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Fri, 10 Jul 2026 21:52:19 +0200 Subject: [PATCH 07/11] fix(pack): select source layout from .apm presence 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> --- src/apm_cli/bundle/plugin_exporter.py | 137 +++++++++++++++++-- src/apm_cli/bundle/plugin_layout.py | 20 +++ src/apm_cli/models/apm_package.py | 5 - tests/unit/test_plugin_exporter.py | 188 +++++++++++++++++++++++++- 4 files changed, 331 insertions(+), 19 deletions(-) create mode 100644 src/apm_cli/bundle/plugin_layout.py diff --git a/src/apm_cli/bundle/plugin_exporter.py b/src/apm_cli/bundle/plugin_exporter.py index f4b912491..07f46b686 100644 --- a/src/apm_cli/bundle/plugin_exporter.py +++ b/src/apm_cli/bundle/plugin_exporter.py @@ -30,8 +30,10 @@ ) from ..utils.console import _rich_warning from ..utils.path_security import PathTraversalError, ensure_path_within, safe_rmtree +from ..utils.paths import portable_relpath from .attest import verify_attested_file from .packer import PackResult +from .plugin_layout import find_plugin_root_sources # --------------------------------------------------------------------------- # Path helpers @@ -130,6 +132,42 @@ def _collect_root_plugin_components(project_root: Path) -> list[tuple[Path, str] return components +def _emit_pack_warning(message: str, logger=None) -> None: + """Send a pack warning through the command logger when available.""" + if logger: + logger.warning(message) + else: + _rich_warning(message, symbol="warning") + + +def _warn_skipped_root_components(project_root: Path, logger=None) -> None: + """Explain why plugin-native root sources are not packed.""" + for source in find_plugin_root_sources(project_root): + if source == "hooks.json": + message = ( + "Skipping root-level hooks.json because .apm/ is present. " + "Move publishable hook configuration to .apm/hooks/ or remove " + "hooks.json to silence this warning." + ) + else: + message = ( + f"Skipping root-level {source}/ because .apm/ is present. " + f"Move publishable files to .apm/{source}/ or remove {source}/ " + "to silence this warning." + ) + _emit_pack_warning(message, logger) + + +def _warn_no_local_primitives(logger=None) -> None: + """Explain how to recover from an empty APM-native source layout.""" + _emit_pack_warning( + "No local primitives found. Expected content under .apm/. " + "Move plugin-native content into .apm/, or remove .apm/ to restore " + "root convention discovery.", + logger, + ) + + def _collect_bare_skill( install_path: Path, dep: "LockedDependency", @@ -483,6 +521,72 @@ def _plugin_rel_for_deployed_path( return PurePosixPath(*plugin_parts).as_posix() +def _collect_explicit_local_components( + project_root: Path, + includes: list[str], +) -> tuple[list[tuple[Path, str]], dict]: + """Collect only explicitly listed local paths and validate each path.""" + components: list[tuple[Path, str]] = [] + hooks: dict = {} + for declared_path in includes: + parts = _deployed_path_parts(declared_path) + candidate = project_root.joinpath(*parts) + if candidate.is_symlink(): + raise ValueError(f"Explicit include path is a symlink: {declared_path}") + try: + source = ensure_path_within(candidate, project_root) + except PathTraversalError as exc: + raise ValueError( + f"includes path {declared_path!r} escapes the project root. " + "Fix the path in apm.yml." + ) from exc + if not source.exists(): + raise ValueError( + f"includes path {declared_path!r} does not exist. " + "Fix the path in apm.yml or create it." + ) + files = ( + [source] + if source.is_file() + else sorted(path for path in source.rglob("*") if path.is_file()) + ) + for file_path in files: + if file_path.is_symlink(): + raise ValueError(f"Explicit include path is a symlink: {file_path}") + try: + file_path = ensure_path_within(file_path, project_root) + except PathTraversalError as exc: + raise ValueError( + f"Explicit include path escapes the project root: {file_path}" + ) from exc + repo_relative = portable_relpath(file_path, project_root) + is_hook = ( + repo_relative == "hooks.json" + or repo_relative.startswith("hooks/") + or repo_relative.startswith(".apm/hooks/") + ) + if is_hook: + try: + hook_data = json.loads(file_path.read_text(encoding="utf-8")) + except (OSError, ValueError, RecursionError) as exc: + raise ValueError( + f"Explicit hook include is not valid JSON: {repo_relative}" + ) from exc + if not isinstance(hook_data, dict): + raise ValueError( + f"Explicit hook include must contain a JSON object: {repo_relative}" + ) + _deep_merge(hooks, hook_data, overwrite=False) + continue + plugin_relative = _plugin_rel_for_deployed_path(repo_relative, None) + if plugin_relative is None: + raise ValueError( + f"Explicit include path is not a packable primitive: {repo_relative}" + ) + components.append((file_path, plugin_relative)) + return components, hooks + + def _verify_attested_hash( source: Path, project_root: Path, @@ -765,19 +869,34 @@ def export_plugin_bundle( _merge_file_map(file_map, dep_components, dep_name, force, collisions) - # 6. Collect own components (.apm/ and root-level) + # 6. Collect own components according to the local source authority. own_apm_dir = project_root / ".apm" - own_components = _collect_apm_components(own_apm_dir) - # An includes declaration scopes local publication to .apm/. Preserve - # root convention discovery only for legacy plugin-native manifests. - if not package.has_declared_includes: - own_components.extend(_collect_root_plugin_components(project_root)) + if isinstance(package.includes, list): + own_components, root_hooks = _collect_explicit_local_components( + project_root, + package.includes, + ) + else: + own_components = _collect_apm_components(own_apm_dir) + root_hooks = _collect_hooks_from_apm(own_apm_dir) + if own_apm_dir.is_dir(): + _warn_skipped_root_components(project_root, logger) + else: + own_components.extend(_collect_root_plugin_components(project_root)) + root_hooks_top = _collect_hooks_from_root(project_root) + _deep_merge(root_hooks, root_hooks_top, overwrite=False) + + if ( + not isinstance(package.includes, list) + and own_apm_dir.is_dir() + and not own_components + and not root_hooks + ): + _warn_no_local_primitives(logger) + _merge_file_map(file_map, own_components, pkg_name, force, collisions) # Hooks -- root package wins on key collision - root_hooks = _collect_hooks_from_apm(own_apm_dir) - root_hooks_top = _collect_hooks_from_root(project_root) - _deep_merge(root_hooks, root_hooks_top, overwrite=False) _deep_merge(merged_hooks, root_hooks, overwrite=True) # MCP -- root package wins on server-name collision diff --git a/src/apm_cli/bundle/plugin_layout.py b/src/apm_cli/bundle/plugin_layout.py new file mode 100644 index 000000000..a66727513 --- /dev/null +++ b/src/apm_cli/bundle/plugin_layout.py @@ -0,0 +1,20 @@ +"""Plugin-native source-layout conventions.""" + +from pathlib import Path + +PLUGIN_ROOT_DIRS = ( + "agents", + "skills", + "commands", + "instructions", + "extensions", + "hooks", +) + + +def find_plugin_root_sources(project_root: Path) -> list[str]: + """Return plugin-native root sources that exist.""" + sources = [name for name in PLUGIN_ROOT_DIRS if (project_root / name).is_dir()] + if (project_root / "hooks.json").is_file(): + sources.append("hooks.json") + return sources diff --git a/src/apm_cli/models/apm_package.py b/src/apm_cli/models/apm_package.py index 9d02d45cc..7a4e6adcf 100644 --- a/src/apm_cli/models/apm_package.py +++ b/src/apm_cli/models/apm_package.py @@ -275,11 +275,6 @@ class APMPackage: # to boolean (e.g. ``{"owner/repo#v1.0": {"hooks": true}}``). allow_executables: dict[str, dict[str, bool]] | None = None - @property - def has_declared_includes(self) -> bool: - """Return whether the manifest explicitly declares ``includes``.""" - return self.includes is not None - @classmethod def _parse_dependency_dict(cls, raw_deps: dict, label: str = "") -> dict: """Parse a dependencies or devDependencies dict from apm.yml. diff --git a/tests/unit/test_plugin_exporter.py b/tests/unit/test_plugin_exporter.py index fa59b7ed2..1456bb4db 100644 --- a/tests/unit/test_plugin_exporter.py +++ b/tests/unit/test_plugin_exporter.py @@ -1203,27 +1203,205 @@ def test_plugin_json_omits_convention_dir_keys(self, tmp_path): def test_root_level_plugin_dirs_collected(self, tmp_path): """Root-level agents/ commands/ etc. are picked up for plugin-native repos.""" project = _setup_plugin_project(tmp_path) - # Create root-level agents dir (no .apm/) + (project / ".apm").rmdir() root_agents = project / "agents" root_agents.mkdir() - (root_agents / "root-bot.agent.md").write_text("root bot") + (root_agents / "root-bot.agent.md").write_text("root bot", encoding="utf-8") out = tmp_path / "build" result = export_plugin_bundle(project, out) assert (result.bundle_path / "agents" / "root-bot.agent.md").exists() - def test_declared_includes_excludes_root_level_plugin_dirs(self, tmp_path): - """Declared includes scopes plugin content to the .apm directory.""" + def test_apm_dir_excludes_root_level_plugin_dirs(self, tmp_path): + """The .apm directory makes APM-native sources authoritative.""" project = _setup_plugin_project(tmp_path) _write_apm_yml(project, extra={"includes": "auto"}) root_agents = project / "agents" root_agents.mkdir() - (root_agents / "root-bot.agent.md").write_text("root bot") + (root_agents / "root-bot.agent.md").write_text("root bot", encoding="utf-8") result = export_plugin_bundle(project, tmp_path / "build") assert not (result.bundle_path / "agents" / "root-bot.agent.md").exists() + def test_auto_includes_preserves_native_root_without_apm_dir(self, tmp_path): + """Auto consent does not select the local source layout.""" + project = _setup_plugin_project(tmp_path) + _write_apm_yml(project, extra={"includes": "auto"}) + (project / ".apm").rmdir() + root_agents = project / "agents" + root_agents.mkdir() + (root_agents / "root-bot.agent.md").write_text("root bot", encoding="utf-8") + + result = export_plugin_bundle(project, tmp_path / "build") + + assert (result.bundle_path / "agents" / "root-bot.agent.md").is_file() + + def test_omitted_includes_with_apm_dir_skips_root_components(self, tmp_path): + """An omitted includes field does not weaken .apm authority.""" + project = _setup_plugin_project(tmp_path) + root_skills = project / "skills" / "root-skill" + root_skills.mkdir(parents=True) + (root_skills / "SKILL.md").write_text("# Root\n", encoding="utf-8") + + result = export_plugin_bundle(project, tmp_path / "build") + + assert not (result.bundle_path / "skills" / "root-skill").exists() + + def test_explicit_includes_are_exhaustive(self, tmp_path): + """An explicit path list packs no unlisted local primitives.""" + project = _setup_plugin_project(tmp_path) + _write_apm_yml( + project, + extra={"includes": [".apm/agents/published.agent.md"]}, + ) + agents = project / ".apm" / "agents" + agents.mkdir(parents=True) + (agents / "published.agent.md").write_text("published", encoding="utf-8") + (agents / "private.agent.md").write_text("private", encoding="utf-8") + root_agents = project / "agents" + root_agents.mkdir() + (root_agents / "draft.agent.md").write_text("draft", encoding="utf-8") + + result = export_plugin_bundle(project, tmp_path / "build") + + assert (result.bundle_path / "agents" / "published.agent.md").is_file() + assert not (result.bundle_path / "agents" / "private.agent.md").exists() + assert not (result.bundle_path / "agents" / "draft.agent.md").exists() + + def test_explicit_includes_can_publish_deliberate_root_path(self, tmp_path): + """An explicit list may deliberately publish a root convention path.""" + project = _setup_plugin_project(tmp_path) + _write_apm_yml(project, extra={"includes": ["agents/published.agent.md"]}) + root_agents = project / "agents" + root_agents.mkdir() + (root_agents / "published.agent.md").write_text("published", encoding="utf-8") + + result = export_plugin_bundle(project, tmp_path / "build") + + assert (result.bundle_path / "agents" / "published.agent.md").is_file() + + def test_missing_explicit_include_fails_pack(self, tmp_path): + """A missing explicit path is a configuration error.""" + project = _setup_plugin_project(tmp_path) + _write_apm_yml( + project, + extra={"includes": [".apm/agents/missing.agent.md"]}, + ) + + with pytest.raises( + ValueError, + match=r"includes path '\.apm/agents/missing\.agent\.md' does not exist", + ): + export_plugin_bundle(project, tmp_path / "build") + + def test_explicit_hook_includes_are_exhaustive(self, tmp_path): + """Only explicitly listed hook configuration reaches the bundle.""" + project = _setup_plugin_project(tmp_path) + _write_apm_yml( + project, + extra={"includes": [".apm/hooks/published.json"]}, + ) + hooks_dir = project / ".apm" / "hooks" + hooks_dir.mkdir(parents=True) + (hooks_dir / "published.json").write_text( + json.dumps({"preCommit": ["published"]}), + encoding="utf-8", + ) + (hooks_dir / "private.json").write_text( + json.dumps({"postPush": ["private"]}), + encoding="utf-8", + ) + + result = export_plugin_bundle(project, tmp_path / "build") + hooks = json.loads((result.bundle_path / "hooks.json").read_text(encoding="utf-8")) + + assert hooks == {"preCommit": ["published"]} + + def test_apm_dir_excludes_root_hook_config(self, tmp_path): + """Root hooks follow the same .apm authority rule as primitives.""" + project = _setup_plugin_project(tmp_path) + apm_hooks = project / ".apm" / "hooks" + apm_hooks.mkdir(parents=True) + (apm_hooks / "hooks.json").write_text( + json.dumps({"preCommit": ["published"]}), + encoding="utf-8", + ) + (project / "hooks.json").write_text( + json.dumps({"postPush": ["draft"]}), + encoding="utf-8", + ) + + result = export_plugin_bundle(project, tmp_path / "build") + hooks = json.loads((result.bundle_path / "hooks.json").read_text(encoding="utf-8")) + + assert hooks == {"preCommit": ["published"]} + + def test_apm_authority_preserves_dependency_components(self, tmp_path): + """Local layout authority does not affect dependency discovery.""" + project = _setup_plugin_project(tmp_path) + deployed = _write_deployed_agent(project, "dep-agent.agent.md", "dependency") + dep = LockedDependency( + repo_url="acme/tools", + depth=1, + deployed_files=deployed, + ) + _write_lockfile(project, [dep]) + + result = export_plugin_bundle(project, tmp_path / "build") + + assert (result.bundle_path / "agents" / "dep-agent.agent.md").is_file() + + def test_mixed_layout_emits_actionable_warnings(self, tmp_path): + """Each skipped root source names the cause and next action.""" + project = _setup_plugin_project(tmp_path) + root_agents = project / "agents" + root_agents.mkdir() + (root_agents / "draft.agent.md").write_text("draft", encoding="utf-8") + (project / "hooks.json").write_text("{}", encoding="utf-8") + captured: list[str] = [] + + class _StubLogger: + def info(self, message, symbol=None): + pass + + def warning(self, message): + captured.append(message) + + export_plugin_bundle(project, tmp_path / "build", logger=_StubLogger()) + + assert captured == [ + "Skipping root-level agents/ because .apm/ is present. " + "Move publishable files to .apm/agents/ or remove agents/ " + "to silence this warning.", + "Skipping root-level hooks.json because .apm/ is present. " + "Move publishable hook configuration to .apm/hooks/ or remove " + "hooks.json to silence this warning.", + "No local primitives found. Expected content under .apm/. " + "Move plugin-native content into .apm/, or remove .apm/ to restore " + "root convention discovery.", + ] + + def test_empty_apm_dir_warns_when_no_local_primitives_exist(self, tmp_path): + """An empty APM-native layout explains how to recover.""" + project = _setup_plugin_project(tmp_path) + captured: list[str] = [] + + class _StubLogger: + def info(self, message, symbol=None): + pass + + def warning(self, message): + captured.append(message) + + export_plugin_bundle(project, tmp_path / "build", logger=_StubLogger()) + + assert captured == [ + "No local primitives found. Expected content under .apm/. " + "Move plugin-native content into .apm/, or remove .apm/ to restore " + "root convention discovery.", + ] + class TestExportPluginBundleViaPackBundle: """Verify pack_bundle(fmt='plugin') delegates correctly.""" From 80853c518b683aec9198daf7de93f35bf5c3368c Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Fri, 10 Jul 2026 21:52:19 +0200 Subject: [PATCH 08/11] test(pack): preserve native plugin init journey 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> --- src/apm_cli/commands/init.py | 13 +++++ .../integration/test_pack_root_skills_e2e.py | 51 +++++++++++++++---- tests/unit/test_init_command.py | 18 +++++++ 3 files changed, 73 insertions(+), 9 deletions(-) diff --git a/src/apm_cli/commands/init.py b/src/apm_cli/commands/init.py index 1db46c296..e0b9ce95b 100644 --- a/src/apm_cli/commands/init.py +++ b/src/apm_cli/commands/init.py @@ -7,6 +7,7 @@ import click +from ..bundle.plugin_layout import find_plugin_root_sources from ..constants import APM_YML_FILENAME from ..core.command_logger import CommandLogger from ..core.target_detection import ( @@ -219,6 +220,18 @@ def _perform_init( # Create apm.yml (with devDependencies for plugin mode) _create_minimal_apm_yml(config, plugin=plugin) + native_sources = find_plugin_root_sources(project_root) + if native_sources and not (project_root / ".apm").is_dir(): + rendered_sources = ", ".join( + source if source.endswith(".json") else f"{source}/" for source in native_sources + ) + logger.warning( + "Found plugin-native sources at the project root: " + f"{rendered_sources}. They remain included by apm pack. " + "Move publishable files under .apm/ when adopting the APM " + "source layout.", + symbol="warning", + ) # Create plugin.json for plugin mode if plugin: diff --git a/tests/integration/test_pack_root_skills_e2e.py b/tests/integration/test_pack_root_skills_e2e.py index b8cb56e65..c0420a9d2 100644 --- a/tests/integration/test_pack_root_skills_e2e.py +++ b/tests/integration/test_pack_root_skills_e2e.py @@ -7,6 +7,19 @@ from pathlib import Path +def _run_apm(project: Path, *args: str) -> subprocess.CompletedProcess[str]: + """Run the installed APM CLI in a project directory.""" + apm_executable = Path(sys.executable).with_name("apm") + return subprocess.run( + [str(apm_executable), *args], + cwd=project, + capture_output=True, + text=True, + timeout=60, + check=False, + ) + + 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" @@ -31,15 +44,7 @@ def test_pack_auto_includes_only_apm_authored_skills(tmp_path: Path) -> None: 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") - result = subprocess.run( - [str(apm_executable), "pack"], - cwd=project, - capture_output=True, - text=True, - timeout=60, - check=False, - ) + result = _run_apm(project, "pack") assert result.returncode == 0, ( f"apm pack failed:\nstdout:\n{result.stdout}\nstderr:\n{result.stderr}" @@ -47,3 +52,31 @@ def test_pack_auto_includes_only_apm_authored_skills(tmp_path: Path) -> None: bundle = project / "build" / "root-skills-test-1.0.0" assert (bundle / "skills" / "published" / "SKILL.md").is_file() assert not (bundle / "skills" / "work-in-progress").exists() + output = " ".join((result.stdout + result.stderr).split()) + assert "Skipping root-level skills/ because .apm/ is present." in output + assert "Move publishable files to .apm/skills/" in output + assert "remove skills/ to silence this warning." in output + + +def test_init_then_pack_preserves_native_claude_skill(tmp_path: Path) -> None: + """Init must not make a native Claude root skill disappear from pack.""" + project = tmp_path / "native-plugin" + skill = project / "skills" / "published" + skill.mkdir(parents=True) + (skill / "SKILL.md").write_text("# Published\n", encoding="utf-8") + + init_result = _run_apm(project, "init", "--yes") + + assert init_result.returncode == 0, init_result.stderr + assert not (project / ".apm").exists() + assert "includes: auto" in (project / "apm.yml").read_text(encoding="utf-8") + init_output = " ".join((init_result.stdout + init_result.stderr).split()) + assert "Found plugin-native sources at the project root: skills/." in init_output + assert "They remain included by apm pack." in init_output + + pack_result = _run_apm(project, "pack") + + assert pack_result.returncode == 0, pack_result.stderr + bundles = [path for path in (project / "build").iterdir() if path.is_dir()] + assert len(bundles) == 1 + assert (bundles[0] / "skills" / "published" / "SKILL.md").is_file() diff --git a/tests/unit/test_init_command.py b/tests/unit/test_init_command.py index 72402d40c..7993b463b 100644 --- a/tests/unit/test_init_command.py +++ b/tests/unit/test_init_command.py @@ -56,6 +56,24 @@ def test_init_current_directory(self): finally: os.chdir(self.original_dir) # restore CWD before TemporaryDirectory cleanup + def test_init_preserves_plugin_native_layout(self): + """Init explains that native root sources remain packable.""" + with tempfile.TemporaryDirectory() as tmp_dir: + os.chdir(tmp_dir) + try: + Path("skills").mkdir() + + result = self.runner.invoke(cli, ["init", "--yes"]) + + assert result.exit_code == 0 + output = " ".join(result.output.split()) + assert "Found plugin-native sources" in output + assert "skills/" in output + assert "remain included by apm pack" in output + assert not Path(".apm").exists() + finally: + os.chdir(self.original_dir) + def test_init_explicit_current_directory(self): """Test initialization with explicit '.' argument.""" with tempfile.TemporaryDirectory() as tmp_dir: From 845b860ebc584667b67a0f21c9f7efc31e854a73 Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Fri, 10 Jul 2026 21:56:00 +0200 Subject: [PATCH 09/11] docs(pack): explain source layout authority Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- CHANGELOG.md | 6 +-- .../docs/getting-started/first-package.md | 17 +++++--- .../content/docs/producer/pack-a-bundle.md | 41 ++++++++++++------- docs/src/content/docs/producer/repo-shapes.md | 17 ++++---- docs/src/content/docs/reference/cli/pack.md | 7 ++-- .../content/docs/reference/manifest-schema.md | 23 +++++++---- .../skills/apm-usage/package-authoring.md | 12 +++--- 7 files changed, 76 insertions(+), 47 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f8b19b27c..4aac4d45e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,9 +26,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed -- `apm pack` with a declared `includes:` value no longer publishes local - root-level plugin convention directories such as `skills/`. (closes #2054, - #2122) +- `apm pack` now treats `.apm/` as the authoritative local source when present, + while native plugin convention directories remain packable before `.apm/` + adoption, including after `apm init`. (#2122) ## [0.24.1] - 2026-07-10 diff --git a/docs/src/content/docs/getting-started/first-package.md b/docs/src/content/docs/getting-started/first-package.md index 4a03894e1..7db7bd22e 100644 --- a/docs/src/content/docs/getting-started/first-package.md +++ b/docs/src/content/docs/getting-started/first-package.md @@ -30,6 +30,13 @@ cd team-skills `apm init` creates exactly one file -- the manifest. The `.apm/` source tree is yours to author. +:::tip[Already have a Claude plugin?] +If `skills/`, `agents/`, or `commands/` already exists at the project root, +`apm init` leaves that layout in place. It writes `includes: auto` without +creating `.apm/`, and those plugin-native directories remain sources for +`apm pack`. Create `.apm/` later when you are ready to switch source authority. +::: + ``` team-skills/ +-- apm.yml @@ -52,11 +59,11 @@ includes: auto scripts: {} ``` -`includes: auto` is the field that makes step 4 work: with no remote -dependencies declared, `apm install` walks your local `.apm/` tree -and deploys what it finds. Set `includes: []` (or omit the field) and -local content stops deploying. Override with an explicit list of -paths to gate exactly what ships. +`includes: auto` records explicit consent to deploy or pack local content. +Omitting the field preserves legacy implicit consent and produces an audit +advisory. Use an explicit list of paths when you need an exhaustive publication +boundary. Source layout is independent: `.apm/` is authoritative when present; +otherwise supported plugin-native root directories remain pack sources. ## 2. Add a skill diff --git a/docs/src/content/docs/producer/pack-a-bundle.md b/docs/src/content/docs/producer/pack-a-bundle.md index f6db29bbf..c28e597b7 100644 --- a/docs/src/content/docs/producer/pack-a-bundle.md +++ b/docs/src/content/docs/producer/pack-a-bundle.md @@ -1,6 +1,6 @@ --- title: Pack a bundle -description: Build a plugin-format bundle from your .apm/ source so others can deploy it with a single apm install command. +description: Build a plugin-format bundle from APM-native or plugin-native source so others can deploy it with one apm install command. --- A bundle is the artifact you hand to a consumer when you do not want to publish @@ -8,7 +8,7 @@ to a registry. It is a directory (or archive -- `.zip` by default, `.tar.gz` via `--archive-format tar.gz`) containing a `plugin.json`, your primitive folders, and an embedded `apm.lock.yaml` that pins every file by SHA-256. Build it with one command from a project that has -`.apm/` and `apm.yml`: +`apm.yml`: ```bash apm pack @@ -127,10 +127,22 @@ For the consumer flags that apply (`--target`, `--global`, `--force`, ## Source layout and install-time discovery -`apm pack` is intentionally liberal: it collects primitives from both -`.apm//` subdirectories and from convention directories at the -package root (`agents/`, `skills/`, `instructions/`, etc.). This lets -you author in whichever layout feels natural during development. +When `.apm/` exists, local primitive content is sourced from `.apm/`. +Without `.apm/`, supported plugin-native root directories such as +`agents/`, `skills/`, `commands/`, and `hooks/` remain pack sources. +`includes: auto` grants publication consent but does not select either +layout. An explicit `includes` list is exhaustive and may deliberately +name an APM-native or root path. + +If both layouts exist, `.apm/` wins and packing succeeds. A warning names +each skipped root source and tells you to move it under `.apm/` or remove +it: + +```text +[!] Skipping root-level skills/ because .apm/ is present. + Move publishable files to .apm/skills/ or remove skills/ to silence + this warning. +``` When packing git dependencies, `apm pack` emits **only** what the lockfile attests, in every format (`--format plugin` and the default @@ -151,8 +163,10 @@ Dependency **hooks-config and MCP-config** (the `hooks.json` / `.mcp.json` entries `apm install` merges into shared host settings) are *not* attested in `deployed_files`, so they are not packed; `apm pack` warns loudly (`[!]`) and names the dependency when this happens. First-party -root hooks/MCP authored by the packaging project itself **are** packed -- -only unattested dependency config is dropped. Hook *scripts* recorded in +hooks/MCP authored by the packaging project itself **are** packed from the +selected local layout. Root `hooks/` and `hooks.json` are skipped when +`.apm/` is present, just like other root convention sources. Only +unattested dependency config is dropped. Hook *scripts* recorded in `deployed_files` still pack normally. `apm install` is per-primitive and stricter. Each integrator has its own @@ -180,12 +194,11 @@ Source: `src/apm_cli/integration/instruction_integrator.py`, ### Canonical layout for marketplace publishers :::caution[Silent install drops can remove intended guardrails] -`apm pack` always accepts primitives from `.apm//`. It also accepts root -convention directories (for example, an `instructions/` folder at the plugin -root) only for legacy manifests that omit `includes:`. Declaring -`includes: auto` or an explicit list scopes local package content to `.apm/`. -See the [`includes` manifest field](../../reference/manifest-schema/#39-includes) -for all three forms. +When `.apm/` exists, it is the authoritative local source. Without `.apm/`, +supported plugin-native root directories remain pack sources, including after +`apm init` writes `includes: auto`. An explicit [`includes` +list](../../reference/manifest-schema/#39-includes) is exhaustive regardless +of layout. `apm install` does NOT discover instructions, commands, or prompts placed in root convention directories. Packages that rely on these primitives for security guardrails or policy enforcement will install silently incomplete, diff --git a/docs/src/content/docs/producer/repo-shapes.md b/docs/src/content/docs/producer/repo-shapes.md index ffc95ec9c..6ebbc3f46 100644 --- a/docs/src/content/docs/producer/repo-shapes.md +++ b/docs/src/content/docs/producer/repo-shapes.md @@ -134,15 +134,14 @@ my-monorepo/ ``` > **Important -- use `.apm//` for every primitive in each plugin.** -> `apm pack` always accepts primitives from `.apm//`. Root convention -> directories (e.g. `instructions/` at the plugin root) are also accepted only -> for legacy manifests that omit `includes:`; declaring `includes: auto` or an -> explicit list scopes local package content to `.apm/`. `apm install` only -> discovers instructions, commands, and prompts under `.apm//`. Authoring -> `packages/plugin-a/instructions/style.instructions.md` -> instead of `packages/plugin-a/.apm/instructions/style.instructions.md` -> can produce a bundle only under the legacy omitted-`includes` behavior, and -> then installs silently incomplete. See [Pack a bundle -- source layout and install-time +> When `.apm/` exists, it is the authoritative local pack source. Without +> `.apm/`, supported plugin-native root directories remain pack sources, +> including after `apm init` writes `includes: auto`. Mixed layouts pack from +> `.apm/` and warn about each skipped root source. `apm install` only discovers +> instructions, commands, and prompts under `.apm//`, so authoring +> `packages/plugin-a/instructions/style.instructions.md` instead of +> `packages/plugin-a/.apm/instructions/style.instructions.md` can install +> incomplete. See [Pack a bundle -- source layout and install-time > discovery](./pack-a-bundle/#source-layout-and-install-time-discovery) > for the full per-primitive scan-path reference. diff --git a/docs/src/content/docs/reference/cli/pack.md b/docs/src/content/docs/reference/cli/pack.md index bde62f068..6f8cbce4c 100644 --- a/docs/src/content/docs/reference/cli/pack.md +++ b/docs/src/content/docs/reference/cli/pack.md @@ -109,9 +109,10 @@ apm pack --archive --dry-run -v A Claude Code plugin directory under `--output`. Contains: - `plugin.json` -- schema-conformant manifest. Convention-dir keys are stripped because Claude Code auto-discovers them. -- Plugin-native subdirs populated from your `.apm/` content and from installed dependencies: `agents/`, `skills/`, `commands/`, `instructions/`, `hooks/`, `extensions/` (canvas extensions, when the `canvas` experimental flag is enabled). - - With [`includes: auto` or an explicit `includes:` list](../manifest-schema/#39-includes), root convention directories such as `skills/` are not package content; put publishable local primitives under `.apm/`. - - Manifests that omit `includes:` retain legacy root convention discovery for plugin-native projects. +- Plugin-native subdirs populated from local source and installed dependencies: `agents/`, `skills/`, `commands/`, `instructions/`, `hooks/`, `extensions/` (canvas extensions, when the `canvas` experimental flag is enabled). + - When `.apm/` exists, local primitives and hooks are sourced from `.apm/`. Root convention sources are skipped with actionable warnings. + - Without `.apm/`, supported plugin-native root directories remain pack sources, including after `apm init` writes [`includes: auto`](../manifest-schema/#39-includes). + - An explicit `includes:` list is exhaustive. A missing or unpackable listed path stops packing instead of falling back to implicit discovery. - Installed dependencies are packed exclusively from lockfile-attested `deployed_files`; the `apm_modules` cache is never packed (it has no provenance or integrity guarantee). Each attested file is verified against its `deployed_file_hashes` SHA-256 before inclusion. - If the dependency declares `skills:`, only the named skills are included; the cache cannot add extras. - If a dependency has cached primitives but no `deployed_files`, `apm pack` fails and tells you to run `apm install`. diff --git a/docs/src/content/docs/reference/manifest-schema.md b/docs/src/content/docs/reference/manifest-schema.md index 0ac62956c..ac4afc6a2 100644 --- a/docs/src/content/docs/reference/manifest-schema.md +++ b/docs/src/content/docs/reference/manifest-schema.md @@ -228,14 +228,21 @@ scripts: | **Default** | Undeclared (legacy implicit auto-publish; flagged by `apm audit`). | | **Allowed values** | `auto` or a list of paths relative to the project root. | -Declares which local `.apm/` content the project consents to publish when packing or deploying. Three forms are supported: - -1. **Undeclared** (field omitted). Legacy behaviour: all local `.apm/` content - is published and `apm pack` also discovers root plugin convention - directories. `apm audit` emits an `includes-consent` advisory whenever local - content is deployed under this form. -2. **`includes: auto`**. Explicit consent to publish all local `.apm/` content via the file scanner. No path enumeration required. Default for newly initialised projects. -3. **`includes: [, ...]`**. Explicit allow-list of paths the project consents to publish. Strongest governance form; changes are reviewable in PR diffs. +Declares which local content the project consents to publish when packing or +deploying. This field does not select the source layout: `.apm/` presence makes +the APM-native layout authoritative, while its absence preserves supported +plugin-native root sources. Three forms are supported: + +1. **Undeclared** (field omitted). Legacy implicit consent for content in the + selected source layout. `apm audit` emits an `includes-consent` advisory + whenever local content is deployed under this form. +2. **`includes: auto`**. Explicit consent to publish all local content from the + selected source layout. No path enumeration required. Default for newly + initialised projects. +3. **`includes: [, ...]`**. Explicit, exhaustive allow-list of paths the + project consents to publish. Listed paths may be under `.apm/` or a + plugin-native root directory. Missing, unsafe, symlinked, or unpackable paths + stop `apm pack`; no implicit source fallback occurs. ```yaml # Form 1: undeclared (legacy; audit advisory) diff --git a/packages/apm-guide/.apm/skills/apm-usage/package-authoring.md b/packages/apm-guide/.apm/skills/apm-usage/package-authoring.md index 729d7952b..b4181ba74 100644 --- a/packages/apm-guide/.apm/skills/apm-usage/package-authoring.md +++ b/packages/apm-guide/.apm/skills/apm-usage/package-authoring.md @@ -62,11 +62,13 @@ my-package/ ## Install-time discovery rules -`apm pack` collects declared local primitives from `.apm//`. -When `apm.yml` sets `includes: auto` or an explicit `includes:` list, -root convention directories (`agents/`, `skills/`, `instructions/`, -etc.) are not package content. Manifests that omit `includes:` retain -legacy root convention discovery for plugin-native projects. Prefer +When `.apm/` exists, `apm pack` sources local primitives and hooks from +`.apm/`. Without `.apm/`, supported plugin-native root directories +(`agents/`, `skills/`, `commands/`, `instructions/`, `extensions/`, and +hooks) remain pack sources, including after `apm init` writes +`includes: auto`. Mixed layouts pack from `.apm/` and warn about skipped +root sources. An explicit `includes:` list is exhaustive; invalid listed +paths fail instead of falling back to implicit discovery. Prefer `.apm//` so pack and install use the same source layout. Per-primitive scan paths for `apm install`: From f297740cb92047f036f4f42c0105ed0c6012c644 Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Fri, 10 Jul 2026 22:19:14 +0200 Subject: [PATCH 10/11] fix(pack): harden explicit source validation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- CHANGELOG.md | 13 +-- .../content/docs/concepts/package-anatomy.md | 4 +- .../docs/getting-started/first-package.md | 4 +- .../content/docs/producer/pack-a-bundle.md | 10 +-- docs/src/content/docs/quickstart.mdx | 4 +- docs/src/content/docs/reference/cli/init.md | 4 + src/apm_cli/bundle/plugin_exporter.py | 23 +++-- src/apm_cli/bundle/plugin_layout.py | 6 +- src/apm_cli/commands/init.py | 5 +- tests/unit/test_plugin_exporter.py | 90 +++++++++++++++++++ 10 files changed, 134 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4aac4d45e..5948c05e9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,17 +18,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `apm install --target intellij` now configures JetBrains Copilot MCP support while routing package file primitives through the Copilot profile. (by @sergio-sisternes-epam; closes #1957) (#2041) - -### Fixed - - Azure DevOps marketplace checks now preserve suffix-free `/_git/` URLs and pass Azure CLI bearer authentication through to `git ls-remote`. (closes #2119) - -### Fixed - -- `apm pack` now treats `.apm/` as the authoritative local source when present, - while native plugin convention directories remain packable before `.apm/` - adoption, including after `apm init`. (#2122) +- Existing Claude plugin directories (`skills/`, `agents/`, and `commands/`) + now pack without migration to `.apm/`. When `.apm/` is present, it becomes + the authoritative source and mixed layouts warn with actionable next steps. + (#2122) ## [0.24.1] - 2026-07-10 diff --git a/docs/src/content/docs/concepts/package-anatomy.md b/docs/src/content/docs/concepts/package-anatomy.md index e116a3ea7..23a76bbe7 100644 --- a/docs/src/content/docs/concepts/package-anatomy.md +++ b/docs/src/content/docs/concepts/package-anatomy.md @@ -101,8 +101,8 @@ targets: - copilot - claude -# Optional. "auto" auto-publishes every primitive under .apm/, or list -# explicit repo paths to publish a subset. +# Optional. "auto" publishes the authoritative local source layout, or list +# explicit repo paths to define the complete publication set. includes: auto # Optional. Runtime dependencies, grouped by kind. diff --git a/docs/src/content/docs/getting-started/first-package.md b/docs/src/content/docs/getting-started/first-package.md index 7db7bd22e..7370ef32c 100644 --- a/docs/src/content/docs/getting-started/first-package.md +++ b/docs/src/content/docs/getting-started/first-package.md @@ -34,7 +34,9 @@ is yours to author. If `skills/`, `agents/`, or `commands/` already exists at the project root, `apm init` leaves that layout in place. It writes `includes: auto` without creating `.apm/`, and those plugin-native directories remain sources for -`apm pack`. Create `.apm/` later when you are ready to switch source authority. +`apm pack`. Create `.apm/` later when you want `apm pack` to source from that +directory instead of the project root. See [source layout and install-time +discovery](../../producer/pack-a-bundle/#source-layout-and-install-time-discovery). ::: ``` diff --git a/docs/src/content/docs/producer/pack-a-bundle.md b/docs/src/content/docs/producer/pack-a-bundle.md index c28e597b7..42f89e5e0 100644 --- a/docs/src/content/docs/producer/pack-a-bundle.md +++ b/docs/src/content/docs/producer/pack-a-bundle.md @@ -162,10 +162,10 @@ pack` fails and tells you to run `apm install` to record provenance. Dependency **hooks-config and MCP-config** (the `hooks.json` / `.mcp.json` entries `apm install` merges into shared host settings) are *not* attested in `deployed_files`, so they are not packed; `apm pack` warns -loudly (`[!]`) and names the dependency when this happens. First-party -hooks/MCP authored by the packaging project itself **are** packed from the -selected local layout. Root `hooks/` and `hooks.json` are skipped when -`.apm/` is present, just like other root convention sources. Only +loudly (`[!]`) and names the dependency when this happens. First-party hooks +authored by the packaging project follow the selected local layout: root +`hooks/` and `hooks.json` are skipped when `.apm/` is present. The packaging +project's root `.mcp.json` is packed independently of source layout. Only unattested dependency config is dropped. Hook *scripts* recorded in `deployed_files` still pack normally. @@ -197,7 +197,7 @@ Source: `src/apm_cli/integration/instruction_integrator.py`, When `.apm/` exists, it is the authoritative local source. Without `.apm/`, supported plugin-native root directories remain pack sources, including after `apm init` writes `includes: auto`. An explicit [`includes` -list](../../reference/manifest-schema/#39-includes) is exhaustive regardless +list](../reference/manifest-schema/#39-includes) is exhaustive regardless of layout. `apm install` does NOT discover instructions, commands, or prompts placed in root convention directories. Packages that rely on these primitives for diff --git a/docs/src/content/docs/quickstart.mdx b/docs/src/content/docs/quickstart.mdx index 346e61097..a4c074a02 100644 --- a/docs/src/content/docs/quickstart.mdx +++ b/docs/src/content/docs/quickstart.mdx @@ -79,7 +79,9 @@ Three fields matter day one: - `scripts` -- optional named shell commands you run with the experimental `apm run` (see [Run scripts](/apm/consumer/run-scripts/)). -`includes: auto` ships any local `.apm/` content you author. The +`includes: auto` ships local content from the authoritative source layout: +`.apm/` when that directory exists, or supported plugin-native directories at +the project root otherwise. The `targets:` block is commented out so APM auto-detects the harnesses present in your repo (`.github/`, `.claude/`, `.cursor/`, etc.). For the full schema, see [Package anatomy](/apm/concepts/package-anatomy/). diff --git a/docs/src/content/docs/reference/cli/init.md b/docs/src/content/docs/reference/cli/init.md index 4f859f00f..9e87118eb 100644 --- a/docs/src/content/docs/reference/cli/init.md +++ b/docs/src/content/docs/reference/cli/init.md @@ -104,6 +104,10 @@ $ apm init --yes --target copilot,claude,cursor pre-checks targets read from its existing `target:` field. - **Codex hint:** if `.codex/` is present, suggests `--target agent-skills` to also deploy skills to `.agents/skills/`. +- **Existing plugin sources:** when plugin-native directories such as + `skills/`, `agents/`, or `commands/` exist at the project root and `.apm/` + does not, warns that they remain packable. `apm init` does not create + `.apm/` automatically. - **agentrc suggestion:** when no agent instruction files are found (`.github/copilot-instructions.md`, `AGENTS.md`, `.github/instructions/`), the Next Steps panel suggests generating agent instructions: diff --git a/src/apm_cli/bundle/plugin_exporter.py b/src/apm_cli/bundle/plugin_exporter.py index 07f46b686..0b43b59a8 100644 --- a/src/apm_cli/bundle/plugin_exporter.py +++ b/src/apm_cli/bundle/plugin_exporter.py @@ -33,7 +33,7 @@ from ..utils.paths import portable_relpath from .attest import verify_attested_file from .packer import PackResult -from .plugin_layout import find_plugin_root_sources +from .plugin_layout import PLUGIN_ROOT_DIRS, find_plugin_root_sources # --------------------------------------------------------------------------- # Path helpers @@ -127,7 +127,9 @@ def _collect_root_plugin_components(project_root: Path) -> list[tuple[Path, str] ``skills/``, etc. at the repo root) have their files picked up here. """ components: list[tuple[Path, str]] = [] - for dir_name in ("agents", "skills", "commands", "instructions", "extensions"): + for dir_name in PLUGIN_ROOT_DIRS: + if dir_name == "hooks": + continue _collect_recursive(project_root / dir_name, dir_name, components) return components @@ -216,7 +218,7 @@ def _collect_flat( rename=None, ) -> None: """Add every regular non-symlink file directly inside *src_dir*.""" - if not src_dir.is_dir(): + if src_dir.is_symlink() or not src_dir.is_dir(): return for f in sorted(src_dir.iterdir()): if f.is_file() and not f.is_symlink(): @@ -232,7 +234,7 @@ def _collect_recursive( rename=None, ) -> None: """Add every regular non-symlink file under *src_dir*, preserving hierarchy.""" - if not src_dir.is_dir(): + if src_dir.is_symlink() or not src_dir.is_dir(): return for f in sorted(src_dir.rglob("*")): if not f.is_file() or f.is_symlink(): @@ -532,7 +534,10 @@ def _collect_explicit_local_components( parts = _deployed_path_parts(declared_path) candidate = project_root.joinpath(*parts) if candidate.is_symlink(): - raise ValueError(f"Explicit include path is a symlink: {declared_path}") + raise ValueError( + f"includes path {declared_path!r} is a symlink. " + "Replace it with a regular file or directory." + ) try: source = ensure_path_within(candidate, project_root) except PathTraversalError as exc: @@ -552,12 +557,16 @@ def _collect_explicit_local_components( ) for file_path in files: if file_path.is_symlink(): - raise ValueError(f"Explicit include path is a symlink: {file_path}") + raise ValueError( + f"Symlink found inside includes path {declared_path!r}: " + f"{file_path.name}. Remove the symlink or list a regular path." + ) try: file_path = ensure_path_within(file_path, project_root) except PathTraversalError as exc: raise ValueError( - f"Explicit include path escapes the project root: {file_path}" + f"A file inside includes path {declared_path!r} escapes the " + "project root. Remove the symlink or fix the path in apm.yml." ) from exc repo_relative = portable_relpath(file_path, project_root) is_hook = ( diff --git a/src/apm_cli/bundle/plugin_layout.py b/src/apm_cli/bundle/plugin_layout.py index a66727513..f2d6b0e5c 100644 --- a/src/apm_cli/bundle/plugin_layout.py +++ b/src/apm_cli/bundle/plugin_layout.py @@ -14,7 +14,11 @@ def find_plugin_root_sources(project_root: Path) -> list[str]: """Return plugin-native root sources that exist.""" - sources = [name for name in PLUGIN_ROOT_DIRS if (project_root / name).is_dir()] + sources = [ + name + for name in PLUGIN_ROOT_DIRS + if (project_root / name).is_dir() and not (project_root / name).is_symlink() + ] if (project_root / "hooks.json").is_file(): sources.append("hooks.json") return sources diff --git a/src/apm_cli/commands/init.py b/src/apm_cli/commands/init.py index e0b9ce95b..a862bb516 100644 --- a/src/apm_cli/commands/init.py +++ b/src/apm_cli/commands/init.py @@ -228,9 +228,8 @@ def _perform_init( logger.warning( "Found plugin-native sources at the project root: " f"{rendered_sources}. They remain included by apm pack. " - "Move publishable files under .apm/ when adopting the APM " - "source layout.", - symbol="warning", + "Move publishable files under .apm/ when you want apm pack " + "to source from that directory.", ) # Create plugin.json for plugin mode diff --git a/tests/unit/test_plugin_exporter.py b/tests/unit/test_plugin_exporter.py index 1456bb4db..6236aecd2 100644 --- a/tests/unit/test_plugin_exporter.py +++ b/tests/unit/test_plugin_exporter.py @@ -1295,6 +1295,96 @@ def test_missing_explicit_include_fails_pack(self, tmp_path): ): export_plugin_bundle(project, tmp_path / "build") + def test_explicit_include_rejects_symlink(self, tmp_path): + """An explicit path cannot publish through a symlink.""" + project = _setup_plugin_project(tmp_path) + target = project / ".apm" / "agents" / "real.agent.md" + target.parent.mkdir(parents=True, exist_ok=True) + target.write_text("real", encoding="utf-8") + link = project / "linked.agent.md" + try: + os.symlink(target, link) + except OSError: + pytest.skip("symlinks not supported") + _write_apm_yml(project, extra={"includes": ["linked.agent.md"]}) + + with pytest.raises( + ValueError, + match=r"includes path 'linked\.agent\.md' is a symlink", + ): + export_plugin_bundle(project, tmp_path / "build") + + def test_explicit_include_rejects_nested_symlink(self, tmp_path): + """A listed directory cannot publish a nested symlink.""" + project = _setup_plugin_project(tmp_path) + agents = project / ".apm" / "agents" + agents.mkdir(parents=True, exist_ok=True) + target = agents / "real.agent.md" + target.write_text("real", encoding="utf-8") + link = agents / "linked.agent.md" + try: + os.symlink(target, link) + except OSError: + pytest.skip("symlinks not supported") + _write_apm_yml(project, extra={"includes": [".apm/agents"]}) + + with pytest.raises( + ValueError, + match=r"Symlink found inside includes path '\.apm/agents': " + r"linked\.agent\.md", + ): + export_plugin_bundle(project, tmp_path / "build") + + @pytest.mark.parametrize( + ("content", "message"), + [ + ("not-json", r"Explicit hook include is not valid JSON"), + ("[]", r"Explicit hook include must contain a JSON object"), + ], + ) + def test_explicit_hook_include_rejects_invalid_content( + self, + tmp_path, + content, + message, + ): + """Explicit hook files must contain valid JSON objects.""" + project = _setup_plugin_project(tmp_path) + hook = project / ".apm" / "hooks" / "invalid.json" + hook.parent.mkdir(parents=True) + hook.write_text(content, encoding="utf-8") + _write_apm_yml(project, extra={"includes": [".apm/hooks/invalid.json"]}) + + with pytest.raises(ValueError, match=message): + export_plugin_bundle(project, tmp_path / "build") + + def test_explicit_include_rejects_non_packable_path(self, tmp_path): + """Explicit paths must map to a supported plugin primitive.""" + project = _setup_plugin_project(tmp_path) + (project / "README.md").write_text("not a primitive", encoding="utf-8") + _write_apm_yml(project, extra={"includes": ["README.md"]}) + + with pytest.raises(ValueError, match=r"is not a packable primitive"): + export_plugin_bundle(project, tmp_path / "build") + + def test_native_root_symlink_is_not_packed(self, tmp_path): + """Implicit root discovery does not follow convention-dir symlinks.""" + project = _setup_plugin_project(tmp_path) + _write_apm_yml(project, extra={"includes": "auto"}) + (project / ".apm").rmdir() + external = tmp_path / "external-skills" + skill = external / "outside" + skill.mkdir(parents=True) + (skill / "SKILL.md").write_text("# Outside\n", encoding="utf-8") + try: + os.symlink(external, project / "skills") + except OSError: + pytest.skip("symlinks not supported") + + result = export_plugin_bundle(project, tmp_path / "build") + + assert not (result.bundle_path / "skills" / "outside").exists() + def test_explicit_hook_includes_are_exhaustive(self, tmp_path): """Only explicitly listed hook configuration reaches the bundle.""" project = _setup_plugin_project(tmp_path) From 5f07a42fdcf5ee5b8b1c3deba9b8d93fe9784e54 Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Fri, 10 Jul 2026 22:33:34 +0200 Subject: [PATCH 11/11] fix(pack): reject nested source symlinks Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- CHANGELOG.md | 6 ++-- .../content/docs/reference/manifest-schema.md | 33 +++++++++++++------ src/apm_cli/bundle/plugin_exporter.py | 13 +++----- tests/unit/test_plugin_exporter.py | 20 +++++++++++ 4 files changed, 51 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5948c05e9..b24614372 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,9 +20,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 (by @sergio-sisternes-epam; closes #1957) (#2041) - Azure DevOps marketplace checks now preserve suffix-free `/_git/` URLs and pass Azure CLI bearer authentication through to `git ls-remote`. (closes #2119) -- Existing Claude plugin directories (`skills/`, `agents/`, and `commands/`) - now pack without migration to `.apm/`. When `.apm/` is present, it becomes - the authoritative source and mixed layouts warn with actionable next steps. +- `apm pack` now treats `.apm/` as the authoritative local source, warns on + mixed layouts, and enforces explicit `includes:` lists exhaustively while + preserving root-only Claude plugin directories, including after `apm init`. (#2122) ## [0.24.1] - 2026-07-10 diff --git a/docs/src/content/docs/reference/manifest-schema.md b/docs/src/content/docs/reference/manifest-schema.md index ac4afc6a2..53d55d727 100644 --- a/docs/src/content/docs/reference/manifest-schema.md +++ b/docs/src/content/docs/reference/manifest-schema.md @@ -228,10 +228,11 @@ scripts: | **Default** | Undeclared (legacy implicit auto-publish; flagged by `apm audit`). | | **Allowed values** | `auto` or a list of paths relative to the project root. | -Declares which local content the project consents to publish when packing or -deploying. This field does not select the source layout: `.apm/` presence makes -the APM-native layout authoritative, while its absence preserves supported -plugin-native root sources. Three forms are supported: +Records consent to publish local content during deployment and controls local +content selection when producing plugin bundles with `apm pack`. This field +does not select the source layout: `.apm/` presence makes the APM-native layout +authoritative, while its absence preserves supported plugin-native root +sources. Three forms are supported: 1. **Undeclared** (field omitted). Legacy implicit consent for content in the selected source layout. `apm audit` emits an `includes-consent` advisory @@ -239,10 +240,12 @@ plugin-native root sources. Three forms are supported: 2. **`includes: auto`**. Explicit consent to publish all local content from the selected source layout. No path enumeration required. Default for newly initialised projects. -3. **`includes: [, ...]`**. Explicit, exhaustive allow-list of paths the - project consents to publish. Listed paths may be under `.apm/` or a - plugin-native root directory. Missing, unsafe, symlinked, or unpackable paths - stop `apm pack`; no implicit source fallback occurs. +3. **`includes: [, ...]`**. Explicit consent list for deployment and the + exhaustive allow-list for plugin packing. Listed paths may be under `.apm/` + or a plugin-native root directory. Missing, unsafe, symlinked, or unpackable + paths stop `apm pack`; no implicit source fallback occurs. During + `apm install`, the list satisfies explicit-consent policy but does not filter + integrator discovery. ```yaml # Form 1: undeclared (legacy; audit advisory) @@ -257,7 +260,10 @@ includes: auto # - .apm/skills/my-skill/ ``` -`includes:` is allow-list only. There is no `exclude:` form. To keep maintainer-only primitives out of shipped artifacts, author them OUTSIDE `.apm/` and reference them via a local-path devDependency. See [Dev-only Primitives](../concepts/primitives-and-targets/#dev-only-primitives). +For plugin packing, `includes:` is allow-list only. There is no `exclude:` +form. To keep maintainer-only primitives out of shipped artifacts, author them +outside the selected source layout and reference them via a local-path +devDependency. See [Dev-only Primitives](../concepts/primitives-and-targets/#dev-only-primitives). When `policy.manifest.require_explicit_includes` is `true` (see [Policy reference](../enterprise/policy-reference/)), only form 3 passes; `auto` and undeclared are rejected at install/audit time by the `explicit-includes` check (not at YAML parse time). @@ -723,7 +729,14 @@ Created automatically by [`apm plugin init`](./cli/plugin/). Use [`apm install - apm install --dev owner/test-helpers ``` -Plain `apm install` (no flag) deploys both `dependencies` and `devDependencies`. There is no `--omit=dev` flag today; the dev/prod separation kicks in at `apm pack` (plugin format, the default). The local-content scanner that builds plugin bundles operates on `.apm/` only and does not consult the devDep marker. To keep maintainer-only primitives out of shipped artifacts, author them outside `.apm/` and reference them via a local-path devDependency. See [Dev-only Primitives](../concepts/primitives-and-targets/#dev-only-primitives). +Plain `apm install` (no flag) deploys both `dependencies` and +`devDependencies`. There is no `--omit=dev` flag today; the dev/prod separation +kicks in at `apm pack` (plugin format, the default). The local-content scanner +uses `.apm/` when present, otherwise supported plugin-native root directories; +it does not consult the devDep marker. To keep maintainer-only primitives out +of shipped artifacts, author them outside the selected source layout and +reference them via a local-path devDependency. See [Dev-only +Primitives](../concepts/primitives-and-targets/#dev-only-primitives). Local-path devDependency example: diff --git a/src/apm_cli/bundle/plugin_exporter.py b/src/apm_cli/bundle/plugin_exporter.py index 0b43b59a8..203e00bf2 100644 --- a/src/apm_cli/bundle/plugin_exporter.py +++ b/src/apm_cli/bundle/plugin_exporter.py @@ -550,17 +550,14 @@ def _collect_explicit_local_components( f"includes path {declared_path!r} does not exist. " "Fix the path in apm.yml or create it." ) - files = ( - [source] - if source.is_file() - else sorted(path for path in source.rglob("*") if path.is_file()) - ) - for file_path in files: - if file_path.is_symlink(): + entries = [source] if source.is_file() else sorted(source.rglob("*")) + for entry in entries: + if entry.is_symlink(): raise ValueError( f"Symlink found inside includes path {declared_path!r}: " - f"{file_path.name}. Remove the symlink or list a regular path." + f"{entry.name}. Remove the symlink or list a regular path." ) + for file_path in (entry for entry in entries if entry.is_file()): try: file_path = ensure_path_within(file_path, project_root) except PathTraversalError as exc: diff --git a/tests/unit/test_plugin_exporter.py b/tests/unit/test_plugin_exporter.py index 6236aecd2..f966cc8ef 100644 --- a/tests/unit/test_plugin_exporter.py +++ b/tests/unit/test_plugin_exporter.py @@ -1335,6 +1335,26 @@ def test_explicit_include_rejects_nested_symlink(self, tmp_path): ): export_plugin_bundle(project, tmp_path / "build") + def test_explicit_include_rejects_nested_directory_symlink(self, tmp_path): + """A listed directory cannot publish through a symlinked directory.""" + project = _setup_plugin_project(tmp_path) + skills = project / ".apm" / "skills" + skills.mkdir(parents=True, exist_ok=True) + external = tmp_path / "external-skill" + external.mkdir() + (external / "SKILL.md").write_text("# External\n", encoding="utf-8") + try: + os.symlink(external, skills / "linked") + except OSError: + pytest.skip("symlinks not supported") + _write_apm_yml(project, extra={"includes": [".apm/skills"]}) + + with pytest.raises( + ValueError, + match=r"Symlink found inside includes path '\.apm/skills': linked", + ): + export_plugin_bundle(project, tmp_path / "build") + @pytest.mark.parametrize( ("content", "message"), [