feat(plugins): add before/after node callbacks

[kalenkevich]

Link to Issue or Description of Change

2. Or, if no issue exists, describe the change:

Problem:

BasePlugin has hooks for runs, agents, models and tools — but nothing for workflow nodes. A plugin could observe every layer of an agent's execution except the graph it was running inside. A node-level cache, a per-node audit trail, or stubbing one node out in a test all had nowhere to attach.

Solution:

Two hooks, following the early-exit convention the other plugin callbacks already use (first plugin to return a non-undefined value wins):

• beforeNodeCallback({node, nodeContext, input}) — returning anything other than undefined skips the node's body and uses that value as its output. This is how a plugin implements a node cache or a stub.
• afterNodeCallback({node, nodeContext, output}) — runs on success; a returned value replaces the node's output.

They hang off executeChildNode, so they cover graph nodes, dynamic ctx.runNode() children, and a nested Workflow (itself a node) alike. The before hook sits outside the retry loop, so a node that retries reports one before/after pair rather than one per attempt.

Two deliberate omissions:

• a node whose body was skipped fires no after callback — nothing ran to report on;
• a node that throws fires neither; failures are reported as a NodeErrorEvent (feat(workflow): record a failed node as a NodeErrorEvent #657) rather than through this pair.

The plugin types import BaseNode/NodeContext as type-only imports. The runtime chain already runs workflow → InvocationContext → PluginManager, so a value import would close that cycle.

Testing Plan

Unit Tests:

• I have added or updated unit tests for my change.
• All unit tests pass locally.

New core/test/workflow/node_plugin_hooks_test.ts, 6 tests: both hooks fire with the right node/input/output; a returned before-value skips the body (asserted by the body setting a flag that stays false) and suppresses the after hook; a returned after-value replaces the output; hooks fire for every node of a graph and for the workflow node itself, in the expected order (wf, a, b before / a, b, wf after); a throwing node fires no after hook; and a PluginManager with no plugins leaves execution untouched.

npx vitest run --project unit:core
 Test Files  208 passed (208)
      Tests  2850 passed (2850)

tsc --noEmit: 0 errors repo-wide. eslint and prettier clean on all 4 files.

Manual End-to-End (E2E) Tests:

Not run. The hooks are exercised through the real PluginManager in unit tests; what is unverified is a plugin registered on a live Runner against a real model.

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 or that my feature works.
• 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.

Additional context

Only the TSDoc on the two new public callbacks is present; per repo-owner instruction the change carries no other code comments.

Independent of the other workflow PRs in flight (#649, #653, #654, #656, #657); it touches node_runner.ts, which #653 and #657 also touch, so expect a small conflict for whichever lands last.

[AmaadMartin]

Held on CI, not on the code. The diff reads clean: both hooks are behavior-neutral when no plugin is registered (runCallbacks returns undefined on an empty set), the eslint-disable no-unused-vars matches every other no-op callback in base_plugin.ts, and the type-only imports correctly avoid the runtime cycle.

CI is red on tests/integration/workflows/parallel_worker/parallel_worker_test.ts (Cannot read properties of undefined (reading 'topic') in the sample agent). This is a live-model integration test; the PR does not touch it and cannot change its path with no plugin set. The macOS and Windows jobs were fail-fast cancellations, not failures. I re-ran the failed jobs; they are in progress.

[AmaadMartin]

Nit, optional. Promise<unknown | undefined> is the same type as Promise<unknown> — unknown already includes undefined, so the | undefined is redundant here. The sibling callbacks differ: their type is Content | undefined, where the union matters. Same at line 279 (afterNodeCallback). Drop it, or keep it for symmetry.

[AmaadMartin]

I re-checked the code at 8175051. It is clean. The two new node hooks follow the existing plugin-callback pattern, the pluginManager?.hasPlugins gate keeps the awaits off the scheduling path when no plugin runs, and the before hook sits outside the retry loop as intended. No any, no instanceof; the two new eslint-disable lines match every other callback stub in base_plugin.ts.

I cannot approve yet: run-tests on ubuntu, macOS, and Windows is still pending. My only open item is the optional nit on the redundant | undefined, which does not block. I will approve once the three test jobs pass.

[kalenkevich]

Thanks for the review — the nit is fixed, but I have to correct the CI call: the red run was mine, not a live-model flake.

I bisected it. parallel_worker passes 5/5 on the base commit 8611219 and fails 10/10 on 4a7a61c, in isolation, on a clean checkout. Deterministic, not flaky.

The mechanism is the one thing we both assumed away — that the hooks are inert with no plugin registered:

const skipOutput = await pluginManager?.runBeforeNodeCallback({...});

That await ran unconditionally. With zero plugins it still resolves undefined, but it inserts a microtask boundary into executeChildNode before every node body. Invisible in a sequential workflow — but parallel workers share a session, and the extra tick let all three make_upper_case outputs land in the session before the explain_topic workers built their requests. Each worker's prompt gained a For context: block it did not have when the fixture was recorded:

recorded:  [..., find_related_topics ctx, "SQL"]
on my PR:  [..., find_related_topics ctx, "SQL", make_upper_case ctx x3]

The harness matches recorded responses by a fingerprint over request contents, so two of the three workers missed, produced nothing, and aggregate read .topic off undefined. That is what the TypeError was — the real error was the [sample-harness] No recorded model response ... line on stderr just above it.

Fixed in 8175051 by gating both hooks on a plugin actually being registered. Worth noting ?. alone does not do it — await undefined still yields a tick, so the await itself has to sit inside the branch:

if (pluginManager?.hasPlugins) {
  const skipOutput = await pluginManager.runBeforeNodeCallback({...});
  ...
}

Re-recording the fixture would also have gone green, but it would have baked in a prompt change that an unregistered plugin has no business causing. The hooks are now genuinely zero-impact when unused, which is what the PR claimed in the first place.

On the nit: dropped | undefined from both callbacks in 0892346, and from their mirrored PluginManager methods, which had copied the same redundancy — they now line up with runCallbacks, which has returned plain Promise<unknown> all along. 4c87b06 does the same for the last two pre-existing cases in functions.ts and operations.ts.

CI is green on all three platforms. The one Windows red along the way was unsafe_local_code_executor timing out at 5s, which also fails on main at this PR's base commit (run 31570504892) — unrelated, and I am fixing it separately.

[AmaadMartin]

Approve. My earlier nit is fixed: both node hooks now return Promise<unknown> (base_plugin.ts:258,279), and the author swept the same redundant unknown | undefined in functions.ts and operations.ts.

I verified the source at head 4c87b06. The two eslint-disable no-unused-vars lines match the pattern every other default hook in base_plugin.ts uses, so they are not new suppressions. No any, as any, @ts-expect-error, or instanceof in the diff. No new named public type needs an export. The before/after calls sit behind hasPlugins, so an empty PluginManager adds no await.

CI is green on ubuntu, macOS, and Windows run-tests.