mirror of
https://github.com/turnstonelabs/turnstone.git
synced 2026-08-28 06:44:51 -06:00
fix: isolate parallel tool exceptions + gate web_search without backend
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
This commit is contained in:
committed by
Patrick Buckley
parent
c76a61841e
commit
ba07409724
@@ -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
|
||||
|
||||
+33
-11
@@ -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":
|
||||
|
||||
Reference in New Issue
Block a user