Suggest cortex serve when a context named "cortex" refuses the connection

cmd/root.go

@@ -12,6 +12,7 @@ import (
 	"fmt"
 	"net/http"
 	"os"
+	"syscall"
 
 	"github.com/spf13/cobra"
 
@@ -30,6 +31,15 @@ var (
 // defaultServer is the API endpoint used when --server is not supplied.
 const defaultServer = "http://rossoctl-ui.localtest.me:8080/api/v1/"
 
+// cortexContextName is the context name that marks a context as pointing at a
+// local `rossoctl cortex serve`, which is the one server rossoctl can start
+// itself. It is the trigger for the hint in connectionRefusedHint.
+//
+// A name is the marker because nothing else distinguishes such a context: a
+// cortex is reached over HTTP like any other server, and the config's type field
+// is "api" for every context every command creates.
+const cortexContextName = "cortex"
+
 // Persistent flags shared by every command.
 var (
 	verbose bool
@@ -210,21 +220,64 @@ func Execute() {
 // each call site: a 401 can come from any command that reaches the API, and
 // eleven files call one of these clients. Adding it here covers agents, tools,
 // status, envvars, and anything added later for free.
+// Two shapes of failure carry a remedy: a response the server sent (a
+// StatusError) and never reaching the server at all (a refused connection).
 func errorHint(err error) string {
 	var statusErr *apiclient.StatusError
-	if !errors.As(err, &statusErr) {
+	if errors.As(err, &statusErr) {
+		switch statusErr.StatusCode {
+		case http.StatusUnauthorized:
+			// Deliberately not suggested for 403: that is an authenticated
+			// identity lacking permission, where signing in again changes
+			// nothing and the advice would send the user in a circle.
+			return "Hint: the server rejected the credentials. Run `rossoctl login` to sign in."
+		default:
+			return ""
+		}
+	}
+
+	if hint := connectionRefusedHint(err); hint != "" {
+		return hint
+	}
+	return ""
+}
+
+// connectionRefusedHint suggests starting the local API when the context that
+// could not be reached is the one that names it.
+//
+// The test is the syscall, via errors.Is, not the words "connection refused" in
+// the message. apiclient wraps the dial failure with %w, so the errno survives
+// in the chain, and matching on it distinguishes a refused connection from a
+// timeout or an unresolvable host — neither of which a local server would fix.
+// It also leaves an unrelated error that merely says "connection refused" alone.
+//
+// The suggestion is offered only for a context named "cortex". `cortex serve` is
+// the one server rossoctl can start itself, so it is a real remedy there and
+// misdirection anywhere else: a production API that is down is not fixed by
+// running a local server, and saying so would send the user after the wrong
+// problem. Naming the context is a convention rather than a guarantee, which is
+// why this is a hint appended to the real error rather than a replacement for it.
+func connectionRefusedHint(err error) string {
+	if !errors.Is(err, syscall.ECONNREFUSED) {
 		return ""
 	}
 
-	switch statusErr.StatusCode {
-	case http.StatusUnauthorized:
-		// Deliberately not suggested for 403: that is an authenticated
-		// identity lacking permission, where signing in again changes
-		// nothing and the advice would send the user in a circle.
-		return "Hint: the server rejected the credentials. Run `rossoctl login` to sign in."
-	default:
+	// An explicit --server overrides every context (see resolveServer), so the
+	// current context's name says nothing about what was actually dialed.
+	if server != "" {
 		return ""
 	}
+
+	// Read the context defensively. This runs while an error is already being
+	// reported, so a config that cannot be loaded must leave the hint silent
+	// rather than replace the failure the user is trying to read.
+	ctx, ctxErr := resolveContext()
+	if ctxErr != nil || ctx == nil || ctx.Name != cortexContextName {
+		return ""
+	}
+
+	return "Hint: nothing is listening at " + ctx.Server +
+		". Run `rossoctl cortex serve` to start the local API."
 }
 
 func init() {

cmd/root_test.go

@@ -8,13 +8,15 @@ import (
 	"os"
 	"path/filepath"
 	"strings"
+	"syscall"
 	"testing"
 	"time"
 
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
 
 	"github.com/rossoctl/rossoctl-cli/internal/apiclient"
+	"github.com/rossoctl/rossoctl-cli/internal/config"
 )
 
 // TestMain isolates HOME to a throwaway directory for the whole cmd test
@@ -303,3 +305,139 @@ func TestErrorHintQuietForOtherErrors(t *testing.T) {
 		})
 	}
 }
+
+// refusedError produces the error chain a real dial failure yields, by asking
+// net/http for a port nothing is listening on and wrapping it the way
+// apiclient.doJSON does.
+//
+// It is built from a real connection rather than a hand-made syscall.Errno so the
+// test proves errorHint matches the chain the client actually produces. A
+// constructed errno would pass even if net/http stopped surfacing the syscall.
+func refusedError(t *testing.T) error {
+	t.Helper()
+
+	// Port 1 on loopback: privileged, so nothing is listening, and the connection
+	// is refused rather than timing out as a filtered address would.
+	_, err := (&http.Client{Timeout: 5 * time.Second}).Get("http://127.0.0.1:1/api/v1/agents")
+	if err == nil {
+		t.Fatal("expected a dial failure against 127.0.0.1:1")
+	}
+	if !errors.Is(err, syscall.ECONNREFUSED) {
+		t.Fatalf("test precondition: %v is not ECONNREFUSED", err)
+	}
+	return fmt.Errorf("requesting %s: %w", "http://127.0.0.1:1/api/v1/agents", err)
+}
+
+// seedContext writes a single named context and makes it current, bypassing
+// create-context so the name is exactly as given.
+func seedContext(t *testing.T, name, server string) {
+	t.Helper()
+	path := isolateHome(t)
+	cfg, err := config.Load(path)
+	if err != nil {
+		t.Fatalf("load config: %v", err)
+	}
+	cfg.Upsert(config.Context{Name: name, Type: config.TypeAPI, Server: server})
+	if err := cfg.SetCurrent(name); err != nil {
+		t.Fatalf("set current: %v", err)
+	}
+	if err := cfg.Save(); err != nil {
+		t.Fatalf("save config: %v", err)
+	}
+}
+
+// TestErrorHintSuggestsCortexServeOnRefusedConnection is the case this hint
+// exists for: a context pointed at a local `cortex serve` that was never started.
+func TestErrorHintSuggestsCortexServeOnRefusedConnection(t *testing.T) {
+	resetFlags(rootCmd)
+	seedContext(t, "cortex", "http://localhost:9097/api/v1/")
+
+	hint := errorHint(refusedError(t))
+	if hint == "" {
+		t.Fatal("a refused connection on the cortex context should produce a hint")
+	}
+	if !strings.Contains(hint, "rossoctl cortex serve") {
+		t.Errorf("hint %q should name `rossoctl cortex serve`", hint)
+	}
+	// The server is named so the user can see which address was unreachable.
+	if !strings.Contains(hint, "http://localhost:9097/api/v1/") {
+		t.Errorf("hint %q should name the context's server", hint)
+	}
+}
+
+// TestErrorHintQuietForRefusedConnectionOnOtherContexts keeps the suggestion from
+// becoming misdirection. A production API that is down is not fixed by starting a
+// local server, so only the context that names one gets the advice.
+func TestErrorHintQuietForRefusedConnectionOnOtherContexts(t *testing.T) {
+	resetFlags(rootCmd)
+	seedContext(t, "prod", "http://rossoctl.example.com/api/v1/")
+
+	if hint := errorHint(refusedError(t)); hint != "" {
+		t.Errorf("errorHint on a non-cortex context = %q, want no hint", hint)
+	}
+}
+
+// TestErrorHintQuietForRefusedConnectionWithExplicitServer covers --server, which
+// overrides every context: the current context's name then says nothing about
+// what was actually dialed.
+func TestErrorHintQuietForRefusedConnectionWithExplicitServer(t *testing.T) {
+	resetFlags(rootCmd)
+	seedContext(t, "cortex", "http://localhost:9097/api/v1/")
+
+	server = "http://elsewhere.example.com/api/v1/"
+	t.Cleanup(func() { server = "" })
+
+	if hint := errorHint(refusedError(t)); hint != "" {
+		t.Errorf("errorHint with an explicit --server = %q, want no hint", hint)
+	}
+}
+
+// TestErrorHintDistinguishesRefusedFromOtherTransportFailures pins the choice of
+// errors.Is over message matching. An unresolvable host is not a refused
+// connection, and starting a local server would not fix it.
+func TestErrorHintDistinguishesRefusedFromOtherTransportFailures(t *testing.T) {
+	resetFlags(rootCmd)
+	seedContext(t, "cortex", "http://localhost:9097/api/v1/")
+
+	// .invalid is reserved by RFC 2606 and never resolves.
+	_, dnsErr := (&http.Client{Timeout: 5 * time.Second}).Get("http://nonexistent.invalid./x")
+	if dnsErr == nil {
+		t.Skip("this network resolves nonexistent.invalid.; cannot test a DNS failure")
+	}
+	wrapped := fmt.Errorf("requesting %s: %w", "http://nonexistent.invalid./x", dnsErr)
+
+	if hint := errorHint(wrapped); hint != "" {
+		t.Errorf("errorHint on a DNS failure = %q, want no hint", hint)
+	}
+}
+
+// TestErrorHintRefusedSurvivesWrapping verifies the hint survives the extra
+// context commands add on the way up, as the 401 hint does.
+func TestErrorHintRefusedSurvivesWrapping(t *testing.T) {
+	resetFlags(rootCmd)
+	seedContext(t, "cortex", "http://localhost:9097/api/v1/")
+
+	wrapped := fmt.Errorf("listing agents in namespace %q: %w", "team1", refusedError(t))
+	if hint := errorHint(wrapped); hint == "" {
+		t.Error("a wrapped refused connection should still produce a hint")
+	}
+}
+
+// TestErrorHintQuietWhenConfigIsUnreadable covers the defensive context lookup:
+// errorHint runs while an error is already being reported, so a broken config
+// must leave the hint silent rather than obscure the real failure.
+func TestErrorHintQuietWhenConfigIsUnreadable(t *testing.T) {
+	resetFlags(rootCmd)
+	path := isolateHome(t)
+	if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil {
+		t.Fatalf("mkdir: %v", err)
+	}
+	if err := os.WriteFile(path, []byte("{{ not yaml"), 0o600); err != nil {
+		t.Fatalf("write bad config: %v", err)
+	}
+
+	// Must not panic, and must not produce a hint.
+	if hint := errorHint(refusedError(t)); hint != "" {
+		t.Errorf("errorHint with an unreadable config = %q, want no hint", hint)
+	}
+}

cmd/ui_test.go

@@ -51,7 +51,7 @@ func TestUIOpenBlankServerFails(t *testing.T) {
 	if err != nil {
 		t.Fatalf("load config: %v", err)
 	}
-	cfg.Upsert(config.Context{Name: "empty", Type: config.TypeCortex})
+	cfg.Upsert(config.Context{Name: "empty", Type: config.TypeAPI})
 	if err := cfg.SetCurrent("empty"); err != nil {
 		t.Fatalf("set current: %v", err)
 	}

internal/config/config.go

@@ -23,21 +23,24 @@ const (
 	filePerm os.FileMode = 0o600
 )
 
-// Type records what kind of server a Context targets: a hosted Rossoctl API
-// ("api") or a local Cortex ("cortex").
+// Type records what kind of server a Context targets. Every context is reached
+// the same way, over HTTP at its server URI, so the type does not select a
+// client implementation — it is a label describing what is at the other end.
 //
-// Both are reached the same way, over HTTP at the context's server URI, so the
-// type does not select a client implementation — it is a label describing what
-// is at the other end.
+// There was once a second value, "cortex", for a context served by `rossoctl
+// cortex serve`. Nothing ever set it: every context-creating path assigned
+// "api", so no condition keyed on it could fire. A local cortex is an ordinary
+// HTTP server reached by pointing a context at its address, which "api" already
+// describes.
+//
+// Type is not validated on load, so a config file written when "cortex" existed
+// still parses; the value is simply carried as-is.
 type Type string
 
 const (
-	// TypeAPI is a context served by the Rossoctl HTTP API (backed by a
-	// Kubernetes cluster).
+	// TypeAPI is a context served by the Rossoctl HTTP API — whether that is a
+	// backend fronting a Kubernetes cluster or a local `rossoctl cortex serve`.
 	TypeAPI Type = "api"
-	// TypeCortex is a context served by a local Cortex, as started by
-	// `rossoctl cortex serve`.
-	TypeCortex Type = "cortex"
 )
 
 // Context is a single named target: a type, a server URI, an optional default

internal/rossoctlclient/rossoctlclient.go

@@ -74,7 +74,7 @@ var _ Rossoctl = (*apiclient.Client)(nil)
 // NewClient builds a Rossoctl backend for ctx: an HTTP apiclient.Client for the
 // context's server and bearer token.
 //
-// Every context type reaches the API over HTTP, including TypeCortex. A cortex
+// Every context reaches the API over HTTP, a local cortex included. A cortex
 // context once had its own file-backed client reading agents.json directly; that
 // backend is gone, and a cortex is now reached the same way as any other server —
 // by pointing the context at a `rossoctl cortex serve` address.

internal/rossoctlclient/rossoctlclient_test.go

@@ -7,14 +7,16 @@ import (
 	"github.com/rossoctl/rossoctl-cli/internal/config"
 )
 
-// TestNewClientIsAlwaysHTTP verifies every context type yields the HTTP client.
-// A cortex context is no exception: it is reached by pointing it at a
-// `rossoctl cortex serve` address, not by a separate file-backed client.
+// TestNewClientIsAlwaysHTTP verifies every context yields the HTTP client,
+// including one whose type is unset or holds a value this build no longer
+// defines — such as the "cortex" type a config file may still carry from an
+// older release. A cortex is reached by pointing a context at a `rossoctl cortex
+// serve` address, not by a separate file-backed client.
 func TestNewClientIsAlwaysHTTP(t *testing.T) {
 	for _, ctxType := range []config.Type{
 		config.TypeAPI,
-		config.TypeCortex,
-		"", // unset
+		"cortex", // retired from this build; still valid on disk
+		"",       // unset
 	} {
 		t.Run(string(ctxType), func(t *testing.T) {
 			c := NewClient(&config.Context{Type: ctxType, Server: "http://x/api/v1/"})
@@ -38,9 +40,9 @@ func TestNewClientCarriesContextFields(t *testing.T) {
 		t.Errorf("BearerToken = %q, want %q", c.BearerToken, ctx.BearerToken)
 	}
 
-	// A cortex context's server is honored the same way, so a context pointed at
-	// a local `cortex serve` reaches it rather than being routed elsewhere.
-	cortex := &config.Context{Type: config.TypeCortex, Name: "mycortex", Server: "http://localhost:9097/api/v1/"}
+	// A localhost server is honored the same way, so a context pointed at a local
+	// `cortex serve` reaches it rather than being routed elsewhere.
+	cortex := &config.Context{Type: config.TypeAPI, Name: "cortex", Server: "http://localhost:9097/api/v1/"}
 	cc, ok := NewClient(cortex).(*apiclient.Client)
 	if !ok {
 		t.Fatalf("expected *apiclient.Client for a cortex context, got %T", cc)