diff --git a/libs/beacon/src/beacon/data/skills/contribute-warehouse/scripts/summarize_changes.py b/libs/beacon/src/beacon/data/skills/contribute-warehouse/scripts/summarize_changes.py index 733351f..5ee891d 100644 --- a/libs/beacon/src/beacon/data/skills/contribute-warehouse/scripts/summarize_changes.py +++ b/libs/beacon/src/beacon/data/skills/contribute-warehouse/scripts/summarize_changes.py @@ -40,26 +40,55 @@ # ───────────────────────────────────────────────────────────────────────────── +def _git_tracked_or_staged_deleted(warehouse_path: Path, pathspec: str) -> set[str]: + """Return tracked paths matching pathspec, INCLUDING staged-for-deletion ones.""" + found = set() + rc, stdout, _ = _run_git(warehouse_path, ["ls-files", "--", pathspec]) + if rc == 0: + for line in stdout.strip().splitlines(): + if line and ".git" not in Path(line).parts: + found.add(line) + rc, stdout, _ = _run_git( + warehouse_path, + ["diff", "--cached", "--name-only", "--diff-filter=D", "--", pathspec], + ) + if rc == 0: + for line in stdout.strip().splitlines(): + if line and ".git" not in Path(line).parts: + found.add(line) + return found + + def _expand_pattern(warehouse_path: Path, pattern: str) -> list[str]: """Expand a beacon.yaml pattern to concrete relative paths.""" if "*" in pattern or "?" in pattern: + # Glob finds existing files (including untracked) matches = glob.glob(str(warehouse_path / pattern), recursive=True) - return [ + paths = { str(Path(m).relative_to(warehouse_path)) for m in matches if Path(m).is_file() and ".git" not in Path(m).relative_to(warehouse_path).parts - ] + } + # Supplement with tracked files, including staged deletions + paths |= _git_tracked_or_staged_deleted(warehouse_path, pattern) + return sorted(paths) p = warehouse_path / pattern - if p.is_dir(): - matches = glob.glob(str(p / "**" / "*"), recursive=True) - return [ + # Treat as directory pattern if it ends with '/' OR the path exists as a dir. + # The '/' suffix is the beacon.yaml convention for directories; we must also + # handle the case where git rm has removed an empty directory from disk. + if pattern.endswith("/") or p.is_dir(): + matches = glob.glob(str(p / "**" / "*"), recursive=True) if p.exists() else [] + paths = { str(Path(m).relative_to(warehouse_path)) for m in matches if Path(m).is_file() and ".git" not in Path(m).relative_to(warehouse_path).parts - ] + } + # Supplement with tracked files, including staged deletions + paths |= _git_tracked_or_staged_deleted(warehouse_path, pattern) + return sorted(paths) if p.is_file(): return [pattern] diff --git a/libs/beacon/src/beacon/domains/warehouse/_tracked_paths.py b/libs/beacon/src/beacon/domains/warehouse/_tracked_paths.py index d5e0f8f..747d326 100644 --- a/libs/beacon/src/beacon/domains/warehouse/_tracked_paths.py +++ b/libs/beacon/src/beacon/domains/warehouse/_tracked_paths.py @@ -1,11 +1,22 @@ """Internal helpers for expanding beacon.yaml tracked paths.""" import glob +import subprocess from pathlib import Path from beacon.core.manifest.beacon import BeaconManifest +def _run_git(warehouse_path: Path, args: list[str]) -> tuple[int, str, str]: + """Run a git command inside warehouse, return (returncode, stdout, stderr).""" + result = subprocess.run( + ["git", "-C", str(warehouse_path)] + args, + capture_output=True, + text=True, + ) + return result.returncode, result.stdout, result.stderr + + def get_tracked_paths(warehouse_path: Path, beacon_yaml: Path) -> list[str]: """Return the list of beacon.yaml-matched paths relative to warehouse root.""" if not beacon_yaml.exists(): @@ -30,26 +41,55 @@ def get_tracked_paths(warehouse_path: Path, beacon_yaml: Path) -> list[str]: return paths +def _git_tracked_or_staged_deleted(warehouse_path: Path, pathspec: str) -> set[str]: + """Return tracked paths matching pathspec, INCLUDING staged-for-deletion ones.""" + found = set() + rc, stdout, _ = _run_git(warehouse_path, ["ls-files", "--", pathspec]) + if rc == 0: + for line in stdout.strip().splitlines(): + if line and ".git" not in Path(line).parts: + found.add(line) + rc, stdout, _ = _run_git( + warehouse_path, + ["diff", "--cached", "--name-only", "--diff-filter=D", "--", pathspec], + ) + if rc == 0: + for line in stdout.strip().splitlines(): + if line and ".git" not in Path(line).parts: + found.add(line) + return found + + def _expand_pattern(warehouse_path: Path, pattern: str) -> list[str]: """Expand a beacon.yaml pattern to concrete relative paths.""" if "*" in pattern or "?" in pattern: + # Glob finds existing files (including untracked) matches = glob.glob(str(warehouse_path / pattern), recursive=True) - return [ + paths = { str(Path(m).relative_to(warehouse_path)) for m in matches if Path(m).is_file() and ".git" not in Path(m).relative_to(warehouse_path).parts - ] + } + # Supplement with tracked files, including staged deletions + paths |= _git_tracked_or_staged_deleted(warehouse_path, pattern) + return sorted(paths) p = warehouse_path / pattern - if p.is_dir(): - matches = glob.glob(str(p / "**" / "*"), recursive=True) - return [ + # Treat as directory pattern if it ends with '/' OR the path exists as a dir. + # The '/' suffix is the beacon.yaml convention for directories; we must also + # handle the case where git rm has removed an empty directory from disk. + if pattern.endswith("/") or p.is_dir(): + matches = glob.glob(str(p / "**" / "*"), recursive=True) if p.exists() else [] + paths = { str(Path(m).relative_to(warehouse_path)) for m in matches if Path(m).is_file() and ".git" not in Path(m).relative_to(warehouse_path).parts - ] + } + # Supplement with tracked files, including staged deletions + paths |= _git_tracked_or_staged_deleted(warehouse_path, pattern) + return sorted(paths) if p.is_file(): return [pattern] diff --git a/libs/beacon/tests/unit/data/skills/contribute_warehouse/test_summarize_changes.py b/libs/beacon/tests/unit/data/skills/contribute_warehouse/test_summarize_changes.py index 206f49b..879e9f7 100644 --- a/libs/beacon/tests/unit/data/skills/contribute_warehouse/test_summarize_changes.py +++ b/libs/beacon/tests/unit/data/skills/contribute_warehouse/test_summarize_changes.py @@ -524,6 +524,167 @@ def test_no_beacon_package_imports(self): ) +# ───────────────────────────────────────────────────────────────────────────── +# PER-186: deleted-but-tracked files must appear in summary +# ───────────────────────────────────────────────────────────────────────────── + + +class TestPer186DeletedTrackedFiles: + """PER-186: deleted-but-tracked files must appear in summary.""" + + def test_deleted_glob_match_appears_with_deletion_status(self, tmp_path): + """Tracked file matching glob pattern, deleted → appears with D status.""" + warehouse = _make_git_warehouse(tmp_path) + beacon_yaml = _write_beacon_yaml(warehouse, ["contexts/*.md"]) + + ctx_file = warehouse / "contexts" / "to-delete.md" + ctx_file.write_text("# To delete\n") + _initial_commit(warehouse, [ctx_file]) + + # Delete the file (unstaged deletion) + ctx_file.unlink() + + mod = _load_script() + result = mod.summarize(warehouse, beacon_yaml) + paths = [e["path"] for e in result["tracked_paths"]] + assert "contexts/to-delete.md" in paths, ( + f"PER-186: deleted tracked file should appear. Got: {paths}" + ) + + entry = next( + e for e in result["tracked_paths"] if e["path"] == "contexts/to-delete.md" + ) + assert "D" in entry["git_status"], ( + f"Expected deletion status, got: {entry['git_status']!r}" + ) + + def test_deleted_explicit_path_appears_with_deletion_status(self, tmp_path): + """Tracked explicit path, deleted → appears with D status.""" + warehouse = _make_git_warehouse(tmp_path) + beacon_yaml = _write_beacon_yaml_full( + warehouse, contexts=["contexts/explicit.md"] + ) + + ctx_file = warehouse / "contexts" / "explicit.md" + ctx_file.write_text("# Explicit\n") + _initial_commit(warehouse, [ctx_file]) + + # Delete the file + ctx_file.unlink() + + mod = _load_script() + result = mod.summarize(warehouse, beacon_yaml) + paths = [e["path"] for e in result["tracked_paths"]] + assert "contexts/explicit.md" in paths, ( + f"PER-186: deleted explicit path should appear. Got: {paths}" + ) + + entry = next( + e for e in result["tracked_paths"] if e["path"] == "contexts/explicit.md" + ) + assert "D" in entry["git_status"], ( + f"Expected deletion status, got: {entry['git_status']!r}" + ) + + def test_is_dirty_classifies_unstaged_deletion(self, tmp_path): + """PER-186: is_dirty must classify ' D' as dirty.""" + mod = _load_script() + assert mod.is_dirty(" D") is True + + def test_is_dirty_classifies_staged_deletion(self, tmp_path): + """PER-186: is_dirty must classify 'D ' as dirty.""" + mod = _load_script() + assert mod.is_dirty("D ") is True + + def test_staged_deletion_glob_match_appears(self, tmp_path): + """Tracked file matching glob pattern, git-rm staged → appears with D status.""" + warehouse = _make_git_warehouse(tmp_path) + beacon_yaml = _write_beacon_yaml(warehouse, ["contexts/*.md"]) + + ctx_file = warehouse / "contexts" / "to-delete.md" + ctx_file.write_text("# To delete\n") + _initial_commit(warehouse, [ctx_file]) + + # Stage the deletion + subprocess.run( + ["git", "-C", str(warehouse), "rm", "contexts/to-delete.md"], + check=True, + capture_output=True, + ) + + mod = _load_script() + result = mod.summarize(warehouse, beacon_yaml) + paths = [e["path"] for e in result["tracked_paths"]] + assert "contexts/to-delete.md" in paths, ( + f"PER-186 round 2: staged-deleted tracked file should appear. Got: {paths}" + ) + + entry = next( + e for e in result["tracked_paths"] if e["path"] == "contexts/to-delete.md" + ) + assert "D" in entry["git_status"], ( + f"Expected deletion status, got: {entry['git_status']!r}" + ) + + def test_directory_pattern_deletion_appears(self, tmp_path): + """Directory pattern (no glob), file deleted unstaged → appears in summary.""" + warehouse = _make_git_warehouse(tmp_path) + beacon_yaml = _write_beacon_yaml_full(warehouse, contexts=["contexts/"]) + + ctx_file = warehouse / "contexts" / "nested.md" + ctx_file.write_text("# Nested\n") + _initial_commit(warehouse, [ctx_file]) + + # Delete the file (unstaged) + ctx_file.unlink() + + mod = _load_script() + result = mod.summarize(warehouse, beacon_yaml) + paths = [e["path"] for e in result["tracked_paths"]] + assert "contexts/nested.md" in paths, ( + f"PER-186 round 2: deleted file under directory pattern should appear. Got: {paths}" + ) + + entry = next( + e for e in result["tracked_paths"] if e["path"] == "contexts/nested.md" + ) + assert "D" in entry["git_status"], ( + f"Expected deletion status, got: {entry['git_status']!r}" + ) + + def test_directory_pattern_staged_deletion_appears(self, tmp_path): + """Directory pattern (no glob), file git-rm staged → appears in summary.""" + warehouse = _make_git_warehouse(tmp_path) + beacon_yaml = _write_beacon_yaml_full(warehouse, contexts=["contexts/"]) + + ctx_file = warehouse / "contexts" / "staged-delete.md" + ctx_file.write_text("# Staged delete\n") + _initial_commit(warehouse, [ctx_file]) + + # Stage the deletion + subprocess.run( + ["git", "-C", str(warehouse), "rm", "contexts/staged-delete.md"], + check=True, + capture_output=True, + ) + + mod = _load_script() + result = mod.summarize(warehouse, beacon_yaml) + paths = [e["path"] for e in result["tracked_paths"]] + assert "contexts/staged-delete.md" in paths, ( + f"PER-186 round 2: staged-deleted file under directory pattern should appear. Got: {paths}" + ) + + entry = next( + e + for e in result["tracked_paths"] + if e["path"] == "contexts/staged-delete.md" + ) + assert "D" in entry["git_status"], ( + f"Expected deletion status, got: {entry['git_status']!r}" + ) + + # ───────────────────────────────────────────────────────────────────────────── # PER-183: get_tracked_paths must walk artifacts.agents (not just skills + contexts) # ───────────────────────────────────────────────────────────────────────────── diff --git a/libs/beacon/tests/unit/warehouse/test_tracked_paths.py b/libs/beacon/tests/unit/warehouse/test_tracked_paths.py index 91fbc77..5299955 100644 --- a/libs/beacon/tests/unit/warehouse/test_tracked_paths.py +++ b/libs/beacon/tests/unit/warehouse/test_tracked_paths.py @@ -47,6 +47,23 @@ def _section(name: str, items: list[str] | None) -> str: return beacon_yaml +def _init_git(warehouse: Path) -> None: + """Initialize a git repo in warehouse and configure user.""" + import subprocess + + subprocess.run(["git", "init", str(warehouse)], check=True, capture_output=True) + subprocess.run( + ["git", "-C", str(warehouse), "config", "user.email", "test@test.com"], + check=True, + capture_output=True, + ) + subprocess.run( + ["git", "-C", str(warehouse), "config", "user.name", "Test"], + check=True, + capture_output=True, + ) + + class TestGetTrackedPathsArtifactsCoverage: """PER-183: walking artifacts.skills + contexts + agents — all three.""" @@ -122,3 +139,165 @@ def test_missing_beacon_yaml_returns_empty(self, tmp_path): wh.mkdir() result = get_tracked_paths(wh, wh / "no-such-beacon.yaml") assert result == [] + + +class TestPer186DeletedTrackedFiles: + """PER-186: deleted-but-tracked files must be included in expansion.""" + + def test_deleted_glob_match_included(self, tmp_path): + """Tracked file matching glob pattern, then deleted → still included.""" + import subprocess + + wh = tmp_path / "warehouse" + (wh / "contexts").mkdir(parents=True) + ctx_file = wh / "contexts" / "deleted.md" + ctx_file.write_text("# Deleted\n") + + _init_git(wh) + subprocess.run( + ["git", "-C", str(wh), "add", "contexts/deleted.md"], + check=True, + capture_output=True, + ) + subprocess.run( + ["git", "-C", str(wh), "commit", "-m", "initial"], + check=True, + capture_output=True, + ) + + # Delete the file + ctx_file.unlink() + + beacon_yaml = _write_beacon_yaml(wh, contexts=["contexts/*.md"]) + result = get_tracked_paths(wh, beacon_yaml) + assert "contexts/deleted.md" in result, ( + f"PER-186: deleted tracked file should be included. Got: {result}" + ) + + def test_deleted_explicit_path_included(self, tmp_path): + """Tracked explicit path, then deleted → still included.""" + import subprocess + + wh = tmp_path / "warehouse" + (wh / "contexts").mkdir(parents=True) + ctx_file = wh / "contexts" / "explicit.md" + ctx_file.write_text("# Explicit\n") + + _init_git(wh) + subprocess.run( + ["git", "-C", str(wh), "add", "contexts/explicit.md"], + check=True, + capture_output=True, + ) + subprocess.run( + ["git", "-C", str(wh), "commit", "-m", "initial"], + check=True, + capture_output=True, + ) + + # Delete the file + ctx_file.unlink() + + beacon_yaml = _write_beacon_yaml(wh, contexts=["contexts/explicit.md"]) + result = get_tracked_paths(wh, beacon_yaml) + assert "contexts/explicit.md" in result, ( + f"PER-186: deleted explicit path should be included. Got: {result}" + ) + + def test_staged_deletion_glob_match_included(self, tmp_path): + """Tracked file matching glob pattern, git-rm staged → still included.""" + import subprocess + + wh = tmp_path / "warehouse" + (wh / "contexts").mkdir(parents=True) + ctx_file = wh / "contexts" / "staged-delete.md" + ctx_file.write_text("# Staged delete\n") + + _init_git(wh) + subprocess.run( + ["git", "-C", str(wh), "add", "contexts/staged-delete.md"], + check=True, + capture_output=True, + ) + subprocess.run( + ["git", "-C", str(wh), "commit", "-m", "initial"], + check=True, + capture_output=True, + ) + + # Stage the deletion + subprocess.run( + ["git", "-C", str(wh), "rm", "contexts/staged-delete.md"], + check=True, + capture_output=True, + ) + + beacon_yaml = _write_beacon_yaml(wh, contexts=["contexts/*.md"]) + result = get_tracked_paths(wh, beacon_yaml) + assert "contexts/staged-delete.md" in result, ( + f"PER-186 round 2: staged-deleted tracked file should be included. Got: {result}" + ) + + def test_directory_pattern_deletion_included(self, tmp_path): + """Directory pattern (no glob), file deleted unstaged → included.""" + import subprocess + + wh = tmp_path / "warehouse" + (wh / "contexts").mkdir(parents=True) + ctx_file = wh / "contexts" / "nested.md" + ctx_file.write_text("# Nested\n") + + _init_git(wh) + subprocess.run( + ["git", "-C", str(wh), "add", "contexts/nested.md"], + check=True, + capture_output=True, + ) + subprocess.run( + ["git", "-C", str(wh), "commit", "-m", "initial"], + check=True, + capture_output=True, + ) + + # Delete the file (unstaged) + ctx_file.unlink() + + beacon_yaml = _write_beacon_yaml(wh, contexts=["contexts/"]) + result = get_tracked_paths(wh, beacon_yaml) + assert "contexts/nested.md" in result, ( + f"PER-186 round 2: deleted file under directory pattern should be included. Got: {result}" + ) + + def test_directory_pattern_staged_deletion_included(self, tmp_path): + """Directory pattern (no glob), file git-rm staged → included.""" + import subprocess + + wh = tmp_path / "warehouse" + (wh / "contexts").mkdir(parents=True) + ctx_file = wh / "contexts" / "dir-staged.md" + ctx_file.write_text("# Dir staged\n") + + _init_git(wh) + subprocess.run( + ["git", "-C", str(wh), "add", "contexts/dir-staged.md"], + check=True, + capture_output=True, + ) + subprocess.run( + ["git", "-C", str(wh), "commit", "-m", "initial"], + check=True, + capture_output=True, + ) + + # Stage the deletion + subprocess.run( + ["git", "-C", str(wh), "rm", "contexts/dir-staged.md"], + check=True, + capture_output=True, + ) + + beacon_yaml = _write_beacon_yaml(wh, contexts=["contexts/"]) + result = get_tracked_paths(wh, beacon_yaml) + assert "contexts/dir-staged.md" in result, ( + f"PER-186 round 2: staged-deleted file under directory pattern should be included. Got: {result}" + )