feat: add per-request handler factory overloads for multi-agent hosting - #382
feat: add per-request handler factory overloads for multi-agent hosting#382rokonec wants to merge 2 commits into
Conversation
- add MapA2A, MapWellKnownAgentCard, MapHttpA2A overloads with Func<HttpContext, T> factories - add combined MapA2A overload mapping both JSON-RPC and well-known agent card - make A2AJsonRpcProcessor.ProcessRequestAsync public for custom middleware - add 10 unit tests for factory overload registration and null validation 🔧 - Generated by Copilot
There was a problem hiding this comment.
Code Review
This pull request introduces new overloads for MapA2A, MapWellKnownAgentCard, and MapHttpA2A to support per-request resolution of handlers and agent cards using factory functions. Additionally, A2AJsonRpcProcessor.ProcessRequestAsync has been made public to support custom routing scenarios, and comprehensive unit tests for the new overloads have been added. Feedback was provided regarding the MapHttpA2A implementation, where factory calls are not wrapped in error handling for A2AException, potentially leading to inconsistent HTTP 500 responses instead of protocol-appropriate status codes.
🔧 - Generated by Copilot
| /// <returns>An endpoint convention builder for further configuration.</returns> | ||
| public static IEndpointConventionBuilder MapWellKnownAgentCard( | ||
| this IEndpointRouteBuilder endpoints, | ||
| Func<HttpContext, AgentCard> agentCardFactory, |
There was a problem hiding this comment.
This per-request card resolution is exactly what #353 was asking for — nice. One thing worth considering before it locks in: the factory signature is synchronous (Func<HttpContext, AgentCard>), and it's invoked synchronously in the GET handler just below (Results.Ok(agentCardFactory(httpContext))).
In practice, resolving the right agent card per request is often I/O-bound — looking it up from a tenant directory, a database, or a remote agent-definition service keyed off the subdomain/user in HttpContext. A synchronous signature forces those consumers into sync-over-async (.Result / .GetAwaiter().GetResult()) on the request path, which risks thread-pool starvation / deadlocks under load. #353 itself notes that v0.3's OnAgentCardQuery was awaitable, so async card resolution was a supported capability there.
Would you consider an async overload — Func<HttpContext, Task<AgentCard>> (or ValueTask<AgentCard>)? The GET handler can then await it cleanly:
routeGroup.MapGet(".well-known/agent-card.json",
async (HttpContext httpContext) => Results.Ok(await agentCardFactory(httpContext)));The same applies to the combined handler + card overloads in MapA2A and MapHttpA2A. Happy to help with the change if useful.
There was a problem hiding this comment.
FWIW the A2A.V0_3Compat layer in this same repo already takes an async card factory — Func<Task<AgentCard>> in MapAgentCardGetWithV03Compat (it awaits the factory before serving). Making the v1 overload async would keep it consistent with that existing precedent.
| /// <param name="agentCardFactory">Factory that resolves an <see cref="AgentCard"/> per request.</param> | ||
| /// <param name="path">The route prefix for all REST endpoints.</param> | ||
| /// <returns>An endpoint convention builder for further configuration.</returns> | ||
| public static IEndpointConventionBuilder MapHttpA2A( |
There was a problem hiding this comment.
This overload is the right call for multi-agent hosting. One maintainability thought: it re-declares the full HTTP+JSON route table (tasks / message / pushNotificationConfigs / extendedAgentCard) that the existing fixed-handler MapHttpA2A overload just above already declares. Two parallel route tables tend to drift over time — adding a new endpoint, or changing one (a new query param, an error-mapping tweak), has to be mirrored by hand in both, and it's easy to update one and miss the other.
Since the fixed-handler case is just "the same routing with a constant handler/card," you could keep both public overloads but have the fixed one delegate to this factory one, so the route table lives in a single place:
public static IEndpointConventionBuilder MapHttpA2A(
this IEndpointRouteBuilder endpoints, IA2ARequestHandler requestHandler, AgentCard agentCard, [StringSyntax("Route")] string path = "")
=> endpoints.MapHttpA2A(_ => requestHandler, _ => agentCard, path);To be clear, this keeps the fixed-handler overload exactly as-is for callers (it's the ergonomic single-agent API) — it just routes its body through the factory version instead of duplicating the route registrations. The same pattern applies to the MapA2A overloads. Happy to help if useful.
| Assert.Throws<ArgumentNullException>(() => app.MapWellKnownAgentCard((AgentCard)null!)); | ||
| } | ||
|
|
||
| // ── Factory overload tests ── |
There was a problem hiding this comment.
Nice to see the factory overloads covered. These tests verify the wiring — the endpoint registers, null args throw — but they don't exercise the behavior the overloads actually add: none of them send a request through the factory. A few behaviors worth a request-level test (e.g. via TestServer / WebApplicationFactory):
- The factory is actually invoked per request and the resolved handler processes it — ideally asserting that two requests can resolve to different handlers, since per-request resolution is the whole point of the overload.
- When the factory throws
A2AException, the response is the mapped JSON-RPC / HTTP error (the try/catch added here), not a 500. - The documented REST limitation (
Tenantalways null for these routes) — a small guard test would keep it from silently regressing.
As written, a regression in the per-request resolution or the error mapping would still pass these tests. Happy to help add a couple if useful.
Summary
Addresses #353 — adds per-request
Func<HttpContext, T>factory overloads toMapA2A,MapWellKnownAgentCard, andMapHttpA2A, enabling multi-agent hosting without adapter handler boilerplate. Also makesProcessRequestAsyncpublic for advanced custom middleware scenarios.Problem
The current
MapA2A,MapWellKnownAgentCard, andMapHttpA2Aall capture a singleIA2ARequestHandlerorAgentCardat startup. Enterprise platforms hosting many agents on one server (e.g., subdomain-per-user) had to implement a 70-lineIA2ARequestHandleradapter that delegates all 11 interface methods to the correct handler per request.New overloads
MapA2A(Func<HttpContext, IA2ARequestHandler>, path)MapA2A(Func<HttpContext, IA2ARequestHandler>, Func<HttpContext, AgentCard>, path)MapWellKnownAgentCard(Func<HttpContext, AgentCard>, path)MapHttpA2A(Func<HttpContext, IA2ARequestHandler>, Func<HttpContext, AgentCard>, path)Usage
Additional changes
A2AJsonRpcProcessor.ProcessRequestAsyncchanged frominternaltopublicwith XML docs — enables custom routing middlewareMapA2Afactory overload wraps the factory call in try/catch soA2AExceptionfrom the factory produces a proper JSON-RPC error response (not HTTP 500)Tests
Relates to #353