Skip to content

feat: add GitHub Copilot marketplace output - #2600

Open
Aryan Singh K. (aryansk) wants to merge 4 commits into
microsoft:mainfrom
aryansk:feat/2430-copilot-marketplace-output
Open

Aryan Singh K. (aryansk) wants to merge 4 commits into
microsoft:mainfrom
aryansk:feat/2430-copilot-marketplace-output

Conversation

@aryansk

Copy link
Copy Markdown
Contributor

Closes #2430

Summary

  • register a copilot marketplace output profile with the Copilot CLI default path
  • emit Copilot marketplace metadata and plugin entries through a dedicated mapper
  • preserve local relative sources and remote pin/ref information
  • add unit coverage for the profile path, metadata shape, local sources, and remote pins

Validation

  • patch syntax and application context were checked against current main
  • repository tests could not be executed in this chat environment; CI should run the project test suite

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 support for generating a GitHub Copilot CLI-compatible marketplace artifact by introducing a new output profile and mapper, along with unit tests to validate the produced JSON.

Changes:

  • Register a new copilot marketplace output profile with a default discovery path.
  • Implement CopilotMarketplaceMapper to compose Copilot CLI marketplace JSON (including remote pin info).
  • Add unit tests covering local source preservation and remote pin emission.

Reviewed changes

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

File Description
tests/unit/marketplace/test_copilot_output_mapper.py Adds unit tests validating the new Copilot output profile and mapper behavior.
src/apm_cli/marketplace/output_profiles.py Registers the new copilot output profile and includes it in MARKETPLACE_OUTPUTS.
src/apm_cli/marketplace/output_mappers.py Introduces CopilotMarketplaceMapper and _copilot_source, and registers the mapper.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/apm_cli/marketplace/output_mappers.py Outdated
Comment thread src/apm_cli/marketplace/output_mappers.py
Comment thread src/apm_cli/marketplace/output_mappers.py Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 3 out of 3 changed files in this pull request and generated 2 comments.

Suppressed comments (6)

tests/unit/marketplace/test_copilot_output_mapper.py:70

  • Test functions here should follow the common def test_...(...) -> None: style used across tests/unit/marketplace/ for consistency.
def test_copilot_mapper_preserves_remote_pin_information():
    entry = PackageEntry(

src/apm_cli/marketplace/output_profiles.py:99

  • The new copilot profile declares config_attr="copilot", but MarketplaceConfig currently has no copilot config object (only claude/codex), and the apm.yml marketplace block rejects unknown keys (so marketplace: { copilot: ... } cannot be expressed). This makes config_attr effectively dead and contradicts the issue’s requested MarketplaceCopilotConfig / _APM_MARKETPLACE_KEYS integration.
COPILOT_MARKETPLACE_OUTPUT = MarketplaceOutputProfile(
    name="copilot",
    config_attr="copilot",
    default_output=".github/plugin/marketplace.json",
    mapper="copilot",
    path_env_var="APM_MARKETPLACE_COPILOT_PATH",
)

tests/unit/marketplace/test_copilot_output_mapper.py:6

  • This new test module doesn’t follow the established unit-test conventions in this repo (module docstring + from __future__ import annotations at top). Several other tests/unit/marketplace/* files use that header style for consistency.
from types import SimpleNamespace

from apm_cli.marketplace.output_mappers import CopilotMarketplaceMapper
from apm_cli.marketplace.output_profiles import MARKETPLACE_OUTPUTS
from apm_cli.marketplace.yml_schema import MarketplaceConfig, MarketplaceOwner, PackageEntry

tests/unit/marketplace/test_copilot_output_mapper.py:9

  • Type hints are consistently used in the surrounding unit tests (helpers and test functions use -> None, typed params, etc.). This file introduces un-annotated helpers/test functions, which is inconsistent and may violate the repo’s typing/style expectations.
def _resolved(**overrides):
    values = {

tests/unit/marketplace/test_copilot_output_mapper.py:41

  • Test functions here should follow the common def test_...(...) -> None: style used across tests/unit/marketplace/ for consistency.

This issue also appears on line 69 of the same file.

def test_copilot_profile_uses_default_discovery_path():
    profile = MARKETPLACE_OUTPUTS["copilot"]

    assert profile.default_output == ".github/plugin/marketplace.json"
    assert profile.mapper == "copilot"


def test_copilot_mapper_nests_marketplace_metadata_and_keeps_local_source():

src/apm_cli/marketplace/output_profiles.py:99

  • This change adds a new marketplace output format (copilot) and a new default output path. The user-facing docs/examples should be updated accordingly (e.g., docs/src/content/docs/reference/cli/pack.md sections that describe marketplace artifacts and show marketplace.outputs examples currently only mention claude/codex; the apm-guide usage docs under packages/apm-guide/.apm/skills/apm-usage/ likely need a corresponding update as well).
COPILOT_MARKETPLACE_OUTPUT = MarketplaceOutputProfile(
    name="copilot",
    config_attr="copilot",
    default_output=".github/plugin/marketplace.json",
    mapper="copilot",
    path_env_var="APM_MARKETPLACE_COPILOT_PATH",
)

Comment thread src/apm_cli/marketplace/output_profiles.py
Comment thread src/apm_cli/marketplace/output_mappers.py
@sergio-sisternes-epam

Copy link
Copy Markdown
Collaborator

APM Review Panel: ship_with_followups

Ship Copilot marketplace output now; follow up with docs update and url-source test before next release.

cc Aryan Singh K. (@aryansk) Daniel Meppiel (@danielmeppiel) -- a fresh advisory pass is ready for your review.

This PR cleanly extends APM's multi-harness story to the largest installed-base agent ecosystem. Architecture is pattern-consistent (frozen-dataclass profile + Base mapper subclass), performance is neutral, and no auth or logging surfaces are touched. All nine panelists converged on approval with no blocking findings -- the strongest consensus signal a panel can emit.

The substantive gaps are documentation (doc-writer surfaced three in-place edits to a single page) and one untested code branch (the url-source path for non-GitHub hosts). The test-coverage-expert marked this as outcome:missing on a portability-by-manifest principle surface, which per our weighting rules elevates it above opinion-only nits. Supply-chain's metadata-sanitization recommendation is sound defense-in-depth but non-urgent: the mapper is a local serializer, not a resolver, and the consumed metadata already passed through APM's resolution pipeline. I side with shipping now and tracking sanitization as a hardening follow-up across all three mappers, not just this one.

Strategically, this is the PR that completes the 'one manifest -> Claude, Codex, and Copilot' triptych. Delaying it for docs or a single test branch costs us a launch beat with no proportional safety gain. The follow-ups are bounded (one page edit, one test function, one regex guard) and should land same-week.

Aligned with: Multi-harness support -- core delivery of this PR; Copilot joins Claude and Codex as first-class output profiles. Portability-by-manifest -- the url-source test gap (outcome:missing) weakens the portability guarantee for non-GitHub hosts -- follow-up required before release notes can claim full coverage. Secure-by-default -- no regression; supply-chain findings are hardening recommendations, not regressions from the current baseline.

Growth signal. Copilot is the largest installed-base agent. This PR unlocks a launch beat: 'One manifest -> Claude, Codex, and now Copilot.' Recommend a short social/blog post timed to the release that includes the marketplace docs page with a Copilot tab. The README hero line already mentions Copilot -- add the one-liner command example (apm marketplace pack --output copilot) as a 60-second proof point.

Panel summary

Persona B R N Takeaway
Python Architect 0 0 2 Clean additive feature that follows the established Base+Subclass mapper pattern and frozen-dataclass profile pattern exactly. No architectural concerns.
CLI Logging Expert 0 0 0 No CLI output, logging, or diagnostic rendering is introduced or modified by this PR. No concerns.
DevX UX Expert 0 1 1 Profile is well-structured; empty-string description emitted instead of omitting the field, and default output path is non-standard without docs.
Supply Chain Security Expert 0 2 1 Read-only output serializer with moderate concerns: unsanitized remote metadata and unvalidated SHA field.
OSS Growth Hacker 0 1 2 Strong story-fit; adding Copilot as first-class target reinforces 'one manifest, every agent' positioning.
Auth Expert -- -- -- Inactive: no auth, token, credential, or remote-host surfaces touched.
Doc Writer 0 3 1 Three in-place edits needed on publish-to-a-marketplace.md: inventory, YAML example, and .gitignore block all omit the new copilot output.
Test Coverage Expert 0 1 0 Unit tests cover profile registration, local source, and remote-pin branches; the url-source branch (remote host without subdir) is untested.
Performance Expert 0 0 0 No performance concerns. Mapper is O(n) with dict-indexed lookups, mirrors existing mappers.

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

Top 5 follow-ups

  1. [Test Coverage Expert] Add url-source branch test for non-GitHub hosts -- outcome:missing on portability-by-manifest surface; one test function, bounded scope.
  2. [Doc Writer] Update publish-to-a-marketplace.md: inventory, YAML example, .gitignore block -- three in-place edits to one page; users enabling copilot output will hit silent .gitignore failures without the guidance.
  3. [Supply Chain Security Expert] Add SHA format validation (40-char hex regex) before emission -- defense-in-depth for artifact integrity; apply across all three mappers in a single follow-up PR.
  4. [DevX UX Expert] Omit empty-string description instead of emitting it -- aligns with Claude mapper behavior and improves discoverability of missing data.
  5. [Supply Chain Security Expert] Add remote-metadata length cap and control-char strip -- supply-chain hardening; non-urgent since mapper is local-only, but good hygiene before the output format is considered stable.

Architecture

classDiagram
    direction LR
    class MarketplaceOutputMapper {
      <<ABC>>
      +uses_remote_metadata bool
      +compose(config, resolved, remote_metadata) MapperResult
    }
    class ClaudeMarketplaceMapper {
      +compose(...) MapperResult
    }
    class CodexMarketplaceMapper {
      +compose(...) MapperResult
    }
    class CopilotMarketplaceMapper {
      +uses_remote_metadata = True
      +compose(...) MapperResult
    }
    class MarketplaceOutputProfile {
      <<ValueObject frozen>>
      +name str
      +config_attr str
      +default_output str
      +mapper str
      +path_env_var str
    }
    class MapperResult {
      <<ValueObject>>
      +doc dict
      +warnings tuple
      +diagnostics tuple
    }
    class MarketplaceBuilder {
      +build(profile, config, resolved)
    }
    MarketplaceOutputMapper <|-- ClaudeMarketplaceMapper
    MarketplaceOutputMapper <|-- CodexMarketplaceMapper
    MarketplaceOutputMapper <|-- CopilotMarketplaceMapper
    MarketplaceBuilder ..> MarketplaceOutputProfile : selects
    MarketplaceBuilder ..> MarketplaceOutputMapper : delegates compose
    MarketplaceOutputMapper ..> MapperResult : returns
    class CopilotMarketplaceMapper:::touched
    class MarketplaceOutputProfile:::touched
    classDef touched fill:#fff3b0,stroke:#d47600
Loading
flowchart TD
    A["apm pack --marketplace copilot"] --> B["MarketplaceBuilder.build()"]
    B --> C{"Profile lookup\nMARKETPLACE_OUTPUTS['copilot']"}
    C --> D["resolve_effective_output_path()\n.github/plugin/marketplace.json"]
    C --> E["MARKETPLACE_OUTPUT_MAPPERS['copilot']"]
    E --> F["CopilotMarketplaceMapper.compose()"]
    F --> G["_sanitized_name_with_diagnostic()"]
    F --> H{"For each resolved pkg"}
    H --> I["_copilot_source(entry, pkg)"]
    I --> J{"entry.is_local?"}
    J -->|Yes| K["return entry.source (str)"]
    J -->|No| L["_remote_source_url(pkg)"]
    L --> M["Build source dict with ref/sha/path"]
    F --> N["Return MapperResult"]
    N --> O["Write marketplace.json"]
Loading

Recommendation

Merge now. The PR delivers a complete, pattern-consistent, well-tested Copilot marketplace output that directly advances APM's multi-harness positioning. The two material follow-ups (url-source test + docs page update) should land same-week and can be tracked as a paired issue. No blocking findings from any panelist.


Full per-persona findings

Python Architect

  • [nit] Missing type annotations on compose parameters in diff snippet at src/apm_cli/marketplace/output_mappers.py:336
    The actual committed code has full type annotations confirmed in-tree. The PR diff description was misleading but the code is correct. No action needed.
  • [nit] entry.version fallback on local packages may emit None-ish empty string at src/apm_cli/marketplace/output_mappers.py:370
    Line 370: version = entry.version if entry.is_local else meta.get('version') or entry.version -- if entry.version is an empty string, the if version: guard on line 371 correctly suppresses it. Intent is well-guarded.

CLI Logging Expert

No findings.

DevX UX Expert

  • [recommended] Empty-string description degrades discoverability of missing data at src/apm_cli/marketplace/output_mappers.py:358
    When neither entry.description nor remote_metadata supplies a description, the mapper emits "description": "". The Claude mapper omits the field entirely in this case, which is a better UX signal -- an absent key tells the user 'not configured yet' while an empty string looks like a deliberate blank. Consider omitting the field or emitting a diagnostic so users know to add one.
  • [nit] Default path .github/plugin/ is unconventional -- note in help/docs at src/apm_cli/marketplace/output_profiles.py:97
    .github/plugin/marketplace.json is not an established convention. Fine as a default, but the apm marketplace build --help or quickstart should mention it so users know where to look.

Supply Chain Security Expert

  • [recommended] Remote metadata written to artifact without validation at src/apm_cli/marketplace/output_mappers.py
    Values from remote_metadata (description, version) flow directly into the output JSON with no length cap, character-set check, or schema validation. A compromised or malicious registry response could inject oversized strings, control characters, or crafted payloads. Consider a lightweight sanitization pass -- type-assert str, cap length, and strip control chars.
  • [recommended] SHA field not validated as 40-char hex at src/apm_cli/marketplace/output_mappers.py
    pkg.sha is emitted into the source dict without asserting it is a full 40-character lowercase hex string. A truncated or malformed SHA weakens the integrity guarantee. A one-line regex guard before emission would enforce deterministic provenance.
    Suggested: if pkg.sha and re.fullmatch(r'[0-9a-f]{40}', pkg.sha): source["sha"] = pkg.sha
  • [nit] Local source path emitted verbatim at src/apm_cli/marketplace/output_mappers.py
    entry.source is returned as-is for local packages. Acceptable for a user-controlled local dev artifact, but if the output could ever be shared or committed, it leaks absolute filesystem paths. Worth a comment acknowledging this is intentional for local-only use.

OSS Growth Hacker

  • [recommended] README hero line already lists Copilot -- link the feature at README.md
    The hero line mentions GitHub Copilot but there is no visible mention of marketplace output anywhere a visitor would see it. Consider adding a one-liner showing apm marketplace pack --output copilot as a hook visitors can repost.
  • [nit] Default path discoverability for docs/quickstart at src/apm_cli/marketplace/output_profiles.py
    The default output .github/plugin/marketplace.json is buried in code. A short doc comment or entry in the quickstart/marketplace docs page showing the happy-path command would turn this into a 60-second proof point for new users.
  • [nit] CopilotMarketplaceMapper docstring could carry a story-shaped sentence at src/apm_cli/marketplace/output_mappers.py
    The one-line class docstring could double as copy source for release notes. Something like 'Generate a Copilot-compatible plugin registry from your apm.yml manifest' reads better in changelogs and social posts.

Auth Expert -- inactive

Inactive: no auth, token, credential, or remote-host surfaces are touched by this PR.

Doc Writer

  • [recommended] 'What lives where' inventory omits the copilot output at docs/src/content/docs/producer/publish-to-a-marketplace.md:59
    Lines 56-64 enumerate the files that apm pack writes but say nothing about .github/plugin/marketplace.json. A user who adds copilot to outputs: will have no documentation explaining what file is generated.
    Suggested: Add: - .github/plugin/marketplace.json -- optional Copilot CLI output. Enable it by adding 'copilot' to marketplace.outputs.
  • [recommended] YAML outputs: example does not show copilot at docs/src/content/docs/producer/publish-to-a-marketplace.md:87
    The annotated apm.yml scaffold shows outputs: { claude: {} } and documents codex: but does not include copilot. Users copying this example will not know copilot is a valid value.
  • [recommended] .gitignore pitfall block is missing the copilot unignore line at docs/src/content/docs/producer/publish-to-a-marketplace.md:262
    Lines 259-263 document the *.json in .gitignore trap but only list !.claude-plugin/marketplace.json and !.agents/plugins/marketplace.json. Users who enable copilot output will hit the same silent-skip problem but find no guidance.
    Suggested: Add !.github/plugin/marketplace.json to the unignore list.
  • [nit] End-to-end example hardcodes a single git add path at docs/src/content/docs/producer/publish-to-a-marketplace.md:34
    Line 34 shows git add apm.yml .claude-plugin/marketplace.json. A parenthetical note 'add all enabled output files' would prevent confusion for users enabling copilot or codex outputs.

Test Coverage Expert

  • [recommended] _copilot_source url branch (non-GitHub host, no subdir) has no test at tests/unit/marketplace/test_copilot_output_mapper.py
    The CopilotMarketplaceMapper has three remote-source branches: git-subdir, url, and github. Only the github branch is tested. The url branch -- triggered when _remote_source_url(pkg) returns a URL -- is not exercised. Grepped test file for 'url' -- only 1 hit (source_url=None fixture default).
    Proof (missing): tests/unit/marketplace/test_copilot_output_mapper.py::test_copilot_mapper_emits_url_source_for_non_github_host -- proves: Copilot marketplace JSON emits a clone-ready URL when packages come from a non-github.com host [portability-by-manifest, multi-harness-support]
    assert plugin['source'] == {'source': 'url', 'url': 'https://ghe.corp.example/acme/demo', 'ref': 'v1.0.0'}

Performance Expert

No findings.

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

@sergio-sisternes-epam Sergio Sisternes (sergio-sisternes-epam) removed the panel-review Request an advisory PR review; consumed after review. Not human approval or a merge gate. label Aug 18, 2026
@sergio-sisternes-epam

Copy link
Copy Markdown
Collaborator

PR triage recommendation

needs-issue

Recommendation only, not merge or scope approval. A responsible
human maintainer must approve. Labels, automated advice, and
silence are not approval.

Linked issue

#2430 -- open; labels include priority/low, theme/portability,
area/marketplace, area/docs-site, type/feature,
triage/recommended. It does not carry status/accepted.
A 2026-09-12 maintainer comment withdrew prior acceptance; that
withdrawal is the current human record. This PR therefore has no
same-repo linked issue labelled status/accepted.

Proposed classification

type/feature, area/marketplace, area/docs-site, theme/portability
(aligned with #2430; the PR had no classification labels).

Processing: triage/recommended. Auto-defer: status/deferred
(no linked status/accepted issue; the PR did not already have
status/accepted).

CODEOWNERS for this tree: Daniel Meppiel (@danielmeppiel) Sergio Sisternes (@sergio-sisternes-epam)
(already in reviewRequests; this comment does not add or remove
review requests).

Suggested next action

Wait for a responsible human maintainer to record bounded scope,
acceptance criteria, and review contact on #2430 (see CONTRIBUTING.md)
before treating this PR as invited implementation. Do not merge from
this recommendation.

Suggested PR comment

Thank you for contributing this pull request, @aryansk.

APM starts with an issue, not an implementation
(https://github.com/microsoft/apm/blob/main/CONTRIBUTING.md).
This PR correctly links #2430, but that issue is not currently
labelled `status/accepted`. Maintainer direction on 2026-09-12
withdrew prior acceptance, so previous labels or comments are not
approval to continue substantive implementation.

Please keep #2430 as the tracking issue and wait for a responsible
human maintainer to record scope, done-when criteria, and a review
contact there before this work is invited for inclusion. This PR is
labelled `status/deferred` until that happens. CODEOWNERS remain
@danielmeppiel and @sergio-sisternes-epam; this note does not
request additional review.

@sergio-sisternes-epam Sergio Sisternes (sergio-sisternes-epam) added triage/recommended Automated advice completed; not human scope approval. status/deferred Not invited for implementation now; no release commitment. type/feature New capability, new flag, new primitive. area/marketplace marketplace.json schema, federation, authoring suite, source parity. area/docs-site docs/src/content (Starlight), README, doc generation. theme/portability One manifest, every target. Multi-target deploy, marketplace, packaging, install. status/accepted Human scope approval; verify the issue's approval record and review contact before work. and removed status/deferred Not invited for implementation now; no release commitment. labels Sep 16, 2026
@sergio-sisternes-epam

Copy link
Copy Markdown
Collaborator

Thanks for the PR, Aryan Singh K. (@aryansk) - and for the Copilot marketplace output work on #2430.

Build & Test Shard 1 (Linux) and the Merge Gate are red on the current tip (unit tests still expect only claude/codex after the new copilot profile landed). Delivery is taking the CI fix from here on your branch, keeping your authorship, then driving the PR toward merge readiness for human approval.


Generated by autopilot-pr-ready-takeover. This comment is AI-generated and may contain errors.

@sergio-sisternes-epam Sergio Sisternes (sergio-sisternes-epam) added the panel-review Request an advisory PR review; consumed after review. Not human approval or a merge gate. label Oct 2, 2026
@sergio-sisternes-epam

Copy link
Copy Markdown
Collaborator

Thanks for the PR, Aryan Singh K. (@aryansk) - and for the Copilot marketplace output work on #2430.

Four open Copilot review threads on the marketplace mapper still needed a schema-aligned pass (relative-path source strings, omit empty description / unsupported plugin fields). Delivery is taking those thread fixes from here on your branch, keeping your authorship, then driving the PR toward merge readiness for human approval.


Generated by autopilot-pr-ready-takeover. This comment is AI-generated and may contain errors.

@sergio-sisternes-epam Sergio Sisternes (sergio-sisternes-epam) added panel-review Request an advisory PR review; consumed after review. Not human approval or a merge gate. and removed panel-review Request an advisory PR review; consumed after review. Not human approval or a merge gate. labels Oct 2, 2026
@github-actions github-actions Bot removed the panel-review Request an advisory PR review; consumed after review. Not human approval or a merge gate. label Oct 2, 2026
@github-actions

github-actions Bot commented Oct 2, 2026

Copy link
Copy Markdown

APM Review Panel: ship_with_followups

Copilot marketplace output now emits schema-aligned relative-path sources; code is sound, docs and a few tests are still missing.

panel-mode=lean; personas=python-architect,test-coverage-expert,doc-writer,apm-ceo

cc Aryan Singh K. (@aryansk) @danielmeppiel @sergio-sisternes-epam -- a fresh advisory pass is ready for your review.

What changed since the last advisory (2026-08-18). The mapper was rewritten on tip 5aeac29: plugins[].source is now always a relative path string, unsupported fields (author, tags, ref, sha, etc.) are omitted, empty descriptions are dropped, and all five Copilot review threads are resolved. The previously flagged url-source test gap is moot because that branch is gone. The remaining gaps are documentation and test depth, not correctness. Per the 2026-10-02 takeover note, delivery is finishing this on the contributor's branch; this pass is context for that work.

The one design point worth a maintainer's eye: remote packages with no subdir emit ./<pkg.name>, and git pin data (ref/sha/host) is dropped, so a curator's pins are not forwarded. If Copilot CLI truly accepts only relative strings (per #2430) that is a schema limit, but it should be commented in the code and ideally surfaced as a diagnostic.

Aligned with: multi-harness support (Copilot joins Claude and Codex as an output profile); portable-by-manifest (reproducibility trade-off above should be stated explicitly).

Panel summary

Persona B R N Takeaway
Python Architect 0 2 1 Pattern-consistent; config_attr="copilot" is dead (no config object / apm.yml key) and remote pin loss is undocumented.
Test Coverage Expert 0 2 1 No builder write_output test for copilot (codex has one); subdir normalization edges untested.
Doc Writer 0 3 0 Marketplace guide, pack reference, apm-guide resources and CHANGELOG all omit copilot.

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

Top 5 follow-ups

  1. [Doc Writer] Document copilot in publish-to-a-marketplace.md and reference/cli/pack.md (enable via outputs, default .github/plugin/marketplace.json, APM_MARKETPLACE_COPILOT_PATH, relative-string sources, no pin) -- users otherwise cannot discover the output; also add the .gitignore unignore line.
  2. [Doc Writer] Mirror the change in packages/apm-guide/.apm/skills/apm-usage/ (commands.md, package-authoring.md) and add a CHANGELOG [Unreleased] entry citing feat: add GitHub Copilot marketplace output #2600 -- repo rule 4 and changelog convention.
  3. [Test Coverage Expert] Add test_write_copilot_output_profile mirroring the codex builder test -- proves builder wiring, output path, and env override, not just the mapper.
  4. [Python Architect] Resolve the dead config_attr="copilot" (add a config dataclass + schema key, or use the existing no-config sentinel) -- avoids misleading maintainers.
  5. [Python Architect] Comment (or emit a diagnostic) where remote ref/sha are discarded and ./<name> is fabricated -- makes the reproducibility trade-off explicit. Optionally add subdir normalization cases (./x, x/) and from __future__ import annotations / -> None in the new test file.

Recommendation

Ship with follow-ups. No blocking findings; the schema rewrite resolved the earlier concerns. Fold the docs/CHANGELOG/apm-guide updates and the builder test into this PR (bounded, same-scope); the config_attr cleanup and pin diagnostic can follow. Merge decision stays with the CODEOWNERS (@danielmeppiel, @sergio-sisternes-epam).


Full per-persona findings

Python Architect

  • [recommended] config_attr="copilot" references a non-existent MarketplaceConfig attribute at src/apm_cli/marketplace/output_profiles.py
    MarketplaceConfig only has claude/codex, and _APM_MARKETPLACE_KEYS rejects a copilot: block. Resolution falls back to default_output, so no runtime bug, but the declaration is dead and misleading.
  • [recommended] Remote source fabricates a relative path and drops ref/sha/host pin at src/apm_cli/marketplace/output_mappers.py
    Claude emits a structured, reproducible source; Copilot emits ./<subdir> or ./<name>. Document why in the remote branch and consider a diagnostic when pins are discarded.
  • [nit] Header assembly (name, owner, metadata) duplicates Claude mapper logic at src/apm_cli/marketplace/output_mappers.py
    Acknowledged in the class docstring; a shared helper would reduce drift later.

Test Coverage Expert

  • [recommended] No write_output integration test for copilot at tests/unit/marketplace/test_local_path_compose.py
    Codex has test_write_codex_output_profile; copilot has only isolated mapper tests.
    Suggested: add test_write_copilot_output_profile asserting .github/plugin/marketplace.json path and nested metadata.
  • [recommended] Subdir normalization edge cases untested at tests/unit/marketplace/test_copilot_output_mapper.py
    Leading ./ preserved and trailing / stripped are not covered.
  • [nit] New test file omits from __future__ import annotations and -> None annotations used by sibling modules.

Doc Writer (ad-hoc: user-facing output added with no docs touched)

  • [recommended] Marketplace docs enumerate claude/codex only at docs/src/content/docs/producer/publish-to-a-marketplace.md
    Add copilot enablement, default path, env var, schema notes, and the relative-source/no-pin limitation; update reference/cli/pack.md artifact list and --marketplace-path row.
  • [recommended] apm-guide resources not updated at packages/apm-guide/.apm/skills/apm-usage/package-authoring.md
    Repo rule 4 requires mirroring format changes there and in commands.md.
  • [recommended] No CHANGELOG [Unreleased] entry at CHANGELOG.md
    Suggested: "apm pack can emit a GitHub Copilot marketplace.json (.github/plugin/marketplace.json) when copilot is in marketplace.outputs; override with APM_MARKETPLACE_COPILOT_PATH. (feat: add GitHub Copilot marketplace output #2600)"

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

Generated by PR Review Panel for #2600 · copilot · auto · 208 AIC · ⌖ 6.28 AIC · ⊞ 10K · ◷

@sergio-sisternes-epam

Copy link
Copy Markdown
Collaborator

APM Review Panel: ship_with_followups

Copilot marketplace output now emits schema-aligned relative-path sources; code is sound, docs and a few tests are still missing.

panel-mode=lean; personas=python-architect,test-coverage-expert,doc-writer,apm-ceo

cc Aryan Singh K. (@aryansk) Daniel Meppiel (@danielmeppiel) -- a fresh advisory pass is ready for your review.

What changed since the last advisory (2026-08-18). The mapper was rewritten on tip 5aeac29: plugins[].source is now always a relative path string, unsupported fields (author, tags, ref, sha, etc.) are omitted, empty descriptions are dropped, and all five Copilot review threads are resolved. The previously flagged url-source test gap is moot because that branch is gone. The remaining gaps are documentation and test depth, not correctness. Per the 2026-10-02 takeover note, delivery is finishing this on the contributor's branch; this pass is context for that work.

The one design point worth a maintainer's eye: remote packages with no subdir emit ./<pkg.name>, and git pin data (ref/sha/host) is dropped, so a curator's pins are not forwarded. If Copilot CLI truly accepts only relative strings (per #2430) that is a schema limit, but it should be commented in the code and ideally surfaced as a diagnostic.

Aligned with: multi-harness support (Copilot joins Claude and Codex as an output profile); portable-by-manifest (reproducibility trade-off above should be stated explicitly).

Panel summary

Persona B R N Takeaway
Python Architect 0 2 1 Pattern-consistent; config_attr="copilot" is dead (no config object / apm.yml key) and remote pin loss is undocumented.
Test Coverage Expert 0 2 1 No builder write_output test for copilot (codex has one); subdir normalization edges untested.
Doc Writer 0 3 0 Marketplace guide, pack reference, apm-guide resources and CHANGELOG all omit copilot.

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

Top 5 follow-ups

  1. [Doc Writer] Document copilot in publish-to-a-marketplace.md and reference/cli/pack.md (enable via outputs, default .github/plugin/marketplace.json, APM_MARKETPLACE_COPILOT_PATH, relative-string sources, no pin) -- users otherwise cannot discover the output; also add the .gitignore unignore line.
  2. [Doc Writer] Mirror the change in packages/apm-guide/.apm/skills/apm-usage/ (commands.md, package-authoring.md) and add a CHANGELOG [Unreleased] entry citing feat: add GitHub Copilot marketplace output #2600 -- repo rule 4 and changelog convention.
  3. [Test Coverage Expert] Add test_write_copilot_output_profile mirroring the codex builder test -- proves builder wiring, output path, and env override, not just the mapper.
  4. [Python Architect] Resolve the dead config_attr="copilot" (add a config dataclass + schema key, or use the existing no-config sentinel) -- avoids misleading maintainers.
  5. [Python Architect] Comment (or emit a diagnostic) where remote ref/sha are discarded and ./<name> is fabricated -- makes the reproducibility trade-off explicit. Optionally add subdir normalization cases (./x, x/) and from __future__ import annotations / -> None in the new test file.

Recommendation

Ship with follow-ups. No blocking findings; the schema rewrite resolved the earlier concerns. Fold the docs/CHANGELOG/apm-guide updates and the builder test into this PR (bounded, same-scope); the config_attr cleanup and pin diagnostic can follow. Merge decision stays with the CODEOWNERS (@danielmeppiel, @sergio-sisternes-epam).


Full per-persona findings

Python Architect

  • [recommended] config_attr="copilot" references a non-existent MarketplaceConfig attribute at src/apm_cli/marketplace/output_profiles.py
    MarketplaceConfig only has claude/codex, and _APM_MARKETPLACE_KEYS rejects a copilot: block. Resolution falls back to default_output, so no runtime bug, but the declaration is dead and misleading.
  • [recommended] Remote source fabricates a relative path and drops ref/sha/host pin at src/apm_cli/marketplace/output_mappers.py
    Claude emits a structured, reproducible source; Copilot emits ./<subdir> or ./<name>. Document why in the remote branch and consider a diagnostic when pins are discarded.
  • [nit] Header assembly (name, owner, metadata) duplicates Claude mapper logic at src/apm_cli/marketplace/output_mappers.py
    Acknowledged in the class docstring; a shared helper would reduce drift later.

Test Coverage Expert

  • [recommended] No write_output integration test for copilot at tests/unit/marketplace/test_local_path_compose.py
    Codex has test_write_codex_output_profile; copilot has only isolated mapper tests.
    Suggested: add test_write_copilot_output_profile asserting .github/plugin/marketplace.json path and nested metadata.
  • [recommended] Subdir normalization edge cases untested at tests/unit/marketplace/test_copilot_output_mapper.py
    Leading ./ preserved and trailing / stripped are not covered.
  • [nit] New test file omits from __future__ import annotations and -> None annotations used by sibling modules.

Doc Writer (ad-hoc: user-facing output added with no docs touched)

  • [recommended] Marketplace docs enumerate claude/codex only at docs/src/content/docs/producer/publish-to-a-marketplace.md
    Add copilot enablement, default path, env var, schema notes, and the relative-source/no-pin limitation; update reference/cli/pack.md artifact list and --marketplace-path row.
  • [recommended] apm-guide resources not updated at packages/apm-guide/.apm/skills/apm-usage/package-authoring.md
    Repo rule 4 requires mirroring format changes there and in commands.md.
  • [recommended] No CHANGELOG [Unreleased] entry at CHANGELOG.md
    Suggested: "apm pack can emit a GitHub Copilot marketplace.json (.github/plugin/marketplace.json) when copilot is in marketplace.outputs; override with APM_MARKETPLACE_COPILOT_PATH. (feat: add GitHub Copilot marketplace output #2600)"

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


Generated by autopilot-pr-review-worker. This comment is AI-generated and may contain errors.

Sergio Sisternes (sergio-sisternes-epam) pushed a commit to aryansk/apm that referenced this pull request Oct 2, 2026
Document copilot in pack/publish guides and apm-guide; add write_output
builder test; resolve dead config_attr; clarify remote pin/subdir trade-off
in the Copilot mapper (microsoft#2600 panel soft Top 5).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@sergio-sisternes-epam

Copy link
Copy Markdown
Collaborator

Thanks for the PR, Aryan Singh K. (@aryansk) — and for the Copilot marketplace output work on #2430.

A soft panel follow-up recommended a small docs/test/config cleanup on this tip (publish/pack docs, apm-guide mirror + CHANGELOG, a builder write_output test, the dead config_attr, and a clearer note where remote pins become relative paths). Delivery is taking those bounded follow-ups on your branch while keeping your authorship.

No further status drip on this thread unless the situation changes or you ask.


Generated by autopilot-pr-ready-takeover. This comment is AI-generated and may contain errors.

pr-relay and others added 4 commits October 2, 2026 18:35
Shard 1 failed because known_output_names and doctor format-coverage
fixtures still assumed only claude/codex after the new copilot profile.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Align CopilotMarketplaceMapper with microsoft#2430 / Copilot CLI marketplace.json:
relative-path source strings, omit empty description and unsupported
plugin fields (author/tags/sha/ref/etc.).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Document copilot in pack/publish guides and apm-guide; add write_output
builder test; resolve dead config_attr; clarify remote pin/subdir trade-off
in the Copilot mapper (microsoft#2600 panel soft Top 5).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@sergio-sisternes-epam

Copy link
Copy Markdown
Collaborator

Thanks for the PR, Aryan Singh K. (@aryansk) — and for the Copilot marketplace output work on #2430.

A soft panel follow-up recommended a small docs/test/config cleanup on this tip (publish/pack docs, apm-guide mirror + CHANGELOG, a builder write_output test, the dead config_attr, and a clearer note where remote pins become relative paths). Delivery took those bounded follow-ups on your branch (keeping your authorship) and rebased onto the latest base so the tip stays mergeable.

No further status drip on this thread unless the situation changes or you ask.


Generated by autopilot-pr-ready-takeover. This comment is AI-generated and may contain errors.

@sergio-sisternes-epam Sergio Sisternes (sergio-sisternes-epam) added the panel-review Request an advisory PR review; consumed after review. Not human approval or a merge gate. label Oct 2, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/docs-site docs/src/content (Starlight), README, doc generation. area/marketplace marketplace.json schema, federation, authoring suite, source parity. panel-review Request an advisory PR review; consumed after review. Not human approval or a merge gate. status/accepted Human scope approval; verify the issue's approval record and review contact before work. theme/portability One manifest, every target. Multi-target deploy, marketplace, packaging, install. triage/recommended Automated advice completed; not human scope approval. type/feature New capability, new flag, new primitive.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEATURE] Add "copilot" marketplace output format for GitHub Copilot CLI

3 participants