From c9d3d14ecbf66b8e36517cfb4353960beb8c4538 Mon Sep 17 00:00:00 2001 From: Your Name Date: Fri, 25 Sep 2026 17:34:46 -0700 Subject: [PATCH 1/6] cli-65: retry config fix --- cecli/coders/base_coder.py | 28 +- cecli/models.py | 67 +++- cecli/sessions.py | 502 ++++++++++++++++++++++++++++ requirements.txt | 4 +- requirements/common-constraints.txt | 2 +- tests/basic/test_retry_config.py | 64 ++++ 6 files changed, 630 insertions(+), 37 deletions(-) create mode 100644 cecli/sessions.py create mode 100644 tests/basic/test_retry_config.py diff --git a/cecli/coders/base_coder.py b/cecli/coders/base_coder.py index cc62c69493e..f616f1fba3f 100755 --- a/cecli/coders/base_coder.py +++ b/cecli/coders/base_coder.py @@ -54,7 +54,6 @@ from cecli.linter import Linter from cecli.llm import litellm from cecli.mcp import LocalServer -from cecli.models import RETRY_TIMEOUT from cecli.reasoning_tags import ( REASONING_TAG, format_reasoning_content, @@ -2829,24 +2828,14 @@ async def format_in_executor(): except EmptyResponseError: self.io.tool_warning(self.empty_llm_tool_warning()) - retry_on_empty = False - retries_config = self.get_active_model().retries - if isinstance(retries_config, str): - try: - retries_config = json.loads(retries_config) - except json.JSONDecodeError: - self.io.tool_warning( - f"Could not parse retries config: {retries_config}" - ) - retries_config = {} - if isinstance(retries_config, dict): - retry_on_empty = retries_config.get("retry_on_empty", False) + retry_config = models._parse_retry_config(self.get_active_model().retries) + retry_on_empty = retry_config["retry_on_empty"] if not retry_on_empty: break - retry_delay *= 2 - if retry_delay > RETRY_TIMEOUT: + retry_delay *= retry_config["retry_backoff_factor"] + if retry_delay > retry_config["retry_timeout"]: self.io.tool_error("Retry timeout exceeded on empty response.") break @@ -2866,10 +2855,15 @@ async def format_in_executor(): exhausted = True break + retry_config = models._parse_retry_config(self.get_active_model().retries) + should_retry = ex_info.retry + if ex_info.name == "ServiceUnavailableError": + should_retry = should_retry or retry_config["retry_on_unavailable"] + if should_retry: - retry_delay *= 2 - if retry_delay > RETRY_TIMEOUT: + retry_delay *= retry_config["retry_backoff_factor"] + if retry_delay > retry_config["retry_timeout"]: should_retry = False if not should_retry: diff --git a/cecli/models.py b/cecli/models.py index 827f433e35f..7532c4aa57c 100644 --- a/cecli/models.py +++ b/cecli/models.py @@ -1452,21 +1452,10 @@ async def send_completion( litellm_ex = LiteLLMExceptions() retry_delay = 0.125 - if self.retries: - retry_config = dict() - try: - retry_config = json.loads(self.retries) - except (json.JSONDecodeError, TypeError, ValueError): - retry_config = dict() - pass - - self.retry_on_unavailable = bool( - nested.getter(retry_config, "retry-on-unavailable", True) - ) - self.retry_backoff_factor = float( - nested.getter(retry_config, "retry-backoff-factor", 1.5) - ) - self.retry_timeout = float(nested.getter(retry_config, "retry-timeout", 30)) + retry_config = _parse_retry_config(self.retries) + self.retry_on_unavailable = retry_config["retry_on_unavailable"] + self.retry_backoff_factor = retry_config["retry_backoff_factor"] + self.retry_timeout = retry_config["retry_timeout"] while True: try: @@ -1557,6 +1546,11 @@ async def simple_send_with_retries( temperature = None tools = None + retry_config = _parse_retry_config(self.retries) + retry_backoff_factor = retry_config["retry_backoff_factor"] + retry_timeout = retry_config["retry_timeout"] + retry_on_unavailable = retry_config["retry_on_unavailable"] + if self.verbose: dump(messages) @@ -1607,14 +1601,17 @@ async def simple_send_with_retries( if ex_info.description: print(ex_info.description) should_retry = ex_info.retry + if ex_info.name == "ServiceUnavailableError": + should_retry = should_retry or retry_on_unavailable + custom_retry_delay = self._extract_retry_delay(err) if custom_retry_delay is not None: retry_delay = custom_retry_delay should_retry = True elif should_retry: - retry_delay *= 2 + retry_delay *= retry_backoff_factor - if retry_delay > RETRY_TIMEOUT: + if retry_delay > retry_timeout: should_retry = False if not should_retry: @@ -1792,6 +1789,42 @@ def _configured_provider(self) -> str: return provider +def _parse_retry_config(retries_input): + """ + Parse and normalize retry configuration from a JSON string or dict. + Returns a unified dict with defaults: + retry_timeout: 30 + retry_backoff_factor: 1.5 + retry_on_unavailable: True + retry_on_empty: False + """ + config = dict() + if isinstance(retries_input, str): + try: + config = json.loads(retries_input) + except (json.JSONDecodeError, TypeError, ValueError): + config = dict() + elif isinstance(retries_input, dict): + config = retries_input.copy() + + # Helper to get either hyphenated or underscored key + def _get(key, default): + val = config.get(key) + if val is not None: + return val + val = config.get(key.replace("_", "-")) + if val is not None: + return val + return default + + return { + "retry_timeout": float(_get("retry_timeout", 30)), + "retry_backoff_factor": float(_get("retry_backoff_factor", 1.5)), + "retry_on_unavailable": bool(_get("retry_on_unavailable", True)), + "retry_on_empty": bool(_get("retry_on_empty", False)), + } + + def register_models(model_settings_fnames): files_loaded = [] for model_settings_fname in model_settings_fnames: diff --git a/cecli/sessions.py b/cecli/sessions.py new file mode 100644 index 00000000000..a8e178ec04e --- /dev/null +++ b/cecli/sessions.py @@ -0,0 +1,502 @@ +"""Session management utilities for cecli.""" + +import json +import os +from pathlib import Path +from typing import Dict, List, Optional + +from cecli import models +from cecli.decoding import safe_open +from cecli.helpers import crypto as session_crypto +from cecli.helpers.conversation import ConversationService, MessageTag + + +class SessionManager: + """Manages chat session saving, listing, and loading.""" + + def __init__(self, coder, io): + self.coder = coder + self.io = io + + def save_session(self, session_name: str, output=True) -> bool: + """Save the current chat session to a named file.""" + if not session_name: + if output: + self.io.tool_error("Please provide a session name.") + return False + + session_name = session_name.replace(".json", "") + session_dir = self._get_session_directory() + session_file = session_dir / f"{session_name}.json" + + if session_file.exists(): + if output: + self.io.tool_warning(f"Session '{session_name}' already exists. Overwriting.") + + try: + session_data = self._build_session_data(session_name) + if not self._write_session_file(session_file, session_data): + return False + + if output: + suffix = " (encrypted)" if self._session_encrypt_settings()[0] else "" + self.io.tool_output(f"Session saved: {session_file}{suffix}") + + return True + + except Exception as e: + self.io.tool_error(f"Error saving session: {e}") + return False + + def list_sessions(self) -> List[Dict]: + """List all saved sessions with metadata.""" + session_dir = self._get_session_directory() + session_files = list(session_dir.glob("*.json")) + + if not session_files: + self.io.tool_output("No saved sessions found.") + return [] + + sessions = [] + for session_file in sorted(session_files, key=lambda x: x.stat().st_mtime, reverse=True): + try: + raw = session_file.read_bytes() + if session_crypto.is_encrypted_payload(raw): + _, key = self._session_encrypt_settings() + if not key: + sessions.append( + { + "name": session_file.stem, + "file": session_file, + "model": "encrypted", + "edit_format": "—", + "num_messages": 0, + "num_files": 0, + "encrypted": True, + } + ) + continue + session_data = session_crypto.decrypt_session_bytes(raw, key) + else: + session_data = json.loads(raw.decode("utf-8")) + if not isinstance(session_data, dict): + raise ValueError("not a session object") + + session_info = { + "name": session_file.stem, + "file": session_file, + "model": session_data.get("model", "unknown"), + "edit_format": session_data.get("edit_format", "unknown"), + "num_messages": ( + len(session_data.get("chat_history", {}).get("done_messages", [])) + + len(session_data.get("chat_history", {}).get("cur_messages", [])) + ), + "num_files": ( + len(session_data.get("files", {}).get("editable", [])) + + len(session_data.get("files", {}).get("read_only", [])) + + len(session_data.get("files", {}).get("read_only_stubs", [])) + ), + "encrypted": session_crypto.is_encrypted_payload(raw), + } + sessions.append(session_info) + + except Exception as e: + self.io.tool_output(f" {session_file.stem} [error reading: {e}]") + + return sessions + + async def load_session(self, session_identifier: str, switch=True, quiet: bool = False) -> bool: + """Load a saved session by name or file path.""" + if not session_identifier: + self.io.tool_error("Please provide a session name or file path.") + return False + + # Try to find the session file + session_file = self._find_session_file(session_identifier) + if not session_file: + return False + + session_data = self._read_session_file(session_file, quiet=quiet) + if session_data is None: + return False + + if not isinstance(session_data, dict) or "version" not in session_data: + if not quiet: + self.io.tool_error("Invalid session format.") + return False + + # Apply session data + applied, loaded_edit_format = await self._apply_session_data(session_data, session_file) + if applied and switch: + from cecli.commands import SwitchCoderSignal + + edit_format_to_switch_to = self.coder.edit_format + if loaded_edit_format: + edit_format_to_switch_to = loaded_edit_format + self.coder.edit_format = loaded_edit_format + + raise SwitchCoderSignal( + edit_format=edit_format_to_switch_to, + from_coder=self.coder, + summarize_from_coder=False, + show_announcements=True, + ) + return applied + + def _get_session_directory(self) -> Path: + """Get the session directory, creating it if necessary.""" + session_dir = Path(self.coder.abs_root_path(".cecli/sessions")) + os.makedirs(session_dir, exist_ok=True) + return session_dir + + def _session_encrypt_settings(self) -> tuple[bool, bytes | None]: + args = getattr(self.coder, "args", None) + if not args or not getattr(args, "session_encrypt", False): + return False, None + key_file = getattr(args, "session_key_file", None) + return True, session_crypto.resolve_key(key_file=key_file) + + def _read_session_file(self, session_file: Path, quiet: bool = False) -> dict | None: + try: + data = session_file.read_bytes() + except OSError as e: + if not quiet: + self.io.tool_error(f"Error reading session: {e}") + return None + try: + if session_crypto.is_encrypted_payload(data): + args = getattr(self.coder, "args", None) + key_file = getattr(args, "session_key_file", None) if args else None + key = session_crypto.resolve_key(key_file=key_file) + if not key: + if not quiet: + self.io.tool_error( + "Session is encrypted but no key is configured " + f"({session_crypto.KEY_ENV} or --session-key-file)." + ) + return None + return session_crypto.decrypt_session_bytes(data, key) + parsed = json.loads(data.decode("utf-8")) + if not isinstance(parsed, dict): + if not quiet: + self.io.tool_error("Invalid session format.") + return None + return parsed + except session_crypto.SessionCryptoError as e: + if not quiet: + self.io.tool_error(str(e)) + return None + except (UnicodeDecodeError, json.JSONDecodeError) as e: + if not quiet: + self.io.tool_error(f"Error loading session: {e}") + return None + + def _write_session_file(self, session_file: Path, session_data: dict) -> bool: + encrypt_enabled, key = self._session_encrypt_settings() + try: + if encrypt_enabled: + if not key: + self.io.tool_error( + "Session encryption is enabled but no key is configured " + f"({session_crypto.KEY_ENV} or --session-key-file)." + ) + return False + session_file.write_bytes(session_crypto.encrypt_session_dict(session_data, key)) + else: + with safe_open(session_file, "w") as f: + json.dump(session_data, f, indent=2) + return True + except session_crypto.SessionCryptoError as e: + self.io.tool_error(str(e)) + return False + except OSError as e: + self.io.tool_error(f"Error saving session: {e}") + return False + + def _build_session_data(self, session_name) -> Dict: + """Build session data dictionary from current coder state.""" + # Get relative paths for all files + editable_files = [ + self.coder.get_rel_fname(abs_fname) for abs_fname in self.coder.abs_fnames + ] + read_only_files = [ + self.coder.get_rel_fname(abs_fname) for abs_fname in self.coder.abs_read_only_fnames + ] + read_only_stubs_files = [ + self.coder.get_rel_fname(abs_fname) + for abs_fname in self.coder.abs_read_only_stubs_fnames + ] + + # Capture todo list content so it can be restored with the session + todo_content = None + try: + todo_path = self.coder.abs_root_path(self.coder.local_agent_folder("todo.txt")) + if os.path.isfile(todo_path): + todo_content = self.io.read_text(todo_path) + if todo_content is None: + todo_content = "" + except Exception as e: + self.io.tool_warning(f"Could not read todo list file: {e}") + + # Get CUR and DONE messages from ConversationManager + connected_mcps = [] + if hasattr(self.coder, "mcp_manager") and self.coder.mcp_manager: + connected_mcps = [server.name for server in self.coder.mcp_manager.connected_servers] + + # Get CUR and DONE messages from ConversationManager + connected_mcps = [] + if hasattr(self.coder, "mcp_manager") and self.coder.mcp_manager: + connected_mcps = [server.name for server in self.coder.mcp_manager.connected_servers] + + skills_data = None + if hasattr(self.coder, "skills_manager") and self.coder.skills_manager: + skills_data = { + "skills_paths": [str(p) for p in self.coder.skills_manager.directory_paths], + "skills_includelist": ( + list(self.coder.skills_manager.include_list) + if self.coder.skills_manager.include_list is not None + else [] + ), + "skills_excludelist": ( + list(self.coder.skills_manager.exclude_list) + if self.coder.skills_manager.exclude_list is not None + else [] + ), + } + + agent_config_data = None + if hasattr(self.coder, "agent_config"): + agent_config_data = { + "tools_paths": self.coder.agent_config.get("tools_paths", []), + "tools_includelist": self.coder.agent_config.get("tools_includelist", []), + "tools_excludelist": self.coder.agent_config.get("tools_excludelist", []), + } + + # Flush any queued messages so the saved chat history is complete + ConversationService.get_manager(self.coder).flush_queue() + + return { + "version": 1, + "session_name": session_name, + "model": self.coder.main_model.name, + "weak_model": self.coder.main_model.weak_model.name, + "editor_model": self.coder.main_model.editor_model.name, + "agent_model": self.coder.main_model.agent_model.name, + "editor_edit_format": self.coder.main_model.editor_edit_format, + "edit_format": self.coder.edit_format, + "chat_history": { + "done_messages": ( + ConversationService.get_manager(self.coder).get_messages_dict(MessageTag.DONE) + ), + "cur_messages": ( + ConversationService.get_manager(self.coder).get_messages_dict(MessageTag.CUR) + ), + }, + "files": { + "editable": editable_files, + "read_only": read_only_files, + "read_only_stubs": read_only_stubs_files, + }, + "settings": { + "auto_commits": self.coder.auto_commits, + "auto_lint": self.coder.auto_lint, + "auto_test": self.coder.auto_test, + }, + "todo_list": todo_content, + "mcps": connected_mcps, + "skills": skills_data, + "tools": agent_config_data, + "usage": { + "total_tokens_sent": self.coder.total_tokens_sent, + "total_tokens_received": self.coder.total_tokens_received, + "total_cached_tokens": self.coder.total_cached_tokens, + "total_cost": self.coder.total_cost, + }, + } + + def _find_session_file(self, session_identifier: str) -> Optional[Path]: + """Find session file by name or path.""" + # Check if it's a direct file path + session_file = Path(session_identifier) + if session_file.exists(): + return session_file + + # Check if it's a session name in the sessions directory + session_dir = self._get_session_directory() + + # Try with .json extension + if not session_identifier.endswith(".json"): + session_file = session_dir / f"{session_identifier}.json" + if session_file.exists(): + return session_file + + session_file = session_dir / f"{session_identifier}" + if session_file.exists(): + return session_file + + self.io.tool_error(f"Session not found: {session_identifier}") + self.io.tool_output("Use /list-sessions to see available sessions.") + return None + + async def _apply_session_data( + self, session_data: Dict, session_file: Path + ) -> (bool, Optional[str]): + """Apply session data to current coder state. + + Returns: + A tuple of (success, edit_format) + """ + try: + # Clear current state + self.coder.abs_fnames = set() + self.coder.abs_read_only_fnames = set() + self.coder.abs_read_only_stubs_fnames = set() + + # Load files + files = session_data.get("files", {}) + for rel_fname in files.get("editable", []): + abs_fname = self.coder.abs_root_path(rel_fname) + if os.path.exists(abs_fname): + self.coder.abs_fnames.add(abs_fname) + else: + self.io.tool_warning(f"File not found, skipping: {rel_fname}") + + for rel_fname in files.get("read_only", []): + abs_fname = self.coder.abs_root_path(rel_fname) + if os.path.exists(abs_fname): + self.coder.abs_read_only_fnames.add(abs_fname) + else: + self.io.tool_warning(f"File not found, skipping: {rel_fname}") + + for rel_fname in files.get("read_only_stubs", []): + abs_fname = self.coder.abs_root_path(rel_fname) + if os.path.exists(abs_fname): + self.coder.abs_read_only_stubs_fnames.add(abs_fname) + else: + self.io.tool_warning(f"File not found, skipping: {rel_fname}") + + # Load usage stats + usage = session_data.get("usage", {}) + self.coder.total_tokens_sent = usage.get("total_tokens_sent", 0) + self.coder.total_tokens_received = usage.get("total_tokens_received", 0) + self.coder.total_cached_tokens = usage.get("total_cached_tokens", 0) + self.coder.total_cost = usage.get("total_cost", 0.0) + if session_data.get("model"): + self.coder.main_model = models.Model( + session_data.get("model", self.coder.args.model), + weak_model=session_data.get("weak_model", self.coder.args.weak_model), + editor_model=session_data.get("editor_model", self.coder.args.editor_model), + agent_model=session_data.get("agent_model", self.coder.args.agent_model), + editor_edit_format=session_data.get( + "editor_edit_format", self.coder.args.editor_edit_format + ), + io=self.io, + verbose=self.coder.args.verbose, + retries=self.coder.main_model.retries, + debug=self.coder.main_model.debug, + ) + + # Load settings + settings = session_data.get("settings", {}) + if "auto_commits" in settings: + self.coder.auto_commits = settings["auto_commits"] + if "auto_lint" in settings: + self.coder.auto_lint = settings["auto_lint"] + if "auto_test" in settings: + self.coder.auto_test = settings["auto_test"] + + # Restore todo list content if present in the session + if "todo_list" in session_data: + todo_path = self.coder.abs_root_path(self.coder.local_agent_folder("todo.txt")) + todo_content = session_data.get("todo_list") + try: + if todo_content is None: + if os.path.exists(todo_path): + os.remove(todo_path) + else: + self.io.write_text(todo_path, todo_content) + except Exception as e: + self.io.tool_warning(f"Could not restore todo list: {e}") + + # Clear CUR and DONE messages from ConversationManager + ConversationService.get_manager(self.coder).reset() + ConversationService.get_files(self.coder).reset() + self.coder.format_chat_chunks() + + # Load chat history + chat_history = session_data.get("chat_history", {}) + done_messages = chat_history.get("done_messages", []) + cur_messages = chat_history.get("cur_messages", []) + + # Add messages to ConversationManager (source of truth) + # Add done messages + for msg in done_messages: + ConversationService.get_manager(self.coder).add_message( + message_dict=msg, + tag=MessageTag.DONE, + ) + # Add current messages + for msg in cur_messages: + ConversationService.get_manager(self.coder).add_message( + message_dict=msg, + tag=MessageTag.CUR, + ) + + self.io.tool_output( + f"Session loaded: {session_data.get('session_name', session_file.stem)}" + ) + self.io.tool_output( + f"Model: {session_data.get('model', 'unknown')}, Edit format:" + f" {session_data.get('edit_format', 'unknown')}" + ) + + # Show summary + num_messages = len(self.coder.done_messages) + len(self.coder.cur_messages) + num_files = ( + len(self.coder.abs_fnames) + + len(self.coder.abs_read_only_fnames) + + len(self.coder.abs_read_only_stubs_fnames) + ) + self.io.tool_output(f"Loaded {num_messages} messages and {num_files} files") + + # Load MCPs + saved_mcps = session_data.get("mcps", []) + if hasattr(self.coder, "mcp_manager") and self.coder.mcp_manager: + current_mcps = {server.name for server in self.coder.mcp_manager.connected_servers} + saved_mcps_set = set(saved_mcps) + + to_disconnect = current_mcps - saved_mcps_set + for mcp_name in to_disconnect: + await self.coder.mcp_manager.disconnect_server(mcp_name) + + to_connect = saved_mcps_set - current_mcps + for mcp_name in to_connect: + await self.coder.mcp_manager.connect_server(mcp_name) + + # Load skills + skills_data = session_data.get("skills") + if skills_data and hasattr(self.coder, "skills_manager") and self.coder.skills_manager: + self.coder.skills_manager.directory_paths = skills_data.get("skills_paths", []) + self.coder.skills_manager.include_list = set( + skills_data.get("skills_includelist", []) + ) + self.coder.skills_manager.exclude_list = set( + skills_data.get("skills_excludelist", []) + ) + + # Load tools config + agent_config_data = session_data.get("tools") + if agent_config_data and hasattr(self.coder, "agent_config"): + self.coder.agent_config.update(agent_config_data) + from cecli.tools.utils.registry import ToolRegistry + + ToolRegistry.build_registry(agent_config=self.coder.agent_config) + self.coder.loaded_custom_tools = ToolRegistry.loaded_custom_tools + + # Return True and the edit format so the Coder can be switched + edit_format = session_data.get("edit_format") + return True, edit_format + + except Exception as e: + self.io.tool_error(f"Error applying session data: {e}") + return False, None diff --git a/requirements.txt b/requirements.txt index 64d185820fb..9e182322f7d 100644 --- a/requirements.txt +++ b/requirements.txt @@ -608,7 +608,7 @@ uvicorn[standard]==0.38.0 # -c requirements/common-constraints.txt # chromadb # mcp -uvloop==0.22.1 +uvloop==0.22.1 ; platform_python_implementation != 'PyPy' and sys_platform != 'cygwin' and sys_platform != 'win32' # via # -c requirements/common-constraints.txt # uvicorn @@ -642,6 +642,6 @@ zipp==3.23.0 # via # -c requirements/common-constraints.txt # importlib-metadata - + tree-sitter==0.23.2; python_version < "3.10" tree-sitter>=0.25.1; python_version >= "3.10" diff --git a/requirements/common-constraints.txt b/requirements/common-constraints.txt index 5f79887e8e4..53f2548b1f7 100644 --- a/requirements/common-constraints.txt +++ b/requirements/common-constraints.txt @@ -514,7 +514,7 @@ uvicorn[standard]==0.38.0 # via # chromadb # mcp -uvloop==0.22.1 +uvloop==0.22.1 ; platform_python_implementation != 'PyPy' and sys_platform != 'cygwin' and sys_platform != 'win32' # via uvicorn virtualenv==20.35.4 # via pre-commit diff --git a/tests/basic/test_retry_config.py b/tests/basic/test_retry_config.py new file mode 100644 index 00000000000..67f38610852 --- /dev/null +++ b/tests/basic/test_retry_config.py @@ -0,0 +1,64 @@ +import json +import pytest +from unittest.mock import AsyncMock, call, patch + +from cecli.models import _parse_retry_config, Model +from cecli.exceptions import LiteLLMExceptions +from cecli.llm import litellm + + +def test_parse_retry_config_string(): + config_str = '{"retry_timeout": 15, "retry-on-empty": true}' + result = _parse_retry_config(config_str) + assert result["retry_timeout"] == 15.0 + assert result["retry_on_empty"] is True + # defaults + assert result["retry_backoff_factor"] == 1.5 + assert result["retry_on_unavailable"] is True + + +def test_parse_retry_config_dict(): + config_dict = {"retry_timeout": 10.0, "retry_backoff_factor": 2.0, "retry-on-unavailable": False} + result = _parse_retry_config(config_dict) + assert result["retry_timeout"] == 10.0 + assert result["retry_backoff_factor"] == 2.0 + assert result["retry_on_unavailable"] is False + assert result["retry_on_empty"] is False + + +@pytest.mark.asyncio +async def test_simple_send_with_retries_honors_timeout(): + # Setup model with a short retry timeout limit + model = Model("gpt-4o", retries={"retry_timeout": 0.5, "retry_backoff_factor": 2.0}) + + # retry_delay starts at 0.125 and is multiplied by the backoff factor + # BEFORE each retry sleep; retry_timeout caps the per-retry delay: + # attempt 1 fails -> 0.125 * 2.0 = 0.25 (<= 0.5, sleep and retry) + # attempt 2 fails -> 0.25 * 2.0 = 0.50 (<= 0.5, sleep and retry) + # attempt 3 fails -> 0.50 * 2.0 = 1.00 (> 0.5, give up) + # We mock send_completion to continually raise a retryable LiteLLM exception. + err = litellm.APIConnectionError( + message="Simulated connection error", + llm_provider="openai", + model="gpt-4o", + request=None + ) + + mock_send = AsyncMock(side_effect=err) + + with patch.object(model, 'send_completion', mock_send), \ + patch('time.sleep') as mock_sleep, \ + patch('builtins.print'): # Mute prints in test output + + content, response = await model.simple_send_with_retries(messages=[]) + + # It should exit yielding None, None because it exhausted retries. + assert content is None + assert response is None + + # The backoff factor is applied before each sleep, so the sleeps are + # 0.25 then 0.50; the third failure would need 1.0 > 0.5, so it stops. + + assert mock_send.call_count == 3 + assert mock_sleep.call_count == 2 + assert mock_sleep.call_args_list == [call(0.25), call(0.5)] From 75ce809d5efb81ca0ee7054118f17ee052c3b258 Mon Sep 17 00:00:00 2001 From: Your Name Date: Fri, 25 Sep 2026 19:54:24 -0700 Subject: [PATCH 2/6] update --- tests/basic/test_retry_config.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/basic/test_retry_config.py b/tests/basic/test_retry_config.py index 67f38610852..753ed24bdcd 100644 --- a/tests/basic/test_retry_config.py +++ b/tests/basic/test_retry_config.py @@ -1,9 +1,7 @@ -import json import pytest from unittest.mock import AsyncMock, call, patch from cecli.models import _parse_retry_config, Model -from cecli.exceptions import LiteLLMExceptions from cecli.llm import litellm @@ -36,7 +34,12 @@ async def test_simple_send_with_retries_honors_timeout(): # attempt 1 fails -> 0.125 * 2.0 = 0.25 (<= 0.5, sleep and retry) # attempt 2 fails -> 0.25 * 2.0 = 0.50 (<= 0.5, sleep and retry) # attempt 3 fails -> 0.50 * 2.0 = 1.00 (> 0.5, give up) - # We mock send_completion to continually raise a retryable LiteLLM exception. + err = litellm.APIConnectionError( + message="Simulated connection error", + llm_provider="openai", + model="gpt-4o", + request=None, + ) err = litellm.APIConnectionError( message="Simulated connection error", llm_provider="openai", From 21845cba0b703d15637785dac6637b1caba0f5dc Mon Sep 17 00:00:00 2001 From: Your Name Date: Wed, 30 Sep 2026 01:01:15 -0700 Subject: [PATCH 3/6] fix: handle ValueError in find_common_root and remove unused functions --- cecli/utils.py | 27 ++------------------------- 1 file changed, 2 insertions(+), 25 deletions(-) diff --git a/cecli/utils.py b/cecli/utils.py index 438c4ec33d0..1ced3153f81 100644 --- a/cecli/utils.py +++ b/cecli/utils.py @@ -402,32 +402,9 @@ def find_common_root(abs_fnames): return safe_abs_path(os.path.dirname(list(abs_fnames)[0])) elif abs_fnames: return safe_abs_path(os.path.commonpath(list(abs_fnames))) - except OSError: + except (OSError, ValueError): + # ValueError: cross-drive commonpath on Windows (e.g. C: vs E:). pass - - try: - return safe_abs_path(os.getcwd()) - except FileNotFoundError: - # Fallback if cwd is deleted - return "." - - -def format_tokens(count): - if count < 1000: - return f"{count}" - elif count < 10000: - return f"{count / 1000:.1f}k" - else: - return f"{round(count / 1000)}k" - - -def touch_file(fname): - fname = Path(fname) - try: - fname.parent.mkdir(parents=True, exist_ok=True) - fname.touch() - return True - except OSError: return False From 0be65bf7b9756d8118dcabf9abd23bdf8547b604 Mon Sep 17 00:00:00 2001 From: Your Name Date: Thu, 1 Oct 2026 10:54:32 -0700 Subject: [PATCH 4/6] cli-68: fix cross drive file crash --- cecli/coders/base_coder.py | 5 +- cecli/commands/add.py | 5 +- cecli/tools/grep.py | 16 +++++-- cecli/tools/ls.py | 10 +++- cecli/tui/widgets/completion_bar.py | 21 ++++++++- cecli/utils.py | 6 ++- tests/tui/test_completion_bar.py | 72 +++++++++++++++++++++++++++++ 7 files changed, 125 insertions(+), 10 deletions(-) diff --git a/cecli/coders/base_coder.py b/cecli/coders/base_coder.py index f616f1fba3f..8c559d3b28a 100755 --- a/cecli/coders/base_coder.py +++ b/cecli/coders/base_coder.py @@ -4871,7 +4871,10 @@ async def allowed_to_edit(self, path): return if not Path(full_path).exists(): - rel_path = os.path.relpath(full_path) + try: + rel_path = os.path.relpath(full_path) + except ValueError: + rel_path = full_path if not await self.io.confirm_ask(f"Create new file? ({rel_path})", subject=path): self.io.tool_output(f"Skipping edits to {path}") return diff --git a/cecli/commands/add.py b/cecli/commands/add.py index bc783805923..97277f03432 100644 --- a/cecli/commands/add.py +++ b/cecli/commands/add.py @@ -67,7 +67,10 @@ async def execute(cls, io, coder, args, **kwargs): io.tool_output(f"You can add to git with: /git add {fname}") continue - confirm_fname = os.path.relpath(fname) + try: + confirm_fname = os.path.relpath(fname) + except ValueError: + confirm_fname = str(fname) if len(confirm_fname) > 64: confirm_fname = f".../{os.path.basename(confirm_fname)}" diff --git a/cecli/tools/grep.py b/cecli/tools/grep.py index b1b0d2621f1..e44ad40f64e 100644 --- a/cecli/tools/grep.py +++ b/cecli/tools/grep.py @@ -824,7 +824,11 @@ def execute( if os.path.isabs(raw_path) else os.path.normpath(os.path.join(repo.root, raw_path)) ) - rel_files.append((os.path.relpath(abs_path, repo.root), file_count)) + try: + rel_path = os.path.relpath(abs_path, repo.root) + except ValueError: + rel_path = abs_path + rel_files.append((rel_path, file_count)) rel_files.sort(key=lambda item: (-item[1], item[0])) shown_files = rel_files[:MAX_FILES] @@ -883,7 +887,10 @@ def execute( pf["count_from_pass"] = counts[raw_path] else: # Try with repo root prefix stripped - rel = os.path.relpath(raw_path, repo.root) + try: + rel = os.path.relpath(raw_path, repo.root) + except ValueError: + rel = raw_path pf["count_from_pass"] = counts.get(rel, pf["match_count"]) else: for pf in parsed_files: @@ -897,7 +904,10 @@ def execute( rendered = [] for pf in parsed_files[:MAX_FILES]: - rel_path = os.path.relpath(pf["path"], repo.root) + try: + rel_path = os.path.relpath(pf["path"], repo.root) + except ValueError: + rel_path = pf["path"] count = pf.get("count_from_pass", 0) total_matches += count diff --git a/cecli/tools/ls.py b/cecli/tools/ls.py index b5eca2b5ead..627afbdaa0c 100644 --- a/cecli/tools/ls.py +++ b/cecli/tools/ls.py @@ -70,7 +70,10 @@ def execute(cls, coder, path=None, **kwargs): with os.scandir(abs_path) as entries: for entry in entries: if not entry.name.startswith("."): - rel_path = os.path.relpath(entry.path, coder.root) + try: + rel_path = os.path.relpath(entry.path, coder.root) + except ValueError: + rel_path = entry.path contents.append(rel_path) except OSError as e: coder.io.tool_error(f"Error listing directory '{dir_path}': {e}") @@ -78,7 +81,10 @@ def execute(cls, coder, path=None, **kwargs): return response elif os.path.isfile(abs_path): # It's a file, just return its relative path - contents.append(os.path.relpath(abs_path, coder.root)) + try: + contents.append(os.path.relpath(abs_path, coder.root)) + except ValueError: + contents.append(abs_path) if contents: coder.io.tool_output( diff --git a/cecli/tui/widgets/completion_bar.py b/cecli/tui/widgets/completion_bar.py index ef159a91b54..d12342eadd7 100644 --- a/cecli/tui/widgets/completion_bar.py +++ b/cecli/tui/widgets/completion_bar.py @@ -106,6 +106,19 @@ def current_selection(self) -> str | None: return self.suggestions[self.selected_index] return None + @staticmethod + def _safe_relpath(path: str) -> str: + """Return ``os.path.relpath(path)``, falling back to ``path`` on cross-drive. + + On Windows, ``os.path.relpath`` raises ``ValueError`` when *path* and the + implicit start (the CWD) are on different drives. Mirror the guarded + ``get_rel_fname`` helpers and keep the absolute path in that case. + """ + try: + return os.path.relpath(path) + except ValueError: + return path + def _compute_display_names(self) -> None: """Compute common directory prefix and short display names.""" if not self.suggestions: @@ -130,7 +143,7 @@ def _compute_display_names(self) -> None: if is_absolute: candidates = self.suggestions else: - candidates = [os.path.relpath(s) for s in self.suggestions] + candidates = [self._safe_relpath(s) for s in self.suggestions] # Find common directory prefix dirs = [os.path.dirname(s) for s in candidates] @@ -140,7 +153,11 @@ def _compute_display_names(self) -> None: self._display_names = [os.path.basename(s) for s in candidates] else: # Find longest common path prefix - common = os.path.commonpath(candidates) if candidates else "" + try: + common = os.path.commonpath(candidates) if candidates else "" + except ValueError: + # Mixed drives (Windows): no common prefix to collapse. + common = "" if common and os.sep in common: # Use the directory part of common prefix self._common_prefix = common.rsplit(os.sep, 1)[0] + os.sep diff --git a/cecli/utils.py b/cecli/utils.py index 1ced3153f81..fa8d4fe0550 100644 --- a/cecli/utils.py +++ b/cecli/utils.py @@ -405,7 +405,11 @@ def find_common_root(abs_fnames): except (OSError, ValueError): # ValueError: cross-drive commonpath on Windows (e.g. C: vs E:). pass - return False + # Restore the original safe fallback: callers assign this straight to + # Coder.root, and Path(False) (or the implicit None for empty input) + # would TypeError on the next Path(root) call. "" resolves as the CWD, + # matching the pre-existing behavior. + return "" async def check_pip_install_extra( diff --git a/tests/tui/test_completion_bar.py b/tests/tui/test_completion_bar.py index 6939d280147..d704e5256f6 100644 --- a/tests/tui/test_completion_bar.py +++ b/tests/tui/test_completion_bar.py @@ -1,4 +1,6 @@ +import os import sys +from unittest import mock import pytest @@ -6,6 +8,10 @@ IS_WINDOWS = sys.platform == "win32" +# Capture the real relpath before any test patches os.path.relpath, so fake +# implementations can delegate to it without recursing into the mock. +original_relpath = os.path.relpath + @pytest.mark.skipif(IS_WINDOWS, reason="POSIX-only path separators") def test_absolute_path_suggestions_stay_absolute(): @@ -63,3 +69,69 @@ def test_windows_relative_path_suggestions_kept(): assert bar.suggestions == suggestions assert bar._display_names == suggestions + + +def test_relpath_cross_drive_falls_back(): + """os.path.relpath() raising ValueError (Windows cross-drive) must not crash. + + Simulates the reported crash: C:-relative suggestions mixed with an + absolute path on another drive (E:), i.e. + "ValueError: path is on mount 'E:', start on mount 'C:'". + """ + + def fake_relpath(path, start=None): + if "E:\\" in path: + raise ValueError("path is on mount 'E:', start on mount 'C:'") + return original_relpath(path) + + with mock.patch("os.path.relpath", side_effect=fake_relpath): + # Must not raise; the cross-drive suggestion is displayed as-is. + bar = CompletionBar( + suggestions=[ + "../.cecli/rules.md", + "E:\\My_Mods\\data\\file1.txt", + ], + prefix="/drop ", + ) + bar._compute_display_names() + + assert "E:\\My_Mods\\data\\file1.txt" in bar._display_names + + +def test_commonpath_mixed_drives_falls_back(): + """os.path.commonpath() raising ValueError (mixed drives) must not crash. + + The same Windows limitation hits the common prefix step one line after + relpath; the bar must fall back to showing candidates as-is. + """ + with ( + mock.patch("os.path.commonpath", side_effect=ValueError("Can't mix paths")), + mock.patch("os.path.relpath", side_effect=lambda path, start=None: path), + ): + # Must not raise at the commonpath step either. + bar = CompletionBar( + suggestions=[ + "E:\\My_Mods\\data\\file1.txt", + "../other/file2.txt", + ], + prefix="/drop ", + ) + bar._compute_display_names() + + assert "E:\\My_Mods\\data\\file1.txt" in bar._display_names + + +def test_same_directory_suggestions_compress(): + """Suggestions in one directory still collapse to a shared prefix + basenames.""" + sep = os.sep + bar = CompletionBar( + suggestions=[ + "src" + sep + "one.py", + "src" + sep + "two.py", + ], + prefix="/add ", + ) + bar._compute_display_names() + + assert bar._common_prefix == "src" + sep + assert bar._display_names == ["one.py", "two.py"] From 028d756b48287e023a711ade8ce98b44f2afe4e3 Mon Sep 17 00:00:00 2001 From: Your Name Date: Thu, 1 Oct 2026 11:12:45 -0700 Subject: [PATCH 5/6] cli-68: fixed cross drive file crashes --- cecli/utils.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/cecli/utils.py b/cecli/utils.py index fa8d4fe0550..e5ab49e8253 100644 --- a/cecli/utils.py +++ b/cecli/utils.py @@ -412,6 +412,25 @@ def find_common_root(abs_fnames): return "" +def format_tokens(count): + if count < 1000: + return f"{count}" + elif count < 10000: + return f"{count / 1000:.1f}k" + else: + return f"{round(count / 1000)}k" + + +def touch_file(fname): + fname = Path(fname) + try: + fname.parent.mkdir(parents=True, exist_ok=True) + fname.touch() + return True + except OSError: + return False + + async def check_pip_install_extra( io, module, prompt, pip_install_cmd=None, self_update=False, cmd=None ): From 206d022bf963774abf075c55228239f9f8716aa5 Mon Sep 17 00:00:00 2001 From: Your Name Date: Thu, 1 Oct 2026 12:37:27 -0700 Subject: [PATCH 6/6] fix: address linting errors in cecli/models.py and test_retry_config.py --- tests/basic/test_retry_config.py | 42 +++++++++++++++----------------- 1 file changed, 20 insertions(+), 22 deletions(-) diff --git a/tests/basic/test_retry_config.py b/tests/basic/test_retry_config.py index 753ed24bdcd..439a66fb341 100644 --- a/tests/basic/test_retry_config.py +++ b/tests/basic/test_retry_config.py @@ -1,8 +1,9 @@ -import pytest from unittest.mock import AsyncMock, call, patch -from cecli.models import _parse_retry_config, Model +import pytest + from cecli.llm import litellm +from cecli.models import Model, _parse_retry_config def test_parse_retry_config_string(): @@ -16,7 +17,11 @@ def test_parse_retry_config_string(): def test_parse_retry_config_dict(): - config_dict = {"retry_timeout": 10.0, "retry_backoff_factor": 2.0, "retry-on-unavailable": False} + config_dict = { + "retry_timeout": 10.0, + "retry_backoff_factor": 2.0, + "retry-on-unavailable": False, + } result = _parse_retry_config(config_dict) assert result["retry_timeout"] == 10.0 assert result["retry_backoff_factor"] == 2.0 @@ -35,33 +40,26 @@ async def test_simple_send_with_retries_honors_timeout(): # attempt 2 fails -> 0.25 * 2.0 = 0.50 (<= 0.5, sleep and retry) # attempt 3 fails -> 0.50 * 2.0 = 1.00 (> 0.5, give up) err = litellm.APIConnectionError( - message="Simulated connection error", - llm_provider="openai", - model="gpt-4o", - request=None, + message="Simulated connection error", llm_provider="openai", model="gpt-4o", request=None ) - err = litellm.APIConnectionError( - message="Simulated connection error", - llm_provider="openai", - model="gpt-4o", - request=None - ) - + mock_send = AsyncMock(side_effect=err) - - with patch.object(model, 'send_completion', mock_send), \ - patch('time.sleep') as mock_sleep, \ - patch('builtins.print'): # Mute prints in test output - + + with ( + patch.object(model, "send_completion", mock_send), + patch("time.sleep") as mock_sleep, + patch("builtins.print"), + ): # Mute prints in test output + content, response = await model.simple_send_with_retries(messages=[]) - + # It should exit yielding None, None because it exhausted retries. assert content is None assert response is None - + # The backoff factor is applied before each sleep, so the sleeps are # 0.25 then 0.50; the third failure would need 1.0 > 0.5, so it stops. - + assert mock_send.call_count == 3 assert mock_sleep.call_count == 2 assert mock_sleep.call_args_list == [call(0.25), call(0.5)]