fix(adk): validate ask_user questions - #2486
Conversation
Signed-off-by: Evan Rauner <raunerevan@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR tightens validation for the built-in ask_user tool in both the Go and Python ADK runtimes to prevent creating unusable HITL interactions (e.g., empty questions or whitespace-only question text), aligning behavior with the intent of returning correctable tool-call errors instead of pausing execution without a usable prompt.
Changes:
- Add runtime validation to require at least one question and require each question’s text to be non-blank (Go + Python).
- Ensure invalid inputs fail fast (before calling the confirmation mechanism), while preserving pending/confirmed behavior for valid inputs.
- Add focused unit tests in both languages covering empty/blank rejection and confirmation-call behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| python/packages/kagent-adk/src/kagent/adk/tools/ask_user_tool.py | Adds validation for empty/blank questions and ensures confirmation is only requested for valid inputs. |
| python/packages/kagent-adk/tests/unittests/test_ask_user_tool.py | Adds async tests ensuring invalid inputs error without confirmation and valid flows remain unchanged. |
| go/adk/pkg/tools/ask_user.go | Adds input validation prior to confirmation request for empty/blank questions. |
| go/adk/pkg/tools/ask_user_test.go | Adds Go unit tests asserting invalid inputs don’t request confirmation and valid flows behave as expected. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| "Ask the user at least one question and wait for their answers before continuing. " | ||
| "Every question must include non-empty text. Use this when you need clarifying information, " | ||
| "preferences, or explicit confirmation from the user." |
| const askUserDescription = "Ask the user at least one question and wait for their answers before continuing. " + | ||
| "Every question must include non-empty text. Use this when you need clarifying information, " + | ||
| "preferences, or explicit confirmation from the user." |
Forward-port noteThis validation fix is independent of #2483–#2485. A main-oriented implementation is available in erauner12#14; its production behavior is equivalent to this PR. A forward-port should retain the provider-independent runtime guards and valid pending/confirmed behavior while adapting tests to current main APIs if they have moved. |
|
Thanks for the review! |
CI updateThe only remaining required check is Playwright E2E, failing on the unrelated missing-session UI assertion in The current Next step is to rerun the failed Playwright job once. GitHub does not permit the PR author to rerun this upstream workflow, so a maintainer will need to select Re-run failed jobs. If the same failure reproduces, I’ll inspect the uploaded trace/screenshot before proposing any change; no code change is warranted from this result alone. @supreme-gg-gg could you take a look? |
Summary
Validate built-in
ask_userinputs in both Go and Python ADK runtimes onrelease/v0.10.xbefore confirmation handling.Fixes #2468.
Problem
Both implementations accepted an empty
questionslist and entries whose question text was empty or whitespace-only. Those invalid calls could pause a task without presenting a usable question.An invalid model tool call should return an error the model can correct; it should not create an unanswerable
input-requiredinteraction.Change
Both runtimes now:
Runtime validation remains authoritative and does not depend on a provider enforcing a generated schema.
Tests
Go:
go test ./adk/pkg/toolsgo test -race ./adk/pkg/toolsPython:
uv run pytest packages/kagent-adk/tests/unittests/test_ask_user_tool.py packages/kagent-adk/tests/unittests/test_hitl.py -qCoverage verifies empty-list rejection, blank-text rejection, no confirmation call for invalid input, and unchanged valid pending/confirmed behavior in both runtimes.
Validation
An independent A2A client against a compatible v0.10 deployment completed a valid structured-question round trip: one nonblank question was returned, a later process answered the retained interaction, and the same task reached terminal completion without resending the original request.
This PR changes only built-in
ask_uservalidation; it does not change identity propagation, persistence, or A2A transport formats.