diff --git a/go/internal/codex/catalog_provider_fetch.go b/go/internal/codex/catalog_provider_fetch.go index 3d744f19b..5b6baa28f 100644 --- a/go/internal/codex/catalog_provider_fetch.go +++ b/go/internal/codex/catalog_provider_fetch.go @@ -1,6 +1,7 @@ package codex import ( + "bytes" "context" "encoding/json" "errors" @@ -100,16 +101,13 @@ func FetchProviderModels(ctx context.Context, options ProviderModelFetchOptions) if err != nil || len(body) > maxProviderModelsResponseBytes { return providerFetchFallback(options, configured, DiscoveryFailureInvalidResponse, 0, errors.New("provider models response is invalid")) } - var payload struct { - Data []ProviderModelsAPIItem `json:"data"` - } - decoder := json.NewDecoder(strings.NewReader(string(body))) - if err := decoder.Decode(&payload); err != nil || decoder.Decode(&struct{}{}) != io.EOF { + items, ok := providerModelsAPIItemsFromResponse(body) + if !ok { return providerFetchFallback(options, configured, DiscoveryFailureInvalidResponse, 0, errors.New("provider models response is invalid")) } - live := make([]CatalogModel, 0, len(payload.Data)+len(configured)) - seen := make(map[string]bool, len(payload.Data)) - for _, item := range payload.Data { + live := make([]CatalogModel, 0, len(items)+len(configured)) + seen := make(map[string]bool, len(items)) + for _, item := range items { item.ID = strings.TrimSpace(item.ID) if item.ID == "" || seen[item.ID] { continue @@ -133,6 +131,46 @@ func FetchProviderModels(ctx context.Context, options ProviderModelFetchOptions) return cloneCatalogModels(live), nil } +// providerModelsAPIItemsFromResponse normalizes OpenAI-compatible /models payloads. +// Supports { "data": [...] } and top-level arrays (Together AI #617). +// Google's { "models": [...] } is intentionally not accepted here — catalog discovery +// must not treat a stray models key on openai-chat responses as valid. +func providerModelsAPIItemsFromResponse(body []byte) ([]ProviderModelsAPIItem, bool) { + trimmed := bytes.TrimSpace(body) + if len(trimmed) == 0 { + return nil, false + } + switch trimmed[0] { + case '[': + var items []ProviderModelsAPIItem + if err := json.Unmarshal(trimmed, &items); err != nil { + return nil, false + } + return items, true + case '{': + var payload map[string]json.RawMessage + decoder := json.NewDecoder(bytes.NewReader(trimmed)) + if err := decoder.Decode(&payload); err != nil || decoder.Decode(&struct{}{}) != io.EOF { + return nil, false + } + data, ok := payload["data"] + if !ok { + return nil, false + } + data = bytes.TrimSpace(data) + if len(data) == 0 || data[0] != '[' { + return nil, false + } + var items []ProviderModelsAPIItem + if err := json.Unmarshal(data, &items); err != nil { + return nil, false + } + return items, true + default: + return nil, false + } +} + func configuredCatalogModels(providerName string, provider config.ProviderConfig) []CatalogModel { models := make([]CatalogModel, 0, len(provider.Models)+1) seen := map[string]bool{} diff --git a/go/internal/codex/catalog_provider_fetch_test.go b/go/internal/codex/catalog_provider_fetch_test.go index 997a23608..dc94334a8 100644 --- a/go/internal/codex/catalog_provider_fetch_test.go +++ b/go/internal/codex/catalog_provider_fetch_test.go @@ -58,3 +58,46 @@ func TestFetchProviderModelsDegradesAndRecordsFailure(t *testing.T) { t.Fatalf("status = %#v", status) } } + +func TestFetchProviderModelsAcceptsTogetherTopLevelArray(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`[{"id":"meta/llama"},{"id":"Qwen/Qwen"}]`)) + })) + defer server.Close() + + models, err := FetchProviderModels(context.Background(), ProviderModelFetchOptions{ + ProviderName: "together", + Provider: config.ProviderConfig{Adapter: "openai-chat", BaseURL: server.URL + "/v1", AllowPrivateNetwork: true}, + Cache: NewModelCache(), + TTL: time.Minute, + }) + if err != nil { + t.Fatal(err) + } + if len(models) != 2 || models[0].ID != "meta/llama" || models[1].ID != "Qwen/Qwen" { + t.Fatalf("models = %#v", models) + } +} + +func TestProviderModelsAPIItemsFromResponseShapes(t *testing.T) { + items, ok := providerModelsAPIItemsFromResponse([]byte(`[{"id":"a"}]`)) + if !ok || len(items) != 1 || items[0].ID != "a" { + t.Fatalf("top-level array: items=%#v ok=%v", items, ok) + } + items, ok = providerModelsAPIItemsFromResponse([]byte(`{"data":[{"id":"b"}]}`)) + if !ok || len(items) != 1 || items[0].ID != "b" { + t.Fatalf("data wrapper: items=%#v ok=%v", items, ok) + } + items, ok = providerModelsAPIItemsFromResponse([]byte(`{"data":[]}`)) + if !ok || items == nil || len(items) != 0 { + t.Fatalf("empty data: items=%#v ok=%v", items, ok) + } + // Google {models} and objects without data must not be accepted for catalog discovery. + if _, ok = providerModelsAPIItemsFromResponse([]byte(`{"models":[{"id":"c"}]}`)); ok { + t.Fatal("expected {models} to be rejected for catalog discovery") + } + if _, ok = providerModelsAPIItemsFromResponse([]byte(`{"nope":true}`)); ok { + t.Fatal("expected object without data to be rejected") + } +} diff --git a/src/codex/catalog/provider-fetch.ts b/src/codex/catalog/provider-fetch.ts index 7ad985eef..916babd25 100644 --- a/src/codex/catalog/provider-fetch.ts +++ b/src/codex/catalog/provider-fetch.ts @@ -71,6 +71,32 @@ export function isProviderModelsApiItems(value: unknown): value is ProviderModel ); } +/** + * Normalize OpenAI-compatible /models payloads for catalog discovery. + * Supports `{ data: [...] }` and top-level arrays (Together AI `#617`). + * Google's `{ models: [...] }` is handled by the connectivity probe only — catalog + * discovery must not treat a stray `models` key on openai-chat responses as valid. + */ +export function providerModelsListFromResponse(json: unknown): unknown { + if (Array.isArray(json)) return json; + if (json !== null && typeof json === "object" && !Array.isArray(json)) { + const data = (json as { data?: unknown }).data; + if (Array.isArray(data)) return data; + } + return undefined; +} + +/** Connectivity-probe shape: also accepts Google `{ models: [...] }`. */ +export function providerModelsListFromProbeResponse(json: unknown): unknown { + if (Array.isArray(json)) return json; + if (json !== null && typeof json === "object" && !Array.isArray(json)) { + const obj = json as { data?: unknown; models?: unknown }; + if (Array.isArray(obj.data)) return obj.data; + if (Array.isArray(obj.models)) return obj.models; + } + return undefined; +} + export function configuredContextWindow(prov: OcxProviderConfig, id: string): number | undefined { const configured = modelRecordValue(prov.modelContextWindows, id) ?? prov.contextWindow; return typeof configured === "number" && configured > 0 ? configured : undefined; @@ -366,9 +392,7 @@ export async function fetchProviderModels(name: string, prov: OcxProviderConfig, } return models; } - const data = json !== null && typeof json === "object" && !Array.isArray(json) - ? (json as { data?: unknown }).data - : undefined; + const data = providerModelsListFromResponse(json); if (!isProviderModelsApiItems(data)) { const { models, fallback, shouldLog } = failedDiscoveryFallback({ reason: "invalid_response" }); if (shouldLog) { diff --git a/src/server/management/provider-routes.ts b/src/server/management/provider-routes.ts index f4b0fed2b..7735d22a4 100644 --- a/src/server/management/provider-routes.ts +++ b/src/server/management/provider-routes.ts @@ -2,6 +2,7 @@ import { randomUUID } from "node:crypto"; import { readFileSync } from "node:fs"; import type { CatalogModel } from "../../codex/catalog"; import { catalogModelSlug, invalidateCodexModelsCache, nativeModelRows, uniqueCatalogModelsForPublicList } from "../../codex/catalog"; +import { providerModelsListFromProbeResponse } from "../../codex/catalog/provider-fetch"; import { DEFAULT_SUBAGENT_MODELS, codexAutoStartEnabled, @@ -344,12 +345,9 @@ export async function handleProviderRoutes(ctx: ManagementContext): Promise null) as { data?: unknown; models?: unknown } | null; - // OpenAI-style lists use { data: [...] }; Google's /v1beta/models (the other shape - // buildModelsRequest can produce) returns { models: [...] }. - const list = json && typeof json === "object" && !Array.isArray(json) - ? (Array.isArray(json.data) ? json.data : Array.isArray(json.models) ? json.models : undefined) - : undefined; + const json = await res.json().catch(() => null); + // OpenAI-style { data }, Google { models }, and Together-style top-level arrays (#617). + const list = providerModelsListFromProbeResponse(json); if (!Array.isArray(list)) { return jsonResponse({ ok: false, latencyMs, error: "upstream /models returned an unexpected shape" }); } diff --git a/tests/provider-connection-test.test.ts b/tests/provider-connection-test.test.ts index f4bf64775..55cb7d9e4 100644 --- a/tests/provider-connection-test.test.ts +++ b/tests/provider-connection-test.test.ts @@ -135,6 +135,19 @@ describe("POST /api/providers/test (WP040 connectivity probe)", () => { expect(body.models).toBe(3); }); + test("Together-style top-level /models array is accepted (#617)", async () => { + globalThis.fetch = (async () => new Response(JSON.stringify([{ id: "meta/llama" }, { id: "Qwen/Qwen" }]), { + status: 200, + headers: { "content-type": "application/json" }, + })) as typeof fetch; + const config = baseConfig({ + together: { adapter: "openai-chat", baseUrl: "https://api.together.xyz/v1", apiKey: "tg-key" }, + }); + const { body } = await probe(config, "together"); + expect(body.ok).toBe(true); + expect(body.models).toBe(2); + }); + test("malformed 2xx data is an explicit failure, not a silent pass", async () => { globalThis.fetch = (async () => new Response(JSON.stringify({ nope: true }), { status: 200,