feat(workflow): built-in Function and Tool nodes (Part 3)

[kalenkevich]

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

Link to Issue or Description of Change

1. Link to an existing issue (if applicable):

• Related: Support for Workflows #366

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

Problem:
Continuing the stacked split of the large feature/workflows branch. With the engine core and its node-builder registry in place (Part 2), the workflow engine needs its first concrete node types.

Solution:
This is Part 3 of 9 — the built-in Function and Tool nodes — stacked on Part 2.

Stacked on: #part2_pr_number (Part 2 — engine core). Please merge Part 2 first.

Included:

• nodes/function_node.ts — wraps a plain function / async function / (sync or async) generator as a node. Supports inputSchema/outputSchema validation and an auth gate for HITL (the gate's request processors land in Part 8).
• nodes/tool_node.ts — wraps a BaseTool as a node.

Both node modules self-register with the engine's node-builder registry (registerNodeBuilder) at import time, so buildNode() / isNodeLike() — and thus node() and graph parsing — turn a bare function or tool into the right node without the engine statically importing these modules. This is the registration side that Part 2's decoupling refactor was built for; the public barrel (Part 6) imports the node modules so registration is guaranteed in real usage.

Intentionally deferred: the node() user API (node.ts) and the auth-gate integration test move to Part 6 (they need the runner/barrel). Parallelism (Part 4), dynamic scheduling (Part 5), LLM-as-node (Part 7), and HITL processors (Part 8) follow.

Testing Plan

Unit Tests:

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

Bundled tests (9): workflow/schema_validation_test.ts (input/output schema coercion + rejection through driveNode) and workflow/node_builders_test.ts (registry wiring: function → FunctionNode with name resolution, unnamed-function error, tool → ToolNode, existing-BaseNode passthrough, and isNodeLike).

$ npx vitest run --project unit:core \
    core/test/workflow/schema_validation_test.ts \
    core/test/workflow/node_builders_test.ts
 Test Files  2 passed (2)
      Tests  9 passed (9)

Full core suite green (2375 tests). Typecheck clean: npx tsc --noEmit -p core/tsconfig.json.

Manual End-to-End (E2E) Tests:

N/A — node-level units; graph/runner E2E coverage lands in Part 6.

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

Stacked split — merge in order (…Part 2 → Part 3 → Part 4 → …). Diff: 4 files, +462.

[AmaadMartin]

Reviewed the Part 3 delta only (both node modules plus the two new test files), verified against the head SHA and the surrounding engine from Parts 1-2. The FunctionNode side is largely sound — the auth gate's deterministic interrupt id and the Event/Content/null coercion in toEvent are right, and I confirmed error propagation is consistent between the two node types (neither swallows a throw, both let the runner's retry/failure path see it). ToolNode is where the substance is: it invokes tool.runAsync directly, so the plugin and before/after tool callback chain, the onToolError hook, the confirmation gate and long-running handling from core/src/agents/functions.ts are all bypassed, and everything the tool writes to its context except stateDelta is dropped. It also has no execution test. One thing I checked and can rule out: Event.output is in PRESERVE_KEYS (event.ts:371/:394), so structured node output survives the snake/camel round-trip.

[AmaadMartin]

Not a nit. inputSchema does not apply on the path that most needs it, and the coerced args reach the tool unvalidated.

if (isContent(args)) {
  args = extractText(args);
}

BaseNode.validateInput deliberately skips genai Content (base_node.ts:151: if (!this.inputSchema || isContent(input)) return input;) on the assumption that nodes coerce it themselves. ToolNode does coerce it — into the tool's argument object — but never re-validates afterwards. So when the input is Content (i.e. produced by an LLM node, which is the whole point of Part 7), model-authored text is JSON.parsed and handed straight to tool.runAsync with inputSchema never applied and no check against the tool's own _getDeclaration() parameters. Model-populated fields are attacker-influenced; this is the boundary where that matters.

Minimum: run the coerced object back through this.validateInput/the tool declaration before calling runAsync.

Two smaller things in the same helper: JSON.parse('null') yields null, which falls through to return {} and invokes the tool with no arguments at all rather than reporting bad input; and the TypeError thrown below is a permanent input error that a user-configured retryConfig will happily retry (shouldRetryNode matches on error.name). Also 'must be a dictionary of tool arguments' reads as Python — "object" is the TS word.

[AmaadMartin]

Not a nit. ToolNode has no execution test at all — 117 lines of runtime logic with only this construction check.

it('builds a ToolNode from a BaseTool', () => {
  const node = buildNode(new TestTool());
  expect(node).toBeInstanceOf(ToolNode);
  expect(node.name).toBe('test_tool');
});

Nothing drives ToolNode.runImpl. Untested: that the tool is actually invoked with the coerced args, that a returned value lands on event.output, that toolContext state writes propagate, and every branch of coerceToolArgs (Content -> text, JSON string, empty string -> {}, array/scalar -> TypeError). driveNode already exists in test_helpers.ts and schema_validation_test.ts uses it for FunctionNode, so the harness is right there.

Several of the issues I flagged in tool_node.ts (dropped artifactDelta/requestedAuthConfigs, missing content, long-running tools completing silently) would each be caught by one such test.

[AmaadMartin]

Re-reviewed at b105f256. The blocking finding is properly fixed, not worked around.

ToolNode now executes through handleFunctionCallList, so the plugin before/after/onError callbacks, tracing, and the confirmation gate all run on a workflow tool call instead of being skipped by a direct tool.runAsync. That also fixes it at the source for #594 — requireConfirmation was inert inside workflows purely because this path bypassed the gate, so that whole class of problem goes away here rather than needing a patch in Part 7. Routing through the canonical path also means the tool's full actions (artifactDelta, requestedAuthConfigs, requestedToolConfirmations, escalate) now flow through instead of only stateDelta being hand-copied.

The other substantive items are addressed too, and in each case the comment explains the reasoning rather than just changing the line:

• functionCall.id is now ${ctx.nodePath}:${ctx.runId} instead of a fresh randomUUID() per invocation, so a credential or confirmation request can actually be matched to its resume response across turns and retries.
• this.validateInput(coerceToolArgs(input)) re-validates after coercion, which closes the trust-boundary gap — model-authored text parsed out of Content is now schema-checked before it reaches the tool, and the comment says explicitly that this is the only point where that happens.
• The node now yields the canonical response event (keeping functionResponse content for history) and sets output from it, instead of emitting output with no content.

Passing beforeToolCallbacks: [] / afterToolCallbacks: [] is the right call and I checked it deliberately: those are the agent-level lists, which a workflow node has no equivalent of, while plugin callbacks still fire via invocationContext.pluginManager. The comment already says so.

CI is green across ubuntu/macOS/Windows — verified against the individual job list rather than the rollup. LGTM.