feat(a2a): add clientFactory to A2AAgent for auth support - #806
agent-of-mkmeral wants to merge 2 commits into
Conversation
Add clientFactoryOptions to A2AAgentConfig to allow configuring
authentication, custom transports, interceptors, and agent card
resolvers for the underlying A2A SDK ClientFactory.
Previously, A2AAgent created a bare ClientFactory() with no options,
which meant agent card resolution and message sending used plain
unauthenticated fetch calls. This caused 403 errors when connecting
to protected endpoints (SigV4, OAuth, bearer tokens).
Users can now pass Partial<ClientFactoryOptions> which gets merged
with SDK defaults via ClientFactoryOptions.createFrom(). This enables:
- Custom card resolvers with authenticated fetch
- Per-request interceptors for dynamic auth (token refresh)
- Custom transport factories
- Transport preferences
Example:
const agent = new A2AAgent({
url: 'https://protected-agent.example.com',
clientFactoryOptions: {
cardResolver: new DefaultAgentCardResolver({
fetchImpl: createAuthenticatingFetchWithRetry(fetch, handler),
}),
},
})
|
/strands review |
| * }) | ||
| * ``` | ||
| */ | ||
| clientFactoryOptions?: Partial<ClientFactoryOptions> |
There was a problem hiding this comment.
Issue: ClientFactoryOptions is not re-exported from src/a2a/index.ts. Users who want to construct clientFactoryOptions values will need to import the type directly from @a2a-js/sdk/client, which is not discoverable and requires knowledge of the underlying dependency.
Suggestion: Re-export ClientFactoryOptions from src/a2a/index.ts so users can import everything they need from @strands-agents/sdk/a2a:
export { ClientFactoryOptions } from '@a2a-js/sdk/client'This aligns with the "Prefer Flat Namespaces Over Nested Modules" decision record — users shouldn't need to import from transitive dependencies for common functionality.
|
Issue: The PR description mentions a "Documentation PR" section is missing. This change adds a new public configuration option ( Suggestion: Consider adding a documentation PR to |
|
Issue: The Suggestion: This is a pre-existing concern with the |
| }) | ||
| }) | ||
|
|
||
| it('passes agentCardPath to createFromUrl', async () => { |
There was a problem hiding this comment.
Issue: The agentCardPath test is in the clientFactoryOptions describe block but doesn't actually test clientFactoryOptions behavior — it tests the existing agentCardPath config passthrough to createFromUrl. This makes the test grouping misleading.
Suggestion: Move this test to a more appropriate describe block (e.g., the identity properties or invoke block), or rename the describe to something broader like client configuration.
|
Assessment: Comment Clean, well-scoped change that solves a real auth problem for protected A2A endpoints. The implementation is minimal and follows existing patterns well. Review Categories
The TSDoc with |
… options
Simplify the API: instead of accepting Partial<ClientFactoryOptions> and
reconstructing the factory internally, accept a ClientFactory directly.
The factory already handles card resolution and client creation. Users
configure their factory however they want (auth, interceptors, transports)
and pass it in. We just use it.
Before:
clientFactoryOptions: { cardResolver: myResolver }
After:
clientFactory: new ClientFactory({ ...myOptions })
- Removes ClientFactoryOptions import (no longer needed in our API surface)
- Simplifies _getClient() to a single nullish coalescing expression
- Updates tests to verify no extra factory is constructed when one is provided
| expect(mockCreateFromUrl).toHaveBeenCalledWith('http://localhost:9000', undefined) | ||
| }) | ||
|
|
||
| it('passes agentCardPath to createFromUrl', async () => { |
There was a problem hiding this comment.
Issue: The agentCardPath test at line 172 (without a custom factory) is grouped under the clientFactory describe block, but it tests the agentCardPath config passthrough to createFromUrl — not the clientFactory feature itself. The test at line 183 that combines agentCardPath + clientFactory does belong here.
Suggestion: Consider moving the standalone agentCardPath test (line 172) to the invoke describe block where connection/URL behavior is tested, keeping only factory-related tests in the clientFactory block.
|
Assessment: Approve Well-designed, minimal change that solves a real auth problem. The v2 approach of accepting a Review Details
The 1-line production change ( |
|
duplicate of #810 |
Summary
Add
clientFactory?: ClientFactorytoA2AAgentConfigto allow passing a pre-configured A2A SDKClientFactoryfor authentication, custom transports, interceptors, and agent card resolution.Problem
A2AAgentcreates a barenew ClientFactory()with no options, which means agent card resolution and message sending use plain unauthenticatedfetchcalls. This causes 403 errors when connecting to protected endpoints (SigV4, OAuth, bearer tokens).This is the same auth bug fixed in the Python SDK via PR #2103.
Solution
Accept an optional
clientFactory?: ClientFactory— a pre-configured factory instance. The factory already handles card resolution (createFromUrl) and client creation, so we just use it directly.When not provided, a default
new ClientFactory()is created (existing behavior).Usage
Changes
src/a2a/a2a-agent.tsclientFactoryOptionswithclientFactory: ClientFactory, simplify_getClient()tothis._config.clientFactory ?? new ClientFactory()src/a2a/__tests__/a2a-agent.test.tsKey Design Decision
We accept a
ClientFactoryinstance instead ofPartial<ClientFactoryOptions>because:A2AAgentinstancesTesting
Related