Skip to content

fix(nacos-starter): auto-configurations should be opt-in (matchIfMissing=false) and fix A2A server-addr override #1709

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
BukJiang wants to merge 1 commit into
base: main
Choose a base branch
from
+5 −5
Open

fix(nacos-starter): auto-configurations should be opt-in (matchIfMissing=false) and fix A2A server-addr override #1709

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions ...er/src/main/java/io/agentscope/spring/boot/nacos/AgentscopeA2aNacosAutoConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
prefix = NacosConstants.A2A_NACOS_PREFIX,
name = "enabled",
havingValue = "true",
matchIfMissing = true)
matchIfMissing = false)

@AgentScopeJavaBot AgentScopeJavaBot Jun 11, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[critical] Changing matchIfMissing to false is correct for the production code, but at least 10 existing tests in AgentscopeA2aNacosAutoConfigurationTest will break because they never set agentscope.a2a.nacos.enabled=true. Tests like shouldCreateDefaultBeansWhenEnabled, shouldCreateNacosAgentCardResolverBean, shouldCreateNacosAgentRegistryBean, shouldNotCreateNacosAgentCardResolverWhenDiscoveryDisabled, shouldNotCreateNacosAgentRegistryWhenRegistryDisabled, shouldBindNacosProperties, shouldCreateWithCustomAgentCardResolver, shouldCreateWithCustomAgentRegistry, shouldCreateBeansWithRegistryProperties, shouldCreateBeansWithDiscoveryProperties, and shouldCloseNacosServiceOnShutdown all rely on the old matchIfMissing=true behavior.

All positive test cases must be updated to add agentscope.a2a.nacos.enabled=true. Additionally, tests that expect AgentCardResolver or AgentRegistry beans must also set agentscope.a2a.nacos.discovery.enabled=true and/or agentscope.a2a.nacos.registry.enabled=true respectively, since those bean-level conditions also changed to matchIfMissing=false.

Example fix for shouldCreateDefaultBeansWhenEnabled:

.withPropertyValues(
    "agentscope.a2a.nacos.enabled=true",
    "agentscope.a2a.nacos.discovery.enabled=true",
    "agentscope.a2a.nacos.registry.enabled=true",
    "agentscope.a2a.nacos.server-addr=127.0.0.1:8848",
    "agentscope.a2a.nacos.namespace=public")

@AgentScopeJavaBot AgentScopeJavaBot Jun 11, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[critical] Changing matchIfMissing to false is correct for the production code, but at least 10 existing tests in AgentscopeA2aNacosAutoConfigurationTest will break because they never set agentscope.a2a.nacos.enabled=true. Tests like shouldCreateDefaultBeansWhenEnabled, shouldCreateNacosAgentCardResolverBean, shouldCreateNacosAgentRegistryBean, shouldNotCreateNacosAgentCardResolverWhenDiscoveryDisabled, shouldNotCreateNacosAgentRegistryWhenRegistryDisabled, shouldBindNacosProperties, shouldCreateWithCustomAgentCardResolver, shouldCreateWithCustomAgentRegistry, shouldCreateBeansWithRegistryProperties, shouldCreateBeansWithDiscoveryProperties, and shouldCloseNacosServiceOnShutdown all rely on the old matchIfMissing=true behavior.

All positive test cases must be updated to add agentscope.a2a.nacos.enabled=true. Additionally, tests that expect AgentCardResolver or AgentRegistry beans must also set agentscope.a2a.nacos.discovery.enabled=true and/or agentscope.a2a.nacos.registry.enabled=true respectively, since those bean-level conditions also changed to matchIfMissing=false.

Example fix for shouldCreateDefaultBeansWhenEnabled:

.withPropertyValues(
    "agentscope.a2a.nacos.enabled=true",
    "agentscope.a2a.nacos.discovery.enabled=true",
    "agentscope.a2a.nacos.registry.enabled=true",
    "agentscope.a2a.nacos.server-addr=127.0.0.1:8848",
    "agentscope.a2a.nacos.namespace=public")

public class AgentscopeA2aNacosAutoConfiguration implements Closeable {

private static final Logger log =
Expand All @@ -81,7 +81,7 @@ private AiService a2aService(
AgentScopeA2aNacosProperties a2aNacosProperties)
throws NacosException {
Properties nacosClientProperties = nacosProperties.getNacosProperties();
nacosClientProperties.putAll(a2aNacosProperties.getNacosProperties());
nacosClientProperties.putAll(a2aNacosProperties.getExplicitNacosProperties());

@AgentScopeJavaBot AgentScopeJavaBot Jun 11, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[praise] Good fix. Using getExplicitNacosProperties() instead of getNacosProperties() is the correct approach for the overlay/merge pattern — it avoids injecting default serverAddr=127.0.0.1:8848 which would silently overwrite the user-configured agentscope.nacos.server-addr via putAll. This is now consistent with AgentscopeNacosPromptAutoConfiguration (line 68) which already uses getExplicitNacosProperties().

@AgentScopeJavaBot AgentScopeJavaBot Jun 11, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[praise] Good fix. Using getExplicitNacosProperties() instead of getNacosProperties() is the correct approach for the overlay/merge pattern — it avoids injecting default serverAddr=127.0.0.1:8848 which would silently overwrite the user-configured agentscope.nacos.server-addr via putAll. This is now consistent with AgentscopeNacosPromptAutoConfiguration (line 68) which already uses getExplicitNacosProperties().

return AiFactory.createAiService(nacosClientProperties);
}

Expand Down Expand Up @@ -112,7 +112,7 @@ public void close() throws IOException {
prefix = NacosConstants.A2A_NACOS_DISCOVERY_PREFIX,
name = "enabled",
havingValue = "true",
matchIfMissing = true)
matchIfMissing = false)
public AgentCardResolver nacosAgentCardResolver() {
return new NacosAgentCardResolver(a2aService);
}
Expand All @@ -128,7 +128,7 @@ public AgentCardResolver nacosAgentCardResolver() {
prefix = NacosConstants.A2A_NACOS_REGISTRY_PREFIX,
name = "enabled",
havingValue = "true",
matchIfMissing = true)
matchIfMissing = false)
public AgentRegistry nacosAgentRegistry(AgentScopeA2aNacosProperties a2aNacosProperties) {
return NacosAgentRegistry.builder(a2aService)
.nacosA2aProperties(buildNacosA2aProperties(a2aNacosProperties))
Expand Down
2 changes: 1 addition & 1 deletion ...src/main/java/io/agentscope/spring/boot/nacos/AgentscopeNacosPromptAutoConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
prefix = NacosConstants.NACOS_PROMPT_PREFIX,
name = "enabled",
havingValue = "true",
matchIfMissing = true)
matchIfMissing = false)
public class AgentscopeNacosPromptAutoConfiguration {

@Bean(name = "agentscopePromptAiService")
Expand Down
Loading