diff --git a/CHANGELOG.md b/CHANGELOG.md index beb4e53002..91aa4d23c1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- `apm pack --check-clean` now certifies GitHub-hosted manifestless skills when `SKILL.md` has valid frontmatter at the resolved repository, ref, and subdirectory, while invalid skill metadata and missing repositories, refs, and package paths remain fail-closed. (#3064) - Codex agent conversion preserves native `model` and `model_reasoning_effort` settings and warns about dropped metadata instead of silently losing it. (#3150) - Cursor hooks use native v1 events and flat handlers; unsupported input and overlapping Claude imports now fail explicitly. Choose one hook route per dependency; see the [supported mappings](docs/src/content/docs/producer/author-primitives/hooks-and-commands.md#cursor-native-hooks-and-claude-import) and `openapm-v0.1.md` requirements `req-tg-016/017`. (#3149) 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 7e805328ac..e93613697e 100644 --- a/docs/src/content/docs/producer/publish-to-a-marketplace.md +++ b/docs/src/content/docs/producer/publish-to-a-marketplace.md @@ -237,7 +237,9 @@ apm pack --strict-metadata # require remote Claude package metadata fetches Strict remote metadata inheritance currently uses GitHub APIs. For GitLab, Azure DevOps, or another host, give each remote package a fixed `description` and fixed display `version` (not a range) so `--strict-metadata` can certify it -without fetching `apm.yml`. +without fetching `apm.yml`. On GitHub, an absent remote `apm.yml` is also +certifiable when APM verifies a `SKILL.md` at the same resolved repository, +ref, and package subdirectory. For the release-gate flags (`--check-versions`, `--check-clean`), see [Releasing from any CI](../releasing-from-any-ci/). 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 df3c9aad19..25b7eb4092 100644 --- a/docs/src/content/docs/producer/releasing-from-any-ci.md +++ b/docs/src/content/docs/producer/releasing-from-any-ci.md @@ -217,7 +217,7 @@ 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, 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. | +| 4 | `--check-clean` | Committed `marketplace.json` does not match a fresh pack, or remote Claude package metadata was unfetchable. A missing `apm.yml` is accepted only when `SKILL.md` verifies a manifestless skill at the same resolved path. 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-clean` is always read-only. `--check-versions` does not suppress normal diff --git a/docs/src/content/docs/reference/cli/pack.md b/docs/src/content/docs/reference/cli/pack.md index 434fc6ed93..6218e0fcaf 100644 --- a/docs/src/content/docs/reference/cli/pack.md +++ b/docs/src/content/docs/reference/cli/pack.md @@ -61,6 +61,7 @@ The `metadata_enrichment` object has a closed per-package status vocabulary: |---|---|---| | `fetched` | Remote `apm.yml` supplied metadata. | yes | | `empty` | Remote `apm.yml` was reachable but had no description or version. | yes | +| `manifestless` | Remote `apm.yml` was absent, but `SKILL.md` had valid `name` and `description` frontmatter at the resolved path. | yes | | `local` | Metadata came from a local package. | yes | | `explicit` | Fixed description and version came from the marketplace entry. | yes | | `failed` | The remote manifest could not be fetched. | no | @@ -92,9 +93,15 @@ uses `marketplace_metadata_uncertifiable` and exits `4`; `--strict-metadata` uses `metadata_incomplete` and exits `5` before writing. GitHub-hosted packages can inherit description and version from their remote -`apm.yml`. For GitLab, Azure DevOps, and other hosts, set fixed `description` -and `version` fields on the marketplace package entry so strict checks can -certify without a GitHub metadata request. +`apm.yml`. When that file returns `404`, APM verifies `SKILL.md` at the same +resolved repository, ref, and subdirectory. A verified manifestless skill is +certifiable when its frontmatter has non-empty string `name` and `description` +fields. Malformed frontmatter, missing fields, non-string values, and +whitespace-only values remain `failed` and make `--check-clean` exit `4`, as do +a missing repository, ref, or package path. +For GitLab, Azure DevOps, and other hosts, set fixed `description` and `version` +fields on the marketplace package entry so strict checks can certify without a +GitHub metadata request. ## Examples diff --git a/packages/apm-guide/.apm/skills/apm-usage/commands.md b/packages/apm-guide/.apm/skills/apm-usage/commands.md index 58c7eb3dae..3f04d555c6 100644 --- a/packages/apm-guide/.apm/skills/apm-usage/commands.md +++ b/packages/apm-guide/.apm/skills/apm-usage/commands.md @@ -345,9 +345,14 @@ marketplace output before any artifact write and exits `5` if remote metadata is unavailable. `--check-clean` also exits `4` without comparing bytes when regeneration is not certifiable. With `--json`, inspect `metadata_enrichment.certifiable` and each outcome's `status` -(`fetched`, `empty`, `explicit`, `failed`, `offline`, or `local`). GitLab, -Azure DevOps, and other non-GitHub packages need fixed `description` and -`version` fields for strict certification. +(`fetched`, `empty`, `manifestless`, `explicit`, `failed`, `offline`, or +`local`). On GitHub, `manifestless` means `apm.yml` was absent while +`SKILL.md` with non-empty string `name` and `description` frontmatter was +verified at the same resolved repository, ref, and subdirectory. Malformed +frontmatter, missing fields, non-string values, and whitespace-only values stay +`failed` and make `--check-clean` exit `4`. GitLab, Azure DevOps, and other +non-GitHub packages need fixed `description` and `version` fields for strict +certification. `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`), the imperative bundle route fails closed instead of dissecting it, with a message pointing at the declarative dependency route (`apm.yml` + `apm install --target copilot`) that registers the plugin natively with GitHub Copilot. 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/architecture_linter/checks/marketplace_tag_and_version.py b/scripts/architecture_linter/checks/marketplace_tag_and_version.py index 1d9dc16695..025c059891 100644 --- a/scripts/architecture_linter/checks/marketplace_tag_and_version.py +++ b/scripts/architecture_linter/checks/marketplace_tag_and_version.py @@ -315,6 +315,21 @@ def _check_metadata_enrichment(provider: FactsProvider) -> tuple[Violation, ...] parse=True, ) ) + findings.extend( + _require_subs( + provider, + inv, + _RID_METADATA, + _METADATA_OWNER, + ( + 'skill_path = f"{path_prefix}SKILL.md"', + "skill = loads_frontmatter(_request_text(url, rest=rest))", + 'return MetadataEnrichmentOutcome(pkg.name, "manifestless")', + ), + "marketplace metadata owner must parse manifestless skills before certification", + parse=True, + ) + ) findings.extend( _require_subs( provider, diff --git a/src/apm_cli/marketplace/builder.py b/src/apm_cli/marketplace/builder.py index 9e273a39b6..02bfbafeba 100644 --- a/src/apm_cli/marketplace/builder.py +++ b/src/apm_cli/marketplace/builder.py @@ -34,7 +34,7 @@ from ..utils.github_host import default_host, is_azure_devops_hostname from ..utils.path_security import ensure_path_within -from ..utils.yaml_io import load_yaml_str +from ..utils.yaml_io import load_yaml_str, loads_frontmatter from ._io import atomic_write from ._shared import iter_semver_tags from .auth_helpers import resolve_auth_for_host @@ -74,10 +74,22 @@ logger = logging.getLogger(__name__) _LOCAL_METADATA_MAX_BYTES = 64 * 1024 -_METADATA_STATUSES = frozenset({"fetched", "empty", "failed", "offline", "local", "explicit"}) -_CERTIFIABLE_METADATA_STATUSES = frozenset({"fetched", "empty", "local", "explicit"}) +_METADATA_STATUSES = frozenset( + {"fetched", "empty", "manifestless", "failed", "offline", "local", "explicit"} +) +_CERTIFIABLE_METADATA_STATUSES = frozenset( + {"fetched", "empty", "manifestless", "local", "explicit"} +) _CERTIFYING_METADATA_FIELDS = frozenset({"description", "version"}) -MetadataEnrichmentStatus = Literal["fetched", "empty", "failed", "offline", "local", "explicit"] +MetadataEnrichmentStatus = Literal[ + "fetched", + "empty", + "manifestless", + "failed", + "offline", + "local", + "explicit", +] def _read_capped_text(resp: Any) -> str: @@ -170,6 +182,14 @@ def to_json_dict(self) -> dict[str, Any]: return payload +@dataclass(frozen=True) +class _RemoteMetadataDocument: + """Remote manifest content or a verified manifestless skill marker.""" + + content: str | None + manifestless: bool = False + + @dataclass(frozen=True) class MetadataEnrichmentResult(Mapping[str, dict[str, str]]): """Canonical metadata enrichment state for one marketplace build. @@ -1153,7 +1173,7 @@ def _fetch_remote_metadata_outcome( self, pkg: ResolvedPackage, ) -> MetadataEnrichmentOutcome: - """Fetch remote metadata while preserving complete failure state. + """Fetch remote metadata while preserving complete package state. When a token is available for the package's host, it is included as an ``Authorization`` header so private repos can be accessed. @@ -1164,14 +1184,17 @@ def _fetch_remote_metadata_outcome( first, then fall back to the GitHub REST Contents endpoint when raw returns 404 (the private / INTERNAL repository symptom). GHES and GHE Cloud packages use the GitHub REST API on the - package's host. For non-GitHub-class hosts, metadata enrichment - is skipped. + package's host. When ``apm.yml`` is absent, the same transport + verifies a root ``SKILL.md`` before returning the certifiable + ``manifestless`` state. For non-GitHub-class hosts, metadata + enrichment is skipped. """ try: from ..core.auth import AuthResolver path_prefix = f"{pkg.subdir}/" if pkg.subdir else "" - file_path = f"{path_prefix}apm.yml" + manifest_path = f"{path_prefix}apm.yml" + skill_path = f"{path_prefix}SKILL.md" # Resolve the effective host for this package and its # classification. Falls back to the builder default when the @@ -1202,66 +1225,95 @@ def _fetch_remote_metadata_outcome( auth_resolver = self._auth_resolver - def _open_metadata(token: str | None, _git_env: dict[str, str]) -> str: + def _open_metadata( + token: str | None, + _git_env: dict[str, str], + ) -> _RemoteMetadataDocument: """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(raw_url) # noqa: S310 + + def _request_text(url: str, *, rest: bool = False) -> str: + """Read one bounded remote document with attempt credentials.""" + req = urllib.request.Request(url) # noqa: S310 + if rest: + 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) + + def _is_valid_skill(url: str, *, rest: bool = False) -> bool: + """Return whether the URL contains parseable Agent Skills metadata.""" try: - with urllib.request.urlopen(req, timeout=5) as resp: # noqa: S310 - return _read_capped_text(resp) + skill = loads_frontmatter(_request_text(url, rest=rest)) + except urllib.error.HTTPError as exc: + if exc.code == 404: + return False + raise + name = skill.metadata.get("name") + description = skill.metadata.get("description") + return ( + isinstance(name, str) + and bool(name.strip()) + and isinstance(description, str) + and bool(description.strip()) + ) + + def _rest_url(file_path: str) -> str: + """Build a Contents API URL for the resolved package path.""" + api_base = ( + host_info.api_base if host_info else None + ) or f"https://{effective_host}/api/v3" + return f"{api_base}/repos/{pkg.source_repo}/contents/{file_path}?ref={pkg.sha}" + + if effective_host == "github.com": + raw_manifest_url = ( + "https://raw.githubusercontent.com/" + f"{pkg.source_repo}/{pkg.sha}/{manifest_path}" + ) + try: + return _RemoteMetadataDocument(_request_text(raw_manifest_url)) except urllib.error.HTTPError as exc: if exc.code != 404: raise + raw_skill_url = ( + "https://raw.githubusercontent.com/" + f"{pkg.source_repo}/{pkg.sha}/{skill_path}" + ) + if _is_valid_skill(raw_skill_url): + return _RemoteMetadataDocument(None, manifestless=True) if token is None: 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" - url = f"{api_base}/repos/{pkg.source_repo}/contents/{file_path}?ref={pkg.sha}" - req = urllib.request.Request(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) + rest_manifest_url = _rest_url(manifest_path) + try: + return _RemoteMetadataDocument(_request_text(rest_manifest_url, rest=True)) + except urllib.error.HTTPError as exc: + if exc.code != 404: + raise + if _is_valid_skill(_rest_url(skill_path), rest=True): + return _RemoteMetadataDocument(None, manifestless=True) + raise 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, {}) + document = _open_metadata(self._github_token, {}) else: unauth_first = auth_resolver.uses_public_github_anonymous_first(effective_host) - raw = auth_resolver.try_with_fallback( + document = auth_resolver.try_with_fallback( effective_host, _open_metadata, org=org, path=pkg.source_repo, unauth_first=unauth_first, ) - data = load_yaml_str(raw) + if document.manifestless: + return MetadataEnrichmentOutcome(pkg.name, "manifestless") + data = load_yaml_str(document.content or "") if not isinstance(data, dict): return MetadataEnrichmentOutcome(pkg.name, "empty") result: dict[str, str] = {} @@ -1349,7 +1401,8 @@ def _prefetch_metadata( 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 + version is ``empty``; a verified skill without a manifest is + ``manifestless``; an unavailable remote source is ``failed`` or intentionally ``offline`` and cannot certify a clean regeneration. """ if self._metadata_enrichment is not None: diff --git a/tests/unit/marketplace/test_builder.py b/tests/unit/marketplace/test_builder.py index a866cc9b7b..0e8cfb794a 100644 --- a/tests/unit/marketplace/test_builder.py +++ b/tests/unit/marketplace/test_builder.py @@ -2477,6 +2477,75 @@ def test_metadata_fetch_ghes_uses_rest_api(self, tmp_path: Path) -> None: assert parsed.path.startswith("/api/v3/repos/") assert req.get_header("Accept") == "application/vnd.github.raw" + def test_metadata_fetch_ghes_certifies_manifestless_skill(self, tmp_path: Path) -> None: + """GHES verifies SKILL.md after an absent package apm.yml.""" + import urllib.error + + pkg = self._make_pkg(subdir="skills/review") + builder = self._make_builder(tmp_path) + builder._host = "corp.ghe.com" + builder._github_token = "test-token" + builder._host_info = SimpleNamespace( + kind="ghes", + api_base="https://corp.ghe.com/api/v3", + ) + manifest_404 = urllib.error.HTTPError( + url="https://corp.ghe.com/api/v3/repos/acme/tools/contents/skills/review/apm.yml", + code=404, + msg="Not Found", + hdrs=None, + fp=None, # type: ignore[arg-type] + ) + with patch( + "apm_cli.marketplace.builder.urllib.request.urlopen", + side_effect=[ + manifest_404, + _FakeHTTPResponse(b"---\nname: review\ndescription: Review skill\n---\n# Review\n"), + ], + ) as mock_open: + outcome = builder._fetch_remote_metadata_outcome(pkg) + + assert outcome.status == "manifestless" + assert outcome.metadata == {} + skill_req = mock_open.call_args_list[1][0][0] + parsed = urllib.parse.urlparse(skill_req.full_url) + assert parsed.hostname == "corp.ghe.com" + assert parsed.path == "/api/v3/repos/acme/tools/contents/skills/review/SKILL.md" + assert parsed.query == f"ref={_SHA_A}" + assert skill_req.get_header("Accept") == "application/vnd.github.raw" + assert skill_req.get_header("Authorization") == "token test-token" + + def test_metadata_fetch_does_not_certify_directory_named_skill_md( + self, + tmp_path: Path, + ) -> None: + """A Contents API directory response is not a manifestless skill.""" + import urllib.error + + pkg = self._make_pkg(subdir="skills/review") + builder = self._make_builder(tmp_path) + builder._host = "corp.ghe.com" + builder._github_token = "test-token" + builder._host_info = SimpleNamespace( + kind="ghes", + api_base="https://corp.ghe.com/api/v3", + ) + manifest_404 = urllib.error.HTTPError( + url="https://corp.ghe.com/api/v3/repos/acme/tools/contents/skills/review/apm.yml", + code=404, + msg="Not Found", + hdrs=None, + fp=None, # type: ignore[arg-type] + ) + with patch( + "apm_cli.marketplace.builder.urllib.request.urlopen", + side_effect=[manifest_404, _FakeHTTPResponse(b'[{"type":"file","name":"child"}]')], + ): + outcome = builder._fetch_remote_metadata_outcome(pkg) + + assert outcome.status == "failed" + assert outcome.cause == "HTTP 404" + def test_metadata_fetch_github_com_falls_back_to_rest_api_on_raw_404( self, tmp_path: Path, @@ -2499,11 +2568,18 @@ def test_metadata_fetch_github_com_falls_back_to_rest_api_on_raw_404( hdrs=None, fp=None, # type: ignore[arg-type] ) + raw_skill_404 = urllib.error.HTTPError( + url="https://raw.githubusercontent.com/acme/private-tools/sha/plugins/core/SKILL.md", + code=404, + msg="Not Found", + hdrs=None, + fp=None, # type: ignore[arg-type] + ) yaml_body = b"description: Private tool\nversion: 2.1.0\n" mock_resp = _FakeHTTPResponse(yaml_body) with patch( "apm_cli.marketplace.builder.urllib.request.urlopen", - side_effect=[raw_404, mock_resp], + side_effect=[raw_404, raw_skill_404, mock_resp], ) as mock_open: result = builder._fetch_remote_metadata(pkg) assert result == {"description": "Private tool", "version": "2.1.0"} @@ -2513,7 +2589,13 @@ def test_metadata_fetch_github_com_falls_back_to_rest_api_on_raw_404( assert raw_parsed.path == (f"/acme/private-tools/{_SHA_A}/plugins/core/apm.yml") assert raw_req.get_header("Authorization") == "token ghp_test_token" - rest_req = mock_open.call_args_list[1][0][0] + raw_skill_req = mock_open.call_args_list[1][0][0] + raw_skill_parsed = urllib.parse.urlparse(raw_skill_req.full_url) + assert raw_skill_parsed.hostname == "raw.githubusercontent.com" + assert raw_skill_parsed.path == (f"/acme/private-tools/{_SHA_A}/plugins/core/SKILL.md") + assert raw_skill_req.get_header("Authorization") == "token ghp_test_token" + + rest_req = mock_open.call_args_list[2][0][0] rest_parsed = urllib.parse.urlparse(rest_req.full_url) assert rest_parsed.hostname == "api.github.com" assert rest_parsed.path == ("/repos/acme/private-tools/contents/plugins/core/apm.yml") diff --git a/tests/unit/marketplace/test_metadata_enrichment.py b/tests/unit/marketplace/test_metadata_enrichment.py index faf7916129..3150d3a55a 100644 --- a/tests/unit/marketplace/test_metadata_enrichment.py +++ b/tests/unit/marketplace/test_metadata_enrichment.py @@ -3,6 +3,10 @@ from __future__ import annotations import json +import subprocess +import urllib.error +import urllib.parse +from io import BytesIO from pathlib import Path from unittest.mock import MagicMock @@ -51,6 +55,46 @@ def _write_config(project_root: Path) -> None: ) +def _write_manifestless_range_config(project_root: Path) -> None: + """Create the manifestless marketplace fixture from issue #3055.""" + (project_root / "apm.yml").write_text( + """\ +name: generic-marketplace +description: Manifestless metadata reproduction +version: 0.1.0 +marketplace: + owner: + name: public-repro + url: https://github.com/public-repro + build: + tagPattern: "v{version}" + outputs: + claude: {} + packages: + - name: brainstorming + description: Public brainstorming skill + source: obra/superpowers + subdir: skills/brainstorming + version: "^4.0.0" +""", + encoding="utf-8", + ) + + +def _resolved_range_refs( + *_args: object, + **_kwargs: object, +) -> subprocess.CompletedProcess[str]: + """Return one compatible remote tag without invoking Git.""" + resolved_sha = "a" * 40 + return subprocess.CompletedProcess( + args=["git", "ls-remote"], + returncode=0, + stdout=f"{resolved_sha}\trefs/tags/v4.3.1\n", + stderr="", + ) + + def _resolved_package() -> ResolvedPackage: """Return the pinned package used by the test marketplace.""" return ResolvedPackage( @@ -288,6 +332,306 @@ def test_pack_json_warns_and_strict_metadata_prevents_writes( assert not artifact.exists() +def test_pack_check_clean_certifies_manifestless_skill_with_version_range( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A verified remote SKILL.md makes an absent apm.yml certifiable.""" + _write_manifestless_range_config(tmp_path) + resolved_sha = "a" * 40 + + requested_urls: list[tuple[str | None, str, str | None]] = [] + + def fake_urlopen( + request: urllib.request.Request, + *, + timeout: int, + ) -> BytesIO: + """Return the issue fixture's remote package documents.""" + assert timeout == 5 + parsed = urllib.parse.urlparse(request.full_url) + requested_urls.append((parsed.hostname, parsed.path, request.get_header("Authorization"))) + if parsed.path.endswith("/apm.yml"): + raise urllib.error.HTTPError( + url=request.full_url, + code=404, + msg="Not Found", + hdrs=None, + fp=None, + ) + if parsed.path.endswith("/SKILL.md"): + return BytesIO( + b"---\nname: brainstorming\ndescription: Public brainstorming skill\n---\n" + ) + raise AssertionError(f"unexpected metadata URL path: {parsed.path}") + + monkeypatch.setenv("GITHUB_APM_PAT", "test-token") + monkeypatch.setattr("apm_cli.utils.git_env.git_remote_refs", _resolved_range_refs) + monkeypatch.setattr("apm_cli.marketplace.builder.urllib.request.urlopen", fake_urlopen) + monkeypatch.chdir(tmp_path) + runner = CliRunner() + + packed = runner.invoke(pack_cmd, []) + checked = runner.invoke(pack_cmd, ["--check-clean", "--dry-run", "--json"]) + + assert packed.exit_code == 0, packed.output + assert checked.exit_code == 0, checked.output + assert "metadata enrichment failed" not in (packed.output + checked.output) + payload = json.loads(checked.output) + assert payload["metadata_enrichment"] == { + "certifiable": True, + "outcomes": [{"package": "brainstorming", "status": "manifestless"}], + } + expected_base = f"/obra/superpowers/{resolved_sha}/skills/brainstorming" + assert requested_urls == [ + ("raw.githubusercontent.com", f"{expected_base}/apm.yml", None), + ("raw.githubusercontent.com", f"{expected_base}/SKILL.md", None), + ("raw.githubusercontent.com", f"{expected_base}/apm.yml", None), + ("raw.githubusercontent.com", f"{expected_base}/SKILL.md", None), + ] + + +@pytest.mark.parametrize("transport", ["raw", "rest"]) +@pytest.mark.parametrize( + "skill_body", + [ + pytest.param(b"# Skill without frontmatter\n", id="missing-frontmatter"), + pytest.param( + b"---\nname: brainstorming\ndescription: [unterminated\n---\n", + id="malformed-yaml", + ), + pytest.param( + b"---\nname: brainstorming\ndescription: Valid skill\n", + id="unclosed-frontmatter", + ), + pytest.param(b"---\n- brainstorming\n- Valid skill\n---\n", id="sequence-frontmatter"), + pytest.param(b"---\nbrainstorming\n---\n", id="scalar-frontmatter"), + pytest.param(b"---\n---\n", id="empty-frontmatter"), + pytest.param(b"---\ndescription: Valid skill\n---\n", id="missing-name"), + pytest.param(b"---\nname: brainstorming\n---\n", id="missing-description"), + *[ + pytest.param( + f"---\n{field}: {value}\n{other_field}: Valid skill\n---\n".encode(), + id=f"{field}-{case}", + ) + for field, other_field in [ + ("name", "description"), + ("description", "name"), + ] + for case, value in [ + ("null", "null"), + ("number", "42"), + ("boolean", "true"), + ("list", "[brainstorming]"), + ("mapping", "{text: brainstorming}"), + ("empty", '""'), + ("blank", '" \\t "'), + ] + ], + ], +) +def test_pack_check_clean_rejects_invalid_manifestless_skill_metadata( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + transport: str, + skill_body: bytes, +) -> None: + """Invalid remote skill content stays failed through parsing and the clean gate.""" + _write_manifestless_range_config(tmp_path) + requested_skills: list[urllib.parse.ParseResult] = [] + + def fake_urlopen( + request: urllib.request.Request, + *, + timeout: int, + ) -> BytesIO: + """Serve invalid skill content after the manifest's expected 404.""" + assert timeout == 5 + parsed = urllib.parse.urlparse(request.full_url) + if parsed.path.endswith("/SKILL.md"): + requested_skills.append(parsed) + if transport == "raw" or parsed.hostname == "api.github.com": + return BytesIO(skill_body) + elif not parsed.path.endswith("/apm.yml"): + raise AssertionError(f"unexpected metadata URL path: {parsed.path}") + raise urllib.error.HTTPError( + url=request.full_url, + code=404, + msg="Not Found", + hdrs=None, + fp=None, + ) + + monkeypatch.setenv("GITHUB_APM_PAT", "test-token") + monkeypatch.setattr("apm_cli.utils.git_env.git_remote_refs", _resolved_range_refs) + monkeypatch.setattr("apm_cli.marketplace.builder.urllib.request.urlopen", fake_urlopen) + monkeypatch.chdir(tmp_path) + + checked = CliRunner().invoke(pack_cmd, ["--check-clean", "--dry-run", "--json"]) + + assert checked.exit_code == 4, checked.output + payload = json.loads(checked.output) + assert payload["metadata_enrichment"]["certifiable"] is False + assert len(payload["metadata_enrichment"]["outcomes"]) == 1 + outcome = payload["metadata_enrichment"]["outcomes"][0] + assert outcome["package"] == "brainstorming" + assert outcome["status"] == "failed" + assert payload["drift"]["outputs"][0]["status"] == "uncertifiable" + assert payload["errors"][0]["code"] == "marketplace_metadata_uncertifiable" + assert not (tmp_path / ".claude-plugin" / "marketplace.json").exists() + if transport == "raw": + assert any( + parsed.hostname == "raw.githubusercontent.com" + and parsed.path == f"/obra/superpowers/{'a' * 40}/skills/brainstorming/SKILL.md" + for parsed in requested_skills + ) + else: + assert any( + parsed.hostname == "api.github.com" + and parsed.path == "/repos/obra/superpowers/contents/skills/brainstorming/SKILL.md" + and parsed.query == f"ref={'a' * 40}" + for parsed in requested_skills + ) + + +def test_pack_check_clean_rejects_missing_manifestless_skill_path( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A missing apm.yml remains uncertifiable when SKILL.md is also absent.""" + _write_manifestless_range_config(tmp_path) + requested_urls: list[tuple[str | None, str]] = [] + + def fake_urlopen( + request: urllib.request.Request, + *, + timeout: int, + ) -> BytesIO: + """Return 404 for every manifest and skill marker probe.""" + assert timeout == 5 + parsed = urllib.parse.urlparse(request.full_url) + requested_urls.append((parsed.hostname, parsed.path)) + raise urllib.error.HTTPError( + url=request.full_url, + code=404, + msg="Not Found", + hdrs=None, + fp=None, + ) + + monkeypatch.setenv("GITHUB_APM_PAT", "test-token") + monkeypatch.setattr("apm_cli.utils.git_env.git_remote_refs", _resolved_range_refs) + monkeypatch.setattr("apm_cli.marketplace.builder.urllib.request.urlopen", fake_urlopen) + monkeypatch.chdir(tmp_path) + runner = CliRunner() + + packed = runner.invoke(pack_cmd, []) + checked = runner.invoke(pack_cmd, ["--check-clean", "--dry-run", "--json"]) + + assert packed.exit_code == 0, packed.output + assert checked.exit_code == 4, checked.output + payload = json.loads(checked.output) + assert payload["metadata_enrichment"]["certifiable"] is False + assert payload["metadata_enrichment"]["outcomes"] == [ + {"package": "brainstorming", "status": "failed", "cause": "HTTP 404"} + ] + assert {hostname for hostname, _path in requested_urls} == { + "api.github.com", + "raw.githubusercontent.com", + } + assert {Path(path).name for _hostname, path in requested_urls} == { + "SKILL.md", + "apm.yml", + } + + +@pytest.mark.parametrize( + ("metadata_error", "expected_cause"), + [ + (PermissionError("authentication required"), "authentication required"), + (urllib.error.URLError("connection refused"), "network request failed"), + ], +) +def test_pack_check_clean_rejects_metadata_access_failures( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + metadata_error: Exception, + expected_cause: str, +) -> None: + """Authentication and network failures remain uncertifiable.""" + _write_manifestless_range_config(tmp_path) + + def fake_urlopen( + request: urllib.request.Request, + *, + timeout: int, + ) -> BytesIO: + """Raise the selected external metadata-access failure.""" + del request + assert timeout == 5 + raise metadata_error + + monkeypatch.setattr("apm_cli.utils.git_env.git_remote_refs", _resolved_range_refs) + monkeypatch.setattr("apm_cli.marketplace.builder.urllib.request.urlopen", fake_urlopen) + monkeypatch.chdir(tmp_path) + + checked = CliRunner().invoke(pack_cmd, ["--check-clean", "--dry-run", "--json"]) + + assert checked.exit_code == 4, checked.output + payload = json.loads(checked.output) + assert payload["metadata_enrichment"]["outcomes"] == [ + {"package": "brainstorming", "status": "failed", "cause": expected_cause} + ] + + +@pytest.mark.parametrize( + ("remote_result", "expected_error"), + [ + ( + subprocess.CompletedProcess( + args=["git", "ls-remote"], + returncode=128, + stdout="", + stderr="fatal: repository not found", + ), + "repository not found during ls-remote", + ), + ( + subprocess.CompletedProcess( + args=["git", "ls-remote"], + returncode=0, + stdout=("b" * 40) + "\trefs/tags/v5.0.0\n", + stderr="", + ), + "No tag matching version '^4.0.0'", + ), + ], +) +def test_pack_check_clean_rejects_invalid_remote_or_ref( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + remote_result: subprocess.CompletedProcess[str], + expected_error: str, +) -> None: + """Missing repositories and incompatible refs fail before certification.""" + _write_manifestless_range_config(tmp_path) + + def fake_remote_refs( + *_args: object, + **_kwargs: object, + ) -> subprocess.CompletedProcess[str]: + """Return the selected repository or ref resolution failure.""" + return remote_result + + monkeypatch.setattr("apm_cli.utils.git_env.git_remote_refs", fake_remote_refs) + monkeypatch.chdir(tmp_path) + + checked = CliRunner().invoke(pack_cmd, ["--check-clean", "--dry-run"]) + + assert checked.exit_code == 1, checked.output + assert expected_error in checked.output + + def test_check_clean_never_writes_before_reporting_uncertifiable_metadata( tmp_path: Path, monkeypatch,