feat(remoteagent): apply per-request auth to A2A via Config.Auth

[wolo-lab]

Problem

The A2A remote agent (agent/remoteagent/v2) had no way to attach a per-request end-user credential to outgoing A2A calls. To talk to a remote agent that requires authentication, the caller had to hand-build a client and wire in auth themselves. adk-python's RemoteA2aAgent already takes an auth scheme and a credential and applies them to every call, and now that the core auth package exists, the Go remote agent should do the same.

Summary

Adds A2AConfig.Auth auth.CredentialProvider — additive and backward-compatible. When set, NewA2A installs an HTTP transport on the A2A client that resolves the credential and applies it to every call the agent makes: the message send, the CancelTask the run loop issues when it exits before a terminal event, and the agent card fetch done by NewAgentCardProvider.

Nothing changes for anyone on the current release who does not set the field. With Auth unset the client is built exactly as before, and the contexts handed to ClientProvider and RemoteTaskCleanupCallback keep the dynamic type and the values they had.

Auth combined with a custom ClientProvider is a configuration error, since the transport cannot be injected into a caller-built client. CredentialScope is exported for that case, so a caller wiring their own auth can key on the same collision-free scope.

Parity with adk-python

The behavior follows adk-python's RemoteA2aAgent, checked against its main and by running release 2.10.0:

• The credential decides where it goes. It is written by its own Apply, as mcptoolset.Config.Auth applies it, except for a bare auth.OAuth2Credential, which is minted here and sent as a bearer token so the mint can be bounded. The agent card's security section is not consulted, so every auth.Credential works, including BasicCredential and auth.WithHeaders, and a card that declares no security still gets the credential. adk-python likewise writes the header its configured auth scheme names.
• It fails closed. A credential that cannot be resolved or applied fails the call. It never goes out unauthenticated.
• It is resolved once per invocation and reused for the card fetch, the send and the cleanup cancel.
• The card fetch is authenticated, and a card source that is neither https nor a loopback host is refused before the fetch goes out.

One difference remains: adk-python answers a credential that needs the user's consent by pausing the invocation with an adk_request_credential event. That round-trip is not implemented here yet, so an *auth.ConsentRequiredError fails the call like any other error. Static tokens, API keys, and 2-legged or service-account sources work.

Scoping

The scope handed to a session-aware provider through a2aclient.SessionIDFrom is the whole calling identity — app name, user id, session id and the remote agent's Name — each percent-encoded and joined with /. The session id alone is caller-supplied and optional, so two users each holding a session named default would otherwise resolve to one credential. The agent name is part of the key because a bearer token is scoped to an audience as well as a user. The provider's context is also the agent.InvocationContext.

The adka2a server issues its own CancelTask for an abandoned child task, reusing the subagent's client. It attaches the same scope, so that cancel is authenticated too. The scope helper lives in internal/agent/remoteagent because both packages need it.

The scope is only as trustworthy as the identity behind it. Behind an adka2a server with no authenticator configured, the user id is synthesized from the context id the calling peer chose, so a provider caching per scope would hand one peer the credential resolved for another. The field doc says so.

Transport hardening

The transport applies the credential on every hop, so the client refuses a redirect that leaves the card's host or downgrades its scheme, checked against the original request and the hop just taken. adk-python's HTTP client follows no redirects at all by default.

Every blocking credential step — an OAuth2 mint, or a credential's own Apply, which may mint — runs bounded and single-flighted per scope. oauth2.TokenSource.Token takes no context, so without that a hung token endpoint would hold the request past every deadline, and every request arriving meanwhile would park another goroutine. The bound belongs to the attempt rather than the waiter, and an attempt past its deadline is retired, so a hang cannot lock an identity out for the life of the process.

An error from a token endpoint never reaches the caller with the response body in it. The message is rebuilt from the status and the error code, because the source behind auth.ServiceAccount with an Audience wraps the body in an error that prints it. error_description and error_uri are free text parsed out of the same body, so they go with it.

With Auth set, a card naming a non-loopback http interface is logged once per interface, since the credential will travel in cleartext. A typed-nil provider is a constructor error rather than a panic on first use.

[karolpiotrowicz]

The shape is right (additive opt-in field, session-scoped resolution, cleanup path authenticated too) and the test file is unusually thorough for a first pass. I dug into it fairly deeply because it puts credentials on the wire, and I found a few things worth resolving before this lands; the first three are the ones I'd hold on.

Blocking

• The credential scope key can collide across users (agent/remoteagent/v2/a2a_agent.go:283). The scope is the bare ADK session id, but an ADK session is the triple (AppName, UserID, SessionID) and the session id is caller-provided. Two users holding a session named "default" get the same credential. Inline comment has the reproduction and a suggested composite key.

Worth fixing before merge

• A hung token mint can't be interrupted (agent/remoteagent/v2/auth.go:77) — not by the invocation context, not by the 5s cleanup budget, not by the client's own timeout. It blocks Run itself.
• Some legitimate auth.Credential values silently become unauthenticated requests (agent/remoteagent/v2/auth.go:85) — auth.BasicCredential, anything from auth.WithHeaders, and pointer forms of the supported types. Nothing reaches the caller.

Smaller points, inline

Doc accuracy on multi-scheme cards, redirect hardening, the mutual-exclusion error's remediation advice, a typed-nil provider panic, the API-key test's discriminating power, the divergence from mcptoolset.Config.Auth, and two uncovered error branches.

Notes that didn't seem worth their own comment

• The provider is now called concurrently (I measured 8 in-flight calls across 8 concurrent invocations of one agent) and may be called more than once per outgoing request (a card declaring 20 unusable schemes produced 20 Credential calls for a single request — token mints stay at 1 thanks to ReuseTokenSource caching, so this is mostly a provider-invocation cost). Neither is stated on the field doc; one sentence would cover both.
• No test in the package starts two invocations concurrently, so the green -race run doesn't actually cover the shared-interceptor case. Writing that test currently trips over serveRecordingA2A sharing one event slice with a replay helper that stamps TaskID/ContextID into those events per request — worth fixing together.
• fmt.Errorf("...: %w", err) at agent/remoteagent/v2/auth.go:79 propagates oauth2.RetrieveError, whose message embeds the verbatim token-endpoint response body; the interceptor logs that at ERROR. I saw a planted body value make it into the error string intact. Whether a real provider's error body carries anything sensitive is provider-dependent, but a redacted wrap here would remove the question.
• Nothing requires TLS before the credential is attached — a card naming an http:// interface gets it in cleartext. Same absence exists on the auth.Transport path, so this may be a package-level decision rather than yours.
• OAuth2Credential's token type is dropped, so a non-bearer token would be transmitted mislabeled. In practice oauth2.Token.Type() returns Bearer when unset and the interceptor hardcodes the prefix anyway, so rejecting a non-bearer type is probably the only sensible option — very low priority.
• The recorders in the new tests overwrite rather than accumulate, so each asserts on the last request only. Correct today (one call per scenario), fragile if a scenario ever produces two.

A question rather than a claim

• When resolution fails during cleanup, the CancelTask goes out unauthenticated and the remote task stays alive — which is the failure TestRemoteAgent_AuthAttachedToCleanupCancel's comment says the cleanup auth exists to prevent. I can see the argument that this is just fail-open behaving as documented, since cleanupRemoteTask already tolerates any cancel failure with a log.Warn. Is that the intent, or would you want the fail-open case called out in the Auth doc as "a resolution failure at cleanup can leave the remote task running"?

For what it's worth, everything objective is green: go build, the full go test ./..., go test -race on the package, gofmt, go vet, golangci-lint, and staticcheck. apidiff reports the change as compatible (A2AConfig.Auth: added, no incompatible changes), and I confirmed no existing caller of NewA2A sets the new field or can reach the new validation error. The new tests correctly fail to compile against the merge base, so they do exercise new API.

[wolo-lab]

All ten comments are addressed in the latest push (9c9ac81). Detail is on each thread — this is the shape of the change and the parts that go beyond what you raised.

The three you held on

The scope collision reproduced exactly as you described, and the fix goes further than a composite key: the scope now carries the remote agent's name as well, because a bearer token is scoped to an audience and one provider shared between two remote agents in a session would otherwise cross their tokens. The mint is bounded by the caller's context and by a budget of its own, since the send path — unlike your cleanup reproduction — inherits no deadline from the runner. Pointer credentials are accepted, and WithHeaders is refused rather than unwrapped: the protocol hands over one string, so unwrapping would silently drop the header that was the reason for using it.

Three things your comments led to that they did not name

The adka2a server issues a second CancelTask, and it was unauthenticated. cancelChildInputRequiredTasks reuses the subagent's auth-wired client with a context that never carried a scope, so the interceptor returns before resolving anything and a secured remote rejects the cancel — leaking the child task, which is the failure the cleanup auth exists to prevent. It now attaches the scope and the card (executor.go), which is why the credential scope helper moved to internal/agent/remoteagent: the server needs the identical key and cannot import agent/remoteagent/v2.

Redirect hardening had to be checked against the previous hop, not just the original. Every hop of http → https → http looks same-origin against the original request, so checking only that would let a chain climb to TLS and drop back to cleartext.

The remediation in the mutual-exclusion error did not work as written. A caller reading the scope inside their ClientProvider gets nothing, because the interceptor reads the context of each call rather than the one the provider was built with. The message now says to attach it per call, and CredentialScope is exported so the key is the same one this package uses.

Your notes-without-comments

The concurrency observation was right and there is now a test that starts eight concurrent invocations of one agent value with distinct users and asserts the exact set of credentials the server saw, so -race covers the shared interceptor. It needed the replay helper fixed first, as you predicted — the per-request task and context ids were being stamped into one shared event slice.

oauth2.RetrieveError was the note I'm most glad you wrote down. It prints the response body verbatim, and only when the response was not a well-formed OAuth2 error, so the case where the body reaches an ERROR log is the case where its content is least predictable. Redacted, with the status and the error chain kept.

The multiple-calls-per-request and concurrency facts are on the field doc now, as is the cleanup answer to your question: a resolution failure there does send CancelTask unauthenticated and can leave the remote task running. That is fail-open behaving as documented, and it is worth a caller knowing before they choose a provider that can fail.

Two I did not take. Requiring TLS before attaching stays documented rather than enforced, since it would break local development and every in-process test server. And the token type is dropped no longer — a non-bearer type is rejected outright, which as you said is the only sensible option.

Checks

go build, go vet, gofmt, golangci-lint, go mod tidy -diff and go test -race -count=1 -shuffle=on ./... are clean. apidiff reports A2AConfig.Auth: added plus the new CredentialScope, no incompatible changes. Coverage on the package went 89.2% to 91.0%.

[karolpiotrowicz]

Round 1's blocker is properly closed and I can show it: reverting only the scope change turns TestRemoteAgent_AuthScopeIsPerUser and TestRemoteAgent_AuthConcurrentInvocations red, and a generated run over 400,000 identity tuples containing /, %, %2F and NUL produced no colliding key. The scheme-placement check, the redirect policy and the context-type preservation on cleanup all hold up under deliberate attempts to break them. What I would fix before this merges is one behaviour and four guards that no test can currently fail on.

A card with an empty requirement object goes out unauthenticated, silently

The warning at a2a_agent.go:466 tests len(card.SecurityRequirements) == 0. A card that declares a scheme and one empty requirement object has length 1, so the warning does not fire, and the a2a interceptor's inner for schemeName := range requirement has nothing to iterate, so Get is never called. The request leaves with no credential and nothing is logged anywhere. security: [{}] is the OpenAPI spelling of "authentication optional", so a real card can carry it.

Reproduction — drop into agent/remoteagent/v2 and run go test -run TestEmptyRequirementObject ./agent/remoteagent/v2/

func TestEmptyRequirementObject(t *testing.T) {
	var mu sync.Mutex
	var authz []string
	srv := serveFreshA2A(t, "ok", func(r *http.Request) {
		mu.Lock()
		authz = append(authz, r.Header.Get("Authorization"))
		mu.Unlock()
	})
	card := newSecureCard(srv.URL,
		a2a.NamedSecuritySchemes{"bearer": a2a.HTTPAuthSecurityScheme{Scheme: "Bearer"}},
		a2a.SecurityRequirementsOptions{{}}, // one EMPTY requirement object
	)
	called := 0
	prov := auth.ProviderFunc(func(ctx context.Context) (auth.Credential, error) {
		called++
		return auth.BearerCredential{Token: "secret-token"}, nil
	})
	a, err := NewA2A(A2AConfig{Name: "a2a", AgentCard: card, Auth: prov})
	if err != nil {
		t.Fatalf("NewA2A: %v", err)
	}
	warns := &countingHandler{match: "a2a auth"}
	ctx := newInvocationContextFor(t, t.Name(), "u", "s")
	scoped := ctx.WithContext(log.AttachLogger(ctx, slog.New(warns)))
	if _, err := runAndCollect(scoped, a); err != nil {
		t.Fatalf("run: %v", err)
	}
	mu.Lock()
	defer mu.Unlock()
	t.Logf("provider called %d times, Authorization=%q, warnings=%d", called, authz, warns.count.Load())
}

// observed: provider called 0 times, Authorization=[""], warnings=0

The invariant worth holding is that every path which reaches the wire without a credential emits exactly one signal, rather than that this particular shape is special-cased.

Four guards no test can fail on

I checked these by neutering each one and re-running ./agent/remoteagent/... ./server/adka2a/... ./internal/agent/remoteagent/.... Twenty-one other guards in this change do fail their tests when broken, which is why these four stand out rather than reading as ordinary coverage debt.

a2a_agent.go:420 — OwnsAuthScope: cfg.Auth != nil. Changing it to false leaves the entire suite green. It is the only line joining the user-facing Auth field to the server-side cancel path, so if it were ever wrong, every Auth user's adka2a-issued cancel for an abandoned child task would go out unauthenticated — the exact failure executor.go:350 was added to fix. The executor tests cannot call NewA2A because of the import cycle and hand-set the field instead, so nothing anywhere asserts the derivation.

auth.go:51 — the exported CredentialScope wrapper. Swapping s.UserID() and s.ID() survives, and so does dropping agentName entirely. auth_test.go:274 pins the internal four-string function against literals, but every test that touches the exported wrapper computes both the expectation and the actual value from it. The two mutations matter for different reasons: the argument swap desynchronizes this path from executor.go:336, which builds the key from the internal function directly, so a provider caching per scope would resolve the cancel under a different key than the send. Dropping agentName reinstates the cross-agent token leak that component was added to prevent. One row driving the exported wrapper with three distinct literals would close both.

executor.go:350-351 — the card re-attachment. Deleting it keeps both executor tests green, because cancel_auth_test.go:101 installs a2aclient.NewInMemoryCredentialsStore(), which resolves on (scope, scheme name) and never looks at the card. Production always uses this package's own credentialsService, which does read it — with no card on the context schemeAccepts returns false, the cancel goes out unauthenticated and the remote task leaks. Separately, the config those tests build sets OwnsAuthScope: true alongside a caller-supplied ClientProvider, and a2a_agent.go:397 makes that combination unreachable, so the fixture is a shape NewA2A cannot produce.

a2a_agent.go:466-476 — warnNoRequirement. Deleting the whole block leaves the suite green and no test matches its message, unlike its sibling warnMismatch. This is the same block as the finding above.

The ClientProvider documentation describes a state that cannot occur

a2a_agent.go:315-318 tells the reader that the context a ClientProvider receives carries the credential scope and is an agent.InvocationContext wrapper. But a2a_agent.go:397 rejects Auth together with ClientProvider, so whenever a caller-supplied provider runs, Auth is nil, and auth.go:114 returns the invocation context untouched — no scope and no wrapper. TestRemoteAgent_AuthUnsetLeavesTheContextAlone asserts precisely that. Someone who trusts this paragraph writes a2aclient.SessionIDFrom(ctx) in their provider and gets ok == false on every call, which is the opposite of what the error message at line 397 sends them to do. The wrapper half of the sentence does become true on one call, the cleanup CancelTask, which makes the paragraph harder to spot as wrong rather than easier.

Already true before this change

Worth knowing, not for this PR to fix. An AgentCardProvider returning (nil, nil) panics on a nil dereference — I checked, and main panics identically, one frame further in, because a2aclient.Factory.CreateFromCard dereferences the card on its first line. The adka2a identity derivation at metadata.go:62 and the unbounded serial cancel loop are likewise base behaviour, though the loop now carries credential resolution and so has a larger worst case than it did.

I also chased the worry that giving the client an explicit http.Client newly caps SSE streams at three minutes. It does not: in a2a-go v2.5.0 the JSON-RPC and REST transports each build &http.Client{Timeout: defaultRequestTimeout} when handed no client, which is the same application site, so the comment at auth.go:365-368 is exactly right.

What I would want before merge

A test that fails if OwnsAuthScope stops tracking Auth, one that drives the exported CredentialScope with three distinct literals, one that exercises the executor cancel through this package's own credentials adapter rather than the in-memory store, and a decision on the empty-requirement case — either widen the check or say in the field doc that a card shaped that way is treated as needing no credential. The documentation correction on ClientProvider matters as much as the tests, since that paragraph actively sends a reader the wrong way. Everything under the inline comments is optional.

Superseded: every finding from this round is fixed. A new review follows.

[karolpiotrowicz]

Two things need to change before this can merge: TestMintGroupSeparatesScopes cannot fail on the cross-user merge it is named for, and the remediation text in NewA2A's ClientProvider error is no longer true for the cleanup cancel. Both are new in 2c28db5, and both come from suggestions I made last round.

Everything from the previous round is fixed. I broke each guard I flagged last time again (the OwnsAuthScope derivation, both CredentialScope arguments, the executor card re-attachment, the nil-card default-deny, the no-requirement warning, and the old length-only check on the requirement list), and every one of them now turns a test red. The [{}] card now warns, and a [{}, {bearer}] card still sends the bearer token. Documenting the CredentialScope(nil, …) panic instead of guarding it holds up, since any fallback key would merge identities.

TestMintGroupSeparatesScopes cannot fail when two identities share a mint

The test at auth_test.go:1825-1842 makes its two g.token calls one after the other. By the time the second call looks up inFlight, run has already deleted the first entry at auth.go:427-430. So the test passes whatever the map is keyed on. I keyed mintGroup on the app segment of the scope alone, and the whole of ./agent/remoteagent/... ./server/adka2a/... ./internal/agent/remoteagent/... still passes. Under that key, a request for bob that arrives while alice's mint is in flight is sent with alice's access token. The production key is correct, and on this head bob gets his own token. The problem is that nothing in the suite would notice if the key stopped being correct.

What I ran: overlapping mints for two users of one app. Passes on this head, and bob receives alice-tok when the key is narrowed to the app segment. Run with go test -run TestCrossUserOverlap ./agent/remoteagent/v2/

func TestCrossUserOverlap(t *testing.T) {
	g := newMintGroup()
	release := make(chan struct{})
	entered := make(chan struct{})
	alice := tokenSourceFunc(func() (*oauth2.Token, error) {
		close(entered)
		<-release
		return &oauth2.Token{AccessToken: "alice-tok"}, nil
	})
	bob := tokenSourceFunc(func() (*oauth2.Token, error) {
		return &oauth2.Token{AccessToken: "bob-tok"}, nil
	})
	aliceDone := make(chan struct{})
	go func() {
		_, _ = g.token(context.Background(), iremoteagent.CredentialScope("shop", "alice", "s1", "crm"), alice)
		close(aliceDone)
	}()
	<-entered
	bobGot := make(chan string, 1)
	go func() {
		tok, _ := g.token(context.Background(), iremoteagent.CredentialScope("shop", "bob", "s9", "crm"), bob)
		bobGot <- tok
	}()
	var got string
	select {
	case got = <-bobGot:
		close(release)
	case <-time.After(500 * time.Millisecond):
		close(release)
		got = <-bobGot
	}
	<-aliceDone
	if got != "bob-tok" {
		t.Fatalf("bob received %q", got)
	}
}

The property to hold is that a token minted for one identity never reaches another, and that some test fails when two overlapping identities end up sharing a mint. One thing to keep in mind: single-segment keys like "scope-a" and "scope-b" would still pass under a key that merges on a prefix, even with overlapping mints, because they have no prefix to share.

The ClientProvider remediation in NewA2A's error no longer holds for the cleanup cancel

The error at a2a_agent.go:402 tells a caller that their client "sees the ADK invocation context on every call, so it can key on remoteagent.CredentialScope(ctx.Session(), name)". A custom ClientProvider always runs with Auth unset, and with Auth unset a2a_agent.go:649-655 now hands CancelTask a plain context.WithTimeout context. I drove NewA2A with a ClientProvider whose client records the context type on each call, then broke out of a streaming run after the first event. On this head SendStreamingMessage got an agent.InvocationContext and CancelTask did not. On the previous head (2ff5dae) both did. A caller who follows the message gets no scope on that one call, so the cleanup cancel goes out unauthenticated and the remote task is left running, or an unchecked assertion panics inside the deferred cleanup. That is the failure the cleanup auth in this PR exists to prevent.

This is a side effect of gating the re-wrap on Auth, which I suggested last round, and I did not spot the interaction with this message then. The plain context is what RemoteTaskCleanupCallback's doc and the Auth-unset contract promise, so the code and the message cannot both stand as written. The comment at a2a_agent.go:632-634 saying "nothing else reads the context back" has the same blind spot. TestRemoteAgent_AuthClientProviderScopeRemediation does not catch it, because it captures the scope before the run and never reaches the cancel. What has to hold is that whatever NewA2A tells a caller to do works on every call their client receives, the cleanup CancelTask included.

Not blocking: a request that joins an in-flight mint never uses its own token source

The single-flight I suggested also has a cost I did not flag. A request that joins an in-flight mint at auth.go:396-411 takes the leader's result and never calls the token source its own provider call returned. With a token source that does not cache, and whose first Token() call hangs while later calls would succeed, four sequential runs in one session sent Authorization as ["" "" "" ""] on this head and ["" "Bearer fresh" "Bearer fresh" "Bearer fresh"] on the previous head. With one shared oauth2.ReuseTokenSource, which is what the x/oauth2 JWT and compute sources return, both heads behave the same, because that type already serializes its callers. So this only affects providers that build a fresh source per call. The assumption that a provider resolves one source per scope is written only on the unexported mintGroup at auth.go:363-366. The exported Auth doc at a2a_agent.go:362-366 still reads as though every request is served by what the provider just returned.

The three inline notes are optional.

[wolo-lab]

Both blocking points are fixed in the latest commit (cb47736).

TestMintGroupSeparatesScopes. The mints now overlap, on four real scopes that share their app, session and agent segments with the first one. While the first mint is held open, each of the other three has to get its own token within a bounded wait. Keying the group on the app segment alone, which is the change you made, now turns it red, and so does dropping the user segment or the agent segment.

The ClientProvider remediation. I kept the code and changed the message, since the plain detached context on the cleanup cancel is what RemoteTaskCleanupCallback and the opted-out contract promise. The error now says the context the provider receives is the invocation context, so the scope is computed there and the client attaches it to every call, the cleanup CancelTask included. It also says not to read the scope back from each call's context. The ClientProvider field doc and the cleanup comment say the same. TestRemoteAgent_AuthClientProviderScopeRemediation now follows the message literally, breaks out of a streaming run so the cleanup cancel fires, and asserts the bearer token on both the send and the CancelTask. A client that does not attach the scope to CancelTask turns it red.

Joining an in-flight mint. Two things changed after the head you reviewed. The exported Auth doc now says a token source should depend on nothing finer than the scope, because concurrent mints for one scope are collapsed. And an in-flight mint now carries a deadline of its own (9a8442a), so a caller arriving after it retires the attempt and mints with its own source. In your four-run scenario the first run times out and the later runs use their fresh source, as they did on the earlier head.

Both items in this review are fixed on the current head. The new review replaces it.

[karolpiotrowicz]

Two things need to change before this can merge. The ClientProvider remediation in NewA2A's error still fails on one call, the adka2a server's cancel of an abandoned child task. And the mintCall comment credits auth/gcp with a design that auth/gcp explicitly rejects.

Both blockers from last round are fixed. Keying mintGroup on the app segment, or dropping the user segment, now turns TestMintGroupSeparatesScopes red. TestRemoteAgent_AuthClientProviderScopeRemediation fails when the client stops attaching the scope to CancelTask. The panic check now catches a recover branch that leaves its entry behind. The hung-first-mint scenario from last round now fails the first run closed and sends the fresh token on the next three. I re-ran the build, tests, vet and lint on this head, and they are clean.

Must change before merge:

The ClientProvider remediation fails on the adka2a cancel

The error at a2a_agent.go:433 and the field doc at a2a_agent.go:338-349 tell a caller that the context the provider receives is the ADK invocation context. They say to compute CredentialScope(ctx.Session(), name) there and attach it on every call. The adka2a server calls that same provider when it cancels an abandoned input-required child task. Executor.Cleanup reaches CreateA2AClient at executor.go:331-334 with the a2asrv context, and AttachAuthScope does nothing there because OwnsAuthScope is false for a custom ClientProvider (auth_scope.go:84-87).

I drove Executor.Cleanup with a cancel of an input-required parent task, using a remote subagent whose ClientProvider does exactly what the message says. The provider received a *context.cancelCtx:

• Checked type assertion: the provider returned an error, so no CancelTask was sent and the child task keeps running.
• Unchecked assertion: the panic propagated out of Executor.Cleanup.
• Control: the same harness, with a provider that skips the assertion, sent the CancelTask.

The Auth doc already names this exception at a2a_agent.go:392-394. The ClientProvider doc and the error do not. The message I reviewed last round was wrong for this path too, and I missed it then. The property is the same one as last round: whatever NewA2A tells a caller to do has to work on every call their provider and client receive, and that includes the adka2a cancel.

What I ran: a message-following ClientProvider behind Executor.Cleanup, plus a control. Run with go test -run TestClientProviderRemediationOnAdka2aCancel -v ./server/adka2a/v2/

package adka2a

import (
	"context"
	"fmt"
	"net/http"
	"net/http/httptest"
	"sync"
	"testing"

	"github.com/a2aproject/a2a-go/v2/a2a"
	"github.com/a2aproject/a2a-go/v2/a2aclient"
	"github.com/a2aproject/a2a-go/v2/a2asrv"
	"google.golang.org/genai"

	"google.golang.org/adk/v2/agent"
	"google.golang.org/adk/v2/plugin"
	iremoteagent "google.golang.org/adk/v2/internal/agent/remoteagent"
	"google.golang.org/adk/v2/session"
)

// A ClientProvider written exactly as NewA2A's error message instructs, used
// as a remote subagent of an adka2a-hosted app. Drives the executor's cancel of
// an abandoned input-required child task (Executor.Cleanup ->
// cancelChildInputRequiredTasks, executor.go:273).
func TestClientProviderRemediationOnAdka2aCancel(t *testing.T) {
	for _, mode := range []string{"checked", "unchecked", "control-no-assertion"} {
		checked := mode == "checked"
		t.Run(mode, func(t *testing.T) {
			const (
				appName   = "app"
				agentName = "remote"
				contextID = "ctx-1"
				taskID    = "task-1"
				callID    = "call-1"
			)
			userID, sessionID := "A2A_USER_"+contextID, contextID
			var mu sync.Mutex
			authByMethod := map[string]string{}
			inner := a2asrv.NewJSONRPCHandler(a2asrv.NewHandler(cancelOnlyExecutor{}))
			srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
				mu.Lock()
				authByMethod[peekJSONRPCMethod(r)] = r.Header.Get("Authorization")
				mu.Unlock()
				inner.ServeHTTP(w, r)
			}))
			defer srv.Close()
			card := &a2a.AgentCard{
				Name:                 agentName,
				SupportedInterfaces:  []*a2a.AgentInterface{a2a.NewAgentInterface(srv.URL, a2a.TransportProtocolJSONRPC)},
				SecuritySchemes:      a2a.NamedSecuritySchemes{"bearer": a2a.HTTPAuthSecurityScheme{Scheme: "Bearer"}},
				SecurityRequirements: a2a.SecurityRequirementsOptions{{a2a.SecuritySchemeName("bearer"): a2a.SecuritySchemeScopes{}}},
			}
			store := a2aclient.NewInMemoryCredentialsStore()
			store.Set(iremoteagent.CredentialScope(appName, userID, sessionID, agentName), "bearer", "own-token")
			factory := a2aclient.NewFactory(a2aclient.WithCallInterceptors(&a2aclient.AuthInterceptor{Service: store}))
			var providerSaw string
			remoteCfg := &iremoteagent.A2AServerConfig{
				AgentCard: card,
				ClientProvider: clientProviderFunc(func(ctx context.Context, c *a2a.AgentCard) (iremoteagent.A2AClient, error) {
					providerSaw = fmt.Sprintf("%T", ctx)
					if mode == "control-no-assertion" {
						return factory.CreateFromCard(ctx, c)
					}
					var inv agent.InvocationContext
					if checked {
						var ok bool
						inv, ok = ctx.(agent.InvocationContext)
						if !ok {
							return nil, fmt.Errorf("ClientProvider got %T", ctx)
						}
					} else {
						inv = ctx.(agent.InvocationContext)
					}
					s := inv.Session()
					_ = iremoteagent.CredentialScope(s.AppName(), s.UserID(), s.ID(), agentName)
					return factory.CreateFromCard(ctx, c)
				}),
			}
			ctx := t.Context()
			svc := session.InMemoryService()
			created, err := svc.Create(ctx, &session.CreateRequest{AppName: appName, UserID: userID, SessionID: sessionID})
			if err != nil {
				t.Fatal(err)
			}
			event := session.NewEvent(ctx, "invocation")
			event.Author = agentName
			event.Content = &genai.Content{Role: string(genai.RoleModel), Parts: []*genai.Part{{FunctionCall: &genai.FunctionCall{ID: callID, Name: "ask"}}}}
			event.CustomMetadata = map[string]any{customMetaTaskIDKey: taskID, customMetaContextIDKey: contextID}
			if err := svc.AppendEvent(ctx, created.Session, event); err != nil {
				t.Fatal(err)
			}
			statusParts, err := ToA2AParts(event.Content.Parts, nil)
			if err != nil {
				t.Fatal(err)
			}
			status := a2a.TaskStatus{State: a2a.TaskStateInputRequired, Message: a2a.NewMessage(a2a.MessageRoleAgent, statusParts...)}
			cfg := RunnerConfig{AppName: appName, Agent: newRemoteStateAgent(t, agentName, remoteCfg), SessionService: svc}
			e := NewExecutor(ExecutorConfig{RunnerProvider: func(context.Context, *a2asrv.ExecutorContext, *plugin.Plugin) (RunnerConfig, Runner, error) {
				return cfg, nil, nil
			}})
			// A cancel request (Message nil) for a stored input-required task
			// that the cancel moved to canceled: the shape Cleanup acts on.
			execCtx := &a2asrv.ExecutorContext{ContextID: contextID, StoredTask: &a2a.Task{ID: "parent", ContextID: contextID, Status: status}}
			result := &a2a.Task{ID: "parent", ContextID: contextID, Status: a2a.TaskStatus{State: a2a.TaskStateCanceled}}
			var cancelErr error
			var panicked any
			func() {
				defer func() { panicked = recover() }()
				e.Cleanup(ctx, execCtx, result, nil)
			}()
			mu.Lock()
			defer mu.Unlock()
			_, reached := authByMethod["CancelTask"]
			t.Logf("provider saw ctx %s; CancelTask reached remote=%v; cancel err=%v; panic=%v", providerSaw, reached, cancelErr, panicked)
		})
	}
}

The mintCall comment credits auth/gcp with the opposite design

The comment at auth.go:372-382 says a caller arriving after the attempt's deadline "retires the attempt and starts a new one", and that "auth/gcp's provider reached the same design for the same reason". auth/gcp says the opposite at provider.go:399-402: "A hung attempt is not abandoned. The lookup cannot be cancelled, so retiring it would start a fresh one every initTimeout". The two agree on the shared deadline and nothing else.

The cost auth/gcp's comment names does show up here. With a token source that never returns and mintTimeout lowered to 20ms, ten requests for one scope, each arriving after the previous attempt's deadline, left ten goroutines parked. In production that is up to one goroutine per scope every 30 seconds, for as long as the endpoint hangs. That may be the right trade against the permanent wedge the comment describes, but the comment gives a reason that is not true, and anyone who opens auth/gcp to understand it will find the opposite argument. The comment has to describe the trade this code actually makes.

The inline notes are optional. One of them asks for a follow-up test.

[wolo-lab]

Both are fixed in the latest fix commit (aa36981).

The ClientProvider remediation on the adka2a cancel. The error and the field doc now say the provider is called from two places. When the agent runs, the provider gets the invocation context, so the scope is computed there and attached to every call. When an adka2a server cancels an abandoned child task, the provider gets the server's request context, which is not an agent.InvocationContext and carries no scope, so the assertion has to be checked. TestCancelChildInputRequiredTasksCustomProviderContext drives that cancel through a custom provider and asserts it sees neither an invocation nor a scope, and that the cancel still goes out.

The mintCall comment. It now describes the trade this code makes. It shares the attempt's deadline with auth/gcp, and past the deadline it does what auth/gcp declines to do: it retires the attempt, so one more goroutine per scope stays parked every mintTimeout while an endpoint hangs. The reason given is that nothing promises the step returns — Token() takes no context and the JWT and ADC sources post through http.DefaultClient, which has no timeout — so a kept attempt could lock the identity out even after the endpoint recovers.

[wolo-lab]

One more commit on top of the fixes for your review (3a12fb0). Re-checking this round turned up five lines this PR changed that no test could fail on, and one of them no longer had a job:

• The re-wrap of the cleanup CancelTask context is gone. The transport recovers the invocation from the context's values whatever type the context has, so that call now takes the plain timeout context, as on main. The re-wrap for RemoteTaskCleanupCallback stays, since that callback's doc promises it the invocation context.
• WithICDelta with a delta that carries no context, or with no delta at all, is now covered. Swapping its && for || would have dereferenced a nil context with the suite green.
• RoundTrip closing the request body on its early returns is now checked.
• The redirect table gains rows where only one half of "http to https" holds, and one that pairs an explicit default port with an implicit one.
• The cleartext warning test runs three invocations instead of two. Two could not tell "warn the first time" from "warn every time but the first", since both log once.

[karolpiotrowicz]

Replaced by a non-blocking review below. The inline notes here still apply.

Changing this to a non-blocking comment.

[karolpiotrowicz]

Nothing here blocks merging. The one thing I would still change is the mintCall comment, which gives auth/gcp a reason for its design that auth/gcp's own doc rules out.

The ClientProvider point from the last change request is fixed. The doc and the error now name the adka2a cancel as the one call without an invocation, and TestCancelChildInputRequiredTasksCustomProviderContext fails if that cancel starts attaching a scope for a provider that does not own it. Dropping the re-wrap on the cleanup CancelTask in 3a12fb0 is safe: the transport does recover the invocation from the context's values, and removing that recovery fails TestRemoteAgent_AuthCleanupReusesTheInvocationCredential and two other tests. Each of the new tests fails on the change it was written for. I re-ran the build, tests, vet and lint on this head, and they are clean.

Should change: the auth/gcp comparison in the mintCall comment

The comment at auth.go:381-386 says auth/gcp keeps a hung attempt "because its lookup eventually returns and publishes". It then gives this group's reason as "nothing here promises the step returns". auth/gcp makes no such promise. Its ErrClientUnavailable doc at provider.go:98-103 covers the lookup that never returns. The provider keeps that attempt for the rest of the process, fails every later call, and calls this "the deliberate trade rather than an oversight".

So both packages face the same step that may never return, and they pick opposite costs. auth/gcp accepts a permanent lockout so that it never starts uncancellable lookups on a timer. This group accepts parked goroutines so that it never locks an identity out. The rest of the comment describes this code's side of that trade correctly. The clause about why auth/gcp keeps its attempt is the part that has to match what auth/gcp says.

Worth fixing in the same pass: two comments still give the old reason for the re-wrap

The new comment above the cleanup at a2a_agent.go:674-682 says the re-wrap is there for RemoteTaskCleanupCallback, and that the CancelTask needs no help because the transport reads the invocation from the context's values. Two comments outside this commit still say the re-wrap exists for the credential provider:

• The reattachInvocation doc at auth.go:136-139 names context.WithTimeout as a reason for it and says the re-wrap keeps the provider's type assertion working. Its one remaining caller passes a context.WithoutCancel, and the provider no longer depends on it.
• The doc of TestRemoteAgent_CleanupContextTypeTracksAuth at auth_test.go:1621-1625 says "the wrapper exists so a credential provider can still recover the ADK context there".

Someone reading either one would put the WithTimeout re-wrap back for a reason the new comment says does not exist.

Not blocking: what a provider can recover on the adka2a cancel

The ClientProvider doc at a2a_agent.go:343-356 offers CredentialScope as the key. For the adka2a cancel, it says to "identify the caller from that request instead". On that path the session part of the key never reaches the provider. adka2a builds the session ID from the A2A context ID, which it receives as an argument (metadata.go:61-69), and neither adka2a nor a2a-go v2.5.0 puts that ID on the context the provider gets (executor.go:330-334).

The caller's user is on that context through a2asrv.CallContextFrom(ctx).User.Name, but only when the server runs an authenticator. Without one, the user ID adka2a uses is A2A_USER_ plus the context ID, which cannot be recovered either. A provider that keys its credentials on CredentialScope, as the paragraph suggests, therefore cannot find them on this call, and the child task's cancel goes out unauthenticated or not at all. It would help to say what a provider can rely on there, so nobody builds on a key they cannot rebuild.

The inline notes on my previous review still apply, and they are follow-ups and nits.

[wolo-lab]

All of this is in the latest commit (c3e9d8d), along with the four inline notes.

The auth/gcp comparison. The mintCall comment now describes it as you put it. Both face a step that may never return. auth/gcp keeps the hung attempt and accepts a permanent lockout, so it never starts uncancellable lookups on a timer, and the comment points at its ErrClientUnavailable doc. This group retires the attempt and accepts a parked goroutine per scope per mintTimeout, so a hang never locks an identity out.

The two stale re-wrap comments. reattachInvocation's doc now says its caller is the cleanup's context.WithoutCancel, that it exists for RemoteTaskCleanupCallback, and that the provider does not depend on it. TestRemoteAgent_CleanupContextTypeTracksAuth's doc now describes the callback contract rather than the provider.

What a provider can rely on during the adka2a cancel. The ClientProvider doc now says that CredentialScope cannot be rebuilt on that call, because nothing on its context carries the session id. It says the only thing a provider can rely on there is the authenticated caller, through a2asrv.CallContextFrom(ctx), and only when the server runs an authenticator. Without one, nothing on that call identifies the caller.

Since these were all comments that had drifted from the code, I went through every comment and error message the PR adds and checked each against the code it describes. That turned up more of the same, now fixed:

• The Auth doc and the transport's doc said a credential always applies itself, as under mcptoolset. A bare OAuth2Credential is minted here instead.
• mintGroup's doc said at most one step per scope runs at a time, which a retired attempt makes untrue.
• The Auth doc said a redirect that leaves the card's scheme is refused. An upgrade to https on the same host is allowed.
• redactTokenError kept error_uri as a "short enumerable field". It is free text parsed out of the same body, so it now goes with error_description. I also removed the claim that the endpoint puts a signed assertion in error_description, which was never verified. The PR description carried both, and it is updated.
• Several test comments overstated what their test catches.

Along the way, the auth.CredentialProvider contract tells a provider to recover the acting user with agent.IdentityFromContext, not by type-asserting. The Auth doc now says so, and TestRemoteAgent_AuthProviderSeesTheIdentity pins it on the card fetch and the send.

[karolpiotrowicz]

Nothing here blocks merge. The auth/gcp comparison in the mintCall comment now matches what auth/gcp's ErrClientUnavailable doc says, and the reattachInvocation doc, the cleanup-context test doc and the ClientProvider paragraph all describe the code as it is now.

Each new test fails on the change it was written for. Giving every request fresh single-flight groups, or never joining a step already in flight, fails both TestAuthTransportShares* tests. Moving bodyOwned = false up to just after req.Clone fails the new apply-failure case, and putting error_uri back fails TestRedactTokenError. I re-ran the build, tests, vet, lint and -race on this head and on a merge with current main, and they are clean.

Three test comments from the comment pass still claim more than their test checks. They are inline, and none of them needs to hold this up.

The branch now conflicts with #1697 and #1324 in agent/remoteagent/v2/a2a_agent.go, so whichever lands second will need a rebase.

[karolpiotrowicz]

Approving. The inline test-comment notes are non-blocking.