fix(go-adk): surface the sub-agent's ask_user question in RemoteHitlHint - #2475
Conversation
be0a702 to
eb714e3
Compare
supreme-gg-gg
left a comment
There was a problem hiding this comment.
This fix makes sense to me, two comments and this is ready to go
askUserQuestionText() type-asserted a nested ask_user HitlTool's Args["questions"] as []map[string]any, but Args round-trips through JSON into a plain map[string]any, which decodes "questions" as []any instead — silently dropping the question for nested (two-level) ask_user pauses. AskUserRequest.Questions is already correctly typed in both the direct and nested case (see BuildHITLStatusMessage), so read it directly instead of re-deriving from VisibleTools() output. Addresses review feedback from supreme-gg-gg on kagent-dev#2475. Signed-off-by: Vivien Ramahandry <56304555+vramahandry@users.noreply.github.com>
eb714e3 to
9c3ac9e
Compare
| if state == nil { | ||
| return "Remote agent requires human input before continuing." | ||
| } | ||
| // AskUserRequest.Questions carries the real question text whether or not |
There was a problem hiding this comment.
nit: this inline comment seems a bit too long (but not worth blocking on this)
There was a problem hiding this comment.
Trimmed in 40bec6b, along with the similarly verbose comments added alongside it.
…hitl_hint (#2495) ## Summary `remote_hitl_hint()` in the Python `kagent-adk` runtime builds the hint text shown to a human when a sub-agent's own HITL pause bubbles up to the parent agent. Unlike the Go runtime's `RemoteHitlHint()`, it only ever lists the paused tool's *name* (e.g. `ask_user`) — never the actual question — even though `AskUserRequest.questions` (set directly on the request in `build_hitl_status_message`, whether or not `nested` is populated) has the real content right there. - Before: `"Remote agent 'github_agent' requires approval for tool(s): ask_user"` - After: `"Remote agent 'github_agent' asks: What is the GitHub owner/org for the repo?"` Real tool-approval hints (the non-`ask_user` case) are unaffected. This is the Python-runtime counterpart to #2475, which fixed the same class of bug in `go/adk/pkg/a2a/hitl.go`. The Go and Python runtimes maintain independent implementations of this HITL hint logic, and the Python side never had question-surfacing added — so a parent agent built on `kagent-adk` still drops the question today even after #2475 merges. Unlike the Go fix, there's no `[]any` vs `[]map[string]any]` JSON-decode subtlety to worry about here: Pydantic's `HitlTool.args: dict[str, Any]` keeps nested question dicts intact, so reading `AskUserRequest.questions` directly is straightforward in both the direct and nested case. Fixes #2473 ## Changes - `python/packages/kagent-adk/src/kagent/adk/_hitl.py`: `remote_hitl_hint()` now checks `AskUserRequest.questions` first and returns `"Remote agent '{name}' asks: {question}"` when present, falling back to the existing tool-name-only wording otherwise. - `python/packages/kagent-adk/tests/unittests/test_hitl.py`: adds `test_remote_hitl_hint_tool_approval`, `test_remote_hitl_hint_ask_user`, and `test_remote_hitl_hint_ask_user_nested` (the last covering a two-level nested `ask_user` pause). ## Test plan - [x] `uv run pytest packages/kagent-adk/tests/unittests/test_hitl.py` — 18 passed (3 new) - [x] `uv run ruff format --diff` / `uv run ruff check` — clean - [x] Existing tests unaffected — no changes to tool-approval hint wording Signed-off-by: Vivien Ramahandry <56304555+vramahandry@users.noreply.github.com>
|
@vramahandry this commit is not properly signed off: 40bec6b you need to do that for DCO to pass |
2675a2b to
bc38318
Compare
…hitl_hint (kagent-dev#2495) ## Summary `remote_hitl_hint()` in the Python `kagent-adk` runtime builds the hint text shown to a human when a sub-agent's own HITL pause bubbles up to the parent agent. Unlike the Go runtime's `RemoteHitlHint()`, it only ever lists the paused tool's *name* (e.g. `ask_user`) — never the actual question — even though `AskUserRequest.questions` (set directly on the request in `build_hitl_status_message`, whether or not `nested` is populated) has the real content right there. - Before: `"Remote agent 'github_agent' requires approval for tool(s): ask_user"` - After: `"Remote agent 'github_agent' asks: What is the GitHub owner/org for the repo?"` Real tool-approval hints (the non-`ask_user` case) are unaffected. This is the Python-runtime counterpart to kagent-dev#2475, which fixed the same class of bug in `go/adk/pkg/a2a/hitl.go`. The Go and Python runtimes maintain independent implementations of this HITL hint logic, and the Python side never had question-surfacing added — so a parent agent built on `kagent-adk` still drops the question today even after kagent-dev#2475 merges. Unlike the Go fix, there's no `[]any` vs `[]map[string]any]` JSON-decode subtlety to worry about here: Pydantic's `HitlTool.args: dict[str, Any]` keeps nested question dicts intact, so reading `AskUserRequest.questions` directly is straightforward in both the direct and nested case. Fixes kagent-dev#2473 ## Changes - `python/packages/kagent-adk/src/kagent/adk/_hitl.py`: `remote_hitl_hint()` now checks `AskUserRequest.questions` first and returns `"Remote agent '{name}' asks: {question}"` when present, falling back to the existing tool-name-only wording otherwise. - `python/packages/kagent-adk/tests/unittests/test_hitl.py`: adds `test_remote_hitl_hint_tool_approval`, `test_remote_hitl_hint_ask_user`, and `test_remote_hitl_hint_ask_user_nested` (the last covering a two-level nested `ask_user` pause). ## Test plan - [x] `uv run pytest packages/kagent-adk/tests/unittests/test_hitl.py` — 18 passed (3 new) - [x] `uv run ruff format --diff` / `uv run ruff check` — clean - [x] Existing tests unaffected — no changes to tool-approval hint wording Signed-off-by: Vivien Ramahandry <56304555+vramahandry@users.noreply.github.com>
bc38318 to
1b8ac72
Compare
RemoteHitlHint() only listed the paused tool's name ('ask_user'), never
the actual question text, even though HitlTool.Args already carries it
via VisibleTools(). A human relayed a bubbled-up sub-agent HITL pause
saw 'requires approval for tool(s): ask_user' with no way to know what
was actually being asked.
Fixes kagent-dev#2473
Signed-off-by: Vivien Ramahandry <56304555+vramahandry@users.noreply.github.com>
askUserQuestionText() type-asserted a nested ask_user HitlTool's Args["questions"] as []map[string]any, but Args round-trips through JSON into a plain map[string]any, which decodes "questions" as []any instead — silently dropping the question for nested (two-level) ask_user pauses. AskUserRequest.Questions is already correctly typed in both the direct and nested case (see BuildHITLStatusMessage), so read it directly instead of re-deriving from VisibleTools() output. Addresses review feedback from supreme-gg-gg on kagent-dev#2475. Signed-off-by: Vivien Ramahandry <56304555+vramahandry@users.noreply.github.com>
supreme-gg-gg flagged the inline comment as too long; tightened it and the similarly verbose doc/test comments added in the same change while keeping the JSON round-trip explanation. Signed-off-by: Vivien Ramahandry <56304555+vramahandry@users.noreply.github.com>
1b8ac72 to
3a338cf
Compare
|
Hello @supreme-gg-gg , sorry I forgot the DCO. |
|
Hello @supreme-gg-gg is there any issue of not being merged ? |
|
Hello @EItanya do you know when it will be released to a tagged version ? |
If you need this on 0.10 you will need to backport it. Hopefully we'll be starting beta releases on main early next week |
How I can backport myself ? Do I create a new tag on my fork from the latest tag v0.10.1 ? |
Open a PR against |
|
Hello @EItanya, Tried cherry-picking #2396 alone onto If this is needed on 0.10.x, the options are either porting the A2A v1.0 + HITL extension migration itself (multi-day effort, real risk to the release branch), or a small hand-written fix against the old |
Summary
RemoteHitlHint()builds the hint text shown to a human when a sub-agent's own HITL pause bubbles up to the parent agent. Today it only lists the paused tool's name (e.g.ask_user) — never the actual question — even thoughAskUserRequest.Questions(already threaded throughVisibleTools()intoHitlTool.Args["questions"]) has the real content right there."Remote agent 'github_agent' requires approval for tool(s): ask_user""Remote agent 'github_agent' asks: What is the GitHub owner/org for the repo?"Real tool-approval hints (the non-
ask_usercase) are unaffected — same wording as before, existingTestBuildRemoteHitlStateAndHintstill passes unchanged.Discovered while building an external A2A chat bridge that correctly renders every other kagent HITL flow (direct tool approval, direct
ask_user) from the same session — this was the one case where the confirmation card reaching a downstream A2A client carried no actionable information, because the hint itself never had it. Confirmed by inspecting the sub-agent's own ADK session directly (GET /api/sessions/<id>), where the real question is visible inadk_request_confirmation— it's just discarded before reachingRemoteHitlHint.Fixes #2473
Changes
go/adk/pkg/a2a/hitl.go: newaskUserQuestionText()helper extracts question text from anask_userHitlTool's args;RemoteHitlHint()uses it when present, falling back to the existing tool-name-only wording otherwise.go/adk/pkg/a2a/hitl_test.go: newTestBuildRemoteHitlStateAndHintAskUsercovering the fixed behavior.Test plan
go build ./adk/...— cleango test ./adk/...— all packages pass (a2a, agent, app, tools, etc.)gofmt -l/go vet ./adk/pkg/a2a/...— cleanTestBuildRemoteHitlStateAndHint(tool-approval case) passes unchanged