From 6b92de7704bf7cb7fe872a13a5b95a5d27b4668b Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Tue, 13 Oct 2026 12:16:01 -0500 Subject: [PATCH 1/9] refactor(artifact): extract lookup operation Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 31d7c920-5bbd-4570-a38e-4d0e6a0e9877 --- .../artifacts/_operation_lookup.py | 482 +++++++++ src/specify_cli/artifacts/command_lookup.py | 47 +- .../artifacts/test_command_lookup.py | 296 +++++- .../artifacts/test_operation_lookup.py | 984 ++++++++++++++++++ 4 files changed, 1783 insertions(+), 26 deletions(-) create mode 100644 src/specify_cli/artifacts/_operation_lookup.py create mode 100644 tests/specify_cli/artifacts/test_operation_lookup.py diff --git a/src/specify_cli/artifacts/_operation_lookup.py b/src/specify_cli/artifacts/_operation_lookup.py new file mode 100644 index 0000000000..f070b558a0 --- /dev/null +++ b/src/specify_cli/artifacts/_operation_lookup.py @@ -0,0 +1,482 @@ +"""Shared application operation for ``artifact.lookup``.""" + +from __future__ import annotations + +import math +from dataclasses import dataclass +from pathlib import Path +from typing import Any, Literal, TypeAlias + +from .._file_policy import ProjectPolicyError, project_files +from ..extensions import ExtensionRegistry +from ..integrations import installer +from ..presets import PresetError +from . import ( + ArtifactCatalog, + ArtifactError, + ArtifactResolutionError, + ContributionNotFoundError, + NotASpecKitProjectError, +) +from ._identifiers import IdentifierComponentError, parse_lookup_id + +JsonScalar: TypeAlias = None | bool | int | float | str +JsonProjection: TypeAlias = ( + JsonScalar | list["JsonProjection"] | dict[str, "JsonProjection"] +) + + +@dataclass(frozen=True) +class ArtifactLookupJsonObject: + """Immutable string-keyed JSON object used at the operation boundary.""" + + entries: tuple[tuple[str, JsonValue], ...] + + def to_json_dict(self) -> dict[str, JsonProjection]: + return {key: _project_json_value(value) for key, value in self.entries} + + +JsonValue: TypeAlias = JsonScalar | tuple["JsonValue", ...] | ArtifactLookupJsonObject + + +@dataclass(frozen=True) +class ArtifactLookupRequest: + """Explicit project context and contribution identifier.""" + + project_directory: Path + lookup_id: str + + +@dataclass(frozen=True) +class ArtifactLookupResult: + """One validated preset or extension manifest contribution.""" + + id: str + layer: Literal["preset", "extension"] + sourceId: str + kind: Literal["command", "template", "script", "hook"] + name: str + manifestPath: str + sourcePath: str | None + contribution: ArtifactLookupJsonObject + + def to_json_dict(self) -> dict[str, JsonProjection]: + """Return the exact legacy JSON projection used by the CLI.""" + return { + "id": self.id, + "layer": self.layer, + "sourceId": self.sourceId, + "kind": self.kind, + "name": self.name, + "manifestPath": self.manifestPath, + "sourcePath": self.sourcePath, + "contribution": self.contribution.to_json_dict(), + } + + +class ArtifactLookupError(ArtifactError): + """Expected transport-neutral failure from ``artifact.lookup``.""" + + code: str + details: dict[str, Any] + retryable: bool + + def __init__( + self, + *, + code: str, + message: str, + details: dict[str, Any], + retryable: bool = False, + ) -> None: + self.code = code + self.message = message + self.details = details + self.retryable = retryable + super().__init__(message) + + +class ArtifactLookupProjectDirectoryError(ArtifactLookupError): + """The supplied project directory is not an absolute path.""" + + def __init__(self, project_directory: Path) -> None: + super().__init__( + code="invalid_project_directory", + message="project_directory must be an absolute path", + details={"project_directory": str(project_directory)}, + ) + + +class ArtifactLookupProjectPolicyError(ArtifactLookupError): + """The selected project cannot establish its persisted path policy.""" + + def __init__(self, project_directory: Path, message: str) -> None: + super().__init__( + code="project_policy_error", + message=message, + details={"project_directory": str(project_directory)}, + ) + + +class ArtifactLookupProjectError(ArtifactLookupError): + """The supplied directory is not a Spec Kit project root.""" + + def __init__(self, project_directory: Path) -> None: + super().__init__( + code="not_a_spec_kit_project", + message="not a Spec Kit project: no .specify/ directory found", + details={"project_directory": str(project_directory)}, + ) + + +class ArtifactLookupIdentifierError(ArtifactLookupError): + """The contribution lookup identifier is structurally invalid.""" + + def __init__(self, lookup_id: object) -> None: + super().__init__( + code="invalid_lookup_id", + message=f"unknown contribution {lookup_id}", + details={"lookup_id": lookup_id}, + ) + + +class ArtifactLookupNotFoundError(ArtifactLookupError): + """No contribution matched a valid lookup identifier.""" + + def __init__(self, lookup_id: str) -> None: + super().__init__( + code="unknown_contribution", + message=f"unknown contribution {lookup_id}", + details={"lookup_id": lookup_id}, + ) + + +class ArtifactLookupRegistryError(ArtifactLookupError): + """The extension registry is corrupt or unreadable.""" + + def __init__(self, project_directory: Path, lookup_id: str) -> None: + super().__init__( + code="corrupt_extension_registry", + message="artifact resolution failed", + details={ + "project_directory": str(project_directory), + "lookup_id": lookup_id, + }, + ) + + +class ArtifactLookupIntegrationError(ArtifactLookupError): + """Installed integration loading failed before catalog resolution.""" + + def __init__( + self, + project_directory: Path, + lookup_id: str, + detail: str, + ) -> None: + super().__init__( + code="integration_resolution_failed", + message=f"artifact resolution failed: {detail}", + details={ + "project_directory": str(project_directory), + "lookup_id": lookup_id, + }, + ) + + +class ArtifactLookupPresetError(ArtifactLookupError): + """Preset resolution failed while resolving the contribution.""" + + def __init__(self, project_directory: Path, lookup_id: str) -> None: + super().__init__( + code="preset_resolution_failed", + message="artifact resolution failed", + details={ + "project_directory": str(project_directory), + "lookup_id": lookup_id, + }, + ) + + +class ArtifactLookupFilesystemError(ArtifactLookupError): + """A filesystem operation failed while resolving the contribution.""" + + def __init__(self, project_directory: Path, lookup_id: str) -> None: + super().__init__( + code="filesystem_error", + message="artifact resolution failed", + details={ + "project_directory": str(project_directory), + "lookup_id": lookup_id, + }, + ) + + +class ArtifactLookupResolutionError(ArtifactLookupError): + """Artifact catalog resolution failed.""" + + def __init__( + self, + project_directory: Path, + lookup_id: str, + *, + message: str = "artifact resolution failed", + ) -> None: + super().__init__( + code="artifact_resolution_failed", + message=message, + details={ + "project_directory": str(project_directory), + "lookup_id": lookup_id, + }, + ) + + +class ArtifactLookupInvalidResultError(ArtifactLookupError): + """The catalog returned an incomplete or non-JSON-safe result.""" + + def __init__(self, project_directory: Path, lookup_id: str) -> None: + super().__init__( + code="invalid_operation_result", + message="artifact resolution failed", + details={ + "project_directory": str(project_directory), + "lookup_id": lookup_id, + }, + ) + + +@dataclass(frozen=True) +class ArtifactLookupOperationDescriptor: + """Stable metadata shared by delivery adapters for ``artifact.lookup``.""" + + operation_id: Literal["artifact.lookup"] + contract_version: Literal["1"] + request_type: type[ArtifactLookupRequest] + result_type: type[ArtifactLookupResult] + warning_types: tuple[type[object], ...] + error_types: tuple[type[ArtifactLookupError], ...] + capabilities: frozenset[Literal["local-read"]] + network_access: Literal["none"] + + +ARTIFACT_LOOKUP_OPERATION = ArtifactLookupOperationDescriptor( + operation_id="artifact.lookup", + contract_version="1", + request_type=ArtifactLookupRequest, + result_type=ArtifactLookupResult, + warning_types=(), + error_types=( + ArtifactLookupProjectDirectoryError, + ArtifactLookupProjectPolicyError, + ArtifactLookupProjectError, + ArtifactLookupIdentifierError, + ArtifactLookupNotFoundError, + ArtifactLookupRegistryError, + ArtifactLookupIntegrationError, + ArtifactLookupPresetError, + ArtifactLookupFilesystemError, + ArtifactLookupResolutionError, + ArtifactLookupInvalidResultError, + ), + capabilities=frozenset({"local-read"}), + network_access="none", +) + +_RESULT_KEYS = { + "id", + "layer", + "sourceId", + "kind", + "name", + "manifestPath", + "sourcePath", + "contribution", +} + + +def _invalid_result(request: ArtifactLookupRequest) -> ArtifactLookupInvalidResultError: + return ArtifactLookupInvalidResultError( + Path(request.project_directory), + request.lookup_id, + ) + + +def _validate_json_string(value: object) -> bool: + if type(value) is not str: + return False + try: + value.encode("utf-8") + except UnicodeEncodeError: + return False + return True + + +def _freeze_json_value( + value: object, + request: ArtifactLookupRequest, +) -> JsonValue: + if value is None: + return None + if type(value) is bool: + return value + if type(value) is int: + return value + if type(value) is float: + if not math.isfinite(value): + raise _invalid_result(request) + return value + if type(value) is str: + if not _validate_json_string(value): + raise _invalid_result(request) + return value + if type(value) is list: + try: + return tuple(_freeze_json_value(item, request) for item in value) + except RecursionError as exc: + raise _invalid_result(request) from exc + if type(value) is dict: + entries: list[tuple[str, JsonValue]] = [] + try: + for key, item in value.items(): + if not _validate_json_string(key): + raise _invalid_result(request) + entries.append((key, _freeze_json_value(item, request))) + except RecursionError as exc: + raise _invalid_result(request) from exc + return ArtifactLookupJsonObject(tuple(entries)) + raise _invalid_result(request) + + +def _project_json_value(value: JsonValue) -> JsonProjection: + if isinstance(value, ArtifactLookupJsonObject): + return value.to_json_dict() + if isinstance(value, tuple): + return [_project_json_value(item) for item in value] + return value + + +def _convert_catalog_result( + payload: object, + request: ArtifactLookupRequest, + parsed_lookup_id: tuple[str, str, str, str], +) -> ArtifactLookupResult: + if type(payload) is not dict or set(payload) != _RESULT_KEYS: + raise _invalid_result(request) + + layer, source_id, kind, name = parsed_lookup_id + if ( + layer not in ("preset", "extension") + or kind not in ("command", "template", "script", "hook") + or payload["id"] != request.lookup_id + or payload["layer"] != layer + or payload["sourceId"] != source_id + or payload["kind"] != kind + or payload["name"] != name + or not _validate_json_string(payload["id"]) + or not _validate_json_string(payload["sourceId"]) + or not _validate_json_string(payload["name"]) + or not _validate_json_string(payload["manifestPath"]) + or ( + payload["sourcePath"] is not None + and not _validate_json_string(payload["sourcePath"]) + ) + ): + raise _invalid_result(request) + + contribution = _freeze_json_value(payload["contribution"], request) + if not isinstance(contribution, ArtifactLookupJsonObject): + raise _invalid_result(request) + + return ArtifactLookupResult( + id=payload["id"], + layer=layer, + sourceId=source_id, + kind=kind, + name=name, + manifestPath=payload["manifestPath"], + sourcePath=payload["sourcePath"], + contribution=contribution, + ) + + +def lookup_artifact(request: ArtifactLookupRequest) -> ArtifactLookupResult: + """Resolve one manifest contribution for an explicit Spec Kit project.""" + project_directory = Path(request.project_directory) + if not project_directory.is_absolute(): + raise ArtifactLookupProjectDirectoryError(project_directory) + + try: + if project_directory.is_dir(): + project_files(project_directory) + except ProjectPolicyError as exc: + raise ArtifactLookupProjectPolicyError( + project_directory, + str(exc), + ) from exc + if not (project_directory / ".specify").is_dir(): + raise ArtifactLookupProjectError(project_directory) + + try: + installer.load_installed_integrations(project_directory) + except (installer.IntegrationInstallError, OSError) as exc: + raise ArtifactLookupIntegrationError( + project_directory, + request.lookup_id, + str(exc), + ) from exc + + extensions_directory = project_directory / ".specify" / "extensions" + try: + if ( + extensions_directory.exists() + and ExtensionRegistry(extensions_directory).is_corrupt() + ): + raise ArtifactLookupRegistryError( + project_directory, + request.lookup_id, + ) + except ArtifactLookupRegistryError: + raise + except OSError as exc: + raise ArtifactLookupFilesystemError( + project_directory, + request.lookup_id, + ) from exc + + if not _validate_json_string(request.lookup_id): + raise ArtifactLookupIdentifierError(request.lookup_id) + try: + parsed_lookup_id = parse_lookup_id(request.lookup_id) + except IdentifierComponentError as exc: + raise ArtifactLookupIdentifierError(request.lookup_id) from exc + + if parsed_lookup_id[0] == "project": + raise ArtifactLookupNotFoundError(request.lookup_id) + + try: + payload = ArtifactCatalog(project_directory).get_contribution_info( + request.lookup_id + ) + except NotASpecKitProjectError as exc: + raise ArtifactLookupProjectError(project_directory) from exc + except ContributionNotFoundError as exc: + raise ArtifactLookupNotFoundError(request.lookup_id) from exc + except ArtifactResolutionError as exc: + raise ArtifactLookupResolutionError( + project_directory, + request.lookup_id, + message=exc.message, + ) from exc + except PresetError as exc: + raise ArtifactLookupPresetError( + project_directory, + request.lookup_id, + ) from exc + except OSError as exc: + raise ArtifactLookupFilesystemError( + project_directory, + request.lookup_id, + ) from exc + + return _convert_catalog_result(payload, request, parsed_lookup_id) diff --git a/src/specify_cli/artifacts/command_lookup.py b/src/specify_cli/artifacts/command_lookup.py index e3f34411c2..3fc55f5769 100644 --- a/src/specify_cli/artifacts/command_lookup.py +++ b/src/specify_cli/artifacts/command_lookup.py @@ -3,23 +3,44 @@ from __future__ import annotations import json +import os import sys +from pathlib import Path import typer -from ..presets import PresetError -from . import ArtifactCatalog, ArtifactError, ArtifactResolutionError +from . import ArtifactError from ._commands import ( _emit_error_and_exit, _require_json_flag, - _resolve_project_root, artifact_app, ) +from ._operation_lookup import ( + ArtifactLookupRequest, + ArtifactLookupResolutionError, + lookup_artifact, +) + + +def _project_directory_from_cli_context() -> Path: + """Return the explicit project directory selected by the CLI invocation.""" + invocation_directory = Path.cwd() + override = os.environ.get("SPECIFY_INIT_DIR", "") + if not override: + return invocation_directory + return (invocation_directory / override).resolve() + + +def _resolve_project_root() -> Path: + """Preserve the command module's established project-root patch seam.""" + return _project_directory_from_cli_context() @artifact_app.command("lookup") def artifact_lookup( - lookup_id: str = typer.Argument(..., help="Contribution lookupId from an artifact stack."), + lookup_id: str = typer.Argument( + ..., help="Contribution lookupId from an artifact stack." + ), json_flag: bool = typer.Option( False, "--json", @@ -29,26 +50,30 @@ def artifact_lookup( """Resolve a stack lookupId to its effective preset or extension contribution.""" _require_json_flag(json_flag) try: - root = _resolve_project_root() - payload = ArtifactCatalog(root).get_contribution_info(lookup_id) + result = lookup_artifact( + ArtifactLookupRequest( + project_directory=_resolve_project_root(), + lookup_id=lookup_id, + ) + ) except ArtifactError as exc: _emit_error_and_exit(exc) return # pragma: no cover - except (OSError, PresetError): - _emit_error_and_exit(ArtifactResolutionError()) + except OSError: + _emit_error_and_exit(ArtifactLookupResolutionError(Path("."), lookup_id)) return # pragma: no cover try: rendered = json.dumps( - payload, + result.to_json_dict(), indent=2, sort_keys=True, ensure_ascii=False, allow_nan=False, ) rendered.encode("utf-8") - except (TypeError, ValueError, UnicodeEncodeError): - _emit_error_and_exit(ArtifactResolutionError()) + except (TypeError, ValueError, UnicodeEncodeError, RecursionError): + _emit_error_and_exit(ArtifactLookupResolutionError(Path("."), lookup_id)) return # pragma: no cover sys.stdout.write(rendered) diff --git a/tests/specify_cli/artifacts/test_command_lookup.py b/tests/specify_cli/artifacts/test_command_lookup.py index cde4b94ad5..6f85e49179 100644 --- a/tests/specify_cli/artifacts/test_command_lookup.py +++ b/tests/specify_cli/artifacts/test_command_lookup.py @@ -11,7 +11,16 @@ from typer.testing import CliRunner from specify_cli import app -from specify_cli.artifacts import ArtifactCatalog, command_lookup +from specify_cli.artifacts import ( + ArtifactCatalog, + _operation_lookup, + command_lookup, +) +from specify_cli.artifacts._operation_lookup import ( + ArtifactLookupJsonObject, + ArtifactLookupNotFoundError, + ArtifactLookupResult, +) from specify_cli.integrations import installer from tests.conftest import install_preset @@ -54,9 +63,7 @@ def test_lookup_json_cross_references_manifest_contribution( lookup_id = ArtifactCatalog(spec_kit_project).get_artifact_info( "template:lookup-template" )["stack"][0]["lookupId"] - result = CliRunner().invoke( - app, ["artifact", "lookup", lookup_id, "--json"] - ) + result = CliRunner().invoke(app, ["artifact", "lookup", lookup_id, "--json"]) assert result.exit_code == 0, result.stderr payload = json.loads(result.stdout) @@ -101,11 +108,16 @@ def test_lookup_policy_failure_precedes_loader_and_catalog( loader = Mock(side_effect=AssertionError("installed loader reached")) catalog = Mock(side_effect=AssertionError("artifact catalog reached")) monkeypatch.setattr(installer, "load_installed_integrations", loader) - monkeypatch.setattr(command_lookup, "ArtifactCatalog", catalog) + monkeypatch.setattr(_operation_lookup, "ArtifactCatalog", catalog) result = CliRunner().invoke( app, - ["artifact", "lookup", "preset:lookup-pack:template:lookup-template", "--json"], + [ + "artifact", + "lookup", + "preset:lookup-pack:template:lookup-template", + "--json", + ], ) assert result.exit_code == 1 @@ -123,9 +135,7 @@ def test_lookup_json_rejects_unknown_contribution( monkeypatch.chdir(spec_kit_project) lookup_id = "extension:missing:command:speckit.missing.command" - result = CliRunner().invoke( - app, ["artifact", "lookup", lookup_id, "--json"] - ) + result = CliRunner().invoke(app, ["artifact", "lookup", lookup_id, "--json"]) assert result.exit_code == 1 assert result.stdout == "" @@ -133,6 +143,244 @@ def test_lookup_json_rejects_unknown_contribution( "error": f"unknown contribution {lookup_id}" } + def test_lookup_dispatches_directly_to_shared_operation( + self, + spec_kit_project: Path, + monkeypatch: pytest.MonkeyPatch, + ): + expected = ArtifactLookupResult( + id="extension:quality:template:réview", + layer="extension", + sourceId="quality", + kind="template", + name="réview", + manifestPath=".specify/extensions/quality/extension.yml", + sourcePath=".specify/extensions/quality/templates/réview.md", + contribution=ArtifactLookupJsonObject( + ( + ("name", "réview"), + ("description", "Réview ✓"), + ) + ), + ) + operation = Mock(return_value=expected) + monkeypatch.setattr(command_lookup, "lookup_artifact", operation) + monkeypatch.chdir(spec_kit_project) + + result = CliRunner().invoke( + app, + [ + "artifact", + "lookup", + "extension:quality:template:réview", + "--json", + ], + ) + + assert result.exit_code == 0, result.stderr + assert json.loads(result.stdout) == expected.to_json_dict() + request = operation.call_args.args[0] + assert request.project_directory == spec_kit_project + assert request.lookup_id == "extension:quality:template:réview" + assert "ArtifactCatalog" not in command_lookup.__dict__ + + def test_lookup_json_matches_shared_operation_result( + self, + spec_kit_project: Path, + monkeypatch: pytest.MonkeyPatch, + ): + pack = install_preset( + spec_kit_project, + "parity", + { + "templates": [ + { + "type": "template", + "name": "parity", + "file": "templates/parity.md", + "description": "Parity ✓", + } + ] + }, + ) + (pack / "templates").mkdir() + (pack / "templates" / "parity.md").write_text( + "body", + encoding="utf-8", + ) + lookup_id = "preset:parity:template:parity" + expected = command_lookup.lookup_artifact( + command_lookup.ArtifactLookupRequest( + spec_kit_project, + lookup_id, + ) + ) + monkeypatch.chdir(spec_kit_project) + + result = CliRunner().invoke( + app, + ["artifact", "lookup", lookup_id, "--json"], + ) + + assert result.exit_code == 0, result.stderr + assert json.loads(result.stdout) == expected.to_json_dict() + + def test_lookup_success_stream_is_exact_legacy_json( + self, + spec_kit_project: Path, + monkeypatch: pytest.MonkeyPatch, + ): + expected = ArtifactLookupResult( + id="preset:quality:template:réview", + layer="preset", + sourceId="quality", + kind="template", + name="réview", + manifestPath=".specify/presets/quality/preset.yml", + sourcePath=None, + contribution=ArtifactLookupJsonObject( + ( + ("zeta", (1, 2)), + ("alpha", "Réview ✓"), + ) + ), + ) + monkeypatch.setattr( + command_lookup, + "lookup_artifact", + Mock(return_value=expected), + ) + monkeypatch.chdir(spec_kit_project) + + result = CliRunner().invoke( + app, + ["artifact", "lookup", expected.id, "--json"], + ) + + assert result.exit_code == 0 + payload = expected.to_json_dict() + assert result.stdout == ( + json.dumps( + payload, + indent=2, + sort_keys=True, + ensure_ascii=False, + allow_nan=False, + ) + + "\n" + ) + result.stdout.encode("utf-8") + assert json.loads(result.stdout) == payload + assert result.stderr == "" + assert "\x1b[" not in result.stdout + + def test_lookup_expected_operation_failure_is_one_legacy_json_error( + self, + spec_kit_project: Path, + monkeypatch: pytest.MonkeyPatch, + ): + lookup_id = "extension:missing:command:speckit.missing" + monkeypatch.setattr( + command_lookup, + "lookup_artifact", + Mock(side_effect=ArtifactLookupNotFoundError(lookup_id)), + ) + monkeypatch.chdir(spec_kit_project) + + result = CliRunner().invoke( + app, + ["artifact", "lookup", lookup_id, "--json"], + ) + + assert result.exit_code == 1 + assert result.stdout == "" + assert result.stderr == ( + '{"error": "unknown contribution ' + 'extension:missing:command:speckit.missing"}\n' + ) + assert result.stderr.count("\n") == 1 + assert "\x1b[" not in result.stderr + assert "Traceback" not in result.stderr + + def test_lookup_cli_context_filesystem_failure_is_json_error( + self, + monkeypatch: pytest.MonkeyPatch, + ): + monkeypatch.setattr( + command_lookup, + "_project_directory_from_cli_context", + Mock(side_effect=OSError("unreadable cwd")), + ) + + result = CliRunner().invoke( + app, + [ + "artifact", + "lookup", + "preset:sample:template:sample", + "--json", + ], + ) + + assert result.exit_code == 1 + assert result.stdout == "" + assert json.loads(result.stderr) == {"error": "artifact resolution failed"} + + def test_lookup_cli_context_resolves_init_dir_override( + self, + spec_kit_project: Path, + non_project: Path, + monkeypatch: pytest.MonkeyPatch, + ): + operation = Mock( + side_effect=ArtifactLookupNotFoundError("preset:missing:template:missing") + ) + monkeypatch.setattr(command_lookup, "lookup_artifact", operation) + monkeypatch.chdir(non_project) + monkeypatch.setenv("SPECIFY_INIT_DIR", str(spec_kit_project)) + + result = CliRunner().invoke( + app, + [ + "artifact", + "lookup", + "preset:missing:template:missing", + "--json", + ], + ) + + assert result.exit_code == 1 + request = operation.call_args.args[0] + assert request.project_directory == spec_kit_project.resolve() + + def test_lookup_rendering_failure_is_json_error( + self, + spec_kit_project: Path, + monkeypatch: pytest.MonkeyPatch, + ): + result_object = Mock() + result_object.to_json_dict.return_value = {"value": date(2026, 1, 1)} + monkeypatch.setattr( + command_lookup, + "lookup_artifact", + Mock(return_value=result_object), + ) + monkeypatch.chdir(spec_kit_project) + + result = CliRunner().invoke( + app, + [ + "artifact", + "lookup", + "preset:sample:template:sample", + "--json", + ], + ) + + assert result.exit_code == 1 + assert result.stdout == "" + assert json.loads(result.stderr) == {"error": "artifact resolution failed"} + @pytest.mark.parametrize( "manifest_value", [ @@ -183,9 +431,7 @@ def test_lookup_json_rejects_non_json_manifest_value( assert result.exit_code == 1 assert result.stdout == "" - assert json.loads(result.stderr) == { - "error": "artifact resolution failed" - } + assert json.loads(result.stderr) == {"error": "artifact resolution failed"} @pytest.mark.parametrize( "lookup_id", @@ -204,9 +450,7 @@ def test_lookup_json_rejects_malformed_lookup_id( ): monkeypatch.chdir(spec_kit_project) - result = CliRunner().invoke( - app, ["artifact", "lookup", lookup_id, "--json"] - ) + result = CliRunner().invoke(app, ["artifact", "lookup", lookup_id, "--json"]) assert result.exit_code == 1 assert result.stdout == "" @@ -244,3 +488,25 @@ def test_lookup_validates_project_before_lookup_id( assert json.loads(result.stderr) == { "error": "not a Spec Kit project: no .specify/ directory found" } + + def test_lookup_validates_project_before_unknown_lookup_id( + self, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + ): + monkeypatch.chdir(tmp_path) + result = CliRunner().invoke( + app, + [ + "artifact", + "lookup", + "extension:missing:command:speckit.missing", + "--json", + ], + ) + + assert result.exit_code == 1 + assert result.stdout == "" + assert json.loads(result.stderr) == { + "error": "not a Spec Kit project: no .specify/ directory found" + } diff --git a/tests/specify_cli/artifacts/test_operation_lookup.py b/tests/specify_cli/artifacts/test_operation_lookup.py new file mode 100644 index 0000000000..95c31a6cd4 --- /dev/null +++ b/tests/specify_cli/artifacts/test_operation_lookup.py @@ -0,0 +1,984 @@ +"""Tests for the shared ``artifact.lookup`` operation.""" + +from __future__ import annotations + +from datetime import date +from pathlib import Path +from unittest.mock import Mock + +import pytest +import yaml + +from specify_cli import artifacts +from specify_cli.artifacts import ( + ArtifactCatalog, + ArtifactResolutionError, + _operation_lookup, +) +from specify_cli.artifacts._identifiers import derive_hook_lookup_id +from specify_cli.artifacts._operation_lookup import ( + ARTIFACT_LOOKUP_OPERATION, + ArtifactLookupFilesystemError, + ArtifactLookupIdentifierError, + ArtifactLookupIntegrationError, + ArtifactLookupInvalidResultError, + ArtifactLookupJsonObject, + ArtifactLookupNotFoundError, + ArtifactLookupPresetError, + ArtifactLookupProjectDirectoryError, + ArtifactLookupProjectError, + ArtifactLookupProjectPolicyError, + ArtifactLookupRegistryError, + ArtifactLookupRequest, + ArtifactLookupResolutionError, + ArtifactLookupResult, + lookup_artifact, +) +from specify_cli.extensions import ExtensionRegistry +from specify_cli.integrations.installer import IntegrationInstallError +from specify_cli.presets import PresetError +from tests.conftest import install_preset +from tests.specify_cli.artifacts.helpers import install_extension_with_hooks + + +def _install_extension_contributions(project_root: Path) -> Path: + extension_dir = project_root / ".specify" / "extensions" / "quality" + extension_dir.mkdir() + for directory in ("commands", "templates", "scripts"): + (extension_dir / directory).mkdir() + (extension_dir / "commands" / "review.md").write_text( + "command body", + encoding="utf-8", + ) + (extension_dir / "templates" / "review.md").write_text( + "template body", + encoding="utf-8", + ) + (extension_dir / "scripts" / "review.sh").write_text( + "#!/bin/sh\n", + encoding="utf-8", + ) + (extension_dir / "extension.yml").write_text( + yaml.safe_dump( + { + "schema_version": "1.0", + "extension": { + "id": "quality", + "name": "Quality", + "version": "1.0.0", + "description": "Quality extension", + "author": "test", + "repository": "https://example.com", + "license": "MIT", + }, + "requires": {"speckit_version": ">=0.2.0"}, + "provides": { + "commands": [ + { + "name": "speckit.quality.review", + "file": "commands/review.md", + "description": "Review command", + } + ], + "templates": [ + { + "name": "quality-review", + "file": "templates/review.md", + "description": "Review template", + } + ], + "scripts": [ + { + "name": "quality-review", + "file": "scripts/review.sh", + "description": "Review script", + } + ], + }, + } + ), + encoding="utf-8", + ) + ExtensionRegistry(project_root / ".specify" / "extensions").add( + "quality", + {"version": "1.0.0", "enabled": True}, + ) + return extension_dir + + +def _valid_payload( + lookup_id: str = "preset:sample:template:sample", +) -> dict[str, object]: + layer, source_id, kind, name = lookup_id.split(":", 3) + return { + "id": lookup_id, + "layer": layer, + "sourceId": source_id, + "kind": kind, + "name": name, + "manifestPath": f".specify/{layer}s/{source_id}/{layer}.yml", + "sourcePath": None, + "contribution": {"name": name, "type": kind}, + } + + +def test_artifact_lookup_operation_descriptor_is_stable(): + assert ARTIFACT_LOOKUP_OPERATION.operation_id == "artifact.lookup" + assert ARTIFACT_LOOKUP_OPERATION.contract_version == "1" + assert ARTIFACT_LOOKUP_OPERATION.request_type is ArtifactLookupRequest + assert ARTIFACT_LOOKUP_OPERATION.result_type is ArtifactLookupResult + assert ARTIFACT_LOOKUP_OPERATION.warning_types == () + assert ARTIFACT_LOOKUP_OPERATION.error_types == ( + ArtifactLookupProjectDirectoryError, + ArtifactLookupProjectPolicyError, + ArtifactLookupProjectError, + ArtifactLookupIdentifierError, + ArtifactLookupNotFoundError, + ArtifactLookupRegistryError, + ArtifactLookupIntegrationError, + ArtifactLookupPresetError, + ArtifactLookupFilesystemError, + ArtifactLookupResolutionError, + ArtifactLookupInvalidResultError, + ) + assert ARTIFACT_LOOKUP_OPERATION.capabilities == frozenset({"local-read"}) + assert ARTIFACT_LOOKUP_OPERATION.network_access == "none" + + +def test_lookup_returns_complete_typed_preset_result( + spec_kit_project: Path, +): + pack = install_preset( + spec_kit_project, + "typed", + { + "templates": [ + { + "type": "template", + "name": "typed", + "file": "templates/typed.md", + "description": "Typed ✓", + "metadata": { + "none": None, + "boolean": True, + "integer": 7, + "float": 2.5, + "unicode": {"clé": "välue ✓"}, + "array": [None, False, 9, 1.25, "text"], + }, + } + ] + }, + ) + (pack / "templates").mkdir() + (pack / "templates" / "typed.md").write_text("body", encoding="utf-8") + request = ArtifactLookupRequest( + spec_kit_project, + "preset:typed:template:typed", + ) + + result = lookup_artifact(request) + payload = result.to_json_dict() + + assert isinstance(result, ArtifactLookupResult) + assert isinstance(result.contribution, ArtifactLookupJsonObject) + assert payload == ArtifactCatalog(spec_kit_project).get_contribution_info( + request.lookup_id + ) + assert payload == { + "id": request.lookup_id, + "layer": "preset", + "sourceId": "typed", + "kind": "template", + "name": "typed", + "manifestPath": ".specify/presets/typed/preset.yml", + "sourcePath": ".specify/presets/typed/templates/typed.md", + "contribution": { + "type": "template", + "name": "typed", + "file": "templates/typed.md", + "description": "Typed ✓", + "metadata": { + "none": None, + "boolean": True, + "integer": 7, + "float": 2.5, + "unicode": {"clé": "välue ✓"}, + "array": [None, False, 9, 1.25, "text"], + }, + }, + } + + +@pytest.mark.parametrize( + ("kind", "name", "lookup_id"), + [ + ( + "command", + "speckit.quality.review", + "extension:quality:command:speckit.quality.review", + ), + ( + "template", + "quality-review", + "extension:quality:template:quality-review", + ), + ( + "script", + "quality-review", + "extension:quality:script:quality-review", + ), + ], +) +def test_lookup_accepts_extension_contribution_families( + spec_kit_project: Path, + kind: str, + name: str, + lookup_id: str, +): + _install_extension_contributions(spec_kit_project) + + result = lookup_artifact(ArtifactLookupRequest(spec_kit_project, lookup_id)) + payload = result.to_json_dict() + + assert payload["layer"] == "extension" + assert payload["kind"] == kind + assert payload["name"] == name + assert payload["contribution"]["name"] == name + assert payload["sourcePath"] == ( + f".specify/extensions/quality/{kind}s/review." + f"{'sh' if kind == 'script' else 'md'}" + ) + + +@pytest.mark.parametrize( + ("kind", "name"), + [ + ("command", "speckit.preset.command"), + ("template", "preset-template"), + ("script", "preset-script"), + ], +) +def test_lookup_accepts_preset_contribution_families( + spec_kit_project: Path, + kind: str, + name: str, +): + pack = install_preset( + spec_kit_project, + "families", + { + "templates": [ + { + "type": kind, + "name": name, + "description": f"{kind} contribution", + } + ] + }, + ) + relative_file = { + "command": Path("commands") / f"{name}.md", + "template": Path("templates") / f"{name}.md", + "script": Path("scripts") / f"{name}.sh", + }[kind] + source = pack / relative_file + source.parent.mkdir() + source.write_text("body", encoding="utf-8") + lookup_id = f"preset:families:{kind}:{name}" + + result = lookup_artifact(ArtifactLookupRequest(spec_kit_project, lookup_id)) + + assert result.kind == kind + assert result.name == name + assert result.sourcePath == ( + f".specify/presets/families/{relative_file.as_posix()}" + ) + + +def test_lookup_preserves_percent_encoded_unicode_hook_identity( + spec_kit_project: Path, +): + event_name = "custom:after/✓" + command = "/skill:speckit-réview:run" + install_extension_with_hooks( + spec_kit_project, + "quality", + hooks={ + event_name: [ + { + "command": command, + "description": "Réview ✓", + "priority": 4, + "optional": False, + } + ] + }, + ) + lookup_id = derive_hook_lookup_id( + "extension", + "quality", + event_name, + command, + ) + + result = lookup_artifact(ArtifactLookupRequest(spec_kit_project, lookup_id)) + payload = result.to_json_dict() + + assert result.id == lookup_id + assert result.kind == "hook" + assert result.name == lookup_id.split(":", 3)[3] + assert payload["contribution"] == { + "eventName": event_name, + "command": command, + "description": "Réview ✓", + "priority": 4, + "optional": False, + } + assert result.sourcePath is None + + +def test_list_info_lookup_round_trip_matches_manifest_and_stack_layer( + spec_kit_project: Path, +): + pack = install_preset( + spec_kit_project, + "round-trip", + { + "templates": [ + { + "type": "template", + "name": "round-trip", + "file": "templates/round-trip.md", + "description": "Round trip", + } + ] + }, + ) + (pack / "templates").mkdir() + (pack / "templates" / "round-trip.md").write_text( + "body", + encoding="utf-8", + ) + catalog = ArtifactCatalog(spec_kit_project) + row = next( + item + for item in catalog.list_artifacts_with_stack() + if item["id"] == "template:round-trip" + ) + info = catalog.get_artifact_info(row["id"]) + layer = next(item for item in info["stack"] if item["sourceId"] == "round-trip") + + result = lookup_artifact( + ArtifactLookupRequest(spec_kit_project, layer["lookupId"]) + ).to_json_dict() + + assert info == row + assert result == catalog.get_contribution_info(layer["lookupId"]) + assert result["manifestPath"] == layer["manifestPath"] + assert result["sourcePath"] == layer["sourcePath"] + manifest = yaml.safe_load( + (spec_kit_project / result["manifestPath"]).read_text(encoding="utf-8") + ) + assert result["contribution"] == manifest["provides"]["templates"][0] + + +def test_lookup_is_stable_across_repeated_calls(spec_kit_project: Path): + install_preset( + spec_kit_project, + "stable", + {"templates": [{"type": "template", "name": "stable"}]}, + ) + request = ArtifactLookupRequest( + spec_kit_project, + "preset:stable:template:stable", + ) + + assert lookup_artifact(request) == lookup_artifact(request) + + +def test_lookup_isolates_sequential_explicit_projects( + spec_kit_project: Path, + tmp_path: Path, + non_project: Path, + monkeypatch: pytest.MonkeyPatch, +): + second = tmp_path / "second" + second.mkdir() + for relative in ( + ".specify", + ".specify/presets", + ".specify/extensions", + ".specify/templates", + ): + (second / relative).mkdir(exist_ok=True) + for project, description in ( + (spec_kit_project, "First"), + (second, "Second"), + ): + install_preset( + project, + "isolated", + { + "templates": [ + { + "type": "template", + "name": "isolated", + "description": description, + } + ] + }, + ) + request_id = "preset:isolated:template:isolated" + monkeypatch.chdir(non_project) + + first = lookup_artifact(ArtifactLookupRequest(spec_kit_project, request_id)) + second_result = lookup_artifact(ArtifactLookupRequest(second, request_id)) + + assert first.to_json_dict()["contribution"]["description"] == "First" + assert second_result.to_json_dict()["contribution"]["description"] == "Second" + assert Path.cwd() == non_project + + +def test_lookup_preserves_trusted_symlinked_project_root( + spec_kit_project: Path, + tmp_path: Path, +): + pack = install_preset( + spec_kit_project, + "linked", + { + "templates": [ + { + "type": "template", + "name": "linked", + "file": "templates/linked.md", + } + ] + }, + ) + (pack / "templates").mkdir() + (pack / "templates" / "linked.md").write_text("body", encoding="utf-8") + linked_project = tmp_path / "linked-project" + try: + linked_project.symlink_to(spec_kit_project, target_is_directory=True) + except (OSError, NotImplementedError) as exc: + pytest.skip(f"Symlinks unavailable: {exc}") + + result = lookup_artifact( + ArtifactLookupRequest( + linked_project, + "preset:linked:template:linked", + ) + ) + + assert result.sourcePath == ".specify/presets/linked/templates/linked.md" + + +def test_lookup_rejects_relative_project_directory(): + with pytest.raises(ArtifactLookupProjectDirectoryError) as exc_info: + lookup_artifact( + ArtifactLookupRequest( + Path("relative"), + "preset:sample:template:sample", + ) + ) + + assert exc_info.value.code == "invalid_project_directory" + assert exc_info.value.details == {"project_directory": "relative"} + + +def test_lookup_rejects_missing_absolute_project_directory( + tmp_path: Path, +): + missing = tmp_path / "missing" + + with pytest.raises(ArtifactLookupProjectError): + lookup_artifact( + ArtifactLookupRequest( + missing, + "preset:sample:template:sample", + ) + ) + + +@pytest.mark.parametrize( + "lookup_id", + [ + "invalid:source:command:name", + "extension:source:command:missing", + ], +) +def test_lookup_rejects_non_project_before_lookup_identifier( + non_project: Path, + monkeypatch: pytest.MonkeyPatch, + lookup_id: str, +): + catalog = Mock(side_effect=AssertionError("catalog reached")) + monkeypatch.setattr(_operation_lookup, "ArtifactCatalog", catalog) + + with pytest.raises(ArtifactLookupProjectError) as exc_info: + lookup_artifact(ArtifactLookupRequest(non_project, lookup_id)) + + assert exc_info.value.code == "not_a_spec_kit_project" + catalog.assert_not_called() + + +@pytest.mark.parametrize( + "lookup_id", + [ + "invalid:source:command:name", + "extension:source:invalid:name", + "extension:_:command:name", + "extension:source:hook:event", + "extension:source:hook:event:command:extra", + "extension:source:hook:event:bad%escape", + "extension:source:hook:event:%FF", + "extension:source:hook:event:%", + ], +) +def test_lookup_maps_malformed_lookup_ids( + spec_kit_project: Path, + lookup_id: str, +): + with pytest.raises(ArtifactLookupIdentifierError) as exc_info: + lookup_artifact(ArtifactLookupRequest(spec_kit_project, lookup_id)) + + assert exc_info.value.code == "invalid_lookup_id" + assert exc_info.value.message == f"unknown contribution {lookup_id}" + + +def test_lookup_maps_invalid_unicode_lookup_id_after_project_validation( + spec_kit_project: Path, +): + lookup_id = "extension:source:template:\ud800" + + with pytest.raises(ArtifactLookupIdentifierError): + lookup_artifact(ArtifactLookupRequest(spec_kit_project, lookup_id)) + + +@pytest.mark.parametrize( + "lookup_id", + [ + "preset:missing:template:missing", + "extension:missing:command:speckit.missing", + "project:_:command:local", + ], +) +def test_lookup_maps_unknown_contributions( + spec_kit_project: Path, + lookup_id: str, +): + with pytest.raises(ArtifactLookupNotFoundError) as exc_info: + lookup_artifact(ArtifactLookupRequest(spec_kit_project, lookup_id)) + + assert exc_info.value.code == "unknown_contribution" + assert exc_info.value.message == f"unknown contribution {lookup_id}" + + +def test_lookup_maps_project_policy_failure_before_loader_and_catalog( + spec_kit_project: Path, + monkeypatch: pytest.MonkeyPatch, +): + config = spec_kit_project / ".specify" / "init-options.json" + config.write_text("{", encoding="utf-8") + loader = Mock(side_effect=AssertionError("loader reached")) + catalog = Mock(side_effect=AssertionError("catalog reached")) + monkeypatch.setattr( + _operation_lookup.installer, + "load_installed_integrations", + loader, + ) + monkeypatch.setattr(_operation_lookup, "ArtifactCatalog", catalog) + + with pytest.raises(ArtifactLookupProjectPolicyError) as exc_info: + lookup_artifact( + ArtifactLookupRequest( + spec_kit_project, + "preset:sample:template:sample", + ) + ) + + assert exc_info.value.code == "project_policy_error" + assert "Invalid JSON" in exc_info.value.message + loader.assert_not_called() + catalog.assert_not_called() + + +def test_lookup_maps_corrupt_registry_before_malformed_identifier( + spec_kit_project: Path, + monkeypatch: pytest.MonkeyPatch, +): + registry = spec_kit_project / ".specify" / "extensions" / ".registry" + registry.write_text("{invalid", encoding="utf-8") + catalog = Mock(side_effect=AssertionError("catalog reached")) + monkeypatch.setattr(_operation_lookup, "ArtifactCatalog", catalog) + + with pytest.raises(ArtifactLookupRegistryError) as exc_info: + lookup_artifact( + ArtifactLookupRequest( + spec_kit_project, + "invalid:source:command:name", + ) + ) + + assert exc_info.value.code == "corrupt_extension_registry" + catalog.assert_not_called() + + +def test_lookup_maps_registry_filesystem_failure( + spec_kit_project: Path, + monkeypatch: pytest.MonkeyPatch, +): + monkeypatch.setattr( + _operation_lookup, + "ExtensionRegistry", + Mock(side_effect=OSError("unreadable registry")), + ) + + with pytest.raises(ArtifactLookupFilesystemError) as exc_info: + lookup_artifact( + ArtifactLookupRequest( + spec_kit_project, + "preset:sample:template:sample", + ) + ) + + assert exc_info.value.code == "filesystem_error" + + +def test_lookup_maps_integration_loader_failure( + spec_kit_project: Path, + monkeypatch: pytest.MonkeyPatch, +): + failure = IntegrationInstallError("installed package has been modified") + monkeypatch.setattr( + _operation_lookup.installer, + "load_installed_integrations", + Mock(side_effect=failure), + ) + + with pytest.raises(ArtifactLookupIntegrationError) as exc_info: + lookup_artifact( + ArtifactLookupRequest( + spec_kit_project, + "preset:sample:template:sample", + ) + ) + + assert exc_info.value.code == "integration_resolution_failed" + assert exc_info.value.message == ( + "artifact resolution failed: installed package has been modified" + ) + assert exc_info.value.__cause__ is failure + + +@pytest.mark.parametrize( + ("failure", "error_type", "code"), + [ + ( + ArtifactResolutionError("catalog detail"), + ArtifactLookupResolutionError, + "artifact_resolution_failed", + ), + ( + PresetError("broken preset"), + ArtifactLookupPresetError, + "preset_resolution_failed", + ), + ( + OSError("unreadable"), + ArtifactLookupFilesystemError, + "filesystem_error", + ), + ], +) +def test_lookup_converts_catalog_resolution_failures( + spec_kit_project: Path, + monkeypatch: pytest.MonkeyPatch, + failure: Exception, + error_type: type[Exception], + code: str, +): + monkeypatch.setattr( + artifacts.ArtifactCatalog, + "get_contribution_info", + Mock(side_effect=failure), + ) + + with pytest.raises(error_type) as exc_info: + lookup_artifact( + ArtifactLookupRequest( + spec_kit_project, + "preset:sample:template:sample", + ) + ) + + assert exc_info.value.code == code + assert exc_info.value.__cause__ is failure + if isinstance(failure, ArtifactResolutionError): + assert exc_info.value.message == failure.message + + +def test_lookup_converts_catalog_project_error( + spec_kit_project: Path, + monkeypatch: pytest.MonkeyPatch, +): + catalog = Mock() + catalog.get_contribution_info.side_effect = artifacts.NotASpecKitProjectError() + monkeypatch.setattr( + _operation_lookup, + "ArtifactCatalog", + Mock(return_value=catalog), + ) + + with pytest.raises(ArtifactLookupProjectError): + lookup_artifact( + ArtifactLookupRequest( + spec_kit_project, + "preset:sample:template:sample", + ) + ) + + +@pytest.mark.parametrize( + "value", + [ + date(2026, 1, 1), + Path("unsupported"), + object(), + ], + ids=["date", "path", "custom-object"], +) +def test_lookup_rejects_unsupported_nested_values( + spec_kit_project: Path, + monkeypatch: pytest.MonkeyPatch, + value: object, +): + payload = _valid_payload() + payload["contribution"] = {"nested": {"value": value}} + monkeypatch.setattr( + _operation_lookup, + "ArtifactCatalog", + lambda _project_directory: Mock( + get_contribution_info=Mock(return_value=payload) + ), + ) + + with pytest.raises(ArtifactLookupInvalidResultError): + lookup_artifact( + ArtifactLookupRequest( + spec_kit_project, + "preset:sample:template:sample", + ) + ) + + +def test_lookup_rejects_non_string_nested_mapping_key( + spec_kit_project: Path, + monkeypatch: pytest.MonkeyPatch, +): + payload = _valid_payload() + payload["contribution"] = {"nested": {1: "value"}} + monkeypatch.setattr( + _operation_lookup, + "ArtifactCatalog", + lambda _project_directory: Mock( + get_contribution_info=Mock(return_value=payload) + ), + ) + + with pytest.raises(ArtifactLookupInvalidResultError): + lookup_artifact( + ArtifactLookupRequest( + spec_kit_project, + "preset:sample:template:sample", + ) + ) + + +@pytest.mark.parametrize( + ("value", "nested"), + [ + (float("nan"), False), + (float("inf"), False), + (float("-inf"), False), + (float("nan"), True), + (float("inf"), True), + (float("-inf"), True), + ], + ids=[ + "top-nan", + "top-positive-infinity", + "top-negative-infinity", + "nested-nan", + "nested-positive-infinity", + "nested-negative-infinity", + ], +) +def test_lookup_rejects_non_finite_floats( + spec_kit_project: Path, + monkeypatch: pytest.MonkeyPatch, + value: float, + nested: bool, +): + payload: object + if nested: + payload = _valid_payload() + payload["contribution"] = {"items": [0, {"value": value}]} + else: + payload = value + monkeypatch.setattr( + _operation_lookup, + "ArtifactCatalog", + lambda _project_directory: Mock( + get_contribution_info=Mock(return_value=payload) + ), + ) + + with pytest.raises(ArtifactLookupInvalidResultError): + lookup_artifact( + ArtifactLookupRequest( + spec_kit_project, + "preset:sample:template:sample", + ) + ) + + +@pytest.mark.parametrize("location", ["value", "key"]) +def test_lookup_rejects_invalid_unicode_in_contribution( + spec_kit_project: Path, + monkeypatch: pytest.MonkeyPatch, + location: str, +): + payload = _valid_payload() + payload["contribution"] = ( + {"value": "\ud800"} if location == "value" else {"\ud800": "value"} + ) + monkeypatch.setattr( + _operation_lookup, + "ArtifactCatalog", + lambda _project_directory: Mock( + get_contribution_info=Mock(return_value=payload) + ), + ) + + with pytest.raises(ArtifactLookupInvalidResultError): + lookup_artifact( + ArtifactLookupRequest( + spec_kit_project, + "preset:sample:template:sample", + ) + ) + + +@pytest.mark.parametrize( + "payload", + [ + None, + {}, + {**_valid_payload(), "unexpected": True}, + {key: value for key, value in _valid_payload().items() if key != "name"}, + {**_valid_payload(), "id": "preset:sample:template:wrong"}, + {**_valid_payload(), "layer": "extension"}, + {**_valid_payload(), "sourceId": "other"}, + {**_valid_payload(), "kind": "invalid"}, + {**_valid_payload(), "name": "wrong"}, + {**_valid_payload(), "manifestPath": None}, + {**_valid_payload(), "sourcePath": 1}, + {**_valid_payload(), "contribution": []}, + {**_valid_payload(), "manifestPath": "\ud800"}, + {**_valid_payload(), "sourcePath": "\ud800"}, + ], +) +def test_lookup_rejects_invalid_catalog_result_shapes( + spec_kit_project: Path, + monkeypatch: pytest.MonkeyPatch, + payload: object, +): + monkeypatch.setattr( + _operation_lookup, + "ArtifactCatalog", + lambda _project_directory: Mock( + get_contribution_info=Mock(return_value=payload) + ), + ) + + with pytest.raises(ArtifactLookupInvalidResultError) as exc_info: + lookup_artifact( + ArtifactLookupRequest( + spec_kit_project, + "preset:sample:template:sample", + ) + ) + + assert exc_info.value.code == "invalid_operation_result" + assert exc_info.value.message == "artifact resolution failed" + + +def test_lookup_rejects_recursive_catalog_result( + spec_kit_project: Path, + monkeypatch: pytest.MonkeyPatch, +): + recursive: list[object] = [] + recursive.append(recursive) + payload = _valid_payload() + payload["contribution"] = {"recursive": recursive} + monkeypatch.setattr( + _operation_lookup, + "ArtifactCatalog", + lambda _project_directory: Mock( + get_contribution_info=Mock(return_value=payload) + ), + ) + + with pytest.raises(ArtifactLookupInvalidResultError): + lookup_artifact( + ArtifactLookupRequest( + spec_kit_project, + "preset:sample:template:sample", + ) + ) + + +def test_lookup_rejects_recursive_object_catalog_result( + spec_kit_project: Path, + monkeypatch: pytest.MonkeyPatch, +): + recursive: dict[str, object] = {} + recursive["self"] = recursive + payload = _valid_payload() + payload["contribution"] = recursive + monkeypatch.setattr( + _operation_lookup, + "ArtifactCatalog", + lambda _project_directory: Mock( + get_contribution_info=Mock(return_value=payload) + ), + ) + + with pytest.raises(ArtifactLookupInvalidResultError): + lookup_artifact( + ArtifactLookupRequest( + spec_kit_project, + "preset:sample:template:sample", + ) + ) + + +def test_operation_contract_is_transport_neutral(): + forbidden_names = { + "typer", + "rich", + "mcp", + "console", + "subprocess", + "os", + "chdir", + "getcwd", + } + + assert forbidden_names.isdisjoint(_operation_lookup.__dict__) + assert ArtifactLookupResult.__module__ == ( + "specify_cli.artifacts._operation_lookup" + ) From 0dabb6499d7fed02c6b7483dbd6024a7bfe3178f Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Tue, 13 Oct 2026 14:28:49 -0500 Subject: [PATCH 2/9] fix(artifact): address lookup review findings Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 31d7c920-5bbd-4570-a38e-4d0e6a0e9877 --- src/specify_cli/artifacts/_mcp.py | 12 ++-- src/specify_cli/artifacts/command_lookup.py | 8 ++- .../artifacts/test_command_lookup.py | 55 +++++++++++++++++++ tests/specify_cli/artifacts/test_mcp_info.py | 13 ++++- tests/specify_cli/artifacts/test_mcp_list.py | 15 ++++- 5 files changed, 95 insertions(+), 8 deletions(-) diff --git a/src/specify_cli/artifacts/_mcp.py b/src/specify_cli/artifacts/_mcp.py index b51526285b..9be36a2159 100644 --- a/src/specify_cli/artifacts/_mcp.py +++ b/src/specify_cli/artifacts/_mcp.py @@ -10,6 +10,7 @@ from ._operation_info import ARTIFACT_INFO_OPERATION from ._operation_list import ARTIFACT_LIST_OPERATION +from ._operation_lookup import ARTIFACT_LOOKUP_OPERATION from .mcp_info import register as register_info from .mcp_list import register as register_list @@ -63,16 +64,17 @@ class ArtifactToolInventory: network_access=ARTIFACT_INFO_OPERATION.network_access, ), ArtifactToolInventory( - operation_id="artifact.lookup", + operation_id=ARTIFACT_LOOKUP_OPERATION.operation_id, cli_path="specify artifact lookup", mcp_tool_name="specify_artifact_lookup", - contract_version=None, + contract_version=ARTIFACT_LOOKUP_OPERATION.contract_version, disposition="unavailable", disposition_reason=( - "The artifact.lookup CLI leaf does not yet have a shared typed operation." + "The artifact.lookup shared operation exists, but its first-class " + "MCP adapter is not yet implemented." ), - capabilities=frozenset({"local-read"}), - network_access="none", + capabilities=ARTIFACT_LOOKUP_OPERATION.capabilities, + network_access=ARTIFACT_LOOKUP_OPERATION.network_access, ), ) diff --git a/src/specify_cli/artifacts/command_lookup.py b/src/specify_cli/artifacts/command_lookup.py index 3fc55f5769..24431e2528 100644 --- a/src/specify_cli/artifacts/command_lookup.py +++ b/src/specify_cli/artifacts/command_lookup.py @@ -49,10 +49,16 @@ def artifact_lookup( ) -> None: """Resolve a stack lookupId to its effective preset or extension contribution.""" _require_json_flag(json_flag) + try: + project_directory = _resolve_project_root() + except (OSError, RuntimeError): + _emit_error_and_exit(ArtifactLookupResolutionError(Path("."), lookup_id)) + return # pragma: no cover + try: result = lookup_artifact( ArtifactLookupRequest( - project_directory=_resolve_project_root(), + project_directory=project_directory, lookup_id=lookup_id, ) ) diff --git a/tests/specify_cli/artifacts/test_command_lookup.py b/tests/specify_cli/artifacts/test_command_lookup.py index 6f85e49179..996012fb95 100644 --- a/tests/specify_cli/artifacts/test_command_lookup.py +++ b/tests/specify_cli/artifacts/test_command_lookup.py @@ -326,6 +326,61 @@ def test_lookup_cli_context_filesystem_failure_is_json_error( assert result.stdout == "" assert json.loads(result.stderr) == {"error": "artifact resolution failed"} + def test_lookup_cli_context_path_resolution_runtime_error_is_json_error( + self, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + ): + monkeypatch.chdir(tmp_path) + monkeypatch.setenv("SPECIFY_INIT_DIR", "cyclic-project") + monkeypatch.setattr( + Path, + "resolve", + Mock(side_effect=RuntimeError("Symlink loop from path resolution")), + ) + + result = CliRunner().invoke( + app, + [ + "artifact", + "lookup", + "preset:sample:template:sample", + "--json", + ], + ) + + assert result.exit_code == 1 + assert result.stdout == "" + assert json.loads(result.stderr) == {"error": "artifact resolution failed"} + assert "Traceback" not in result.stderr + + def test_lookup_unexpected_operation_filesystem_failure_is_json_error( + self, + spec_kit_project: Path, + monkeypatch: pytest.MonkeyPatch, + ): + monkeypatch.setattr( + command_lookup, + "lookup_artifact", + Mock(side_effect=OSError("unreadable catalog")), + ) + monkeypatch.chdir(spec_kit_project) + + result = CliRunner().invoke( + app, + [ + "artifact", + "lookup", + "preset:sample:template:sample", + "--json", + ], + ) + + assert result.exit_code == 1 + assert result.stdout == "" + assert json.loads(result.stderr) == {"error": "artifact resolution failed"} + assert "Traceback" not in result.stderr + def test_lookup_cli_context_resolves_init_dir_override( self, spec_kit_project: Path, diff --git a/tests/specify_cli/artifacts/test_mcp_info.py b/tests/specify_cli/artifacts/test_mcp_info.py index 8fad3f1944..81782f072e 100644 --- a/tests/specify_cli/artifacts/test_mcp_info.py +++ b/tests/specify_cli/artifacts/test_mcp_info.py @@ -22,6 +22,7 @@ _commands, _mcp, _operation_info, + _operation_lookup, command_info, mcp_info, ) @@ -1030,9 +1031,19 @@ def test_artifact_info_inventory_matches_operation_and_registration( lookup_tool = next( item for item in _mcp.ARTIFACT_TOOLS if item.operation_id == "artifact.lookup" ) + assert lookup_tool.contract_version == ( + _operation_lookup.ARTIFACT_LOOKUP_OPERATION.contract_version + ) assert lookup_tool.disposition == "unavailable" assert lookup_tool.disposition_reason == ( - "The artifact.lookup CLI leaf does not yet have a shared typed operation." + "The artifact.lookup shared operation exists, but its first-class " + "MCP adapter is not yet implemented." + ) + assert lookup_tool.capabilities == ( + _operation_lookup.ARTIFACT_LOOKUP_OPERATION.capabilities + ) + assert lookup_tool.network_access == ( + _operation_lookup.ARTIFACT_LOOKUP_OPERATION.network_access ) assert lookup_tool.mcp_tool_name not in registered_tool_names diff --git a/tests/specify_cli/artifacts/test_mcp_list.py b/tests/specify_cli/artifacts/test_mcp_list.py index 7c57883838..ba15dfc20f 100644 --- a/tests/specify_cli/artifacts/test_mcp_list.py +++ b/tests/specify_cli/artifacts/test_mcp_list.py @@ -21,6 +21,7 @@ _mcp, _operation_info, _operation_list, + _operation_lookup, mcp_list, ) from specify_cli.artifacts._operation_list import ( @@ -886,7 +887,19 @@ def test_artifact_inventory_matches_cli_leaves_and_operation_contract( lookup_tool = next( item for item in unavailable if item.operation_id == "artifact.lookup" ) - assert lookup_tool.contract_version is None + assert lookup_tool.contract_version == ( + _operation_lookup.ARTIFACT_LOOKUP_OPERATION.contract_version + ) + assert lookup_tool.capabilities == ( + _operation_lookup.ARTIFACT_LOOKUP_OPERATION.capabilities + ) + assert lookup_tool.network_access == ( + _operation_lookup.ARTIFACT_LOOKUP_OPERATION.network_access + ) + assert lookup_tool.disposition_reason == ( + "The artifact.lookup shared operation exists, but its first-class " + "MCP adapter is not yet implemented." + ) def test_artifact_registration_adds_available_tool_once_and_rejects_collision( From 5d6b925790623bf728000e7820156799eabb36ea Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Tue, 13 Oct 2026 14:57:34 -0500 Subject: [PATCH 3/9] fix(artifact): preserve lookup project diagnostics Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 31d7c920-5bbd-4570-a38e-4d0e6a0e9877 --- src/specify_cli/_project.py | 27 ++------- src/specify_cli/_project_policy.py | 28 +++++++++ .../artifacts/_operation_lookup.py | 6 +- .../artifacts/test_command_lookup.py | 44 ++++++++++++++ .../artifacts/test_operation_lookup.py | 60 ++++++++++++++++++- 5 files changed, 138 insertions(+), 27 deletions(-) create mode 100644 src/specify_cli/_project_policy.py diff --git a/src/specify_cli/_project.py b/src/specify_cli/_project.py index 54bec14d86..d64796b823 100644 --- a/src/specify_cli/_project.py +++ b/src/specify_cli/_project.py @@ -11,7 +11,8 @@ from rich.markup import escape from ._console import err_console -from ._file_policy import ProjectPolicyError, project_files +from ._file_policy import project_files +from ._project_policy import ProjectResolutionError, preflight_project_policy from .file_helper import FileHelper try: @@ -22,10 +23,6 @@ from click import get_current_context -class ProjectResolutionError(RuntimeError): - """A project-root error that callers can render for their own surface.""" - - def _resolve_init_dir_override_unrendered() -> Path | None: """Resolve ``SPECIFY_INIT_DIR`` without emitting user-facing output.""" raw = os.environ.get("SPECIFY_INIT_DIR", "") @@ -50,23 +47,6 @@ def _resolve_init_dir_override_unrendered() -> Path | None: return init_root -def preflight_project_policy(project_root: Path) -> None: - """Validate at the resolved project boundary without printing or caching.""" - try: - project_files(project_root) - except ProjectPolicyError as exc: - marker = project_root / ".specify" - if marker.is_symlink(): - raise ProjectResolutionError( - f"Refusing to use symlinked .specify path: {marker}. {exc}" - ) from exc - if marker.exists() and not marker.is_dir(): - raise ProjectResolutionError( - f"Not a Spec Kit project: .specify path exists but is not a directory: {marker}" - ) from exc - raise ProjectResolutionError(str(exc)) from exc - - def _resolve_init_dir_override() -> Path | None: """Resolve the ``SPECIFY_INIT_DIR`` project override for the Python CLI. @@ -117,7 +97,8 @@ def render_project_error(error: ProjectResolutionError) -> NoReturn: """Render a project failure on the current command's error surface.""" context = get_current_context(silent=True) if context is not None and ( - context.params.get("json_output") is True or context.params.get("as_json") is True + context.params.get("json_output") is True + or context.params.get("as_json") is True ): typer.echo(json.dumps({"error": str(error)}, ensure_ascii=False), err=True) else: diff --git a/src/specify_cli/_project_policy.py b/src/specify_cli/_project_policy.py new file mode 100644 index 0000000000..1a2bee3562 --- /dev/null +++ b/src/specify_cli/_project_policy.py @@ -0,0 +1,28 @@ +"""Transport-neutral project-boundary policy validation.""" + +from __future__ import annotations + +from pathlib import Path + +from ._file_policy import ProjectPolicyError, project_files + + +class ProjectResolutionError(RuntimeError): + """A project-root error that callers can render for their own surface.""" + + +def preflight_project_policy(project_root: Path) -> None: + """Validate at the resolved project boundary without printing or caching.""" + try: + project_files(project_root) + except ProjectPolicyError as exc: + marker = project_root / ".specify" + if marker.is_symlink(): + raise ProjectResolutionError( + f"Refusing to use symlinked .specify path: {marker}. {exc}" + ) from exc + if marker.exists() and not marker.is_dir(): + raise ProjectResolutionError( + f"Not a Spec Kit project: .specify path exists but is not a directory: {marker}" + ) from exc + raise ProjectResolutionError(str(exc)) from exc diff --git a/src/specify_cli/artifacts/_operation_lookup.py b/src/specify_cli/artifacts/_operation_lookup.py index f070b558a0..40d56b9049 100644 --- a/src/specify_cli/artifacts/_operation_lookup.py +++ b/src/specify_cli/artifacts/_operation_lookup.py @@ -7,7 +7,7 @@ from pathlib import Path from typing import Any, Literal, TypeAlias -from .._file_policy import ProjectPolicyError, project_files +from .._project_policy import ProjectResolutionError, preflight_project_policy from ..extensions import ExtensionRegistry from ..integrations import installer from ..presets import PresetError @@ -408,8 +408,8 @@ def lookup_artifact(request: ArtifactLookupRequest) -> ArtifactLookupResult: try: if project_directory.is_dir(): - project_files(project_directory) - except ProjectPolicyError as exc: + preflight_project_policy(project_directory) + except ProjectResolutionError as exc: raise ArtifactLookupProjectPolicyError( project_directory, str(exc), diff --git a/tests/specify_cli/artifacts/test_command_lookup.py b/tests/specify_cli/artifacts/test_command_lookup.py index 996012fb95..9c560c9cfe 100644 --- a/tests/specify_cli/artifacts/test_command_lookup.py +++ b/tests/specify_cli/artifacts/test_command_lookup.py @@ -11,6 +11,10 @@ from typer.testing import CliRunner from specify_cli import app +from specify_cli._project_policy import ( + ProjectResolutionError, + preflight_project_policy, +) from specify_cli.artifacts import ( ArtifactCatalog, _operation_lookup, @@ -129,6 +133,46 @@ def test_lookup_policy_failure_precedes_loader_and_catalog( loader.assert_not_called() catalog.assert_not_called() + @pytest.mark.parametrize("marker_kind", ["symlink", "file"]) + def test_lookup_preserves_structural_project_policy_json_error( + self, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + marker_kind: str, + ): + project_directory = tmp_path / "project" + project_directory.mkdir() + marker = project_directory / ".specify" + if marker_kind == "symlink": + target = tmp_path / "real-specify" + target.mkdir() + try: + marker.symlink_to(target, target_is_directory=True) + except (OSError, NotImplementedError) as exc: + pytest.skip(f"Symlinks unavailable: {exc}") + else: + marker.write_text("not a directory", encoding="utf-8") + + with pytest.raises(ProjectResolutionError) as preflight_exc: + preflight_project_policy(project_directory) + + monkeypatch.chdir(project_directory) + result = CliRunner().invoke( + app, + [ + "artifact", + "lookup", + "malformed", + "--json", + ], + ) + + assert result.exit_code == 1 + assert result.stdout == "" + assert json.loads(result.stderr) == {"error": str(preflight_exc.value)} + assert result.stderr.count("\n") == 1 + assert "Traceback" not in result.stderr + def test_lookup_json_rejects_unknown_contribution( self, spec_kit_project: Path, monkeypatch: pytest.MonkeyPatch ): diff --git a/tests/specify_cli/artifacts/test_operation_lookup.py b/tests/specify_cli/artifacts/test_operation_lookup.py index 95c31a6cd4..6963f8c63b 100644 --- a/tests/specify_cli/artifacts/test_operation_lookup.py +++ b/tests/specify_cli/artifacts/test_operation_lookup.py @@ -9,7 +9,11 @@ import pytest import yaml -from specify_cli import artifacts +from specify_cli import _project_policy, artifacts +from specify_cli._project_policy import ( + ProjectResolutionError, + preflight_project_policy, +) from specify_cli.artifacts import ( ArtifactCatalog, ArtifactResolutionError, @@ -605,6 +609,59 @@ def test_lookup_maps_project_policy_failure_before_loader_and_catalog( catalog.assert_not_called() +@pytest.mark.parametrize("marker_kind", ["symlink", "file"]) +def test_lookup_preserves_structural_project_policy_diagnostic( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + marker_kind: str, +): + project_directory = tmp_path / "project" + project_directory.mkdir() + marker = project_directory / ".specify" + if marker_kind == "symlink": + target = tmp_path / "real-specify" + target.mkdir() + try: + marker.symlink_to(target, target_is_directory=True) + except (OSError, NotImplementedError) as exc: + pytest.skip(f"Symlinks unavailable: {exc}") + else: + marker.write_text("not a directory", encoding="utf-8") + + with pytest.raises(ProjectResolutionError) as preflight_exc: + preflight_project_policy(project_directory) + + loader = Mock(side_effect=AssertionError("loader reached")) + catalog = Mock(side_effect=AssertionError("catalog reached")) + monkeypatch.setattr( + _operation_lookup.installer, + "load_installed_integrations", + loader, + ) + monkeypatch.setattr(_operation_lookup, "ArtifactCatalog", catalog) + + with pytest.raises(ArtifactLookupProjectPolicyError) as operation_exc: + lookup_artifact( + ArtifactLookupRequest( + project_directory, + "malformed", + ) + ) + + assert operation_exc.value.message == str(preflight_exc.value) + if marker_kind == "symlink": + assert operation_exc.value.message.startswith( + f"Refusing to use symlinked .specify path: {marker}." + ) + else: + assert operation_exc.value.message == ( + "Not a Spec Kit project: .specify path exists but is not a directory: " + f"{marker}" + ) + loader.assert_not_called() + catalog.assert_not_called() + + def test_lookup_maps_corrupt_registry_before_malformed_identifier( spec_kit_project: Path, monkeypatch: pytest.MonkeyPatch, @@ -979,6 +1036,7 @@ def test_operation_contract_is_transport_neutral(): } assert forbidden_names.isdisjoint(_operation_lookup.__dict__) + assert forbidden_names.isdisjoint(_project_policy.__dict__) assert ArtifactLookupResult.__module__ == ( "specify_cli.artifacts._operation_lookup" ) From 171a3e8ab39693150780f6c847dbbe812579433d Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Tue, 13 Oct 2026 17:46:52 -0500 Subject: [PATCH 4/9] fix(artifact): sanitize unexpected lookup failures Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 31d7c920-5bbd-4570-a38e-4d0e6a0e9877 --- src/specify_cli/artifacts/command_lookup.py | 2 +- tests/specify_cli/artifacts/test_command_lookup.py | 13 +++++++++++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/specify_cli/artifacts/command_lookup.py b/src/specify_cli/artifacts/command_lookup.py index 24431e2528..8ba00f6926 100644 --- a/src/specify_cli/artifacts/command_lookup.py +++ b/src/specify_cli/artifacts/command_lookup.py @@ -65,7 +65,7 @@ def artifact_lookup( except ArtifactError as exc: _emit_error_and_exit(exc) return # pragma: no cover - except OSError: + except Exception: # noqa: BLE001 - preserve the sanitized JSON error contract _emit_error_and_exit(ArtifactLookupResolutionError(Path("."), lookup_id)) return # pragma: no cover diff --git a/tests/specify_cli/artifacts/test_command_lookup.py b/tests/specify_cli/artifacts/test_command_lookup.py index 9c560c9cfe..3210105070 100644 --- a/tests/specify_cli/artifacts/test_command_lookup.py +++ b/tests/specify_cli/artifacts/test_command_lookup.py @@ -398,15 +398,24 @@ def test_lookup_cli_context_path_resolution_runtime_error_is_json_error( assert json.loads(result.stderr) == {"error": "artifact resolution failed"} assert "Traceback" not in result.stderr - def test_lookup_unexpected_operation_filesystem_failure_is_json_error( + @pytest.mark.parametrize( + "failure", + [ + OSError("unreadable catalog"), + RuntimeError("unexpected catalog failure"), + ], + ids=["filesystem", "non-filesystem"], + ) + def test_lookup_unexpected_operation_failure_is_json_error( self, spec_kit_project: Path, monkeypatch: pytest.MonkeyPatch, + failure: Exception, ): monkeypatch.setattr( command_lookup, "lookup_artifact", - Mock(side_effect=OSError("unreadable catalog")), + Mock(side_effect=failure), ) monkeypatch.chdir(spec_kit_project) From 907b5046a7d1dbf5c276081eba0af31d375816c6 Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Wed, 14 Oct 2026 08:58:10 -0500 Subject: [PATCH 5/9] fix(artifact): preserve lookup adapter diagnostics Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 31d7c920-5bbd-4570-a38e-4d0e6a0e9877 --- src/specify_cli/artifacts/command_lookup.py | 10 ++++- .../artifacts/test_command_lookup.py | 42 +++++++++++++------ 2 files changed, 37 insertions(+), 15 deletions(-) diff --git a/src/specify_cli/artifacts/command_lookup.py b/src/specify_cli/artifacts/command_lookup.py index 8ba00f6926..2f7d84f3db 100644 --- a/src/specify_cli/artifacts/command_lookup.py +++ b/src/specify_cli/artifacts/command_lookup.py @@ -3,6 +3,7 @@ from __future__ import annotations import json +import logging import os import sys from pathlib import Path @@ -16,11 +17,15 @@ artifact_app, ) from ._operation_lookup import ( + ArtifactLookupProjectError, ArtifactLookupRequest, ArtifactLookupResolutionError, lookup_artifact, ) +logger = logging.getLogger(__name__) +logger.addHandler(logging.NullHandler()) + def _project_directory_from_cli_context() -> Path: """Return the explicit project directory selected by the CLI invocation.""" @@ -52,7 +57,7 @@ def artifact_lookup( try: project_directory = _resolve_project_root() except (OSError, RuntimeError): - _emit_error_and_exit(ArtifactLookupResolutionError(Path("."), lookup_id)) + _emit_error_and_exit(ArtifactLookupProjectError(Path("."))) return # pragma: no cover try: @@ -65,7 +70,8 @@ def artifact_lookup( except ArtifactError as exc: _emit_error_and_exit(exc) return # pragma: no cover - except Exception: # noqa: BLE001 - preserve the sanitized JSON error contract + except Exception: + logger.exception("Unexpected failure in the artifact lookup CLI adapter.") _emit_error_and_exit(ArtifactLookupResolutionError(Path("."), lookup_id)) return # pragma: no cover diff --git a/tests/specify_cli/artifacts/test_command_lookup.py b/tests/specify_cli/artifacts/test_command_lookup.py index 3210105070..d5dfa18894 100644 --- a/tests/specify_cli/artifacts/test_command_lookup.py +++ b/tests/specify_cli/artifacts/test_command_lookup.py @@ -3,6 +3,7 @@ from __future__ import annotations import json +import logging from datetime import date from pathlib import Path from unittest.mock import Mock @@ -346,7 +347,7 @@ def test_lookup_expected_operation_failure_is_one_legacy_json_error( assert "\x1b[" not in result.stderr assert "Traceback" not in result.stderr - def test_lookup_cli_context_filesystem_failure_is_json_error( + def test_lookup_cli_context_filesystem_failure_preserves_project_error( self, monkeypatch: pytest.MonkeyPatch, ): @@ -368,9 +369,11 @@ def test_lookup_cli_context_filesystem_failure_is_json_error( assert result.exit_code == 1 assert result.stdout == "" - assert json.loads(result.stderr) == {"error": "artifact resolution failed"} + assert json.loads(result.stderr) == { + "error": "not a Spec Kit project: no .specify/ directory found" + } - def test_lookup_cli_context_path_resolution_runtime_error_is_json_error( + def test_lookup_cli_context_path_resolution_runtime_error_preserves_project_error( self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, @@ -395,7 +398,9 @@ def test_lookup_cli_context_path_resolution_runtime_error_is_json_error( assert result.exit_code == 1 assert result.stdout == "" - assert json.loads(result.stderr) == {"error": "artifact resolution failed"} + assert json.loads(result.stderr) == { + "error": "not a Spec Kit project: no .specify/ directory found" + } assert "Traceback" not in result.stderr @pytest.mark.parametrize( @@ -410,6 +415,7 @@ def test_lookup_unexpected_operation_failure_is_json_error( self, spec_kit_project: Path, monkeypatch: pytest.MonkeyPatch, + caplog: pytest.LogCaptureFixture, failure: Exception, ): monkeypatch.setattr( @@ -419,20 +425,30 @@ def test_lookup_unexpected_operation_failure_is_json_error( ) monkeypatch.chdir(spec_kit_project) - result = CliRunner().invoke( - app, - [ - "artifact", - "lookup", - "preset:sample:template:sample", - "--json", - ], - ) + with caplog.at_level( + logging.ERROR, + logger="specify_cli.artifacts.command_lookup", + ): + result = CliRunner().invoke( + app, + [ + "artifact", + "lookup", + "preset:sample:template:sample", + "--json", + ], + ) assert result.exit_code == 1 assert result.stdout == "" assert json.loads(result.stderr) == {"error": "artifact resolution failed"} + assert str(failure) not in result.stderr assert "Traceback" not in result.stderr + assert len(caplog.records) == 1 + assert caplog.records[0].message == ( + "Unexpected failure in the artifact lookup CLI adapter." + ) + assert caplog.records[0].exc_info is not None def test_lookup_cli_context_resolves_init_dir_override( self, From 25f324903257c9c5348116dd4aa450fe804bdd83 Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Wed, 14 Oct 2026 09:12:02 -0500 Subject: [PATCH 6/9] fix(artifact): harden lookup error serialization Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 31d7c920-5bbd-4570-a38e-4d0e6a0e9877 --- .../artifacts/_operation_lookup.py | 11 ++++- src/specify_cli/artifacts/command_lookup.py | 10 ++++- .../artifacts/test_command_lookup.py | 41 ++++++++++++++++--- .../artifacts/test_operation_lookup.py | 11 ++++- 4 files changed, 62 insertions(+), 11 deletions(-) diff --git a/src/specify_cli/artifacts/_operation_lookup.py b/src/specify_cli/artifacts/_operation_lookup.py index 40d56b9049..d802f7c7c2 100644 --- a/src/specify_cli/artifacts/_operation_lookup.py +++ b/src/specify_cli/artifacts/_operation_lookup.py @@ -39,6 +39,12 @@ def to_json_dict(self) -> dict[str, JsonProjection]: JsonValue: TypeAlias = JsonScalar | tuple["JsonValue", ...] | ArtifactLookupJsonObject +def _json_safe_error_text(value: object) -> str: + """Return diagnostic text that can always be encoded as UTF-8 JSON.""" + text = value if type(value) is str else str(value) + return text.encode("utf-8", errors="backslashreplace").decode("utf-8") + + @dataclass(frozen=True) class ArtifactLookupRequest: """Explicit project context and contribution identifier.""" @@ -133,10 +139,11 @@ class ArtifactLookupIdentifierError(ArtifactLookupError): """The contribution lookup identifier is structurally invalid.""" def __init__(self, lookup_id: object) -> None: + safe_lookup_id = _json_safe_error_text(lookup_id) super().__init__( code="invalid_lookup_id", - message=f"unknown contribution {lookup_id}", - details={"lookup_id": lookup_id}, + message=f"unknown contribution {safe_lookup_id}", + details={"lookup_id": safe_lookup_id}, ) diff --git a/src/specify_cli/artifacts/command_lookup.py b/src/specify_cli/artifacts/command_lookup.py index 2f7d84f3db..3ff2581650 100644 --- a/src/specify_cli/artifacts/command_lookup.py +++ b/src/specify_cli/artifacts/command_lookup.py @@ -33,7 +33,10 @@ def _project_directory_from_cli_context() -> Path: override = os.environ.get("SPECIFY_INIT_DIR", "") if not override: return invocation_directory - return (invocation_directory / override).resolve() + try: + return (invocation_directory / override).resolve() + except (OSError, RuntimeError) as exc: + raise ArtifactLookupProjectError(Path(".")) from exc def _resolve_project_root() -> Path: @@ -56,8 +59,11 @@ def artifact_lookup( _require_json_flag(json_flag) try: project_directory = _resolve_project_root() + except ArtifactError as exc: + _emit_error_and_exit(exc) + return # pragma: no cover except (OSError, RuntimeError): - _emit_error_and_exit(ArtifactLookupProjectError(Path("."))) + _emit_error_and_exit(ArtifactLookupResolutionError(Path("."), lookup_id)) return # pragma: no cover try: diff --git a/tests/specify_cli/artifacts/test_command_lookup.py b/tests/specify_cli/artifacts/test_command_lookup.py index d5dfa18894..a68b806c43 100644 --- a/tests/specify_cli/artifacts/test_command_lookup.py +++ b/tests/specify_cli/artifacts/test_command_lookup.py @@ -347,7 +347,7 @@ def test_lookup_expected_operation_failure_is_one_legacy_json_error( assert "\x1b[" not in result.stderr assert "Traceback" not in result.stderr - def test_lookup_cli_context_filesystem_failure_preserves_project_error( + def test_lookup_cli_context_filesystem_failure_is_resolution_error( self, monkeypatch: pytest.MonkeyPatch, ): @@ -369,21 +369,28 @@ def test_lookup_cli_context_filesystem_failure_preserves_project_error( assert result.exit_code == 1 assert result.stdout == "" - assert json.loads(result.stderr) == { - "error": "not a Spec Kit project: no .specify/ directory found" - } + assert json.loads(result.stderr) == {"error": "artifact resolution failed"} - def test_lookup_cli_context_path_resolution_runtime_error_preserves_project_error( + @pytest.mark.parametrize( + "failure", + [ + OSError("unreadable override"), + RuntimeError("Symlink loop from path resolution"), + ], + ids=["filesystem", "runtime"], + ) + def test_lookup_cli_override_resolution_failure_preserves_project_error( self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, + failure: Exception, ): monkeypatch.chdir(tmp_path) monkeypatch.setenv("SPECIFY_INIT_DIR", "cyclic-project") monkeypatch.setattr( Path, "resolve", - Mock(side_effect=RuntimeError("Symlink loop from path resolution")), + Mock(side_effect=failure), ) result = CliRunner().invoke( @@ -403,6 +410,28 @@ def test_lookup_cli_context_path_resolution_runtime_error_preserves_project_erro } assert "Traceback" not in result.stderr + def test_lookup_invalid_unicode_identifier_is_utf8_json_error( + self, + spec_kit_project: Path, + monkeypatch: pytest.MonkeyPatch, + ): + lookup_id = "extension:source:template:\ud800" + monkeypatch.chdir(spec_kit_project) + + result = CliRunner().invoke( + app, + ["artifact", "lookup", lookup_id, "--json"], + ) + + assert result.exit_code == 1 + assert result.stdout == "" + result.stderr.encode("utf-8") + assert json.loads(result.stderr) == { + "error": "unknown contribution extension:source:template:\\ud800" + } + assert result.stderr.count("\n") == 1 + assert "Traceback" not in result.stderr + @pytest.mark.parametrize( "failure", [ diff --git a/tests/specify_cli/artifacts/test_operation_lookup.py b/tests/specify_cli/artifacts/test_operation_lookup.py index 6963f8c63b..dee90bdbd3 100644 --- a/tests/specify_cli/artifacts/test_operation_lookup.py +++ b/tests/specify_cli/artifacts/test_operation_lookup.py @@ -2,6 +2,7 @@ from __future__ import annotations +import json from datetime import date from pathlib import Path from unittest.mock import Mock @@ -557,9 +558,17 @@ def test_lookup_maps_invalid_unicode_lookup_id_after_project_validation( ): lookup_id = "extension:source:template:\ud800" - with pytest.raises(ArtifactLookupIdentifierError): + with pytest.raises(ArtifactLookupIdentifierError) as exc_info: lookup_artifact(ArtifactLookupRequest(spec_kit_project, lookup_id)) + escaped_lookup_id = "extension:source:template:\\ud800" + assert exc_info.value.message == f"unknown contribution {escaped_lookup_id}" + assert exc_info.value.details == {"lookup_id": escaped_lookup_id} + json.dumps( + {"error": exc_info.value.message}, + ensure_ascii=False, + ).encode("utf-8") + @pytest.mark.parametrize( "lookup_id", From bd7fdd0528c8608dd4a67ed9b4e18d14f10275b6 Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Wed, 14 Oct 2026 09:36:41 -0500 Subject: [PATCH 7/9] fix(artifact): normalize lookup error messages Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 31d7c920-5bbd-4570-a38e-4d0e6a0e9877 --- .../artifacts/_operation_lookup.py | 5 +-- .../artifacts/test_command_lookup.py | 35 +++++++++++++++++++ .../artifacts/test_operation_lookup.py | 29 +++++++++++++++ 3 files changed, 67 insertions(+), 2 deletions(-) diff --git a/src/specify_cli/artifacts/_operation_lookup.py b/src/specify_cli/artifacts/_operation_lookup.py index d802f7c7c2..de72beba2a 100644 --- a/src/specify_cli/artifacts/_operation_lookup.py +++ b/src/specify_cli/artifacts/_operation_lookup.py @@ -95,11 +95,12 @@ def __init__( details: dict[str, Any], retryable: bool = False, ) -> None: + safe_message = _json_safe_error_text(message) self.code = code - self.message = message + self.message = safe_message self.details = details self.retryable = retryable - super().__init__(message) + super().__init__(safe_message) class ArtifactLookupProjectDirectoryError(ArtifactLookupError): diff --git a/tests/specify_cli/artifacts/test_command_lookup.py b/tests/specify_cli/artifacts/test_command_lookup.py index a68b806c43..22db7fba48 100644 --- a/tests/specify_cli/artifacts/test_command_lookup.py +++ b/tests/specify_cli/artifacts/test_command_lookup.py @@ -432,6 +432,41 @@ def test_lookup_invalid_unicode_identifier_is_utf8_json_error( assert result.stderr.count("\n") == 1 assert "Traceback" not in result.stderr + def test_lookup_filesystem_derived_message_is_utf8_json_error( + self, + spec_kit_project: Path, + monkeypatch: pytest.MonkeyPatch, + ): + monkeypatch.setattr( + installer, + "load_installed_integrations", + Mock( + side_effect=installer.IntegrationInstallError( + "unregistered child \ud800" + ) + ), + ) + monkeypatch.chdir(spec_kit_project) + + result = CliRunner().invoke( + app, + [ + "artifact", + "lookup", + "preset:sample:template:sample", + "--json", + ], + ) + + assert result.exit_code == 1 + assert result.stdout == "" + result.stderr.encode("utf-8") + assert json.loads(result.stderr) == { + "error": "artifact resolution failed: unregistered child \\ud800" + } + assert result.stderr.count("\n") == 1 + assert "Traceback" not in result.stderr + @pytest.mark.parametrize( "failure", [ diff --git a/tests/specify_cli/artifacts/test_operation_lookup.py b/tests/specify_cli/artifacts/test_operation_lookup.py index dee90bdbd3..79393aded8 100644 --- a/tests/specify_cli/artifacts/test_operation_lookup.py +++ b/tests/specify_cli/artifacts/test_operation_lookup.py @@ -739,6 +739,35 @@ def test_lookup_maps_integration_loader_failure( assert exc_info.value.__cause__ is failure +def test_lookup_normalizes_filesystem_derived_error_message( + spec_kit_project: Path, + monkeypatch: pytest.MonkeyPatch, +): + failure = IntegrationInstallError("unregistered child \ud800") + monkeypatch.setattr( + _operation_lookup.installer, + "load_installed_integrations", + Mock(side_effect=failure), + ) + + with pytest.raises(ArtifactLookupIntegrationError) as exc_info: + lookup_artifact( + ArtifactLookupRequest( + spec_kit_project, + "preset:sample:template:sample", + ) + ) + + assert exc_info.value.message == ( + "artifact resolution failed: unregistered child \\ud800" + ) + json.dumps( + {"error": exc_info.value.message}, + ensure_ascii=False, + ).encode("utf-8") + assert exc_info.value.__cause__ is failure + + @pytest.mark.parametrize( ("failure", "error_type", "code"), [ From f9252830a83ac22a169f3dae73be705c29f82804 Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Wed, 14 Oct 2026 09:45:32 -0500 Subject: [PATCH 8/9] fix(artifact): stabilize lookup error details Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 31d7c920-5bbd-4570-a38e-4d0e6a0e9877 --- .../artifacts/_operation_lookup.py | 21 +++++- .../artifacts/test_operation_lookup.py | 64 +++++++++++++++++++ 2 files changed, 82 insertions(+), 3 deletions(-) diff --git a/src/specify_cli/artifacts/_operation_lookup.py b/src/specify_cli/artifacts/_operation_lookup.py index de72beba2a..fe6ec4a8de 100644 --- a/src/specify_cli/artifacts/_operation_lookup.py +++ b/src/specify_cli/artifacts/_operation_lookup.py @@ -45,6 +45,14 @@ def _json_safe_error_text(value: object) -> str: return text.encode("utf-8", errors="backslashreplace").decode("utf-8") +def _json_safe_error_details(details: dict[str, Any]) -> dict[str, Any]: + """Normalize string detail values without changing their declared types.""" + return { + key: _json_safe_error_text(value) if type(value) is str else value + for key, value in details.items() + } + + @dataclass(frozen=True) class ArtifactLookupRequest: """Explicit project context and contribution identifier.""" @@ -98,7 +106,7 @@ def __init__( safe_message = _json_safe_error_text(message) self.code = code self.message = safe_message - self.details = details + self.details = _json_safe_error_details(details) self.retryable = retryable super().__init__(safe_message) @@ -415,14 +423,21 @@ def lookup_artifact(request: ArtifactLookupRequest) -> ArtifactLookupResult: raise ArtifactLookupProjectDirectoryError(project_directory) try: - if project_directory.is_dir(): + project_is_directory = project_directory.is_dir() + if project_is_directory: preflight_project_policy(project_directory) + has_project_marker = (project_directory / ".specify").is_dir() except ProjectResolutionError as exc: raise ArtifactLookupProjectPolicyError( project_directory, str(exc), ) from exc - if not (project_directory / ".specify").is_dir(): + except OSError as exc: + raise ArtifactLookupFilesystemError( + project_directory, + request.lookup_id, + ) from exc + if not has_project_marker: raise ArtifactLookupProjectError(project_directory) try: diff --git a/tests/specify_cli/artifacts/test_operation_lookup.py b/tests/specify_cli/artifacts/test_operation_lookup.py index 79393aded8..e5763ab407 100644 --- a/tests/specify_cli/artifacts/test_operation_lookup.py +++ b/tests/specify_cli/artifacts/test_operation_lookup.py @@ -689,6 +689,70 @@ def test_lookup_maps_corrupt_registry_before_malformed_identifier( ) assert exc_info.value.code == "corrupt_extension_registry" + + +def test_lookup_normalizes_prevalidation_identifier_error_details( + spec_kit_project: Path, +): + lookup_id = "extension:source:template:\ud800" + registry = spec_kit_project / ".specify" / "extensions" / ".registry" + registry.write_text("{invalid", encoding="utf-8") + + with pytest.raises(ArtifactLookupRegistryError) as exc_info: + lookup_artifact(ArtifactLookupRequest(spec_kit_project, lookup_id)) + + assert exc_info.value.details["lookup_id"] == ("extension:source:template:\\ud800") + json.dumps( + {"details": exc_info.value.details}, + ensure_ascii=False, + ).encode("utf-8") + + +@pytest.mark.parametrize( + "probe_results", + [ + [OSError("project probe failed")], + [True, OSError("marker probe failed")], + ], + ids=["project-directory", "project-marker"], +) +def test_lookup_maps_project_probe_filesystem_failures( + spec_kit_project: Path, + monkeypatch: pytest.MonkeyPatch, + probe_results: list[bool | OSError], +): + failure = next(value for value in probe_results if isinstance(value, OSError)) + results = iter(probe_results) + loader = Mock(side_effect=AssertionError("loader reached")) + catalog = Mock(side_effect=AssertionError("catalog reached")) + + def probe_is_dir(_path: Path) -> bool: + result = next(results) + if isinstance(result, OSError): + raise result + return result + + monkeypatch.setattr(Path, "is_dir", probe_is_dir) + monkeypatch.setattr(_operation_lookup, "preflight_project_policy", Mock()) + monkeypatch.setattr( + _operation_lookup.installer, + "load_installed_integrations", + loader, + ) + monkeypatch.setattr(_operation_lookup, "ArtifactCatalog", catalog) + + with pytest.raises(ArtifactLookupFilesystemError) as exc_info: + lookup_artifact( + ArtifactLookupRequest( + spec_kit_project, + "preset:sample:template:sample", + ) + ) + + assert exc_info.value.code == "filesystem_error" + assert exc_info.value.__cause__ is failure + loader.assert_not_called() + catalog.assert_not_called() catalog.assert_not_called() From da6aedfcbc93a8f2b2ca7152f803e425410c05b7 Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Wed, 14 Oct 2026 09:52:47 -0500 Subject: [PATCH 9/9] fix(artifact): narrow lookup expected errors Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 31d7c920-5bbd-4570-a38e-4d0e6a0e9877 --- src/specify_cli/artifacts/command_lookup.py | 6 +++--- tests/specify_cli/artifacts/test_command_lookup.py | 4 +++- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/specify_cli/artifacts/command_lookup.py b/src/specify_cli/artifacts/command_lookup.py index 3ff2581650..0703297c41 100644 --- a/src/specify_cli/artifacts/command_lookup.py +++ b/src/specify_cli/artifacts/command_lookup.py @@ -10,13 +10,13 @@ import typer -from . import ArtifactError from ._commands import ( _emit_error_and_exit, _require_json_flag, artifact_app, ) from ._operation_lookup import ( + ArtifactLookupError, ArtifactLookupProjectError, ArtifactLookupRequest, ArtifactLookupResolutionError, @@ -59,7 +59,7 @@ def artifact_lookup( _require_json_flag(json_flag) try: project_directory = _resolve_project_root() - except ArtifactError as exc: + except ArtifactLookupError as exc: _emit_error_and_exit(exc) return # pragma: no cover except (OSError, RuntimeError): @@ -73,7 +73,7 @@ def artifact_lookup( lookup_id=lookup_id, ) ) - except ArtifactError as exc: + except ArtifactLookupError as exc: _emit_error_and_exit(exc) return # pragma: no cover except Exception: diff --git a/tests/specify_cli/artifacts/test_command_lookup.py b/tests/specify_cli/artifacts/test_command_lookup.py index 22db7fba48..7950d27b08 100644 --- a/tests/specify_cli/artifacts/test_command_lookup.py +++ b/tests/specify_cli/artifacts/test_command_lookup.py @@ -18,6 +18,7 @@ ) from specify_cli.artifacts import ( ArtifactCatalog, + ArtifactResolutionError, _operation_lookup, command_lookup, ) @@ -472,8 +473,9 @@ def test_lookup_filesystem_derived_message_is_utf8_json_error( [ OSError("unreadable catalog"), RuntimeError("unexpected catalog failure"), + ArtifactResolutionError("unexpected catalog detail"), ], - ids=["filesystem", "non-filesystem"], + ids=["filesystem", "non-filesystem", "sibling-artifact-error"], ) def test_lookup_unexpected_operation_failure_is_json_error( self,