Skip to content

fix(pack): preserve marketplace metadata outcomes - #2661

Open
Daniel Meppiel (danielmeppiel) wants to merge 3 commits into
mainfrom
danielmeppiel-fix-pack-metadata-outcome
Open

fix(pack): preserve marketplace metadata outcomes#2661
Daniel Meppiel (danielmeppiel) wants to merge 3 commits into
mainfrom
danielmeppiel-fix-pack-metadata-outcome

Conversation

@danielmeppiel

Copy link
Copy Markdown
Collaborator

Closes #2524.

Preserves per-package metadata enrichment outcomes through pack output, JSON, strict mode, and the check-clean gate. Failed or offline remote metadata now warns by default; --strict-metadata exits 5 before writing; --check-clean exits 4 without comparing uncertifiable regeneration.

Validation: PYTHONPATH=src python3 -m pytest tests/unit/marketplace/test_metadata_enrichment.py tests/integration/test_remote_marketplace_metadata_outage_lifecycle.py -q -x (3 passed, 1 skipped); lifecycle test collected. Ruff could not run because the local Python environment has no ruff module and uv cannot fetch build dependencies.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Ensures apm pack preserves and reports per-package remote metadata enrichment outcomes (success/empty/failed/offline/local) end-to-end, so degraded metadata cannot silently pass --check-clean, and CI can opt into failing early via --strict-metadata.

Changes:

  • Introduces a canonical metadata-enrichment outcome/result model in the marketplace builder, surfaces it in JSON output, and wires a strict failure mode (--strict-metadata, exit 5).
  • Updates the --check-clean drift gate to refuse comparing outputs when regenerated metadata is uncertifiable (exit 4) and emits actionable warnings instead.
  • Adds unit, integration, and architecture-boundary coverage plus docs updates for the new behavior and flags.
Show a summary per file
File Description
tests/unit/marketplace/test_metadata_enrichment.py New regression tests for metadata outcomes, strict mode, JSON envelope, and drift “uncertifiable” behavior.
tests/unit/marketplace/test_builder.py Updates expectations to reflect offline metadata warnings now being preserved.
tests/integration/test_remote_marketplace_metadata_outage_lifecycle.py E2E lifecycle proof that outages warn and --check-clean refuses to certify degraded metadata.
tests/integration/test_architecture_authorities.py Adds architecture authority assertions + rejection test for parallel metadata outcome owners.
src/apm_cli/marketplace/drift_check.py Adds “uncertifiable” drift status carrying metadata warnings and blocks equality comparison in that case.
src/apm_cli/marketplace/builder.py Implements MetadataEnrichmentOutcome/Result, preserves per-package outcomes, and plumbs them through build reports and JSON.
src/apm_cli/core/build_orchestrator.py Adds strict-metadata build option and raises a dedicated error before any output write when uncertifiable.
src/apm_cli/commands/pack.py Adds --strict-metadata, new exit code 5 handling, and renders “uncertifiable” drift output + JSON error codes.
scripts/lint-architecture-boundaries.sh Adds AC34 guard enforcing single-owner authority for metadata enrichment outcomes/certifiability.
packages/apm-guide/.apm/skills/apm-usage/commands.md Updates apm pack reference with new failure modes and --strict-metadata.
docs/src/content/docs/reference/cli/pack.md Documents new JSON shape and strict/uncertifiable semantics + exit codes.
docs/src/content/docs/producer/releasing-from-any-ci.md Updates CI guidance for --strict-metadata and uncertifiable --check-clean failures.
docs/src/content/docs/producer/publish-to-a-marketplace.md Adds --strict-metadata to publishing examples.
CHANGELOG.md Adds an Unreleased “Fixed” entry for the metadata-outcome behavior changes.
.github/instructions/architecture.instructions.md Adds metadata-enrichment certifiability to the canonical-owner table.
.apm/instructions/architecture.instructions.md Mirrors canonical-owner table update for metadata-enrichment certifiability.

Review details

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

  • Files reviewed: 16/16 changed files
  • Comments generated: 3
  • Review effort level: Lite

Comment thread CHANGELOG.md
Comment on lines +26 to +29
- `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)
Comment on lines +84 to +93
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)
)
Comment on lines 65 to +67
3 Version alignment check failed (--check-versions)
4 Marketplace working-tree drift detected (--check-clean)
5 Strict marketplace metadata check failed (--strict-metadata)
@danielmeppiel

Copy link
Copy Markdown
Collaborator Author

APM Review Panel: needs_rework

Preserving marketplace metadata outcomes is valuable, but strict multi-output failure semantics must be atomic and phase-aligned before release.

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

The panel converges on a trust issue: strict marketplace metadata validation currently occurs during the per-output write loop, so a later failure can leave an earlier artifact or bundle written before exit 5. Packaging must not report a failed strict outcome after mutating only part of the requested output set. Preflight validation before every write, backed by an ordered multi-output no-mutation and idempotence proof, is the right correction.

DevX identified a related contract mismatch: JSON can record a successful metadata enrichment before an independently fetched --check-clean gate later fails with exit 4. The emitted result must describe the completed command phase, not an earlier intermediate state. Documentation should distinguish metadata unavailability from ordinary cleanliness drift and make dry-run ordering explicit. Architecture is otherwise appropriate; the logging polish can follow separately.

Aligned with: portable manifest packaging through consistent output state; secure defaults through fail-before-write strict behavior; pragmatic automation through phase-aligned JSON.

Growth signal. Reliable failure semantics let CI and marketplace users trust that APM's machine-readable result describes the command that actually completed.

Panel summary

Persona High R N Takeaway
Python Architect 0 0 1 Outcome ownership is appropriately scoped.
CLI Logging 0 1 0 Builder warnings currently duplicate the CLI status prefix.
DevX UX 1 1 0 Final JSON outcome can disagree with clean-check failure.
Supply Chain 1 0 0 Strict mode can leave partial artifacts.
OSS Growth 0 1 0 Outage and drift recovery need distinct guidance.
Auth 1 0 0 Inspect fallback behavior before expanding scope.
Docs 0 1 0 Check-clean docs need dry-run and outage clarity.
Test Coverage 1 1 0 Add atomic ordered multi-output lifecycle proof.
Performance 0 1 0 Verify proposed metadata lookup optimization first.

Top 3 follow-ups

  1. [Supply Chain Security Expert and Test Coverage Expert] Preflight strict metadata before every output write and prove an ordered multi-output failure leaves every artifact and bundle unchanged across a retry. This prevents partial publication state.
  2. [DevX UX Expert] Align metadata_enrichment in JSON with the final command phase so an exit-4 clean-check failure cannot claim certification. Machine consumers need one coherent outcome.
  3. [Doc Writer] Separate metadata outage remediation from ordinary drift and document dry-run validation before artifact generation. This gives CI users an actionable recovery path.

Recommendation

Correct the atomic strict-metadata and phase-aligned JSON failure semantics, with the multi-output no-mutation and idempotence regression proof, before shipping. Then fold the focused documentation clarification in the same change.


Full per-persona findings
  • [recommended] CLI Logging: remove presentation prefixes from builder warning data.
  • [high priority] DevX UX: surface the clean-check metadata result in the JSON envelope.
  • [high priority] Supply Chain and Test Coverage: strict metadata must be validated before all writes, including bundle output.
  • [recommended] Docs and Growth: metadata outage is not drift; validation should use --dry-run.
  • [review before scope expansion] Auth and Performance: verify credential fallback and lookup complexity against the exact implementation.

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

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

Copilot-Session: abb3728c-0b82-4dbc-9977-031de5196627
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] apm pack silently drops description/version for remote packages when metadata fetch fails (exit 0, no warning); --check-clean then passes

2 participants