[gulldan]

Fixes #466.

Problem

The proto gives every HTTP+JSON method a tenant-prefixed binding next to the plain one (additional_bindings, e.g. post: "/{tenant}/message:send", get: "/{tenant}/tasks/{id=*}"), and §8.3.2 rule 4 has the client send the selected interface's tenant on every request. On HTTP+JSON the tenant travels in the path.

• Server: rest_router served only the plain paths, so POST /acme/message:send answered 404, and every handler built its request with tenant: None.
• Client: RestTransport left req.tenant out of every path. a2a-client: the selected AgentInterface's tenant is never echoed on requests (A2A §8.3.2) #199 fills tenant in requests, but on this binding it only reached POST bodies.

Fix

• Server: each canonical binding is routed again under /{tenant}: the eleven additional_bindings of the proto, plus POST …:subscribe as on the plain route, since handle_post_task_action serves :cancel and :subscribe on the same POST route (a comment at the route says so). Legacy aliases stay plain. The task and push-config handlers read their path as TaskPath / PushConfigPath with the tenant optional; the four handlers without a task id (SendMessage, SendStreamingMessage, ListTasks, GetExtendedAgentCard) take Option<Path<String>>, which is None on the plain binding and, for a tenant that fails to decode (POST /%FF/message:send), a 400 before the handler runs, as the task routes already answer. CancelTask still reads its metadata from the body as fix(rest): pass CancelTask metadata from the request body #458 made it. The tenant routes sit inside the A2A-Version route layer from fix(server): enforce A2A-Version on the REST and gRPC bindings #443 and keep the extractor rejections from fix(rest): answer extractor rejections with google.rpc.Status #451.
• Tenant in path and body: google/api/http.proto has body: "*" map "all request fields not captured by the path pattern", so on a /{tenant}/… request the body should not carry tenant. The reference clients do carry it: a2a-python's REST transport posts MessageToDict(request) with tenant in it, a2a-go marshals the request with json:"tenant,omitempty", and a2a-go's server (fillTenant) overwrites the body with the path. So reconcile_tenant, shared by the four POST bodies that carry a tenant (SendMessage, SendStreamingMessage, CancelTask, push-config creation), takes the path tenant when the body has none (an empty cancel body included), accepts an equal body tenant, and answers a different one with 400: a google.rpc.Status whose google.rpc.BadRequest names field tenant ("tenant in the body (body) does not match the tenant in the path (acme)"); nothing reaches the handler. The plain binding takes the body's tenant as before.
• The tenant routes are always on, like Python's Mount('/{tenant}'), and are served for any tenant string; the path tenant only reaches the request. The default stack (DefaultRequestHandler, InMemoryTaskStore) does not isolate tenants: GET /acme/tasks/t1 and GET /other/tasks/t1 return the same task unless the executor or the task store separates them, so isolation is the application's job. A gateway rule written for /tasks/** now also has to match /{tenant}/tasks/**. The rest_router docs say both.
• Client: tenant_path puts a non-empty req.tenant, percent-encoded with the same segment as the ids (fix(client): percent-encode task and config ids in REST paths #450), in front of the path of all eleven methods, including the legacy subscribe fallback, so a request for a tenant never silently reaches the default one.

Tests

• New test_every_method_is_routed_under_a_tenant: all eleven /acme/… bindings (subscribe both as GET and POST) reach the handler with tenant = "acme", and the plain POST /message:send with none. CapturingHandler now also records the tenant each method saw (take_tenant). Without the change every tenant request is a 404.
• New test_an_undecodable_tenant_is_rejected: /%FF/message:send, /%FF/message:stream, /%FF/tasks, /%FF/tasks/t1 and /%FF/extendedAgentCard answer 400 and no handler runs. With the first revision of this PR the four routes without a task id ran as the default tenant.
• New test_path_and_body_tenant, a table over message:send, message:stream, tasks/t1:cancel and tasks/t1/pushNotificationConfigs: under /acme/ a body without a tenant (the empty cancel body included) or with "tenant":"acme" reaches the handler with acme; a body with "tenant":"body" is a 400 and the handler is not called, the BadRequest detail naming field tenant; the same "tenant":"body" on the plain paths reaches the handler with body.
• New test_requests_with_a_tenant_use_the_tenant_path (a2a-client): GetTask, DeletePushConfig and GetExtendedAgentCard with tenant acme eu go to /acme%20eu/…; an empty tenant keeps the plain path. Without the change every path lacks the tenant.
• CI steps on Linux, stable 1.99.1, on top of main da0a00b: all green (879 tests). Patch coverage as coverage.yml builds it: 380 coverable added lines, 0 missed.

Overlap

• One commit on main da0a00b, after fix(rest): pass CancelTask metadata from the request body #458 (handle_cancel_task_inner takes both the body and the path tenant) and fix(server): keep the INPUT_REQUIRED question in task history #462–fix(server): reject a message that does not answer the request alone #464, fix(jsonrpc): read A2A-Version from the query parameter #469.
• The A2A 1.1 stack (feat(client): expose A2A 1.1 operations across transports #387/feat(server)!: add live exchanges with a shared execution runtime #388/feat(server)!: let executor output decide task creation #392) reworks the REST server and client; this diff conflicts textually there. It is the largest change of the set and closer to a feature, so happy to hold it until after 1.1 if you prefer.

🤖 Generated with Claude Code

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[muscariello]

The problem is real and the change covers all eleven methods on the server and the client; CI is green. Requesting changes for the first inline comment only (a tenant that fails to decode falls back to "no tenant"). The other two inline comments and the note below are small.

Tenant isolation is not enforced. This makes the transport carry the tenant, but nothing in the default stack enforces it, and the description doesn't say so.

As far as I can see, DefaultRequestHandler only passes req.tenant into ExecutorContext and defaults a push config's tenant. InMemoryTaskStore doesn't scope by it. So GET /acme/tasks/t1 and GET /other/tasks/t1 return the same task unless the executor or store separates them. Please say that in the description and in the rest_router docs: tenant routes are served for any tenant string, and isolating tenants is the application's job.

The routes are also always on, like Python's Mount('/{tenant}') and unlike Go's opt-in NewTenantRESTHandler. That seems right, since the proto bindings are normative. The effect is that a gateway rule written for /tasks/** now also needs to match /{tenant}/tasks/**. A line in the docs would cover it. No code change is needed unless you'd prefer an opt-in switch. But I would prefer not to.

Thanks @gulldan

[muscariello]

A tenant that fails to decode is treated as "no tenant" here.

Result<Path<TenantPath>, PathRejection> has two Err cases. MissingPathParams is the plain route, which is what this is meant to ignore. FailedToDeserializePathParams is a tenant segment that is not valid UTF-8 after percent-decoding (axum 0.8.9, InvalidUtf8InPathParam). if let Ok(..) and .ok() drop both, so POST /%FF/message:send runs as the default tenant instead of failing. That is the outcome the client half of this PR avoids ("never silently reaches the default one"). The TaskPath routes already answer 400 for the same input, so the two sets of routes disagree.

The same pattern is at the stream handler (233), handle_list_tasks (333) and handle_get_extended_agent_card (531).

Suggest treating only MissingPathParams as None and answering the other rejections, in one helper shared by the four handlers (send, stream, list, extended card):

fn path_tenant(
    tenant: Result<Path<TenantPath>, PathRejection>,
) -> Result<Option<String>, Response> {
    match tenant {
        Ok(Path(path)) => Ok(Some(path.tenant)),
        Err(PathRejection::MissingPathParams(_)) => Ok(None),
        Err(rejection) => Err(rest_payload_error_response(rejection.body_text())),
    }
}

and a case in test_every_method_is_routed_under_a_tenant for /%FF/message:send that expects 400 and no handler call. I read this in axum's source and did not run it.

[muscariello]

The description says the path tenant overrides one in the body for SendMessage, SendStreamingMessage, CancelTask and push-config creation, but no test pins it. if tenant.is_some() in the cancel and create handlers is the kind of line a later refactor drops without a failure.

With take_tenant this is cheap: post a body that carries "tenant":"body" to /acme/message:send and /acme/tasks/t1:cancel and expect acme, then post the same bodies to the plain paths and expect body.

Smaller: the client test covers 3 of the 11 methods. They all go through tenant_path, so that is probably enough. A table over every method, like the server's, would catch one that forgets to call it.

[Tehsmash]

Based on the http transcoding docs: https://github.com/googleapis/googleapis/blob/master/google/api/http.proto it seems like if tenant is in the URL then it shouldn't be allowed in the body to avoid the server needing to handle this conflicting case. So perhaps it should be a BadRequest if tenant is in the URL and in the body?

[muscariello]

reconcile_tenant does what I suggested, thanks. One thing left: the description still says the path tenant overrides a body tenant, which is no longer true (a different value is a 400). Please update that line, and the others that describe the earlier design (the Result<Path<…>> extractor, "exactly the eleven bindings", the base commit).

[gulldan]

Agreed on the transcoding rule: with body: "*" the body maps the fields the path did not capture, so tenant has no place in the body of a /{tenant}/… request. A flat rejection would break both reference clients, though: a2a-python's REST transport posts MessageToDict(request) with tenant in it and a2a-go marshals the request with json:"tenant,omitempty", so each sends the same value in both places (and a2a-go's server fillTenant just overwrites the body with the path). The PR now rejects only a conflicting body tenant with a BadRequest on field tenant and accepts an equal one; a path tenant alone, including an empty cancel body, keeps working. Pinned in test_path_and_body_tenant.

[gulldan]

Description updated: the body-tenant rule (an equal value is accepted, a different one is a 400), the Option<Path<String>> extractor in place of Result<Path<…>>, "the eleven bindings, plus POST subscribe as on the plain route", and the base commit (main da0a00b). The patch-coverage figure is from the previous revision; I will refresh it when the re-run on 5f311f2 finishes. Thanks for the review.

[gulldan]

Re-run on 5f311f2 done: CI mirror all green (879 tests), patch coverage 380 coverable added lines, 0 missed. Description updated with the final numbers.

[muscariello]

The description says the tenant routes are "exactly the eleven additional_bindings of the proto", but this route also serves POST /{tenant}/tasks/{id}:subscribe, and the proto has no such binding. SubscribeToTask has only get and additional_bindings { get: "/{tenant}/tasks/{id=*}:subscribe" }. The plain POST is a legacy alias, which the description says stays plain. Here it is carried over because handle_post_task_action dispatches on the :subscribe suffix and the route has to keep POST for :cancel.

Keeping it is harmless and matches the plain route. I'd reword the description ("the eleven bindings, plus POST subscribe as on the plain route") and add a comment at the route so it reads as deliberate. If you'd rather keep the tenant routes strictly to the proto, handle_post_task_action can refuse :subscribe when tenant.is_some().

[gulldan]

Thanks for the careful read. All four points, and @Tehsmash's note on the body tenant, are in the new revision, squashed to one commit on main da0a00b (5f311f2).

Tenant that fails to decode (blocking). Fixed, and your reading of axum was right: %FF is FailedToDeserializePathParams, and .ok() ran it as the default tenant. On a route without a path parameter axum still inserts an empty parameter list, so MissingPathParams never fires and a struct extractor fails with "missing field" instead; the four handlers now take Option<Path<String>> (axum 0.8's OptionalFromRequestParts), which is None for zero parameters and rejects everything else before the handler, with the same 400 the Path<TaskPath> routes give. No helper needed. test_an_undecodable_tenant_is_rejected posts /%FF/… to all five routes and checks 400 and no handler call.

Path tenant and body tenant. Pinned in test_path_and_body_tenant, a table over send, stream, cancel and push-config creation; following @Tehsmash's comment the rule changed from "path overrides" to "a different body tenant is a 400" (see below), so the table also has the conflict rows. I left the client test at three methods since all eleven go through tenant_path; happy to add the table if you want it.

POST subscribe under /{tenant}. Kept, with a comment at the route saying it is the legacy alias carried over because handle_post_task_action serves :cancel and :subscribe on one POST route; the description now says "the eleven bindings, plus POST subscribe as on the plain route".

Isolation and always-on routes. The rest_router docs and the description now say that the tenant routes are always on and served for any tenant string, that the default stack (DefaultRequestHandler, InMemoryTaskStore) does not isolate tenants so GET /acme/tasks/t1 and GET /other/tasks/t1 return the same task unless the executor or store separates them, and that a gateway rule for /tasks/** now also has to match /{tenant}/tasks/**. No opt-in switch.

From my side that covers everything raised in this thread. The CI mirror on 5f311f2 is green (879 tests) and the patch coverage is 380 coverable added lines, 0 missed; the description reflects the final design. Over to you.