Skip to content

feat(marketplace): support multi-profile marketplace outputs#1281

Merged
danielmeppiel merged 5 commits into
microsoft:mainfrom
henrydennis:feature/multi-marketplace-outputs
May 14, 2026
Merged

feat(marketplace): support multi-profile marketplace outputs#1281
danielmeppiel merged 5 commits into
microsoft:mainfrom
henrydennis:feature/multi-marketplace-outputs

Conversation

@henrydennis
Copy link
Copy Markdown
Contributor

@henrydennis henrydennis commented May 11, 2026

TL;DR

apm pack can now build more than one marketplace artifact from the same marketplace: block. Claude/Anthropic output remains the default and keeps the existing .claude-plugin/marketplace.json behavior, while maintainers can opt into Codex repo marketplace output with marketplace.outputs: [claude, codex].

Problem

Marketplace authoring was effectively single-output: the schema and builder assumed one Claude-compatible marketplace.json path. That made it awkward to support another repo marketplace shape without either duplicating resolution logic or letting Codex-specific fields leak into the Claude artifact.

There was also an ambiguous boundary around --marketplace-output: once multiple artifacts exist, the CLI override needs to target the Claude/Anthropic compatibility artifact only, not whichever configured path happens to compare equal to a legacy field.

Approach

Change What
Output profiles Add MarketplaceOutputProfile entries for claude and codex, including profile-specific config namespace, mapper name, required package fields, and whether the CLI output override applies.
Output mappers Move format-specific JSON composition into ClaudeMarketplaceMapper and CodexMarketplaceMapper; keep resolution and writing in MarketplaceBuilder.
Multi-output producer Update MarketplaceProducer to resolve packages once, then write every selected profile and return a BuildReport containing all per-profile output reports.
Schema extensions Add marketplace.outputs, marketplace.claude.output, marketplace.codex.output, and package-level category required only when Codex output is selected.
Docs and init flow Document the multi-output selector, Claude-only override behavior, Codex category requirement, and generated artifact tracking.

Implementation

  • src/apm_cli/marketplace/yml_schema.py parses outputs, output-specific config blocks, and category, while preserving legacy output: as shorthand for claude.output.
  • src/apm_cli/marketplace/output_profiles.py centralizes output capabilities; supports_cli_output_override is true only for the Claude profile.
  • src/apm_cli/marketplace/output_mappers.py owns format-specific JSON shape, so builder no longer contains both Claude and Codex field-mapping logic.
  • src/apm_cli/marketplace/builder.py now writes one profile at a time and returns BuildReport(outputs=(...)); compatibility accessors such as report.output_path and report.resolved continue to work for single-output callers.
  • src/apm_cli/core/build_orchestrator.py loops over selected profiles, applies --marketplace-output only to profiles that support it, and keeps all profile reports in the marketplace producer payload.
  • src/apm_cli/commands/pack.py renders each marketplace output line separately, including the profile name.

Behavior

flowchart TD
    A["apm.yml marketplace block"] --> B["MarketplaceConfig"]
    B --> C["MarketplaceProducer"]
    C --> D["MarketplaceBuilder.resolve()"]
    D --> E["ResolvedPackage tuple"]
    B --> F["MarketplaceOutputProfile registry"]
    F --> G["claude profile"]
    F --> H["codex profile"]
    E --> I["ClaudeMarketplaceMapper"]
    E --> J["CodexMarketplaceMapper"]
    I --> K[".claude-plugin/marketplace.json"]
    J --> L[".agents/plugins/marketplace.json"]
Loading

Example authoring config:

marketplace:
  outputs: [claude, codex]

  claude:
    output: .claude-plugin/marketplace.json

  codex:
    output: .agents/plugins/marketplace.json

  packages:
    - name: local-tool
      source: ./plugins/local-tool
      category: Productivity

Trade-offs

  • BuildReport is now multi-output internally, but keeps single-output compatibility properties to avoid forcing unrelated marketplace callers to change immediately.
  • Codex category is validated at schema load time when codex is selected, so maintainers get an early error instead of a partially written artifact.
  • --marketplace-output remains Claude-only by design. Codex output path is configured declaratively through marketplace.codex.output, which avoids one CLI flag having two different meanings.

Validation

uv run pytest tests/unit tests/test_console.py -x
# 8318 passed, 1 warning in 29.08s

uv run --extra dev ruff check src/ tests/
# All checks passed!

uv run --extra dev ruff format --check src/ tests/
# 752 files already formatted

How to test

  1. Create an apm.yml with marketplace.outputs: [claude, codex] and local package entries that include category.
  2. Run apm pack --offline.
  3. Confirm both .claude-plugin/marketplace.json and .agents/plugins/marketplace.json are written.
  4. Run apm pack --marketplace-output ./build/marketplace.json and confirm only the Claude artifact path is overridden.

@henrydennis
Copy link
Copy Markdown
Contributor Author

@microsoft-github-policy-service agree

@henrydennis henrydennis marked this pull request as ready for review May 11, 2026 17:26
Copilot AI review requested due to automatic review settings May 11, 2026 17:26
@henrydennis henrydennis force-pushed the feature/multi-marketplace-outputs branch from 74a3a20 to 7778108 Compare May 11, 2026 17:31
@henrydennis henrydennis marked this pull request as draft May 11, 2026 17:32
@henrydennis henrydennis marked this pull request as ready for review May 11, 2026 17:32
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

Note

Copilot was unable to run its full agentic suite in this review.

Adds multi-profile marketplace builds so apm pack can emit multiple marketplace artifacts (Claude + optional Codex) from a single marketplace: block, while keeping backwards compatibility with the existing Claude .claude-plugin/marketplace.json behavior.

Changes:

  • Introduces marketplace output profiles (claude, codex) and output-specific mappers, moving format composition out of the builder.
  • Extends the marketplace YAML schema to support marketplace.outputs, marketplace.claude, marketplace.codex, plus conditional package.category for Codex.
  • Updates pack/orchestrator rendering and init/docs/tests to support and validate multi-output behavior and Claude-only CLI override.

Reviewed changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
tests/unit/marketplace/test_local_path_compose.py Adds Codex composition/write tests and verifies Claude output omits Codex-only fields.
tests/unit/marketplace/test_builder.py Updates tests for renamed --marketplace-output plumbing in build options.
tests/unit/marketplace/test_apm_yml_marketplace_loader.py Adds schema/output-profile parsing + validation coverage for multi-output and Codex category requirement.
tests/unit/core/test_build_orchestrator.py Validates orchestrator/producer writes multiple outputs and applies CLI override only to Claude.
tests/unit/commands/test_marketplace_init.py Confirms init template and warnings mention Codex output + gitignore patterns.
tests/integration/test_pack_unified.py Integration coverage for pack writing Codex output when selected.
src/apm_cli/marketplace/yml_schema.py Implements outputs, per-output blocks, and conditional package validation (category for Codex).
src/apm_cli/marketplace/output_profiles.py Adds centralized profile registry for output capabilities and defaults.
src/apm_cli/marketplace/output_mappers.py Introduces Claude/Codex mappers and shared mapping helpers.
src/apm_cli/marketplace/init_template.py Documents multi-output config in the scaffolded marketplace block.
src/apm_cli/marketplace/diagnostics.py Extracts shared build diagnostics type used by mappers/reports.
src/apm_cli/marketplace/builder.py Refactors builder to map/write per-profile and returns multi-output BuildReport.
src/apm_cli/core/build_orchestrator.py Orchestrates resolving once and writing each selected marketplace profile; applies CLI override only to Claude.
src/apm_cli/commands/pack.py Renders per-profile marketplace output lines; clarifies CLI override scope.
src/apm_cli/commands/marketplace/init.py Updates next-steps messaging for optional Codex output and multiple generated artifacts.
src/apm_cli/commands/marketplace/init.py Expands .gitignore checks to cover both Claude and Codex default output paths.
docs/src/content/docs/reference/cli-commands.md Documents multi-output behavior and Claude-only --marketplace-output semantics.
docs/src/content/docs/guides/pack-distribute.md Updates pack guide to describe selectable marketplace outputs.
docs/src/content/docs/guides/marketplace-authoring.md Documents marketplace.outputs, Codex output shape/requirements, and updated examples.

Comment thread src/apm_cli/core/build_orchestrator.py
Comment thread src/apm_cli/marketplace/builder.py
Comment thread src/apm_cli/marketplace/yml_schema.py Outdated
@henrydennis henrydennis force-pushed the feature/multi-marketplace-outputs branch from c8fb212 to 152c781 Compare May 13, 2026 08:29
@henrydennis henrydennis requested a review from Copilot May 13, 2026 08:55
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 18 out of 18 changed files in this pull request and generated 2 comments.

Comment thread src/apm_cli/commands/pack.py Outdated
Comment on lines 238 to 251
def _render_marketplace_result(logger, report, dry_run, extra_warnings=None, outputs=None):
"""Render the marketplace producer's report (one-liner summary)."""
if report is None:
return
for warn_msg in extra_warnings or []:
logger.warning(warn_msg)
output_reports = tuple(getattr(report, "outputs", ()) or ())
if not output_reports:
for output in outputs or []:
if dry_run:
logger.dry_run_notice(f"Would write marketplace.json -> {output}")
else:
logger.success(f"Built marketplace.json -> {output}")
return
for warn_msg in report.warnings:
logger.warning(warn_msg)
Comment on lines +334 to +338
def _mapper_for_profile(self, profile: MarketplaceOutputProfile):
mapper = MARKETPLACE_OUTPUT_MAPPERS.get(profile.mapper)
if mapper is None:
raise BuildError(f"Unknown marketplace output mapper: {profile.mapper}")
return mapper
@sergio-sisternes-epam sergio-sisternes-epam added panel-review Trigger the apm-review-panel gh-aw workflow status/triaged Initial agentic triage complete; pending maintainer ratification (silence = approval). labels May 13, 2026
@danielmeppiel danielmeppiel added status/accepted Direction approved, safe to start work. panel-review Trigger the apm-review-panel gh-aw workflow and removed panel-review Trigger the apm-review-panel gh-aw workflow labels May 14, 2026
@github-actions
Copy link
Copy Markdown

APM Review Panel: needs_rework

Adds Codex marketplace output target via Strategy extraction, making APM the first AI package manager to publish to both Claude Code and GitHub Copilot from one config file.

cc @danielmeppiel -- a fresh advisory pass is ready for your review.

This PR is the most strategically significant feature since the marketplace builder launched. The "write once, publish everywhere" proposition is exactly the competitive moat APM needs: incumbents are single-ecosystem tools; APM is now a multi-ecosystem platform. The architecture is directionally correct -- Strategy extraction, frozen-dataclass profiles, output registry -- and the growth signal is real. The panel converges on three things that must be fixed before this can ship cleanly, and none of them require rethinking the design.

The python-architect blocking finding is the most urgent: the primary_output stub will raise TypeError at runtime for any caller that reaches an empty outputs tuple. This is not a theoretical edge case -- it fires on any code path that constructs a BuildReport without a successful write, including dry-run and error-recovery paths. The fix is small (add defaults or a __post_init__ guard) but it must land before merge. The doc-writer blocking finding is equally concrete: the quick-start git add command hard-codes only the Claude path, which means every dual-output producer following the guide will silently omit the Codex artifact from their commit. That is a workflow-breaking documentation bug that will generate support noise the moment anyone enables outputs:[claude,codex]. The devx-ux blocking flag-name finding is real but tractable: renaming --marketplace-output to --claude-output with a deprecation alias is a one-commit fix that prevents silent scope confusion in CI scripts.

The remaining panel findings (private-symbol import inversion, write_output return-type round-trip, KeyError on unknown output name, missing traversal test on Codex write path, warnings dropped in fallback branch, package count lost in fallback message) are all recommended-tier and do not change the ship calculus -- they are concrete enough to track as follow-up issues and close within one sprint.

Dissent. No direct persona-vs-persona disagreement exists. The only tension worth naming: devx-ux classifies the --marketplace-output flag name as blocking on the grounds that it silently narrows scope; python-architect does not flag it at all, treating it as out of scope. The CEO sides with devx-ux: a flag that implies full-marketplace scope but affects only one output target is a composability trap in CI scripts. However, the fix (deprecation alias, not immediate rename) is low-cost enough that it does not elevate to rework on its own -- it is the TypeError that tips the stance to needs_rework.

Aligned with: Portable by manifest (outputs selector in apm.yml is manifest-driven; adding a third ecosystem is a one-file change), Secure by default (path containment correctly applied on both write paths; KeyError on unknown output name is the one deviation to fix), Pragmatic as npm (UX gaps chip away at first-run ergonomics; recommended followups address this tier directly).

Growth signal. Codex dual-output is the first feature that repositions APM from a Claude-specific build tool to a multi-ecosystem marketplace platform. The "write once, publish to all AI coding assistants" is the strongest quotable claim APM has ever had and it currently appears nowhere on any conversion surface. The apm pack success message when multiple outputs are written is the highest-frequency impression point; adding a single reinforcing line there costs nothing and compounds with every pack run. Watch Codex output adoption rate as the leading indicator for whether a third target (Cursor, Windsurf) justifies a public call for contributors; the output_profiles.py registry already makes that a one-file contribution.

Panel summary

Persona B R N Takeaway
Python Architect 1 3 2 Strategy pattern extraction is well-scoped; primary_output stub TypeError and private-symbol import inversion need fixing before the shape compounds.
CLI Logging Expert 0 2 3 Multi-output refactor is mostly clean; report.warnings silently dropped in fallback branch and package count lost in same branch.
DevX UX Expert 1 3 2 Directionally sound but --marketplace-output flag name now silently narrows scope, category has no upfront validation, and default state is not self-documenting.
Supply Chain Security Expert 0 1 2 Path containment correctly applied on both write paths; KeyError on unknown output name leaks internal module paths via raw traceback.
OSS Growth Hacker 0 3 2 Genuine reach multiplier; "write once, publish to all" hook missing from every conversion surface; git add snippet silently breaks dual-output workflows.
Auth Expert -- -- -- inactive
Doc Writer 1 2 2 Largely accurate; quick-start git add omits Codex path (workflow-breaking), missing-category error undocumented in Pitfalls.
Test Coverage Expert 0 3 1 Dual-output pack and Codex mapper well covered; traversal guard test missing on Codex path; --marketplace-output scoping only at unit tier.

B = blocking-severity findings, R = recommended, N = nits.
Counts are signal strength, not gates. The maintainer ships.

Top 5 follow-ups

  1. [Python Architect] (blocking-severity) Fix primary_output stub: add defaults to MarketplaceOutputReport optional-context fields or add __post_init__ non-empty guard -- Runtime TypeError on any code path that constructs BuildReport with empty outputs -- fires in dry-run and error-recovery paths. Crash before any user-visible output.
  2. [Doc Writer] (blocking-severity) Fix quick-start git add command to include both output paths or make it output-list-driven -- Every dual-output producer following the guide will silently omit .agents/plugins/marketplace.json from their commit. Directly breaks the feature for the most motivated early adopters.
  3. [DevX UX Expert] (blocking-severity) Add --claude-output as canonical flag name with --marketplace-output as deprecation alias; print deprecation warning when old name is used -- Current flag name implies full-marketplace scope but affects only the Claude output path. Silent scope mismatch in CI scripts breaks composability without error.
  4. [Supply Chain Security Expert] Replace MARKETPLACE_OUTPUTS[output_name] with .get() and raise BuildError on unknown name -- Raw KeyError on unrecognised output name in apm.yml leaks internal module paths in the traceback. Clean BuildError is the secure-by-default standard for all config validation in the codebase.
  5. [Test Coverage Expert] Add traversal-payload test for ensure_path_within on the Codex-specific write path and integration test for --marketplace-output scoping with dual-output config -- Path containment logic is confirmed correct by supply-chain review, but the absence of an automated regression guard on the Codex write path means the next refactor has no safety net.

Architecture

classDiagram
    direction LR

    class MarketplaceOutputMapper {
        <<Strategy>>
        +uses_remote_metadata bool
        +compose(config, resolved, remote_metadata) MapperResult
    }

    class ClaudeMarketplaceMapper {
        <<ConcreteStrategy>>
        +uses_remote_metadata = True
        +compose(config, resolved, remote_metadata) MapperResult
    }

    class CodexMarketplaceMapper {
        <<ConcreteStrategy>>
        +uses_remote_metadata = False
        +compose(config, resolved, remote_metadata) MapperResult
    }

    class MapperResult {
        <<ValueObject>>
        +document dict
        +warnings tuple
        +diagnostics tuple
    }

    class MarketplaceOutputProfile {
        <<ValueObject>>
        +name str
        +config_attr str
        +mapper str
        +required_package_fields tuple
        +supports_cli_output_override bool
    }

    class MarketplaceOutputReport {
        <<ValueObject>>
        +profile str
        +resolved tuple
        +errors tuple
        +warnings tuple
        +output_path Path
        +dry_run bool
    }

    class BuildReport {
        <<Aggregate>>
        +outputs tuple
        +primary_output MarketplaceOutputReport
        +warnings tuple
        +dry_run bool
    }

    class MarketplaceBuilder {
        <<IOBoundary>>
        +resolve() ResolvedResult
        +write_output(profile, resolved, path) BuildReport
        +compose_output(profile, resolved) tuple
        +remote_metadata_for_profile(profile, resolved) dict
        +build() BuildReport
    }

    class MarketplaceConfig {
        <<ValueObject>>
        +outputs tuple
        +claude MarketplaceClaudeConfig
        +codex MarketplaceCodexConfig
        +packages tuple
    }

    class MarketplaceOutputMapper:::touched
    class ClaudeMarketplaceMapper:::touched
    class CodexMarketplaceMapper:::touched
    class MapperResult:::touched
    class MarketplaceOutputProfile:::touched
    class MarketplaceOutputReport:::touched
    class BuildReport:::touched
    class MarketplaceBuilder:::touched
    class MarketplaceConfig:::touched

    classDef touched fill:#fff3b0,stroke:#d47600

    MarketplaceOutputMapper <|-- ClaudeMarketplaceMapper : extends
    MarketplaceOutputMapper <|-- CodexMarketplaceMapper : extends
    MarketplaceOutputMapper ..> MapperResult : returns
    MarketplaceBuilder *-- MarketplaceOutputMapper : dispatches via registry
    MarketplaceBuilder ..> MarketplaceOutputProfile : reads profile.mapper
    MarketplaceBuilder ..> MarketplaceConfig : loads yml
    MarketplaceBuilder ..> MarketplaceOutputReport : produces
    BuildReport *-- MarketplaceOutputReport : aggregates
Loading
flowchart TD
    A(["apm pack --marketplace-output PATH"])
    B["commands/pack.py\ncli_pack()"]:::touched
    C["core/build_orchestrator.py\nMarketplaceProducer.produce()"]:::touched
    D["[I/O] marketplace/yml_schema.py\nload_marketplace_config()"]
    E{"for output_name in config.outputs"}
    F["marketplace/output_profiles.py\nMARKETPLACE_OUTPUTS lookup"]:::touched
    G["marketplace/builder.py\nMarketplaceBuilder.resolve()\n[NET] git ls-remote per package"]
    H["marketplace/builder.py\nremote_metadata_for_profile()\n[NET] fetch if uses_remote_metadata"]
    I["marketplace/builder.py\nwrite_output()\n[FS] ensure_path_within guard"]:::touched
    J["marketplace/builder.py\ncompose_output()"]
    K{"profile.mapper lookup\nMARKETPLACE_OUTPUT_MAPPERS"}:::touched
    L["output_mappers.py\nClaudeMarketplaceMapper.compose()"]:::touched
    M["output_mappers.py\nCodexMarketplaceMapper.compose()"]:::touched
    N["[FS] atomic_write -> output_path"]
    O["MarketplaceOutputReport\n(per profile)"]:::touched
    P["orchestrator aggregates\noutput_reports list"]
    Q["BuildReport(outputs=tuple(output_reports))"]:::touched
    R["_render_marketplace_result()"]

    A --> B
    B --> C
    C --> D
    D --> E
    E -->|"first iteration only"| G
    G --> H
    E --> F
    F --> I
    H --> I
    I --> J
    J --> K
    K -->|"mapper=claude"| L
    K -->|"mapper=codex"| M
    L --> N
    M --> N
    N --> O
    O --> P
    P -->|"next output_name"| E
    P --> Q
    Q --> R

    classDef touched fill:#fff3b0,stroke:#d47600
Loading

Recommendation

Three fixes are required before merge: (1) the primary_output TypeError crash, (2) the git add documentation bug, and (3) the --marketplace-output deprecation alias. All three are small and surgical -- this is not a design rework, it is a pre-flight patch. Once those land, ship immediately. The strategic window is real: the multi-ecosystem marketplace positioning is a genuine differentiator and the architecture is sound enough to build on. Do not let the recommended-tier findings (private symbol imports, write_output return type, warnings dropped in fallback, KeyError on unknown output name) hold this up -- they are tracked issues, not merge blockers. The growth story is ready to write the moment the three required fixes are in.


Full per-persona findings

Python Architect

  • [blocking] primary_output empty-guard stub omits required dataclass fields, causing TypeError at runtime at src/apm_cli/marketplace/builder.py
    MarketplaceOutputReport declares profile, resolved, errors, warnings, diagnostics, output_path, unchanged_count, added_count, updated_count, removed_count as required fields. The primary_output property constructs a stub fallback that omits diagnostics, output_path, unchanged_count, added_count, updated_count, and removed_count. Python dataclasses raise TypeError at construction time for missing required fields.
    Suggested: Option A -- add __post_init__ guard requiring non-empty outputs. Option B -- give MarketplaceOutputReport safe defaults for all optional-context fields (output_path: Path | None = None, unchanged_count: int = 0, etc.).

  • [recommended] builder.py imports private symbols from output_mappers.py by underscore name, inverting the extraction boundary at src/apm_cli/marketplace/builder.py
    Three import statements cross into output_mappers.py private API (_is_display_version, _subtract_plugin_root). This couples builder.py back to format-specific internals and inverts the dependency direction the Strategy extraction was meant to establish.
    Suggested: Create src/apm_cli/marketplace/_compose_utils.py with public helpers. Both builder.py and output_mappers.py import from there.

  • [recommended] write_output wraps a single MarketplaceOutputReport in BuildReport only for the orchestrator to immediately unpack it at src/apm_cli/marketplace/builder.py
    Orchestrator does output_reports.extend(report.outputs) immediately after the call. The round-trip BuildReport -> .outputs[0] is confusing and signals the wrong contract.
    Suggested: Change write_output return type to MarketplaceOutputReport. Orchestrator accumulates into BuildReport at the end.

  • [recommended] required_package_fields on MarketplaceOutputProfile is declared but never enforced at src/apm_cli/marketplace/output_profiles.py
    CODEX_MARKETPLACE_OUTPUT declares required_package_fields=("category",) but no validation call site exists. Users who omit category get a silent failure or KeyError inside the mapper, not an early actionable error.
    Suggested: Add validate_profile_requirements() called from orchestrator before write_output.

  • [nit] CODEX_MARKETPLACE_OUTPUT imported in builder.py but unused at src/apm_cli/marketplace/builder.py

  • [nit] _render_marketplace_result fallback branch renders without profile labels at src/apm_cli/commands/pack.py

CLI Logging Expert

  • [recommended] report.warnings silently dropped in the no-output_reports fallback branch at src/apm_cli/commands/pack.py
    In the AFTER code, report.warnings is only iterated inside the for output_report loop. If report is not None but report.outputs is empty, warnings are swallowed.
    Suggested: Move for warn_msg in report.warnings: logger.warning(warn_msg) before the if not output_reports branch, guarded by if report is not None.

  • [recommended] Package count lost in fallback branch success/dry-run messages at src/apm_cli/commands/pack.py
    BEFORE: "Built marketplace.json (N package(s)) -> path". Fallback AFTER: "Built marketplace.json -> path". Silent information regression.
    Suggested: If report is not None in the fallback branch, include package count in the message.

  • [nit] Implicit None-safety via getattr is surprising; removed explicit None guard not documented at src/apm_cli/commands/pack.py
    Suggested: Add a short comment explaining None is tolerated via getattr guard.

  • [nit] dry_run check in fallback branch does not inspect report.dry_run at src/apm_cli/commands/pack.py
    Suggested: Change fallback check to if dry_run or (report is not None and getattr(report, 'dry_run', False)):.

  • [nit] .gitignore warning message could drive the path list from the output-target registry to stay future-proof at src/apm_cli/marketplace/__init__.py

DevX UX Expert

  • [blocking] --marketplace-output flag name no longer matches its scope
    The flag is now Claude/Anthropic-only but still named --marketplace-output. A user who adds codex to outputs and runs apm pack --marketplace-output /custom/path silently overrides only the Claude path. Composability in CI scripts breaks without error.
    Suggested: Rename to --claude-output with a deprecation alias for --marketplace-output that prints a one-line warning.

  • [recommended] category: required for Codex has no proactive validation -- error surfaces too late
    If outputs includes codex but packages lack category, the failure fires at build time rather than as a pre-flight check.
    Suggested: Validate category presence for all packages before any output is written.

  • [recommended] Selector-list default state is implicit and not self-documenting
    A user who omits outputs entirely gets Claude output silently. apm marketplace init should write outputs: [claude] explicitly.

  • [recommended] Multi-output CLI lines have no failure differentiation -- partial failure is invisible
    On partial failure there is no per-line status token to distinguish success from failure.

  • [nit] Next-steps hint phrasing is passive rather than imperative (npm/cargo convention)

  • [nit] Gitignore warning mentions both paths even when only one output is active
    Suggested: Filter the warning to only mention paths corresponding to the configured outputs.

Supply Chain Security Expert

  • [recommended] Unhandled KeyError when config.outputs contains an unknown name at src/apm_cli/marketplace/build_orchestrator.py
    MARKETPLACE_OUTPUTS[output_name] is executed before the try block begins. An unrecognised output name raises a raw KeyError producing a Python traceback.
    Suggested: Replace with .get() and raise BuildError(f'Unknown marketplace output target: {output_name!r}. Valid targets: {list(MARKETPLACE_OUTPUTS)}').

  • [nit] Local-source path copied verbatim into published JSON -- absolute build paths could leak
    If entry.source is an absolute path it is embedded in the Codex marketplace.json output artifact.
    Suggested: Normalise local source paths to be relative to the project root before embedding.

  • [nit] .agents/plugins/ naming overlaps APM agent primitive directory; worth documenting in security model
    The .agents/ directory tree is used for APM agent primitives. Any future agent loader that scans .agents/ should exclude or sandbox this path explicitly.

OSS Growth Hacker

  • [recommended] The "write once, publish to all" hook never appears as a quotable one-liner on any conversion surface
    The dual-output proposition -- one apm.yml reaches both Claude Code and GitHub Copilot/Codex users -- is not articulated as a headline anywhere.
    Suggested: Add a one-liner to the producer guide intro and the apm pack success message when multiple outputs are written.

  • [recommended] Producer guide end-to-end snippet git-adds only .claude-plugin/marketplace.json, silently breaking dual-output workflows at docs/src/content/docs/producer/publish-to-a-marketplace.md
    A producer who enables outputs:[claude,codex] following the guide will ship without committing .agents/plugins/marketplace.json.
    Suggested: Show both files in the git add command with a comment.

  • [recommended] apm marketplace init treats Codex as a post-init footnote rather than an activation moment
    The init next-steps panel buries Codex as step 3 of 4. This is the highest-intent moment to prompt for dual-output adoption.
    Suggested: Add a --codex flag to apm marketplace init.

  • [nit] Missing category validated at apm pack time, not apm marketplace check time -- undermines "check is the gate" mental model

  • [nit] output_profiles.py plugin pattern is an untapped contributor funnel with no guide
    Suggested: Add docstring block to output_profiles.py explaining how to register a new output. Create a stub section in CONTRIBUTING.md.

Auth Expert -- inactive

No auth-relevant files changed. builder.py retains existing _ensure_auth() and RefResolver call paths unchanged. Multi-output profiles add no new remote host resolution, token selection, or credential fallback logic.

Doc Writer

  • [blocking] Quick-start git add command omits the Codex output path at docs/src/content/docs/producer/publish-to-a-marketplace.md
    The end-to-end script runs git add apm.yml .claude-plugin/marketplace.json. A producer who sets outputs:[claude,codex] will generate two artifacts but the example only commits one. Silently breaks dual-output workflows.
    Suggested: Change to show both paths: git add apm.yml .claude-plugin/marketplace.json .agents/plugins/marketplace.json with a note about staging only files matching your outputs list.

  • [recommended] Missing-category error not documented in Pitfalls at docs/src/content/docs/producer/publish-to-a-marketplace.md
    The code raises BuildError (exit 1) when category is absent for Codex output. The Pitfalls section mentions category is required but says nothing about what fails or how to fix it.

  • [recommended] "Both files are committed" statement overstates the default at docs/src/content/docs/producer/publish-to-a-marketplace.md
    Default outputs list is [claude] so only one file exists unless codex is explicitly enabled.
    Suggested: Replace with "Commit every file matching your enabled outputs."

  • [nit] Codex category accepted values not stated or linked

  • [nit] pack.md Behavior bullet does not connect missing-category error to exit code table

Test Coverage Expert

  • [recommended] ensure_path_within guard on Codex output path has no test exercising a traversal payload at tests/unit/marketplace/test_builder_security.py
    No test passes a malicious value in codex.output and asserts the guard fires. Grepped tests/ for codex.*traversal -- zero matches.
    Proof (missing): tests/unit/marketplace/test_builder_security.py::test_codex_output_path_traversal_rejected -- proves: A user who sets codex.output: ../../etc/passwd gets an error, not a file written outside project root [secure-by-default]

  • [recommended] --marketplace-output Claude-only scoping verified only at unit tier; no integration test with outputs:[claude,codex] at tests/integration/test_pack_unified.py
    Unit test exists for producer behavior but no integration test invokes pack_cmd with --marketplace-output AND dual-output config.
    Proof (missing at integration-with-fixtures): tests/integration/test_pack_unified.py::test_pack_marketplace_output_override_does_not_affect_codex -- proves: apm pack --marketplace-output /custom/path with outputs:[claude,codex] writes Codex to its own path unaffected [devx]

  • [recommended] Dual-output integration test exists but could not be run -- outcome unknown per S7 PROBE RULE at tests/integration/test_pack_unified.py
    test_pack_unified.py::test_pack_marketplace_writes_codex_when_selected is exactly the right test but pytest could not be invoked in review environment. Maintainer should verify CI passes before merging.
    Proof (unknown at integration-with-fixtures): tests/integration/test_pack_unified.py::test_pack_marketplace_writes_codex_when_selected -- proves: apm pack with outputs:[claude,codex] writes both marketplace.json files and emits the Codex source shape [portability-by-manifest,devx]

  • [nit] BuildReport.primary_output empty-outputs sentinel path has no test at tests/unit/marketplace/test_builder.py
    Proof (missing at unit): tests/unit/marketplace/test_builder.py::test_build_report_primary_output_empty_sentinel

This panel is advisory. It does not block merge. Re-apply the
panel-review label after addressing feedback to re-run.

Note

🔒 Integrity filter blocked 8 items

The following items were blocked because they don't meet the GitHub integrity level.

  • #1281 pull_request_read: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
  • feat(marketplace): support multi-profile marketplace outputs #1281 pull_request_read: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
  • #1312 list_pull_requests: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
  • #1308 list_pull_requests: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
  • #1306 list_pull_requests: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
  • #1303 list_pull_requests: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
  • #1301 list_pull_requests: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
  • #1288 list_pull_requests: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".

To allow these resources, lower min-integrity in your GitHub frontmatter:

tools:
  github:
    min-integrity: approved  # merged | approved | unapproved | none

Generated by PR Review Panel for issue #1281 · ● 4.8M ·

@github-actions github-actions Bot removed the panel-review Trigger the apm-review-panel gh-aw workflow label May 14, 2026
@henrydennis
Copy link
Copy Markdown
Contributor Author

Thanks for the review. I've marked the --marketplace-output flag as deprecated with the suggestion to use the config file. There's the possibility of adding --claude-output, --codex-output, but that should probably be considered from the overall pack cli standpoint.

@danielmeppiel
Copy link
Copy Markdown
Collaborator

Hey @henrydennis — thanks for the deprecation framing in 66346756; that's the move that lets us treat the canonical-flag work as a separate, focused conversation rather than blocking this PR.

We've opened two follow-ups so the "overall pack CLI standpoint" question you flagged has a tracked home:

No action needed on this PR — the foundation here (MarketplaceOutputProfile registry + per-profile BuildReport) is exactly the surface those issues build on. Happy to ship as-is.

@danielmeppiel danielmeppiel added this pull request to the merge queue May 14, 2026
Merged via the queue into microsoft:main with commit a59709b May 14, 2026
11 checks passed
@henrydennis henrydennis deleted the feature/multi-marketplace-outputs branch May 14, 2026 10:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status/accepted Direction approved, safe to start work. status/triaged Initial agentic triage complete; pending maintainer ratification (silence = approval).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants