fix(plugins): use notification-only dispatch for error callbacks

Error callbacks are documented as notification-only (all plugins must
be notified), but were dispatched through _run_callbacks() which
short-circuits on the first non-None return. A buggy plugin returning
a value would prevent subsequent plugins from seeing the error.

Add _run_notification_callbacks() that always iterates all plugins
regardless of return values, and switch run_on_agent_error_callback()
and run_on_run_error_callback() to use it.

Update tests to assert both plugins are always called even when one
returns non-None.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Author: caohy1988

src/google/adk/plugins/plugin_manager.py

@@ -267,7 +267,7 @@ async def run_on_agent_error_callback(
       error: Exception,
   ) -> None:
     """Runs the `on_agent_error_callback` for all plugins."""
-    await self._run_callbacks(
+    await self._run_notification_callbacks(
         "on_agent_error_callback",
         agent=agent,
         callback_context=callback_context,
@@ -281,7 +281,7 @@ async def run_on_run_error_callback(
       error: Exception,
   ) -> None:
     """Runs the `on_run_error_callback` for all plugins."""
-    await self._run_callbacks(
+    await self._run_notification_callbacks(
         "on_run_error_callback",
         invocation_context=invocation_context,
         error=error,
@@ -336,6 +336,35 @@ async def _run_callbacks(
 
     return None
 
+  async def _run_notification_callbacks(
+      self, callback_name: PluginCallbackName, **kwargs: Any
+  ) -> None:
+    """Executes a notification-only callback for all registered plugins.
+
+    Unlike ``_run_callbacks``, this method **always iterates all plugins**
+    regardless of return values.  It is intended for error callbacks where
+    every plugin must be notified (the return value is discarded).
+
+    Args:
+      callback_name: The name of the callback method to execute.
+      **kwargs: Keyword arguments to be passed to the callback method.
+
+    Raises:
+      RuntimeError: If a plugin encounters an unhandled exception during
+        execution. The original exception is chained.
+    """
+    for plugin in self.plugins:
+      callback_method = getattr(plugin, callback_name)
+      try:
+        await callback_method(**kwargs)
+      except Exception as e:
+        error_message = (
+            f"Error in plugin '{plugin.name}' during '{callback_name}'"
+            f" callback: {e}"
+        )
+        logger.error(error_message, exc_info=True)
+        raise RuntimeError(error_message) from e
+
   async def close(self) -> None:
     """Calls the close method on all registered plugins concurrently.
 

tests/unittests/plugins/test_error_callbacks.py

@@ -443,45 +443,60 @@ async def test_run_on_run_error_callback_dispatches(self):
     assert len(plugin2.run_errors) == 1
 
   @pytest.mark.asyncio
-  async def test_error_callbacks_do_not_short_circuit(self):
-    """Error callbacks are notification-only: a non-None return from
-    one plugin does NOT skip subsequent plugins."""
+  async def test_agent_error_callback_does_not_short_circuit(self):
+    """on_agent_error_callback is notification-only: a non-None return
+    from one plugin does NOT skip subsequent plugins."""
 
     class _ReturningPlugin(BasePlugin):
       __test__ = False
 
       def __init__(self, name):
         super().__init__(name)
         self.agent_error_called = False
-        self.run_error_called = False
 
       async def on_agent_error_callback(self, **kwargs):
         self.agent_error_called = True
         return "should be ignored"
 
-      async def on_run_error_callback(self, **kwargs):
-        self.run_error_called = True
-        return "should be ignored"
-
     p1 = _ReturningPlugin(name="p1")
     p2 = _ReturningPlugin(name="p2")
     pm = PluginManager(plugins=[p1, p2])
 
-    # The _run_callbacks early-exit logic returns on non-None,
-    # but for notification-only callbacks the base class returns None.
-    # However, if a plugin DOES return non-None, the current
-    # _run_callbacks stops. This test documents that behavior.
-    # The base class default returns None (pass), so in practice
-    # all plugins are always called.
     await pm.run_on_agent_error_callback(
         agent=Mock(spec=BaseAgent),
         callback_context=Mock(spec=CallbackContext),
         error=RuntimeError("x"),
     )
 
-    # p1 returns non-None which triggers early exit in _run_callbacks.
-    # This is an inherent behavior of the generic dispatch.
+    # Both plugins must be called even though p1 returns non-None.
     assert p1.agent_error_called
-    # Note: p2 may or may not be called depending on _run_callbacks
-    # early-exit. Since base class returns None (pass), normal usage
-    # always calls all plugins. This test just verifies p1 was called.
+    assert p2.agent_error_called
+
+  @pytest.mark.asyncio
+  async def test_run_error_callback_does_not_short_circuit(self):
+    """on_run_error_callback is notification-only: a non-None return
+    from one plugin does NOT skip subsequent plugins."""
+
+    class _ReturningPlugin(BasePlugin):
+      __test__ = False
+
+      def __init__(self, name):
+        super().__init__(name)
+        self.run_error_called = False
+
+      async def on_run_error_callback(self, **kwargs):
+        self.run_error_called = True
+        return "should be ignored"
+
+    p1 = _ReturningPlugin(name="p1")
+    p2 = _ReturningPlugin(name="p2")
+    pm = PluginManager(plugins=[p1, p2])
+
+    await pm.run_on_run_error_callback(
+        invocation_context=Mock(spec=InvocationContext),
+        error=RuntimeError("x"),
+    )
+
+    # Both plugins must be called even though p1 returns non-None.
+    assert p1.run_error_called
+    assert p2.run_error_called