refactor(controller): invoke agents directly in MCP handler#1855
Open
onematchfox wants to merge 2 commits into
Open
refactor(controller): invoke agents directly in MCP handler#1855onematchfox wants to merge 2 commits into
onematchfox wants to merge 2 commits into
Conversation
onematchfox
requested review from
iplay88keys,
jmhbh and
supreme-gg-gg
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors the controller’s MCP invoke_agent tool path to avoid an HTTP round-trip through the controller’s own A2A listener by invoking agents in-process via a new shared AgentClientRegistry owned/populated by A2ARegistrar.
Changes:
- Added
AgentClientRegistryand wiredA2ARegistrarto keep it in sync with agent add/update/delete events. - Updated MCP handler to send A2A messages via the registry (direct client invocation) and to recover auth sessions from
RequestExtraheaders for token propagation. - Updated app wiring and added MCP auth-propagation tests.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| go/core/pkg/app/app.go | Wires A2ARegistrar and passes its ClientRegistry() into the MCP handler. |
| go/core/internal/mcp/mcp_handler.go | Switches MCP invocation to registry-based A2A client calls; rehydrates auth session from MCP RequestExtra. |
| go/core/internal/mcp/mcp_handler_test.go | Adds coverage for auth propagation behavior through MCP -> A2A. |
| go/core/internal/a2a/agent_client_registry.go | Introduces a thread-safe registry of A2A clients keyed by agent route. |
| go/core/internal/a2a/a2a_registrar.go | Populates/cleans the registry alongside existing mux handler lifecycle. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+52
to
59
| // NewMCPHandler creates a new MCP handler that bridges MCP tool calls directly | ||
| // to agent A2A clients, bypassing the controller's own HTTP A2A listener. | ||
| func NewMCPHandler(kubeClient client.Client, agentClients *a2a.AgentClientRegistry, authenticator auth.AuthProvider) (*MCPHandler, error) { | ||
| handler := &MCPHandler{ | ||
| kubeClient: kubeClient, | ||
| a2aBaseURL: a2aBaseURL, | ||
| a2aTimeout: a2aTimeout, | ||
| agentClients: agentClients, | ||
| authenticator: authenticator, | ||
| } |
Comment on lines
181
to
191
| @@ -194,7 +189,6 @@ func (h *MCPHandler) handleInvokeAgent(ctx context.Context, req *mcpsdk.CallTool | |||
| }, InvokeAgentOutput{}, nil | |||
| } | |||
| agentRef := agentNS + "/" + agentName | |||
onematchfox
force-pushed
the
refactor-mcp-agent-invocation
branch
2 times, most recently
from
7a6babf to
496c320
Compare
This was referenced May 12, 2026
Replace the HTTP round-trip through the controller's own A2A listener with direct invocation via a new `AgentClientRegistry`. The registry is owned by `A2ARegistrar`, which already maintains an `A2AClient` per agent for its HTTP mux — the registry gives the MCP handler access to those same clients without an extra network hop. The old approach routed through the controller's public A2A endpoint, meaning requests could traverse the external network (and any ingress or load-balancer in front of it) unnecessarily. The new path stays in-process. The old handler also cached its own `A2AClient` per agent in a `sync.Map` with no eviction, so clients for deleted agents would remain indefinitely. The registry is kept consistent by the registrar's add/update/delete lifecycle, eliminating that staleness. `A2ARegistrar.upsertAgentHandler` writes to both the HTTP mux (for inbound /api/a2a/<ns>/<name>/ routing) and the registry (for direct invocation). The registry is exposed via `ClientRegistry()` and passed to `NewMCPHandler` in app.go. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Brian Fox <878612+onematchfox@users.noreply.github.com>
onematchfox
force-pushed
the
refactor-mcp-agent-invocation
branch
from
496c320 to
b4fde78
Compare
Contributor
Author
|
Now rebased and adjusted to account for #1853 being closed |
EItanya
reviewed
May 18, 2026
Comment on lines
+67
to
+69
| func (a *A2ARegistrar) ClientRegistry() *AgentClientRegistry { | ||
| return a.clientRegistry | ||
| } |
Contributor
There was a problem hiding this comment.
I think this can be avoided if we just construct the client registry outside of this component and pass it into here AND the mcp handler, what do you think? I personally prefer injecting dependencies that way.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replace the HTTP round-trip through the controller's own A2A listener with direct invocation via a new
AgentClientRegistry. The registry is owned byA2ARegistrar, which already maintains anA2AClientper agent for its HTTP mux — the registry gives the MCP handler access to those same clients without an extra network hop.The old approach routed through the controller's public A2A endpoint, meaning requests could traverse the external network (and any ingress or load-balancer in front of it) unnecessarily. The new path stays in-process.
The old handler also cached its own
A2AClientper agent in async.Mapwith no eviction, so clients for deleted agents would remain indefinitely. The registry is kept consistent by the registrar's add/update/delete lifecycle, eliminating that staleness.A2ARegistrar.upsertAgentHandlerwrites to both the HTTP mux (for inbound /api/a2a/// routing) and the registry (for direct invocation). The registry is exposed viaClientRegistry()and passed toNewMCPHandlerin app.go.Note:- currently includes changes from #1853 as well since this needs to adjust the tests added in that PR.