diff --git a/.gitignore b/.gitignore index 5026da659..e880a0c9b 100644 --- a/.gitignore +++ b/.gitignore @@ -77,3 +77,4 @@ scout-pipeline-result.png .playwright-mcp/ server.pid .docs-rewrite-plan/ +build/apm-*/ diff --git a/CHANGELOG.md b/CHANGELOG.md index 216663d80..6672b8593 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `apm install --target claude` now preserves self-defined stdio MCP `env` values from `apm.yml` and writes non-string values such as `PORT: 3000` and `DEBUG: false` as MCP-compatible strings. (#1222) - Non-skill integrators (agent, instruction, prompt, command, hook script-copy) silently adopt byte-identical pre-existing files so a degraded `deployed_files=[]` lockfile no longer permanently blocks installs gated by `required-packages-deployed`. (#1313) - `apm audit` drift check now returns skip-with-info (`passed=True`) when the install cache is cold, instead of failing the audit; bare `apm audit` surfaces the skip reason on stderr so CI pipelines that have not yet run `apm install` are not incorrectly red-marked. (#1289) + +### Added + +- `apm pack --marketplace=FORMATS` filters which marketplace formats are built in a single run; accepts comma-separated names and sentinels `all`/`none`. (#1317) +- `apm pack --marketplace-path FORMAT=PATH` overrides the output path for a specific marketplace format at invocation time. Env var overrides (`APM_MARKETPLACE__PATH`) are planned for v0.15. (#1317) +- `apm pack --json` emits a stable JSON contract to stdout (`{ok, dry_run, warnings, errors, marketplace: {outputs: [{format, path, ...}]}}`); all logs move to stderr so downstream tooling can `jq` the output safely. (#1317) +- `marketplace.outputs` in `apm.yml` now accepts a map form keyed by format name (`outputs: {claude: {}, codex: {path: ...}}`), replacing the deprecated list form; the list form still parses with a one-cycle deprecation warning. (#1317) +- `apm marketplace init` now scaffolds the explicit map-form `outputs: {claude: {}}` so the default state is observable in the manifest. (#1317) + +### Changed + +- `--marketplace-output PATH` is now hidden from `--help` and emits a stderr deprecation warning; it auto-translates to `--marketplace-path claude=PATH`. Removal tracked in #1318. (#1317) - `extends: org` now correctly layers `dependencies.require` and `dependencies.deny` from the parent policy when the child omits the `dependencies:` block entirely; `None` signals "no opinion" (transparent) while `[]` signals explicit override. (#1290) - CI self-check job now uses `setup-only: true` + `apm audit --ci --no-drift` so managed files are not overwritten by `apm install` before `content-integrity` runs; documented the audit-only CI pattern and the install-before-audit blind spot in the enterprise and CI/CD guides. (#1291) - Pin `Path.home()` under unit tests via a session-scoped autouse conftest fixture, fixing 56 Windows runner failures on the new `windows-2025-vs2026` GitHub-hosted image where `USERPROFILE`/`HOMEDRIVE`+`HOMEPATH` are not seeded for pytest workers; also patch the `_check_and_notify_updates` import binding in the disabled-self-update test so it no longer races on the version-check cache. (#1270) 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 3d8587db1..cc61626a1 100644 --- a/docs/src/content/docs/producer/publish-to-a-marketplace.md +++ b/docs/src/content/docs/producer/publish-to-a-marketplace.md @@ -71,7 +71,8 @@ marketplace: name: acme-org url: https://github.com/acme-org - outputs: [claude] # default; add codex for Codex repo output + outputs: # map form (recommended) + claude: {} # default; add codex for Codex repo output claude: output: .claude-plugin/marketplace.json @@ -108,13 +109,19 @@ For the full field reference (every key on every entry, including `pluginRoot`, `outputs`, `claude`, `codex`, and pass-through fields like `tags`, `author`, `license`), see the reference below. -Marketplace output targets use a selector-list pattern: +Marketplace output targets use a map-form pattern: ```yaml marketplace: - outputs: [claude, codex] + outputs: + claude: {} + codex: + path: .agents/plugins/marketplace.json ``` +The legacy list form (`outputs: [claude, codex]`) still parses with a +deprecation warning but new projects should use the map form above. + Claude output is selected by default for backwards compatibility. The legacy `marketplace.output` field remains supported as shorthand for `marketplace.claude.output`; when both are set, the explicit @@ -143,6 +150,7 @@ apm pack --dry-run # resolve and print; do not write 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 ``` Exit codes: `0` build succeeded, `1` build error (network, missing diff --git a/docs/src/content/docs/reference/cli/pack.md b/docs/src/content/docs/reference/cli/pack.md index 8f2a3f2f9..5854094ea 100644 --- a/docs/src/content/docs/reference/cli/pack.md +++ b/docs/src/content/docs/reference/cli/pack.md @@ -35,7 +35,10 @@ Bundles are target-agnostic. The consumer's project decides where files land at | `--verbose`, `-v` | off | Show per-file paths and detailed packer output. | | `--offline` | off | Marketplace: resolve version ranges from cached refs only; skip `git ls-remote`. | | `--include-prerelease` | off | Marketplace: allow pre-release tags to satisfy version ranges. | -| `--marketplace-output PATH` | `.claude-plugin/marketplace.json` | Marketplace legacy compatibility: override only the Claude/Anthropic output path. Prefer `marketplace.claude.output` in `apm.yml`. | +| `-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: [...]}}`. | +| `--marketplace-output PATH` | _(hidden)_ | **Deprecated.** Translates to `--marketplace-path claude=PATH` with a stderr warning. Will be removed in v0.15 (see #1318). | | `--legacy-skill-paths` | off | Bundle skills under per-client paths (e.g. `.cursor/skills/`) instead of the converged `.agents/skills/`. Compatibility flag. | | `--target`, `-t VALUE` | auto-detect | **Deprecated.** Recorded as informational `pack.target` metadata only; ignored by `apm install`. Will be removed in a future release. | @@ -54,6 +57,15 @@ apm pack --format apm -o ./dist # legacy APM bundle layout ```bash apm pack apm pack --offline --dry-run + +# Build only Claude format, output as JSON for CI: +apm pack --marketplace=claude --json + +# Override codex output path: +apm pack --marketplace-path codex=./dist/codex-marketplace.json + +# Build all formats, preview paths: +apm pack --marketplace=all --json | jq -r '.marketplace.outputs[].path' ``` ### Both artifacts in one run @@ -67,11 +79,10 @@ apm pack --archive --offline ```yaml marketplace: - outputs: [claude, codex] - claude: - output: ./build/claude-marketplace.json - codex: - output: ./build/codex-marketplace.json + outputs: + claude: {} + codex: + path: ./build/codex-marketplace.json ``` ### Preview without writing @@ -124,7 +135,8 @@ Configure marketplace artifact paths in `apm.yml`: `marketplace.claude.output` c - **Empty bundle warning.** If no files match (e.g. nothing was installed yet), `apm pack` emits a warning and exits `0` with an empty bundle. Verbose mode prints a hint to run `apm install` first. - **Share line.** On success, `apm pack` prints `Share with: apm install ` so the produced bundle is immediately copy-pasteable. - **Marketplace fallback.** With no `marketplace:` block in `apm.yml`, a legacy `marketplace.yml` file is read with a deprecation warning. Both files present is a hard error. -- **Marketplace outputs.** `marketplace.outputs` defaults to `[claude]`. Add `codex` to also write `.agents/plugins/marketplace.json`; when selected, each package must define `category`. +- **Marketplace outputs.** Configure via `marketplace.outputs` map (keyed by format). Claude is included by default. The legacy list form (`outputs: [claude]`) still parses with a deprecation warning. Use `--marketplace=` to filter which formats are built in a given invocation. +- **JSON mode.** `--json` makes `apm pack` machine-friendly: stdout is a single JSON object, all human-readable logs move to stderr. Combine with `--marketplace=` for selective CI matrix builds. ## Exit codes diff --git a/src/apm_cli/commands/pack.py b/src/apm_cli/commands/pack.py index 03e583b8d..69aa72467 100644 --- a/src/apm_cli/commands/pack.py +++ b/src/apm_cli/commands/pack.py @@ -1,5 +1,6 @@ """Click commands for ``apm pack`` and ``apm unpack``.""" +import json as json_mod import sys from pathlib import Path @@ -14,6 +15,7 @@ ) from ..core.command_logger import CommandLogger from ..core.target_detection import TargetParamType +from ..utils.console import set_console_stderr _PACK_HELP = """\ Pack distributable artifacts from your APM project. @@ -52,6 +54,21 @@ """ +def _emit_json_error_or_raise(ctx, json_output: bool, code: str, message: str): + """Emit a JSON error envelope to stdout or raise ClickException.""" + if json_output: + from ..marketplace.builder import BuildReport + + click.echo( + json_mod.dumps( + BuildReport.failure_to_json_dict(errors=[{"code": code, "message": message}]) + ) + ) + ctx.exit(1) + else: + raise click.ClickException(message) + + @click.command(name="pack", help=_PACK_HELP) @click.option( "--format", @@ -104,11 +121,38 @@ "marketplace_output", type=click.Path(), default=None, + hidden=True, + help=("[Deprecated] Override Claude output path. Use --marketplace-path claude=PATH instead."), +) +@click.option( + "-m", + "--marketplace", + "marketplace_filter", + type=str, + default=None, + help=( + "Comma-separated marketplace outputs to build (e.g. 'claude,codex'). " + "Use 'all' for every configured output, 'none' to skip marketplace. " + "Default: build all configured outputs." + ), +) +@click.option( + "--marketplace-path", + "marketplace_path_overrides", + type=str, + multiple=True, help=( - "Marketplace legacy compatibility: override only the Claude/Anthropic " - "output path. Prefer marketplace.claude.output in apm.yml." + "Override output path for a format: FORMAT=PATH (repeatable). " + "Example: --marketplace-path claude=dist/marketplace.json" ), ) +@click.option( + "--json", + "json_output", + is_flag=True, + default=False, + help="Emit machine-readable JSON to stdout; logs go to stderr.", +) @click.option( "--legacy-skill-paths", "legacy_skill_paths", @@ -133,10 +177,79 @@ def pack_cmd( offline, include_prerelease, marketplace_output, + marketplace_filter, + marketplace_path_overrides, + json_output, legacy_skill_paths, ): """Pack APM artifacts: bundle and/or marketplace.json.""" + from ..marketplace.output_profiles import known_output_names + from ..utils.path_security import validate_path_segments + + # -- Stream discipline: under --json, route ALL output to stderr -- + if json_output: + set_console_stderr(True) + logger = CommandLogger("pack", verbose=verbose, dry_run=dry_run) + + # -- Deprecation: --marketplace-output → --marketplace-path claude=PATH -- + if marketplace_output is not None: + translated = f"--marketplace-path claude={marketplace_output}" + click.echo( + f"Warning: --marketplace-output is deprecated and will be removed in v0.15. " + f"Use {translated} instead.", + err=True, + ) + marketplace_path_overrides = ( + *marketplace_path_overrides, + f"claude={marketplace_output}", + ) + marketplace_output = None + + # -- Parse --marketplace-path overrides -- + path_overrides: dict[str, str] = {} + for override in marketplace_path_overrides: + if "=" not in override: + msg = f"--marketplace-path must be FORMAT=PATH, got: {override!r}" + _emit_json_error_or_raise(ctx, json_output, "cli_error", msg) + return + fmt_name, path_val = override.split("=", 1) + fmt_name = fmt_name.strip() + path_val = path_val.strip() + if fmt_name not in known_output_names(): + msg = ( + f"Unknown marketplace format '{fmt_name}' in --marketplace-path. " + f"Known formats: {', '.join(sorted(known_output_names()))}" + ) + _emit_json_error_or_raise(ctx, json_output, "unknown_format", msg) + return + # Security: validate path to prevent traversal attacks + try: + validate_path_segments(path_val, context="--marketplace-path", allow_current_dir=True) + except Exception as exc: + _emit_json_error_or_raise(ctx, json_output, "path_error", str(exc)) + return + path_overrides[fmt_name] = path_val + + # -- Parse --marketplace filter -- + marketplace_formats: tuple[str, ...] | None = None + if marketplace_filter is not None: + if marketplace_filter.strip().lower() == "none": + marketplace_formats = () + elif marketplace_filter.strip().lower() == "all": + marketplace_formats = None # all configured + else: + requested = [f.strip() for f in marketplace_filter.split(",") if f.strip()] + known = known_output_names() + for r in requested: + if r not in known: + msg = ( + f"Unknown marketplace format '{r}' in --marketplace. " + f"Known formats: {', '.join(sorted(known))}" + ) + _emit_json_error_or_raise(ctx, json_output, "unknown_format", msg) + return + marketplace_formats = tuple(requested) project_root = Path(".").resolve() # Issue #1207 D1: when --target is not given, detect the project's # actual target so the embedded ``pack.target`` reflects what was @@ -169,7 +282,9 @@ def pack_cmd( bundle_force=force, marketplace_offline=offline, marketplace_include_prerelease=include_prerelease, - marketplace_output=Path(marketplace_output) if marketplace_output else None, + marketplace_output=None, + marketplace_formats=marketplace_formats, + marketplace_path_overrides=path_overrides if path_overrides else None, dry_run=dry_run, verbose=verbose, ) @@ -177,7 +292,27 @@ def pack_cmd( try: result = BuildOrchestrator().run(options, logger=logger) except BuildError as exc: - raise click.ClickException(str(exc)) # noqa: B904 + _emit_json_error_or_raise(ctx, json_output, "build_error", str(exc)) + return + + # -- JSON output mode: consistent envelope -- + if json_output: + envelope = { + "ok": True, + "dry_run": dry_run, + "warnings": [], + "errors": [], + "marketplace": {"outputs": []}, + "bundle": None, + } + for sub in result.producer_results: + if sub.kind is OutputKind.MARKETPLACE and sub.payload is not None: + payload = sub.payload.to_json_dict() + envelope["warnings"] = payload.get("warnings", []) + envelope["marketplace"] = payload.get("marketplace", {"outputs": []}) + break + click.echo(json_mod.dumps(envelope, indent=2)) + return for sub in result.producer_results: if sub.kind is OutputKind.BUNDLE: diff --git a/src/apm_cli/core/build_orchestrator.py b/src/apm_cli/core/build_orchestrator.py index 1b9e788fd..9e5cfd730 100644 --- a/src/apm_cli/core/build_orchestrator.py +++ b/src/apm_cli/core/build_orchestrator.py @@ -46,6 +46,8 @@ class BuildOptions: marketplace_offline: bool = False marketplace_include_prerelease: bool = False marketplace_output: Path | None = None + marketplace_formats: tuple[str, ...] | None = None + marketplace_path_overrides: dict[str, str] | None = None # Common options dry_run: bool = False verbose: bool = False @@ -182,7 +184,13 @@ def _warn(msg: str) -> None: resolve_result = None output_reports = [] outputs: list[Path] = [] - for output_name in config.outputs: + + # 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] + + for output_name in active_outputs: profile = MARKETPLACE_OUTPUTS.get(output_name) if profile is None: valid_targets = ", ".join(sorted(MARKETPLACE_OUTPUTS)) @@ -198,7 +206,16 @@ def _warn(msg: str) -> None: configured_output_value = getattr(config, profile.config_attr).output configured_output = Path(configured_output_value) output_path = project_root / configured_output - if profile.supports_cli_output_override and options.marketplace_output is not None: + + # Apply --marketplace-path override + if ( + options.marketplace_path_overrides + and output_name in options.marketplace_path_overrides + ): + output_path = project_root / options.marketplace_path_overrides[output_name] + elif ( + profile.supports_cli_output_override and options.marketplace_output is not None + ): output_path = options.marketplace_output output_report = builder.write_output( diff --git a/src/apm_cli/marketplace/builder.py b/src/apm_cli/marketplace/builder.py index f506f1679..55080b281 100644 --- a/src/apm_cli/marketplace/builder.py +++ b/src/apm_cli/marketplace/builder.py @@ -181,6 +181,67 @@ def output_path(self) -> Path: def dry_run(self) -> bool: return any(output.dry_run for output in self.outputs) + def to_json_dict(self) -> dict[str, Any]: + """Serialize build report as the §4 JSON contract. + + Shape: {ok, dry_run, warnings[], errors[], + marketplace: {outputs: [{format, path, added, updated, + unchanged, skipped}]}, bundle: null} + """ + all_warnings = list(self.warnings) + all_errors: list[dict[str, str]] = [] + output_entries: list[dict[str, Any]] = [] + + for out in self.outputs: + output_entries.append( + { + "format": out.profile, + "path": str(out.output_path), + "added": out.added_count, + "updated": out.updated_count, + "unchanged": out.unchanged_count, + "skipped": out.removed_count, + } + ) + for pkg_name, err_msg in out.errors: + all_errors.append({"code": "build_error", "message": f"{pkg_name}: {err_msg}"}) + + ok = len(all_errors) == 0 + return { + "ok": ok, + "dry_run": self.dry_run, + "warnings": all_warnings, + "errors": all_errors, + "marketplace": { + "outputs": output_entries, + }, + "bundle": None, + } + + @classmethod + def failure_to_json_dict( + cls, + *, + errors: list[dict[str, str]], + warnings: list[str] | None = None, + dry_run: bool = False, + ) -> dict[str, Any]: + """Produce the §4 JSON shape for a pre-build failure. + + Used when the build cannot even start (e.g., config parse error, + unknown format filter). + """ + return { + "ok": False, + "dry_run": dry_run, + "warnings": warnings or [], + "errors": errors, + "marketplace": { + "outputs": [], + }, + "bundle": None, + } + @dataclass class BuildOptions: diff --git a/src/apm_cli/marketplace/init_template.py b/src/apm_cli/marketplace/init_template.py index e7dac3d5f..112481a12 100644 --- a/src/apm_cli/marketplace/init_template.py +++ b/src/apm_cli/marketplace/init_template.py @@ -107,17 +107,17 @@ def render_marketplace_yml_template( build: tagPattern: "v{{version}}" - # Output targets. Claude output is the default for backwards compatibility. - # Add outputs: [claude, codex] to also write .agents/plugins/marketplace.json, - # or outputs: [codex] to build Codex only. - # outputs: [claude, codex] - # - # claude: - # output: .claude-plugin/marketplace.json - # - # Optional Codex output overrides: - # codex: - # output: .agents/plugins/marketplace.json + # Output targets (map form). Claude is enabled by default. + outputs: + claude: {{}} + # Uncomment to also build the Codex marketplace artifact: + # codex: + # path: .agents/plugins/marketplace.json + # NOTE: codex output requires every package to declare 'category:' + # in apm.yml -- see the packages section below. + + # CI tip: build one or all formats with a machine-readable manifest: + # apm pack --marketplace=claude,codex --json | jq -r '.marketplace.outputs[].path' packages: - name: example-package diff --git a/src/apm_cli/marketplace/output_profiles.py b/src/apm_cli/marketplace/output_profiles.py index 087f5b92b..7c25b9b2c 100644 --- a/src/apm_cli/marketplace/output_profiles.py +++ b/src/apm_cli/marketplace/output_profiles.py @@ -7,8 +7,13 @@ from __future__ import annotations +import re from dataclasses import dataclass +_ENV_VAR_PATTERN = re.compile(r"^APM_MARKETPLACE_[A-Z0-9_]+_PATH$") +_RESERVED_NAMES = frozenset({"all", "none"}) +_INVALID_NAME_CHARS = frozenset("=,/ \t") + @dataclass(frozen=True) class MarketplaceOutputProfile: @@ -26,6 +31,14 @@ class MarketplaceOutputProfile: mapper: str """Mapper identifier used by ``MarketplaceBuilder`` to build the JSON.""" + path_env_var: str + """Environment variable that overrides the output path for this profile. + + Declared for schema validation at registration time. Env-var consumption + is NOT yet implemented — planned for v0.15. The field is validated against + ``_ENV_VAR_PATTERN`` to prevent collisions with sensitive variables. + """ + required_package_fields: tuple[str, ...] = () """PackageEntry fields required when this output is selected.""" @@ -33,11 +46,30 @@ class MarketplaceOutputProfile: """Whether ``apm pack --marketplace-output`` can override this output path.""" +def _validate_profile(profile: MarketplaceOutputProfile) -> None: + """Validate a profile at registration time. + + Guards against reserved sentinel names, CLI-unfriendly characters, + and env-var names that could collide with sensitive variables. + """ + if profile.name in _RESERVED_NAMES: + raise ValueError(f"Profile name {profile.name!r} is reserved as a --marketplace sentinel.") + if any(c in _INVALID_NAME_CHARS for c in profile.name) or profile.name.startswith("-"): + raise ValueError(f"Profile name {profile.name!r} contains a CLI-reserved character.") + if not _ENV_VAR_PATTERN.fullmatch(profile.path_env_var): + raise ValueError( + f"Profile {profile.name!r} declares path_env_var " + f"{profile.path_env_var!r}; expected " + f"APM_MARKETPLACE__PATH." + ) + + DEFAULT_MARKETPLACE_OUTPUT = MarketplaceOutputProfile( name="claude", config_attr="claude", default_output=".claude-plugin/marketplace.json", mapper="claude", + path_env_var="APM_MARKETPLACE_CLAUDE_PATH", supports_cli_output_override=True, ) @@ -46,6 +78,7 @@ class MarketplaceOutputProfile: config_attr="codex", default_output=".agents/plugins/marketplace.json", mapper="codex", + path_env_var="APM_MARKETPLACE_CODEX_PATH", required_package_fields=("category",), ) @@ -57,6 +90,10 @@ class MarketplaceOutputProfile: ) } +# Validate all registered profiles at module import. +for _profile in MARKETPLACE_OUTPUTS.values(): + _validate_profile(_profile) + def known_output_names() -> frozenset[str]: """Return the supported marketplace output names.""" diff --git a/src/apm_cli/marketplace/yml_schema.py b/src/apm_cli/marketplace/yml_schema.py index bb1ae5d63..d97c21521 100644 --- a/src/apm_cli/marketplace/yml_schema.py +++ b/src/apm_cli/marketplace/yml_schema.py @@ -49,6 +49,7 @@ "MarketplaceClaudeConfig", "MarketplaceCodexConfig", "MarketplaceConfig", + "MarketplaceOutputSpec", "MarketplaceOwner", "MarketplaceYml", # backwards-compat alias "MarketplaceYmlError", @@ -263,6 +264,25 @@ class PackageEntry: is_local: bool = False +@dataclass(frozen=True) +class MarketplaceOutputSpec: + """Resolved specification for one marketplace output format. + + Produced by the map-form ``outputs:`` parser. When ``path_explicit`` + is True, the manifest set an explicit ``path:`` value (vs. the + profile default). + """ + + name: str + """Format name (matches a key in ``MARKETPLACE_OUTPUTS``).""" + + path: str + """Resolved output path (explicit or profile default).""" + + path_explicit: bool = False + """True if the user set an explicit ``path:`` in the outputs map.""" + + @dataclass(frozen=True) class MarketplaceConfig: """Parsed marketplace configuration. @@ -292,6 +312,8 @@ class MarketplaceConfig: metadata: dict[str, Any] = field(default_factory=dict) build: MarketplaceBuild = field(default_factory=MarketplaceBuild) packages: tuple[PackageEntry, ...] = () + output_specs: tuple[MarketplaceOutputSpec, ...] = () + warnings: tuple[str, ...] = () # Origin tracking + override-detection metadata source_path: Path | None = None is_legacy: bool = False @@ -453,24 +475,92 @@ def _parse_codex(raw: Any) -> MarketplaceCodexConfig: return MarketplaceCodexConfig(output=output) -def _parse_outputs(raw: Any) -> tuple[str, ...]: - """Parse the marketplace output selector list. +def _parse_outputs( + raw: Any, + warnings_sink: list[str] | None = None, +) -> tuple[tuple[str, ...], tuple[MarketplaceOutputSpec, ...]]: + """Parse the marketplace output selector. + + Accepts: + - ``None`` → default (claude only). + - A list of strings → back-compat list form (emits deprecation warning). + - A string → single-element back-compat list form. + - A dict → new map form with optional per-format ``path:``. - ``outputs`` mirrors the repo's top-level target-selection pattern: - omit it for the backwards-compatible Claude output, or provide one - or more named marketplace artifacts to write. + Returns ``(outputs_tuple, output_specs_tuple)``. """ if raw is None: - return ("claude",) + default_spec = MarketplaceOutputSpec( + name="claude", + path=MARKETPLACE_OUTPUTS["claude"].default_output, + path_explicit=False, + ) + return ("claude",), (default_spec,) + + # --- Map form (new) --- + if isinstance(raw, dict): + outputs: list[str] = [] + specs: list[MarketplaceOutputSpec] = [] + seen: set[str] = set() + known = known_output_names() + + for key, value in raw.items(): + if not isinstance(key, str) or not key.strip(): + raise MarketplaceYmlError("'outputs' map keys must be non-empty strings") + name = key.strip() + if name not in known: + raise MarketplaceYmlError( + f"Unknown marketplace output '{name}'. " + f"Permitted outputs: {', '.join(sorted(known))}" + ) + if name in seen: + raise MarketplaceYmlError(f"Duplicate marketplace output '{name}'") + seen.add(name) + + # Value can be null/{}/mapping with optional path + path_explicit = False + path = MARKETPLACE_OUTPUTS[name].default_output + if value is not None: + if not isinstance(value, dict): + raise MarketplaceYmlError(f"'outputs.{name}' must be a mapping or null") + raw_path = value.get("path") + if raw_path is not None: + if not isinstance(raw_path, str) or not raw_path.strip(): + raise MarketplaceYmlError( + f"'outputs.{name}.path' must be a non-empty string" + ) + path = raw_path.strip() + path_explicit = True + try: + validate_path_segments(path, context=f"outputs.{name}.path") + except PathTraversalError as exc: + raise MarketplaceYmlError(str(exc)) from exc + # Check for unknown keys inside the format entry + _valid_output_entry_keys = {"path"} + unknown = set(value.keys()) - _valid_output_entry_keys + if unknown: + raise MarketplaceYmlError( + f"Unknown key(s) in 'outputs.{name}': {', '.join(sorted(unknown))}" + ) + + outputs.append(name) + specs.append(MarketplaceOutputSpec(name=name, path=path, path_explicit=path_explicit)) + + if not outputs: + raise MarketplaceYmlError("'outputs' must contain at least one marketplace output") + return tuple(outputs), tuple(specs) + + # --- List / string form (deprecated back-compat) --- if isinstance(raw, str): raw_items = [raw] elif isinstance(raw, list): raw_items = raw else: - raise MarketplaceYmlError("'outputs' must be a string or list of strings") + raise MarketplaceYmlError("'outputs' must be a string, list, or mapping") - outputs: list[str] = [] - seen: set[str] = set() + outputs_list: list[str] = [] + specs_list: list[MarketplaceOutputSpec] = [] + seen_set: set[str] = set() for index, item in enumerate(raw_items): if not isinstance(item, str) or not item.strip(): raise MarketplaceYmlError(f"'outputs[{index}]' must be a non-empty string") @@ -481,14 +571,33 @@ def _parse_outputs(raw: Any) -> tuple[str, ...]: f"Unknown marketplace output '{output}'. " f"Permitted outputs: {', '.join(sorted(known_outputs))}" ) - if output in seen: + if output in seen_set: raise MarketplaceYmlError(f"Duplicate marketplace output '{output}'") - seen.add(output) - outputs.append(output) + seen_set.add(output) + outputs_list.append(output) + specs_list.append( + MarketplaceOutputSpec( + name=output, + path=MARKETPLACE_OUTPUTS[output].default_output, + path_explicit=False, + ) + ) - if not outputs: + if not outputs_list: raise MarketplaceYmlError("'outputs' must contain at least one marketplace output") - return tuple(outputs) + + # Emit deprecation warning for list/string form + names_str = ", ".join(outputs_list) + map_lines = "\n".join(f" {n}: {{}}" for n in outputs_list) + deprecation_msg = ( + f"outputs: [{names_str}] is deprecated; use the map form:\n\n" + f" outputs:\n{map_lines}\n\n" + f" The list form will be removed in v0.15." + ) + if warnings_sink is not None: + warnings_sink.append(deprecation_msg) + + return tuple(outputs_list), tuple(specs_list) def _parse_package_entry(raw: Any, index: int) -> PackageEntry: @@ -853,6 +962,8 @@ def _build_config( """Shared parser for the marketplace fields once name/desc/version have been resolved (either inherited or read directly). """ + warnings_sink: list[str] = [] + # -- owner -- raw_owner = marketplace_dict.get("owner") if raw_owner is None: @@ -860,7 +971,9 @@ def _build_config( owner = _parse_owner(raw_owner) # -- output selection -- - outputs = _parse_outputs(marketplace_dict.get("outputs")) + outputs, output_specs = _parse_outputs( + marketplace_dict.get("outputs"), warnings_sink=warnings_sink + ) # -- Claude output (default differs between legacy and new layouts) -- # ``output`` remains as a backwards-compatible shorthand for @@ -906,6 +1019,44 @@ def _build_config( # -- codex output -- codex = _parse_codex(marketplace_dict.get("codex")) + # -- Sibling-vs-map conflict detection (A1: sibling wins) -- + # Only fire when the user EXPLICITLY set a sibling block AND the map + # also has an explicit path. Default/absent sibling is not a conflict. + has_explicit_claude = marketplace_dict.get("claude") is not None + has_explicit_codex = marketplace_dict.get("codex") is not None + + final_specs_list = list(output_specs) + for i, spec in enumerate(final_specs_list): + if spec.path_explicit: + sibling_path: str | None = None + if spec.name == "claude" and has_explicit_claude and claude.output != spec.path: + sibling_path = claude.output + elif spec.name == "codex" and has_explicit_codex and codex.output != spec.path: + sibling_path = codex.output + if sibling_path is not None: + warnings_sink.append( + f"marketplace.outputs.{spec.name}.path ('{spec.path}') " + f"conflicts with marketplace.{spec.name}.output " + f"('{sibling_path}').\n" + f" Using marketplace.{spec.name}.output for backwards " + f"compatibility.\n\n" + f" To resolve: pick one source and remove the other.\n" + f" Keep map form (recommended):\n" + f" outputs:\n" + f" {spec.name}:\n" + f" path: {sibling_path}\n" + f" # remove the marketplace.{spec.name}: block\n\n" + f" The marketplace.{spec.name} sibling block becomes a " + f"schema error in v0.15." + ) + # Sibling wins: override the spec's path + final_specs_list[i] = MarketplaceOutputSpec( + name=spec.name, + path=sibling_path, + path_explicit=True, + ) + output_specs = tuple(final_specs_list) + # -- packages -- raw_packages = marketplace_dict.get("packages") if raw_packages is None: @@ -949,6 +1100,8 @@ def _build_config( metadata=metadata, build=build, packages=tuple(entries), + output_specs=output_specs, + warnings=tuple(warnings_sink), source_path=source_path, is_legacy=is_legacy, name_overridden=name_overridden, diff --git a/src/apm_cli/utils/console.py b/src/apm_cli/utils/console.py index d2361fc78..84d010b99 100644 --- a/src/apm_cli/utils/console.py +++ b/src/apm_cli/utils/console.py @@ -65,6 +65,7 @@ # Thread-safe console singleton ------------------------------------------------ _console_instance: Any | None = None _console_lock = threading.Lock() +_console_stderr: bool = False def _get_console() -> Any | None: @@ -78,17 +79,31 @@ def _get_console() -> Any | None: if _console_instance is not None: return _console_instance try: # noqa: SIM105 - _console_instance = Console() + _console_instance = Console(stderr=_console_stderr) except Exception: pass return _console_instance +def set_console_stderr(enabled: bool = True) -> None: + """Route all console output to stderr. + + Call BEFORE any output is emitted (e.g. at CLI entry under --json). + Resets the console singleton so the next _get_console() call picks + up the new stream. + """ + global _console_stderr, _console_instance + with _console_lock: + _console_stderr = enabled + _console_instance = None + + def _reset_console() -> None: """Reset the console singleton. For testing only.""" - global _console_instance + global _console_instance, _console_stderr with _console_lock: _console_instance = None + _console_stderr = False def _rich_echo( @@ -133,9 +148,9 @@ def _rich_echo( } color_code = color_map.get(color, Fore.WHITE) style_code = Style.BRIGHT if bold else "" - click.echo(f"{color_code}{style_code}{message}{Style.RESET_ALL}") + click.echo(f"{color_code}{style_code}{message}{Style.RESET_ALL}", err=_console_stderr) else: - click.echo(message) + click.echo(message, err=_console_stderr) def _rich_success(message: str, symbol: str = None): # noqa: RUF013 diff --git a/tests/integration/marketplace/test_pack_ux_e2e.py b/tests/integration/marketplace/test_pack_ux_e2e.py new file mode 100644 index 000000000..89606d734 --- /dev/null +++ b/tests/integration/marketplace/test_pack_ux_e2e.py @@ -0,0 +1,298 @@ +"""E2E integration tests for marketplace pack UX (issue #1317). + +Covers: +- --json emits valid JSON with consistent envelope (no stdout contamination) +- --marketplace=FORMAT filter builds only requested formats +- --marketplace-path FORMAT=PATH writes to custom path +- --marketplace-path with path traversal is rejected +- --marketplace-output deprecation warning goes to stderr +- --marketplace=none skips marketplace entirely +""" + +from __future__ import annotations + +import json +from pathlib import Path +from unittest.mock import patch + +import pytest +from click.testing import CliRunner + +from apm_cli.commands.pack import pack_cmd +from apm_cli.utils.console import _reset_console + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +# Minimal apm.yml with marketplace block that uses map-form outputs +_APM_YML_MAP_FORM = """\ +name: test-project +version: 0.1.0 +marketplace: + name: test-marketplace + description: Test + version: 1.0.0 + owner: + name: Test + email: test@example.com + url: https://example.com + metadata: + pluginRoot: plugins + category: testing + packages: + - name: my-skill + description: A test skill + source: acme/my-skill + outputs: + claude: {} + codex: {} +""" + +_APM_YML_CLAUDE_ONLY = """\ +name: test-project +version: 0.1.0 +marketplace: + name: test-marketplace + description: Test + version: 1.0.0 + owner: + name: Test + email: test@example.com + url: https://example.com + metadata: + pluginRoot: plugins + category: testing + packages: + - name: my-skill + description: A test skill + source: acme/my-skill + outputs: + claude: {} +""" + + +def _setup_project(tmp_path: Path, yml_content: str = _APM_YML_MAP_FORM) -> Path: + """Write apm.yml and return the project directory.""" + (tmp_path / "apm.yml").write_text(yml_content, encoding="utf-8") + return tmp_path + + +def _mock_build_result(): + """Create a mock BuildResult that the orchestrator would return.""" + from apm_cli.core.build_orchestrator import BuildResult, OutputKind, ProducerResult + from apm_cli.marketplace.builder import ( + BuildReport, + MarketplaceOutputReport, + ) + + output_report = MarketplaceOutputReport( + profile="claude", + resolved=(), + errors=(), + warnings=(), + output_path=Path(".claude-plugin/marketplace.json"), + added_count=1, + updated_count=0, + unchanged_count=0, + removed_count=0, + ) + marketplace_report = BuildReport(outputs=(output_report,)) + + return BuildResult( + outputs=[], + warnings=[], + producer_results=[ + ProducerResult( + kind=OutputKind.MARKETPLACE, + outputs=[], + warnings=[], + payload=marketplace_report, + ) + ], + ) + + +@pytest.fixture(autouse=True) +def _reset_console_after(): + """Ensure console state is clean after each test.""" + yield + _reset_console() + + +# --------------------------------------------------------------------------- +# JSON output: consistent envelope +# --------------------------------------------------------------------------- + + +class TestJsonEnvelope: + """--json emits a consistent envelope with required top-level keys.""" + + def test_json_success_has_envelope_keys(self, tmp_path): + _setup_project(tmp_path) + mock_result = _mock_build_result() + + with patch("apm_cli.commands.pack.BuildOrchestrator") as MockOrch: + MockOrch.return_value.run.return_value = mock_result + runner = CliRunner() + result = runner.invoke( + pack_cmd, + ["--json"], + catch_exceptions=False, + env={"PWD": str(tmp_path)}, + ) + # CliRunner captures stdout in result.output + # Parse only lines that look like JSON + stdout = result.output.strip() + data = json.loads(stdout) + assert data["ok"] is True + assert "dry_run" in data + assert "warnings" in data + assert "errors" in data + assert "marketplace" in data + assert "bundle" in data + + def test_json_error_has_envelope_keys(self, tmp_path): + """Error JSON must have same top-level shape.""" + _setup_project(tmp_path) + + runner = CliRunner() + result = runner.invoke( + pack_cmd, + ["--json", "--marketplace", "bogus"], + ) + assert result.exit_code != 0 + data = json.loads(result.output) + assert data["ok"] is False + assert "errors" in data + assert isinstance(data["errors"], list) + assert len(data["errors"]) > 0 + + +# --------------------------------------------------------------------------- +# Stdout contamination +# --------------------------------------------------------------------------- + + +class TestStdoutContamination: + """Under --json, stdout must contain ONLY valid JSON.""" + + def test_no_log_contamination_in_json_stdout(self, tmp_path): + """When --json is set, no Rich/click output should appear on stdout.""" + _setup_project(tmp_path) + mock_result = _mock_build_result() + + with patch("apm_cli.commands.pack.BuildOrchestrator") as MockOrch: + MockOrch.return_value.run.return_value = mock_result + runner = CliRunner() + result = runner.invoke( + pack_cmd, + ["--json"], + catch_exceptions=False, + ) + stdout = result.output.strip() + # Every line of stdout must be part of the JSON object + # (no progress bars, no Rich formatting, no logger output) + try: + json.loads(stdout) + except json.JSONDecodeError: + pytest.fail(f"stdout under --json is not valid JSON:\n{stdout[:500]}") + + +# --------------------------------------------------------------------------- +# Path traversal rejection +# --------------------------------------------------------------------------- + + +class TestPathTraversal: + """--marketplace-path with traversal sequences must be rejected.""" + + def test_dotdot_rejected(self): + runner = CliRunner() + result = runner.invoke( + pack_cmd, + ["--marketplace-path", "claude=../../etc/passwd"], + ) + assert result.exit_code != 0 + combined = result.output + str(result.exception or "") + assert ".." in combined or "traversal" in combined.lower() + + def test_dotdot_rejected_json(self): + runner = CliRunner() + result = runner.invoke( + pack_cmd, + ["--marketplace-path", "claude=../../etc/passwd", "--json"], + ) + assert result.exit_code != 0 + data = json.loads(result.output) + assert data["ok"] is False + assert any( + ".." in e["message"] or "traversal" in e["message"].lower() for e in data["errors"] + ) + + +# --------------------------------------------------------------------------- +# Deprecation warning routing +# --------------------------------------------------------------------------- + + +class TestDeprecationRouting: + """--marketplace-output deprecation warning must go to stderr, not stdout.""" + + def test_deprecation_on_stderr(self, tmp_path): + """Deprecation message for --marketplace-output should be on stderr.""" + _setup_project(tmp_path) + mock_result = _mock_build_result() + + with patch("apm_cli.commands.pack.BuildOrchestrator") as MockOrch: + MockOrch.return_value.run.return_value = mock_result + runner = CliRunner() + result = runner.invoke( + pack_cmd, + ["--marketplace-output", "test.json", "--json"], + catch_exceptions=False, + ) + stdout = result.output.strip() + # Under --json, deprecation uses click.echo(err=True) so + # with default CliRunner (mix_stderr=True) it appears in output. + # Key assertion: the JSON portion is still parseable — strip + # the deprecation line and parse the rest. + lines = stdout.split("\n") + json_lines = [line for line in lines if not line.startswith("Warning:")] + json_str = "\n".join(json_lines).strip() + if json_str: + try: + data = json.loads(json_str) + assert data["ok"] is True + except json.JSONDecodeError: + pytest.fail(f"Non-JSON content leaked to stdout:\n{json_str[:500]}") + + +# --------------------------------------------------------------------------- +# Marketplace filter: --marketplace=none +# --------------------------------------------------------------------------- + + +class TestMarketplaceNone: + """--marketplace=none should skip marketplace entirely.""" + + def test_none_sentinel_json(self, tmp_path): + """With --marketplace=none and --json, marketplace.outputs is empty.""" + _setup_project(tmp_path) + + # Mock orchestrator to return empty result (no marketplace producer fires) + from apm_cli.core.build_orchestrator import BuildResult + + empty_result = BuildResult(outputs=[], warnings=[], producer_results=[]) + + with patch("apm_cli.commands.pack.BuildOrchestrator") as MockOrch: + MockOrch.return_value.run.return_value = empty_result + runner = CliRunner() + result = runner.invoke( + pack_cmd, + ["--marketplace=none", "--json"], + catch_exceptions=False, + ) + data = json.loads(result.output) + assert data["ok"] is True + assert data["marketplace"]["outputs"] == [] diff --git a/tests/unit/commands/test_pack.py b/tests/unit/commands/test_pack.py index 0ae71f5f5..311d6a6eb 100644 --- a/tests/unit/commands/test_pack.py +++ b/tests/unit/commands/test_pack.py @@ -31,8 +31,10 @@ def test_pack_help_recommends_manifest_marketplace_output_config() -> None: result = CliRunner().invoke(pack_cmd, ["--help"]) assert result.exit_code == 0 - assert "Prefer" in result.output - assert "marketplace.claude.output in apm.yml" in result.output + # The new --marketplace-path flag is shown in help + assert "--marketplace-path" in result.output + # The deprecated --marketplace-output is hidden + assert "--marketplace-output" not in result.output assert "--claude-output" not in result.output diff --git a/tests/unit/commands/test_pack_cli_flags.py b/tests/unit/commands/test_pack_cli_flags.py new file mode 100644 index 000000000..69c6c3189 --- /dev/null +++ b/tests/unit/commands/test_pack_cli_flags.py @@ -0,0 +1,83 @@ +"""Tests for new CLI flags in pack command (phase-3c, T-3c-01..12). + +Covers: +- --marketplace filter validation (unknown format → error) +- --marketplace-path FORMAT=PATH parsing + validation +- --json flag emits valid JSON on failure +- --marketplace-output deprecation warning +""" + +from __future__ import annotations + +from click.testing import CliRunner + +from apm_cli.commands.pack import pack_cmd + + +class TestMarketplaceFilterFlag: + """T-3c-01..04: --marketplace flag parsing.""" + + def test_unknown_format_raises(self) -> None: + result = CliRunner().invoke(pack_cmd, ["--marketplace", "bogus"]) + assert result.exit_code != 0 + assert "Unknown marketplace format" in ( + result.output + (result.exception.__str__() if result.exception else "") + ) + + def test_unknown_format_json_mode(self) -> None: + import json + + result = CliRunner().invoke(pack_cmd, ["--marketplace", "bogus", "--json"]) + # Should output valid JSON to stdout even on error + assert result.exit_code != 0 + data = json.loads(result.output) + assert data["ok"] is False + assert any("bogus" in e["message"] for e in data["errors"]) + + +class TestMarketplacePathFlag: + """T-3c-05..08: --marketplace-path parsing.""" + + def test_missing_equals_raises(self) -> None: + result = CliRunner().invoke(pack_cmd, ["--marketplace-path", "noequalssign"]) + assert result.exit_code != 0 + assert "FORMAT=PATH" in ( + result.output + (result.exception.__str__() if result.exception else "") + ) + + def test_unknown_format_raises(self) -> None: + result = CliRunner().invoke(pack_cmd, ["--marketplace-path", "bogus=path.json"]) + assert result.exit_code != 0 + assert "Unknown marketplace format" in ( + result.output + (result.exception.__str__() if result.exception else "") + ) + + def test_missing_equals_json_mode(self) -> None: + import json + + result = CliRunner().invoke(pack_cmd, ["--marketplace-path", "noequalssign", "--json"]) + assert result.exit_code != 0 + data = json.loads(result.output) + assert data["ok"] is False + assert any("FORMAT=PATH" in e["message"] for e in data["errors"]) + + +class TestJsonFlag: + """T-3c-09..10: --json flag appears in help.""" + + def test_json_in_help(self) -> None: + result = CliRunner().invoke(pack_cmd, ["--help"]) + assert "--json" in result.output + assert "machine-readable" in result.output.lower() or "JSON" in result.output + + +class TestDeprecationWarning: + """T-3c-11..12: --marketplace-output deprecation.""" + + def test_deprecated_flag_still_accepted(self) -> None: + """The flag doesn't crash immediately (it will fail later + because no apm.yml exists, but that's fine — we check the + deprecation message is printed before the crash).""" + result = CliRunner().invoke(pack_cmd, ["--marketplace-output", "test.json"]) + combined = result.output or "" + assert "deprecated" in combined.lower() or result.exit_code != 0 diff --git a/tests/unit/marketplace/test_builder_json.py b/tests/unit/marketplace/test_builder_json.py new file mode 100644 index 000000000..2f915fc2d --- /dev/null +++ b/tests/unit/marketplace/test_builder_json.py @@ -0,0 +1,137 @@ +"""Tests for BuildReport JSON serialization (phase-3b, T-3b-01..08). + +Covers: +- to_json_dict() produces correct §4 shape +- failure_to_json_dict() classmethod shape +- ok/dry_run flags, warnings/errors aggregation +""" + +from __future__ import annotations + +from pathlib import Path + +from apm_cli.marketplace.builder import ( + BuildReport, + MarketplaceOutputReport, +) + + +def _make_output_report( + *, + profile: str = "claude", + output_path: str = ".claude-plugin/marketplace.json", + added: int = 0, + updated: int = 0, + unchanged: int = 0, + removed: int = 0, + errors: tuple[tuple[str, str], ...] = (), + warnings: tuple[str, ...] = (), + dry_run: bool = False, +) -> MarketplaceOutputReport: + return MarketplaceOutputReport( + profile=profile, + resolved=(), + errors=errors, + warnings=warnings, + added_count=added, + updated_count=updated, + unchanged_count=unchanged, + removed_count=removed, + output_path=Path(output_path), + dry_run=dry_run, + ) + + +class TestBuildReportToJsonDict: + """T-3b-01..05: to_json_dict() shape.""" + + def test_success_shape(self) -> None: + out = _make_output_report(added=2, updated=1, unchanged=3) + report = BuildReport(outputs=(out,)) + result = report.to_json_dict() + + assert result["ok"] is True + assert result["dry_run"] is False + assert result["bundle"] is None + assert result["warnings"] == [] + assert result["errors"] == [] + assert len(result["marketplace"]["outputs"]) == 1 + + entry = result["marketplace"]["outputs"][0] + assert entry["format"] == "claude" + assert entry["added"] == 2 + assert entry["updated"] == 1 + assert entry["unchanged"] == 3 + assert entry["skipped"] == 0 + + def test_multiple_outputs(self) -> None: + out1 = _make_output_report(profile="claude", added=1) + out2 = _make_output_report( + profile="codex", + output_path=".agents/plugins/marketplace.json", + added=2, + ) + report = BuildReport(outputs=(out1, out2)) + result = report.to_json_dict() + + assert result["ok"] is True + assert len(result["marketplace"]["outputs"]) == 2 + formats = [e["format"] for e in result["marketplace"]["outputs"]] + assert "claude" in formats + assert "codex" in formats + + def test_errors_make_ok_false(self) -> None: + out = _make_output_report( + errors=(("my-tool", "git timeout"),), + ) + report = BuildReport(outputs=(out,)) + result = report.to_json_dict() + + assert result["ok"] is False + assert len(result["errors"]) == 1 + assert result["errors"][0]["code"] == "build_error" + assert "my-tool" in result["errors"][0]["message"] + + def test_warnings_aggregated(self) -> None: + out = _make_output_report( + warnings=("warning A", "warning B"), + ) + report = BuildReport(outputs=(out,)) + result = report.to_json_dict() + + assert result["warnings"] == ["warning A", "warning B"] + + def test_dry_run_flag(self) -> None: + out = _make_output_report(dry_run=True) + report = BuildReport(outputs=(out,)) + result = report.to_json_dict() + + assert result["dry_run"] is True + + +class TestFailureToJsonDict: + """T-3b-06..08: failure_to_json_dict() classmethod.""" + + def test_basic_failure_shape(self) -> None: + result = BuildReport.failure_to_json_dict( + errors=[{"code": "config_error", "message": "bad config"}] + ) + assert result["ok"] is False + assert result["dry_run"] is False + assert result["bundle"] is None + assert result["marketplace"]["outputs"] == [] + assert len(result["errors"]) == 1 + + def test_with_warnings(self) -> None: + result = BuildReport.failure_to_json_dict( + errors=[{"code": "unknown_format", "message": "no such format"}], + warnings=["deprecated flag used"], + ) + assert result["warnings"] == ["deprecated flag used"] + + def test_dry_run_passthrough(self) -> None: + result = BuildReport.failure_to_json_dict( + errors=[{"code": "x", "message": "y"}], + dry_run=True, + ) + assert result["dry_run"] is True diff --git a/tests/unit/marketplace/test_output_profiles.py b/tests/unit/marketplace/test_output_profiles.py new file mode 100644 index 000000000..2e34d83df --- /dev/null +++ b/tests/unit/marketplace/test_output_profiles.py @@ -0,0 +1,154 @@ +"""Tests for marketplace output profiles (phase-3a, T-3a-01..08). + +Covers: +- path_env_var field presence and shape validation +- _validate_profile rejection of reserved names / bad chars / bad env-vars +- Registration invariants (no duplicates, all pass validation) +- known_output_names() returns correct frozenset +""" + +from __future__ import annotations + +import pytest + +from apm_cli.marketplace.output_profiles import ( + CODEX_MARKETPLACE_OUTPUT, + DEFAULT_MARKETPLACE_OUTPUT, + MARKETPLACE_OUTPUTS, + MarketplaceOutputProfile, + _validate_profile, + known_output_names, +) + + +class TestProfileFields: + """T-3a-01: Verify all registered profiles have path_env_var.""" + + def test_claude_has_path_env_var(self) -> None: + assert DEFAULT_MARKETPLACE_OUTPUT.path_env_var == "APM_MARKETPLACE_CLAUDE_PATH" + + def test_codex_has_path_env_var(self) -> None: + assert CODEX_MARKETPLACE_OUTPUT.path_env_var == "APM_MARKETPLACE_CODEX_PATH" + + def test_all_profiles_env_var_pattern(self) -> None: + import re + + pattern = re.compile(r"^APM_MARKETPLACE_[A-Z0-9_]+_PATH$") + for profile in MARKETPLACE_OUTPUTS.values(): + assert pattern.fullmatch(profile.path_env_var), ( + f"Profile {profile.name!r} has invalid path_env_var: {profile.path_env_var!r}" + ) + + +class TestValidateProfile: + """T-3a-02..05: _validate_profile guards.""" + + def test_reserved_name_all(self) -> None: + p = MarketplaceOutputProfile( + name="all", + config_attr="x", + default_output="x", + mapper="x", + path_env_var="APM_MARKETPLACE_ALL_PATH", + ) + with pytest.raises(ValueError, match="reserved"): + _validate_profile(p) + + def test_reserved_name_none(self) -> None: + p = MarketplaceOutputProfile( + name="none", + config_attr="x", + default_output="x", + mapper="x", + path_env_var="APM_MARKETPLACE_NONE_PATH", + ) + with pytest.raises(ValueError, match="reserved"): + _validate_profile(p) + + def test_invalid_name_with_equals(self) -> None: + p = MarketplaceOutputProfile( + name="cla=ude", + config_attr="x", + default_output="x", + mapper="x", + path_env_var="APM_MARKETPLACE_CLAUDE_PATH", + ) + with pytest.raises(ValueError, match="CLI-reserved"): + _validate_profile(p) + + def test_invalid_name_leading_dash(self) -> None: + p = MarketplaceOutputProfile( + name="-claude", + config_attr="x", + default_output="x", + mapper="x", + path_env_var="APM_MARKETPLACE_CLAUDE_PATH", + ) + with pytest.raises(ValueError, match="CLI-reserved"): + _validate_profile(p) + + def test_invalid_name_with_comma(self) -> None: + p = MarketplaceOutputProfile( + name="cla,ude", + config_attr="x", + default_output="x", + mapper="x", + path_env_var="APM_MARKETPLACE_CLAUDE_PATH", + ) + with pytest.raises(ValueError, match="CLI-reserved"): + _validate_profile(p) + + def test_invalid_env_var_pattern(self) -> None: + p = MarketplaceOutputProfile( + name="myformat", + config_attr="x", + default_output="x", + mapper="x", + path_env_var="MY_CUSTOM_PATH", + ) + with pytest.raises(ValueError, match="APM_MARKETPLACE_"): + _validate_profile(p) + + def test_valid_profile_passes(self) -> None: + p = MarketplaceOutputProfile( + name="myformat", + config_attr="myformat", + default_output="output.json", + mapper="myformat", + path_env_var="APM_MARKETPLACE_MYFORMAT_PATH", + ) + # Should not raise + _validate_profile(p) + + +class TestKnownOutputNames: + """T-3a-06: known_output_names returns frozenset of registered names.""" + + def test_returns_frozenset(self) -> None: + result = known_output_names() + assert isinstance(result, frozenset) + + def test_contains_claude_and_codex(self) -> None: + result = known_output_names() + assert "claude" in result + assert "codex" in result + + def test_matches_registry_keys(self) -> None: + assert known_output_names() == frozenset(MARKETPLACE_OUTPUTS.keys()) + + +class TestRegistryInvariants: + """T-3a-07: Registry-level invariants.""" + + def test_no_duplicate_names(self) -> None: + names = [p.name for p in MARKETPLACE_OUTPUTS.values()] + assert len(names) == len(set(names)) + + def test_no_duplicate_env_vars(self) -> None: + env_vars = [p.path_env_var for p in MARKETPLACE_OUTPUTS.values()] + assert len(env_vars) == len(set(env_vars)) + + def test_all_registered_profiles_valid(self) -> None: + """All profiles in the registry pass validation (module-load guard).""" + for profile in MARKETPLACE_OUTPUTS.values(): + _validate_profile(profile) # Should not raise diff --git a/tests/unit/marketplace/test_outputs_map_form.py b/tests/unit/marketplace/test_outputs_map_form.py new file mode 100644 index 000000000..ba72638c0 --- /dev/null +++ b/tests/unit/marketplace/test_outputs_map_form.py @@ -0,0 +1,203 @@ +"""Tests for map-form outputs parsing in yml_schema (phase-3a, T-3a-09..26). + +Covers: +- Map form basic parsing (single format, multiple) +- Map form with explicit path +- Map form path validation (traversal rejection) +- Back-compat list form still works + emits deprecation warning +- MarketplaceOutputSpec fields +- Sibling conflict detection +""" + +from __future__ import annotations + +from pathlib import Path +from typing import Any + +import pytest + +from apm_cli.marketplace.yml_schema import ( + MarketplaceOutputSpec, + MarketplaceYmlError, + load_marketplace_from_apm_yml, +) + + +def _write_apm_yml(tmp_path: Path, marketplace_block: dict[str, Any]) -> Path: + """Write a minimal apm.yml with the given marketplace block.""" + import yaml + + content = { + "name": "test-pkg", + "description": "Test package", + "version": "1.0.0", + "marketplace": { + "owner": {"name": "Test Owner"}, + "packages": [ + { + "name": "my-tool", + "source": "org/repo", + "version": "1.0.0", + "description": "desc", + "category": "tools", + } + ], + **marketplace_block, + }, + } + yml_path = tmp_path / "apm.yml" + yml_path.write_text(yaml.dump(content, sort_keys=False), encoding="utf-8") + return yml_path + + +class TestMapFormParsing: + """T-3a-09..14: outputs as a dict (map form).""" + + def test_single_format_null_value(self, tmp_path: Path) -> None: + yml = _write_apm_yml(tmp_path, {"outputs": {"claude": None}}) + config = load_marketplace_from_apm_yml(yml) + assert config.outputs == ("claude",) + assert len(config.output_specs) == 1 + spec = config.output_specs[0] + assert spec.name == "claude" + assert spec.path == ".claude-plugin/marketplace.json" + assert spec.path_explicit is False + + def test_single_format_empty_dict(self, tmp_path: Path) -> None: + yml = _write_apm_yml(tmp_path, {"outputs": {"claude": {}}}) + config = load_marketplace_from_apm_yml(yml) + assert config.outputs == ("claude",) + assert config.output_specs[0].path_explicit is False + + def test_multiple_formats(self, tmp_path: Path) -> None: + yml = _write_apm_yml(tmp_path, {"outputs": {"claude": {}, "codex": {}}}) + config = load_marketplace_from_apm_yml(yml) + assert set(config.outputs) == {"claude", "codex"} + assert len(config.output_specs) == 2 + + def test_explicit_path(self, tmp_path: Path) -> None: + yml = _write_apm_yml(tmp_path, {"outputs": {"claude": {"path": "custom/output.json"}}}) + config = load_marketplace_from_apm_yml(yml) + spec = config.output_specs[0] + assert spec.path == "custom/output.json" + assert spec.path_explicit is True + + def test_unknown_format_raises(self, tmp_path: Path) -> None: + yml = _write_apm_yml(tmp_path, {"outputs": {"unknown_format": {}}}) + with pytest.raises(MarketplaceYmlError, match="Unknown marketplace output"): + load_marketplace_from_apm_yml(yml) + + def test_empty_map_raises(self, tmp_path: Path) -> None: + yml = _write_apm_yml(tmp_path, {"outputs": {}}) + with pytest.raises(MarketplaceYmlError, match="at least one"): + load_marketplace_from_apm_yml(yml) + + def test_path_traversal_rejected(self, tmp_path: Path) -> None: + yml = _write_apm_yml(tmp_path, {"outputs": {"claude": {"path": "../escape/file.json"}}}) + with pytest.raises(MarketplaceYmlError, match="path"): + load_marketplace_from_apm_yml(yml) + + def test_unknown_key_in_format_entry(self, tmp_path: Path) -> None: + yml = _write_apm_yml(tmp_path, {"outputs": {"claude": {"path": "x.json", "bogus": True}}}) + with pytest.raises(MarketplaceYmlError, match="Unknown key"): + load_marketplace_from_apm_yml(yml) + + def test_non_dict_format_value_raises(self, tmp_path: Path) -> None: + yml = _write_apm_yml(tmp_path, {"outputs": {"claude": "not_a_dict"}}) + with pytest.raises(MarketplaceYmlError, match="mapping or null"): + load_marketplace_from_apm_yml(yml) + + def test_no_deprecation_warning_for_map(self, tmp_path: Path) -> None: + yml = _write_apm_yml(tmp_path, {"outputs": {"claude": {}}}) + config = load_marketplace_from_apm_yml(yml) + assert not any("deprecated" in w for w in config.warnings) + + +class TestListFormBackCompat: + """T-3a-15..18: list form back-compat.""" + + def test_list_form_still_works(self, tmp_path: Path) -> None: + yml = _write_apm_yml(tmp_path, {"outputs": ["claude"]}) + config = load_marketplace_from_apm_yml(yml) + assert config.outputs == ("claude",) + assert len(config.output_specs) == 1 + assert config.output_specs[0].name == "claude" + + def test_list_form_emits_deprecation_warning(self, tmp_path: Path) -> None: + yml = _write_apm_yml(tmp_path, {"outputs": ["claude"]}) + config = load_marketplace_from_apm_yml(yml) + assert any("deprecated" in w for w in config.warnings) + assert any("map form" in w for w in config.warnings) + + def test_string_form_still_works(self, tmp_path: Path) -> None: + yml = _write_apm_yml(tmp_path, {"outputs": "claude"}) + config = load_marketplace_from_apm_yml(yml) + assert config.outputs == ("claude",) + assert any("deprecated" in w for w in config.warnings) + + def test_none_outputs_defaults_to_claude(self, tmp_path: Path) -> None: + yml = _write_apm_yml(tmp_path, {}) + config = load_marketplace_from_apm_yml(yml) + assert config.outputs == ("claude",) + assert config.output_specs[0].name == "claude" + # No deprecation warning for the default + assert not any("deprecated" in w for w in config.warnings) + + +class TestOutputSpecFields: + """T-3a-19..22: MarketplaceOutputSpec dataclass.""" + + def test_dataclass_frozen(self) -> None: + spec = MarketplaceOutputSpec(name="claude", path="x.json") + with pytest.raises((TypeError, AttributeError)): + spec.name = "other" # type: ignore[misc] + + def test_defaults(self) -> None: + spec = MarketplaceOutputSpec(name="claude", path="x.json") + assert spec.path_explicit is False + + def test_explicit_path_flag(self) -> None: + spec = MarketplaceOutputSpec(name="claude", path="x.json", path_explicit=True) + assert spec.path_explicit is True + + +class TestSiblingConflict: + """T-3a-23..26: sibling block vs outputs map conflict.""" + + def test_sibling_wins_on_conflict(self, tmp_path: Path) -> None: + """When outputs.claude.path and marketplace.claude.output differ, + sibling (marketplace.claude.output) wins.""" + yml = _write_apm_yml( + tmp_path, + { + "outputs": {"claude": {"path": "map-path.json"}}, + "claude": {"output": "sibling-path.json"}, + }, + ) + config = load_marketplace_from_apm_yml(yml) + # Sibling wins + spec = next(s for s in config.output_specs if s.name == "claude") + assert spec.path == "sibling-path.json" + + def test_sibling_conflict_emits_warning(self, tmp_path: Path) -> None: + yml = _write_apm_yml( + tmp_path, + { + "outputs": {"claude": {"path": "map-path.json"}}, + "claude": {"output": "sibling-path.json"}, + }, + ) + config = load_marketplace_from_apm_yml(yml) + assert any("conflicts" in w for w in config.warnings) + + def test_no_conflict_when_paths_match(self, tmp_path: Path) -> None: + """No warning when both sources agree on the same path.""" + yml = _write_apm_yml( + tmp_path, + { + "outputs": {"claude": {"path": "same.json"}}, + "claude": {"output": "same.json"}, + }, + ) + config = load_marketplace_from_apm_yml(yml) + assert not any("conflicts" in w for w in config.warnings)