From ba074097240b6b53e5522229e347ffcfc340f152 Mon Sep 17 00:00:00 2001 From: Patrick Buckley Date: Tue, 17 Mar 2026 16:18:46 -0700 Subject: [PATCH] fix: isolate parallel tool exceptions + gate web_search without backend MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two bugs: (1) an uncaught exception in one parallel tool call killed the entire batch via pool.map(), losing all results including successful ones. Wrap run_one() in try/except so failures return error strings instead of propagating. (2) web_search was offered to local models even without a Tavily API key — the model would attempt it, only to fail at execution time. Filter web_search from _get_active_tools() and _run_agent() when neither native support nor Tavily is available. Closes https://github.com/turnstonelabs/turnstone/issues/117 --- tests/test_sessions.py | 147 ++++++++++++++++++++++++++++++++++++++ turnstone/core/session.py | 44 +++++++++--- 2 files changed, 180 insertions(+), 11 deletions(-) diff --git a/tests/test_sessions.py b/tests/test_sessions.py index e352c768..e707af2c 100644 --- a/tests/test_sessions.py +++ b/tests/test_sessions.py @@ -607,3 +607,150 @@ class TestPruneWorkstreams: # Config rows should be cleaned up assert load_workstream_config("orphan_cfg") == {} assert load_workstream_config("stale_cfg") == {} + + +# ── Parallel tool exception isolation ──────────────────────────────── + + +class TestParallelToolExceptionIsolation: + """Bug #117: one tool raising should not kill the entire batch.""" + + def test_exception_in_one_tool_does_not_kill_batch(self, tmp_db, mock_openai_client): + from unittest.mock import patch + + session = ChatSession( + client=mock_openai_client, + model="test-model", + ui=MagicMock(), + instructions=None, + temperature=0.5, + max_tokens=1000, + tool_timeout=10, + ) + + def succeed(item): + return item["call_id"], "ok" + + def fail(item): + raise RuntimeError("boom") + + items = [ + { + "call_id": "c1", + "func_name": "bash", + "execute": succeed, + "needs_approval": False, + "header": "test", + "preview": "", + }, + { + "call_id": "c2", + "func_name": "math", + "execute": fail, + "needs_approval": False, + "header": "test", + "preview": "", + }, + ] + + tool_calls = [ + {"id": "c1", "function": {"name": "bash", "arguments": "{}"}}, + {"id": "c2", "function": {"name": "math", "arguments": "{}"}}, + ] + + with ( + patch.object(session, "_prepare_tool", side_effect=items), + patch.object(session, "_evaluate_intent"), + patch.object(session, "_emit_state"), + patch.object(session, "_init_system_messages"), + patch.object(session, "_check_cancelled"), + ): + session.ui.approve_tools.return_value = (True, None) + results, _ = session._execute_tools(tool_calls) + + assert results[0] == ("c1", "ok") + assert results[1][0] == "c2" + assert "Error executing math" in results[1][1] + assert "boom" in results[1][1] + + +# ── Web search tool gating ─────────────────────────────────────────── + + +class TestWebSearchGating: + """Bug #117: web_search should not be offered without a backend.""" + + def test_web_search_filtered_when_no_backend(self, tmp_db, mock_openai_client): + from unittest.mock import patch + + from turnstone.core.providers._protocol import ModelCapabilities + + session = ChatSession( + client=mock_openai_client, + model="local-model", + ui=MagicMock(), + instructions=None, + temperature=0.5, + max_tokens=1000, + tool_timeout=10, + ) + + caps = ModelCapabilities(supports_web_search=False) + with ( + patch.object(session, "_get_capabilities", return_value=caps), + patch("turnstone.core.session.get_tavily_key", return_value=None), + ): + tools = session._get_active_tools() + + names = [t.get("function", {}).get("name") for t in tools] + assert "web_search" not in names + + def test_web_search_kept_when_tavily_available(self, tmp_db, mock_openai_client): + from unittest.mock import patch + + from turnstone.core.providers._protocol import ModelCapabilities + + session = ChatSession( + client=mock_openai_client, + model="local-model", + ui=MagicMock(), + instructions=None, + temperature=0.5, + max_tokens=1000, + tool_timeout=10, + ) + + caps = ModelCapabilities(supports_web_search=False) + with ( + patch.object(session, "_get_capabilities", return_value=caps), + patch("turnstone.core.session.get_tavily_key", return_value="tvly-test-key"), + ): + tools = session._get_active_tools() + + names = [t.get("function", {}).get("name") for t in tools] + assert "web_search" in names + + def test_web_search_kept_when_native_support(self, tmp_db, mock_openai_client): + from unittest.mock import patch + + from turnstone.core.providers._protocol import ModelCapabilities + + session = ChatSession( + client=mock_openai_client, + model="gpt-5-search-api", + ui=MagicMock(), + instructions=None, + temperature=0.5, + max_tokens=1000, + tool_timeout=10, + ) + + caps = ModelCapabilities(supports_web_search=True) + with ( + patch.object(session, "_get_capabilities", return_value=caps), + patch("turnstone.core.session.get_tavily_key", return_value=None), + ): + tools = session._get_active_tools() + + names = [t.get("function", {}).get("name") for t in tools] + assert "web_search" in names diff --git a/turnstone/core/session.py b/turnstone/core/session.py index 33727f02..86afb570 100644 --- a/turnstone/core/session.py +++ b/turnstone/core/session.py @@ -938,19 +938,29 @@ class ChatSession: - Client-side fallback: send visible tools + synthetic tool_search. Without tool search: return self._tools unchanged. + + Web search gating: ``web_search`` is removed when the model has + no native search support and no Tavily API key is configured. """ if self.creative_mode: return None - if not self._tool_search: - return self._tools - # Check if provider supports native tool search caps = self._get_capabilities() - if caps.supports_tool_search: - # Provider handles defer_loading — send all tools - return self._tools - # Client-side fallback: visible tools + search tool - visible = self._tool_search.get_visible_tools() - return visible + [self._tool_search.get_search_tool_definition()] + if not self._tool_search: + tools = self._tools + else: + if caps.supports_tool_search: + # Provider handles defer_loading — send all tools + tools = self._tools + else: + # Client-side fallback: visible tools + search tool + visible = self._tool_search.get_visible_tools() + tools = visible + [self._tool_search.get_search_tool_definition()] + + # Gate web_search: only include when a backend exists + if not caps.supports_web_search and not get_tavily_key(): + tools = [t for t in tools if t.get("function", {}).get("name") != "web_search"] + + return tools def _get_deferred_names(self) -> frozenset[str] | None: """Return names of deferred tools for native provider search, or None.""" @@ -2080,8 +2090,15 @@ class ChatSession: return item["call_id"], item["error"] if item.get("denied"): return item["call_id"], item.get("denial_msg", "Denied by user") - result: tuple[str, str | list[dict[str, Any]]] = item["execute"](item) - return result + try: + result: tuple[str, str | list[dict[str, Any]]] = item["execute"](item) + return result + except (KeyboardInterrupt, GenerationCancelled): + raise + except Exception as e: + func = item.get("func_name", "unknown") + log.warning("tool_exec.failed", tool=func, error=str(e)) + return item["call_id"], f"Error executing {func}: {e}" if len(items) == 1: results = [run_one(items[0])] @@ -3694,6 +3711,11 @@ class ChatSession: agent_client, agent_model, _ = self._registry.resolve(self._registry.agent_model) agent_provider = self._registry.get_provider(self._registry.agent_model) + # Gate web_search: remove when no backend exists for the agent model + agent_caps = agent_provider.get_capabilities(agent_model) + if not agent_caps.supports_web_search and not get_tavily_key(): + tools = [t for t in tools if t.get("function", {}).get("name") != "web_search"] + # Build extra params for agent calls agent_extra: dict[str, Any] | None = None if agent_provider.provider_name == "openai":