diff --git a/PERF.md b/PERF.md index d51871a813..9afc0805e9 100644 --- a/PERF.md +++ b/PERF.md @@ -95,6 +95,12 @@ render the summarizer reads caps a tool result at 2,000 bytes and a call's argum ## Connection recovery bounds +Successful connection model lists share discovery's **16 MiB** response bound +(`catalog.MaxCatalogBytes`). The connection probe reads one extra byte to refuse +an oversized listing rather than admit a truncated prefix; error and one-token +probe responses retain their **4 KiB** read bound. Probe timeouts are unchanged. +These are network/memory safety bounds, not response-time performance targets. + `internal/provider/connectivity.go` limits a connection-recovery episode to two minutes (`connectionRecoveryWindow`), including repeated loss after a successful probe. Caller deadlines and cancellation can end it earlier. Each diff --git a/docs/changes/unreleased/1822-complete-provider-catalogs.md b/docs/changes/unreleased/1822-complete-provider-catalogs.md new file mode 100644 index 0000000000..690aefc63d --- /dev/null +++ b/docs/changes/unreleased/1822-complete-provider-catalogs.md @@ -0,0 +1,8 @@ +--- +kind: fixed +title: provider connections read complete bounded model catalogs +pr: 1822 +surface: [chat, engine, docs] +invalidates: + - "Connection checks read only four KiB of a successful model list, so ai&'s valid larger catalog became incomplete JSON and its key could not connect. Successful model-list probes now read the complete response up to discovery's shared sixteen-MiB bound and refuse oversized responses instead of accepting a truncated prefix." +--- diff --git a/internal/catalog/catalog.go b/internal/catalog/catalog.go index a7a1469e02..2efdc00a25 100644 --- a/internal/catalog/catalog.go +++ b/internal/catalog/catalog.go @@ -28,8 +28,10 @@ import ( ) const ( - TTL = 24 * time.Hour - maxCatalogBytes = 16 << 20 + TTL = 24 * time.Hour + // MaxCatalogBytes is the model-list response bound shared by discovery and + // connection checks, so a catalog discovery can read is also connectable. + MaxCatalogBytes = 16 << 20 cacheName = "model-catalog.json" // DefaultBaseURL lives beside the compiled-in fallback rows because those // rows are this service's ids, so the package that serves them has to be @@ -1222,7 +1224,7 @@ func fetch(ctx context.Context, options Options) ([]Model, error) { var payload struct { Data []json.RawMessage `json:"data"` } - decoder := json.NewDecoder(io.LimitReader(response.Body, maxCatalogBytes)) + decoder := json.NewDecoder(io.LimitReader(response.Body, MaxCatalogBytes)) if err := decoder.Decode(&payload); err != nil { return nil, err } diff --git a/internal/config/sources.go b/internal/config/sources.go index ef7a799cf8..9108fd2583 100644 --- a/internal/config/sources.go +++ b/internal/config/sources.go @@ -571,10 +571,22 @@ func runServiceProbe(ctx context.Context, address, key string, probe modelsource return 0, nil, false } defer response.Body.Close() - body, err := io.ReadAll(io.LimitReader(response.Body, 4<<10)) + // A successful model list must be read whole. The old refusal-sized read + // cut ai&'s valid 6,920-byte catalog into invalid JSON before admission. + // One byte beyond discovery's bound distinguishes an oversized response + // from a complete document that happens to end exactly at the limit. + listing := probe.Method == http.MethodGet && probe.Address == "/models" && acceptsStatus(probe.Accepts, response.StatusCode) + limit := int64(4 << 10) + if listing { + limit = catalog.MaxCatalogBytes + 1 + } + body, err := io.ReadAll(io.LimitReader(response.Body, limit)) if err != nil { return 0, nil, false } + if listing && len(body) > catalog.MaxCatalogBytes { + return response.StatusCode, nil, false + } return response.StatusCode, body, true } diff --git a/internal/config/sources_catalog_probe_test.go b/internal/config/sources_catalog_probe_test.go new file mode 100644 index 0000000000..9c7a4e4bf2 --- /dev/null +++ b/internal/config/sources_catalog_probe_test.go @@ -0,0 +1,153 @@ +package config + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "os" + "slices" + "strconv" + "strings" + "sync/atomic" + "testing" + + "github.com/Agent-Field/codeaf/internal/catalog" + "github.com/Agent-Field/codeaf/internal/modelsource" + "github.com/Agent-Field/codeaf/internal/modelsource/sourcestub" +) + +// The real ai& listing on 2026-10-14 was 6,920 bytes with thirteen rows. A +// connection must keep the whole listing, including the final model, rather +// than mistaking a transport-truncated JSON document for an absent service. +func TestAProviderCatalogBeyondFourKiBConnectsWithoutACompletionProbe(t *testing.T) { + t.Setenv("AIAND_API_KEY", "") + ids := []string{"zai-org/glm-5.3-flash", "motif-technologies/motif-3"} + for len(ids) < 13 { + ids = append(ids, "synthetic/model-"+strings.Repeat("x", len(ids))) + } + data := make([]map[string]string, 0, len(ids)) + for _, id := range ids { + data = append(data, map[string]string{"id": id, "description": "a public model catalog row"}) + } + payload := map[string]any{"object": "list", "data": data, "metadata": ""} + body, err := json.Marshal(payload) + if err != nil { + t.Fatal(err) + } + payload["metadata"] = strings.Repeat("x", 6920-len(body)) + body, err = json.Marshal(payload) + if err != nil || len(body) != 6920 || !json.Valid(body) || json.Valid(body[:4096]) { + t.Fatalf("fixture must be complete 6,920-byte JSON whose first four KiB are incomplete: bytes=%d err=%v", len(body), err) + } + server := sourcestub.New() + defer server.Close() + server.Refuse(http.StatusOK, string(body)) + source := vendoredSource(t, "aiand") + source.Address = server.URL() + row := PersistedSource{ID: source.ID, Written: source.Written, Key: "sk-synthetic-catalog-probe-key"} + dir := t.TempDir() + outcome, err := ConnectService(context.Background(), dir, row, source, nil) + if err != nil || outcome.Kind != modelsource.OutcomeConnected || !outcome.Listed || outcome.Models != 13 || !slices.Equal(outcome.ModelIDs, ids) { + t.Fatalf("complete catalog did not connect with every model: outcome=%+v err=%v", outcome, err) + } + requests := server.Requests() + if len(requests) != 1 || requests[0].Method != http.MethodGet { + t.Fatalf("a valid listing must not buy a completion probe: %+v", requests) + } + rows := PersistedSources(dir) + if len(rows) != 1 || rows[0].ID != source.ID || rows[0].Listed == nil || !*rows[0].Listed { + t.Fatalf("successful listing was not saved: %+v", rows) + } +} + +// A valid document followed by whitespace still exceeds the response bound. +// Accepting its complete prefix would make the limit disappear for a server +// that kept sending bytes after its model list had already parsed. +func TestConnectionCatalogResponseBoundIncludesTheWholeBody(t *testing.T) { + t.Setenv("AIAND_API_KEY", "") + for _, size := range []int{catalog.MaxCatalogBytes - 1, catalog.MaxCatalogBytes, catalog.MaxCatalogBytes + 1} { + t.Run(strconv.Itoa(size), func(t *testing.T) { + prefix := `{"data":[{"id":"synthetic/boundary"}]}` + body := prefix + strings.Repeat(" ", size-len(prefix)) + server := sourcestub.New() + defer server.Close() + server.Refuse(http.StatusOK, body) + source := vendoredSource(t, "aiand") + source.Address = server.URL() + dir := t.TempDir() + if err := WriteAPIKey(dir, "sk-existing-default-test-key"); err != nil { + t.Fatal(err) + } + before, err := os.ReadFile(BudgetConfigPath(dir)) + if err != nil { + t.Fatal(err) + } + outcome, err := ConnectService(context.Background(), dir, PersistedSource{ + ID: source.ID, Written: source.Written, Key: "sk-synthetic-catalog-probe-key", + }, source, nil) + if err != nil { + t.Fatal(err) + } + if size <= catalog.MaxCatalogBytes { + if outcome.Kind != modelsource.OutcomeConnected || !slices.Equal(outcome.ModelIDs, []string{"synthetic/boundary"}) || len(PersistedSources(dir)) != 1 { + t.Fatalf("complete bounded listing did not connect: %+v", outcome) + } + } else { + after, err := os.ReadFile(BudgetConfigPath(dir)) + if err != nil || outcome.Kind != modelsource.OutcomeUnanswered || string(after) != string(before) { + t.Fatalf("oversized listing admitted a prefix or changed the profile: %+v err=%v", outcome, err) + } + } + requests := server.Requests() + if len(requests) != 1 || requests[0].Method != http.MethodGet { + t.Fatalf("catalog bound must not turn into a paid fallback: %+v", requests) + } + }) + } +} + +func TestUnreadableConnectionCatalogsDoNotSaveOrBuyAFallback(t *testing.T) { + t.Setenv("AIAND_API_KEY", "") + for _, kind := range []string{"malformed JSON", "interrupted response", "cancelled request"} { + t.Run(kind, func(t *testing.T) { + var requests atomic.Int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests.Add(1) + body := `{"data":[{"id":"synthetic/model"}]}` + if kind == "malformed JSON" { + body = `{"data":[` + } + if kind == "interrupted response" { + w.Header().Set("Content-Length", strconv.Itoa(len(body)+1)) + } + _, _ = w.Write([]byte(body)) + })) + defer server.Close() + source := vendoredSource(t, "aiand") + source.Address = server.URL + dir := t.TempDir() + if err := WriteAPIKey(dir, "sk-existing-default-test-key"); err != nil { + t.Fatal(err) + } + before, err := os.ReadFile(BudgetConfigPath(dir)) + if err != nil { + t.Fatal(err) + } + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + wantRequests := 1 + if kind == "cancelled request" { + cancel() + wantRequests = 0 + } + outcome, err := ConnectService(ctx, dir, PersistedSource{ + ID: source.ID, Written: source.Written, Key: "sk-synthetic-catalog-probe-key", + }, source, nil) + after, readErr := os.ReadFile(BudgetConfigPath(dir)) + if err != nil || readErr != nil || outcome.Kind != modelsource.OutcomeUnanswered || string(after) != string(before) || requests.Load() != int32(wantRequests) { + t.Fatalf("unreadable catalog was accepted or retried: %+v err=%v readErr=%v requests=%d", outcome, err, readErr, requests.Load()) + } + }) + } +} diff --git a/internal/manual/chat/services.md b/internal/manual/chat/services.md index 4c94381242..e6679eb7d3 100644 --- a/internal/manual/chat/services.md +++ b/internal/manual/chat/services.md @@ -127,6 +127,11 @@ provider's own answer, cut at 120 characters on a word boundary: `deepseek refused that key — Authentication Fails, Your api key is invalid`. No answer is different: `deepseek did not answer · nothing was saved`. +When a connection is proved by its model list, the list is read in full up to 16 MiB, +including providers whose rows carry extra metadata. A larger list, unreadable JSON, +or an interrupted response does not save that connection; previously saved keys and +providers stay as they were. + codeaf checks each billing door in order with a one-token completion and binds the first one that answers. A no-plan or payment answer on one door moves the connection check to the next; a bad-key answer stops immediately in the vendor's words. A missing answer also