From e2d1ba319292545ca30668f014fa083c7c156827 Mon Sep 17 00:00:00 2001 From: Evan Mattson <35585003+moonbox3@users.noreply.github.com> Date: Mon, 27 Oct 2025 14:31:58 +0900 Subject: [PATCH] Python: reject @executor on staticmethod/classmethod with clear error message (#1719) * reject executor on static method w clear error * Simplify * Cleanup --- .../_workflows/_function_executor.py | 60 +++++++++++++++---- .../tests/workflow/test_function_executor.py | 56 +++++++++++++++++ 2 files changed, 104 insertions(+), 12 deletions(-) diff --git a/python/packages/core/agent_framework/_workflows/_function_executor.py b/python/packages/core/agent_framework/_workflows/_function_executor.py index 223dd9c05b..75d4836124 100644 --- a/python/packages/core/agent_framework/_workflows/_function_executor.py +++ b/python/packages/core/agent_framework/_workflows/_function_executor.py @@ -3,11 +3,16 @@ """Function-based Executor and decorator utilities. This module provides: -- FunctionExecutor: an Executor subclass that wraps a user-defined function +- FunctionExecutor: an Executor subclass that wraps a standalone user-defined function with signature (message) or (message, ctx: WorkflowContext[T]). Both sync and async functions are supported. Synchronous functions are executed in a thread pool using asyncio.to_thread() to avoid blocking the event loop. -- executor decorator: converts such a function into a ready-to-use Executor instance +- executor decorator: converts a standalone module-level function into a ready-to-use Executor instance with proper type validation and handler registration. + +Design Pattern: + - Use @executor for standalone module-level or local functions + - Use Executor subclass with @handler for class-based executors with state/dependencies + - Do NOT use @executor with @staticmethod or @classmethod """ import asyncio @@ -49,12 +54,26 @@ class FunctionExecutor(Executor): Args: func: The function to wrap as an executor (can be sync or async) id: Optional executor ID. If None, uses the function name. + + Raises: + ValueError: If func is a staticmethod or classmethod (use @handler on instance methods instead) """ + # Detect misuse of @executor with staticmethod/classmethod + if isinstance(func, (staticmethod, classmethod)): + descriptor_type = "staticmethod" if isinstance(func, staticmethod) else "classmethod" + raise ValueError( + f"The @executor decorator cannot be used with @{descriptor_type}. " + f"Use the @executor decorator on standalone module-level functions, " + f"or create an Executor subclass and use @handler on instance methods instead." + ) + # Validate function signature and extract types message_type, ctx_annotation, output_types, workflow_output_types = self._validate_function(func) # Determine if function has WorkflowContext parameter has_context = ctx_annotation is not None + + # Check if function is async is_async = asyncio.iscoroutinefunction(func) # Initialize parent WITHOUT calling _discover_handlers yet @@ -123,36 +142,53 @@ def executor(*, id: str | None = None) -> Callable[[Callable[..., Any]], Functio def executor( func: Callable[..., Any] | None = None, *, id: str | None = None ) -> Callable[[Callable[..., Any]], FunctionExecutor] | FunctionExecutor: - """Decorator that converts a function into a FunctionExecutor instance. + """Decorator that converts a standalone function into a FunctionExecutor instance. + + The @executor decorator is designed for **standalone module-level functions only**. + For class-based executors, use the Executor base class with @handler on instance methods. Supports both synchronous and asynchronous functions. Synchronous functions are executed in a thread pool to avoid blocking the event loop. + Important: + - Use @executor for standalone functions (module-level or local functions) + - Do NOT use @executor with @staticmethod or @classmethod + - For class-based executors, subclass Executor and use @handler on instance methods + Usage: .. code-block:: python - # With arguments (async function): + # Standalone async function (RECOMMENDED): @executor(id="upper_case") async def to_upper(text: str, ctx: WorkflowContext[str]): await ctx.send_message(text.upper()) - # Without parentheses (sync function - runs in thread pool): + # Standalone sync function (runs in thread pool): @executor def process_data(data: str): - # Process data without sending messages return data.upper() - # Sync function with context (runs in thread pool): - @executor - def sync_with_context(data: int, ctx: WorkflowContext[int]): - # Note: sync functions can still use context - return data * 2 + # For class-based executors, use @handler instead: + class MyExecutor(Executor): + def __init__(self): + super().__init__(id="my_executor") + + @handler + async def process(self, data: str, ctx: WorkflowContext[str]): + await ctx.send_message(data.upper()) + + Args: + func: The function to decorate (when used without parentheses) + id: Optional custom ID for the executor. If None, uses the function name. Returns: - An Executor instance that can be wired into a Workflow. + A FunctionExecutor instance that can be wired into a Workflow. + + Raises: + ValueError: If used with @staticmethod or @classmethod (unsupported pattern) """ def wrapper(func: Callable[..., Any]) -> FunctionExecutor: diff --git a/python/packages/core/tests/workflow/test_function_executor.py b/python/packages/core/tests/workflow/test_function_executor.py index 229e399b09..4498ed712c 100644 --- a/python/packages/core/tests/workflow/test_function_executor.py +++ b/python/packages/core/tests/workflow/test_function_executor.py @@ -478,3 +478,59 @@ class TestFunctionExecutor: # The actual thread execution test would require a full workflow setup, # but the important thing is that asyncio.to_thread is used in the wrapper + + def test_executor_rejects_staticmethod(self): + """Test that @executor decorator properly rejects @staticmethod with clear error.""" + with pytest.raises(ValueError) as exc_info: + + class Example: + @executor + @staticmethod + async def bad_handler(data: str) -> str: + return data.upper() + + assert "cannot be used with @staticmethod" in str(exc_info.value) + assert "@handler on instance methods" in str(exc_info.value) + + def test_executor_rejects_classmethod(self): + """Test that @executor decorator properly rejects @classmethod with clear error.""" + with pytest.raises(ValueError) as exc_info: + + class Example: + @executor + @classmethod + async def bad_handler(cls, data: str) -> str: + return data.upper() + + assert "cannot be used with @classmethod" in str(exc_info.value) + assert "@handler on instance methods" in str(exc_info.value) + + async def test_async_staticmethod_detection_behavior(self): + """Document the behavior of asyncio.iscoroutinefunction with staticmethod descriptors. + + This test explains why the unwrapping is necessary when decorators are stacked. + """ + import asyncio + + # When @staticmethod is applied, it creates a descriptor + async def my_async_func(): + await asyncio.sleep(0.001) + return "done" + + # Apply staticmethod (what happens with innermost decorator) + static_wrapped = staticmethod(my_async_func) + + # Direct check on descriptor object fails (this is the bug) + assert not asyncio.iscoroutinefunction(static_wrapped) + assert isinstance(static_wrapped, staticmethod) + + # But unwrapping __func__ reveals the async function + unwrapped = static_wrapped.__func__ + assert asyncio.iscoroutinefunction(unwrapped) + + # When accessed via class attribute, Python's descriptor protocol + # automatically unwraps it, so it works: + class C: + async_static = static_wrapped + + assert asyncio.iscoroutinefunction(C.async_static) # Works via descriptor protocol