Breaking: refuse Google ADK method references whose class was rebound

python/google_adk/src/temporalio/google_adk/workflow.py

@@ -278,8 +278,8 @@ def _is_method(fn: Callable) -> bool:
 
     A module-level or nested function, or a staticmethod, is called with every
     parameter. A function defined in a class body is found in that class to
-    tell a method from a staticmethod; a class that can't be found raises
-    ``ValueError``.
+    tell a method from a staticmethod; a class that can't be found, or whose
+    attribute doesn't wrap the same function as ``fn``, raises ``ValueError``.
     """
     qualname = fn.__qualname__.split(".")
     if len(qualname) < 2 or qualname[-2] == "<locals>":
@@ -305,7 +305,22 @@ def _is_method(fn: Callable) -> bool:
             " (wrap decorators with functools.wraps), pass a bound method for a"
             " method, or rename a staticmethod's parameter."
         )
-    return not isinstance(attr, staticmethod)
+    static = isinstance(attr, staticmethod)
+    # Compare the innermost functions: a decorator applied to Class.method
+    # wraps the attribute's function, and a wrapt-style one stores a wrapper in
+    # the class that class access wraps again.
+    if inspect.unwrap(fn) is not inspect.unwrap(attr.__func__ if static else attr):
+        # The class's name or attribute was rebound after fn was taken from it,
+        # for example by reloading its module, so the attribute's binding isn't
+        # fn's.
+        raise ValueError(
+            f"Cannot tell whether '{fn.__qualname__}' is a method or a"
+            f" staticmethod: '{'.'.join(qualname)}' reachable from the"
+            " function's globals is another object. Import the class again"
+            " if its module was reloaded, pass a bound method for a method, or"
+            " rename a staticmethod's parameter."
+        )
+    return not static
 
 
 def _tool_signature(
@@ -420,8 +435,9 @@ def activity_as_tool(activity_def: Callable, **kwargs: Any) -> Callable:
     staticmethod, classmethod or ``functools.partialmethod``, can't be read, or
     takes keyword-only arguments or ``**kwargs`` raises ``ValueError``. So does
     a method taking ``self`` whose class can't be reached from the function's
-    globals, such as a class defined in a function: its ``self`` could be a
-    staticmethod's input.
+    globals, such as a class defined in a function, or whose class's name or
+    attribute now holds another object: its ``self`` could be a staticmethod's
+    input.
 
     Outside a Workflow, as in local ADK runs, the tool calls ``activity_def``
     itself. A method passed as ``Class.method`` and a class passed as itself

python/google_adk/tests/helpers/decorators.py

@@ -2,6 +2,7 @@
 
 import functools
 import inspect
+import types
 from collections.abc import Awaitable, Callable
 from typing import Any
 
@@ -26,3 +27,18 @@ def unwrapped(
     delattr(wrapper, "__wrapped__")
     setattr(wrapper, "__signature__", inspect.signature(fn))
     return wrapper
+
+
+class WrapOnAccess:
+    """Stays in the class; each access returns a new ``functools.wraps`` wrapper, as wrapt does."""
+
+    __wrapped__: Callable[..., Awaitable[Any]]
+
+    def __init__(self, fn: Callable[..., Awaitable[Any]]) -> None:
+        self.__wrapped__ = fn
+
+    def __get__(
+        self, obj: object, cls: type | None = None
+    ) -> Callable[..., Awaitable[Any]]:
+        fn = self.__wrapped__
+        return passthrough(fn if obj is None else types.MethodType(fn, obj))

python/google_adk/tests/test_google_adk_agents.py

@@ -14,6 +14,7 @@
 
 """Integration tests for ADK Temporal support."""
 
+import asyncio
 import functools
 import inspect
 import json
@@ -69,7 +70,7 @@
 )
 from temporalio.worker import Worker
 from temporalio.workflow import ActivityConfig
-from tests.helpers.decorators import passthrough, unwrapped
+from tests.helpers.decorators import WrapOnAccess, passthrough, unwrapped
 from tests.helpers.opentelemetry import dump_spans
 from tests.helpers.postponed_annotations import PostponedHolder
 
@@ -1004,6 +1005,12 @@ async def count_decorated(self, passengers: int) -> str:
         """Method wrapped by a decorator from another module."""
         return f"{self.prefix}:{passengers}"
 
+    @WrapOnAccess
+    @activity.defn
+    async def count_wrapped_on_access(self, passengers: int) -> str:
+        """Method whose decorator wraps it again on each access."""
+        return f"{self.prefix}:{passengers}"
+
     class __Private:
         """Its attribute in the class is mangled: ``_ComplexActivityMethodHolder__Private``."""
 
@@ -1679,14 +1686,31 @@ async def nested_echo(self) -> str:  # type: ignore[no-untyped-def] # pyright: i
         (ComplexActivityMethodHolder.private_count, ["passengers"]),
         (PostponedHolder.track, ["parcel_id"]),
         (nested_echo, ["self"]),
+        # A decorator applied to Class.method wraps the class's own function.
+        (passthrough(ComplexActivityMethodHolder.echo_static), ["self"]),
+        (passthrough(ComplexActivityMethodHolder.count_passengers), ["passengers"]),
+        (passthrough(ComplexActivityMethodHolder.echo_decorated), ["self"]),
+        (ComplexActivityMethodHolder.count_wrapped_on_access, ["passengers"]),
     ]:
         tool = temporalio.google_adk.workflow.activity_as_tool(
             fn, start_to_close_timeout=timedelta(seconds=30)
         )
         assert list(inspect.signature(tool).parameters) == params, fn.__qualname__
 
+    wrapped_static_tool = temporalio.google_adk.workflow.activity_as_tool(
+        passthrough(ComplexActivityMethodHolder.echo_static),
+        start_to_close_timeout=timedelta(seconds=30),
+    )
+    declaration = FunctionTool(wrapped_static_tool)._get_declaration()
+    assert declaration is not None
+    assert declaration.parameters_json_schema is not None
+    assert list(declaration.parameters_json_schema["properties"]) == ["self"]
+    assert asyncio.run(wrapped_static_tool(self="model-self")) == "static:model-self"
+
 
-def test_activity_as_tool_rejects_method_of_unreachable_class() -> None:
+def test_activity_as_tool_rejects_method_of_unreachable_class(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
     """A method whose class can't be found can't be told from a staticmethod."""
 
     class LocalHolder:
@@ -1706,6 +1730,40 @@ async def method(self, trip: str) -> str:
             UnwrappedHolder.echo, start_to_close_timeout=timedelta(seconds=30)
         )
 
+    # A reference kept after its class's name is rebound: the class now found
+    # holds another object under that name, so the reference is refused rather
+    # than given that object's binding.
+    echo_static = ComplexActivityMethodHolder.echo_static
+    count_passengers = ComplexActivityMethodHolder.count_passengers
+    monkeypatch.setitem(globals(), "ComplexActivityMethodHolder", ReboundMethodHolder)
+    with pytest.raises(ValueError, match="is another object"):
+        temporalio.google_adk.workflow.activity_as_tool(
+            echo_static, start_to_close_timeout=timedelta(seconds=30)
+        )
+    with pytest.raises(ValueError, match="is another object"):
+        temporalio.google_adk.workflow.activity_as_tool(
+            count_passengers, start_to_close_timeout=timedelta(seconds=30)
+        )
+    # Wrapping a kept reference doesn't make it the rebound attribute.
+    for fn in (echo_static, count_passengers):
+        with pytest.raises(ValueError, match="is another object"):
+            temporalio.google_adk.workflow.activity_as_tool(
+                passthrough(fn), start_to_close_timeout=timedelta(seconds=30)
+            )
+
+
+class ReboundMethodHolder:
+    """Takes ComplexActivityMethodHolder's name, with the opposite bindings."""
+
+    @activity.defn
+    async def echo_static(self) -> str:
+        return "rebound"
+
+    @staticmethod
+    @activity.defn
+    async def count_passengers(self, passengers: int) -> str:  # type: ignore[no-untyped-def] # pyright: ignore[reportMissingParameterType, reportSelfClsParameterName]
+        return str(passengers)
+
 
 class UnwrappedHolder:
     @staticmethod