Skip to content

Commit 0e08ab5

Browse files
fix(agents): prefer the importable function in a tool closure
openai-agents auto-updated to 0.22.3 (the dep is unpinned, >=0.12.2), whose tool closure now holds two candidates: a nested _prepare_arguments(input, tool_name) beside the user's function. _extract_from_closure returned the first candidate whose leading parameter was not ctx/context, so it began returning the framework's wrapper: FunctionRef.of rejected it as unimportable (5 unit failures), and the serializer would have registered a worker around it -- worse than finding nothing. main would fail identically on a re-run. Selection is now preference-ordered: importable by module + qualname wins. That is exactly what FunctionRef.of requires, and it is a property of the candidate rather than a guess from its parameter names. Closures holding nothing importable fall back to the nested candidate, so those callers get the same value and the same actionable SpawnSafetyError as before. Adds TestClosureCandidatePreference, which builds the adverse closure directly and runs without the extra installed -- the existing deep-extract tests are importorskip'd and pin whatever internals happen to be installed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 99ad862 commit 0e08ab5

2 files changed

Lines changed: 94 additions & 11 deletions

File tree

‎src/conductor/ai/agents/runtime/_worker_entries.py‎

Lines changed: 51 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -67,35 +67,69 @@ def _walk_qualname(module_obj, qualname: str):
6767
# .coroutine; our Guardrail / ToolDef → .func.
6868
_CONTAINER_ATTRS = ("func", "coroutine")
6969

70+
def _is_importable_function(fn: Callable) -> bool:
71+
"""Whether *fn* can be referenced by module + qualified name.
72+
73+
Mirrors the condition :meth:`FunctionRef.of` enforces (it raises a
74+
per-case ``SpawnSafetyError`` rather than returning a bool, so the two
75+
can't simply share a body). Factored out so the closure walk can *prefer*
76+
a candidate that will actually survive the trip to a spawn child.
77+
"""
78+
qualname = getattr(fn, "__qualname__", None)
79+
return bool(
80+
getattr(fn, "__module__", None)
81+
and qualname
82+
and "<locals>" not in qualname
83+
and "<lambda>" not in qualname
84+
)
85+
86+
7087
def _extract_from_closure(func: Callable) -> Optional[Callable]:
7188
"""Extract the original user function from a closure's cell variables.
7289
7390
- Shared by :func:`_find_embedded_function` below.
7491
- In turn shared by :mod:`conductor.ai.agents.frameworks.serializer`'s
7592
discovery and :class:`FunctionRef`'s parent-verification /
7693
child-reconstruction — one implementation, can't drift apart.
94+
- Preference-ordered, not first-match: one closure can hold several plain
95+
functions, with the framework's own nested helpers sitting alongside the
96+
user's function. An importable candidate always wins over a nested one,
97+
because importability is exactly what the caller needs
98+
(``FunctionRef.of``) and is a property of the candidate itself rather
99+
than a guess from its parameter names. openai-agents 0.22.3 is why:
100+
it added a nested ``_prepare_arguments(input, tool_name)`` beside
101+
``the_func`` in the same closure, and ``input`` is not ``ctx``, so
102+
first-match started returning the framework's wrapper.
103+
- The nested-candidate fallback is kept for closures that genuinely hold
104+
no importable function: callers get the same value (and the same
105+
actionable ``SpawnSafetyError`` from ``FunctionRef.of``) as before.
77106
"""
78107
closure = getattr(func, "__closure__", None)
79108
if not closure:
80109
return None
81110

111+
fallback = None
82112
for cell in closure:
83113
try:
84114
val = cell.cell_contents
85115
except ValueError:
86116
continue
87-
if inspect.isfunction(val):
88-
# Skip internal wrappers that take (ctx, input) or (context, ...)
89-
try:
90-
sig = inspect.signature(val)
91-
param_names = list(sig.parameters.keys())
92-
# Internal wrappers typically start with ctx/context as first param
93-
if param_names and param_names[0] in ("ctx", "context"):
94-
continue
95-
return val
96-
except (ValueError, TypeError):
117+
if not inspect.isfunction(val):
118+
continue
119+
# Skip internal wrappers that take (ctx, input) or (context, ...)
120+
try:
121+
sig = inspect.signature(val)
122+
param_names = list(sig.parameters.keys())
123+
# Internal wrappers typically start with ctx/context as first param
124+
if param_names and param_names[0] in ("ctx", "context"):
97125
continue
98-
return None
126+
except (ValueError, TypeError):
127+
continue
128+
if _is_importable_function(val):
129+
return val
130+
if fallback is None:
131+
fallback = val
132+
return fallback
99133

100134

101135
# How many attribute-nesting levels _find_embedded_function will descend.
@@ -118,6 +152,12 @@ def _find_embedded_function(obj: Any, max_depth: int = _DEEP_EXTRACT_MAX_DEPTH)
118152
shape-based walk degrades to "not found" (the pre-existing, actionable
119153
``SpawnSafetyError``) instead of breaking outright if openai-agents
120154
restructures its internals.
155+
- "Degrades to not found" only holds while the walk can't mistake a
156+
framework helper for the user's function. openai-agents 0.22.3 showed
157+
the failure mode — a new nested helper in the same closure was returned
158+
*instead of* the user's function, which is worse than None (the
159+
serializer would have registered a worker around the wrapper). See
160+
:func:`_extract_from_closure` for the preference rule that fixes it.
121161
"""
122162
if max_depth <= 0:
123163
return None

‎tests/unit/ai/test_worker_entries.py‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,49 @@ def test_cross_process_spawn_roundtrip(self):
225225
assert p.exitcode == 0
226226

227227

228+
class TestClosureCandidatePreference:
229+
"""_extract_from_closure prefers an importable candidate over a nested one.
230+
231+
Always-runs counterpart to TestFunctionRefDeepExtract above, which is
232+
importorskip'd on the openai-agents extra and pins whatever internals the
233+
installed version happens to have. openai-agents 0.22.3 broke the old
234+
first-match walk by adding a nested ``_prepare_arguments(input, tool_name)``
235+
beside the user's function in one closure — `input` is not `ctx`, so the
236+
wrapper was returned instead. These pin the rule, not a library's shape.
237+
"""
238+
239+
def test_prefers_importable_over_nested_helper(self):
240+
target = helpers.plain_sample # local binding -> a real closure cell
241+
242+
def _prepare_arguments(input: str, tool_name: str) -> dict:
243+
return {"input": input, "tool": tool_name}
244+
245+
def impl():
246+
return _prepare_arguments, target
247+
248+
# co_freevars is alphabetical, so the nested helper is seen first —
249+
# the same adverse ordering openai-agents 0.22.3 produces.
250+
assert impl.__code__.co_freevars == ("_prepare_arguments", "target")
251+
found = we._extract_from_closure(impl)
252+
assert found is helpers.plain_sample
253+
assert FunctionRef.of(found).resolve() is helpers.plain_sample
254+
255+
def test_falls_back_to_nested_when_nothing_importable(self):
256+
# Closures holding no importable function keep the old behaviour: the
257+
# nested candidate is still returned, so callers get the same
258+
# actionable SpawnSafetyError from FunctionRef.of as before.
259+
def nested(city: str) -> str:
260+
return city
261+
262+
def impl():
263+
return nested
264+
265+
found = we._extract_from_closure(impl)
266+
assert found is nested
267+
with pytest.raises(SpawnSafetyError, match="defined inside a function"):
268+
FunctionRef.of(found)
269+
270+
228271
# ── Guardrail spawn transport ─────────────────────────────────────────────
229272

230273

0 commit comments

Comments
 (0)