Skip to content

Plugins returning non-undefined value short-circuit all other plugins downstream #342

@tSte

Description

@tSte

PluginManager.runCallbacks short-circuits the entire plugin chain as soon as any plugin returns a non-undefined value from a callback. This means a plugin that legitimately needs to transform a value (e.g. a response-redaction plugin, a content filter) silently prevents every plugin registered after it from observing that callback for the rest of the invocation event.

Concretely, this breaks the common composition pattern of "transform + observe": a localization plugin's afterModelCallback that returns a translated LlmResponse will skip the afterModelCallback of every subsequent plugin — including logging, metrics, tracing, safety auditing, and any other observer-style plugin. The author of the logging plugin has no way to opt back in, and the failure is silent (only a logger.debug line is emitted).

This affects every short-circuiting callback in PluginManager: onUserMessageCallback, beforeRunCallback, onEventCallback, beforeAgentCallback, afterAgentCallback, beforeToolSelection, beforeToolCallback, afterToolCallback, onModelErrorCallback, beforeModelCallback, afterModelCallback, onToolErrorCallback.

Source: core/src/plugins/plugin_manager.ts, lines 94–115:

private async runCallbacks(
  plugins: Set<BasePlugin>,
  callback: (plugin: BasePlugin) => Promise<unknown>,
  callbackName: string,
): Promise<unknown> {
  for (const plugin of plugins) {
    try {
      const result = await callback(plugin);
      if (result !== undefined) {
        logger.debug(
          `Plugin '${plugin.name}' returned a value for callback '${callbackName}', exiting early.`,
        );
        return result;
      }
    } catch (e) {
      ...
    }
  }
  return undefined;
}

The class-level docstring (lines 28–32) documents this as intentional:
"if any plugin callback returns a non-undefined value, the execution of subsequent plugins for that specific event is halted.". The design conflates two distinct intents:

  1. Short-circuit / veto (e.g. beforeModelCallback returning a cached response to skip the model call). Early exit makes sense.
  2. Transform / decorate (e.g. afterModelCallback translating the response). Early exit is harmful; the next plugin should see the transformed value.

There is currently no way for a plugin to express "I transformed this; keep going" — returning the transformed value short-circuits, and returning undefined discards the transformation.

To Reproduce:

  1. Register two plugins in this order: Plugin 1 - afterModelCallback returns a modified LlmResponse (e.g. translated text). Then Plugin 2 - afterModelCallback logs the response
  2. Run any invocation that produces a model response.
  3. Observe: Plugin 2 .afterModelCallback() is never called. Only a debug-level log mentions the early exit.

Minimal reproducer:

const a = new (class extends BasePlugin {
  name = 'a';
  async afterModelCallback() { return undefined; }
})();
const b = new (class extends BasePlugin {
  name = 'b';
  async afterModelCallback({ llmResponse }) {
    return { ...llmResponse, content: { ...llmResponse.content, parts:
      [{ text: 'translated' }] } };
  }
})();
const c = new (class extends BasePlugin {
  name = 'c';
  afterModelCalled = false;
  async afterModelCallback() { this.afterModelCalled = true; return undefined; }
})();

const mgr = new PluginManager([a, b, c]);
await mgr.runAfterModelCallback({ callbackContext, llmResponse });
// c.afterModelCalled === false  -> bug

Expected behavior:

A plugin should be able to transform a value and still allow downstream plugins to observe (and further transform) the result. Two reasonable resolutions:

  1. Pipeline semantics for transform callbacks (I'd prefer this): chain the returned value through subsequent plugins, feeding each plugin the most recent non-undefined result. Short-circuit only on an explicit sentinel (e.g. a returned { shortCircuit: true, value } wrapper, or a separate shortCircuit() helper on the callback context).
  2. Per-callback policy: classify callbacks as "veto" (current behavior - before* that gate execution) vs. "transform" (after*, onEvent, onUserMessage) and apply chaining semantics only to the latter.

At minimum, the current behavior should be loudly documented at the BasePlugin callback level (not only in PluginManager's internal docstring), since plugin authors writing afterModelCallback have no signal that returning a value disables every other plugin.

If you confirm that this can be desired solution, I will gladly open PR for this.

Screenshots

N/A.

Desktop (please complete the following information):

  • OS: Linux 7.0.5-zen1-1-zen (Arch)
  • TS version/environment: TypeScript strict, Node 22, pnpm 10
  • ADK version: @google/adk 1.1.0

Additional context

Registration order is well-defined (insertion order of the internal Set<BasePlugin>, iterated by for..of in runCallbacks), so this is not a non-determinism bug — it is a semantics bug in how runCallbacks interprets a non-undefined return.

Related lines:
core/src/plugins/plugin_manager.ts:35 (Set storage), :57–69 (registerPlugin), :94–115 (runCallbacks early-exit logic), and every public run*Callback method (:120–391) that
goes through runCallbacks and is therefore affected.

Metadata

Metadata

Assignees

Labels

bugSomething isn't workingneeds review[Status] The PR/issue is awaiting review from the maintainer

Type

No fields configured for Bug.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions