From dda8bdc88e201652e0728faf26ec73b27466b0e1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E8=A8=B1=E5=85=83=E8=B1=AA?=
 <146086744+edenfunf@users.noreply.github.com>
Date: Sat, 8 Aug 2026 19:41:06 +0800
Subject: [PATCH 1/8] fix(plugins): normalize declared skills containers to one
 level (closes #2530)

`--skill` rejected every value on plugins whose manifest declares the
conventional skills container, reporting `Available: (none)` even though a
bare install deployed those same skills.

`_map_plugin_artifacts` copied each declared `skills` entry under its own
name. That is right when the entry IS a skill (`./skills/engineering/tdd` ->
`.apm/skills/tdd`), but a container entry (`./skills/`) was buried as
`.apm/skills/skills/<name>/` -- one level below the depth that deployment,
`--skill` enumeration, the bin/ security scan and primitive counting all read.
The string form had the mirror defect: a lone declared skill was merged, so
its `SKILL.md` landed in the shared root under no name at all.

Classify per entry instead of per manifest shape: an entry carrying its own
`SKILL.md` keeps its name, one holding skills is merged. Undeclared discovery
is unchanged -- the default `skills/` is the convention container by
definition.

Deployment and `--skill` validation also disagreed on where skills come from:
the deploy path preferred a root `skills/` bundle while `available_skill_names`
carried a plugin-only branch reading `.apm/skills/`. Both now resolve through
one `skill_source_dir` helper, so a selectable name and a deployable name
cannot drift apart again.

Knock-on effects, both verified against dotnet/skills/plugins/dotnet-advanced:
`apm deps list` counted 0 skills for such plugins and now counts them, and
skill-level `bin/` directories were invisible to `scan_package_executables`
(bin_count 0 -> 1). The approval gate is inactive by default, so this changes
what the scanner declares, not what a default install deploys.

Merging the two copytree call sites dropped `TestIgnoreNonContentSourceGuard`
below its hand-maintained occurrence floor. Replaced that substring count with
a per-call-site AST assertion that every `shutil.copytree` passes `ignore=`:
it cannot be loosened by merging or splitting call sites, does not confuse a
docstring mention for an argument, and names the offending line.
---
 src/apm_cli/deps/plugin_parser.py             | 56 ++++++++---
 src/apm_cli/integration/skill_integrator.py   | 37 +++++---
 .../test_architecture_outcome_guards.py       | 64 +++++++++++++
 tests/unit/test_plugin_parser.py              | 94 +++++++++++++++++++
 tests/unit/test_symlink_containment.py        | 77 +++++++++++----
 5 files changed, 283 insertions(+), 45 deletions(-)

diff --git a/src/apm_cli/deps/plugin_parser.py b/src/apm_cli/deps/plugin_parser.py
index f988048609..ba65573b49 100644
--- a/src/apm_cli/deps/plugin_parser.py
+++ b/src/apm_cli/deps/plugin_parser.py
@@ -134,6 +134,26 @@ def _is_within_plugin(candidate: Path, plugin_root: Path, *, component: str) ->
     return True
 
 
+def _holds_skill_dirs(candidate: Path) -> bool:
+    """Return True iff *candidate* is a container of skills, not a skill itself.
+
+    A directory carrying its own ``SKILL.md`` is the skill, however deep it
+    sits in the plugin (``skills/engineering/tdd``). Anything else that has at
+    least one immediate child carrying ``SKILL.md`` is a container holding
+    them (the conventional ``skills/``). Directories matching neither shape
+    are not containers, so a declared entry keeps its own name rather than
+    spilling unrecognized contents into the shared skills root.
+    """
+    if (candidate / "SKILL.md").is_file():
+        return False
+    try:
+        return any(
+            (child / "SKILL.md").is_file() for child in candidate.iterdir() if child.is_dir()
+        )
+    except OSError:
+        return False
+
+
 def parse_plugin_manifest(plugin_json_path: Path) -> dict[str, Any]:
     """Parse a plugin.json manifest file.
 
@@ -783,24 +803,30 @@ def _is_same_path(src: Path, dst: Path) -> bool:
         skill_dirs = [s for s in skill_sources if s.is_dir()]
         skill_files = [s for s in skill_sources if s.is_file()]
 
-        is_custom_list = isinstance(manifest.get("skills"), list)
-        if is_custom_list and skill_dirs:
+        # A declared ``skills`` entry is either the skill itself (``SKILL.md``
+        # at its root, e.g. ``./skills/engineering/tdd``) or a container
+        # holding one skill per child (the conventional ``./skills/``).
+        # Classify per entry rather than per manifest shape: naming a
+        # container after itself buries every skill one level too deep, while
+        # merging a lone skill spills a bare ``SKILL.md`` into the shared root
+        # under no name at all. Either way ``--skill`` sees nothing, because
+        # ``.apm/skills/<name>/SKILL.md`` is the exact depth that deployment,
+        # ``--skill`` enumeration, the bin/ security scan and primitive
+        # counting all read. See issue #2530.
+        #
+        # Undeclared discovery keeps merging unconditionally: the default
+        # ``skills/`` is the convention container by definition.
+        declared = isinstance(manifest.get("skills"), (list, str))
+        if skill_dirs:
             target_skills.mkdir(parents=True, exist_ok=True)
             for d in skill_dirs:
-                nested = target_skills / d.name
-                if _is_same_path(d, nested):
-                    continue
-                shutil.copytree(
-                    d,
-                    nested,
-                    ignore=ignore_non_content,
-                    dirs_exist_ok=True,
-                )
-        elif skill_dirs:
-            for d in skill_dirs:
-                if _is_same_path(d, target_skills):
+                if declared and not _holds_skill_dirs(d):
+                    dest = target_skills / d.name
+                else:
+                    dest = target_skills
+                if _is_same_path(d, dest):
                     continue
-                shutil.copytree(d, target_skills, dirs_exist_ok=True, ignore=ignore_non_content)
+                shutil.copytree(d, dest, dirs_exist_ok=True, ignore=ignore_non_content)
         if skill_files:
             target_skills.mkdir(parents=True, exist_ok=True)
             for f in skill_files:
diff --git a/src/apm_cli/integration/skill_integrator.py b/src/apm_cli/integration/skill_integrator.py
index a2c00669ab..612dc150c1 100644
--- a/src/apm_cli/integration/skill_integrator.py
+++ b/src/apm_cli/integration/skill_integrator.py
@@ -615,6 +615,24 @@ def _skill_names_in_directory(skills_dir: Path) -> frozenset[str]:
         except FileNotFoundError:
             return frozenset()
 
+    @staticmethod
+    def skill_source_dir(package_path: Path) -> Path:
+        """Return the directory a package's deployable skills are promoted from.
+
+        Single source of truth for skill routing: deployment and ``--skill``
+        enumeration both resolve through here so the set a user may select can
+        never drift from the set that actually deploys (issue #2530). A root
+        ``skills/`` bundle wins whenever it holds at least one skill; otherwise
+        the normalized ``.apm/skills/`` location supplies them.
+
+        Callers must handle a root ``SKILL.md`` before asking: a native
+        single-skill package deploys the bundle itself, not children.
+        """
+        root_bundle = package_path / "skills"
+        if SkillIntegrator._skill_names_in_directory(root_bundle):
+            return root_bundle
+        return package_path / ".apm" / "skills"
+
     @staticmethod
     def available_skill_names(package_info) -> frozenset[str] | None:
         """Return names selectable through ``--skill`` for one package."""
@@ -622,15 +640,9 @@ def available_skill_names(package_info) -> frozenset[str] | None:
         if (package_path / "SKILL.md").is_file():
             return None
 
-        from apm_cli.models.validation import PackageType
-
-        normalized = package_path / ".apm" / "skills"
-        root_bundle = package_path / "skills"
-        if package_info.package_type is PackageType.MARKETPLACE_PLUGIN:
-            return SkillIntegrator._skill_names_in_directory(normalized)
-
-        root_names = SkillIntegrator._skill_names_in_directory(root_bundle)
-        return root_names or SkillIntegrator._skill_names_in_directory(normalized)
+        return SkillIntegrator._skill_names_in_directory(
+            SkillIntegrator.skill_source_dir(package_path)
+        )
 
     @staticmethod
     def _skill_filter_misses_available(
@@ -1453,10 +1465,11 @@ def integrate_package_skill(
             )
 
         # SKILL_BUNDLE: promote skills from root-level skills/ directory.
+        # Routed through ``skill_source_dir`` -- the same helper backing
+        # ``available_skill_names`` -- so a ``--skill`` value can never be
+        # validated against a directory other than the one that deploys.
         root_skills_dir = package_path / "skills"
-        if root_skills_dir.is_dir() and any(
-            (d / "SKILL.md").exists() for d in root_skills_dir.iterdir() if d.is_dir()
-        ):
+        if self.skill_source_dir(package_path) == root_skills_dir:
             return self._merge_bin_paths(
                 self._integrate_skill_bundle(
                     package_info,
diff --git a/tests/integration/test_architecture_outcome_guards.py b/tests/integration/test_architecture_outcome_guards.py
index 2a6cd7ff9e..5f49cd3c01 100644
--- a/tests/integration/test_architecture_outcome_guards.py
+++ b/tests/integration/test_architecture_outcome_guards.py
@@ -122,6 +122,70 @@ def test_requested_plugin_skill_with_no_match_fails_closed(
     assert not (consumer / ".claude" / "skills" / "resolving-merge-conflicts").exists()
 
 
+def test_declared_skills_container_is_selectable_by_skill(
+    tmp_path: Path,
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    """``--skill`` must subset a plugin that declares its skills container.
+
+    Regression for #2530: ``"skills": ["./skills/"]`` names the conventional
+    container. Normalizing it under its own name left the enumerator backing
+    ``--skill`` with nothing to match, so every requested name was rejected
+    with ``Available: (none)`` even though a bare install deployed them all.
+    """
+    from click.testing import CliRunner
+
+    from apm_cli.cli import cli
+
+    plugin, consumer = _write_plugin_consumer(
+        tmp_path,
+        {
+            "name": "container-skills",
+            "version": "1.0.0",
+            "skills": ["./skills/"],
+        },
+    )
+    for name in ("csharp-scripts", "dotnet-pinvoke"):
+        skill = plugin / "skills" / name
+        skill.mkdir(parents=True)
+        (skill / "SKILL.md").write_text(f"# {name}\n", encoding="utf-8")
+    monkeypatch.chdir(consumer)
+    monkeypatch.setattr("apm_cli.cli._check_and_notify_updates", lambda: None)
+
+    result = CliRunner().invoke(cli, ["install", "--skill", "csharp-scripts"])
+
+    assert result.exit_code == 0, result.output
+    assert "Available: (none)" not in result.output
+    deployed = consumer / ".claude" / "skills"
+    assert (deployed / "csharp-scripts" / "SKILL.md").is_file()
+    # Subsetting is the point: the sibling must stay out.
+    assert not (deployed / "dotnet-pinvoke").exists()
+
+
+def test_skill_enumeration_matches_the_directory_that_deploys(
+    tmp_path: Path,
+) -> None:
+    """``--skill`` validation and deployment must read one routing rule.
+
+    Regression for #2530: the enumerator carried a plugin-only branch that
+    read ``.apm/skills/`` while the deploy path preferred a root ``skills/``
+    bundle, so a selectable name and a deployable name could disagree.
+    """
+    from apm_cli.integration.skill_integrator import SkillIntegrator
+    from apm_cli.models.validation import PackageType
+
+    package = tmp_path / "pkg"
+    for name in ("alpha", "beta"):
+        skill = package / "skills" / name
+        skill.mkdir(parents=True)
+        (skill / "SKILL.md").write_text(f"# {name}\n", encoding="utf-8")
+
+    info = MagicMock(install_path=package, package_type=PackageType.MARKETPLACE_PLUGIN)
+
+    assert SkillIntegrator.skill_source_dir(package) == package / "skills"
+    assert SkillIntegrator.available_skill_names(info) == frozenset({"alpha", "beta"})
+
+
 def test_stale_persisted_skill_pin_warns_instead_of_silent_noop(
     tmp_path: Path,
     monkeypatch: pytest.MonkeyPatch,
diff --git a/tests/unit/test_plugin_parser.py b/tests/unit/test_plugin_parser.py
index ecec09ef02..8a50fdea75 100644
--- a/tests/unit/test_plugin_parser.py
+++ b/tests/unit/test_plugin_parser.py
@@ -255,6 +255,100 @@ def test_custom_skills_path_array(self, tmp_path):
         assert (apm_dir / "skills" / "skills" / "SKILL.md").read_text() == "# A"
         assert (apm_dir / "skills" / "extra-skills" / "SKILL.md").read_text() == "# B"
 
+    def test_declared_skills_container_flattens_to_one_level(self, tmp_path):
+        """A declared container merges its skills instead of nesting itself.
+
+        Regression for #2530: ``"skills": ["./skills/"]`` names the
+        conventional container, not a skill. Copying it under its own name
+        buried every skill at ``.apm/skills/skills/<name>/`` -- one level
+        below where deployment, ``--skill`` enumeration, the bin/ security
+        scan and primitive counting all look.
+        """
+        plugin_dir = tmp_path / "plugin"
+        plugin_dir.mkdir()
+        container = plugin_dir / "skills"
+        for name in ("csharp-scripts", "dotnet-pinvoke"):
+            skill = container / name
+            skill.mkdir(parents=True)
+            (skill / "SKILL.md").write_text(f"# {name}", encoding="utf-8")
+
+        apm_dir = plugin_dir / ".apm"
+        apm_dir.mkdir()
+        _map_plugin_artifacts(plugin_dir, apm_dir, manifest={"skills": ["./skills/"]})
+
+        normalized = apm_dir / "skills"
+        assert not (normalized / "skills").exists()
+        assert (normalized / "csharp-scripts" / "SKILL.md").read_text() == "# csharp-scripts"
+        assert (normalized / "dotnet-pinvoke" / "SKILL.md").read_text() == "# dotnet-pinvoke"
+
+    def test_declared_skills_string_single_skill_keeps_leaf_name(self, tmp_path):
+        """The string form classifies per entry too, not only the array form.
+
+        ``"skills": "./skills/engineering/tdd"`` names one skill. Merging its
+        contents would spill a bare ``SKILL.md`` into the shared skills root
+        under no name, leaving ``--skill`` with nothing to match -- the #2530
+        symptom reached through the other manifest shape.
+        """
+        plugin_dir = tmp_path / "plugin"
+        plugin_dir.mkdir()
+        skill = plugin_dir / "skills" / "engineering" / "tdd"
+        skill.mkdir(parents=True)
+        (skill / "SKILL.md").write_text("# tdd", encoding="utf-8")
+
+        apm_dir = plugin_dir / ".apm"
+        apm_dir.mkdir()
+        _map_plugin_artifacts(
+            plugin_dir,
+            apm_dir,
+            manifest={"skills": "./skills/engineering/tdd"},
+        )
+
+        normalized = apm_dir / "skills"
+        assert (normalized / "tdd" / "SKILL.md").read_text() == "# tdd"
+        assert not (normalized / "SKILL.md").exists()
+
+    def test_declared_skills_string_container_flattens(self, tmp_path):
+        """The string form of a container merges, same as the array form."""
+        plugin_dir = tmp_path / "plugin"
+        plugin_dir.mkdir()
+        for name in ("alpha", "beta"):
+            skill = plugin_dir / "skills" / name
+            skill.mkdir(parents=True)
+            (skill / "SKILL.md").write_text(f"# {name}", encoding="utf-8")
+
+        apm_dir = plugin_dir / ".apm"
+        apm_dir.mkdir()
+        _map_plugin_artifacts(plugin_dir, apm_dir, manifest={"skills": "./skills/"})
+
+        normalized = apm_dir / "skills"
+        assert not (normalized / "skills").exists()
+        assert (normalized / "alpha" / "SKILL.md").read_text() == "# alpha"
+        assert (normalized / "beta" / "SKILL.md").read_text() == "# beta"
+
+    def test_declared_nested_skill_path_keeps_leaf_name(self, tmp_path):
+        """A declared entry that IS a skill lands under its own leaf name."""
+        plugin_dir = tmp_path / "plugin"
+        plugin_dir.mkdir()
+        skill = plugin_dir / "skills" / "engineering" / "tdd"
+        skill.mkdir(parents=True)
+        (skill / "SKILL.md").write_text("# tdd", encoding="utf-8")
+        sibling = plugin_dir / "skills" / "engineering" / "pairing"
+        sibling.mkdir(parents=True)
+        (sibling / "SKILL.md").write_text("# pairing", encoding="utf-8")
+
+        apm_dir = plugin_dir / ".apm"
+        apm_dir.mkdir()
+        _map_plugin_artifacts(
+            plugin_dir,
+            apm_dir,
+            manifest={"skills": ["./skills/engineering/tdd"]},
+        )
+
+        normalized = apm_dir / "skills"
+        assert (normalized / "tdd" / "SKILL.md").read_text() == "# tdd"
+        # Undeclared siblings stay out: the entry is a requirement, not a hint.
+        assert not (normalized / "pairing").exists()
+
     def test_custom_commands_path(self, tmp_path):
         """Manifest commands field redirects command discovery."""
         plugin_dir = tmp_path / "plugin"
diff --git a/tests/unit/test_symlink_containment.py b/tests/unit/test_symlink_containment.py
index f431a7c9ab..90c205a1c7 100644
--- a/tests/unit/test_symlink_containment.py
+++ b/tests/unit/test_symlink_containment.py
@@ -282,14 +282,10 @@ class TestIgnoreNonContentSourceGuard(unittest.TestCase):
     three modules (plugin_parser, packer, unpacker).
     """
 
-    # (import_path, module_attr, min_ignore_non_content_references)
-    # plugin_parser has 7 copytree calls in total: 2 agents + 3 skills +
-    # 2 hooks. After PR #1153 follow-up #2, ALL 7 use ignore_non_content;
-    # the import + 7 call sites yields >= 8 references.
-    _MODULES: typing.ClassVar[list[tuple[str, str, int]]] = [
-        ("apm_cli.deps", "plugin_parser", 5),
-        ("apm_cli.bundle", "packer", 2),
-        ("apm_cli.bundle", "unpacker", 2),
+    _MODULES: typing.ClassVar[list[tuple[str, str]]] = [
+        ("apm_cli.deps", "plugin_parser"),
+        ("apm_cli.bundle", "packer"),
+        ("apm_cli.bundle", "unpacker"),
     ]
 
     def test_copytree_sites_use_ignore_non_content(self):
@@ -299,24 +295,69 @@ def test_copytree_sites_use_ignore_non_content(self):
         Source-level guard: if a future refactor drops the callback at any
         site, this test fails before a stale or malicious ``.apm-pin`` can
         leak into the deploy target.
+
+        Asserted per call site via AST rather than by counting substrings: a
+        hand-maintained occurrence floor silently loosens whenever call sites
+        are merged or split, and it cannot tell a real argument from the same
+        identifier mentioned in a docstring.
         """
+        import ast
         import importlib
         import inspect
 
-        for pkg, mod_name, min_refs in self._MODULES:
+        def _is_shutil_copytree(node: ast.AST) -> bool:
+            """Match ``shutil.copytree(...)`` and a bare imported ``copytree``.
+
+            Deliberately not "any attribute named copytree": an unrelated
+            helper exposing that name is not what this guard is about, and
+            demanding ``ignore=`` from it would be a false failure.
+            """
+            if not isinstance(node, ast.Call):
+                return False
+            func = node.func
+            if isinstance(func, ast.Attribute):
+                return func.attr == "copytree" and (
+                    isinstance(func.value, ast.Name) and func.value.id == "shutil"
+                )
+            return isinstance(func, ast.Name) and func.id == "copytree"
+
+        def _guarded(node: ast.Call) -> bool:
+            """True when the call's ``ignore=`` names the content filter.
+
+            The argument's whole subtree is searched, so a composing helper
+            (``ignore=_wrap(ignore_non_content)``) still counts while
+            ``ignore=None`` or an unrelated callback does not. Presence of
+            the keyword alone is not enough -- that was the hole the earlier
+            occurrence count happened to cover.
+            """
+            for kw in node.keywords:
+                if kw.arg != "ignore":
+                    continue
+                return any(
+                    isinstance(sub, ast.Name) and sub.id == "ignore_non_content"
+                    for sub in ast.walk(kw.value)
+                )
+            return False
+
+        for pkg, mod_name in self._MODULES:
             with self.subTest(module=f"{pkg}.{mod_name}"):
                 mod = importlib.import_module(f"{pkg}.{mod_name}")
-                source = inspect.getsource(mod)
+                tree = ast.parse(inspect.getsource(mod))
 
-                copytree_count = source.count("shutil.copytree(")
-                ignore_refs = source.count("ignore_non_content")
+                sites = [node for node in ast.walk(tree) if _is_shutil_copytree(node)]
+                self.assertTrue(
+                    sites,
+                    f"{pkg}.{mod_name}: no copytree call sites found -- the guard "
+                    "is no longer watching anything; retarget or remove it",
+                )
 
-                self.assertGreaterEqual(
-                    ignore_refs,
-                    min_refs,
-                    f"{pkg}.{mod_name}: expected >={min_refs} ignore_non_content "
-                    f"references (for {copytree_count} copytree calls); "
-                    f"found {ignore_refs}",
+                unguarded = [node.lineno for node in sites if not _guarded(node)]
+                self.assertEqual(
+                    unguarded,
+                    [],
+                    f"{pkg}.{mod_name}: copytree at line(s) {unguarded} do not pass "
+                    "ignore=ignore_non_content -- .apm-pin and symlinks can leak "
+                    "into the deploy tree",
                 )
 
 

From 6dd0687a30d83f2c064601116f6823181a40ee81 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E8=A8=B1=E5=85=83=E8=B1=AA?=
 <146086744+edenfunf@users.noreply.github.com>
Date: Wed, 19 Aug 2026 18:32:07 +0800
Subject: [PATCH 2/8] fix(plugins): cite req-mf-022, cover the skills fallback,
 warn on dead ends

Review follow-ups for #2530.

CHANGELOG: plugin authors who hit the silent breakage need a release-note
signal that the fix shipped -- the symptom made their manifest look wrong.

Spec citation instead of a waiver. Section 8.1 counts a plugin collection
with a named-skills container among the layouts that expose selectable
skills, which is exactly req-mf-022's precondition. Before the fix such a
package enumerated nothing, so the required diagnostic fired but named
`(none)` as available for a dependency exposing two: the requirement's
"available skill names" clause answered with the wrong set. The new
conformance test pins the whole chain -- declared container normalizes one
level deep, enumeration names both skills, the diagnostic reports them.

`skill_source_dir`'s fallback to `.apm/skills/` is the branch every plugin
package traverses and had no test. Covered both halves: no root bundle at
all, and a root `skills/` that exists but carries no SKILL.md -- existing is
not the test, holding a skill is. Toggle-verified in both directions:
returning the root bundle whenever the directory exists turns the second
half red; returning it unconditionally turns the first half red.

A declared entry that is neither a skill nor a container of skills (skills
buried a level deeper, or a container with nothing in it) still keeps its own
name so unrecognized content stays out of the shared skills root -- but it no
longer does so silently. That silence is what made #2530 expensive: the
manifest looked wrong when only the directory depth was.

CONFORMANCE.{json,md} regenerated for the added req-mf-022 test.
---
 CHANGELOG.md                                  | 10 ++++
 CONFORMANCE.json                              |  5 +-
 CONFORMANCE.md                                |  2 +-
 src/apm_cli/deps/plugin_parser.py             | 28 ++++++++++
 .../test_architecture_outcome_guards.py       | 36 +++++++++++++
 tests/spec_conformance/test_manifest_reqs.py  | 52 +++++++++++++++++++
 tests/unit/test_plugin_parser.py              | 40 ++++++++++++++
 7 files changed, 170 insertions(+), 3 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 61e87cfeea..e10fee9513 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -9,6 +9,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- `apm install --skill <name>` now matches on plugins whose manifest declares
+  the conventional skills container (`"skills": ["./skills/"]`). The declared
+  container was normalized under its own name, burying every skill at
+  `.apm/skills/skills/<name>/` -- one level below the depth `--skill`
+  enumeration, deployment, the `bin/` security scan and primitive counting all
+  read, so selection reported `Available: (none)` even though a bare install
+  deployed those same skills. A declared entry that is itself a skill
+  (`"skills": "./skills/engineering/tdd"`) still lands under its own leaf name
+  instead of spilling a bare `SKILL.md` into the shared skills root.
+  (closes #2530)
 - Multi-target `apm compile` now avoids repeating expensive project analysis
   for each target, making multi-target runs scale like single-target runs
   without changing generated output. (closes #2482)
diff --git a/CONFORMANCE.json b/CONFORMANCE.json
index af5eb3d10f..5c5369e9af 100644
--- a/CONFORMANCE.json
+++ b/CONFORMANCE.json
@@ -544,9 +544,10 @@
       "keyword": "MUST",
       "section": "4.3.2",
       "status": "active",
-      "test_count": 1,
+      "test_count": 2,
       "tests": [
-        "tests/spec_conformance/test_manifest_reqs.py::test_consumer_diagnoses_empty_skill_subset_match"
+        "tests/spec_conformance/test_manifest_reqs.py::test_consumer_diagnoses_empty_skill_subset_match",
+        "tests/spec_conformance/test_manifest_reqs.py::test_consumer_names_skills_a_plugin_collection_exposes"
       ]
     },
     {
diff --git a/CONFORMANCE.md b/CONFORMANCE.md
index fb66d99518..8067d1341a 100644
--- a/CONFORMANCE.md
+++ b/CONFORMANCE.md
@@ -74,7 +74,7 @@ All four conformance classes (Producer, Consumer, Registry, Governance) carry ac
 | [req-mf-019](docs/src/content/docs/specs/openapm-v0.1.md#req-mf-019) | MUST | 4.2.4 | consumer | active | 1 |
 | [req-mf-020](docs/src/content/docs/specs/openapm-v0.1.md#req-mf-020) | MUST | 4.1 | consumer | active | 1 |
 | [req-mf-021](docs/src/content/docs/specs/openapm-v0.1.md#req-mf-021) | MUST | 4.8 | producer | active | 1 |
-| [req-mf-022](docs/src/content/docs/specs/openapm-v0.1.md#req-mf-022) | MUST | 4.3.2 | consumer | active | 1 |
+| [req-mf-022](docs/src/content/docs/specs/openapm-v0.1.md#req-mf-022) | MUST | 4.3.2 | consumer | active | 2 |
 | [req-mf-023](docs/src/content/docs/specs/openapm-v0.1.md#req-mf-023) | MUST | 4.5 | consumer | active | 1 |
 | [req-mf-024](docs/src/content/docs/specs/openapm-v0.1.md#req-mf-024) | MUST | 4.3.2 | consumer | active | 1 |
 | [req-pl-001](docs/src/content/docs/specs/openapm-v0.1.md#req-pl-001) | MUST | 6.1 | governance | active | 1 |
diff --git a/src/apm_cli/deps/plugin_parser.py b/src/apm_cli/deps/plugin_parser.py
index ba65573b49..cb9c5b5089 100644
--- a/src/apm_cli/deps/plugin_parser.py
+++ b/src/apm_cli/deps/plugin_parser.py
@@ -154,6 +154,29 @@ def _holds_skill_dirs(candidate: Path) -> bool:
         return False
 
 
+def _warn_skills_entry_holds_no_skill(entry: Path, plugin_path: Path) -> None:
+    """Warn that a declared ``skills`` entry can never yield a skill.
+
+    An entry with no ``SKILL.md`` at its root and none in any immediate child
+    is neither a skill nor a container of them: it is copied under its own
+    name, but nothing inside reaches ``.apm/skills/<name>/SKILL.md``, so it
+    stays invisible to deployment and to ``--skill``. Silence here is what
+    made #2530 expensive to diagnose -- the manifest looked wrong when only
+    the directory depth was -- so say it once, at normalization time.
+    """
+    try:
+        declared = entry.relative_to(plugin_path).as_posix()
+    except ValueError:  # pragma: no cover - entries are verified inside the plugin
+        declared = entry.name
+    _surface_warning(
+        f"Plugin skills entry '{declared}' has no SKILL.md at its root or in "
+        f"any immediate subdirectory; nothing under it will deploy or be "
+        f"selectable with --skill. Declare each skill directory, or place "
+        f"skills one level below the declared container.",
+        _logger,
+    )
+
+
 def parse_plugin_manifest(plugin_json_path: Path) -> dict[str, Any]:
     """Parse a plugin.json manifest file.
 
@@ -822,6 +845,11 @@ def _is_same_path(src: Path, dst: Path) -> bool:
             for d in skill_dirs:
                 if declared and not _holds_skill_dirs(d):
                     dest = target_skills / d.name
+                    if not (d / "SKILL.md").is_file():
+                        # Neither shape: keep the contents isolated under the
+                        # entry's own name, but do not let the dead end pass
+                        # silently -- that silence is the #2530 symptom.
+                        _warn_skills_entry_holds_no_skill(d, plugin_path)
                 else:
                     dest = target_skills
                 if _is_same_path(d, dest):
diff --git a/tests/integration/test_architecture_outcome_guards.py b/tests/integration/test_architecture_outcome_guards.py
index 5f49cd3c01..b40ec74a90 100644
--- a/tests/integration/test_architecture_outcome_guards.py
+++ b/tests/integration/test_architecture_outcome_guards.py
@@ -186,6 +186,42 @@ def test_skill_enumeration_matches_the_directory_that_deploys(
     assert SkillIntegrator.available_skill_names(info) == frozenset({"alpha", "beta"})
 
 
+def test_skill_enumeration_falls_back_to_the_normalized_container(
+    tmp_path: Path,
+) -> None:
+    """The ``.apm/skills/`` fallback is the route every plugin package takes.
+
+    A root ``skills/`` bundle wins only while it actually holds a skill. With
+    no such bundle -- or with one carrying no ``SKILL.md`` at all -- routing
+    must fall back to the normalized container ``_map_plugin_artifacts``
+    writes, or ``--skill`` is back to enumerating nothing (#2530).
+    """
+    from apm_cli.integration.skill_integrator import SkillIntegrator
+    from apm_cli.models.validation import PackageType
+
+    package = tmp_path / "pkg"
+    normalized = package / ".apm" / "skills"
+    for name in ("csharp-scripts", "dotnet-pinvoke"):
+        skill = normalized / name
+        skill.mkdir(parents=True)
+        (skill / "SKILL.md").write_text(f"# {name}\n", encoding="utf-8")
+
+    info = MagicMock(install_path=package, package_type=PackageType.MARKETPLACE_PLUGIN)
+    expected = frozenset({"csharp-scripts", "dotnet-pinvoke"})
+
+    assert SkillIntegrator.skill_source_dir(package) == normalized
+    assert SkillIntegrator.available_skill_names(info) == expected
+
+    # Existing is not the test -- holding a skill is. A root ``skills/`` with
+    # nothing selectable in it must not shadow the normalized container.
+    root_bundle = package / "skills"
+    (root_bundle / "docs").mkdir(parents=True)
+    (root_bundle / "README.md").write_text("# not a skill\n", encoding="utf-8")
+
+    assert SkillIntegrator.skill_source_dir(package) == normalized
+    assert SkillIntegrator.available_skill_names(info) == expected
+
+
 def test_stale_persisted_skill_pin_warns_instead_of_silent_noop(
     tmp_path: Path,
     monkeypatch: pytest.MonkeyPatch,
diff --git a/tests/spec_conformance/test_manifest_reqs.py b/tests/spec_conformance/test_manifest_reqs.py
index 9b672cc59e..6d9c33bfcd 100644
--- a/tests/spec_conformance/test_manifest_reqs.py
+++ b/tests/spec_conformance/test_manifest_reqs.py
@@ -19,6 +19,7 @@
 import pytest
 
 from apm_cli.adapters.client.base import MCPClientAdapter
+from apm_cli.deps.plugin_parser import normalize_plugin_directory
 from apm_cli.install.phases.finalize import _hint_project_compile_needed
 from apm_cli.install.target_filter import resolve_effective_package_targets
 from apm_cli.integration.agent_integrator import AgentIntegrator
@@ -300,6 +301,57 @@ def test_consumer_diagnoses_empty_skill_subset_match(tmp_path: Path) -> None:
     )
 
 
+@pytest.mark.req("req-mf-022")
+def test_consumer_names_skills_a_plugin_collection_exposes(tmp_path: Path) -> None:
+    """The available-names half of req-mf-022 must answer with the real set.
+
+    Section 8.1 counts a plugin collection with a named-skills container
+    among the layouts that expose selectable skills. Such a container,
+    declared in the plugin manifest, was normalized under its own name --
+    one level below the depth enumeration reads -- so a subset that matched
+    nothing reported `(none)` as available for a dependency exposing two.
+    The diagnostic fired as required and named the wrong set (#2530).
+    """
+    from apm_cli.models.validation import PackageType
+
+    plugin = tmp_path / "dotnet-advanced"
+    plugin_json = plugin / ".claude-plugin" / "plugin.json"
+    plugin_json.parent.mkdir(parents=True)
+    plugin_json.write_text(
+        '{"name": "dotnet-advanced", "version": "1.0.0", "skills": ["./skills/"]}',
+        encoding="utf-8",
+    )
+    for name in ("csharp-scripts", "dotnet-pinvoke"):
+        skill = plugin / "skills" / name
+        skill.mkdir(parents=True)
+        (skill / "SKILL.md").write_text(f"# {name}\n", encoding="utf-8")
+
+    normalize_plugin_directory(plugin, plugin_json)
+
+    # The declared container names the skills; it is not itself one.
+    normalized = plugin / ".apm" / "skills"
+    assert not (normalized / "skills").exists()
+    assert (normalized / "csharp-scripts" / "SKILL.md").is_file()
+
+    available = SkillIntegrator.available_skill_names(
+        SimpleNamespace(install_path=plugin, package_type=PackageType.MARKETPLACE_PLUGIN)
+    )
+    assert available == frozenset({"csharp-scripts", "dotnet-pinvoke"})
+
+    diagnostics = DiagnosticCollector()
+    SkillIntegrator._warn_no_skill_filter_match(
+        available,
+        ("missing",),
+        "acme/dotnet-advanced",
+        diagnostics=diagnostics,
+    )
+    warning = diagnostics.by_category()[CATEGORY_WARNING][0]
+    assert "Available: csharp-scripts, dotnet-pinvoke" in warning.message
+    assert "Available: (none)" not in warning.message
+
+    assert_spec_contains("with a named-skills container")
+
+
 @pytest.mark.req("req-mf-024")
 def test_consumer_preserves_registry_identity_on_structured_rewrite(monkeypatch):
     """req-mf-024: a registry-sourced (`id:`) entry MUST NOT be silently
diff --git a/tests/unit/test_plugin_parser.py b/tests/unit/test_plugin_parser.py
index 8a50fdea75..4380319905 100644
--- a/tests/unit/test_plugin_parser.py
+++ b/tests/unit/test_plugin_parser.py
@@ -1,6 +1,7 @@
 """Unit tests for plugin_parser.py and find_plugin_json helper."""
 
 import json
+import logging
 import os  # noqa: F401
 from pathlib import Path
 
@@ -349,6 +350,45 @@ def test_declared_nested_skill_path_keeps_leaf_name(self, tmp_path):
         # Undeclared siblings stay out: the entry is a requirement, not a hint.
         assert not (normalized / "pairing").exists()
 
+    def test_declared_skills_entry_holding_no_skill_warns(self, tmp_path, caplog):
+        """An entry that is neither a skill nor a container must say so.
+
+        A container whose skills sit two levels down reaches no deployable
+        depth under either mapping. The copy stays put -- the entry keeps its
+        own name so unrecognized content stays out of the shared skills root
+        -- but the plugin author gets the one line that #2530 lacked instead
+        of an install that looks clean and deploys nothing.
+        """
+        plugin_dir = tmp_path / "plugin"
+        plugin_dir.mkdir()
+        buried = plugin_dir / "skills" / "engineering" / "tdd"
+        buried.mkdir(parents=True)
+        (buried / "SKILL.md").write_text("# tdd", encoding="utf-8")
+
+        apm_dir = plugin_dir / ".apm"
+        apm_dir.mkdir()
+        with caplog.at_level(logging.WARNING, logger="apm_cli.deps.plugin_parser"):
+            _map_plugin_artifacts(plugin_dir, apm_dir, manifest={"skills": ["./skills/"]})
+
+        assert "skills" in caplog.text
+        assert "no SKILL.md" in caplog.text
+        assert "--skill" in caplog.text
+
+    def test_declared_skills_container_does_not_warn(self, tmp_path, caplog):
+        """The healthy shapes stay quiet -- a warning nobody can act on is noise."""
+        plugin_dir = tmp_path / "plugin"
+        plugin_dir.mkdir()
+        skill = plugin_dir / "skills" / "csharp-scripts"
+        skill.mkdir(parents=True)
+        (skill / "SKILL.md").write_text("# csharp-scripts", encoding="utf-8")
+
+        apm_dir = plugin_dir / ".apm"
+        apm_dir.mkdir()
+        with caplog.at_level(logging.WARNING, logger="apm_cli.deps.plugin_parser"):
+            _map_plugin_artifacts(plugin_dir, apm_dir, manifest={"skills": ["./skills/"]})
+
+        assert "no SKILL.md" not in caplog.text
+
     def test_custom_commands_path(self, tmp_path):
         """Manifest commands field redirects command discovery."""
         plugin_dir = tmp_path / "plugin"

From 029b2068a5169aacc215c307e6aaade957428446 Mon Sep 17 00:00:00 2001
From: danielmeppiel <danielmeppiel@users.noreply.github.com>
Date: Sun, 23 Aug 2026 21:19:09 +0200
Subject: [PATCH 3/8] fix(plugins): preserve manifest-owned skill routing

Fold the review-panel follow-ups by making normalized plugin skills authoritative for selection and deployment, rejecting unsafe source and destination symlinks, failing closed on normalized-name collisions, documenting diagnostics, and adding the dual architecture guardrail.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---
 .../instructions/architecture.instructions.md |   1 +
 CHANGELOG.md                                  |  15 +-
 .../content/docs/reference/package-types.md   |   5 +
 .../skills/apm-usage/package-authoring.md     |   6 +
 scripts/lint-architecture-boundaries.sh       |  65 ++++++++
 src/apm_cli/deps/plugin_parser.py             |  59 ++++++-
 src/apm_cli/integration/skill_integrator.py   |  37 +++--
 .../test_architecture_outcome_guards.py       | 155 +++++++++++++++++-
 tests/unit/test_plugin_parser.py              |  40 +++++
 tests/unit/test_symlink_containment.py        |  58 ++++---
 10 files changed, 388 insertions(+), 53 deletions(-)

diff --git a/.apm/instructions/architecture.instructions.md b/.apm/instructions/architecture.instructions.md
index 8cbc3542aa..270a658874 100644
--- a/.apm/instructions/architecture.instructions.md
+++ b/.apm/instructions/architecture.instructions.md
@@ -71,6 +71,7 @@ semicolon-delimited, and specific to the file(s) that own the fact.
 | Effective marketplace output path | marketplace/output_profiles.py (resolve_effective_output_path) | `src/apm_cli/marketplace/output_profiles.py` |
 | Bootstrap project-name validation and fallback | core/project_name.py (resolve_bootstrap_project_name) | `src/apm_cli/core/project_name.py` |
 | Marketplace raw-structure diagnostics | marketplace/models.py parser; validator.py consumes them | `src/apm_cli/marketplace/models.py`; `src/apm_cli/marketplace/validator.py` |
+| Selectable and deployable skill source | integration/skill_integrator.py (SkillIntegrator.skill_source_dir) | `src/apm_cli/integration/skill_integrator.py` |
 <!-- /canonical-owner-table -->
 
 Host + credential resolution includes public github.com anonymous-first ordering.
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 8cfdacc4b2..f6c273642e 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -19,16 +19,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Windows binary is now Authenticode-signed in the release workflow, eliminating
   the `Trojan:Script/Wacatac.H!ml` Windows Defender false positive on unsigned
   PyInstaller bundles. (#2435)
-- `apm install --skill <name>` now matches on plugins whose manifest declares
-  the conventional skills container (`"skills": ["./skills/"]`). The declared
-  container was normalized under its own name, burying every skill at
-  `.apm/skills/skills/<name>/` -- one level below the depth `--skill`
-  enumeration, deployment, the `bin/` security scan and primitive counting all
-  read, so selection reported `Available: (none)` even though a bare install
-  deployed those same skills. A declared entry that is itself a skill
-  (`"skills": "./skills/engineering/tdd"`) still lands under its own leaf name
-  instead of spilling a bare `SKILL.md` into the shared skills root.
-  (closes #2530)
+- `apm install --skill <name>` now selects individual skills from plugins that
+  declare the conventional `"skills": ["./skills/"]` container instead of
+  reporting `Available: (none)`. Manifest declarations remain authoritative,
+  unsafe symlink sources are ignored, and malformed or colliding skill entries
+  now produce actionable diagnostics. (closes #2530) - by @edenfunf (#2536)
 - Multi-target `apm compile` now avoids repeating expensive project analysis
   for each target, making multi-target runs scale like single-target runs
   without changing generated output. (closes #2482)
diff --git a/docs/src/content/docs/reference/package-types.md b/docs/src/content/docs/reference/package-types.md
index 5b97f9cfdd..de1750d2bb 100644
--- a/docs/src/content/docs/reference/package-types.md
+++ b/docs/src/content/docs/reference/package-types.md
@@ -223,6 +223,11 @@ root, install exits non-zero before deployment or lockfile commit. Likewise,
 Omit an optional field or use an empty list when the plugin has no component
 of that type.
 
+A declared `skills` entry can name either one skill directory or a container
+whose immediate children are skills. If the entry has no reachable `SKILL.md`
+at either depth, APM warns that nothing is selectable or deployable. Declare
+the skill directory itself, or place each skill one level below the container.
+
 **When to choose:** you already have a Claude plugin and want APM to
 consume it without restructuring.
 
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 de4d3d2ffc..bc089740f8 100644
--- a/packages/apm-guide/.apm/skills/apm-usage/package-authoring.md
+++ b/packages/apm-guide/.apm/skills/apm-usage/package-authoring.md
@@ -510,6 +510,12 @@ Packaged distribution format created with `apm pack --format plugin`.
 
 When `apm.yml` declares `target: claude` or `target: copilot` (or the plural `targets:` equivalent), `apm pack` also generates an ecosystem-specific `plugin.json` automatically -- authors no longer need to maintain this file manually. The manifest is synthesised from `apm.yml` identity fields (`name`, `version`, `description`, `author`, `license`). See the apm pack reference (reference/cli/pack/#plugin-manifests) for output paths, credential stripping, and per-ecosystem differences, or run `apm pack --help`.
 
+In a hand-authored `plugin.json`, `skills` may name one skill directory or a
+container whose immediate children each contain `SKILL.md`. APM warns when a
+declared entry has no `SKILL.md` at either depth because it cannot be selected
+or deployed. Declare the skill directory itself, or move skills one level
+below the declared container.
+
 #### Shipping `bin/` executables (Claude Code only)
 
 A marketplace plugin may ship a root `bin/` directory of executable
diff --git a/scripts/lint-architecture-boundaries.sh b/scripts/lint-architecture-boundaries.sh
index 4b6c4f75e0..5361dc92e1 100755
--- a/scripts/lint-architecture-boundaries.sh
+++ b/scripts/lint-architecture-boundaries.sh
@@ -1552,6 +1552,71 @@ if [ "$mcp_runtime_variable_owner_defs" -ne 1 ] \
     violations=$((violations + 1))
 fi
 
+echo "[*] AC33: skill source routing authority"
+skill_source_owner="src/apm_cli/integration/skill_integrator.py"
+skill_source_check=$(python3 - "$skill_source_owner" <<'PY'
+import ast
+import sys
+from pathlib import Path
+
+tree = ast.parse(Path(sys.argv[1]).read_text(encoding="utf-8"))
+methods = {
+    node.name: node
+    for node in ast.walk(tree)
+    if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef))
+    and node.name in {"skill_source_dir", "available_skill_names", "integrate_package_skill"}
+}
+errors = []
+if set(methods) != {"skill_source_dir", "available_skill_names", "integrate_package_skill"}:
+    errors.append("required methods are missing")
+
+
+def calls_owner(node: ast.AST, receiver: str) -> bool:
+    return any(
+        isinstance(call, ast.Call)
+        and isinstance(call.func, ast.Attribute)
+        and call.func.attr == "skill_source_dir"
+        and isinstance(call.func.value, ast.Name)
+        and call.func.value.id == receiver
+        and len(call.args) == 1
+        and isinstance(call.args[0], ast.Name)
+        and call.args[0].id == "package_info"
+        for call in ast.walk(node)
+    )
+
+
+if "available_skill_names" in methods and not calls_owner(
+    methods["available_skill_names"], "SkillIntegrator"
+):
+    errors.append("available_skill_names must call SkillIntegrator.skill_source_dir(package_info)")
+if "integrate_package_skill" in methods and not calls_owner(
+    methods["integrate_package_skill"], "self"
+):
+    errors.append("integrate_package_skill must call self.skill_source_dir(package_info)")
+if "skill_source_dir" in methods:
+    owner_source = ast.unparse(methods["skill_source_dir"])
+    if "PackageType.MARKETPLACE_PLUGIN" not in owner_source:
+        errors.append("skill_source_dir must preserve marketplace plugin manifest authority")
+    owner_constants = {
+        node.value
+        for node in ast.walk(methods["skill_source_dir"])
+        if isinstance(node, ast.Constant) and isinstance(node.value, str)
+    }
+    if not {".apm", "skills"}.issubset(owner_constants):
+        errors.append("skill_source_dir must own the normalized plugin skills path")
+
+if errors:
+    print("\n".join(errors))
+    raise SystemExit(1)
+PY
+)
+skill_source_status=$?
+if [ "$skill_source_status" -ne 0 ]; then
+    echo "[x] Skill source selection must route through SkillIntegrator.skill_source_dir"
+    echo "$skill_source_check"
+    violations=$((violations + 1))
+fi
+
 echo "[*] AC18: bootstrap project-name authority"
 if ! uv run --extra dev python scripts/lint-bootstrap-project-name.py; then
     echo "[x] Manifest bootstrap names must route through core/project_name.py"
diff --git a/src/apm_cli/deps/plugin_parser.py b/src/apm_cli/deps/plugin_parser.py
index cb9c5b5089..99f3dba7e3 100644
--- a/src/apm_cli/deps/plugin_parser.py
+++ b/src/apm_cli/deps/plugin_parser.py
@@ -91,6 +91,12 @@ def _assert_no_symlink_descendants(target: Path) -> None:
                 )
 
 
+def _assert_safe_plugin_destination(apm_dir: Path) -> None:
+    """Require the normalization root to be a real directory, not a symlink."""
+    if apm_dir.is_symlink():
+        raise PluginIntegrityError(f"Refusing to normalize through symlinked directory: {apm_dir}")
+
+
 def _surface_warning(message: str, logger: logging.Logger) -> None:
     """Emit a warning to both the stdlib logger and the rich console.
 
@@ -144,17 +150,26 @@ def _holds_skill_dirs(candidate: Path) -> bool:
     are not containers, so a declared entry keeps its own name rather than
     spilling unrecognized contents into the shared skills root.
     """
-    if (candidate / "SKILL.md").is_file():
+    skill_manifest = candidate / "SKILL.md"
+    if not skill_manifest.is_symlink() and skill_manifest.is_file():
         return False
     try:
         return any(
-            (child / "SKILL.md").is_file() for child in candidate.iterdir() if child.is_dir()
+            not child.is_symlink()
+            and child.is_dir()
+            and not (child / "SKILL.md").is_symlink()
+            and (child / "SKILL.md").is_file()
+            for child in candidate.iterdir()
         )
     except OSError:
         return False
 
 
-def _warn_skills_entry_holds_no_skill(entry: Path, plugin_path: Path) -> None:
+def _warn_skills_entry_holds_no_skill(
+    entry: Path,
+    plugin_path: Path,
+    plugin_name: str,
+) -> None:
     """Warn that a declared ``skills`` entry can never yield a skill.
 
     An entry with no ``SKILL.md`` at its root and none in any immediate child
@@ -169,7 +184,7 @@ def _warn_skills_entry_holds_no_skill(entry: Path, plugin_path: Path) -> None:
     except ValueError:  # pragma: no cover - entries are verified inside the plugin
         declared = entry.name
     _surface_warning(
-        f"Plugin skills entry '{declared}' has no SKILL.md at its root or in "
+        f"Plugin '{plugin_name}' skills entry '{declared}' has no SKILL.md at its root or in "
         f"any immediate subdirectory; nothing under it will deploy or be "
         f"selectable with --skill. Declare each skill directory, or place "
         f"skills one level below the declared container.",
@@ -304,6 +319,7 @@ def synthesize_apm_yml_from_plugin(plugin_path: Path, manifest: dict[str, Any])
 
     # Create .apm directory structure
     apm_dir = plugin_path / ".apm"
+    _assert_safe_plugin_destination(apm_dir)
     apm_dir.mkdir(exist_ok=True)
 
     # Map plugin structure into .apm/ subdirectories
@@ -741,6 +757,8 @@ def _map_plugin_artifacts(
     if manifest is None:
         manifest = {}
 
+    _assert_safe_plugin_destination(apm_dir)
+
     from apm_cli.security.gate import ignore_non_content
 
     # Resolve source paths  -- use manifest arrays if present, else defaults.
@@ -842,14 +860,43 @@ def _is_same_path(src: Path, dst: Path) -> bool:
         declared = isinstance(manifest.get("skills"), (list, str))
         if skill_dirs:
             target_skills.mkdir(parents=True, exist_ok=True)
+            claimed_names: dict[str, Path] = {}
             for d in skill_dirs:
+                if declared and not _holds_skill_dirs(d):
+                    declared_manifest = d / "SKILL.md"
+                    deployable_names = (
+                        (d.name,)
+                        if not declared_manifest.is_symlink() and declared_manifest.is_file()
+                        else ()
+                    )
+                else:
+                    deployable_names = tuple(
+                        child.name
+                        for child in d.iterdir()
+                        if not child.is_symlink()
+                        and child.is_dir()
+                        and not (child / "SKILL.md").is_symlink()
+                        and (child / "SKILL.md").is_file()
+                    )
+                for name in deployable_names:
+                    previous = claimed_names.get(name)
+                    if previous is not None and previous != d:
+                        raise PluginIntegrityError(
+                            f"Plugin skill declarations collide on normalized name '{name}'"
+                        )
+                    claimed_names[name] = d
                 if declared and not _holds_skill_dirs(d):
                     dest = target_skills / d.name
-                    if not (d / "SKILL.md").is_file():
+                    skill_manifest = d / "SKILL.md"
+                    if skill_manifest.is_symlink() or not skill_manifest.is_file():
                         # Neither shape: keep the contents isolated under the
                         # entry's own name, but do not let the dead end pass
                         # silently -- that silence is the #2530 symptom.
-                        _warn_skills_entry_holds_no_skill(d, plugin_path)
+                        _warn_skills_entry_holds_no_skill(
+                            d,
+                            plugin_path,
+                            str(manifest.get("name") or plugin_path.name),
+                        )
                 else:
                     dest = target_skills
                 if _is_same_path(d, dest):
diff --git a/src/apm_cli/integration/skill_integrator.py b/src/apm_cli/integration/skill_integrator.py
index 4c546f20fa..048a47a7ae 100644
--- a/src/apm_cli/integration/skill_integrator.py
+++ b/src/apm_cli/integration/skill_integrator.py
@@ -9,6 +9,7 @@
 from collections.abc import Callable
 from dataclasses import dataclass, replace
 from pathlib import Path
+from typing import TYPE_CHECKING
 
 from apm_cli.core.deployment_state import MaterializationResult
 from apm_cli.integration.base_integrator import BaseIntegrator
@@ -16,6 +17,9 @@
 from apm_cli.models.dependency.subsets import skill_subset_filter_tokens
 from apm_cli.utils.atomic_io import write_text_lf
 
+if TYPE_CHECKING:
+    from apm_cli.models.apm_package import PackageInfo
+
 
 def _build_copy_ignore(
     *,
@@ -604,34 +608,44 @@ def _resolve_markdown_links_in_skill_bundle(
     @staticmethod
     def _skill_names_in_directory(skills_dir: Path) -> frozenset[str]:
         """Return deployable skill names from a directory that may be absent."""
-        if not skills_dir.is_dir():
+        if skills_dir.is_symlink() or not skills_dir.is_dir():
             return frozenset()
         try:
             return frozenset(
                 child.name
                 for child in skills_dir.iterdir()
-                if child.is_dir() and (child / "SKILL.md").is_file()
+                if not child.is_symlink()
+                and child.is_dir()
+                and not (child / "SKILL.md").is_symlink()
+                and (child / "SKILL.md").is_file()
             )
         except FileNotFoundError:
             return frozenset()
 
     @staticmethod
-    def skill_source_dir(package_path: Path) -> Path:
+    def skill_source_dir(package_info: "PackageInfo") -> Path:
         """Return the directory a package's deployable skills are promoted from.
 
         Single source of truth for skill routing: deployment and ``--skill``
         enumeration both resolve through here so the set a user may select can
-        never drift from the set that actually deploys (issue #2530). A root
-        ``skills/`` bundle wins whenever it holds at least one skill; otherwise
-        the normalized ``.apm/skills/`` location supplies them.
+        never drift from the set that actually deploys (issue #2530). Plugin
+        manifests own their declared component set, so normalized plugin skills
+        always route through ``.apm/skills/``. Other package types prefer a root
+        ``skills/`` bundle when it holds at least one deployable skill.
 
         Callers must handle a root ``SKILL.md`` before asking: a native
         single-skill package deploys the bundle itself, not children.
         """
+        from apm_cli.models.apm_package import PackageType
+
+        package_path = package_info.install_path
+        normalized = package_path / ".apm" / "skills"
+        if package_info.package_type == PackageType.MARKETPLACE_PLUGIN:
+            return normalized
         root_bundle = package_path / "skills"
         if SkillIntegrator._skill_names_in_directory(root_bundle):
             return root_bundle
-        return package_path / ".apm" / "skills"
+        return normalized
 
     @staticmethod
     def available_skill_names(package_info) -> frozenset[str] | None:
@@ -641,7 +655,7 @@ def available_skill_names(package_info) -> frozenset[str] | None:
             return None
 
         return SkillIntegrator._skill_names_in_directory(
-            SkillIntegrator.skill_source_dir(package_path)
+            SkillIntegrator.skill_source_dir(package_info)
         )
 
     @staticmethod
@@ -730,9 +744,10 @@ def _promote_sub_skills(
             rel_prefix = target_skills_root.name
 
         for sub_skill_path in sub_skills_dir.iterdir():
-            if not sub_skill_path.is_dir():
+            if sub_skill_path.is_symlink() or not sub_skill_path.is_dir():
                 continue
-            if not (sub_skill_path / "SKILL.md").exists():
+            skill_manifest = sub_skill_path / "SKILL.md"
+            if skill_manifest.is_symlink() or not skill_manifest.is_file():
                 continue
             raw_sub_name = sub_skill_path.name
             # --skill filter: skip skills not in the requested subset
@@ -1481,7 +1496,7 @@ def integrate_package_skill(
         # ``available_skill_names`` -- so a ``--skill`` value can never be
         # validated against a directory other than the one that deploys.
         root_skills_dir = package_path / "skills"
-        if self.skill_source_dir(package_path) == root_skills_dir:
+        if self.skill_source_dir(package_info) == root_skills_dir:
             return self._merge_bin_paths(
                 self._integrate_skill_bundle(
                     package_info,
diff --git a/tests/integration/test_architecture_outcome_guards.py b/tests/integration/test_architecture_outcome_guards.py
index b40ec74a90..0d404b9708 100644
--- a/tests/integration/test_architecture_outcome_guards.py
+++ b/tests/integration/test_architecture_outcome_guards.py
@@ -172,7 +172,7 @@ def test_skill_enumeration_matches_the_directory_that_deploys(
     bundle, so a selectable name and a deployable name could disagree.
     """
     from apm_cli.integration.skill_integrator import SkillIntegrator
-    from apm_cli.models.validation import PackageType
+    from apm_cli.models.apm_package import PackageType
 
     package = tmp_path / "pkg"
     for name in ("alpha", "beta"):
@@ -182,7 +182,12 @@ def test_skill_enumeration_matches_the_directory_that_deploys(
 
     info = MagicMock(install_path=package, package_type=PackageType.MARKETPLACE_PLUGIN)
 
-    assert SkillIntegrator.skill_source_dir(package) == package / "skills"
+    assert SkillIntegrator.skill_source_dir(info) == package / ".apm" / "skills"
+    assert SkillIntegrator.available_skill_names(info) == frozenset()
+
+    info.package_type = PackageType.SKILL_BUNDLE
+
+    assert SkillIntegrator.skill_source_dir(info) == package / "skills"
     assert SkillIntegrator.available_skill_names(info) == frozenset({"alpha", "beta"})
 
 
@@ -197,7 +202,7 @@ def test_skill_enumeration_falls_back_to_the_normalized_container(
     writes, or ``--skill`` is back to enumerating nothing (#2530).
     """
     from apm_cli.integration.skill_integrator import SkillIntegrator
-    from apm_cli.models.validation import PackageType
+    from apm_cli.models.apm_package import PackageType
 
     package = tmp_path / "pkg"
     normalized = package / ".apm" / "skills"
@@ -209,7 +214,7 @@ def test_skill_enumeration_falls_back_to_the_normalized_container(
     info = MagicMock(install_path=package, package_type=PackageType.MARKETPLACE_PLUGIN)
     expected = frozenset({"csharp-scripts", "dotnet-pinvoke"})
 
-    assert SkillIntegrator.skill_source_dir(package) == normalized
+    assert SkillIntegrator.skill_source_dir(info) == normalized
     assert SkillIntegrator.available_skill_names(info) == expected
 
     # Existing is not the test -- holding a skill is. A root ``skills/`` with
@@ -218,10 +223,150 @@ def test_skill_enumeration_falls_back_to_the_normalized_container(
     (root_bundle / "docs").mkdir(parents=True)
     (root_bundle / "README.md").write_text("# not a skill\n", encoding="utf-8")
 
-    assert SkillIntegrator.skill_source_dir(package) == normalized
+    assert SkillIntegrator.skill_source_dir(info) == normalized
     assert SkillIntegrator.available_skill_names(info) == expected
 
 
+def test_plugin_manifest_skill_set_beats_undeclared_root_bundle(
+    tmp_path: Path,
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    """Plugin deployment and selection must honor only manifest-declared skills."""
+    from click.testing import CliRunner
+
+    from apm_cli.cli import cli
+
+    plugin, consumer = _write_plugin_consumer(
+        tmp_path,
+        {
+            "name": "manifest-owned-skills",
+            "version": "1.0.0",
+            "skills": ["./skills/declared"],
+        },
+    )
+    for name in ("declared", "undeclared"):
+        skill = plugin / "skills" / name
+        skill.mkdir(parents=True)
+        (skill / "SKILL.md").write_text(f"# {name}\n", encoding="utf-8")
+    monkeypatch.chdir(consumer)
+    monkeypatch.setattr("apm_cli.cli._check_and_notify_updates", lambda: None)
+
+    rejected = CliRunner().invoke(cli, ["install", "--skill", "undeclared"])
+
+    assert rejected.exit_code != 0, rejected.output
+    assert "matched no declared skills" in rejected.output
+    assert not (consumer / ".claude" / "skills" / "undeclared").exists()
+
+    accepted = CliRunner().invoke(cli, ["install", "--skill", "declared"])
+
+    assert accepted.exit_code == 0, accepted.output
+    assert (consumer / ".claude" / "skills" / "declared" / "SKILL.md").is_file()
+    assert not (consumer / ".claude" / "skills" / "undeclared").exists()
+
+    full_install = CliRunner().invoke(cli, ["install"])
+
+    assert full_install.exit_code == 0, full_install.output
+    assert not (consumer / ".claude" / "skills" / "undeclared").exists()
+
+
+def test_declared_string_skill_installs_from_normalized_source(
+    tmp_path: Path,
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    """The string manifest form must cross normalization and deployment."""
+    from click.testing import CliRunner
+
+    from apm_cli.cli import cli
+
+    plugin, consumer = _write_plugin_consumer(
+        tmp_path,
+        {
+            "name": "single-declared-skill",
+            "version": "1.0.0",
+            "skills": "./custom/tdd",
+        },
+    )
+    skill = plugin / "custom" / "tdd"
+    skill.mkdir(parents=True)
+    (skill / "SKILL.md").write_text("# tdd\n", encoding="utf-8")
+    monkeypatch.chdir(consumer)
+    monkeypatch.setattr("apm_cli.cli._check_and_notify_updates", lambda: None)
+
+    result = CliRunner().invoke(cli, ["install", "--skill", "tdd"])
+
+    assert result.exit_code == 0, result.output
+    assert (consumer / ".claude" / "skills" / "tdd" / "SKILL.md").is_file()
+
+
+def test_malformed_declared_skills_warns_during_install(
+    tmp_path: Path,
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    """A dead plugin declaration must identify itself at the CLI boundary."""
+    from click.testing import CliRunner
+
+    from apm_cli.cli import cli
+
+    plugin, consumer = _write_plugin_consumer(
+        tmp_path,
+        {
+            "name": "buried-skills",
+            "version": "1.0.0",
+            "skills": "./skills",
+        },
+    )
+    buried = plugin / "skills" / "engineering" / "tdd"
+    buried.mkdir(parents=True)
+    (buried / "SKILL.md").write_text("# tdd\n", encoding="utf-8")
+    monkeypatch.chdir(consumer)
+    monkeypatch.setattr("apm_cli.cli._check_and_notify_updates", lambda: None)
+
+    result = CliRunner().invoke(cli, ["install"])
+
+    assert result.exit_code == 0, result.output
+    assert "Plugin 'buried-skills' skills entry 'skills'" in result.output
+    assert "no SKILL.md" in result.output
+    assert "--skill" in result.output
+
+
+def test_symlinked_skill_source_is_not_deployable(tmp_path: Path) -> None:
+    """A top-level skill symlink must never become a copytree source root."""
+    from apm_cli.integration.skill_integrator import SkillIntegrator
+
+    external = tmp_path / "external"
+    external.mkdir()
+    (external / "SKILL.md").write_text("# external\n", encoding="utf-8")
+    (external / "secret.txt").write_text("secret\n", encoding="utf-8")
+    source = tmp_path / "skills"
+    source.mkdir()
+    linked = source / "linked"
+    try:
+        linked.symlink_to(external, target_is_directory=True)
+    except OSError:
+        pytest.skip("Symlinks are unavailable")
+
+    target = tmp_path / "deployed"
+    target.mkdir()
+    count, deployed = SkillIntegrator._promote_sub_skills(source, target, "plugin")
+
+    assert count == 0
+    assert deployed == []
+    assert not (target / "linked").exists()
+
+
+def test_skill_source_routing_has_one_static_owner() -> None:
+    """The boundary lint must defend both skill-source consumer edges."""
+    root = Path(__file__).parents[2]
+    guard = (root / "scripts/lint-architecture-boundaries.sh").read_text(encoding="utf-8")
+    architecture_doc = (root / ".apm/instructions/architecture.instructions.md").read_text(
+        encoding="utf-8"
+    )
+
+    assert "AC33: skill source routing authority" in guard
+    assert "Skill source selection must route through SkillIntegrator.skill_source_dir" in guard
+    assert "Selectable and deployable skill source" in architecture_doc
+
+
 def test_stale_persisted_skill_pin_warns_instead_of_silent_noop(
     tmp_path: Path,
     monkeypatch: pytest.MonkeyPatch,
diff --git a/tests/unit/test_plugin_parser.py b/tests/unit/test_plugin_parser.py
index 4380319905..9cb0da679a 100644
--- a/tests/unit/test_plugin_parser.py
+++ b/tests/unit/test_plugin_parser.py
@@ -370,10 +370,50 @@ def test_declared_skills_entry_holding_no_skill_warns(self, tmp_path, caplog):
         with caplog.at_level(logging.WARNING, logger="apm_cli.deps.plugin_parser"):
             _map_plugin_artifacts(plugin_dir, apm_dir, manifest={"skills": ["./skills/"]})
 
+        assert "Plugin 'plugin'" in caplog.text
         assert "skills" in caplog.text
         assert "no SKILL.md" in caplog.text
         assert "--skill" in caplog.text
 
+    def test_colliding_declared_skill_containers_fail_closed(self, tmp_path):
+        """Two declared containers cannot silently merge one skill name."""
+        plugin_dir = tmp_path / "plugin"
+        plugin_dir.mkdir()
+        for container_name in ("first", "second"):
+            skill = plugin_dir / container_name / "shared"
+            skill.mkdir(parents=True)
+            (skill / "SKILL.md").write_text(f"# {container_name}", encoding="utf-8")
+
+        apm_dir = plugin_dir / ".apm"
+        apm_dir.mkdir()
+
+        with pytest.raises(PluginIntegrityError, match="collide"):
+            _map_plugin_artifacts(
+                plugin_dir,
+                apm_dir,
+                manifest={"skills": ["./first", "./second"]},
+            )
+
+    def test_symlinked_apm_directory_cannot_redirect_normalization(self, tmp_path):
+        """A plugin-controlled .apm symlink must not redirect artifact writes."""
+        plugin_dir = tmp_path / "plugin"
+        plugin_dir.mkdir()
+        skill = plugin_dir / "skills" / "safe"
+        skill.mkdir(parents=True)
+        (skill / "SKILL.md").write_text("# safe", encoding="utf-8")
+        outside = tmp_path / "outside"
+        outside.mkdir()
+        apm_dir = plugin_dir / ".apm"
+        try:
+            apm_dir.symlink_to(outside, target_is_directory=True)
+        except OSError:
+            pytest.skip("Symlinks are unavailable")
+
+        with pytest.raises(PluginIntegrityError, match="symlinked directory"):
+            _map_plugin_artifacts(plugin_dir, apm_dir, manifest={"skills": ["./skills"]})
+
+        assert not (outside / "skills").exists()
+
     def test_declared_skills_container_does_not_warn(self, tmp_path, caplog):
         """The healthy shapes stay quiet -- a warning nobody can act on is noise."""
         plugin_dir = tmp_path / "plugin"
diff --git a/tests/unit/test_symlink_containment.py b/tests/unit/test_symlink_containment.py
index 90c205a1c7..c279ee6d76 100644
--- a/tests/unit/test_symlink_containment.py
+++ b/tests/unit/test_symlink_containment.py
@@ -247,28 +247,47 @@ def test_skill_integrator_native_skill_copytree_uses_ignore_non_content(self):
         Source-level guard: if a future refactor drops the callback,
         this test fails before any malicious package can exploit it.
         """
+        import ast
         import inspect
 
         from apm_cli.integration import skill_integrator
 
         source = inspect.getsource(skill_integrator)
-        # All three copytree calls in skill_integrator.py must reference
-        # ignore_non_content (directly or via a composing helper).
-        copytree_count = source.count("shutil.copytree(")
-        ignore_non_content_refs = source.count("ignore_non_content")
-        self.assertGreaterEqual(
-            copytree_count,
-            3,
-            f"Expected >=3 copytree calls in skill_integrator, found {copytree_count}",
-        )
-        # Each copytree must be matched by at least one ignore_non_content
-        # reference (the helper composes one import + one usage inside a
-        # closure -- still >=copytree_count).
-        self.assertGreaterEqual(
-            ignore_non_content_refs,
-            copytree_count,
-            f"Expected >={copytree_count} ignore_non_content references "
-            f"(one per copytree); found {ignore_non_content_refs}",
+        tree = ast.parse(source)
+        allowed_callbacks = {
+            "ignore_non_content",
+            "_build_copy_ignore",
+            "_ignore_non_content_and_apm",
+        }
+        sites = [
+            node
+            for node in ast.walk(tree)
+            if isinstance(node, ast.Call)
+            and isinstance(node.func, ast.Attribute)
+            and isinstance(node.func.value, ast.Name)
+            and node.func.value.id == "shutil"
+            and node.func.attr == "copytree"
+        ]
+
+        def _callback_name(node: ast.Call) -> str | None:
+            for keyword in node.keywords:
+                if keyword.arg != "ignore":
+                    continue
+                value = keyword.value
+                if isinstance(value, ast.Name):
+                    return value.id
+                if isinstance(value, ast.Call) and isinstance(value.func, ast.Name):
+                    return value.func.id
+            return None
+
+        self.assertGreaterEqual(len(sites), 3)
+        self.assertEqual(
+            [
+                (node.lineno, _callback_name(node))
+                for node in sites
+                if _callback_name(node) not in allowed_callbacks
+            ],
+            [],
         )
 
 
@@ -333,10 +352,7 @@ def _guarded(node: ast.Call) -> bool:
             for kw in node.keywords:
                 if kw.arg != "ignore":
                     continue
-                return any(
-                    isinstance(sub, ast.Name) and sub.id == "ignore_non_content"
-                    for sub in ast.walk(kw.value)
-                )
+                return isinstance(kw.value, ast.Name) and kw.value.id == "ignore_non_content"
             return False
 
         for pkg, mod_name in self._MODULES:

From 7d9a987f5112a56e6a464204a2e00489219e9051 Mon Sep 17 00:00:00 2001
From: danielmeppiel <danielmeppiel@users.noreply.github.com>
Date: Sun, 23 Aug 2026 21:50:10 +0200
Subject: [PATCH 4/8] fix(plugins): fail closed on skill identities

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---
 .../skills/apm-usage/package-authoring.md     |   8 +-
 scripts/lint-architecture-boundaries.sh       |  34 ++-
 src/apm_cli/deps/plugin_parser.py             | 211 ++++++++++--------
 src/apm_cli/integration/skill_integrator.py   |  41 +++-
 .../test_architecture_outcome_guards.py       |  63 ++++++
 tests/unit/test_plugin_parser.py              |  81 ++++++-
 6 files changed, 316 insertions(+), 122 deletions(-)

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 bc089740f8..df846d5f59 100644
--- a/packages/apm-guide/.apm/skills/apm-usage/package-authoring.md
+++ b/packages/apm-guide/.apm/skills/apm-usage/package-authoring.md
@@ -510,11 +510,9 @@ Packaged distribution format created with `apm pack --format plugin`.
 
 When `apm.yml` declares `target: claude` or `target: copilot` (or the plural `targets:` equivalent), `apm pack` also generates an ecosystem-specific `plugin.json` automatically -- authors no longer need to maintain this file manually. The manifest is synthesised from `apm.yml` identity fields (`name`, `version`, `description`, `author`, `license`). See the apm pack reference (reference/cli/pack/#plugin-manifests) for output paths, credential stripping, and per-ecosystem differences, or run `apm pack --help`.
 
-In a hand-authored `plugin.json`, `skills` may name one skill directory or a
-container whose immediate children each contain `SKILL.md`. APM warns when a
-declared entry has no `SKILL.md` at either depth because it cannot be selected
-or deployed. Declare the skill directory itself, or move skills one level
-below the declared container.
+In a hand-authored `plugin.json`, declare either one skill directory or a
+container of immediate skill directories; see the
+[plugin collection reference](../../../../../docs/src/content/docs/reference/package-types.md#plugin-collection-pluginjson).
 
 #### Shipping `bin/` executables (Claude Code only)
 
diff --git a/scripts/lint-architecture-boundaries.sh b/scripts/lint-architecture-boundaries.sh
index 5361dc92e1..17c8cc52aa 100755
--- a/scripts/lint-architecture-boundaries.sh
+++ b/scripts/lint-architecture-boundaries.sh
@@ -1564,10 +1564,21 @@ methods = {
     node.name: node
     for node in ast.walk(tree)
     if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef))
-    and node.name in {"skill_source_dir", "available_skill_names", "integrate_package_skill"}
+    and node.name
+    in {
+        "skill_source_dir",
+        "available_skill_names",
+        "integrate_package_skill",
+        "_promote_sub_skills_standalone",
+    }
 }
 errors = []
-if set(methods) != {"skill_source_dir", "available_skill_names", "integrate_package_skill"}:
+if set(methods) != {
+    "skill_source_dir",
+    "available_skill_names",
+    "integrate_package_skill",
+    "_promote_sub_skills_standalone",
+}:
     errors.append("required methods are missing")
 
 
@@ -1593,6 +1604,25 @@ if "integrate_package_skill" in methods and not calls_owner(
     methods["integrate_package_skill"], "self"
 ):
     errors.append("integrate_package_skill must call self.skill_source_dir(package_info)")
+if "integrate_package_skill" in methods:
+    standalone_calls = [
+        call
+        for call in ast.walk(methods["integrate_package_skill"])
+        if isinstance(call, ast.Call)
+        and isinstance(call.func, ast.Attribute)
+        and call.func.attr == "_promote_sub_skills_standalone"
+    ]
+    if not standalone_calls or any(
+        len(call.args) < 3
+        or not isinstance(call.args[2], ast.Name)
+        or call.args[2].id != "source_dir"
+        for call in standalone_calls
+    ):
+        errors.append("standalone deployment must receive the canonical source_dir")
+if "_promote_sub_skills_standalone" in methods:
+    standalone_source = ast.unparse(methods["_promote_sub_skills_standalone"])
+    if "package_path / '.apm' / 'skills'" in standalone_source:
+        errors.append("standalone deployment must not reconstruct the normalized source path")
 if "skill_source_dir" in methods:
     owner_source = ast.unparse(methods["skill_source_dir"])
     if "PackageType.MARKETPLACE_PLUGIN" not in owner_source:
diff --git a/src/apm_cli/deps/plugin_parser.py b/src/apm_cli/deps/plugin_parser.py
index 99f3dba7e3..5dd9b43684 100644
--- a/src/apm_cli/deps/plugin_parser.py
+++ b/src/apm_cli/deps/plugin_parser.py
@@ -97,16 +97,8 @@ def _assert_safe_plugin_destination(apm_dir: Path) -> None:
         raise PluginIntegrityError(f"Refusing to normalize through symlinked directory: {apm_dir}")
 
 
-def _surface_warning(message: str, logger: logging.Logger) -> None:
-    """Emit a warning to both the stdlib logger and the rich console.
-
-    The ``apm`` stdlib logger has no handlers configured by default, so
-    ``logger.warning`` calls are silently dropped in non-debug runs. For
-    user-visible plugin-parse issues (skipped MCP servers, validation
-    failures), also route through ``_rich_warning`` so the user sees them
-    even without ``--verbose``. Falls back gracefully if Rich is unavailable.
-    """
-    logger.warning(message)
+def _surface_warning(message: str) -> None:
+    """Emit one user-visible plugin warning through the console."""
     try:  # noqa: SIM105
         _rich_warning(message, symbol="warning")
     except Exception:
@@ -188,7 +180,6 @@ def _warn_skills_entry_holds_no_skill(
         f"any immediate subdirectory; nothing under it will deploy or be "
         f"selectable with --skill. Declare each skill directory, or place "
         f"skills one level below the declared container.",
-        _logger,
     )
 
 
@@ -356,8 +347,7 @@ def synthesize_apm_yml_from_plugin(plugin_path: Path, manifest: dict[str, Any])
             # the #1666 symptom (transitive deps dropped with no diagnostic).
             _surface_warning(
                 f"Could not load existing apm.yml for merge; transitive "
-                f"dependencies may not be preserved: {exc}",
-                _logger,
+                f"dependencies may not be preserved: {exc}"
             )
 
     # Generate apm.yml from plugin metadata, merging with existing manifest
@@ -539,8 +529,7 @@ def _mcp_servers_to_apm_deps(servers: dict[str, Any], plugin_path: Path) -> list
         else:
             _surface_warning(
                 f"Skipping MCP server '{name}' from plugin "
-                f"'{plugin_path.name}': no 'command' or 'url'",
-                logger,
+                f"'{plugin_path.name}': no 'command' or 'url'"
             )
             continue
 
@@ -558,8 +547,7 @@ def _mcp_servers_to_apm_deps(servers: dict[str, Any], plugin_path: Path) -> list
             MCPDependency.from_dict(dep)
         except (ValueError, Exception) as exc:
             _surface_warning(
-                f"Skipping invalid MCP server '{name}' from plugin '{plugin_path.name}': {exc}",
-                logger,
+                f"Skipping invalid MCP server '{name}' from plugin '{plugin_path.name}': {exc}"
             )
             continue
 
@@ -720,8 +708,7 @@ def _lsp_servers_to_apm_deps(servers: dict[str, Any], plugin_path: Path) -> list
             LSPDependency.from_dict(dep)
         except Exception as exc:
             _surface_warning(
-                f"Skipping invalid LSP server '{name}' from plugin '{plugin_path.name}': {exc}",
-                logger,
+                f"Skipping invalid LSP server '{name}' from plugin '{plugin_path.name}': {exc}"
             )
             continue
 
@@ -730,6 +717,111 @@ def _lsp_servers_to_apm_deps(servers: dict[str, Any], plugin_path: Path) -> list
     return deps
 
 
+def _is_same_path(source: Path, destination: Path) -> bool:
+    """Return whether two paths resolve to the same location."""
+    try:
+        return source.resolve() == destination.resolve()
+    except OSError:
+        return False
+
+
+def _map_plugin_skill_artifacts(
+    plugin_path: Path,
+    apm_dir: Path,
+    manifest: dict[str, Any],
+    skill_sources: list[Path],
+) -> None:
+    """Normalize plugin skill declarations into the canonical skills tree."""
+    from apm_cli.security.gate import ignore_non_content
+
+    target_skills = apm_dir / "skills"
+    _assert_no_symlink_descendants(target_skills)
+    skill_dirs = [source for source in skill_sources if source.is_dir()]
+    skill_files = [source for source in skill_sources if source.is_file()]
+    declared = isinstance(manifest.get("skills"), (list, str))
+    plugin_name = str(manifest.get("name") or plugin_path.name)
+    expected_target_names: set[str] = set()
+    if skill_dirs:
+        from ..integration.skill_integrator import normalize_skill_name, validate_skill_name
+
+        claimed_names: dict[str, Path] = {}
+        copy_specs: list[tuple[Path, Path]] = []
+        for source_dir in skill_dirs:
+            holds_skill_dirs = _holds_skill_dirs(source_dir)
+            if declared and not holds_skill_dirs:
+                declared_manifest = source_dir / "SKILL.md"
+                deployable_sources = (
+                    ((source_dir.name, source_dir),)
+                    if not declared_manifest.is_symlink() and declared_manifest.is_file()
+                    else ()
+                )
+                destination = target_skills / source_dir.name
+                expected_target_names.add(source_dir.name)
+            else:
+                deployable_sources = tuple(
+                    (child.name, child)
+                    for child in source_dir.iterdir()
+                    if not child.is_symlink()
+                    and child.is_dir()
+                    and not (child / "SKILL.md").is_symlink()
+                    and (child / "SKILL.md").is_file()
+                )
+                destination = target_skills
+                expected_target_names.update(name for name, _ in deployable_sources)
+            for raw_name, source in deployable_sources:
+                canonical_name = normalize_skill_name(raw_name)
+                valid_name, _ = validate_skill_name(canonical_name)
+                if not canonical_name or not valid_name:
+                    raise PluginIntegrityError(
+                        f"Plugin '{plugin_name}' skill declaration "
+                        f"'{source.relative_to(plugin_path).as_posix()}' has no valid "
+                        "deployable name. Rename the skill and reinstall."
+                    )
+                previous = claimed_names.get(canonical_name)
+                if previous is not None and previous != source:
+                    previous_path = previous.relative_to(plugin_path).as_posix()
+                    source_path = source.relative_to(plugin_path).as_posix()
+                    raise PluginIntegrityError(
+                        f"Plugin '{plugin_name}' skill declarations '{previous_path}' and "
+                        f"'{source_path}' collide on deployed name '{canonical_name}'. "
+                        "Rename one skill or remove one declaration, then reinstall."
+                    )
+                claimed_names[canonical_name] = source
+            if declared and not holds_skill_dirs:
+                skill_manifest = source_dir / "SKILL.md"
+                if skill_manifest.is_symlink() or not skill_manifest.is_file():
+                    _warn_skills_entry_holds_no_skill(source_dir, plugin_path, plugin_name)
+            copy_specs.append((source_dir, destination))
+
+        target_skills.mkdir(parents=True, exist_ok=True)
+        for source_dir, destination in copy_specs:
+            if _is_same_path(source_dir, destination):
+                continue
+            shutil.copytree(
+                source_dir,
+                destination,
+                dirs_exist_ok=True,
+                ignore=ignore_non_content,
+            )
+    if skill_files:
+        target_skills.mkdir(parents=True, exist_ok=True)
+        for source_file in skill_files:
+            if declared:
+                expected_target_names.add(source_file.name)
+            destination = target_skills / source_file.name
+            if _is_same_path(source_file, destination):
+                continue
+            shutil.copy2(source_file, destination)
+    if declared and target_skills.is_dir():
+        for child in target_skills.iterdir():
+            if child.name in expected_target_names:
+                continue
+            if child.is_dir():
+                shutil.rmtree(child)
+            else:
+                child.unlink()
+
+
 def _map_plugin_artifacts(
     plugin_path: Path, apm_dir: Path, manifest: dict[str, Any] | None = None
 ) -> None:
@@ -804,16 +896,6 @@ def _resolve_sources(component: str, default_dir: str):
             return [default]
         return []
 
-    # Helper: True when *src* and *dst* resolve to the same filesystem path
-    # (e.g. a manifest entry pointing at a file already inside the target).
-    # Copying onto self raises ``shutil.SameFileError`` and ``shutil.copytree``
-    # over identical directories triggers it per-file, so callers must skip.
-    def _is_same_path(src: Path, dst: Path) -> bool:
-        try:
-            return src.resolve() == dst.resolve()
-        except OSError:
-            return False
-
     # Map agents/
     # Unlike skills (which are named directories containing SKILL.md), agents
     # are flat files  -- each .md is one agent.  So we always merge directory
@@ -839,76 +921,7 @@ def _is_same_path(src: Path, dst: Path) -> bool:
     # Map skills/
     skill_sources = _resolve_sources("skills", "skills")
     if skill_sources:
-        target_skills = apm_dir / "skills"
-        _assert_no_symlink_descendants(target_skills)
-        skill_dirs = [s for s in skill_sources if s.is_dir()]
-        skill_files = [s for s in skill_sources if s.is_file()]
-
-        # A declared ``skills`` entry is either the skill itself (``SKILL.md``
-        # at its root, e.g. ``./skills/engineering/tdd``) or a container
-        # holding one skill per child (the conventional ``./skills/``).
-        # Classify per entry rather than per manifest shape: naming a
-        # container after itself buries every skill one level too deep, while
-        # merging a lone skill spills a bare ``SKILL.md`` into the shared root
-        # under no name at all. Either way ``--skill`` sees nothing, because
-        # ``.apm/skills/<name>/SKILL.md`` is the exact depth that deployment,
-        # ``--skill`` enumeration, the bin/ security scan and primitive
-        # counting all read. See issue #2530.
-        #
-        # Undeclared discovery keeps merging unconditionally: the default
-        # ``skills/`` is the convention container by definition.
-        declared = isinstance(manifest.get("skills"), (list, str))
-        if skill_dirs:
-            target_skills.mkdir(parents=True, exist_ok=True)
-            claimed_names: dict[str, Path] = {}
-            for d in skill_dirs:
-                if declared and not _holds_skill_dirs(d):
-                    declared_manifest = d / "SKILL.md"
-                    deployable_names = (
-                        (d.name,)
-                        if not declared_manifest.is_symlink() and declared_manifest.is_file()
-                        else ()
-                    )
-                else:
-                    deployable_names = tuple(
-                        child.name
-                        for child in d.iterdir()
-                        if not child.is_symlink()
-                        and child.is_dir()
-                        and not (child / "SKILL.md").is_symlink()
-                        and (child / "SKILL.md").is_file()
-                    )
-                for name in deployable_names:
-                    previous = claimed_names.get(name)
-                    if previous is not None and previous != d:
-                        raise PluginIntegrityError(
-                            f"Plugin skill declarations collide on normalized name '{name}'"
-                        )
-                    claimed_names[name] = d
-                if declared and not _holds_skill_dirs(d):
-                    dest = target_skills / d.name
-                    skill_manifest = d / "SKILL.md"
-                    if skill_manifest.is_symlink() or not skill_manifest.is_file():
-                        # Neither shape: keep the contents isolated under the
-                        # entry's own name, but do not let the dead end pass
-                        # silently -- that silence is the #2530 symptom.
-                        _warn_skills_entry_holds_no_skill(
-                            d,
-                            plugin_path,
-                            str(manifest.get("name") or plugin_path.name),
-                        )
-                else:
-                    dest = target_skills
-                if _is_same_path(d, dest):
-                    continue
-                shutil.copytree(d, dest, dirs_exist_ok=True, ignore=ignore_non_content)
-        if skill_files:
-            target_skills.mkdir(parents=True, exist_ok=True)
-            for f in skill_files:
-                dst = target_skills / f.name
-                if _is_same_path(f, dst):
-                    continue
-                shutil.copy2(f, dst)
+        _map_plugin_skill_artifacts(plugin_path, apm_dir, manifest, skill_sources)
 
     # Map commands/ -> .apm/prompts/ (normalize .md -> .prompt.md)
     command_sources = _resolve_sources("commands", "commands")
diff --git a/src/apm_cli/integration/skill_integrator.py b/src/apm_cli/integration/skill_integrator.py
index 048a47a7ae..330ee0a713 100644
--- a/src/apm_cli/integration/skill_integrator.py
+++ b/src/apm_cli/integration/skill_integrator.py
@@ -743,6 +743,11 @@ def _promote_sub_skills(
         else:
             rel_prefix = target_skills_root.name
 
+        from apm_cli.utils.path_security import ensure_path_within
+
+        deployable_skills: list[tuple[Path, str, Path]] = []
+        claimed_names: dict[str, Path] = {}
+        target_root_resolved = target_skills_root.resolve()
         for sub_skill_path in sub_skills_dir.iterdir():
             if sub_skill_path.is_symlink() or not sub_skill_path.is_dir():
                 continue
@@ -756,6 +761,28 @@ def _promote_sub_skills(
             is_valid, _ = validate_skill_name(raw_sub_name)
             sub_name = raw_sub_name if is_valid else normalize_skill_name(raw_sub_name)
             target = target_skills_root / sub_name
+            if not sub_name:
+                raise ValueError(
+                    f"Skill '{raw_sub_name}' from package '{parent_name}' has no valid "
+                    "deployable name. Rename the skill and reinstall."
+                )
+            target_resolved = ensure_path_within(target, target_skills_root)
+            if target_resolved == target_root_resolved:
+                raise ValueError(
+                    f"Skill '{raw_sub_name}' from package '{parent_name}' resolves to the "
+                    "shared skills root. Rename the skill and reinstall."
+                )
+            previous = claimed_names.get(sub_name)
+            if previous is not None and previous != sub_skill_path:
+                raise ValueError(
+                    f"Skills '{previous.name}' and '{raw_sub_name}' from package "
+                    f"'{parent_name}' collide on deployed name '{sub_name}'. "
+                    "Rename one skill and reinstall."
+                )
+            claimed_names[sub_name] = sub_skill_path
+            deployable_skills.append((sub_skill_path, sub_name, target))
+
+        for sub_skill_path, sub_name, target in deployable_skills:
             rel_path = f"{rel_prefix}/{sub_name}"
             if target.exists():
                 # Content-identical: skip entirely (no copy, no warning)
@@ -888,6 +915,7 @@ def _promote_sub_skills_standalone(
         self,
         package_info,
         project_root: Path,
+        sub_skills_dir: Path | None = None,
         diagnostics=None,
         managed_files=None,
         force: bool = False,
@@ -906,6 +934,7 @@ def _promote_sub_skills_standalone(
         Args:
             package_info: PackageInfo object with package metadata.
             project_root: Root directory of the project.
+            sub_skills_dir: Canonical source returned by ``skill_source_dir``.
             targets: Optional explicit list of TargetProfile objects.
             skill_subset: Optional tuple of skill names or paths to install (None = all).
 
@@ -914,7 +943,8 @@ def _promote_sub_skills_standalone(
         """
         self.init_link_resolver(package_info, project_root)
         package_path = package_info.install_path
-        sub_skills_dir = package_path / ".apm" / "skills"
+        if sub_skills_dir is None:
+            sub_skills_dir = self.skill_source_dir(package_info)
         if not sub_skills_dir.is_dir():
             return 0, []
 
@@ -1397,6 +1427,9 @@ def integrate_package_skill(
         Returns:
             SkillIntegrationResult: Results of the integration operation
         """
+        package_path = package_info.install_path
+        source_dir = self.skill_source_dir(package_info)
+
         # Check if package type allows skill installation (T4 routing)
         # SKILL and HYBRID -> install as skill
         # INSTRUCTIONS and PROMPTS -> skip skill installation
@@ -1406,6 +1439,7 @@ def integrate_package_skill(
             sub_skills_count, sub_deployed = self._promote_sub_skills_standalone(
                 package_info,
                 project_root,
+                source_dir,
                 diagnostics=diagnostics,
                 managed_files=managed_files,
                 force=force,
@@ -1440,8 +1474,6 @@ def integrate_package_skill(
                     links_resolved=0,
                 )
 
-        package_path = package_info.install_path
-
         # MARKETPLACE_PLUGIN: deploy bin/ executables + plugin manifest BEFORE
         # skill routing.  bin/ deployment is orthogonal to whether the plugin
         # also ships a root SKILL.md or a skills/ bundle, so it must run for
@@ -1496,7 +1528,7 @@ def integrate_package_skill(
         # ``available_skill_names`` -- so a ``--skill`` value can never be
         # validated against a directory other than the one that deploys.
         root_skills_dir = package_path / "skills"
-        if self.skill_source_dir(package_info) == root_skills_dir:
+        if source_dir == root_skills_dir:
             return self._merge_bin_paths(
                 self._integrate_skill_bundle(
                     package_info,
@@ -1519,6 +1551,7 @@ def integrate_package_skill(
         sub_skills_count, sub_deployed = self._promote_sub_skills_standalone(
             package_info,
             project_root,
+            source_dir,
             diagnostics=diagnostics,
             managed_files=managed_files,
             force=force,
diff --git a/tests/integration/test_architecture_outcome_guards.py b/tests/integration/test_architecture_outcome_guards.py
index 0d404b9708..bb0d33a902 100644
--- a/tests/integration/test_architecture_outcome_guards.py
+++ b/tests/integration/test_architecture_outcome_guards.py
@@ -248,6 +248,9 @@ def test_plugin_manifest_skill_set_beats_undeclared_root_bundle(
         skill = plugin / "skills" / name
         skill.mkdir(parents=True)
         (skill / "SKILL.md").write_text(f"# {name}\n", encoding="utf-8")
+    pre_normalized = plugin / ".apm" / "skills" / "pre-normalized-undeclared"
+    pre_normalized.mkdir(parents=True)
+    (pre_normalized / "SKILL.md").write_text("# undeclared\n", encoding="utf-8")
     monkeypatch.chdir(consumer)
     monkeypatch.setattr("apm_cli.cli._check_and_notify_updates", lambda: None)
 
@@ -262,11 +265,13 @@ def test_plugin_manifest_skill_set_beats_undeclared_root_bundle(
     assert accepted.exit_code == 0, accepted.output
     assert (consumer / ".claude" / "skills" / "declared" / "SKILL.md").is_file()
     assert not (consumer / ".claude" / "skills" / "undeclared").exists()
+    assert not (consumer / ".claude" / "skills" / "pre-normalized-undeclared").exists()
 
     full_install = CliRunner().invoke(cli, ["install"])
 
     assert full_install.exit_code == 0, full_install.output
     assert not (consumer / ".claude" / "skills" / "undeclared").exists()
+    assert not (consumer / ".claude" / "skills" / "pre-normalized-undeclared").exists()
 
 
 def test_declared_string_skill_installs_from_normalized_source(
@@ -298,6 +303,43 @@ def test_declared_string_skill_installs_from_normalized_source(
     assert (consumer / ".claude" / "skills" / "tdd" / "SKILL.md").is_file()
 
 
+def test_normalized_declared_skill_name_collision_fails_closed(
+    tmp_path: Path,
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    """Two declarations with one deployed identity cannot overwrite each other."""
+    from click.testing import CliRunner
+
+    from apm_cli.cli import cli
+
+    plugin, consumer = _write_plugin_consumer(
+        tmp_path,
+        {
+            "name": "colliding-skills",
+            "version": "1.0.0",
+            "skills": ["./one/Foo Bar", "./two/foo-bar"],
+        },
+    )
+    for relative in ("one/Foo Bar", "two/foo-bar"):
+        skill = plugin / relative
+        skill.mkdir(parents=True)
+        (skill / "SKILL.md").write_text(f"# {relative}\n", encoding="utf-8")
+    existing = consumer / ".claude" / "skills" / "existing"
+    existing.mkdir(parents=True)
+    (existing / "SKILL.md").write_text("# existing\n", encoding="utf-8")
+    monkeypatch.chdir(consumer)
+    monkeypatch.setattr("apm_cli.cli._check_and_notify_updates", lambda: None)
+
+    result = CliRunner().invoke(cli, ["install"])
+
+    assert result.exit_code != 0, result.output
+    assert "collide on deployed name 'foo-bar'" in result.output
+    assert "colliding-skills" in result.output
+    assert "one/Foo Bar" in result.output
+    assert "two/foo-bar" in result.output
+    assert (existing / "SKILL.md").read_text(encoding="utf-8") == "# existing\n"
+
+
 def test_malformed_declared_skills_warns_during_install(
     tmp_path: Path,
     monkeypatch: pytest.MonkeyPatch,
@@ -325,6 +367,7 @@ def test_malformed_declared_skills_warns_during_install(
 
     assert result.exit_code == 0, result.output
     assert "Plugin 'buried-skills' skills entry 'skills'" in result.output
+    assert result.output.count("Plugin 'buried-skills' skills entry 'skills'") == 1
     assert "no SKILL.md" in result.output
     assert "--skill" in result.output
 
@@ -354,6 +397,26 @@ def test_symlinked_skill_source_is_not_deployable(tmp_path: Path) -> None:
     assert not (target / "linked").exists()
 
 
+def test_empty_normalized_skill_name_cannot_replace_target_root(tmp_path: Path) -> None:
+    """A source name that normalizes empty must not alias the shared root."""
+    from apm_cli.integration.skill_integrator import SkillIntegrator
+
+    source = tmp_path / "source"
+    malicious = source / "!!!"
+    malicious.mkdir(parents=True)
+    (malicious / "SKILL.md").write_text("# payload\n", encoding="utf-8")
+    target = tmp_path / "target"
+    existing = target / "existing"
+    existing.mkdir(parents=True)
+    (existing / "SKILL.md").write_text("# existing\n", encoding="utf-8")
+
+    with pytest.raises(ValueError, match="no valid deployable name"):
+        SkillIntegrator._promote_sub_skills(source, target, "malicious-package")
+
+    assert (existing / "SKILL.md").read_text(encoding="utf-8") == "# existing\n"
+    assert not (target / "SKILL.md").exists()
+
+
 def test_skill_source_routing_has_one_static_owner() -> None:
     """The boundary lint must defend both skill-source consumer edges."""
     root = Path(__file__).parents[2]
diff --git a/tests/unit/test_plugin_parser.py b/tests/unit/test_plugin_parser.py
index 9cb0da679a..9cee1719c2 100644
--- a/tests/unit/test_plugin_parser.py
+++ b/tests/unit/test_plugin_parser.py
@@ -1,9 +1,9 @@
 """Unit tests for plugin_parser.py and find_plugin_json helper."""
 
 import json
-import logging
 import os  # noqa: F401
 from pathlib import Path
+from unittest.mock import patch
 
 import pytest
 import yaml
@@ -350,7 +350,7 @@ def test_declared_nested_skill_path_keeps_leaf_name(self, tmp_path):
         # Undeclared siblings stay out: the entry is a requirement, not a hint.
         assert not (normalized / "pairing").exists()
 
-    def test_declared_skills_entry_holding_no_skill_warns(self, tmp_path, caplog):
+    def test_declared_skills_entry_holding_no_skill_warns(self, tmp_path):
         """An entry that is neither a skill nor a container must say so.
 
         A container whose skills sit two levels down reaches no deployable
@@ -367,13 +367,14 @@ def test_declared_skills_entry_holding_no_skill_warns(self, tmp_path, caplog):
 
         apm_dir = plugin_dir / ".apm"
         apm_dir.mkdir()
-        with caplog.at_level(logging.WARNING, logger="apm_cli.deps.plugin_parser"):
+        with patch("apm_cli.deps.plugin_parser._rich_warning") as warning:
             _map_plugin_artifacts(plugin_dir, apm_dir, manifest={"skills": ["./skills/"]})
 
-        assert "Plugin 'plugin'" in caplog.text
-        assert "skills" in caplog.text
-        assert "no SKILL.md" in caplog.text
-        assert "--skill" in caplog.text
+        message = warning.call_args.args[0]
+        assert "Plugin 'plugin'" in message
+        assert "skills" in message
+        assert "no SKILL.md" in message
+        assert "--skill" in message
 
     def test_colliding_declared_skill_containers_fail_closed(self, tmp_path):
         """Two declared containers cannot silently merge one skill name."""
@@ -394,6 +395,64 @@ def test_colliding_declared_skill_containers_fail_closed(self, tmp_path):
                 manifest={"skills": ["./first", "./second"]},
             )
 
+    @pytest.mark.parametrize(
+        ("first_name", "second_name"),
+        [
+            ("Foo Bar", "foo-bar"),
+            (f"{'a' * 64}x", f"{'a' * 64}y"),
+        ],
+    )
+    def test_canonical_declared_skill_collisions_fail_closed(
+        self,
+        tmp_path,
+        first_name,
+        second_name,
+    ):
+        """Collision checks use the final deployed identity, including truncation."""
+        plugin_dir = tmp_path / "plugin"
+        plugin_dir.mkdir()
+        for container_name, skill_name in (
+            ("first", first_name),
+            ("second", second_name),
+        ):
+            skill = plugin_dir / container_name / skill_name
+            skill.mkdir(parents=True)
+            (skill / "SKILL.md").write_text(f"# {container_name}", encoding="utf-8")
+        apm_dir = plugin_dir / ".apm"
+        apm_dir.mkdir()
+
+        with pytest.raises(PluginIntegrityError, match="Rename one skill"):
+            _map_plugin_artifacts(
+                plugin_dir,
+                apm_dir,
+                manifest={
+                    "name": "canonical-collision",
+                    "skills": ["./first", "./second"],
+                },
+            )
+
+        assert not (apm_dir / "skills").exists()
+
+    def test_empty_canonical_skill_name_fails_before_destination_mutation(self, tmp_path):
+        """An empty normalized name cannot alias the shared skills root."""
+        plugin_dir = tmp_path / "plugin"
+        skill = plugin_dir / "skills" / "!!!"
+        skill.mkdir(parents=True)
+        (skill / "SKILL.md").write_text("# invalid", encoding="utf-8")
+        apm_dir = plugin_dir / ".apm"
+        existing = apm_dir / "skills" / "existing"
+        existing.mkdir(parents=True)
+        (existing / "SKILL.md").write_text("# existing", encoding="utf-8")
+
+        with pytest.raises(PluginIntegrityError, match="no valid deployable name"):
+            _map_plugin_artifacts(
+                plugin_dir,
+                apm_dir,
+                manifest={"name": "empty-name", "skills": ["./skills"]},
+            )
+
+        assert (existing / "SKILL.md").read_text(encoding="utf-8") == "# existing"
+
     def test_symlinked_apm_directory_cannot_redirect_normalization(self, tmp_path):
         """A plugin-controlled .apm symlink must not redirect artifact writes."""
         plugin_dir = tmp_path / "plugin"
@@ -414,7 +473,7 @@ def test_symlinked_apm_directory_cannot_redirect_normalization(self, tmp_path):
 
         assert not (outside / "skills").exists()
 
-    def test_declared_skills_container_does_not_warn(self, tmp_path, caplog):
+    def test_declared_skills_container_does_not_warn(self, tmp_path):
         """The healthy shapes stay quiet -- a warning nobody can act on is noise."""
         plugin_dir = tmp_path / "plugin"
         plugin_dir.mkdir()
@@ -424,10 +483,10 @@ def test_declared_skills_container_does_not_warn(self, tmp_path, caplog):
 
         apm_dir = plugin_dir / ".apm"
         apm_dir.mkdir()
-        with caplog.at_level(logging.WARNING, logger="apm_cli.deps.plugin_parser"):
+        with patch("apm_cli.deps.plugin_parser._rich_warning") as warning:
             _map_plugin_artifacts(plugin_dir, apm_dir, manifest={"skills": ["./skills/"]})
 
-        assert "no SKILL.md" not in caplog.text
+        warning.assert_not_called()
 
     def test_custom_commands_path(self, tmp_path):
         """Manifest commands field redirects command discovery."""
@@ -1567,8 +1626,6 @@ def test_malformed_apm_yml_fallback_surfaces_warning(self, tmp_path):
         ``_surface_warning`` rather than swallowed, otherwise the malformed
         file re-creates the exact #1666 symptom with zero diagnostic output.
         """
-        from unittest.mock import patch
-
         # Write syntactically invalid YAML (unbalanced bracket triggers a
         # yaml.YAMLError inside load_yaml).
         (tmp_path / "apm.yml").write_text("name: bad\ndependencies: [unterminated\n")

From 464ce99c2d045d7d99cc61e9a1ba8e9f0b66a737 Mon Sep 17 00:00:00 2001
From: danielmeppiel <danielmeppiel@users.noreply.github.com>
Date: Sun, 23 Aug 2026 22:02:27 +0200
Subject: [PATCH 5/8] fix(plugins): honor empty skill declarations

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---
 src/apm_cli/deps/plugin_parser.py             |  2 +-
 .../test_architecture_outcome_guards.py       | 29 +++++++++++++++++++
 2 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/src/apm_cli/deps/plugin_parser.py b/src/apm_cli/deps/plugin_parser.py
index 7880569b4f..489703f490 100644
--- a/src/apm_cli/deps/plugin_parser.py
+++ b/src/apm_cli/deps/plugin_parser.py
@@ -944,7 +944,7 @@ def _resolve_sources(component: str, default_dir: str):
 
     # Map skills/
     skill_sources = _resolve_sources("skills", "skills")
-    if skill_sources:
+    if skill_sources or isinstance(manifest.get("skills"), (list, str)):
         _map_plugin_skill_artifacts(plugin_path, apm_dir, manifest, skill_sources)
 
     # Map commands/ -> .apm/prompts/ (normalize .md -> .prompt.md)
diff --git a/tests/integration/test_architecture_outcome_guards.py b/tests/integration/test_architecture_outcome_guards.py
index f8a633e438..946d42217f 100644
--- a/tests/integration/test_architecture_outcome_guards.py
+++ b/tests/integration/test_architecture_outcome_guards.py
@@ -303,6 +303,35 @@ def test_declared_string_skill_installs_from_normalized_source(
     assert (consumer / ".claude" / "skills" / "tdd" / "SKILL.md").is_file()
 
 
+def test_empty_declared_skills_rejects_preexisting_normalized_skills(
+    tmp_path: Path,
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    """An explicit empty skill set must prune package-provided normalized content."""
+    from click.testing import CliRunner
+
+    from apm_cli.cli import cli
+
+    plugin, consumer = _write_plugin_consumer(
+        tmp_path,
+        {
+            "name": "empty-declared-skills",
+            "version": "1.0.0",
+            "skills": [],
+        },
+    )
+    undeclared = plugin / ".apm" / "skills" / "undeclared"
+    undeclared.mkdir(parents=True)
+    (undeclared / "SKILL.md").write_text("# undeclared\n", encoding="utf-8")
+    monkeypatch.chdir(consumer)
+    monkeypatch.setattr("apm_cli.cli._check_and_notify_updates", lambda: None)
+
+    result = CliRunner().invoke(cli, ["install"])
+
+    assert result.exit_code == 0, result.output
+    assert not (consumer / ".claude" / "skills" / "undeclared").exists()
+
+
 def test_normalized_declared_skill_name_collision_fails_closed(
     tmp_path: Path,
     monkeypatch: pytest.MonkeyPatch,

From ea2a63bad5eb214e412413b4a601b16d6bed6833 Mon Sep 17 00:00:00 2001
From: danielmeppiel <danielmeppiel@users.noreply.github.com>
Date: Sun, 23 Aug 2026 22:07:48 +0200
Subject: [PATCH 6/8] fix(plugins): deduplicate skill diagnostics

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---
 .../content/docs/reference/package-types.md    | 18 ++++++++++++++++++
 src/apm_cli/deps/plugin_parser.py              |  9 ++++++---
 tests/unit/test_plugin_parser.py               | 10 ++++++++--
 3 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/docs/src/content/docs/reference/package-types.md b/docs/src/content/docs/reference/package-types.md
index 84ab19c5ef..69a47a560a 100644
--- a/docs/src/content/docs/reference/package-types.md
+++ b/docs/src/content/docs/reference/package-types.md
@@ -231,6 +231,24 @@ whose immediate children are skills. If the entry has no reachable `SKILL.md`
 at either depth, APM warns that nothing is selectable or deployable. Declare
 the skill directory itself, or place each skill one level below the container.
 
+For a container:
+
+```json
+{"skills": ["./skills/"]}
+```
+
+For one skill:
+
+```json
+{"skills": ["./skills/search"]}
+```
+
+Either form makes `search` selectable:
+
+```bash
+apm install owner/repo --skill search
+```
+
 **When to choose:** you already have a Claude plugin and want APM to
 consume it without restructuring. This is still the no-flag default output
 of `apm pack` and `apm plugin init`.
diff --git a/src/apm_cli/deps/plugin_parser.py b/src/apm_cli/deps/plugin_parser.py
index 489703f490..fde28ab5c1 100644
--- a/src/apm_cli/deps/plugin_parser.py
+++ b/src/apm_cli/deps/plugin_parser.py
@@ -95,7 +95,10 @@ def _assert_no_symlink_descendants(target: Path) -> None:
 def _assert_safe_plugin_destination(apm_dir: Path) -> None:
     """Require the normalization root to be a real directory, not a symlink."""
     if apm_dir.is_symlink():
-        raise PluginIntegrityError(f"Refusing to normalize through symlinked directory: {apm_dir}")
+        raise PluginIntegrityError(
+            f"Refusing to normalize through symlinked directory: {apm_dir}. "
+            "Replace the package's symlinked .apm path with a real directory, then reinstall."
+        )
 
 
 def _surface_warning(message: str) -> None:
@@ -760,8 +763,8 @@ def _map_plugin_skill_artifacts(
 
     target_skills = apm_dir / "skills"
     _assert_no_symlink_descendants(target_skills)
-    skill_dirs = [source for source in skill_sources if source.is_dir()]
-    skill_files = [source for source in skill_sources if source.is_file()]
+    skill_dirs = list(dict.fromkeys(source for source in skill_sources if source.is_dir()))
+    skill_files = list(dict.fromkeys(source for source in skill_sources if source.is_file()))
     declared = isinstance(manifest.get("skills"), (list, str))
     plugin_name = str(manifest.get("name") or plugin_path.name)
     expected_target_names: set[str] = set()
diff --git a/tests/unit/test_plugin_parser.py b/tests/unit/test_plugin_parser.py
index 8f24851e9a..92bbc6d29d 100644
--- a/tests/unit/test_plugin_parser.py
+++ b/tests/unit/test_plugin_parser.py
@@ -368,8 +368,13 @@ def test_declared_skills_entry_holding_no_skill_warns(self, tmp_path):
         apm_dir = plugin_dir / ".apm"
         apm_dir.mkdir()
         with patch("apm_cli.deps.plugin_parser._rich_warning") as warning:
-            _map_plugin_artifacts(plugin_dir, apm_dir, manifest={"skills": ["./skills/"]})
+            _map_plugin_artifacts(
+                plugin_dir,
+                apm_dir,
+                manifest={"skills": ["./skills/", "skills"]},
+            )
 
+        assert warning.call_count == 1
         message = warning.call_args.args[0]
         assert "Plugin 'plugin'" in message
         assert "skills" in message
@@ -470,9 +475,10 @@ def test_symlinked_apm_directory_cannot_redirect_normalization(self, tmp_path):
         except OSError:
             pytest.skip("Symlinks are unavailable")
 
-        with pytest.raises(PluginIntegrityError, match="symlinked directory"):
+        with pytest.raises(PluginIntegrityError, match="symlinked directory") as exc_info:
             _map_plugin_artifacts(plugin_dir, apm_dir, manifest={"skills": ["./skills"]})
 
+        assert "real directory, then reinstall" in str(exc_info.value)
         assert not (outside / "skills").exists()
 
     def test_declared_skills_container_does_not_warn(self, tmp_path):

From 904efec21337ff50c337d9c51a0ea74c38bd1309 Mon Sep 17 00:00:00 2001
From: danielmeppiel <danielmeppiel@users.noreply.github.com>
Date: Sun, 23 Aug 2026 22:14:37 +0200
Subject: [PATCH 7/8] chore: sync architecture instruction deployment

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---
 .github/instructions/architecture.instructions.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.github/instructions/architecture.instructions.md b/.github/instructions/architecture.instructions.md
index 2005037fef..781fafc7e7 100644
--- a/.github/instructions/architecture.instructions.md
+++ b/.github/instructions/architecture.instructions.md
@@ -74,6 +74,7 @@ semicolon-delimited, and specific to the file(s) that own the fact.
 | Effective marketplace output path | marketplace/output_profiles.py (resolve_effective_output_path) | `src/apm_cli/marketplace/output_profiles.py` |
 | Bootstrap project-name validation and fallback | core/project_name.py (resolve_bootstrap_project_name) | `src/apm_cli/core/project_name.py` |
 | Marketplace raw-structure diagnostics | marketplace/models.py parser; validator.py consumes them | `src/apm_cli/marketplace/models.py`; `src/apm_cli/marketplace/validator.py` |
+| Selectable and deployable skill source | integration/skill_integrator.py (SkillIntegrator.skill_source_dir) | `src/apm_cli/integration/skill_integrator.py` |
 | Agent Plugins v1 contract interpretation, component discovery, and portable manifest authority | agent_plugins/loader.py (load_agent_plugin, _load_apm_configuration) | `src/apm_cli/agent_plugins/loader.py`; `src/apm_cli/agent_plugins/ir.py` |
 | Agent Plugin producer portable-surface admission | bundle/agent_plugin_exporter.py (_require_portable_agent_plugin) | `src/apm_cli/bundle/agent_plugin_exporter.py` |
 | APMPackage interpreted-manifest construction | models/apm_package.py (APMPackage.from_mapping) | `src/apm_cli/models/apm_package.py` |

From 3a3c1a76fd0da8337dca915c3881d22aa17e65fe Mon Sep 17 00:00:00 2001
From: danielmeppiel <danielmeppiel@users.noreply.github.com>
Date: Sun, 23 Aug 2026 22:22:02 +0200
Subject: [PATCH 8/8] chore: refresh architecture instruction hash

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---
 apm.lock.yaml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/apm.lock.yaml b/apm.lock.yaml
index 804055fa24..e8291aa7df 100644
--- a/apm.lock.yaml
+++ b/apm.lock.yaml
@@ -2783,7 +2783,7 @@ deployments:
   owners:
   - .
   active_owner: .
-  content_hash: sha256:ab625ab2263bde79d1b5fca67c69afdee1a07037a9a0507f39704378eefea9e6
+  content_hash: sha256:8c44ae62413f8af09e0cbf7f93e486683f3aeeed89afc6c9c3fa6128a2b17650
 - kind: project-relative
   target: copilot
   value: .github/instructions/changelog.instructions.md
@@ -3290,7 +3290,7 @@ local_deployed_file_hashes:
   .github/agents/spec-tag-architect.agent.md: sha256:82907265c5e7cf1ac61ad96866fa7c5683b69c8f09b7a4c5f3cc241acc9568ca
   .github/agents/supply-chain-security-expert.agent.md: sha256:8fb8cc426d6af17ba084a28b3f026c2b475b62e3ca63ed2f88b83bd823f877af
   .github/agents/test-coverage-expert.agent.md: sha256:48c2172d1f18a394fa83ef9dc2be0b9b921a4e51e976498165250fed66369711
-  .github/instructions/architecture.instructions.md: sha256:ab625ab2263bde79d1b5fca67c69afdee1a07037a9a0507f39704378eefea9e6
+  .github/instructions/architecture.instructions.md: sha256:8c44ae62413f8af09e0cbf7f93e486683f3aeeed89afc6c9c3fa6128a2b17650
   .github/instructions/changelog.instructions.md: sha256:1e51ec4c74e847967962bd279dc4c6e582c5d3578490b3c28d5f3acd3e05f73e
   .github/instructions/cicd.instructions.md: sha256:33201cb88ea2f34b4950a9b52f87dc8dfb682796aaf53068ba7ae406c0c5e2c2
   .github/instructions/cli.instructions.md: sha256:8e39e8d5047ce88575cb02f87c2bcede584dfef258bd86f7466c7badf136541a