Skip to content

fix: support complete agent card URL in A2ACardResolver#837

Open
ehsavoie wants to merge 1 commit intoa2aproject:mainfrom
ehsavoie:issue_771
Open

fix: support complete agent card URL in A2ACardResolver#837
ehsavoie wants to merge 1 commit intoa2aproject:mainfrom
ehsavoie:issue_771

Conversation

@ehsavoie
Copy link
Copy Markdown
Collaborator

URI.resolve() silently drops intermediate path segments when the
resolved path starts with '/', causing incorrect URLs for agents
hosted under a path prefix (e.g. /spec03). Replace with direct
concatenation and add a pass-through guard when the supplied URL
already ends with the agent card path.

Fixes #771 🦕

@ehsavoie ehsavoie requested a review from jmesnil April 29, 2026 09:59
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

The pull request enhances A2ACardResolver to accept either a base URL or a complete agent card URL, updating the internal URL construction logic to handle these variations correctly. It also introduces several unit tests to validate URL resolution with tenants, custom paths, and authentication headers, while improving error handling in the test suite. Feedback was provided to address a potential double-slash issue when concatenating the base URL and the agent card path.

Comment thread http-client/src/main/java/org/a2aproject/sdk/client/http/A2ACardResolver.java Outdated
  URI.resolve() silently drops intermediate path segments when the
  resolved path starts with '/', causing incorrect URLs for agents
  hosted under a path prefix (e.g. /spec03). Replace with direct
  concatenation and add a pass-through guard when the supplied URL
  already ends with the agent card path.

Signed-off-by: Emmanuel Hugonnet <ehugonne@redhat.com>
@ehsavoie
Copy link
Copy Markdown
Collaborator Author

/gemini review

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the A2ACardResolver to support base URLs that already include the agent card path, ensuring the resolver uses such URLs verbatim. The URL construction logic was modified to handle manual concatenation when URI.resolve() is insufficient, and comprehensive unit tests were added to cover new functionality and error cases. Feedback was provided regarding a potential double-slash issue in the manual URL concatenation logic, along with a suggestion for a more robust normalization approach.

Comment on lines +169 to +176
// URI.resolve() drops the base path for absolute paths (starting with '/'),
// so we concatenate directly, trimming a trailing slash from the base when
// the path already starts with '/' to avoid double slashes.
String base = (builtBaseUrl.endsWith("/") && effectiveAgentCardPath.startsWith("/"))
? builtBaseUrl.substring(0, builtBaseUrl.length() - 1)
: builtBaseUrl;
String separator = effectiveAgentCardPath.startsWith("/") ? "" : "/";
this.url = new URI(base + separator + effectiveAgentCardPath).toString();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

The current logic for URL concatenation is complex and has a bug that can create double slashes. For instance, if builtBaseUrl is http://example.com/ and effectiveAgentCardPath is path, the resulting URL would be http://example.com//path.

The logic can be simplified to be more robust and readable by explicitly normalizing slashes from both parts before joining them.

                // URI.resolve() drops the base path for absolute paths (starting with '/'),
                // so we concatenate directly, normalizing slashes to avoid duplicates.
                String base = builtBaseUrl.endsWith("/") ? builtBaseUrl.substring(0, builtBaseUrl.length() - 1) : builtBaseUrl;
                String path = effectiveAgentCardPath.startsWith("/") ? effectiveAgentCardPath.substring(1) : effectiveAgentCardPath;
                this.url = new URI(base + "/" + path).toString();

Copy link
Copy Markdown
Contributor

@jmesnil jmesnil left a comment

Choose a reason for hiding this comment

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

I'm not sure if we should make the card resolver more complex.
I see 2 use case for fetching an agent card:

  • use the spec's well-known endpoint that is appended to whatever the baseUrl is (it can be http://example.com, http://example.com/agents/v1/foo)
  • point to a specific URL that returns an agent card. This does not have to end with .well-known/.agent-card.json. This URL can be fetched from a registry for example. In that case, there is no URL resolution at all, it's used verbatim.

So the API would be:

  • A2AResolver(String url) (and all its variant)
  • AgentCard getWellKnownAgentCard() -> append `.well-known/.agent-card.json to the base URL
  • AgentCard getAgentCard() -> use the URL verbatim

That's be a API break change so maybe we could have just a new method:

  • AgentCard getAgentCard(boolean fromWellKnownLocation)
    • getAgentCard() delegates to getAgentCard(true)
    • the resolution of the URL is deferred until getAgentCard(boolean) is called to figure out which URL to connect to.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: spec.AgentCard does not allow many agents on same domain

2 participants