From 5fb2560078fba8ec84c3b267793dac888d25edc1 Mon Sep 17 00:00:00 2001 From: Andrew Klatzke Date: Fri, 2 Oct 2026 12:59:26 -0800 Subject: [PATCH 1/2] fix(langchain-agents): accept sync tool handlers Graph handoff tools stay synchronous so routing can record the selected edge. Awaiting every tool result raised before that record happened. Co-authored-by: Cursor --- .../handler.py | 10 +- .../langchain-agents/tests/test_handler.py | 146 ++++++++++++++++++ 2 files changed, 154 insertions(+), 2 deletions(-) diff --git a/packages/langchain-agents/src/launchdarkly_ai_langchain_agents/handler.py b/packages/langchain-agents/src/launchdarkly_ai_langchain_agents/handler.py index bc72beb3..41c69ed6 100644 --- a/packages/langchain-agents/src/launchdarkly_ai_langchain_agents/handler.py +++ b/packages/langchain-agents/src/launchdarkly_ai_langchain_agents/handler.py @@ -6,6 +6,7 @@ from __future__ import annotations import asyncio +import inspect import json from collections.abc import AsyncGenerator from typing import Any @@ -60,8 +61,13 @@ async def _handler(_name: str = name, **kwargs: Any) -> str: fn = tool_handlers.get(_name) if not fn: raise ValueError(f'No handler registered for tool "{_name}"') - res = await fn(kwargs) - return str(res) + # Handlers may be sync or async. Graph ``__handoff_*`` tools stay sync so + # routing records the selected edge on the call itself; awaiting a plain + # return value raises. Same rule as ``tracking.wrap_tool_handlers``. + result = fn(kwargs) + if inspect.isawaitable(result): + result = await result + return str(result) t = tool_fn( name, diff --git a/packages/langchain-agents/tests/test_handler.py b/packages/langchain-agents/tests/test_handler.py index 1861a1ed..f3cd1854 100644 --- a/packages/langchain-agents/tests/test_handler.py +++ b/packages/langchain-agents/tests/test_handler.py @@ -398,6 +398,24 @@ def _capture_tool(fn: Any, **kw: Any) -> Any: # --------------------------------------------------------------------------- +def _built_tool(config_tools: dict[str, Any], handlers: dict[str, Any]) -> Any: + """The LangChain tool callable ``_build_agent_tools`` registers for the first tool.""" + mocks = _make_langchain_mock() + captured: list[Any] = [] + + def _capture_tool(_name: Any, fn: Any = None, **_kw: Any) -> Any: + captured.append(fn) + return fn + + mocks["langchain_core.tools"].tool = MagicMock(side_effect=_capture_tool) + with _patch_lc(mocks): + from launchdarkly_ai_langchain_agents.handler import _build_agent_tools + + _build_agent_tools(config_tools, handlers) + assert captured, "tool was not registered" + return captured[0] + + class TestToolExecutionLoop: @pytest.mark.asyncio async def test_tool_not_found_throws(self) -> None: @@ -448,6 +466,134 @@ async def _bad_handler(args: Any) -> str: with pytest.raises(RuntimeError, match="tool error"): await captured_fns[0](key="val") + @pytest.mark.asyncio + async def test_sync_handler_returns_string(self) -> None: + def sync_handler(args: dict[str, Any]) -> str: + assert args == {"city": "Paris"} + return "sunny" + + tool = _built_tool( + {"weather": {"description": "d", "parameters": {}}}, + {"weather": sync_handler}, + ) + assert await tool(city="Paris") == "sunny" + + @pytest.mark.asyncio + async def test_async_handler_is_awaited(self) -> None: + seen: dict[str, Any] = {} + + async def async_handler(args: dict[str, Any]) -> str: + seen["args"] = args + return "awaited" + + tool = _built_tool( + {"lookup": {"description": "d", "parameters": {}}}, + {"lookup": async_handler}, + ) + assert await tool(q="hi") == "awaited" + assert seen["args"] == {"q": "hi"} + + @pytest.mark.asyncio + async def test_handoff_handler_records_destination(self) -> None: + chosen: list[str] = [] + + def handoff(_args: dict[str, Any]) -> str: + if not chosen: + chosen.append("billing") + return ( + "Handoff to billing recorded. " + "Finish your own work and provide your final response." + ) + + tool = _built_tool( + {"__handoff_billing": {"description": "transfer", "parameters": {}}}, + {"__handoff_billing": handoff}, + ) + assert await tool() == ( + "Handoff to billing recorded. " + "Finish your own work and provide your final response." + ) + assert chosen == ["billing"] + + @pytest.mark.asyncio + @pytest.mark.parametrize("sync", [True, False]) + async def test_handler_exception_propagates(self, sync: bool) -> None: + def sync_handler(_args: dict[str, Any]) -> str: + raise RuntimeError("sync tool error") + + async def async_handler(_args: dict[str, Any]) -> str: + raise RuntimeError("async tool error") + + handler = sync_handler if sync else async_handler + tool = _built_tool( + {"my-tool": {"description": "d", "parameters": {}}}, + {"my-tool": handler}, + ) + match = "sync tool error" if sync else "async tool error" + with pytest.raises(RuntimeError, match=match): + await tool(key="val") + + @pytest.mark.asyncio + async def test_invoke_and_stream_run_sync_handoff(self) -> None: + """Invoke and stream both install tools from ``_build_agent_tools``.""" + recorded: list[str] = [] + + def handoff(_args: dict[str, Any]) -> str: + recorded.append("billing") + return "Handoff to billing recorded." + + config = _make_config( + instructions="route", + tools={"__handoff_billing": {"description": "transfer", "parameters": {}}}, + ) + handlers = {"__handoff_billing": handoff} + + async def _drive(stream: bool) -> None: + mocks = _make_langchain_mock() + built: dict[str, Any] = {} + + def _create(_model: Any, tools: list[Any], **_kwargs: Any) -> Any: + built["tools"] = tools + return mocks["_agent"] + + mocks["langgraph.prebuilt"].create_react_agent = MagicMock( + side_effect=_create + ) + + async def _call_handoff() -> str: + assert built.get("tools"), "route did not receive built tools" + return await built["tools"][0]() + + async def _ainvoke(*_a: Any, **_kw: Any) -> dict[str, Any]: + built["text"] = await _call_handoff() + return {"messages": [mocks["_ai_msg"]]} + + async def _astream(*_a: Any, **_kw: Any) -> AsyncIterator[Any]: + built["text"] = await _call_handoff() + yield {"agent": {"messages": [mocks["_ai_msg"]]}} + + mocks["_agent"].ainvoke = _ainvoke + mocks["_agent"].astream = _astream + + with _patch_lc(mocks), patch.object(spans_mod, "_HAS_OTEL", False): + h = create_langchain_agents_handler(llm=MagicMock()) + if stream: + async for _event in await h.stream(config, "hi", handlers): + pass + else: + await h(config, "hi", handlers) + + assert built["text"] == "Handoff to billing recorded." + + with patch.object( + handler_mod, "_build_agent_tools", wraps=handler_mod._build_agent_tools + ) as build_tools: + await _drive(False) + await _drive(True) + assert build_tools.call_count == 2 + + assert recorded == ["billing", "billing"] + @pytest.mark.asyncio async def test_no_tools_in_config_handler_never_invoked(self) -> None: mocks = _make_langchain_mock() From 100ca1f6ef071ac4867ddbedc478b8f3a63a7475 Mon Sep 17 00:00:00 2001 From: Andrew Klatzke Date: Mon, 5 Oct 2026 10:20:50 -0800 Subject: [PATCH 2/2] fix(agents): accept sync handlers on native-graph tool paths LangChain and OpenAI native graphs read tool_handlers without wrap_tool_handlers, so sync handlers raised TypeError on await. Match the handler/tracking isawaitable rule and document the sync/async contract. Co-authored-by: Cursor --- packages/langchain-agents/README.md | 5 + .../native_graph.py | 9 +- .../langchain-agents/tests/test_handler.py | 7 ++ .../tests/test_native_graph.py | 80 ++++++++++++- packages/openai-agents/README.md | 5 + .../native_graph.py | 9 +- .../openai-agents/tests/test_native_graph.py | 106 +++++++++++++++++- 7 files changed, 215 insertions(+), 6 deletions(-) diff --git a/packages/langchain-agents/README.md b/packages/langchain-agents/README.md index 71599fed..801801b7 100644 --- a/packages/langchain-agents/README.md +++ b/packages/langchain-agents/README.md @@ -37,6 +37,11 @@ async def main(): asyncio.run(main()) ``` +Tool handlers may be sync or async. Sync handlers run on the event-loop thread, so +blocking I/O stalls the agent. Keep graph `__handoff_*` handlers synchronous: routing +records the selected edge on the call itself, and moving them onto `asyncio.to_thread` +would break that. + ### With a custom `BaseChatModel` ```python diff --git a/packages/langchain-agents/src/launchdarkly_ai_langchain_agents/native_graph.py b/packages/langchain-agents/src/launchdarkly_ai_langchain_agents/native_graph.py index b0635866..190e5f87 100644 --- a/packages/langchain-agents/src/launchdarkly_ai_langchain_agents/native_graph.py +++ b/packages/langchain-agents/src/launchdarkly_ai_langchain_agents/native_graph.py @@ -5,6 +5,7 @@ from __future__ import annotations +import inspect import re import time import types @@ -79,8 +80,12 @@ async def _handler(_name: str = name, **kwargs: Any) -> str: fn = tool_handlers.get(_name) if not fn or isinstance(fn, NativeTool): return "" - res = await fn(kwargs) - return str(res) + # Handlers may be sync or async. Same rule as handler._build_agent_tools + # and tracking.wrap_tool_handlers โ€” awaiting a plain return raises. + result = fn(kwargs) + if inspect.isawaitable(result): + result = await result + return str(result) t = tool_fn( name, diff --git a/packages/langchain-agents/tests/test_handler.py b/packages/langchain-agents/tests/test_handler.py index f3cd1854..e43cb3b0 100644 --- a/packages/langchain-agents/tests/test_handler.py +++ b/packages/langchain-agents/tests/test_handler.py @@ -468,6 +468,13 @@ async def _bad_handler(args: Any) -> str: @pytest.mark.asyncio async def test_sync_handler_returns_string(self) -> None: + """Direct ``_build_agent_tools`` call (skips ``wrap_tool_handlers``). + + On the normal invoke/stream path, user handlers are wrapped async before + they reach here, so the production failure was primarily ``__handoff_*`` + tools (and native-graph handlers that also bypass wrapping). + """ + def sync_handler(args: dict[str, Any]) -> str: assert args == {"city": "Paris"} return "sunny" diff --git a/packages/langchain-agents/tests/test_native_graph.py b/packages/langchain-agents/tests/test_native_graph.py index 53457ddc..2deb0a1e 100644 --- a/packages/langchain-agents/tests/test_native_graph.py +++ b/packages/langchain-agents/tests/test_native_graph.py @@ -13,7 +13,11 @@ import pytest -from launchdarkly_ai_langchain_agents.native_graph import _extract_usage, to_lang_graph +from launchdarkly_ai_langchain_agents.native_graph import ( + _build_node_tools, + _extract_usage, + to_lang_graph, +) from launchdarkly_ai_server import GraphDefinition, GraphEdge, GraphNode # --------------------------------------------------------------------------- @@ -877,6 +881,80 @@ async def test_config_tools_creates_tool_node(self) -> None: ) +class TestBuildNodeToolsSyncHandlers: + """``_build_node_tools`` must accept sync handlers (native path skips wrap_tool_handlers).""" + + @pytest.mark.asyncio + async def test_sync_handler_returns_string(self) -> None: + captured: list[Any] = [] + + def _capture_tool(_name: Any, fn: Any = None, **_kw: Any) -> Any: + captured.append(fn) + return fn + + mock_lc_tools = MagicMock() + mock_lc_tools.tool = MagicMock(side_effect=_capture_tool) + + node = _to_graph_node( + { + "key": "root", + "config": { + "tools": {"weather": {"description": "d", "parameters": {}}}, + }, + "meta": {}, + "edges": [], + "is_terminal": True, + } + ) + + with patch( + "importlib.import_module", + side_effect=lambda n: ( + mock_lc_tools if n == "langchain_core.tools" else __import__(n) + ), + ): + _build_node_tools(node, {"weather": lambda a: "sunny"}) + + assert captured, "tool was not registered" + assert await captured[0](city="Paris") == "sunny" + + @pytest.mark.asyncio + async def test_async_handler_is_awaited(self) -> None: + captured: list[Any] = [] + + def _capture_tool(_name: Any, fn: Any = None, **_kw: Any) -> Any: + captured.append(fn) + return fn + + mock_lc_tools = MagicMock() + mock_lc_tools.tool = MagicMock(side_effect=_capture_tool) + + async def async_handler(args: dict[str, Any]) -> str: + return f"async:{args.get('q')}" + + node = _to_graph_node( + { + "key": "root", + "config": { + "tools": {"lookup": {"description": "d", "parameters": {}}}, + }, + "meta": {}, + "edges": [], + "is_terminal": True, + } + ) + + with patch( + "importlib.import_module", + side_effect=lambda n: ( + mock_lc_tools if n == "langchain_core.tools" else __import__(n) + ), + ): + _build_node_tools(node, {"lookup": async_handler}) + + assert await captured[0](q="hi") == "async:hi" + + # --------------------------------------------------------------------------- # ยง2.x.3 WorkflowState annotations resolve โ€” real StateGraph (no mock) # --------------------------------------------------------------------------- diff --git a/packages/openai-agents/README.md b/packages/openai-agents/README.md index 70fb27b1..e06a5873 100644 --- a/packages/openai-agents/README.md +++ b/packages/openai-agents/README.md @@ -34,6 +34,11 @@ async def main(): asyncio.run(main()) ``` +Tool handlers may be sync or async. Sync handlers run on the event-loop thread, so +blocking I/O stalls the agent. Keep graph `__handoff_*` handlers synchronous: routing +records the selected edge on the call itself, and moving them onto `asyncio.to_thread` +would break that. + ### Convenience wrapper ```python diff --git a/packages/openai-agents/src/launchdarkly_ai_openai_agents/native_graph.py b/packages/openai-agents/src/launchdarkly_ai_openai_agents/native_graph.py index a8b8a4a1..ac8b0a90 100644 --- a/packages/openai-agents/src/launchdarkly_ai_openai_agents/native_graph.py +++ b/packages/openai-agents/src/launchdarkly_ai_openai_agents/native_graph.py @@ -5,6 +5,7 @@ from __future__ import annotations +import inspect import re import time import types @@ -67,8 +68,12 @@ async def _execute(args: Any, _name: str = name) -> str: handler = tool_handlers.get(_name) if not handler or isinstance(handler, NativeTool): return "" - res = await handler(args) - return str(res) + # Handlers may be sync or async. Same rule as handler._build_agent_tools + # and tracking.wrap_tool_handlers โ€” awaiting a plain return raises. + result = handler(args) + if inspect.isawaitable(result): + result = await result + return str(result) t = tool_fn( name=name, diff --git a/packages/openai-agents/tests/test_native_graph.py b/packages/openai-agents/tests/test_native_graph.py index 2df4696f..5b87afa4 100644 --- a/packages/openai-agents/tests/test_native_graph.py +++ b/packages/openai-agents/tests/test_native_graph.py @@ -11,7 +11,10 @@ import pytest import launchdarkly_ai_openai_agents.native_graph as _openai_ng -from launchdarkly_ai_openai_agents.native_graph import to_openai_agents +from launchdarkly_ai_openai_agents.native_graph import ( + _build_node_tools, + to_openai_agents, +) from launchdarkly_ai_server import GraphDefinition, GraphEdge, GraphNode # --------------------------------------------------------------------------- @@ -761,3 +764,104 @@ async def _run_and_fire_hook(agent: Any, text: str, hooks: Any = None) -> Any: assert captured_hooks, "hooks were not passed to Runner.run" assert "$ld:ai:graph:handoff_success" in track_calls + + +class TestBuildNodeToolsSyncHandlers: + """``_build_node_tools`` must accept sync handlers (native path skips wrap_tool_handlers).""" + + @pytest.mark.asyncio + async def test_sync_handler_returns_string(self) -> None: + captured: list[Any] = [] + + agents_mock = MagicMock() + agents_mock.tool = MagicMock( + side_effect=lambda **kw: lambda fn: (captured.append(fn), fn)[1] + ) + + node = _to_graph_node( + { + "key": "root", + "config": { + "tools": {"weather": {"description": "d", "parameters": {}}}, + }, + "meta": {}, + "edges": [], + "is_terminal": True, + } + ) + + with patch( + "importlib.import_module", + side_effect=lambda n: agents_mock if n == "agents" else __import__(n), + ): + _build_node_tools(node, {"weather": lambda a: "sunny"}) + + assert captured, "tool was not registered" + assert await captured[0]({"city": "Paris"}) == "sunny" + + @pytest.mark.asyncio + async def test_async_handler_is_awaited(self) -> None: + captured: list[Any] = [] + + agents_mock = MagicMock() + agents_mock.tool = MagicMock( + side_effect=lambda **kw: lambda fn: (captured.append(fn), fn)[1] + ) + + async def async_handler(args: Any) -> str: + return f"async:{args.get('q')}" + + node = _to_graph_node( + { + "key": "root", + "config": { + "tools": {"lookup": {"description": "d", "parameters": {}}}, + }, + "meta": {}, + "edges": [], + "is_terminal": True, + } + ) + + with patch( + "importlib.import_module", + side_effect=lambda n: agents_mock if n == "agents" else __import__(n), + ): + _build_node_tools(node, {"lookup": async_handler}) + + assert await captured[0]({"q": "hi"}) == "async:hi" + + @pytest.mark.asyncio + async def test_sync_handler_returning_awaitable_is_awaited(self) -> None: + captured: list[Any] = [] + + agents_mock = MagicMock() + agents_mock.tool = MagicMock( + side_effect=lambda **kw: lambda fn: (captured.append(fn), fn)[1] + ) + + async def _inner() -> str: + return "done" + + def sync_wrapper(_args: Any) -> Any: + return _inner() + + node = _to_graph_node( + { + "key": "root", + "config": { + "tools": {"my-tool": {"description": "d", "parameters": {}}}, + }, + "meta": {}, + "edges": [], + "is_terminal": True, + } + ) + + with patch( + "importlib.import_module", + side_effect=lambda n: agents_mock if n == "agents" else __import__(n), + ): + _build_node_tools(node, {"my-tool": sync_wrapper}) + + assert await captured[0]({}) == "done"