Skip to content

fix: install versioned marketplace package directories - #3156

Open
sama Pyb (Pybsama) wants to merge 16 commits into
microsoft:mainfrom
Pybsama:codex/fix-versioned-marketplace-dirs
Open

sama Pyb (Pybsama) wants to merge 16 commits into
microsoft:mainfrom
Pybsama:codex/fix-versioned-marketplace-dirs

Conversation

@Pybsama

@Pybsama sama Pyb (Pybsama) commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Description

Installing my-plugin@marketplace fails when the catalog points at a versioned directory produced by apm pack, such as plugins/my-plugin-1.2.3. Shorthand parsing mistakes the dotted suffix for an unsupported file extension.

Use the existing explicit Git/path reference when the marketplace already identifies the repository and path. Keep that interpreted identity through selective installation and dry-run planning using the existing package-selection module instead of reparsing an ambiguous display string. GitHub/GHE Contents API success requires a directory listing, not merely HTTP 200. The Git fallback requires an actual tree entry, so file, symbolic-link, and submodule responses cannot bypass directory validation.

Supported individual files, package-content validation, containment checks, bundle naming, and local marketplace behavior retain their existing contracts.

Issue and approved scope

Fixes #3078.

Human scope approval: #3078 (comment)

This completes the bounded versioned-directory installation repair; it does not change the bundle layout from #2432.

Type of change

  • Bug fix
  • Documentation

Testing

  • Tested locally
  • Added regression tests

The hermetic lifecycle test runs apm pack, commits the generated bundle to a local Git repository, previews installation without changing the consumer, verifies the first install deploys the expected skill, and replays the lockfile. Semver, calendar versions, and the local-marketplace control are covered. Negative cases reject unknown files, version-named blobs, malformed package content, and traversal without writing the consumer manifest or lockfile; real Git tree probes also reject symbolic links.

The GitHub catalog fetch is substituted at the transport boundary with the existing Git fetcher against the committed local fixture. Source classification, resolution, download, validation, deployment, and lock replay run normally with external network access denied. This is not a live GitHub API integration test.

Focused validation:

  • Affected API/downloader/selection tests: 191 passed, including 44 API-response cases and ten real Git cases. GitHub/GHE directory arrays (including empty arrays), non-directory responses, malformed JSON, explicit-ref fallback, and default-ref validation are covered.
  • Authentication, anonymous-first, and throttling contracts: 90 passed.
  • Selector/install-phase/dry-run and architecture contracts: 266 passed.
  • Marketplace classification and round-trip cases: 16 passed.
  • Hermetic pack/marketplace/install lifecycle: 8 passed.

These focused runs overlap; their counts are not a whole-suite total.

The six CI test-quality/architecture-ratchet modules also pass locally: 91 passed. Full-source Ruff lint/format, pylint duplication checks, architecture/auth boundaries, YAML I/O/file-length/portable-path guards, and git diff --check pass.

A broader earlier run had 4,791 passes and 25 failures: one introduced module-length violation was fixed by the shared selection helper and passes in the final checks; 24 Unix installer fixture failures came from writing the Chinese checkout's Python path as ASCII. The same encoding failure was reproduced using an unchanged HEAD fixture. The full suite is therefore not claimed green, and Windows was not tested locally.

Spec conformance (OpenAPM v0.1)

No normative requirement changes. This repairs consumption of existing explicit Git/path coordinates and preserves path validation, package-content checks, and lockfile identity; it introduces no new manifest syntax or bundle format.

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.

Copilot review overview

🟡 Changes recommended

The Contents API path can still accept non-directory entries before the stricter Git tree check runs.

Review effort: Balanced
Findings: 1 High severity · 1 Low severity

Open (2)
What changed in this PR

Fixes installation of versioned marketplace package directories while preserving structured Git/path references.

Changes:

  • Resolves dotted marketplace directories as explicit Git/path dependencies.
  • Preserves structured identities during selective installs and dry runs.
  • Adds Git tree validation and regression coverage.
File Description
.apm/​architecture/​owners/​install-deployment.json Registers package-selection ownership.
scripts/​architecture_linter/​checks/​install_dry_run_plan.py Guards structured selector usage.
src/​apm_cli/​deps/​github_downloader_validation.py Tightens Git tree probing.
src/​apm_cli/​install/​dry_run_plan.py Preserves interpreted dependency identities.
src/​apm_cli/​install/​package_selection.py Centralizes selector identity resolution.
src/​apm_cli/​install/​phases/​resolve.py Uses structured references during filtering.
src/​apm_cli/​marketplace/​resolver.py Handles dotted marketplace directory paths.
tests/​integration/​test_architecture_owner_rule_mutations.py Tests architecture guard enforcement.
tests/​integration/​test_marketplace_versioned_directory_lifecycle.py Covers the complete install lifecycle.
tests/​unit/​deps/​test_github_downloader_validation.py Updates expected Git tree output.
tests/​unit/​deps/​test_versioned_directory_probe.py Tests tree, blob, symlink, and gitlink probes.
tests/​unit/​install/​phases/​test_resolve_selective_update_plan.py Tests selective filtering and descendants.
tests/​unit/​install/​test_dry_run_render.py Tests dry-run identity preservation.
tests/​unit/​marketplace/​test_versioned_directory_resolution.py Tests classification and traversal rejection.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/apm_cli/deps/github_downloader_validation.py
Comment thread tests/unit/install/phases/test_resolve_selective_update_plan.py Outdated
sama Pyb (Pybsama) and others added 11 commits October 3, 2026 13:58
Preserve the approved issue microsoft#3078 repair while incorporating current main before exact-head verification.

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

Copilot-Session: f79e464c-180e-4be3-9056-0773aa679a1a
Fold the PR microsoft#3156 panel follow-ups without changing the approved issue microsoft#3078 boundary: retain reusable dependency sequences, distinguish explicit Git paths from shorthand, and keep one Unreleased Fixed section.

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

Copilot-Session: f79e464c-180e-4be3-9056-0773aa679a1a
Keep recent MCP, specification and documentation changes intact before final exact-head verification.

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

Copilot-Session: f79e464c-180e-4be3-9056-0773aa679a1a
Preserve trusted current workflow updates without broadening the marketplace directory repair.

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

Copilot-Session: f79e464c-180e-4be3-9056-0773aa679a1a
Preserve main policy and audit ownership changes while retaining the bounded marketplace directory repair.

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

Copilot-Session: f79e464c-180e-4be3-9056-0773aa679a1a
Preserve the strict default relocation behavior and bounded prune opt-in from main while adding existing-requirement conformance evidence for issue microsoft#3078.

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

Copilot-Session: f79e464c-180e-4be3-9056-0773aa679a1a
Exercise req-mf-008 through real pack, marketplace registration, dry-run, install, and lock replay for dotted Git directories. Reuse the existing lifecycle harness and regenerate conformance counts without a waiver or normative expansion. Update legacy conflict tests to patch DependencyReference at its canonical lookup site, preserving all assertions.

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

Copilot-Session: f79e464c-180e-4be3-9056-0773aa679a1a
Preserve upstream validation, transport fallback and architecture coverage before exact-head CI recovery validation.

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

Copilot-Session: f79e464c-180e-4be3-9056-0773aa679a1a
@danielmeppiel

Daniel Meppiel (danielmeppiel) commented Oct 15, 2026 •

Copy link
Copy Markdown
Collaborator

Conflict recovery: blocked by shared matrix coverage

Versioned Git marketplace directories install and replay correctly without weakening supported-file, containment, or package-content validation.

This PR is not ready: an exact-head architecture matrix check reproduces the notified main-baseline defect described below. Green technical CI and the narrower passing tests do not override that failure. The genuine prior panel's ship_now remains historical review evidence for the unchanged implementation, not a new recommendation to land this head. Original authorship by sama Pyb (@Pybsama) and existing reviewer ownership are preserved.

Faithful conflict recovery

The prior reviewed integration head was 6564e2c07bd1889c5a0c4207afa2f70fc0528628. It was merged with the agreed final batch main, b71cb47cd99e665d285a497ba6d05e810ae42ea4, without rewriting contributor history.

The only conflicting path was tests/integration/test_architecture_owner_rule_mutations.py: #3156's selector-owner test and #3155's hook-sidecar-owner test were independently inserted at the same location. The resolved file is exactly main's file plus the unchanged decorated selector test from this PR. Both complete functions, every parameter case, and all assertions were preserved byte-for-byte. Reviewed marketplace production guards and lifecycle/conformance tests are unchanged. No semantic scope change or broad panel rerun was needed.

The original panel's selector Sequence annotation, CHANGELOG cleanup, and explicit Git/path guidance remain folded. The genuine final test-coverage/CEO delta remains the review evidence for the earlier test-only recovery and existing req-mf-008 coverage. No normative expansion or waiver was used.

Human scope remains the unchanged accepted boundary. Both author-addressed Copilot threads remain resolved. Counters remain three outer iterations, two Copilot rounds, and one CI recovery.

Exact-head validation

  • Published head: 8fe8193c0517e16a43ab09827488b5485c6541d8.
  • Integrated and locally tested base: b71cb47cd99e665d285a497ba6d05e810ae42ea4.
  • 21 tests passed on this exact head: real CLI marketplace pack/register/preview/install/lock replay, rejection controls, both preserved owner-test functions, interpreted selection, req-mf-008, hook-sidecar drift lifecycle, and conservative local cleanup.
  • All eight current CI lint steps passed locally, including Ruff lint/format, YAML I/O, file length, portable paths, duplication, authentication boundaries, and architecture boundaries.
  • Deterministic version-2 owner evidence and executed functional coverage verified both touched canonical owners.
  • A fresh mutation check bypassed the two static guards in memory: both selector cases and all four hook-sidecar cases failed at their intended assertions, with zero errors. Original guard bindings were restored, tracked source bytes stayed unchanged, and the restored 21-test run passed.
  • Earlier production/CLI mutation proof is retained through verified unchanged guard and regression bytes; it is not claimed as newly rerun.
  • Canonical conformance regeneration produced no diff; the isolated Mode B check passed without a waiver.

Readiness blocker

The separately executed check
tests/integration/test_architecture_owner_rule_mutations.py::test_matrix_covers_every_registry_guard_exactly_once
failed on the published head: the registry contains registry-delegation-build-id-verification, but the mutation matrix has no corresponding case (1 failed in 4.27s). This is the same shared baseline failure reported during wind-down. The faithful conflict resolution retained main's matrix; it did not remove a case.

No baseline repair, waiver, new PR, or scope expansion was attempted. Readiness remains blocked until the responsible maintainer addresses the shared defect and the complete relevant validation is rerun under a new instruction.

Technical CI

All six standard workflows completed successfully on the published head:

Latest observed main remains the tested base b71cb47cd99e665d285a497ba6d05e810ae42ea4. Both Python and Actions CodeQL analyses match current synthetic merge 3ceb3a27f5f431dde1851c5aa76a898bfa4a6685, with zero open alerts. These successful workflows do not make the separately failed matrix gate pass.

Landing boundary

GitHub reports MERGEABLE / BLOCKED. The text conflict is resolved, but overall readiness is blocked as recorded above. The PR remains open and non-draft. Under the release wind-down instruction, it will not be merged or queued, and no merge automation, reviewer request, policy change, successor PR, or further work will be started after the final handoff.


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

Faithfully integrate current main for PR microsoft#3156. Preserve every release entry from both parents and carry forward the reviewed versioned-directory implementation and regression guards unchanged.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: f79e464c-180e-4be3-9056-0773aa679a1a
Preserve the intervening Claude cleanup and dependency updates while retaining the reviewed marketplace repair. This is faithful integration, with no new product behavior or policy change.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: f79e464c-180e-4be3-9056-0773aa679a1a
Integrate main microsoft#3190 faithfully, preserving conservative historical output handling. Keep the unchanged microsoft#3156 entry within Unreleased Fixed but after existing entries to avoid repeated adjacent insert conflicts.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: f79e464c-180e-4be3-9056-0773aa679a1a
Faithfully retain both adjacent architecture-test additions without changing either decorated function or reviewed marketplace behavior.

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

Copilot-Session: f79e464c-180e-4be3-9056-0773aa679a1a

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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] apm install <plugin>@<marketplace> rejects the versioned bundle directories apm pack produces ("should not have a file extension")

3 participants