diff --git a/pyproject.toml b/pyproject.toml index 10935cdf..e1a6a77f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -91,6 +91,9 @@ asyncio_mode = "auto" # Prepend the project root so pytest imports the worktree's ``src/`` rather # than the parent repo's editable-install mapping. pythonpath = ["."] +# Bare ``pytest`` otherwise collects reference_projects/, whose module names +# collide with tests/agent and abort collection. +testpaths = ["tests"] markers = [ "integration: marks tests as integration (deselect with '-m \"not integration\"')", "linux_only: marks tests that require Linux (auto-skipped on macOS/Windows)", diff --git a/src/tool_system/tools/web_fetch.py b/src/tool_system/tools/web_fetch.py index f40a9ab8..456ce3da 100644 --- a/src/tool_system/tools/web_fetch.py +++ b/src/tool_system/tools/web_fetch.py @@ -41,7 +41,7 @@ # Removed before conversion so they never leak into the extracted text — markdownify # strips the tags but can otherwise surface their text nodes. _NOISE_BLOCK_RE = re.compile( - r"<(script|style|noscript|svg|template)\b[^>]*>.*?", + r"<(script|style|noscript|svg|template|iframe|object|embed)\b[^>]*>.*?", flags=re.DOTALL | re.IGNORECASE, ) diff --git a/tests/bridge/test_phase0_packages.py b/tests/bridge/test_phase0_packages.py index 898f63d1..2b41b651 100644 --- a/tests/bridge/test_phase0_packages.py +++ b/tests/bridge/test_phase0_packages.py @@ -17,17 +17,21 @@ def test_subsystem_packages_preserve_legacy_metadata_for_porting_workspace() -> def test_legacy_services_bridge_is_removed() -> None: """Phase 18: ``src.services.bridge`` was a deprecation-shim package pointing callers at ``src.bridge``. With the CCR bridge port now - functionally complete (phases 12-17), the shim is removed. - Importing the dotted path must fail with ``ModuleNotFoundError``. + functionally complete (phases 12-17), the shim is removed: no real + module may resolve at the dotted path. + + A leftover ``src/services/bridge/__pycache__`` directory (untracked, so + a branch switch doesn't delete it) still imports as a *namespace* + package — ``spec.origin is None`` — which is not the shim coming back, + so it is tolerated. """ - import importlib + import importlib.util + import sys - try: - importlib.import_module('src.services.bridge') - except ModuleNotFoundError: - return - raise AssertionError( - 'src.services.bridge should be removed but is still importable', + sys.modules.pop('src.services.bridge', None) + spec = importlib.util.find_spec('src.services.bridge') + assert spec is None or spec.origin is None, ( + f'src.services.bridge should be removed but resolves to {spec.origin}' ) diff --git a/tests/parity/test_tool_parity.py b/tests/parity/test_tool_parity.py index c80a9c90..bd4d28a3 100644 --- a/tests/parity/test_tool_parity.py +++ b/tests/parity/test_tool_parity.py @@ -123,13 +123,19 @@ class TestToolPropertyParity(unittest.TestCase): def setUpClass(cls) -> None: cls.registry = build_default_registry(include_user_tools=False) cls.props_snapshot = _load_json("ts_tool_properties.json") + # TS defaults only govern tools ported from TS; Python-only additions + # (advisor, Workflow, ...) are free to pick their own properties. + names_snapshot = _load_json("ts_tool_names.json") + cls.ts_core_names = { + info["python_name"] for info in names_snapshot["core_tools"].values() + } def test_default_is_read_only_false(self) -> None: default_val = self.props_snapshot["defaults"]["is_read_only"] - # Tools not in overrides should use the default + # TS core tools not in overrides should use the default overridden = set(self.props_snapshot["tool_overrides"].keys()) for tool in self.registry.list_tools(): - if tool.name not in overridden: + if tool.name in self.ts_core_names and tool.name not in overridden: result = tool.is_read_only({}) self.assertEqual( result, default_val, @@ -140,7 +146,7 @@ def test_default_is_concurrency_safe_false(self) -> None: default_val = self.props_snapshot["defaults"]["is_concurrency_safe"] overridden = set(self.props_snapshot["tool_overrides"].keys()) for tool in self.registry.list_tools(): - if tool.name not in overridden: + if tool.name in self.ts_core_names and tool.name not in overridden: result = tool.is_concurrency_safe({}) self.assertEqual( result, default_val, diff --git a/tests/test_mcp_config_full.py b/tests/test_mcp_config_full.py index a42f1a14..54109d1d 100644 --- a/tests/test_mcp_config_full.py +++ b/tests/test_mcp_config_full.py @@ -1,5 +1,6 @@ import json import os +import sys import tempfile import pytest from pathlib import Path @@ -71,7 +72,9 @@ def test_discover_vscode_settings(self, tmp_path, monkeypatch): class TestValidateServerConnectivity: def test_valid_command(self): - config = McpStdioServerConfig(command="python") + # sys.executable always exists; bare "python" is absent on systems + # that only ship python3 + config = McpStdioServerConfig(command=sys.executable) issues = validate_server_connectivity(config) assert issues == [] diff --git a/tests/test_mcp_doctor.py b/tests/test_mcp_doctor.py index 26211b35..4db0e9b1 100644 --- a/tests/test_mcp_doctor.py +++ b/tests/test_mcp_doctor.py @@ -1,3 +1,5 @@ +import sys + import pytest from unittest.mock import patch, MagicMock @@ -92,7 +94,9 @@ def test_format_report_with_errors(self): class TestValidateStdioConfig: def test_valid_command(self): - config = McpStdioServerConfig(command="python") + # sys.executable always exists; bare "python" is absent on systems + # that only ship python3 + config = McpStdioServerConfig(command=sys.executable) warnings = _validate_stdio_config("test", config) assert warnings == [] diff --git a/tests/test_providers.py b/tests/test_providers.py index 5a355724..c615a97b 100644 --- a/tests/test_providers.py +++ b/tests/test_providers.py @@ -2,17 +2,71 @@ from __future__ import annotations +import os import unittest from unittest.mock import MagicMock, patch from src.providers import get_provider_class from src.providers.anthropic_provider import AnthropicProvider from src.providers.glm_provider import GLMProvider -from src.providers.openai_compatible import _convert_anthropic_messages_to_openai +from src.providers.openai_compatible import ( + _apply_client_timeout, + _convert_anthropic_messages_to_openai, +) from src.providers.openai_provider import OpenAIProvider from src.providers.base import ChatMessage, ChatResponse +class TestApplyClientTimeout(unittest.TestCase): + """Guards the stalled-stream protection: every openai-compatible client + must get a bounded read timeout (the SDK default is read=600s, which + freezes the event loop on a stalled stream).""" + + def test_wraps_client_with_bounded_timeout_and_retries(self): + import httpx + + mock_client = MagicMock() + # Isolate from ambient CLAWCODEX_LLM_* tuning vars a developer may + # have exported (patch.dict snapshots and restores on exit) + with patch.dict("os.environ"): + for k in ( + "CLAWCODEX_LLM_READ_TIMEOUT", + "CLAWCODEX_LLM_CONNECT_TIMEOUT", + "CLAWCODEX_LLM_MAX_RETRIES", + ): + os.environ.pop(k, None) + wrapped = _apply_client_timeout(mock_client) + + self.assertIs(wrapped, mock_client.with_options.return_value) + mock_client.with_options.assert_called_once() + kwargs = mock_client.with_options.call_args.kwargs + self.assertEqual(kwargs["max_retries"], 1) + timeout = kwargs["timeout"] + self.assertIsInstance(timeout, httpx.Timeout) + self.assertEqual(timeout.read, 120.0) + self.assertEqual(timeout.connect, 15.0) + + def test_env_overrides(self): + mock_client = MagicMock() + env = { + "CLAWCODEX_LLM_READ_TIMEOUT": "33", + "CLAWCODEX_LLM_CONNECT_TIMEOUT": "7", + "CLAWCODEX_LLM_MAX_RETRIES": "4", + } + with patch.dict("os.environ", env): + _apply_client_timeout(mock_client) + + kwargs = mock_client.with_options.call_args.kwargs + self.assertEqual(kwargs["max_retries"], 4) + self.assertEqual(kwargs["timeout"].read, 33.0) + self.assertEqual(kwargs["timeout"].connect, 7.0) + + def test_returns_client_unchanged_when_wrapping_fails(self): + mock_client = MagicMock() + mock_client.with_options.side_effect = TypeError("no with_options") + self.assertIs(_apply_client_timeout(mock_client), mock_client) + + class TestChatMessage(unittest.TestCase): """Test ChatMessage dataclass.""" @@ -329,6 +383,7 @@ def test_chat(self, mock_openai): """Test synchronous chat.""" # Setup mock mock_client = MagicMock() + mock_client.with_options.return_value = mock_client mock_response = MagicMock() mock_response.choices = [MagicMock()] mock_response.choices[0].message.content = "Hello!" @@ -348,11 +403,14 @@ def test_chat(self, mock_openai): self.assertEqual(response.content, "Hello!") self.assertEqual(response.model, "gpt-4") self.assertEqual(response.usage["total_tokens"], 15) + # The client property must wire the timeout wrapper in + mock_client.with_options.assert_called_once() @patch("src.providers.openai_provider.OpenAI") def test_chat_accepts_dict_messages(self, mock_openai): """Test synchronous chat with dict messages.""" mock_client = MagicMock() + mock_client.with_options.return_value = mock_client mock_response = MagicMock() mock_response.choices = [MagicMock()] mock_response.choices[0].message.content = "Hello!" @@ -378,6 +436,7 @@ def test_chat_accepts_dict_messages(self, mock_openai): def test_chat_stream_response_rebuilds_tool_calls(self, mock_openai): """Streaming chunks are rebuilt into a final response with tool calls.""" mock_client = MagicMock() + mock_client.with_options.return_value = mock_client chunk1 = MagicMock() chunk1.model = "gpt-4" @@ -447,6 +506,7 @@ def test_chat(self, mock_zhipu): """Test synchronous chat.""" # Setup mock mock_client = MagicMock() + mock_client.with_options.return_value = mock_client mock_response = MagicMock() mock_response.choices = [MagicMock()] mock_response.choices[0].message.content = "Hello!" @@ -473,6 +533,7 @@ def test_chat_with_reasoning(self, mock_zhipu): """Test chat with reasoning content.""" # Setup mock mock_client = MagicMock() + mock_client.with_options.return_value = mock_client mock_response = MagicMock() mock_response.choices = [MagicMock()] mock_response.choices[0].message.content = "Answer"