Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
30 changes: 30 additions & 0 deletions cmd/a2a.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@ package cmd
import (
"context"
"encoding/json"
"errors"
"fmt"
"io"
"net"
"net/http"
"net/url"
"strings"
Expand Down Expand Up @@ -119,6 +121,14 @@ type a2aSendOptions struct {
transport string
message string
withAuthorization bool

// unresolvedAddressHint, if non-empty, is printed on stderr when the send
// fails because the address did not resolve in DNS. It is supplied by the
// caller rather than built here because the useful advice depends on how the
// address was arrived at: `agents chat` derived it from the agent's card and
// can name the agent to put in an --address, while `a2a send` was handed the
// address by the user and has nothing to add.
unresolvedAddressHint string
}

// a2aLogger is an http.RoundTripper that logs each A2A request and its status,
Expand Down Expand Up @@ -203,6 +213,20 @@ func a2aUnauthorizedHint(status int, withAuthorization bool) string {
"run `rossoctl login` again to pick up the scopes for it."
}

// isUnresolvedHost reports whether err was caused by a hostname that does not
// resolve.
//
// The match is on *net.DNSError with IsNotFound set, not on the "no such host"
// text. The a2a client wraps the failure twice over (fmt.wrapError around
// *url.Error around *net.DNSError), but the chain survives, so errors.As finds
// it — and matching the type keeps a name that does not exist distinct from a
// resolver that timed out or refused, which is a different problem with
// different advice.
func isUnresolvedHost(err error) bool {
var dnsErr *net.DNSError
return errors.As(err, &dnsErr) && dnsErr.IsNotFound
}

// streamA2AMessage sends one message to an A2A agent and prints the events it
// streams back.
//
Expand Down Expand Up @@ -285,6 +309,12 @@ func streamA2AMessage(cmd *cobra.Command, opts a2aSendOptions) error {
if hint := a2aUnauthorizedHint(logger.status, opts.withAuthorization); hint != "" {
fmt.Fprintln(cmd.ErrOrStderr(), hint)
}
// An address that does not resolve is reported by the a2a client as a
// send failure, which says nothing about where the address came from.
// Same contract as the 401 hint: stderr only, error returned unchanged.
if opts.unresolvedAddressHint != "" && isUnresolvedHost(err) {
fmt.Fprintln(cmd.ErrOrStderr(), opts.unresolvedAddressHint)
}
return err
}
eventCount++
Expand Down
52 changes: 52 additions & 0 deletions cmd/a2a_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,69 @@ package cmd
import (
"bytes"
"context"
"errors"
"fmt"
"iter"
"math"
"net"
"net/http"
"net/http/httptest"
"net/url"
"strings"
"testing"

"github.com/a2aproject/a2a-go/v2/a2a"
"github.com/a2aproject/a2a-go/v2/a2asrv"
)

// TestIsUnresolvedHost is the whole taxonomy in one place: which failures count
// as "that hostname does not exist" and which do not.
//
// The wrapped case is the load-bearing one. The a2a client returns the DNS error
// buried two layers down (fmt.wrapError around *url.Error), so a check that only
// looked at the top-level error would find nothing and the hint would never
// appear in the one situation it exists for.
func TestIsUnresolvedHost(t *testing.T) {
notFound := &net.DNSError{Err: "no such host", Name: "orders.team1.invalid", IsNotFound: true}

for _, tc := range []struct {
name string
err error
want bool
}{
{"nil", nil, false},
{"bare not-found", notFound, true},
{
// Exactly the shape the a2a client produces; see the doc comment.
"wrapped as the a2a client wraps it",
fmt.Errorf("failed to send HTTP request: %w",
&url.Error{Op: "Post", URL: "http://orders.team1.invalid:8080", Err: notFound}),
true,
},
{
// A resolver that timed out is not a name that does not exist: the
// address may be perfectly good, so telling the user to go find a
// different one sends them after the wrong problem.
"DNS timeout",
&net.DNSError{Err: "i/o timeout", Name: "orders.team1.invalid", IsTimeout: true},
false,
},
{
"DNS temporary failure",
&net.DNSError{Err: "server misbehaving", Name: "orders.team1.invalid", IsTemporary: true},
false,
},
{"connection refused", &net.OpError{Op: "dial", Err: errors.New("connection refused")}, false},
{"unrelated error", errors.New("no such host"), false}, // the text alone must not match
} {
t.Run(tc.name, func(t *testing.T) {
if got := isUnresolvedHost(tc.err); got != tc.want {
t.Errorf("isUnresolvedHost(%v) = %v, want %v", tc.err, got, tc.want)
}
})
}
}

// echoExecutor is a minimal A2A agent that streams back a status update and a
// message echoing what it was sent. It gives the send tests a real server to
// talk to, so the streaming loop and event printing are exercised over an
Expand Down
47 changes: 43 additions & 4 deletions cmd/agents_chat.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@ Because the card is served by the agent itself and proxied by the backend, the
default path only works while the agent is running — an agent that is not ready
has no card, and so no URL to derive.

Note the message goes straight to the agent, not through the platform API, so the
card's hostname has to resolve from where this runs. A cluster-internal name
often does not, which fails with "no such host" even though the API server is
perfectly reachable; --address http://<route>:<port> is the way past it.

--transport, --message and --with-authorization mean what they do for
` + "`a2a send`" + `: the message text is sent as a single user text part, the
response is streamed event by event as it arrives, and --with-authorization
Expand All @@ -57,15 +62,49 @@ With --verbose both the card lookup and the message are reported on stderr.`,
}
}

// The namespace is resolved again here rather than threaded out of
// agentCardURL because --address skips that call entirely, and the hint is
// most useful in exactly that case's neighbour: an address, from wherever,
// that does not resolve. A namespace that cannot be resolved yields no
// example rather than failing the send.
namespace, _ := agentsNamespace()

return streamA2AMessage(cmd, a2aSendOptions{
address: address,
transport: agentsChatArgs.transport,
message: agentsChatArgs.message,
withAuthorization: agentsChatArgs.withAuthorization,
address: address,
transport: agentsChatArgs.transport,
message: agentsChatArgs.message,
withAuthorization: agentsChatArgs.withAuthorization,
unresolvedAddressHint: unresolvedAgentAddressHint(namespace, name),
})
},
}

// unresolvedAgentAddressHint returns the advice to offer when `agents chat`
// cannot resolve the address it is talking to.
//
// The failure is confusing because of what chat does not do: it does not relay
// the message through the platform API, it speaks A2A straight to the agent, at
// the URL the agent's own card advertises. So a working `agents get` and a
// reachable backend are no guarantee the address in the card resolves from here
// — a cluster-internal hostname routinely does not.
//
// The example is built from the agent's own namespace and name so it can be
// edited into a working flag rather than translated first. localtest.me is the
// local trust domain (see defaultTrustDomain) and resolves to 127.0.0.1, which
// is what a local gateway on :8080 fronts; a real cluster's route differs, hence
// "for example".
func unresolvedAgentAddressHint(namespace, name string) string {
hint := "Hint: `agents chat` talks A2A directly to the agent, at the URL from its " +
"agent-card endpoint — that hostname has to resolve from here, which a " +
"cluster-internal one may not. Pass --address http://<route>:<port> to name a " +
"reachable URL."
if namespace == "" || name == "" {
return hint
}
return fmt.Sprintf("%s\nFor example: --address http://%s.%s.localtest.me:8080",
hint, name, namespace)
}

// agentCardURL returns the URL an agent advertises in its agent card — the same
// value `agents card <name>` prints as its URL row.
//
Expand Down
144 changes: 144 additions & 0 deletions cmd/agents_chat_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,150 @@ func TestAgentsChat401HintWithFlag(t *testing.T) {
}
}

// unresolvableAddress is a URL whose host cannot resolve anywhere. RFC 2606
// reserves .invalid for exactly this, so the DNS failure is guaranteed rather
// than dependent on the network the test runs on — a made-up name under a real
// TLD can be answered by a wildcard or a captive resolver.
const unresolvableAddress = "http://orders.team1.rossoctl-nonexistent.invalid:8080/"

// TestAgentsChatUnresolvedCardURLHint is the central case for this hint: the card
// advertises a hostname that does not resolve from here, which is what happens
// when the URL is cluster-internal. The advice has to explain that chat bypasses
// the platform API — the backend was reachable enough to serve the card, so the
// bare DNS error reads as a puzzle.
func TestAgentsChatUnresolvedCardURLHint(t *testing.T) {
isolateHome(t)
backend, _ := newChatBackend(t, unresolvableAddress)
setupAgentGetContext(t, backend)

_, stderr, err := executeSplit(t, "agents", "chat", "orders", "--message", "hi")
if err == nil {
t.Fatal("an address that does not resolve should fail the command")
}
// The three things the user needs: that this is a direct A2A call, the flag
// that fixes it, and an example naming this very agent.
for _, want := range []string{
"A2A directly",
"agent-card",
"--address http://<route>:<port>",
"--address http://orders.team1.localtest.me:8080",
} {
if !strings.Contains(stderr, want) {
t.Errorf("stderr should mention %q:\n%s", want, stderr)
}
}
}

// TestAgentsChatUnresolvedHintUsesRealNamespace verifies the example is built
// from the namespace actually in effect, not a hardcoded one. An example naming
// the wrong namespace would be copied and would fail.
func TestAgentsChatUnresolvedHintUsesRealNamespace(t *testing.T) {
isolateHome(t)
backend, _ := newChatBackend(t, unresolvableAddress)
setupAgentGetContext(t, backend)

_, stderr, err := executeSplit(t, "agents", "--namespace", "team2", "chat", "orders",
"--message", "hi")
if err == nil {
t.Fatal("an address that does not resolve should fail the command")
}
if !strings.Contains(stderr, "--address http://orders.team2.localtest.me:8080") {
t.Errorf("the example should name the team2 namespace:\n%s", stderr)
}
}

// TestAgentsChatUnresolvedAddressFlagHint verifies the hint also covers an
// address the user supplied. --address is the remedy the hint suggests, so a
// second unresolvable value passed to it is precisely when someone needs to be
// told the address, not the agent, is the problem.
func TestAgentsChatUnresolvedAddressFlagHint(t *testing.T) {
isolateHome(t)
backend, _ := newChatBackend(t, "")
setupAgentGetContext(t, backend)

_, stderr, err := executeSplit(t, "agents", "chat", "orders",
"--address", unresolvableAddress, "--message", "hi")
if err == nil {
t.Fatal("an address that does not resolve should fail the command")
}
if !strings.Contains(stderr, "has to resolve from here") {
t.Errorf("stderr should explain the address must resolve:\n%s", stderr)
}
}

// TestAgentsChatNoUnresolvedHintOnReachableFailure verifies the hint is confined
// to a name that does not resolve. An agent that is reachable and answers an
// error has no address problem, and telling its user to go find a route would
// send them after the wrong thing.
func TestAgentsChatNoUnresolvedHintOnReachableFailure(t *testing.T) {
isolateHome(t)
agent := newRejectingAgent(t, http.StatusInternalServerError)
backend, _ := newChatBackend(t, agent.URL)
setupAgentGetContext(t, backend)

_, stderr, err := executeSplit(t, "agents", "chat", "orders", "--message", "hi")
if err == nil {
t.Fatal("a 500 from the agent should fail the command")
}
if strings.Contains(stderr, "--address") {
t.Errorf("a reachable agent's error should produce no address hint:\n%s", stderr)
}
}

// TestAgentsChatNoUnresolvedHintOnRefusedConnection scopes the hint to DNS
// specifically. A host that resolves but refuses the connection is a different
// problem — the address is right and something is down — so the DNS advice must
// not fire. This is what matching *net.DNSError buys over matching the error
// text of a failed send.
func TestAgentsChatNoUnresolvedHintOnRefusedConnection(t *testing.T) {
isolateHome(t)
// A closed port on loopback: resolves fine, refuses immediately.
backend, _ := newChatBackend(t, "http://127.0.0.1:1/")
setupAgentGetContext(t, backend)

_, stderr, err := executeSplit(t, "agents", "chat", "orders", "--message", "hi")
if err == nil {
t.Fatal("a refused connection should fail the command")
}
if strings.Contains(stderr, "Hint:") {
t.Errorf("a refused connection is not a DNS failure; no hint expected:\n%s", stderr)
}
}

// TestA2ASendNoUnresolvedHint verifies `a2a send` stays silent on the same
// failure. Its address came from the user's own --address, so there is nothing to
// reveal and no name to build an example from; the hint belongs to the command
// that resolved the address on the user's behalf.
func TestA2ASendNoUnresolvedHint(t *testing.T) {
isolateHome(t)

_, stderr, err := executeSplit(t, "a2a", "send",
"--address", unresolvableAddress, "--message", "hi")
if err == nil {
t.Fatal("an address that does not resolve should fail the command")
}
if strings.Contains(stderr, "Hint:") {
t.Errorf("a2a send should add no hint to its own --address:\n%s", stderr)
}
}

// TestUnresolvedAgentAddressHintWithoutNamespace verifies an unresolvable
// namespace degrades to advice without an example, rather than to one naming an
// empty namespace. `--address http://orders..localtest.me:8080` would be a
// malformed URL offered as the fix.
func TestUnresolvedAgentAddressHintWithoutNamespace(t *testing.T) {
hint := unresolvedAgentAddressHint("", "orders")
if hint == "" {
t.Fatal("the advice should survive a missing namespace")
}
if strings.Contains(hint, "For example") {
t.Errorf("no example should be offered without a namespace:\n%s", hint)
}
if !strings.Contains(hint, "--address http://<route>:<port>") {
t.Errorf("the generic form should still be suggested:\n%s", hint)
}
}

// TestAgentsChatNoHintOnOtherFailures verifies the hint is confined to 401. A 403
// and a 500 are not credential problems, and advice that does not apply is worse
// than none.
Expand Down