From 4eb721424904937a0d64e18e9265ecdc5aa8a0be Mon Sep 17 00:00:00 2001 From: Your Name Date: Wed, 12 Aug 2026 08:34:51 -0400 Subject: [PATCH 01/13] #637: Set error code in response parsing errors --- cecli/coders/base_coder.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cecli/coders/base_coder.py b/cecli/coders/base_coder.py index 5a4c6c6cce2..b4d59de656a 100755 --- a/cecli/coders/base_coder.py +++ b/cecli/coders/base_coder.py @@ -1759,6 +1759,7 @@ async def output_task(self, preproc): except (SwitchCoderSignal, SystemExit): raise except Exception as e: + self.error_code = 1 traceback_str = traceback.format_exc() update_error_prefix(traceback_str) @@ -2759,6 +2760,7 @@ async def format_in_executor(): dict(role="assistant", content=self.multi_response_content, prefix=True) ) except Exception as err: + self.error_code = 1 self.mdstream = None lines = traceback.format_exception(type(err), err, err.__traceback__) self.io.tool_warning("".join(lines)) @@ -3508,6 +3510,7 @@ async def add_assistant_reply_to_cur_messages(self): # but response.dict() is the Pydantic V1 method name. response_dict = dict(response) except TypeError: + self.error_code = 1 self.io.tool_warning("Response parsing error.") return From 70a5b7b706e07ddc57b65cbc7fd8e9cf55a1b0b4 Mon Sep 17 00:00:00 2001 From: Your Name Date: Wed, 12 Aug 2026 08:49:00 -0400 Subject: [PATCH 02/13] #639: Fallback to token estimator if usage fields are empty --- cecli/coders/base_coder.py | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/cecli/coders/base_coder.py b/cecli/coders/base_coder.py index b4d59de656a..0fea779967d 100755 --- a/cecli/coders/base_coder.py +++ b/cecli/coders/base_coder.py @@ -4280,9 +4280,21 @@ def calculate_and_show_tokens_and_cost(self, messages, completion=None): cache_hit_tokens = 0 cache_write_tokens = 0 - if completion and hasattr(completion, "usage") and completion.usage is not None: - prompt_tokens = completion.usage.prompt_tokens - completion_tokens = completion.usage.completion_tokens + if ( + completion + and nested.getter(completion, "usage.prompt_tokens") is not None + and nested.getter(completion, "usage.completion_tokens") is not None + ): + prompt_tokens = ( + nested.getter(completion.usage, "prompt_tokens", 0) + or nested.getter(completion.usage, "prompt_eval_count", 0) + or 0 + ) + completion_tokens = ( + nested.getter(completion.usage, "completion_tokens", 0) + or nested.getter(completion.usage, "eval_count", 0) + or 0 + ) cache_hit_tokens = ( getattr(completion.usage, "prompt_cache_hit_tokens", 0) or getattr(completion.usage, "cache_read_input_tokens", 0) From 4360341ec91655a2fb180d1e5b07b0fad80510e1 Mon Sep 17 00:00:00 2001 From: Your Name Date: Thu, 13 Aug 2026 00:06:17 -0400 Subject: [PATCH 03/13] Place warnings that clutter interface under verbose mode --- cecli/coders/base_coder.py | 2 +- cecli/main.py | 2 +- tests/basic/test_main.py | 25 ------------------------- 3 files changed, 2 insertions(+), 27 deletions(-) diff --git a/cecli/coders/base_coder.py b/cecli/coders/base_coder.py index 0fea779967d..03011929b6d 100755 --- a/cecli/coders/base_coder.py +++ b/cecli/coders/base_coder.py @@ -892,7 +892,7 @@ def get_announcements(self): rel_repo_dir = self.repo.get_rel_repo_dir() num_files = len(self.repo.get_tracked_files()) env_items.append(f"{rel_repo_dir} ({num_files:,} files)") - if num_files > 1000: + if num_files > 1000 and self.verbose: env_items.append( "Warning: For large repos, consider using --subtree-only and .cecli.ignore" ) diff --git a/cecli/main.py b/cecli/main.py index 957717bec4a..92694d5e83e 100644 --- a/cecli/main.py +++ b/cecli/main.py @@ -1437,7 +1437,7 @@ def get_io(pretty): ) io.tool_output(f"Cur working dir: {Path.cwd()}") io.tool_output(f"Git working dir: {git_root}") - if args.stream and args.cache_prompts: + if args.stream and args.cache_prompts and args.verbose: io.tool_warning("Cost estimates may be inaccurate when using streaming and caching.") if args.load: await commands.execute("load", args.load) diff --git a/tests/basic/test_main.py b/tests/basic/test_main.py index 87b47b9a4f9..31a0e9423cd 100644 --- a/tests/basic/test_main.py +++ b/tests/basic/test_main.py @@ -1302,31 +1302,6 @@ def test_model_accepts_settings_attribute(dummy_io, git_temp_dir, mocker): mock_instance.set_thinking_tokens.assert_not_called() -@pytest.mark.parametrize( - "flags,should_warn", - [ - (["--stream", "--cache-prompts"], True), - (["--stream"], False), - (["--cache-prompts", "--no-stream"], False), - ], - ids=["stream_and_cache", "stream_only", "cache_only"], -) -def test_stream_cache_warning(dummy_io, git_temp_dir, mocker, flags, should_warn): - """Test warning shown only when both streaming and caching are enabled.""" - MockInputOutput = mocker.patch("cecli.io.InputOutput", autospec=True) - mock_io_instance = MockInputOutput.return_value - mock_io_instance.pretty = True - args = flags + ["--exit", "--yes-always"] - main(args, **dummy_io) - if should_warn: - mock_io_instance.tool_warning.assert_called_with( - "Cost estimates may be inaccurate when using streaming and caching." - ) - else: - for call in mock_io_instance.tool_warning.call_args_list: - assert "Cost estimates may be inaccurate" not in call[0][0] - - def test_argv_file_respects_git(dummy_io, git_temp_dir): fname = Path("not_in_git.txt") fname.touch() From 71de3ac1f339088cbc22b25c2ea5b88da90f7f64 Mon Sep 17 00:00:00 2001 From: Your Name Date: Thu, 13 Aug 2026 00:14:46 -0400 Subject: [PATCH 04/13] Update memorizer system prompt --- cecli/helpers/agents/defaults/memorizer.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cecli/helpers/agents/defaults/memorizer.md b/cecli/helpers/agents/defaults/memorizer.md index ad7311bde45..40ed1eb5e80 100644 --- a/cecli/helpers/agents/defaults/memorizer.md +++ b/cecli/helpers/agents/defaults/memorizer.md @@ -46,7 +46,6 @@ Use these tags (and invent new ones as needed) to categorise facts: - **relationships** – how modules / systems interact with each other - **decisions** – architectural or design choices that were made and why - **entities** – important classes, functions, or data structures -- **changes** – summaries of changes made and their intent ## Tools @@ -67,10 +66,11 @@ context (e.g. compaction / yield summaries). Your job: 2. Use ReplaceFacts to insert new facts and delete the obsolete ones so the database stays clean and up-to-date. 3. Task specific details are not worth recording, focus on user intention and add facts - that would help them explain the project to another person succinctly + that would help them explain the project to another person succinctly. +4. Focus on strategy, purpose, structure, and expectations over project and activity specific details Start each response with an incrementing number at the beginning, e.g. "1) ...", "2) ..." -When this number hits at most 10, update what you can and yield. Do not deliberate over many turns. +Before this number hits at most 10, update what you can and yield. Do not deliberate over many turns. Important facts will be easy to search for and extract from the given context. Always prefer **concrete, reusable** facts over vague prose. From c6acac7f6af6f4873ea678a073188fb3cd5d7f43 Mon Sep 17 00:00:00 2001 From: Your Name Date: Thu, 13 Aug 2026 00:17:13 -0400 Subject: [PATCH 05/13] Fix repo test for windows --- tests/basic/test_sanity_check_repo.py | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/tests/basic/test_sanity_check_repo.py b/tests/basic/test_sanity_check_repo.py index 114a4de952e..e344a4e23eb 100644 --- a/tests/basic/test_sanity_check_repo.py +++ b/tests/basic/test_sanity_check_repo.py @@ -1,7 +1,9 @@ import asyncio import os import shutil +import stat import struct +import sys from unittest import mock import pytest @@ -82,6 +84,16 @@ def get_tracked_files_side_effect(): return mock_repo +def _rmtree_readonly(func, path, exc): + """Retry a failed rmtree operation after clearing the read-only attribute. + + Git marks loose object files read-only, which prevents them from being + deleted on Windows (PermissionError: [WinError 5] Access is denied). + """ + os.chmod(path, stat.S_IWRITE) + func(path) + + async def test_detached_head_state(create_repo, mock_io): repo_path, repo = create_repo # Detach the HEAD @@ -160,8 +172,14 @@ async def test_bare_repository(create_repo, mock_io, tmp_path): async def test_sanity_check_repo_with_corrupt_repo(create_repo, mock_io): repo_path, repo = create_repo - # Simulate a corrupt repository by removing the .git directory - shutil.rmtree(os.path.join(repo_path, ".git")) + # Simulate a corrupt repository by removing the .git directory. + # Git stores loose objects as read-only files, so on Windows we must + # clear the read-only attribute before they can be deleted. + git_dir = os.path.join(repo_path, ".git") + if sys.version_info >= (3, 12): + shutil.rmtree(git_dir, onexc=_rmtree_readonly) + else: + shutil.rmtree(git_dir, onerror=_rmtree_readonly) # Create the mock 'repo' object with GitError git_error = GitError("Unable to read git repository, it may be corrupt?") From 2b289056251d3a71fad3899f6b20fc995ccd4c6f Mon Sep 17 00:00:00 2001 From: Your Name Date: Thu, 13 Aug 2026 00:20:35 -0400 Subject: [PATCH 06/13] More detail for memorizer --- cecli/helpers/agents/defaults/memorizer.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/cecli/helpers/agents/defaults/memorizer.md b/cecli/helpers/agents/defaults/memorizer.md index 40ed1eb5e80..bd4e1e1d84b 100644 --- a/cecli/helpers/agents/defaults/memorizer.md +++ b/cecli/helpers/agents/defaults/memorizer.md @@ -67,7 +67,9 @@ context (e.g. compaction / yield summaries). Your job: the database stays clean and up-to-date. 3. Task specific details are not worth recording, focus on user intention and add facts that would help them explain the project to another person succinctly. -4. Focus on strategy, purpose, structure, and expectations over project and activity specific details +4. Record notes on strategy, purpose, structure, expectations, and unintuitive/novel discoveries over project + and activity specific details. + We are trying to preserve why we took the actions we have done, not a log of the actions themselves. Start each response with an incrementing number at the beginning, e.g. "1) ...", "2) ..." Before this number hits at most 10, update what you can and yield. Do not deliberate over many turns. From 7a4d6e2b6bb16475cc2e0d8287dac5b488f2673c Mon Sep 17 00:00:00 2001 From: Tom Date: Thu, 13 Aug 2026 09:04:17 +0200 Subject: [PATCH 07/13] 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 08/13] 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 09/13] 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 From 9e352965143541640665ee0351832398e590052b Mon Sep 17 00:00:00 2001 From: Your Name Date: Thu, 13 Aug 2026 06:59:06 -0400 Subject: [PATCH 10/13] Keep fact search deterministic --- cecli/tools/search_facts.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/cecli/tools/search_facts.py b/cecli/tools/search_facts.py index e93a90f9d71..f0fba66259c 100644 --- a/cecli/tools/search_facts.py +++ b/cecli/tools/search_facts.py @@ -64,11 +64,6 @@ async def execute(cls, coder, **kwargs): try: results = search_facts(coder, words=words, tags=tags) - if not results: - # Re-search without tags as fallback - if tags: - results = search_facts(coder, words=words, tags=None) - for r in results: response.append_result( { From 31ef9dac1e6c2922f33312cfd976b1fa0bbeb57d Mon Sep 17 00:00:00 2001 From: Your Name Date: Thu, 13 Aug 2026 07:02:46 -0400 Subject: [PATCH 11/13] #642: Encode commandstrings before hashing --- cecli/helpers/background_commands.py | 5 ++++- cecli/tools/command.py | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/cecli/helpers/background_commands.py b/cecli/helpers/background_commands.py index 47d4e1c41fa..988909c870f 100644 --- a/cecli/helpers/background_commands.py +++ b/cecli/helpers/background_commands.py @@ -12,6 +12,8 @@ from collections import deque from typing import Dict, List, Optional, Tuple +import xxhash + from cecli.decoding import safe_open try: @@ -419,7 +421,8 @@ def _generate_command_key(cls, command: str) -> str: Unique command key """ with cls._lock: - key = f"bg_{cls._next_id}_{hash(command) % 10000:04d}" + digest = xxhash.xxh64(command.encode("utf-8")).intdigest() + key = f"bg_{cls._next_id}_{digest % 10000:04d}" cls._next_id += 1 return key diff --git a/cecli/tools/command.py b/cecli/tools/command.py index 6a9b762e2fc..3712fcd5c0a 100644 --- a/cecli/tools/command.py +++ b/cecli/tools/command.py @@ -108,7 +108,7 @@ def _hash_command(command): if not command: return command - return xxhash.xxh64(command).hexdigest() + return xxhash.xxh64(command.encode("utf-8")).hexdigest() @classmethod async def execute( From 31af23b27fa9f6d0537eb0d902405c236fa03fe1 Mon Sep 17 00:00:00 2001 From: Your Name Date: Thu, 13 Aug 2026 07:28:40 -0400 Subject: [PATCH 12/13] Coerce tool calls to remove top level tool calling keys when present --- cecli/coders/base_coder.py | 6 ++ cecli/helpers/responses.py | 53 +++++++++++++++-- tests/tools/test_tool_arguments.py | 94 ++++++++++++++++++++++++++++++ 3 files changed, 147 insertions(+), 6 deletions(-) diff --git a/cecli/coders/base_coder.py b/cecli/coders/base_coder.py index 03011929b6d..40af87cea47 100755 --- a/cecli/coders/base_coder.py +++ b/cecli/coders/base_coder.py @@ -3255,6 +3255,12 @@ async def call_mcp_tool_from_session(self, session, tool_call): if not isinstance(arguments, dict): arguments = {} + # Some models mirror the OpenAI wire format and wrap the real params + # under a single "arguments"/"parameters"/"params" key. Unwrap so the + # server receives the actual parameters instead of rejecting the call + # with a "missing required parameter" error. + arguments = responses.coerce_tool_structure(arguments) + return await session.call_tool(name=name, arguments=arguments) async def process_tool_calls(self, tool_call_response): diff --git a/cecli/helpers/responses.py b/cecli/helpers/responses.py index 4eefc575393..e455d08db03 100644 --- a/cecli/helpers/responses.py +++ b/cecli/helpers/responses.py @@ -367,20 +367,24 @@ def unprefix_tool_call(tool_call): def parse_tool_arguments(args_string: str) -> dict: - """Parse tool-call arguments, merging glued ``{…}{} {…}`` object fragments.""" + """Parse tool-call arguments, merging glued ``{…}{} {…}`` object fragments. + + Also unwraps a single ``arguments``/``parameters``/``params`` wrapper key + that some models emit when mirroring the OpenAI wire format. + """ text = (args_string or "").strip() if not text: return {} try: parsed = json.loads(text) if isinstance(parsed, dict): - return parsed + return coerce_tool_structure(parsed) except json.JSONDecodeError: pass parsed = try_parse_json_value(text) if isinstance(parsed, dict): - return parsed + return coerce_tool_structure(parsed) chunks = utils.split_concatenated_json(text) if len(chunks) <= 1: @@ -388,18 +392,18 @@ def parse_tool_arguments(args_string: str) -> dict: return {} lone = try_parse_json_value(chunks[0]) if isinstance(lone, dict): - return lone + return coerce_tool_structure(lone) try: json_string = json_repair.repair_json(chunks[0], ensure_ascii=False) single = json.loads(json_string) except json.JSONDecodeError as err: return {"@error": f"Malformed JSON arguments: {err}"} - return single if isinstance(single, dict) else {} + return coerce_tool_structure(single) if isinstance(single, dict) else {} merged = merge_glued_json_objects(chunks) if merged is not None: - return merged + return coerce_tool_structure(merged) return { "@error": "Could not merge glued JSON objects: argument fragments are not all JSON objects" @@ -618,3 +622,40 @@ def _parse_bracket_arguments(payload_str: str) -> dict: arguments[key] = val_str return arguments + + +def coerce_tool_structure(args: dict) -> dict: + """Unwrap a single ``arguments``/``parameters``/``params`` wrapper key. + + Some models mirror the OpenAI wire format and emit the real params nested + under a single top-level ``arguments`` key (as a dict or a JSON-encoded + string) instead of as the params dict itself. Without normalization the + wrapper key is forwarded verbatim to the tool, which then fails required + parameter validation (e.g. MCP servers reporting "missing required + parameter"). + + Only unwraps when ``args`` has exactly one key matching one of the wrapper + names and that value is a dict (or a JSON string that parses to a dict), so + tools that legitimately declare a parameter named ``arguments`` are never + clobbered. + """ + if not isinstance(args, dict) or len(args) != 1: + return args + + for key in ("arguments", "parameters", "params"): + if key not in args: + continue + + value = args[key] + if isinstance(value, str): + try: + value = json.loads(value) + except json.JSONDecodeError: + return args + + if isinstance(value, dict): + return value + + return args + + return args diff --git a/tests/tools/test_tool_arguments.py b/tests/tools/test_tool_arguments.py index ab734be40ec..3cbce3e3a97 100644 --- a/tests/tools/test_tool_arguments.py +++ b/tests/tools/test_tool_arguments.py @@ -9,6 +9,7 @@ from cecli.coders.base_coder import Coder from cecli.helpers.responses import ( _repair_local_model_json_text, + coerce_tool_structure, extract_tools_from_content_json, merge_glued_json_objects, parse_tool_arguments, @@ -298,3 +299,96 @@ def test_parse_tool_arguments_uneven_glued_objects_with_list(): # The function tries to parse, failing on the mixed glued content assert "@error" in result assert "Could not merge glued JSON objects" in result["@error"] + + +def test_coerce_tool_structure_dict_value(): + """A single 'arguments' key wrapping a dict should be unwrapped.""" + assert coerce_tool_structure({"arguments": {"query": "x"}}) == {"query": "x"} + + +def test_coerce_tool_structure_json_string_value(): + """A single 'arguments' key holding a JSON-encoded string should be unwrapped.""" + assert coerce_tool_structure({"arguments": '{"query": "x"}'}) == {"query": "x"} + + +def test_coerce_tool_structure_parameters_key(): + """'parameters' and 'params' wrappers should also be unwrapped.""" + assert coerce_tool_structure({"parameters": {"path": "."}}) == {"path": "."} + assert coerce_tool_structure({"params": {"path": "."}}) == {"path": "."} + + +def test_coerce_tool_structure_ignores_multi_key_dict(): + """A dict with other keys alongside 'arguments' is not a wrapper and is left alone.""" + wrapped = {"arguments": {"query": "x"}, "extra": 1} + assert coerce_tool_structure(wrapped) is wrapped + + +def test_coerce_tool_structure_ignores_non_dict_values(): + """Non-dict (or unparsable) wrapper values are left alone.""" + assert coerce_tool_structure({"arguments": 5}) == {"arguments": 5} + assert coerce_tool_structure({"arguments": ["a"]}) == {"arguments": ["a"]} + assert coerce_tool_structure({"arguments": "not json"}) == {"arguments": "not json"} + + +def test_coerce_tool_structure_ignores_empty(): + """An empty dict is not a wrapper.""" + assert coerce_tool_structure({}) == {} + + +def test_parse_tool_arguments_unwraps_wire_format_wrapper(): + """A model mirroring the OpenAI wire format should still bind real params.""" + assert parse_tool_arguments('{"arguments": {"query": "repo:cecli"}}') == {"query": "repo:cecli"} + + +def test_parse_tool_arguments_unwraps_wire_format_string(): + """JSON-encoded string form of the wire-format wrapper should be unwrapped.""" + assert parse_tool_arguments('{"arguments": "{\\"query\\": \\"repo:cecli\\"}"}') == { + "query": "repo:cecli" + } + + +class _FakeMcpSession: + """Minimal stand-in for an mcp ClientSession that records the call.""" + + def __init__(self): + self.calls = [] + + async def call_tool(self, name=None, arguments=None): + self.calls.append((name, arguments)) + return SimpleNamespace(content=[]) + + +def _make_mini_coder(): + class MiniCoder(Coder): + def __init__(self): + pass + + return MiniCoder.__new__(MiniCoder) + + +async def test_call_mcp_tool_from_session_unwraps_arguments_key(): + """call_mcp_tool_from_session must unwrap a wire-format wrapper before session.call_tool.""" + coder = _make_mini_coder() + session = _FakeMcpSession() + tool_call = { + "function": { + "name": "github--search_code", + "arguments": '{"arguments": "{\\"query\\": \\"repo:cecli\\"}"}', + } + } + await coder.call_mcp_tool_from_session(session, tool_call) + assert session.calls == [("github--search_code", {"query": "repo:cecli"})] + + +async def test_call_mcp_tool_from_session_passes_plain_params_through(): + """Normal params must pass through call_mcp_tool_from_session unchanged.""" + coder = _make_mini_coder() + session = _FakeMcpSession() + tool_call = { + "function": { + "name": "github--list_issues", + "arguments": '{"owner": "cecli-dev", "repo": "cecli"}', + } + } + await coder.call_mcp_tool_from_session(session, tool_call) + assert session.calls == [("github--list_issues", {"owner": "cecli-dev", "repo": "cecli"})] From 99e40c476fa5bfe618c7afb68e203a47b38be68b Mon Sep 17 00:00:00 2001 From: Your Name Date: Thu, 13 Aug 2026 07:46:51 -0400 Subject: [PATCH 13/13] Give memorizer overview of recent tags --- cecli/helpers/memory/utils.py | 54 +++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/cecli/helpers/memory/utils.py b/cecli/helpers/memory/utils.py index e7396405947..1da6c8401d7 100644 --- a/cecli/helpers/memory/utils.py +++ b/cecli/helpers/memory/utils.py @@ -6,6 +6,7 @@ - ``add_fact(fact=..., tags=[...])`` – insert a fact with optional tags - ``remove_facts(id_facts=[...])`` – delete facts by id - ``search_facts(words=[...], tags=[...])`` – full-text search the fact store +- ``recent_tags(coder, limit=100)`` – most recently used tags + total count - ``invoke_memorizer(coder, additional_context=...)`` – fire the memorizer sub-agent with current context """ @@ -203,6 +204,51 @@ def search_facts( return results +def recent_tags(coder, limit: int = 100) -> dict: + """Return the most recently used tags plus the total tag count. + + Tags are deduplicated and ordered by the recency of the most recent + fact they are attached to (facts sorted by ``id_fact`` descending), + then truncated to *limit* entries. + + Args: + coder: The active Coder instance (used to locate the project root). + limit: Maximum number of recent tags to return (default 100). + + Returns: + A dict with keys ``tags`` (list of tag strings, most recent first) + and ``total`` (int count of all distinct tags in the store). + """ + + init_db(root=coder.root) + + conn = _get_connection(root=coder.root) + cursor = conn.cursor() + + # Tags ordered by the recency of the most recent fact they tag: + # sort facts by id descending and dedupe on the tag name. + cursor.execute( + """ + SELECT t.tag + FROM Facts f + JOIN FactTags ft ON ft.id_fact = f.id_fact + JOIN Tags t ON t.id_tag = ft.id_tag + GROUP BY t.tag + ORDER BY MAX(f.id_fact) DESC + LIMIT ? + """, + (limit,), + ) + + tags = [row["tag"] for row in cursor.fetchall()] + + cursor.execute("SELECT COUNT(*) AS total FROM Tags") + + total = cursor.fetchone()["total"] + + return {"tags": tags, "total": total} + + async def invoke_memorizer( coder, additional_context: str = "", @@ -242,6 +288,14 @@ async def invoke_memorizer( # Gather context pieces parts: list[str] = [] + # Recent tags (deduplicated, most recently used first) plus the total + # tag count — first context part so the memorizer sees active tags + recent = recent_tags(coder) + + tag_list = ", ".join(recent["tags"]) if recent["tags"] else "(none)" + + parts.append(f"## Recent Tags\n\n{tag_list}\n\n## Total Tags in DB\n\n{recent['total']}") + # Latest user message last_user = getattr(coder, "last_user_message", "")