fix(plugins): let callbacks opt out of short-circuit via shortCircuit: false

[tSte]

Please ensure you have read the contribution guide before creating a pull request.

Link to Issue or Description of Change

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

Problem: PluginManager.runCallbacks short-circuits the plugin chain as soon as a callback returns a non-undefined value. A plugin that legitimately transforms a value (localization rewriting LlmResponse, redaction altering a tool result, content filtering) silently disables every plugin registered after it for that callback — logging, metrics, tracing, safety auditing, etc. The failure is silent (only a logger.debug line is emitted) and there is currently no way for a plugin to express "I transformed this, keep going."

The BasePlugin docstring already documents the intended pipeline semantic ("The modifications will be visible and passed to the next callback in the chain"), but the implementation in PluginManager.runCallbacks does not match it.

Solution: Add an opt-out path. If a callback returns an object whose literal shortCircuit field is false, the flag is stripped, the remaining fields become the new current value, and the chain continues. The next plugin receives that value spliced into the appropriate input field (result / llmResponse / event / userMessage / tools), so transforms genuinely compose. Any other non-undefined return preserves the legacy early-exit behavior, so existing plugins are unaffected.

Example:

// Pipelines transformed LlmResponse to subsequent plugins
override async afterModelCallback({ llmResponse }) {
  return { shortCircuit: false, ...translated(llmResponse) };
}

// Still short-circuits (legacy)
override async beforeModelCallback({ llmRequest }) {
  return cachedResponse;
}

BasePlugin return types for Content / Event / LlmResponse / tools-map are widened with & { shortCircuit?: boolean } so overrides type-check under strict mode without casts. Record<string, unknown>-typed callbacks (beforeTool / afterTool / onToolError) need no widening.

Testing Plan

Unit Tests:

• Existing plugin_manager_test.ts cases pass unchanged (legacy short-circuit semantics preserved).
• Added two new cases covering the opt-out:
  • Plugin returning { shortCircuit: false, ...x } keeps plugin2 running and strips the flag from the final result.
  • Plugin2 receives plugin1's transformed value as its result param (pipeline).

✓ |unit:core| core/test/plugins/plugin_manager_test.ts (9 tests)

 Test Files  1 passed (1)
      Tests  9 passed (9)

Manual End-to-End (E2E) Tests:

Verified against a real voice-agent setup with a ResponseMessagePlugin (localizes result.message) registered before a LoggingPlugin (logs the final tool result). Before the fix, the logging plugin was never invoked for any tool that produced a localized message. After the fix, the logging plugin runs and logs the translated message, confirming both the opt-out and the pipeline plumbing.

Checklist

• I have read the CONTRIBUTING.md document.
• I have performed a self-review of my own code.
• I have commented my code, particularly in hard-to-understand areas.
• I have added tests that prove my fix is effective.
• New and existing unit tests pass locally with my changes.
• I have manually tested my changes end-to-end.
• Any dependent changes have been merged and published in downstream modules. (N/A)

Additional context

The change conflates two distinct intents that the current short-circuit logic merges:

1. Veto — e.g. beforeModelCallback returning a cached LlmResponse to skip the model call. Early exit is correct.
2. Transform / decorate — e.g. afterModelCallback translating the response. Early exit is harmful; the next plugin should see the transformed value.

The shortCircuit: false opt-out lets callers express the second intent without changing the default behavior of the first. All short-circuiting callbacks are affected: onUserMessageCallback, beforeRunCallback, onEventCallback, beforeAgentCallback, afterAgentCallback, beforeToolSelection, beforeToolCallback, afterToolCallback, onModelErrorCallback, beforeModelCallback, afterModelCallback, onToolErrorCallback.

[AmaadMartin]

Here is a review of the PR. The overall idea of allowing plugins to opt-out of short-circuiting makes sense, but the implementation is inconsistent and has some issues that need to be addressed before merging. Specifically, many callbacks were updated in base_plugin.ts but are not correctly wired for chaining in plugin_manager.ts.

[AmaadMartin]

This new signature (accepting latest value for chaining) is only utilized for a subset of callbacks in plugin_manager.ts (e.g., onUserMessageCallback, onEventCallback, beforeToolSelection, afterToolCallback, afterModelCallback).

Other callbacks that had their types updated in base_plugin.ts (like beforeRunCallback, beforeAgentCallback, afterAgentCallback, beforeModelCallback, onModelErrorCallback) were not updated in plugin_manager.ts to pass the latest value. This means they will not actually chain/transform values correctly (they will run, but won't see previous plugins' changes).

[AmaadMartin]

afterAgentCallback return type is updated to support ShortCircuitControl, but the method signature doesn't receive the agent's result in its parameters. Without receiving the result, it is impossible for a plugin to transform it. We should add result to the parameters and update plugin_manager.ts to pipe it.

[AmaadMartin]

Why was beforeModelCallback updated to support ShortCircuitControl here, but beforeToolCallback was not? Both are "before" callbacks that can return a value to skip the operation. They should be treated consistently. (Note: if they are strictly for vetoing, they probably shouldn't support chaining/ShortCircuitControl at all, as they don't have parameters to receive the chained value).

[AmaadMartin]

Note: Immediately following this method (starting at line 418), there is a pre-existing unclosed comment block (/** without closing */). This causes the compiler to treat the entire block up to line 441 as a comment, silently commenting out a duplicate/broken method definition in between. We should clean this up while we are modifying this file.