Skip to content

feat(marketplace): add experimental authoring CLI commands (#722)#790

Merged
sergio-sisternes-epam merged 23 commits intomainfrom
feat/722-marketplace-maintainer-ux
Apr 27, 2026
Merged

feat(marketplace): add experimental authoring CLI commands (#722)#790
sergio-sisternes-epam merged 23 commits intomainfrom
feat/722-marketplace-maintainer-ux

Conversation

@sergio-sisternes-epam
Copy link
Copy Markdown
Collaborator

Description

First pass of the maintainer-side marketplace tooling tracked in #722. Builds on the foundations landing in #677 (semver engine, MarketplaceValidator skeleton, security advisories, publish skeleton).

Design context and the full UX proposal are captured in the discussion thread on the tracking issue: #722 (comment).

Hard rule driving this work

marketplace.json is Anthropic's standard, unaltered. APM emits the artifact byte-for-byte against Anthropic's schema; APM-only build inputs live in marketplace.yml and are stripped at compile time. A golden-file test enforces round-trip compatibility with Claude Code.

High-level scope

  • apm marketplace init — scaffold marketplace.yml
  • apm marketplace build — compile marketplace.yml -> Anthropic-compliant marketplace.json (concurrent ref resolution, diff output, dry-run)
  • apm marketplace check — validation + freshness for CI (--strict, per-rule toggles)
  • apm marketplace outdated — maintainer-side discovery of stale ranges
  • apm marketplace publish — transactional add/update of a package entry (PR-first, --resume/--abort recovery)
  • apm marketplace doctor — preflight probe for git / gh / tokens / ls-remote

Detailed plan, UX previews, and the 45-item UX-risk absorption map live in the session plan and will be distilled into a design doc / changelog entry during delivery.

Fixes #722

Type of change

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

Testing

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

Draft — work in progress. Please hold review until marked ready.

Comment thread tests/unit/marketplace/test_pr_integration.py Dismissed
sergio-sisternes-epam pushed a commit to sergio-sisternes-epam/apm that referenced this pull request Apr 20, 2026
Wrap the state-file path in rich.text.Text(..., no_wrap=True) so it
renders on a single line and does not break the publish-state.json
substring that CI tests rely on when the terminal width is 80 cols.

(microsoft#790)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@sergio-sisternes-epam sergio-sisternes-epam force-pushed the feat/722-marketplace-maintainer-ux branch 3 times, most recently from 72d1b7b to 56c20fe Compare April 21, 2026 10:21
@sergio-sisternes-epam
Copy link
Copy Markdown
Collaborator Author

APM Expert Review Panel -- PR #790

Reviewers: Python Architect, CLI Logging Expert, DevX UX Expert, Supply Chain Security Expert, APM CEO, OSS Growth Hacker

Panel Verdict: Approve with conditions

This is APM's most significant feature since apm install. The marketplace authoring toolkit transforms APM from a consumer of curated registries into the toolchain that powers registry creation itself -- a capability no competitor (npm, cargo, homebrew) offers. The architecture is sound, the test coverage is excellent (1.5:1 test-to-code ratio, 4794 tests), and the feature is entirely additive with zero breaking changes.

Two conditions must be met before merge. Seven high-priority findings should be tracked for fast-follow.


Conditions for Merge

# Condition Owner Rationale
C1 File a follow-up issue to extract commands/marketplace.py (1,835 LOC) into per-command modules under commands/marketplace/ Architect / CEO God-files kill review velocity; this is APM's most complex command group
C2 Wire the authoring guide into the docs sidebar (astro.config.mjs) and add consumer-to-author cross-links Growth Without this, the feature ships invisible to 90%+ of docs visitors

Findings by Severity

Critical (4 findings)

# Area Finding Reviewer
S1 Security output field in marketplace.yml has no path traversal guard -- ../../.bashrc would be written outside the repo Security
L1 Logging publish state-file line crashes without Rich fallback (bare from rich.text import Text with no try/except) Logging
L2 Logging _helpers._get_console() never returns None -- all colorama fallback branches across 9 render functions are dead code Logging
UX1 UX validate and check are two confusingly overlapping validation commands with no clear differentiation DevX

High / Important (14 findings)

# Area Finding Reviewer
S2 Security Token redaction regex misses http:// URLs and ?token= query params; copy-pasted in 3 files Security
S3 Security git ls-remote does not set GIT_TERMINAL_PROMPT=0 -- can hang on interactive credential prompt Security
S4 Security ConsumerTarget.repo and branch are not validated against injection patterns Security
A1 Architecture 3 identical _atomic_write() implementations across builder, yml_editor, publisher Architect
A2 Architecture 3 identical _redact_token() / _TOKEN_RE copies across ref_resolver, publisher, pr_integration Architect
A3 Architecture Builder.resolve() uses hidden state smuggling (self._resolve_errors) instead of returning a result object Architect
A4 Architecture Cross-module private import: yml_editor imports _SOURCE_RE from yml_schema Architect
A5 Architecture check command reimplements builder's ref-matching logic Architect
L3 Logging Stack traces swallowed in verbose mode -- all except Exception handlers print one-line error regardless of --verbose Logging
L4 Logging doctor hardcodes GITHUB_TOKEN/GH_TOKEN env var names instead of using AuthResolver Logging
UX2 UX Three different confirmation patterns across destructive commands; two hang in CI without --yes DevX
UX3 UX Docs list --marketplace-yml PATH option that doesn't exist in code DevX
UX4 UX plugin add doesn't enforce --version/--ref mutual exclusivity at CLI level DevX
UX5 UX outdated has no summary line on default path and no CI-friendly exit code DevX

Notes (11 findings)

# Area Finding Reviewer
S5 Security version_pins.py fail-open design: deleted pin file silently disables ref-swap detection Security
A6 Architecture __init__.py eagerly imports all 40+ symbols, defeating lazy loading Architect
A7 Architecture locals().get("pr") anti-pattern in publish command Architect
L5 Logging 18 bare click.echo() calls bypass logging infrastructure Logging
L6 Logging doctor doesn't use DiagnosticCollector (hand-rolls its own) Logging
L7 Logging Rich markup [dim]...[/dim] used directly in console.print() (markup injection precedent) Logging
UX6 UX Bare except Exception swallows actionable context in consumer commands DevX
UX7 UX publish confirmation uses hand-rolled prompt instead of click.confirm DevX
UX8 UX doctor doesn't check gh CLI presence despite docs claiming it does DevX
UX9 UX plugin subgroup missing list subcommand (incomplete CRUD) DevX
G1 Growth No consumer-to-author bridge in docs (consumer guide, first-package, README) Growth

What's Done Well

The panel unanimously highlighted these strengths:

Area Assessment
Error hierarchy Clean MarketplaceError tree with actionable messages and suggested commands
Frozen dataclasses Consistent frozen=True discipline across all domain models -- thread-safe by default
Security posture yaml.safe_load everywhere, zero shell=True, token redaction on all error paths, path traversal guards on source/subdir fields
Injectable dependencies PrIntegrator(runner=), Publisher(clock=, runner=), RefResolver -- composition over inheritance
Builder pipeline Clean load -> resolve -> compose -> write with concurrent resolution and per-remote locks
Semver implementation Dependency-free, correct prerelease ordering, AND-composition of ranges
Test coverage 1.5:1 test-to-code ratio; every module has dedicated unit + integration tests
Authoring guide Quickstart in 4 commands, complete schema reference, troubleshooting table, recipes section
Scaffolding UX init generates richly-commented YAML with inline docs, working examples, and next-steps panel

Strategic Assessment (CEO)

Positioning: This is APM's moat-defining feature. No competitor offers self-hosted marketplace authoring with semver resolution and cross-repo PR-driven publish. This is the npm publish moment for AI agent registries.

Naming: marketplace namespace and plugin subgroup ratified. Advisory: add one sentence bridging packages: (yml) vs plugin (CLI) naming in the authoring guide.

Dependency: ruamel.yaml addition justified -- no alternative for comment-preserving YAML editing. Correctly lazy-imported (only loaded on plugin add/set/remove).

Release: This merits a dedicated v0.9.0 minor release, not a patch. Story angle: "Build your own AI plugin registry in 4 commands."


Priority Fix Order

Priority Findings Effort
P0 -- before merge S1 (output path traversal), C1 (file split issue), C2 (sidebar + cross-links) Small-medium
P1 -- before merge S3 (GIT_TERMINAL_PROMPT), A7 (locals().get), L1 (Rich crash) Small
P2 -- fast follow S2 (token regex), S4 (target validation), A1-A2 (DRY), L3 (verbose tracebacks) Medium
P3 -- next iteration A3 (state smuggling), A5 (check duplication), UX1 (validate vs check), UX9 (plugin list) Medium-large
Backlog All notes-severity findings Small each

Panel composition and routing

Six specialist agents reviewed independently, with findings consolidated by the orchestrator:

  • Python Architect: Module structure, design patterns, coupling, extensibility (11 findings)
  • CLI Logging Expert: CommandLogger usage, Rich fallbacks, verbose mode, output structure (12 findings)
  • DevX UX Expert: Command naming, help text, flags, interactive/CI patterns (10 findings)
  • Supply Chain Security Expert: Token handling, path safety, YAML safety, injection vectors (8 findings)
  • APM CEO: Positioning, naming, scope, dependency, release strategy (7 strategic assessments)
  • OSS Growth Hacker: Discoverability, story angles, contributor funnel, README (10 findings)

Per panel protocol, specialists raised findings independently. The CEO arbitrated strategic calls (naming ratification, dependency approval, release framing). The Growth Hacker annotated discoverability gaps and escalated the sidebar registration as critical.

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

APM Expert Review Panel -- Round 2 (Fix Commit Validation)

Scope: 3 fix commits (7169bac, e0eb078, 092c6e4), 26 files, +620/-100
Review of: Fixes for all 16 findings from Round 1 (P0/P1/P2) + Round 2 action items

Panel Verdict: APPROVE (6/6 specialists)

Specialist Verdict Summary
Python Architect APPROVE DRY consolidation clean, aliasing pragmatic, no new anti-patterns
CLI Logging Expert APPROVE All logging fixes correct, verbose tracebacks well-placed
DevX UX Expert APPROVE UX fixes clean, exit-code convention correct
Supply Chain Security APPROVE WITH NOTES S1/S2/S4 solid; S3 gap in publisher _run_git() fixed in 092c6e4
OSS Growth Hacker APPROVE WITH NOTES Sidebar + cross-links correct, recommends exit-code docs
APM CEO APPROVE Both merge conditions satisfied. PR cleared for merge.

Merge Conditions -- Verified


Security Fixes -- Validated

ID Fix Security Review
S1 Output path traversal -- defence-in-depth (parse + resolve) PASS -- both layers use sanctioned path_security.py guards
S2 Token regex consolidated -- covers https://, http://, ?token= PASS -- 10 test vectors verified
S3 GIT_TERMINAL_PROMPT=0 on all git subprocesses PASS -- ref_resolver + publisher _run_git() both hardened
S4 ConsumerTarget injection -- _SAFE_REPO_RE + _SHELL_META_RE PASS -- 16 bypass vectors tested, shell=True never used

Architecture Fixes -- Validated

ID Fix Architecture Review
A1 3x _atomic_write consolidated to _io.py PASS -- laser-focused module, __all__ declared
A2 3x _redact_token consolidated to _git_utils.py PASS -- alias pattern pragmatic, zero call-site churn
A4 _SOURCE_RE made public as SOURCE_RE PASS
A7 locals().get("pr") replaced with explicit variable PASS

Logging + UX Fixes -- Validated

ID Fix Review
L1 Rich Text crash fallback PASS -- click.echo fallback correct
L3 Verbose tracebacks in 5 error handlers PASS -- well-placed, SystemExit re-raised
UX3 Phantom --marketplace-yml removed from docs PASS
UX4 --version/--ref mutual exclusivity (plugin add + plugin set) PASS
UX5 outdated summary line + exit code 1 PASS -- matches npm/pip convention
C2 Sidebar wiring for authoring guide PASS
G1 Consumer-to-author cross-link PASS

New Findings from Round 2 (Non-Blocking)

ID Severity Finding Status
N1 Low builder._atomic_write is now a pure passthrough -- can inline Deferred
N2 Low Stale _TOKEN_RE docstring references Fixed (092c6e4)
N3 Low Redundant (BuildError, Exception) catch tuple in outdated Deferred
N4 Low Error rows uncounted in outdated summary Deferred
NEW-1 Medium plugin set needs --version/--ref mutual exclusivity Fixed (092c6e4)

Post-Merge Recommendations

  1. Tag merge commit for v0.9.0 release notes
  2. CHANGELOG lead: "Build your own AI plugin registry in 4 commands"
  3. Monitor Issue refactor: split commands/marketplace.py into per-command modules #821 for community pickup (contributor funnel test)
  4. Track version-pins fail-open (S5) as known-accepted risk with [security] label

Test Coverage

4825 tests passing (+31 new tests from fix commits), 0 failures.

Every security fix has corresponding test coverage:

  • Path traversal: 4 tests (parse + resolve layers)
  • Token redaction: 7 tests (including mixed patterns)
  • Credential prompt suppression: 4 tests (ref_resolver + publisher)
  • ConsumerTarget injection: 5 tests (repo + branch vectors)
  • Mutual exclusivity: 2 tests (plugin add + plugin set)

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

Introduces first-pass maintainer-side apm marketplace authoring tooling, centered around marketplace.yml as the source-of-truth and deterministic compilation to Anthropic-compliant marketplace.json, with supporting CLI commands, ref resolution, publishing orchestration, and expanded test/docs coverage.

Changes:

  • Add marketplace authoring/editing primitives (round-trip YAML editor, semver + tag pattern helpers, git stderr translation, ref resolver cache).
  • Extend CLI with marketplace authoring flows (init, build, check, outdated, doctor, publish) and a marketplace plugin {add,set,remove} subgroup.
  • Add comprehensive unit/integration/live-e2e test scaffolding plus docs and changelog updates; add ruamel.yaml dependency.

Reviewed changes

Copilot reviewed 55 out of 57 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
pyproject.toml Adds ruamel.yaml dependency for round-trip YAML editing.
uv.lock Locks ruamel-yaml artifact to support new YAML editor functionality.
src/apm_cli/marketplace/_io.py Adds atomic write helper used by marketplace editing/build flows.
src/apm_cli/marketplace/_git_utils.py Adds token redaction helper for git/gh stderr strings.
src/apm_cli/marketplace/git_stderr.py Adds stderr classification into actionable, ASCII-only summaries/hints.
src/apm_cli/marketplace/ref_resolver.py Adds cached git ls-remote ref discovery and SHA resolution.
src/apm_cli/marketplace/semver.py Adds semver parsing and range evaluation used for tag selection.
src/apm_cli/marketplace/tag_pattern.py Adds tag rendering + regex compilation for extracting versions from tags.
src/apm_cli/marketplace/yml_editor.py Adds round-trip YAML editor for manipulating marketplace.yml entries.
src/apm_cli/marketplace/pr_integration.py Adds gh-based PR open/update integration for publish workflow.
src/apm_cli/marketplace/init_template.py Adds scaffold template used by apm marketplace init.
src/apm_cli/marketplace/errors.py Adds marketplace build/YML/ref-resolution error types.
src/apm_cli/marketplace/__init__.py Re-exports new marketplace authoring/public API symbols.
src/apm_cli/commands/_helpers.py Adds shared TTY detection helper used by marketplace publish confirmation.
src/apm_cli/commands/marketplace_plugin.py Adds CLI subgroup to add/set/remove marketplace.yml entries.
tests/unit/marketplace/* Adds unit tests for new marketplace primitives and helpers.
tests/unit/commands/test_marketplace_* Adds unit tests for new marketplace CLI commands.
tests/integration/marketplace/* Adds integration + env-var-gated live e2e tests and shared fixtures/docs.
tests/fixtures/marketplace/golden.json Adds golden fixture for Anthropic schema compatibility checks.
docs/src/content/docs/reference/cli-commands.md Documents new marketplace authoring CLI commands.
docs/src/content/docs/guides/marketplaces.md Adds authoring-related guidance (monorepo/subdir + ref auto-resolution).
docs/astro.config.mjs Adds Marketplace Authoring guide to docs nav.
packages/apm-guide/.apm/skills/apm-usage/commands.md Updates skills command reference for marketplace authoring tooling.
packages/apm-guide/.apm/skills/apm-usage/package-authoring.md Adds marketplace authoring section for skill consumers.
CHANGELOG.md Adds Unreleased entries for marketplace authoring tooling.

Comment thread src/apm_cli/marketplace/_git_utils.py Outdated
@sergio-sisternes-epam sergio-sisternes-epam force-pushed the feat/722-marketplace-maintainer-ux branch from 7e05174 to 8ac5007 Compare April 24, 2026 12:25
@sergio-sisternes-epam sergio-sisternes-epam changed the title feat: marketplace maintainer UX (#722) feat(marketplace): add experimental authoring CLI commands (#722) Apr 24, 2026
@danielmeppiel danielmeppiel added panel-review Trigger the apm-review-panel gh-aw workflow and removed panel-review Trigger the apm-review-panel gh-aw workflow labels Apr 24, 2026
@github-actions
Copy link
Copy Markdown

APM Review Panel Verdict

Disposition: REQUEST_CHANGES (one P0 merge-conflict blocker; one P1 auth pattern; two minor clean-ups)


Per-persona findings

Python Architect: This is a major architectural addition: a new src/apm_cli/marketplace/ subpackage (13 modules) plus a 2 006-line command group. The module decomposition is sound: pure modules (semver.py, tag_pattern.py, git_stderr.py, _git_utils.py) are separated from I/O modules (builder.py, publisher.py, ref_resolver.py), and frozen dataclasses flow cleanly between layers.

classDiagram
    direction LR
    class MarketplaceBuilder {
        <<Builder>>
        +build() BuildReport
        -_resolve_entry(entry) ResolvedPackage
        -_prefetch_metadata(resolved) dict
        -_resolve_github_token() str
    }
    class MarketplacePublisher {
        <<Orchestrator>>
        +plan(targets) PublishPlan
        +execute(plan) list
        -_process_single_target(target, plan) TargetResult
    }
    class RefResolver {
        <<Cache>>
        +list_remote_refs(owner_repo) list
        +resolve_ref_sha(owner_repo, ref) str
        -_cache RefCache
        -_remote_locks dict
    }
    class MarketplaceYml {
        <<ValueObject>>
        +name str
        +version str
        +packages list
    }
    class ResolvedPackage {
        <<ValueObject, frozen>>
        +name str
        +ref str
        +sha str
    }
    class BuildReport {
        <<ValueObject, frozen>>
        +resolved tuple
        +errors tuple
        +unchanged_count int
    }
    class PublishPlan {
        <<ValueObject, frozen>>
        +branch_name str
        +commit_message str
        +new_ref str
    }
    class PublishState {
        <<TransactionalState>>
        +begin_run(plan)
        +record_result(result)
        +finalise(dt)
    }
    class PrIntegrator {
        <<Adapter>>
        +open_or_update(plan, target) PrResult
    }
    MarketplaceBuilder *-- RefResolver : delegates git queries
    MarketplaceBuilder ..> MarketplaceYml : reads
    MarketplaceBuilder ..> ResolvedPackage : produces
    MarketplaceBuilder ..> BuildReport : returns
    MarketplacePublisher *-- PublishState : writes atomically
    MarketplacePublisher ..> PublishPlan : executes
    MarketplacePublisher ..> TargetResult : produces
    PrIntegrator ..> PublishPlan : reads
    note for RefResolver "Per-remote threading.Lock\nfor parallel build safety"
    note for PublishState "Atomic write: tmp+fsync+os.replace"
    class MarketplaceBuilder:::touched
    class MarketplacePublisher:::touched
    class RefResolver:::touched
    class PublishState:::touched
    class PrIntegrator:::touched
    classDef touched fill:#fff3b0,stroke:#d47600
Loading
flowchart TD
    A([apm marketplace build]) --> B[_load_yml_or_exit\nmarketplace.py]
    B --> C{marketplace.yml valid?}
    C -- No --> D([sys.exit 2])
    C -- Yes --> E[MarketplaceBuilder.build\nbuilder.py]
    E --> F[NET: RefResolver.list_remote_refs\nper-remote lock + 5-min TTL cache]
    F --> G[EXEC: git ls-remote https://github.com/owner/repo.git\nGIT_TERMINAL_PROMPT=0]
    G --> H{returncode 0?}
    H -- No --> I[GitLsRemoteError\ntranslate_git_stderr]
    H -- Yes --> J[resolve version range / explicit ref]
    J --> K[NET: _fetch_remote_metadata\nraw.githubusercontent.com with AuthResolver token]
    K --> L[FS: atomic_write marketplace.json\ntmp + fsync + os.replace]
    L --> M[BuildReport rendered\n_render_build_report marketplace.py]
    M --> N([sys.exit 0 or 1])

    A2([apm marketplace publish]) --> B2[_load_targets_file\nvalidate_path_segments + _SAFE_REPO_RE]
    B2 --> C2[MarketplacePublisher.plan\nvalidate repo/branch/path]
    C2 --> D2[MarketplacePublisher.execute\nThreadPoolExecutor parallel=4]
    D2 --> E2[NET/FS: git clone --depth=1 https://github.com/repo.git\nGIT_ASKPASS=echo]
    E2 --> F2[FS: ensure_path_within apm_yml_path clone_dir]
    F2 --> G2[FS: yml_editor update version ref]
    G2 --> H2[EXEC: git commit + push branch]
    H2 --> I2[PrIntegrator.open_or_update\ngh pr create/edit]
    I2 --> J2[PublishState.finalise\natomic write .apm/publish-state.json]
    J2 --> K2([sys.exit 0 or 1])
Loading

Design patterns

  • Used in this PR: Dataclass-as-value-object (frozen) -- ResolvedPackage, BuildReport, PublishPlan, TargetResult, RemoteRef carry data safely across thread boundaries; Builder -- MarketplaceBuilder with lazy loaders and injectable BuildOptions; Collect-then-render -- BuildReport / TargetResult collected during execution, rendered only at the CLI layer; Per-resource locking -- RefResolver._remote_locks (dict of threading.Lock keyed by owner_repo) for safe parallelism.
  • Pragmatic suggestion: Extract _render_* helpers from marketplace.py (2 006 LOC) into src/apm_cli/commands/_marketplace_render.py. The rendering functions (_render_publish_plan, _render_publish_summary, _render_build_report, _render_doctor_table) are already stateless and would reduce marketplace.py by ~600 lines without any architectural change.

Two minor concerns: (1) self._github_token is set as a mutable instance attribute before spawning worker threads (builder.py line 483). Currently safe because mutation precedes pool creation, but fragile if the builder is ever reused. Passing the token as a closure captures the value immutably. (2) _warn_duplicate_names and _find_duplicate_names (marketplace.py lines 114-143) perform the same traversal for different callers -- minor DRY gap.


CLI Logging Expert: All seven new commands wire CommandLogger("marketplace-X", verbose=verbose) correctly. Symbol usage (symbol="check", symbol="warning", symbol="error") is consistent with the STATUS_SYMBOLS dict. Rich fallback pattern (except (ImportError, NameError)) is present in every rendering helper. The doctor table (_render_doctor_table) has clean fallback to logger.tree_item with correct [+]/[x]/[i] symbols.

One pattern deviation: _require_authoring_flag() calls _rich_warning() and _rich_info() directly (marketplace.py lines 152-166) rather than delegating to a CommandLogger. This is defensible as a pre-command guard, but means these messages are invisible to structured log capture. Low severity.


DevX UX Expert: The MarketplaceGroup.format_commands() split into "Consumer commands" and "Authoring commands" sections is excellent progressive disclosure -- exactly the right pattern for a feature-flagged surface. The _require_authoring_flag() exit path gives a clear three-line enable recipe plus a docs URL. --dry-run on both build and publish follows npm/cargo conventions. --allow-head and --allow-downgrade are appropriately opt-in-dangerous with self-documenting names. The package add/set/remove subgroup mirrors npm pkg set and cargo add mental models well.

One concern: publish exits with "Non-interactive session: pass --yes to confirm" but does not mention that --dry-run is available for inspection first. Adding "Use --dry-run to preview first." to that error message would improve the non-interactive UX.


Supply Chain Security Expert: The implementation defends well against the relevant threat categories.

  • Shell injection: All git subprocess calls use list form -- no shell=True anywhere. Safe.
  • Path traversal: validate_path_segments is applied to source, subdir, output, and path_in_repo at parse time; ensure_path_within applied at write time. Both chokepoints active.
  • URL construction: Publisher and RefResolver always hardcode https://github.com/ as the host; _SAFE_REPO_RE = re.compile(r"^[a-zA-Z0-9._-]+/[a-zA-Z0-9._-]+$") in publisher and SOURCE_RE = re.compile(r"^[^/]+/[^/]+$") + validate_path_segments in yml_schema jointly prevent injection into the URL authority component.
  • Token redaction: _git_utils.redact_token is applied to all exc.stderr in error paths. _TOKEN_RE covers (token/redacted)@ URLs and ?token= query params.
  • Integrity: marketplace.json is never regenerated by the publisher -- it is copied byte-for-byte from the source repo. SHA pinning happens at build time.
  • Atomic writes: _io.atomic_write (tmp + fsync + os.replace) is used for marketplace.json and publish-state.json.

One defensive gap (low severity): RefResolver.list_remote_refs(owner_repo) does not validate the owner_repo format at the API boundary. It relies on callers to pass values that already passed _validate_source(). If new call sites are added outside the yml_schema path (e.g., in marketplace_plugin.py line 65-67 where resolver.list_remote_refs(source) is called), they must separately validate input. Adding a guard inside list_remote_refs would be belt-and-suspenders.


Auth Expert: ACTIVATED -- builder.py::_resolve_github_token() introduces a new AuthResolver.resolve("github.com") call site; publisher.py introduces new git clone + push operations.

Finding 1 (P1): self._github_token stored as a mutable instance attribute on MarketplaceBuilder (builder.py line 483) violates the Auth Expert rule "never use self.github_token instance vars -- resolve per-dep". The correct pattern is to pass the token as a closed-over local to _fetch_remote_metadata:

# builder.py _prefetch_metadata -- safer pattern
github_token = self._resolve_github_token()
...
pool.submit(self._fetch_remote_metadata, pkg, github_token)

...and update _fetch_remote_metadata(self, pkg, github_token=None) to accept it as a parameter. This eliminates the mutable shared state and makes the threading invariant explicit.

Finding 2 (P2): The publisher clones consumer repos via plain https://github.com/{repo}.git with GIT_TERMINAL_PROMPT=0 and GIT_ASKPASS=echo -- bypassing AuthResolver entirely. This means apm marketplace publish silently fails with an auth error if the user has not run gh auth setup-git or configured a git credential helper. The docs (docs/src/content/docs/guides/marketplace-authoring.md) should add a prerequisite section: "You must have write access to consumer repos configured via gh auth login && gh auth setup-git".

Finding 3 (P2): auth_resolver: Optional[object] in MarketplaceBuilder.__init__ should be Optional["AuthResolver"] for accurate typing.


OSS Growth Hacker: This is the "homebrew tap" moment for APM -- the feature that enables the ecosystem to self-organize around custom marketplaces rather than depending on a central registry. The init -> build -> publish flow is the npm publish loop users already know. The Anthropic-schema compatibility claim ("output conforms byte-for-byte to Anthropic's schema") is a strong positioning statement worth amplifying in the release notes.

The experimental gate is the right call here: it creates a natural cohort of power users who can provide structured feedback before GA. The CHANGELOG entry correctly marks all commands as [Experimental].

Side-channel to CEO: This feature directly enables the "private marketplace" use case Lorenzo Storelli's AI Controls prototype was built around (github/agents/discussions/637). Ship fast and mention it in the release announcement as "Private marketplaces, first-class tooling." Update WIP/growth-strategy.md to reflect that the marketplace-authoring toolchain is now the primary conversion surface for enterprise teams who want to run their own APM-compatible registries.


CEO arbitration

Specialists agree on the overall direction: this is a well-structured, security-conscious addition that fills a real capability gap. The one genuine blocker is the unresolved merge conflict in CHANGELOG.md (lines 36-43 contain live <<<<<<< feat/722-marketplace-maintainer-ux / ======= / >>>>>>> main markers) -- the branch cannot merge in this state and it must be resolved to include both the existing Fixed entries from main AND the two [Experimental] fixes from this PR. The Auth Expert's P1 finding on self._github_token is a real pattern violation that should be fixed pre-merge for correctness and future-safety. The remaining findings (doc gap for publisher auth prereq, loose type annotation, marketplace.py size, list = builtins.list code smell) are deferred to follow-up issues -- none are bugs or security regressions. Merge after the two required actions below.


Required actions before merge

  1. [P0 -- BLOCKING] Resolve the merge conflict in CHANGELOG.md lines 36-43. The conflict must keep BOTH the two [Experimental] Fixed entries from this PR AND the three Fixed entries from main (apm install user scope, audit blindness, packer leak). Do not discard either side.

  2. [P1] Refactor MarketplaceBuilder._prefetch_metadata (builder.py lines 482-499) to pass the resolved token as a parameter to _fetch_remote_metadata rather than storing it on self._github_token. Update _fetch_remote_metadata signature to accept an optional github_token: Optional[str] = None argument. This eliminates mutable shared state on the instance and makes the threading invariant explicit. Also update auth_resolver: Optional[object] -> Optional["AuthResolver"] in __init__.


Optional follow-ups

  • Add RefResolver.list_remote_refs entry-point validation: assert owner_repo matches SOURCE_RE before constructing the URL. Protects future call sites added outside the yml_schema validation chain.
  • Split src/apm_cli/commands/marketplace.py (2 006 LOC) into command handlers + _marketplace_render.py for the rendering helpers. The six _render_* functions are already stateless and self-contained.
  • Add a prerequisites note to docs/src/content/docs/guides/marketplace-authoring.md stating that apm marketplace publish requires gh auth login && gh auth setup-git (or a git credential helper with write access to consumer repos). Without this, the first push silently fails with an auth error.
  • Improve the non-interactive confirmation error in publish (marketplace.py ~line 1639): append "Use --dry-run to preview first." so CI users know how to inspect the plan before adding --yes.

Generated by PR Review Panel for issue #790 · ● 1.8M ·

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.

Harden against all the panel suggestions, not just the summary below. This is important, because the 2 asks it makes are not the only ones that are key. Specially authentication has to be hardened. Let's do a full hardening based on the panelists independent reviews.

@sergio-sisternes-epam sergio-sisternes-epam force-pushed the feat/722-marketplace-maintainer-ux branch from e528dab to d1b080d Compare April 27, 2026 08:36
Comment thread tests/unit/commands/test_marketplace_doctor.py Dismissed
@sergio-sisternes-epam
Copy link
Copy Markdown
Collaborator Author

@danielmeppiel All panel findings have been hardened. Here's the full breakdown:

Security

Finding Fix
S4 ConsumerTarget injection __post_init__ validates repo format, branch safety chars, and path traversal at construction time (shift-left)
S5 version_pins fail-open load_ref_pins() now warns when an expected pin file is missing instead of silently returning empty
L4 Doctor hardcoded env-var auth Replaced raw GITHUB_TOKEN/GH_TOKEN lookup with AuthResolver.resolve("github.com") -- exercises the full credential chain (env vars, gh CLI, git credential helpers)

Architecture

Finding Fix
A3 Builder state smuggling Builder.resolve() returns a frozen ResolveResult dataclass; _resolve_errors instance attribute removed entirely

Logging

Finding Fix
L3 Verbose tracebacks 25 exception handlers now log logger.debug(..., exc_info=True) for debug-level tracebacks
UX6 Bare Exception handlers 3 handlers narrowed to specific types (subprocess.SubprocessError, OSError, MarketplaceNotFoundError); remaining catch-alls annotated with # noqa: BLE001 justifications
L5 Bare click.echo 15 calls replaced with CommandLogger methods (tree_item, progress, verbose_detail)
L7 Rich markup in strings 3 console.print("[dim]...[/dim]") calls replaced with logger.progress(..., symbol="info")

UX

Finding Fix
UX2 CI confirmation safety All 3 click.confirm sites now check _is_interactive() -- non-interactive mode without --yes exits 1 with guidance message
UX5 Outdated exit code Summary simplified to "{N} package(s) can be updated" / "All packages are up to date"; explicit sys.exit(0) on success
UX8 Doctor gh CLI check New informational Check 4 runs gh --version; reports version or install hint

Already fixed in prior commits (verified)

S1 (path traversal), S2 (token redaction), S3 (GIT_TERMINAL_PROMPT), A1 (atomic_write), A2 (redact_token), A4 (_SOURCE_RE), A7 (locals().get), UX3 (--marketplace-yml docs), UX4 (version/ref exclusivity), UX7 (publish prompt), C2 (sidebar + cross-links)

Deferred to follow-up issues (not blocking merge)

Commit: d1b080d | Tests: 6,333 passed | Branch rebased on latest main

@danielmeppiel danielmeppiel added panel-review Trigger the apm-review-panel gh-aw workflow and removed panel-review Trigger the apm-review-panel gh-aw workflow labels Apr 27, 2026
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 27, 2026

APM Review Panel Verdict

Disposition: REQUEST_CHANGES (two small pre-merge fixes; core feature is sound)


Per-persona findings

Python Architect:

This is a major new subsystem: 8 authoring commands (init, build, outdated, check, doctor, publish, package add|set|remove) gated behind apm experimental enable marketplace-authoring. ruamel.yaml is added as a new dependency for YAML round-trip editing. The architecture is well-shaped for experimental code.

1. OO / class diagram

classDiagram
    direction TB
    class MarketplaceGroup {
        <<CustomClickGroup>>
        +format_commands(ctx, formatter)
        +_authoring_visible() bool
    }
    class MarketplaceBuilder {
        <<Builder>>
        +yml_path Path
        +options BuildOptions
        +build() BuildReport
    }
    class BuildOptions {
        <<ValueObject>>
        +dry_run bool
        +offline bool
        +include_prerelease bool
    }
    class BuildReport {
        <<ValueObject>>
        +resolved list
        +warnings list
    }
    class ResolvedPackage {
        <<ValueObject>>
        +name str
        +ref str
        +sha str
    }
    class MarketplacePublisher {
        <<Facade>>
        +publish() list
    }
    class ConsumerTarget {
        <<ValueObject>>
        +repo str
        +branch str
        +path_in_repo str
    }
    class RefResolver {
        <<Adapter>>
        +offline bool
        +list_remote_refs(source) list
        +resolve_ref_sha(source, ref) str
    }
    class MarketplaceYml {
        <<ValueObject>>
        +name str
        +packages list
        +build BuildConfig
    }
    class AuthResolver {
        <<Strategy>>
        +resolve(host) AuthContext
    }
    class MarketplaceGroup:::touched
    class MarketplaceBuilder:::touched
    class MarketplacePublisher:::touched
    class ConsumerTarget:::touched
    class RefResolver:::touched
    MarketplaceBuilder *-- BuildOptions : configured by
    MarketplaceBuilder ..> BuildReport : produces
    BuildReport *-- ResolvedPackage
    MarketplacePublisher *-- ConsumerTarget
    MarketplaceBuilder ..> RefResolver : delegates to
    MarketplaceBuilder ..> MarketplaceYml : reads
    MarketplacePublisher ..> MarketplaceYml : reads
    MarketplacePublisher ..> AuthResolver : uses gh CLI (not AuthResolver)
    note for MarketplacePublisher "PR creation delegates to gh CLI subprocess\nAuth path is separate from AuthResolver"
    note for RefResolver "Adapter over git ls-remote;\noffline mode uses cached refs"
    classDef touched fill:#fff3b0,stroke:#d47600
Loading

2. Execution flow diagram (apm marketplace build)

flowchart TD
    A["apm marketplace build"] --> B{"_require_authoring_flag()"}
    B -- "disabled" --> BX["[i] enable hint; sys.exit(1)"]
    B -- "enabled" --> C["_load_yml_or_exit()\n[FS] read marketplace.yml"]
    C -- "missing" --> CX["sys.exit(1)"]
    C -- "schema error" --> CY["sys.exit(2)"]
    C -- "ok" --> D["MarketplaceBuilder(yml_path, opts).build()"]
    D --> E["[NET] RefResolver.list_remote_refs()\ngit ls-remote per package"]
    E -- "OfflineMissError" --> EX["BuildError; sys.exit(1)"]
    E -- "GitLsRemoteError" --> EY["BuildError; sys.exit(1)"]
    E -- "ok" --> F["tag_pattern regex; semver range resolution"]
    F -- "NoMatchingVersionError" --> FX["BuildError; sys.exit(1)"]
    F -- "match" --> G["ResolvedPackage(name, ref, sha)"]
    G --> H{"dry_run?"}
    H -- "yes" --> I["[i] print result table; skip write"]
    H -- "no" --> J["[FS] marketplace.json atomic write\ntempfile + os.replace"]
    J --> K["[+] Built marketplace.json (N packages)"]
Loading

3. Design patterns

Design patterns

  • Used in this PR: Builder (MarketplaceBuilder + BuildOptions), Value Object / dataclass (BuildReport, ResolvedPackage, ConsumerTarget, BuildOptions), Adapter (RefResolver over git ls-remote), Fail-fast guard (_require_authoring_flag(), _load_yml_or_exit()).
  • Pragmatic suggestion: marketplace.py is 2064 lines. At this scope it is still navigable, but consider extracting publish, build, outdated, and check command handlers into a commands/marketplace/ subpackage as a follow-up before the flag graduates. The existing helper functions (_load_yml_or_exit, _warn_duplicate_names, _require_authoring_flag) are the right extraction candidates for a shared _common.py.

CLI Logging Expert:

Strong compliance. 15 bare click.echo() calls and 3 Rich markup literals replaced with CommandLogger methods -- exactly the right migration. All new commands instantiate CommandLogger("marketplace-<cmd>", verbose=verbose) at the top. Non-interactive CI mode fails loudly before the confirmation prompt (_is_interactive() guard). Verbose traceback via logger.verbose_detail(traceback.format_exc()) is consistent across the new commands.

Two minor observations:

  1. _require_authoring_flag() calls _rich_warning() / _rich_info() directly rather than through a CommandLogger. This is a bootstrapping edge case (the logger isn't yet constructed at that point) and _rich_* is the correct fallback -- acceptable as-is.
  2. One error call uses symbol="cross" (remove command non-interactive guard) -- "cross" is not a key in STATUS_SYMBOLS; the key is "error". Low-severity but technically incorrect symbol lookup. If STATUS_SYMBOLS does not contain "cross", it may silently degrade. Recommend changing to symbol="error".

DevX UX Expert:

The command surface is excellent: doctor mirrors brew doctor, package add|set|remove mirrors npm/cargo, --dry-run / --yes / --no-pr are all familiar. The two-section help (Consumer commands / Authoring commands) is a model of progressive disclosure.

Three observations:

  1. apm marketplace outdated exits 1 when upgrades exist -- matches npm outdated exactly; CHANGELOG records it. Good. The command help text could mention the exit code explicitly ("exits 1 when upgrades are available") so users are not surprised in CI.
  2. _authoring_visible() fails open (returns True on exception) so authoring commands appear in help even when the flag check errors. This is the right choice for discoverability -- users see the command, try it, and get a clear enablement hint from _require_authoring_flag().
  3. publish --no-pr help text says "Push branches but skip PR creation" -- users from npm publish may not know what "push branches" means here. Suggest: "Commit changes to each target repo but do not open a pull request."

Supply Chain Security Expert:

Strong security posture throughout:

  • Mutable refs (HEAD, branch names) are auto-resolved to concrete SHAs in _resolve_ref(). When --no-verify is set and no concrete SHA or version is provided, the command exits 2 with a clear error -- the enforcement is in code, not just documentation.
  • ConsumerTarget validates repo format, branch, and path_in_repo (via validate_path_segments) at construction time (shift-left). Correct.
  • marketplace.json written atomically (tempfile + replace). Correct.
  • Unknown keys in marketplace.yml raise schema error (fail-closed schema). Correct.
  • doctor uses AuthResolver rather than raw os.getenv for token detection. Correct.

Two issues:

[P0 -- Required] _check_gitignore_for_marketplace_json() checks for exact patterns {"marketplace.json", "**/marketplace.json", "/marketplace.json"} but misses "*.json" -- the most common case and the one the documentation explicitly warns about. A *.json rule in .gitignore silently hides marketplace.json from tracking without triggering any warning. Fix: add "*.json" to the patterns set.

[P1 -- Recommended] In _resolve_ref() at line 138-140, when list_remote_refs raises GitLsRemoteError or OfflineMissError for a non-HEAD, non-SHA ref, the function returns the ref as-is (silent fallback). This means a branch name like main can be stored unresolved if the network fails at the right moment. A logger.warning() at that branch point would surface the ambiguity without blocking the operation.

publish-state.json stores PR URLs and repo names. The docs say "safe to commit or to gitignore." For private consumer repos, this leaks repo names into git history. Suggest a note: "If consumer repos are private, consider gitignoring this file."


Auth Expert:

Activated -- the PR changes credential resolution in apm marketplace doctor from raw env-var lookup to AuthResolver.resolve("github.com").token, and introduces the publish command which delegates PR creation to the gh CLI (a separate credential chain).

Findings:

  1. doctor auth check correctly uses AuthResolver().resolve("github.com").token -- full token precedence chain is honoured (GITHUB_APM_PAT -> GITHUB_APM_PAT -> GITHUB_TOKEN -> GH_TOKEN -> git credential fill). This is a net improvement over the previous raw env-var lookup.
  2. publish introduces a two-credential-system: git ls-remote operations use APM's credential chain (git helpers / token), while PR creation uses the gh CLI's own credential store. Users need BOTH configured. The doctor command now surfaces both (git auth check + gh CLI check), which is the right approach.
  3. [P1 -- Recommended] publish does not run gh auth status as a pre-flight check. After cloning targets, computing diffs, and pushing branches, failures at PR creation time due to unauth'd gh are expensive. A subprocess.run(["gh", "auth", "status"], ...) check in pre-flight step 1b (after confirming gh is on PATH) would fail fast and save the user from a half-finished publish run.
  4. The publish command does not document the minimum gh token scope needed (repo write for private repos, public_repo for public repos). Recommend adding a note to the docs.

OSS Growth Hacker:

This PR enables a conversion surface that does not yet exist anywhere: APM as the tool for authoring and publishing Anthropic-compliant marketplaces end-to-end. The two-file model (marketplace.yml -> marketplace.json) will be immediately legible to npm/cargo users. apm marketplace doctor is the kind of command that turns a first-time frustrated user into an advocate.

The experimental flag gating is growth-protective -- shipping fast without consumer disruption, with clear upgrade path. The docs are thorough (391-line guide, quickstart, common errors table).

Missing: a story angle for the release. The hook is "APM is now the first CLI to author, validate, and distribute Anthropic-compliant plugin marketplaces end-to-end." That sentence does not appear anywhere in the PR or CHANGELOG. When this flag graduates, it deserves a launch beat.

Side-channel to CEO: the "marketplace maintainer" persona (org or team that curates an internal plugin index) is a distinct enterprise buyer profile. This feature directly maps to the Lorenzo Storelli / AI Controls scenario. The growth strategy should explicitly track "marketplace maintainer" as a conversion target separate from "package consumer" and "package author." The enterprise docs should call out this flow by name.


CEO arbitration

Specialists agreed on the core quality of this PR. The architecture is solid for experimental code, the CLI surface is familiar and well-designed, and the supply-chain defaults (mutable-ref auto-resolution, fail-closed validation, atomic writes) set the right precedent for marketplace operations. The two required changes are narrow: a one-line fix to the gitignore pattern check (P0) and a CHANGELOG relocation (structural). The auth finding about gh auth status pre-flight is strongly recommended but does not block the feature since publish will fail with a clear gh error message regardless. Strategic call: the "marketplace maintainer" persona the Growth Hacker identified is worth a dedicated CHANGELOG section header and an enterprise docs callout when this flag graduates. The PR is NOT approved in draft state per author request, and requires the CHANGELOG fix before merge.


Required actions before merge

  1. CHANGELOG: The (#790) entries currently land in already-released sections [0.9.2] Added, [0.9.3] Fixed, and [0.9.4] Changed. Move all of them to [Unreleased] (or to whatever version this PR will release into). Adding to past release sections falsely implies these features shipped in those versions.

  2. _check_gitignore_for_marketplace_json() in marketplace.py: Add "*.json" to the patterns set. The current check misses the most common case that the documentation explicitly warns about:

    # marketplace.py, _check_gitignore_for_marketplace_json()
    patterns = {"marketplace.json", "**/marketplace.json", "/marketplace.json", "*.json"}

Optional follow-ups

  • Fix symbol="cross" to symbol="error" in the remove command's non-interactive guard.
  • Add gh auth status pre-flight to publish (before target cloning) to fail fast on missing gh credentials.
  • Add a warning log in _resolve_ref() line 140 (silent fallback when list_remote_refs fails for non-HEAD, non-SHA ref).
  • Add a note to publish-state.json docs: teams with private consumer repos should consider gitignoring it to avoid repo-name leakage.
  • Split marketplace.py (2064 lines) into a commands/marketplace/ subpackage before the experimental flag graduates.
  • Document minimum gh token scope required by publish (repo for private, public_repo for public targets).

Generated by PR Review Panel for issue #790 · ● 1.6M ·

Sergio Sisternes and others added 18 commits April 27, 2026 11:15
Address UX review findings for the plugin subgroup:

- Show subcommand names in plugin group help text
- Guard `plugin set` against zero-field invocations
- Standardise `plugin remove` confirmation via click.confirm
- Extract shared _is_interactive() helper to _helpers.py
- Remove dead --no-verify flag from `plugin set`
- Document plugin commands in CLI reference

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Security:
- Add path traversal guard on marketplace.yml output field (S1)
- Suppress git credential prompts with GIT_TERMINAL_PROMPT=0 (S3)
- Validate ConsumerTarget repo/branch against injection (S4)

Architecture:
- Replace locals().get("pr") with explicit variable (A7)
- Make SOURCE_RE public in yml_schema (A4)

Logging:
- Add Rich fallback for publish state-file display (L1)

UX:
- Enforce --version/--ref mutual exclusivity in plugin add (UX4)
- Remove phantom --marketplace-yml from CLI reference docs (UX3)

Docs:
- Wire marketplace authoring guide into docs sidebar (C2)
- Add consumer-to-author cross-link in marketplace guide (G1)

12 new tests covering all security and UX fixes.
Resolves panel findings S1, S3, S4, A7, A4, L1, UX3, UX4, C2, G1.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Architecture:
- Extract shared atomic_write() to marketplace/_io.py (A1)
- Extract shared redact_token() to marketplace/_git_utils.py (A2)
- Fix token redaction regex to cover http:// and ?token= (S2)

Logging:
- Add verbose traceback output to 5 exception handlers (L3)

UX:
- Add summary line to outdated command output (UX5)
- Exit code 1 when packages are outdated, matching npm/pip (UX5)

17 new tests covering DRY utilities, verbose tracebacks, and
outdated summary/exit behaviour.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…xclusivity

- Add GIT_TERMINAL_PROMPT=0 and GIT_ASKPASS=echo to publisher._run_git()
  chokepoint (8 subprocess calls including clone/push) — completes S3 fix
- Add --version/--ref mutual exclusivity to plugin set (NEW-1 from panel)
- Update stale _TOKEN_RE docstring references in publisher and pr_integration (N2)
- Tests: TestRunGitEnv (publisher), plugin set conflict test

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…/set

When no --ref is provided, plugin add now resolves HEAD to a concrete
40-char SHA via git ls-remote before storing it in marketplace.yml.
When --ref HEAD or a branch name is given, a warning is emitted and
the ref is auto-resolved to its current SHA for supply-chain safety.
Explicit SHAs and tags are stored as-is.

Adds resolve_ref_sha() to RefResolver for single-ref lookups.
26 new tests covering all resolution paths.
Updates CLI reference, marketplace guide, and CHANGELOG.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…nit flags

- Rename 'plugin' subgroup to 'package' for npm/pip/cargo familiarity
- Group marketplace --help into Consumer and Authoring sections
- Add --name/--owner flags to marketplace init
- Hide unimplemented --check-refs flag
- Fix includePrerelease typo in init template
- Add short flags (-d, -s) to package add command
- Update search metavar to QUERY@MARKETPLACE
- Update CHANGELOG, docs, and apm-usage skill

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Builder now detects and warns when multiple packages share the same
name in marketplace.yml. check and doctor commands also flag duplicates.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…uild

When a marketplace.yml entry has no description, the builder now
fetches the package's apm.yml from its resolved git source to extract
the description. This is best-effort -- network failures are silently
ignored and the build never fails because of a missing description.

Explicit descriptions in marketplace.yml always take precedence.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…apm.yml

When a marketplace.yml entry lacks description or version, the builder
now fetches the package's apm.yml from its resolved git source to
extract these fields. Authentication is handled via APM's AuthResolver
so private repos are supported.

- Removed --description CLI flags from package add/set (metadata is
  always sourced from the package's own apm.yml)
- Removed description from PackageEntry, ResolvedPackage, and yml_editor
- Added concurrent metadata pre-fetching with ThreadPoolExecutor
- Best-effort enrichment: network failures are silently ignored
- 20 new tests covering metadata fetch, auth, and enrichment

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Ring-fence all marketplace commands behind the `marketplace_commands`
experimental feature flag (default: disabled). Users opt-in via:

    apm experimental enable marketplace-commands

Commands are hidden from --help when disabled. Defence-in-depth guard
in the marketplace group callback provides a clear enablement message
if somehow reached directly.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Rename the experimental feature flag from marketplace-commands to
marketplace-authoring for clarity. The old name suggested the flag
gated CLI commands generically; the new name communicates that it
gates the marketplace authoring and discovery surface.

Users who previously enabled the old flag will see a stale-config
warning and can re-enable with:

    apm experimental enable marketplace-authoring

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The marketplace_authoring flag was gating ALL marketplace commands
including pre-existing consumer commands (add, list, browse, update,
remove, validate, search) that already ship on main. This broke the
consumer surface for all users.

Now only authoring commands are gated: init, build, check, outdated,
doctor, publish, and the package subgroup (add/set/remove). Consumer
commands are always available without any flag.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Authoring commands (init, build, check, outdated, doctor, publish,
package) are now hidden from `marketplace --help` when the
marketplace-authoring flag is disabled. Consumer commands remain
always visible.

The defence-in-depth guard (_require_authoring_flag) stays in each
authoring command body so that direct invocation still shows the
enablement message rather than a generic Click error.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Use _rich_warning/_rich_info in authoring flag guard with docs link
- Fix "centre" → "center" typo in check table (3 occurrences)
- Switch publish confirmation to click.confirm (fixes double-prompt)
- Standardise --yes help text across all confirmation commands
- Route validate summary through CommandLogger
- Route package remove cancel through CommandLogger

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The regex in redact_token() was dropping ? and & separators when
redacting query-string tokens (?token=VALUE, &token=VALUE), producing
malformed URLs like '...archivetoken=***'.

Capture the separator in a group and re-emit it in the replacement.
Update test assertions to verify separators are preserved.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Security:
- ConsumerTarget.__post_init__ validates repo format, branch chars, path traversal
- Doctor uses AuthResolver instead of raw env-var lookup
- version_pins warns when expected pin file disappears

Architecture:
- Builder.resolve() returns ResolveResult dataclass (no state smuggling)
- 15 bare click.echo -> CommandLogger methods
- 3 Rich markup literals -> logger.progress

UX:
- Doctor checks gh CLI presence (informational)
- Confirmation prompts fail loudly in non-interactive mode
- Outdated summary simplified + exit code consistency

Logging:
- 25 exception handlers get verbose tracebacks (debug level)
- 3 handlers narrowed from bare Exception to specific types

Closes panel findings: S4, S5, L3, L4, L5, L7, A3, UX2, UX5, UX6, UX8

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Move all [Experimental] CHANGELOG entries from released sections
  (0.9.2, 0.9.3, 0.9.4) to [Unreleased]
- Add *.json to gitignore pattern check in _check_gitignore_for_marketplace_json
- Fix symbol="cross" -> symbol="error" (2 sites)
- Add warning log when _resolve_ref falls back to unresolved ref

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@sergio-sisternes-epam sergio-sisternes-epam force-pushed the feat/722-marketplace-maintainer-ux branch from 0555500 to 374f448 Compare April 27, 2026 10:17
@danielmeppiel danielmeppiel self-requested a review April 27, 2026 14:33
@sergio-sisternes-epam sergio-sisternes-epam added this pull request to the merge queue Apr 27, 2026
Merged via the queue into main with commit 5868574 Apr 27, 2026
9 checks passed
@sergio-sisternes-epam sergio-sisternes-epam deleted the feat/722-marketplace-maintainer-ux branch April 27, 2026 14:40
danielmeppiel pushed a commit that referenced this pull request Apr 27, 2026
Folds in #722/#790 marketplace authoring CLI and #989 cowork test fixup
that landed after the initial release branch was opened.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
danielmeppiel pushed a commit that referenced this pull request Apr 27, 2026
Promotes [Unreleased] to [0.10.0] - 2026-04-27. Each PR since v0.9.4
gets one 'so what' line:

- #926 Microsoft 365 Cowork target ships impl
- #790 marketplace authoring CLI (init, package add/set, build, check,
  outdated, doctor, publish) -- collapsed from 20+ bullets to one
- #722 marketplace plugin -> package rename + --help sectioning -- collapsed
- #980 README 'Coming from npx skills add' conversion block
- #981 docs auto-deploy on tag push (real fix for the #953 attempt)
- #985 pr-description-skill evals suite
- #984 pr-description-skill mermaid hardening
- #989 cowork sys.platform mock for Windows CI

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

panel-review Trigger the apm-review-panel gh-aw workflow

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEATURE] Add apm marketplace generate command to pack all plugins and produce a marketplace.json

4 participants