feat(a2a): add client_config param and deprecate a2a_client_factory#2103
feat(a2a): add client_config param and deprecate a2a_client_factory#2103agent-of-mkmeral wants to merge 4 commits intostrands-agents:mainfrom
Conversation
Add client_config: ClientConfig parameter to A2AAgent for configuring authentication and transport settings. This fixes the auth bug where get_agent_card() created a bare httpx.AsyncClient, ignoring any authentication configured via a2a_client_factory. Changes: - Add client_config parameter (recommended way to configure auth) - Deprecate a2a_client_factory with DeprecationWarning - Fix get_agent_card() to use client_config.httpx_client for card resolution, enabling authenticated card discovery (SigV4, OAuth, etc.) - Fix empty string bug: use 'is not None' instead of falsy check for name/description from agent card - Add asyncio.Lock for concurrent card resolution (double-checked locking) - When both client_config and factory are provided, client_config is used for card resolution while factory is used for client creation (preserving interceptors, consumers, custom transports) - 37 tests covering all paths The a2a_client_factory parameter is preserved for backwards compatibility. Existing factory users keep all features (interceptors, consumers, custom transports) and get the card resolution fix via config fallback.
agent-of-mkmeral
requested a deployment
to
manual-approval
GitHub Actions
Waiting
agent-of-mkmeral
requested a deployment
to
manual-approval
GitHub Actions
Waiting
mkmeral
left a comment
mkmeral
left a comment
There was a problem hiding this comment.
_get_a2a_client with client_config reconstructs a new ClientConfig instead of using the one the user passed. It only copies httpx_client and hardcodes streaming=True, discarding any other config the user may have set. It should just set streaming=True on the existing config or copy it properly.
_resolve_client_config accesses self._a2a_client_factory._config — a private attribute of ClientFactory. This is fragile. If the a2a library changes its internals, this breaks silently (the getattr fallback just logs a warning and falls back to bare httpx).
The card resolution path doesn't apply self.timeout when using client_config.httpx_client. The user's httpx client might not have a timeout configured, while the default path uses timeout=self.timeout.
When both client_config and a2a_client_factory are provided, the precedence is split: client_config wins for card resolution, but factory wins for message sending. The PR description documents this, but it's a confusing split that could lead to auth mismatches (e.g., card resolves with one client, messages sent with a different one from the factory).
The asyncio.Lock is fine for concurrent card resolution, but _card_lock is created in init — if someone creates A2AAgent outside an async context and later uses it in multiple event loops, the lock could be bound to the wrong loop. Minor edge case but worth noting.
…nfig, simplify Changes based on PR review: 1. Remove _resolve_client_config() - eliminates private factory._config access. Factory-only users get bare httpx for card resolution (original behavior). The deprecation message directs them to client_config for auth. 2. Fix _get_a2a_client() - use dataclasses.replace() to preserve all user ClientConfig settings (polling, supported_transports, etc.) instead of reconstructing a new config that discards user settings. 3. Shorten docstrings - remove inline code examples, follow existing style. 4. Add lock justification comment - explains why asyncio.Lock prevents redundant HTTP calls during concurrent card resolution. 5. Simplify precedence - factory wins for client creation (backward compat), client_config wins for card resolution (auth fix). No more split behavior or private attribute access.
agent-of-mkmeral
requested a deployment
to
manual-approval
GitHub Actions
Waiting
agent-of-mkmeral
requested a deployment
to
manual-approval
GitHub Actions
Waiting
|
Pushed fixes addressing all review points (commit
Tests: 35 passing ✅ | Replied to each inline thread with details. |
…ths, drop lock 1. Raise ValueError when both client_config and a2a_client_factory are provided instead of split precedence (card resolved with one client, messages with another). 2. Make card resolution and message sending consistent when no httpx_client: both paths now create managed httpx.AsyncClient(timeout=self.timeout). When client_config has no httpx_client (e.g. ClientConfig(polling=True)), _get_a2a_client creates a managed client and injects it into the config via dataclasses.replace, preserving all user settings. 3. Remove asyncio.Lock - problematic across event loops when A2AAgent is instantiated outside async context. The race condition is benign (worst case: two coroutines fetch the same card, one overwrites).
agent-of-mkmeral
requested a deployment
to
manual-approval
GitHub Actions
Waiting
agent-of-mkmeral
requested a deployment
to
manual-approval
GitHub Actions
Waiting
|
/strands review |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
|
||
| if self._client_config is not None and self._client_config.httpx_client is not None: | ||
| # User provided httpx_client — use it directly (same client as card resolution) | ||
| config = dataclasses.replace(self._client_config, streaming=True) |
There was a problem hiding this comment.
Issue: Each call to _send_message → _get_a2a_client creates a new ClientFactory and negotiates transport from scratch (lines 234-235 and 242-245). This includes re-registering default transports and matching against the agent card's supported transports every time. For high-throughput scenarios with many send_message calls to the same agent, this is unnecessary overhead.
Suggestion: Consider caching the Client (or at least the ClientFactory) after first creation, similar to how the agent card is cached. This could be a follow-up improvement — not blocking for this PR.
There was a problem hiding this comment.
Agreed this is worth optimizing. Not a regression (existing behavior), and the per-call factory overhead is lightweight (no network calls — just transport registration). Good candidate for a follow-up PR that caches the ClientFactory instance after first creation.
There was a problem hiding this comment.
@agent-of-mkmeral i think this was intentional right? can you check the original PR that introduced the A2AAgent? iirc it was about having clean closes of the connection
There was a problem hiding this comment.
Confirmed: the per-call async with httpx.AsyncClient(...) pattern was the original design on main — the @asynccontextmanager ensures the httpx client is properly closed after each send_message call. This PR preserves that same lifecycle pattern. The per-call ClientFactory creation is a side effect of needing a new ClientConfig with the managed client each time, but the overhead is lightweight (no I/O, just transport registration).
@mkmeral's question about whether this was intentional for clean connection closing — yes, the async with context manager pattern on main was specifically for ensuring httpx client cleanup. Caching the factory/client would require managing the lifecycle differently (e.g., an explicit close() or making A2AAgent itself an async context manager). That's a reasonable follow-up but separate from this bug fix.
There was a problem hiding this comment.
Checked the history ✅
a2a_agent.py has exactly 1 commit on main: b091f67 from PR #1441 (authored by you, @mkmeral). The per-call @asynccontextmanager + async with httpx.AsyncClient(timeout=self.timeout) pattern was the original merged design. The file has never been modified since.
So yes — intentional for clean connection closing. The async with ensures the httpx client is properly closed after each send_message call. The per-call ClientFactory(config).create(agent_card) is a side effect of wrapping a fresh managed httpx client each time.
This PR preserves that same pattern:
- Factory path (deprecated): delegates to factory as-is (caller manages lifecycle)
client_configwith userhttpx_client: uses it directly, noasync with(user manages lifecycle)- Default /
client_configwithouthttpx_client: creates managed per-call httpx wrapped inasync with(same as original)
Caching the factory/client is a valid optimization but would change lifecycle semantics — either A2AAgent becomes an async context manager itself, or we add an explicit close(). Good follow-up, not part of this bug fix.
🤖 Strands Agents. Feedback welcome!
|
Assessment: Comment This is a well-structured fix for a real auth bug in Review Details
Nice work addressing the previous review feedback comprehensively — the |
Address review feedback: document that when providing an httpx_client via client_config, the user is responsible for configuring its timeout. The default path uses A2AAgent.timeout, but user-managed clients should configure their own timeout settings.
agent-of-mkmeral
requested a deployment
to
manual-approval
GitHub Actions
Waiting
agent-of-mkmeral
requested a deployment
to
manual-approval
GitHub Actions
Waiting
|
Pushed commit
Quality gates: 36 tests ✅ | |
|
/strands review |
|
Assessment: Approve All prior review feedback has been addressed. The code is clean, well-tested, and the API design aligns with SDK tenets. Review Details
|
agent-of-mkmeral
left a comment
agent-of-mkmeral
left a comment
There was a problem hiding this comment.
TL;DR: PASS — no issues found. 39 adversarial tests (including real HTTP server tests with auth header verification) all pass. The auth bug fix works correctly.
Adversarial Testing Report
Scope: Comprehensive adversarial testing of client_config param, a2a_client_factory deprecation, auth header propagation, edge cases, error handling, concurrency, and backward compatibility.
Tests written: 39
Tests passing: 39
Tests failing (findings): 0
Test Categories
| # | Category | Tests | Status |
|---|---|---|---|
| 1 | Real HTTP Auth Propagation | 4 | ✅ All pass |
| 2 | Edge Cases | 7 | ✅ All pass |
| 3 | Mutual Exclusion | 3 | ✅ All pass |
| 4 | Deprecation Warning | 5 | ✅ All pass |
| 5 | Empty String Bug Fix | 3 | ✅ All pass |
| 6 | Config Preservation | 3 | ✅ All pass |
| 7 | Consistency (card vs message) | 2 | ✅ All pass |
| 8 | Failure Modes | 4 | ✅ All pass |
| 9 | Concurrency | 1 | ✅ All pass |
| 10 | Backwards Compatibility | 2 | ✅ All pass |
| 11 | dataclasses.replace Edge Cases | 2 | ✅ All pass |
| 12 | End-to-End Integration | 2 | ✅ All pass |
Key Tests Explained
Real HTTP Server Tests (most important):
- Spun up a real
HTTPServerthat requiresAuthorization: Bearerheader for/.well-known/agent-card.json - ✅
test_auth_header_reaches_server_with_client_config— Verified auth headers actually arrive at the server (the whole point of this PR) - ✅
test_no_auth_gets_403_from_protected_endpoint— Without auth, protected endpoint correctly returns 403 - ✅
test_factory_alone_still_gets_403— Deprecated factory alone does NOT fix auth for card resolution (uses bare httpx, as documented) - ✅
test_custom_headers_propagate_through_client_config— Arbitrary custom headers (X-Custom-Header, X-Request-ID) propagate correctly
Consistency Test:
- ✅
test_same_httpx_client_used_for_card_and_message— Verified the SAME httpx client is used for both card resolution and message sending (no auth mismatches)
Config Immutability:
- ✅
test_all_client_config_fields_preserved— Verified ALL ClientConfig fields (polling, supported_transports, use_client_preference, accepted_output_modes, grpc_channel_factory) are preserved viadataclasses.replace, onlystreamingis overridden toTrue - ✅
test_original_config_not_mutated_after_get_a2a_client— Original user config is never mutated
Error Recovery:
- ✅
test_card_resolution_failure_doesnt_cache— Failed card resolution does NOT cache the error, retries work correctly - ✅
test_card_resolution_network_error_propagates— Network errors propagate cleanly
What Survived (39 tests passed)
- Auth header propagation through
client_config.httpx_clientto real HTTP server - Custom header propagation (beyond just Authorization)
- Mutual exclusion with helpful error messages
- Deprecation warning with correct stacklevel (points to caller, not init)
- Empty string name/description from agent card preserved (not treated as falsy)
- All ClientConfig fields preserved through
dataclasses.replace - Original config immutability
- Consistent httpx client usage between card resolution and message sending
- Consistent timeout between card resolution and message sending paths
- Network error propagation without caching stale state
- Factory error propagation
- Concurrent card resolution (benign race, all calls succeed)
- Full backward compatibility (old API without new params works identically)
dataclasses.replacewith default_factory fields (lists)- End-to-end integration with real HTTP server (auth + no-auth)
🤖 AI agent response. Strands Agents. Feedback welcome!
|
Resolves #2006 |
| yield self._a2a_client_factory.create(agent_card) | ||
| return | ||
|
|
||
| if self._client_config is not None and self._client_config.httpx_client is not None: |
There was a problem hiding this comment.
why do we check is not none here for httpx client? nothing inside the condition depends on it. client factory should be able to handle default httpx client.
also clean up the if self._client_config is not None: in line 241, that's not necessary. you are complicating this.
it's if client config exists -> use it -> otherwise back to original way
3 participants
Description
Add
client_config: ClientConfigparameter toA2AAgentfor configuring authentication and transport settings. This fixes the auth bug whereget_agent_card()created a barehttpx.AsyncClient, ignoring any authentication configured viaa2a_client_factory.Motivation
A2AAgent.get_agent_card()creates a barehttpx.AsyncClient()for card discovery, ignoring any authentication configured viaa2a_client_factory. This causes 403 errors when the agent card endpoint requires authentication (SigV4, OAuth, bearer tokens).Since the card resolution path shipped broken, no working authenticated card resolution code exists in the wild. However, the factory's other features (interceptors, consumers, custom transports) do work for
send_messagecalls viafactory.create(agent_card), so removing the factory parameter entirely would break existing integrations.Public API Changes
New
client_config: ClientConfigparameter onA2AAgent.__init__()for configuring authentication and transport settings. This is the recommended way to pass an authenticated httpx client for both card resolution and message sending.a2a_client_factoryis deprecated with aDeprecationWarningand will be removed in a future version.Mutual exclusion: Providing both
client_configanda2a_client_factoryraisesValueError.Breaking Changes
No breaking changes.
a2a_client_factorycontinues to work with a deprecation warning.Migration
Changes Summary
client_configparameter — recommended way to configure authenticationa2a_client_factory— with clear DeprecationWarning pointing toclient_configValueErrorwhen bothclient_configanda2a_client_factoryare provided (no split precedence)get_agent_card()— usesclient_config.httpx_clientfor card resolution, enabling authenticated card discoveryis not Noneinstead of falsy check for name/description from agent cardget_agent_card()and_get_a2a_client()create managedhttpx.AsyncClient(timeout=self.timeout)when nohttpx_clientis provided; user settings (polling, supported_transports) preserved viadataclasses.replace()httpx_client, user is responsible for configuring its timeoutDocumentation PR
A2AAgent is not yet documented in the public docs. No documentation update needed.
Related Issues
Re-implementation of #1855 (closed) with all review feedback addressed.
Type of Change
Bug fix
Testing
hatch run prepare(format, lint, mypy, all tests pass)Checklist
@mkmeral
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.