Repository navigation
Conversation
Signed-off-by: Evan Rauner <[email protected]>
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? |
kagent 0.10.0 Created-by: HarmonybrewBot Commit-by: HarmonybrewBot Merged-by: HarmonybrewBot Description: Created by `brew bump` --- Created with `brew bump-formula-pr`.<details> <summary>release notes</summary> <pre><!-- Release notes generated using configuration in .github/release.yml at v0.10.0 --> ## What's Changed ### Features * feat(streaming): configurable nginx, controller, and UI inactivity ti… by @yanivmn in kagent-dev/kagent#1973 * feat(helm): add controller service annotations by @anthonyhaussman in kagent-dev/kagent#2088 * feat: enable sharing of chat sessions by @onematchfox in kagent-dev/kagent#1935 * feat: substrate support for BYO and python runtimes for SandboxAgent CR by @jmhbh in kagent-dev/kagent#2043 * feat(substrate): bump agent substrate to 0.0.7 by @supreme-gg-gg in kagent-dev/kagent#2109 * feat(substrate): bump agent substrate to 0.0.8 by @jjamroga in kagent-dev/kagent#2140 * feat(helm/kagent): support deployment annotations by @anthonyhaussman in kagent-dev/kagent#2143 * feat: add nodeSelector support to agent helm charts by @the-mentor in kagent-dev/kagent#2145 * feat: expose icon, provider, documentation URL and version on agent A2A card by @QuentinBisson in kagent-dev/kagent#2188 * feat: add claude ACP sandbox target by @the-mentor in kagent-dev/kagent#2189 * feat(sandbox-agents): Store session state in `durableDir` volume for declarative sandbox agents by @jmhbh in kagent-dev/kagent#2171 * feat(helm): add UI HTTPRoute by @younsl in kagent-dev/kagent#2165 * feat(helm): bump bundled oauth2-proxy subchart to ~10.7.0 by @elihai-risk in kagent-dev/kagent#2254 * feat: default nodeSelector for agent deployments (--default-agent-node-selector) by @elihai-risk in kagent-dev/kagent#2259 * feat(helm): add podLabels for controller and UI pod templates by @elihai-risk in kagent-dev/kagent#2237 * feat(helm): add extraObjects for arbitrary manifests by @younsl in kagent-dev/kagent#2174 * feat(ui-test): Playwright ui tests foundation stage 0 by @cjlg-soloio in kagent-dev/kagent#2262 * feat: allow read-only shares to read A2A tasks by @QuentinBisson in kagent-dev/kagent#2191 * feat(controller): make Go (ADK) runtime agent image independently configurable by @QuentinBisson in kagent-dev/kagent#2021 * feat(helm): allow custom annotations on the default ModelConfig by @marians in kagent-dev/kagent#2201 * feat(ui-test): Playwright UI E2E suite — helpers + feature flows (stage 2) by @cjlg-soloio in kagent-dev/kagent#2264 * feat: serve A2A ListTasks from the task store by @QuentinBisson in kagent-dev/kagent#2187 * feat(models): add maxCompletionTokens for OpenAI by @anthonytwh in kagent-dev/kagent#2175 * feat(models): wire maxOutputTokens for native Gemini and Vertex by @anthonytwh in kagent-dev/kagent#2147 * feat: add envFrom support to agent deployment spec by @uncycler in kagent-dev/kagent#2321 * feat: add native AWS Bedrock Guardrails support to ModelConfig by @sanskar-singh-2403 in kagent-dev/kagent#2323 * feat(bedrock): configurable Bedrock read/connect timeout by @go-faustino in kagent-dev/kagent#2223 * feat: add deploymentAnnotations to agent deployment spec by @anthonyhaussman in kagent-dev/kagent#2359 * feat(helm/kagent): allow disabling default modelconfig by @anthonyhaussman in kagent-dev/kagent#2142 * feat(sts): send RFC 8707 resource and audience on token exchange by @QuentinBisson in kagent-dev/kagent#2106 * feat(sts): send RFC 8707 resource and audience on token exchange (Python) by @QuentinBisson in kagent-dev/kagent#2107 * Backport of 2407 (pg connection) to 0.10.x release by @supreme-gg-gg in kagent-dev/kagent#2399 * Backport 2409 (db session cleanup) to 0.10.x release by @supreme-gg-gg in kagent-dev/kagent#2403 * Backport of 2408 (s3 skills) to 0.10.x release by @supreme-gg-gg in kagent-dev/kagent#2405 * feat: configurable refresh interval for tool server controllers by @supreme-gg-gg in kagent-dev/kagent#2480 * feat: support file upload in agent chat by @supreme-gg-gg in kagent-dev/kagent#2429 ### Bug Fixes * fix(ui): chat interface overflows viewport causing input to be hidden below fold by @onematchfox in kagent-dev/kagent#2081 * fix(ui): avoid false stale chat send guard by @erauner12 in kagent-dev/kagent#2034 * fix(go-adk): extract system prompt from genai.GenerateContentConfig in Ollama and Bedrock model providers by @supreme-gg-gg in kagent-dev/kagent#2118 * fix(helm): support omitting image.registry in grafana-mcp chart by @michaelspinks in kagent-dev/kagent#2125 * fix(helm): support omitting image.registry in querydoc chart by @michaelspinks in kagent-dev/kagent#2124 * fix(security): patch CRITICAL/HIGH CVEs in golang-adk-full and app images by @jsonmp-k8 in kagent-dev/kagent#2133 * fix(go-adk): openai embeddings generation by @supreme-gg-gg in kagent-dev/kagent#2132 * fix(helm): omit empty repository segment in bundled image path by @michaelspinks in kagent-dev/kagent#2120 * fix: intermittent Postgres deadlock for concurrent memory searches by @supreme-gg-gg in kagent-dev/kagent#2146 * fix(deps): bump google-adk to 1.32.0 for Anthropic thinking blocks by @anxkhn in kagent-dev/kagent#2172 * fix: fix typos in part_converter comments by @mesutoezdil in kagent-dev/kagent#2195 * fix(adk): annotate serialize_metadata_value as str | dict by @anxkhn in kagent-dev/kagent#2173 * fix(ui): fix model name editing on the model config form by @Valyrian-Code in kagent-dev/kagent#1979 * fix: correct package comment in v1alpha2 groupversion_info by @mesutoezdil in kagent-dev/kagent#2183 * fix(crewai): unwrap Part before DataPart check so structured inputs are used by @anxkhn in kagent-dev/kagent#2231 * fix(helm): expose nodeSelector and tolerations for kagent-tools subchart by @the-mentor in kagent-dev/kagent#2213 * fix(helm): make the cluster DNS domain configurable by @shekhargit1912 in kagent-dev/kagent#2209 * fix: parse migration filenames the same way golang-migrate does by @mesutoezdil in kagent-dev/kagent#2194 * fix: build sandbox runtime outside QEMU emulation by @peterj in kagent-dev/kagent#2299 * fix: set kgateway.dev/a2a as appProtoco for BYO agents by @supreme-gg-gg in kagent-dev/kagent#2305 * fix(db): normalize agent name in memory vector search by @mesutoezdil in kagent-dev/kagent#2307 * fix(adk/bedrock): coerce nil tool-call args to empty object by @go-faustino in kagent-dev/kagent#2222 * fix: correct webhook typo in CLI help text by @tziyon31 in kagent-dev/kagent#2203 * fix(build): use AZURE_OPENAI_API_KEY env var name consistently by @mesutoezdil in kagent-dev/kagent#2310 * fix(security): scope task get, create, and delete to the owner by @mesutoezdil in kagent-dev/kagent#2265 * fix(adk): keep token refresh loop alive after a failed read by @mesutoezdil in kagent-dev/kagent#2330 * fix(acp-shim): drain child stdout during terminate to avoid teardown deadlock by @tomsen02 in kagent-dev/kagent#2338 * fix(adk): keep full session state when num_recent_events is set by @mesutoezdil in kagent-dev/kagent#2329 * fix(controller): wire up SessionSharesHandler in NewHandlers (#2363) by @onematchfox in kagent-dev/kagent#2420 * fix: ensure API key passthrough works for OpenAI embedding models by @onematchfox in kagent-dev/kagent#2494 * fix(adk): validate ask_user questions by @erauner12 in kagent-dev/kagent#2486 * Backport fix(golang-adk): resolve user identity from A2A call context to 0.10.x by @buravc in kagent-dev/kagent#2507 * fix(a2a): serve exact task reads from the persistent store by @erauner12 in kagent-dev/kagent#2484 * fix(ui): preserve post-login redirect target through `oauth2-proxy` sign-in by @onematchfox in kagent-dev/kagent#2532 ### Documentation * docs: fix duplicate descriptions and broken link in README by @mesutoezdil in kagent-dev/kagent#2068 * docs: fix typos and errors in component READMEs by @mesutoezdil in kagent-dev/kagent#2070 * docs: fix typos and list formatting in contributor docs by @mesutoezdil in kagent-dev/kagent#2069 * docs: replace outdated AutoGen reference in SECURITY.md by @mesutoezdil in kagent-dev/kagent#2110 * docs(python): fix kagent-langgraph README import to kagent.langgraph by @anxkhn in kagent-dev/kagent#2167 * docs(python): fix KAgentSession import path in kagent-openai README by @anxkhn in kagent-dev/kagent#2163 * docs: fix stale references and See merge request: Harmonybrew/homebrew-core!18335
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.