From 7a4d6e2b6bb16475cc2e0d8287dac5b488f2673c Mon Sep 17 00:00:00 2001 From: Tom Date: Thu, 13 Aug 2026 09:04:17 +0200 Subject: [PATCH 1/3] skills: deduplicate same-named skills, prefer local over home directories Skills found in multiple configured skills_paths directories now resolve to a single copy. Directory paths are ordered by priority so that a skill defined in a local project directory (e.g. ./.cecli/skills) shadows the same-named skill in any other directory, followed by other configured directories in the listed order, with home directories (e.g. ~/skills and the implicit ~/.cecli/skills default) last. Exact duplicate directory paths are also dropped. When no skills_paths is configured, the default search directory remains ~/.cecli/skills (unchanged). --- cecli/helpers/skills.py | 67 +++++++++++++++++++++- cecli/website/docs/config/skills.md | 8 +++ tests/basic/test_skills.py | 88 ++++++++++++++++++++++++++++- 3 files changed, 161 insertions(+), 2 deletions(-) diff --git a/cecli/helpers/skills.py b/cecli/helpers/skills.py index 2d1667a2bf8..696220b9f9a 100644 --- a/cecli/helpers/skills.py +++ b/cecli/helpers/skills.py @@ -71,7 +71,33 @@ def __init__( if default_skill_dir not in directory_paths: directory_paths = [default_skill_dir] + list(directory_paths) - self.directory_paths = [Path(p).expanduser().resolve() for p in directory_paths] + # Resolve every path and drop exact directory duplicates. + resolved_paths = [] + seen_paths = set() + for p in directory_paths: + try: + path = Path(p).expanduser().resolve() + except Exception: + continue + if path in seen_paths: + continue + seen_paths.add(path) + resolved_paths.append(path) + + # Order paths so local project directories come first, other configured + # directories follow in the listed order, and home directories always + # come last. Combined with the by-name deduplication in find_skills(), + # a skill defined in a local directory shadows the same-named skill + # found in any other (e.g. home) directory. + git_root_path = Path(git_root).expanduser().resolve() if git_root else None + ordered = sorted( + enumerate(resolved_paths), + key=lambda item: ( + self._directory_priority(item[1], git_root_path), + item[0], + ), + ) + self.directory_paths = [path for _, path in ordered] self.include_list = set(include_list) if include_list else None self.exclude_list = set(exclude_list) if exclude_list else set() self.git_root = Path(git_root).expanduser().resolve() if git_root else None @@ -111,6 +137,36 @@ def __init__( # Save initial state from config + @staticmethod + def _directory_priority(path: Path, git_root: Optional[Path] = None) -> int: + """Return the ordering priority of a skill directory. + + Lower values are scanned first and therefore win by-name conflicts: + + 0 - local project directory (under the git root or working directory) + 1 - any other configured directory + 2 - a home directory (including the implicit ``~/.cecli/skills`` default) + """ + home = Path.home().resolve() + + # The implicit default skills directory is always treated as a home + # directory, even when a project happens to live under the user's home. + if path == (home / ".cecli" / "skills"): + return 2 + + local_anchor = git_root if git_root is not None else Path.cwd() + try: + path.relative_to(local_anchor) + return 0 + except ValueError: + pass + + try: + path.relative_to(home) + return 2 + except ValueError: + return 1 + def _get_coder(self): """Return coder via weak reference, or None if collected.""" if self._coder_ref is not None: @@ -176,6 +232,7 @@ def find_skills(self, reload: bool = False) -> List[SkillMetadata]: return self._skills_find_cache skills = [] + seen_names: set[str] = set() for directory_path in self.directory_paths: directory_path = Path(directory_path) @@ -193,6 +250,14 @@ def find_skills(self, reload: bool = False) -> List[SkillMetadata]: metadata = self._parse_skill_metadata(skill_md_path) skill_name = metadata.name + # Directory paths are ordered by priority (local + # project directories first, home directories last), so + # the first occurrence of a skill name wins and later + # duplicates from lower-priority directories are dropped. + if skill_name in seen_names: + continue + seen_names.add(skill_name) + # Apply include/exclude filters if self.include_list and skill_name not in self.include_list: continue diff --git a/cecli/website/docs/config/skills.md b/cecli/website/docs/config/skills.md index acddc7a2e7d..813c49f5bd0 100644 --- a/cecli/website/docs/config/skills.md +++ b/cecli/website/docs/config/skills.md @@ -77,6 +77,14 @@ Skills are configured through the `agent-config` parameter in the YAML configura - **`skills_includelist`**: Array of skill names to include (whitelist) - **`skills_excludelist`**: Array of skill names to exclude (blacklist) +> **Duplicate skill names**: When the same skill name is found in more than +> one configured directory, only one copy is loaded. Directories are scanned +> in priority order: local project directories (e.g. `./.cecli/skills`) first, +> then other configured directories in the order they are listed, with home +> directories (e.g. `~/skills` and the implicit `~/.cecli/skills` default) +> last. If no `skills_paths` are configured, the only directory searched is +> `~/.cecli/skills`. + Complete configuration example in YAML configuration file (`.cecli.conf.yml` or `~/.cecli.conf.yml`): ```yaml diff --git a/tests/basic/test_skills.py b/tests/basic/test_skills.py index b7e24f5d082..d820e00169a 100644 --- a/tests/basic/test_skills.py +++ b/tests/basic/test_skills.py @@ -5,7 +5,7 @@ import os import tempfile from pathlib import Path -from unittest.mock import MagicMock +from unittest.mock import MagicMock, patch import pytest @@ -180,6 +180,92 @@ def test_resolve_skill_directories(self): paths = SkillsManager.resolve_skill_directories(["/non-existent/path"]) assert len(paths) == 0 + def test_find_skills_deduplicates_by_name_keeping_first_directory(self): + """Same-named skills in multiple directories resolve to the first one.""" + dir1 = Path(self.temp_dir) / "dir1" + dir2 = Path(self.temp_dir) / "dir2" + for d in (dir1, dir2): + d.mkdir() + + def _write_skill(base, name, description): + skill_dir = base / name + skill_dir.mkdir() + (skill_dir / "SKILL.md").write_text( + f"---\nname: {name}\ndescription: {description}\n---\n" + ) + + _write_skill(dir1, "shared", "first version") + _write_skill(dir1, "unique1", "only in dir1") + _write_skill(dir2, "shared", "second version") + _write_skill(dir2, "unique2", "only in dir2") + + # Point the implicit home dir somewhere harmless so it can't pollute the test + with patch.object(Path, "home", return_value=Path(self.temp_dir) / "fake-home"): + manager = SkillsManager([str(dir1), str(dir2)]) + + skills = manager.find_skills() + # Directory iteration order within a dir is filesystem order, so only + # compare names as a set; the important guarantees are that the + # duplicate was dropped and that the first directory's copy won. + assert {s.name for s in skills} == {"shared", "unique1", "unique2"} + assert len(skills) == 3 + shared = next(s for s in skills if s.name == "shared") + assert shared.description == "first version" + assert shared.path == (dir1 / "shared").resolve() + + def test_local_skill_wins_over_home_duplicate(self, monkeypatch): + """A local .cecli/skills skill shadows the same-named ~/skills skill.""" + home_dir = Path(self.temp_dir) / "home" + project_dir = Path(self.temp_dir) / "project" + (home_dir / "skills" / "dupe-skill").mkdir(parents=True) + (project_dir / ".cecli" / "skills" / "dupe-skill").mkdir(parents=True) + + (home_dir / "skills" / "dupe-skill" / "SKILL.md").write_text( + "---\nname: dupe-skill\ndescription: home version\n---\n" + ) + (project_dir / ".cecli" / "skills" / "dupe-skill" / "SKILL.md").write_text( + "---\nname: dupe-skill\ndescription: local version\n---\n" + ) + + monkeypatch.chdir(project_dir) + with patch.object(Path, "home", return_value=home_dir), patch.dict( + os.environ, {"HOME": str(home_dir)} + ): + manager = SkillsManager( + ["~/skills", "./.cecli/skills"], git_root=str(project_dir) + ) + + # Local directory is scanned first, home directories come last + assert manager.directory_paths[0] == (project_dir / ".cecli" / "skills").resolve() + assert manager.directory_paths[-1] == (home_dir / "skills").resolve() + assert (home_dir / ".cecli" / "skills").resolve() in manager.directory_paths + + skills = manager.find_skills() + assert [s.name for s in skills] == ["dupe-skill"] + assert skills[0].description == "local version" + assert skills[0].path == ( + project_dir / ".cecli" / "skills" / "dupe-skill" + ).resolve() + + def test_default_skill_dir_and_local_first_ordering(self, monkeypatch): + """No skills_paths -> only ~/.cecli/skills; local paths still ordered first.""" + home_dir = Path(self.temp_dir) / "fake-home" + project_dir = Path(self.temp_dir) / "project" + (project_dir / ".cecli" / "skills").mkdir(parents=True) + + monkeypatch.chdir(project_dir) + with patch.object(Path, "home", return_value=home_dir), patch.dict( + os.environ, {"HOME": str(home_dir)} + ): + # Default: with no skills_paths the only directory is ~/.cecli/skills + manager = SkillsManager([]) + assert manager.directory_paths == [(home_dir / ".cecli" / "skills").resolve()] + + # When a local path is configured it is scanned before the home default + manager = SkillsManager(["./.cecli/skills"]) + assert manager.directory_paths[0] == (project_dir / ".cecli" / "skills").resolve() + assert manager.directory_paths[-1] == (home_dir / ".cecli" / "skills").resolve() + def test_remove_skill(self): """Test the remove_skill instance method.""" # Create a skill directory structure From ec1b12a618db60fe7a1ce42cb2c1973794430925 Mon Sep 17 00:00:00 2001 From: Tom Date: Thu, 13 Aug 2026 10:11:04 +0200 Subject: [PATCH 2/3] style: black-format added skills tests (line-length 100, preview) --- tests/basic/test_skills.py | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/tests/basic/test_skills.py b/tests/basic/test_skills.py index d820e00169a..fdbd602a452 100644 --- a/tests/basic/test_skills.py +++ b/tests/basic/test_skills.py @@ -228,12 +228,11 @@ def test_local_skill_wins_over_home_duplicate(self, monkeypatch): ) monkeypatch.chdir(project_dir) - with patch.object(Path, "home", return_value=home_dir), patch.dict( - os.environ, {"HOME": str(home_dir)} + with ( + patch.object(Path, "home", return_value=home_dir), + patch.dict(os.environ, {"HOME": str(home_dir)}), ): - manager = SkillsManager( - ["~/skills", "./.cecli/skills"], git_root=str(project_dir) - ) + manager = SkillsManager(["~/skills", "./.cecli/skills"], git_root=str(project_dir)) # Local directory is scanned first, home directories come last assert manager.directory_paths[0] == (project_dir / ".cecli" / "skills").resolve() @@ -243,9 +242,7 @@ def test_local_skill_wins_over_home_duplicate(self, monkeypatch): skills = manager.find_skills() assert [s.name for s in skills] == ["dupe-skill"] assert skills[0].description == "local version" - assert skills[0].path == ( - project_dir / ".cecli" / "skills" / "dupe-skill" - ).resolve() + assert skills[0].path == (project_dir / ".cecli" / "skills" / "dupe-skill").resolve() def test_default_skill_dir_and_local_first_ordering(self, monkeypatch): """No skills_paths -> only ~/.cecli/skills; local paths still ordered first.""" @@ -254,8 +251,9 @@ def test_default_skill_dir_and_local_first_ordering(self, monkeypatch): (project_dir / ".cecli" / "skills").mkdir(parents=True) monkeypatch.chdir(project_dir) - with patch.object(Path, "home", return_value=home_dir), patch.dict( - os.environ, {"HOME": str(home_dir)} + with ( + patch.object(Path, "home", return_value=home_dir), + patch.dict(os.environ, {"HOME": str(home_dir)}), ): # Default: with no skills_paths the only directory is ~/.cecli/skills manager = SkillsManager([]) From 5f8f32be98cff890c6c5d81a04593f0c693b2841 Mon Sep 17 00:00:00 2001 From: Tom Date: Thu, 13 Aug 2026 10:58:04 +0200 Subject: [PATCH 3/3] skills: dedupe same-named skills by directory priority; trim comments; revert skills tests to main - Order skill dirs local-first, home last so first occurrence wins - Reduce added comments to single lines - Revert tests/basic/test_skills.py to main version (removes test churn) --- cecli/helpers/skills.py | 14 ++----- tests/basic/test_skills.py | 86 +------------------------------------- 2 files changed, 4 insertions(+), 96 deletions(-) diff --git a/cecli/helpers/skills.py b/cecli/helpers/skills.py index 696220b9f9a..7d6fbea31f4 100644 --- a/cecli/helpers/skills.py +++ b/cecli/helpers/skills.py @@ -84,11 +84,7 @@ def __init__( seen_paths.add(path) resolved_paths.append(path) - # Order paths so local project directories come first, other configured - # directories follow in the listed order, and home directories always - # come last. Combined with the by-name deduplication in find_skills(), - # a skill defined in a local directory shadows the same-named skill - # found in any other (e.g. home) directory. + # Order paths: local project dirs first, then configured, home dirs last. git_root_path = Path(git_root).expanduser().resolve() if git_root else None ordered = sorted( enumerate(resolved_paths), @@ -149,8 +145,7 @@ def _directory_priority(path: Path, git_root: Optional[Path] = None) -> int: """ home = Path.home().resolve() - # The implicit default skills directory is always treated as a home - # directory, even when a project happens to live under the user's home. + # Implicit default skills dir is always treated as a home dir. if path == (home / ".cecli" / "skills"): return 2 @@ -250,10 +245,7 @@ def find_skills(self, reload: bool = False) -> List[SkillMetadata]: metadata = self._parse_skill_metadata(skill_md_path) skill_name = metadata.name - # Directory paths are ordered by priority (local - # project directories first, home directories last), so - # the first occurrence of a skill name wins and later - # duplicates from lower-priority directories are dropped. + # First directory wins for duplicate skill names. if skill_name in seen_names: continue seen_names.add(skill_name) diff --git a/tests/basic/test_skills.py b/tests/basic/test_skills.py index fdbd602a452..b7e24f5d082 100644 --- a/tests/basic/test_skills.py +++ b/tests/basic/test_skills.py @@ -5,7 +5,7 @@ import os import tempfile from pathlib import Path -from unittest.mock import MagicMock, patch +from unittest.mock import MagicMock import pytest @@ -180,90 +180,6 @@ def test_resolve_skill_directories(self): paths = SkillsManager.resolve_skill_directories(["/non-existent/path"]) assert len(paths) == 0 - def test_find_skills_deduplicates_by_name_keeping_first_directory(self): - """Same-named skills in multiple directories resolve to the first one.""" - dir1 = Path(self.temp_dir) / "dir1" - dir2 = Path(self.temp_dir) / "dir2" - for d in (dir1, dir2): - d.mkdir() - - def _write_skill(base, name, description): - skill_dir = base / name - skill_dir.mkdir() - (skill_dir / "SKILL.md").write_text( - f"---\nname: {name}\ndescription: {description}\n---\n" - ) - - _write_skill(dir1, "shared", "first version") - _write_skill(dir1, "unique1", "only in dir1") - _write_skill(dir2, "shared", "second version") - _write_skill(dir2, "unique2", "only in dir2") - - # Point the implicit home dir somewhere harmless so it can't pollute the test - with patch.object(Path, "home", return_value=Path(self.temp_dir) / "fake-home"): - manager = SkillsManager([str(dir1), str(dir2)]) - - skills = manager.find_skills() - # Directory iteration order within a dir is filesystem order, so only - # compare names as a set; the important guarantees are that the - # duplicate was dropped and that the first directory's copy won. - assert {s.name for s in skills} == {"shared", "unique1", "unique2"} - assert len(skills) == 3 - shared = next(s for s in skills if s.name == "shared") - assert shared.description == "first version" - assert shared.path == (dir1 / "shared").resolve() - - def test_local_skill_wins_over_home_duplicate(self, monkeypatch): - """A local .cecli/skills skill shadows the same-named ~/skills skill.""" - home_dir = Path(self.temp_dir) / "home" - project_dir = Path(self.temp_dir) / "project" - (home_dir / "skills" / "dupe-skill").mkdir(parents=True) - (project_dir / ".cecli" / "skills" / "dupe-skill").mkdir(parents=True) - - (home_dir / "skills" / "dupe-skill" / "SKILL.md").write_text( - "---\nname: dupe-skill\ndescription: home version\n---\n" - ) - (project_dir / ".cecli" / "skills" / "dupe-skill" / "SKILL.md").write_text( - "---\nname: dupe-skill\ndescription: local version\n---\n" - ) - - monkeypatch.chdir(project_dir) - with ( - patch.object(Path, "home", return_value=home_dir), - patch.dict(os.environ, {"HOME": str(home_dir)}), - ): - manager = SkillsManager(["~/skills", "./.cecli/skills"], git_root=str(project_dir)) - - # Local directory is scanned first, home directories come last - assert manager.directory_paths[0] == (project_dir / ".cecli" / "skills").resolve() - assert manager.directory_paths[-1] == (home_dir / "skills").resolve() - assert (home_dir / ".cecli" / "skills").resolve() in manager.directory_paths - - skills = manager.find_skills() - assert [s.name for s in skills] == ["dupe-skill"] - assert skills[0].description == "local version" - assert skills[0].path == (project_dir / ".cecli" / "skills" / "dupe-skill").resolve() - - def test_default_skill_dir_and_local_first_ordering(self, monkeypatch): - """No skills_paths -> only ~/.cecli/skills; local paths still ordered first.""" - home_dir = Path(self.temp_dir) / "fake-home" - project_dir = Path(self.temp_dir) / "project" - (project_dir / ".cecli" / "skills").mkdir(parents=True) - - monkeypatch.chdir(project_dir) - with ( - patch.object(Path, "home", return_value=home_dir), - patch.dict(os.environ, {"HOME": str(home_dir)}), - ): - # Default: with no skills_paths the only directory is ~/.cecli/skills - manager = SkillsManager([]) - assert manager.directory_paths == [(home_dir / ".cecli" / "skills").resolve()] - - # When a local path is configured it is scanned before the home default - manager = SkillsManager(["./.cecli/skills"]) - assert manager.directory_paths[0] == (project_dir / ".cecli" / "skills").resolve() - assert manager.directory_paths[-1] == (home_dir / ".cecli" / "skills").resolve() - def test_remove_skill(self): """Test the remove_skill instance method.""" # Create a skill directory structure