feat(marketplace): inherit description/version from local-path apm.yml#1581
feat(marketplace): inherit description/version from local-path apm.yml#1581imbabamba wants to merge 3 commits into
Conversation
Local-path packages (`source: ./...`) now use the same fallback as remote sources when the curator entry under `marketplace.packages` omits `description` or `version`: `apm pack` reads the field from the package's own `apm.yml` and writes it to `marketplace.json`. A curator-side value still wins when set. Path resolution is constrained to the project root, and a source that resolves to the marketplace's own `apm.yml` is skipped. Follow-up to microsoft#1061, which added the same behavior for remote sources.
246ac18 to
023c1be
Compare
Tighten the local metadata inheritance docs and diagnostics, add the missing curator-side version precedence regression test, and record the bug fix in CHANGELOG. Addresses apm-review-panel follow-ups for PR microsoft#1581. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR aligns local-path marketplace packages with the existing remote-source behavior by allowing apm pack to auto-fill description and version in marketplace.json from each referenced package’s own apm.yml when the curator entry omits them.
Changes:
- Add best-effort local metadata loading (
description/version) from<project_root>/<local source>/apm.ymlduring metadata prefetch. - Apply the same “curator wins, otherwise manifest fallback” mapping logic for local packages in
ClaudeMarketplaceMapper.compose, including verbose diagnostics for divergence. - Add unit tests + compose-level tests for the local fallback behavior, and document the fallback rule in the manifest schema reference.
Show a summary per file
| File | Description |
|---|---|
src/apm_cli/marketplace/builder.py |
Adds _fetch_local_metadata() and updates _prefetch_metadata() to enrich local packages from on-disk apm.yml before optional remote fetches. |
src/apm_cli/marketplace/output_mappers.py |
Extends the local compose branch to inherit description/version from prefetched metadata when curator values are absent; adds _diagnostic_preview(). |
tests/unit/marketplace/test_builder.py |
Adds focused unit tests for _fetch_local_metadata() covering happy path + edge cases. |
tests/unit/marketplace/test_local_path_compose.py |
Adds compose-level tests verifying local inheritance and curator-override precedence. |
docs/src/content/docs/reference/manifest-schema.md |
Documents the new fallback behavior for description/version when omitted in marketplace.packages[]. |
CHANGELOG.md |
Adds an Unreleased “Fixed” entry describing the new local fallback behavior. |
Copilot's findings
- Files reviewed: 6/6 changed files
- Comments generated: 1
| package_root = ensure_path_within(self._project_root / pkg.subdir, self._project_root) | ||
| if package_root == self._project_root.resolve(): | ||
| return None | ||
| file_path = package_root / "apm.yml" | ||
| if not file_path.is_file(): | ||
| return None | ||
| data = yaml.safe_load(file_path.read_text(encoding="utf-8")) |
Tighten the local metadata inheritance docs and diagnostics, add the missing curator-side version precedence regression test, and record the bug fix in CHANGELOG. Addresses apm-review-panel follow-ups for PR microsoft#1581. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
54b298d to
734f4ed
Compare
APM Review Panel: ship_now
cc @imbabamba -- a fresh advisory pass is ready for your review. All panel signals converge cleanly from a code-quality perspective: zero correctness, security, auth, CLI UX, or coverage concerns remain after the shepherd fold commit. The PR preserves the established curator-entry-wins precedence, reads local metadata under the project root, keeps remote auth/token behavior unchanged, and now carries docs, changelog, and regression coverage for the missing version-precedence case. Aligned with: portable_by_manifest: package-level apm.yml metadata now flows into marketplace output without root-level duplication; pragmatic_as_npm: workspace-style metadata inheritance reduces surprise for monorepo producers; oss_community_driven: fixes community-reported #1725 with all in-scope review feedback folded. Growth signal. Monorepo packages now show rich marketplace browse rows (description, version) without duplicating metadata in the root manifest. Panel summary
ArchitectureclassDiagram
class MarketplaceBuilder {
+_fetch_local_metadata(pkg) dict or None
+_fetch_remote_metadata(pkg) dict or None
+_prefetch_metadata(resolved) dict
}
class ClaudeMarketplaceMapper {
+compose(config, resolved, remote_metadata) MapperResult
}
class ResolvedPackage {
+name str
+source_repo str
+subdir str
}
MarketplaceBuilder ..> ResolvedPackage : reads
MarketplaceBuilder ..> ClaudeMarketplaceMapper : passes metadata
ClaudeMarketplaceMapper ..> ResolvedPackage : emits plugin
flowchart TD
A[apm pack] --> B[_prefetch_metadata]
B --> C{local package?}
C -- yes --> D[read package apm.yml under project root]
C -- no --> E[fetch remote apm.yml unless offline]
D --> F[metadata map]
E --> F
F --> G[ClaudeMarketplaceMapper]
G --> H{packages entry field set?}
H -- yes --> I[use entry value]
H -- no --> J[inherit package apm.yml field]
RecommendationCode-side recommendation is ship. One required GitHub check ( Folded in this run
Copilot signals reviewedNo Regression-trap evidence (mutation-break gate)
Lint contract
CI
Mergeability statusCaptured from
Convergence1 outer iteration(s); 2 Copilot round(s). Final panel verdict: Not ready-to-merge until the queued Full per-persona findingsAll in-scope findings from the initial panel pass were folded in commit This panel is advisory. It does not block merge. Re-apply the |
|
Thanks for this fix, @imbabamba -- the approach was sound and went through full review. Unfortunately we can't merge it because the CLA hasn't been signed, so it's blocked indefinitely on our side. To get the fix to users we've opened an equivalent maintainer-authored PR (#1755) that mirrors your approach (local-path Closing in favor of #1755. |
TL;DR
apm packnow auto-fillsdescriptionandversioninmarketplace.jsonfrom each local-path package's ownapm.yml--the same fallback the remote source path has used since #1061.
A curator-side value under
marketplace.packagesstill wins whenset, and the local read is path-traversal-guarded against the
project root. Validated against the full unit suite (15837 tests)
with 13 new tests covering the local fallback path.
Problem (WHY)
The remote branch in
ClaudeMarketplaceMapper.composealready readsdescriptionandversionfrom each resolved package's ownapm.ymlwhen the curator entry omits them;_fetch_remote_metadatapopulates that lookup over HTTPS.
The local branch had no such fallback. It emitted only what the
curator wrote in root
apm.yml'smarketplace.packages[]. A stalecomment in
_prefetch_metadataeven claimed "Local-path packages areskipped (they carry their own metadata)" -- but the local code path
never read any metadata at all.
For a monorepo marketplace using
source: ./plugins/<name>entries,that gap leaves producers with two options:
apm.yml'smarketplace.packages[]and eachplugins/<name>/apm.yml. Thetwo copies drift over time -- the producer edits one and forgets
the other.
apm packto keep them aligned.Remote sources do not have this gap. This PR closes it for local
sources by mirroring the existing remote behavior exactly.
Approach (WHAT)
builder._prefetch_metadata{}under--offline.--offline). Returns one dict the mapper consumes the same way regardless of source kind.builder._fetch_local_metadata<project_root>/<subdir>/apm.yml, extractsdescriptionandversion. Path-traversal-guarded viaensure_path_within. Skips a source that resolves to the project root itself.output_mappers.ClaudeMarketplaceMapper.compose(is_localbranch)docs/reference/manifest-schema.mddescriptionrow read "Pass-through tomarketplace.json" with no mention of the fallback.marketplace.packagestable covers the fallback for both source kinds.Remote-source override semantics are unchanged, including the
verbose diagnostic that logs divergence between curator and package
manifest. Local sources log the equivalent diagnostic with
(package: ...)instead of(remote: ...).Implementation (HOW)
src/apm_cli/marketplace/builder.py_fetch_local_metadata(pkg), mirroring the_fetch_remote_metadatashape -- readspkg.subdirfrom disk, validates path containment, returns dict orNone._prefetch_metadatarewritten to process local packages serially first (no thread pool needed), then remote concurrently. Stale "Local-path packages are skipped" comment removed.src/apm_cli/marketplace/output_mappers.pyis_localbranch inClaudeMarketplaceMapper.composereadsmeta = remote_metadata.get(pkg.name, {})and applies the same curator-wins-else-meta hierarchy as the remote branch. Verbose diagnostic on divergence.tests/unit/marketplace/test_builder.pyTestFetchLocalMetadataclass -- 8 tests: happy path (description + version), description-only, missing apm.yml, missing subdir, path-escape, project-root edge, malformed YAML, emptysubdirfield.tests/unit/marketplace/test_local_path_compose.pyapm.yml.docs/src/content/docs/reference/manifest-schema.mdmarketplace.packagestable covering the fallback rule.Edge cases handled
apm.yml: returnsNone; no key emitted.None; error logged at debug level;the build continues.
ensure_path_withinraises;_fetch_local_metadatacatches and returnsNone.returns
Noneso the marketplace's ownapm.ymlis never readas a package manifest.
subdiron theResolvedPackage: defensive earlyreturn.
--offline: local reads always run (filesystem, no network);remote reads are still skipped.
Validation evidence
Tests:
15837 passed, 1 skipped, 21 xfailed(fulltests/unit+tests/test_console.py, ~1:25).1418 passedacrosstests/unit/marketplace, including the 13 new tests added in thisPR.
Follow-up to #1061
#1061 added the remote-source fallback (
_fetch_remote_metadata)to close the "remote-source pass-through metadata dropped" gap.
The same gap existed for local sources. This PR closes it.