fix(a2a-client): echo selected interface tenant on requests (spec §8.3.2)#80
fix(a2a-client): echo selected interface tenant on requests (spec §8.3.2)#80arkavo-com wants to merge 1 commit into
Conversation
…3.2) Spec §8.3.2 rule 4 requires clients to set the tenant field in every request message to the value declared in the selected AgentInterface (omitted when not declared). The client had no mechanism for this: all request types carry tenant: Option<String>, but nothing filled it from the agent card, so the tenant was never transmitted. Give A2AClient an optional tenant configured via with_tenant(); every request method now fills the request's tenant field from it when the caller left it unset, cloning only when a fill is needed. An explicitly set request tenant is never overridden. A2AClientFactory:: create_from_card propagates the selected interface's declared tenant to the constructed client, treating an empty string as "not declared" (proto3 default). Found by cross-SDK interop testing (a2a-conformance).
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements tenant propagation for the A2A client as required by spec §8.3.2 (rule 4). It adds a tenant field to A2AClient, automatically populates unset tenant fields in outgoing requests, and propagates the tenant declared by the selected interface during client creation. Comprehensive unit tests have been added to verify this behavior. The feedback highlights a performance improvement opportunity in apply_tenant to avoid cloning the request unconditionally before checking if the tenant is already set.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| fn apply_tenant<'a, R: Clone>( | ||
| &self, | ||
| req: &'a R, | ||
| tenant_field: impl FnOnce(&mut R) -> &mut Option<String>, | ||
| ) -> Cow<'a, R> { | ||
| let Some(tenant) = &self.tenant else { | ||
| return Cow::Borrowed(req); | ||
| }; | ||
| let mut filled = req.clone(); | ||
| let field = tenant_field(&mut filled); | ||
| if field.is_some() { | ||
| return Cow::Borrowed(req); | ||
| } | ||
| *field = Some(tenant.clone()); | ||
| Cow::Owned(filled) | ||
| } |
There was a problem hiding this comment.
In apply_tenant, the request req is cloned unconditionally before checking if its tenant field is already set. If the request already has an explicit tenant, this clone is entirely redundant and wasted.
Since these request types can be large (containing messages, metadata, etc.), we should avoid cloning them unless we actually need to populate the tenant.
We can achieve this by passing two closures to apply_tenant: one to inspect the tenant field on the borrowed request, and one to set it on the cloned request. For example, the call sites would then be updated to:
let req = self.apply_tenant(req, |r| &r.tenant, |r, t| r.tenant = Some(t));| fn apply_tenant<'a, R: Clone>( | |
| &self, | |
| req: &'a R, | |
| tenant_field: impl FnOnce(&mut R) -> &mut Option<String>, | |
| ) -> Cow<'a, R> { | |
| let Some(tenant) = &self.tenant else { | |
| return Cow::Borrowed(req); | |
| }; | |
| let mut filled = req.clone(); | |
| let field = tenant_field(&mut filled); | |
| if field.is_some() { | |
| return Cow::Borrowed(req); | |
| } | |
| *field = Some(tenant.clone()); | |
| Cow::Owned(filled) | |
| } | |
| fn apply_tenant<'a, R: Clone>( | |
| &self, | |
| req: &'a R, | |
| get_tenant: impl FnOnce(&R) -> &Option<String>, | |
| set_tenant: impl FnOnce(&mut R, String), | |
| ) -> Cow<'a, R> { | |
| let Some(tenant) = &self.tenant else { | |
| return Cow::Borrowed(req); | |
| }; | |
| if get_tenant(req).is_some() { | |
| return Cow::Borrowed(req); | |
| } | |
| let mut filled = req.clone(); | |
| set_tenant(&mut filled, tenant.clone()); | |
| Cow::Owned(filled) | |
| } |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
1 participant
Problem
Spec §8.3.2 rule 4: clients MUST "set the
tenantfield in every request message to exactly the value declared in the selectedAgentInterfaceentry (omit the field iftenantis not set in that entry)".a2a-clienthas no mechanism for this — request types carrytenant: Option<String>but nothing populates it from the agent card, so multi-tenant A2A deployments cannot be addressed through this SDK.Fix
A2AClientgainswith_tenant(impl Into<String>), atenant()getter, and aCow-basedapply_tenanthelper applied in all 11 request methods (includingget_extended_agent_card). An explicitly set request tenant always wins; with no client tenant there is no clone and no behavior change.A2AClientFactory::create_from_cardsets the tenant from the selected interface, treating the empty string as undeclared (proto3 default semantics).No changes to
a2atypes were needed (request types already deriveClone).Semver note: adds public API (
with_tenant,tenant()) — a minor-version addition. Behavior is opt-in: nothing changes unless a tenant is configured or the selected card interface declares one.Found by / verified with
Cross-SDK interop testing (a2a-conformance, scenario
discovery/tenant-echowith the runner observing the request server-side; FINDINGS.md finding 1): previously failed against every server including a2a-rs's own; passes with this branch. 6 new unit tests (all-methods propagation, explicit-tenant-wins, factory propagation incl. empty-string-as-unset);cargo test --workspace453 passed / 0 failed (baseline 447/0); fmt + clippy clean.🤖 Generated with Claude Code