diff --git a/docs/src/content/docs/consumer/manage-dependencies.md b/docs/src/content/docs/consumer/manage-dependencies.md index b5a3464e2..23417b99b 100644 --- a/docs/src/content/docs/consumer/manage-dependencies.md +++ b/docs/src/content/docs/consumer/manage-dependencies.md @@ -54,8 +54,6 @@ parser. The supported forms: |---|---|---| | GitHub shorthand | `owner/repo` | Public GitHub repo, latest default branch. | | Pinned ref | `owner/repo#v1.0.0` | Pin to a tag, branch, or full commit SHA. | -| Aliased | `owner/repo@my-alias` | Install under a custom directory name. | -| Pinned + aliased | `owner/repo#v1.0.0@my-alias` | Combine the two. | | FQDN shorthand | `gitlab.com/acme/repo#v2.0` | Any git host, not just github.com. | | Virtual subdirectory | `owner/repo/skills/review` | Install one skill folder from a monorepo. | | Virtual file | `owner/repo/prompts/review.prompt.md` | Install a single primitive file. | @@ -63,7 +61,7 @@ parser. The supported forms: | SSH SCP-style | `git@gitlab.com:acme/repo.git` | SSH with default port. | | SSH protocol | `ssh://git@gitlab.com/acme/repo.git` | SSH with explicit scheme or port. | | Local path | `./packages/shared` or `/abs/path` | Sibling package on disk. | -| Object form | `{ git: , path: , ref: }` | Escape hatch for nested groups, monorepo subpaths, or aliases that the string forms cannot express. | +| Object form | `{ git: , path: , ref: , alias: }` | Custom directory name (`alias`), nested groups, monorepo subpaths, or anything the string forms cannot express. | Object form in YAML: diff --git a/src/apm_cli/models/dependency/reference.py b/src/apm_cli/models/dependency/reference.py index 52c65a27a..7a25b6907 100644 --- a/src/apm_cli/models/dependency/reference.py +++ b/src/apm_cli/models/dependency/reference.py @@ -388,6 +388,32 @@ def get_install_path(self, apm_modules_dir: Path) -> Path: ensure_path_within(result, apm_modules_dir) return result + @staticmethod + def _reject_shorthand_alias(dependency_str: str) -> None: + """Reject bare-shorthand ``@alias`` with an actionable migration error. + + Bare ``@alias`` is not part of the supported reference grammar (#340 + retired the ``@`` separator to avoid the npm/go/cargo ``@version`` + collision). The dedicated SSH parsers extract ``@alias`` from + ``ssh://`` URLs and SCP shorthand (``@host:path``); this guard + fires for the remaining cases like ``owner/repo[/sub][#ref]@alias``, + which would otherwise silently leak the alias into ``virtual_path`` + or ``reference``. + """ + stripped = dependency_str.strip() + if "@" not in stripped: + return + if stripped.lower().startswith(("https://", "http://", "ssh://")): + return + if SCP_LIKE_RE.match(stripped): + return + raise ValueError( + f"Shorthand '@alias' is not supported in '{dependency_str}'. " + f"Use the object form with an 'alias:' field to install a " + f"dependency under a custom directory name. " + f"See: https://microsoft.github.io/apm/consumer/manage-dependencies/#reference-formats" + ) + @staticmethod def _parse_ssh_protocol_url(url: str): """Parse an ``ssh://`` protocol URL using ``urllib.parse.urlparse``. @@ -1348,8 +1374,6 @@ def parse(cls, dependency_str: str) -> "DependencyReference": - user/repo#v1.0.0 - user/repo#commit_sha - github.com/user/repo#ref - - user/repo@alias - - user/repo#ref@alias - user/repo/path/to/file.prompt.md (virtual file package) - user/repo/skills/foo (virtual subdirectory package) - user/repo/collections/foo (virtual subdirectory package) @@ -1406,6 +1430,8 @@ def parse(cls, dependency_str: str) -> "DependencyReference": unsupported_host_error("//...", context="Protocol-relative URLs are not supported") ) + cls._reject_shorthand_alias(dependency_str) + maybe_raise_bare_fqdn_github_gitlab_conflict(dependency_str) # Phase 1: detect virtual packages diff --git a/tests/unit/test_canonicalization.py b/tests/unit/test_canonicalization.py index 3d9d83c8a..f4e6ec006 100644 --- a/tests/unit/test_canonicalization.py +++ b/tests/unit/test_canonicalization.py @@ -32,16 +32,51 @@ def test_shorthand_with_ref(self): dep = DependencyReference.parse("microsoft/apm-sample-package#v1.0") assert dep.to_canonical() == "microsoft/apm-sample-package#v1.0" - def test_shorthand_with_alias_shorthand_removed(self): - """Shorthand @alias syntax is no longer supported in parsing.""" - with pytest.raises(ValueError): + def test_shorthand_alias_rejected(self): + """Shorthand @alias syntax is rejected with a migration error.""" + with pytest.raises(ValueError, match="Shorthand '@alias' is not supported"): DependencyReference.parse("microsoft/apm-sample-package@my-alias") - def test_shorthand_with_ref_and_alias_shorthand_not_parsed(self): - """Shorthand #ref@alias — @ is no longer parsed as alias separator.""" - dep = DependencyReference.parse("microsoft/apm-sample-package#main@my-alias") - assert dep.to_canonical() == "microsoft/apm-sample-package#main@my-alias" - assert dep.alias is None # @ is part of the ref, not an alias + def test_shorthand_with_ref_and_alias_rejected(self): + """Shorthand #ref@alias is rejected with a migration error.""" + with pytest.raises(ValueError, match="Shorthand '@alias' is not supported"): + DependencyReference.parse("microsoft/apm-sample-package#main@my-alias") + + def test_shorthand_with_subpath_and_alias_rejected(self): + """Subpath + @alias rejected loudly (was the silent-miscoercion bug).""" + with pytest.raises(ValueError, match="Shorthand '@alias' is not supported"): + DependencyReference.parse("stablyai/orca/skills/orchestration@orca-stration") + + def test_shorthand_with_deeper_subpath_and_alias_rejected(self): + """Multi-segment subpath + @alias is also rejected.""" + with pytest.raises(ValueError, match="Shorthand '@alias' is not supported"): + DependencyReference.parse("owner/repo/skills/foo/deeper@my-alias") + + def test_shorthand_with_subpath_ref_and_alias_rejected(self): + """All four parts (subpath + #ref + @alias) trip the same uniform error.""" + with pytest.raises(ValueError, match="Shorthand '@alias' is not supported"): + DependencyReference.parse("owner/repo/skills/foo#main@my-alias") + + def test_fqdn_shorthand_with_alias_rejected(self): + """FQDN shorthand + @alias is rejected (covers the non-nested-group FQDN path).""" + with pytest.raises(ValueError, match="Shorthand '@alias' is not supported"): + DependencyReference.parse("github.com/owner/repo@my-alias") + + def test_url_encoded_at_in_alias_shorthand_rejected(self): + """Percent-encoded ``@`` in shorthand is also rejected.""" + with pytest.raises(ValueError, match="Shorthand '@alias' is not supported"): + DependencyReference.parse("owner/repo%40my-alias") + + def test_trailing_at_with_no_alias_rejected(self): + """A bare trailing ``@`` is rejected (not silently stripped).""" + with pytest.raises(ValueError, match="Shorthand '@alias' is not supported"): + DependencyReference.parse("owner/repo@") + + def test_https_with_embedded_credentials_parses(self): + """Guard must not fire on HTTPS userinfo (regression: don't over-reject ``@``).""" + dep = DependencyReference.parse("https://user@github.com/owner/repo.git") + assert dep.repo_url == "owner/repo" + assert dep.alias is None def test_fqdn_github(self): """FQDN with default host strips the host.""" @@ -145,15 +180,9 @@ def test_shorthand_with_ref(self): dep = DependencyReference.parse("owner/repo#v1.0") assert dep.get_identity() == "owner/repo" - def test_shorthand_with_alias_shorthand_removed(self): - """Shorthand @alias syntax is no longer supported.""" - with pytest.raises(ValueError): - DependencyReference.parse("owner/repo@my-alias") - - def test_shorthand_with_ref_and_alias_shorthand_not_parsed(self): - """Shorthand #ref@alias — @ becomes part of the ref, identity still strips ref.""" - dep = DependencyReference.parse("owner/repo#main@my-alias") - assert dep.get_identity() == "owner/repo" + # Shorthand @alias rejection is covered in TestToCanonical; parse() raises + # before get_identity() runs, so duplicating the cases here would prove + # nothing about identity semantics. def test_fqdn_github(self): """Default host is stripped from identity.""" diff --git a/tests/unit/test_generic_git_urls.py b/tests/unit/test_generic_git_urls.py index f74f9abf9..10ef803fe 100644 --- a/tests/unit/test_generic_git_urls.py +++ b/tests/unit/test_generic_git_urls.py @@ -552,17 +552,20 @@ def test_nested_group_with_ref(self): assert dep.reference == "v2.0" assert dep.is_virtual is False - def test_nested_group_with_alias_shorthand_removed(self): - """Shorthand @alias on nested groups is no longer supported.""" - with pytest.raises(ValueError): + def test_nested_group_alias_rejected(self): + """Shorthand @alias on nested groups is rejected.""" + with pytest.raises(ValueError, match="Shorthand '@alias' is not supported"): DependencyReference.parse("gitlab.com/group/subgroup/repo@my-alias") - def test_nested_group_with_ref_and_alias_shorthand_not_parsed(self): - """Shorthand #ref@alias on nested groups — @ is no longer parsed as alias separator.""" - dep = DependencyReference.parse("gitlab.com/group/subgroup/repo#main@alias") - assert dep.repo_url == "group/subgroup/repo" - assert dep.reference == "main@alias" - assert dep.alias is None + def test_nested_group_with_ref_and_alias_rejected(self): + """Shorthand #ref@alias on nested groups is rejected at parse time.""" + with pytest.raises(ValueError, match="Shorthand '@alias' is not supported"): + DependencyReference.parse("gitlab.com/group/subgroup/repo#main@alias") + + def test_nested_group_with_subpath_and_alias_rejected(self): + """Subpath + alias under a nested group is rejected (silent-miscoercion bug fix).""" + with pytest.raises(ValueError, match="Shorthand '@alias' is not supported"): + DependencyReference.parse("gitlab.com/group/subgroup/repo/skills/foo@my-alias") # --- SSH URLs --- diff --git a/tests/unit/test_package_identity.py b/tests/unit/test_package_identity.py index aea1143a7..b2dbb22bb 100644 --- a/tests/unit/test_package_identity.py +++ b/tests/unit/test_package_identity.py @@ -54,16 +54,15 @@ def test_regular_github_package_with_reference(self): dep = DependencyReference.parse("owner/repo#v1.0.0") assert dep.get_canonical_dependency_string() == "owner/repo" - def test_regular_github_package_with_alias_shorthand_removed(self): - """Shorthand @alias syntax is no longer supported.""" - with pytest.raises(ValueError): + def test_regular_github_package_alias_rejected(self): + """Shorthand @alias syntax is rejected with a migration error.""" + with pytest.raises(ValueError, match="Shorthand '@alias' is not supported"): DependencyReference.parse("owner/repo@myalias") - def test_regular_github_package_with_reference_and_alias_shorthand_not_parsed(self): - """Shorthand #ref@alias — @ is no longer parsed as alias separator.""" - dep = DependencyReference.parse("owner/repo#main@myalias") - assert dep.reference == "main@myalias" - assert dep.alias is None + def test_regular_github_package_with_reference_and_alias_rejected(self): + """Shorthand #ref@alias is rejected at parse time with a migration error.""" + with pytest.raises(ValueError, match="Shorthand '@alias' is not supported"): + DependencyReference.parse("owner/repo#main@myalias") def test_virtual_file_package(self): """Virtual file includes full path."""