Skip to content

Refactor: marketplace commands into a package and refresh maintainer docs#853

Open
shreejaykurhade wants to merge 1 commit intomicrosoft:feat/722-marketplace-maintainer-uxfrom
shreejaykurhade:feat/722-marketplace-maintainer-ux
Open

Refactor: marketplace commands into a package and refresh maintainer docs#853
shreejaykurhade wants to merge 1 commit intomicrosoft:feat/722-marketplace-maintainer-uxfrom
shreejaykurhade:feat/722-marketplace-maintainer-ux

Conversation

@shreejaykurhade
Copy link
Copy Markdown

@shreejaykurhade shreejaykurhade commented Apr 22, 2026 •
edited
Loading

Summary

This PR refactors the apm marketplace command surface from a large single module into a package-based layout with one file per substantial subcommand. Issue #821 taken from PR #790

The goal is to make the marketplace CLI easier to review, maintain, and extend without changing behavior.

What Changed

Command structure

Replaced the monolithic marketplace command module with:

  • src/apm_cli/commands/marketplace/__init__.py
  • src/apm_cli/commands/marketplace/init.py
  • src/apm_cli/commands/marketplace/build.py
  • src/apm_cli/commands/marketplace/validate.py
  • src/apm_cli/commands/marketplace/check.py
  • src/apm_cli/commands/marketplace/outdated.py
  • src/apm_cli/commands/marketplace/doctor.py
  • src/apm_cli/commands/marketplace/publish.py
  • src/apm_cli/commands/marketplace/plugin/__init__.py
  • src/apm_cli/commands/marketplace/plugin/add.py
  • src/apm_cli/commands/marketplace/plugin/set.py
  • src/apm_cli/commands/marketplace/plugin/remove.py

Shared logic and compatibility

  • Kept click group wiring and shared marketplace helpers in src/apm_cli/commands/marketplace/__init__.py
  • Preserved compatibility for older imports via src/apm_cli/commands/marketplace_plugin.py
  • Added targeted docstrings and type hints across the marketplace command package
  • Fixed type-checking issues surfaced during the refactor

Documentation

Updated maintainer-facing docs to reflect the new package layout:

  • docs/src/content/docs/contributing/development-guide.md
  • docs/src/content/docs/contributing/integration-testing.md
  • tests/integration/marketplace/README.md

Behavior / Risk

  • No functional changes intended
  • This is a structural refactor plus documentation/type-hint cleanup
  • Existing import paths used by tests were preserved through re-exports

Verification

Tests

pytest tests/unit/commands/test_marketplace_build.py \
  tests/unit/commands/test_marketplace_check.py \
  tests/unit/commands/test_marketplace_doctor.py \
  tests/unit/commands/test_marketplace_init.py \
  tests/unit/commands/test_marketplace_outdated.py \
  tests/unit/commands/test_marketplace_plugin.py \
  tests/unit/commands/test_marketplace_publish.py \
  tests/unit/marketplace/test_marketplace_commands.py \
  tests/unit/marketplace/test_marketplace_validator.py -q

@danielmeppiel danielmeppiel mentioned this pull request Apr 23, 2026
Merged
danielmeppiel added a commit that referenced this pull request Apr 23, 2026
@danielmeppiel
ci: add merge-gate orchestrator to harden against dropped pull_reques…
9d2d0e6 
…t webhooks (#865)

* ci: add merge-gate orchestrator (shadow) + stuck-PR watchdog

PR #856 surfaced a structural CI fragility: required status checks are
satisfied by two independent webhook channels (pull_request emits
'Build & Test (Linux)', pull_request_target emits the four Tier 2
stubs). When the pull_request delivery is dropped, 4/5 stubs go green
and the 5th hangs in 'Expected -- Waiting' forever -- ambiguous yellow
indistinguishable from a slow build. Recovery is folklore.

This PR ships two safety nets in shadow mode:

* .github/workflows/merge-gate.yml + scripts/ci/merge_gate_wait.sh
  Single orchestrator that polls the Checks API for 'Build & Test
  (Linux)' on the PR head SHA and aggregates into one verdict. Triggers
  on BOTH pull_request and pull_request_target so a single dropped
  delivery is recoverable; concurrency dedupes. Times out cleanly with
  a clear error message if Tier 1 never dispatched -- turning the
  invisible failure into a loud red check. NOT YET REQUIRED -- shadow
  observation first, ruleset flip after merge.

* .github/workflows/watchdog-stuck-prs.yml + scripts/ci/watchdog_scan.sh
  Cron every 15 min. For open non-draft PRs with no ci.yml run on the
  head SHA AND non-paths-ignored files, posts one recovery comment.
  Backstop for any required check that stops dispatching.

Tested live (dry-run) against microsoft/apm: watchdog correctly
distinguishes stuck PRs (#853, #409) from docs-only PRs (#864, #461,
#825). Both scripts shellcheck-clean. merge_gate_wait.sh tested
end-to-end against PR #856 head SHA (success path, exit 0) and a
non-existent SHA (timeout path, exit 2 with clear error annotation).

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

* ci(merge-gate): handle self-bootstrap and use checkout on pull_request

Two fixes for the script-fetch step:

1. On 'pull_request' the runner has no secrets and a read-only token, so
   actions/checkout of PR head is safe -- use it for simplicity. We only
   need API-fetch under 'pull_request_target' where checkout would be a
   security risk.
2. On 'pull_request_target', when the script does not yet exist on base
   (i.e. THIS PR is the one adding it), curl returns 404 and we degrade
   to a self-bootstrap no-op pass instead of failing. Once the script
   lands on main, the gate becomes real.

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

* ci: address Copilot review feedback on PR #865

- merge_gate_wait.sh: use $EXPECTED_CHECK in failure annotation instead
  of hardcoded 'Build & Test (Linux)' so the orchestrator stays generic.
- merge-gate.yml: add curl --retry/--max-time on the script bootstrap
  fetch so a transient GitHub API blip does not redden the gate.
- watchdog_scan.sh: fail loudly with stderr capture if 'gh pr list'
  errors out, instead of silently treating it as zero PRs (which would
  hide auth regressions or rate limiting).
- watchdog_scan.sh: paginate the changed-files API call so PRs touching
  >100 files cannot be misclassified as docs-only and skipped.
- CHANGELOG: append (#865) to follow the repo convention.

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

* ci: drop watchdog -- gate's dual-trigger redundancy is sufficient

The watchdog (cron every 15min, posts recovery comments on stuck PRs)
was originally justified for the shadow-mode transition window. Since
we are flipping to required immediately after this PR merges, that
justification disappears.

The merge-gate workflow already triggers on both 'pull_request' and
'pull_request_target' with concurrency dedup, so a single dropped
webhook still fires the gate. The watchdog only added value for the
double-drop case (both webhook channels fail for the same PR), which
is vanishingly rare. We can add it back later if we ever observe one.

Removes:
- .github/workflows/watchdog-stuck-prs.yml
- .github/scripts/ci/watchdog_scan.sh

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

---------

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

Reviewers

@danielmeppiel danielmeppiel Awaiting requested review from danielmeppiel danielmeppiel is a code owner

@sergio-sisternes-epam sergio-sisternes-epam Awaiting requested review from sergio-sisternes-epam sergio-sisternes-epam is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@shreejaykurhade