Repository: agentscope-ai/CoPaw (python)

Issue:
[Bug]: 0.0.7版本中技能显示出现大量的重复，active，customized，还有venv里的技能都显示出来了

如图所示
<img width="778" height="379" alt="Image" src="https://github.com/user-attachments/assets/a3302062-ce05-48b6-b3a7-71923400e1ef" />


Patch:
diff --git a/src/copaw/agents/skills_manager.py b/src/copaw/agents/skills_manager.py
index a8f683331..76ba47d81 100644
--- a/src/copaw/agents/skills_manager.py
+++ b/src/copaw/agents/skills_manager.py
@@ -4,6 +4,8 @@
 import filecmp
 import logging
 import shutil
+from collections.abc import Iterable
+from itertools import zip_longest
 from pathlib import Path
 from typing import Any
 from pydantic import BaseModel
@@ -14,6 +16,93 @@
 logger = logging.getLogger(__name__)
 
 
+IGNORED_RUNTIME_ARTIFACT_NAMES = {
+    "__pycache__",
+    ".DS_Store",
+    "Thumbs.db",
+    ".pytest_cache",
+}
+IGNORED_RUNTIME_ARTIFACT_SUFFIXES = {
+    ".pyc",
+    ".pyo",
+}
+
+
+def _should_ignore_runtime_artifact(path: Path) -> bool:
+    """Return True for generated runtime files that should not sync."""
+    if path.name in IGNORED_RUNTIME_ARTIFACT_NAMES:
+        return True
+    if path.is_file() and path.suffix in IGNORED_RUNTIME_ARTIFACT_SUFFIXES:
+        return True
+    return False
+
+
+def _iter_relevant_directory_entries(
+    directory: Path,
+) -> Iterable[tuple[Path, Path]]:
+    """Yield relative paths for non-generated files and directories."""
+    if not directory.exists():
+        return
+
+    yield from _iter_relevant_directory_entries_from(
+        root_dir=directory,
+        current_dir=directory,
+    )
+
+
+def _iter_relevant_directory_entries_from(
+    root_dir: Path,
+    current_dir: Path,
+) -> Iterable[tuple[Path, Path]]:
+    """Yield sorted non-generated directory entries without buffering."""
+    for item in sorted(current_dir.iterdir(), key=lambda path: path.name):
+        if _should_ignore_runtime_artifact(item):
+            continue
+
+        yield item.relative_to(root_dir), item
+
+        if item.is_dir():
+            yield from _iter_relevant_directory_entries_from(
+                root_dir=root_dir,
+                current_dir=item,
+            )
+
+
+def _directories_match_ignoring_runtime_artifacts(
+    dir1: Path,
+    dir2: Path,
+) -> bool:
+    """Compare two directories while ignoring generated runtime artifacts."""
+    if not dir1.exists() or not dir2.exists():
+        return False
+
+    for entry1, entry2 in zip_longest(
+        _iter_relevant_directory_entries(dir1),
+        _iter_relevant_directory_entries(dir2),
+    ):
+        if entry1 is None or entry2 is None:
+            return False
+
+        relative_path1, left = entry1
+        relative_path2, right = entry2
+        if relative_path1 != relative_path2:
+            return False
+        if left.is_dir() != right.is_dir():
+            return False
+        if left.is_file() and not filecmp.cmp(left, right, shallow=False):
+            return False
+
+    return True
+
+
+def _dedupe_skills_by_name(skills: list["SkillInfo"]) -> list["SkillInfo"]:
+    """Return one skill per name, preferring customized over builtin."""
+    merged: dict[str, SkillInfo] = {}
+    for skill in skills:
+        merged[skill.name] = skill
+    return list(merged.values())
+
+
 class SkillInfo(BaseModel):
     """Skill information structure.
 
@@ -206,53 +295,6 @@ def sync_skills_to_working_dir(
     return synced_count, skipped_count
 
 
-def _is_directory_same(dir1: Path, dir2: Path) -> bool:
-    """
-    Check if two directories have the same content.
-
-    Args:
-        dir1: First directory path.
-        dir2: Second directory path.
-
-    Returns:
-        True if directories have the same structure and file contents.
-    """
-    if not dir1.exists() or not dir2.exists():
-        return False
-
-    dcmp = filecmp.dircmp(dir1, dir2)
-
-    if dcmp.left_only or dcmp.right_only or dcmp.funny_files:
-        return False
-
-    if dcmp.diff_files:
-        return False
-
-    for sub_dcmp in dcmp.subdirs.values():
-        if not _compare_dircmp(sub_dcmp):
-            return False
-
-    return True
-
-
-def _compare_dircmp(dcmp: "filecmp.dircmp") -> bool:
-    """Helper to recursively compare dircmp objects."""
-    has_diff = any(
-        [
-            dcmp.left_only,
-            dcmp.right_only,
-            dcmp.funny_files,
-            dcmp.diff_files,
-        ],
-    )
-    if has_diff:
-        return False
-    for sub_dcmp in dcmp.subdirs.values():
-        if not _compare_dircmp(sub_dcmp):
-            return False
-    return True
-
-
 def sync_skills_from_active_to_customized(
     skill_names: list[str] | None = None,
 ) -> tuple[int, int]:
@@ -287,7 +329,10 @@ def sync_skills_from_active_to_customized(
 
         if skill_name in builtin_skills_dict:
             builtin_skill_dir = builtin_skills_dict[skill_name]
-            if _is_directory_same(skill_dir, builtin_skill_dir):
+            if _directories_match_ignoring_runtime_artifacts(
+                skill_dir,
+                builtin_skill_dir,
+            ):
                 skipped_count += 1
                 continue
 
@@ -513,7 +558,8 @@ def list_all_skills() -> list[SkillInfo]:
 
         skills: list[SkillInfo] = []
 
-        # Collect from builtin and customized skills
+        # Collect from builtin and customized skills. Customized skills
+        # override built-in skills with the same name in the UI/API listing.
         skills.extend(
             _read_skills_from_dir(get_builtin_skills_dir(), "builtin"),
         )
@@ -521,7 +567,7 @@ def list_all_skills() -> list[SkillInfo]:
             _read_skills_from_dir(get_customized_skills_dir(), "customized"),
         )
 
-        return skills
+        return _dedupe_skills_by_name(skills)
 
     @staticmethod
     def list_available_skills() -> list[SkillInfo]:


Test patch:
diff --git a/tests/unit/agents/test_skills_manager.py b/tests/unit/agents/test_skills_manager.py
new file mode 100644
index 000000000..4a2113443
--- /dev/null
+++ b/tests/unit/agents/test_skills_manager.py
@@ -0,0 +1,187 @@
+# -*- coding: utf-8 -*-
+from __future__ import annotations
+
+import importlib
+from pathlib import Path
+import sys
+import types
+
+
+def _install_frontmatter_stub() -> None:
+    if "frontmatter" in sys.modules:
+        return
+
+    def loads(content: str):
+        data: dict[str, str] = {}
+        lines = content.splitlines()
+        if len(lines) >= 3 and lines[0].strip() == "---":
+            for line in lines[1:]:
+                if line.strip() == "---":
+                    break
+                key, _, value = line.partition(":")
+                if key and _:
+                    data[key.strip()] = value.strip()
+        return data
+
+    sys.modules["frontmatter"] = types.SimpleNamespace(loads=loads)
+
+
+def _load_skills_manager_module():
+    _install_frontmatter_stub()
+    return importlib.import_module("copaw.agents.skills_manager")
+
+
+SKILL_TEMPLATE = """---
+name: {name}
+description: {description}
+---
+
+# {name}
+"""
+
+
+def _write_skill(
+    root: Path,
+    name: str,
+    description: str,
+    *,
+    script_body: str = "print(1)\n",
+) -> Path:
+    skill_dir = root / name
+    skill_dir.mkdir(parents=True, exist_ok=True)
+    (skill_dir / "SKILL.md").write_text(
+        SKILL_TEMPLATE.format(name=name, description=description),
+        encoding="utf-8",
+    )
+    scripts_dir = skill_dir / "scripts"
+    scripts_dir.mkdir(parents=True, exist_ok=True)
+    (scripts_dir / "main.py").write_text(script_body, encoding="utf-8")
+    return skill_dir
+
+
+def _patch_skill_dirs(
+    monkeypatch,
+    skills_manager_module,
+    *,
+    builtin_dir: Path,
+    customized_dir: Path,
+    active_dir: Path,
+) -> None:
+    monkeypatch.setattr(
+        skills_manager_module,
+        "get_builtin_skills_dir",
+        lambda: builtin_dir,
+    )
+    monkeypatch.setattr(
+        skills_manager_module,
+        "get_customized_skills_dir",
+        lambda: customized_dir,
+    )
+    monkeypatch.setattr(
+        skills_manager_module,
+        "get_active_skills_dir",
+        lambda: active_dir,
+    )
+
+
+def test_sync_from_active_to_customized_ignores_runtime_artifacts(
+    monkeypatch,
+    tmp_path,
+) -> None:
+    skills_manager_module = _load_skills_manager_module()
+    builtin_dir = tmp_path / "builtin"
+    customized_dir = tmp_path / "customized"
+    active_dir = tmp_path / "active"
+    builtin_skill = _write_skill(builtin_dir, "calendar", "builtin calendar")
+    active_skill = _write_skill(active_dir, "calendar", "builtin calendar")
+
+    pycache_dir = active_skill / "__pycache__"
+    pycache_dir.mkdir(parents=True)
+    (pycache_dir / "main.cpython-312.pyc").write_bytes(b"compiled")
+    (active_skill / ".DS_Store").write_text("finder", encoding="utf-8")
+
+    _patch_skill_dirs(
+        monkeypatch,
+        skills_manager_module,
+        builtin_dir=builtin_dir,
+        customized_dir=customized_dir,
+        active_dir=active_dir,
+    )
+
+    (
+        synced,
+        skipped,
+    ) = skills_manager_module.sync_skills_from_active_to_customized()
+
+    assert synced == 0
+    assert skipped == 1
+    assert not (customized_dir / builtin_skill.name).exists()
+
+
+def test_sync_from_active_to_customized_keeps_real_builtin_edits(
+    monkeypatch,
+    tmp_path,
+) -> None:
+    skills_manager_module = _load_skills_manager_module()
+    builtin_dir = tmp_path / "builtin"
+    customized_dir = tmp_path / "customized"
+    active_dir = tmp_path / "active"
+    _write_skill(builtin_dir, "calendar", "builtin calendar")
+    active_skill = _write_skill(
+        active_dir,
+        "calendar",
+        "builtin calendar",
+        script_body="print(2)\n",
+    )
+
+    _patch_skill_dirs(
+        monkeypatch,
+        skills_manager_module,
+        builtin_dir=builtin_dir,
+        customized_dir=customized_dir,
+        active_dir=active_dir,
+    )
+
+    (
+        synced,
+        skipped,
+    ) = skills_manager_module.sync_skills_from_active_to_customized()
+
+    assert synced == 1
+    assert skipped == 0
+    copied_skill = customized_dir / active_skill.name
+    assert copied_skill.exists()
+    assert (copied_skill / "scripts" / "main.py").read_text(
+        encoding="utf-8",
+    ) == "print(2)\n"
+
+
+def test_list_all_skills_prefers_customized_over_builtin_duplicates(
+    monkeypatch,
+    tmp_path,
+) -> None:
+    skills_manager_module = _load_skills_manager_module()
+    builtin_dir = tmp_path / "builtin"
+    customized_dir = tmp_path / "customized"
+    active_dir = tmp_path / "active"
+    _write_skill(builtin_dir, "calendar", "builtin calendar")
+    _write_skill(builtin_dir, "mail", "builtin mail")
+    _write_skill(customized_dir, "calendar", "customized calendar")
+    _write_skill(customized_dir, "notes", "customized notes")
+
+    _patch_skill_dirs(
+        monkeypatch,
+        skills_manager_module,
+        builtin_dir=builtin_dir,
+        customized_dir=customized_dir,
+        active_dir=active_dir,
+    )
+
+    skills = skills_manager_module.SkillService.list_all_skills()
+
+    assert [skill.name for skill in skills] == ["calendar", "mail", "notes"]
+    skills_by_name = {skill.name: skill for skill in skills}
+    assert skills_by_name["calendar"].source == "customized"
+    assert skills_by_name["calendar"].description == "customized calendar"
+    assert skills_by_name["mail"].source == "builtin"
+    assert skills_by_name["notes"].source == "customized"