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
+44
to
+54
| type a2aBackend struct { | ||
| server *httptest.Server | ||
| lastAuthHeader string | ||
| } | ||
|
|
||
| func newA2ABackend(t *testing.T) *a2aBackend { | ||
| t.Helper() | ||
| b := &a2aBackend{} | ||
| b.server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| b.lastAuthHeader = r.Header.Get("Authorization") | ||
| resp := map[string]any{ |
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
+175
to
+177
| if extra := req.GetExtra(); extra != nil { | ||
| if session, err := h.authenticator.Authenticate(ctx, extra.Header, nil); err == nil { | ||
| ctx = auth.AuthSessionTo(ctx, session) |
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
from
d86531b to
7a6babf
Compare
… handlers The Go MCP SDK detaches the HTTP request context before dispatching to tool handlers. From the [SDK source](https://github.com/modelcontextprotocol/go-sdk/blob/v1.5.0/mcp/streamable.go#L485-L487): > // Pass req.Context() here, to allow middleware to add context values. > // The context is detached in the jsonrpc2 library when handling the > // long-running stream. This means the auth session placed by `AuthnMiddleware` is not visible via `auth.AuthSessionFrom(ctx)` in tool handlers. The SDK does preserve the original HTTP headers in [RequestExtra.Header](https://github.com/modelcontextprotocol/go-sdk/blob/v1.5.0/mcp/streamable.go#L1155-L1158) though. Re-authenticate from those headers at the top of handleInvokeAgent so the A2A client's outbound request to the agent carries the user's JWT. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Brian Fox <878612+onematchfox@users.noreply.github.com>
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
7a6babf to
496c320
Compare
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:
RequestExtrain tool handlers #1853 as well since this needs to adjust the tests added in that PR.