Ability to open sub-agent sessions in chat#1466
Ability to open sub-agent sessions in chat#1466dobesv wants to merge 2 commits intokagent-dev:mainfrom
Conversation
a261d04 to
115b9c0
Compare
There was a problem hiding this comment.
Pull request overview
Adds end-to-end support for opening sub-agent chat sessions from a parent chat by correlating agent tool calls to the spawned child session, using propagated metadata and a new backend lookup endpoint.
Changes:
- UI: Passes parent
sessionIddown to agent tool call rendering and adds an “open in tab” button that looks up and opens the sub-agent session. - Python (kagent-adk/core): Propagates caller metadata (session/tool-call identifiers + user identity) across A2A boundaries and injects it into child task metadata via a new plugin + request metadata provider.
- Go (core): Adds
GET /sessions/{session_id}/subagentsessions/{tool_call_id}and database lookup logic to resolve a child session ID from task metadata; includes tests.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| ui/src/components/chat/ToolCallDisplay.tsx | Threads sessionId into AgentCallDisplay. |
| ui/src/components/chat/ChatMessage.tsx | Adds sessionId prop and passes it to tool-call rendering. |
| ui/src/components/chat/ChatInterface.tsx | Provides the active session id down to message rendering. |
| ui/src/components/chat/AgentCallDisplay.tsx | Adds “open in tab” button that fetches sub-agent session ID and opens it. |
| ui/src/app/actions/sessions.ts | Adds getSubAgentSession action calling the new backend endpoint. |
| python/packages/kagent-core/src/kagent/core/a2a/_consts.py | Introduces constants for caller metadata keys/prefix. |
| python/packages/kagent-core/src/kagent/core/a2a/init.py | Exposes new caller metadata constants via module imports. |
| python/packages/kagent-adk/src/kagent/adk/types.py | Adds A2A request metadata provider and wires it into RemoteA2aAgent. |
| python/packages/kagent-adk/src/kagent/adk/converters/event_converter.py | Includes kagent_* state metadata in emitted event metadata. |
| python/packages/kagent-adk/src/kagent/adk/_agent_executor.py | Applies propagated user_id and persists kagent_* metadata into task metadata. |
| python/packages/kagent-adk/src/kagent/adk/_session_linking_plugin.py | New plugin that injects parent session/tool-call metadata into session state before AgentTool runs. |
| python/packages/kagent-adk/src/kagent/adk/cli.py | Registers SessionLinkingPlugin alongside STS integration. |
| python/packages/kagent-adk/tests/unittests/converters/test_event_converter.py | Updates mock invocation context to include session.state. |
| go/core/internal/httpserver/server.go | Registers new sub-agent session lookup route. |
| go/core/internal/httpserver/handlers/sessions.go | Adds handler to resolve child session and return session info. |
| go/core/internal/httpserver/handlers/sessions_test.go | Adds handler tests for the new endpoint. |
| go/api/database/client.go | Extends DB client interface with GetSubAgentSession. |
| go/core/internal/database/client.go | Implements DB query to find child session via task metadata. |
| go/core/internal/database/fake/client.go | Implements in-memory GetSubAgentSession for handler tests. |
| go/core/internal/database/client_test.go | Adds DB-level test for GetSubAgentSession. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
python/packages/kagent-adk/src/kagent/adk/_session_linking_plugin.py
Outdated
Show resolved
Hide resolved
44f0858 to
0013634
Compare
When viewing a chat session with sub-agent calls, a new "open in tab" icon button will show. Clicking the button looks up the session ID of the sub-agent session and opens it in a new tab. To support this: - the python agent was updated to propagate the parent session ID and tool call ID into the child task information as metadata field `kagent_caller_session_id` and `kagent_caller_tool_call_id` - an endpoint was added to the go server to lookup a child session ID by finding the task with the matching parent session ID and tool call and using the session ID from that task to identify the session Signed-off-by: Dobes Vandermeer <dobes.vandermeer@newsela.com>
0013634 to
fdb0e35
Compare
jsonmp-k8
left a comment
There was a problem hiding this comment.
Review: Ability to open sub-agent sessions in chat
Clean end-to-end implementation. The plugin approach for injecting metadata into session state before AgentTool clones it is a good design choice — avoids modifying AgentTool internals. The Go endpoint, fake client, and handler tests are solid. Some issues below.
1. URL construction from display-formatted name will break for namespaced agents
AgentCallDisplay.tsx builds the sub-agent URL from agentDisplay:
const agentDisplay = useMemo(() => convertToUserFriendlyName(call.name), [call.name]);
// ...
const subagentSessionUrl = `/agents/${agentDisplay}/chat/${response.data.id}`;The route pattern is /agents/{namespace}/{name}/chat/{sessionId} (two path segments), but agentDisplay is a single display string. For a tool named default__NS__k8s_agent, convertToUserFriendlyName produces a flat display name (e.g. k8s-agent), losing the namespace and producing /agents/k8s-agent/chat/... instead of /agents/default/k8s-agent/chat/....
This works only when the namespace happens to not be required by the routing, or when you're testing with agents whose names match the display transformation. Build the URL by splitting call.name on __NS__ to extract namespace and name separately.
2. No tests for SessionLinkingPlugin
The plugin (62 lines) manipulates session state before AgentTool runs, which determines what metadata ends up in child tasks. It accesses a private attribute (tool_context._invocation_context), has conditional logic (early return if not AgentTool, warning if no invocation context), and the correctness of the entire feature depends on it injecting the right keys. None of this is tested.
At minimum:
- Verify it injects the three expected metadata keys into
tool_context.stateforAgentTool - Verify it's a no-op for non-
AgentTooltools - Verify it handles missing
_invocation_contextgracefully
3. Blanket kagent_* propagation may grow metadata unboundedly
Both the event converter and executor blindly propagate every kagent_*-prefixed key from session state / request metadata:
# event_converter.py
for key, value in invocation_context.session.state.items():
if key.startswith(KAGENT_METADATA_KEY_PREFIX):
metadata[key] = _serialize_metadata_value(value)
# _agent_executor.py
for key, value in context.metadata.items():
if key.startswith(KAGENT_METADATA_KEY_PREFIX):
run_metadata[key] = valueToday this is just 3 keys, but any future feature that puts kagent_* keys in session state will have them silently propagated into every A2A event and every task's metadata. This is an implicit coupling that's easy to miss. Consider either:
- Using an explicit allowlist of keys to propagate
- Documenting that
kagent_*session state keys are automatically propagated (so future developers know)
4. GetSubAgentSession — JSON column scan with no index
The query does JSON extraction on every row in the task table:
query.Where("json_extract(data, '$.metadata.kagent_caller_session_id') = ?", sessionID)No index exists on these JSON paths. For deployments with many tasks, this is a full table scan. Worth adding a comment noting this, or creating a generated/virtual column with an index if performance becomes an issue.
5. Plugin accesses private _invocation_context
invocation_context = tool_context._invocation_contextThis is an ADK internal. If the ADK refactors this, the plugin breaks silently (attribute access returns None → early return → metadata not propagated → sub-agent sessions unlinkable). No public API alternative may exist, but worth a comment explaining the dependency and why it's necessary.
6. sessionId! non-null assertion
ChatInterface.tsx:692:
sessionId={sessionId!}If sessionId is null at this point, the ! suppresses the type checker and passes undefined through silently. The "open in tab" button would then show a toast error. Either guard with sessionId ?? undefined and handle it in AgentCallDisplay, or assert it's non-null earlier in the render logic.
7. Sub-agent sessions polluting the agent chat list
The PR description acknowledges: sub-agent sessions now appear in the agent's chat list as if they were the user's own conversations. This could be confusing — users might see sessions they didn't directly initiate. Consider filtering these out by checking for the presence of kagent_caller_session_id in task metadata. Even if deferred, it should be tracked as a follow-up.
Minor
window.open(url, '_blank')— addnoopenerfor defense-in-depth, even for internal links:window.open(url, '_blank', 'noopener')Pluck("session_id", &sessionName)with scalar destination is undocumented GORM usage (documented for slices). Works in practice but fragile across GORM versions._kagent_metadata_provideraccessesctx.session.user_id— if a session exists withoutuser_idset, this raisesAttributeError. Guard withgetattr(ctx.session, "user_id", None).
Summary
| Issue | Severity |
|---|---|
| URL built from display name, not raw namespace/name | High — broken links for namespaced agents |
No tests for SessionLinkingPlugin |
Medium |
Blanket kagent_* propagation |
Medium — implicit coupling |
| JSON query with no index on task table | Low-Medium (scales poorly) |
Private API _invocation_context access |
Low (fragile but no alternative) |
sessionId! non-null assertion |
Low |
| Sub-agent sessions in chat list | UX — track as follow-up |
|
Superceded by #1513 |
When viewing a chat session with sub-agent calls, a new "open in tab" icon button will show.
Clicking the button looks up the session ID of the sub-agent session and opens it in a new tab.
To support this:
kagent_user_id,kagent_caller_session_id, andkagent_caller_tool_call_idA side-effect of this change is that if you click an agent you can now see the sub-agent sessions listed there as if they were your own chats. Personally I prefer it, but if this is a problem we could look for a way to hide those, maybe filter out sub-agent sessions on that list based on the metadata fields we use to find sub-agent sessions.
Signed-off-by: Dobes Vandermeer dobes.vandermeer@newsela.com