feat(skills): support package namespaces for skills#1028
feat(skills): support package namespaces for skills#1028shreejaykurhade wants to merge 16 commits intomicrosoft:mainfrom
Conversation
Add an optional namespace field to apm.yml, deploy package-owned skills and promoted sub-skills under skills/<namespace>/<skill-name>, and persist the namespace in apm.lock.yaml. Also document the namespace schema and add coverage for validation, namespaced deployment, lockfile round-tripping, and lockfile assembly.
…ge-namespaces # Conflicts: # src/apm_cli/install/phases/lockfile.py # src/apm_cli/integration/skill_integrator.py # src/apm_cli/models/apm_package.py
shreejaykurhade
requested review from
danielmeppiel and
sergio-sisternes-epam
as code owners
|
@danielmeppiel @sergio-sisternes-epam please review. |
shreejaykurhade
changed the title
Resolve conflicts: keep namespace fields and modern PEP 604 union syntax; align skill_integrator collision tracking with namespace-aware owner keys. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
_build_ownership_maps now keys by skill identity (path under skills/), not just leaf SKILL.md, so the test fixture's expected key changes from 'SKILL.md' to 'remote-skill/SKILL.md'. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
APM Review Panel Verdict: REJECT
Required before merge (8 items)
Nits (11 items, skip if you want)
CEO arbitrationThis PR ships a genuine enterprise capability -- package namespaces -- but ships it half-dressed. Eight required findings across four active panelists share a single root cause: the feature is complete at the code level and invisible at every user-facing surface. The CHANGELOG The second cluster of required findings is the in-session visibility gap. When a skill installs under The path to APPROVE is well-defined: (1) add CHANGELOG entries for namespace support and the copilot-instructions.md removal with a migration line, (2) emit a per-skill log line in verbose mode when namespace routing changes the install path, (3) audit Dissent resolved: cli-logging-expert (REQUIRED: silent namespace path change) and devx-ux-expert (REQUIRED: no user-visible signal for namespaced installs) are not in conflict -- they are the same finding from logging and UX frames respectively. Both are upheld. A single verbose-mode log line per skill and an audit of tree-output consumers satisfies both panelists. All four active panelists converged on the CHANGELOG gap independently, which strengthens confidence in that finding. Growth/positioning note: Namespace support is the "package manager for teams" unlock that lets an org publish Per-persona findings (full)Python ArchitectclassDiagram
direction LR
class APMPackage {
<<ValueObject>>
+name str
+version str
+namespace str | None
+from_apm_yml(path) APMPackage
}
class LockedDependency {
<<ValueObject>>
+repo_url str
+namespace str | None
+deployed_files list~str~
+to_dict() dict
+from_dict(d) LockedDependency
}
class InstallContext {
<<ValueObject>>
+package_namespaces dict~str,str~
+package_types dict~str,str~
+package_hashes dict~str,str~
}
class DependencySource {
<<Strategy>>
+materialize(dep_ref, ctx) Materialization
}
class LocalDependencySource {
<<ConcreteStrategy>>
+materialize(dep_ref, ctx) Materialization
}
class CachedDependencySource {
<<ConcreteStrategy>>
+materialize(dep_ref, ctx) Materialization
}
class FreshDependencySource {
<<ConcreteStrategy>>
+materialize(dep_ref, ctx) Materialization
}
class LockfileBuilder {
+build(lockfile) LockFile
+_attach_package_namespaces(lockfile)
+_attach_package_types(lockfile)
}
class SkillIntegrator {
<<BaseIntegrator>>
+copy_skill_to_target(package_info, ...)
+_promote_sub_skills(sub_skills_dir, ..., namespace)
+_build_ownership_maps(project_root)
}
DependencySource <|-- LocalDependencySource
DependencySource <|-- CachedDependencySource
DependencySource <|-- FreshDependencySource
LocalDependencySource ..> InstallContext : writes package_namespaces
CachedDependencySource ..> InstallContext : writes package_namespaces
FreshDependencySource ..> InstallContext : writes package_namespaces
LockfileBuilder ..> InstallContext : reads package_namespaces
LockfileBuilder ..> LockedDependency : sets namespace
SkillIntegrator ..> APMPackage : reads namespace
class APMPackage:::touched
class LockedDependency:::touched
class InstallContext:::touched
class LocalDependencySource:::touched
class CachedDependencySource:::touched
class FreshDependencySource:::touched
class LockfileBuilder:::touched
class SkillIntegrator:::touched
classDef touched fill:#fff3b0,stroke:#d47600
flowchart TD
A([apm install]) --> B[DependencySource.materialize\ninstall/sources.py]
B --> B1["[FS] read apm.yml via APMPackage.from_apm_yml\nmodels/apm_package.py"]
B1 --> B2{namespace field\nin apm.yml?}
B2 -- yes --> B3[validate_path_segments + slash/regex checks\nmodels/apm_package.py:225-240]
B2 -- no --> B4[APMPackage.namespace = None]
B3 --> B5["[CTX] ctx.package_namespaces[dep_key] = namespace\ninstall/sources.py:226"]
B4 --> C
B5 --> C[LockfileBuilder.build\ninstall/phases/lockfile.py]
C --> C1[_attach_package_namespaces\ninstall/phases/lockfile.py:131]
C1 --> C2["[LOCK] LockedDependency.namespace = namespace\ndeps/lockfile.py:37"]
C2 --> D[SkillIntegrator.copy_skill_to_target\nintegration/skill_integrator.py:372]
D --> D1["_package_namespace(package_info)\nskill_integrator.py:157"]
D1 --> D2{namespace?}
D2 -- yes --> D3["[FS] skill_dir = skills/ns/skill-name/\nskill_integrator.py:163"]
D2 -- no --> D4["[FS] skill_dir = skills/skill-name/\nskill_integrator.py:167"]
D3 --> D5["shutil.copytree to namespaced dir"]
D4 --> D5
D5 --> E([collision check via _build_ownership_maps])
Design patterns
No required findings. CLI Logging ExpertRequired findings are in the top section. Nits:
DevX UX ExpertRequired findings are in the top section. Nits:
Supply Chain Security ExpertRequired findings are in the top section. Nits:
Auth ExpertInactive -- No fast-path auth files are touched; drift.py refactors lockfile host-restoration guards (not AuthResolver token resolution). OSS Growth HackerRequired findings are in the top section. Nits:
Verdict computed deterministically: 8 required findings across 5 active panelists. APPROVE iff N == 0. Push a new commit to clear this verdict label automatically. Note 🔒 Integrity filter blocked 2 itemsThe following items were blocked because they don't meet the GitHub integrity level.
To allow these resources, lower tools:
github:
min-integrity: approved # merged | approved | unapproved | none
|
github-actions
Bot
added
panel-rejected
…crosoft#1028 round 2) Folds 8 panel-required findings from the round-2 verdict. Code beneath namespace routing was sound but the feature was invisible at every CLI and docs surface. This commit closes that visibility gap. - CHANGELOG [Unreleased]: add namespace feature entry and BREAKING entry for the removal of .github/copilot-instructions.md generation (findings 3, 7, 8). - skill_integrator: emit verbose_detail per skill when namespace routing changes the deploy path (finding 1). - services (install summary): preserve the namespace segment in the tree-output label and add per-skill confirmation 'skill ns/name integrated -> .github/skills/ns/' so namespaced installs are visible without --verbose (findings 2, 4). - templates/hello-world/apm.yml: commented namespace field for discoverability via 'apm init' (finding 5). - tests/unit/install/test_mcp_warnings.py: restored from origin/main; the file was deleted on this branch with no replacement, regressing SSRF and shell-metachar coverage on the MCP trust surface (finding 6). - migration.md: note that APM no longer writes .github/copilot-instructions.md so the 'leaves untouched' contract remains accurate (finding 8). - packages/apm-guide/.apm/skills/apm-usage/package-authoring.md: document the namespace field and the user-visible install signals (Rule 4 alignment). Regression tests: - verbose log surfaces namespace and is silent when absent - IntegrationResult.target_paths preserves the namespace segment - install tree label preserves namespace and per-skill ns/name label Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
APM Review Panel Verdict: REJECT
Required before merge (3 items)
Nits (12 items, skip if you want)
CEO arbitrationThree required findings block this PR across two domains: message-casing consistency (cli-logging-expert, 2 findings) and a path-containment gap (supply-chain-security-expert, 1 finding). There is no disagreement between panelists -- the findings are orthogonal and all five active reviewers landed without conflict. The security finding is the more urgent of the two: the namespaced Dissent resolved: No panelist disagreements to resolve. python-architect and cli-logging-expert both flagged the inline cross-boundary private import ( Growth/positioning note: Namespace is the org-level unlock APM has been missing -- the "ship 5 packages from one org without collisions" story is the adoption wedge for platform and devex teams. Recommend a short demo GIF of Per-persona findings (full)Python ArchitectclassDiagram
direction LR
class APMPackage {
<<ValueObject>>
+name str
+version str
+namespace str | None
+from_apm_yml(path) APMPackage
}
class PackageInfo {
<<ValueObject>>
+package APMPackage
+install_path Path
+package_type PackageType
+dependency_ref DependencyReference
}
class LockedDependency {
<<ValueObject>>
+repo_url str
+namespace str | None
+deployed_files list[str]
+get_unique_key() str
}
class InstallContext {
<<ValueObject>>
+project_root Path
+package_namespaces dict[str, str]
+package_deployed_files dict[str, list[str]]
+package_types dict[str, str]
}
class SkillIntegrator {
<<BaseIntegrator>>
-_native_skill_session_owners dict
+integrate_package_skill(package_info, project_root) SkillIntegrationResult
+_integrate_native_skill(package_info) SkillIntegrationResult
+_promote_sub_skills(sub_skills_dir, target_skills_root, parent_name, namespace) tuple
+_build_ownership_maps(project_root) tuple
}
class BaseIntegrator {
<<Abstract>>
}
class LockfileBuilder {
+build_and_save(ctx) None
+_attach_package_namespaces(lockfile) None
}
class _namespace_helpers {
<<Pure>>
+_package_namespace(package_info) str | None
+_skill_dir(root, skill_name, namespace) Path
+_skill_owner_key(skill_name, namespace) str
+_skill_owner_key_from_deployed_path(deployed_path) str
}
class DependencySource {
<<Strategy>>
+acquire(ctx) Materialization
}
class LocalDependencySource {
<<ConcreteStrategy>>
+acquire(ctx) Materialization
}
class CachedDependencySource {
<<ConcreteStrategy>>
+acquire(ctx) Materialization
}
class FreshDependencySource {
<<ConcreteStrategy>>
+acquire(ctx) Materialization
}
note for _namespace_helpers "All four helpers are module-level pure\nfunctions in skill_integrator.py --\nno state, no I/O, no side-effects"
note for DependencySource "Strategy pattern: three concrete sources\nall write ctx.package_namespaces[dep_key]\nfrom package.namespace after acquire()"
BaseIntegrator <|-- SkillIntegrator
DependencySource <|-- LocalDependencySource
DependencySource <|-- CachedDependencySource
DependencySource <|-- FreshDependencySource
PackageInfo *-- APMPackage
SkillIntegrator ..> _namespace_helpers : calls
SkillIntegrator ..> PackageInfo : reads
LockfileBuilder ..> InstallContext : reads ctx.package_namespaces
LockfileBuilder ..> LockedDependency : writes namespace
LocalDependencySource ..> InstallContext : writes package_namespaces
CachedDependencySource ..> InstallContext : writes package_namespaces
FreshDependencySource ..> InstallContext : writes package_namespaces
class APMPackage:::touched
class LockedDependency:::touched
class InstallContext:::touched
class SkillIntegrator:::touched
class LockfileBuilder:::touched
class _namespace_helpers:::touched
classDef touched fill:#fff3b0,stroke:#d47600
flowchart TD
A([apm install]) --> B[sources.py\nLocalDependencySource.acquire / CachedDependencySource.acquire / FreshDependencySource.acquire]
B --> C{"getattr(package_info.package,\n'namespace', None) truthy?"}
C -- yes --> D["[I/O] ctx.package_namespaces[dep_key] = namespace\n(sources.py:227 / 383 / 554)"]
C -- no --> E[skip namespace registration]
D --> F[services.py\nintegrate_package_primitives]
E --> F
F --> G["lazy import _package_namespace from skill_integrator\n(services.py:239)"]
G --> H[skill_integrator.py\n_package_namespace(package_info)\n-> getattr chain -> str | None]
H --> I["SkillIntegrator.integrate_package_skill\n-> _integrate_native_skill OR _promote_sub_skills_standalone"]
I --> J["_skill_dir(target_skills_root, skill_name, namespace)\n-> root/namespace/skill_name/ OR root/skill_name/"]
J --> K{"[FS] target_skill_dir.exists()?"}
K -- yes --> L["[FS] shutil.rmtree(target_skill_dir)\nafter collision/ownership check via _skill_owner_key"]
K -- no --> M["[FS] target_skill_dir.parent.mkdir(parents=True)"]
L --> M
M --> N["[FS] shutil.copytree(package_path, target_skill_dir,\nignore=_ignore_symlinks_and_apm)"]
N --> O["SkillIntegrationResult.target_paths.append(target_skill_dir)"]
O --> P[phases/lockfile.py\nLockfileBuilder._attach_package_namespaces]
P --> Q{dep_key in ctx.package_namespaces?}
Q -- yes --> R["[LOCK] dep.namespace = ctx.package_namespaces[dep_key]"]
Q -- no --> S[dep.namespace stays None]
R --> T([apm.lock.yaml written with namespace field])
S --> T
Design patterns
CLI Logging ExpertRequired:
Nits:
DevX UX ExpertNo findings. Nits:
Supply Chain Security ExpertRequired:
Nits:
Auth ExpertInactive -- No auth-fast-path files changed; PR introduces package namespace routing and compile behavior change, neither of which touches AuthResolver, token management, credential resolution, or host classification. OSS Growth HackerNo findings. Nits:
Growth side-channel: Namespace is the org-level unlock APM has been missing. "Ship 5 packages from one org without collisions" is the platform/devex adoption wedge. Recommend a demo GIF + pinned FAQ issue for the compile breaking change. Verdict computed deterministically: 3 required findings across 5 active panelists. APPROVE iff N == 0. Push a new commit to clear this verdict label automatically. Note 🔒 Integrity filter blocked 2 itemsThe following items were blocked because they don't meet the GitHub integrity level.
To allow these resources, lower tools:
github:
min-integrity: approved # merged | approved | unapproved | none
|
github-actions
Bot
removed
the
panel-review
Address all 3 required findings from PR microsoft#1028 round 3 review panel: 1. supply-chain-security: add runtime ensure_path_within containment guard inside _skill_dir (skill_integrator.py:163). Parse-time regex blocks traversal in namespace values, but a symlink planted at skills/<namespace> by a prior malicious install would otherwise let shutil.copytree write outside target_skills_root. Guard covers both namespaced and flat branches at the single chokepoint. 2. cli-logging: normalise capital-S in namespaced per-skill confirmation message (services.py:266) so namespaced and non-namespaced events share identical casing. 3. cli-logging: drop the redundant '(namespace: ...)' suffix from the sub-skills integrated message (services.py:272-274). The target path already encodes the namespace; the parenthetical was pure duplication. Adds TestSkillDirContainmentGuard regression tests (3) exercising the exact symlink-planted attack vector. Full unit suite (6904 tests) green. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
APM Review Panel Verdict: REJECT
Required before merge (12 items)
Nits (16 items, skip if you want)
CEO arbitrationThree independent panelists -- python-architect, supply-chain-security-expert, and devx-ux-expert -- converge on a single root defect: the namespace field has no enforced validation contract. The regex in manifest-schema.md documents constraints (max 64 chars, no consecutive hyphens) that the actual regex Two panelists -- devx-ux-expert and oss-growth-hacker -- independently flag the migration path as insufficient. The CHANGELOG sends users to 'copy your last generated version' with no command, no before/after example, and no CI guidance. Projects that win community trust on breaking changes provide an exact shell one-liner. The cli-logging-expert's finding that namespace identity is buried mid-message and the oss-growth-hacker's finding that skills.md opens with YAML syntax rather than the 'why' are complementary legibility failures -- one in terminal output, one in documentation -- both indicating the namespace concept is not yet legible to a first-time user. The devx-ux-expert raises two independent UX gaps: cli-commands.md is not updated to reflect namespace behavior, and Dissent resolved: No panelist-vs-panelist contradiction exists in this return set. The python-architect and supply-chain-security-expert findings on namespace validation are reinforcing, not competing. On the cli-logging question of whether to gate the services.py namespace label on non-verbose output or consolidate into services.py: the panel sides with consolidation into services.py with proper verbose gating, as it eliminates the dual-layer inconsistency rather than papering over it. Growth/positioning note: The namespace feature is the first signal that APM is moving toward a multi-publisher ecosystem -- analogous to npm orgs or OCI namespaces for containers. This is a significant positioning moment: 'APM is where AI skill packages are published, versioned, and namespaced.' The current PR buries this. A short release post framing namespaces as 'APM now has package scopes' -- with a GIF or terminal recording of installing two same-named skills from different orgs without collision -- would convert a breaking change news cycle into a positioning win. The breaking compile change should get its own callout with a migration one-liner to absorb negative signal before it becomes support volume. Per-persona findings (full)Python ArchitectclassDiagram
direction LR
class LockedDependency {
+str name
+str version
+str namespace
+list deployed_files
+dict deployed_file_hashes
+to_dict() dict
+from_dict(data) LockedDependency
}
class InstallContext {
<<dataclass>>
+dict package_types
+dict package_namespaces
+dict package_deployed_files
+set intended_dep_keys
}
class LockfileBuildPhase {
<<Phase>>
+ctx InstallContext
+build_and_save() None
-_attach_deployed_files(lockfile) None
-_attach_package_types(lockfile) None
-_attach_package_namespaces(lockfile) None
}
class SkillIntegratorHelpers {
<<module-level functions>>
+_package_namespace(package_info) str|None
+_skill_dir(root, name, ns) Path
+_skill_owner_key(name, ns) str
+_skill_owner_key_from_deployed_path(path) str
}
class InstallSourcesAcquire {
<<Strategy>>
+acquire() Materialization|None
}
class LockFile {
+dict dependencies
}
LockfileBuildPhase --> InstallContext : reads
LockfileBuildPhase --> LockFile : mutates
LockFile --> LockedDependency : contains
InstallSourcesAcquire --> InstallContext : writes package_namespaces
SkillIntegratorHelpers ..> InstallContext : indirectly via services.py
class LockedDependency:::touched
class InstallContext:::touched
class LockfileBuildPhase:::touched
class SkillIntegratorHelpers:::touched
class InstallSourcesAcquire:::touched
classDef touched fill:#fff3b0,stroke:#d47600
flowchart TD
A([apm install invoked]) --> B[sources.py acquire\nLocalSource / CachedSource / RemoteSource]
B -- IO: read apm.yml --> C{package.namespace set?}
C -- yes --> D[ctx.package_namespaces dep_key = namespace\nsources.py:223,379,550]
C -- no --> E[skip]
D --> F[LockfileBuildPhase.build_and_save\ninstall/phases/lockfile.py]
E --> F
F --> G[_attach_package_namespaces\nlockfile.py]
G -- LOCK: mutate lockfile --> H[LockedDependency.namespace = namespace\ndeps/lockfile.py]
H --> I[FS: write apm.lock.yaml]
F --> J[install/services.py: integrate_skill]
J -- lazy import --> K[_package_namespace\nskill_integrator.py:154]
K --> L{namespace truthy?}
L -- yes --> M[_skill_dir\nskill_integrator.py:162\nensure_path_within check]
L -- no --> N[legacy flat path\nskill_integrator.py]
M -- FS: copytree --> O[skills/namespace/skill_name/]
N -- FS: copytree --> P[skills/skill_name/]
O --> Q[_skill_owner_key\nskill_integrator.py:175]
Q --> R[update owned_by map\nFS: write .apm-manifest.json]
J --> S[log: Skill ns/name integrated -> path/]
Design patterns
Required findings:
Nits: See aggregated nits section above. CLI Logging ExpertRequired findings:
Nits:
DevX UX ExpertRequired findings:
Nits:
Supply Chain Security ExpertRequired findings:
Nits:
Auth ExpertInactive -- No auth-sensitive files changed; drift.py refactor uses getattr for registry_prefix/host/resolved_ref but does not alter token management, credential resolution, or AuthResolver behavior. OSS Growth HackerRequired findings:
Nits:
Growth/positioning side-channel: See growth signal in CEO arbitration above. Verdict computed deterministically: 12 required findings across 5 active panelists. APPROVE iff N == 0. Push a new commit to clear this verdict label automatically. Note 🔒 Integrity filter blocked 2 itemsThe following items were blocked because they don't meet the GitHub integrity level.
To allow these resources, lower tools:
github:
min-integrity: approved # merged | approved | unapproved | none
|
APM Review Panel Verdict: REJECT
Required before merge (12 items)
Nits (16 items, skip if you want)
CEO arbitrationThree independent panelists -- python-architect, supply-chain-security-expert, and devx-ux-expert -- converge on a single root defect: the namespace field has no enforced validation contract. The regex in manifest-schema.md documents constraints (max 64 chars, no consecutive hyphens) that the actual regex Two panelists -- devx-ux-expert and oss-growth-hacker -- independently flag the migration path as insufficient. The CHANGELOG sends users to 'copy your last generated version' with no command, no example, and no CI guidance. The cli-logging-expert's finding that namespace identity is buried mid-message and the oss-growth-hacker's finding that skills.md opens with YAML syntax rather than the 'why' are complementary legibility failures -- both indicating the namespace concept is not yet legible to a first-time user. The devx-ux-expert raises two independent UX gaps: cli-commands.md is not updated and Dissent resolved: No panelist-vs-panelist contradiction exists. On the cli-logging question: the panel sides with consolidating into services.py with proper verbose gating, eliminating the dual-layer inconsistency rather than papering over it. Growth/positioning note: The namespace feature is the first signal that APM is moving toward a multi-publisher ecosystem -- analogous to npm orgs or OCI namespaces for containers. This is a significant positioning moment: 'APM is where AI skill packages are published, versioned, and namespaced.' A short release post framing namespaces as 'APM now has package scopes' -- with a terminal recording of installing two same-named skills from different orgs without collision -- would convert a breaking change news cycle into a positioning win. Per-persona findings (full)Python ArchitectclassDiagram
direction LR
class LockedDependency {
+str name
+str version
+str namespace
+list deployed_files
+to_dict() dict
+from_dict(data) LockedDependency
}
class InstallContext {
<<dataclass>>
+dict package_types
+dict package_namespaces
+dict package_deployed_files
}
class LockfileBuildPhase {
<<Phase>>
+ctx InstallContext
+build_and_save() None
-_attach_package_namespaces(lockfile) None
}
class SkillIntegratorHelpers {
<<module-level functions>>
+_package_namespace(package_info) str|None
+_skill_dir(root, name, ns) Path
+_skill_owner_key(name, ns) str
+_skill_owner_key_from_deployed_path(path) str
}
class InstallSourcesAcquire {
<<Strategy>>
+acquire() Materialization|None
}
class LockFile {
+dict dependencies
}
LockfileBuildPhase --> InstallContext : reads
LockfileBuildPhase --> LockFile : mutates
LockFile --> LockedDependency : contains
InstallSourcesAcquire --> InstallContext : writes package_namespaces
class LockedDependency:::touched
class InstallContext:::touched
class LockfileBuildPhase:::touched
class SkillIntegratorHelpers:::touched
class InstallSourcesAcquire:::touched
classDef touched fill:#fff3b0,stroke:#d47600
flowchart TD
A([apm install invoked]) --> B[sources.py acquire\nLocalSource / CachedSource / RemoteSource]
B -- IO: read apm.yml --> C{package.namespace set?}
C -- yes --> D[ctx.package_namespaces dep_key = namespace\nsources.py 223 379 550]
C -- no --> E[skip]
D --> F[LockfileBuildPhase.build_and_save\ninstall/phases/lockfile.py]
E --> F
F --> G[_attach_package_namespaces\nphases/lockfile.py]
G -- LOCK: mutate lockfile --> H[LockedDependency.namespace = namespace\ndeps/lockfile.py]
H --> I[FS: write apm.lock.yaml]
F --> J[install/services.py: integrate_skill]
J -- lazy import --> K[_package_namespace\nskill_integrator.py:154]
K --> L{namespace truthy?}
L -- yes --> M[_skill_dir\nskill_integrator.py:162\nensure_path_within check]
L -- no --> N[legacy flat path]
M -- FS: copytree --> O[skills/namespace/skill_name/]
N -- FS: copytree --> P[skills/skill_name/]
O --> Q[_skill_owner_key\nskill_integrator.py:175]
Q --> R[update owned_by map\nFS: write .apm-manifest.json]
J --> S[log: Integrated skill ns/name -> path/]
Design patterns
Required: 3. Nits: 4. (See aggregated sections above.) CLI Logging ExpertRequired: 2. Nits: 3. (See aggregated sections above.) DevX UX ExpertRequired: 4. Nits: 3. (See aggregated sections above.) Supply Chain Security ExpertRequired: 1. Nits: 3. (See aggregated sections above.) Auth ExpertInactive -- No auth-sensitive files changed; drift.py refactor uses getattr for registry_prefix/host/resolved_ref but does not alter token management, credential resolution, or AuthResolver behavior. OSS Growth HackerRequired: 2. Nits: 4. (See aggregated sections above.) Growth/positioning side-channel: The namespace feature is the first signal that APM is moving toward a multi-publisher ecosystem. See growth note in CEO arbitration above. Verdict computed deterministically: 12 required findings across 5 active panelists. APPROVE iff N == 0. Push a new commit to clear this verdict label automatically. Note 🔒 Integrity filter blocked 2 itemsThe following items were blocked because they don't meet the GitHub integrity level.
To allow these resources, lower tools:
github:
min-integrity: approved # merged | approved | unapproved | none
|
github-actions
Bot
removed
the
panel-review
|
@shreejaykurhade it is not clear to me which harnesses support finding skills in subdirectories? |
|
@danielmeppiel Copilot SDK documents discovery from immediate subdirectories of the skill parent, and Claude’s nested discovery is for So I changed the implementation to avoid requiring recursive discovery:
|
Add an optional namespace field to
apm.yml, deploy package-owned skills and promoted sub-skills underskills/<namespace>/<skill-name>, and persist the namespace inapm.lock.yaml.Also document the namespace schema and add coverage for validation, namespaced deployment, lockfile round-tripping, and lockfile assembly.
Description
This PR implements opt-in package namespaces for skill deployment.
Today, installed skills from project packages, direct dependencies, and transitive dependencies are flattened into the same
skills/directory. That makes it harder to identify where a skill came from and easier to accidentally edit dependency-owned skills while working in a project.With this change, package authors can declare a safe single-segment
namespaceinapm.yml. When present, APM deploys native package skills and promoted.apm/skills/sub-skills under:Packages without namespace keep the existing flat layout, so current packages remain backward compatible.
Fixes #739
Type of change
Testing
Local checks run:
python -m pytest tests\test_apm_package_models.py
python -m pytest tests\unit\integration\test_skill_integrator.py tests\test_lockfile.py
python -m pytest tests\unit\test_drift_detection.py tests\unit\test_install_update.py -k "build_download_ref or drift"
python -m pytest tests\unit\test_artifactory_support.py -k "download_ref or registry_prefix"