fix(multitenancy): resolve agent card via router when no default bean… (#1184)

fix(multitenancy): resolve agent cards via router in all transports

- Thread AgentCardRouter through handlers for tenant-scoped card resolution
- Map TenantNotFoundException to client errors instead of HTTP 500
- Gate extended card router on non-blank tenant; fall back to CDI bean otherwise
- Track per-card validation with ConcurrentHashMap instead of single AtomicBoolean

Fixes #1182

Signed-off-by: Emmanuel Hugonnet <ehugonne@redhat.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>

Author: ehsavoie

server-common/src/main/java/org/a2aproject/sdk/server/AgentCardValidator.java

@@ -5,19 +5,21 @@
 import java.util.List;
 import java.util.ServiceLoader;
 import java.util.Set;
-import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.function.Consumer;
 import java.util.function.Supplier;
 import java.util.logging.Logger;
 import java.util.stream.Collectors;
 
 import jakarta.enterprise.inject.Instance;
 
+import org.a2aproject.sdk.server.multitenancy.AgentCardRouter;
+import org.a2aproject.sdk.server.multitenancy.TenantNotFoundException;
 import org.a2aproject.sdk.server.util.CdiUtils;
 import org.a2aproject.sdk.spec.AgentCard;
-import org.jspecify.annotations.Nullable;
 import org.a2aproject.sdk.spec.AgentInterface;
 import org.a2aproject.sdk.spec.TransportProtocol;
+import org.jspecify.annotations.Nullable;
 
 /**
  * Validates AgentCard transport configuration against available transport endpoints.
@@ -35,41 +37,52 @@ public class AgentCardValidator {
     public static final String SKIP_GRPC_PROPERTY = "org.a2aproject.sdk.transport.grpc.skipValidation";
     public static final String SKIP_REST_PROPERTY = "org.a2aproject.sdk.transport.rest.skipValidation";
 
+    /**
+     * Creates a new thread-safe set for tracking which {@link AgentCard} instances have already
+     * been validated. Each distinct card is validated at most once; subsequent requests for the
+     * same card skip validation.
+     *
+     * @return a concurrent set suitable for use as the {@code validatedCards} parameter
+     */
+    public static Set<AgentCard> newValidatedCardsSet() {
+        return ConcurrentHashMap.newKeySet();
+    }
+
     /**
      * Resolves an {@link AgentCard} from the given {@link Instance} and validates its transport
-     * configuration exactly once using the default {@link #validateTransportConfiguration} check.
-     * The {@code transportValidated} guard is reset on failure so validation can be retried on
-     * the next call.
+     * configuration once per distinct card using the default
+     * {@link #validateTransportConfiguration} check. On failure the card is removed from the
+     * set so validation can be retried on the next call.
      *
      * @param agentCardInstance the CDI instance holding the agent card
-     * @param transportValidated atomic guard ensuring validation runs once
+     * @param validatedCards set tracking which cards have already been validated
      * @return the resolved agent card
      */
     public static AgentCard resolveAndValidateOnce(Instance<AgentCard> agentCardInstance,
-            AtomicBoolean transportValidated) {
-        return resolveAndValidateOnce(agentCardInstance::get, transportValidated,
+            Set<AgentCard> validatedCards) {
+        return resolveAndValidateOnce(agentCardInstance::get, validatedCards,
                 AgentCardValidator::validateTransportConfiguration);
     }
 
     /**
      * Obtains an {@link AgentCard} from the given supplier and applies the provided validator
-     * exactly once. The {@code transportValidated} guard is reset on failure so validation can
-     * be retried on the next call.
+     * once per distinct card. On failure the card is removed from the set so validation can be
+     * retried on the next call.
      *
      * @param agentCardSupplier supplier that produces the agent card
-     * @param transportValidated atomic guard ensuring validation runs once
+     * @param validatedCards set tracking which cards have already been validated
      * @param validator validation logic to apply on first access
      * @return the resolved agent card
      */
     public static AgentCard resolveAndValidateOnce(Supplier<AgentCard> agentCardSupplier,
-            AtomicBoolean transportValidated,
+            Set<AgentCard> validatedCards,
             Consumer<AgentCard> validator) {
         AgentCard card = agentCardSupplier.get();
-        if (transportValidated.compareAndSet(false, true)) {
+        if (validatedCards.add(card)) {
             try {
                 validator.accept(card);
             } catch (RuntimeException e) {
-                transportValidated.set(false);
+                validatedCards.remove(card);
                 throw e;
             }
         }
@@ -82,22 +95,118 @@ public static AgentCard resolveAndValidateOnce(Supplier<AgentCard> agentCardSupp
      *
      * @param publicCard the CDI instance for the {@code @PublicAgentCard}
      * @param extendedCard the CDI instance for the {@code @ExtendedAgentCard}, may be {@code null}
-     * @param transportValidated atomic guard ensuring validation runs once
+     * @param validatedCards set tracking which cards have already been validated
      * @return the resolved agent card
      * @throws IllegalStateException if neither card is available
      */
     public static AgentCard resolveWithFallback(Instance<AgentCard> publicCard,
             @Nullable Instance<AgentCard> extendedCard,
-            AtomicBoolean transportValidated) {
+            Set<AgentCard> validatedCards) {
+        return resolveWithFallback(publicCard, extendedCard, null, null, validatedCards);
+    }
+
+    /**
+     * Convenience overload that uses the default {@link #validateTransportConfiguration} validator.
+     *
+     * @see #resolveWithFallback(Instance, Instance, AgentCardRouter, String, Set, Consumer)
+     */
+    public static AgentCard resolveWithFallback(Instance<AgentCard> publicCard,
+            @Nullable Instance<AgentCard> extendedCard,
+            @Nullable AgentCardRouter agentCardRouter,
+            @Nullable String tenant,
+            Set<AgentCard> validatedCards) {
+        return resolveWithFallback(publicCard, extendedCard, agentCardRouter, tenant, validatedCards,
+                AgentCardValidator::validateTransportConfiguration);
+    }
+
+    /**
+     * Resolves an agent card using one of two resolution strategies depending on whether a
+     * tenant-scoped request is being made, and validates using the supplied validator.
+     *
+     * @param publicCard the CDI instance for the {@code @PublicAgentCard}
+     * @param extendedCard the CDI instance for the {@code @ExtendedAgentCard}, may be {@code null}
+     * @param agentCardRouter optional router for tenant-specific card resolution
+     * @param tenant the tenant identifier, may be {@code null}
+     * @param validatedCards set tracking which cards have already been validated
+     * @param validator validation logic to apply on first access of each distinct card
+     * @return the resolved agent card
+     * @throws TenantNotFoundException if a non-blank tenant is specified, a router is available,
+     *         but neither a public nor extended card is registered for that tenant
+     * @throws IllegalStateException if no card can be resolved (non-tenant-scoped path)
+     * @see #resolveTenantScoped(AgentCardRouter, String, Set, Consumer)
+     * @see #resolveDefaultWithRouterFallback(Instance, Instance, AgentCardRouter, String, Set, Consumer)
+     */
+    public static AgentCard resolveWithFallback(Instance<AgentCard> publicCard,
+            @Nullable Instance<AgentCard> extendedCard,
+            @Nullable AgentCardRouter agentCardRouter,
+            @Nullable String tenant,
+            Set<AgentCard> validatedCards,
+            Consumer<AgentCard> validator) {
+        if (tenant != null && !tenant.isBlank() && agentCardRouter != null) {
+            return resolveTenantScoped(agentCardRouter, tenant, validatedCards, validator);
+        }
+        return resolveDefaultWithRouterFallback(publicCard, extendedCard, agentCardRouter, tenant,
+                validatedCards, validator);
+    }
+
+    /**
+     * Tenant-scoped resolution: only the {@link AgentCardRouter} is consulted. CDI default beans
+     * are <em>not</em> used as fallbacks — doing so would let the request proceed against the
+     * wrong tenant's card.
+     * <ol>
+     *   <li>Tenant-specific public card via the router</li>
+     *   <li>Tenant-specific extended card via the router</li>
+     *   <li>{@link TenantNotFoundException} if neither is registered</li>
+     * </ol>
+     */
+    private static AgentCard resolveTenantScoped(AgentCardRouter agentCardRouter, String tenant,
+            Set<AgentCard> validatedCards, Consumer<AgentCard> validator) {
+        AgentCard routerCard = agentCardRouter.resolvePublicCard(tenant);
+        if (routerCard != null) {
+            return resolveAndValidateOnce(() -> routerCard, validatedCards, validator);
+        }
+        AgentCard routerExtCard = agentCardRouter.resolveExtendedCard(tenant);
+        if (routerExtCard != null) {
+            return resolveAndValidateOnce(() -> routerExtCard, validatedCards, validator);
+        }
+        throw new TenantNotFoundException(tenant);
+    }
+
+    /**
+     * Non-tenant-scoped resolution with optional router fallback:
+     * <ol>
+     *   <li>Unqualified {@code @PublicAgentCard} CDI bean</li>
+     *   <li>Router's public card (when no default bean exists)</li>
+     *   <li>Unqualified {@code @ExtendedAgentCard} CDI bean</li>
+     *   <li>Router's extended card (when no default bean exists)</li>
+     *   <li>{@link IllegalStateException} if nothing resolves</li>
+     * </ol>
+     */
+    private static AgentCard resolveDefaultWithRouterFallback(Instance<AgentCard> publicCard,
+            @Nullable Instance<AgentCard> extendedCard,
+            @Nullable AgentCardRouter agentCardRouter,
+            @Nullable String tenant,
+            Set<AgentCard> validatedCards,
+            Consumer<AgentCard> validator) {
         AgentCard resolved = CdiUtils.resolveDefault(publicCard);
         if (resolved != null) {
-            return resolveAndValidateOnce(() -> resolved, transportValidated,
-                    AgentCardValidator::validateTransportConfiguration);
+            return resolveAndValidateOnce(() -> resolved, validatedCards, validator);
+        }
+        if (agentCardRouter != null) {
+            AgentCard routerCard = agentCardRouter.resolvePublicCard(tenant);
+            if (routerCard != null) {
+                return resolveAndValidateOnce(() -> routerCard, validatedCards, validator);
+            }
         }
         AgentCard extResolved = CdiUtils.resolveDefault(extendedCard);
         if (extResolved != null) {
-            return resolveAndValidateOnce(() -> extResolved, transportValidated,
-                    AgentCardValidator::validateTransportConfiguration);
+            return resolveAndValidateOnce(() -> extResolved, validatedCards, validator);
+        }
+        if (agentCardRouter != null) {
+            AgentCard routerExtCard = agentCardRouter.resolveExtendedCard(tenant);
+            if (routerExtCard != null) {
+                return resolveAndValidateOnce(() -> routerExtCard, validatedCards, validator);
+            }
         }
         throw new IllegalStateException(NO_AGENT_CARD_MESSAGE);
     }

server-common/src/main/java/org/a2aproject/sdk/server/requesthandlers/DefaultRequestHandler.java

@@ -262,10 +262,15 @@ public class DefaultRequestHandler implements RequestHandler {
      */
     int reconciliationTimeoutSeconds;
 
+    // Only used inside initConfig() (CDI lifecycle). In static create() paths this field
+    // remains null and is never accessed, hence the NullAway suppression.
+    @SuppressWarnings("NullAway")
+    private @Nullable Instance<AgentExecutor> agentExecutorInstance;
+
     // Fields set by constructor injection cannot be final. We need a noargs constructor for
     // Jakarta compatibility, and it seems that making fields set by constructor injection
     // final, is not proxyable in all runtimes
-    private AgentExecutor agentExecutor;
+    private @Nullable AgentExecutor agentExecutor;
     private TaskStore taskStore;
     private QueueManager queueManager;
     private PushNotificationConfigStore pushConfigStore;
@@ -298,6 +303,7 @@ public class DefaultRequestHandler implements RequestHandler {
     @SuppressWarnings("NullAway")
     protected DefaultRequestHandler() {
         // For CDI proxy creation
+        this.agentExecutorInstance = null;
         this.agentExecutor = null;
         this.taskStore = null;
         this.queueManager = null;
@@ -313,22 +319,40 @@ protected DefaultRequestHandler() {
      * releases; application code should use {@link #builder()} to configure and create a handler.
      */
     @Inject
-    public DefaultRequestHandler(AgentExecutor agentExecutor, TaskStore taskStore,
+    public DefaultRequestHandler(@Any Instance<AgentExecutor> agentExecutorInstance, TaskStore taskStore,
                                  QueueManager queueManager, PushNotificationConfigStore pushConfigStore,
                                  MainEventBusProcessor mainEventBusProcessor,
                                  @Internal Executor executor,
                                  @EventConsumerExecutor Executor eventConsumerExecutor) {
+        this.agentExecutorInstance = agentExecutorInstance;
+        this.agentExecutor = null;
+        this.taskStore = taskStore;
+        this.queueManager = queueManager;
+        this.pushConfigStore = pushConfigStore;
+        this.mainEventBusProcessor = mainEventBusProcessor;
+        this.executor = executor;
+        this.eventConsumerExecutor = eventConsumerExecutor;
+        this.requestContextBuilder = () -> new SimpleRequestContextBuilder(taskStore, false, null);
+        this.mainEventBusProcessor.start();
+    }
+
+    /**
+     * Constructor used by the {@link Builder} and tests.
+     * The builder always supplies a concrete executor, so no CDI resolution is needed.
+     */
+    DefaultRequestHandler(AgentExecutor agentExecutor, TaskStore taskStore,
+                           QueueManager queueManager, PushNotificationConfigStore pushConfigStore,
+                           MainEventBusProcessor mainEventBusProcessor,
+                           Executor executor,
+                           Executor eventConsumerExecutor) {
+        this.agentExecutorInstance = null;
         this.agentExecutor = agentExecutor;
         this.taskStore = taskStore;
         this.queueManager = queueManager;
         this.pushConfigStore = pushConfigStore;
         this.mainEventBusProcessor = mainEventBusProcessor;
         this.executor = executor;
         this.eventConsumerExecutor = eventConsumerExecutor;
-        // TODO In Python this is also a constructor parameter defaulting to this SimpleRequestContextBuilder
-        //  implementation if the parameter is null. Skip that for now, since otherwise I get CDI errors, and
-        //  I am unsure about the correct scope.
-        //  Also reworked to make a Supplier since otherwise the builder gets polluted with wrong tasks
         this.requestContextBuilder = () -> new SimpleRequestContextBuilder(taskStore, false, null);
         this.mainEventBusProcessor.start();
     }
@@ -344,6 +368,9 @@ void initConfig() {
                 configProvider.getValue(A2A_BLOCKING_RECONCILIATION_TIMEOUT_SECONDS));
         authorizationProvider = CdiUtils.getIfResolvable(authorizationProviderInstance);
         agentExecutorRouter = CdiUtils.getIfResolvable(agentExecutorRouterInstance);
+        if (agentExecutor == null && agentExecutorInstance != null) {
+            agentExecutor = CdiUtils.resolveDefault(agentExecutorInstance);
+        }
         pushNotificationsEnabled = Boolean.parseBoolean(
                 configProvider.getValue(A2A_PUSH_NOTIFICATIONS_ENABLED));
 
@@ -1436,10 +1463,16 @@ private MessageSendSetup initMessageSend(MessageSendParams params, ServerCallCon
         return new MessageSendSetup(taskManager, task, requestContext);
     }
 
-    private AgentExecutor resolveAgentExecutor(@Nullable String tenant) {
+    AgentExecutor resolveAgentExecutor(@Nullable String tenant) {
         if (agentExecutorRouter != null) {
             return agentExecutorRouter.resolve(tenant);
         }
+        if (agentExecutor == null) {
+            throw new IllegalStateException(
+                    "No AgentExecutor available. Either provide an unqualified AgentExecutor "
+                    + "CDI bean, or add the multitenancy extension (a2a-java-sdk-extras-multitenancy) "
+                    + "which provides an AgentExecutorRouter for @Tenant-qualified beans.");
+        }
         return agentExecutor;
     }