fix(deps): reject shorthand '@alias' at parse time with migration error#1301
Open
prateek wants to merge 1 commit into
Open
fix(deps): reject shorthand '@alias' at parse time with migration error#1301prateek wants to merge 1 commit into
prateek wants to merge 1 commit into
Conversation
`apm.yml` entries like `owner/repo/sub/path@alias` were silently miscoerced -- the `@alias` ended up glued to the virtual path and failed downstream with "Subdirectory not found in repository". Shorthand `@alias` was deliberately removed in microsoft#340 (npm/go/cargo `@version` collision); the whole-repo case already failed loudly, but the subpath case slipped past `_detect_virtual_package`. Add a single parse-time guard (`_reject_shorthand_alias`) that rejects shorthand `@alias` uniformly with an actionable migration error pointing to the object form. SSH `@alias` extraction is preserved -- the `ssh://` and SCP parsers already handle it cleanly, so the guard skips those forms. Also drop two stale `@alias` example lines from the public `parse()` docstring and remove the surviving `@alias` rows from `consumer/manage-dependencies.md`.
Author
|
@microsoft-github-policy-service agree |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR tightens DependencyReference.parse() so deprecated shorthand @alias forms (especially with subpaths) are rejected early with a clear migration error, preventing the alias from being silently misinterpreted as part of a virtual path or ref.
Changes:
- Add a parse-time guard (
_reject_shorthand_alias) to uniformly reject shorthand@aliasin non-URL, non-SSH forms. - Update unit tests to expect the new rejection behavior across shorthand, FQDN, subpath, and URL-encoded
@cases (and add an HTTPS userinfo regression case). - Update docs to remove stale shorthand alias examples and document
alias:via object form.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/test_package_identity.py | Updates shorthand alias tests to expect parse-time rejection with the migration message. |
| tests/unit/test_generic_git_urls.py | Adds/updates rejection cases for nested-group shorthand, including the subpath+alias bug case. |
| tests/unit/test_canonicalization.py | Expands coverage for shorthand alias rejection (subpath, encoded @, trailing @) and adds HTTPS userinfo regression coverage. |
| src/apm_cli/models/dependency/reference.py | Introduces _reject_shorthand_alias() and calls it early in parse() to reject deprecated shorthand alias forms consistently. |
| docs/src/content/docs/consumer/manage-dependencies.md | Removes stale shorthand alias rows and highlights alias: support in object form. |
Comment on lines
+403
to
+410
| 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( |
Comment on lines
+1433
to
+1434
| cls._reject_shorthand_alias(dependency_str) | ||
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
apm.ymlstring-form entries likeowner/repo/sub/path@alias(subpath + trailing@alias) were silently miscoerced: the@aliasended up glued tovirtual_pathand the install failed downstream with the unhelpfulSubdirectory 'skills/orchestration@orca-stration' not found in repository.Shorthand
@aliaswas deliberately retired in #340 (npm/go/cargo@versioncollision). The whole-repo case (owner/repo@alias) already failed loudly withInvalid repository path component, but the subpath case slipped past_detect_virtual_packagebecause it never looked for@in the trailing segments.This PR adds a single parse-time guard so all three shorthand-
@aliasshapes —owner/repo@alias,owner/repo/sub@alias,owner/repo#ref@alias— produce the same actionable error pointing the user to the object form.Repro
Before:
After:
What changed
src/apm_cli/models/dependency/reference.py— new_reject_shorthand_alias()static method called fromparse()after the local-path ///checks. Skipshttps://,http://,ssh://, and SCP shorthand (<user>@host:path) — those forms have dedicated parsers that legitimately extract@alias. Anything else with@raises with migration guidance.docs/src/content/docs/consumer/manage-dependencies.md— removed the two staleAliased/Pinned + aliasedrows that survived feat: improve version pinning guidance and CLI visibility #340, and foldedalias:into theObject formrow description.user/repo@aliasexamples from the publicparse()docstring.%40, trailing@) plus a positivehttps://user@github.com/...regression test that locks in the guard's HTTPS-userinfo carve-out. Three pre-existing tests that asserted the old "absorb@into ref" behavior were tightened to assert the new uniform rejection.Compatibility
to_canonical()never emits@alias, andLockedDependency.from_dependency_ref/to_dependency_refwork on structured fields, never re-parse()-ing a string. Historical lockfiles cannot contain@aliasand so cannot trip the new guard on load.ssh://...@aliasandgit@host:path@aliasstill parse and extract the alias as before.{ git: ..., path: ..., alias: ... }is unaffected.Test plan
pytest tests/unit/— 8,301 passing.agents/skills/orca-stration/as expected