fix(discovery): 错误分类 + 去掉重复请求 URL

演练错误路径时发现两个真问题。

**① 密钥错误被报成 502（上游故障）。** 这是最常见的失败，却和「厂商挂了」
给出完全相同的错误。两者需要的动作正好相反：重新输 key vs 去查厂商状态页。
操作员只能猜。现在 401/403 会得到 401 并在信息里点名 api key；
DNS 不解析、TLS 失败、连接被拒也各自带一句可执行的说明。

**② 错误信息里的 tried 列表有重复。** base "https://h/v1" + path "/models"
和 base "https://h" + path "/v1/models" 请求的是同一个地址，但候选去重只按
(base, path) 对，于是同一条 URL 在错误信息里出现两次——读起来像「试了两个
地方」，其实只试了一个。改成按最终 URL 去重。

去重后候选从 3 个变成 2 个，两个判据的期望值需要跟着改：
TestDiscoveryCandidates_DedupesAndPrefersTyped 和变异测试的
「fallback search」原本断言 >= 3，那是在测量 bug 而不是覆盖度。覆盖没有丢失：
剩下的两条是 https://h/v1/models 和 https://h/models，两个不同地址。

新增判据：
  TestDiscoverModels_WrongKeyIsUnauthorizedNotBadGateway
  TestDiscoverModels_UnreachableStaysBadGateway（防止过度纠正）
  TestDiscoveryCandidates_NoDuplicateRequests
  TestDiscoverModels_FailureListsEachURLOnce（对操作员真正读到的文本断言）

internal/gateway/discover.go

@@ -3,6 +3,7 @@ package gateway
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
 	"net/http"
@@ -127,6 +128,26 @@ type discoveryCandidate struct {
 	prefer int
 }
 
+// discoveryFailure carries the HTTP status the caller should answer with.
+//
+// The status is part of the error rather than left to the handler because the
+// whole point of classifying a failure is that the operator must not have to
+// guess whether the key, the endpoint, or the vendor is at fault.
+type discoveryFailure struct {
+	msg    string
+	status int
+}
+
+func (e *discoveryFailure) Error() string { return e.msg }
+
+// StatusOrDefault reports the intended HTTP status.
+func (e *discoveryFailure) StatusOrDefault() int {
+	if e.status == 0 {
+		return http.StatusBadGateway
+	}
+	return e.status
+}
+
 // discoveryCandidates enumerates the URL forms worth trying for a raw input.
 //
 // The ranking encodes a real observation rather than a guess: the amr-link
@@ -146,13 +167,20 @@ func discoveryCandidates(raw string) []discoveryCandidate {
 	if raw == "" {
 		return nil
 	}
-	seen := map[string]bool{}
+	// Deduplicate on the URL that will actually be requested, not on the
+	// (base, path) pair. base "https://h/v1" + path "/models" and base "https://h"
+	// + path "/v1/models" both request the same address, and listing it twice in
+	// an error message makes the operator think two things were tried.
+	seenURL := map[string]bool{}
+	seenPair := map[string]bool{}
 	var out []discoveryCandidate
 	add := func(base, path string, prefer int) {
-		if seen[base+"|"+path] {
+		pair := base + "|" + path
+		if seenPair[pair] || seenURL[base+path] {
 			return
 		}
-		seen[base+"|"+path] = true
+		seenPair[pair] = true
+		seenURL[base+path] = true
 		out = append(out, discoveryCandidate{baseURL: base, modelsPath: path, prefer: prefer})
 	}
 
@@ -321,8 +349,10 @@ func DiscoverModels(ctx context.Context, rawURL, apiKey string) (*discoveryResul
 	client := &http.Client{Timeout: discoveryTimeout}
 	var tried []string
 	var firstErr string
+	var authErr string
 	typedBase := strings.TrimRight(strings.TrimSpace(rawURL), "/")
 
+	var lastErr string
 	for _, c := range cands {
 		full := c.baseURL + c.modelsPath
 		tried = append(tried, full)
@@ -340,7 +370,10 @@ func DiscoverModels(ctx context.Context, rawURL, apiKey string) (*discoveryResul
 		start := time.Now()
 		resp, err := client.Do(req)
 		if err != nil {
-			firstErr = err.Error()
+			lastErr = classifyTransport(err)
+			if lastErr == "" {
+				lastErr = err.Error()
+			}
 			continue
 		}
 		body, readErr := io.ReadAll(io.LimitReader(resp.Body, discoveryMaxBody))
@@ -351,6 +384,13 @@ func DiscoverModels(ctx context.Context, rawURL, apiKey string) (*discoveryResul
 			continue
 		}
 		if resp.StatusCode != http.StatusOK {
+			// An auth rejection is the operator's key being wrong, not the
+			// upstream being down. Keep it: it is the single most common
+			// failure and 502 would send them to the wrong place.
+			if af := classifyAuthStatus(resp.StatusCode, body); af != "" {
+				authErr = af
+				continue
+			}
 			firstErr = fmt.Sprintf("%s returned %d", full, resp.StatusCode)
 			continue
 		}
@@ -406,7 +446,74 @@ func DiscoverModels(ctx context.Context, rawURL, apiKey string) (*discoveryResul
 		}
 		return res, nil
 	}
-	return nil, fmt.Errorf("no model catalog found; tried %s; last error: %s", strings.Join(tried, ", "), firstErr)
+	return nil, buildDiscoveryError(tried, authErr, firstErr, lastErr)
+}
+
+// classifyAuthStatus turns a 401/403 from the catalog endpoint into a message
+// that names the credential as the cause.
+//
+// This distinction is the whole point: a wrong key and an unreachable vendor
+// are the two most common discovery failures, and they need opposite actions
+// (re-type the key vs. go check the vendor). Reporting both as "upstream
+// error" makes the operator guess.
+func classifyAuthStatus(status int, body []byte) string {
+	if status != http.StatusUnauthorized && status != http.StatusForbidden {
+		return ""
+	}
+	snippet := strings.TrimSpace(string(body))
+	// Vendors put the reason in wildly different shapes; a compact quote is
+	// more useful than a parse attempt that fails on most of them.
+	snippet = strings.Join(strings.Fields(snippet), " ")
+	if len(snippet) > 200 {
+		snippet = snippet[:200] + "…"
+	}
+	return fmt.Sprintf("upstream rejected the api key (HTTP %d): %s — "+
+		"check the key, not the endpoint", status, snippet)
+}
+
+// classifyTransport turns low-level network failures into something an
+// operator can act on. An empty result means "no specific advice", and the
+// caller keeps the raw error.
+func classifyTransport(err error) string {
+	if err == nil {
+		return ""
+	}
+	msg := err.Error()
+	lower := strings.ToLower(msg)
+	switch {
+	case strings.Contains(lower, "no such host"):
+		return "hostname does not resolve: " + msg
+	case strings.Contains(lower, "certificate"), strings.Contains(lower, "tls"):
+		return "TLS verification failed: " + msg
+	case strings.Contains(lower, "connection refused"):
+		return "connection refused: " + msg
+	}
+	return ""
+}
+
+// buildDiscoveryError assembles the final error, preferring the most specific
+// diagnosis available. Every URL tried is listed: an operator staring at a
+// failed probe needs to know the search actually happened, not just the last
+// attempt.
+func buildDiscoveryError(tried []string, authErr, firstErr, lastErr string) error {
+	triedPart := "no model catalog found"
+	if len(tried) > 0 {
+		triedPart = "no model catalog found; tried " + strings.Join(tried, ", ")
+	}
+	if authErr != "" {
+		return &discoveryFailure{status: http.StatusUnauthorized, msg: authErr + " (" + triedPart + ")"}
+	}
+	detail := firstErr
+	if detail == "" {
+		detail = lastErr
+	}
+	if detail == "" {
+		return errors.New(triedPart)
+	}
+	if lastErr != "" && lastErr != firstErr {
+		detail += " (last: " + lastErr + ")"
+	}
+	return &discoveryFailure{status: http.StatusBadGateway, msg: triedPart + "; " + detail}
 }
 
 // upstreamModel is the subset of a /models entry this code reads. Vendors add

internal/gateway/discover_api.go

@@ -3,6 +3,7 @@ package gateway
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"io"
 	"net/http"
 	"strings"
@@ -131,8 +132,15 @@ func (g *Gateway) handleSourceDiscoverAPI(w http.ResponseWriter, r *http.Request
 
 	res, err := DiscoverModels(ctx, req.BaseURL, req.APIKey)
 	if err != nil {
-		// 502: the upstream is what failed. The message lists every URL tried.
-		writeError(w, http.StatusBadGateway, "discover_failed", err.Error())
+		// A rejected key must not be reported as an upstream outage: the two
+		// need opposite actions from the operator, and 502 sends them to the
+		// wrong one.
+		status := http.StatusBadGateway
+		var f *discoveryFailure
+		if errors.As(err, &f) {
+			status = f.StatusOrDefault()
+		}
+		writeError(w, status, "discover_failed", err.Error())
 		return
 	}
 

internal/gateway/discover_api_test.go

@@ -3,6 +3,7 @@ package gateway
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"net/http"
 	"net/http/httptest"
 	"strings"
@@ -168,6 +169,100 @@ func TestDiscoverModels_SendsAuthorizationHeader(t *testing.T) {
 	}
 }
 
+// TestDiscoverModels_WrongKeyIsUnauthorizedNotBadGateway pins the distinction
+// that decides whether the operator knows what to do next.
+//
+// A rejected key and an unreachable vendor are the two most common discovery
+// failures, and they need opposite actions: re-type the key, versus go check the
+// vendor's status page. Both used to surface as one generic error, so the
+// operator had to guess.
+func TestDiscoverModels_WrongKeyIsUnauthorizedNotBadGateway(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		w.WriteHeader(http.StatusUnauthorized)
+		_, _ = w.Write([]byte(`{"error":{"message":"invalid api key","code":"invalid_api_key"}}`))
+	}))
+	defer srv.Close()
+
+	_, err := DiscoverModels(context.Background(), srv.URL+"/v1", "sk-wrong")
+	if err == nil {
+		t.Fatal("expected a failure against a 401 endpoint")
+	}
+	var f *discoveryFailure
+	if !errors.As(err, &f) {
+		t.Fatalf("error should be classified, got %T: %v", err, err)
+	}
+	if got := f.StatusOrDefault(); got != http.StatusUnauthorized {
+		t.Errorf("status = %d, want %d — a wrong key must not read as an upstream outage",
+			got, http.StatusUnauthorized)
+	}
+	// The message has to name which credential is at fault, or the status code
+	// alone still sends the operator to the wrong place.
+	if !strings.Contains(strings.ToLower(f.Error()), "api key") {
+		t.Errorf("message %q should name the api key as the cause", f.Error())
+	}
+	if !strings.Contains(f.Error(), "/v1/models") {
+		t.Errorf("message %q should list what was tried", f.Error())
+	}
+}
+
+// TestDiscoverModels_UnreachableStaysBadGateway: the classification must not
+// over-correct — an unresolvable host is still the vendor's or the network's
+// problem, not the key's.
+func TestDiscoverModels_UnreachableStaysBadGateway(t *testing.T) {
+	_, err := DiscoverModels(context.Background(), "https://no-such-host-xyz.invalid/v1", "k")
+	if err == nil {
+		t.Fatal("expected a failure for an unresolvable host")
+	}
+	var f *discoveryFailure
+	if !errors.As(err, &f) {
+		t.Fatalf("error should be classified, got %T", err)
+	}
+	if got := f.StatusOrDefault(); got != http.StatusBadGateway {
+		t.Errorf("status = %d, want %d", got, http.StatusBadGateway)
+	}
+	if !strings.Contains(strings.ToLower(f.Error()), "resolve") {
+		t.Errorf("message %q should say the hostname does not resolve", f.Error())
+	}
+}
+
+// TestDiscoveryCandidates_NoDuplicateRequests: base "https://h/v1" with path
+// "/models" and base "https://h" with path "/v1/models" request the SAME URL.
+// Listing it twice in a failure message reads as "two things were tried" when
+// only one was.
+func TestDiscoveryCandidates_NoDuplicateRequests(t *testing.T) {
+	seen := map[string]bool{}
+	for _, c := range discoveryCandidates("https://h/v1") {
+		url := c.baseURL + c.modelsPath
+		if seen[url] {
+			t.Errorf("candidate %q would be requested twice", url)
+		}
+		seen[url] = true
+	}
+	if len(seen) < 2 {
+		t.Errorf("only %d distinct URLs; the search collapsed", len(seen))
+	}
+}
+
+// TestDiscoverModels_FailureListsEachURLOnce is the end-to-end form of the same
+// rule, on the error text an operator actually reads.
+func TestDiscoverModels_FailureListsEachURLOnce(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusNotFound)
+	}))
+	defer srv.Close()
+
+	_, err := DiscoverModels(context.Background(), srv.URL+"/v1", "k")
+	if err == nil {
+		t.Fatal("expected failure")
+	}
+	msg := err.Error()
+	needle := srv.URL + "/v1/models"
+	if n := strings.Count(msg, needle); n > 1 {
+		t.Errorf("URL %s appears %d times in %q", needle, n, msg)
+	}
+}
+
 func TestDiscoverModels_EmptyKeyOmitsHeader(t *testing.T) {
 	var present bool
 	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

internal/gateway/discover_mutation_test.go

@@ -11,6 +11,7 @@ package gateway
 
 import (
 	"encoding/json"
+	"net/http"
 	"strings"
 	"testing"
 )
@@ -116,10 +117,47 @@ func TestMutantsAreDetectable(t *testing.T) {
 		}
 	})
 
+	t.Run("a rejected key is reported as unauthorized, not as an outage", func(t *testing.T) {
+		// The mutant: collapse every failure into one generic error, which is
+		// what made a typo'd key look like the vendor being down.
+		if classifyAuthStatus(http.StatusUnauthorized, []byte(`{"error":"invalid api key"}`)) == "" {
+			t.Error("mutant survived: 401 is not classified")
+		}
+		if classifyAuthStatus(http.StatusNotFound, []byte("nope")) != "" {
+			t.Error("a 404 must not be reported as an auth problem")
+		}
+		if classifyAuthStatus(http.StatusOK, nil) != "" {
+			t.Error("a 200 must not be reported as an auth problem")
+		}
+	})
+
+	t.Run("duplicate candidate URLs are collapsed", func(t *testing.T) {
+		seen := map[string]bool{}
+		for _, c := range discoveryCandidates("https://h/v1") {
+			u := c.baseURL + c.modelsPath
+			if seen[u] {
+				t.Errorf("mutant survived: %s requested twice", u)
+			}
+			seen[u] = true
+		}
+	})
+
 	t.Run("discovery falls back across URL forms", func(t *testing.T) {
 		cands := discoveryCandidates("https://h/v1")
-		if len(cands) < 3 {
-			t.Errorf("mutant survived: only %d candidates, fallback search removed", len(cands))
+		// Two distinct URLs is the correct count after deduping: the two
+		// spellings of "https://h/v1/models" collapse into one request. An
+		// earlier expectation of three was measuring the bug, not the coverage.
+		distinct := map[string]bool{}
+		for _, c := range cands {
+			distinct[c.baseURL+c.modelsPath] = true
+		}
+		if len(distinct) != 2 {
+			t.Errorf("mutant survived: %d distinct URLs (%v), want 2", len(distinct), distinct)
+		}
+		for _, want := range []string{"https://h/v1/models", "https://h/models"} {
+			if !distinct[want] {
+				t.Errorf("mutant survived: %s is not searched", want)
+			}
 		}
 	})
 

internal/gateway/discover_test.go

@@ -315,14 +315,24 @@ func TestDiscoveryCandidates_DedupesAndPrefersTyped(t *testing.T) {
 	}
 	seen := map[string]bool{}
 	for _, c := range cands {
-		k := c.baseURL + "|" + c.modelsPath
+		// Dedup is on the requested URL, so that is the key to assert on.
+		k := c.baseURL + c.modelsPath
 		if seen[k] {
-			t.Errorf("duplicate candidate %s", k)
+			t.Errorf("duplicate request for %s", k)
 		}
 		seen[k] = true
 	}
-	if !seen["https://h|/v1/models"] {
-		t.Error("host root with the canonical path was never tried; that is the shape the real amr-link endpoint uses")
+	if !seen["https://h/v1/models"] {
+		t.Error("the typed base with a bare /models path must be tried; it is the first candidate and the one that answers on the real amr-link endpoint")
+	}
+	if !seen["https://h/models"] {
+		t.Error("host root with a bare /models path was never tried; vendors that imply the version would be missed")
+	}
+	// The two candidate PAIRS that both spell "https://h/v1/models" collapse to
+	// one request. That is the point of deduping on the final URL: listing the
+	// same address twice in an error message reads as two attempts.
+	if seen["https://h|v1/models"] && seen["https://h/models"] {
+		t.Error("both spellings of the versioned URL are present; one is redundant")
 	}
 	// The bare "/models" form must exist too — some gateways serve it there.
 	hasBare := false