Skip to content

feat: marketplace UX, security hardening, and lockfile provenance (#514)#677

Merged
sergio-sisternes-epam merged 1 commit intomainfrom
514-marketplace-versioning
Apr 20, 2026
Merged

feat: marketplace UX, security hardening, and lockfile provenance (#514)#677
sergio-sisternes-epam merged 1 commit intomainfrom
514-marketplace-versioning

Conversation

@sergio-sisternes-epam
Copy link
Copy Markdown
Collaborator

@sergio-sisternes-epam sergio-sisternes-epam commented Apr 11, 2026

Description

Adds marketplace-based version management for monorepo packages, allowing per-package semver versioning through versions[] arrays in marketplace.json. This solves the monorepo versioning problem (repo-wide tags vs. tag pollution vs. poly-repos) by leveraging the marketplace as a version registry.

Users can now install specific versions (apm install plugin@marketplace#^2.0.0), view available versions (apm view plugin@marketplace), check for updates with range awareness (apm outdated), and publish new versions (apm marketplace publish). Security hardening includes advisory immutability warnings when version refs change and multi-marketplace shadow detection.

Fully backward compatible: plugins without versions[] use the existing single-ref flow. Direct git dependencies are completely unaffected.

Related to #514

Type of change

  • Bug fix
  • New feature
  • Documentation
  • Maintenance / refactor

Changes

Phase 1: Marketplace Version Schema + Semver Resolution

  • marketplace/models.py: VersionEntry dataclass, versions field on MarketplacePlugin
  • marketplace/version_resolver.py (new): Lightweight semver engine supporting ^, ~, >=, >, <, <=, !=, exact, and compound ranges. No external dependencies.
  • marketplace/resolver.py: Version-aware resolve_marketplace_plugin() returning 3-tuple (canonical, plugin, resolved_version). Falls back to single-ref flow when no versions[] present.
  • commands/install.py: Marketplace intercept with version spec parsing, resolved_version provenance in lockfile
  • commands/view.py: _display_marketplace_versions() — Rich table showing all published versions with "latest" tag
  • commands/outdated.py: _check_marketplace_versions() — range-aware update checking with "(outside range)" annotations
  • deps/lockfile.py: New fields: version_spec, resolved_version, discovered_via, marketplace_plugin_name

Phase 2: Publishing Tooling

  • commands/marketplace.py: apm marketplace publish — publishes current package version to marketplace.json with SHA-pinned refs. apm marketplace validate — validates marketplace integrity (4 checks: refs, duplicates, semver format, source URLs).
  • marketplace/validator.py (new): Marketplace validation engine

Phase 3: Security Hardening

  • marketplace/version_pins.py (new): Advisory immutability — caches version-to-ref mappings, warns on ref changes (potential ref-swap attacks)
  • marketplace/shadow_detector.py (new): Multi-marketplace shadow detection — warns when the same plugin name appears in multiple registered marketplaces
  • Security warnings route through CommandLogger via warning_handler callback (visible at -v)

Documentation

  • Marketplace guide with versioning section, publishing workflow, and security model
  • CLI reference for marketplace publish and marketplace validate
  • Lockfile spec updated with new fields
  • CHANGELOG.md entry
  • Skill files updated (commands.md, dependencies.md)

Backward Compatibility

Scenario Behavior
Plugin WITH versions[] New version resolution flow
Plugin WITHOUT versions[] Existing single-ref flow (unchanged)
Direct git dependency Existing git-based flow (unchanged)
Lockfile without version_spec Treated as exact pin (safe default)

Testing

  • Tested locally
  • All existing tests pass
  • Added tests for new functionality (if applicable)

Unit Tests

4017 tests passing (+227 new tests across 12 test files covering version resolver, models, publish, validate, shadow detector, version pins, versioned resolver, outdated marketplace, view versions, and install integration).

Integration Tests (50 tests against a live marketplace)

End-to-end tested against a private marketplace with 68 versioned plugins (including multi-version packages with 5-6 versions each):

Suite Tests Result
Install (exact, range, latest, lockfile, exit codes) 10 PASS
View / Outdated / Validate 9 PASS
Security (immutability, shadow detection, pin persistence) 6 PASS
Version Ranges (^, ~, >=, exact, compound, error) 10 PASS
Non-Marketplace Regression (init, local, compile, config, uninstall, mixed) 15 PASS

Copilot AI review requested due to automatic review settings April 11, 2026 18:13
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds marketplace-based, per-plugin semver versioning and resolution to APM so monorepo packages can publish and install independently versioned releases via marketplace.json (versions[]), with supporting CLI UX (install/view/outdated/publish/validate) and supply-chain advisories (immutability pins + shadow detection).

Changes:

  • Introduces versions[] schema (VersionEntry) and a built-in semver range resolver for marketplace plugins.
  • Updates resolution + install/outdated/view flows to understand NAME@MARKETPLACE[#version_spec], record provenance in the lockfile, and display version-aware UX.
  • Adds apm marketplace publish / validate, plus security advisories (ref immutability pins; cross-marketplace shadow detection), and expands tests/docs/changelog accordingly.

Reviewed changes

Copilot reviewed 30 out of 30 changed files in this pull request and generated 10 comments.

Show a summary per file
File Description
tests/unit/test_view_versions.py Adds CLI tests for apm view NAME@MARKETPLACE versions output/sorting/badges.
tests/unit/test_view_command.py Ensures apm view NAME@MARKETPLACE routes to marketplace versions display.
tests/unit/test_version_resolver.py Unit tests for semver spec detection and range resolution.
tests/unit/test_outdated_marketplace.py Unit tests for marketplace-aware apm outdated behavior and range annotations.
tests/unit/marketplace/test_versioned_resolver.py Tests version-aware marketplace resolution + lockfile field round-trips + warning routing.
tests/unit/marketplace/test_version_pins.py Tests version pin cache persistence, atomic writes, and fail-open behavior.
tests/unit/marketplace/test_shadow_detector.py Tests multi-marketplace shadow detection and resolver integration warnings.
tests/unit/marketplace/test_marketplace_validator.py Tests manifest validator + apm marketplace validate output/exit codes.
tests/unit/marketplace/test_marketplace_resolver.py Updates parsing tests for the new #... fragment support.
tests/unit/marketplace/test_marketplace_publish.py Tests apm marketplace publish defaults/options/conflicts/dry-run and JSON updates.
tests/unit/marketplace/test_marketplace_models.py Tests VersionEntry + parsing of versions[] from marketplace.json.
tests/unit/marketplace/test_marketplace_install_integration.py Tests install interception changes + exit code + verbose resolved-version logging.
src/apm_cli/marketplace/version_resolver.py New semver parser/range resolver and is_version_specifier() heuristic.
src/apm_cli/marketplace/version_pins.py New local cache for version->ref pins (immutability advisory).
src/apm_cli/marketplace/validator.py New manifest validation engine (schema/versions/duplicates).
src/apm_cli/marketplace/shadow_detector.py New advisory scan for same plugin name across registered marketplaces.
src/apm_cli/marketplace/resolver.py Extends parsing to NAME@MKT#...; resolves semver to refs; emits advisories via handler.
src/apm_cli/marketplace/models.py Adds VersionEntry + MarketplacePlugin.versions and parsing support.
src/apm_cli/marketplace/client.py Switches marketplace fetch to auth-first to avoid private-repo 404 swallowing.
src/apm_cli/deps/lockfile.py Adds version_spec + resolved_version fields to lockfile entries.
src/apm_cli/commands/view.py Adds marketplace version display and routes apm view NAME@MKT to it.
src/apm_cli/commands/outdated.py Adds marketplace-aware update checks and a “Source” column.
src/apm_cli/commands/marketplace.py Adds marketplace validate and marketplace publish (plus helper functions).
src/apm_cli/commands/install.py Parses #version_spec, resolves marketplace versions, records provenance/lockfile fields, fixes all-failed exit code.
packages/apm-guide/.apm/skills/apm-usage/dependencies.md Documents marketplace semver specifiers in dependency syntax.
packages/apm-guide/.apm/skills/apm-usage/commands.md Updates skill command reference for new marketplace/version commands.
docs/src/content/docs/reference/lockfile-spec.md Documents new lockfile field(s) for marketplace versioning.
docs/src/content/docs/reference/cli-commands.md Documents new CLI syntax/commands and updated outdated/view behaviors.
docs/src/content/docs/guides/marketplaces.md Adds versioning/publishing/validation/security sections to marketplace guide.
CHANGELOG.md Adds Unreleased entries for marketplace version management feature set.

Comment thread docs/src/content/docs/guides/marketplaces.md Outdated
Comment thread src/apm_cli/commands/view.py Outdated
Comment thread src/apm_cli/commands/view.py Outdated
Comment thread src/apm_cli/commands/outdated.py Outdated
Comment thread src/apm_cli/commands/marketplace.py Outdated
Comment thread docs/src/content/docs/reference/lockfile-spec.md Outdated
Comment thread docs/src/content/docs/reference/cli-commands.md Outdated
Comment thread docs/src/content/docs/guides/marketplaces.md Outdated
Comment thread docs/src/content/docs/reference/cli-commands.md Outdated
Comment thread packages/apm-guide/.apm/skills/apm-usage/commands.md Outdated
@danielmeppiel
Copy link
Copy Markdown
Collaborator

This seems to extend the plugin/marketplace spec with the new versions array? In my opinion, we should not extend or modify it since the spec is evolving rapidly and thus we may introduce difficult issues to reconcile down the line @sergio-sisternes-epam

@sergio-sisternes-epam
Copy link
Copy Markdown
Collaborator Author

@danielmeppiel Yes. leverage the marketplace.json and add a versions[] array, and add a "publish" command helping to manage the edits on the file, rather than letting users do it manually, is the only idea I could come up to mitigate the current gap in mono-repo version management.

When designing it, we ensure retro-compatibility: If versions[] is absent, APM follows the default behaviour it has today. When present, the marketplace versions supersede the default behaviour. This should mitigate/prevent any problems down the line.

This branch has been tested in closed preview in a couple of scenarios, and feedback has been positive (pre-merge).

@danielmeppiel
Copy link
Copy Markdown
Collaborator

Thanks for pushing this forward, @sergio-sisternes-epam — and for the patience while I caught up with the broader marketplace cluster (see threads on #514, #676, #722).

I want to land the parts of this PR that are unambiguously valuable while pulling back on the schema extension. Concretely:

Where I land on versions[]

I appreciate that you scoped it as additive (absent → today's behavior, present → opt-in). That's a clean design choice. But the spec drift concern remains, and it's load-bearing:

  • Anthropic is iterating marketplace.json actively. Adding fields outside their schema means the same marketplace.json will be read differently by APM vs Claude Code vs whatever Anthropic ships next — and end users will get different resolved versions of the same plugin in different tools. That's exactly the failure mode that erodes trust in the catalog.
  • The "monorepo per-package versioning" gap that motivated versions[] is actually solvable using only standard spec fields, via per-entry git-subdir sources with prefixed git tags. I laid out the full pattern in #514 (one entry per plugin, each pinned to its own <plugin>-vX.Y.Z ref + sha). The discovery surface is the marketplace itself — apm marketplace browse lists all plugins with versions, so the "which tag do I install" friction Werner raised dissolves.
  • Where standard-schema falls short for maintainers is the authoring overhead of bumping refs/shas across N entries on every release. That's the right problem to solve, and it's what [FEATURE] Add apm marketplace generate command to pack all plugins and produce a marketplace.json #722 (apm marketplace generate + companion publish / lint) is now scoped for on the roadmap.

So the architectural call is: don't extend the schema, automate the authoring instead.

What I'd love to see this PR become

The pieces below are exactly what the "Marketplace authoring suite" track needs and would land cleanly as a tighter PR:

  • apm marketplace publish — re-targeted to update standard-spec ref + sha on a single entry (atomic per-package release: tag → bump entry → optional PR). Drop the versions[] list-mutation path.
  • apm marketplace validate + MarketplaceValidator — schema validation against the canonical Anthropic spec (and future Cloudflare RFC .well-known/agent-skills/index.json shape, which Marketplace add source parity with the Anthropic spec #676 is wiring in).
  • apm view NAME@MARKETPLACE + apm outdated marketplace-awareness — both incredibly useful, no schema change needed (works against standard version field per entry).
  • LockedDependency.source_url / source_digest — already format-neutral, keep as-is.
  • client.py auth-first fetch fix — solid bug fix, keep.
  • marketplace/shadow_detector.py — same-name-across-marketplaces advisory is a real supply-chain win, keep.
  • marketplace/version_pins.py + immutability advisories — these still apply to standard spec resolution (verifying that a tag resolved to an immutable sha at install time), keep with that re-framing.

The pieces to drop:

  • VersionEntry model + MarketplacePlugin.versions field
  • Tests covering versions[] parsing (test_marketplace_models.py parts, test_versioned_resolver.py parts)
  • version_resolver.py's heuristic that #X.Y.Z resolves against versions[] — the standard-schema equivalent is #<ref> against the entry's source.ref, which APM already does
  • Docs sections describing versions[] syntax

This is more rework than I'd normally ask, and I appreciate that's not free given how much you've shipped here. The shadow detection, publish/validate skeleton, view/outdated UX, and lockfile provenance are genuinely great work — they just deserve to land on the standard schema so they age well.

Happy to discuss on the issues directly (#722 is the natural home for the authoring side), or jump on a quick async thread here. What's the easiest path forward for you?

@danielmeppiel danielmeppiel added the marketplace Marketplace federation: marketplace.json publishing, sourcing, and federation label Apr 19, 2026
Copy link
Copy Markdown
Collaborator

@danielmeppiel danielmeppiel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes to gate merge until the scope reduction in #677 (comment) is addressed.

Summary of asks (full detail in the linked comment):

  1. Drop the versions[] schema extension on MarketplacePlugin. It diverges from Anthropic's marketplace.json spec and creates cross-tool drift (Claude CLI and APM would resolve different versions from the same marketplace.json).
  2. Drop version_resolver.py — heuristic version selection on top of a non-standard schema compounds the drift.
  3. Redirect apm marketplace publish work to #722 (Marketplace authoring suite) so the publish/validate UX lands in the right home with consistent UX alongside apm marketplace generate / lint.

Keepers (this PR is the right vehicle for these):

  • MarketplaceValidator — schema validation against the standard spec
  • Shadow detector
  • view / outdated UX additions
  • LockedDependency.source_url / source_digest provenance
  • Auth-first client fix
  • version_pins.py (separate from version_resolver — pin enforcement, not invented schema)

Once the versions[] work is split out and either dropped or moved behind a feature gate, happy to re-review and approve the slimmer PR. The non-controversial pieces above are valuable and we want them in.

@sergio-sisternes-epam
Copy link
Copy Markdown
Collaborator Author

Thanks for the detailed and thoughtful review, Daniel.

Pushed the scope reduction you requested:

Dropped:

Kept and refactored:

  • MarketplaceValidator — schema + duplicate-names checks (standard spec only)
  • Shadow detector — unchanged
  • View / outdated UX — rewritten to work with the standard single-entry version + source.ref fields
  • Auth-first client fix — unchanged
  • version_pins.py — reframed as ref immutability pins (flat {mkt/plugin: ref} model, no version dimension)
  • Lockfile provenancediscovered_via + marketplace_plugin_name (kept); version_spec + resolved_version (removed)

The # suffix in NAME@MKT#ref now acts purely as a raw git ref override against the marketplace entry's source.ref — no semver interpretation. This aligns with the standard-spec approach you outlined in #514.

3,907 tests pass. Ready for re-review when you have a moment.

@sergio-sisternes-epam sergio-sisternes-epam changed the title feat: marketplace-based version management (#514) feat: marketplace UX, security hardening, and lockfile provenance (#514) Apr 20, 2026
@sergio-sisternes-epam sergio-sisternes-epam force-pushed the 514-marketplace-versioning branch from 3230c01 to 4fc3a22 Compare April 20, 2026 11:38
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 25 out of 26 changed files in this pull request and generated 8 comments.

Comment thread src/apm_cli/commands/install.py
Comment thread src/apm_cli/commands/outdated.py Outdated
Comment thread src/apm_cli/commands/view.py Outdated
Comment thread src/apm_cli/marketplace/resolver.py Outdated
Comment thread src/apm_cli/marketplace/models.py Outdated
Comment thread docs/src/content/docs/reference/cli-commands.md Outdated
Comment thread docs/src/content/docs/reference/cli-commands.md Outdated
Comment thread tests/unit/marketplace/test_marketplace_install_integration.py
@danielmeppiel
Copy link
Copy Markdown
Collaborator

APM Review Panel: PR #677 (post-scope-reduction)

Re-reviewing the slimmed PR after the April-20 scope reduction. Verdict at the bottom.

Context refresh: this PR was originally a versions[] schema extension that diverged from Anthropic's marketplace.json spec. After the scope-reduction request, Sergio dropped ~1,800 LOC (versions[], version_resolver, publish command) and reframed version_pins.py as ref-immutability pins on the standard schema. Authoring/publishing UX is now correctly redirected to the #722 marketplace authoring epic.

What's in scope for this re-review: 26 files / +1357/-81 LOC. Six concrete deliverables:

  1. MarketplaceValidator (schema + duplicate names) + apm marketplace validate CLI
  2. Cross-marketplace shadow_detector
  3. apm view NAME@MARKETPLACE plugin metadata display
  4. apm outdated marketplace awareness (Source column + per-entry ref check)
  5. Marketplace client unauth_first=False auth-first fix
  6. version_pins.py — ref immutability advisory cache (reframed from versions[] to flat marketplace/plugin -> ref)

supply-chain-security-expert

Strong agree on direction; one P1 regression introduced by the reframe.

The scope-reduction is the right call from a supply-chain perspective: extending marketplace.json outside Anthropic's schema would have created catalog-level cross-tool resolution drift (same JSON, different resolved versions in APM vs Claude Code). That's the scariest class of supply-chain bug — "the artifact your security review approved is not the artifact your runtime installed". Excised cleanly.

Three findings:

P1 -- version_pins.py reframe creates false-positive ref-swap warnings on legitimate version bumps

The versions[] design pinned (marketplace, plugin, version) -> ref. So v2.0.0 always SHA abc..., and a swap on v2.0.0 was a real attack signal. The reframe collapses the version dimension: the cache is now {"marketplace/plugin": "ref"} (one pin per plugin, no version axis). See version_pins.py:60 (_pin_key) and resolver.py:295-313.

Concrete failure mode:

  1. User installs code-review@acme; marketplace has source.ref: v2.0.0. Pin recorded: {"acme/code-review": "v2.0.0"}.
  2. Marketplace maintainer ships v2.1.0, updates source.ref: v2.1.0. Legitimate.
  3. Next user runs apm install code-review@acme. APM emits: "Plugin code-review@acme ref changed: was 'v2.0.0', now 'v2.1.0'. This may indicate a ref swap attack."

That's the boy-who-cried-wolf failure mode — security warnings fire on every routine update. Within two weeks, users mute or ignore them, and a real ref swap goes unnoticed. This is worse than no warning at all because it normalizes dismissal of the warning text.

Three fixable shapes (pick one):

  • (a) Re-introduce the version dimension in the pin key only (no schema extension): pin (mkt, plugin, version_field) -> ref. The plugin's version field is already on the standard spec. Warn only when same version resolves to a different ref.
  • (b) Drop ref-pinning entirely from this PR; reland it once [FEATURE] Add apm marketplace generate command to pack all plugins and produce a marketplace.json #722 lands real per-version sources. Defensible — the validator + shadow detector + UX fixes still ship.
  • (c) Compare resolved commit SHA, not the user-facing ref label: pin (mkt, plugin) -> commit_sha. Tag moves are fine; commit changes for the same tag are an attack signal. Requires resolving the ref to a SHA at pin time (one extra git ls-remote).

(a) is cleanest and matches Anthropic's spec.

P1 -- security warnings emitted at verbose-only visibility

commands/install.py:198 wires warning_handler = lambda msg: logger.verbose_detail(...). So ref-swap and shadow warnings only render with -v. Default apm install swallows them.

Shadow detection and ref-immutability are security signals, not diagnostic chatter. They must be visible at the default verbosity. Suggested fix: route through logger.warning(...) (which renders by default) and gate only the details behind verbose. This is a one-line swap.

P2 -- shadow detector: cache TTL not enforced

shadow_detector.py:62 calls fetch_or_cache(source, ...) for every registered marketplace on every install. If a user has 5 registered marketplaces, that's 5 manifest fetches per package install. Cache hits on warm runs, but: (i) the comment says "no extra network round-trips in the common case" -- worth verifying with a test that mocks the fetcher and asserts hit count under repeated installs; (ii) a marketplace whose cache entry was evicted will re-fetch silently, pulling tokens through auth_resolver for every secondary marketplace -- worth measuring on a 10-marketplace setup.

Not a blocker. File a follow-up.

Keepers (no concerns)

  • client.py unauth_first=False fix: correct. Private marketplace repos return 404 (not 403) for unauth requests, and _do_fetch returns None on 404 (no exception), so unauth_first=True would silently swallow the error and never retry with auth. Defensive comment in the diff is exactly right.
  • Validator covers schema + duplicate names. --check-refs deferred with a "Planned" note in docs. Acceptable scope boundary.
  • Shadow detector is fail-open (debug-log on exception, never propagates) -- correct stance for advisory checks.

python-architect

Mostly clean; two cleanup items.

P2 -- resolve_marketplace_plugin returns a permanently-None third element

resolver.py:248 -- the function still returns (canonical, plugin, resolved_version) where resolved_version is set to None and never reassigned. Docstring says "reserved for future use". Three call sites had to be updated to ignore it (commands/install.py:201, commands/view.py:354,461).

This is dead-code-as-API. Two clean options:

  • (a) Remove the third element and update the three call sites (small, contained, correct).
  • (b) Keep it but rename to Optional[ResolutionMetadata] with a real type so future code can attach more than one signal without another N-tuple grow.

Recommend (a) for this PR; (b) when #722 actually attaches data.

P2 -- outdated.py 5-tuple -> 6-tuple ripple

The "Source" column promotion grows the result tuple from 5 to 6 elements across _check_one_dep, _check_marketplace_ref, _check_parallel, _check_parallel_plain, the rendering loop, and the count summary. Six call sites all use positional unpacking. A dataclass(frozen=True) here (OutdatedRow(package, current, latest, status, extra_tags, source)) would (i) name the fields, (ii) prevent the next addition from being a 7-tuple, and (iii) make the test assertions self-documenting. Same pattern we just shipped for TransportPlan/TransportAttempt in #779.

Not a blocker; do it now while the surface is small.

Other notes

  • models.py diff at line 124 is a vestigial empty comment block ("Parse optional versions array (reserved for future use -- currently ignored)"). Either remove or wire it. Suggest remove -- the parser shouldn't comment on what it isn't doing.
  • _display_marketplace_plugin in view.py is a long display function that builds info in two formats (Rich + plain). Same shape as the existing display_package_info. Fine for now; if it grows, factor into a MarketplacePluginRenderer.
  • Module split is healthy: validator.py / shadow_detector.py / version_pins.py are each small, single-responsibility, and testable in isolation. Validator returns structured ValidationResult rather than printing -- the right separation of policy from presentation.

devx-ux-expert (npm/cargo lens)

The semantic shift on # needs a doc nudge; everything else is on-trend.

P1 -- # semantics changed under users without a release-notes call-out

Before: name@mkt#spec was a semver range (in the dropped versions[] design). Now: name@mkt#ref is a raw git ref override. The CHANGELOG entry says "ref" but the original design sketched in the PR body said "semver". Anyone who tracked the early versions of this PR will assume #^2.0.0 works. It silently won't (the regex matches but the resolver treats it as a ref name -- git checkout '^2.0.0' will explode at clone time, not at parse time).

Two-part fix:

  • (a) Validate the ref shape at parse time: reject ^, ~, >=, <, != with a clear "semver ranges are not supported in marketplace refs; use a raw tag, branch, or SHA" error. One regex, points users at the right mental model.
  • (b) Add an explicit "What this is, what it isn't" callout in the marketplaces guide. The current docs show working examples but never call out "this is a raw git ref, not a version constraint". npm users will assume the latter without a counter-statement.

P2 -- apm view NAME@MARKETPLACE shows the entry but not "what would I get if I install this"

The Rich panel renders name / version / source / description. Useful, but missing the install-resolution preview that npm users get from npm view:

Source:      github / acme/code-review-plugin @ v2.1.0
Resolved:    acme/code-review-plugin#v2.1.0
Install:     apm install code-review@acme-plugins

The "Resolved" line bridges the marketplace abstraction to the underlying git source -- which is exactly the trust signal a careful user wants before typing install. One line, no new code paths (already computed in resolve_marketplace_plugin).

P2 -- outdated Source column placement

Current order: Package | Current | Latest | Source | Status. npm-trained eyes scan Status first, then drill into Source if the row is not green. Putting Source after Status matches that scan path:

Package        Current   Latest    Status      Source

Trivial reorder; nicer eye flow.

Keepers

  • apm marketplace validate follows the cargo/npm verb convention. --check-refs listed as "not yet implemented" in both --help and docs is honest; users hate finding out a flag is a no-op.
  • apm install code-review@acme-plugins#v2.0.0 is the right minimal syntax. Doesn't invent a new operator.

cli-logging-ux

One must-fix (security visibility, already covered above), two polish items.

P1 (already raised by security)

warning_handler = lambda msg: logger.verbose_detail(...) in commands/install.py:198 hides security warnings under -v. Use logger.warning(...) so they render with [!] at default verbosity. Diagnostic detail (e.g. "comparing pin acme/code-review v2.0.0 vs v2.1.0") can stay on verbose_detail; the warning itself must not.

P2 -- marketplace.py validate uses click.echo("") for spacing

Lines 425, 446 use raw click.echo() blank lines for visual separation. Fine on its own, but the rest of the file uses logger.progress(...) / logger.success(...) for output. Mixing the two means the validate command can't be silenced consistently with --quiet once that lands. Wrap blanks behind the logger or use console.print().

P2 -- ASCII rule audit clean

Skimmed the new files for the cross-platform encoding rule (no emojis, no em dashes, no unicode arrows). All clean. [+] / [!] / [x] symbols routed through logger.success / logger.warning / logger.error with the proper symbol= keys. Validator output uses bracket notation throughout.

Keepers

  • "Source" column in outdated correctly uses style="dim" so it visually recedes when the row is up-to-date. Status remains the visual anchor.
  • Rich panel for view plugin display has a plain-text fallback (the established pattern). Good.

apm-ceo

Strategic verdict: ship after the P1s land. This unblocks the marketplace cluster.

Why this PR matters strategically

We have an internal coordination problem on the marketplace cluster (#514, #676, #677, #722, #757) that's been blocking ourselves more than blocking external contributors. PR #677 was the most concrete piece of that knot. Landing the slim version unsticks the whole epic. Concretely:

  • MarketplaceValidator is the substrate that [FEATURE] Add apm marketplace generate command to pack all plugins and produce a marketplace.json #722's apm marketplace lint and --check mode will reuse.
  • The auth-first client fix unblocks private/org-scoped marketplace adoption -- which is the enterprise slice of the marketplace story.
  • The view and outdated UX additions close two of the three "I installed a marketplace plugin and now I can't reason about it" papercuts external contributors have flagged.
  • The shadow detector seeds the supply-chain trust narrative we'll lean on when announcing the marketplace authoring suite.

Decision call: ship this scope, don't expand it

I am explicitly not asking Sergio to add per-version pinning, schema-versioning, or --check-refs implementation in this PR. Those belong in #722 or follow-ups. The spec-divergence concern that motivated the original change request is fully resolved by the scope reduction. Two more rounds and we lose Sergio's momentum on the contribution; that's a worse outcome than landing this slim and iterating in subsequent PRs.

What I want before approval

  1. P1 from security: fix the version_pins reframe so it doesn't false-positive on legitimate version bumps. Recommend approach (a) -- pin (mkt, plugin, version) keyed on the standard version field. ~30 LOC change.
  2. P1 from security/cli-ux: route security warnings through logger.warning (not verbose_detail).
  3. P1 from devx: reject semver-range refs at parse_marketplace_ref time with a friendly error pointing at the docs.

If those three land, I sign SHIP. Everything else (3-tuple cleanup, OutdatedRow dataclass, view "Resolved" line, column reorder) goes on a follow-up issue and we batch them.

Coordination with #722

Once this lands, I'll edit the #722 roadmap comment to reflect: validator + shadow detector + ref-pinning are in main, apm marketplace generate / lint / publish (atomic-release) are the remaining authoring-suite work. Sergio's contribution here directly seeds three of those features.

Community optics

External contributor delivered a substantial PR, took thorough scope-reduction feedback constructively, and re-shipped within a working day with a clean diff and clear summary of what changed. This is exactly the contributor experience we want to broadcast. Approving with care here pays back tenfold in inbound contributions.


oss-growth-hacker

This is a marketplace-cluster narrative win. Three growth angles to capture.

Angle 1 -- "the marketplace cluster is now live and external"

The fact that #677's heaviest lifting was done by an external EPAM contributor is the headline. I'll feed this to the growth-strategy.md tracker as a "external contributor at the heart of an epic" data point. When we eventually announce the marketplace authoring suite (post-#722), the launch-week thread should explicitly credit #677 as the foundation.

Angle 2 -- private marketplace support is the enterprise wedge

The unauth_first=False fix is small but unlocks a concrete persona we don't talk about enough: internal team curating private marketplace for their org. This is the killer feature for inbound enterprise interest -- "use your existing private GitHub org as your skill marketplace". When #722 lands the authoring suite, this PR's auth-first fix is what makes the demo work end-to-end. Worth a short blog post once both ship: "Stand up an internal AI skill marketplace in 30 minutes using only GitHub repos you already have."

Angle 3 -- supply-chain trust is the marketing differentiator

Shadow detection + ref immutability advisories (once the false-positive issue is fixed) are exactly the language enterprise-security-conscious teams scan for. No competing CLI in this space ships these checks today. Pair this with the per-file content-hash work coming next on the roadmap, and APM has a defensible "supply-chain-aware AI package manager" position that npm/pip arguably don't hold. Worth a security model deep-dive update once the version_pins fix is in -- referenced from the marketplace guide so users discover it on the path of natural learning.

I'd hold the public storytelling until the P1 fixes land (a false-positive blast radius story would burn the trust narrative we're trying to build), then go big on launch.


Final routing through the CEO

Every panelist independently lands on conditional approval, three P1s to fix, P2s deferred. No disagreement to escalate. The fixes are concrete, contained (~50 LOC total), and don't expand scope -- they restore the security guarantees the original versions[] design provided, now on the standard schema.

Action items

# Owner Item Fix size
1 sergio-sisternes-epam version_pins.py -- key on (marketplace, plugin, version) using the standard version field; only warn when same version resolves to a different ref ~30 LOC + test
2 sergio-sisternes-epam commands/install.py:198 -- route security warnings through logger.warning (not verbose_detail) ~3 LOC
3 sergio-sisternes-epam marketplace/resolver.py parse_marketplace_ref -- reject semver-range characters with a friendly error linking to docs ~5 LOC + test

Deferred to follow-up issues:

  • 3-tuple -> 2-tuple cleanup of resolve_marketplace_plugin (drop the always-None element)
  • OutdatedRow dataclass to replace the 6-tuple
  • view NAME@MKT "Resolved" line + Source/Status column reorder
  • Shadow detector cache-hit measurement under N-marketplace setups
  • Validator --check-refs implementation (already documented as planned)

Verdict

Conditional ship. Three concrete P1 fixes, then approve. Sergio has earned the trust to land this iteration with a final review pass rather than another cycle of changes-requested.

@danielmeppiel
Copy link
Copy Markdown
Collaborator

CEO addendum -- promoting the P2s to required

Reconsidered after panel deliberation. Sergio has demonstrated throughput on this PR (1,800-LOC scope reduction landed in a working day, clean diff, accurate PR description). Trust isn't the bottleneck -- iteration cost is. Given the throughput, batching the cleanup now is cheaper for everyone than carrying it as follow-up debt across the marketplace cluster (#722, #757, #676 all touch this surface).

Promoting the following P2s to required for this PR:

# Item Why now
4 resolve_marketplace_plugin -- drop the always-None 3rd return; update the 3 call sites (install.py:201, view.py:354, view.py:461) Dead-code-as-API ages badly; #722 will touch this signature again and inheriting a vestigial slot forces awkward versioning
5 OutdatedRow dataclass(frozen=True) to replace the 6-tuple in outdated.py; same pattern as TransportPlan/TransportAttempt from #779 Six positional unpack sites today; the next column addition becomes a 7-tuple migration. Lock the shape now while the surface is small
6 view NAME@MKT -- add the "Resolved" line (acme/code-review-plugin#v2.1.0) Already computed by resolve_marketplace_plugin; one line render, completes the trust-signal story the panel called out
7 outdated column reorder: `Package Current
8 Drop the vestigial empty comment block in models.py:124 ("Parse optional versions array (reserved for future use -- currently ignored)") Parser shouldn't comment on what it isn't doing; cleanup leftover from versions[] removal

Explicitly still deferred to follow-up issues (genuine new work, not cleanup):

Updated final action list

P1 (security/correctness):

  1. Re-key version_pins.py on (marketplace, plugin, version) using the standard version field
  2. Route security warnings through logger.warning (not verbose_detail)
  3. Reject semver-range characters in parse_marketplace_ref with a friendly error

P2 (now required, batched while contributor is hot):
4. Drop always-None 3rd return from resolve_marketplace_plugin
5. OutdatedRow dataclass replacing the 6-tuple
6. "Resolved" line in view NAME@MKT panel
7. outdated column reorder (Status before Source)
8. Remove vestigial comment in models.py:124

Total estimated diff: ~80 LOC + a couple of test updates. Well within a single-pass iteration for someone with Sergio's throughput on this surface. Once landed, this PR earns a clean approval and we close the marketplace-foundations chapter cleanly.

Sergio -- thank you for the velocity on the scope reduction. The expanded ask isn't a trust signal, it's the inverse: you're moving fast enough that batching the cleanup now is the higher-leverage path for both you and the reviewers.

@sergio-sisternes-epam sergio-sisternes-epam force-pushed the 514-marketplace-versioning branch from ebe5196 to 4eb1df8 Compare April 20, 2026 13:28
@sergio-sisternes-epam
Copy link
Copy Markdown
Collaborator Author

Panel Feedback Addressed

All 8 items from the APM Review Panel (3 P1s + 5 promoted P2s) have been implemented. Summary:

P1 -- Security & Correctness

# Item Resolution
P1-1 Re-key version_pins on (mkt, plugin, version) _pin_key() now builds marketplace/plugin/version triple. check_ref_pin() and record_ref_pin() accept a version parameter. Legitimate version bumps create separate pin entries -- no more false-positive ref-swap warnings.
P1-2 Security warnings invisible at default verbosity warning_handler in install.py now routes through logger.warning instead of logger.verbose_detail. Ref-swap and shadow-detection warnings are visible without --verbose.
P1-3 Reject semver-range chars in parse_marketplace_ref parse_marketplace_ref() now raises ValueError when the # suffix contains ~, ^, >=, <, or !=. Error message points users at the docs page. Raw git refs (tags, branches, SHAs) remain valid.

P2 -- Promoted Cleanup

# Item Resolution
P2-4 Drop always-None 3rd return element resolve_marketplace_plugin() now returns Tuple[str, MarketplacePlugin]. All 4 call sites updated (install.py, view.py, shadow_detector test).
P2-5 OutdatedRow frozen dataclass Replaced all 6-tuple returns with @dataclass(frozen=True) OutdatedRow (fields: package, current, latest, status, extra_tags, source). All render loops and error fallbacks updated.
P2-6 "Resolved" line in view NAME@MKT panel _display_marketplace_plugin() now calls resolve_marketplace_plugin() and shows a "Resolved: acme/plugin#v2.1.0" line in both Rich panel and plain-text fallback.
P2-7 Reorder outdated columns (Status before Source) Both Rich table and plain-text fallback now render: Package
P2-8 Remove vestigial comment in models.py Removed the "Parse optional versions array / reserved for future use" comment from _parse_plugin_entry().

Test coverage

  • Added version-scoped pin isolation test (test_version_scoped_pins_do_not_conflict)
  • Updated semver parse tests to assert pytest.raises(ValueError)
  • Updated all 3-tuple unpacks to 2-tuple across test files
  • Updated outdated test assertions to use OutdatedRow named fields
  • 4112 tests passing

- Marketplace-aware view and outdated commands (standard spec)
- Shadow detector for cross-marketplace supply-chain advisories
- Ref immutability pins (version_pins.py)
- MarketplaceValidator (schema + duplicate names)
- Auth-first client fix for private repos
- Lockfile provenance (discovered_via, marketplace_plugin_name)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@sergio-sisternes-epam sergio-sisternes-epam force-pushed the 514-marketplace-versioning branch from 4eb1df8 to 9041eb1 Compare April 20, 2026 13:31
@sergio-sisternes-epam sergio-sisternes-epam added this pull request to the merge queue Apr 20, 2026
Merged via the queue into main with commit 891be0e Apr 20, 2026
21 checks passed
@sergio-sisternes-epam sergio-sisternes-epam deleted the 514-marketplace-versioning branch April 20, 2026 14:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

marketplace Marketplace federation: marketplace.json publishing, sourcing, and federation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants