fix(cli): keep one broken agent from killing the dev server; surface HITL prompts and errors in adk run

[kalenkevich]

Link to Issue or Description of Change

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

Two devtools bugs found while manually exercising the new workflow engine, plus
the small core API the second one needed. One commit per concern.

---

Problem 1 — one malformed agent takes down the whole dev server

AgentLoader.loadAgentFromFile / loadAgentFromDirectory caught only
AgentFileLoadingError and rethrew everything else. But AgentFileLoadingError
means this file is not an agent; an agent that throws while constructing
raises a plain Error. Workflows validate their graph in the constructor, so
one bad graph escaped the handler, rejected the Promise.all in
preloadAgents, and broke every endpoint that lists or resolves agents.

Reproduced with 3 healthy agents + 1 invalid graph in the same directory:

GET /list-apps
{"error":"Failed to list apps: Error: Graph validation failed.
  The following nodes are unreachable from START: [\"b\",\"orphan\"]"}

Every app became unreachable and the dev UI listed nothing.

Solution: record load failures instead of propagating them. Broken agents are
skipped with an error log naming the file and the reason, healthy agents still
load, and the failure is reported against the app it belongs to via a new
listLoadFailures().

getAgentFile also returned undefined for an unknown name, surfacing later as
cannot read properties of undefined. It now throws either the recorded load
error or a not-found error listing the available agents.

GET /list-apps                    -> ["good_a","good_b"]
POST /run  (appName: broken_graph)
-> Agent 'broken_graph' failed to load from /.../broken_graph/agent.ts:
   Graph validation failed. The following nodes are unreachable from START: ["b","orphan"]

---

Problem 2 — human-in-the-loop prompts are invisible in adk run

The REPL printed only part.text. An interrupt is carried in a functionCall
part instead: RequestInput.message, the credential request, and a tool
confirmation hint all live in functionCall.args. So when a workflow paused,
adk run printed nothing and dropped back to [user]: with the run
blocked — the user had to already know a reply was expected, and answer blind.

Solution: render the three interrupt kinds with their message, payload /
response schema, auth scheme or tool hint, and a line saying how to answer.

The same turn also swallowed errors: a failure is reported on the event
itself (errorCode / errorMessage), not as a text part, so a turn blocked by
a safety filter or a model error simply ended with no output. Errors now print
to stderr as [author] error: CODE: message.

---

Supporting core change — UserInputRequest helpers

Detecting a pause is not CLI-specific: any client (the dev UI, an SDK consumer)
that wants to ask "is input required, and what for?" has to know that the three
interrupt kinds each store their id in a different place (functionCall.id,
args.interruptId, args.functionCallId) and their prompt under a different
key. Left in the CLI, that parsing gets reimplemented per surface — or skipped,
which is exactly how Problem 2 happened.

Adds to @google/adk:

getUserInputRequests(event): UserInputRequest[]          // what this event asks for
requiresUserInput(event): boolean                        // the simple "should I prompt?"
getPendingUserInputRequests(events): UserInputRequest[]  // still unanswered across a session

flattening all three encodings into one shape (kind, interruptId,
functionCallName, message, payload, responseSchema, toolName,
authConfig). A tool-confirmation hint is surfaced as message so a caller can
render any kind without a switch. getPendingUserInputRequests resolves
requests against later functionResponse parts — the question a UI actually
has is "is this session waiting on me right now?", not "was this event an
interrupt?" — and reports a re-raised id once, since a rerunOnResume node
raises the same id per attempt but the user still owes one answer.

renderUserInputRequest in the CLI is now formatting only.

Also deduplicates the interrupt names. They had three definitions between
them: adk_request_credential in both agents/functions.ts and
workflow/utils/hitl_utils.ts, adk_request_confirmation in both
agents/functions.ts and plugins/security_plugin.ts, and adk_request_input
in hitl_utils.ts plus a bare literal in tools/request_input_tool.ts. All now
live in agents/functions.ts; the other sites import or re-export, so no
public import path changes — verified all three still resolve to the same
values off @google/adk.

functions.ts sits in a pre-existing import cycle (functions -> llm_agent
-> request_input_llm_request_processor -> back to functions and
hitl_utils), and tools/request_input_tool.ts uses the name at module top
level — where a TDZ error would break on import rather than at call time. I
checked this empirically rather than by reasoning: imported the built ESM with
six different entry modules first (hitl_utils, functions,
security_plugin, user_input_request, request_input_tool, index), all
resolve correctly, because consumers use the constants lazily inside function
bodies. Worth knowing it stays safe only while that remains true — a future
top-level use inside the cycle would break. That hazard already applied to
the two names that always lived in functions.ts, so this does not make it
worse, but it does not fix it either. A zero-import leaf module would; happy
to do that instead if you'd prefer.

Testing Plan

Unit Tests:

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

npx vitest run --project unit:core --project unit:dev

Test Files  1 failed | 217 passed (218)
     Tests  1 failed | 3000 passed (3001)

The single failure is dev/test/cli/cli_create_test.ts — pre-existing and
environment-dependent (it reads the local gcloud project). It fails identically
on a clean checkout of main.

31 new tests:

• core/test/agents/user_input_request_test.ts (15) — each interrupt kind
  summarized; id fallback; several requests in one event; ordinary tool calls
  and plain text ignored; an interrupt with no id to answer ignored; pending
  resolution against responses, partial answers, re-raised ids, and ordering.
• dev/test/utils/agent_loader_test.ts (5) — a directory containing an agent
  that throws on construction still lists and loads the healthy agents; the
  failure is attributed to the right app; requesting the broken app rethrows the
  original error; an unknown name reports the available agents.
• dev/test/cli/cli_run_test.ts (11) — each interrupt kind renders its prompt;
  an event error is reported; ordinary and unnamed function calls do not
  announce a pause.

Two pre-existing test-harness problems were fixed along the way, both of which
were hiding real gaps:

afterEach(vi.restoreAllMocks) stripped the Runner implementation set in the
vi.mock factory, so from the second test onward runner.runAsync was
undefined. Nobody noticed because runAgent swallows errors into
console.log, which made the existing should run interactively by default
test pass vacuously. Runner is now re-established in beforeEach.

The same file faked @google/adk wholesale, so its assertions ran against
stand-in constants rather than shipped code. It now passes the real module
through with importOriginal and fakes only the Runner and services, so the
interrupt cases exercise the real detection logic.

Manual End-to-End (E2E) Tests:

Every claim above was verified end-to-end against real samples, before and after.

Problem 1 — a directory with healthy agents plus one invalid graph:

node dev/dist/esm/cli_entrypoint.js api_server <dir> --port 8150
curl -s localhost:8150/list-apps
# before: {"error":"Failed to list apps: ..."}      (all apps unusable)
# after : ["good_a","good_b"]                       (broken one skipped + logged)

Problem 2 — offline HITL sample, no API key needed:

$ adk run samples/workflows/human_input/get_started/agent.ts
[user]: start
--- [step1] is waiting for your input ---
Enter a number:
Type your reply at the next prompt to continue.
[user]: 21
[step2]: 42
[root_agent]: 42

Before this change, the block between the two [user]: prompts was empty.

Credential interrupt, offline, tests/integration/workflows/auth_api_key:

[user]: weather
--- [fetch_weather] is waiting for a credential ---
Please provide your API key.
Auth scheme: apiKey (header X-Api-Key)
Type the credential at the next prompt to continue.
[user]: SECRET123456
[fetch_weather]: {"city":"San Francisco",...,"apiKeyUsed":"SECR****"}
[summarize]: Weather for San Francisco: 18C, Sunny. (Authenticated with key: SECR****)

npx tsc --noEmit, npx eslint and npx prettier --check are clean.

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

Commits are split so each lands in the right release-please changelog — the
feat(core) in core/CHANGELOG.md, the two fix(cli) in dev/CHANGELOG.md:

5b550d8 docs: trim redundant comments
905696c fix(cli): report event errors in `adk run`
4e78e11 feat(core): add helpers to inspect pending requests for user input
d5a7383 fix(cli): show human-in-the-loop prompts in `adk run`
658944e fix(cli): isolate agent load failures so one bad agent can't down the server

Three smaller bugs turned up in the same pass and are not addressed here, to
keep this reviewable. Happy to send them separately:

• adk run mangles absolute paths — cli_run.ts does
  path.join(process.cwd(), agentPath). web / api_server get this right via
  getAbsolutePath().
• --save_session always fails: it joins onto the agent file path, producing
  agent.ts/<id>.session.json -> ENOTDIR. Its unit test mocks saveToFile,
  so CI does not catch it. One-line fix.
• The @experimental warning prints minified class names (Class kC is experimental) under the bundling loader, since the decorator reads
  constructor.name.

[AmaadMartin]

The load-failure isolation is right, and I verified the risky part of it: getAgentFile now throws where it used to return undefined, and no caller depended on that. All three call sites in adk_api_server.ts and the one in deploy_utils.ts either iterate listAgents() or dereference the result immediately, so the old path produced "cannot read properties of undefined" — this is strictly better. No test asserts the old return.

Four items below. The first two are real: the credential branch handles only one of the two encodings in the tree, and replaying a saved session re-announces answered pauses.

[AmaadMartin]

Not a nit. Replaying a saved session re-announces pauses the user already answered.

for (const event of loadedSession.events) {
  await sessionService.appendEvent({session, event});
  printEvent(event);
}

printEvent calls getUserInputRequests, which reports what an event asked for, with no knowledge of the reply. So --saved_session_file prints "is waiting for your input" and "Type your reply at the next prompt" for every historical interrupt, then hands over to the live prompt. The user cannot tell which pause is real.

getPendingUserInputRequests(loadedSession.events) answers exactly this, and this PR adds it. Render the transcript text here, then print the pending requests once after the loop.

[kalenkevich]

Agreed, that was wrong. Fixed in e72da44.

The replay loop now prints transcript text only — printEvent(event, {announcePauses: false}) — and getPendingUserInputRequests(loadedSession.events) is rendered once after the loop, so exactly the pauses no functionResponse ever answered, immediately before the live prompt.

Two tests: a transcript whose interrupt was answered prints no "is waiting" at all; a transcript with one answered and one open interrupt prints exactly one. Both fail against the previous code.

[AmaadMartin]

Nit. Two of these three exports have no caller.

export {
  getPendingUserInputRequests,
  getUserInputRequests,
  requiresUserInput,
} from './agents/user_input_request.js';

getUserInputRequests is used by cli_run.ts. requiresUserInput and getPendingUserInputRequests are referenced only by their own definitions and this line. AgentLoader.listLoadFailures() is in the same position: the description says a failure "is reported against the app it belongs to", but no server route calls it, and adk_api_server.ts is not in this diff.

My other comment gives getPendingUserInputRequests a real caller. For the remaining two, either add the caller in this PR or hold them back — a public export is hard to withdraw later.

[kalenkevich]

Fair. All three have real callers now rather than being held back:

• getPendingUserInputRequests — the saved-session replay in your other comment.
• requiresUserInput — runFromInputFile uses it to warn when a scripted --input_file run ends blocked on a human. That run has no prompt to answer at, so before this it just stopped mid-workflow with no output explaining why.
• AgentLoader.listLoadFailures() — served at GET /list-app-errors (name, file path, reason). That is the gap you are pointing at: /list-apps omits a broken agent, so it disappears from the dev UI and the only trace is a server log line. /list-apps keeps returning string[], so the UI client is untouched. Three server tests cover the route.

[kalenkevich]

Correction to the above: the /list-app-errors endpoint is out again (5113073). Exposing load failures over HTTP is a product decision, not something a bug-fix branch should settle, so it should not ride along here.

That leaves listLoadFailures() where your comment found it — a loader method with no server caller. Keeping it deliberately: it is dev-package API rather than a @google/adk export, so it is far cheaper to withdraw than the core helpers, and the surface it will feed is the route decision above. The behaviour that matters is unchanged either way: getAgentFile rethrows the original error for a named app, and every skipped agent is logged with its file and reason.

The other two callers in that reply stand: getPendingUserInputRequests in the saved-session replay, requiresUserInput in the --input_file warning.

[AmaadMartin]

Approved. All four comments are addressed, and I verified each against head 51130736 rather than the replies.

Two of your fixes are better than what I asked for. normalizeArgs rewrites credential args only, because the other kinds carry a caller-supplied payload whose keys must survive — I had said to normalize at the read site, which would have corrupted them. And the new credential test is built by calling generateAuthEvent itself, so the two encodings cannot drift apart; hand-rolling the shape was the actual weakness in the original tests.

One correction to my own comment: I wrote that authConfig and message were both undefined for the agent-auth flow. Only authConfig was a casing problem — generateAuthEvent writes no message key at all, so those pauses still render without one, while the doc on UserInputRequest.message says "Populated for every kind". Cosmetic, and your call.

One thing to coordinate: this raises TEST_EXECUTION_TIMEOUT in app_loader_test.ts to 60000 and #639 raises the same line to 80000. Whoever lands second gets a conflict, and the two disagree on the value.

[AmaadMartin]

Nit, and the revert was the right call — this is just the leftover.

Taking /list-app-errors out of a bug-fix branch is correct scoping; an HTTP surface for load failures is a product decision. But it puts listLoadFailures() back where my earlier comment found it: a public method with a test and no production caller.

Your own revert message lists the two ways a failure already reaches someone:

getAgentFile still rethrows the original error for a named app, and each skipped agent is still logged with its file and reason.

Both are true, and neither needs this method. Dropping it — with dev/test/utils/agent_loader_test.ts:756 — costs nothing and it comes back with the endpoint that wants it. Keeping it is fine too; it just wants a line saying it is there for the UI change.

Not blocking either way.