From 080d7494ab6533cd6d1c110aba24700464fc6164 Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Tue, 17 Mar 2026 10:13:48 +0100 Subject: [PATCH 1/5] feat: improve version pinning guidance and CLI visibility - Show resolved ref in install output (e.g. 'owner/repo @ main (a1b2c3d4)') - Add one-time info hint when unversioned deps are installed - Update docs and templates to show pinned examples (#v1.0.0) - Tag microsoft/apm-sample-package with v1.0.0 Closes #336 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- README.md | 4 ++-- .../content/docs/getting-started/quick-start.md | 4 ++-- docs/src/content/docs/guides/dependencies.md | 4 ++-- .../content/docs/reference/manifest-schema.md | 10 +++++----- src/apm_cli/commands/install.py | 17 ++++++++++++++++- templates/hello-world/apm.yml | 2 +- 6 files changed, 28 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index 9de6cfa8..7ced1019 100644 --- a/README.md +++ b/README.md @@ -27,7 +27,7 @@ dependencies: # Specific agent primitives from any repository - github/awesome-copilot/agents/api-architect.agent.md # A full APM package with instructions, skills, prompts, hooks... - - microsoft/apm-sample-package + - microsoft/apm-sample-package#v1.0.0 ``` ```bash @@ -88,7 +88,7 @@ pip install apm-cli Then start adding packages: ```bash -apm install microsoft/apm-sample-package +apm install microsoft/apm-sample-package#v1.0.0 ``` See the **[Getting Started guide](https://microsoft.github.io/apm/getting-started/quick-start/)** for the full walkthrough. diff --git a/docs/src/content/docs/getting-started/quick-start.md b/docs/src/content/docs/getting-started/quick-start.md index ca52e4c8..80da34f6 100644 --- a/docs/src/content/docs/getting-started/quick-start.md +++ b/docs/src/content/docs/getting-started/quick-start.md @@ -58,7 +58,7 @@ dependencies: This is where it gets interesting. Install a package and watch what happens: ```bash -apm install microsoft/apm-sample-package +apm install microsoft/apm-sample-package#v1.0.0 ``` APM downloads the package, resolves its dependencies, and deploys files directly into the directories your AI tools already watch: @@ -107,7 +107,7 @@ name: my-project version: 1.0.0 dependencies: apm: - - microsoft/apm-sample-package + - microsoft/apm-sample-package#v1.0.0 ``` ## That's it diff --git a/docs/src/content/docs/guides/dependencies.md b/docs/src/content/docs/guides/dependencies.md index 54e859c7..5138ed44 100644 --- a/docs/src/content/docs/guides/dependencies.md +++ b/docs/src/content/docs/guides/dependencies.md @@ -80,7 +80,7 @@ version: 1.0.0 dependencies: apm: # GitHub shorthand (default) - - microsoft/apm-sample-package + - microsoft/apm-sample-package#v1.0.0 - github/awesome-copilot/skills/review-and-refactor # Full HTTPS git URL (any host) @@ -405,7 +405,7 @@ version: 1.0.0 description: Corporate website with design standards and code review dependencies: apm: - - microsoft/apm-sample-package + - microsoft/apm-sample-package#v1.0.0 - github/awesome-copilot/skills/review-and-refactor mcp: - io.github.github/github-mcp-server diff --git a/docs/src/content/docs/reference/manifest-schema.md b/docs/src/content/docs/reference/manifest-schema.md index 8f2235ad..91f34ab8 100644 --- a/docs/src/content/docs/reference/manifest-schema.md +++ b/docs/src/content/docs/reference/manifest-schema.md @@ -189,9 +189,9 @@ local_path_form = ("./" / "../" / "/" / "~/" / ".\\" / "..\\" / "~\\") path dependencies: apm: # GitHub shorthand (default host) - - microsoft/apm-sample-package - - microsoft/apm-sample-package#v1.0.0 - - microsoft/apm-sample-package@standards + - microsoft/apm-sample-package # latest (lockfile pins commit SHA) + - microsoft/apm-sample-package#v1.0.0 # pinned to tag + - microsoft/apm-sample-package@standards # pinned to branch # Non-GitHub hosts (FQDN preserved) - gitlab.com/acme/coding-standards @@ -418,8 +418,8 @@ scripts: dependencies: apm: - - microsoft/apm-sample-package - - gitlab.com/acme/coding-standards + - microsoft/apm-sample-package#v1.0.0 + - gitlab.com/acme/coding-standards#main - git: https://gitlab.com/acme/repo.git path: instructions/security ref: v2.0 diff --git a/src/apm_cli/commands/install.py b/src/apm_cli/commands/install.py index e05ac57f..bf72f7f6 100644 --- a/src/apm_cli/commands/install.py +++ b/src/apm_cli/commands/install.py @@ -1120,6 +1120,7 @@ def _collect_descendants(node, visited=None): # downloader already created above for transitive resolution installed_count = 0 + has_unpinned_deps = False # Phase 4 (#171): Parallel package downloads using ThreadPoolExecutor # Pre-download all non-cached packages in parallel for wall-clock speedup. @@ -1396,6 +1397,8 @@ def _collect_descendants(node, visited=None): ref_str = f" @{dep_ref.reference}" if dep_ref.reference else "" _rich_info(f"✓ {display_name}{ref_str} (cached)") installed_count += 1 + if not dep_ref.reference: + has_unpinned_deps = True # Still need to integrate prompts for cached packages (zero-config behavior) if integrate_vscode or integrate_claude or integrate_opencode: @@ -1790,7 +1793,16 @@ def _collect_descendants(node, visited=None): progress.refresh() # Force immediate refresh to hide the bar installed_count += 1 - _rich_success(f"✓ {display_name}") + + # Show resolved ref alongside package name for visibility + ref_suffix = "" + if hasattr(package_info, 'resolved_reference') and package_info.resolved_reference: + ref_suffix = f" @ {package_info.resolved_reference}" + _rich_success(f"✓ {display_name}{ref_suffix}") + + # Track whether any dep had no explicit ref (for hint) + if not dep_ref.reference: + has_unpinned_deps = True # Collect for lockfile: get resolved commit and depth resolved_commit = None @@ -2220,6 +2232,9 @@ def _collect_descendants(node, visited=None): _rich_success(f"Installed {installed_count} APM dependencies") + if has_unpinned_deps: + _rich_info("Tip: Pin versions with #tag or #sha for reproducible installs (e.g. owner/repo#v1.0.0)") + return installed_count, total_prompts_integrated, total_agents_integrated, diagnostics except Exception as e: diff --git a/templates/hello-world/apm.yml b/templates/hello-world/apm.yml index 1468c8a9..becbfc5e 100644 --- a/templates/hello-world/apm.yml +++ b/templates/hello-world/apm.yml @@ -10,7 +10,7 @@ scripts: dependencies: apm: - - microsoft/apm-sample-package # Design standards, prompts, and skills + - microsoft/apm-sample-package#v1.0.0 # Design standards, prompts, and skills mcp: - microsoft/azure-devops-mcp From dd26331ed178141bb72d7c5687bc4a6c470bee4a Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Tue, 17 Mar 2026 15:13:30 +0100 Subject: [PATCH 2/5] fix: polish install output aesthetics per review feedback - Cached path now shows resolved commit SHA from lockfile (consistent with fresh installs) - Consistent spacing: all ref output uses ' @ ref' format - Replace hasattr() with idiomatic getattr(..., None) - Clarify branch refs are mutable in manifest-schema docs Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../content/docs/reference/manifest-schema.md | 4 ++-- src/apm_cli/commands/install.py | 18 +++++++++++++----- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/docs/src/content/docs/reference/manifest-schema.md b/docs/src/content/docs/reference/manifest-schema.md index 91f34ab8..2a67d491 100644 --- a/docs/src/content/docs/reference/manifest-schema.md +++ b/docs/src/content/docs/reference/manifest-schema.md @@ -190,8 +190,8 @@ dependencies: apm: # GitHub shorthand (default host) - microsoft/apm-sample-package # latest (lockfile pins commit SHA) - - microsoft/apm-sample-package#v1.0.0 # pinned to tag - - microsoft/apm-sample-package@standards # pinned to branch + - microsoft/apm-sample-package#v1.0.0 # pinned to tag (immutable) + - microsoft/apm-sample-package@standards # branch ref (may change over time) # Non-GitHub hosts (FQDN preserved) - gitlab.com/acme/coding-standards diff --git a/src/apm_cli/commands/install.py b/src/apm_cli/commands/install.py index 1b9f0783..49264459 100644 --- a/src/apm_cli/commands/install.py +++ b/src/apm_cli/commands/install.py @@ -1353,7 +1353,16 @@ def _collect_descendants(node, visited=None): display_name = ( str(dep_ref) if dep_ref.is_virtual else dep_ref.repo_url ) - ref_str = f" @{dep_ref.reference}" if dep_ref.reference else "" + # Show resolved ref from lockfile for consistency with fresh installs + ref_str = "" + if _dep_locked_chk and _dep_locked_chk.resolved_commit and _dep_locked_chk.resolved_commit != "cached": + short_sha = _dep_locked_chk.resolved_commit[:8] + if dep_ref.reference: + ref_str = f" @ {dep_ref.reference} ({short_sha})" + else: + ref_str = f" @ {short_sha}" + elif dep_ref.reference: + ref_str = f" @ {dep_ref.reference}" _rich_info(f"✓ {display_name}{ref_str} (cached)") installed_count += 1 if not dep_ref.reference: @@ -1754,9 +1763,8 @@ def _collect_descendants(node, visited=None): installed_count += 1 # Show resolved ref alongside package name for visibility - ref_suffix = "" - if hasattr(package_info, 'resolved_reference') and package_info.resolved_reference: - ref_suffix = f" @ {package_info.resolved_reference}" + resolved = getattr(package_info, 'resolved_reference', None) + ref_suffix = f" @ {resolved}" if resolved else "" _rich_success(f"✓ {display_name}{ref_suffix}") # Track whether any dep had no explicit ref (for hint) @@ -1765,7 +1773,7 @@ def _collect_descendants(node, visited=None): # Collect for lockfile: get resolved commit and depth resolved_commit = None - if hasattr(package_info, 'resolved_reference') and package_info.resolved_reference: + if resolved: resolved_commit = package_info.resolved_reference.resolved_commit # Get depth from dependency tree node = dependency_graph.dependency_tree.get_node(dep_ref.get_unique_key()) From fe0f6a348edbdb102d6dc4360025ffa5e7886505 Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Tue, 17 Mar 2026 15:18:01 +0100 Subject: [PATCH 3/5] fix: correct @branch misuse in manifest-schema examples MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The @ separator is for aliases, not branch refs (per the ABNF grammar: shorthand = [...] ["#" ref] ["@" alias]). The old example 'repo@standards' was silently parsed as alias='standards' with no ref, not as a branch checkout. Replace with correct syntax: - repo#main → branch ref - repo#v1.0.0@standards → pinned tag with local alias Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/src/content/docs/reference/manifest-schema.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/src/content/docs/reference/manifest-schema.md b/docs/src/content/docs/reference/manifest-schema.md index 2a67d491..3f209117 100644 --- a/docs/src/content/docs/reference/manifest-schema.md +++ b/docs/src/content/docs/reference/manifest-schema.md @@ -191,7 +191,8 @@ dependencies: # GitHub shorthand (default host) - microsoft/apm-sample-package # latest (lockfile pins commit SHA) - microsoft/apm-sample-package#v1.0.0 # pinned to tag (immutable) - - microsoft/apm-sample-package@standards # branch ref (may change over time) + - microsoft/apm-sample-package#main # branch ref (may change over time) + - microsoft/apm-sample-package#v1.0.0@standards # pinned tag with local alias # Non-GitHub hosts (FQDN preserved) - gitlab.com/acme/coding-standards From 74e12d5c6ec39f5bae77cfc2285647b6a03cb0c1 Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Tue, 17 Mar 2026 16:13:53 +0100 Subject: [PATCH 4/5] refactor: remove @alias shorthand syntax from dependency strings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The @ separator conflicted with conventions in npm (@scope), Go (@version), and Cargo (@version), causing real confusion — our own docs had a bug where @standards was intended as a branch ref. The feature had zero real-world usage (0 YAML fixtures, 0 production examples, 0 guide mentions). The dict format alias: field remains available for edge cases. - Remove @ alias parsing from DependencyReference.parse() - Remove @ from ABNF grammar and shorthand field table - Remove @alias from all doc examples (manifest-schema, dependencies, primitive-types) - Update tests: shorthand @alias now rejected, dict alias: still works - CHANGELOG entry under Removed Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- CHANGELOG.md | 6 +++ docs/src/content/docs/guides/dependencies.md | 9 ++-- .../content/docs/reference/manifest-schema.md | 7 +-- .../content/docs/reference/primitive-types.md | 9 ++-- src/apm_cli/models/dependency.py | 35 +++---------- tests/test_apm_package_models.py | 50 ++++++------------- tests/test_enhanced_discovery.py | 4 +- tests/unit/test_canonicalization.py | 26 +++++----- tests/unit/test_generic_git_urls.py | 32 ++++++------ tests/unit/test_package_identity.py | 16 +++--- 10 files changed, 79 insertions(+), 115 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6714b4ed..14682c68 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,12 +11,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Audit hardening — `apm unpack` content scanning, SARIF/JSON/Markdown `--format`/`--output` for CI capture, `SecurityGate` policy engine, non-zero exits on critical findings (#330) +- Install output now shows resolved git ref alongside package name (e.g. `✓ owner/repo @ main (a1b2c3d4)`) (#340) +- One-time info hint when dependencies have no explicit ref: `Tip: Pin versions with #tag or #sha for reproducible installs` (#340) ### Fixed - File-level downloads from private repos now use OS credential helpers (macOS Keychain, `gh auth login`, Windows Credential Manager) — closes auth gap between folder and file dependencies (#332) - Lockfile now preserves the host for GitHub Enterprise custom domains so subsequent `apm install` clones from the correct server (#338) +### Removed + +- Shorthand `@alias` syntax removed from dependency strings — the `@` separator conflicted with conventions in npm, Go, and Cargo where `@` denotes scope or version. Use the dict format `alias:` field instead (#340) + ## [0.8.0] - 2026-03-16 ### Added diff --git a/docs/src/content/docs/guides/dependencies.md b/docs/src/content/docs/guides/dependencies.md index 5138ed44..5d96e995 100644 --- a/docs/src/content/docs/guides/dependencies.md +++ b/docs/src/content/docs/guides/dependencies.md @@ -97,7 +97,7 @@ dependencies: - ./packages/my-shared-skills # relative to project root - /home/user/repos/my-ai-package # absolute path - # Object format: git URL + sub-path / ref / alias + # Object format: git URL + sub-path / ref - git: https://gitlab.com/acme/coding-standards.git path: instructions/security ref: v2.0 @@ -126,7 +126,7 @@ APM accepts dependencies in two forms: - For nested groups + virtual paths, use the object format below - **Local path** (`./path`, `../path`, `/absolute/path`) — local filesystem package -**Object format** (when you need `path`, `ref`, or `alias` on a git URL): +**Object format** (when you need `path` or `ref` on a git URL): ```yaml dependencies: @@ -136,10 +136,9 @@ dependencies: ref: v2.0 # pin to a tag, branch, or commit - git: git@bitbucket.org:team/rules.git path: prompts/review.prompt.md - alias: review ``` -Fields: `git` (required), `path`, `ref`, `alias` (all optional). The `git` value is any HTTPS or SSH clone URL. +Fields: `git` (required), `path`, `ref` (all optional). The `git` value is any HTTPS or SSH clone URL. > **Nested groups (GitLab, Gitea, etc.):** APM treats all path segments after the host as the repo path, so `gitlab.com/group/subgroup/repo` resolves to a repo at `group/subgroup/repo`. Virtual paths on simple 2-segment repos work with shorthand (`gitlab.com/owner/repo/file.prompt.md`). But for **nested-group repos + virtual paths**, use the object format — the shorthand is ambiguous: > @@ -173,7 +172,7 @@ APM normalizes every dependency entry on write — no matter how you specify a p | `./packages/my-skills` | `./packages/my-skills` | | `/home/user/repos/my-pkg` | `/home/user/repos/my-pkg` | -Virtual paths, refs, and aliases are preserved: +Virtual paths and refs are preserved: | You type | Stored in apm.yml | |----------|-------------------| diff --git a/docs/src/content/docs/reference/manifest-schema.md b/docs/src/content/docs/reference/manifest-schema.md index 3f209117..78d37f98 100644 --- a/docs/src/content/docs/reference/manifest-schema.md +++ b/docs/src/content/docs/reference/manifest-schema.md @@ -171,7 +171,7 @@ Grammar (ABNF-style): ``` dependency = url_form / shorthand_form / local_path_form url_form = ("https://" / "http://" / "ssh://git@" / "git@") clone-url -shorthand_form = [host "/"] owner "/" repo ["/" virtual_path] ["#" ref] ["@" alias] +shorthand_form = [host "/"] owner "/" repo ["/" virtual_path] ["#" ref] local_path_form = ("./" / "../" / "/" / "~/" / ".\\" / "..\\" / "~\\") path ``` @@ -181,18 +181,16 @@ local_path_form = ("./" / "../" / "/" / "~/" / ".\\" / "..\\" / "~\\") path | `owner/repo` | REQUIRED | 2+ path segments of `[a-zA-Z0-9._-]+` | Repository path. GitHub uses exactly 2 segments (`owner/repo`). Non-GitHub hosts MAY use nested groups (e.g. `gitlab.com/group/sub/repo`). | | `virtual_path` | OPTIONAL | Path segments after repo | Subdirectory, file, or collection within the repo. See §4.1.3. | | `ref` | OPTIONAL | Branch, tag, or commit SHA | Git reference. Commit SHAs matched by `^[a-f0-9]{7,40}$`. Semver tags matched by `^v?\d+\.\d+\.\d+`. | -| `alias` | OPTIONAL | `^[a-zA-Z0-9._-]+$` | Local alias for the dependency. Appears after `#ref` in the string. | **Examples:** ```yaml dependencies: apm: - # GitHub shorthand (default host) + # GitHub shorthand (default host) — each line shows a syntax variant - microsoft/apm-sample-package # latest (lockfile pins commit SHA) - microsoft/apm-sample-package#v1.0.0 # pinned to tag (immutable) - microsoft/apm-sample-package#main # branch ref (may change over time) - - microsoft/apm-sample-package#v1.0.0@standards # pinned tag with local alias # Non-GitHub hosts (FQDN preserved) - gitlab.com/acme/coding-standards @@ -424,7 +422,6 @@ dependencies: - git: https://gitlab.com/acme/repo.git path: instructions/security ref: v2.0 - alias: acme-sec mcp: - io.github.github/github-mcp-server - name: my-private-server diff --git a/docs/src/content/docs/reference/primitive-types.md b/docs/src/content/docs/reference/primitive-types.md index 59222261..0d34c544 100644 --- a/docs/src/content/docs/reference/primitive-types.md +++ b/docs/src/content/docs/reference/primitive-types.md @@ -129,7 +129,7 @@ version: 1.0.0 dependencies: apm: - company/standards#v1.0.0 - - team/workflows@workflow-alias + - team/workflows - user/utilities ``` @@ -151,9 +151,10 @@ project/ │ └── .apm/ │ ├── chatmodes/ │ └── instructions/ - ├── workflow-alias/ # From team/workflows (uses alias) - │ └── .apm/ - │ └── contexts/ + ├── team/ + │ └── workflows/ # From team/workflows + │ └── .apm/ + │ └── contexts/ └── utilities/ # From user/utilities └── .apm/ └── instructions/ diff --git a/src/apm_cli/models/dependency.py b/src/apm_cli/models/dependency.py index 48b4689a..eb9d2774 100644 --- a/src/apm_cli/models/dependency.py +++ b/src/apm_cli/models/dependency.py @@ -172,7 +172,6 @@ def to_canonical(self) -> str: - Non-default hosts are preserved -> gitlab.com/owner/repo - Virtual paths are appended -> owner/repo/path/to/thing - Refs are appended with # -> owner/repo#v1.0 - - Aliases are appended with @ -> owner/repo@my-alias - Local paths are returned as-is -> ./packages/my-pkg No .git suffix, no https://, no git@ -- just the canonical identifier. @@ -200,10 +199,6 @@ def to_canonical(self) -> str: if self.reference: result = f"{result}#{self.reference}" - # Append alias - if self.alias: - result = f"{result}@{self.alias}" - return result def get_identity(self) -> str: @@ -433,7 +428,10 @@ def parse_from_dict(cls, entry: dict) -> "DependencyReference": if alias_override is not None: if not isinstance(alias_override, str) or not alias_override.strip(): raise ValueError("'alias' field must be a non-empty string") - dep.alias = alias_override.strip() + alias_override = alias_override.strip() + if not re.match(r'^[a-zA-Z0-9._-]+$', alias_override): + raise ValueError(f"Invalid alias: {alias_override}. Aliases can only contain letters, numbers, dots, underscores, and hyphens") + dep.alias = alias_override # Apply sub-path as virtual package if sub_path: @@ -452,8 +450,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/collections/name (virtual collection package) - https://gitlab.com/owner/repo.git (generic HTTPS git URL) @@ -515,10 +511,8 @@ def parse(cls, dependency_str: str) -> "DependencyReference": # Extract the core path before processing reference (#) and alias (@) work_str = dependency_str - # Temporarily remove reference and alias for path segment counting + # Temporarily remove reference for path segment counting temp_str = work_str - if '@' in temp_str and not temp_str.startswith('git@'): - temp_str = temp_str.rsplit('@', 1)[0] if '#' in temp_str: temp_str = temp_str.rsplit('#', 1)[0] @@ -653,14 +647,10 @@ def parse(cls, dependency_str: str) -> "DependencyReference": host = ssh_match.group(1) ssh_repo_part = ssh_match.group(2) - # Handle reference and alias in SSH URL (extract before .git stripping) + # Handle reference in SSH URL (extract before .git stripping) reference = None alias = None - if "@" in ssh_repo_part: - ssh_repo_part, alias = ssh_repo_part.rsplit("@", 1) - alias = alias.strip() - if "#" in ssh_repo_part: repo_part, reference = ssh_repo_part.rsplit("#", 1) reference = reference.strip() @@ -673,13 +663,8 @@ def parse(cls, dependency_str: str) -> "DependencyReference": repo_url = repo_part.strip() else: - # Handle alias (@alias) for non-SSH URLs - alias = None - if "@" in dependency_str: - dependency_str, alias = dependency_str.rsplit("@", 1) - alias = alias.strip() - # Handle reference (#ref) + alias = None reference = None if "#" in dependency_str: repo_part, reference = dependency_str.rsplit("#", 1) @@ -881,10 +866,6 @@ def parse(cls, dependency_str: str) -> "DependencyReference": ado_project = None ado_repo = None - # Validate alias characters if present - if alias and not re.match(r'^[a-zA-Z0-9._-]+$', alias): - raise ValueError(f"Invalid alias: {alias}. Aliases can only contain letters, numbers, dots, underscores, and hyphens") - return cls( repo_url=repo_url, host=host, @@ -943,8 +924,6 @@ def __str__(self) -> str: result += f"/{self.virtual_path}" if self.reference: result += f"#{self.reference}" - if self.alias: - result += f"@{self.alias}" return result diff --git a/tests/test_apm_package_models.py b/tests/test_apm_package_models.py index 82749daf..d3e3893b 100644 --- a/tests/test_apm_package_models.py +++ b/tests/test_apm_package_models.py @@ -54,19 +54,17 @@ def test_parse_with_commit(self): assert dep.reference == "abc123def" assert dep.alias is None - def test_parse_with_alias(self): - """Test parsing with alias.""" - dep = DependencyReference.parse("user/repo@myalias") - assert dep.repo_url == "user/repo" - assert dep.reference is None - assert dep.alias == "myalias" + def test_parse_with_alias_shorthand_removed(self): + """Shorthand @alias syntax is no longer supported — @ in shorthand is rejected.""" + with pytest.raises(ValueError): + DependencyReference.parse("user/repo@myalias") - def test_parse_with_reference_and_alias(self): - """Test parsing with both reference and alias.""" + def test_parse_with_reference_and_alias_shorthand_not_parsed(self): + """Shorthand #ref@alias — @ is no longer parsed as alias separator.""" dep = DependencyReference.parse("user/repo#main@myalias") assert dep.repo_url == "user/repo" - assert dep.reference == "main" - assert dep.alias == "myalias" + assert dep.reference == "main@myalias" # @ treated as part of ref + assert dep.alias is None def test_parse_github_urls(self): """Test parsing various GitHub URL formats.""" @@ -375,10 +373,10 @@ def test_virtual_package_str_representation(self): assert "prompts/code-review.prompt.md" in str(dep) assert "#v1.0.0" in str(dep) - dep_with_alias = DependencyReference.parse("owner/test-repo/prompts/test.prompt.md@myalias") - assert "owner/test-repo" in str(dep_with_alias) - assert "prompts/test.prompt.md" in str(dep_with_alias) - assert "@myalias" in str(dep_with_alias) + dep_with_ref = DependencyReference.parse("owner/test-repo/prompts/test.prompt.md#v2.0") + assert "owner/test-repo" in str(dep_with_ref) + assert "prompts/test.prompt.md" in str(dep_with_ref) + assert "#v2.0" in str(dep_with_ref) def test_regular_package_not_virtual(self): """Test that regular packages (2 segments) are not marked as virtual.""" @@ -416,7 +414,8 @@ def test_get_display_name(self): dep1 = DependencyReference.parse("user/repo") assert dep1.get_display_name() == "user/repo" - dep2 = DependencyReference.parse("user/repo@myalias") + # Dict format alias still works for display name + dep2 = DependencyReference.parse_from_dict({"git": "https://github.com/user/repo.git", "alias": "myalias"}) assert dep2.get_display_name() == "myalias" def test_string_representation(self): @@ -434,17 +433,6 @@ def test_string_representation(self): assert dep2.repo_url == "user/repo" assert dep2.reference == "main" assert "user/repo" in str(dep2) and "#main" in str(dep2) - - dep3 = DependencyReference.parse("user/repo@myalias") - assert dep3.repo_url == "user/repo" - assert dep3.alias == "myalias" - assert "user/repo" in str(dep3) and "@myalias" in str(dep3) - - dep4 = DependencyReference.parse("user/repo#main@myalias") - assert dep4.repo_url == "user/repo" - assert dep4.reference == "main" - assert dep4.alias == "myalias" - assert "user/repo" in str(dep4) and "#main" in str(dep4) and "@myalias" in str(dep4) def test_string_representation_with_enterprise_host(self): """Test that string representation includes host for enterprise dependencies. @@ -460,14 +448,6 @@ def test_string_representation_with_enterprise_host(self): dep2 = DependencyReference.parse("company.ghe.com/user/repo#v1.0.0") assert str(dep2) == "company.ghe.com/user/repo#v1.0.0" - # Enterprise host with alias - dep3 = DependencyReference.parse("company.ghe.com/user/repo@myalias") - assert str(dep3) == "company.ghe.com/user/repo@myalias" - - # Enterprise host with reference and alias - dep4 = DependencyReference.parse("company.ghe.com/user/repo#main@myalias") - assert str(dep4) == "company.ghe.com/user/repo#main@myalias" - # Explicit github.com should also include host dep5 = DependencyReference.parse("github.com/user/repo") assert str(dep5) == "github.com/user/repo" @@ -505,7 +485,7 @@ def test_from_apm_yml_complete(self): 'author': 'Test Author', 'license': 'MIT', 'dependencies': { - 'apm': ['user/repo#main', 'another/repo@alias'], + 'apm': ['user/repo#main', 'another/repo#v2.0'], 'mcp': ['some-mcp-server'] }, 'scripts': { diff --git a/tests/test_enhanced_discovery.py b/tests/test_enhanced_discovery.py index b4215b7d..271b320f 100644 --- a/tests/test_enhanced_discovery.py +++ b/tests/test_enhanced_discovery.py @@ -167,14 +167,14 @@ def test_dependency_order_from_apm_yml(self): "apm": [ "company/standards", "team/workflows#v1.0.0", - "user/utilities@util-alias" + "user/utilities" ] } self._create_apm_yml(dependencies) # Test dependency order extraction order = get_dependency_declaration_order(str(self.temp_dir_path)) - expected = ["company/standards", "team/workflows", "util-alias"] # Full org/repo paths, except alias + expected = ["company/standards", "team/workflows", "user/utilities"] self.assertEqual(order, expected) def test_dependency_order_no_apm_yml(self): diff --git a/tests/unit/test_canonicalization.py b/tests/unit/test_canonicalization.py index b01b3fd8..5ef4e285 100644 --- a/tests/unit/test_canonicalization.py +++ b/tests/unit/test_canonicalization.py @@ -31,15 +31,16 @@ 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(self): - """Shorthand with alias preserves the alias.""" - dep = DependencyReference.parse("microsoft/apm-sample-package@my-alias") - assert dep.to_canonical() == "microsoft/apm-sample-package@my-alias" + def test_shorthand_with_alias_shorthand_removed(self): + """Shorthand @alias syntax is no longer supported in parsing.""" + with pytest.raises(ValueError): + DependencyReference.parse("microsoft/apm-sample-package@my-alias") - def test_shorthand_with_ref_and_alias(self): - """Shorthand with both ref and alias preserves both.""" + 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_fqdn_github(self): """FQDN with default host strips the host.""" @@ -142,13 +143,13 @@ def test_shorthand_with_ref(self): dep = DependencyReference.parse("owner/repo#v1.0") assert dep.get_identity() == "owner/repo" - def test_shorthand_with_alias(self): - """Alias is stripped from identity.""" - dep = DependencyReference.parse("owner/repo@my-alias") - 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(self): - """Both ref and alias are stripped from identity.""" + 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" @@ -201,7 +202,6 @@ def test_same_identity_different_forms(self): "git@github.com:microsoft/apm-sample-package.git", "ssh://git@github.com/microsoft/apm-sample-package.git", "microsoft/apm-sample-package#main", - "microsoft/apm-sample-package@alias", ] identities = {DependencyReference.parse(f).get_identity() for f in forms} assert len(identities) == 1, f"Expected 1 identity, got {identities}" diff --git a/tests/unit/test_generic_git_urls.py b/tests/unit/test_generic_git_urls.py index 23dbeb4e..31e35381 100644 --- a/tests/unit/test_generic_git_urls.py +++ b/tests/unit/test_generic_git_urls.py @@ -64,18 +64,18 @@ def test_gitlab_https_url_with_ref(self): assert dep.repo_url == "acme/coding-standards" assert dep.reference == "v2.0" - def test_gitlab_https_url_with_alias(self): - dep = DependencyReference.parse("https://gitlab.com/acme/coding-standards.git@my-rules") - assert dep.host == "gitlab.com" - assert dep.repo_url == "acme/coding-standards" - assert dep.alias == "my-rules" + def test_gitlab_https_url_with_alias_shorthand_removed(self): + """Shorthand @alias on HTTPS URLs is no longer supported.""" + with pytest.raises(ValueError): + DependencyReference.parse("https://gitlab.com/acme/coding-standards.git@my-rules") - def test_gitlab_https_url_with_ref_and_alias(self): + def test_gitlab_https_url_with_ref_and_alias_shorthand_not_parsed(self): + """Shorthand #ref@alias on HTTPS URLs — @ is no longer parsed as alias separator.""" dep = DependencyReference.parse("https://gitlab.com/acme/coding-standards.git#main@rules") assert dep.host == "gitlab.com" assert dep.repo_url == "acme/coding-standards" - assert dep.reference == "main" - assert dep.alias == "rules" + assert dep.reference == "main@rules" + assert dep.alias is None def test_gitlab_fqdn_format(self): """Test gitlab.com/owner/repo format (without https://).""" @@ -431,17 +431,17 @@ def test_nested_group_with_ref(self): assert dep.reference == "v2.0" assert dep.is_virtual is False - def test_nested_group_with_alias(self): - dep = DependencyReference.parse("gitlab.com/group/subgroup/repo@my-alias") - assert dep.host == "gitlab.com" - assert dep.repo_url == "group/subgroup/repo" - assert dep.alias == "my-alias" + def test_nested_group_with_alias_shorthand_removed(self): + """Shorthand @alias on nested groups is no longer supported.""" + with pytest.raises(ValueError): + DependencyReference.parse("gitlab.com/group/subgroup/repo@my-alias") - def test_nested_group_with_ref_and_alias(self): + 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" - assert dep.alias == "alias" + assert dep.reference == "main@alias" + assert dep.alias is None # --- SSH URLs --- diff --git a/tests/unit/test_package_identity.py b/tests/unit/test_package_identity.py index 6aec1917..c1bb18ae 100644 --- a/tests/unit/test_package_identity.py +++ b/tests/unit/test_package_identity.py @@ -7,6 +7,7 @@ 3. Agent/prompt metadata for orphan detection """ +import pytest from pathlib import Path from urllib.parse import urlparse @@ -52,15 +53,16 @@ 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(self): - """Alias (@) does NOT affect canonical string.""" - dep = DependencyReference.parse("owner/repo@myalias") - 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): + DependencyReference.parse("owner/repo@myalias") - def test_regular_github_package_with_reference_and_alias(self): - """Both reference and alias do NOT affect canonical string.""" + 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.get_canonical_dependency_string() == "owner/repo" + assert dep.reference == "main@myalias" + assert dep.alias is None def test_virtual_file_package(self): """Virtual file includes full path.""" From 40cd957db4861214f718bbddcde05b179bea3b07 Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Tue, 17 Mar 2026 16:25:47 +0100 Subject: [PATCH 5/5] =?UTF-8?q?fix:=20address=20PR=20review=20=E2=80=94=20?= =?UTF-8?q?restore=20dict=20alias=20docs,=20add=20output=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Re-add alias to object-format docs in dependencies.md (dict alias: field is still supported, only shorthand @alias was removed) - Add 11 unit tests for install output formatting: resolved ref display, cached-path ref formatting, unpinned deps detection - Update PR description to document @alias removal scope Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/src/content/docs/guides/dependencies.md | 7 +- tests/unit/test_install_output.py | 114 +++++++++++++++++++ 2 files changed, 118 insertions(+), 3 deletions(-) create mode 100644 tests/unit/test_install_output.py diff --git a/docs/src/content/docs/guides/dependencies.md b/docs/src/content/docs/guides/dependencies.md index 5d96e995..4b8a5702 100644 --- a/docs/src/content/docs/guides/dependencies.md +++ b/docs/src/content/docs/guides/dependencies.md @@ -97,7 +97,7 @@ dependencies: - ./packages/my-shared-skills # relative to project root - /home/user/repos/my-ai-package # absolute path - # Object format: git URL + sub-path / ref + # Object format: git URL + sub-path / ref / alias - git: https://gitlab.com/acme/coding-standards.git path: instructions/security ref: v2.0 @@ -126,7 +126,7 @@ APM accepts dependencies in two forms: - For nested groups + virtual paths, use the object format below - **Local path** (`./path`, `../path`, `/absolute/path`) — local filesystem package -**Object format** (when you need `path` or `ref` on a git URL): +**Object format** (when you need `path`, `ref`, or `alias` on a git URL): ```yaml dependencies: @@ -136,9 +136,10 @@ dependencies: ref: v2.0 # pin to a tag, branch, or commit - git: git@bitbucket.org:team/rules.git path: prompts/review.prompt.md + alias: review # local alias (controls install directory name) ``` -Fields: `git` (required), `path`, `ref` (all optional). The `git` value is any HTTPS or SSH clone URL. +Fields: `git` (required), `path`, `ref`, `alias` (all optional). The `git` value is any HTTPS or SSH clone URL. > **Nested groups (GitLab, Gitea, etc.):** APM treats all path segments after the host as the repo path, so `gitlab.com/group/subgroup/repo` resolves to a repo at `group/subgroup/repo`. Virtual paths on simple 2-segment repos work with shorthand (`gitlab.com/owner/repo/file.prompt.md`). But for **nested-group repos + virtual paths**, use the object format — the shorthand is ambiguous: > diff --git a/tests/unit/test_install_output.py b/tests/unit/test_install_output.py new file mode 100644 index 00000000..fe445a9f --- /dev/null +++ b/tests/unit/test_install_output.py @@ -0,0 +1,114 @@ +"""Tests for install command output formatting: resolved refs and pinning hints.""" + +import pytest +from unittest.mock import MagicMock + +from apm_cli.models.dependency import ( + DependencyReference, + ResolvedReference, + GitReferenceType, +) + + +class TestInstallOutputFormatting: + """Test the formatting logic used in install output messages.""" + + def test_resolved_reference_str_tag(self): + """ResolvedReference for a tag shows ref name and short SHA.""" + ref = ResolvedReference( + original_ref="v1.0.0", + ref_type=GitReferenceType.TAG, + resolved_commit="a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2", + ref_name="v1.0.0", + ) + assert str(ref) == "v1.0.0 (a1b2c3d4)" + + def test_resolved_reference_str_branch(self): + """ResolvedReference for a branch shows ref name and short SHA.""" + ref = ResolvedReference( + original_ref="main", + ref_type=GitReferenceType.BRANCH, + resolved_commit="deadbeef12345678deadbeef12345678deadbeef", + ref_name="main", + ) + assert str(ref) == "main (deadbeef)" + + def test_resolved_reference_str_commit(self): + """ResolvedReference for a commit shows only short SHA.""" + ref = ResolvedReference( + original_ref="a1b2c3d4", + ref_type=GitReferenceType.COMMIT, + resolved_commit="a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2", + ref_name="a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2", + ) + assert str(ref) == "a1b2c3d4" + + +class TestCachedRefFormatting: + """Test the cached-path ref formatting logic (mirrors install.py inline code).""" + + @staticmethod + def _format_cached_ref(dep_ref, locked_dep): + """Reproduce the cached-path ref formatting from install.py.""" + ref_str = "" + if locked_dep and locked_dep.resolved_commit and locked_dep.resolved_commit != "cached": + short_sha = locked_dep.resolved_commit[:8] + if dep_ref.reference: + ref_str = f" @ {dep_ref.reference} ({short_sha})" + else: + ref_str = f" @ {short_sha}" + elif dep_ref.reference: + ref_str = f" @ {dep_ref.reference}" + return ref_str + + def test_cached_with_lockfile_and_ref(self): + """Cached dep with lockfile SHA and user ref shows both.""" + dep = DependencyReference.parse("owner/repo#v1.0.0") + locked = MagicMock(resolved_commit="a1b2c3d4e5f6a1b2") + result = self._format_cached_ref(dep, locked) + assert result == " @ v1.0.0 (a1b2c3d4)" + + def test_cached_with_lockfile_no_ref(self): + """Cached dep with lockfile SHA but no user ref shows SHA only.""" + dep = DependencyReference.parse("owner/repo") + locked = MagicMock(resolved_commit="deadbeef12345678") + result = self._format_cached_ref(dep, locked) + assert result == " @ deadbeef" + + def test_cached_no_lockfile_with_ref(self): + """Cached dep without lockfile shows user ref only.""" + dep = DependencyReference.parse("owner/repo#main") + result = self._format_cached_ref(dep, None) + assert result == " @ main" + + def test_cached_no_lockfile_no_ref(self): + """Cached dep without lockfile and no ref shows nothing.""" + dep = DependencyReference.parse("owner/repo") + result = self._format_cached_ref(dep, None) + assert result == "" + + def test_cached_lockfile_marked_as_cached(self): + """Lockfile with resolved_commit='cached' falls through to user ref.""" + dep = DependencyReference.parse("owner/repo#v2.0") + locked = MagicMock(resolved_commit="cached") + result = self._format_cached_ref(dep, locked) + assert result == " @ v2.0" + + +class TestUnpinnedDepsDetection: + """Test the has_unpinned_deps tracking logic.""" + + def test_dep_with_ref_is_pinned(self): + """Dependencies with explicit ref are not unpinned.""" + dep = DependencyReference.parse("owner/repo#v1.0.0") + assert dep.reference is not None # has_unpinned_deps would NOT be set + + def test_dep_without_ref_is_unpinned(self): + """Dependencies without explicit ref are unpinned.""" + dep = DependencyReference.parse("owner/repo") + assert dep.reference is None # has_unpinned_deps WOULD be set + + def test_branch_ref_is_still_pinned_for_hint(self): + """Branch refs count as 'pinned' for the hint (user explicitly chose a ref).""" + dep = DependencyReference.parse("owner/repo#main") + assert dep.reference == "main" # has_unpinned_deps would NOT be set