diff --git a/code_puppy/agents/base_agent.py b/code_puppy/agents/base_agent.py index 86ff147de..ac94d6e2e 100644 --- a/code_puppy/agents/base_agent.py +++ b/code_puppy/agents/base_agent.py @@ -1308,6 +1308,12 @@ def reload_code_generation_agent(self, message_group: Optional[str] = None): register_tools_for_agent, ) + # Invalidate the project-local rules cache so a fresh read from the + # current working directory is performed on the next load_puppy_rules() + # call. This is critical for /cd: the user may have switched to a + # different project that has its own AGENT.md (or none at all). + self._puppy_rules = None + if message_group is None: message_group = str(uuid.uuid4()) diff --git a/code_puppy/command_line/core_commands.py b/code_puppy/command_line/core_commands.py index 26360f6e1..0608baf89 100644 --- a/code_puppy/command_line/core_commands.py +++ b/code_puppy/command_line/core_commands.py @@ -55,7 +55,7 @@ def handle_cd_command(command: str) -> bool: # Use shlex.split to handle quoted paths properly import shlex - from code_puppy.messaging import emit_error, emit_info, emit_success + from code_puppy.messaging import emit_error, emit_info, emit_success, emit_warning try: tokens = shlex.split(command) @@ -77,6 +77,20 @@ def handle_cd_command(command: str) -> bool: if os.path.isdir(target): os.chdir(target) emit_success(f"Changed directory to: {target}") + # Reload the agent so the system prompt and project-local + # AGENT.md rules reflect the new working directory. Without + # this, the LLM keeps receiving stale path information for the + # remainder of the session (the PydanticAgent instructions are + # baked in at construction time and never refreshed otherwise). + try: + from code_puppy.agents.agent_manager import get_current_agent + + get_current_agent().reload_code_generation_agent() + except Exception as e: + emit_warning( + f"Directory changed, but agent reload failed: {e}. " + "You may need to run /agent or /model to force a refresh." + ) else: emit_error(f"Not a directory: {dirname}") return True diff --git a/code_puppy/mcp_/server_registry_catalog.py b/code_puppy/mcp_/server_registry_catalog.py index 67a08ceaf..31ee242b0 100644 --- a/code_puppy/mcp_/server_registry_catalog.py +++ b/code_puppy/mcp_/server_registry_catalog.py @@ -63,7 +63,11 @@ def get_environment_vars(self) -> List[str]: # Also check config for env vars (existing logic) if "env" in self.config: for _key, value in self.config["env"].items(): - if isinstance(value, str) and value.startswith("$"): + if ( + isinstance(value, str) + and value.startswith("$") + and not value.startswith("${") + ): var_name = value[1:] if var_name not in env_vars: env_vars.append(var_name) @@ -460,6 +464,34 @@ def to_server_config(self, custom_name: Optional[str] = None, **cmd_args) -> Dic ), ), # ========== Web & Browser ========== + MCPServerTemplate( + id="lightpanda", + name="lightpanda", + display_name="Lightpanda Browser", + description="Headless browser automation using a Lightpanda CDP endpoint", + category="Web", + tags=["browser", "web", "scraping", "automation", "lightpanda", "cdp"], + type="stdio", + config={ + "command": "npx", + "args": ["-y", "openclaw-lightpanda-mcp"], + "env": {"LIGHTPANDA_CDP_URL": "${cdp_url}"}, + "timeout": 60, + }, + requires=MCPServerRequirements( + command_line_args=[ + { + "name": "cdp_url", + "prompt": "Lightpanda CDP URL", + "default": "ws://127.0.0.1:9222", + "required": False, + } + ], + required_tools=["node", "npm", "npx", "lightpanda"], + package_dependencies=["openclaw-lightpanda-mcp"], + system_requirements=["Running Lightpanda CDP endpoint"], + ), + ), MCPServerTemplate( id="puppeteer", name="puppeteer", diff --git a/code_puppy/tools/browser/browser_control.py b/code_puppy/tools/browser/browser_control.py index 25d1e35c4..9c370c1ae 100644 --- a/code_puppy/tools/browser/browser_control.py +++ b/code_puppy/tools/browser/browser_control.py @@ -218,7 +218,7 @@ async def browser_initialize( Args: headless: Run browser in headless mode (no GUI) - browser_type: Browser engine (chromium, firefox, webkit) + browser_type: Browser engine (chromium, firefox, webkit, lightpanda) homepage: Initial page to load Returns: diff --git a/code_puppy/tools/browser/browser_manager.py b/code_puppy/tools/browser/browser_manager.py index d1b4da07d..2d8369216 100644 --- a/code_puppy/tools/browser/browser_manager.py +++ b/code_puppy/tools/browser/browser_manager.py @@ -1,12 +1,18 @@ -"""Playwright browser manager for browser automation. +"""Playwright-compatible browser manager for browser automation. Supports multiple simultaneous instances with unique profile directories. """ import asyncio import atexit +import contextlib import contextvars +import math import os +import shlex +import shutil +import socket +from collections import deque from pathlib import Path from typing import Callable, Dict, Optional @@ -18,6 +24,7 @@ # Registry for custom browser types from plugins (e.g., Camoufox for stealth browsing) _CUSTOM_BROWSER_TYPES: Dict[str, Callable] = {} _BROWSER_TYPES_LOADED: bool = False +_BUILTIN_PLAYWRIGHT_BROWSERS = {"chromium", "firefox", "webkit"} def _load_plugin_browser_types() -> None: @@ -97,14 +104,19 @@ def get_session_browser_manager() -> "BrowserManager": class BrowserManager: - """Browser manager for Playwright-based browser automation. + """Browser manager for browser automation. Supports multiple simultaneous instances, each with its own profile directory. - Uses Chromium by default for maximum compatibility. + Uses Playwright Chromium by default for maximum compatibility. + Supports Lightpanda as an optional CDP backend. """ _browser: Optional[Browser] = None _context: Optional[BrowserContext] = None + _playwright: Optional[object] = None + _lightpanda_process: Optional[asyncio.subprocess.Process] = None + _lightpanda_endpoint: Optional[str] = None + _lightpanda_stderr_task: Optional[asyncio.Task] = None _initialized: bool = False def __init__( @@ -128,6 +140,7 @@ def __init__( # Unique profile directory per session for browser state self.profile_dir = self._get_profile_directory() + self._lightpanda_stderr_buffer: deque[str] = deque(maxlen=128) def _get_profile_directory(self) -> Path: """Get or create the profile directory for this session. @@ -143,13 +156,314 @@ def _get_profile_directory(self) -> Path: profile_path.mkdir(parents=True, exist_ok=True, mode=0o700) return profile_path + @staticmethod + def _find_free_port() -> int: + """Find an available local TCP port.""" + with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock: + sock.bind(("127.0.0.1", 0)) + return int(sock.getsockname()[1]) + + def _resolve_lightpanda_executable(self) -> str: + """Resolve the Lightpanda executable path from env or PATH.""" + executable = os.getenv("LIGHTPANDA_EXECUTABLE", "lightpanda") + + path_like = os.path.sep in executable or ( + os.path.altsep is not None and os.path.altsep in executable + ) + + if path_like: + executable_path = Path(executable).expanduser() + if executable_path.is_file() and os.access(executable_path, os.X_OK): + return str(executable_path) + else: + resolved = shutil.which(executable) + if resolved: + return resolved + + raise RuntimeError( + "Lightpanda executable not found or not executable. Install Lightpanda " + "or set LIGHTPANDA_EXECUTABLE to a valid executable path." + ) + + def _get_lightpanda_host(self) -> str: + """Get Lightpanda host.""" + return os.getenv("LIGHTPANDA_HOST", "127.0.0.1") + + @staticmethod + def _validate_tcp_port(port: int, source_name: str) -> int: + """Validate a TCP port number and return it.""" + if not 1 <= port <= 65535: + raise RuntimeError( + f"{source_name} must be between 1 and 65535, got: {port}" + ) + return port + + def _get_lightpanda_port(self) -> int: + """Get Lightpanda CDP port from env or an ephemeral free port.""" + configured_port = os.getenv("LIGHTPANDA_PORT") + if configured_port: + try: + parsed_port = int(configured_port) + except ValueError as exc: + raise RuntimeError( + f"Invalid LIGHTPANDA_PORT value: {configured_port}" + ) from exc + return self._validate_tcp_port(parsed_port, "LIGHTPANDA_PORT") + + return self._validate_tcp_port( + self._find_free_port(), "auto-selected Lightpanda port" + ) + + @staticmethod + def _get_lightpanda_startup_timeout() -> float: + """Get Lightpanda startup timeout in seconds.""" + timeout_raw = os.getenv("LIGHTPANDA_STARTUP_TIMEOUT", "10") + try: + timeout = float(timeout_raw) + except ValueError as exc: + raise RuntimeError( + f"Invalid LIGHTPANDA_STARTUP_TIMEOUT value: {timeout_raw}" + ) from exc + + if not math.isfinite(timeout) or timeout <= 0: + raise RuntimeError( + "LIGHTPANDA_STARTUP_TIMEOUT must be a finite positive number, " + f"got: {timeout_raw}" + ) + return max(timeout, 1.0) + + @staticmethod + def _get_lightpanda_startup_retries() -> int: + """Get number of startup retries for auto-selected ports.""" + retries_raw = os.getenv("LIGHTPANDA_STARTUP_RETRIES", "3") + try: + retries = int(retries_raw) + except ValueError as exc: + raise RuntimeError( + f"Invalid LIGHTPANDA_STARTUP_RETRIES value: {retries_raw}" + ) from exc + return max(retries, 1) + + async def _read_lightpanda_stderr(self) -> str: + """Read Lightpanda stderr if available for better startup errors.""" + if not self._lightpanda_stderr_buffer: + return "" + stderr_text = "\n".join(self._lightpanda_stderr_buffer).strip() + return stderr_text[-500:] + + async def _drain_lightpanda_stderr(self, stream: asyncio.StreamReader) -> None: + """Drain stderr continuously to avoid subprocess pipe backpressure.""" + try: + while True: + chunk = await stream.read(4096) + if not chunk: + return + decoded = chunk.decode(errors="replace") + for line in decoded.splitlines(): + stripped = line.strip() + if stripped: + self._lightpanda_stderr_buffer.append(stripped) + except asyncio.CancelledError: + raise + except Exception: + # Stderr draining is best effort and should not fail startup. + return + + def _start_lightpanda_stderr_drain(self) -> None: + """Start background stderr drain task when stream is available.""" + if self._lightpanda_stderr_task and not self._lightpanda_stderr_task.done(): + self._lightpanda_stderr_task.cancel() + + stream = self._lightpanda_process.stderr if self._lightpanda_process else None + + if isinstance(stream, asyncio.StreamReader): + self._lightpanda_stderr_task = asyncio.create_task( + self._drain_lightpanda_stderr(stream) + ) + else: + self._lightpanda_stderr_task = None + + async def _stop_lightpanda_stderr_drain(self) -> None: + """Stop background stderr drain task.""" + task = self._lightpanda_stderr_task + self._lightpanda_stderr_task = None + + if not task: + return + + if task.done(): + with contextlib.suppress(Exception): + await task + return + + task.cancel() + with contextlib.suppress(asyncio.CancelledError, Exception): + await task + + def _build_lightpanda_command(self, host: str, port: int) -> list[str]: + """Build the Lightpanda startup command.""" + executable = self._resolve_lightpanda_executable() + command = [ + executable, + "serve", + f"--host={host}", + f"--port={port}", + ] + + extra_args_raw = os.getenv("LIGHTPANDA_ARGS", "").strip() + if extra_args_raw: + command.extend(shlex.split(extra_args_raw, posix=os.name != "nt")) + + return command + + async def _connect_lightpanda_over_cdp(self, endpoint: str) -> Browser: + """Connect Playwright to Lightpanda CDP with retry.""" + timeout_s = self._get_lightpanda_startup_timeout() + loop = asyncio.get_running_loop() + deadline = loop.time() + timeout_s + last_error: Optional[Exception] = None + + while loop.time() < deadline: + if ( + self._lightpanda_process + and self._lightpanda_process.returncode is not None + ): + stderr_text = await self._read_lightpanda_stderr() + raise RuntimeError( + "Lightpanda process exited before CDP connection was ready " + f"(code={self._lightpanda_process.returncode}). " + f"{stderr_text}" + ) + + try: + if not self._playwright: + raise RuntimeError("Playwright is not initialized.") + remaining = deadline - loop.time() + if remaining <= 0: + break + return await asyncio.wait_for( + self._playwright.chromium.connect_over_cdp(endpoint), + timeout=remaining, + ) + except Exception as exc: + last_error = exc + remaining = deadline - loop.time() + if remaining <= 0: + break + await asyncio.sleep(min(0.2, remaining)) + + raise RuntimeError( + f"Timed out connecting to Lightpanda CDP at {endpoint}: {last_error}" + ) + + async def _initialize_lightpanda_browser(self) -> None: + """Initialize Lightpanda and attach Playwright over CDP. + + Uses retry-on-failure for auto-selected ports to reduce startup + flakiness caused by bind races between probing and process launch. + """ + from playwright.async_api import async_playwright + + if not self.headless: + emit_warning( + "Lightpanda is headless-only; forcing headless mode for this session." + ) + self.headless = True + + host = self._get_lightpanda_host() + self._playwright = await async_playwright().start() + fixed_port = bool(os.getenv("LIGHTPANDA_PORT")) + max_attempts = 1 if fixed_port else self._get_lightpanda_startup_retries() + last_error: Optional[Exception] = None + + for attempt in range(1, max_attempts + 1): + port = self._get_lightpanda_port() + self._lightpanda_endpoint = f"http://{host}:{port}" + + command = self._build_lightpanda_command(host, port) + emit_info( + f"Starting Lightpanda CDP endpoint at {host}:{port} " + f"(attempt {attempt}/{max_attempts})" + ) + + self._lightpanda_process = await asyncio.create_subprocess_exec( + *command, + stdout=asyncio.subprocess.DEVNULL, + stderr=asyncio.subprocess.PIPE, + ) + self._lightpanda_stderr_buffer.clear() + self._start_lightpanda_stderr_drain() + + try: + browser = await self._connect_lightpanda_over_cdp( + self._lightpanda_endpoint + ) + + # Reuse existing context when available (CDP default context), + # otherwise create one for consistent manager behavior. + if browser.contexts: + context = browser.contexts[0] + else: + context = await browser.new_context() + + self._browser = browser + self._context = context + return + except Exception as exc: + last_error = exc + await self._stop_lightpanda_process() + + if attempt < max_attempts: + emit_warning( + "Lightpanda startup failed; retrying with a new port " + f"(attempt {attempt + 1}/{max_attempts})." + ) + await asyncio.sleep(0.1) + + raise RuntimeError( + f"Failed to initialize Lightpanda after {max_attempts} attempts: " + f"{last_error}" + ) from last_error + + async def _stop_lightpanda_process(self) -> None: + """Stop Lightpanda process if this manager started one.""" + process = self._lightpanda_process + self._lightpanda_process = None + self._lightpanda_endpoint = None + + try: + if not process: + return + + if process.returncode is None: + with contextlib.suppress(Exception): + process.terminate() + try: + await asyncio.wait_for(process.wait(), timeout=3) + except asyncio.TimeoutError: + with contextlib.suppress(Exception): + process.kill() + with contextlib.suppress(Exception): + await process.wait() + except Exception: + with contextlib.suppress(Exception): + await process.wait() + else: + with contextlib.suppress(Exception): + await process.wait() + finally: + await self._stop_lightpanda_stderr_drain() + async def async_initialize(self) -> None: - """Initialize Chromium browser via Playwright.""" + """Initialize a browser backend.""" if self._initialized: return try: - emit_info(f"Initializing Chromium browser (session: {self.session_id})...") + browser_name = self.browser_type or "chromium" + emit_info( + f"Initializing {browser_name} browser (session: {self.session_id})..." + ) await self._initialize_browser() self._initialized = True @@ -165,27 +479,70 @@ async def _initialize_browser(self) -> None: """ # Load plugin browser types on first initialization _load_plugin_browser_types() - - # Check if a custom browser type was requested and is available - if self.browser_type and self.browser_type in _CUSTOM_BROWSER_TYPES: + requested_browser = (self.browser_type or "chromium").lower() + + # Check if a custom browser type was requested and is available. + # Match exact key first, then case-insensitive for consistency with + # built-in/lightpanda browser routing. + custom_browser_key: Optional[str] = None + if self.browser_type: + if self.browser_type in _CUSTOM_BROWSER_TYPES: + custom_browser_key = self.browser_type + else: + matched_custom_keys = [ + key + for key in _CUSTOM_BROWSER_TYPES + if key.lower() == requested_browser + ] + if len(matched_custom_keys) == 1: + custom_browser_key = matched_custom_keys[0] + elif len(matched_custom_keys) > 1: + raise ValueError( + f"Ambiguous custom browser_type '{self.browser_type}'. " + "Multiple plugin browser types match case-insensitively: " + f"{', '.join(sorted(matched_custom_keys))}" + ) + + if custom_browser_key: emit_info( - f"Using custom browser type '{self.browser_type}' " + f"Using custom browser type '{custom_browser_key}' " f"(session: {self.session_id})" ) - init_func = _CUSTOM_BROWSER_TYPES[self.browser_type] + init_func = _CUSTOM_BROWSER_TYPES[custom_browser_key] # Custom init functions should set self._context and self._browser await init_func(self) self._initialized = True return - # Default: use Playwright Chromium + if requested_browser == "lightpanda": + emit_info(f"Using Lightpanda browser (session: {self.session_id})") + await self._initialize_lightpanda_browser() + self._initialized = True + return + + if requested_browser not in _BUILTIN_PLAYWRIGHT_BROWSERS: + supported_browsers = sorted( + _BUILTIN_PLAYWRIGHT_BROWSERS + | {"lightpanda"} + | set(_CUSTOM_BROWSER_TYPES.keys()) + ) + raise ValueError( + f"Unsupported browser_type '{self.browser_type}'. " + f"Supported values: {', '.join(supported_browsers)}" + ) + + # Default: use built-in Playwright browser backends from playwright.async_api import async_playwright - emit_info(f"Using persistent profile: {self.profile_dir}") + emit_info( + f"Using built-in Playwright browser '{requested_browser}' " + f"with persistent profile: {self.profile_dir}" + ) pw = await async_playwright().start() - # Use persistent context directory for Chromium to preserve browser state - context = await pw.chromium.launch_persistent_context( + self._playwright = pw + browser_launcher = getattr(pw, requested_browser) + context = await browser_launcher.launch_persistent_context( user_data_dir=str(self.profile_dir), headless=self.headless ) self._context = context @@ -258,6 +615,19 @@ async def _cleanup(self, silent: bool = False) -> None: pass # Ignore errors during browser close self._browser = None + if self._playwright: + try: + await self._playwright.stop() + except Exception: + pass # Ignore errors during playwright shutdown + self._playwright = None + + if self._lightpanda_process: + try: + await self._stop_lightpanda_process() + except Exception: + pass # Ignore errors during Lightpanda shutdown + self._initialized = False # Remove from active managers diff --git a/pyproject.toml b/pyproject.toml index 86e625758..ad4b00274 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "code-puppy" -version = "0.0.424" +version = "0.0.425" description = "Code generation agent" readme = "README.md" requires-python = ">=3.11,<3.14" diff --git a/tests/agents/test_base_agent_full_coverage.py b/tests/agents/test_base_agent_full_coverage.py index 875c68e58..4165df12e 100644 --- a/tests/agents/test_base_agent_full_coverage.py +++ b/tests/agents/test_base_agent_full_coverage.py @@ -940,6 +940,69 @@ def test_dbos_reload( agent.reload_code_generation_agent() mock_dbos_agent.assert_called() + def test_reload_clears_puppy_rules_cache(self, agent): + """reload_code_generation_agent must invalidate the _puppy_rules cache. + + This ensures that /cd picks up the new project's AGENT.md rules instead + of keeping the old directory's rules baked in for the session. + """ + # Seed a stale cached value as if a previous directory's AGENT.md was loaded. + stale_rules = "old project rules" + agent._puppy_rules = stale_rules + + # Capture the _puppy_rules value at the moment load_puppy_rules() is + # entered to prove it was cleared *before* the fresh read. + value_at_load_time = [] + original_load = agent.load_puppy_rules + + def spy_load(): + # _puppy_rules should already be None when we arrive here + value_at_load_time.append(agent._puppy_rules) + return original_load() + + with ( + patch.object(agent, "load_puppy_rules", side_effect=spy_load), + patch.object(agent, "get_model_name", return_value="model"), + patch( + "code_puppy.agents.base_agent.ModelFactory.load_config", + return_value={"model": {}}, + ), + patch( + "code_puppy.agents.base_agent.ModelFactory.get_model", + return_value=MagicMock(), + ), + patch( + "code_puppy.agents.base_agent.get_agent_pinned_model", + return_value=None, + ), + patch( + "code_puppy.agents.base_agent.get_mcp_manager", + ) as mock_mcp, + patch( + "code_puppy.model_utils.prepare_prompt_for_model", + return_value=MagicMock(instructions="test"), + ), + patch( + "code_puppy.agents.base_agent.PydanticAgent", + return_value=MagicMock(_tools={}), + ), + patch("code_puppy.tools.register_tools_for_agent"), + patch("code_puppy.tools.has_extended_thinking_active", return_value=False), + patch("code_puppy.agents.base_agent.get_use_dbos", return_value=False), + patch("code_puppy.agents.base_agent.make_model_settings", return_value={}), + ): + mock_mcp.return_value.get_servers_for_agent.return_value = [] + agent.reload_code_generation_agent() + + # The spy must have been called (load_puppy_rules was invoked during reload). + assert value_at_load_time, "load_puppy_rules was never called during reload" + # Crucially, _puppy_rules must have been None at entry to load_puppy_rules, + # proving the cache was cleared before the fresh disk read. + assert value_at_load_time[0] is None, ( + f"_puppy_rules was not cleared before re-loading rules; " + f"got: {value_at_load_time[0]!r}" + ) + class TestCreateAgentWithOutputType: """Tests for _create_agent_with_output_type (lines 1603-1657).""" diff --git a/tests/mcp/test_server_registry_catalog.py b/tests/mcp/test_server_registry_catalog.py index f19cd0a69..67a813d22 100644 --- a/tests/mcp/test_server_registry_catalog.py +++ b/tests/mcp/test_server_registry_catalog.py @@ -240,6 +240,29 @@ def test_get_environment_vars_mixed_sources(self): assert "MY_API_KEY" in env_vars assert len(env_vars) == 2 # No duplicates + def test_get_environment_vars_ignores_cmd_placeholders(self): + """Test env placeholder syntax `${...}` is not treated as env var input.""" + template = MCPServerTemplate( + id="test", + name="test", + display_name="Test", + description="Test", + category="Test", + tags=["test"], + type="stdio", + config={ + "env": { + "LIGHTPANDA_CDP_URL": "${cdp_url}", + "API_KEY": "$REAL_API_KEY", + } + }, + ) + + env_vars = template.get_environment_vars() + assert "REAL_API_KEY" in env_vars + assert "{cdp_url}" not in env_vars + assert "cdp_url" not in env_vars + def test_get_command_line_args(self): """Test getting command line arguments from requirements.""" args = [ diff --git a/tests/test_command_handler.py b/tests/test_command_handler.py index 0368ca090..ed7459daa 100644 --- a/tests/test_command_handler.py +++ b/tests/test_command_handler.py @@ -59,24 +59,68 @@ def test_cd_show_lists_directories(): def test_cd_valid_change(): + """Successful /cd must chdir, emit success, and reload the agent.""" mocks = setup_messaging_mocks() mock_emit_success = mocks["emit_success"].start() try: + mock_agent = MagicMock() with ( patch("os.path.expanduser", side_effect=lambda x: x), patch("os.path.isabs", return_value=True), patch("os.path.isdir", return_value=True), patch("os.chdir") as mock_chdir, + patch( + "code_puppy.agents.agent_manager.get_current_agent", + return_value=mock_agent, + ), ): result = handle_command("/cd /some/dir") assert result is True mock_chdir.assert_called_once_with("/some/dir") mock_emit_success.assert_called_with("Changed directory to: /some/dir") + # Agent must be reloaded so the system prompt and AGENT.md rules + # reflect the new working directory. + mock_agent.reload_code_generation_agent.assert_called_once() finally: mocks["emit_success"].stop() +def test_cd_valid_change_reload_failure_is_nonfatal(): + """A reload failure after /cd must not abort the directory change.""" + mocks = setup_messaging_mocks() + mock_emit_success = mocks["emit_success"].start() + mock_emit_warning = mocks["emit_warning"].start() + + try: + mock_agent = MagicMock() + mock_agent.reload_code_generation_agent.side_effect = Exception("boom") + with ( + patch("os.path.expanduser", side_effect=lambda x: x), + patch("os.path.isabs", return_value=True), + patch("os.path.isdir", return_value=True), + patch("os.chdir") as mock_chdir, + patch( + "code_puppy.agents.agent_manager.get_current_agent", + return_value=mock_agent, + ), + ): + # Should not raise even though reload raises. + result = handle_command("/cd /some/dir") + assert result is True + mock_chdir.assert_called_once_with("/some/dir") + mock_emit_success.assert_called_once_with("Changed directory to: /some/dir") + mock_agent.reload_code_generation_agent.assert_called_once() + # Reload failure should emit a warning, not silently pass + mock_emit_warning.assert_called_once() + warning_msg = str(mock_emit_warning.call_args) + assert "agent reload failed" in warning_msg + assert "boom" in warning_msg + finally: + mocks["emit_success"].stop() + mocks["emit_warning"].stop() + + def test_cd_invalid_directory(): mocks = setup_messaging_mocks() mock_emit_error = mocks["emit_error"].start() diff --git a/tests/test_config.py b/tests/test_config.py index 5fa231bbd..406944a11 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -299,7 +299,6 @@ def test_get_config_keys_with_existing_keys( "banner_color_subagent_response", "banner_color_terminal_tool", "banner_color_thinking", - "banner_color_mcp_tool_call", "banner_color_universal_constructor", "cancel_agent_key", "compaction_strategy", @@ -354,7 +353,6 @@ def test_get_config_keys_empty_config( "banner_color_subagent_response", "banner_color_terminal_tool", "banner_color_thinking", - "banner_color_mcp_tool_call", "banner_color_universal_constructor", "cancel_agent_key", "compaction_strategy", diff --git a/tests/tools/browser/test_browser_manager_full.py b/tests/tools/browser/test_browser_manager_full.py index 975b8e3f0..bfd6c39e5 100644 --- a/tests/tools/browser/test_browser_manager_full.py +++ b/tests/tools/browser/test_browser_manager_full.py @@ -1,5 +1,6 @@ """Full coverage tests for browser_manager.py.""" +import asyncio from unittest.mock import AsyncMock, MagicMock, patch import pytest @@ -80,6 +81,7 @@ def test_get_session_browser_manager(self): class TestInitializeBrowser: @pytest.mark.asyncio async def test_initialize_browser_default_chromium(self): + """Test default browser initialization uses Chromium backend.""" from code_puppy.tools.browser.browser_manager import BrowserManager mgr = BrowserManager("init-test") @@ -106,6 +108,7 @@ async def test_initialize_browser_default_chromium(self): @pytest.mark.asyncio async def test_initialize_browser_custom_type(self): + """Test custom registered browser type initialization path.""" from code_puppy.tools.browser import browser_manager as bm from code_puppy.tools.browser.browser_manager import BrowserManager @@ -125,6 +128,265 @@ async def custom_init(manager): del bm._CUSTOM_BROWSER_TYPES["custom"] + @pytest.mark.asyncio + async def test_initialize_browser_custom_type_case_insensitive(self): + """Test mixed-case custom browser type resolves case-insensitively.""" + from code_puppy.tools.browser import browser_manager as bm + from code_puppy.tools.browser.browser_manager import BrowserManager + + async def custom_init(manager): + manager._context = AsyncMock() + manager._browser = AsyncMock() + + bm._CUSTOM_BROWSER_TYPES["myplugin"] = custom_init + bm._BROWSER_TYPES_LOADED = True + + mgr = BrowserManager("custom-case-test") + mgr.browser_type = "MyPlugin" + + with patch("code_puppy.tools.browser.browser_manager.emit_info"): + await mgr._initialize_browser() + assert mgr._initialized is True + + del bm._CUSTOM_BROWSER_TYPES["myplugin"] + + @pytest.mark.asyncio + async def test_initialize_browser_lightpanda_type(self): + """Test `browser_type=lightpanda` dispatches to Lightpanda initializer.""" + from code_puppy.tools.browser.browser_manager import BrowserManager + + mgr = BrowserManager("lightpanda-test") + mgr.browser_type = "lightpanda" + + with ( + patch( + "code_puppy.tools.browser.browser_manager._load_plugin_browser_types" + ), + patch("code_puppy.tools.browser.browser_manager.emit_info"), + patch.object( + mgr, + "_initialize_lightpanda_browser", + new=AsyncMock(), + ) as mock_init_lightpanda, + ): + await mgr._initialize_browser() + mock_init_lightpanda.assert_awaited_once() + assert mgr._initialized is True + + @pytest.mark.asyncio + async def test_initialize_browser_lightpanda_type_case_insensitive(self): + """Test mixed-case lightpanda browser type dispatches correctly.""" + from code_puppy.tools.browser.browser_manager import BrowserManager + + mgr = BrowserManager("lightpanda-case-test") + mgr.browser_type = "LightPanda" + + with ( + patch( + "code_puppy.tools.browser.browser_manager._load_plugin_browser_types" + ), + patch("code_puppy.tools.browser.browser_manager.emit_info"), + patch.object( + mgr, + "_initialize_lightpanda_browser", + new=AsyncMock(), + ) as mock_init_lightpanda, + ): + await mgr._initialize_browser() + mock_init_lightpanda.assert_awaited_once() + assert mgr._initialized is True + + @pytest.mark.asyncio + async def test_initialize_lightpanda_browser_sets_context(self): + """Test Lightpanda init sets Playwright, browser, and context references.""" + from code_puppy.tools.browser.browser_manager import BrowserManager + + mgr = BrowserManager("lightpanda-init") + + mock_pw_instance = AsyncMock() + mock_browser = AsyncMock() + mock_context = AsyncMock() + mock_browser.contexts = [mock_context] + mock_pw_instance.chromium.connect_over_cdp.return_value = mock_browser + + mock_pw_class = AsyncMock() + mock_pw_class.start.return_value = mock_pw_instance + + mock_process = AsyncMock() + mock_process.returncode = None + stream = asyncio.StreamReader() + stream.feed_data(b"lightpanda started\n") + stream.feed_eof() + mock_process.stderr = stream + + with ( + patch("code_puppy.tools.browser.browser_manager.emit_info"), + patch( + "code_puppy.tools.browser.browser_manager.asyncio.create_subprocess_exec", + return_value=mock_process, + ), + patch("playwright.async_api.async_playwright", return_value=mock_pw_class), + patch.object(mgr, "_build_lightpanda_command", return_value=["lightpanda"]), + patch.object(mgr, "_get_lightpanda_host", return_value="127.0.0.1"), + patch.object(mgr, "_get_lightpanda_port", return_value=9222), + ): + await mgr._initialize_lightpanda_browser() + await asyncio.sleep(0) + + assert mgr._playwright is mock_pw_instance + assert mgr._browser is mock_browser + assert mgr._context is mock_context + assert mgr._lightpanda_endpoint == "http://127.0.0.1:9222" + assert mgr._lightpanda_stderr_task is not None + + @pytest.mark.asyncio + async def test_initialize_lightpanda_retries_on_auto_port_failure(self): + """Test Lightpanda retries startup when an auto-port attempt fails.""" + from code_puppy.tools.browser.browser_manager import BrowserManager + + mgr = BrowserManager("lightpanda-retry") + + mock_pw_instance = AsyncMock() + mock_pw_class = AsyncMock() + mock_pw_class.start.return_value = mock_pw_instance + + mock_browser = AsyncMock() + mock_context = AsyncMock() + mock_browser.contexts = [mock_context] + + mock_process_a = AsyncMock() + mock_process_a.returncode = None + stream_a = asyncio.StreamReader() + stream_a.feed_data(b"bind failed\n") + stream_a.feed_eof() + mock_process_a.stderr = stream_a + mock_process_b = AsyncMock() + mock_process_b.returncode = None + stream_b = asyncio.StreamReader() + stream_b.feed_data(b"retrying\n") + stream_b.feed_eof() + mock_process_b.stderr = stream_b + + with ( + patch("code_puppy.tools.browser.browser_manager.emit_info"), + patch("code_puppy.tools.browser.browser_manager.emit_warning"), + patch("playwright.async_api.async_playwright", return_value=mock_pw_class), + patch( + "code_puppy.tools.browser.browser_manager.asyncio.create_subprocess_exec", + side_effect=[mock_process_a, mock_process_b], + ), + patch.object(mgr, "_get_lightpanda_host", return_value="127.0.0.1"), + patch.object(mgr, "_get_lightpanda_port", side_effect=[9222, 9223]), + patch.object(mgr, "_get_lightpanda_startup_retries", return_value=2), + patch.object(mgr, "_build_lightpanda_command", return_value=["lightpanda"]), + patch.object( + mgr, + "_connect_lightpanda_over_cdp", + side_effect=[RuntimeError("bind fail"), mock_browser], + ), + patch.object(mgr, "_stop_lightpanda_process", new=AsyncMock()) as mock_stop, + patch.dict("os.environ", {"LIGHTPANDA_PORT": ""}, clear=False), + ): + await mgr._initialize_lightpanda_browser() + + assert mock_stop.await_count == 1 + assert mgr._context is mock_context + + @pytest.mark.asyncio + async def test_connect_lightpanda_over_cdp_attempt_respects_timeout_budget(self): + """Test CDP connect attempts are bounded by LIGHTPANDA_STARTUP_TIMEOUT.""" + import asyncio + + from code_puppy.tools.browser.browser_manager import BrowserManager + + mgr = BrowserManager("lightpanda-cdp-timeout") + mock_playwright = MagicMock() + mock_playwright.chromium = MagicMock() + + async def slow_connect(_endpoint): + await asyncio.sleep(5) + + mock_playwright.chromium.connect_over_cdp = AsyncMock(side_effect=slow_connect) + mgr._playwright = mock_playwright + + with patch.object(mgr, "_get_lightpanda_startup_timeout", return_value=0.1): + with pytest.raises( + RuntimeError, match="Timed out connecting to Lightpanda" + ): + await mgr._connect_lightpanda_over_cdp("http://127.0.0.1:9222") + + @pytest.mark.asyncio + async def test_initialize_browser_unknown_type_raises(self): + """Test unsupported built-in browser types fail fast with clear error.""" + from code_puppy.tools.browser.browser_manager import BrowserManager + + mgr = BrowserManager("bad-browser-type") + mgr.browser_type = "unknown-browser" + + with ( + patch( + "code_puppy.tools.browser.browser_manager._load_plugin_browser_types" + ), + pytest.raises(ValueError, match="Unsupported browser_type"), + ): + await mgr._initialize_browser() + + def test_resolve_lightpanda_executable_rejects_directory_path(self, tmp_path): + """Test path-like LIGHTPANDA_EXECUTABLE must be an executable file.""" + from code_puppy.tools.browser.browser_manager import BrowserManager + + mgr = BrowserManager("bad-executable") + + with patch.dict( + "os.environ", {"LIGHTPANDA_EXECUTABLE": str(tmp_path)}, clear=False + ): + with pytest.raises(RuntimeError, match="not found or not executable"): + mgr._resolve_lightpanda_executable() + + def test_resolve_lightpanda_executable_accepts_pathlike_executable(self): + """Test path-like LIGHTPANDA_EXECUTABLE resolves a valid executable file.""" + import sys + from pathlib import Path + + from code_puppy.tools.browser.browser_manager import BrowserManager + + mgr = BrowserManager("good-executable") + + with patch.dict( + "os.environ", {"LIGHTPANDA_EXECUTABLE": sys.executable}, clear=False + ): + resolved = mgr._resolve_lightpanda_executable() + + assert Path(resolved).resolve() == Path(sys.executable).resolve() + + def test_get_lightpanda_port_rejects_invalid_range(self): + """Test LIGHTPANDA_PORT values outside valid TCP range are rejected.""" + from code_puppy.tools.browser.browser_manager import BrowserManager + + mgr = BrowserManager("bad-port") + + with patch.dict("os.environ", {"LIGHTPANDA_PORT": "70000"}, clear=False): + with pytest.raises( + RuntimeError, match="LIGHTPANDA_PORT must be between 1 and 65535" + ): + mgr._get_lightpanda_port() + + @pytest.mark.parametrize("raw_timeout", ["nan", "inf", "-1", "0"]) + def test_get_lightpanda_startup_timeout_rejects_non_finite_or_non_positive( + self, raw_timeout + ): + """Test invalid LIGHTPANDA_STARTUP_TIMEOUT values fail with clear errors.""" + from code_puppy.tools.browser.browser_manager import BrowserManager + + with patch.dict( + "os.environ", {"LIGHTPANDA_STARTUP_TIMEOUT": raw_timeout}, clear=False + ): + with pytest.raises( + RuntimeError, + match="LIGHTPANDA_STARTUP_TIMEOUT must be a finite positive number", + ): + BrowserManager._get_lightpanda_startup_timeout() + class TestCleanupSilent: @pytest.mark.asyncio @@ -186,6 +448,52 @@ async def test_cleanup_outer_exception(self): await mgr._cleanup(silent=False) assert mgr._initialized is False + @pytest.mark.asyncio + async def test_cleanup_stops_lightpanda_and_playwright(self): + """Test cleanup stops both Playwright and Lightpanda process.""" + from code_puppy.tools.browser.browser_manager import BrowserManager + + mgr = BrowserManager("lightpanda-cleanup") + mgr._initialized = True + mgr._context = AsyncMock() + mgr._context.storage_state = AsyncMock() + mgr._browser = AsyncMock() + mock_playwright = AsyncMock() + mgr._playwright = mock_playwright + mgr._lightpanda_process = AsyncMock() + + with ( + patch("code_puppy.tools.browser.browser_manager.emit_success"), + patch("code_puppy.tools.browser.browser_manager.emit_warning"), + patch.object( + mgr, "_stop_lightpanda_process", new=AsyncMock() + ) as mock_stop_lightpanda, + ): + await mgr._cleanup(silent=True) + + mock_playwright.stop.assert_awaited_once() + mock_stop_lightpanda.assert_awaited_once() + + @pytest.mark.asyncio + async def test_stop_lightpanda_process_always_stops_stderr_drain(self): + """Test stderr drain cleanup still runs when process termination fails.""" + from code_puppy.tools.browser.browser_manager import BrowserManager + + mgr = BrowserManager("lightpanda-stop-failure") + + mock_process = MagicMock() + mock_process.returncode = None + mock_process.terminate.side_effect = ProcessLookupError("already exited") + mock_process.wait = AsyncMock(return_value=None) + mgr._lightpanda_process = mock_process + + with patch.object( + mgr, "_stop_lightpanda_stderr_drain", new=AsyncMock() + ) as mock_stop_stderr: + await mgr._stop_lightpanda_process() + + mock_stop_stderr.assert_awaited_once() + class TestSyncCleanup: def test_sync_cleanup_with_active_managers(self): diff --git a/uv.lock b/uv.lock index 898d5dfad..189ad0f0a 100644 --- a/uv.lock +++ b/uv.lock @@ -189,7 +189,7 @@ wheels = [ [[package]] name = "code-puppy" -version = "0.0.424" +version = "0.0.425" source = { editable = "." } dependencies = [ { name = "anthropic" },