Skip to content

fix(catalog): accept Together top-level /models arrays #639

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Wibias merged 2 commits into from
Jul 29, 2026
+44 −9
Merged

fix(catalog): accept Together top-level /models arrays #639

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a296219
fix(catalog): accept Together top-level /models arrays
Wibias Jul 28, 2026
a2b8458
fix(catalog): keep models[] for probe only; array+data for discovery
Wibias Jul 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 27 additions & 3 deletions src/codex/catalog/provider-fetch.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

@chatgpt-codex-connector chatgpt-codex-connector Bot Jul 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Reconcile the catalog test before accepting models

For an OpenAI-chat provider returning { models: [] }, this branch now yields an empty array; isProviderModelsApiItems accepts it vacuously, so discovery becomes authoritative-empty and drops the configured fallback. That directly contradicts the unchanged missing-data case in tests/codex-catalog.test.ts:1888-1917, which expects static-fallback and an invalid_response status, so the catalog suite will fail. Either restrict this envelope to provider-aware Google handling or update the catalog contract and add a focused top-level-array catalog regression, rather than testing only the management probe.

AGENTS.md reference: AGENTS.md:L93-L95

Useful? React with 👍 / 👎.

}
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;
Expand Down Expand Up @@ -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) {
Expand Down
10 changes: 4 additions & 6 deletions src/server/management/provider-routes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -339,12 +340,9 @@ export async function handleProviderRoutes(ctx: ManagementContext): Promise<Resp
if (!res.ok) {
return jsonResponse({ ok: false, latencyMs, error: `upstream /models returned ${res.status}` });
}
const json = await res.json().catch(() => 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" });
}
Expand Down
13 changes: 13 additions & 0 deletions tests/provider-connection-test.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
Comment on lines +138 to +149

@coderabbitai coderabbitai Bot Jul 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add catalog-discovery coverage for the Together response.

This test verifies only /api/providers/test; it never exercises fetchProviderModels, which is the primary path fixed by the PR. A future regression in catalog normalization could therefore leave this test green while Together model discovery fails.

Add a focused catalog test using the same top-level array and assert that the discovered catalog contains meta/llama and Qwen/Qwen.

As per path instructions, a behavior change in src/ should have a focused regression test near the existing tests for that subsystem; the current test covers only the connectivity probe.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/provider-connection-test.test.ts` around lines 138 - 149, Add focused
coverage for fetchProviderModels using the Together-style top-level array
response, reusing the existing mock payload and Together configuration. Assert
the discovered catalog includes both “meta/llama” and “Qwen/Qwen”, alongside—not
instead of—the existing probe test.

Source: Path instructions


test("malformed 2xx data is an explicit failure, not a silent pass", async () => {
globalThis.fetch = (async () => new Response(JSON.stringify({ nope: true }), {
status: 200,
Expand Down
Loading