diff --git a/src/config.ts b/src/config.ts index 4b83c8295..b3a90756b 100644 --- a/src/config.ts +++ b/src/config.ts @@ -60,6 +60,7 @@ import { OPENAI_PROVIDER_TIER_VERSION, pinnedWireAdapter, REASONING_SUMMARY_DELIVERY_VALUES, + UPSTREAM_HTTP_VERSION_VALUES, type OcxClaudeCodeConfig, type OcxConfig, type OcxApiKeyEntry, @@ -734,6 +735,7 @@ const providerConfigSchema = z.object({ modelSupportsServiceTier: z.record(z.string().min(1), z.boolean()).optional(), preserveResponsesReasoningContent: z.boolean().optional(), allowPrivateNetwork: z.boolean().optional(), + upstreamHttpVersion: z.enum(UPSTREAM_HTTP_VERSION_VALUES).optional(), noStructuredOutputModels: z.array(z.string().min(1)) .transform(normalizeNonBlankStringArray) .optional(), @@ -906,6 +908,20 @@ export function apiKeyTransportConfigError( return null; } +/** + * Shared runtime boundary for the per-provider upstream HTTP-version pin (#1668). Used by + * the management write path (providerManagementConfigError / PATCH) so it can never disagree + * with the strict zod load schema: a value that survives POST/PATCH is always loadable, and + * a value the loader rejects is rejected at write time too. + */ +export function upstreamHttpVersionConfigError(value: unknown): string | null { + if (value === undefined || value === null) return null; + if (typeof value !== "string" || !(UPSTREAM_HTTP_VERSION_VALUES as readonly string[]).includes(value)) { + return 'upstreamHttpVersion must be one of "auto", "http1.1", "h1", "http2", "h2", or null to clear'; + } + return null; +} + export function positiveIntegerRecordConfigError(value: unknown, field: string): string | null { if (value === undefined) return null; if (!value || typeof value !== "object" || Array.isArray(value)) return `${field} must be a plain object`; diff --git a/src/server/auth-cors.ts b/src/server/auth-cors.ts index 66226af9a..ecc488c55 100644 --- a/src/server/auth-cors.ts +++ b/src/server/auth-cors.ts @@ -16,6 +16,7 @@ import { retryOn429PolicyConfigError, requestPacingConfigError, sanitizeModelCostsForDisplay, + upstreamHttpVersionConfigError, } from "../config"; import { providerDestinationConfigError } from "../lib/destination-policy"; import { redactSecretString } from "../lib/redact"; @@ -520,6 +521,10 @@ export function providerManagementConfigError(name: unknown, provider: unknown): if (requestPacingError) { return `provider ${JSON.stringify(redactSecretString(name))} ${requestPacingError}`; } + const upstreamHttpVersionError = upstreamHttpVersionConfigError(raw.upstreamHttpVersion); + if (upstreamHttpVersionError) { + return `provider ${JSON.stringify(redactSecretString(name))} ${upstreamHttpVersionError}`; + } const modelCostsError = providerModelCostsConfigError(raw.modelCosts); if (modelCostsError) { // The provider name is caller-controlled and can be token-shaped; redact and JSON-escape @@ -640,6 +645,7 @@ export function safeConfigDTO(config: OcxConfig): unknown { "noTopPModels", "noPenaltyModels", "noStructuredOutputModels", + "upstreamHttpVersion", "autoToolChoiceOnlyModels", "preserveReasoningContentModels", "requiresReasoningPlaceholderModels", diff --git a/src/server/management/provider-routes.ts b/src/server/management/provider-routes.ts index 8701bc752..b8627d3ba 100644 --- a/src/server/management/provider-routes.ts +++ b/src/server/management/provider-routes.ts @@ -17,6 +17,7 @@ import { requestPacingConfigError, readConfigAdmissionSnapshot, saveConfigPreservingClaudeCode, + upstreamHttpVersionConfigError, withConfigMutationLockSync, } from "../../config"; import { @@ -194,6 +195,19 @@ function applyProviderPatchFields( } touched = true; } + if (Object.hasOwn(rawBody, "upstreamHttpVersion")) { + const value = rawBody.upstreamHttpVersion; + if (value === null || value === "") { + delete next.upstreamHttpVersion; + } else { + const versionError = upstreamHttpVersionConfigError(value); + if (versionError) return { error: versionError }; + // `upstreamHttpVersionConfigError` is the shared write boundary; the assertion is + // explicit because the incoming value is an unknown JSON scalar. + next.upstreamHttpVersion = value as OcxProviderConfig["upstreamHttpVersion"]; + } + touched = true; + } // The Models page edits the catalog hints in place; keep them on the existing // provider mutation path so validation, cache invalidation, and convergence stay unified (#1073). if (Object.hasOwn(rawBody, "contextWindow")) { @@ -374,6 +388,7 @@ export async function handleProviderRoutes(ctx: ManagementContext): Promise, string> = { + "http1.1": "http1.1", + h1: "h1", + http2: "http2", + h2: "h2", +}; + +/** Attach Bun's `protocol` pin when the provider opted into a fixed HTTP version. */ +export function withUpstreamHttpVersion( + input: Parameters[0], + init: RequestInit | undefined, + provider: OcxProviderConfig, +): RequestInit | undefined { + const version = provider.upstreamHttpVersion; + if (!version || version === "auto") return init; + // Bun's protocol pin requires an https: target; local/plaintext upstreams keep + // their existing transport untouched. + const target = typeof input === "string" ? input : input instanceof URL ? input.href : input.url; + try { + if (new URL(target).protocol !== "https:") return init; + } catch { + return init; + } + return { ...(init ?? {}), protocol: UPSTREAM_HTTP_VERSION_PROTOCOL[version] } as RequestInit; +} + export function providerFetch( provider: OcxProviderConfig, runtime: BunRuntimeGateInput = currentBunRuntimeIdentity(), @@ -162,7 +194,7 @@ export function providerFetch( if (typeof input === "string" && init && shouldUseCodexWsUpstream(input, init, runtime)) { return codexWsUpstreamFetch(input, init, base, runtime); } - return base(input, init); + return base(input, withUpstreamHttpVersion(input, init, provider)); }; const waitForPacing = (signal?: AbortSignal) => options.providerName ? waitForProviderRequestSlot(options.providerName, provider, options.modelId, signal) diff --git a/src/types.ts b/src/types.ts index 2a58dfbe7..7f409d333 100644 --- a/src/types.ts +++ b/src/types.ts @@ -1375,6 +1375,14 @@ export interface OcxProviderConfig { * link-local, or unique-local upstreams. Metadata endpoints remain blocked. */ allowPrivateNetwork?: boolean; + /** + * Pin the HTTP version used for upstream provider requests. Bun's fetch negotiates + * HTTP/2 via TLS ALPN by default; some Cloudflare-fronted SSE endpoints hang on + * HTTP/2 streaming responses (issue #1668). "http1.1" / "h1" forces HTTP/1.1, + * "http2" / "h2" forces HTTP/2. Absent or "auto" keeps Bun's default negotiation + * (current behavior unchanged). Only meaningful for https: base URLs. + */ + upstreamHttpVersion?: UpstreamHttpVersion; /** Keep provider settings on disk but exclude it from routing and model/catalog listings. */ disabled?: boolean; /** @@ -1668,6 +1676,21 @@ export interface OcxProviderConfig { nativeLocalExec?: "off" | "codex-sandbox" | "on"; } +/** + * Accepted values for the per-provider upstream HTTP-version pin (#1668). Shared by the + * zod load schema, the management write boundary (POST/PATCH), and the fetch runtime, so + * a value that one boundary accepts can never be rejected by another. + */ +export const UPSTREAM_HTTP_VERSION_VALUES = [ + "auto", + "http1.1", + "h1", + "http2", + "h2", +] as const; + +export type UpstreamHttpVersion = (typeof UPSTREAM_HTTP_VERSION_VALUES)[number]; + export const REASONING_SUMMARY_DELIVERY_VALUES = [ "sequential", "sequential_cutoff", diff --git a/tests/management-provider-validation.test.ts b/tests/management-provider-validation.test.ts index 1087de1c9..7366ada19 100644 --- a/tests/management-provider-validation.test.ts +++ b/tests/management-provider-validation.test.ts @@ -3170,3 +3170,165 @@ describe("provider management validation", () => { } }); }); + +describe("provider upstreamHttpVersion management contract (#1668)", () => { + function makeConfig(): OcxConfig { + return { + port: 0, + hostname: "127.0.0.1", + defaultProvider: "nvidia", + providers: { + nvidia: { + adapter: "openai-chat", + baseUrl: "https://integrate.api.nvidia.com/v1", + apiKey: "sk-nvidia", + }, + }, + }; + } + + // Direct handleManagementAPI calls (no startServer) keep the whole contract in one + // synchronous authority, matching the request-pacing PATCH tests above. + async function withRequest(liveConfig: OcxConfig, run: (request: (path: string, init?: RequestInit) => Promise) => Promise): Promise { + const resolvedError = spyOn(destinationPolicy, "providerDestinationResolvedError") + .mockResolvedValue(null); + try { + const request = async (path: string, init?: RequestInit) => { + const req = new Request(`http://127.0.0.1${path}`, init); + return handleManagementAPI(req, new URL(req.url), liveConfig, { + createManagementConvergeCodex: catalogConvergenceFactory(), + }); + }; + await run(request); + } finally { + resolvedError.mockRestore(); + } + } + + test("POST accepts a valid upstreamHttpVersion and persists it; GET exposes it", async () => { + if (existsSync(TEST_DIR)) rmSync(TEST_DIR, { recursive: true }); + mkdirSync(TEST_DIR, { recursive: true }); + process.env.OPENCODEX_HOME = TEST_DIR; + const liveConfig = makeConfig(); + saveConfig(liveConfig); + await withRequest(liveConfig, async (request) => { + const created = await request("/api/providers", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ + name: "h1-provider", + provider: { + adapter: "openai-chat", + baseUrl: "https://api.example.test/v1", + upstreamHttpVersion: "http1.1", + }, + }), + }); + expect(created?.status).toBe(200); + // Live config, disk reload, and the public GET row must all carry the pin. + expect(liveConfig.providers["h1-provider"]?.upstreamHttpVersion).toBe("http1.1"); + expect(loadConfig().providers["h1-provider"]?.upstreamHttpVersion).toBe("http1.1"); + const list = await request("/api/providers"); + expect(await list?.json()).toContainEqual(expect.objectContaining({ + name: "h1-provider", + upstreamHttpVersion: "http1.1", + })); + }); + }); + + test("POST rejects an invalid upstreamHttpVersion at the write boundary without persisting", async () => { + if (existsSync(TEST_DIR)) rmSync(TEST_DIR, { recursive: true }); + mkdirSync(TEST_DIR, { recursive: true }); + process.env.OPENCODEX_HOME = TEST_DIR; + const liveConfig = makeConfig(); + saveConfig(liveConfig); + await withRequest(liveConfig, async (request) => { + const rejected = await request("/api/providers", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ + name: "bad-version", + provider: { + adapter: "openai-chat", + baseUrl: "https://api.example.test/v1", + upstreamHttpVersion: "http3", + }, + }), + }); + expect(rejected?.status).toBe(400); + expect(await rejected?.json()).toMatchObject({ + error: expect.stringContaining("upstreamHttpVersion"), + }); + expect(loadConfig().providers["bad-version"]).toBeUndefined(); + }); + }); + + test("PATCH sets, then clears upstreamHttpVersion with live + disk persistence", async () => { + if (existsSync(TEST_DIR)) rmSync(TEST_DIR, { recursive: true }); + mkdirSync(TEST_DIR, { recursive: true }); + process.env.OPENCODEX_HOME = TEST_DIR; + const liveConfig = makeConfig(); + saveConfig(liveConfig); + await withRequest(liveConfig, async (request) => { + const set = await request("/api/providers?name=nvidia", { + method: "PATCH", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ upstreamHttpVersion: "http1.1" }), + }); + expect(set?.status).toBe(200); + expect(liveConfig.providers.nvidia?.upstreamHttpVersion).toBe("http1.1"); + expect(loadConfig().providers.nvidia?.upstreamHttpVersion).toBe("http1.1"); + + const invalid = await request("/api/providers?name=nvidia", { + method: "PATCH", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ upstreamHttpVersion: "h3" }), + }); + expect(invalid?.status).toBe(400); + expect(liveConfig.providers.nvidia?.upstreamHttpVersion).toBe("http1.1"); + + const clear = await request("/api/providers?name=nvidia", { + method: "PATCH", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ upstreamHttpVersion: null }), + }); + expect(clear?.status).toBe(200); + expect(liveConfig.providers.nvidia?.upstreamHttpVersion).toBeUndefined(); + expect(loadConfig().providers.nvidia?.upstreamHttpVersion).toBeUndefined(); + }); + }); + + test("safeConfigDTO exposes upstreamHttpVersion without leaking it into the live row", async () => { + if (existsSync(TEST_DIR)) rmSync(TEST_DIR, { recursive: true }); + mkdirSync(TEST_DIR, { recursive: true }); + process.env.OPENCODEX_HOME = TEST_DIR; + const liveConfig = makeConfig(); + liveConfig.providers.nvidia = { + ...liveConfig.providers.nvidia!, + upstreamHttpVersion: "http1.1", + }; + saveConfig(liveConfig); + const dto = safeConfigDTO(loadConfig()) as { + providers?: Record>; + }; + expect(dto.providers?.nvidia?.upstreamHttpVersion).toBe("http1.1"); + }); + + test("providerManagementConfigError rejects invalid upstreamHttpVersion values", () => { + expect(providerManagementConfigError("x", { + adapter: "openai-chat", + baseUrl: "https://api.example.test/v1", + upstreamHttpVersion: "http3", + })).toContain("upstreamHttpVersion"); + expect(providerManagementConfigError("x", { + adapter: "openai-chat", + baseUrl: "https://api.example.test/v1", + upstreamHttpVersion: "http1.1", + })).toBeNull(); + expect(providerManagementConfigError("x", { + adapter: "openai-chat", + baseUrl: "https://api.example.test/v1", + upstreamHttpVersion: 42, + })).toContain("upstreamHttpVersion"); + }); +}); diff --git a/tests/upstream-http-version.test.ts b/tests/upstream-http-version.test.ts new file mode 100644 index 000000000..d23bce3f5 --- /dev/null +++ b/tests/upstream-http-version.test.ts @@ -0,0 +1,119 @@ +import { describe, expect, test } from "bun:test"; +import { providerFetch, withUpstreamHttpVersion } from "../src/server/responses/fetch-helpers"; +import type { OcxProviderConfig } from "../src/types"; + +const HTTPS_URL = "https://opencode.ai/zen/go/v1/chat/completions"; +const HTTP_URL = "http://127.0.0.1:10900/zen/go/v1/chat/completions"; + +// OcxProviderConfig has no fetch member; the stub fetch used by the propagation +// tests is a test-only transport override, so the helper needs an intersection. +type TestProvider = OcxProviderConfig & { fetch?: typeof globalThis.fetch }; + +function provider(overrides: Partial = {}): TestProvider { + return { + adapter: "openai-chat", + baseUrl: "https://opencode.ai/zen/go/v1", + ...overrides, + }; +} + +describe("withUpstreamHttpVersion", () => { + test("absent upstreamHttpVersion keeps the init untouched", () => { + const init = { method: "POST", headers: {} }; + expect(withUpstreamHttpVersion(HTTPS_URL, init, provider())).toBe(init); + }); + + test("auto keeps the init untouched (default negotiation)", () => { + const init = { method: "POST", headers: {} }; + expect(withUpstreamHttpVersion(HTTPS_URL, init, provider({ upstreamHttpVersion: "auto" }))).toBe(init); + }); + + test("undefined init without a pin stays undefined", () => { + expect(withUpstreamHttpVersion(HTTPS_URL, undefined, provider())).toBeUndefined(); + expect(withUpstreamHttpVersion(HTTPS_URL, undefined, provider({ upstreamHttpVersion: "auto" }))).toBeUndefined(); + }); + + test("http1.1 pins the protocol on https targets", () => { + const init = { method: "POST", headers: {} }; + const out = withUpstreamHttpVersion(HTTPS_URL, init, provider({ upstreamHttpVersion: "http1.1" }))!; + expect(out).not.toBe(init); + expect((out as RequestInit & { protocol?: string }).protocol).toBe("http1.1"); + }); + + test("h1/h2/http2 map through to Bun protocol values", () => { + for (const [version, expected] of [ + ["h1", "h1"], + ["http2", "http2"], + ["h2", "h2"], + ] as const) { + const out = withUpstreamHttpVersion( + HTTPS_URL, + { method: "POST" }, + provider({ upstreamHttpVersion: version }), + )!; + expect((out as RequestInit & { protocol?: string }).protocol).toBe(expected); + } + }); + + test("plain-http targets are left untouched (Bun protocol requires https)", () => { + const init = { method: "POST", headers: {} }; + expect(withUpstreamHttpVersion(HTTP_URL, init, provider({ upstreamHttpVersion: "http1.1" }))).toBe(init); + }); + + test("Request objects resolve their url for the https guard", () => { + const request = new Request(HTTPS_URL); + const init = { method: "POST" }; + const out = withUpstreamHttpVersion(request, init, provider({ upstreamHttpVersion: "http1.1" }))!; + expect((out as RequestInit & { protocol?: string }).protocol).toBe("http1.1"); + }); + + test("unparseable targets degrade to the untouched init", () => { + const init = { method: "POST" }; + expect(withUpstreamHttpVersion("not a url", init, provider({ upstreamHttpVersion: "http1.1" }))).toBe(init); + }); + + test("absent init still applies the pin (providerFetch without init)", () => { + const out = withUpstreamHttpVersion(HTTPS_URL, undefined, provider({ upstreamHttpVersion: "http1.1" }))!; + expect(out).toEqual({ protocol: "http1.1" }); + const untouched = withUpstreamHttpVersion(HTTPS_URL, undefined, provider()); + expect(untouched).toBeUndefined(); + }); +}); + +describe("providerFetch upstreamHttpVersion propagation", () => { + type FetchOverride = typeof globalThis.fetch; + + function stubFetch(seen: { init?: RequestInit }): FetchOverride { + return async (_input: RequestInfo | URL, init?: RequestInit) => { + seen.init = init; + return new Response("ok"); + }; + } + + test("a provider-pinned version reaches the underlying fetch call", async () => { + const seen: { init?: RequestInit } = {}; + const fetcher = providerFetch(provider({ + upstreamHttpVersion: "http1.1", + fetch: stubFetch(seen), + })); + await fetcher(HTTPS_URL, { method: "POST", body: "{}" }); + expect((seen.init as RequestInit & { protocol?: string })?.protocol).toBe("http1.1"); + }); + + test("no pin keeps the caller init verbatim", async () => { + const seen: { init?: RequestInit } = {}; + const fetcher = providerFetch(provider({ fetch: stubFetch(seen) })); + await fetcher(HTTPS_URL, { method: "POST", body: "{}" }); + expect(seen.init).toEqual({ method: "POST", body: "{}" }); + }); + + test("no init still applies a pinned version to the fetch call", async () => { + const seen: { init?: RequestInit } = {}; + const fetcher = providerFetch(provider({ + upstreamHttpVersion: "http1.1", + fetch: stubFetch(seen), + })); + await fetcher(HTTPS_URL); + expect((seen.init as RequestInit & { protocol?: string })?.protocol).toBe("http1.1"); + }); +});