GeezerVison

@microsoft-github-policy-service agree

Thank you for implementing the scoped fix. Maintainer approval for the bounded scope is recorded in #3283, but neither the PR nor linked issue currently carries status/accepted. Under the triage rule, this PR is labelled status/deferred until the maintainer reconciles that status. This recommendation is advisory only, not merge approval.

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

APM Review Panel: ship_now

Zero Contents API calls for public marker validation: a community-contributed raw-first sweep that removes the main rate-limit friction for anonymous installs of nested packages and Claude plugins.

panel-mode=full; personas=python-architect,auth-expert,performance-expert,test-coverage-expert,doc-writer,apm-ceo (terminal delta: doc-writer, no new findings)

cc ShaoLOWu Sergio Sisternes (@sergio-sisternes-epam) -- a fresh advisory pass is ready for your review.

All five panelists converge on ship. The architect confirms the raw_only seam threads cleanly through the existing shared download chain with no canonical-owner violation and no new downloader or cache. Auth finds no regressions: the raw path stays token-free, the sweep is gated to public github.com, and private recovery, throttle propagation and GHE/ADO/GitLab/SSH behavior are unchanged. Performance confirms the Done-when request budget, measured through the installed CLI. Test coverage confirms the scope's evidence list is met by new lifecycle tests plus existing throttle/SSH/other-host suites. All doc-writer findings were folded.

The one open item is a performance nit: on full-miss and private fallback paths, the per-marker loop repeats seven raw CDN 404s that the sweep already saw. This is bounded, un-metered, and explicitly accepted by the #3283 scope record; removing it needs a new inverse seam outside this PR.

Aligned with: pragmatic as npm (public nested installs validate markers with zero API calls); secure by default (anonymous-first and private recovery preserved); multi-harness/multi-host (sweep limited to github.com, other hosts untouched); OSS community driven (community contribution, CLA signed, maintainer folds for tests and docs).

Panel summary

Measured request counts (installed CLI, loopback HTTPS fixture)

Verified against both the uv console script and a locally built frozen binary.

Recommended follow-ups

• (performance-expert) Avoid the repeated raw CDN pass after a missed sweep. Bounded today; would need an inverse api-only seam through the download chain.

Folded in this run

• (copilot) Host-boundary unit test now uses a real acme.ghe.com dependency through the real resolver and asserts the CDN is never contacted -- resolved in 0818391.
• (scope) Installed-CLI request-count and state lifecycle tests via a loopback MITM HTTPS fixture: multi-plugin, repeat, --update, missing marker, private recovery, marketplace add reuse plus marketplace installs, and no registration after failed discovery -- resolved in f75184a.
• (scope) Docs for raw-first marker validation and catalog reuse (getting-started authentication, marketplace reference, apm-guide) -- resolved in e7a840a.
• (scope) CHANGELOG entry crediting ShaoLOWu -- resolved in 595034e.
• (doc-writer) Raw-first behavior stated once in the apm-guide Install validation chain with all seven markers; accurate getting-started wording; shorter changelog and marketplace note -- resolved in ae1007a.
• (python-architect) Probe comment names _auto_detect_path_with_payload; commit carries the apm-spec-waiver line for the Mode B gate (request scheduling only, no OpenAPM-observable delta) -- resolved in 68de490.

Copilot signals reviewed

• src/apm_cli/commands/marketplace/__init__.py:451 -- LEGIT: stale _auto_detect_path patches would hit the real fetch path (resolved in 3bfc1f4 by the author).
• tests/unit/deps/test_virtual_package_cdn_first_sweep.py -- LEGIT: boundary test forced the resolver flag on a github.com dependency (resolved in 0818391).
• tests/unit/deps/test_virtual_package_cdn_first_sweep.py:192 -- LEGIT: single-file probe left the Contents API unmocked (resolved in 3bfc1f4 by the author).

Deferred (out-of-scope follow-ups)

• (panel) Skip the repeated raw CDN pass on full-miss/private fallback -- scope boundary: [BUG] Validating a public in-repo plugin spends one anonymous API call per marker the CDN misses (four for a .claude-plugin plugin) #3283 accepts extra bounded raw attempts on fallback-only paths; avoiding them needs a new inverse api-only seam; suggested follow-up: perf: avoid duplicate raw CDN probes after a missed marker sweep.

Regression-trap evidence (mutation-break gate)

• test_public_nested_plugins_validate_without_contents_api -- deleted the raw-only marker sweep; test FAILED as expected; guard restored.
• test_marketplace_add_reuses_discovered_catalog_and_installs_without_contents_api -- deleted discovery_payload reuse; test FAILED as expected; guard restored.
• TestRawCdnFirstSweep::test_non_public_host_skips_sweep -- deleted the host gate in _raw_cdn_first_applies; test FAILED as expected; guard restored.

Lint contract

ruff check and ruff format --check on src/ tests/ silent; pylint R0801 10.00/10; lint-auth-signals.sh and lint-architecture-boundaries.sh clean. Canonical-owner gate: two existing owners touched (transport selection, single-file layout) as an owner extension; exact-head functional tests passed (242 and 24) and the semantic verifier passed.

CI

All checks green on 68de490: 28 pass, 3 skipping, 0 failing, including Lint, Build and Test shards, Lifecycle Smoke shards (which run the new lifecycle file), Windows gates, Spec conformance and the required-checks aggregator (after 1 CI fix iteration: Spec conformance Mode B waiver).

Mergeability status

Labels were not changed. The approved apm-scope:v1 record on #3283 sets the scope for this PR.

flowchart TD
    A["validate_virtual_package_exists"] --> C{"public github.com?"}
    C -- "yes" --> D["sweep markers raw_only=True"]
    D -- "raw hit" --> G["valid, zero Contents API"]
    D -- "all miss" --> J["unchanged per-marker loop: raw then Contents API with auth fallback"]
    C -- "no: GHE / ADO / other" --> J
    J -- "hit" --> G
    J -- "miss" --> N["directory Contents API, then ls-remote + ls-tree"]
    N -- "hit" --> G
    N -- "miss" --> P["invalid, diagnostic"]
    MA["marketplace add"] --> MB["_auto_detect_path_with_payload"]
    MB -- "path + catalog" --> MF["fetch_marketplace discovery_payload=catalog"]
    MF --> MJ["parse + write cache, no second fetch"]
    MB -- "none" --> ME["exit 1, nothing registered"]

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