fix(a2a-server,core-tool): harden host fallback and guard schema definition conflicts#1191
fix(a2a-server,core-tool): harden host fallback and guard schema definition conflicts#1191RuleViz wants to merge 1 commit intoagentscope-ai:mainfrom
Conversation
- fallback deployment host to localhost when local IP lookup fails\n- detect conflicting schema definitions during defs hoisting\n- add regression tests for fallback and conflict/equivalent defs behavior\n\nCo-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Improves reliability in A2A server deployment defaults and tool JSON Schema generation by adding safer fallbacks and fail-fast behavior for schema definition conflicts.
Changes:
- Fall back to
localhostwhen local IP resolution throwsSocketException, and warn with the exception. - Detect and reject conflicting
$defs/definitionsentries during schema hoisting instead of silently overwriting. - Add regression tests covering both the host fallback behavior and
$defsconflict/equivalence handling.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| agentscope-extensions/agentscope-extensions-a2a/agentscope-extensions-a2a-server/src/main/java/io/agentscope/core/a2a/server/transport/DeploymentProperties.java | Adds localhost fallback + warning log when local IP lookup fails. |
| agentscope-extensions/agentscope-extensions-a2a/agentscope-extensions-a2a-server/src/test/java/io/agentscope/core/a2a/server/transport/DeploymentPropertiesTest.java | Updates expectation to assert localhost on local IP lookup failure. |
| agentscope-core/src/main/java/io/agentscope/core/tool/ToolSchemaGenerator.java | Guards $defs hoisting against conflicting definitions using equality checks + fail-fast exception. |
| agentscope-core/src/test/java/io/agentscope/core/tool/ToolSchemaGeneratorTest.java | Adds unit tests for conflicting vs equivalent $defs merges via reflective invocation. |
| } catch (SocketException exception) { | ||
| host = DEFAULT_HOST; | ||
| log.warn( | ||
| "Failed to resolve local IP address, fallback to default host: {}", |
There was a problem hiding this comment.
Log message grammar: "fallback" is a noun; as a verb it should be "fall back" (e.g., "...; falling back to default host"). Adjusting this improves readability/searchability in logs.
| "Failed to resolve local IP address, fallback to default host: {}", | |
| "Failed to resolve local IP address, falling back to default host: {}", |
| } | ||
| if (!Objects.equals(existingDef, incomingDef)) { | ||
| throw new IllegalStateException( | ||
| "Conflicting schema definition found for key: " + defKey); |
There was a problem hiding this comment.
The IllegalStateException message will be useful for debugging schema generation failures, but it could be more actionable by including whether the conflict came from "$defs" vs "definitions" and a short hint that this can be caused by simple-name collisions under PLAIN_DEFINITION_KEYS. Consider enriching the message accordingly.
| "Conflicting schema definition found for key: " + defKey); | |
| "Conflicting schema definition found for key '" | |
| + defKey | |
| + "' while hoisting from '" | |
| + key | |
| + "'. This can be caused by simple-name collisions when using plain definition keys."); |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
This PR fixes two reliability issues across A2A server setup and tool schema generation.
localhostwhen local IP lookup throws aSocketException, and logs a warning instead of leaving host asnull.$defshoisting now detects conflicting definitions for the same key and throws an explicit exception instead of silently overriding previous entries.The change includes regression tests for both cases:
$defsmerge behavior for both conflicting and equivalent definitions