Skip to content

Commit 373f868

Browse files
Kowserclaude
andcommitted
Fix spawn-safety gaps surfaced by live e2e validation
Three findings from running the 20 previously pickle-failing e2e tests against a live server under default spawn: - RegexGuardrail / LLMGuardrail built their check functions as __init__ closures, making every regex/LLM guardrail spawn-unsafe by construction (SpawnSafetyError: "Can't pickle local object RegexGuardrail.__init__.<locals>._check"). Both now bind methods — bound methods pickle with their instance, and all attrs are plain data / re.Pattern. Verified: suite8 tool_output_regex_retry green under spawn against a live server. - _worker_entries.py used `from __future__ import annotations`, so entry __call__ annotations were strings at runtime. AsyncTaskRunner reads parameter types from inspect.signature(execute_function) and feeds them to isinstance-based input conversion — strings broke every async entry ("isinstance() arg 2 must be a type"). Future-import removed (with a warning note) and StopWhenEntry's params annotated with real types, matching the explicit __annotations__ dict the old closure carried. Verified: suite13 all_callbacks + suite14 stateful stop_when green under spawn against a live server. - e2e suite14 defined its stop_when predicate inside the test method — moved to module level per the spawn worker contract (nested defs cannot cross the process boundary; the probe rejects them by design). Also: always-spawn alignment — removed the fork-premised probe test and fork wording in transport docstrings (CONDUCTOR_MP_START_METHOD was removed repo-wide in 9a989af; direct transport exists for picklable instances/bound methods, not as a fork fallback). tests/unit: 2518 passed, 1 skipped. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 1b03c81 commit 373f868

4 files changed

Lines changed: 40 additions & 38 deletions

File tree

e2e/test_suite14_stateful_domain.py

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,13 @@ def echo_tool(message: str) -> str:
6565
return f"ECHO:{message}"
6666

6767

68+
def should_stop_on_echo(context, **kwargs):
69+
"""Module-level stop_when predicate — spawn workers require importable
70+
callables (nested defs can't cross the process boundary)."""
71+
result = context.get("result", "")
72+
return "ECHO:" in result
73+
74+
6875
@tool(stateful=True)
6976
def stateful_echo(message: str) -> str:
7077
"""A stateful tool that echoes with a prefix."""
@@ -250,10 +257,6 @@ def test_stateful_stop_when_completes(self, fresh_runtime, model):
250257
The stop_when function checks for a marker in the output.
251258
Validates the stop_when worker task is COMPLETED with the correct domain.
252259
"""
253-
def _should_stop(context, **kwargs):
254-
result = context.get("result", "")
255-
return "ECHO:" in result
256-
257260
agent = Agent(
258261
name="e2e_s14_stateful_stop",
259262
model=model,
@@ -264,7 +267,7 @@ def _should_stop(context, **kwargs):
264267
"Then report the tool's response."
265268
),
266269
tools=[echo_tool],
267-
stop_when=_should_stop,
270+
stop_when=should_stop_on_echo,
268271
)
269272
result = fresh_runtime.run(agent, "Call echo_tool with stop_test", timeout=TIMEOUT)
270273
diag = _run_diagnostic(result)

src/conductor/ai/agents/guardrail.py

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -252,25 +252,28 @@ def __init__(
252252
self._mode = mode
253253
self._custom_message = message
254254

255-
def _check(content: str) -> GuardrailResult:
256-
matched = any(p.search(content) for p in self._patterns)
257-
258-
if mode == "block" and matched:
259-
msg = message or "Content matched a blocked pattern."
260-
return GuardrailResult(passed=False, message=msg)
261-
elif mode == "allow" and not matched:
262-
msg = message or "Content did not match any allowed pattern."
263-
return GuardrailResult(passed=False, message=msg)
264-
return GuardrailResult(passed=True)
265-
255+
# Bound method, not a nested closure: bound methods pickle with their
256+
# instance (all attrs here are plain data / re.Pattern), so the
257+
# guardrail worker survives 'spawn' pickling (idea-5 spawn safety).
266258
super().__init__(
267-
func=_check,
259+
func=self._check,
268260
position=position,
269261
on_fail=on_fail,
270262
name=name or "regex_guardrail",
271263
max_retries=max_retries,
272264
)
273265

266+
def _check(self, content: str) -> GuardrailResult:
267+
matched = any(p.search(content) for p in self._patterns)
268+
269+
if self._mode == "block" and matched:
270+
msg = self._custom_message or "Content matched a blocked pattern."
271+
return GuardrailResult(passed=False, message=msg)
272+
elif self._mode == "allow" and not matched:
273+
msg = self._custom_message or "Content did not match any allowed pattern."
274+
return GuardrailResult(passed=False, message=msg)
275+
return GuardrailResult(passed=True)
276+
274277
def __repr__(self) -> str:
275278
return (
276279
f"RegexGuardrail(name={self.name!r}, mode={self._mode!r}, "
@@ -319,11 +322,10 @@ def __init__(
319322
self._policy = policy
320323
self._max_tokens = max_tokens
321324

322-
def _check(content: str) -> GuardrailResult:
323-
return self._evaluate(content)
324-
325+
# Bound method, not a nested closure — spawn-picklable with the
326+
# instance (idea-5 spawn safety).
325327
super().__init__(
326-
func=_check,
328+
func=self._evaluate,
327329
position=position,
328330
on_fail=on_fail,
329331
name=name or "llm_guardrail",

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

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,13 @@
2424
worker group as each group's workers become spawn-safe.
2525
2626
Design: idea-5 ``design.md`` in the combine-agentspan analysis workspace.
27-
"""
2827
29-
from __future__ import annotations
28+
NOTE: deliberately NO ``from __future__ import annotations`` here — the task
29+
runners read parameter types from ``inspect.signature(execute_function)``
30+
(the class ``__call__``'s annotations for entry instances) and pass them to
31+
``isinstance``-based input conversion; string annotations would break that
32+
(``TypeError: isinstance() arg 2 must be a type``).
33+
"""
3034

3135
import importlib
3236
import inspect
@@ -173,9 +177,9 @@ def __init__(self, tool_name, fn_ref=None, fn_direct=None, guardrails=None,
173177
def for_callable(cls, fn, tool_name, guardrails=None, credential_names=None):
174178
"""Build an entry for *fn*, preferring by-reference transport.
175179
176-
Falls back to direct transport for callable instances (picklable by
177-
value) and for closures — the latter only work under ``fork``; the
178-
registration probe polices that under ``spawn``.
180+
Falls back to direct transport for picklable callables (instances,
181+
bound methods of picklable objects); unpicklable closures are caught
182+
by the registration probe with an actionable error.
179183
"""
180184
framework = bool(getattr(fn, "_agentspan_framework_callable", False))
181185
try:
@@ -218,8 +222,9 @@ def __call__(self, task):
218222
def wrap_callable(fn):
219223
"""Best transport for a user callable: FunctionRef when resolvable, raw otherwise.
220224
221-
Raw transport only survives ``fork`` (or picklable instances); the
222-
registration probe polices that under ``spawn``.
225+
Raw transport works for picklable callables (instances with plain-data
226+
attrs, bound methods of picklable objects); anything else is caught by the
227+
registration probe with an actionable error.
223228
"""
224229
if fn is None:
225230
return None
@@ -354,7 +359,8 @@ class StopWhenEntry:
354359
def __init__(self, stop_when_fn):
355360
self.fn_t = wrap_callable(stop_when_fn)
356361

357-
async def __call__(self, result="", iteration: int = 0, messages=None) -> object:
362+
async def __call__(self, result: object = "", iteration: int = 0,
363+
messages: object = None) -> object:
358364
from conductor.ai.agents.runtime.runtime import _call_user_fn, _resolve_loop_iteration
359365

360366
iteration = _resolve_loop_iteration(iteration)

tests/unit/ai/test_worker_entries.py

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -246,15 +246,6 @@ def test_lambda_callback_fails_fast_at_registration(self, force_spawn_probe):
246246
with pytest.raises(SpawnSafetyError, match="not spawn-safe"):
247247
probe_spawn_safety(entry, "t_before_model", group="system")
248248

249-
def test_lambda_tolerated_under_fork(self, monkeypatch):
250-
from conductor.ai.agents.runtime._worker_entries import CallbackEntry
251-
252-
monkeypatch.setattr(
253-
we.multiprocessing, "get_start_method", lambda allow_none=True: "fork"
254-
)
255-
entry = CallbackEntry("before_model", [], lambda **kw: {}, "t_before_model")
256-
probe_spawn_safety(entry, "t_before_model", group="system") # no raise
257-
258249
def test_module_level_callback_passes_probe(self, force_spawn_probe):
259250
from conductor.ai.agents.runtime._worker_entries import CallbackEntry
260251

0 commit comments

Comments
 (0)