From 171c1b76f8a9f9715523a9d2d796e003552f06bb Mon Sep 17 00:00:00 2001 From: "xzq.xu" Date: Thu, 26 Mar 2026 13:55:24 +0800 Subject: [PATCH 1/5] fix: replace ErrorCode enum calls with GroundingError raises ErrorCode is a str Enum whose members are not callable. Calling `raise ErrorCode.SESSION_NOT_FOUND(name)` produces a TypeError instead of the intended session-not-found error. Replace all 5 occurrences with `raise GroundingError(..., code=ErrorCode.SESSION_NOT_FOUND)`. Closes #11 Made-with: Cursor --- openspace/grounding/core/grounding_client.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/openspace/grounding/core/grounding_client.py b/openspace/grounding/core/grounding_client.py index 4f97a7f..ad6c51c 100644 --- a/openspace/grounding/core/grounding_client.py +++ b/openspace/grounding/core/grounding_client.py @@ -337,13 +337,13 @@ class GroundingClient: def get_session_info(self, name: str) -> SessionInfo: """Get session monitoring info""" if name not in self._session_info: - raise ErrorCode.SESSION_NOT_FOUND(name) + raise GroundingError(f"Session not found: {name}", code=ErrorCode.SESSION_NOT_FOUND) return self._session_info[name] def get_session(self, name: str) -> BaseSession: """Get session""" if name not in self._sessions: - raise ErrorCode.SESSION_NOT_FOUND(name) + raise GroundingError(f"Session not found: {name}", code=ErrorCode.SESSION_NOT_FOUND) return self._sessions[name] @@ -479,7 +479,7 @@ class GroundingClient: # Session-level if session_name: if session_name not in self._sessions: - raise ErrorCode.SESSION_NOT_FOUND(session_name) + raise GroundingError(f"Session not found: {session_name}", code=ErrorCode.SESSION_NOT_FOUND) backend_type = self._session_info[session_name].backend_type return await self._fetch_tools( backend_type, @@ -531,7 +531,7 @@ class GroundingClient: use_cache: bool = False ) -> list[BaseTool]: if session_name not in self._session_info: - raise ErrorCode.SESSION_NOT_FOUND(session_name) + raise GroundingError(f"Session not found: {session_name}", code=ErrorCode.SESSION_NOT_FOUND) backend = self._session_info[session_name].backend_type return await self.list_tools(backend, session_name, use_cache) @@ -838,7 +838,7 @@ class GroundingClient: runtime_backend = backend else: if runtime_session not in self._session_info: - raise ErrorCode.SESSION_NOT_FOUND(runtime_session) + raise GroundingError(f"Session not found: {runtime_session}", code=ErrorCode.SESSION_NOT_FOUND) runtime_backend = self._session_info[ runtime_session ].backend_type From 6f581f6de4067e3b5cebd73713e904909279fc19 Mon Sep 17 00:00:00 2001 From: "xzq.xu" Date: Thu, 26 Mar 2026 13:57:03 +0800 Subject: [PATCH 2/5] fix: add missing Logger.set_level() method __main__.py calls Logger.set_level(args.log_level) when --log-level is passed, but the method did not exist on Logger, causing an AttributeError. Add set_level(level: str) that resolves the name to a logging constant and reconfigures via configure(force=True). Closes #13 Made-with: Cursor --- openspace/utils/logging.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/openspace/utils/logging.py b/openspace/utils/logging.py index fabb5b1..ce6a3b8 100644 --- a/openspace/utils/logging.py +++ b/openspace/utils/logging.py @@ -233,6 +233,14 @@ class Logger: cls._configured = True + @classmethod + def set_level(cls, level: str) -> None: + """Set log level by name (e.g. ``"DEBUG"``, ``"INFO"``, ``"WARNING"``).""" + resolved = getattr(logging, level.upper(), None) + if resolved is None or not isinstance(resolved, int): + raise ValueError(f"Unknown log level: {level!r}") + cls.configure(level=resolved, force=True) + @classmethod def set_debug(cls, debug_level: int = 2) -> None: """Dynamically switch debug level: 0 = WARNING, 1 = INFO, 2 = DEBUG.""" From f89ea89ffb1feff081a8b7725947b160079481fc Mon Sep 17 00:00:00 2001 From: "xzq.xu" Date: Thu, 26 Mar 2026 13:58:10 +0800 Subject: [PATCH 3/5] fix: use resolved tool_obj for fallback tool execution When the LLM returns a short tool name that doesn't match the deduped key in tool_map, the fallback scan correctly resolves tool_obj via schema.name. However the execution branch still checked `tool_name not in tool_map` and passed `tool_map[tool_name]`, so fallback-resolved tools were never executed. Change the condition to check `tool_obj is None` and pass `tool_obj` directly to _execute_tool_call. Closes #15 Made-with: Cursor --- openspace/llm/client.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/openspace/llm/client.py b/openspace/llm/client.py index cc14c64..ded3ab8 100644 --- a/openspace/llm/client.py +++ b/openspace/llm/client.py @@ -754,7 +754,7 @@ class LLMClient: except: pass - if tool_name not in tool_map: + if tool_obj is None: result = ToolResult( status=ToolStatus.ERROR, error=f"Tool '{tool_name}' not found" @@ -762,7 +762,7 @@ class LLMClient: else: try: result = await _execute_tool_call( - tool=tool_map[tool_name], + tool=tool_obj, openai_tool_call={ "id": tool_call.id, "type": "function", From af1eb5bbe68abb6b17922f069dce24383bab9e97 Mon Sep 17 00:00:00 2001 From: "xzq.xu" Date: Thu, 26 Mar 2026 14:01:03 +0800 Subject: [PATCH 4/5] fix(security): stop leaking Python tracebacks to MCP clients Error handlers in execute_task, fix_skill, and upload_skill returned traceback.format_exc() to MCP clients, exposing internal file paths, code structure, and potentially sensitive details. The full traceback is already logged server-side via logger.error(exc_info=True). Remove the traceback field from client-facing error responses and clean up the unused traceback import. Closes #19 Made-with: Cursor --- openspace/mcp_server.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/openspace/mcp_server.py b/openspace/mcp_server.py index b010f2f..168485c 100644 --- a/openspace/mcp_server.py +++ b/openspace/mcp_server.py @@ -22,7 +22,6 @@ import json import logging import os import sys -import traceback from pathlib import Path from typing import Any, Dict, List, Optional @@ -597,7 +596,7 @@ async def execute_task( except Exception as e: logger.error(f"execute_task failed: {e}", exc_info=True) - return _json_error(e, status="error", traceback=traceback.format_exc(limit=5)) + return _json_error(e, status="error") @mcp.tool() @@ -818,7 +817,7 @@ async def fix_skill( except Exception as e: logger.error(f"fix_skill failed: {e}", exc_info=True) - return _json_error(e, status="error", traceback=traceback.format_exc(limit=5)) + return _json_error(e, status="error") @mcp.tool() @@ -889,7 +888,7 @@ async def upload_skill( except Exception as e: logger.error(f"upload_skill failed: {e}", exc_info=True) - return _json_error(e, status="error", traceback=traceback.format_exc(limit=5)) + return _json_error(e, status="error") def run_mcp_server() -> None: """Console-script entry point for ``openspace-mcp``.""" From 34d82b735e80b87efbdab668438fe33d39d20309 Mon Sep 17 00:00:00 2001 From: Dennis-yxchen Date: Fri, 3 Apr 2026 23:39:16 +0800 Subject: [PATCH 5/5] fix: make tool fallback conservative Co-authored-by: xzq.xu --- openspace/llm/client.py | 61 ++++++++++++++++++++++++++++++-------- openspace/utils/logging.py | 13 ++++++-- 2 files changed, 59 insertions(+), 15 deletions(-) diff --git a/openspace/llm/client.py b/openspace/llm/client.py index ded3ab8..c3c4620 100644 --- a/openspace/llm/client.py +++ b/openspace/llm/client.py @@ -190,6 +190,37 @@ def _infer_backend_from_tool_name(tool_name: str) -> Optional[str]: return None +def _resolve_tool_call_target( + tool_name: str, + tool_map: Dict[str, BaseTool], +) -> tuple[Optional[BaseTool], List[str]]: + """Resolve a returned tool name to a concrete tool object. + + The LLM is expected to return the deduped tool key from ``tool_map``. + Some providers occasionally return the short schema name instead. In that + case we only recover when exactly one tool shares that schema name; if + multiple tools match, the call is ambiguous and should not be executed. + """ + tool_obj = tool_map.get(tool_name) + if tool_obj is not None or not tool_name: + return tool_obj, [] + + fallback_matches = [ + (llm_name, tool) + for llm_name, tool in tool_map.items() + if getattr(getattr(tool, "schema", None), "name", None) == tool_name + ] + if len(fallback_matches) == 1: + resolved_name, resolved_tool = fallback_matches[0] + logger.info( + f"[TOOL_FALLBACK] Resolved short tool name '{tool_name}' to '{resolved_name}'" + ) + return resolved_tool, [] + if len(fallback_matches) > 1: + return None, [llm_name for llm_name, _tool in fallback_matches] + return None, [] + + DEFAULT_SUMMARIZE_THRESHOLD_CHARS = 200000 # ~50K tokens, lowered from 400K to prevent context overflow MAX_TOOL_RESULT_CHARS = 200000 # Fallback truncation limit when summarization fails (~50K tokens) @@ -705,14 +736,9 @@ class LLMClient: for tool_call in tool_calls: tool_name = tool_call.function.name - # Resolve tool instance: key might differ from model response (e.g. API returns - # "read_file" while we stored "server__read_file" for dedup), so fallback by schema.name - tool_obj = tool_map.get(tool_name) - if tool_obj is None and tool_name: - for _k, _t in tool_map.items(): - if getattr(getattr(_t, "schema", None), "name", None) == tool_name: - tool_obj = _t - break + # Resolve tool instance: some providers return the short schema + # name instead of the deduped LLM-visible tool key. + tool_obj, ambiguous_tool_names = _resolve_tool_call_target(tool_name, tool_map) backend = None server_name = None @@ -755,10 +781,19 @@ class LLMClient: pass if tool_obj is None: - result = ToolResult( - status=ToolStatus.ERROR, - error=f"Tool '{tool_name}' not found" - ) + if ambiguous_tool_names: + result = ToolResult( + status=ToolStatus.ERROR, + error=( + f"Tool '{tool_name}' is ambiguous; matches: " + f"{', '.join(ambiguous_tool_names)}" + ) + ) + else: + result = ToolResult( + status=ToolStatus.ERROR, + error=f"Tool '{tool_name}' not found" + ) else: try: result = await _execute_tool_call( @@ -871,4 +906,4 @@ class LLMClient: role = msg.get("role", "unknown").upper() content = msg.get("content", "") formatted += f"[{role}]\n{content}\n\n" - return formatted \ No newline at end of file + return formatted diff --git a/openspace/utils/logging.py b/openspace/utils/logging.py index ce6a3b8..7382374 100644 --- a/openspace/utils/logging.py +++ b/openspace/utils/logging.py @@ -239,7 +239,16 @@ class Logger: resolved = getattr(logging, level.upper(), None) if resolved is None or not isinstance(resolved, int): raise ValueError(f"Unknown log level: {level!r}") - cls.configure(level=resolved, force=True) + if not cls._configured: + cls.configure(level=resolved, attach_to_root=True) + return + + root_logger = logging.getLogger() + root_logger.setLevel(resolved) + for handler in root_logger.handlers: + handler.setLevel(resolved) + + cls._update_level(resolved) @classmethod def set_debug(cls, debug_level: int = 2) -> None: @@ -317,4 +326,4 @@ Logger.configure(attach_to_root=True) # Get openspace logger for internal logging logger = Logger.get_logger() -logger.debug("OpenSpace logging initialized") \ No newline at end of file +logger.debug("OpenSpace logging initialized")