From e41fa3a962b7b68461db178b19ded85e1b0d8516 Mon Sep 17 00:00:00 2001 From: Chris Hagglund Date: Fri, 18 Sep 2026 12:18:09 -0600 Subject: [PATCH] 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) --- .../ai/agents/runtime/_worker_entries.py | 62 +++++++++++++++---- tests/unit/ai/test_worker_entries.py | 43 +++++++++++++ 2 files changed, 94 insertions(+), 11 deletions(-) diff --git a/src/conductor/ai/agents/runtime/_worker_entries.py b/src/conductor/ai/agents/runtime/_worker_entries.py index 044dc32a..33d05573 100644 --- a/src/conductor/ai/agents/runtime/_worker_entries.py +++ b/src/conductor/ai/agents/runtime/_worker_entries.py @@ -67,6 +67,23 @@ def _walk_qualname(module_obj, qualname: str): # .coroutine; our Guardrail / ToolDef → .func. _CONTAINER_ATTRS = ("func", "coroutine") +def _is_importable_function(fn: Callable) -> bool: + """Whether *fn* can be referenced by module + qualified name. + + Mirrors the condition :meth:`FunctionRef.of` enforces (it raises a + per-case ``SpawnSafetyError`` rather than returning a bool, so the two + can't simply share a body). Factored out so the closure walk can *prefer* + a candidate that will actually survive the trip to a spawn child. + """ + qualname = getattr(fn, "__qualname__", None) + return bool( + getattr(fn, "__module__", None) + and qualname + and "" not in qualname + and "" not in qualname + ) + + def _extract_from_closure(func: Callable) -> Optional[Callable]: """Extract the original user function from a closure's cell variables. @@ -74,28 +91,45 @@ def _extract_from_closure(func: Callable) -> Optional[Callable]: - In turn shared by :mod:`conductor.ai.agents.frameworks.serializer`'s discovery and :class:`FunctionRef`'s parent-verification / child-reconstruction — one implementation, can't drift apart. + - Preference-ordered, not first-match: one closure can hold several plain + functions, with the framework's own nested helpers sitting alongside the + user's function. An importable candidate always wins over a nested one, + because importability is exactly what the caller needs + (``FunctionRef.of``) and is a property of the candidate itself rather + than a guess from its parameter names. openai-agents 0.22.3 is why: + it added a nested ``_prepare_arguments(input, tool_name)`` beside + ``the_func`` in the same closure, and ``input`` is not ``ctx``, so + first-match started returning the framework's wrapper. + - The nested-candidate fallback is kept for closures that genuinely hold + no importable function: callers get the same value (and the same + actionable ``SpawnSafetyError`` from ``FunctionRef.of``) as before. """ closure = getattr(func, "__closure__", None) if not closure: return None + fallback = None for cell in closure: try: val = cell.cell_contents except ValueError: continue - if inspect.isfunction(val): - # Skip internal wrappers that take (ctx, input) or (context, ...) - try: - sig = inspect.signature(val) - param_names = list(sig.parameters.keys()) - # Internal wrappers typically start with ctx/context as first param - if param_names and param_names[0] in ("ctx", "context"): - continue - return val - except (ValueError, TypeError): + if not inspect.isfunction(val): + continue + # Skip internal wrappers that take (ctx, input) or (context, ...) + try: + sig = inspect.signature(val) + param_names = list(sig.parameters.keys()) + # Internal wrappers typically start with ctx/context as first param + if param_names and param_names[0] in ("ctx", "context"): continue - return None + except (ValueError, TypeError): + continue + if _is_importable_function(val): + return val + if fallback is None: + fallback = val + return fallback # 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) shape-based walk degrades to "not found" (the pre-existing, actionable ``SpawnSafetyError``) instead of breaking outright if openai-agents restructures its internals. + - "Degrades to not found" only holds while the walk can't mistake a + framework helper for the user's function. openai-agents 0.22.3 showed + the failure mode — a new nested helper in the same closure was returned + *instead of* the user's function, which is worse than None (the + serializer would have registered a worker around the wrapper). See + :func:`_extract_from_closure` for the preference rule that fixes it. """ if max_depth <= 0: return None diff --git a/tests/unit/ai/test_worker_entries.py b/tests/unit/ai/test_worker_entries.py index 45124043..13703d60 100644 --- a/tests/unit/ai/test_worker_entries.py +++ b/tests/unit/ai/test_worker_entries.py @@ -225,6 +225,49 @@ def test_cross_process_spawn_roundtrip(self): assert p.exitcode == 0 +class TestClosureCandidatePreference: + """_extract_from_closure prefers an importable candidate over a nested one. + + Always-runs counterpart to TestFunctionRefDeepExtract above, which is + importorskip'd on the openai-agents extra and pins whatever internals the + installed version happens to have. openai-agents 0.22.3 broke the old + first-match walk by adding a nested ``_prepare_arguments(input, tool_name)`` + beside the user's function in one closure — `input` is not `ctx`, so the + wrapper was returned instead. These pin the rule, not a library's shape. + """ + + def test_prefers_importable_over_nested_helper(self): + target = helpers.plain_sample # local binding -> a real closure cell + + def _prepare_arguments(input: str, tool_name: str) -> dict: + return {"input": input, "tool": tool_name} + + def impl(): + return _prepare_arguments, target + + # co_freevars is alphabetical, so the nested helper is seen first — + # the same adverse ordering openai-agents 0.22.3 produces. + assert impl.__code__.co_freevars == ("_prepare_arguments", "target") + found = we._extract_from_closure(impl) + assert found is helpers.plain_sample + assert FunctionRef.of(found).resolve() is helpers.plain_sample + + def test_falls_back_to_nested_when_nothing_importable(self): + # Closures holding no importable function keep the old behaviour: the + # nested candidate is still returned, so callers get the same + # actionable SpawnSafetyError from FunctionRef.of as before. + def nested(city: str) -> str: + return city + + def impl(): + return nested + + found = we._extract_from_closure(impl) + assert found is nested + with pytest.raises(SpawnSafetyError, match="defined inside a function"): + FunctionRef.of(found) + + # ── Guardrail spawn transport ─────────────────────────────────────────────