diff --git a/.apm/instructions/architecture.instructions.md b/.apm/instructions/architecture.instructions.md index 8f39a15bd..6818ab11a 100644 --- a/.apm/instructions/architecture.instructions.md +++ b/.apm/instructions/architecture.instructions.md @@ -73,6 +73,7 @@ semicolon-delimited, and specific to the file(s) that own the fact. | Local marketplace package-version manifest precedence | marketplace/version_check.py (_read_local_version) | `src/apm_cli/marketplace/version_check.py` | | applyTo normalization and hidden-tool placement | utils/patterns.py (normalize_apply_to); compilation/context_optimizer.py (ContextOptimizer) | `src/apm_cli/utils/patterns.py`; `src/apm_cli/compilation/context_optimizer.py` | | Effective marketplace output path | marketplace/output_profiles.py (resolve_effective_output_path) | `src/apm_cli/marketplace/output_profiles.py` | +| Marketplace metadata-enrichment outcome and certifiability | marketplace/builder.py (MetadataEnrichmentResult) | `src/apm_cli/marketplace/builder.py` | | Bootstrap project-name validation and fallback | core/project_name.py (resolve_bootstrap_project_name) | `src/apm_cli/core/project_name.py` | | Marketplace raw-structure diagnostics | marketplace/models.py parser; validator.py consumes them | `src/apm_cli/marketplace/models.py`; `src/apm_cli/marketplace/validator.py` | | Agent Plugins v1 contract interpretation, component discovery, and portable manifest authority | agent_plugins/loader.py (load_agent_plugin, _load_apm_configuration) | `src/apm_cli/agent_plugins/loader.py`; `src/apm_cli/agent_plugins/ir.py` | diff --git a/.github/instructions/architecture.instructions.md b/.github/instructions/architecture.instructions.md index 8f39a15bd..6818ab11a 100644 --- a/.github/instructions/architecture.instructions.md +++ b/.github/instructions/architecture.instructions.md @@ -73,6 +73,7 @@ semicolon-delimited, and specific to the file(s) that own the fact. | Local marketplace package-version manifest precedence | marketplace/version_check.py (_read_local_version) | `src/apm_cli/marketplace/version_check.py` | | applyTo normalization and hidden-tool placement | utils/patterns.py (normalize_apply_to); compilation/context_optimizer.py (ContextOptimizer) | `src/apm_cli/utils/patterns.py`; `src/apm_cli/compilation/context_optimizer.py` | | Effective marketplace output path | marketplace/output_profiles.py (resolve_effective_output_path) | `src/apm_cli/marketplace/output_profiles.py` | +| Marketplace metadata-enrichment outcome and certifiability | marketplace/builder.py (MetadataEnrichmentResult) | `src/apm_cli/marketplace/builder.py` | | Bootstrap project-name validation and fallback | core/project_name.py (resolve_bootstrap_project_name) | `src/apm_cli/core/project_name.py` | | Marketplace raw-structure diagnostics | marketplace/models.py parser; validator.py consumes them | `src/apm_cli/marketplace/models.py`; `src/apm_cli/marketplace/validator.py` | | Agent Plugins v1 contract interpretation, component discovery, and portable manifest authority | agent_plugins/loader.py (load_agent_plugin, _load_apm_configuration) | `src/apm_cli/agent_plugins/loader.py`; `src/apm_cli/agent_plugins/ir.py` | diff --git a/CHANGELOG.md b/CHANGELOG.md index dac662647..ed572c6a2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -55,6 +55,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 (by @MohammedAlkindi; closes #2639) (#2645) - `apm install` now resolves positional virtual-subdirectory git semver ranges before literal-ref preflight, while preserving registry version validation. (by @aryansk; closes #2514) (#2590) - `apm uninstall --global` now cleans removed-only target files before deleting their ownership state, while preserving files owned by surviving packages. (#2658) +- `apm pack` now reports unavailable remote package metadata, exposes its + certifiability in JSON, and prevents `--check-clean` from certifying a + degraded regeneration. `--strict-metadata` exits before writing an incomplete + marketplace artifact. (closes #2524) - Windows binary is now Authenticode-signed in the release workflow, eliminating the `Trojan:Script/Wacatac.H!ml` Windows Defender false positive on unsigned PyInstaller bundles. (#2435) diff --git a/apm.lock.yaml b/apm.lock.yaml index eef676317..c7d5bae00 100644 --- a/apm.lock.yaml +++ b/apm.lock.yaml @@ -2783,7 +2783,7 @@ deployments: owners: - . active_owner: . - content_hash: sha256:c085444fd1341c4a8daf421fa1403c41200bf8f077fd187fe04633f6384a553a + content_hash: sha256:f79f633b93e55cf577df2b29a5cbe559a26462776d1d846d08254082feea2bf2 - kind: project-relative target: copilot value: .github/instructions/changelog.instructions.md @@ -3290,7 +3290,7 @@ local_deployed_file_hashes: .github/agents/spec-tag-architect.agent.md: sha256:82907265c5e7cf1ac61ad96866fa7c5683b69c8f09b7a4c5f3cc241acc9568ca .github/agents/supply-chain-security-expert.agent.md: sha256:8fb8cc426d6af17ba084a28b3f026c2b475b62e3ca63ed2f88b83bd823f877af .github/agents/test-coverage-expert.agent.md: sha256:48c2172d1f18a394fa83ef9dc2be0b9b921a4e51e976498165250fed66369711 - .github/instructions/architecture.instructions.md: sha256:c085444fd1341c4a8daf421fa1403c41200bf8f077fd187fe04633f6384a553a + .github/instructions/architecture.instructions.md: sha256:f79f633b93e55cf577df2b29a5cbe559a26462776d1d846d08254082feea2bf2 .github/instructions/changelog.instructions.md: sha256:1e51ec4c74e847967962bd279dc4c6e582c5d3578490b3c28d5f3acd3e05f73e .github/instructions/cicd.instructions.md: sha256:33201cb88ea2f34b4950a9b52f87dc8dfb682796aaf53068ba7ae406c0c5e2c2 .github/instructions/cli.instructions.md: sha256:8e39e8d5047ce88575cb02f87c2bcede584dfef258bd86f7466c7badf136541a diff --git a/docs/src/content/docs/producer/publish-to-a-marketplace.md b/docs/src/content/docs/producer/publish-to-a-marketplace.md index 39ba13175..2f1c292ea 100644 --- a/docs/src/content/docs/producer/publish-to-a-marketplace.md +++ b/docs/src/content/docs/producer/publish-to-a-marketplace.md @@ -231,6 +231,7 @@ apm pack --offline # cached refs only apm pack --include-prerelease # allow pre-release tags apm pack -v # per-entry resolution detail apm pack --marketplace=claude --json # JSON output for CI pipelines +apm pack --strict-metadata # require remote Claude package metadata fetches ``` For the release-gate flags (`--check-versions`, `--check-clean`), diff --git a/docs/src/content/docs/producer/releasing-from-any-ci.md b/docs/src/content/docs/producer/releasing-from-any-ci.md index da3868294..65ad4ce42 100644 --- a/docs/src/content/docs/producer/releasing-from-any-ci.md +++ b/docs/src/content/docs/producer/releasing-from-any-ci.md @@ -19,7 +19,8 @@ shell-script translation of these lines. set -euo pipefail VERSION="${VERSION:?VERSION must be set, e.g. v1.2.3}" -apm pack --check-versions --check-clean --json > pack-report.json +apm pack --check-versions --check-clean --dry-run --json > pack-report.json +apm pack --check-versions --strict-metadata for f in build/*.zip .claude-plugin/marketplace.json; do [ -f "$f" ] || continue @@ -37,12 +38,16 @@ gh release create "$VERSION" \ What each command does: -- `apm pack --check-versions --check-clean --json` runs the pack with - the release gates enabled. `--check-versions` fails if per-package +- `apm pack --check-versions --check-clean --dry-run --json` verifies + the committed artifacts without writing them. `--check-versions` fails if per-package versions disagree with `marketplace.versioning.strategy`. `--check-clean` fails if the on-disk `marketplace.json` does not - match what a fresh pack would produce. `--json` writes a - machine-readable summary to stdout; human logs go to stderr. + match what a fresh pack would produce, or if remote Claude package metadata + could not be fetched to certify that regeneration -- see + [Marketplace artifacts](../../reference/cli/pack/#marketplace-artifacts) + for the failure modes. `apm pack --check-versions --strict-metadata` + generates the release artifacts only after remote metadata is certifiable. + `--json` writes a machine-readable summary to stdout; human logs go to stderr. - `sha256sum` produces one sidecar per artifact. Consumers verify with `sha256sum -c .sha256`. - `gh release create` uploads the bundle, the marketplace artifact, @@ -76,11 +81,13 @@ jobs: [`microsoft/apm-action@v1`](https://github.com/microsoft/apm-action) with `mode: release` is a convenience wrapper for the canonical -sequence above. It installs the CLI, runs `apm pack ---check-versions --check-clean --json`, generates the sidecars, and +sequence above. It installs the CLI, verifies with `apm pack +--check-versions --check-clean --dry-run --json`, generates the artifacts +and sidecars, and calls `gh release create` against the pushed tag. Use it when you want one less script to maintain; use the raw `run:` form below when -you need to customise any step. +you need to customise any step, including `--strict-metadata` +certification before artifact generation. > **Reference deployment.** [`DevExpGbb/zava-agent-config`](https://github.com/DevExpGbb/zava-agent-config) > runs this exact pipeline. The @@ -102,7 +109,8 @@ artifact format. with: { python-version: "3.12" } - run: pip install apm-cli - run: | - apm pack --check-versions --check-clean --json > pack-report.json + apm pack --check-versions --check-clean --dry-run --json > pack-report.json + apm pack --check-versions --strict-metadata for f in build/*.zip .claude-plugin/marketplace.json; do [ -f "$f" ] || continue sha256sum "$f" > "${f}.sha256" @@ -125,7 +133,8 @@ release: - if: '$CI_COMMIT_TAG =~ /^v/' script: - pip install apm-cli - - apm pack --check-versions --check-clean --json > pack-report.json + - apm pack --check-versions --check-clean --dry-run --json > pack-report.json + - apm pack --check-versions --strict-metadata - | for f in build/*.zip .claude-plugin/marketplace.json; do [ -f "$f" ] || continue @@ -149,7 +158,8 @@ pipeline { steps { sh ''' pip install apm-cli - apm pack --check-versions --check-clean --json > pack-report.json + apm pack --check-versions --check-clean --dry-run --json > pack-report.json + apm pack --check-versions --strict-metadata for f in build/*.zip .claude-plugin/marketplace.json; do [ -f "$f" ] || continue sha256sum "$f" > "${f}.sha256" @@ -176,7 +186,8 @@ steps: - task: UsePythonVersion@0 inputs: { versionSpec: "3.12" } - script: pip install apm-cli - - script: apm pack --check-versions --check-clean --json > pack-report.json + - script: apm pack --check-versions --check-clean --dry-run --json > pack-report.json + - script: apm pack --check-versions --strict-metadata - script: | for f in build/*.zip .claude-plugin/marketplace.json; do [ -f "$f" ] || continue @@ -201,11 +212,16 @@ steps: | 1 | runtime | Build or network error. Inspect the JSON report; rerun. | | 2 | schema | `apm.yml` is invalid. Fix the manifest before tagging. | | 3 | `--check-versions`| Per-package versions disagree with `marketplace.versioning.strategy`. See [Versioning strategies](../versioning-strategies/). | -| 4 | `--check-clean` | Committed `marketplace.json` does not match a fresh pack. Run `apm pack` locally, commit the diff (or `git commit --amend --no-edit` to fold into the current commit), then re-tag and push the updated tag (`git tag -f vX.Y.Z && git push --force-with-lease origin vX.Y.Z`). | - -The gates never write to disk -- they only refuse to release. -Recover by running the same `apm pack` locally without `--check-*`, -inspecting the diff, and pushing a clean tag. +| 4 | `--check-clean` | Committed `marketplace.json` does not match a fresh pack, or remote Claude package metadata was unfetchable. For drift, run `apm pack` locally, commit the diff, then re-tag. For metadata unavailability, restore the remote source or CI credentials and rerun; committing a regenerated file cannot certify unavailable metadata. | +| 5 | `--strict-metadata`| Remote Claude package metadata could not be fetched, so `apm pack` refused to write. Retry with network access, or omit `--strict-metadata` when the default warning is acceptable. | + +`--check-versions` and `--check-clean` are validation-only and never +write to disk. `--strict-metadata` certifies metadata before the +subsequent pack writes artifacts. Recover drift by running `apm pack` +locally without `--check-*`, inspecting the diff, and pushing a clean +tag. For metadata unavailability, restore the remote source or CI +credentials instead; regenerating a file cannot certify missing +metadata. :::note `microsoft/apm-action@v1` is a thin convenience wrapper, not a new diff --git a/docs/src/content/docs/reference/cli/pack.md b/docs/src/content/docs/reference/cli/pack.md index e75d7cacf..2af01b66c 100644 --- a/docs/src/content/docs/reference/cli/pack.md +++ b/docs/src/content/docs/reference/cli/pack.md @@ -40,10 +40,11 @@ Bundles are target-agnostic. The consumer's project decides where files land at | `--include-prerelease` | off | Marketplace: allow pre-release tags to satisfy version ranges. | | `-m`, `--marketplace FORMATS` | all configured | Comma-separated list of marketplace formats to build. Sentinels: `all` (every configured format), `none` (skip marketplace entirely). | | `--marketplace-path FORMAT=PATH` | manifest default | Override the output path for a specific format. Repeatable. Example: `--marketplace-path codex=./dist/codex.json`. | -| `--json` | off | Emit machine-readable JSON to stdout. All logs move to stderr. Shape: `{ok, dry_run, warnings, errors, marketplace: {outputs: [...]}}`. | +| `--json` | off | Emit machine-readable JSON to stdout. All logs move to stderr. Shape: `{ok, dry_run, warnings, errors, metadata_enrichment: {certifiable, outcomes: [...]}, marketplace: {outputs: [...]}}`. | | `--legacy-skill-paths` | off | Bundle skills under per-client paths (e.g. `.cursor/skills/`) instead of the converged `.agents/skills/`. Compatibility flag. | | `--check-versions` | off | Release gate: verify per-package versions agree with the configured `marketplace.versioning.strategy` (`lockstep`, `tag_pattern`, or `per_package`). Exits `3` on misalignment. Composes with `--check-clean` and `--dry-run`. | -| `--check-clean` | off | Release gate: regenerate every configured marketplace output to a temp representation and diff against the same effective path used by `apm pack`, including `--marketplace-path` overrides. Exits `4` for drift. Combine with `--dry-run` to compare without normal pack output generation. | +| `--check-clean` | off | Release gate: regenerate every configured marketplace output to a temp representation and diff against the same effective path used by `apm pack`, including `--marketplace-path` overrides. Exits `4` for drift or for remote Claude metadata that can't be fetched to certify the regeneration. Combine with `--dry-run` to compare without normal pack output generation. | +| `--strict-metadata` | off | Claude marketplace: fail before writing when remote package metadata cannot be fetched. Use it in publishing CI to require those fetches to succeed. Exits `5`. | | `--target`, `-t VALUE` | auto-detect | **Deprecated.** Recorded as informational `pack.target` metadata only; ignored by `apm install`. Will be removed in a future release. | :::caution[Migrating automation from `.tar.gz`?] @@ -170,6 +171,13 @@ dependencies: Configure marketplace artifact paths in `apm.yml` with the `marketplace.outputs` map, keyed by format. Use `--marketplace-path FORMAT=PATH` to override per-format output paths at pack time. +Remote Claude entries can inherit `description` and `version` from their own +`apm.yml`. If APM cannot fetch that metadata, normal packing writes the artifact +with an actionable warning so authors can add those fields to the marketplace +entry or retry with network access. Use `--strict-metadata` in publishing CI to +fail before writing with uncertifiable remote metadata. `--check-clean` also fails with exit +`4` rather than certifying a regeneration whose metadata could not be fetched. + ### Plugin manifests Ship one APM package; consumers get a native plugin for their tool of choice. When `apm.yml` declares a [`target:`](../../manifest-schema/#36-target) (or `targets:`) field containing `claude` or `copilot`, `apm pack` generates an ecosystem-specific `plugin.json` so the same source tree drops into a Claude Code plugin directory or a Copilot plugin path with no hand-editing. @@ -244,7 +252,8 @@ Plugin manifest generation runs after BUNDLE and MARKETPLACE phases so the gener | `1` | Build or runtime error: network failure, ref not found, no tag matches a marketplace range, lockfile read error, or unhandled packer exception. | | `2` | `apm.yml` schema validation error. | | `3` | `--check-versions` failed: per-package versions disagree with the configured marketplace versioning strategy. | -| `4` | `--check-clean` failed: marketplace working tree is dirty (regenerated output differs from on-disk file). | +| `4` | `--check-clean` failed: marketplace working tree is dirty (regenerated output differs from on-disk file), or remote Claude metadata could not be fetched to certify the comparison. | +| `5` | `--strict-metadata` failed: remote marketplace metadata was unavailable, so APM did not write the artifact. | ## Related diff --git a/packages/apm-guide/.apm/skills/apm-usage/commands.md b/packages/apm-guide/.apm/skills/apm-usage/commands.md index 49164f6a7..2c89da385 100644 --- a/packages/apm-guide/.apm/skills/apm-usage/commands.md +++ b/packages/apm-guide/.apm/skills/apm-usage/commands.md @@ -183,7 +183,7 @@ Lifecycle scripts fire on six events: `pre-install`, `post-install`, `pre-update | Command | Purpose | Key flags | |---------|---------|-----------| -| `apm pack` | Build distributable artifacts (bundle and/or marketplace.json -- driven by `apm.yml`). A `dependencies:` mapping, including `dependencies: {}`, produces a bundle of local package content; omitted or null `dependencies:` does not. Default output (no format flag) is a Claude Code plugin directory. Pass `--format agent-plugin` to opt into a portable Agent Plugins v1 bundle instead -- strict portable core only (root `plugin.json`, `skills/`, root `mcp.json` written even when empty; no `agents/`, `commands/`, `instructions/`, `extensions/`, `hooks/`, or LSP payload). That bundle build fails before any output is written if the source project has non-portable agents/commands/instructions/extensions/hooks/LSP, naming the surfaces and pointing to `--format claude-plugin` (and to configuring LSP in the target directly, since neither pack format carries it). `apm install` does not deploy Agent Plugin bundles yet -- it fails closed with a message directing consumers back to a Claude-compatible package. Bundles are **target-agnostic**: `pack.target` is recorded in every bundle for diagnostic purposes (typically `"all"` for target-agnostic packs, or the project's detected target) and is not authoritative at install time; `pack.bundle_files` (path -> sha256) drives integrity verification. The consumer's project decides where files land. Dependency content is packed **exclusively** from lockfile-attested `deployed_files` (in every bundle format); the `apm_modules` cache is never packed. Each file is verified against its `deployed_file_hashes` SHA-256 before inclusion, so a file tampered after `apm install` (hash mismatch) or deleted (missing on disk) fails the pack with a message pointing at `apm install`; files with no recorded hash (older lockfiles) pack unverified. Dependency hooks-config / MCP-config is not attested, so it is not packed -- `apm pack` warns (`[!]`) and names the dependency (first-party root hooks/MCP are still packed). Marketplace-publishing projects (`marketplace:` block, no `dependencies:`) no longer emit the misleading "No plugin.json found" warning; after a successful build, a vendor-neutral catalog of artifact paths is appended together with a single docs pointer (`producer/publish-to-a-marketplace/#consume-from-any-assistant`) listing per-assistant install paths. Release-time gates `--check-versions` and `--check-clean` are opt-in: when present, they run after the build and exit non-zero on misalignment / drift (codes 3 and 4 respectively) so release pipelines can fail fast. The version gate reads a local package's `apm.yml` first; Plugin collections without `apm.yml` use `plugin.json`'s `version`. Invalid or versionless `apm.yml` fails closed, and the fallback likewise rejects malformed or non-object JSON and a missing or blank version. When `apm.yml` declares `target: claude` or `target: copilot` (or the plural `targets:` equivalent), `apm pack` also generates an ecosystem-specific `plugin.json`: `.claude-plugin/plugin.json` for Claude (includes `mcpServers` from `.mcp.json` if present) and `.github/plugin/plugin.json` for Copilot (omits `mcpServers`). An existing file at the target path is preserved (a warning is emitted and the write is skipped) unless `--force` is passed; `--dry-run` prevents writes. Credential-bearing keys and secret-shaped values in `.mcp.json` are stripped recursively at any depth from the Claude manifest before writing, so a committed manifest never leaks secrets (see the apm pack reference, `reference/cli/pack/#credential-stripping-claude-mcpservers`). | `-o PATH`, `--archive` (produce a `.zip` archive instead of a directory; changed from `.tar.gz`), `--archive-format [zip\|tar.gz]` (default `zip`; use `tar.gz` for smaller legacy CI artifacts; only active with `--archive`), `--dry-run`, `--format [plugin\|agent-plugin\|claude\|claude-plugin\|apm]` (`agent-plugin` is the sole selector for the Agent Plugin bundle; `plugin` is a compatibility alias for the Claude plugin bundle, not for `agent-plugin`; `claude`/`claude-plugin` also select the Claude plugin bundle; `apm` selects the legacy APM layout; default `claude-plugin`), `--claude-plugin` (shortcut for `--format claude-plugin`; passing more than one of `--claude-plugin`/`--format` is a usage error), `--force`, `--offline`, `--include-prerelease`, `--marketplace=FORMATS`, `--marketplace-path FORMAT=PATH`, `--json`, `--check-versions` (release gate: per-package versions match `marketplace.versioning.strategy`; exit 3 on failure), `--check-clean` (release gate: regenerate-and-diff against the effective marketplace path, including `--marketplace-path` overrides; exit 4 on drift; pair with `--dry-run` to avoid normal pack output generation). `-t/--target` is **deprecated** (warn only). Exit codes: `0` success, `1` build/runtime error, `2` schema validation error, `3` `--check-versions` misalignment, `4` `--check-clean` drift. | +| `apm pack` | Build distributable artifacts (bundle and/or marketplace.json -- driven by `apm.yml`). A `dependencies:` mapping, including `dependencies: {}`, produces a bundle of local package content; omitted or null `dependencies:` does not. Default output (no format flag) is a Claude Code plugin directory. Pass `--format agent-plugin` to opt into a portable Agent Plugins v1 bundle instead -- strict portable core only (root `plugin.json`, `skills/`, root `mcp.json` written even when empty; no `agents/`, `commands/`, `instructions/`, `extensions/`, `hooks/`, or LSP payload). That bundle build fails before any output is written if the source project has non-portable agents/commands/instructions/extensions/hooks/LSP, naming the surfaces and pointing to `--format claude-plugin` (and to configuring LSP in the target directly, since neither pack format carries it). `apm install` does not deploy Agent Plugin bundles yet -- it fails closed with a message directing consumers back to a Claude-compatible package. Bundles are **target-agnostic**: `pack.target` is recorded in every bundle for diagnostic purposes (typically `"all"` for target-agnostic packs, or the project's detected target) and is not authoritative at install time; `pack.bundle_files` (path -> sha256) drives integrity verification. The consumer's project decides where files land. Dependency content is packed **exclusively** from lockfile-attested `deployed_files` (in every bundle format); the `apm_modules` cache is never packed. Each file is verified against its `deployed_file_hashes` SHA-256 before inclusion, so a file tampered after `apm install` (hash mismatch) or deleted (missing on disk) fails the pack with a message pointing at `apm install`; files with no recorded hash (older lockfiles) pack unverified. Dependency hooks-config / MCP-config is not attested, so it is not packed -- `apm pack` warns (`[!]`) and names the dependency (first-party root hooks/MCP are still packed). Marketplace-publishing projects (`marketplace:` block, no `dependencies:`) no longer emit the misleading "No plugin.json found" warning; after a successful build, a vendor-neutral catalog of artifact paths is appended together with a single docs pointer (`producer/publish-to-a-marketplace/#consume-from-any-assistant`) listing per-assistant install paths. Release-time gates `--check-versions` and `--check-clean` are opt-in: when present, they exit non-zero on misalignment / drift (codes 3 and 4 respectively) so release pipelines can fail fast. `--strict-metadata` preflights every selected marketplace output before any bundle, plugin manifest, or marketplace artifact write, then exits 5 if remote metadata remains uncertifiable. The version gate reads a local package's `apm.yml` first; Plugin collections without `apm.yml` use `plugin.json`'s `version`. Invalid or versionless `apm.yml` fails closed, and the fallback likewise rejects malformed or non-object JSON and a missing or blank version. When `apm.yml` declares `target: claude` or `target: copilot` (or the plural `targets:` equivalent), `apm pack` also generates an ecosystem-specific `plugin.json`: `.claude-plugin/plugin.json` for Claude (includes `mcpServers` from `.mcp.json` if present) and `.github/plugin/plugin.json` for Copilot (omits `mcpServers`). An existing file at the target path is preserved (a warning is emitted and the write is skipped) unless `--force` is passed; `--dry-run` prevents writes. Credential-bearing keys and secret-shaped values in `.mcp.json` are stripped recursively at any depth from the Claude manifest before writing, so a committed manifest never leaks secrets (see the apm pack reference, `reference/cli/pack/#credential-stripping-claude-mcpservers`). | `-o PATH`, `--archive` (produce a `.zip` archive instead of a directory; changed from `.tar.gz`), `--archive-format [zip\|tar.gz]` (default `zip`; use `tar.gz` for smaller legacy CI artifacts; only active with `--archive`), `--dry-run`, `--format [plugin\|agent-plugin\|claude\|claude-plugin\|apm]` (`agent-plugin` is the sole selector for the Agent Plugin bundle; `plugin` is a compatibility alias for the Claude plugin bundle, not for `agent-plugin`; `claude`/`claude-plugin` also select the Claude plugin bundle; `apm` selects the legacy APM layout; default `claude-plugin`), `--claude-plugin` (shortcut for `--format claude-plugin`; passing more than one of `--claude-plugin`/`--format` is a usage error), `--force`, `--offline`, `--include-prerelease`, `--marketplace=FORMATS`, `--marketplace-path FORMAT=PATH`, `--json`, `--check-versions` (release gate: per-package versions match `marketplace.versioning.strategy`; exit 3 on failure), `--check-clean` (release gate: regenerate-and-diff against the effective marketplace path, including `--marketplace-path` overrides; exit 4 on drift or uncertifiable remote Claude metadata; pair with `--dry-run` to avoid normal pack output generation), `--strict-metadata` (fail before writing when remote Claude package metadata cannot be fetched; exit 5). `-t/--target` is **deprecated** (warn only). Exit codes: `0` success, `1` build/runtime error, `2` schema validation error, `3` `--check-versions` misalignment, `4` `--check-clean` drift or uncertifiable remote Claude metadata, `5` `--strict-metadata` failed remote Claude metadata fetch. | | `apm unpack BUNDLE` | **[Deprecated]** Extract a bundle. Use `apm install ` instead -- it deploys directly with integrity verification and target resolution. | `-o PATH`, `--skip-verify`, `--force`, `--dry-run` | `apm install ` -- when the positional argument resolves to a directory containing `plugin.json` at its root, or to a `.zip` (or legacy `.tar.gz`/`.tgz`) archive whose extracted root contains `plugin.json`, install switches to local-bundle mode: the bundle is integrity-verified against its embedded `apm.lock.yaml` (`pack.bundle_files`) and deployed into the consumer's resolved target. Root `plugin.json` routing is exclusively schema-driven: only a `plugin.json` with **no** `$schema` key follows the legacy Claude/APM path described below. If `plugin.json` declares the exact recognized Agent Plugins v1 `$schema` (`1.0.0`), install fails closed instead of dissecting it -- Agent Plugin deployment is not implemented yet. Any other schema-bearing `plugin.json` hard-fails rather than falling back to legacy routing: a non-string `$schema`, an Agent Plugins schema at an unsupported version, or a foreign schema id from another tool entirely all raise before any dissection is attempted. Target resolution follows the same precedence as registry installs (`--target` > `apm.yml` > directory detection); the bundle itself carries no target binding. Targets without target-native instruction deployment (opencode, codex, gemini) receive instructions staged under `apm_modules//.apm/instructions/` and the install emits a hint to run `apm compile` to merge them. Grok Build deploys native instructions to `.grok/rules/`; run `apm compile` separately when you also want `AGENTS.md`. Other existing paths (e.g. a source-package directory without `plugin.json`) still flow through the normal local-path dependency-resolver pipeline. Files are recorded under `local_deployed_files` in the project lockfile -- `apm.yml` is **never** mutated. Honours `--target`, `--global`, `--force`, `--dry-run`, `--verbose`, plus `--as ALIAS` (log/display label only). Resolver/MCP/registry/policy flags (`--update`, `--mcp`, `--parallel-downloads`, `--allow-insecure-host`, `--skill`, ...) are rejected with a single consolidated error -- local-bundle install is an imperative deploy and bypasses those subsystems. diff --git a/scripts/lint-architecture-boundaries.sh b/scripts/lint-architecture-boundaries.sh index bbf03c799..e7c48c7c8 100755 --- a/scripts/lint-architecture-boundaries.sh +++ b/scripts/lint-architecture-boundaries.sh @@ -1610,6 +1610,29 @@ if ! bash scripts/check_marketplace_output_path_authority.sh; then violations=$((violations + 1)) fi +echo "[*] AC34: marketplace metadata-enrichment outcome authority" +metadata_enrichment_owner="src/apm_cli/marketplace/builder.py" +metadata_enrichment_class_count=$(grep -rEc --include='*.py' \ + '^class MetadataEnrichment(Outcome|Result)(\(|:)' src/apm_cli \ + | awk -F: '{sum += $2} END {print sum + 0}') +metadata_enrichment_prefetch_defs=$(grep -rEc --include='*.py' \ + '^[[:space:]]*def _prefetch_metadata\(' src/apm_cli \ + | awk -F: '{sum += $2} END {print sum + 0}') +metadata_enrichment_private_consumers=$( + grep -rEn --include='*.py' '_prefetch_metadata\(' src/apm_cli \ + | grep -v "^${metadata_enrichment_owner}:" \ + || true +) +if [ "$metadata_enrichment_class_count" -ne 2 ] \ + || [ "$metadata_enrichment_prefetch_defs" -ne 1 ] \ + || ! grep -q '^class MetadataEnrichmentResult(' "$metadata_enrichment_owner" \ + || ! grep -q '^ def remote_metadata_for_profile(' "$metadata_enrichment_owner" \ + || [ -n "$metadata_enrichment_private_consumers" ]; then + echo "[x] Marketplace metadata certifiability must remain owned by marketplace/builder.py" + [ -n "$metadata_enrichment_private_consumers" ] && echo "$metadata_enrichment_private_consumers" + violations=$((violations + 1)) +fi + echo "[*] AC33: marketplace structural-diagnostic authority" marketplace_structure_owner="src/apm_cli/marketplace/models.py" marketplace_structure_validator="src/apm_cli/marketplace/validator.py" diff --git a/src/apm_cli/commands/pack.py b/src/apm_cli/commands/pack.py index e316df21f..672a30ca7 100644 --- a/src/apm_cli/commands/pack.py +++ b/src/apm_cli/commands/pack.py @@ -13,6 +13,7 @@ BuildError, BuildOptions, BuildOrchestrator, + MetadataEnrichmentError, OutputKind, ) from ..core.command_logger import CommandLogger @@ -63,6 +64,7 @@ 2 Manifest schema validation error 3 Version alignment check failed (--check-versions) 4 Marketplace working-tree drift detected (--check-clean) + 5 Strict marketplace metadata check failed (--strict-metadata) """ @@ -254,6 +256,16 @@ def _parse_marketplace_filter( "with --dry-run to check without normal pack output generation." ), ) +@click.option( + "--strict-metadata", + is_flag=True, + default=False, + help=( + "Marketplace: fail before writing when remote description/version metadata " + "cannot be fetched. Exits 5; --check-clean always rejects uncertifiable " + "metadata with exit 4." + ), +) @click.option( "-m", "--marketplace", @@ -295,7 +307,7 @@ def _parse_marketplace_filter( ), ) @click.pass_context -def pack_cmd( # noqa: PLR0913 -- Click handler, one param per CLI option +def pack_cmd( # noqa: C901, PLR0913 -- Click handler, one param per CLI option ctx, select_claude_plugin, fmt, @@ -314,6 +326,7 @@ def pack_cmd( # noqa: PLR0913 -- Click handler, one param per CLI option legacy_skill_paths, check_versions, check_clean, + strict_metadata, ): """Pack APM artifacts: bundle and/or marketplace.json.""" logger = CommandLogger("pack", verbose=verbose, dry_run=dry_run) @@ -373,6 +386,7 @@ def pack_cmd( # noqa: PLR0913 -- Click handler, one param per CLI option "pack.target metadata only and is ignored by 'apm install'." ) effective_target = target + effective_dry_run = dry_run or check_clean options = BuildOptions( project_root=project_root, apm_yml_path=project_root / "apm.yml", @@ -384,14 +398,32 @@ def pack_cmd( # noqa: PLR0913 -- Click handler, one param per CLI option bundle_force=force, marketplace_offline=offline, marketplace_include_prerelease=include_prerelease, + marketplace_strict_metadata=strict_metadata, marketplace_formats=marketplace_formats, marketplace_path_overrides=path_overrides if path_overrides else None, - dry_run=dry_run, + dry_run=effective_dry_run, verbose=verbose, ) try: result = BuildOrchestrator().run(options, logger=logger) + except MetadataEnrichmentError as exc: + if json_output: + from ..marketplace.builder import BuildReport + + click.echo( + json_mod.dumps( + BuildReport.failure_to_json_dict( + errors=[{"code": "metadata_incomplete", "message": str(exc)}], + warnings=list(exc.metadata_enrichment.warnings), + dry_run=effective_dry_run, + metadata_enrichment=exc.metadata_enrichment, + ) + ) + ) + else: + logger.error(str(exc)) + ctx.exit(5) except BuildError as exc: _emit_json_error_or_raise(ctx, json_output, "build_error", str(exc)) return @@ -399,6 +431,7 @@ def pack_cmd( # noqa: PLR0913 -- Click handler, one param per CLI option # -- Release gates (--check-versions / --check-clean) -- version_alignment_payload: dict | None = None drift_payload: dict | None = None + drift_metadata_enrichment = None gate_errors: list[dict] = [] version_gate_failed = False drift_gate_failed = False @@ -488,6 +521,7 @@ def pack_cmd( # noqa: PLR0913 -- Click handler, one param per CLI option options.marketplace_path_overrides, ) drift_payload = d_report.to_json_dict() + drift_metadata_enrichment = d_report.metadata_enrichment if d_report.ok: if not json_output: formats = ", ".join(o.format for o in d_report.outputs) @@ -512,6 +546,12 @@ def pack_cmd( # noqa: PLR0913 -- Click handler, one param per CLI option out.format, (options.marketplace_path_overrides or {}).get(out.format), ) + elif out.status == "uncertifiable": + logger.error( + f" {out.path} [cannot certify regenerated metadata]" + ) + for warning in out.metadata_warnings: + logger.warning(f" {warning}") else: count = len(out.differences) logger.info(f" {out.path} [drift: {count} differences]") @@ -523,16 +563,37 @@ def pack_cmd( # noqa: PLR0913 -- Click handler, one param per CLI option out.format, (options.marketplace_path_overrides or {}).get(out.format), ) - for msg in d_report.error_messages(): - gate_errors.append({"code": "marketplace_drift", "message": msg}) + for out in d_report.outputs: + if out.status == "unchanged": + continue + code = ( + "marketplace_metadata_uncertifiable" + if out.status == "uncertifiable" + else "marketplace_drift" + ) + messages = ( + out.metadata_warnings + if out.status == "uncertifiable" + else ( + next( + msg + for msg in d_report.error_messages() + if msg.startswith(out.path) + ), + ) + ) + gate_errors.extend( + {"code": code, "message": message} for message in messages + ) # -- JSON output mode: consistent envelope -- if json_output: envelope = { "ok": True, - "dry_run": dry_run, + "dry_run": effective_dry_run, "warnings": list(result.warnings), "errors": [], + "metadata_enrichment": {"certifiable": True, "outcomes": []}, "marketplace": {"outputs": []}, "bundle": None, "plugin_manifests": {"written": [], "skipped": [], "dry_run": []}, @@ -543,8 +604,14 @@ def pack_cmd( # noqa: PLR0913 -- Click handler, one param per CLI option if sub.kind is OutputKind.MARKETPLACE and sub.payload is not None: payload = sub.payload.to_json_dict() envelope["marketplace"] = payload.get("marketplace", {"outputs": []}) + envelope["metadata_enrichment"] = payload.get( + "metadata_enrichment", + envelope["metadata_enrichment"], + ) elif sub.kind is OutputKind.PLUGIN_MANIFEST and isinstance(sub.payload, dict): envelope["plugin_manifests"] = sub.payload + if drift_metadata_enrichment is not None: + envelope["metadata_enrichment"] = drift_metadata_enrichment.to_json_dict() if gate_errors: envelope["errors"] = list(envelope["errors"]) + gate_errors envelope["ok"] = False @@ -562,7 +629,7 @@ def pack_cmd( # noqa: PLR0913 -- Click handler, one param per CLI option sub.payload, bundle_format, target, - dry_run, + effective_dry_run, show_zip_migration_notice=( archive and archive_format == "zip" @@ -571,7 +638,13 @@ def pack_cmd( # noqa: PLR0913 -- Click handler, one param per CLI option ), ) elif sub.kind is OutputKind.MARKETPLACE: - _render_marketplace_result(logger, sub.payload, dry_run, sub.warnings, sub.outputs) + _render_marketplace_result( + logger, + sub.payload, + effective_dry_run, + sub.warnings, + sub.outputs, + ) # Gate exit codes (after non-JSON rendering above): 3 wins over 4. if version_gate_failed: diff --git a/src/apm_cli/core/build_orchestrator.py b/src/apm_cli/core/build_orchestrator.py index 72cbcd6b4..20b33bd7e 100644 --- a/src/apm_cli/core/build_orchestrator.py +++ b/src/apm_cli/core/build_orchestrator.py @@ -50,6 +50,7 @@ class BuildOptions: # Marketplace-only options marketplace_offline: bool = False marketplace_include_prerelease: bool = False + marketplace_strict_metadata: bool = False marketplace_formats: tuple[str, ...] | None = None marketplace_path_overrides: dict[str, str] | None = None # Common options @@ -80,6 +81,17 @@ class BuildError(Exception): """User-facing build error. The CLI maps this to exit code 1.""" +class MetadataEnrichmentError(BuildError): + """Raised when strict marketplace metadata cannot be certified.""" + + def __init__(self, metadata_enrichment: Any) -> None: + """Preserve the canonical result for the CLI's error envelope.""" + self.metadata_enrichment = metadata_enrichment + super().__init__( + "Marketplace metadata is incomplete: " + " ".join(metadata_enrichment.warnings) + ) + + class ArtifactProducer(Protocol): """Protocol that every concrete producer must implement.""" @@ -138,11 +150,11 @@ class MarketplaceProducer: kind = OutputKind.MARKETPLACE - def produce(self, options: BuildOptions, logger: Any) -> ProducerResult: + def prepare(self, options: BuildOptions) -> _MarketplaceBuildPlan: + """Resolve every selected marketplace output before any artifact write.""" from ..marketplace.builder import ( BuildOptions as MktBuildOptions, ) - from ..marketplace.builder import BuildReport as MarketplaceBuildReport from ..marketplace.builder import ( MarketplaceBuilder, ) @@ -186,15 +198,12 @@ def _warn(msg: str) -> None: # exists so any downstream diagnostics report a real location. builder._yml_path = yml_for_builder - resolve_result = None - output_reports = [] - outputs: list[Path] = [] - # Apply --marketplace filter: skip outputs not in the requested set active_outputs = list(config.outputs) if options.marketplace_formats is not None: active_outputs = [o for o in active_outputs if o in options.marketplace_formats] + output_profiles: list[Any] = [] for output_name in active_outputs: profile = MARKETPLACE_OUTPUTS.get(output_name) if profile is None: @@ -203,36 +212,77 @@ def _warn(msg: str) -> None: f"Unknown marketplace output target: {output_name!r}. " f"Valid targets: {valid_targets}" ) - try: - if resolve_result is None: - resolve_result = builder.resolve() - resolved = resolve_result.entries + output_profiles.append(profile) - from ..marketplace.output_profiles import resolve_effective_output_path + try: + resolve_result = builder.resolve() + except MktBuildError as exc: + raise BuildError(str(exc)) from exc + + planned_outputs: list[_PlannedMarketplaceOutput] = [] + for profile in output_profiles: + from ..marketplace.output_profiles import resolve_effective_output_path - output_path = resolve_effective_output_path( - config, - profile, - project_root, - options.marketplace_path_overrides, + output_path = resolve_effective_output_path( + config, + profile, + project_root, + options.marketplace_path_overrides, + ) + planned_outputs.append( + _PlannedMarketplaceOutput( + profile=profile, + output_path=output_path, + metadata=builder.remote_metadata_for_profile(profile, resolve_result.entries), ) + ) + + return _MarketplaceBuildPlan( + builder=builder, + resolve_result=resolve_result, + warnings=warnings, + outputs=tuple(planned_outputs), + ) + + @staticmethod + def validate_strict_metadata(plan: _MarketplaceBuildPlan) -> None: + """Fail before mutation when a selected output cannot be certified.""" + for output in plan.outputs: + if output.metadata is not None and not output.metadata.certifiable: + raise MetadataEnrichmentError(output.metadata) - output_report = builder.write_output( - profile, - resolved, - output_path, + def produce_prepared( + self, + plan: _MarketplaceBuildPlan, + options: BuildOptions, + ) -> ProducerResult: + """Write a marketplace plan whose metadata has already been resolved.""" + from ..marketplace.builder import BuildReport as MarketplaceBuildReport + from ..marketplace.errors import BuildError as MktBuildError + + if options.marketplace_strict_metadata: + self.validate_strict_metadata(plan) + + output_reports = [] + outputs: list[Path] = [] + for output in plan.outputs: + try: + output_report = plan.builder.write_output( + output.profile, + plan.resolve_result.entries, + output.output_path, include_diff=True, - remote_metadata=builder.remote_metadata_for_profile(profile, resolved), - errors=resolve_result.errors, + remote_metadata=output.metadata, + errors=plan.resolve_result.errors, ) - output_reports.extend(output_report.outputs) - if output_report.output_path is not None: - outputs.append(Path(output_report.output_path)) except MktBuildError as exc: raise BuildError(str(exc)) from exc + output_reports.extend(output_report.outputs) + if output_report.output_path is not None: + outputs.append(Path(output_report.output_path)) marketplace_report = MarketplaceBuildReport(outputs=tuple(output_reports)) - warnings.extend(marketplace_report.warnings) + warnings = [*plan.warnings, *marketplace_report.warnings] return ProducerResult( kind=OutputKind.MARKETPLACE, @@ -241,6 +291,29 @@ def _warn(msg: str) -> None: payload=marketplace_report, ) + def produce(self, options: BuildOptions, logger: Any) -> ProducerResult: + """Prepare and write every selected marketplace output.""" + return self.produce_prepared(self.prepare(options), options) + + +@dataclass(frozen=True) +class _PlannedMarketplaceOutput: + """One resolved marketplace output ready for a later write.""" + + profile: Any + output_path: Path + metadata: Any + + +@dataclass(frozen=True) +class _MarketplaceBuildPlan: + """All marketplace inputs resolved before an artifact producer writes.""" + + builder: Any + resolve_result: Any + warnings: list[str] + outputs: tuple[_PlannedMarketplaceOutput, ...] + # --------------------------------------------------------------------------- # Plugin manifest producer -- generates plugin.json for each target ecosystem @@ -445,10 +518,21 @@ def run(self, options: BuildOptions, logger: Any = None) -> BuildResult: ) result = BuildResult() + marketplace_plans: dict[MarketplaceProducer, _MarketplaceBuildPlan] = {} + if options.marketplace_strict_metadata and OutputKind.MARKETPLACE in outputs_needed: + for producer in self._producers: + if isinstance(producer, MarketplaceProducer): + plan = producer.prepare(options) + producer.validate_strict_metadata(plan) + marketplace_plans[producer] = plan + for producer in self._producers: if producer.kind not in outputs_needed: continue - sub = producer.produce(options, logger) + if isinstance(producer, MarketplaceProducer) and producer in marketplace_plans: + sub = producer.produce_prepared(marketplace_plans[producer], options) + else: + sub = producer.produce(options, logger) result.outputs.extend(sub.outputs) result.warnings.extend(sub.warnings) result.producer_results.append(sub) diff --git a/src/apm_cli/marketplace/builder.py b/src/apm_cli/marketplace/builder.py index b14fdab55..195ae4c36 100644 --- a/src/apm_cli/marketplace/builder.py +++ b/src/apm_cli/marketplace/builder.py @@ -23,6 +23,7 @@ import threading import urllib.error import urllib.request +from collections.abc import Iterator, Mapping from concurrent.futures import ThreadPoolExecutor, as_completed from dataclasses import dataclass, field from pathlib import Path @@ -94,6 +95,8 @@ def _read_capped_text(resp: Any) -> str: "BuildOptions", "BuildReport", "MarketplaceBuilder", + "MetadataEnrichmentOutcome", + "MetadataEnrichmentResult", "ResolveResult", "ResolvedPackage", ] @@ -122,6 +125,95 @@ class ResolvedPackage: effective_tag_pattern: str = "" +@dataclass(frozen=True) +class MetadataEnrichmentOutcome: + """The metadata-fetch result for one resolved marketplace package.""" + + package: str + status: str # "fetched" | "empty" | "failed" | "offline" | "local" + values: tuple[tuple[str, str], ...] = () + cause: str | None = None + + @property + def metadata(self) -> dict[str, str]: + """Return this outcome's mapper-ready description/version values.""" + return dict(self.values) + + def to_json_dict(self) -> dict[str, Any]: + """Serialize the durable outcome without exposing source document contents.""" + payload: dict[str, Any] = {"package": self.package, "status": self.status} + if self.cause is not None: + payload["cause"] = self.cause + return payload + + +@dataclass(frozen=True) +class MetadataEnrichmentResult(Mapping[str, dict[str, str]]): + """Canonical metadata enrichment state for one marketplace build. + + The mapping interface preserves mapper compatibility while this object owns + the distinction between an empty source manifest and an unavailable source. + Consumers must use ``certifiable`` rather than inferring it from omitted + mapped fields. + """ + + outcomes: tuple[MetadataEnrichmentOutcome, ...] = () + _metadata_by_package: dict[str, dict[str, str]] = field( + init=False, + repr=False, + compare=False, + ) + + def __post_init__(self) -> None: + """Index mapper-ready metadata once for every output composition.""" + object.__setattr__( + self, + "_metadata_by_package", + {outcome.package: dict(outcome.values) for outcome in self.outcomes if outcome.values}, + ) + + def __getitem__(self, package: str) -> dict[str, str]: + return dict(self._metadata_by_package[package]) + + def __iter__(self) -> Iterator[str]: + return iter(self._metadata_by_package) + + def __len__(self) -> int: + return len(self._metadata_by_package) + + @property + def certifiable(self) -> bool: + """Whether output based on this enrichment may certify clean drift.""" + return all(outcome.status not in {"failed", "offline"} for outcome in self.outcomes) + + @property + def warnings(self) -> tuple[str, ...]: + """Return user-actionable warnings for incomplete remote enrichment.""" + warnings: list[str] = [] + for outcome in self.outcomes: + if outcome.status == "offline": + warnings.append( + f"Package '{outcome.package}': metadata enrichment skipped by " + "--offline. Add description/version to the marketplace entry or " + "rerun without --offline." + ) + elif outcome.status == "failed": + cause = outcome.cause or "unknown failure" + warnings.append( + f"Package '{outcome.package}': metadata enrichment failed " + f"({cause}). Add description/version to the marketplace entry or " + "retry with network access." + ) + return tuple(warnings) + + def to_json_dict(self) -> dict[str, Any]: + """Serialize metadata completeness for machine-readable pack output.""" + return { + "certifiable": self.certifiable, + "outcomes": [outcome.to_json_dict() for outcome in self.outcomes], + } + + @dataclass(frozen=True) class _SourceBaseCoords: """Parsed sourceBase coordinates cached for one marketplace build.""" @@ -164,6 +256,7 @@ class MarketplaceOutputReport: removed_count: int = 0 output_path: Path = field(default_factory=lambda: Path(".")) dry_run: bool = False + metadata_enrichment: MetadataEnrichmentResult | None = None @dataclass(frozen=True) @@ -194,7 +287,17 @@ def errors(self) -> tuple[tuple[str, str], ...]: @property def warnings(self) -> tuple[str, ...]: - return tuple(warn for output in self.outputs for warn in output.warnings) + warnings = [warn for output in self.outputs for warn in output.warnings] + warnings.extend(self.metadata_enrichment.warnings) + return tuple(dict.fromkeys(warnings)) + + @property + def metadata_enrichment(self) -> MetadataEnrichmentResult: + """Return the canonical result shared by metadata-aware outputs.""" + for output in self.outputs: + if output.metadata_enrichment is not None: + return output.metadata_enrichment + return MetadataEnrichmentResult() @property def diagnostics(self) -> tuple[BuildDiagnostic, ...]: @@ -255,6 +358,7 @@ def to_json_dict(self) -> dict[str, Any]: "dry_run": self.dry_run, "warnings": all_warnings, "errors": all_errors, + "metadata_enrichment": self.metadata_enrichment.to_json_dict(), "marketplace": { "outputs": output_entries, }, @@ -268,6 +372,7 @@ def failure_to_json_dict( errors: list[dict[str, str]], warnings: list[str] | None = None, dry_run: bool = False, + metadata_enrichment: MetadataEnrichmentResult | None = None, ) -> dict[str, Any]: """Produce the Section 4 JSON shape for a pre-build failure. @@ -279,6 +384,11 @@ def failure_to_json_dict( "dry_run": dry_run, "warnings": warnings or [], "errors": errors, + "metadata_enrichment": ( + metadata_enrichment + if metadata_enrichment is not None + else MetadataEnrichmentResult() + ).to_json_dict(), "marketplace": { "outputs": [], }, @@ -358,6 +468,9 @@ def __init__( self._host_resolvers_lock = threading.Lock() self._source_base_parts: _SourceBaseCoords | None = None self._source_base_parts_loaded = False + self._metadata_enrichment: MetadataEnrichmentResult | None = None + self._metadata_fetch_outcomes: dict[str, MetadataEnrichmentOutcome] = {} + self._metadata_fetch_outcomes_lock = threading.Lock() @classmethod def from_config( @@ -556,18 +669,32 @@ def remote_metadata_for_profile( self, profile: MarketplaceOutputProfile, resolved: tuple[ResolvedPackage, ...], - ) -> dict[str, dict[str, Any]] | None: - """Return remote metadata needed to compose this output, if any.""" + ) -> MetadataEnrichmentResult | None: + """Return the canonical enrichment result needed to compose *profile*.""" mapper = self._mapper_for_profile(profile) if not mapper.uses_remote_metadata: return None - return self._prefetch_metadata(resolved) + result = self._prefetch_metadata(resolved) + if isinstance(result, MetadataEnrichmentResult): + return result + # Compatibility for tests and third-party callers that substituted the + # old mapping return value while the builder remained the owner. + return MetadataEnrichmentResult( + outcomes=tuple( + MetadataEnrichmentOutcome( + package=pkg.name, + status="fetched" if pkg.name in result else "empty", + values=tuple(sorted(result.get(pkg.name, {}).items())), + ) + for pkg in resolved + ) + ) def _map_output( self, profile: MarketplaceOutputProfile, resolved: tuple[ResolvedPackage, ...], - remote_metadata: dict[str, dict[str, Any]] | None = None, + remote_metadata: Mapping[str, dict[str, str]] | None = None, ) -> MapperResult: """Map resolved packages into one marketplace output format.""" mapper = self._mapper_for_profile(profile) @@ -977,13 +1104,11 @@ def _fetch_local_metadata(self, pkg: ResolvedPackage) -> dict[str, str] | None: ) return None - def _fetch_remote_metadata(self, pkg: ResolvedPackage) -> dict[str, str] | None: - """Best-effort: fetch ``description`` and ``version`` from the - package's remote ``apm.yml``. - - Returns a dict with ``description`` and/or ``version`` keys, or - ``None`` on any error. This is purely cosmetic enrichment -- - failures are silently logged at debug level and never propagate. + def _fetch_remote_metadata_outcome( + self, + pkg: ResolvedPackage, + ) -> MetadataEnrichmentOutcome: + """Fetch remote metadata while preserving complete failure state. When a token is available for the package's host, it is included as an ``Authorization`` header so private repos can be accessed. @@ -998,6 +1123,8 @@ def _fetch_remote_metadata(self, pkg: ResolvedPackage) -> dict[str, str] | None: is skipped. """ try: + from ..core.auth import AuthResolver + path_prefix = f"{pkg.subdir}/" if pkg.subdir else "" file_path = f"{path_prefix}apm.yml" @@ -1007,15 +1134,11 @@ def _fetch_remote_metadata(self, pkg: ResolvedPackage) -> dict[str, str] | None: effective_host = pkg.host or self._host if pkg.host is None or pkg.host == self._host: host_info = self._host_info - token = self._github_token else: - from ..core.auth import AuthResolver # lazy import - try: host_info = AuthResolver.classify_host(effective_host) except Exception: host_info = None - token = self._resolve_token_for_host(effective_host) host_kind = host_info.kind if host_info else "github" @@ -1026,41 +1149,46 @@ def _fetch_remote_metadata(self, pkg: ResolvedPackage) -> dict[str, str] | None: pkg.name, effective_host, ) - return None - - if host_kind == "ghe_cloud" and not token: - logger.debug( - "Skipping metadata fetch for %s (GHE Cloud requires auth)", + return MetadataEnrichmentOutcome( pkg.name, + "failed", + cause=f"metadata fetch is unsupported for host '{effective_host}'", ) - return None - if effective_host == "github.com": - raw_url = ( - f"https://raw.githubusercontent.com/{pkg.source_repo}/{pkg.sha}/{file_path}" - ) - req = urllib.request.Request(raw_url) # noqa: S310 - if token: - req.add_header("Authorization", f"token {token}") - try: - with urllib.request.urlopen(req, timeout=5) as resp: # noqa: S310 - raw = _read_capped_text(resp) - except urllib.error.HTTPError as exc: - if exc.code != 404: - raise - api_base = ( - host_info.api_base if host_info else None - ) or "https://api.github.com" - rest_url = ( - f"{api_base}/repos/{pkg.source_repo}/contents/{file_path}?ref={pkg.sha}" + auth_resolver = self._auth_resolver + + def _open_metadata(token: str | None, _git_env: dict[str, str]) -> str: + """Fetch the manifest with the credential selected for this attempt.""" + if host_kind == "ghe_cloud" and token is None: + raise PermissionError( + f"metadata fetch for '{effective_host}' requires a credential" + ) + if effective_host == "github.com": + raw_url = ( + f"https://raw.githubusercontent.com/{pkg.source_repo}/{pkg.sha}/{file_path}" ) - req = urllib.request.Request(rest_url) # noqa: S310 - req.add_header("Accept", "application/vnd.github.raw") + req = urllib.request.Request(raw_url) # noqa: S310 if token: req.add_header("Authorization", f"token {token}") - with urllib.request.urlopen(req, timeout=5) as resp: # noqa: S310 - raw = _read_capped_text(resp) - else: + try: + with urllib.request.urlopen(req, timeout=5) as resp: # noqa: S310 + return _read_capped_text(resp) + except urllib.error.HTTPError as exc: + if exc.code != 404: + raise + api_base = ( + host_info.api_base if host_info else None + ) or "https://api.github.com" + rest_url = ( + f"{api_base}/repos/{pkg.source_repo}/contents/{file_path}?ref={pkg.sha}" + ) + req = urllib.request.Request(rest_url) # noqa: S310 + req.add_header("Accept", "application/vnd.github.raw") + if token: + req.add_header("Authorization", f"token {token}") + with urllib.request.urlopen(req, timeout=5) as resp: # noqa: S310 + return _read_capped_text(resp) + api_base = ( host_info.api_base if host_info else None ) or f"https://{effective_host}/api/v3" @@ -1069,12 +1197,25 @@ def _fetch_remote_metadata(self, pkg: ResolvedPackage) -> dict[str, str] | None: req.add_header("Accept", "application/vnd.github.raw") if token: req.add_header("Authorization", f"token {token}") - with urllib.request.urlopen(req, timeout=5) as resp: # noqa: S310 - raw = _read_capped_text(resp) + return _read_capped_text(resp) + + org = pkg.source_repo.split("/", 1)[0] if pkg.source_repo else None + if auth_resolver is None: + # Legacy direct callers do not enter the concurrent prefetch + # path, which initializes AuthResolver before worker threads. + raw = _open_metadata(self._github_token, {}) + else: + raw = auth_resolver.try_with_fallback( + effective_host, + _open_metadata, + org=org, + path=pkg.source_repo, + unauth_first=False, + ) data = load_yaml_str(raw) if not isinstance(data, dict): - return None + return MetadataEnrichmentOutcome(pkg.name, "empty") result: dict[str, str] = {} desc = data.get("description") if isinstance(desc, str) and desc: @@ -1090,14 +1231,41 @@ def _fetch_remote_metadata(self, pkg: ResolvedPackage) -> dict[str, str] | None: pkg.name, ", ".join(result.keys()), ) - return result - except Exception: + return MetadataEnrichmentOutcome( + pkg.name, + "fetched", + values=tuple(sorted(result.items())), + ) + return MetadataEnrichmentOutcome(pkg.name, "empty") + except Exception as exc: logger.debug( "Could not fetch remote metadata for %s", pkg.name, exc_info=True, ) - return None + cause = str(exc).strip() or type(exc).__name__ + return MetadataEnrichmentOutcome(pkg.name, "failed", cause=cause) + + def _fetch_remote_metadata(self, pkg: ResolvedPackage) -> dict[str, str] | None: + """Return remote metadata for legacy direct callers. + + The full outcome remains owned by + :meth:`_fetch_remote_metadata_outcome`; this compatibility adapter must + not be used to decide whether an output is certifiable. + """ + outcome = self._fetch_remote_metadata_outcome(pkg) + with self._metadata_fetch_outcomes_lock: + self._metadata_fetch_outcomes[pkg.name] = outcome + return outcome.metadata or None + + def _fetch_local_metadata_outcome(self, pkg: ResolvedPackage) -> MetadataEnrichmentOutcome: + """Return local metadata as an intentional, certifiable enrichment state.""" + metadata = self._fetch_local_metadata(pkg) + return MetadataEnrichmentOutcome( + pkg.name, + "local", + values=tuple(sorted((metadata or {}).items())), + ) def _resolve_github_token(self) -> str | None: """Resolve a GitHub token using ``AuthResolver``. @@ -1125,54 +1293,76 @@ def _resolve_github_token(self) -> str | None: logger.debug("Could not resolve GitHub token for metadata fetch", exc_info=True) return None - def _prefetch_metadata(self, resolved: list[ResolvedPackage]) -> dict[str, dict[str, str]]: - """Fetch ``description``/``version`` metadata for resolved packages. - - Returns a mapping of ``{package_name: {"description": ..., "version": ...}}`` - for successful fetches. Both local-path and remote packages are - read from each package's own ``apm.yml`` so the output mapper can - apply one fallback rule regardless of source kind. + def _prefetch_metadata( + self, + resolved: tuple[ResolvedPackage, ...], + ) -> MetadataEnrichmentResult: + """Fetch package metadata and retain every per-package outcome. - Local reads always run (filesystem only). Remote fetches are - skipped when ``--offline`` is set. A GitHub token is resolved - once before spawning worker threads and stored on - ``self._github_token`` for the workers to read. + This is the single authority for deciding whether metadata-dependent + output is certifiable. A reachable manifest without description or + version is ``empty``; an unavailable remote source is ``failed`` or + intentionally ``offline`` and cannot certify a clean regeneration. """ - results: dict[str, dict[str, str]] = {} + if self._metadata_enrichment is not None: + return self._metadata_enrichment - # Local-path packages: read each apm.yml directly from disk. - # Cheap and serial -- no network, no thread pool needed. + outcomes_by_package: dict[str, MetadataEnrichmentOutcome] = {} + remote: list[ResolvedPackage] = [] for pkg in resolved: if pkg.source_repo: - continue - meta = self._fetch_local_metadata(pkg) - if meta: - results[pkg.name] = meta + remote.append(pkg) + else: + outcomes_by_package[pkg.name] = self._fetch_local_metadata_outcome(pkg) if self._options.offline: - return results - - remote = [pkg for pkg in resolved if pkg.source_repo] - if not remote: - return results - - # Resolve token once -- threads read self._github_token (immutable). - self._ensure_auth() - - workers = min(self._options.concurrency, len(remote)) - with ThreadPoolExecutor(max_workers=workers) as pool: - future_to_name = { - pool.submit(self._fetch_remote_metadata, pkg): pkg.name for pkg in remote - } - for future in as_completed(future_to_name): - name = future_to_name[future] - try: - meta = future.result() - if meta: - results[name] = meta - except Exception: - pass - return results + for pkg in remote: + outcomes_by_package[pkg.name] = MetadataEnrichmentOutcome( + pkg.name, + "offline", + cause="metadata fetch skipped by --offline", + ) + elif remote: + # Resolve token once -- threads read self._github_token (immutable). + self._ensure_auth() + workers = min(self._options.concurrency, len(remote)) + with ThreadPoolExecutor(max_workers=workers) as pool: + future_to_package = { + pool.submit(self._fetch_remote_metadata, pkg): pkg for pkg in remote + } + for future in as_completed(future_to_package): + pkg = future_to_package[future] + try: + metadata = future.result() + with self._metadata_fetch_outcomes_lock: + outcome = self._metadata_fetch_outcomes.pop(pkg.name, None) + if ( + isinstance(outcome, MetadataEnrichmentOutcome) + and outcome.package == pkg.name + ): + outcomes_by_package[pkg.name] = outcome + else: + # Preserve the legacy extension seam: callers that + # override _fetch_remote_metadata still feed the + # canonical outcome owner without reimplementing + # certifiability logic. + outcomes_by_package[pkg.name] = MetadataEnrichmentOutcome( + pkg.name, + "fetched" if metadata else "empty", + values=tuple(sorted((metadata or {}).items())), + ) + except Exception as exc: + cause = str(exc).strip() or type(exc).__name__ + outcomes_by_package[pkg.name] = MetadataEnrichmentOutcome( + pkg.name, + "failed", + cause=cause, + ) + + self._metadata_enrichment = MetadataEnrichmentResult( + outcomes=tuple(outcomes_by_package[pkg.name] for pkg in resolved) + ) + return self._metadata_enrichment # -- composition -------------------------------------------------------- @@ -1228,7 +1418,7 @@ def compose_output( self, profile: MarketplaceOutputProfile, resolved: tuple[ResolvedPackage, ...], - remote_metadata: dict[str, dict[str, Any]] | None = None, + remote_metadata: Mapping[str, dict[str, str]] | None = None, ) -> tuple[dict[str, Any], tuple[str, ...], tuple[BuildDiagnostic, ...]]: """Compose the JSON document for a marketplace output profile.""" mapper_result = self._map_output(profile, resolved, remote_metadata=remote_metadata) @@ -1241,7 +1431,7 @@ def write_output( output_path: Path, *, include_diff: bool = False, - remote_metadata: dict[str, dict[str, Any]] | None = None, + remote_metadata: MetadataEnrichmentResult | None = None, errors: tuple[tuple[str, str], ...] = (), ) -> BuildReport: """Write one marketplace output profile using already resolved packages.""" @@ -1273,6 +1463,7 @@ def write_output( removed_count=removed, output_path=output_path, dry_run=self._options.dry_run, + metadata_enrichment=remote_metadata, ) return BuildReport(outputs=(output_report,)) diff --git a/src/apm_cli/marketplace/drift_check.py b/src/apm_cli/marketplace/drift_check.py index d81b553e4..31e97d8f9 100644 --- a/src/apm_cli/marketplace/drift_check.py +++ b/src/apm_cli/marketplace/drift_check.py @@ -24,7 +24,11 @@ from pathlib import Path from typing import Any -from apm_cli.marketplace.builder import MarketplaceBuilder, ResolveResult +from apm_cli.marketplace.builder import ( + MarketplaceBuilder, + MetadataEnrichmentResult, + ResolveResult, +) from apm_cli.marketplace.output_profiles import ( MARKETPLACE_OUTPUTS, resolve_effective_output_path, @@ -53,8 +57,9 @@ class DriftOutputReport: format: str path: str - status: str # "unchanged" | "missing" | "drift" + status: str # "unchanged" | "missing" | "drift" | "uncertifiable" differences: tuple[DriftDifference, ...] = () + metadata_warnings: tuple[str, ...] = () def to_json_dict(self) -> dict[str, Any]: return { @@ -62,6 +67,7 @@ def to_json_dict(self) -> dict[str, Any]: "path": self.path, "status": self.status, "differences": [d.to_json_dict() for d in self.differences], + "metadata_warnings": list(self.metadata_warnings), } @@ -71,6 +77,7 @@ class DriftReport: ok: bool outputs: tuple[DriftOutputReport, ...] = field(default_factory=tuple) + metadata_enrichment: MetadataEnrichmentResult = field(default_factory=MetadataEnrichmentResult) def to_json_dict(self) -> dict[str, Any]: return { @@ -86,6 +93,8 @@ def error_messages(self) -> list[str]: elif out.status == "drift": count = len(out.differences) msgs.append(f"{out.path}: {count} differences vs. regenerated output") + elif out.status == "uncertifiable": + msgs.extend(out.metadata_warnings) return msgs @@ -163,6 +172,7 @@ def check_marketplace_drift( # Honor the configured outputs list (claude / codex / ...). configured = tuple(config.outputs) if config.outputs else ("claude",) output_reports: list[DriftOutputReport] = [] + metadata_enrichment = MetadataEnrichmentResult() for name in configured: profile = MARKETPLACE_OUTPUTS.get(name) @@ -181,6 +191,20 @@ def check_marketplace_drift( ) remote_metadata = builder.remote_metadata_for_profile(profile, resolve_result.entries) + if remote_metadata is not None: + metadata_enrichment = remote_metadata + if remote_metadata is not None and not remote_metadata.certifiable: + # Do not compare two equally degraded documents. The builder owns + # whether the regenerated metadata may certify this output. + output_reports.append( + DriftOutputReport( + format=profile.name, + path=rel_display, + status="uncertifiable", + metadata_warnings=remote_metadata.warnings, + ) + ) + continue new_doc, _warnings, _diagnostics = builder.compose_output( profile, resolve_result.entries, remote_metadata=remote_metadata ) @@ -235,7 +259,11 @@ def check_marketplace_drift( output_reports.sort(key=lambda r: r.format) overall_ok = all(r.status == "unchanged" for r in output_reports) - return DriftReport(ok=overall_ok, outputs=tuple(output_reports)) + return DriftReport( + ok=overall_ok, + outputs=tuple(output_reports), + metadata_enrichment=metadata_enrichment, + ) def render_diff_lines(report: DriftOutputReport, limit: int = _MAX_DIFFS_RENDERED) -> list[str]: diff --git a/tests/integration/test_architecture_authorities.py b/tests/integration/test_architecture_authorities.py index 6aae4749b..9cff2d155 100644 --- a/tests/integration/test_architecture_authorities.py +++ b/tests/integration/test_architecture_authorities.py @@ -4280,3 +4280,64 @@ def test_bootstrap_project_name_guard_rejects_variable_bypass(tmp_path: Path) -> assert result.returncode == 1 assert "ScriptRunner bootstrap name must be the resolver result" in result.stdout + + +def test_marketplace_metadata_certifiability_has_single_owner() -> None: + """Metadata outcomes and certification must route through the builder.""" + root = Path(__file__).parents[2] + owner = (root / "src/apm_cli/marketplace/builder.py").read_text(encoding="utf-8") + drift = (root / "src/apm_cli/marketplace/drift_check.py").read_text(encoding="utf-8") + guard = (root / "scripts/lint-architecture-boundaries.sh").read_text(encoding="utf-8") + architecture = (root / ".github/instructions/architecture.instructions.md").read_text( + encoding="utf-8" + ) + + assert owner.count("class MetadataEnrichmentOutcome:") == 1 + assert owner.count("class MetadataEnrichmentResult(") == 1 + assert owner.count("def _prefetch_metadata(") == 1 + assert "def remote_metadata_for_profile(" in owner + assert "not remote_metadata.certifiable" in drift + assert "AC34: marketplace metadata-enrichment outcome authority" in guard + assert ( + "Marketplace metadata certifiability must remain owned by marketplace/builder.py" in guard + ) + assert "Marketplace metadata-enrichment outcome and certifiability" in architecture + + +def test_metadata_certifiability_guard_rejects_parallel_owner(tmp_path: Path) -> None: + """The architecture gate rejects a duplicate outcome owner outside marketplace.""" + root = Path(__file__).parents[2] + sandbox = tmp_path / "repo" + shutil.copytree( + root, + sandbox, + ignore=shutil.ignore_patterns( + ".git", + ".venv", + ".pytest_cache", + "__pycache__", + "build", + "dist", + "node_modules", + ), + ) + consumer_path = sandbox / "src/apm_cli/core/build_orchestrator.py" + consumer_path.write_text( + consumer_path.read_text(encoding="utf-8") + + "\n\nclass MetadataEnrichmentResult:\n pass\n", + encoding="utf-8", + ) + + result = subprocess.run( + ("bash", "scripts/lint-architecture-boundaries.sh"), + cwd=sandbox, + capture_output=True, + text=True, + check=False, + timeout=300, + ) + + assert result.returncode == 1 + assert "Marketplace metadata certifiability must remain owned by marketplace/builder.py" in ( + result.stdout + ) diff --git a/tests/integration/test_remote_marketplace_metadata_outage_lifecycle.py b/tests/integration/test_remote_marketplace_metadata_outage_lifecycle.py new file mode 100644 index 000000000..57f7326bc --- /dev/null +++ b/tests/integration/test_remote_marketplace_metadata_outage_lifecycle.py @@ -0,0 +1,161 @@ +"""Installed-binary lifecycle proof for remote marketplace metadata outages.""" + +from __future__ import annotations + +import json +import os +from pathlib import Path + +import pytest + +from tests.utils.apm_lifecycle_runner import ApmLifecycleRunner, CommandResult + +pytestmark = [ + pytest.mark.integration, + pytest.mark.e2e, + pytest.mark.lifecycle_smoke, + pytest.mark.requires_apm_binary, + pytest.mark.requires_e2e_mode, + pytest.mark.requires_network_integration, +] + +_ARTIFACT = ".claude-plugin/marketplace.json" +_SHA = "09c4bf708d787eba45046904ab5b40b9ac597c6b" + + +def _evidence(result: CommandResult) -> str: + """Return process evidence for a failed lifecycle assertion.""" + return ( + f"cwd={result.cwd!s}\n" + f"command={result.command!r}\n" + f"returncode={result.returncode}\n" + f"stdout={result.stdout!r}\n" + f"stderr={result.stderr!r}" + ) + + +def _write_project(project_root: Path) -> None: + """Create the pinned remote marketplace fixture from issue #2524.""" + (project_root / "apm.yml").write_text( + f"""\ +name: remote-metadata-outage +description: Remote metadata outage lifecycle fixture +version: 1.0.0 +dependencies: {{}} +marketplace: + owner: + name: APM Lifecycle Tests + outputs: [codex, claude] + packages: + - name: adapt-nifi-flows-to-2-x + source: Netcracker/qubership-nifi + subdir: agent-packages/adapt-nifi-flows-to-2-x + ref: {_SHA} + category: Productivity +""", + encoding="utf-8", + ) + + +def _blocked_transport_environment() -> dict[str, str]: + """Return an environment that deterministically rejects HTTPS metadata fetches.""" + environment = dict(os.environ) + for name in ("https_proxy", "HTTPS_PROXY", "all_proxy", "ALL_PROXY", "no_proxy", "NO_PROXY"): + environment.pop(name, None) + environment.update( + { + "https_proxy": "http://127.0.0.1:9", + "HTTPS_PROXY": "http://127.0.0.1:9", + "ALL_PROXY": "http://127.0.0.1:9", + } + ) + return environment + + +def _bundle_snapshot(bundle_path: Path) -> dict[Path, bytes]: + """Capture a bundle's file content so failure paths cannot hide mutation.""" + return { + path.relative_to(bundle_path): path.read_bytes() + for path in bundle_path.rglob("*") + if path.is_file() + } + + +def test_remote_metadata_outage_never_certifies_degraded_marketplace( + tmp_path: Path, + apm_binary_path: Path, +) -> None: + """An outage warns in dry-run and makes --check-clean exit 4 before equality.""" + project_root = tmp_path / "remote-metadata-outage" + project_root.mkdir() + _write_project(project_root) + (project_root / "apm.lock.yaml").write_text("dependencies: []\n", encoding="utf-8") + runner = ApmLifecycleRunner((str(apm_binary_path),), scenario_timeout_seconds=300) + available_environment = dict(os.environ) + + seeded = runner.run( + ("pack",), + scenario_id="remote-metadata-outage-seed", + cwd=project_root, + env=available_environment, + ) + assert seeded.returncode == 0, _evidence(seeded) + artifact = project_root / _ARTIFACT + on_disk = json.loads(artifact.read_text(encoding="utf-8")) + plugin = on_disk["plugins"][0] + assert plugin["description"] + assert plugin["version"] + plugin.pop("description") + plugin.pop("version") + artifact.write_text(json.dumps(on_disk, indent=2) + "\n", encoding="utf-8") + truncated_bytes = artifact.read_bytes() + codex_artifact = project_root / ".agents" / "plugins" / "marketplace.json" + codex_bytes = codex_artifact.read_bytes() + bundle_path = project_root / "build" / "remote-metadata-outage-1.0.0" + bundle_snapshot = _bundle_snapshot(bundle_path) + + dry_run, uncertifiable = runner.run_sequence( + ( + ("pack", "--dry-run"), + ("pack", "--check-clean", "--dry-run"), + ), + expected_returncodes=(0, 4), + scenario_id="remote-metadata-outage-gate", + cwd=project_root, + env=_blocked_transport_environment(), + ) + + assert "metadata enrichment failed" in (dry_run.stdout + dry_run.stderr) + assert "adapt-nifi-flows-to-2-x" in (dry_run.stdout + dry_run.stderr) + assert artifact.read_bytes() == truncated_bytes + assert codex_artifact.read_bytes() == codex_bytes + assert _bundle_snapshot(bundle_path) == bundle_snapshot + assert "cannot certify regenerated metadata" in (uncertifiable.stdout + uncertifiable.stderr) + + strict, strict_retry = runner.run_sequence( + ( + ("pack", "--strict-metadata"), + ("pack", "--strict-metadata"), + ), + expected_returncodes=(5, 5), + scenario_id="remote-metadata-outage-strict", + cwd=project_root, + env=_blocked_transport_environment(), + ) + + assert "metadata enrichment failed" in (strict.stdout + strict.stderr) + assert "metadata enrichment failed" in (strict_retry.stdout + strict_retry.stderr) + assert artifact.read_bytes() == truncated_bytes + assert codex_artifact.read_bytes() == codex_bytes + assert _bundle_snapshot(bundle_path) == bundle_snapshot + + restored = runner.run( + ("pack", "--check-clean", "--dry-run"), + scenario_id="remote-metadata-outage-restored", + cwd=project_root, + env=available_environment, + ) + assert restored.returncode == 4, _evidence(restored) + restored_output = restored.stdout + restored.stderr + assert "plugins[0].description" in restored_output + assert "plugins[0].version" in restored_output diff --git a/tests/unit/marketplace/test_builder.py b/tests/unit/marketplace/test_builder.py index 27125f1de..6f30da6cc 100644 --- a/tests/unit/marketplace/test_builder.py +++ b/tests/unit/marketplace/test_builder.py @@ -9,7 +9,7 @@ from pathlib import Path from types import SimpleNamespace from typing import Any, Dict, List, Optional # noqa: F401, UP035 -from unittest.mock import patch +from unittest.mock import ANY, MagicMock, patch import pytest @@ -1560,7 +1560,13 @@ def test_no_warnings_when_names_unique(self, tmp_path: Path) -> None: "acme/pkg-beta": _make_refs("v1.0.0"), } report = _build_with_mock(tmp_path, yml, refs) - assert report.warnings == () + assert all( + "Duplicate marketplace package name" not in warning for warning in report.warnings + ) + assert len(report.warnings) == 2 + assert all( + "metadata enrichment skipped by --offline" in warning for warning in report.warnings + ) def test_duplicate_names_produce_warning(self, tmp_path: Path) -> None: """Bypass yml_schema by feeding resolved packages directly.""" @@ -1667,7 +1673,7 @@ def test_duplicate_names_without_subdir_uses_repository( assert "acme/tool-b" in warnings[0] def test_build_report_carries_warnings(self, tmp_path: Path) -> None: - """BuildReport.warnings is empty for a clean build.""" + """BuildReport carries offline metadata warnings from its canonical result.""" yml = """\ name: test-mkt description: Test @@ -1682,7 +1688,8 @@ def test_build_report_carries_warnings(self, tmp_path: Path) -> None: refs = {"acme/solo": _make_refs("v1.0.0")} report = _build_with_mock(tmp_path, yml, refs) assert isinstance(report.warnings, tuple) - assert len(report.warnings) == 0 + assert len(report.warnings) == 1 + assert "metadata enrichment skipped by --offline" in report.warnings[0] def test_empty_build_report_primary_output_is_safe(self) -> None: report = BuildReport(outputs=()) @@ -1871,11 +1878,15 @@ def test_numeric_version_coerced_to_string(self, tmp_path: Path) -> None: assert result is not None assert result["version"] == "1.0" - def test_auth_header_added_when_token_present(self, tmp_path: Path) -> None: - """When _github_token is set, Authorization header is included.""" + def test_auth_header_uses_the_resolver_selected_token(self, tmp_path: Path) -> None: + """Metadata reads use the credential supplied by AuthResolver.""" pkg = self._make_pkg() builder = self._make_builder(tmp_path) - builder._github_token = "ghp_faketoken123" + resolver = MagicMock() + builder._auth_resolver = resolver + resolver.try_with_fallback.side_effect = lambda _host, operation, **_kwargs: operation( + "ghp_faketoken123", {} + ) yaml_body = b"description: Private plugin\nversion: 1.0.0\n" mock_resp = _FakeHTTPResponse(yaml_body) with patch( @@ -1889,12 +1900,21 @@ def test_auth_header_added_when_token_present(self, tmp_path: Path) -> None: call_args = mock_open.call_args req = call_args[0][0] assert req.get_header("Authorization") == "token ghp_faketoken123" + resolver.try_with_fallback.assert_called_once_with( + "github.com", + ANY, + org="acme", + path="acme/my-tool", + unauth_first=False, + ) - def test_no_auth_header_when_no_token(self, tmp_path: Path) -> None: - """When _github_token is None, no Authorization header is set.""" + def test_no_auth_header_when_resolver_has_no_token(self, tmp_path: Path) -> None: + """Metadata reads omit Authorization when AuthResolver has no token.""" pkg = self._make_pkg() builder = self._make_builder(tmp_path) - builder._github_token = None + builder._auth_resolver = SimpleNamespace( + try_with_fallback=lambda _host, operation, **_kwargs: operation(None, {}) + ) yaml_body = b"description: Public plugin\nversion: 2.0.0\n" mock_resp = _FakeHTTPResponse(yaml_body) with patch( diff --git a/tests/unit/marketplace/test_metadata_enrichment.py b/tests/unit/marketplace/test_metadata_enrichment.py new file mode 100644 index 000000000..0001477c4 --- /dev/null +++ b/tests/unit/marketplace/test_metadata_enrichment.py @@ -0,0 +1,306 @@ +"""Regression tests for marketplace metadata enrichment outcomes.""" + +from __future__ import annotations + +import json +from pathlib import Path +from unittest.mock import MagicMock + +import pytest +from click.testing import CliRunner + +from apm_cli.commands.pack import pack_cmd +from apm_cli.core.build_orchestrator import ( + BuildOptions as OrchestratorBuildOptions, +) +from apm_cli.core.build_orchestrator import ( + BuildOrchestrator, + MarketplaceProducer, + MetadataEnrichmentError, + OutputKind, + ProducerResult, +) +from apm_cli.marketplace.builder import ( + BuildOptions, + MarketplaceBuilder, + MetadataEnrichmentOutcome, + MetadataEnrichmentResult, + ResolvedPackage, +) +from apm_cli.marketplace.drift_check import check_marketplace_drift +from apm_cli.marketplace.migration import load_marketplace_config + + +def _write_config(project_root: Path) -> None: + """Create a marketplace with one pinned remote package.""" + (project_root / "apm.yml").write_text( + """\ +name: metadata-outcome +description: Metadata outcome regression fixture +version: 1.0.0 +marketplace: + owner: + name: APM Tests + packages: + - name: remote-tool + source: acme/remote-tool + ref: 0123456789abcdef0123456789abcdef01234567 + category: Productivity +""", + encoding="utf-8", + ) + + +def _resolved_package() -> ResolvedPackage: + """Return the pinned package used by the test marketplace.""" + return ResolvedPackage( + name="remote-tool", + source_repo="acme/remote-tool", + subdir=None, + ref="0123456789abcdef0123456789abcdef01234567", + sha="0123456789abcdef0123456789abcdef01234567", + requested_version=None, + tags=(), + is_prerelease=False, + ) + + +def _builder(project_root: Path) -> MarketplaceBuilder: + """Build a dry-run builder from the fixture configuration.""" + return MarketplaceBuilder.from_config( + load_marketplace_config(project_root), + project_root, + options=BuildOptions(dry_run=True), + ) + + +def test_metadata_outcomes_preserve_success_empty_failed_offline_and_local( + tmp_path: Path, + monkeypatch, +) -> None: + """Only failed and intentionally offline remote enrichment is uncertifiable.""" + _write_config(tmp_path) + builder = _builder(tmp_path) + remote = _resolved_package() + local = ResolvedPackage( + name="local-tool", + source_repo="", + subdir="./packages/local-tool", + ref="", + sha="", + requested_version=None, + tags=(), + is_prerelease=False, + ) + monkeypatch.setattr( + builder, + "_fetch_remote_metadata", + lambda _pkg: None, + ) + monkeypatch.setattr( + builder, + "_fetch_local_metadata_outcome", + lambda _pkg: MetadataEnrichmentOutcome(local.name, "local"), + ) + + result = builder._prefetch_metadata((remote, local)) + + assert result.certifiable + assert result.warnings == () + assert [outcome.status for outcome in result.outcomes] == ["empty", "local"] + + +def test_metadata_outcome_mapping_uses_the_precomputed_package_index() -> None: + """Mapper lookups do not rescan every preserved metadata outcome.""" + result = MetadataEnrichmentResult( + ( + MetadataEnrichmentOutcome("first", "fetched", (("version", "1.0.0"),)), + MetadataEnrichmentOutcome("second", "empty"), + ) + ) + + assert dict(result) == {"first": {"version": "1.0.0"}} + assert result["first"] == {"version": "1.0.0"} + + +def test_drift_refuses_uncertifiable_metadata_before_comparing_output( + tmp_path: Path, + monkeypatch, +) -> None: + """A failed fetch must not let --check-clean certify equal degraded JSON.""" + _write_config(tmp_path) + builder = _builder(tmp_path) + remote = _resolved_package() + monkeypatch.setattr(builder, "resolve", lambda: type("Resolved", (), {"entries": (remote,)})()) + monkeypatch.setattr( + builder, + "_prefetch_metadata", + lambda _resolved: MetadataEnrichmentResult( + (MetadataEnrichmentOutcome("remote-tool", "failed", cause="timeout"),) + ), + ) + monkeypatch.setattr( + builder, + "compose_output", + lambda *_args, **_kwargs: (_ for _ in ()).throw(AssertionError("must not compare")), + ) + + report = check_marketplace_drift(builder, load_marketplace_config(tmp_path), tmp_path) + + assert not report.ok + assert report.outputs[0].status == "uncertifiable" + assert len(report.outputs[0].metadata_warnings) == 1 + assert "metadata enrichment failed (timeout)" in report.outputs[0].metadata_warnings[0] + + +def test_pack_json_warns_and_strict_metadata_prevents_writes( + tmp_path: Path, + monkeypatch, +) -> None: + """Default mode reports degraded metadata; strict mode refuses the artifact.""" + _write_config(tmp_path) + + monkeypatch.setattr(MarketplaceBuilder, "_ensure_auth", lambda _self: None) + monkeypatch.setattr( + MarketplaceBuilder, + "_fetch_remote_metadata_outcome", + lambda _self, pkg: MetadataEnrichmentOutcome(pkg.name, "failed", cause="transport closed"), + ) + monkeypatch.chdir(tmp_path) + runner = CliRunner() + + warned = runner.invoke(pack_cmd, ["--json"]) + + assert warned.exit_code == 0, warned.output + payload = json.loads(warned.output) + assert payload["metadata_enrichment"]["certifiable"] is False + assert payload["metadata_enrichment"]["outcomes"] == [ + {"package": "remote-tool", "status": "failed", "cause": "transport closed"} + ] + assert len(payload["warnings"]) == 1 + artifact = tmp_path / ".claude-plugin" / "marketplace.json" + assert artifact.is_file() + + uncertifiable = runner.invoke(pack_cmd, ["--check-clean", "--dry-run", "--json"]) + + assert uncertifiable.exit_code == 4, uncertifiable.output + uncertifiable_payload = json.loads(uncertifiable.output) + assert uncertifiable_payload["drift"]["outputs"][0]["status"] == "uncertifiable" + assert uncertifiable_payload["drift"]["outputs"][0]["metadata_warnings"] == payload["warnings"] + assert uncertifiable_payload["errors"][0]["code"] == "marketplace_metadata_uncertifiable" + artifact.unlink() + + strict = runner.invoke(pack_cmd, ["--strict-metadata", "--json"]) + + assert strict.exit_code == 5, strict.output + strict_payload = json.loads(strict.output) + assert strict_payload["errors"][0]["code"] == "metadata_incomplete" + assert strict_payload["metadata_enrichment"]["certifiable"] is False + assert ( + strict_payload["metadata_enrichment"]["outcomes"] + == payload["metadata_enrichment"]["outcomes"] + ) + assert strict_payload["warnings"] == payload["warnings"] + assert not artifact.exists() + + +def test_check_clean_never_writes_before_reporting_uncertifiable_metadata( + tmp_path: Path, + monkeypatch, +) -> None: + """The clean gate must not overwrite an artifact before reporting failure.""" + _write_config(tmp_path) + monkeypatch.setattr(MarketplaceBuilder, "_ensure_auth", lambda _self: None) + monkeypatch.setattr( + MarketplaceBuilder, + "_fetch_remote_metadata_outcome", + lambda _self, pkg: MetadataEnrichmentOutcome(pkg.name, "failed", cause="transport closed"), + ) + monkeypatch.chdir(tmp_path) + runner = CliRunner() + + initial = runner.invoke(pack_cmd, []) + + assert initial.exit_code == 0, initial.output + artifact = tmp_path / ".claude-plugin" / "marketplace.json" + artifact.write_text('{"sentinel": "unchanged"}\n', encoding="utf-8") + before = artifact.read_bytes() + + clean_check = runner.invoke(pack_cmd, ["--check-clean"]) + + assert clean_check.exit_code == 4, clean_check.output + assert artifact.read_bytes() == before + + +def test_pack_json_reports_the_final_clean_check_metadata_outcome( + tmp_path: Path, + monkeypatch, +) -> None: + """A later clean-check fetch failure must replace an earlier success in JSON.""" + _write_config(tmp_path) + outcomes = iter( + ( + MetadataEnrichmentOutcome( + "remote-tool", + "fetched", + values=(("description", "available"), ("version", "1.0.0")), + ), + MetadataEnrichmentOutcome("remote-tool", "failed", cause="transport closed"), + ) + ) + monkeypatch.setattr(MarketplaceBuilder, "_ensure_auth", lambda _self: None) + monkeypatch.setattr( + MarketplaceBuilder, + "_fetch_remote_metadata_outcome", + lambda _self, _pkg: next(outcomes), + ) + monkeypatch.chdir(tmp_path) + + result = CliRunner().invoke(pack_cmd, ["--check-clean", "--dry-run", "--json"]) + + assert result.exit_code == 4, result.output + payload = json.loads(result.output) + assert payload["metadata_enrichment"]["certifiable"] is False + assert payload["metadata_enrichment"]["outcomes"] == [ + {"package": "remote-tool", "status": "failed", "cause": "transport closed"} + ] + assert payload["errors"][0]["code"] == "marketplace_metadata_uncertifiable" + + +def test_strict_metadata_preflight_prevents_bundle_and_marketplace_mutations( + tmp_path: Path, + monkeypatch, +) -> None: + """Strict metadata failure must precede every requested artifact producer.""" + _write_config(tmp_path) + config_path = tmp_path / "apm.yml" + config_path.write_text( + config_path.read_text(encoding="utf-8").replace( + " packages:", " outputs: [codex, claude]\n packages:" + ) + + "dependencies: {}\n", + encoding="utf-8", + ) + bundle = MagicMock(spec=["kind", "produce"]) + bundle.kind = OutputKind.BUNDLE + bundle.produce.return_value = ProducerResult(kind=OutputKind.BUNDLE) + marketplace = MarketplaceProducer() + monkeypatch.setattr(MarketplaceBuilder, "_ensure_auth", lambda _self: None) + monkeypatch.setattr( + MarketplaceBuilder, + "_fetch_remote_metadata_outcome", + lambda _self, pkg: MetadataEnrichmentOutcome(pkg.name, "failed", cause="transport closed"), + ) + options = OrchestratorBuildOptions( + project_root=tmp_path, + apm_yml_path=tmp_path / "apm.yml", + marketplace_strict_metadata=True, + ) + + for _ in range(2): + with pytest.raises(MetadataEnrichmentError, match="metadata enrichment failed"): + BuildOrchestrator(producers=[bundle, marketplace]).run(options) + assert not (tmp_path / ".claude-plugin" / "marketplace.json").exists() + assert not (tmp_path / ".agents" / "plugins" / "marketplace.json").exists() + + bundle.produce.assert_not_called()