chore: update agr.lock after running agr sync

agr.lock

@@ -16,6 +16,6 @@ installed-name = "code-review"
 [[skill]]
 handle = "microsoft/playwright-cli/playwright-cli"
 source = "github"
-commit = "4e37405ffe6b40db68bb6a787cd1dadbe999e4a0"
-content-hash = "sha256:50aa57a0172e6ed2544ac7c161a1c61f424dbdd57511ef0e0a6a3f35c509ed65"
+commit = "a0d5bfd4d9658073029d33f979ac5a027568caec"
+content-hash = "sha256:75e47183c30bc8651e76286680eddac88a3024a7ee5a7f1bc486d4d3fdee34ce"
 installed-name = "playwright-cli"

agr/commands/remove.py

@@ -140,5 +140,6 @@ def _print_remove_result(result: CommandResult) -> None:
         if lockfile is not None:
             for candidates, is_ralph_lf in zip(removed_candidates, removed_ralph_flags):
                 for identifier in candidates:
-                    remove_lockfile_entry(lockfile, identifier, ralph=is_ralph_lf)
+                    if remove_lockfile_entry(lockfile, identifier, ralph=is_ralph_lf):
+                        break
             save_lockfile(lockfile, lockfile_path)

agr/commands/sync.py

@@ -431,6 +431,13 @@ def _run_global_sync() -> None:
             handle, source_name = dep.resolve(
                 config.default_source, config.default_owner
             )
+            if dep.type == DEPENDENCY_TYPE_RALPH:
+                # Ralphs are project-level only; skip in global mode.
+                console.print(
+                    f"[yellow]Skipped:[/yellow] {dep.identifier} "
+                    "(ralphs are not supported in global installs)"
+                )
+                continue
             result = _sync_one_dependency(
                 handle,
                 source_name,

agr/fetcher.py

@@ -324,18 +324,20 @@ def _copy_skill_to_destination(
     return dest
 
 
-def skill_not_found_message(name: str) -> str:
-    """Build a user-friendly message for a missing skill in a repository.
-
-    Used by both fetcher and sync to produce consistent error text.
-    """
+def _dep_not_found_message(kind: str, name: str, marker: str, subdir: str) -> str:
+    """Build a user-friendly message for a missing dependency in a repository."""
     return (
-        f"Skill '{name}' not found in repository.\n"
-        f"No directory named '{name}' containing SKILL.md was found.\n"
-        f"Hint: Create a skill at 'skills/{name}/SKILL.md' or '{name}/SKILL.md'"
+        f"{kind} '{name}' not found in repository.\n"
+        f"No directory named '{name}' containing {marker} was found.\n"
+        f"Hint: Create a {kind.lower()} at '{subdir}/{name}/{marker}' or '{name}/{marker}'"
     )
 
 
+def skill_not_found_message(name: str) -> str:
+    """Build a user-friendly message for a missing skill in a repository."""
+    return _dep_not_found_message("Skill", name, SKILL_MARKER, "skills")
+
+
 def install_skill_from_repo(
     repo_dir: Path,
     skill_name: str,
@@ -998,11 +1000,7 @@ def _copy_ralph_to_destination(
 
 def ralph_not_found_message(name: str) -> str:
     """Build a user-friendly message for a missing ralph in a repository."""
-    return (
-        f"Ralph '{name}' not found in repository.\n"
-        f"No directory named '{name}' containing RALPH.md was found.\n"
-        f"Hint: Create a ralph at 'ralphs/{name}/RALPH.md' or '{name}/RALPH.md'"
-    )
+    return _dep_not_found_message("Ralph", name, RALPH_MARKER, "ralphs")
 
 
 def prepare_repo_for_ralph(repo_dir: Path, ralph_name: str) -> Path | None:
@@ -1070,6 +1068,41 @@ def install_ralph_from_repo(
     )
 
 
+def _find_local_ralph_name_conflicts(
+    handle: ParsedHandle,
+    ralphs_dir: Path,
+    repo_root: Path | None,
+    default_dest: Path,
+) -> tuple[list[Path], bool]:
+    """Find conflicting local installs with the same ralph name.
+
+    Returns a tuple of (conflict_paths, has_unknown_metadata).
+    """
+    handle_id = build_handle_id(handle, repo_root)
+    conflicts: list[Path] = []
+    has_unknown = False
+
+    candidates = [ralphs_dir / handle.name, ralphs_dir / handle.to_installed_name()]
+
+    for path in candidates:
+        if path.resolve() == default_dest.resolve():
+            continue
+        if not is_valid_ralph_dir(path):
+            continue
+        meta = read_skill_metadata(path)
+        if meta:
+            if meta.get(METADATA_KEY_TYPE) != METADATA_TYPE_LOCAL:
+                continue
+            if meta.get(METADATA_KEY_ID) == handle_id:
+                continue
+            conflicts.append(path)
+            continue
+        has_unknown = True
+        conflicts.append(path)
+
+    return conflicts, has_unknown
+
+
 def install_local_ralph(
     source_path: Path,
     ralphs_dir: Path,
@@ -1104,6 +1137,25 @@ def install_local_ralph(
             stamp_ralph_metadata(default_dest, handle, repo_root, default_dest.name)
         return default_dest
 
+    conflicts, has_unknown = _find_local_ralph_name_conflicts(
+        handle, ralphs_dir, repo_root, default_dest
+    )
+    if conflicts:
+        locations = ", ".join(str(path) for path in conflicts)
+        hint = ""
+        if has_unknown:
+            hint = (
+                " If this is a remote ralph, run "
+                "`agr sync` or reinstall it to "
+                "add metadata."
+            )
+        raise AgrError(
+            f"Local ralph name '{handle.name}' is already installed at {locations}. "
+            "agr allows only one local ralph with a given name. "
+            "Rename the ralph or remove the existing one."
+            f"{hint}"
+        )
+
     ralph_dest = _resolve_ralph_destination(handle, ralphs_dir, repo_root)
 
     return _copy_ralph_to_destination(

agr/lockfile.py

@@ -153,8 +153,7 @@ def _build_aot(entries: list[LockedSkill]) -> tomlkit.items.AoT:
         return aot
 
     doc["skill"] = _build_aot(lockfile.skills)
-    if lockfile.ralphs:
-        doc["ralph"] = _build_aot(lockfile.ralphs)
+    doc["ralph"] = _build_aot(lockfile.ralphs)
     path.write_text(tomlkit.dumps(doc))
 
 
@@ -212,9 +211,16 @@ def update_lockfile_entry(
 
 def remove_lockfile_entry(
     lockfile: Lockfile, identifier: str, *, ralph: bool = False
-) -> None:
-    """Remove an entry from the lockfile by identifier."""
+) -> bool:
+    """Remove an entry from the lockfile by identifier.
+
+    Returns True if an entry was removed, False if no match was found.
+    """
     if ralph:
+        before = len(lockfile.ralphs)
         lockfile.ralphs = [r for r in lockfile.ralphs if r.identifier != identifier]
+        return len(lockfile.ralphs) < before
     else:
+        before = len(lockfile.skills)
         lockfile.skills = [s for s in lockfile.skills if s.identifier != identifier]
+        return len(lockfile.skills) < before

agr/ralph.py

@@ -2,29 +2,21 @@
 
 from pathlib import Path, PurePosixPath
 
-from agr.skill import EXCLUDED_DIRS, _shallowest
+from agr.skill import _is_excluded_skill_path, _shallowest
 
 
 # Marker file for ralphs
 RALPH_MARKER = "RALPH.md"
 
-
-def _is_excluded_ralph_path(parts: tuple[str, ...]) -> bool:
-    """Check if a relative RALPH.md path should be excluded from discovery.
-
-    Same rules as skill discovery:
-    1. Root-level RALPH.md is a repo marker, not a ralph directory.
-    2. Any path component matching EXCLUDED_DIRS disqualifies the entry.
-    """
-    if len(parts) == 1:
-        return True
-    return any(part in EXCLUDED_DIRS for part in parts)
+# Reuse skill exclusion logic — same rules apply to ralphs:
+# root-level marker is a repo marker, and EXCLUDED_DIRS are skipped.
+_is_excluded_marker_path = _is_excluded_skill_path
 
 
 def _is_excluded_path(path: Path, repo_dir: Path) -> bool:
     """Check if a path should be excluded from ralph discovery."""
     rel = path.relative_to(repo_dir)
-    return _is_excluded_ralph_path(rel.parts)
+    return _is_excluded_marker_path(rel.parts)
 
 
 def is_valid_ralph_dir(path: Path) -> bool:
@@ -63,7 +55,7 @@ def _find_ralph_dirs_in_listing(paths: list[str]) -> list[PurePosixPath]:
         rel_path = PurePosixPath(rel)
         if rel_path.name != RALPH_MARKER:
             continue
-        if _is_excluded_ralph_path(rel_path.parts):
+        if _is_excluded_marker_path(rel_path.parts):
             continue
         results.append(rel_path.parent)
     return results

tests/cli/agr/test_add.py

@@ -120,3 +120,14 @@ def test_add_local_ralph_not_installed_to_tools(self, agr, cli_project, cli_ralp
         agr("add", "./ralphs/test-ralph")
         tool_dir = cli_project / ".claude" / "skills" / "test-ralph"
         assert not tool_dir.exists()
+
+    def test_add_both_markers_fails(self, agr, cli_project):
+        """agr add fails when directory has both SKILL.md and RALPH.md."""
+        both_dir = cli_project / "both-type"
+        both_dir.mkdir()
+        (both_dir / "SKILL.md").write_text("# Skill")
+        (both_dir / "RALPH.md").write_text("# Ralph")
+
+        result = agr("add", "./both-type")
+
+        assert_cli(result).failed().stdout_contains("both SKILL.md and RALPH.md")

tests/cli/agr/test_remove.py

@@ -40,3 +40,30 @@ def test_remove_not_installed_fails(self, agr, cli_skill):
         result = agr("remove", "./skills/test-skill")
 
         assert_cli(result).failed()
+
+
+class TestAgrRemoveRalph:
+    """Tests for agr remove with ralph type."""
+
+    def test_remove_ralph_succeeds(self, agr, cli_ralph):
+        """agr remove removes installed ralph."""
+        agr("add", "./ralphs/test-ralph")
+        result = agr("remove", "./ralphs/test-ralph")
+        assert_cli(result).succeeded().stdout_contains("Removed:")
+
+    def test_remove_ralph_cleans_directory(self, agr, cli_project, cli_ralph):
+        """agr remove deletes installed ralph directory."""
+        agr("add", "./ralphs/test-ralph")
+        installed = cli_project / ".agents" / "ralphs" / "test-ralph"
+        assert installed.exists()
+
+        agr("remove", "./ralphs/test-ralph")
+        assert not installed.exists()
+
+    def test_remove_ralph_updates_config(self, agr, cli_project, cli_ralph):
+        """agr remove updates agr.toml."""
+        agr("add", "./ralphs/test-ralph")
+        agr("remove", "./ralphs/test-ralph")
+
+        config = (cli_project / "agr.toml").read_text()
+        assert "ralphs/test-ralph" not in config

tests/cli/agr/test_sync.py

@@ -86,3 +86,28 @@ def test_sync_instructions_creates_agents_for_cursor(
         assert_cli(result).succeeded()
         assert (cli_project / "AGENTS.md").exists()
         assert (cli_project / "AGENTS.md").read_text() == "Claude instructions\n"
+
+
+class TestAgrSyncRalph:
+    """Tests for agr sync with ralph dependencies."""
+
+    def test_sync_ralph_installs_from_config(
+        self, agr, cli_project, cli_ralph, cli_config
+    ):
+        """agr sync installs ralph from agr.toml config."""
+        cli_config('dependencies = [{path = "./ralphs/test-ralph", type = "ralph"}]')
+
+        result = agr("sync")
+
+        assert_cli(result).succeeded().stdout_contains("Installed:")
+        installed = cli_project / ".agents" / "ralphs" / "test-ralph"
+        assert installed.exists()
+        assert (installed / "RALPH.md").exists()
+
+    def test_sync_ralph_reports_up_to_date(self, agr, cli_project, cli_ralph):
+        """agr sync reports already installed ralph as up to date."""
+        agr("add", "./ralphs/test-ralph")
+
+        result = agr("sync")
+
+        assert_cli(result).succeeded().stdout_contains("up to date")

tests/unit/test_lockfile.py

@@ -394,3 +394,35 @@ def test_is_lockfile_current_missing_ralph(self):
             Dependency(type="ralph", handle="user/repo/ralph"),
         ]
         assert is_lockfile_current(lockfile, deps) is False
+
+
+class TestRemoveLockfileEntryReturnValue:
+    """Tests for remove_lockfile_entry return value."""
+
+    def test_returns_true_on_match(self):
+        lockfile = Lockfile(
+            skills=[LockedSkill(handle="user/repo/a", installed_name="a")]
+        )
+        assert remove_lockfile_entry(lockfile, "user/repo/a") is True
+        assert len(lockfile.skills) == 0
+
+    def test_returns_false_on_miss(self):
+        lockfile = Lockfile(
+            skills=[LockedSkill(handle="user/repo/a", installed_name="a")]
+        )
+        assert remove_lockfile_entry(lockfile, "user/repo/unknown") is False
+        assert len(lockfile.skills) == 1
+
+    def test_returns_true_on_ralph_match(self):
+        lockfile = Lockfile(
+            ralphs=[LockedSkill(handle="user/repo/r", installed_name="r")]
+        )
+        assert remove_lockfile_entry(lockfile, "user/repo/r", ralph=True) is True
+        assert len(lockfile.ralphs) == 0
+
+    def test_returns_false_on_ralph_miss(self):
+        lockfile = Lockfile(
+            ralphs=[LockedSkill(handle="user/repo/r", installed_name="r")]
+        )
+        assert remove_lockfile_entry(lockfile, "user/repo/x", ralph=True) is False
+        assert len(lockfile.ralphs) == 1