Fix: publish the bound port in A2A agent card URLs - #300
Open
AmaadMartin wants to merge 14 commits into
Open
Conversation
This was referenced Jul 30, 2026
* fix(cli): make the getting-started path actually work `adk create` wrote the API key to `GOOGLE_API_KEY`, a variable only Vertex AI Express Mode reads, while the same `.env` set `GOOGLE_GENAI_USE_VERTEXAI=0` to select the Gemini API path — which reads `GOOGLE_GENAI_API_KEY` / `GEMINI_API_KEY`. Every scaffolded project therefore failed at its first model call with "API key must be provided ...", regardless of the key supplied. The root README quickstart never mentioned an API key at all, and told users to run `npx adk run agent.ts`. The `adk` bin ships in `@google/adk-devtools`, a separate `-D` install, so for anyone who installed only `@google/adk` that command resolves to an unrelated third-party package on the public registry. - Write `GOOGLE_GENAI_API_KEY` from `adk create`. - Add the missing API-key step to the README quickstart. - Use `npx @google/adk-devtools ...` in the README, matching the scripts the scaffolder already generates. - Add a regression test that scaffolds a project, loads only the generated `.env`, and asserts the key reaches the Gemini endpoint — a test on the literal `.env` string cannot catch a wrong-but-consistent variable name. * test(cli): narrow the .env regression test to its load-bearing assertion The scaffold test file had grown three cases where one carries the guarantee. The string assertion duplicated cli_create_test.ts and was exactly the kind of check the fix showed to be insufficient; the Vertex case passed identically before and after the fix and characterises core's geminiInitParams rather than the scaffolder. Also trims the README note on naming the devtools package. --------- Co-authored-by: amaadmartin <amaadmartin@google.com>
isDangerousZipEntryName rejected entries via string prefix/substring
checks (startsWith('/'), startsWith('../'), includes('/../')), which
misses entries whose traversal segment isn't bounded by a following
'/' -- e.g. an entry named exactly 'scripts/..' (no trailing content)
passes all three checks, as does any entry using backslash as a
separator instead of forward slash (e.g. 'scripts\\..\\..\\evil').
Confirmed via a dynamic PoC (crafted a real zip with Python's zipfile
module, bypassing AdmZip's own write-side path normalization, and fed
the raw bytes through the actual compiled loadSkillFromZipBuffer) that
'scripts/..' passes the old check and reaches
skill.resources.scripts['..']. A second, independent hardened check in
materializeFiles (utils/file_utils.ts) was confirmed to catch and
reject the resulting escape attempt in every consumer of this data
that writes to disk, so this fix closes a genuine gap in
defense-in-depth rather than a currently-reachable filesystem escape.
Replaces the prefix/substring checks with a segment-based check: split
on both '/' and '\\' and reject if any segment is exactly '..', plus
an explicit absolute-path check (POSIX and Windows style). Verified
against the previous bypass cases, the existing test suite (68 tests,
all still pass), and additional false-positive traps (filenames merely
containing '..' as a substring, e.g. 'a/b..c/d', must not be rejected).
* fix(security): prevent prototype pollution via untrusted map keys
appName, userId, sessionId and state keys arrive straight off request
paths and bodies on the dev server. Held in plain `{}` maps, a key of
`__proto__` aliases Object.prototype instead of creating an own
property, so a single unauthenticated request writes onto
Object.prototype for the lifetime of the process. The appendEvent
stateDelta path makes the planted value fully attacker-controlled.
Key the affected maps with Object.create(null) so these names become
ordinary own properties:
- InMemorySessionService: sessions, userState, appState.
- InMemoryCredentialService: credentials. Also stops an inherited
credentialKey such as `toString` resolving to a Function rather
than undefined.
- AdkApiServer: runnerCache, traceDict, sessionTraceDict. Here `in`
matched inherited names, so `appName in runnerCache` reported a hit
and yielded a Function where a Runner was expected, and
GET /debug/trace/toString returned 200 instead of 404.
An app literally named __proto__ keeps working, and no longer leaks
phantom sessions across apps.
* fix(security): address review on prototype pollution fix
Close the same primitive one level up, on the same request path, and drop
the duplicated helper.
- `trimTempState` / `trimTempDeltaState` copied caller-controlled keys into
plain object literals, so a `{"state": {"__proto__": {...}}}` request body
re-parented the new session state onto the attacker's object. `State.get`
and `State.has` use `in`, so every key on it read back as session state.
Both filtered maps are now null-prototype.
- `updateSessionState` wrote the delta into `session.state`, which is not
always null-prototype, so fixing the delta map alone would have routed the
attacker key straight into the plain assignment. It now uses
`Object.defineProperty`, which always creates an own property.
- `InMemoryMemoryService.sessionEvents` inner map is keyed by `session.id`,
which holds no `/` and so can be exactly `__proto__`; such a session was
silently dropped from the `Object.values` scan in `searchMemory`.
- Replace the three `createNullProtoMap()` copies with direct
`Object.create(null)` assignments; the cast was a no-op because
`Object.create` returns `any`. The rationale moves onto the fields.
Tests: the `__proto__` state-key test asserted `({}).baseUrl`, which is
undefined before and after the fix. It now reads the key back through a
*sibling* session, because `updateSessionState` also writes the prefixed key
into the originating session's own state and masks the `userState` loss.
Cleanup list gains `u1` and `s1`, both of which land on `Object.prototype`
when the fix is reverted. Three new tests cover the sinks above; all 10
guard tests were confirmed to fail on the unfixed tree.
* Release: v1.6.0 * chore(changelog): add missing #596 and #621 security fixes to adk 1.6.0 Both PR titles used "Fix <subject>" with no colon, so the squash subjects were not conventional commits and release-please dropped them from the generated changelog: d3f250e Fix unsafe A2A peer-supplied transferToAgent metadata (#596) 7bc05f6 Fix zip-slip blacklist bypass in isDangerousZipEntryName (#621) Both are core-only security fixes landed after adk-v1.5.0, so they belong in the adk 1.6.0 Bug Fixes section. Entries are inserted in the position release-please would have emitted them.
…art 1) (#587) * feat(workflow): add event model extensions and shared node primitives First slice of the workflow engine split (Part 1/9). Adds the leaf primitives the engine builds on, none of which depend on the engine core: - workflow/errors, node_status, node_state, retry_config, utils/retry_utils, trigger, branch_path, and utils/event_channel - Event model extensions mirroring adk-python: Event.{output, route, nodeInfo, isolationScope}, the NodeInfo type, CreateEventParams, an isEvent guard, and additive optional EventActions fields; PRESERVE_KEYS entries so node output and checkpointed state survive snake/camel round-trips - export the new event types from common.ts Bundled tests: foundations, event_channel, event_model, and event. Recomposed from the final feature/workflows tree onto current main (3-way merged event.ts to preserve main's relocated function-call-id helpers). Stacked PRs follow. * refactor(events): brand Event with a signature symbol for isEvent Address PR1 review feedback: identify Event objects via a Symbol.for('google.adk.event') brand — set by createEvent and checked by isEvent — matching the signature-symbol guards used across ADK (isBaseTool, isBaseAgent, ...) instead of structural duck-typing. The brand is declared optional on the Event interface and is a non-serializable runtime marker, so events reconstructed from storage/session payloads are intentionally unbranded (documented on the interface and covered by a round-trip test). Adds isEvent unit tests (branded event, impostor rejection, non-object rejection, round-trip brand drop). * refactor(workflow): address PR1 review feedback Resolves review comments on #587: - events: drop 6 unused EventActions fields (output, joinCompleted, toolExecution, requestInput, nodeExecutionReplay, route) that were dead across the whole branch and left arbitrary `unknown` payloads unpreserved; removing them also eliminates the round-trip hole and the ambiguous second `route` carrier. Add shared RouteKey/Route types for Event.route. - errors: add name-based guards (isNodeInterruptedError, isNodeTimeoutError, isDynamicNodeFailError) so catch sites stay correct across duplicate-package boundaries instead of relying on `instanceof`. - branch_path: convert createSubBranch/commonPrefixOf from static methods to standalone util functions (kept fromString/commonPrefix as factories). - async_queue: fold EventChannel into the existing AsyncQueue instead of shipping a duplicate queue — port drain-before-error, sticky failure, and error()-closes; add isClosed/size getters and a fail() method (error() retained as an alias). Delete event_channel.ts and move its tests. This also fixes the close()-then-fail() error-swallow bug. - retry: introduce prepareRetryConfig/PreparedRetryConfig so a node's exception filter is normalized+validated once at construction, not re-normalized (and potentially thrown) on every retry check; make shouldRetryNode/ getRetryDelaySeconds take a required prepared config and drop the unreachable no-config branches (and the misleading test that pinned one). Full core suite green (2338 tests). * docs(events): drop @link to internal isEvent in the Event brand comment typedoc `docs:check` runs with --treatWarningsAsErrors and flagged the [EVENT_SIGNATURE_SYMBOL] doc comment (inherited by Event, CreateEventParams and CompactedEvent) for linking to `isEvent`, which is not part of the exported/ documented API. Reference it as inline code instead of a doc link. No API or behavior change. * refactor(workflow): apply workflow conventions to part 1 primitives Aligns the Part 1 shared primitives with the workflow conventions applied across the stack. - branch_path: convert the BranchPath.fromString / BranchPath.commonPrefix static methods to plain functions (branchPathFromString / commonPrefixOfPaths); keep the instance methods. Internal callers (createSubBranch, commonPrefixOf) updated. - retry_utils: shouldRetryNode and getRetryDelaySeconds now take a single params object; errorName is duck-typed (no `instanceof Error`) so it works for error-like values across package copies. - Add branch_path unit coverage (previously untested) and update the retry_utils call sites in foundations_test to the params-object form. Error type guards keep the name-based `instanceof Error` form by design (the one sanctioned use of instanceof).
…egistry (Part 2) (#588) * feat(workflow): add engine core — execution model, graph, and node registry Part 2/9 of the feature/workflows split. Adds the strongly-connected engine core, stacked on Part 1's primitives: - base_node: BaseNode abstract class + START sentinel - node_context / node_runner: per-node execution context and the run loop (retry, timeout/cancellation, streaming) - graph + utils/graph_parser + utils/graph_validation: chain/edge parsing, routing maps, and structural validation (START reachability, duplicate names/edges, DEFAULT_ROUTE, unconditional-cycle detection) - schedule_dynamic_node: dynamic scheduling primitive used by node_context - request_input + utils/hitl_utils: human-in-the-loop request primitives consumed by BaseNode Decoupling refactor (enables the layered split): utils/workflow_graph_utils no longer statically imports the concrete node classes. buildNode/isNodeLike now consult a self-registration registry (registerNodeBuilder, registerParallelWorkerFactory); concrete node modules register at load time (Parts 3+). This breaks the engine<->nodes import cycle. Builds on Part 1's review-driven APIs: streams events via the shared AsyncQueue (not a bespoke channel), reads branches via the createSubBranch util function, and normalizes retry configs once at construction (BaseNode.preparedRetryConfig + prepareRetryConfig) so the retry loop never re-normalizes or throws mid-retry. Bundled tests (28): node_execution, graph_parser, graph_validation, plus shared test_helpers. Full core suite green (2366 tests). * refactor(workflow): address PR review feedback Addresses the Part 2 review plus two follow-up requests (no static methods, no `instanceof`) and support for multiple schema formats. Review fixes: - Graph is validated at construction (`createGraphFromEdgeItems`) instead of leaving validation opt-in. - Route keys are matched by string value so a node emitting '2'/'true' can't silently miss its edge; drop the dead route-key guard branch. - `toContent` reuses `isContent` (single predicate) and handles JSON.stringify returning undefined. - Retry now clears the failed attempt's `stateDelta` (in place) before retrying; event-replay semantics documented on `executeChildNode`. - `withBranch` builds the child context via `new InvocationContext(ic)` (the ParallelAgent pattern) instead of a lossy double-cast spread. - Node registry is keyed by builder id (idempotent) with explicit priority ordering; parallel-worker factory rejects a conflicting re-registration; single-global-registry decision documented. - Accurate unconditional-cycle error message; engine-owned event `path` documented; typed `InvocationAbortedError` for abort-during-backoff. - Deduplicate the test harness onto `test_helpers` (no double casts; real `BaseAgent` + `createSession`). No static methods: convert `Graph.fromEdgeItems` and the `BranchPath` statics to plain functions. No `instanceof`: brand `BaseNode`/`Edge`/`RequestInput` and the workflow error classes with `Symbol.for('google.adk.*')` signatures and match on the brand (copy-safe); duck-type the retry error-name helper. Schema support: add `SchemaLike` (Zod v3 | Zod v4 | genai `Schema`) plus `parseWithSchema`/`toJsonSchema` in utils/schema.ts and reuse across base_node, workflow_graph_utils, and request_input/hitl_utils. * refactor(workflow): use instanceof (name-based) for error guards Revert the error classes back to `instanceof Error` + name matching for their type guards, dropping the Symbol.for brands. Brand-based guards are kept for the non-error types (BaseNode, Edge, RequestInput). * fix: fix issues. * test(workflow): add engine coverage; params-object helpers; toContent fix Adds essential unit coverage across the Part 2 surface, plus two follow-ups on the same iteration. Tests (6 new suites + extensions): - schema: parseWithSchema / toJsonSchema across Zod v3, Zod v4, and genai Schema (validation, passthrough, JSON conversion). - branch_path: fromString, append, isDescendantOf, common-prefix, and the createSubBranch / commonPrefixOf wrappers. - graph: isEdge brand, getNextPendingNodes routing (unconditional, specific, numeric/boolean string-coercion matching, route lists, fan-out, default-route precedence), createGraphFromEdgeItems validation. - workflow_graph_utils: isPlainObject / isNodeLike / buildNode and the registry (idempotency by id, priority + tie-break, parallel-worker factory conflict rejection). - base_node: isBaseNode / isContent brands and toContent across all branches (incl. circular-ref safety). - hitl: RequestInput + isRequestInput, createRequestInputEvent with responseSchema conversion, createRequestInputResponse, auth-resume. - foundations / node_execution: InvocationAbortedError guard, input-schema validation, and abort-during-retry-backoff. Refactor: helper/utility functions taking more than two arguments now accept a single destructured params object (executeChildNode, runOnce, enrichEvent, the graph_parser edge helpers, shouldRetryNode, getRetryDelaySeconds, processAuthResume); call sites updated. The fluent runNode and the Edge constructor stay positional. Fix: toContent no longer throws on an arbitrary node output — a plain object/number/boolean is serialized to text instead of being passed straight to createModelContent (which only accepts strings, Parts, or arrays of them). * refactor(workflow): replace node-builder registry with a static const list Drop the runtime node-builder registry (registerNodeBuilder / registerParallelWorkerFactory and the id/priority/idempotency/ conflict-rejection machinery it needed) in favor of a single explicit, statically-imported list. - Add node_builders.ts exporting NODE_BUILDERS (ordered; first match wins) and PARALLEL_WORKER_FACTORY. Node-type parts wire their builders in here instead of self-registering at import time. - buildNode / isNodeLike consult NODE_BUILDERS; buildNode uses PARALLEL_WORKER_FACTORY. NodeBuilder is now just {match, build} (no id / priority — order is precedence). This removes global mutable state and import-order side effects (the "builder only exists if the module was imported" fragility). The list is empty in the engine-core part; concrete node types populate it in their parts. * refactor(workflow): use condition loops in node_runner Replace the two infinite loops with condition-driven ones: the retry loop becomes while(!succeeded) (throw on a non-retryable failure), and the timeout drive-loop becomes while(!result.done). Behavior is unchanged.
* feat(workflow): add built-in Function and Tool nodes
Part 3/9 of the feature/workflows split, stacked on the engine core.
- nodes/function_node: wraps a plain function / async function / (async)
generator as a node; supports input/output schema validation and an auth
gate for HITL (the gate's processors land in Part 8).
- nodes/tool_node: wraps a BaseTool as a node.
Both self-register with the engine's node-builder registry
(registerNodeBuilder) at import time, so buildNode()/isNodeLike() — and thus
node()/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.
Tests (9): schema_validation (input/output schema coercion + rejection) and
node_builders (registry wiring: function -> FunctionNode with name resolution,
unnamed-function error, tool -> ToolNode, existing-BaseNode passthrough, and
isNodeLike). Full core suite green (2375 tests).
The node() user API and the auth-gate integration test land in later parts
(they need the runner/barrel).
* fix(workflow): address Part 3 review; route ToolNode through the tool chain
Conformance with the updated Part 2 (restores a green build):
- registerNodeBuilder now passes the required `id` ('function' / 'tool').
- processAuthResume is called with a single params object.
ToolNode — route through the canonical execution path
(agents/functions.ts::handleFunctionCallList) instead of calling
tool.runAsync directly. This restores the plugin before/after/onError tool
callbacks, the confirmation gate, telemetry, and full `actions`
propagation (stateDelta, artifactDelta, requested credentials /
confirmations), and emits a canonical `functionResponse` event with
`content` — fixing review comments on the bypassed contract, dropped
context actions, and the missing content.
- Deterministic function-call id (`${nodePath}:${runId}`) so credential /
confirmation resume can match across turns (was a fresh UUID per run).
- Throw for a long-running tool at construction (suspend machinery lands
later) rather than silently completing the call.
- Re-validate the coerced args against `inputSchema` before invoking, so
model-authored (Content-path) args are checked; reword the args error
("object", not "dictionary").
- Spread `config` before the name fallback so an explicit `undefined`
name can't clobber it (same fix in FunctionNode).
FunctionNode:
- Attach each written state key to an event only once per run (no more
re-emitting the growing delta on every generator item), and let a
handler's own event delta win over the context delta.
- Move the builder registration to the end of the module and drop the
dead string check in isSyncGenerator.
Tests: add ToolNode execution coverage (invocation + args coercion
branches, state propagation, plugin-chain override, long-running guard)
and FunctionNode coverage (generator/Content/null/Event results, state
de-dup + precedence, auth-gate interrupt + resume).
* refactor(workflow): wire Function/Tool builders via the static const list
Follow the registry removal from the engine core: the function and tool
builders now live in node_builders.ts (NODE_BUILDERS) instead of calling
registerNodeBuilder at module load. function_node.ts / tool_node.ts export
only their node classes; no import-time side effects.
* refactor(workflow): name builder consts in UPPER_SNAKE_CASE
Rename the module-level builder constants functionBuilder/toolBuilder to
FUNCTION_BUILDER/TOOL_BUILDER, matching the const naming used for
NODE_BUILDERS / PARALLEL_WORKER_FACTORY.
* feat(workflow): add ParallelWorker and JoinNode
Part 4/9 of the feature/workflows split, stacked on the built-in nodes.
- nodes/parallel_worker: runs a wrapped node once per item of a list input,
order-preserving, bounded by maxParallelWorkers, cancelling on first error
(a non-list input is treated as a single-element list). Registers a factory
with the engine (registerParallelWorkerFactory) so
buildNode(..., {parallelWorker: true}) works without a static import.
- nodes/join_node: a fan-in barrier that requires all predecessors and emits
the aggregated predecessor outputs as its output.
Tests (9): ParallelWorker mapping/order, single-item + empty-list handling,
concurrency bounding, first-error propagation, the registry factory
(buildNode + parallelWorker / maxParallelWorkers guard), and JoinNode
passthrough — all driven directly against a NodeContext. The graph-level
parallel/fan-in integration tests land in Part 6 with the runner. Full core
suite green (2384 tests).
* refactor(workflow): wire ParallelWorker factory via the static const
Follow the registry removal: PARALLEL_WORKER_FACTORY is set in
node_builders.ts instead of parallel_worker.ts calling
registerParallelWorkerFactory at import time. Update the engine-util test
now that the factory is present (parallelWorker wraps in a ParallelWorker).
* fix(workflow): address Part 4 review — ParallelWorker cancellation, bounds, retry
ParallelWorker:
- Don't apply retryConfig/timeout to the wrapper — they belong on the inner
node (per item), so the two levels no longer compose. Dropped from
ParallelWorkerConfig, the ParallelWorkerFactory options, and buildNode's
factory call.
- Bound default concurrency (DEFAULT_MAX_PARALLEL_WORKERS = 8) instead of
unlimited; pass Infinity for unbounded.
- Observe cancellation: the worker loop now stops claiming items when
ctx.abortSignal or the invocation's abort signal fires (documented as
stops-scheduling only — in-flight items still finish), and doesn't emit a
partial list on abort.
- Track failure with a dedicated `failed` flag so an item that rejects with
`undefined` still fails instead of leaving a silent hole.
- Give each child a distinct node path (overrideNodePath) so its events are
attributable, not just a distinct branch/runId.
- Doc: "stopping on first error" (nothing is cancelled), and state the
all-or-nothing semantics explicitly.
JoinNode: doc now says it emits its input unchanged (the engine supplies the
predecessor-name -> output map); the barrier is enforced by the orchestrator
via requiresAllPredecessors in a later part.
Tests: pin the concurrency peak (toBe), add default-bound / undefined-reject /
abort-stops-scheduling cases.
* refactor(workflow): use a condition loop in ParallelWorker's worker pool
Replace the infinite worker loop with while(!failed && !isAborted()) so the
termination conditions live in the loop header instead of an infinite loop
with internal breaks.
* feat(workflow): add the workflow runner and public API Part 5/9 of the feature/workflows split. Brings the engine together into a runnable workflow and exposes the public surface. - workflow.ts: the Workflow orchestrator — triggers, routing/fan-out, dynamic entry, and resume/fast-forward. - workflow_agent.ts: BaseAgent adapter so a Workflow runs under the ADK Runner (streams events via the shared AsyncQueue). - dynamic_node_scheduler.ts + utils/rehydration_utils.ts: dynamic scheduling and event-driven state reconstruction for resume. - node.ts: the node() user API. - workflow/index.ts + core/src/index.ts: public barrel exports; typedoc.json marks the internal AsyncQueue/ScheduleDynamicNode/NodeContextOptions as intentionally-not-exported. - register_builtin_nodes.ts: side-effect module imported by node()/workflow so the built-in Function/Tool/Parallel builders are registered even when those entry points are imported directly (not via the barrel). Adapted to Part 1's review APIs: workflow_agent uses AsyncQueue, workflow uses the commonPrefixOf util function. Tests (53): workflow, workflow_advanced, routing, parallel, dynamic_workflow, dynamic_resume, resume, runner_integration, auth_gate, hitl. Full core suite green (2444), docs:check clean, tsc clean. The LLM-agent-as-node tests (node_api, multi_agent, llm_agent) land in Part 7 with the agent builder. * fix(workflow): reconcile Part 5 runner with the updated engine Rebased Part 5 onto the current Part 4 and reconciled it with the convention changes from Parts 2-4: - Graph.fromEdgeItems -> createGraphFromEdgeItems (validates internally, so the separate validate() call is dropped). - executeChildNode now takes a single params object (workflow.ts and dynamic_node_scheduler.ts call sites updated). - Delete register_builtin_nodes.ts and its imports: with the static node-builder const list, importing node modules for side-effect registration is obsolete (node_builders.ts wires them, loaded via buildNode). - parallel_test.ts uses branchPathFromString (the static BranchPath factory is now a plain function). Also split the name-collided hitl_test.ts: the Part 3 request-input unit tests keep hitl_test.ts; Part 5's runner pause/resume tests move to hitl_flow_test.ts. * docs(workflow): fix TypeDoc link/reference warnings - Export SchemaLike (it is the type of the public BaseNode.inputSchema / outputSchema / stateSchema fields) from the public barrel. - Stop @link-ing internal, undocumented symbols from doc comments (parseWithSchema, toJsonSchema, isBaseNode, isEdge, REQUEST_INPUT_SIGNATURE_SYMBOL, DEFAULT_MAX_PARALLEL_WORKERS) — use plain inline code instead. `npm run docs:check` (typedoc --treatWarningsAsErrors) is now clean. * fix(workflow): correct config typing, results, cancellation & persistence Addresses the behavioral review feedback on PR #592 (Part 5 runner): - WorkflowConfig is now a discriminated union so exactly one of `edges` / `dynamicEntry` is required at compile time (the runtime throws stay for JS callers), retiring the `config.edges!` non-null assertion. - Introduce a real `NodeResult` type and widen `ctx.runNode()` / `ScheduleDynamicNode.schedule` to `NodeContext | NodeResult`, replacing the two `... as unknown as NodeContext` casts on the resume fast-forward paths that hid a "result is not a live context" bug from the compiler. - Cooperative sibling cancellation: a Workflow now owns an AbortController (chained to the invocation signal) threaded to each child via executeChildNode and aborted in cleanupPending, so an in-flight sibling stops when a node fails. The node runner exposes `ctx.abortSignal` on the non-timeout path too; an external abort lets a cooperative node wind down (no hard throw), while a `timeout` deadline still raises NodeTimeoutError. - WorkflowAgent drains inside try/finally, closing the channel and awaiting the producer if the consumer stops early (break or Runner cancel). - Restrict the plain-text HITL resume to the single-pending-interrupt case; a reply is no longer broadcast to every pause. - maxConcurrency must be a positive integer (0 no longer silently means "unlimited"; `undefined` is the sentinel), validated at construction. - Persist arbitrary node payloads verbatim across snake/camel round-trips: add `route` to both event PRESERVE_KEYS lists (output/agentState already present) and guard the agentState read with an isRecord narrowing instead of a cast. - Mark Workflow and WorkflowAgent @experimental; name the require-yield eslint-disable directives. * refactor(workflow): tighten and correctly publish the public API Addresses the public-surface review feedback on PR #592: - Rename the subclassing base class `Node` -> `WorkflowNode` (the `node()` factory is unchanged). `Node` shadowed the DOM / @types/node global in the flat @google/adk namespace and read confusingly next to `node()`; `WorkflowNode` lines up with `WorkflowAgent` / `WorkflowConfig`. Mark it @experimental. - Replace `export * from './workflow/index.js'` in index.ts with explicit named re-exports in common.ts, so the top-level surface is intentional (collisions become compile errors) and the workflow API also reaches the web entry point (index_web.ts re-exports common.ts). - Export the types reachable from the public NodeContext surface — AsyncQueue (via common.ts), NodeContextOptions / NodeResult / ScheduleDynamicNode / ScheduleDynamicNodeOptions (via the workflow barrel) — and drop the typedoc.json `intentionallyNotExported` block that was hiding, rather than fixing, that gap. `docs:check` stays clean. * test(workflow): share the driver harness and cover the review fixes Addresses the test-hygiene review feedback on PR #592: - Lift the duplicated `createIc()` / `driveWorkflow()` fixtures out of five test files (workflow, parallel, hitl_flow, dynamic_workflow, auth_gate) into the shared `test_helpers.ts` harness, removing ~200 lines and every `as unknown as Session` / `as unknown as BaseAgent` double cast (the shared helpers build a real Session via createSession and a real BaseAgent subclass). - Add coverage for the behavioral fixes: single-vs-multi pending plain-text resume (workflow_agent_test.ts), cooperative sibling cancellation on failure, maxConcurrency validation, and a snake/camel round-trip that verifies user-defined keys in output/route/agentState survive persistence. - node_execution_test uses executeChildNode where it needs the concrete child NodeContext (runNode's return type now widens to NodeContext | NodeResult).
…6) (#593) * feat(workflow): add LLM-agent-as-node, task mode, and node-as-tool Part 6/9 of the feature/workflows split. Lets agents participate in workflows. - nodes/llm_agent_wrapper: runs a BaseAgent as a workflow 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). It 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: exposes a node/workflow as a tool an agent can call. - tools/finish_task_tool: the finish_task tool backing task mode. - agents/llm_agent: task mode (mode/finishTaskTool) and registers the request-input + request-confirmation LLM request processors. - agents/processors/request_input_llm_request_processor: 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 public barrel (LLMAgentWrapper, NodeTool) and register_builtin_nodes; llm_agent 3-way merged onto current main. 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 green (2476), integration workflows green, docs:check + tsc clean. * fix(workflow): reconcile Part 6 with the updated engine Rebased Part 6 onto the current Part 5 and reconciled with the Parts 2-5 conventions: - executeChildNode now takes a single params object (node_tool.ts). - Move the LLM-agent-wrapper builder into the static node_builders.ts const list; drop its registerNodeBuilder self-registration and the now-obsolete register_builtin_nodes.ts (the const list replaces side-effect registration). * fix(workflow): address Part 6 review comments node-as-tool (node_tool.ts): - Bound node -> tool -> node recursion with a MAX_NODE_TOOL_DEPTH cap, tracked via a new immutable InvocationContext.nodeToolDepth carried through a depth+1 clone (so it survives agent-run ic clones). - Require the invocation event queue and a function-call id (throw otherwise) instead of falling back to a dead queue / a collapsing runId; drop the structural cast on eventQueue. - Pass an empty parent nodePath so the child path is a single segment, not the node name doubled. - Derive the tool parameter schema by narrowing isZodObject inline (drops the `as never`). instanceof -> brand guards: - llm_agent.ts uses isBaseNode; the request-input processor uses isNodeTool (new brand + guard on NodeTool). Both imports become type-only, removing the agents -> workflow value cycle. task mode (llm_agent_wrapper.ts): throw when a task-mode agent ends without a successful finish_task, instead of reporting the node COMPLETE with no output (its turn loop stays bounded by the invocation maxLlmCalls). InvocationContext: add clone(overrides) and use it for both the workflow-instruction-scope and node-tool-depth children (removes the `as unknown as InvocationContextParams` cast); remove the unused agentStates / endOfAgents fields; export WorkflowInstructionScope from common.ts / index.ts (public field type) instead of suppressing the TypeDoc warning. * test(workflow): use the shared createIc fixture (drop hand-rolled casts) Replace the duplicated hand-rolled createIc (with `as unknown as Session` / `as unknown as BaseAgent`) in the Part 6 workflow tests with the shared test_helpers.createIc (createSession + a real BaseAgent), removing the repeated double-casts the review flagged. * docs(workflow): de-link isNodeTool in NodeTool brand comment * fix(workflow): re-publish Part 6 nodes after the Part 5 export change Part 5 replaced the `export * from './workflow/index.js'` star in index.ts with explicit named re-exports in common.ts, so Part 6's public additions must be listed there too: - Add LLMAgentWrapper / NodeTool and the LLMAgentWrapperConfig type to the common.ts workflow block (they reach the web entry point this way, and the block now mirrors the barrel exactly). - Update node_api_test to subclass the renamed `WorkflowNode` base class.
…7) (#594) * feat(tools): add FunctionTool require_confirmation (human-in-the-loop approval) Part 7/9 of the feature/workflows split. - tools/function_tool: a `requireConfirmation` option so a FunctionTool pauses for human approval before executing. - agents/processors/request_confirmation_llm_request_processor: handles the confirmation request/resume round-trip for such tools. This tool-approval HITL is independent of the workflow engine (it works for any FunctionTool), so it is a small, self-contained slice. Tests: tools/function_tool_confirmation_test (5). Full core suite green (2481), docs:check + tsc clean. * fix(tools): gate and harden plain-text tool confirmation (PR #594) Addresses the security/API review on FunctionTool require_confirmation: - The plain-text confirmation fallback no longer runs on every LlmAgent invocation. It is now opt-in via a new `RunConfig.plainTextToolConfirmation` flag (default off), which the interactive `adk run` CLI sets — so on a web/API surface an ordinary chat message is never silently reinterpreted as a tool-gate decision. The structured FunctionResponse path is unchanged. - Harden the fallback itself: resolve only the SINGLE most-recent pending confirmation (never a broadcast across every unanswered gate), require the reply to IMMEDIATELY follow the request (no intervening user turn), and treat unrecognized text as NO decision — the gate stays pending instead of being silently denied (only explicit negatives deny). - Extract a `RequireConfirmation<TParameters>` type with a `toolContext` (not snake_case `tool_context`) parameter, reuse it for both the option and the field, and export it from common.ts. - Correct the `requireConfirmation` doc: the HITL gate is enforced on the LlmAgent path; a workflow ToolNode does not yet route through it (it returns the "requires confirmation" error as node output rather than pausing). - Inline the redundant `await` in runAsync and drop the stale comment. * test(tools): cover the confirmation resume round-trip (PR #594) - Add end-to-end tests that drive a session event list back through RequestConfirmationLlmRequestProcessor with a real LlmAgent + real FunctionTool (no mocks) and assert the original tool is actually re-invoked with the right decision — the step where an id mismatch on resume would show up, and the first coverage of the plain-text fallback: opt-in gating, single-gate binding, unrecognized-text-stays-pending, and no cross-gate broadcast. - Replace the `agent: ... as never` fixture with a real LlmAgent instance so it breaks if InvocationContext's contract changes.
* docs(workflow): add workflow samples Part 8/9 (final) of the feature/workflows split. Runnable examples covering the workflow API surface: - basics: sequence, loop, loop_self, route, multi_triggers, state, node_output, use_as_output, message - parallelism & dynamic: fan_out_fan_in, parallel_worker, dynamic_fan_out_fan_in, dynamic_nodes, nested_workflow - HITL & auth: request_input, request_input_advanced, request_input_rerun, auth_api_key, auth_oauth - agents & tools: agent_in_workflow, node_as_tool, retry - samples/workflows/README.md and a root `sample` script to run them Samples import only the public `@google/adk` surface and typecheck cleanly against source. * docs(workflow): address PR #595 sample review feedback - dynamic_nodes now uses a real `WorkflowConfig.dynamicEntry` (driving children via `ctx.runNode()`) instead of a static `edges` graph, so it actually demonstrates what the README row and Feature-coverage section claim — and the imperative loop is bounded by `MAX_ATTEMPTS` instead of `for (;;)`, so an off-topic input can't spin forever on live model calls. - parallel_worker sets `maxParallelWorkers: 2`, demonstrating the bounded concurrency the README advertises. - Normalize all nine sample headers that still used the raw `node dev/dist/esm/cli_entrypoint.js run ...` form to `npm run sample -- ...`, matching the README and the other samples. - Unify how the four HITL samples parse a human reply: normalize with `.trim().toLowerCase()` (so "Approve"/"approve " no longer fall through) and share one affirmative vocabulary, instead of three different idioms. - README: note that `loop`'s graph cycle is intentionally uncapped and can iterate many times; add a dynamicEntry bullet to Feature coverage. - Fix a prompt typo ("relates the the" -> "relates to the") in parallel_worker. * test(workflow): record/replay sample integration tests + per-test subfolders Add integration tests that run the real workflow samples end-to-end with only the model mocked, and reorganize tests/integration/workflows so every test lives in its own subfolder. - Harness (tests/integration/workflows/_harness/): a RecordReplayModel registered into LLMRegistry mocks the model boundary for every agent — including ones captured inside a dynamicEntry/ctx.runNode closure — matching recorded responses to requests by a stable, id-normalized fingerprint (concurrency/order independent). sample_harness runs the real sample rootAgent through an InMemoryRunner; record mode (RECORD_MODEL_RESPONSES=1) calls the live model and writes the fixture, replay is offline. rng provides a seeded PRNG for the model-free non-deterministic samples (retry, loop_self). - 21 of 22 samples covered, one folder each: agent.ts (vendored from the sample) + <sample>_test.ts + model_responses.json where model-backed; offline samples need no fixture. auth_oauth is skipped (needs a live OAuth provider). - Add `npm run record:samples` to re-record the model-backed fixtures. - Move the existing Part 6 workflow integration tests into per-test subfolders (workflow_test_utils.ts -> _harness/; node_as_tool_test.ts -> node_as_tool_llm/ to avoid colliding with the sample's node_as_tool/ folder), fixing relative imports only. No Part 6 test logic changed. * chore: drop the sampels * feat(workflow): let WorkflowAgent take Workflow options directly Adds an overload so the common case drops a layer of nesting: new WorkflowAgent({name: 'root_agent', edges: [...]}) instead of new WorkflowAgent(new Workflow({name: 'root_agent', edges: [...]})) When given a WorkflowConfig, the agent constructs the Workflow internally and takes its name/description from that config. The existing `new WorkflowAgent(workflow, {name, description})` form is unchanged and still supported, so this is purely additive. The two forms are told apart with the `isBaseNode` brand rather than `instanceof` (per the workflow conventions): a branded node is an already-built Workflow, anything else is config to build one from. * refactor(workflow): use the flattened WorkflowAgent signature in tests Adopt the new `new WorkflowAgent({name, edges})` form across the vendored agents under tests/integration/workflows, dropping the `new Workflow(...)` wrapper and one level of nesting: export const rootAgent = new WorkflowAgent({ name: 'root_agent', edges: [['START', processInput, classifyInput]], }); - Migrates the 20 vendored agents that build a Workflow (node_as_tool is a plain LlmAgent and is unchanged), dropping the now-unused `Workflow` import where nothing else needs it. nested_workflow keeps it for its sub-workflow node. - The samples this code was originally vendored from were removed in the preceding "chore: drop the sampels" commit, so only the self-contained test copies are updated here. The change is purely syntactic: the workflows built are identical, so the recorded model_responses.json fixtures still match and all 56 workflow integration tests pass unchanged.
7 tasks
added 2 commits
August 5, 2026 21:29
AdkApiServer defaults to port 0 (OS-assigned ephemeral port), and the `url` getter already resolved the real bound port from `server.address()`. initA2A() did not: it passed the raw configured value to toA2a(), which bakes it into the agent card transport URLs. With `--a2a --port 0` the served card therefore advertised http://localhost:0/a2a/<app>/jsonrpc, an unroutable destination, so no RemoteA2AAgent resolving that card could call back. Extract the existing address() narrowing into a private `boundPort` accessor so the `url` getter and the card URLs share one source of truth, and use it in initA2A(). initA2A() only runs from inside the listen callback, so the socket is already bound by then. The EADDRINUSE message keeps reporting the configured port: on that path nothing bound and address() is null. Behavior with an explicit non-zero port is unchanged.
…back Adds two tests to dev/test/server/adk_api_server_test.ts: - an A2A test that reuses the block's startA2aServer() helper, which configures no port so the OS assigns one. It derives the truth from `new URL(url).port` and asserts the served agent card's `url` and every `additionalInterfaces[].url` carry that bound port. - a Startup test asserting `url` reports the configured port before `start()` binds a socket, covering the `boundPort` fallback branch. Existing A2A and Startup tests are left untouched as the regression net for the unchanged paths.
AmaadMartin
force-pushed
the
fix/a2a-ephemeral-port-agent-card-url
branch
from
August 6, 2026 04:35
6ace461 to
a065eac
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Please ensure you have read the contribution guide before creating a pull request.
Link to Issue or Description of Change
N/A
Problem:
AdkApiServertreatsport: 0as "let the OS pick a free port", butinitA2A()passed the raw configured port totoA2a(). With--a2a --port 0the served agent card therefore advertisedhttp://localhost:0/a2a/<app>/jsonrpc. ARemoteA2AAgentresolves that card and builds its client from the card's own URL, so every call after card resolution failed.Solution: The
urlgetter already read the bound port offserver.address(). I extracted that read into a privateboundPortaccessor and used it in both places, so the card URLs andurlshare one source of truth.core/is deliberately unchanged:toA2a()only advertises the port it is given, so the defect was entirely in the caller.Two notes on scope. The
EADDRINUSEmessage still reports the configured port, because nothing bound on that path andaddress()isnull.tests/integration/is untouched; its harness maps a requested port of0onto a fixed random port, so it never reached the bug.Collision check:
gh pr list --repo AmaadMartin/adk-js --state open --limit 1000plusgh pr diff --name-onlyon every adjacent A2A and port PR (#646, #584, #546, #591, #314, #558). None touchdev/src/server/adk_api_server.ts. This PR updates an existing branch in place; I rebased it from a 51-commit-stale base onto currentmain.Testing Plan
Please describe the tests that you ran to verify your changes. This is required for all PRs that are not small documentation or typo fixes.
Unit Tests:
[x] I have added or updated unit tests for my change.
[x] All unit tests pass locally.
npx vitest run --project unit:dev dev/test/server/adk_api_server_test.ts— 55 passed. I added two tests and changed no existing test.Coverage of the changed lines is 100%. Both branches of the
boundPortgetter and theport: this.boundPortargument are absent from the uncovered-line list. The 84.22% figure reported for the whole file is pre-existing and unrelated.Proof each new test can fail. I ran both against unfixed code:
port: this.boundPort->port: this.portexpected 'http://localhost:0/a2a/testApp/jsonrpc' to be 'http://localhost:40863/a2a/testApp/jsonrpc'return this.port->return 0expected 'http://localhost:0' to be 'http://localhost:8123'I restored the source after each mutation and confirmed with
git diffthat none was left behind.No-regression check:
npx vitest run --project integration tests/integration/a2a/basic/a2a_agent_test.ts— 1 passed. That test drives a realRemoteA2AAgentover a non-zero port, so it proves the unchanged path.Manual End-to-End (E2E) Tests:
Please provide instructions on how to manually test your changes, including any necessary setup or configuration.
Before (server bound to 45359):
After (server bound to 37759):
I also pointed a
RemoteA2AAgentathttp://localhost:<bound>/a2a/weather_time_agent/on both builds. Before the fix the turn failed withA2ARemoteAgent remote_a2a_agent failed: TypeError: fetch failed. After the fix it returnedThe weather in New York is sunny with a temperature of 25 degrees Celsius (77 degrees Fahrenheit).Checklist
[x] I have read the CONTRIBUTING.md document.
[x] I have performed a self-review of my own code.
[x] I have commented my code, particularly in hard-to-understand areas.
[x] I have added tests that prove my fix is effective or that my feature works.
[x] New and existing unit tests pass locally with my changes.