From 617ec07579c0fefa03924fbb2588aa5ce7d4e55d Mon Sep 17 00:00:00 2001 From: Song Yikun Date: Tue, 19 May 2026 19:38:43 +0800 Subject: [PATCH 1/3] feat(doctor): extend abc doctor with project-side symlink + @path integrity checks (PER-193) Add four new check categories to abc doctor, implemented in the setup domain: 1. Symlink hygiene under .agentic-beacon/artifacts/: - Dangling symlinks (target missing in warehouse) - Symlinks pointing outside the configured warehouse path - Regular files where symlinks should be 2. @path reference integrity in project-committed files: - Parse @path references from CLAUDE.md and opencode.json - Verify each target exists locally - Classify cause: missing-from-beacon.yaml / wired-directly / genuinely broken - Flag warehouse-only absolute paths that break for teammates 3. Stale beacon.yaml glob detection: - Entries with glob characters that expand to zero warehouse files 4. Sanity checks: - Warehouse path not a git working tree - Platform warning on Windows - Minimal CLI wiring in cli/diagnostics.py (single domain call) - 31 unit tests with fixture project layouts - Integration test seeds broken links and confirms each is caught - Backward-compatible: existing checks and output format preserved --- libs/beacon/src/beacon/cli/diagnostics.py | 11 + .../src/beacon/domains/setup/diagnostics.py | 361 ++++++++++++++++++ .../tests/integration/test_doctor_command.py | 57 +++ .../unit/domains/setup/test_diagnostics.py | 349 +++++++++++++++++ 4 files changed, 778 insertions(+) create mode 100644 libs/beacon/src/beacon/domains/setup/diagnostics.py create mode 100644 libs/beacon/tests/unit/domains/setup/test_diagnostics.py diff --git a/libs/beacon/src/beacon/cli/diagnostics.py b/libs/beacon/src/beacon/cli/diagnostics.py index de2bc9f7..7a0ded6e 100644 --- a/libs/beacon/src/beacon/cli/diagnostics.py +++ b/libs/beacon/src/beacon/cli/diagnostics.py @@ -7,6 +7,7 @@ from beacon.core.manifest.beacon import BeaconManifest from beacon.core.manifest.workspace import WorkspaceConfig +from beacon.domains.setup.diagnostics import run_project_health_checks from beacon.utils.display import print_doctor_summary from beacon.utils.git import find_project_root @@ -140,4 +141,14 @@ def _err(msg: str, detail: str = "") -> None: f"Context entries: all {len(context_entries)} context(s) exist in warehouse" ) + # Project-side checks (PER-193) + project_issues = run_project_health_checks( + project_root, warehouse_path, beacon_settings + ) + for issue in project_issues: + if issue.severity == "warn": + _warn(issue.message, issue.detail) + else: + _err(issue.message, issue.detail) + print_doctor_summary(issues, fixes_applied) diff --git a/libs/beacon/src/beacon/domains/setup/diagnostics.py b/libs/beacon/src/beacon/domains/setup/diagnostics.py new file mode 100644 index 00000000..cf9de6ae --- /dev/null +++ b/libs/beacon/src/beacon/domains/setup/diagnostics.py @@ -0,0 +1,361 @@ +"""Project-side diagnostic checks for abc doctor. + +Implements checks that need a project root (not a warehouse root): +- Symlink hygiene under .agentic-beacon/artifacts/ +- @path reference integrity in CLAUDE.md / opencode.json +- Stale beacon.yaml globs +- Sanity checks (warehouse git, platform) +""" + +from __future__ import annotations + +import json +import re +import sys +from dataclasses import dataclass +from pathlib import Path + +from beacon.core.manifest.beacon import BeaconManifest + + +@dataclass(frozen=True) +class DoctorIssue: + """A single diagnostic issue with optional detail.""" + + message: str + detail: str = "" + severity: str = "error" # "error" or "warn" + + +def run_project_health_checks( + project_root: Path, + warehouse_path: Path | None, + beacon_manifest: BeaconManifest | None, +) -> list[DoctorIssue]: + """Run all project-side diagnostic checks. + + Returns a list of DoctorIssue records. The caller (CLI layer) decides how + to render them so the domain stays free of Rich/Click. + """ + issues: list[DoctorIssue] = [] + + if warehouse_path is not None: + issues.extend(_check_symlink_hygiene(project_root, warehouse_path)) + issues.extend(_check_stale_globs(warehouse_path, beacon_manifest)) + issues.extend(_check_warehouse_git(warehouse_path)) + + issues.extend(_check_path_references(project_root, beacon_manifest)) + issues.extend(_check_platform()) + + return issues + + +# --------------------------------------------------------------------------- +# Check 1: Symlink hygiene +# --------------------------------------------------------------------------- + + +_GLOB_CHARS = frozenset("*?[]") + + +def _is_glob_pattern(path: str) -> bool: + return any(ch in path for ch in _GLOB_CHARS) + + +def _check_symlink_hygiene( + project_root: Path, warehouse_path: Path +) -> list[DoctorIssue]: + artifacts_path = project_root / ".agentic-beacon" / "artifacts" + if not artifacts_path.exists(): + return [] + + issues: list[DoctorIssue] = [] + resolved_warehouse = warehouse_path.resolve() + + for path in sorted(artifacts_path.rglob("*")): + if path.is_symlink(): + try: + target = path.readlink() + except OSError: + continue + + if target.is_absolute(): + resolved_target = target.resolve() + else: + resolved_target = (path.parent / target).resolve() + + # Dangling symlink + if not resolved_target.exists(): + rel = str(path.relative_to(artifacts_path)) + issues.append( + DoctorIssue( + message=f"Dangling symlink: {rel}", + detail=f"Target missing: {target}", + severity="error", + ) + ) + continue + + # Symlink pointing outside warehouse + try: + resolved_target.relative_to(resolved_warehouse) + except ValueError: + rel = str(path.relative_to(artifacts_path)) + issues.append( + DoctorIssue( + message=f"Symlink outside warehouse: {rel}", + detail=f"Target: {resolved_target}", + severity="error", + ) + ) + + elif path.is_file(): + rel = str(path.relative_to(artifacts_path)) + issues.append( + DoctorIssue( + message=f"Regular file where symlink should be: {rel}", + detail="Expected a symlink to the warehouse", + severity="error", + ) + ) + + return issues + + +# --------------------------------------------------------------------------- +# Check 2: @path references +# --------------------------------------------------------------------------- + + +_ATPATH_RE = re.compile(r"^@(.+)$") + + +def _check_path_references( + project_root: Path, + beacon_manifest: BeaconManifest | None, +) -> list[DoctorIssue]: + issues: list[DoctorIssue] = [] + + # CLAUDE.md + for claude_md in ( + project_root / ".claude" / "CLAUDE.md", + project_root / "CLAUDE.md", + ): + if claude_md.exists(): + issues.extend(_check_claude_md(claude_md, project_root, beacon_manifest)) + + # opencode.json + for opencode_json in ( + project_root / ".opencode" / "opencode.json", + project_root / "opencode.json", + ): + if opencode_json.exists(): + issues.extend( + _check_opencode_json(opencode_json, project_root, beacon_manifest) + ) + + # .claude/settings.json (defensive — not used by current wiring but may exist) + settings_json = project_root / ".claude" / "settings.json" + if settings_json.exists(): + issues.extend( + _check_opencode_json(settings_json, project_root, beacon_manifest) + ) + + return issues + + +def _check_claude_md( + claude_md: Path, + project_root: Path, + beacon_manifest: BeaconManifest | None, +) -> list[DoctorIssue]: + issues: list[DoctorIssue] = [] + try: + content = claude_md.read_text(encoding="utf-8") + except OSError: + return issues + + for line in content.splitlines(): + line = line.strip() + match = _ATPATH_RE.match(line) + if not match: + continue + + raw_path = match.group(1).strip() + if not raw_path: + continue + + issue = _classify_reference( + raw_path, project_root, beacon_manifest, str(claude_md) + ) + if issue: + issues.append(issue) + + return issues + + +def _check_opencode_json( + json_path: Path, + project_root: Path, + beacon_manifest: BeaconManifest | None, +) -> list[DoctorIssue]: + issues: list[DoctorIssue] = [] + try: + data = json.loads(json_path.read_text(encoding="utf-8")) + except (json.JSONDecodeError, OSError): + return issues + + instructions = data.get("instructions", []) if isinstance(data, dict) else [] + if not isinstance(instructions, list): + return issues + + for item in instructions: + if not isinstance(item, str): + continue + if "/" not in item and "\\" not in item: + continue + issue = _classify_reference(item, project_root, beacon_manifest, str(json_path)) + if issue: + issues.append(issue) + + return issues + + +def _classify_reference( + raw_path: str, + project_root: Path, + beacon_manifest: BeaconManifest | None, + source_file: str, +) -> DoctorIssue | None: + """Classify a single @path/reference and return an Issue or None.""" + + # Flag absolute paths that point into the warehouse + if raw_path.startswith("/"): + return DoctorIssue( + message=f"Warehouse-only absolute path in {Path(source_file).name}", + detail=f"{raw_path} will break for other teammates", + severity="error", + ) + + # Resolve relative to project root + candidate = project_root / raw_path + # Also try under artifacts/ if the path looks like a bare artifact reference + if not candidate.exists() and not raw_path.startswith(".agentic-beacon"): + candidate = project_root / ".agentic-beacon" / "artifacts" / raw_path + + if not candidate.exists(): + return DoctorIssue( + message=f"Broken reference in {Path(source_file).name}", + detail=f"{raw_path} does not exist", + severity="error", + ) + + # Path exists locally — check if it's wired in beacon.yaml + if beacon_manifest is not None: + wired = _is_wired_in_beacon(raw_path, beacon_manifest) + if not wired: + return DoctorIssue( + message=f"Unmanaged reference in {Path(source_file).name}", + detail=f"{raw_path} exists but is not listed in beacon.yaml", + severity="warn", + ) + + return None + + +def _is_wired_in_beacon(raw_path: str, beacon_manifest: BeaconManifest) -> bool: + """Return True if the referenced path matches a beacon.yaml entry.""" + + # Normalize: strip leading .agentic-beacon/artifacts/ if present + normalized = raw_path + prefix = ".agentic-beacon/artifacts/" + if normalized.startswith(prefix): + normalized = normalized[len(prefix) :] + + # Exact match against any artifact type + for entry_list in ( + beacon_manifest.artifacts.skills, + beacon_manifest.artifacts.contexts, + beacon_manifest.artifacts.agents, + ): + for entry in entry_list: + entry_stripped = entry.rstrip("/") + norm_stripped = normalized.rstrip("/") + if entry_stripped == norm_stripped: + return True + # Directory entry: path is under the entry + if norm_stripped.startswith(entry_stripped + "/"): + return True + + return False + + +# --------------------------------------------------------------------------- +# Check 3: Stale globs +# --------------------------------------------------------------------------- + + +def _check_stale_globs( + warehouse_path: Path, + beacon_manifest: BeaconManifest | None, +) -> list[DoctorIssue]: + if beacon_manifest is None: + return [] + + issues: list[DoctorIssue] = [] + all_entries: list[tuple[str, list[str]]] = [ + ("skills", beacon_manifest.artifacts.skills), + ("contexts", beacon_manifest.artifacts.contexts), + ("agents", beacon_manifest.artifacts.agents), + ] + + for artifact_type, entries in all_entries: + for entry in entries: + if not _is_glob_pattern(entry): + continue + matches = list(warehouse_path.glob(entry)) + # Filter out .git and non-file entries for files; keep dirs for skill dirs + file_matches = [m for m in matches if m.is_file()] + dir_matches = [m for m in matches if m.is_dir()] + if not file_matches and not dir_matches: + issues.append( + DoctorIssue( + message=f"Stale glob in beacon.yaml ({artifact_type})", + detail=f"'{entry}' matches 0 files in warehouse", + severity="error", + ) + ) + + return issues + + +# --------------------------------------------------------------------------- +# Check 4: Sanity checks +# --------------------------------------------------------------------------- + + +def _check_warehouse_git(warehouse_path: Path) -> list[DoctorIssue]: + issues: list[DoctorIssue] = [] + if not (warehouse_path / ".git").exists(): + issues.append( + DoctorIssue( + message="Warehouse is not a git working tree", + detail=f"{warehouse_path} missing .git directory", + severity="warn", + ) + ) + return issues + + +def _check_platform() -> list[DoctorIssue]: + issues: list[DoctorIssue] = [] + plat = sys.platform + if plat.startswith("win") or plat == "cygwin": + issues.append( + DoctorIssue( + message="Windows platform detected", + detail="Symlink-based artifact sync may not work correctly", + severity="warn", + ) + ) + return issues diff --git a/libs/beacon/tests/integration/test_doctor_command.py b/libs/beacon/tests/integration/test_doctor_command.py index a48d76e1..4031a3e0 100644 --- a/libs/beacon/tests/integration/test_doctor_command.py +++ b/libs/beacon/tests/integration/test_doctor_command.py @@ -1,5 +1,7 @@ """Tests for abc doctor command.""" +import json +import subprocess from pathlib import Path import pytest @@ -13,6 +15,7 @@ def _setup_project( monkeypatch, *, beacon_yaml_content: str = "artifacts:\n contexts: []\n skills: []\n knowledge: []\n", + init_git: bool = True, ) -> tuple[Path, Path]: """Create a connected project and return (project_dir, warehouse_dir).""" project = tmp_path / "project" @@ -25,6 +28,13 @@ def _setup_project( (warehouse / "skills").mkdir() (warehouse / "knowledge").mkdir() + if init_git: + subprocess.run( + ["git", "init", str(warehouse)], + capture_output=True, + check=False, + ) + beacon_dir = project / ".agentic-beacon" beacon_dir.mkdir() # Write config.toml (warehouse connection) @@ -233,6 +243,53 @@ def test_missing_context_flagged(self, tmp_path, monkeypatch): assert "missing" in result.output.lower() or "✗" in result.output +class TestDoctorProjectSideChecks: + def test_catches_multiple_broken_links(self, tmp_path, monkeypatch): + """Seed a project with multiple broken links; doctor catches each.""" + project, warehouse = _setup_project(tmp_path, monkeypatch, init_git=False) + + # 1. Dangling symlink + artifacts = project / ".agentic-beacon" / "artifacts" + dangling = artifacts / "contexts" / "dangling.md" + dangling.parent.mkdir(parents=True) + dangling.symlink_to(warehouse / "contexts" / "dangling.md") + + # 2. Symlink outside warehouse + outside = tmp_path / "outside.md" + outside.write_text("x") + bad_link = artifacts / "contexts" / "outside.md" + bad_link.symlink_to(outside) + + # 3. Regular file where symlink should be + regular = artifacts / "skills" / "regular-skill" / "SKILL.md" + regular.parent.mkdir(parents=True) + regular.write_text("regular file") + + # 4. Broken @path reference in CLAUDE.md + (project / "CLAUDE.md").write_text("@nonexistent.md\n") + + # 5. Broken @path reference in opencode.json + data = {"instructions": [".agentic-beacon/artifacts/contexts/ghost.md"]} + (project / "opencode.json").write_text(json.dumps(data)) + + # 6. Stale glob in beacon.yaml + (project / ".agentic-beacon" / "beacon.yaml").write_text( + "artifacts:\n contexts:\n - contexts/*.md\n skills: []\n agents: []\n" + ) + + runner = CliRunner() + result = runner.invoke(main, ["doctor"]) + assert result.exit_code == 0 + + output = result.output + assert "Dangling symlink" in output + assert "outside warehouse" in output + assert "Regular file where symlink should be" in output + assert "Broken reference" in output + assert "Stale glob" in output + assert "not a git working tree" in output + + class TestDoctorNoProject: def test_no_beacon_dir_reports_error(self, tmp_path, monkeypatch): """Running doctor outside a project reports an error and exits cleanly.""" diff --git a/libs/beacon/tests/unit/domains/setup/test_diagnostics.py b/libs/beacon/tests/unit/domains/setup/test_diagnostics.py new file mode 100644 index 00000000..ff3ed3fc --- /dev/null +++ b/libs/beacon/tests/unit/domains/setup/test_diagnostics.py @@ -0,0 +1,349 @@ +"""Unit tests for beacon.domains.setup.diagnostics.""" + +import json + +from beacon.core.manifest.beacon import BeaconManifest +from beacon.domains.setup.diagnostics import ( + _check_path_references, + _check_platform, + _check_stale_globs, + _check_symlink_hygiene, + _check_warehouse_git, + _is_glob_pattern, + run_project_health_checks, +) + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _make_manifest(**kwargs) -> BeaconManifest: + defaults = {"contexts": [], "skills": [], "agents": []} + defaults.update(kwargs) + return BeaconManifest(artifacts=defaults) + + +# --------------------------------------------------------------------------- +# _is_glob_pattern +# --------------------------------------------------------------------------- + + +class TestIsGlobPattern: + def test_asterisk_is_glob(self): + assert _is_glob_pattern("skills/*") + + def test_question_is_glob(self): + assert _is_glob_pattern("contexts/?.md") + + def test_bracket_is_glob(self): + assert _is_glob_pattern("agents/[ab].md") + + def test_plain_path_is_not_glob(self): + assert not _is_glob_pattern("skills/my-skill/") + + +# --------------------------------------------------------------------------- +# Symlink hygiene +# --------------------------------------------------------------------------- + + +class TestSymlinkHygiene: + def test_no_artifacts_dir_returns_empty(self, tmp_path): + issues = _check_symlink_hygiene(tmp_path, tmp_path / "warehouse") + assert issues == [] + + def test_good_symlink_passes(self, tmp_path): + warehouse = tmp_path / "warehouse" + warehouse.mkdir() + (warehouse / "contexts").mkdir() + (warehouse / "contexts" / "team.md").write_text("# Team") + + artifacts = tmp_path / ".agentic-beacon" / "artifacts" + artifacts.mkdir(parents=True) + ctx_link = artifacts / "contexts" / "team.md" + ctx_link.parent.mkdir(parents=True) + ctx_link.symlink_to(warehouse / "contexts" / "team.md") + + issues = _check_symlink_hygiene(tmp_path, warehouse) + assert issues == [] + + def test_dangling_symlink_flagged(self, tmp_path): + warehouse = tmp_path / "warehouse" + warehouse.mkdir() + + artifacts = tmp_path / ".agentic-beacon" / "artifacts" + artifacts.mkdir(parents=True) + ctx_link = artifacts / "contexts" / "team.md" + ctx_link.parent.mkdir(parents=True) + ctx_link.symlink_to(warehouse / "contexts" / "team.md") + + issues = _check_symlink_hygiene(tmp_path, warehouse) + assert len(issues) == 1 + assert "Dangling symlink" in issues[0].message + assert issues[0].severity == "error" + + def test_symlink_outside_warehouse_flagged(self, tmp_path): + warehouse = tmp_path / "warehouse" + warehouse.mkdir() + other = tmp_path / "other" + other.mkdir() + (other / "file.md").write_text("x") + + artifacts = tmp_path / ".agentic-beacon" / "artifacts" + artifacts.mkdir(parents=True) + ctx_link = artifacts / "contexts" / "team.md" + ctx_link.parent.mkdir(parents=True) + ctx_link.symlink_to(other / "file.md") + + issues = _check_symlink_hygiene(tmp_path, warehouse) + assert len(issues) == 1 + assert "outside warehouse" in issues[0].message + assert issues[0].severity == "error" + + def test_regular_file_where_symlink_should_be(self, tmp_path): + warehouse = tmp_path / "warehouse" + warehouse.mkdir() + + artifacts = tmp_path / ".agentic-beacon" / "artifacts" + artifacts.mkdir(parents=True) + ctx_file = artifacts / "contexts" / "team.md" + ctx_file.parent.mkdir(parents=True) + ctx_file.write_text("regular file") + + issues = _check_symlink_hygiene(tmp_path, warehouse) + assert len(issues) == 1 + assert "Regular file where symlink should be" in issues[0].message + assert issues[0].severity == "error" + + +# --------------------------------------------------------------------------- +# @path references +# --------------------------------------------------------------------------- + + +class TestPathReferences: + def test_claude_md_broken_reference(self, tmp_path): + (tmp_path / "CLAUDE.md").write_text("@nonexistent.md\n") + manifest = _make_manifest() + issues = _check_path_references(tmp_path, manifest) + assert len(issues) == 1 + assert "Broken reference" in issues[0].message + assert issues[0].severity == "error" + + def test_claude_md_unmanaged_reference(self, tmp_path): + (tmp_path / ".agentic-beacon" / "artifacts" / "contexts").mkdir(parents=True) + ( + tmp_path / ".agentic-beacon" / "artifacts" / "contexts" / "team.md" + ).write_text("# Team") + (tmp_path / "CLAUDE.md").write_text( + "@.agentic-beacon/artifacts/contexts/team.md\n" + ) + manifest = _make_manifest(contexts=[]) + issues = _check_path_references(tmp_path, manifest) + assert len(issues) == 1 + assert "Unmanaged reference" in issues[0].message + assert issues[0].severity == "warn" + + def test_claude_md_wired_reference_no_issue(self, tmp_path): + (tmp_path / ".agentic-beacon" / "artifacts" / "contexts").mkdir(parents=True) + ( + tmp_path / ".agentic-beacon" / "artifacts" / "contexts" / "team.md" + ).write_text("# Team") + (tmp_path / "CLAUDE.md").write_text( + "@.agentic-beacon/artifacts/contexts/team.md\n" + ) + manifest = _make_manifest(contexts=["contexts/team.md"]) + issues = _check_path_references(tmp_path, manifest) + assert issues == [] + + def test_claude_md_warehouse_absolute_path_flagged(self, tmp_path): + (tmp_path / "CLAUDE.md").write_text( + "@/Users/someone/warehouse/contexts/team.md\n" + ) + manifest = _make_manifest() + issues = _check_path_references(tmp_path, manifest) + assert len(issues) == 1 + assert "Warehouse-only absolute path" in issues[0].message + assert issues[0].severity == "error" + + def test_opencode_json_broken_reference(self, tmp_path): + data = {"instructions": [".agentic-beacon/artifacts/contexts/ghost.md"]} + (tmp_path / "opencode.json").write_text(json.dumps(data)) + manifest = _make_manifest() + issues = _check_path_references(tmp_path, manifest) + assert len(issues) == 1 + assert "Broken reference" in issues[0].message + + def test_opencode_json_wired_reference_no_issue(self, tmp_path): + (tmp_path / ".agentic-beacon" / "artifacts" / "contexts").mkdir(parents=True) + ( + tmp_path / ".agentic-beacon" / "artifacts" / "contexts" / "team.md" + ).write_text("# Team") + data = {"instructions": [".agentic-beacon/artifacts/contexts/team.md"]} + (tmp_path / "opencode.json").write_text(json.dumps(data)) + manifest = _make_manifest(contexts=["contexts/team.md"]) + issues = _check_path_references(tmp_path, manifest) + assert issues == [] + + def test_opencode_json_non_path_instructions_skipped(self, tmp_path): + data = {"instructions": ["Be helpful", "Use Python"]} + (tmp_path / "opencode.json").write_text(json.dumps(data)) + manifest = _make_manifest() + issues = _check_path_references(tmp_path, manifest) + assert issues == [] + + def test_bare_artifact_path_resolved(self, tmp_path): + (tmp_path / ".agentic-beacon" / "artifacts" / "contexts").mkdir(parents=True) + ( + tmp_path / ".agentic-beacon" / "artifacts" / "contexts" / "team.md" + ).write_text("# Team") + (tmp_path / "CLAUDE.md").write_text("@contexts/team.md\n") + manifest = _make_manifest(contexts=["contexts/team.md"]) + issues = _check_path_references(tmp_path, manifest) + assert issues == [] + + +# --------------------------------------------------------------------------- +# Stale globs +# --------------------------------------------------------------------------- + + +class TestStaleGlobs: + def test_stale_glob_flagged(self, tmp_path): + warehouse = tmp_path / "warehouse" + warehouse.mkdir() + (warehouse / "contexts").mkdir() + + manifest = _make_manifest(contexts=["contexts/*.md"]) + issues = _check_stale_globs(warehouse, manifest) + assert len(issues) == 1 + assert "Stale glob" in issues[0].message + assert "contexts/*.md" in issues[0].detail + assert issues[0].severity == "error" + + def test_active_glob_passes(self, tmp_path): + warehouse = tmp_path / "warehouse" + warehouse.mkdir() + (warehouse / "contexts").mkdir() + (warehouse / "contexts" / "team.md").write_text("# Team") + + manifest = _make_manifest(contexts=["contexts/*.md"]) + issues = _check_stale_globs(warehouse, manifest) + assert issues == [] + + def test_non_glob_entries_ignored(self, tmp_path): + warehouse = tmp_path / "warehouse" + warehouse.mkdir() + + manifest = _make_manifest(skills=["skills/my-skill/"]) + issues = _check_stale_globs(warehouse, manifest) + assert issues == [] + + def test_stale_skill_glob(self, tmp_path): + warehouse = tmp_path / "warehouse" + warehouse.mkdir() + (warehouse / "skills").mkdir() + + manifest = _make_manifest(skills=["skills/code-*/"]) + issues = _check_stale_globs(warehouse, manifest) + assert len(issues) == 1 + assert "skills" in issues[0].message + + def test_no_manifest_returns_empty(self, tmp_path): + warehouse = tmp_path / "warehouse" + warehouse.mkdir() + issues = _check_stale_globs(warehouse, None) + assert issues == [] + + +# --------------------------------------------------------------------------- +# Warehouse git sanity +# --------------------------------------------------------------------------- + + +class TestWarehouseGit: + def test_git_present_no_issue(self, tmp_path): + (tmp_path / ".git").mkdir() + issues = _check_warehouse_git(tmp_path) + assert issues == [] + + def test_no_git_flagged(self, tmp_path): + issues = _check_warehouse_git(tmp_path) + assert len(issues) == 1 + assert "not a git working tree" in issues[0].message + assert issues[0].severity == "warn" + + +# --------------------------------------------------------------------------- +# Platform check +# --------------------------------------------------------------------------- + + +class TestPlatformCheck: + def test_darwin_no_issue(self, monkeypatch): + monkeypatch.setattr("sys.platform", "darwin") + issues = _check_platform() + assert issues == [] + + def test_linux_no_issue(self, monkeypatch): + monkeypatch.setattr("sys.platform", "linux") + issues = _check_platform() + assert issues == [] + + def test_windows_warns(self, monkeypatch): + monkeypatch.setattr("sys.platform", "win32") + issues = _check_platform() + assert len(issues) == 1 + assert "Windows" in issues[0].message + assert issues[0].severity == "warn" + + def test_cygwin_warns(self, monkeypatch): + monkeypatch.setattr("sys.platform", "cygwin") + issues = _check_platform() + assert len(issues) == 1 + assert "Windows" in issues[0].message + + +# --------------------------------------------------------------------------- +# run_project_health_checks +# --------------------------------------------------------------------------- + + +class TestRunProjectHealthChecks: + def test_all_checks_combined(self, tmp_path): + warehouse = tmp_path / "warehouse" + warehouse.mkdir() + (warehouse / "contexts").mkdir() + + artifacts = tmp_path / ".agentic-beacon" / "artifacts" + artifacts.mkdir(parents=True) + bad_link = artifacts / "contexts" / "ghost.md" + bad_link.parent.mkdir(parents=True) + bad_link.symlink_to(warehouse / "contexts" / "ghost.md") + + (tmp_path / "CLAUDE.md").write_text("@nonexistent.md\n") + + manifest = _make_manifest(contexts=["contexts/*.md"]) + issues = run_project_health_checks(tmp_path, warehouse, manifest) + + messages = [i.message for i in issues] + assert any("Dangling symlink" in m for m in messages) + assert any("Broken reference" in m for m in messages) + assert any("Stale glob" in m for m in messages) + assert any("not a git working tree" in m for m in messages) + + def test_no_warehouse_skips_warehouse_checks(self, tmp_path): + manifest = _make_manifest() + issues = run_project_health_checks(tmp_path, None, manifest) + messages = {i.message for i in issues} + assert "Dangling symlink" not in messages + assert "Stale glob" not in messages + + def test_no_manifest_skips_manifest_checks(self, tmp_path): + warehouse = tmp_path / "warehouse" + warehouse.mkdir() + (warehouse / ".git").mkdir() + issues = run_project_health_checks(tmp_path, warehouse, None) + messages = {i.message for i in issues} + assert "Stale glob" not in messages From 583dce4c09062c136da419da684d634bef530e28 Mon Sep 17 00:00:00 2001 From: Song Yikun Date: Tue, 19 May 2026 19:48:40 +0800 Subject: [PATCH 2/3] PER-193 round 2: address opencode-review bot findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Finding 1 – Glob-managed references now matched via fnmatch in _is_wired_in_beacon, fixing false unmanaged-reference warnings for glob entries like contexts/*.md. Finding 2 – Stale-glob check now uses SyncEngine.expand_glob (files-only semantics), so directory-only glob matches are correctly flagged as stale. --- .../src/beacon/domains/setup/diagnostics.py | 16 +++++--- .../unit/domains/setup/test_diagnostics.py | 41 +++++++++++++++++++ 2 files changed, 52 insertions(+), 5 deletions(-) diff --git a/libs/beacon/src/beacon/domains/setup/diagnostics.py b/libs/beacon/src/beacon/domains/setup/diagnostics.py index cf9de6ae..4028c5d3 100644 --- a/libs/beacon/src/beacon/domains/setup/diagnostics.py +++ b/libs/beacon/src/beacon/domains/setup/diagnostics.py @@ -9,6 +9,7 @@ from __future__ import annotations +import fnmatch import json import re import sys @@ -16,6 +17,7 @@ from pathlib import Path from beacon.core.manifest.beacon import BeaconManifest +from beacon.domains.distribution.sync_engine import SyncEngine @dataclass(frozen=True) @@ -286,6 +288,11 @@ def _is_wired_in_beacon(raw_path: str, beacon_manifest: BeaconManifest) -> bool: # Directory entry: path is under the entry if norm_stripped.startswith(entry_stripped + "/"): return True + # Glob entry: match using fnmatch + if _is_glob_pattern(entry_stripped) and fnmatch.fnmatchcase( + norm_stripped, entry_stripped + ): + return True return False @@ -309,15 +316,14 @@ def _check_stale_globs( ("agents", beacon_manifest.artifacts.agents), ] + engine = SyncEngine(warehouse_path=warehouse_path, artifacts_path=Path()) + for artifact_type, entries in all_entries: for entry in entries: if not _is_glob_pattern(entry): continue - matches = list(warehouse_path.glob(entry)) - # Filter out .git and non-file entries for files; keep dirs for skill dirs - file_matches = [m for m in matches if m.is_file()] - dir_matches = [m for m in matches if m.is_dir()] - if not file_matches and not dir_matches: + matches = engine.expand_glob(entry) + if not matches: issues.append( DoctorIssue( message=f"Stale glob in beacon.yaml ({artifact_type})", diff --git a/libs/beacon/tests/unit/domains/setup/test_diagnostics.py b/libs/beacon/tests/unit/domains/setup/test_diagnostics.py index ff3ed3fc..3b37f6a0 100644 --- a/libs/beacon/tests/unit/domains/setup/test_diagnostics.py +++ b/libs/beacon/tests/unit/domains/setup/test_diagnostics.py @@ -203,6 +203,34 @@ def test_bare_artifact_path_resolved(self, tmp_path): issues = _check_path_references(tmp_path, manifest) assert issues == [] + def test_glob_entry_matches_referenced_path(self, tmp_path): + (tmp_path / ".agentic-beacon" / "artifacts" / "contexts").mkdir(parents=True) + (tmp_path / ".agentic-beacon" / "artifacts" / "contexts" / "foo.md").write_text( + "# Foo" + ) + (tmp_path / "CLAUDE.md").write_text( + "@.agentic-beacon/artifacts/contexts/foo.md\n" + ) + manifest = _make_manifest(contexts=["contexts/*.md"]) + issues = _check_path_references(tmp_path, manifest) + assert issues == [] + + def test_glob_entry_does_not_match_other_directory(self, tmp_path): + (tmp_path / ".agentic-beacon" / "artifacts" / "skills" / "foo").mkdir( + parents=True + ) + ( + tmp_path / ".agentic-beacon" / "artifacts" / "skills" / "foo" / "SKILL.md" + ).write_text("# Skill") + (tmp_path / "CLAUDE.md").write_text( + "@.agentic-beacon/artifacts/skills/foo/SKILL.md\n" + ) + manifest = _make_manifest(contexts=["contexts/*.md"]) + issues = _check_path_references(tmp_path, manifest) + assert len(issues) == 1 + assert "Unmanaged reference" in issues[0].message + assert issues[0].severity == "warn" + # --------------------------------------------------------------------------- # Stale globs @@ -250,6 +278,19 @@ def test_stale_skill_glob(self, tmp_path): assert len(issues) == 1 assert "skills" in issues[0].message + def test_stale_glob_directory_only_matches_is_flagged(self, tmp_path): + warehouse = tmp_path / "warehouse" + warehouse.mkdir() + # Create directories matching the glob but no files inside + (warehouse / "skills" / "code-review").mkdir(parents=True) + (warehouse / "skills" / "code-lint").mkdir(parents=True) + + manifest = _make_manifest(skills=["skills/code-*/"]) + issues = _check_stale_globs(warehouse, manifest) + assert len(issues) == 1 + assert "Stale glob" in issues[0].message + assert "skills" in issues[0].message + def test_no_manifest_returns_empty(self, tmp_path): warehouse = tmp_path / "warehouse" warehouse.mkdir() From f3ae2abe35895c64a7a1261cef762bdee3755dde Mon Sep 17 00:00:00 2001 From: Song Yikun Date: Wed, 20 May 2026 12:15:00 +0800 Subject: [PATCH 3/3] fix(doctor): accurate message for non-portable absolute paths (PER-193) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `_classify_reference` labelled every absolute @path reference as a "Warehouse-only absolute path" without doing any warehouse-relation check. The headline over-claimed — an absolute path could point anywhere. Reword to "Non-portable absolute path" — accurate for any absolute reference in a project-committed harness config. Still severity=error: absolute paths break for teammates with different home directories. Applied directly by the diligent-supervisor: the opencode/kimi delegate hit two consecutive transport-level failures (empty `!! ERROR {}`) during round-4 dispatch. The fix is a fully-specified two-string change. Addresses opencode-review PR #155 round-3 finding (the one verified-real finding of four — the other three were false alarms, see PR triage). Co-Authored-By: Claude Opus 4.7 --- libs/beacon/src/beacon/domains/setup/diagnostics.py | 6 +++--- libs/beacon/tests/unit/domains/setup/test_diagnostics.py | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/libs/beacon/src/beacon/domains/setup/diagnostics.py b/libs/beacon/src/beacon/domains/setup/diagnostics.py index 7ef75985..7bb4bca5 100644 --- a/libs/beacon/src/beacon/domains/setup/diagnostics.py +++ b/libs/beacon/src/beacon/domains/setup/diagnostics.py @@ -231,11 +231,11 @@ def _classify_reference( ) -> DoctorIssue | None: """Classify a single @path/reference and return an Issue or None.""" - # Flag absolute paths that point into the warehouse + # Absolute paths in project-committed config are not portable across machines. if raw_path.startswith("/"): return DoctorIssue( - message=f"Warehouse-only absolute path in {Path(source_file).name}", - detail=f"{raw_path} will break for other teammates", + message=f"Non-portable absolute path in {Path(source_file).name}", + detail=f"{raw_path} is an absolute path and will break for other teammates", severity="error", ) diff --git a/libs/beacon/tests/unit/domains/setup/test_diagnostics.py b/libs/beacon/tests/unit/domains/setup/test_diagnostics.py index 48e14336..8751162e 100644 --- a/libs/beacon/tests/unit/domains/setup/test_diagnostics.py +++ b/libs/beacon/tests/unit/domains/setup/test_diagnostics.py @@ -157,14 +157,14 @@ def test_claude_md_wired_reference_no_issue(self, tmp_path): issues = _check_path_references(tmp_path, manifest) assert issues == [] - def test_claude_md_warehouse_absolute_path_flagged(self, tmp_path): + def test_claude_md_absolute_path_flagged_as_non_portable(self, tmp_path): (tmp_path / "CLAUDE.md").write_text( "@/Users/someone/warehouse/contexts/team.md\n" ) manifest = _make_manifest() issues = _check_path_references(tmp_path, manifest) assert len(issues) == 1 - assert "Warehouse-only absolute path" in issues[0].message + assert "Non-portable absolute path" in issues[0].message assert issues[0].severity == "error" def test_opencode_json_broken_reference(self, tmp_path):