feat(workflow): LLM-agent-as-node, task mode, and node-as-tool (Part 6)

[kalenkevich]

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

Link to Issue or Description of Change

• Related: Support for Workflows #366

Problem: Workflows need to run ADK agents as nodes (and expose nodes/workflows to agents as tools).

Solution — Part 6 of 8. Stacked on Part 5.

Included:

• nodes/llm_agent_wrapper.ts — runs a BaseAgent as a node: streaming, transfer_to_agent hand-offs, workflow instruction scope, and task mode (loops until the agent calls finish_task, whose args become the node output). Registers the agent node-builder, explicitly excluding BaseTool (which also exposes runAsync) so tool-before-agent precedence holds regardless of registration order.
• nodes/node_tool.ts — exposes a node/workflow as a tool an agent can call.
• tools/finish_task_tool.ts — the finish_task tool backing task mode.
• agents/llm_agent.ts — task mode (mode/finishTaskTool) + registers the request-input & request-confirmation processors (3-way merged onto current main).
• agents/processors/request_input_llm_request_processor.ts — agent-side HITL (request user input mid-run).
• agents/{invocation_context,instructions}, basic_llm_request_processor — workflow instruction scope and {Class.field}/<field from node> placeholder resolution.

Wired into the barrel (LLMAgentWrapper, NodeTool) and register_builtin_nodes.

Testing Plan

• Unit tests added/updated; all pass locally.

Unit tests: node_api (16), llm_agent (7), multi_agent (3), instructions (37). Plus the workflow integration suite (15 files / 35 tests, recorded model responses). Full core suite 2476 green; integration workflows green; docs:check + tsc clean.

Manual E2E: Covered by the integration suite (replayed model responses).

Checklist

• I have read CONTRIBUTING.md.
• I have performed a self-review.
• Commented hard-to-understand areas.
• Added tests.
• New and existing unit tests pass locally.
• Manually tested end-to-end.
• Dependent changes merged.

Additional context

Stacked split — merge in order (…Part 5 → Part 6 → Part 7 → …). llm_agent.ts was 3-way merged onto current main.

[AmaadMartin]

Reviewed the three integration seams rather than line-by-line. The node lifecycle is handled correctly on the node-as-tool path (it goes through executeChildNode, so timeout/retry/input validation apply), and the transfer loop is capped — but the node->tool->node path itself has no depth or cycle guard, and task mode is documented as a loop while being implemented as a single pass with no failure path when finish_task is never called. Two instanceof checks and a set of unused InvocationContext fields are the other structural items. Comments are advisory; the recursion and task-termination ones are the two I would not merge without an answer to.

[AmaadMartin]

Not a nit. Task mode does not loop, and it has no failure path when the agent never calls finish_task.

    for await (const event of agent.runAsync(agentIc)) {

The docs promise a loop — llm_agent.ts:271 says task mode "runs a multi-round loop until it calls finish_task", and the docstring above says "the agent loops (LLM <-> tools) until it calls the finish_task tool" — but this is a single pass over agent.runAsync with no re-prompt. LlmAgent.runAsyncImpl breaks out on the first final response (llm_agent.ts:741-746), so a model that answers in plain text without calling finish_task ends the pass, drops out of this for await, and the generator returns normally.

On that path nothing sets the output: unlike runWithTransfers, runTaskMode never calls maybeSetOutput, so child.output stays undefined, the node is reported COMPLETE, and downstream {Class.field} / <field from node> placeholders silently resolve to nothing. A task that fails to finish should be a FAILED node or a re-prompt, not an empty success. Either throw when the loop exits with pendingArgs === undefined, or add the real loop with an explicit cap.

Related, and the reason the cap matters: nothing bounds this at all. runAsyncImpl is while (true), and FinishTaskTool.processLlmRequest injects "Do NOT call finish_task prematurely. Use your available tools to fully complete every aspect of the task first" — an instruction that deliberately pushes the model to keep calling tools. Task mode is therefore the one place in the workflow engine with an unbounded LLM+tool spend and no maxSteps / iteration cap. A named constant next to MAX_TRANSFER_DEPTH would match how transfers are already handled here.

[AmaadMartin]

Re-reviewed at head 92464c5b. All eight findings from the prior round are resolved, including the two I would not have merged without — the node-as-tool recursion and task-mode termination.

What I verified against the source at this head:

• Node→tool→node recursion (blocker) — fixed. MAX_NODE_TOOL_DEPTH = 8 is enforced in node_tool.ts:144 against nodeToolDepth, a new readonly field on InvocationContext (invocation_context.ts:226) that runNode increments via ic.clone({nodeToolDepth: ic.nodeToolDepth + 1}) (node_tool.ts:152), so the depth carries across the spawned agent runs. runId now requires functionCallId and throws when absent (node_tool.ts:136-142), and nodePath: '' (node_tool.ts:159) removes the doubled path segment.
• Task-mode termination (blocker) — fixed. runTaskMode now throws when the agent finishes without a successful finish_task (llm_agent_wrapper.ts:155-158) instead of silently reporting COMPLETE with no output; the turn loop is bounded by the invocation's maxLlmCalls.
• Paused-node event queue — fixed. The ?? new AsyncQueue() fallback is gone; a missing ic.eventQueue now throws (node_tool.ts:126-132), and the structural cast was removed.
• instanceof type detection — fixed. llm_agent.ts:340 uses isBaseNode (now a type-only import) and request_input_llm_request_processor.ts:61 uses isNodeTool — both brand-based guards. Remaining instanceof uses are only e instanceof Error narrowing.
• InvocationContext.clone() extracted (invocation_context.ts:292) with the by-value/by-reference caveat documented; the as unknown as InvocationContextParams double cast is gone. agentStates/endOfAgents (previously dead) were removed.
• Public exports — fixed at the cause. WorkflowInstructionScope and InvocationContextParams are exported from common.ts and index.ts; FinishTaskTool, NodeTool, LLMAgentWrapper/Config are exported too. typedoc.json is untouched — no intentionallyNotExported band-aid.
• Suppression sweep — src is clean. The as never at node_tool.ts:65 is gone (inline narrowing). No any/as any/as unknown as/@ts-expect-error in src; the three require-yield disables are throw-only/return-only generators (the documented exception, consistent with the existing StartNode). Diff hygiene is clean (no .only/.skip/console.log).

CI: run-tests passed on ubuntu-latest, macOS-latest and the generic job. The one Windows failure was code_executors/unsafe_local_code_executor_test.ts "should execute shell code and return stdout" timing out at 5000ms — a documented pre-existing Windows flake in a file this PR does not touch (the same test passed on ubuntu and macOS). I re-triggered the Windows job; merge should confirm it green.

Only residual, non-blocking: a few as unknown as/as never casts remain in test fixtures (the nit portion of the suppression sweep) — author's discretion. LGTM.

[AmaadMartin]

Re-reviewed at 92464c5b. Both structural gaps from the last round are properly closed, and the fixes address the mechanism rather than the symptom.

Node-as-tool recursion is now bounded. MAX_NODE_TOOL_DEPTH = 8, checked before the child runs, and — the part that actually makes it work — the depth is carried across the agent boundary via ic.clone({nodeToolDepth: ic.nodeToolDepth + 1}). A counter that reset on each agent run would not have caught the workflow=[node(agent)] + agent.tools=[workflow] cycle; this one does.

Task mode both loops and fails correctly. It is a real multi-round loop against finish_task now, and the case I was most worried about is handled explicitly: an agent that answers in plain text without calling finish_task produces a stated failure instead of completing with output === undefined. A silent empty success in a task node is the kind of bug that surfaces three layers away, so having it terminate loudly matters more than the loop itself.

The paused-node dead end is gone. The ?? new AsyncQueue() fallback that wrote an interrupt event into a queue nobody drains — while returning undefined and leaving the tool call unresumable — is replaced by a hard error when ic.eventQueue is absent, with the reason stated at the throw. Failing loudly at the boundary is right; the previous shape lost the pause silently.

Also resolved: the two instanceof sites are brand-based now (the only remaining occurrence of the word is a doc comment explaining the choice); agentStates/endOfAgents were removed rather than left as unread fields; the typedoc.json suppression for WorkflowInstructionScope is gone; and withWorkflowInstructionScope dropped its as unknown as cast and now names its relationship to withBranch in the doc instead of silently duplicating it. Suppressions in the diff dropped from 17 to 6.

One leftover nit, not blocking: the three eslint-disable-next-line require-yield comments are bare. Part 5 gives each of its two a specific reason ("streams via ctx.channel and yields nothing itself", "must be an AsyncGenerator per BaseAgent but only throws"), and the same treatment here would keep them from reading as blanket silencing to the next reviewer.

On CI — read this before merging. Ubuntu and macOS are green. Windows is red, and I am not certifying it. I re-ran it twice and it failed both times, but on two different tests: first code_executors/unsafe_local_code_executor_test.ts (a 5s shell-spawn timeout), then tests/integration/a2a/input_required/input_required_test.ts. This PR touches neither area — it adds nothing under code_executors or a2a — and #591 and #592 both pass Windows on the same base lineage. A different unrelated test failing on each run is an environmental signature, not a regression from Part 6, which is why I am approving the change. But it is a real red check: if the merge gate requires a green Windows leg, this needs another spin, and the underlying flakiness is worth a separate issue rather than being re-run away each time.

[AmaadMartin]

Status update on the CI caveat in my approval above — Windows is now green, on the same head (92464c5b), so that caveat is stale and should not hold up a merge. All checks: ubuntu, macOS, Windows, check-changes, check-license, scan-pr, CLA all passing.

It took three attempts to get there, and the failure rotated each time rather than repeating:

1. code_executors/unsafe_local_code_executor_test.ts — 5s shell-spawn timeout
2. tests/integration/a2a/input_required/input_required_test.ts
3. clean

A different unrelated test failing on each attempt, always a timeout, always subprocess-adjacent — and this PR touches neither code_executors nor a2a. That is Windows runner contention, not anything Part 6 introduced, which is what the approval was based on; the green run just confirms it.

Worth flagging beyond this PR: #591 also needed three attempts on its validation run for the same reason. So the whole stack is paying roughly ten minutes per retry on an intermittent Windows failure that has nothing to do with the changes under review. That seems worth a separate issue — either raising the timeout on those two suites on Windows or marking them flaky — rather than everyone re-running past it.