From 4d2dfbefbccd2ffa7d458d254f9296a1099295e6 Mon Sep 17 00:00:00 2001 From: "cuishengze@" Date: Thu, 26 Mar 2026 11:07:42 +0800 Subject: [PATCH 1/3] fix: stabilize execution paths and add regression tests --- openspace/dashboard_server.py | 43 ++++++++-- openspace/mcp_server.py | 10 ++- openspace/tool_layer.py | 21 +++-- tests/test_dashboard_server.py | 100 ++++++++++++++++++++++++ tests/test_mcp_server.py | 97 +++++++++++++++++++++++ tests/test_tool_layer.py | 139 +++++++++++++++++++++++++++++++++ 6 files changed, 395 insertions(+), 15 deletions(-) create mode 100644 tests/test_dashboard_server.py create mode 100644 tests/test_mcp_server.py create mode 100644 tests/test_tool_layer.py diff --git a/openspace/dashboard_server.py b/openspace/dashboard_server.py index 96d3e50..e063b36 100644 --- a/openspace/dashboard_server.py +++ b/openspace/dashboard_server.py @@ -424,11 +424,18 @@ def _discover_workflow_dirs() -> List[Path]: for root in WORKFLOW_ROOTS: if not root.exists(): continue - _scan_workflow_tree(root, discovered) + _scan_workflow_tree(root, discovered, root=root) return sorted(discovered.values(), key=lambda item: item.stat().st_mtime, reverse=True) -def _scan_workflow_tree(directory: Path, discovered: Dict[str, Path], *, _depth: int = 0, _max_depth: int = 6) -> None: +def _scan_workflow_tree( + directory: Path, + discovered: Dict[str, Path], + *, + root: Path, + _depth: int = 0, + _max_depth: int = 6, +) -> None: if _depth > _max_depth: return try: @@ -439,9 +446,35 @@ def _scan_workflow_tree(directory: Path, discovered: Dict[str, Path], *, _depth: if not child.is_dir(): continue if (child / "metadata.json").exists() or (child / "traj.jsonl").exists(): - discovered.setdefault(child.name, child) + discovered.setdefault(_workflow_id(child, root=root), child) else: - _scan_workflow_tree(child, discovered, _depth=_depth + 1, _max_depth=_max_depth) + _scan_workflow_tree( + child, + discovered, + root=root, + _depth=_depth + 1, + _max_depth=_max_depth, + ) + + +def _workflow_root(path: Path) -> Optional[Path]: + resolved = path.resolve() + for root in WORKFLOW_ROOTS: + try: + resolved.relative_to(root.resolve()) + return root + except ValueError: + continue + return None + + +def _workflow_id(path: Path, *, root: Optional[Path] = None) -> str: + workflow_root = root or _workflow_root(path) + if workflow_root is None: + return path.name + + relative = path.resolve().relative_to(workflow_root.resolve()) + return f"{workflow_root.name}__{'__'.join(relative.parts)}" def _get_workflow_dir(workflow_id: str) -> Optional[Path]: @@ -514,7 +547,7 @@ def _build_workflow_summary(workflow_dir: Path) -> Dict[str, Any]: iterations = len(trajectory) return { - "id": workflow_dir.name, + "id": _workflow_id(workflow_dir), "path": str(workflow_dir), "task_id": metadata.get("task_id") or metadata.get("task_name") or workflow_dir.name, "task_name": metadata.get("task_name") or metadata.get("task_id") or workflow_dir.name, diff --git a/openspace/mcp_server.py b/openspace/mcp_server.py index b1edf55..772786f 100644 --- a/openspace/mcp_server.py +++ b/openspace/mcp_server.py @@ -111,6 +111,11 @@ def seekable(self): _UPLOAD_META_FILENAME = ".upload_meta.json" +def _normalize_skill_dir(path_str: str) -> str: + """Return a canonical string form for a skill directory path.""" + return str(Path(path_str).expanduser().resolve()) + + async def _get_openspace(): """Lazy-initialise the OpenSpace engine.""" global _openspace_instance @@ -281,8 +286,9 @@ async def _auto_register_skill_dirs(skill_dirs: List[str]) -> int: store = _get_store() db_created = await store.sync_from_registry(added) - is_first = any(d not in _registered_skill_dirs for d in skill_dirs) - for d in skill_dirs: + normalized_valid_dirs = [_normalize_skill_dir(str(d)) for d in valid_dirs] + is_first = any(d not in _registered_skill_dirs for d in normalized_valid_dirs) + for d in normalized_valid_dirs: _registered_skill_dirs.add(d) if added: diff --git a/openspace/tool_layer.py b/openspace/tool_layer.py index 1ea419f..972a4ea 100644 --- a/openspace/tool_layer.py +++ b/openspace/tool_layer.py @@ -88,6 +88,7 @@ def __init__(self, config: Optional[OpenSpaceConfig] = None): self._skill_store: Optional[SkillStore] = None self._execution_analyzer: Optional[ExecutionAnalyzer] = None self._skill_evolver: Optional[SkillEvolver] = None + self._skill_selection_llm: Optional[LLMClient] = None self._execution_count: int = 0 # For periodic metric-based evolution self._last_evolved_skills: List[Dict[str, Any]] = [] # Tracks skills evolved during last execute() @@ -507,7 +508,9 @@ async def execute( f"Executing with GroundingAgent " f"(max {max_iterations} iterations, no skills)..." ) - result = await self._grounding_agent.process(execution_context) + execution_context_p0 = {**execution_context} + execution_context_p0["max_iterations"] = max_iterations + result = await self._grounding_agent.process(execution_context_p0) execution_time = asyncio.get_event_loop().time() - start_time @@ -730,12 +733,14 @@ def _get_skill_selection_llm(self) -> Optional[LLMClient]: """ # 1. Dedicated skill selection model (OpenSpaceConfig.skill_registry_model) if self.config.skill_registry_model: - return LLMClient( - model=self.config.skill_registry_model, - timeout=30.0, # skill selection should be fast - max_retries=2, - **self.config.llm_kwargs, - ) + if self._skill_selection_llm is None: + self._skill_selection_llm = LLMClient( + model=self.config.skill_registry_model, + timeout=30.0, # skill selection should be fast + max_retries=2, + **self.config.llm_kwargs, + ) + return self._skill_selection_llm # 2. Tool retrieval model if hasattr(self._grounding_agent, '_tool_retrieval_llm') and self._grounding_agent._tool_retrieval_llm: @@ -933,4 +938,4 @@ def __repr__(self) -> str: if self._running: status = "running" backends = ", ".join(self.config.backend_scope) if self.config.backend_scope else "all" - return f"" \ No newline at end of file + return f"" diff --git a/tests/test_dashboard_server.py b/tests/test_dashboard_server.py new file mode 100644 index 0000000..547eef7 --- /dev/null +++ b/tests/test_dashboard_server.py @@ -0,0 +1,100 @@ +import importlib +import sys +import tempfile +import types +import unittest +from pathlib import Path + + +def _install_dashboard_stubs(): + flask_mod = types.ModuleType("flask") + + class Flask: + def __init__(self, *args, **kwargs): + pass + + def route(self, *args, **kwargs): + def decorator(fn): + return fn + + return decorator + + flask_mod.Flask = Flask + flask_mod.abort = lambda *args, **kwargs: None + flask_mod.jsonify = lambda payload=None, **kwargs: payload if payload is not None else kwargs + flask_mod.send_from_directory = lambda *args, **kwargs: None + flask_mod.url_for = lambda *args, **kwargs: "/artifact" + + action_recorder_mod = types.ModuleType("openspace.recording.action_recorder") + action_recorder_mod.analyze_agent_actions = lambda actions: {} + action_recorder_mod.load_agent_actions = lambda path: [] + + recording_utils_mod = types.ModuleType("openspace.recording.utils") + recording_utils_mod.load_recording_session = lambda path: { + "metadata": {}, + "trajectory": [], + "plans": [], + "decisions": [], + "statistics": {}, + } + + skill_engine_mod = types.ModuleType("openspace.skill_engine") + skill_engine_mod.SkillStore = type("SkillStore", (), {}) + + skill_types_mod = types.ModuleType("openspace.skill_engine.types") + skill_types_mod.SkillRecord = type("SkillRecord", (), {}) + + stubs = { + "flask": flask_mod, + "openspace.recording.action_recorder": action_recorder_mod, + "openspace.recording.utils": recording_utils_mod, + "openspace.skill_engine": skill_engine_mod, + "openspace.skill_engine.types": skill_types_mod, + } + originals = {name: sys.modules.get(name) for name in stubs} + sys.modules.update(stubs) + return originals + + +class DashboardServerTests(unittest.TestCase): + @classmethod + def setUpClass(cls): + cls._originals = _install_dashboard_stubs() + sys.modules.pop("openspace.dashboard_server", None) + cls.dashboard = importlib.import_module("openspace.dashboard_server") + + @classmethod + def tearDownClass(cls): + sys.modules.pop("openspace.dashboard_server", None) + for name, module in cls._originals.items(): + if module is None: + sys.modules.pop(name, None) + else: + sys.modules[name] = module + + def test_discover_workflow_dirs_keeps_duplicate_leaf_names_from_different_roots(self): + with tempfile.TemporaryDirectory() as tmp: + base = Path(tmp) + root_a = base / "recordings" + root_b = base / "results" + dir_a = root_a / "shared-name" + dir_b = root_b / "shared-name" + dir_a.mkdir(parents=True) + dir_b.mkdir(parents=True) + (dir_a / "metadata.json").write_text("{}", encoding="utf-8") + (dir_b / "metadata.json").write_text("{}", encoding="utf-8") + + original_roots = self.dashboard.WORKFLOW_ROOTS + self.dashboard.WORKFLOW_ROOTS = [root_a, root_b] + try: + workflows = self.dashboard._discover_workflow_dirs() + workflow_ids = {self.dashboard._workflow_id(path) for path in workflows} + finally: + self.dashboard.WORKFLOW_ROOTS = original_roots + + self.assertEqual(len(workflows), 2) + self.assertEqual(len(workflow_ids), 2) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_mcp_server.py b/tests/test_mcp_server.py new file mode 100644 index 0000000..62faf09 --- /dev/null +++ b/tests/test_mcp_server.py @@ -0,0 +1,97 @@ +import importlib +import sys +import tempfile +import types +import unittest +from pathlib import Path + + +def _install_mcp_stubs(): + fastmcp_mod = types.ModuleType("mcp.server.fastmcp") + + class FastMCP: + def __init__(self, *args, **kwargs): + pass + + def tool(self): + def decorator(fn): + return fn + + return decorator + + def run(self, *args, **kwargs): + return None + + fastmcp_mod.FastMCP = FastMCP + + stubs = { + "mcp.server.fastmcp": fastmcp_mod, + } + originals = {name: sys.modules.get(name) for name in stubs} + sys.modules.update(stubs) + return originals + + +class MCPServerTests(unittest.IsolatedAsyncioTestCase): + @classmethod + def setUpClass(cls): + cls._originals = _install_mcp_stubs() + sys.modules.pop("openspace.mcp_server", None) + cls.mcp_server = importlib.import_module("openspace.mcp_server") + + @classmethod + def tearDownClass(cls): + sys.modules.pop("openspace.mcp_server", None) + for name, module in cls._originals.items(): + if module is None: + sys.modules.pop(name, None) + else: + sys.modules[name] = module + + async def test_auto_register_does_not_cache_missing_directories(self): + self.mcp_server._registered_skill_dirs.clear() + + class Registry: + def __init__(self): + self.calls = [] + + def discover_from_dirs(self, dirs): + self.calls.append([str(d.resolve()) for d in dirs]) + return dirs + + registry = Registry() + fake_store = types.SimpleNamespace() + + async def sync_from_registry(added): + return len(added) + + fake_store.sync_from_registry = sync_from_registry + fake_openspace = types.SimpleNamespace(_skill_registry=registry) + + async def fake_get_openspace(): + return fake_openspace + + def fake_get_store(): + return fake_store + + self.mcp_server._get_openspace = fake_get_openspace + self.mcp_server._get_store = fake_get_store + + with tempfile.TemporaryDirectory() as tmp: + base = Path(tmp) + valid_dir = base / "valid" + valid_dir.mkdir() + missing_dir = base / "missing" + + await self.mcp_server._auto_register_skill_dirs([str(missing_dir), str(valid_dir)]) + self.assertEqual(registry.calls[0], [str(valid_dir.resolve())]) + self.assertNotIn(str(missing_dir.resolve()), self.mcp_server._registered_skill_dirs) + + missing_dir.mkdir() + await self.mcp_server._auto_register_skill_dirs([str(missing_dir)]) + + self.assertEqual(registry.calls[1], [str(missing_dir.resolve())]) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_tool_layer.py b/tests/test_tool_layer.py new file mode 100644 index 0000000..9b5adc3 --- /dev/null +++ b/tests/test_tool_layer.py @@ -0,0 +1,139 @@ +import asyncio +import importlib +import sys +import types +import unittest + + +def _install_tool_layer_stubs(): + agents_mod = types.ModuleType("openspace.agents") + agents_mod.GroundingAgent = type("GroundingAgent", (), {}) + + llm_mod = types.ModuleType("openspace.llm") + + class FakeLLMClient: + instances = 0 + + def __init__(self, *args, **kwargs): + type(self).instances += 1 + self.model = kwargs.get("model") + + llm_mod.LLMClient = FakeLLMClient + + grounding_client_mod = types.ModuleType("openspace.grounding.core.grounding_client") + grounding_client_mod.GroundingClient = type("GroundingClient", (), {}) + + config_mod = types.ModuleType("openspace.config") + config_mod.get_config = lambda: None + config_mod.load_config = lambda *args, **kwargs: None + + config_loader_mod = types.ModuleType("openspace.config.loader") + config_loader_mod.get_agent_config = lambda *args, **kwargs: None + + recording_mod = types.ModuleType("openspace.recording") + recording_mod.RecordingManager = type("RecordingManager", (), {}) + + skill_engine_mod = types.ModuleType("openspace.skill_engine") + skill_engine_mod.SkillRegistry = type("SkillRegistry", (), {}) + skill_engine_mod.ExecutionAnalyzer = type("ExecutionAnalyzer", (), {}) + skill_engine_mod.SkillStore = type("SkillStore", (), {}) + + evolver_mod = types.ModuleType("openspace.skill_engine.evolver") + evolver_mod.SkillEvolver = type("SkillEvolver", (), {}) + + logging_mod = types.ModuleType("openspace.utils.logging") + + class Logger: + @staticmethod + def get_logger(name): + return types.SimpleNamespace( + info=lambda *a, **k: None, + debug=lambda *a, **k: None, + warning=lambda *a, **k: None, + error=lambda *a, **k: None, + ) + + logging_mod.Logger = Logger + + stubs = { + "openspace.agents": agents_mod, + "openspace.llm": llm_mod, + "openspace.grounding.core.grounding_client": grounding_client_mod, + "openspace.config": config_mod, + "openspace.config.loader": config_loader_mod, + "openspace.recording": recording_mod, + "openspace.skill_engine": skill_engine_mod, + "openspace.skill_engine.evolver": evolver_mod, + "openspace.utils.logging": logging_mod, + } + + originals = {name: sys.modules.get(name) for name in stubs} + sys.modules.update(stubs) + return originals, FakeLLMClient + + +class ToolLayerTests(unittest.IsolatedAsyncioTestCase): + @classmethod + def setUpClass(cls): + cls._originals, cls.fake_llm_class = _install_tool_layer_stubs() + sys.modules.pop("openspace.tool_layer", None) + cls.tool_layer = importlib.import_module("openspace.tool_layer") + + @classmethod + def tearDownClass(cls): + sys.modules.pop("openspace.tool_layer", None) + for name, module in cls._originals.items(): + if module is None: + sys.modules.pop(name, None) + else: + sys.modules[name] = module + + async def test_execute_passes_resolved_iterations_without_skills(self): + config = self.tool_layer.OpenSpaceConfig() + config.grounding_max_iterations = 7 + + openspace = self.tool_layer.OpenSpace(config) + openspace._initialized = True + openspace._task_done = asyncio.Event() + openspace._task_done.set() + openspace._grounding_client = types.SimpleNamespace(_registry={}) + openspace._recording_manager = None + openspace._skill_registry = None + openspace._execution_analyzer = None + openspace._skill_evolver = None + + recorded_contexts = [] + + class FakeAgent: + async def process(self, context): + recorded_contexts.append(dict(context)) + return {"status": "success", "iterations": 1, "tool_executions": []} + + openspace._grounding_agent = FakeAgent() + + async def no_op(*args, **kwargs): + return None + + openspace._maybe_analyze_execution = no_op + openspace._maybe_evolve_quality = no_op + + result = await openspace.execute("do work", max_iterations=3) + + self.assertEqual(result["status"], "success") + self.assertEqual(recorded_contexts[0]["max_iterations"], 7) + + def test_get_skill_selection_llm_reuses_dedicated_client(self): + self.fake_llm_class.instances = 0 + config = self.tool_layer.OpenSpaceConfig() + config.skill_registry_model = "selector-model" + openspace = self.tool_layer.OpenSpace(config) + + first = openspace._get_skill_selection_llm() + second = openspace._get_skill_selection_llm() + + self.assertIs(first, second) + self.assertEqual(self.fake_llm_class.instances, 1) + + +if __name__ == "__main__": + unittest.main() From b52a0c378057a37f169a34293ca6408acf5c466d Mon Sep 17 00:00:00 2001 From: "cuishengze@" Date: Thu, 26 Mar 2026 11:42:17 +0800 Subject: [PATCH 2/3] fix: resolve UI summary f-string syntax error --- openspace/utils/ui.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/openspace/utils/ui.py b/openspace/utils/ui.py index 22729ec..5e68524 100644 --- a/openspace/utils/ui.py +++ b/openspace/utils/ui.py @@ -424,7 +424,8 @@ def print_summary(self, result: Dict[str, Any]): status_text = status_display.get(status, status) print(box.text_line(f" Status: {status_text}", indent=4, text_color='')) - print(box.text_line(f" Execution Time: {colorize(f'{result.get('execution_time', 0):.2f}s', 'c')}", indent=4, text_color='')) + exec_time = result.get("execution_time", 0) + print(box.text_line(f" Execution Time: {colorize(f'{exec_time:.2f}s', 'c')}", indent=4, text_color='')) print(box.text_line(f" Iterations: {colorize(str(result.get('iterations', 0)), 'y')}", indent=4, text_color='')) print(box.text_line(f" Completed Tasks: {colorize(str(result.get('completed_tasks', 0)), 'g')}", indent=4, text_color='')) @@ -443,4 +444,4 @@ def create_ui(enable_live: bool = True, compact: bool = False) -> OpenSpaceUI: enable_live: Whether to enable live display updates compact: Use compact layout for smaller terminals """ - return OpenSpaceUI(enable_live=enable_live, compact=compact) \ No newline at end of file + return OpenSpaceUI(enable_live=enable_live, compact=compact) From c46468d2f415693efb630a03c8adc6637ae6e2fb Mon Sep 17 00:00:00 2001 From: "cuishengze@" Date: Thu, 26 Mar 2026 14:24:25 +0800 Subject: [PATCH 3/3] fix: degrade gracefully when websockets dependency is missing --- .../mcp/transport/connectors/__init__.py | 15 +++++++- tests/test_mcp_connectors_optional.py | 36 +++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-) create mode 100644 tests/test_mcp_connectors_optional.py diff --git a/openspace/grounding/backends/mcp/transport/connectors/__init__.py b/openspace/grounding/backends/mcp/transport/connectors/__init__.py index c2e2fcc..b0e27ac 100644 --- a/openspace/grounding/backends/mcp/transport/connectors/__init__.py +++ b/openspace/grounding/backends/mcp/transport/connectors/__init__.py @@ -5,11 +5,24 @@ through different transport mechanisms. """ +from importlib.util import find_spec + from .base import MCPBaseConnector # noqa: F401 from .http import HttpConnector # noqa: F401 from .sandbox import SandboxConnector # noqa: F401 from .stdio import StdioConnector # noqa: F401 -from .websocket import WebSocketConnector # noqa: F401 + +if find_spec("websockets") is not None: + from .websocket import WebSocketConnector # noqa: F401 +else: + class WebSocketConnector: + """Fallback connector when optional websocket dependency is unavailable.""" + + def __init__(self, *args, **kwargs): + raise ImportError( + "WebSocket MCP transport requires optional dependency 'websockets'. " + "Install it with: pip install websockets" + ) __all__ = [ "MCPBaseConnector", diff --git a/tests/test_mcp_connectors_optional.py b/tests/test_mcp_connectors_optional.py new file mode 100644 index 0000000..2fbeb04 --- /dev/null +++ b/tests/test_mcp_connectors_optional.py @@ -0,0 +1,36 @@ +import importlib +import importlib.util +import unittest +from unittest.mock import patch + + +MODULE_NAME = "openspace.grounding.backends.mcp.transport.connectors" + + +class MCPConnectorsOptionalDepsTests(unittest.TestCase): + def test_websocket_connector_available_when_dependency_installed(self): + connectors = importlib.import_module(MODULE_NAME) + self.assertEqual( + connectors.WebSocketConnector.__module__, + f"{MODULE_NAME}.websocket", + ) + + def test_websocket_connector_fallback_when_dependency_missing(self): + connectors = importlib.import_module(MODULE_NAME) + original_find_spec = importlib.util.find_spec + + def fake_find_spec(name: str, *args, **kwargs): + if name == "websockets": + return None + return original_find_spec(name, *args, **kwargs) + + with patch("importlib.util.find_spec", side_effect=fake_find_spec): + connectors = importlib.reload(connectors) + with self.assertRaisesRegex(ImportError, "pip install websockets"): + connectors.WebSocketConnector(url="ws://localhost:1234") + + importlib.reload(connectors) + + +if __name__ == "__main__": + unittest.main()