From bbe32b308b3ef57a09a0ff25762d6f23170c9a5e Mon Sep 17 00:00:00 2001 From: Pybsama <294532706+Pybsama@users.noreply.github.com> Date: Sat, 3 Oct 2026 13:44:57 +0800 Subject: [PATCH 1/4] fix: install versioned marketplace package directories --- .../owners/install-deployment.json | 6 +- .../checks/install_dry_run_plan.py | 47 +++- .../deps/github_downloader_validation.py | 10 +- src/apm_cli/install/dry_run_plan.py | 7 +- src/apm_cli/install/package_selection.py | 15 +- src/apm_cli/install/phases/resolve.py | 9 +- src/apm_cli/marketplace/resolver.py | 18 +- .../test_architecture_owner_rule_mutations.py | 27 +++ ...rketplace_versioned_directory_lifecycle.py | 220 ++++++++++++++++++ .../deps/test_github_downloader_validation.py | 2 +- .../deps/test_versioned_directory_probe.py | 72 ++++++ .../test_resolve_selective_update_plan.py | 23 ++ tests/unit/install/test_dry_run_render.py | 25 ++ .../test_versioned_directory_resolution.py | 63 +++++ 14 files changed, 519 insertions(+), 25 deletions(-) create mode 100644 tests/integration/test_marketplace_versioned_directory_lifecycle.py create mode 100644 tests/unit/deps/test_versioned_directory_probe.py create mode 100644 tests/unit/marketplace/test_versioned_directory_resolution.py diff --git a/.apm/architecture/owners/install-deployment.json b/.apm/architecture/owners/install-deployment.json index 746f2b5f18..aa471e89e9 100644 --- a/.apm/architecture/owners/install-deployment.json +++ b/.apm/architecture/owners/install-deployment.json @@ -70,9 +70,9 @@ }, { "id": "prospective-dry-run-install-plan", - "decision": "Prospective dry-run install plan", - "owner": "install/dry_run_plan.py (ProspectiveInstallPlan)", - "selectors": ["src/apm_cli/install/dry_run_plan.py"], + "decision": "Prospective dry-run install plan and interpreted package selection", + "owner": "install/dry_run_plan.py (ProspectiveInstallPlan) delegates selector identity to install/package_selection.py", + "selectors": ["src/apm_cli/install/dry_run_plan.py", "src/apm_cli/install/package_selection.py"], "guards": ["install-deployment-prospective-dry-run-plan"] }, { diff --git a/scripts/architecture_linter/checks/install_dry_run_plan.py b/scripts/architecture_linter/checks/install_dry_run_plan.py index 827696d02b..f9d531f72f 100644 --- a/scripts/architecture_linter/checks/install_dry_run_plan.py +++ b/scripts/architecture_linter/checks/install_dry_run_plan.py @@ -2,6 +2,7 @@ from __future__ import annotations +import ast import re from scripts.architecture_linter.checks.install_deployment_shared import ( @@ -19,6 +20,35 @@ _OWNER = "src/apm_cli/install/dry_run_plan.py" _COMMAND = "src/apm_cli/commands/install.py" _RENDERER = "src/apm_cli/install/presentation/dry_run.py" +_SELECTION_OWNER = "src/apm_cli/install/package_selection.py" +_RESOLVE = "src/apm_cli/install/phases/resolve.py" + + +def _selection_uses_interpreted_identity(provider: FactsProvider) -> bool: + """Install and preview must preserve structured references through one owner.""" + for path, function_name in ( + (_OWNER, "ProspectiveInstallPlan.from_apm_package"), + (_RESOLVE, "_apply_only_filter"), + ): + index = provider.tree_index(path) + function = index.function(function_name) if index is not None else None + if function is None: + return False + calls = [node for node in ast.walk(function) if isinstance(node, ast.Call)] + if not any( + isinstance(call.func, ast.Name) and call.func.id == "selected_dependency_identity" + for call in calls + ): + return False + if any( + isinstance(call.func, ast.Attribute) + and isinstance(call.func.value, ast.Name) + and call.func.value.id == "DependencyReference" + and call.func.attr == "parse" + for call in calls + ): + return False + return True def check_prospective_dry_run_plan(provider: FactsProvider) -> tuple[Violation, ...]: @@ -27,7 +57,8 @@ def check_prospective_dry_run_plan(provider: FactsProvider) -> tuple[Violation, owner, owner_fail = _facts_for(provider, _OWNER, rule_id) command, command_fail = _facts_for(provider, _COMMAND, rule_id) renderer, renderer_fail = _facts_for(provider, _RENDERER, rule_id) - failures = list(owner_fail) + list(command_fail) + list(renderer_fail) + selection, selection_fail = _facts_for(provider, _SELECTION_OWNER, rule_id) + failures = list(owner_fail) + list(command_fail) + list(renderer_fail) + list(selection_fail) if failures: return tuple(failures) @@ -41,8 +72,20 @@ def check_prospective_dry_run_plan(provider: FactsProvider) -> tuple[Violation, message="ProspectiveInstallPlan must remain the sole dry-run preview owner", respect_exempt=False, ) + identity_pattern = re.compile(r"^def selected_dependency_identity\(") + duplicates += _duplicate_definition_lines( + provider, + rule_id=rule_id, + prefix=_SRC_PREFIX, + pattern=identity_pattern, + owner=_SELECTION_OWNER, + message="Install selector identity must remain owned by package_selection", + respect_exempt=False, + ) contract_holds = ( _count_re(owner, class_pattern) == 1 + and _count_re(selection, identity_pattern) == 1 + and _selection_uses_interpreted_identity(provider) and _present(owner, "def from_apm_package(") and _present(owner, "def with_allowed_lsp_dependencies(") and _present(owner, "selected_apm_dependencies=selected_apm_dependencies") @@ -74,7 +117,7 @@ def check_prospective_dry_run_plan(provider: FactsProvider) -> tuple[Violation, rule_id, _OWNER, "Dry-run dependencies, selection, checks, rendering, and counts must route " - "through ProspectiveInstallPlan", + "through ProspectiveInstallPlan and the shared package-selection identity owner", ), *duplicates, ) diff --git a/src/apm_cli/deps/github_downloader_validation.py b/src/apm_cli/deps/github_downloader_validation.py index 8012c0371e..d76443fc8b 100644 --- a/src/apm_cli/deps/github_downloader_validation.py +++ b/src/apm_cli/deps/github_downloader_validation.py @@ -730,7 +730,7 @@ def _path_exists_in_tree_at_ref( log: Callable[[str], None], winning_attempt: AttemptSpec, ) -> bool: - """Confirm ``vpath`` exists at ``ref`` via shallow fetch + ``ls-tree``. + """Confirm ``vpath`` is a directory at ``ref`` via shallow fetch + ``ls-tree``. Closes the fail-open hole in ``_ref_exists_via_ls_remote``: knowing that the ref exists is not the same as knowing the subdirectory @@ -748,7 +748,7 @@ def _path_exists_in_tree_at_ref( Returns: True iff the shallow fetch succeeded AND ``ls-tree`` reported - at least one entry for ``vpath`` at the resolved ref. + a directory entry for ``vpath`` at the resolved ref. """ label, url, env = winning_attempt @@ -804,7 +804,7 @@ def _path_exists_in_tree_at_ref( try: result = subprocess.run( - [git_exe, "--git-dir", str(bare), "ls-tree", "FETCH_HEAD", vpath], + [git_exe, "--git-dir", str(bare), "ls-tree", "-d", "FETCH_HEAD", vpath], check=True, capture_output=True, text=True, @@ -816,7 +816,9 @@ def _path_exists_in_tree_at_ref( log(f" [x] ls-tree failed via {label}: {error}") return False - if output and output.strip(): + # ``-d`` also includes gitlinks (160000 commit), which cannot be + # materialized as plugin content without following another repository. + if output and output.split(maxsplit=2)[:2] == ["040000", "tree"]: log(f" [+] {vpath}@{ref} present in tree") return True log(f" [!] {vpath} not present in tree at {ref}") diff --git a/src/apm_cli/install/dry_run_plan.py b/src/apm_cli/install/dry_run_plan.py index 922e13fc9b..849b6952e9 100644 --- a/src/apm_cli/install/dry_run_plan.py +++ b/src/apm_cli/install/dry_run_plan.py @@ -6,6 +6,7 @@ from dataclasses import dataclass, replace from typing import Any +from apm_cli.install.package_selection import selected_dependency_identity from apm_cli.models.dependency.reference import DependencyReference from apm_cli.security.executables import filter_lsp_by_allow_executables @@ -41,7 +42,8 @@ def from_apm_package( selected_apm_dependencies = all_apm_dependencies if only_packages is not None: selected_identities = { - DependencyReference.parse(package).get_identity() for package in only_packages + selected_dependency_identity(package, all_apm_dependencies) + for package in only_packages } selected_apm_dependencies = tuple( dependency @@ -60,7 +62,8 @@ def from_apm_package( only_packages=tuple(only_packages) if only_packages is not None else None, lsp_dependencies=tuple(apm_package.get_lsp_dependencies()), updated_apm_identities=frozenset( - DependencyReference.parse(package).get_identity() for package in updated_packages + selected_dependency_identity(package, all_apm_dependencies) + for package in updated_packages ), ) diff --git a/src/apm_cli/install/package_selection.py b/src/apm_cli/install/package_selection.py index 82dec494a4..2857c9f9b7 100644 --- a/src/apm_cli/install/package_selection.py +++ b/src/apm_cli/install/package_selection.py @@ -2,16 +2,27 @@ from __future__ import annotations +from collections.abc import Iterable from typing import TYPE_CHECKING +from apm_cli.models.dependency.reference import DependencyReference + if TYPE_CHECKING: from apm_cli.core.command_logger import _ValidationOutcome +def selected_dependency_identity(package: str, dependencies: Iterable[DependencyReference]) -> str: + """Resolve a selector using interpreted references before shorthand parsing.""" + for dependency in dependencies: + # Explicit git+path coordinates can contain dotted bundle directories + # whose canonical display is not a valid shorthand file reference. + if dependency.to_canonical() == package: + return dependency.get_identity() + return DependencyReference.parse(package).get_identity() + + def existing_dependency_identities(current_dependencies: list[object]) -> set[str]: """Return canonical identities for every parseable manifest dependency.""" - from apm_cli.models.apm_package import DependencyReference - identities: set[str] = set() for entry in current_dependencies: try: diff --git a/src/apm_cli/install/phases/resolve.py b/src/apm_cli/install/phases/resolve.py index a89462eda0..4de8ede143 100644 --- a/src/apm_cli/install/phases/resolve.py +++ b/src/apm_cli/install/phases/resolve.py @@ -928,23 +928,22 @@ def _apply_only_filter(ctx: InstallContext) -> None: # ------------------------------------------------------------------ # 7. --only filtering # ------------------------------------------------------------------ - from apm_cli.models.apm_package import DependencyReference + from apm_cli.install.package_selection import selected_dependency_identity # Build identity set from user-supplied package specs. # Accepts any input form: git URLs, FQDN, shorthand. + tree = ctx.dependency_graph.dependency_tree + resolved_refs = [node.dependency_ref for node in tree.nodes.values()] only_identities: builtins.set = builtins.set() for p in ctx.only_packages: try: - ref = DependencyReference.parse(p) - only_identities.add(ref.get_identity()) + only_identities.add(selected_dependency_identity(p, resolved_refs)) except Exception: only_identities.add(p) # Expand the set to include transitive descendants of the # requested packages so their MCP servers, primitives, etc. # are correctly installed and written to the lockfile. - tree = ctx.dependency_graph.dependency_tree - def _collect_descendants(node: object, visited: builtins.set | None = None) -> None: """Walk the tree and add every child identity (cycle-safe).""" if visited is None: diff --git a/src/apm_cli/marketplace/resolver.py b/src/apm_cli/marketplace/resolver.py index dbbe00d6e4..a3ceda11d8 100644 --- a/src/apm_cli/marketplace/resolver.py +++ b/src/apm_cli/marketplace/resolver.py @@ -35,6 +35,7 @@ from ..deps.transport_selection import initial_transport_scheme from ..models.dependency.host_virtual import dependency_repository_owner from ..models.dependency.reference import DependencyReference +from ..models.validation import InvalidVirtualPackageExtensionError from ..utils.github_host import ( build_ado_ssh_url, build_ssh_url, @@ -975,15 +976,20 @@ def _emit_warning(msg: str) -> None: plugin_root=manifest.plugin_root, ) - if ( - dep_ref is None - and _source_needs_explicit_git_path(source) - and _is_in_marketplace_source(plugin, source) - ): + if dep_ref is None and _is_in_marketplace_source(plugin, source): in_repo_path, path_ref = _extract_in_repo_path_and_ref( plugin, plugin_root=manifest.plugin_root ) - if in_repo_path: + needs_explicit_path = _source_needs_explicit_git_path(source) + if in_repo_path and not needs_explicit_path: + try: + DependencyReference.parse(canonical) + except InvalidVirtualPackageExtensionError: + # The catalog already identifies the repository and path. A + # dotted bundle directory need not fit shorthand's file-name + # heuristic; the normal downloader still validates its content. + needs_explicit_path = True + if in_repo_path and needs_explicit_path: # Fall back to the marketplace's registered ref when the plugin # source itself declares no ref and no version_spec overrides it. # "main" / "HEAD" are excluded because they represent the default diff --git a/tests/integration/test_architecture_owner_rule_mutations.py b/tests/integration/test_architecture_owner_rule_mutations.py index dc1c320ae7..dbfb52288b 100644 --- a/tests/integration/test_architecture_owner_rule_mutations.py +++ b/tests/integration/test_architecture_owner_rule_mutations.py @@ -55,6 +55,33 @@ OWNERS_DIR = ROOT / ".apm/architecture/owners" +@pytest.mark.parametrize( + ("path", "old", "new"), + [ + ( + "src/apm_cli/install/phases/resolve.py", + "selected_dependency_identity(p, resolved_refs)", + "DependencyReference.parse(p).get_identity()", + ), + ( + "src/apm_cli/install/dry_run_plan.py", + "selected_dependency_identity(package, all_apm_dependencies)", + "DependencyReference.parse(package).get_identity()", + ), + ], +) +def test_package_selector_identity_cannot_bypass_interpreted_references( + path: str, old: str, new: str +) -> None: + """Install and preview cannot revert to lossy shorthand reparsing.""" + rule_id = "install-deployment-prospective-dry-run-plan" + source = (ROOT / path).read_text(encoding="utf-8") + assert old in source + report = run_selected_rules(ROOT, (rule_id,), source_overrides={path: source.replace(old, new)}) + assert report.failures == () + assert any(violation.rule_id == rule_id for violation in report.violations) + + @dataclass(frozen=True) class MutationCase: """One guard's minimal source mutation and the rule that must catch it. diff --git a/tests/integration/test_marketplace_versioned_directory_lifecycle.py b/tests/integration/test_marketplace_versioned_directory_lifecycle.py new file mode 100644 index 0000000000..778e6f2057 --- /dev/null +++ b/tests/integration/test_marketplace_versioned_directory_lifecycle.py @@ -0,0 +1,220 @@ +"""Pack-to-marketplace installation contracts for versioned bundle directories.""" + +from __future__ import annotations + +import json +import os +from pathlib import Path + +import pytest + +from apm_cli.utils.yaml_io import dump_yaml, load_yaml +from tests.utils.apm_lifecycle_runner import ApmLifecycleRunner +from tests.utils.isolated_apm_environment import IsolatedApmEnvironment +from tests.utils.local_git_repository import LocalGitRepositoryFactory +from tests.utils.local_package import LocalPackageFactory + +pytestmark = [pytest.mark.integration, pytest.mark.lifecycle_smoke] + +_REMOTE = "https://github.com/apm-fixtures/versioned-marketplace" +_SKILL = "---\nname: versioned-skill\ndescription: Versioned package fixture\n---\n# Skill\n" + + +def _runner(apm_engine_command: tuple[str, ...]) -> ApmLifecycleRunner: + # Substitute only catalog transport: the GitHub Contents API reads the same + # committed manifest through local Git. Registration, source classification, + # package download, validation, deployment, and lock replay remain real. + return ApmLifecycleRunner( + ( + apm_engine_command[0], + "-c", + "from apm_cli.marketplace import client\n" + "client._FETCHERS['github'] = client._fetch_git\n" + "from apm_cli.cli import cli\n" + "cli()\n", + ) + ) + + +@pytest.mark.parametrize( + ("version", "local"), [("1.2.3", False), ("2026.9.3", False), ("1.2.3", True)] +) +def test_pack_bundle_installs_from_versioned_marketplace_directory( + tmp_path: Path, + apm_engine_command: tuple[str, ...], + version: str, + local: bool, +) -> None: + """A pack-generated dotted directory survives Git resolution and lock replay.""" + isolated = IsolatedApmEnvironment.create(tmp_path / "scenario", base_env=dict(os.environ)) + environment = isolated.subprocess_env() + runner = _runner(apm_engine_command) + packages = LocalPackageFactory(isolated.package_root) + producer = packages.create("my-plugin", version=version, targets=("claude",)) + packages.add_skill(producer, "versioned-skill", _SKILL) + manifest = load_yaml(producer.manifest_path) + manifest.update({"dependencies": {"apm": []}, "includes": ["skills"]}) + dump_yaml(manifest, producer.manifest_path) + marketplace = isolated.work_root / "marketplace" + runner.run_sequence( + ( + ("install", "--no-policy"), + ("pack", "--output", str(marketplace / "plugins")), + ), + expected_returncodes=(0, 0), + scenario_id="versioned-marketplace-producer", + cwd=producer.root, + env=environment, + ) + virtual_path = f"plugins/my-plugin-{version}" + bundle = marketplace / virtual_path + assert (bundle / "plugin.json").is_file() + (marketplace / ".claude-plugin").mkdir() + (marketplace / ".claude-plugin" / "marketplace.json").write_text( + json.dumps( + { + "name": "versioned-marketplace", + "owner": {"name": "APM Tests"}, + "plugins": [{"name": "my-plugin", "source": f"./{virtual_path}"}], + } + ), + encoding="utf-8", + ) + repositories = LocalGitRepositoryFactory(isolated.repository_root, env=environment) + repository = repositories.create("versioned-marketplace", source_tree=marketplace) + commit = repositories.commit(repository, message="seed packed versioned marketplace") + environment = repositories.url_rewrite_subprocess_env(repository, _REMOTE) + consumer = LocalPackageFactory(isolated.work_root).create("consumer", targets=("claude",)) + runner.run_sequence( + ( + ( + "marketplace", + "add", + str(marketplace) if local else _REMOTE, + "--name", + "versioned-marketplace", + "--ref", + commit.sha, + ), + ), + expected_returncodes=(0,), + scenario_id="versioned-marketplace-consumer", + cwd=consumer.root, + env=environment, + ) + install_args = ( + "install", + "my-plugin@versioned-marketplace", + "--target", + "claude", + "--no-policy", + ) + before_manifest = consumer.manifest_path.read_bytes() + preview = runner.run( + (*install_args, "--dry-run"), + scenario_id="versioned-marketplace-preview", + cwd=consumer.root, + env=environment, + ) + assert preview.returncode == 0, preview.stdout + preview.stderr + assert consumer.manifest_path.read_bytes() == before_manifest + assert not (consumer.root / "apm.lock.yaml").exists() + assert not (consumer.root / ".claude" / "skills").exists() + installed = runner.run( + install_args, + scenario_id="versioned-marketplace-install", + cwd=consumer.root, + env=environment, + ) + assert installed.returncode == 0, installed.stdout + installed.stderr + assert (consumer.root / ".claude" / "skills" / "versioned-skill" / "SKILL.md").read_text( + encoding="utf-8" + ) == _SKILL + locked = load_yaml(consumer.root / "apm.lock.yaml")["dependencies"] + assert len(locked) == 1 + if not local: + assert locked[0]["virtual_path"] == virtual_path + assert locked[0]["resolved_commit"] == commit.sha + replay = runner.run( + ("install", "--no-policy"), + scenario_id="versioned-marketplace-replay", + cwd=consumer.root, + env=environment, + ) + assert replay.returncode == 0, replay.stdout + replay.stderr + assert load_yaml(consumer.root / "apm.lock.yaml")["dependencies"] == locked + + +@pytest.mark.parametrize( + "name", ["unknown", "versioned-file", "invalid", "invalid-unversioned", "traversal"] +) +def test_git_marketplace_rejects_unknown_files_and_invalid_versioned_packages( + tmp_path: Path, + apm_engine_command: tuple[str, ...], + name: str, +) -> None: + """Explicit paths cannot turn arbitrary files or invalid content into packages.""" + isolated = IsolatedApmEnvironment.create(tmp_path / "scenario", base_env=dict(os.environ)) + environment = isolated.subprocess_env() + runner = _runner(apm_engine_command) + repositories = LocalGitRepositoryFactory(isolated.repository_root, env=environment) + repository = repositories.create("invalid-marketplace") + plugins = repository.worktree / "plugins" + plugins.mkdir() + (plugins / "unknown.txt").write_text("Not an APM package", encoding="utf-8") + (plugins / "file-1.2.3").write_text("Not a directory", encoding="utf-8") + invalid = plugins / "invalid-1.2.3" + invalid.mkdir() + (invalid / "plugin.json").write_text("{not valid JSON", encoding="utf-8") + unversioned = plugins / "invalid-package" + unversioned.mkdir() + (unversioned / "plugin.json").write_text("{not valid JSON", encoding="utf-8") + entries = { + "unknown": "./plugins/unknown.txt", + "versioned-file": "./plugins/file-1.2.3", + "invalid": "./plugins/invalid-1.2.3", + "invalid-unversioned": "./plugins/invalid-package", + "traversal": "./plugins/../invalid-1.2.3", + } + (repository.worktree / "marketplace.json").write_text( + json.dumps( + { + "name": "invalid-marketplace", + "owner": {"name": "APM Tests"}, + "plugins": [{"name": name, "source": path} for name, path in entries.items()], + } + ), + encoding="utf-8", + ) + commit = repositories.commit(repository, message="seed invalid marketplace packages") + environment = repositories.url_rewrite_subprocess_env(repository, _REMOTE) + consumers = LocalPackageFactory(isolated.work_root) + consumer = consumers.create(f"consumer-{name}", targets=("claude",)) + before_manifest = consumer.manifest_path.read_bytes() + runner.run_sequence( + ( + ( + "marketplace", + "add", + _REMOTE, + "--name", + "invalid-marketplace", + "--ref", + commit.sha, + ), + ), + expected_returncodes=(0,), + scenario_id=f"invalid-marketplace-{name}-register", + cwd=consumer.root, + env=environment, + ) + result = runner.run( + ("install", f"{name}@invalid-marketplace", "--no-policy"), + scenario_id=f"invalid-marketplace-{name}-install", + cwd=consumer.root, + env=environment, + ) + assert result.returncode != 0, result.stdout + result.stderr + assert consumer.manifest_path.read_bytes() == before_manifest + assert not (consumer.root / "apm.lock.yaml").exists() + assert not (consumer.root / ".claude" / "skills").exists() diff --git a/tests/unit/deps/test_github_downloader_validation.py b/tests/unit/deps/test_github_downloader_validation.py index 4d8c0acf2b..f61a4a9593 100644 --- a/tests/unit/deps/test_github_downloader_validation.py +++ b/tests/unit/deps/test_github_downloader_validation.py @@ -561,7 +561,7 @@ def test_safe_rmtree_called_not_robust_rmtree_direct(self) -> None: dep_ref = _make_subdir_dep(vpath="skills/x", ref="main") winning = gdv.AttemptSpec("plain HTTPS w/ credential helper", "https://x", {}) - completed = MagicMock(stdout="100644 blob abc\tskills/x") + completed = MagicMock(stdout="040000 tree abc\tskills/x") with ( patch("apm_cli.deps.github_downloader_validation.safe_rmtree") as safe_rm_mock, patch( diff --git a/tests/unit/deps/test_versioned_directory_probe.py b/tests/unit/deps/test_versioned_directory_probe.py new file mode 100644 index 0000000000..fd173f00b3 --- /dev/null +++ b/tests/unit/deps/test_versioned_directory_probe.py @@ -0,0 +1,72 @@ +"""Real Git tree probes distinguish directories from files and symbolic links.""" + +import os +import subprocess +from pathlib import Path + +import pytest + +from apm_cli.deps import github_downloader_validation as validation +from apm_cli.deps.github_downloader import GitHubPackageDownloader +from apm_cli.models.dependency.reference import DependencyReference +from tests.utils.isolated_apm_environment import IsolatedApmEnvironment +from tests.utils.local_git_repository import LocalGitRepositoryFactory + + +@pytest.mark.parametrize( + ("path", "expected"), + [ + ("directory-1.2.3", True), + ("unknown.txt", False), + ("file-1.2.3", False), + ("link-1.2.3", False), + ("submodule-1.2.3", False), + ], +) +def test_subdirectory_probe_requires_a_git_tree( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, path: str, expected: bool +) -> None: + isolated = IsolatedApmEnvironment.create(tmp_path / "scenario", base_env=dict(os.environ)) + environment = isolated.subprocess_env() + repositories = LocalGitRepositoryFactory(isolated.repository_root, env=environment) + repository = repositories.create("versioned-paths") + directory = repository.worktree / "directory-1.2.3" + directory.mkdir() + (directory / "README.md").write_text("A directory", encoding="utf-8") + for filename in ("unknown.txt", "file-1.2.3"): + (repository.worktree / filename).write_text("A file", encoding="utf-8") + if path == "link-1.2.3": + try: + (repository.worktree / path).symlink_to("directory-1.2.3", target_is_directory=True) + except OSError: + pytest.skip("Symbolic links are unavailable on this platform") + commit = repositories.commit(repository, message="seed directory and file entries") + if path == "submodule-1.2.3": + (repository.worktree / path).mkdir() + subprocess.run( + [ + "git", + "-C", + str(repository.worktree), + "update-index", + "--add", + "--cacheinfo", + f"160000,{commit.sha},{path}", + ], + check=True, + capture_output=True, + env=environment, + ) + commit = repositories.commit(repository, message="seed gitlink entry") + monkeypatch.setattr(validation, "get_apm_temp_dir", lambda: isolated.temp_root) + dependency = DependencyReference.parse_from_dict( + {"git": "https://github.com/acme/catalog", "path": path, "ref": commit.sha} + ) + attempt = validation.AttemptSpec("local fixture", repository.file_url, environment) + + assert ( + validation._path_exists_in_tree_at_ref( + GitHubPackageDownloader(), dependency, path, commit.sha, lambda _message: None, attempt + ) + is expected + ) diff --git a/tests/unit/install/phases/test_resolve_selective_update_plan.py b/tests/unit/install/phases/test_resolve_selective_update_plan.py index d8c0af169e..1bf4097f8e 100644 --- a/tests/unit/install/phases/test_resolve_selective_update_plan.py +++ b/tests/unit/install/phases/test_resolve_selective_update_plan.py @@ -47,3 +47,26 @@ def test_run_records_complete_keys_before_only_filter(monkeypatch): } assert ctx.deps_to_install == [selected] assert ctx.intended_dep_keys == {selected.get_unique_key()} + + +def test_only_filter_preserves_structured_versioned_path_and_descendants(): + selected = DependencyReference.parse_from_dict( + {"git": "https://github.com/acme/catalog", "path": "plugins/tool-1.2.3", "ref": "release"} + ) + child = DependencyReference(repo_url="acme/child") + unselected = DependencyReference(repo_url="acme/other") + child_node = SimpleNamespace(dependency_ref=child, children=[]) + nodes = { + selected.get_unique_key(): SimpleNamespace(dependency_ref=selected, children=[child_node]), + child.get_unique_key(): child_node, + unselected.get_unique_key(): SimpleNamespace(dependency_ref=unselected, children=[]), + } + ctx = SimpleNamespace( + only_packages=["acme/catalog/plugins/tool-1.2.3#release"], + deps_to_install=[selected, child, unselected], + dependency_graph=SimpleNamespace(dependency_tree=SimpleNamespace(nodes=nodes)), + ) + + resolve._apply_only_filter(ctx) + + assert ctx.deps_to_install == [selected, child] diff --git a/tests/unit/install/test_dry_run_render.py b/tests/unit/install/test_dry_run_render.py index 4841c94837..e04f9282c4 100644 --- a/tests/unit/install/test_dry_run_render.py +++ b/tests/unit/install/test_dry_run_render.py @@ -121,6 +121,31 @@ def test_plan_selects_only_requested_dependency_without_reparsing_it(self) -> No assert plan.selected_apm_dependencies[0].source == "registry" assert plan.selected_apm_dependencies[0].registry_name == "private" + def test_plan_retains_structured_versioned_path_for_selection_and_updates(self) -> None: + requested = DependencyReference.parse_from_dict( + { + "git": "https://github.com/acme/catalog", + "path": "plugins/tool-1.2.3", + "ref": "release", + } + ) + package = MagicMock() + package.get_apm_dependencies.return_value = [requested] + package.get_dev_apm_dependencies.return_value = [] + package.get_all_mcp_dependencies.return_value = [] + package.get_lsp_dependencies.return_value = [] + + plan = ProspectiveInstallPlan.from_apm_package( + package, + should_install_apm=True, + should_install_mcp=False, + only_packages=["acme/catalog/plugins/tool-1.2.3#release"], + updated_packages=["acme/catalog/plugins/tool-1.2.3#release"], + ) + + assert plan.selected_apm_dependencies == (requested,) + assert plan.updated_apm_identities == {"acme/catalog/plugins/tool-1.2.3"} + def test_plan_excludes_apm_selection_when_only_mcp_is_requested(self) -> None: """The prospective plan does not leak APM work into --only=mcp previews.""" dep = DependencyReference.parse("owner/repo#main") diff --git a/tests/unit/marketplace/test_versioned_directory_resolution.py b/tests/unit/marketplace/test_versioned_directory_resolution.py new file mode 100644 index 0000000000..a12e73adb0 --- /dev/null +++ b/tests/unit/marketplace/test_versioned_directory_resolution.py @@ -0,0 +1,63 @@ +"""Marketplace paths retain their explicit repository boundary despite dots.""" + +from unittest.mock import patch + +import pytest + +from apm_cli.install.package_resolution import dependency_reference_to_yaml_entry +from apm_cli.marketplace.models import MarketplaceManifest, MarketplacePlugin, MarketplaceSource +from apm_cli.marketplace.resolver import resolve_marketplace_plugin +from apm_cli.models.dependency.reference import DependencyReference +from apm_cli.models.dependency.types import VirtualPackageType + + +@pytest.mark.parametrize("host", ["github.com", "corp.ghe.com"]) +@pytest.mark.parametrize( + ("path", "expected_type"), + [ + ("plugins/my-plugin-1.2.3", VirtualPackageType.SUBDIRECTORY), + ("collections/my-plugin-2026.9.3", VirtualPackageType.SUBDIRECTORY), + ("plugins/my-plugin-1.2.3-rc.1", VirtualPackageType.SUBDIRECTORY), + ("plugins/my-plugin", VirtualPackageType.SUBDIRECTORY), + ("prompts/review.prompt.md", VirtualPackageType.FILE), + ("instructions/review.instructions.md", VirtualPackageType.FILE), + ("agents/review.agent.md", VirtualPackageType.FILE), + ], +) +def test_marketplace_path_classification_and_manifest_round_trip( + host: str, path: str, expected_type: VirtualPackageType +) -> None: + source = MarketplaceSource(name="catalog", url=f"https://{host}/acme/catalog", ref="release") + plugin = MarketplacePlugin(name="plugin", source=f"./{path}") + manifest = MarketplaceManifest(name="catalog", plugins=(plugin,)) + with ( + patch("apm_cli.marketplace.resolver.get_marketplace_by_name", return_value=source), + patch("apm_cli.marketplace.resolver.fetch_or_cache", return_value=manifest), + ): + resolution = resolve_marketplace_plugin("plugin", "catalog") + + dep = resolution.dependency_reference or DependencyReference.parse(resolution.canonical) + assert (dep.host, dep.repo_url, dep.virtual_path, dep.reference) == ( + host, + "acme/catalog", + path, + "release", + ) + assert dep.virtual_type == expected_type + entry = dependency_reference_to_yaml_entry(dep) + replayed = DependencyReference.parse_from_dict(entry) + assert replayed.virtual_path == path + assert replayed.virtual_type == expected_type + + +@pytest.mark.parametrize("path", ["../my-plugin-1.2.3", "plugins/../my-plugin-1.2.3"]) +def test_versioned_marketplace_paths_reject_traversal(path: str) -> None: + source = MarketplaceSource(name="catalog", url="https://github.com/acme/catalog") + plugin = MarketplacePlugin(name="plugin", source=path) + manifest = MarketplaceManifest(name="catalog", plugins=(plugin,)) + with ( + patch("apm_cli.marketplace.resolver.get_marketplace_by_name", return_value=source), + patch("apm_cli.marketplace.resolver.fetch_or_cache", return_value=manifest), + pytest.raises(ValueError, match="traversal"), + ): + resolve_marketplace_plugin("plugin", "catalog") From b9b01e0064e6aa530e8b4ff6ddcf3ef898f82e93 Mon Sep 17 00:00:00 2001 From: Pybsama <294532706+Pybsama@users.noreply.github.com> Date: Sat, 3 Oct 2026 13:58:19 +0800 Subject: [PATCH 2/4] docs: explain versioned marketplace directory installs --- .../content/docs/consumer/installing-from-marketplaces.md | 6 ++++++ packages/apm-guide/.apm/skills/apm-usage/commands.md | 5 +++++ 2 files changed, 11 insertions(+) diff --git a/docs/src/content/docs/consumer/installing-from-marketplaces.md b/docs/src/content/docs/consumer/installing-from-marketplaces.md index 361b2c2ca0..5d25ca944e 100644 --- a/docs/src/content/docs/consumer/installing-from-marketplaces.md +++ b/docs/src/content/docs/consumer/installing-from-marketplaces.md @@ -112,6 +112,12 @@ identity; bare cross-repository entries retain their normal dependency defaults. Invalid URLs and unsafe subdirectory paths fail before manifest, lockfile, or deployment writes. +Catalog paths can point to versioned directories generated by `apm pack`, such +as `plugins/my-plugin-1.2.3`. APM keeps their repository/path coordinates through +installation and `--dry-run` instead of treating the version suffix as a file +extension. Directory content and path checks still apply; arbitrary files, +symbolic links, and Git submodules are not accepted as directory packages. + Use `apm install pkg@catalog#v1.0.1` for a literal Git tag. For a range, declare the marketplace dependency in `apm.yml`: diff --git a/packages/apm-guide/.apm/skills/apm-usage/commands.md b/packages/apm-guide/.apm/skills/apm-usage/commands.md index f8055348c3..6e47294911 100644 --- a/packages/apm-guide/.apm/skills/apm-usage/commands.md +++ b/packages/apm-guide/.apm/skills/apm-usage/commands.md @@ -90,6 +90,11 @@ lowercase before constructing the package-registry path. ### Install validation chain (virtual subdirectory packages) +Marketplace installation supports versioned directories produced by `apm pack`, +such as `plugins/my-plugin-1.2.3`, including positional `--dry-run` previews. +Directory content and path validation still apply; arbitrary files, symbolic +links, and Git submodules are not treated as directory packages. + `apm install` validates subdirectory packages (`owner/repo/path#ref`) before writing to `apm.yml` using the same credential chain as the actual install. Git-source semver ranges (for example, `owner/repo/path#^1.2.0`) defer raw-ref validation to semver tag resolution; registry-routed dependencies retain registry version validation. See [Authentication > Install validation chain](../authentication/) for the full probe sequence and troubleshooting. When a default registry is configured, plain shorthand deps (`owner/repo#`) bypass the GitHub probe. `apm install` requires a version selector before writing to `apm.yml`; deps with no `#` at all are rejected. Semver selectors (`1.0.0`, `^1.2.3`) use range matching; non-semver selectors (`stable`, `v1.4.2`, any opaque label) are matched exactly against the registry's published versions. From cd5e8d37738cb3e76efd9277f94b8651a7303a6a Mon Sep 17 00:00:00 2001 From: Pybsama <294532706+Pybsama@users.noreply.github.com> Date: Sat, 3 Oct 2026 13:59:14 +0800 Subject: [PATCH 3/4] docs: scope directory probe guidance to Git sources --- .../src/content/docs/consumer/installing-from-marketplaces.md | 4 ++-- packages/apm-guide/.apm/skills/apm-usage/commands.md | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/src/content/docs/consumer/installing-from-marketplaces.md b/docs/src/content/docs/consumer/installing-from-marketplaces.md index 5d25ca944e..6486675d55 100644 --- a/docs/src/content/docs/consumer/installing-from-marketplaces.md +++ b/docs/src/content/docs/consumer/installing-from-marketplaces.md @@ -115,8 +115,8 @@ paths fail before manifest, lockfile, or deployment writes. Catalog paths can point to versioned directories generated by `apm pack`, such as `plugins/my-plugin-1.2.3`. APM keeps their repository/path coordinates through installation and `--dry-run` instead of treating the version suffix as a file -extension. Directory content and path checks still apply; arbitrary files, -symbolic links, and Git submodules are not accepted as directory packages. +extension. Directory content and path checks still apply. Git directory probes +reject files, symbolic links, and submodules. Use `apm install pkg@catalog#v1.0.1` for a literal Git tag. For a range, declare the marketplace dependency in `apm.yml`: diff --git a/packages/apm-guide/.apm/skills/apm-usage/commands.md b/packages/apm-guide/.apm/skills/apm-usage/commands.md index 6e47294911..b2c4e5285f 100644 --- a/packages/apm-guide/.apm/skills/apm-usage/commands.md +++ b/packages/apm-guide/.apm/skills/apm-usage/commands.md @@ -92,8 +92,8 @@ lowercase before constructing the package-registry path. Marketplace installation supports versioned directories produced by `apm pack`, such as `plugins/my-plugin-1.2.3`, including positional `--dry-run` previews. -Directory content and path validation still apply; arbitrary files, symbolic -links, and Git submodules are not treated as directory packages. +Directory content and path validation still apply. Git directory probes reject +files, symbolic links, and submodules. `apm install` validates subdirectory packages (`owner/repo/path#ref`) before writing to `apm.yml` using the same credential chain as the actual install. Git-source semver ranges (for example, `owner/repo/path#^1.2.0`) defer raw-ref validation to semver tag resolution; registry-routed dependencies retain registry version validation. See [Authentication > Install validation chain](../authentication/) for the full probe sequence and troubleshooting. From 3fe2734e372cf0450279e4f173cb8baaa49df5cf Mon Sep 17 00:00:00 2001 From: Pybsama <294532706+Pybsama@users.noreply.github.com> Date: Sat, 3 Oct 2026 14:12:28 +0800 Subject: [PATCH 4/4] fix: validate GitHub directory response shapes before fallback --- CHANGELOG.md | 4 ++ .../deps/github_downloader_validation.py | 17 +++++- .../test_integrators_hooks_execution.py | 2 + .../test_integrators_hooks_phase3c.py | 2 + ...test_github_downloader_input_validation.py | 57 +++++++++++++++++++ ...est_github_downloader_validation_phase3.py | 2 + .../deps/test_versioned_directory_probe.py | 25 +++++--- .../test_resolve_selective_update_plan.py | 2 +- 8 files changed, 100 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8c1a995f25..cae3daab3b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- Git marketplace installs accept versioned plugin directories while rejecting file, symlink, and submodule responses during directory validation. (by @Pybsama, #3156) + ## [0.33.0] - 2026-10-02 ### Added diff --git a/src/apm_cli/deps/github_downloader_validation.py b/src/apm_cli/deps/github_downloader_validation.py index d76443fc8b..c57cdbfcb6 100644 --- a/src/apm_cli/deps/github_downloader_validation.py +++ b/src/apm_cli/deps/github_downloader_validation.py @@ -238,10 +238,11 @@ def _directory_exists_at_ref( Uses the default ``Accept: application/vnd.github+json`` so the endpoint returns the directory listing for directories (and file - metadata for files). A 200 means the path resolves at the ref, - which is what install needs. + metadata for files). Only an array of entry objects confirms a + directory; file, symlink, and submodule objects do not. - Returns ``True`` on 200; ``False`` on 404 or any error. Only + Returns ``True`` for a directory listing, including an empty array; + ``False`` for non-directory content, 404, or any error. Only implemented for github.com / GHE; non-GitHub hosts return ``False`` and rely on the marker-file probes above. """ @@ -287,6 +288,16 @@ def _request(token: str | None, _git_env: dict[str, str]) -> bool: ) raise_for_github_throttle(response, host) if response.status_code == 200: + try: + entries = response.json() + except ValueError: + log(f" [x] {path}@{ref} (invalid Contents API JSON)") + return False + if not isinstance(entries, list) or not all( + isinstance(entry, dict) for entry in entries + ): + log(f" [!] {path}@{ref} (not a directory listing)") + return False log(f" [+] {path}@{ref} (directory)") return True response.raise_for_status() diff --git a/tests/integration/test_integrators_hooks_execution.py b/tests/integration/test_integrators_hooks_execution.py index 1006b3081e..fb54f953a5 100644 --- a/tests/integration/test_integrators_hooks_execution.py +++ b/tests/integration/test_integrators_hooks_execution.py @@ -330,6 +330,7 @@ def test_github_200_returns_true(self) -> None: mock_resp = MagicMock() mock_resp.status_code = 200 + mock_resp.json.return_value = [] with patch.object(downloader, "_resilient_get", return_value=mock_resp): result = _directory_exists_at_ref(downloader, dep, "skills/foo", "main", lambda m: None) @@ -372,6 +373,7 @@ def test_no_token_still_makes_request(self) -> None: mock_resp = MagicMock() mock_resp.status_code = 200 + mock_resp.json.return_value = [] with patch.object(downloader, "_resilient_get", return_value=mock_resp) as mock_get: result = _directory_exists_at_ref(downloader, dep, "path", "ref", lambda m: None) diff --git a/tests/integration/test_integrators_hooks_phase3c.py b/tests/integration/test_integrators_hooks_phase3c.py index 272d01c3d6..a31548dc49 100644 --- a/tests/integration/test_integrators_hooks_phase3c.py +++ b/tests/integration/test_integrators_hooks_phase3c.py @@ -330,6 +330,7 @@ def test_github_200_returns_true(self) -> None: mock_resp = MagicMock() mock_resp.status_code = 200 + mock_resp.json.return_value = [] with patch.object(downloader, "_resilient_get", return_value=mock_resp): result = _directory_exists_at_ref(downloader, dep, "skills/foo", "main", lambda m: None) @@ -372,6 +373,7 @@ def test_no_token_still_makes_request(self) -> None: mock_resp = MagicMock() mock_resp.status_code = 200 + mock_resp.json.return_value = [] with patch.object(downloader, "_resilient_get", return_value=mock_resp) as mock_get: result = _directory_exists_at_ref(downloader, dep, "path", "ref", lambda m: None) diff --git a/tests/unit/deps/test_github_downloader_input_validation.py b/tests/unit/deps/test_github_downloader_input_validation.py index 71f08ec826..7e87c2d079 100644 --- a/tests/unit/deps/test_github_downloader_input_validation.py +++ b/tests/unit/deps/test_github_downloader_input_validation.py @@ -34,6 +34,7 @@ from unittest.mock import MagicMock, patch import pytest +import requests from apm_cli.core.auth import AuthResolver from apm_cli.deps import github_downloader_validation as gdv @@ -236,6 +237,60 @@ class TestDirectoryExistsAtRef: def _log(self, _msg: str) -> None: pass + @pytest.mark.parametrize("host", ["github.com", "corp.ghe.com"]) + @pytest.mark.parametrize("ref", [None, "main"]) + @pytest.mark.parametrize( + ("body", "expected"), + [ + (b'[{"type":"file","name":"plugin.json"},{"type":"symlink","name":"link"}]', True), + (b"[]", True), + (b'{"type":"file","name":"plugin-1.2.3"}', False), + (b'{"type":"symlink","target":"plugin-1.2.3"}', False), + (b'{"type":"submodule","submodule_git_url":"https://github.com/acme/other"}', False), + (b'{"type":"file","submodule_git_url":"https://github.com/acme/other"}', False), + (b'{"type":"dir","entries":[]}', False), + (b"null", False), + (b'"directory"', False), + (b'["not an entry object"]', False), + (b"not json", False), + ], + ids=[ + "directory-listing", + "empty-directory", + "file-or-symlink-to-file", + "symlink", + "submodule", + "legacy-submodule", + "unexpected-object-media-type", + "null", + "string", + "invalid-listing-entry", + "invalid-json", + ], + ) + def test_validation_requires_directory_listing_response( + self, host: str, ref: str | None, body: bytes, expected: bool + ) -> None: + """The normal validation entrypoint must inspect a successful API payload.""" + downloader = _make_downloader(host=host) + dependency = DependencyReference.parse_from_dict( + {"git": f"https://{host}/acme/catalog", "path": "plugins/tool-1.2.3", "ref": ref} + ) + response = requests.Response() + response.status_code = 200 + response._content = body + with ( + patch.object(downloader, "download_raw_file", side_effect=RuntimeError("404")), + patch.object(downloader, "_resilient_get", return_value=response) as api_get, + patch.object(gdv, "_ref_exists_via_ls_remote", return_value=(False, None)) as fallback, + ): + result = downloader.validate_virtual_package_exists(dependency) + + assert result is expected + assert fallback.called is (not expected and ref is not None) + assert api_get.call_count == 1 + assert api_get.call_args.kwargs["headers"]["Accept"] == "application/vnd.github+json" + def test_azure_devops_returns_false_without_probe(self) -> None: dl = _make_downloader() dep = _make_github_dep() @@ -265,6 +320,7 @@ def test_github_com_200_returns_true(self) -> None: resp = MagicMock() resp.status_code = 200 + resp.json.return_value = [] with patch.object(dl, "_resilient_get", return_value=resp): result = _directory_exists_at_ref(dl, dep, "skills/foo", "main", self._log) @@ -320,6 +376,7 @@ def _capture(url, **kwargs): captured_urls.append(url) resp = MagicMock() resp.status_code = 200 + resp.json.return_value = [] return resp with patch.object(dl, "_resilient_get", side_effect=_capture): diff --git a/tests/unit/deps/test_github_downloader_validation_phase3.py b/tests/unit/deps/test_github_downloader_validation_phase3.py index 58d5a7c3c9..889d22b40d 100644 --- a/tests/unit/deps/test_github_downloader_validation_phase3.py +++ b/tests/unit/deps/test_github_downloader_validation_phase3.py @@ -277,6 +277,7 @@ def test_github_com_200_returns_true(self) -> None: resp = MagicMock() resp.status_code = 200 + resp.json.return_value = [] with patch.object(dl, "_resilient_get", return_value=resp): result = _directory_exists_at_ref(dl, dep, "skills/foo", "main", self._log) @@ -332,6 +333,7 @@ def _capture(url, **kwargs): captured_urls.append(url) resp = MagicMock() resp.status_code = 200 + resp.json.return_value = [] return resp with patch.object(dl, "_resilient_get", side_effect=_capture): diff --git a/tests/unit/deps/test_versioned_directory_probe.py b/tests/unit/deps/test_versioned_directory_probe.py index fd173f00b3..34a71ef136 100644 --- a/tests/unit/deps/test_versioned_directory_probe.py +++ b/tests/unit/deps/test_versioned_directory_probe.py @@ -3,8 +3,10 @@ import os import subprocess from pathlib import Path +from unittest.mock import MagicMock, patch import pytest +import requests from apm_cli.deps import github_downloader_validation as validation from apm_cli.deps.github_downloader import GitHubPackageDownloader @@ -23,8 +25,9 @@ ("submodule-1.2.3", False), ], ) +@pytest.mark.parametrize("api_body", [b'{"type":"file"}', b"invalid json"]) def test_subdirectory_probe_requires_a_git_tree( - tmp_path: Path, monkeypatch: pytest.MonkeyPatch, path: str, expected: bool + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, path: str, expected: bool, api_body: bytes ) -> None: isolated = IsolatedApmEnvironment.create(tmp_path / "scenario", base_env=dict(os.environ)) environment = isolated.subprocess_env() @@ -63,10 +66,18 @@ def test_subdirectory_probe_requires_a_git_tree( {"git": "https://github.com/acme/catalog", "path": path, "ref": commit.sha} ) attempt = validation.AttemptSpec("local fixture", repository.file_url, environment) - - assert ( - validation._path_exists_in_tree_at_ref( - GitHubPackageDownloader(), dependency, path, commit.sha, lambda _message: None, attempt - ) - is expected + downloader = GitHubPackageDownloader() + downloader.auth_resolver = MagicMock() + downloader.auth_resolver.uses_public_github_anonymous_first.return_value = False + downloader.auth_resolver.resolve_for_dep.return_value = MagicMock( + token=None, git_env=environment ) + response = requests.Response() + response.status_code = 200 + response._content = api_body + with ( + patch.object(downloader, "download_raw_file", side_effect=RuntimeError("404")), + patch.object(downloader, "_resilient_get", return_value=response), + patch.object(validation, "_build_validation_attempts", return_value=[attempt]), + ): + assert downloader.validate_virtual_package_exists(dependency) is expected diff --git a/tests/unit/install/phases/test_resolve_selective_update_plan.py b/tests/unit/install/phases/test_resolve_selective_update_plan.py index 1bf4097f8e..b6523929d5 100644 --- a/tests/unit/install/phases/test_resolve_selective_update_plan.py +++ b/tests/unit/install/phases/test_resolve_selective_update_plan.py @@ -49,7 +49,7 @@ def test_run_records_complete_keys_before_only_filter(monkeypatch): assert ctx.intended_dep_keys == {selected.get_unique_key()} -def test_only_filter_preserves_structured_versioned_path_and_descendants(): +def test_only_filter_preserves_structured_versioned_path_and_descendants() -> None: selected = DependencyReference.parse_from_dict( {"git": "https://github.com/acme/catalog", "path": "plugins/tool-1.2.3", "ref": "release"} )