From e277c49abc427489460c7e13c46df4c7033bd9bb Mon Sep 17 00:00:00 2001 From: Ed Snible Date: Thu, 13 Aug 2026 16:24:12 -0400 Subject: [PATCH] Explain an unresolvable agent address in `agents chat` `agents chat` speaks A2A straight to the agent, at the URL from its agent card, so a cluster-internal hostname fails with "no such host" even though the API server was reachable enough to serve the card. Report that on stderr and suggest --address, with an example built from the agent's own namespace and name. Matched on *net.DNSError with IsNotFound rather than the error text: the a2a client wraps the failure twice over, but the chain survives errors.As, and the type keeps a nonexistent name distinct from a resolver timeout or a refused connection. The hint is supplied by `agents chat` rather than built in the shared send path, so `a2a send` stays silent about an address its user typed. Assisted by Claude. Signed-off-by: Ed Snible --- cmd/a2a.go | 30 +++++++++ cmd/a2a_test.go | 52 +++++++++++++++ cmd/agents_chat.go | 47 +++++++++++-- cmd/agents_chat_test.go | 144 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 269 insertions(+), 4 deletions(-) diff --git a/cmd/a2a.go b/cmd/a2a.go index d4f0c2a..9eb4a7f 100644 --- a/cmd/a2a.go +++ b/cmd/a2a.go @@ -3,8 +3,10 @@ package cmd import ( "context" "encoding/json" + "errors" "fmt" "io" + "net" "net/http" "net/url" "strings" @@ -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, @@ -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. // @@ -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++ diff --git a/cmd/a2a_test.go b/cmd/a2a_test.go index d576910..b528d39 100644 --- a/cmd/a2a_test.go +++ b/cmd/a2a_test.go @@ -3,10 +3,14 @@ package cmd import ( "bytes" "context" + "errors" + "fmt" "iter" "math" + "net" "net/http" "net/http/httptest" + "net/url" "strings" "testing" @@ -14,6 +18,54 @@ import ( "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 diff --git a/cmd/agents_chat.go b/cmd/agents_chat.go index ad4aec8..5d8ff77 100644 --- a/cmd/agents_chat.go +++ b/cmd/agents_chat.go @@ -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://: 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 @@ -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://: 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 ` prints as its URL row. // diff --git a/cmd/agents_chat_test.go b/cmd/agents_chat_test.go index 357d8dc..6beb5b7 100644 --- a/cmd/agents_chat_test.go +++ b/cmd/agents_chat_test.go @@ -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://:", + "--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://:") { + 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.