From 950024c8a95553d35d314a4ae62353f5a036272d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 11 Mar 2026 22:01:36 +0000 Subject: [PATCH 1/4] Initial plan From 30af2eb32e2e558bcf28343afb9517cbf2552980 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 11 Mar 2026 22:10:19 +0000 Subject: [PATCH 2/4] Add detailed file-level logging to unpack command - Add dependency_files and skipped_count to UnpackResult - Show per-dependency file tree in unpack output (tree-style, consistent with install) - Log source -> target header, per-file destinations, and skipped files warning - Add unit tests for new fields and CLI logging output (20 tests, all passing) Co-authored-by: danielmeppiel <51440732+danielmeppiel@users.noreply.github.com> --- src/apm_cli/bundle/unpacker.py | 31 +++-- src/apm_cli/commands/pack.py | 27 +++- tests/unit/test_unpacker.py | 227 ++++++++++++++++++++++++++++++++- 3 files changed, 269 insertions(+), 16 deletions(-) diff --git a/src/apm_cli/bundle/unpacker.py b/src/apm_cli/bundle/unpacker.py index e20e952e..352a817e 100644 --- a/src/apm_cli/bundle/unpacker.py +++ b/src/apm_cli/bundle/unpacker.py @@ -6,7 +6,7 @@ import tempfile from dataclasses import dataclass, field from pathlib import Path -from typing import List +from typing import Dict, List from ..deps.lockfile import LockFile @@ -18,6 +18,8 @@ class UnpackResult: extracted_dir: Path files: List[str] = field(default_factory=list) verified: bool = False + dependency_files: Dict[str, List[str]] = field(default_factory=dict) + skipped_count: int = 0 def unpack_bundle( @@ -93,18 +95,20 @@ def unpack_bundle( "apm.lock in the bundle could not be parsed — the bundle may be corrupt." ) - # Collect all deployed_files from lockfile - all_deployed: list[str] = [] - for dep in lockfile.get_all_dependencies(): - all_deployed.extend(dep.deployed_files) - - # Deduplicate + # Collect deployed_files per dependency and deduplicated global list + dep_file_map: Dict[str, List[str]] = {} seen: set[str] = set() unique_files: list[str] = [] - for f in all_deployed: - if f not in seen: - seen.add(f) - unique_files.append(f) + for dep in lockfile.get_all_dependencies(): + dep_key = dep.repo_url or "unknown" + dep_files: list[str] = [] + for f in dep.deployed_files: + if f not in seen: + seen.add(f) + unique_files.append(f) + dep_files.append(f) + if dep_files: + dep_file_map[dep_key] = dep_files # 3. Verify completeness verified = True @@ -128,11 +132,13 @@ def unpack_bundle( extracted_dir=bundle_path, files=unique_files, verified=verified, + dependency_files=dep_file_map, ) # 4. Copy target files to output_dir (additive, no deletes) output_dir = Path(output_dir) output_dir_resolved = output_dir.resolve() + skipped = 0 for rel_path in unique_files: # Guard against absolute paths or path-traversal entries in deployed_files p = Path(rel_path) @@ -147,6 +153,7 @@ def unpack_bundle( ) src = source_dir / rel_path if not src.exists(): + skipped += 1 continue # skip_verify may allow missing files if src.is_dir(): shutil.copytree(src, dest, dirs_exist_ok=True) @@ -158,6 +165,8 @@ def unpack_bundle( extracted_dir=bundle_path, files=unique_files, verified=verified, + dependency_files=dep_file_map, + skipped_count=skipped, ) finally: # Clean up temp dir if we created one diff --git a/src/apm_cli/commands/pack.py b/src/apm_cli/commands/pack.py index ccd9f23e..a0e5864c 100644 --- a/src/apm_cli/commands/pack.py +++ b/src/apm_cli/commands/pack.py @@ -7,7 +7,7 @@ from ..bundle.packer import pack_bundle from ..bundle.unpacker import unpack_bundle -from ..utils.console import _rich_success, _rich_error, _rich_info, _rich_warning +from ..utils.console import _rich_echo, _rich_success, _rich_error, _rich_info, _rich_warning @click.command(name="pack", help="Create a self-contained bundle from installed dependencies") @@ -82,6 +82,8 @@ def pack_cmd(ctx, fmt, target, archive, output, dry_run): def unpack_cmd(ctx, bundle_path, output, skip_verify, dry_run): """Extract an APM bundle into the project.""" try: + _rich_info(f"Unpacking {bundle_path} -> {output}") + result = unpack_bundle( bundle_path=Path(bundle_path), output_dir=Path(output), @@ -90,11 +92,10 @@ def unpack_cmd(ctx, bundle_path, output, skip_verify, dry_run): ) if dry_run: - _rich_info("Dry run — no files written") + _rich_info("Dry run -- no files written") if result.files: _rich_info(f"Would unpack {len(result.files)} file(s):") - for f in result.files: - click.echo(f" {f}") + _log_unpack_file_list(result) else: _rich_warning("No files in bundle") return @@ -102,9 +103,27 @@ def unpack_cmd(ctx, bundle_path, output, skip_verify, dry_run): if not result.files: _rich_warning("No files were unpacked") else: + _log_unpack_file_list(result) + if result.skipped_count > 0: + _rich_warning( + f" {result.skipped_count} file(s) skipped (missing from bundle)" + ) verified_msg = " (verified)" if result.verified else "" _rich_success(f"Unpacked {len(result.files)} file(s){verified_msg}") except (FileNotFoundError, ValueError) as exc: _rich_error(str(exc)) sys.exit(1) + + +def _log_unpack_file_list(result): + """Log unpacked files grouped by dependency, using tree-style output.""" + if result.dependency_files: + for dep_name, dep_files in result.dependency_files.items(): + _rich_echo(f" {dep_name}", color="cyan") + for f in dep_files: + _rich_echo(f" └─ {f}", color="white") + else: + # Fallback: flat file list (no dependency info) + for f in result.files: + _rich_echo(f" └─ {f}", color="white") diff --git a/tests/unit/test_unpacker.py b/tests/unit/test_unpacker.py index 73a03186..cb9d97ec 100644 --- a/tests/unit/test_unpacker.py +++ b/tests/unit/test_unpacker.py @@ -5,7 +5,7 @@ import pytest -from apm_cli.bundle.unpacker import unpack_bundle +from apm_cli.bundle.unpacker import unpack_bundle, UnpackResult from apm_cli.deps.lockfile import LockFile, LockedDependency @@ -209,3 +209,228 @@ def test_unpack_rejects_traversal_path_in_deployed_files(self, tmp_path): with pytest.raises(ValueError, match="unsafe path"): unpack_bundle(bundle_dir, output, skip_verify=True) + + def test_unpack_dependency_files_single_dep(self, tmp_path): + """dependency_files maps repo_url to its deployed files.""" + deployed = [".github/agents/a.md", ".github/prompts/b.md"] + bundle = _build_bundle_dir(tmp_path, deployed) + output = tmp_path / "target" + output.mkdir() + + result = unpack_bundle(bundle, output) + + assert "owner/repo" in result.dependency_files + assert set(result.dependency_files["owner/repo"]) == set(deployed) + + def test_unpack_dependency_files_multiple_deps(self, tmp_path): + """dependency_files tracks files per dependency when bundle has many.""" + bundle_dir = tmp_path / "bundle" / "multi-pkg" + bundle_dir.mkdir(parents=True) + + files_a = [".github/agents/a.md"] + files_b = [".github/prompts/b.md", ".github/instructions/c.md"] + for f in files_a + files_b: + p = bundle_dir / f + p.parent.mkdir(parents=True, exist_ok=True) + p.write_text(f"content of {f}") + + lockfile = LockFile() + lockfile.add_dependency( + LockedDependency(repo_url="org/repo-a", deployed_files=files_a) + ) + lockfile.add_dependency( + LockedDependency(repo_url="org/repo-b", deployed_files=files_b) + ) + lockfile.write(bundle_dir / "apm.lock") + + output = tmp_path / "target" + output.mkdir() + result = unpack_bundle(bundle_dir, output) + + assert result.dependency_files["org/repo-a"] == files_a + assert set(result.dependency_files["org/repo-b"]) == set(files_b) + assert len(result.files) == 3 + + def test_unpack_dependency_files_dry_run(self, tmp_path): + """dependency_files is populated even in dry-run mode.""" + deployed = [".github/agents/a.md"] + bundle = _build_bundle_dir(tmp_path, deployed) + output = tmp_path / "target" + output.mkdir() + + result = unpack_bundle(bundle, output, dry_run=True) + + assert "owner/repo" in result.dependency_files + assert result.dependency_files["owner/repo"] == deployed + + def test_unpack_skipped_count(self, tmp_path): + """skipped_count tracks files missing from bundle when skip_verify.""" + deployed = [".github/agents/a.md", ".github/agents/missing.md"] + bundle_dir = tmp_path / "bundle" / "test-pkg" + bundle_dir.mkdir(parents=True) + + (bundle_dir / ".github" / "agents").mkdir(parents=True) + (bundle_dir / ".github" / "agents" / "a.md").write_text("ok") + + lockfile = LockFile() + lockfile.add_dependency( + LockedDependency(repo_url="owner/repo", deployed_files=deployed) + ) + lockfile.write(bundle_dir / "apm.lock") + + output = tmp_path / "target" + output.mkdir() + + result = unpack_bundle(bundle_dir, output, skip_verify=True) + + assert result.skipped_count == 1 + assert (output / ".github" / "agents" / "a.md").exists() + + def test_unpack_skipped_count_zero_when_all_present(self, tmp_path): + """skipped_count is zero when all files are present.""" + deployed = [".github/agents/a.md"] + bundle = _build_bundle_dir(tmp_path, deployed) + output = tmp_path / "target" + output.mkdir() + + result = unpack_bundle(bundle, output) + + assert result.skipped_count == 0 + + +class TestUnpackCmdLogging: + """Verify CLI output for the unpack command.""" + + def test_unpack_cmd_logs_file_list(self, tmp_path): + """unpack command outputs each file under its dependency name.""" + from click.testing import CliRunner + from apm_cli.commands.pack import unpack_cmd + import os + + deployed = [".github/agents/a.md", ".github/prompts/b.md"] + bundle = _build_bundle_dir(tmp_path, deployed) + output = tmp_path / "target" + output.mkdir() + + runner = CliRunner() + original_dir = os.getcwd() + try: + result = runner.invoke( + unpack_cmd, [str(bundle), "-o", str(output)], catch_exceptions=False + ) + finally: + os.chdir(original_dir) + + assert result.exit_code == 0 + assert "Unpacking" in result.output + assert "owner/repo" in result.output + assert ".github/agents/a.md" in result.output + assert ".github/prompts/b.md" in result.output + assert "Unpacked 2 file(s)" in result.output + + def test_unpack_cmd_dry_run_logs_files(self, tmp_path): + """Dry-run output includes per-dependency file listing.""" + from click.testing import CliRunner + from apm_cli.commands.pack import unpack_cmd + import os + + deployed = [".github/agents/a.md"] + bundle = _build_bundle_dir(tmp_path, deployed) + output = tmp_path / "target" + output.mkdir() + + runner = CliRunner() + original_dir = os.getcwd() + try: + result = runner.invoke( + unpack_cmd, + [str(bundle), "-o", str(output), "--dry-run"], + catch_exceptions=False, + ) + finally: + os.chdir(original_dir) + + assert result.exit_code == 0 + assert "Dry run" in result.output + assert "Would unpack 1 file(s)" in result.output + assert ".github/agents/a.md" in result.output + + def test_unpack_cmd_logs_skipped_files(self, tmp_path): + """Skipped files warning appears when skip_verify allows missing files.""" + from click.testing import CliRunner + from apm_cli.commands.pack import unpack_cmd + import os + + deployed = [".github/agents/a.md", ".github/agents/missing.md"] + bundle_dir = tmp_path / "bundle" / "test-pkg" + bundle_dir.mkdir(parents=True) + + (bundle_dir / ".github" / "agents").mkdir(parents=True) + (bundle_dir / ".github" / "agents" / "a.md").write_text("ok") + + lockfile = LockFile() + lockfile.add_dependency( + LockedDependency(repo_url="owner/repo", deployed_files=deployed) + ) + lockfile.write(bundle_dir / "apm.lock") + + output = tmp_path / "target" + output.mkdir() + + runner = CliRunner() + original_dir = os.getcwd() + try: + result = runner.invoke( + unpack_cmd, + [str(bundle_dir), "-o", str(output), "--skip-verify"], + catch_exceptions=False, + ) + finally: + os.chdir(original_dir) + + assert result.exit_code == 0 + assert "1 file(s) skipped" in result.output + + def test_unpack_cmd_multi_dep_logging(self, tmp_path): + """Multiple dependencies are each logged with their file lists.""" + from click.testing import CliRunner + from apm_cli.commands.pack import unpack_cmd + import os + + bundle_dir = tmp_path / "bundle" / "multi-pkg" + bundle_dir.mkdir(parents=True) + + files_a = [".github/agents/a.md"] + files_b = [".github/prompts/b.md"] + for f in files_a + files_b: + p = bundle_dir / f + p.parent.mkdir(parents=True, exist_ok=True) + p.write_text(f"content of {f}") + + lockfile = LockFile() + lockfile.add_dependency( + LockedDependency(repo_url="org/repo-a", deployed_files=files_a) + ) + lockfile.add_dependency( + LockedDependency(repo_url="org/repo-b", deployed_files=files_b) + ) + lockfile.write(bundle_dir / "apm.lock") + + output = tmp_path / "target" + output.mkdir() + + runner = CliRunner() + original_dir = os.getcwd() + try: + result = runner.invoke( + unpack_cmd, [str(bundle_dir), "-o", str(output)], catch_exceptions=False + ) + finally: + os.chdir(original_dir) + + assert result.exit_code == 0 + assert "org/repo-a" in result.output + assert "org/repo-b" in result.output + assert ".github/agents/a.md" in result.output + assert ".github/prompts/b.md" in result.output + assert "Unpacked 2 file(s)" in result.output From 284c0eeaa45edcdde21a849602cf900667e82426 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 11 Mar 2026 22:11:43 +0000 Subject: [PATCH 3/4] Address code review: fix dep_files tracking, use em-dash and arrow consistently Co-authored-by: danielmeppiel <51440732+danielmeppiel@users.noreply.github.com> --- src/apm_cli/bundle/unpacker.py | 2 +- src/apm_cli/commands/pack.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/apm_cli/bundle/unpacker.py b/src/apm_cli/bundle/unpacker.py index 352a817e..f136e7a1 100644 --- a/src/apm_cli/bundle/unpacker.py +++ b/src/apm_cli/bundle/unpacker.py @@ -103,10 +103,10 @@ def unpack_bundle( dep_key = dep.repo_url or "unknown" dep_files: list[str] = [] for f in dep.deployed_files: + dep_files.append(f) if f not in seen: seen.add(f) unique_files.append(f) - dep_files.append(f) if dep_files: dep_file_map[dep_key] = dep_files diff --git a/src/apm_cli/commands/pack.py b/src/apm_cli/commands/pack.py index a0e5864c..72fae82e 100644 --- a/src/apm_cli/commands/pack.py +++ b/src/apm_cli/commands/pack.py @@ -82,7 +82,7 @@ def pack_cmd(ctx, fmt, target, archive, output, dry_run): def unpack_cmd(ctx, bundle_path, output, skip_verify, dry_run): """Extract an APM bundle into the project.""" try: - _rich_info(f"Unpacking {bundle_path} -> {output}") + _rich_info(f"Unpacking {bundle_path} → {output}") result = unpack_bundle( bundle_path=Path(bundle_path), @@ -92,7 +92,7 @@ def unpack_cmd(ctx, bundle_path, output, skip_verify, dry_run): ) if dry_run: - _rich_info("Dry run -- no files written") + _rich_info("Dry run — no files written") if result.files: _rich_info(f"Would unpack {len(result.files)} file(s):") _log_unpack_file_list(result) From b574d323850b9a03aa0d527bb2c206116708c82f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 11 Mar 2026 22:22:00 +0000 Subject: [PATCH 4/4] Address PR review: use get_unique_key() for virtual deps, remove unused import, add virtual dep test Co-authored-by: danielmeppiel <51440732+danielmeppiel@users.noreply.github.com> --- src/apm_cli/bundle/unpacker.py | 2 +- tests/unit/test_unpacker.py | 43 +++++++++++++++++++++++++++++++++- 2 files changed, 43 insertions(+), 2 deletions(-) diff --git a/src/apm_cli/bundle/unpacker.py b/src/apm_cli/bundle/unpacker.py index f136e7a1..a851bd57 100644 --- a/src/apm_cli/bundle/unpacker.py +++ b/src/apm_cli/bundle/unpacker.py @@ -100,7 +100,7 @@ def unpack_bundle( seen: set[str] = set() unique_files: list[str] = [] for dep in lockfile.get_all_dependencies(): - dep_key = dep.repo_url or "unknown" + dep_key = dep.get_unique_key() dep_files: list[str] = [] for f in dep.deployed_files: dep_files.append(f) diff --git a/tests/unit/test_unpacker.py b/tests/unit/test_unpacker.py index cb9d97ec..135280e5 100644 --- a/tests/unit/test_unpacker.py +++ b/tests/unit/test_unpacker.py @@ -5,7 +5,7 @@ import pytest -from apm_cli.bundle.unpacker import unpack_bundle, UnpackResult +from apm_cli.bundle.unpacker import unpack_bundle from apm_cli.deps.lockfile import LockFile, LockedDependency @@ -251,6 +251,47 @@ def test_unpack_dependency_files_multiple_deps(self, tmp_path): assert set(result.dependency_files["org/repo-b"]) == set(files_b) assert len(result.files) == 3 + def test_unpack_dependency_files_virtual_deps(self, tmp_path): + """Virtual deps from the same repo are tracked separately.""" + bundle_dir = tmp_path / "bundle" / "virtual-pkg" + bundle_dir.mkdir(parents=True) + + files_a = [".github/agents/a.md"] + files_b = [".github/prompts/b.md"] + for f in files_a + files_b: + p = bundle_dir / f + p.parent.mkdir(parents=True, exist_ok=True) + p.write_text(f"content of {f}") + + lockfile = LockFile() + lockfile.add_dependency( + LockedDependency( + repo_url="org/monorepo", + virtual_path="packages/alpha", + is_virtual=True, + deployed_files=files_a, + ) + ) + lockfile.add_dependency( + LockedDependency( + repo_url="org/monorepo", + virtual_path="packages/beta", + is_virtual=True, + deployed_files=files_b, + ) + ) + lockfile.write(bundle_dir / "apm.lock") + + output = tmp_path / "target" + output.mkdir() + result = unpack_bundle(bundle_dir, output) + + assert "org/monorepo/packages/alpha" in result.dependency_files + assert "org/monorepo/packages/beta" in result.dependency_files + assert result.dependency_files["org/monorepo/packages/alpha"] == files_a + assert result.dependency_files["org/monorepo/packages/beta"] == files_b + assert len(result.files) == 2 + def test_unpack_dependency_files_dry_run(self, tmp_path): """dependency_files is populated even in dry-run mode.""" deployed = [".github/agents/a.md"]