|
| 1 | +# Plan: MCP bean lookup by type, not hard-coded name |
| 2 | + |
| 3 | +## Scope |
| 4 | + |
| 5 | +`McpPlugin.getMcpClients()` currently does: |
| 6 | + |
| 7 | +```java |
| 8 | +if (applicationContext.containsBean("mcpSyncClients")) { |
| 9 | + Object bean = applicationContext.getBean("mcpSyncClients"); |
| 10 | + if (bean instanceof List<?> clientList && !clientList.isEmpty()) { |
| 11 | + mcpClients = (List<McpSyncClient>) clientList; |
| 12 | + } |
| 13 | +} |
| 14 | +``` |
| 15 | + |
| 16 | +This is brittle: |
| 17 | +- Hard-coded bean name — any Spring AI rename (e.g. `mcpAsyncClients` vs |
| 18 | + `mcpSyncClients`, or a future `mcpClientRegistry`) breaks the integration |
| 19 | + silently. |
| 20 | +- Users who define additional `McpSyncClient` beans outside the Spring AI |
| 21 | + auto-configuration aren't picked up. |
| 22 | +- The unchecked cast is a runtime landmine if anyone ever puts a |
| 23 | + differently-typed `List` bean at that name. |
| 24 | + |
| 25 | +## Fix |
| 26 | + |
| 27 | +Replace with type-based lookup: |
| 28 | + |
| 29 | +```java |
| 30 | +Map<String, McpSyncClient> beans = |
| 31 | + applicationContext.getBeansOfType(McpSyncClient.class); |
| 32 | +``` |
| 33 | + |
| 34 | +Collect `beans.values()` into a `List<McpSyncClient>`. This picks up: |
| 35 | +- Whatever Spring AI's MCP auto-configuration registers today. |
| 36 | +- Any user-defined `McpSyncClient @Bean` declarations. |
| 37 | +- Any future rename by Spring AI, as long as the type stays the same. |
| 38 | + |
| 39 | +## Files to change |
| 40 | + |
| 41 | +- `src/main/java/io/temporal/springai/plugin/McpPlugin.java` |
| 42 | + - Replace the `containsBean("mcpSyncClients")` block in `getMcpClients()` |
| 43 | + with `applicationContext.getBeansOfType(McpSyncClient.class)`. |
| 44 | + - Remove the `@SuppressWarnings("unchecked")` and the unchecked cast. |
| 45 | + - Log the bean names discovered, not just the count, to aid debugging. |
| 46 | + - Keep `McpClientActivityImpl`'s duplicate-name check — if two clients |
| 47 | + advertise the same `clientInfo().name()`, that still needs to error. |
| 48 | + |
| 49 | +## Test plan |
| 50 | + |
| 51 | +- Unit test with a mocked `ApplicationContext.getBeansOfType` returning two |
| 52 | + distinct `McpSyncClient` beans — verify both reach |
| 53 | + `McpClientActivityImpl`. |
| 54 | +- Unit test where the context has zero `McpSyncClient` beans — verify the |
| 55 | + plugin stays in the "no clients yet" state and |
| 56 | + `SmartInitializingSingleton.afterSingletonsInstantiated` handles it |
| 57 | + cleanly. |
| 58 | + |
| 59 | +## PR |
| 60 | + |
| 61 | +**Title:** `temporal-spring-ai: discover MCP clients by type, not by bean name` |
| 62 | + |
| 63 | +**Body:** |
| 64 | + |
| 65 | +``` |
| 66 | +## What was changed |
| 67 | +`McpPlugin.getMcpClients()` now calls |
| 68 | +`ApplicationContext.getBeansOfType(McpSyncClient.class)` instead of |
| 69 | +looking up the hard-coded bean name `"mcpSyncClients"`. The unchecked |
| 70 | +cast is removed; discovered bean names are logged for easier debugging. |
| 71 | +
|
| 72 | +## Why? |
| 73 | +The old bean-name lookup broke silently whenever Spring AI changed the |
| 74 | +name its MCP auto-configuration used, and it ignored user-defined |
| 75 | +`McpSyncClient` beans entirely. Type-based discovery is more robust, |
| 76 | +works with arbitrary user configuration, and removes an unchecked cast. |
| 77 | +``` |
0 commit comments