fix(install): route HYBRID and CLAUDE_SKILL packages through skill-bundle path#946
fix(install): route HYBRID and CLAUDE_SKILL packages through skill-bundle path#946danielmeppiel merged 4 commits intomainfrom
Conversation
…ndle path HYBRID packages (apm.yml + SKILL.md without .apm/) and CLAUDE_SKILL packages (SKILL.md alone with co-located agents/, assets/, scripts/) are now installed as single skill bundles per agentskills.io semantics instead of being rejected by the validator. CEO-ratified principle: APM respects authorial intent at the layout level. The shape of the package root determines install semantics: .apm/ -> hoist primitives; SKILL.md -> bundle copy; plugin.json -> plugin dissection. Direct-dependency integration failures now print a [x] line inline and exit 1 via fail-loud propagation through pipeline. Errors are pushed to DiagnosticCollector for the end-of-install summary. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR updates APM's install/validation flow to treat repo-root skill bundles (SKILL.md at root, optionally with apm.yml = HYBRID) as first-class package layouts, routing them through the skill-bundle install path and surfacing direct-dependency failures loudly instead of silently continuing.
Changes:
- Add HYBRID (apm.yml + SKILL.md, no
.apm/) validation path and relax.apm/requirements where appropriate. - Fail-loud on direct dependency integration failures (inline error + diagnostics summary + non-zero exit).
- Document the supported package layouts and update help text/tests accordingly.
Show a summary per file
| File | Description |
|---|---|
src/apm_cli/models/validation.py |
Routes HYBRID packages through a new validator and improves certain validation errors. |
src/apm_cli/deps/package_validator.py |
Gates .apm/ requirement by detected package type. |
src/apm_cli/install/phases/integrate.py |
Marks and logs direct-dependency failures when integration returns None. |
src/apm_cli/install/context.py |
Adds direct_dep_failed flag to the install context. |
src/apm_cli/install/pipeline.py |
Aborts install after integrate when a direct dependency failed. |
src/apm_cli/commands/install.py |
Updates CLI help text to mention supported layouts. |
tests/test_apm_package_models.py |
Adds tests for HYBRID and CLAUDE_SKILL validation behaviors. |
docs/src/content/docs/reference/package-types.md |
Adds a new reference page describing package layouts and semantics. |
docs/src/content/docs/getting-started/first-package.md |
Adds guidance on choosing a package layout and links to the reference. |
docs/src/content/docs/reference/examples.md |
Sidebar order adjustment to accommodate the new reference page. |
docs/src/content/docs/reference/experimental.md |
Sidebar order adjustment to accommodate the new reference page. |
packages/apm-guide/.apm/skills/apm-usage/package-authoring.md |
Updates package-authoring guidance for the new layouts. |
CHANGELOG.md |
Adds an Unreleased “Fixed” entry for the new behavior. |
Copilot's findings
- Files reviewed: 13/13 changed files
- Comments generated: 6
| if deltas is None: | ||
| # Direct dependency failure: log immediately so the user | ||
| # sees [x] output inline instead of only in the deferred | ||
| # diagnostic summary (fixes "perceived hang" on HYBRID | ||
| # packages whose validation previously failed silently). | ||
| if dep_key in direct_dep_keys: | ||
| logger = ctx.logger | ||
| _fail_msg = ( | ||
| f"Package {dep_key} failed to integrate " | ||
| f"(validation or download error)." | ||
| ) | ||
| if logger: | ||
| logger.error(_fail_msg) | ||
| if ctx.diagnostics: | ||
| ctx.diagnostics.error( | ||
| _fail_msg, | ||
| package=dep_key, | ||
| detail=( | ||
| f"Resolved at {install_path}. " | ||
| f"Run with --verbose for details." | ||
| ), | ||
| ) | ||
| ctx.direct_dep_failed = True |
There was a problem hiding this comment.
New fail-loud behavior (inline [x] log + ctx.direct_dep_failed -> pipeline abort) is user-facing, but there does not appear to be a unit test exercising this path (e.g., patch run_integration_template to return None for a direct dep and assert the pipeline/command exits non-zero and emits the diagnostic). Adding a focused test would prevent regressions back to the previous silent-continue behavior.
|
|
||
| ### Fixed | ||
|
|
||
| - HYBRID packages (apm.yml + SKILL.md without `.apm/`) now install as skill bundles per agentskills.io semantics. Previously `apm install` rejected this layout in the validator and emitted no inline error, producing a perceived hang. The new validator dispatcher also accepts CLAUDE_SKILL packages (SKILL.md only) that ship co-located resource directories like `agents/`, `assets/`, or `scripts/`. Direct-dependency integration failures now print a `[x]` line immediately and exit 1 instead of failing silently. |
There was a problem hiding this comment.
CHANGELOG entry does not follow the repository's Keep a Changelog convention: it should be a single concise bullet ending with the PR number in parentheses (e.g. "... (#123)"). Consider splitting this into shorter bullets (HYBRID support, CLAUDE_SKILL resource dirs, fail-loud direct-dep integration) and adding the PR number.
| - apm.yml wins for `name`, `version`, `license`, `dependencies`, `scripts`. | ||
| - SKILL.md frontmatter wins for `description`, `allowed-tools`. |
There was a problem hiding this comment.
The docs claim SKILL.md frontmatter wins for description, but the implementation only uses SKILL.md as a fallback when apm.yml omits description (see _validate_hybrid_package: sets description only when not package.description). Please update this precedence description to match actual behavior (or adjust code/tests if SKILL.md should override).
| - apm.yml wins for `name`, `version`, `license`, `dependencies`, `scripts`. | |
| - SKILL.md frontmatter wins for `description`, `allowed-tools`. | |
| - apm.yml wins for `name`, `version`, `license`, `dependencies`, `scripts`, and any other shared field it sets. | |
| - `SKILL.md` frontmatter provides `description` only as a fallback when `apm.yml` omits it. |
| - **apm.yml wins** for: `name`, `version`, `license`, `dependencies`, `scripts`. | ||
| - **SKILL.md frontmatter wins** for: `description`, `allowed-tools`. | ||
| - On any shared-field conflict, apm.yml takes precedence and APM emits a |
There was a problem hiding this comment.
Metadata precedence documented here says SKILL.md frontmatter wins for description, but current validation logic treats SKILL.md as a fallback only when apm.yml does not set description. Please align this section with the implemented merge behavior (or change implementation/tests if SKILL.md is intended to override).
| - **apm.yml wins** for: `name`, `version`, `license`, `dependencies`, `scripts`. | |
| - **SKILL.md frontmatter wins** for: `description`, `allowed-tools`. | |
| - On any shared-field conflict, apm.yml takes precedence and APM emits a | |
| - **apm.yml wins** for: `name`, `version`, `license`, `description`, | |
| `dependencies`, `scripts`. | |
| - **SKILL.md frontmatter provides** skill-specific metadata such as | |
| `allowed-tools`, and can supply `description` only when `apm.yml` does not. | |
| - On any shared-field conflict, `apm.yml` takes precedence and APM emits a |
| # Check for .apm directory -- only mandatory for APM_PACKAGE layout. | ||
| # HYBRID and CLAUDE_SKILL packages may ship without .apm/. | ||
| from ..models.validation import detect_package_type, PackageType | ||
|
|
||
| pkg_type, _ = detect_package_type(package_path) | ||
| apm_dir = package_path / ".apm" | ||
| if not apm_dir.exists(): | ||
| result.add_error("Missing required directory: .apm/") | ||
| return result | ||
|
|
||
| if not apm_dir.is_dir(): | ||
| result.add_error(".apm must be a directory") | ||
| return result | ||
| if pkg_type in (PackageType.APM_PACKAGE, PackageType.INVALID) or pkg_type is None: | ||
| if not apm_dir.exists(): | ||
| result.add_error("Missing required directory: .apm/") | ||
| return result | ||
|
|
||
| if not apm_dir.is_dir(): | ||
| result.add_error(".apm must be a directory") | ||
| return result |
There was a problem hiding this comment.
For HYBRID packages you now allow missing .apm/, but the rest of this method still assumes .apm/ exists and will emit a misleading warning ("No primitive files found in .apm/ directory") even though HYBRID skill bundles are not expected to have primitives there. Consider short-circuiting for HYBRID/CLAUDE_SKILL layouts (or gating the primitive scan/warning behind apm_dir.exists()) so validation output matches the selected package layout.
| if ctx.direct_dep_failed: | ||
| if ctx.diagnostics and ctx.diagnostics.has_diagnostics: | ||
| ctx.diagnostics.render_summary() | ||
| raise RuntimeError( | ||
| "One or more direct dependencies failed validation. " | ||
| "Run with --verbose for details." | ||
| ) |
There was a problem hiding this comment.
This raises a new RuntimeError after integrate, but run_install_pipeline's broad except Exception will wrap it as RuntimeError("Failed to resolve APM dependencies: ..."), which is both misleading (failure was integrate/validation) and may hide the intended message. Consider using a dedicated exception type (and re-raising it like PolicyViolationError), or adjusting the outer wrapper to preserve the original message for this failure mode.
APM Review Panel VerdictDisposition: REQUEST_CHANGES (two required pre-merge fixes; core logic is correct and well-tested) Per-persona findingsPython Architect: This is a routine PR (new validation branch + new dataclass field + fail-loud error surfacing). No new base classes or protocol changes; the existing validator dispatcher pattern is cleanly extended. 1. OO / Class DiagramclassDiagram
direction TD
class PackageType {
<<Enum>>
APM_PACKAGE
CLAUDE_SKILL
HYBRID
HOOK_PACKAGE
MARKETPLACE_PLUGIN
INVALID
}
class ValidationResult {
<<ValueObject>>
+is_valid bool
+errors List[str]
+warnings List[str]
+package APMPackage
+package_type PackageType
+add_error(error)
+add_warning(warning)
}
class DetectionEvidence {
<<ValueObject>>
+has_apm_yml bool
+has_skill_md bool
+has_plugin_evidence bool
}
class validate_apm_package {
<<Pure>>
+detect_package_type()
+dispatch to sub-validator
}
class _validate_hybrid_package {
<<Pure>>
+check .apm/ present
+parse apm.yml
+merge SKILL.md frontmatter
}
class PackageValidator {
<<IOBoundary>>
+validate(package_path) ValidationResult
+_check_primitive_content()
}
class InstallContext {
<<ValueObject>>
+direct_dep_failed bool
+all_apm_deps List
+deps_to_install List
+diagnostics DiagnosticCollector
+logger CommandLogger
}
class integrate_run {
<<IOBoundary>>
+run(ctx) None
+direct_dep_keys set
}
class run_install_pipeline {
<<IOBoundary>>
+run_install_pipeline()
+fail-loud on direct_dep_failed
}
validate_apm_package ..> PackageType : classifies with
validate_apm_package ..> ValidationResult : returns
validate_apm_package ..> _validate_hybrid_package : dispatches HYBRID
validate_apm_package ..> DetectionEvidence : reads
_validate_hybrid_package ..> ValidationResult : returns
PackageValidator ..> validate_apm_package : calls
integrate_run ..> InstallContext : reads/writes
run_install_pipeline ..> integrate_run : calls
run_install_pipeline ..> InstallContext : reads direct_dep_failed
class _validate_hybrid_package:::touched
class PackageValidator:::touched
class InstallContext:::touched
class integrate_run:::touched
class run_install_pipeline:::touched
classDef touched fill:#fff3b0,stroke:#d47600
2. Execution Flow Diagramflowchart TD
A["apm install pkg"] --> B["run_install_pipeline() [install/pipeline.py]"]
B --> C["_integrate_phase.run(ctx) [install/phases/integrate.py]"]
C --> D["direct_dep_keys = set(dep.get_unique_key() for dep in ctx.all_apm_deps)"]
D --> E["for dep in ctx.deps_to_install"]
E --> F["run_integration_template(source) [FS]"]
F --> G["validate_apm_package(package_path) [models/validation.py]"]
G --> H["detect_package_type(package_path)"]
H --> I{PackageType?}
I -- HYBRID + no .apm/ --> J["_validate_hybrid_package()\nparse apm.yml, read SKILL.md frontmatter [I/O]"]
I -- HYBRID + .apm/ present --> K["_validate_apm_package_with_yml() [I/O]"]
I -- APM_PACKAGE --> K
I -- CLAUDE_SKILL --> L["_validate_claude_skill() [I/O]"]
J --> M["ValidationResult returned"]
K --> M
L --> M
M --> N{deltas is None?}
N -- "No -- integrated OK" --> O["accumulate deltas; installed_count += 1"]
O --> E
N -- "Yes + dep_key in direct_dep_keys" --> P["logger.error(_fail_msg)\nctx.diagnostics.error(...)\nctx.direct_dep_failed = True"]
P --> E
N -- "Yes + transitive dep" --> E
E --> Q{ctx.direct_dep_failed?}
Q -- Yes --> R["diagnostics.render_summary()\nraise RuntimeError [exit 1]"]
Q -- No --> S["_update_gitignore_for_apm_modules() [FS]"]
S --> T["install complete [+]"]
Design patterns
Minor concern: CLI Logging Expert: The fail-loud error path (
DevX UX Expert: The experience improvement is real and well-executed. Specific observations:
Supply Chain Security Expert: No new security surface introduced. Observations:
Auth Expert: Not activated -- this PR modifies package type detection ( OSS Growth Hacker: Strong growth signal in this PR.
CEO arbitrationThe core change is correct, well-tested, and strategically important -- agentskills.io compatibility is an adoption unlock that aligns with the (1) Doc/code inconsistency on description precedence. Both (2) The double-logging concern and the Required actions before merge
Optional follow-ups
|
…eview fixes Round-2 review fixes for PR #946: Conceptual: apm.yml.description and SKILL.md description are independent fields with different consumers (CLI/search vs. agent invocation matcher per agentskills.io) and are NOT merged. Removed the silent backfill that let SKILL.md leak into APMPackage.description. apm pack now emits a hard warning when apm.yml is missing 'description' on a HYBRID package -- the publish-time gate for the AUTHOR. Security: ignore_symlinks now wraps all 3 shutil.copytree call sites in skill_integrator.py (lines 330, 568, 818). The third site composes ignore_symlinks with shutil.ignore_patterns('.apm') via a closure. UX: integrate.py emits direct-dep failures via diagnostics OR logger (not both), with full detail deferred to render_summary. apm view shows a HYBRID-aware hint when apm.yml.description is missing. Docs/skill-resources: replaced 'metadata precedence' framing with 'metadata model' (independence + two consumers) in docs/reference and in packages/apm-guide/.apm/skills/apm-usage/. commands.md install row purpose updated to match new help text. CHANGELOG entry extended and tagged with (#946). Tests: 5494 passed (1 pre-existing failure unrelated to this PR). Added TestPackHybridDescriptionWarning (3 tests), TestSkillIntegratorCopytreeSymlinkContainment (2 tests), and 2 HYBRID-independence tests in test_apm_package_models.py. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Round 3 update — all required findings addressedPushed Conceptual fix: metadata modelThe reviewer caught that docs claimed "SKILL.md frontmatter wins for description, allowed-tools" — wrong on both counts. Reframed as two independent fields with two consumers (CLI/search vs. agent invocation matcher per agentskills.io). APM no longer merges them. The silent backfill in What changed
Tests
Panel verdict (round 3 pre-push)APPROVED-WITH-NITS from
These are post-merge follow-ups (will file issues if approved). |
danielmeppiel
added
panel-review
APM Review Panel VerdictDisposition: REQUEST_CHANGES (two small surgical pre-merge fixes required; all security and architectural findings are positive) Per-persona findingsPython Architect: The PR correctly extends the existing Strategy + type-dispatch pattern ( classDiagram
direction LR
class PackageType {
<<Enumeration>>
APM_PACKAGE
CLAUDE_SKILL
HYBRID
MARKETPLACE_PLUGIN
INVALID
}
class ValidationResult {
<<ValueObject>>
+package_type PackageType
+package APMPackage
+errors List
+add_error(msg)
+add_warning(msg)
}
class validate_apm_package {
<<Dispatcher>>
routes_by_PackageType()
}
class _validate_hybrid_package {
back_compat_apm_dir_fallback()
skill_bundle_no_apm_path()
}
class _validate_apm_package_with_yml {
}
class _validate_claude_skill {
}
class PackageValidator {
validate(package_path)
}
class SkillIntegrator {
copy_skill_to_target()
integrate_package_skills()
sync_integration()
}
class BaseIntegrator {
<<Abstract>>
}
class InstallContext {
all_apm_deps List
direct_dep_failed bool
}
validate_apm_package ..> _validate_hybrid_package : HYBRID
validate_apm_package ..> _validate_apm_package_with_yml : APM_PACKAGE
validate_apm_package ..> _validate_claude_skill : CLAUDE_SKILL
_validate_hybrid_package ..> _validate_apm_package_with_yml : delegates when .apm/ exists
PackageValidator ..> validate_apm_package : calls
SkillIntegrator --|> BaseIntegrator
ValidationResult o-- PackageType
flowchart TD
A[apm install pkg] --> B[resolve phase]
B --> C[integrate phase]
C --> D[build direct_dep_keys from ctx.all_apm_deps]
D --> E{for each dep}
E --> F[run_integration_template]
F --> G{deltas is None?}
G -- No --> H[accumulate counters]
H --> E
G -- Yes, direct dep --> I[diagnostics.error OR logger.error]
I --> J[ctx.direct_dep_failed = True]
J --> E
E -- done --> K{ctx.direct_dep_failed?}
K -- Yes --> L[diagnostics.render_summary]
L --> M[raise RuntimeError -- exits non-zero]
K -- No --> N[write lockfile, update gitignore]
Design patterns:
Minor issues: (1) CLI Logging Expert: The direct-dep failure path in Two output regressions flagged:
DevX UX Expert: Error message improvements are textbook good UX.
Blocking: The Supply Chain Security Expert: This PR resolves a known symlink containment gap. All 3 Path containment for the new HYBRID path: Fail-loud raises Auth Expert: Not activated -- PR changes OSS Growth Hacker: HYBRID + agentskills.io support is APM's strongest 2026 growth lever. Side-channel to CEO: consider whether HYBRID support warrants a v0.x minor bump and a dedicated announcement to ride the CEO arbitrationSpecialists are in strong agreement: the architecture is correct, the security hardening is a genuine fix of a known gap, and the HYBRID + agentskills.io support is strategically sound. The only disagreements are on surface-level output issues -- both CLI Logging Expert and DevX UX Expert independently flag the Required actions before merge
Optional follow-ups
|
|
@copilot fix the identified regressions as per last comment |
…r warning, fail-loud test, CHANGELOG format Agent-Logs-Url: https://github.com/microsoft/apm/sessions/72a23a26-82de-47ba-a39e-3d4f2aa44094 Co-authored-by: danielmeppiel <51440732+danielmeppiel@users.noreply.github.com>
Fixed in
5617 passed, 1 deselected (pre-existing), 0 failures. |
TL;DR
apm install danielmeppiel/genesis -gappeared to hang because the validator silently rejected packages shipped as a single skill bundle at the repo root (apm.yml + SKILL.md, no.apm/). This PR teaches APM to recognize the skill-bundle layout as a first-class shape and route it through the existing_integrate_native_skillcopy path, so the skill installs as one bundle with its co-locatedagents/,assets/, andscripts/intact. CLAUDE_SKILL packages (SKILL.md alone) get the same treatment. Direct-dependency failures now print a[x]line inline and exit1instead of failing silently.Problem (WHY)
models/validation.pyhad three branches: APM (.apm/), CLAUDE_SKILL (SKILL.mdonly), CLAUDE_PLUGIN (plugin.json). HYBRID (apm.yml + SKILL.md) was detected bydetect_package_typebut then fell through to_validate_apm_package_with_yml, which mandates a.apm/directory and producesMissing required directory: .apm/.package_validator.pyenforced the same.apm/mandate without consulting the package type, so even fixing validation alone would not help.integrate.pyreturneddeltas is Noneand silentlycontinued the loop. No[x]line, no diagnostic. The user sees CLI sit at the integrate phase until the run completes with a clean exit code, hence "perceived hang."agents/into.github/agents/would violate authorial intent. agentskills.io's encapsulation contract is thatagents/,assets/,scripts/inside a skill directory are private resources, "agents pattern-match well against concrete structures" -- the structure IS the contract.Note
CEO-ratified principle from the panel review: APM respects authorial intent at the layout level. Three layouts -> three install semantics. The shape of the package root determines how APM installs it.
Approach (WHAT)
.apm/(with or without apm.yml)SKILL.md(alone or with apm.yml -- HYBRID)<target>/skills/<name>/plugin.json/.claude-plugin/_map_plugin_artifactsMetadata precedence for HYBRID: apm.yml wins on
name,version,license,dependencies,scripts; SKILL.md frontmatter wins ondescription,allowed-tools. Conflicts go to apm.yml with a verbose-mode warning. This keeps APM-owned fields (deps, scripts) authoritative while leaving Claude-runtime fields owned by the skill.Implementation (HOW)
src/apm_cli/models/validation.py_validate_hybrid_package()validator (~60 lines); dispatcher routing at_validate_package_structure. Improved error wording for INVALID and missing-.apm/cases.src/apm_cli/deps/package_validator.py.apm/directory mandate behinddetect_package_type()-- only required forAPM_PACKAGEandINVALID. HYBRID and CLAUDE_SKILL are exempt.src/apm_cli/install/phases/integrate.pydirect_dep_keysfrom manifest; emits[x]vialogger.error()and pushes toDiagnosticCollectorwhendeltas is Nonefor a direct dep; setsctx.direct_dep_failed = True.src/apm_cli/install/context.pydirect_dep_failed: bool = Falsefield.src/apm_cli/install/pipeline.pyRuntimeErrorwhenctx.direct_dep_failed. Outer handler incommands/install.pycatches and exits1.tests/test_apm_package_models.pyTestHybridPackageValidation(5 tests) +TestClaudeSkillPackageValidation(2 tests).docs/src/content/docs/reference/package-types.mddocs/src/content/docs/getting-started/first-package.mdpackages/apm-guide/.apm/skills/apm-usage/package-authoring.mdsrc/apm_cli/commands/install.pyCHANGELOG.md### Fixed.The skill integrator's existing
_integrate_native_skill(shutil.copytree(package_path, target_skill_dir, ignore=shutil.ignore_patterns('.apm'))) already copies arbitrary subdirectories. No changes needed there. Encapsulation is preserved becausedistributed_compiler.pyonly scans.github/agents/*.agent.mdandDEPENDENCY_PRIMITIVE_PATTERNSonly scans inside.apm/agents/-- bundledagents/files insideskills/<name>/agents/are never hoisted.Diagrams
Flow: validator dispatch by detected package type. Each shape routes to its own validator and integrator -- HYBRID joins CLAUDE_SKILL on the bundle path.
flowchart TD A[Package root] --> B{detect_package_type} B -->|.apm/ present, no SKILL.md| C[APM_PACKAGE] B -->|SKILL.md only| D[CLAUDE_SKILL] B -->|apm.yml + SKILL.md| E[HYBRID] B -->|plugin.json| F[CLAUDE_PLUGIN] C --> G[_validate_apm_package_with_yml<br/>mandates .apm/] D --> H[_validate_claude_skill] E --> I[_validate_hybrid_package<br/>NEW] F --> J[_validate_claude_plugin] G --> K[Hoist primitives to target dirs] H --> L[_integrate_native_skill<br/>copy tree to skills/name/] I --> L J --> M[_map_plugin_artifacts<br/>dissect by manifest]Failure path: when a direct dep returns
deltas is None, the new fail-loud path surfaces the error to the user and exits non-zero instead of silently continuing.sequenceDiagram participant User participant Pipeline participant Integrate as integrate phase participant Logger as CommandLogger participant Diag as DiagnosticCollector User->>Pipeline: apm install <dep> Pipeline->>Integrate: run integrate Integrate->>Integrate: run_integration_template -> deltas=None Note over Integrate: dep_key in direct_dep_keys Integrate->>Logger: error("[x] Package X failed to integrate") Integrate->>Diag: error(msg, package=X, detail=...) Integrate->>Pipeline: ctx.direct_dep_failed=True Pipeline->>Diag: render_summary() Pipeline->>User: raise RuntimeError -> exit 1Trade-offs
dependencies/scriptsare APM-owned semantics; the skill's runtime metadata (description,allowed-tools) stays SKILL.md-owned. Verbose warning gives authors a deterministic signal when they conflict.ensure_path_withinguards to_integrate_native_skillin this PR. Sec audit confirmed the gap is pre-existing onmain(CLAUDE_SKILL already reaches that path). Filing as a P1 follow-up rather than expanding scope. "Favor small, chainable primitives over monolithic frameworks."deployed_filesrecords directory paths so the cleanup gate cannot detect intra-bundle edits. Filing as a P2 follow-up; out of scope here.package_type_info()verbose line already fires fromsources.py:536for downloaded packages -- no duplicate log added at validation time.Benefits
apm install danielmeppiel/genesis -gworks end-to-end (was the bug report).agents//assets//scripts/-- the agentskills.io canonical shape.1with a[x]line and an entry in the install summary -- no more "perceived hang".Validation
The deselected test is a pre-existing Mock-wiring bug unrelated to this PR (Mock object compared against a string --
result.host == 'git.example.com'fails becausehostwas wired through a Mock). It also fails onmain.New tests added (7)
TestHybridPackageValidation:test_hybrid_package_with_apm_yml_and_skill_md_validatestest_hybrid_package_without_apm_dir_validatestest_hybrid_package_with_optional_subdirs_validatestest_hybrid_package_missing_skill_md_invalidtest_hybrid_package_with_apm_dir_still_validatesTestClaudeSkillPackageValidation:test_claude_skill_with_agents_dir_not_misclassified_as_plugintest_claude_skill_only_skill_md_validatesHow to test
cd /tmp && uv tool run --from /Users/<you>/Repos/awd-cli-hybrid-skill apm install danielmeppiel/genesis -g-- expect a clean install with a[+] Installed skill genesisline and exit0.~/.apm/skills/genesis/-- expectSKILL.md,agents/genesis-architect.agent.md, and any other co-located bundle resources to be present.~/.apm/agents/-- expectgenesis-architect.agent.mdto NOT be there (encapsulation invariant).apm.yml(e.g. invalid YAML), install with--verbose, expect a[x]line naming the package and a non-zero exit code.apm install --help-- expect the new layout hint in the description.Panel review trail
Single-comment verdict from the
apm-review-panelwas synthesized in the planning conversation. The 5 mandatory specialists + CEO + Growth Hacker ratified the design;auth-expertwas inactive (nocore/auth.py,token_manager.py,azure_cli.py,github_downloader.py,marketplace/client.py, orvalidation.pyauth surface touched). Subsequent subagent reviews on the implementation:skill_integrator.copytree, per-file deployed-file hashes for bundles,enterprise/security.md:205accuracy).DiagnosticCollector). All paths route throughCommandLogger.package-types.md,first-package.mdsection, install help one-liner).Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com