From 1bc639715cd194fc80b975d1e36ccef4366c8fcc Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Sun, 22 Mar 2026 17:52:45 +0100 Subject: [PATCH 1/3] fix: resolve both sides of relative_to() for Windows 8.3 path compat MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On Windows CI runners, Path.resolve() expands 8.3 short names (RUNNER~1 → runneradmin). If only base_dir is resolved but agents_path is not, relative_to() raises ValueError and falls back to absolute paths. Fix: call .resolve() on both sides of relative_to() in _generate_placement_summary and _generate_distributed_summary. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/apm_cli/compilation/agents_compiler.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/apm_cli/compilation/agents_compiler.py b/src/apm_cli/compilation/agents_compiler.py index 17fb2a27..0df0ec41 100644 --- a/src/apm_cli/compilation/agents_compiler.py +++ b/src/apm_cli/compilation/agents_compiler.py @@ -838,7 +838,7 @@ def _generate_placement_summary(self, distributed_result) -> str: for placement in distributed_result.placements: try: - rel_path = placement.agents_path.relative_to(self.base_dir.resolve()).as_posix() + rel_path = placement.agents_path.resolve().relative_to(self.base_dir.resolve()).as_posix() except ValueError: rel_path = str(placement.agents_path) lines.append(f"{rel_path}") @@ -868,7 +868,7 @@ def _generate_distributed_summary(self, distributed_result, config: CompilationC for placement in distributed_result.placements: try: - rel_path = placement.agents_path.relative_to(self.base_dir.resolve()).as_posix() + rel_path = placement.agents_path.resolve().relative_to(self.base_dir.resolve()).as_posix() except ValueError: rel_path = str(placement.agents_path) lines.append(f"- {rel_path} ({len(placement.instructions)} instructions)") From dfbdde71e4b02a823404daefc48bb8361fa4fa12 Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Sun, 22 Mar 2026 22:17:58 +0100 Subject: [PATCH 2/3] fix: show auth diagnostics in --verbose for virtual package validation When virtual package validation fails (subdirectory skills, files, collections), running with --verbose now shows the same auth resolution details and actionable error context that non-virtual packages already showed. Previously, --verbose produced no additional output for virtual packages despite the hint suggesting it would. Also reuses the existing AuthResolver instance instead of creating a new one in the downloader, ensuring consistent auth state. Closes #413 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/apm_cli/commands/install.py | 17 +++++++++++-- tests/unit/test_install_command.py | 40 ++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 2 deletions(-) diff --git a/src/apm_cli/commands/install.py b/src/apm_cli/commands/install.py index 71786f5e..840669ca 100644 --- a/src/apm_cli/commands/install.py +++ b/src/apm_cli/commands/install.py @@ -247,8 +247,21 @@ def _validate_package_exists(package, verbose=False): # For virtual packages, use the downloader's validation method if dep_ref.is_virtual: - downloader = GitHubPackageDownloader() - return downloader.validate_virtual_package_exists(dep_ref) + host = dep_ref.host or default_host() + org = dep_ref.repo_url.split('/')[0] if dep_ref.repo_url and '/' in dep_ref.repo_url else None + if verbose_log: + ctx = auth_resolver.resolve(host, org=org) + verbose_log(f"Auth resolved: host={host}, org={org}, source={ctx.source}, type={ctx.token_type}") + downloader = GitHubPackageDownloader(auth_resolver=auth_resolver) + result = downloader.validate_virtual_package_exists(dep_ref) + if not result and verbose_log: + try: + err_ctx = auth_resolver.build_error_context(host, f"accessing {package}", org=org) + for line in err_ctx.splitlines(): + verbose_log(line) + except Exception: + pass + return result # For Azure DevOps or GitHub Enterprise (non-github.com hosts), # use the downloader which handles authentication properly diff --git a/tests/unit/test_install_command.py b/tests/unit/test_install_command.py index 4d65946e..ce4fdbaf 100644 --- a/tests/unit/test_install_command.py +++ b/tests/unit/test_install_command.py @@ -314,6 +314,46 @@ def test_verbose_validation_failure_calls_build_error_context(self, mock_urlopen assert call_args[0][0] == "github.com" # host assert call_args[0][1].endswith("owner/repo") # operation + def test_verbose_virtual_package_validation_shows_auth_diagnostics(self): + """When virtual package validation fails in verbose mode, auth diagnostics are shown.""" + from apm_cli.commands.install import _validate_package_exists + + with patch( + "apm_cli.deps.github_downloader.GitHubPackageDownloader.validate_virtual_package_exists", + return_value=False, + ), patch.object( + __import__("apm_cli.core.auth", fromlist=["AuthResolver"]).AuthResolver, + "resolve", + return_value=MagicMock(source="none", token_type="none", token=None), + ), patch.object( + __import__("apm_cli.core.auth", fromlist=["AuthResolver"]).AuthResolver, + "build_error_context", + return_value="Authentication failed for accessing owner/repo/skills/my-skill on github.com.\nNo token available.", + ) as mock_build_ctx: + result = _validate_package_exists("owner/repo/skills/my-skill", verbose=True) + assert result is False + mock_build_ctx.assert_called_once() + call_args = mock_build_ctx.call_args + assert call_args[0][0] == "github.com" # host + assert "owner/repo/skills/my-skill" in call_args[0][1] # operation + + def test_virtual_package_validation_reuses_auth_resolver(self): + """Virtual package validation should pass its AuthResolver to the downloader.""" + from apm_cli.commands.install import _validate_package_exists + + with patch( + "apm_cli.deps.github_downloader.GitHubPackageDownloader.__init__", + return_value=None, + ) as mock_init, patch( + "apm_cli.deps.github_downloader.GitHubPackageDownloader.validate_virtual_package_exists", + return_value=True, + ): + _validate_package_exists("owner/repo/skills/my-skill", verbose=False) + mock_init.assert_called_once() + # The auth_resolver kwarg should be passed (not creating a new one) + _, kwargs = mock_init.call_args + assert "auth_resolver" in kwargs + # --------------------------------------------------------------------------- # Transitive dep parent chain breadcrumb From 8a685110b339df7206ac35677d717b3d4f905035 Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Mon, 23 Mar 2026 08:59:32 +0100 Subject: [PATCH 3/3] refactor: use resolve_for_dep for virtual package auth resolution Address review feedback: use AuthResolver.resolve_for_dep(dep_ref) instead of manually extracting host/org, reducing drift between the virtual and non-virtual validation paths. Also assert resolve_for_dep is called in the test. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/apm_cli/commands/install.py | 2 +- tests/unit/test_install_command.py | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/apm_cli/commands/install.py b/src/apm_cli/commands/install.py index 840669ca..e7a0ca73 100644 --- a/src/apm_cli/commands/install.py +++ b/src/apm_cli/commands/install.py @@ -247,10 +247,10 @@ def _validate_package_exists(package, verbose=False): # For virtual packages, use the downloader's validation method if dep_ref.is_virtual: + ctx = auth_resolver.resolve_for_dep(dep_ref) host = dep_ref.host or default_host() org = dep_ref.repo_url.split('/')[0] if dep_ref.repo_url and '/' in dep_ref.repo_url else None if verbose_log: - ctx = auth_resolver.resolve(host, org=org) verbose_log(f"Auth resolved: host={host}, org={org}, source={ctx.source}, type={ctx.token_type}") downloader = GitHubPackageDownloader(auth_resolver=auth_resolver) result = downloader.validate_virtual_package_exists(dep_ref) diff --git a/tests/unit/test_install_command.py b/tests/unit/test_install_command.py index ce4fdbaf..e5cb15d5 100644 --- a/tests/unit/test_install_command.py +++ b/tests/unit/test_install_command.py @@ -323,15 +323,16 @@ def test_verbose_virtual_package_validation_shows_auth_diagnostics(self): return_value=False, ), patch.object( __import__("apm_cli.core.auth", fromlist=["AuthResolver"]).AuthResolver, - "resolve", + "resolve_for_dep", return_value=MagicMock(source="none", token_type="none", token=None), - ), patch.object( + ) as mock_resolve, patch.object( __import__("apm_cli.core.auth", fromlist=["AuthResolver"]).AuthResolver, "build_error_context", return_value="Authentication failed for accessing owner/repo/skills/my-skill on github.com.\nNo token available.", ) as mock_build_ctx: result = _validate_package_exists("owner/repo/skills/my-skill", verbose=True) assert result is False + mock_resolve.assert_called_once() mock_build_ctx.assert_called_once() call_args = mock_build_ctx.call_args assert call_args[0][0] == "github.com" # host