Skip to content

Commit 7d340ac

Browse files
CCOR-13508 - fix(agents): prefer the importable function in a tool closure (#510)
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 3a0516a commit 7d340ac

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)