From 21e695fcb6e379018687db7445a578aba981f67d Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 29 Apr 2026 23:53:17 -0700 Subject: [PATCH] fix: clean up defensive shims and finish CI stabilization from #17660 (#17801) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #17660 landed a sweep of CI fixes but left three loose ends: 1. tests/cli/test_cli_loading_indicator.py::test_reload_mcp_sets_busy_state_ and_prints_status — /reload-mcp gained a prompt-cache-invalidation confirmation (commit 4d7fc0f37) that was never wired into this test. The test exercises the loading-indicator path, so pre-approve via config and go straight into _reload_mcp(). 2. tools/mcp_tool.py _make_tool_handler — the added getattr(server, '_rpc_lock', None) + 'skip the lock if missing' branch is inconsistent with four sibling call sites that still direct-access server._rpc_lock. The lock is guaranteed by MCPServerTask.__init__; falling through to an unlocked session.call_tool would silently serialize-strip RPCs if the guard ever triggered. Restore direct access. 3. tui_gateway/server.py _messages_as_conversation — the helper existed only to catch 'TypeError: include_ancestors unexpected' from mocked SessionDBs that don't actually exist. The real SessionDB.get_messages_as_conversation has accepted include_ancestors since introduction, and every test FakeDB in the repo already declares the kwarg. Remove the shim, inline the two call sites. --- tests/cli/test_cli_loading_indicator.py | 9 ++++++++- tools/mcp_tool.py | 6 +----- tui_gateway/server.py | 22 +++++----------------- 3 files changed, 14 insertions(+), 23 deletions(-) diff --git a/tests/cli/test_cli_loading_indicator.py b/tests/cli/test_cli_loading_indicator.py index 6cec9eca3dc..dd7bdb68d13 100644 --- a/tests/cli/test_cli_loading_indicator.py +++ b/tests/cli/test_cli_loading_indicator.py @@ -49,8 +49,15 @@ class TestCLILoadingIndicator: seen["status"] = cli_obj._command_status print("reload done") + # /reload-mcp now wraps the actual reload in a prompt-cache-invalidation + # confirmation prompt (commit 4d7fc0f37). This test exercises the + # loading-indicator path, not the confirmation UX, so pre-approve the + # reload via config so the handler goes straight into _reload_mcp(). + fake_cfg = {"approvals": {"mcp_reload_confirm": False}} + with patch.object(cli_obj, "_reload_mcp", side_effect=fake_reload), \ - patch.object(cli_obj, "_invalidate") as invalidate_mock: + patch.object(cli_obj, "_invalidate") as invalidate_mock, \ + patch("cli.load_cli_config", return_value=fake_cfg): assert cli_obj.process_command("/reload-mcp") output = capsys.readouterr().out diff --git a/tools/mcp_tool.py b/tools/mcp_tool.py index 277928bc2f6..2a0115ec858 100644 --- a/tools/mcp_tool.py +++ b/tools/mcp_tool.py @@ -2010,12 +2010,8 @@ def _make_tool_handler(server_name: str, tool_name: str, tool_timeout: float): }, ensure_ascii=False) async def _call(): - rpc_lock = getattr(server, "_rpc_lock", None) - if rpc_lock is None: + async with server._rpc_lock: result = await server.session.call_tool(tool_name, arguments=args) - else: - async with rpc_lock: - result = await server.session.call_tool(tool_name, arguments=args) # MCP CallToolResult has .content (list of content blocks) and .isError if result.isError: error_text = "" diff --git a/tui_gateway/server.py b/tui_gateway/server.py index 6b00dde3748..82e97506114 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -1860,18 +1860,6 @@ def _enrich_with_attached_images(user_text: str, image_paths: list[str]) -> str: return text or "What do you see in this image?" -def _messages_as_conversation(db, session_id: str, *, include_ancestors: bool = False): - if include_ancestors: - try: - return db.get_messages_as_conversation( - session_id, include_ancestors=True - ) - except TypeError as exc: - if "include_ancestors" not in str(exc): - raise - return db.get_messages_as_conversation(session_id) - - def _history_to_messages(history: list[dict]) -> list[dict]: messages = [] tool_call_args = {} @@ -2080,9 +2068,9 @@ def _(rid, params: dict) -> dict: _enable_gateway_prompts() try: db.reopen_session(target) - history = _messages_as_conversation(db, target) - display_history = _messages_as_conversation( - db, target, include_ancestors=True + history = db.get_messages_as_conversation(target) + display_history = db.get_messages_as_conversation( + target, include_ancestors=True ) messages = _history_to_messages(display_history) tokens = _set_session_context(target) @@ -2227,8 +2215,8 @@ def _(rid, params: dict) -> dict: db = _get_db() if db is not None and session.get("session_key"): try: - history = _messages_as_conversation( - db, session["session_key"], include_ancestors=True + history = db.get_messages_as_conversation( + session["session_key"], include_ancestors=True ) except Exception: pass