Repository navigation
fix(transport): decide fresh-connection opt-out at the dispatch boundary #4977
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.
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
Changes from all commits
5302a68
e010296
8a69219
bc6c81d
014fe65
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -44,6 +44,31 @@ export function safeOriginLabel(url: string): string { | |
| } | ||
| } | ||
|
|
||
| /** | ||
| * Check whether a target host should bypass Bun's keep-alive pool reuse. | ||
| * Configured via the `OCX_FRESH_CONNECTION_HOSTS` environment variable (comma-separated). | ||
| */ | ||
| export function wantsFreshConnection( | ||
| input: Parameters<typeof globalThis.fetch>[0], | ||
| hostsEnv = process.env.OCX_FRESH_CONNECTION_HOSTS, | ||
| ): boolean { | ||
| if (!hostsEnv) return false; | ||
| try { | ||
| const rawUrl = typeof input === "string" ? input : input instanceof URL ? input.href : input.url; | ||
| const host = new URL(rawUrl).hostname.toLowerCase(); | ||
| const targets = hostsEnv | ||
| .split(",") | ||
| .map(h => h.trim().toLowerCase().replace(/^\.+/, "")) | ||
| .filter(Boolean); | ||
| for (const target of targets) { | ||
| if (host === target || host.endsWith(`.${target}`)) return true; | ||
| } | ||
| } catch { | ||
| /* unparseable target URL keeps default connection behavior */ | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
|
|
||
|
|
||
| export interface PaceAwareFetch { | ||
|
|
@@ -79,12 +104,27 @@ export function providerFetch( | |
| // Rebuilt dispatches must use the same physical-send boundary as ordinary HTTP sends. | ||
| // Return the original 3xx so the response owner retains its retry/health/relay contract. | ||
| const dispatch = Object.assign( | ||
| (input: Parameters<typeof globalThis.fetch>[0], init?: RequestInit) => | ||
| base(input, { ...init, redirect: "manual" }), | ||
| (input: Parameters<typeof globalThis.fetch>[0], init?: RequestInit) => { | ||
| const headers = new Headers(init?.headers ?? (input instanceof Request ? input.headers : undefined)); | ||
| const fresh = wantsFreshConnection(input); | ||
| if (fresh) { | ||
| headers.set("Connection", "close"); | ||
| } | ||
| return base(input, { | ||
| ...init, | ||
| headers, | ||
| redirect: "manual", | ||
| ...(fresh ? { keepalive: false } : {}), | ||
| }); | ||
| }, | ||
| { preconnect }, | ||
| ) as typeof globalThis.fetch; | ||
| const httpFetch = Object.assign( | ||
| async (input: Parameters<typeof globalThis.fetch>[0], init?: RequestInit) => { | ||
| // The hook inspects the outgoing headers and refuses the send by throwing; it is not a | ||
| // mutator, and the copy it receives is deliberately not threaded onward. `Connection` | ||
| // is decided inside `dispatch`, which runs after this, so the fresh-connection policy | ||
| // wins regardless of what any caller or hook put in the header. | ||
| options.beforeDispatch?.(new Headers(init?.headers ?? (input instanceof Request ? input.headers : undefined))); | ||
| const dispatchInit = { ...withUpstreamHttpVersion(input, init, provider), timeout: 0 }; | ||
| return options.dispatchOverride | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a Useful? React with 👍 / 👎. |
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -43,6 +43,12 @@ the upstream HTTP-version helper. Server, provider, and WebSocket data types rem | |
| It must not import routing, combos, OAuth, adapters, sidecars, response parsing, logging, or relay | ||
| modules merely because those imports existed in the pre-split `responses.ts` monolith. | ||
|
|
||
| `OCX_FRESH_CONNECTION_HOSTS` accepts comma-separated hostnames whose outbound HTTP sends bypass | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
AGENTS.md reference: src/AGENTS.md:L29-L29 Useful? React with 👍 / 👎. |
||
| keep-alive reuse with `Connection: close` and `keepalive: false`; exact hosts and their subdomains | ||
| match case-insensitively. The helper applies this policy at the final executor boundary, after a | ||
| dispatch override has selected or rebuilt the destination, so matching follows the URL sent on the | ||
| wire rather than the URL supplied before credential revalidation. | ||
|
|
||
| ### Semantic progress ownership | ||
|
|
||
| The Responses proxy does not treat transcript growth as repository progress. It can observe request | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,248 @@ | ||
| import { describe, expect, test } from "bun:test"; | ||
| import { wantsFreshConnection, providerFetch } from "../../src/server/responses/fetch-helpers"; | ||
| import type { OcxProviderConfig } from "../../src/types"; | ||
|
|
||
| describe("wantsFreshConnection", () => { | ||
| test("returns false when env is unset or empty", () => { | ||
| expect(wantsFreshConnection("https://opencode.ai/zen/v1", undefined)).toBe(false); | ||
| expect(wantsFreshConnection("https://opencode.ai/zen/v1", "")).toBe(false); | ||
| expect(wantsFreshConnection("https://opencode.ai/zen/v1", " ")).toBe(false); | ||
| }); | ||
|
|
||
| test("matches exact hostname and subdomain suffixes case-insensitively, trimming leading dots", () => { | ||
| const env = "opencode.ai, .API.Cloudflare.Com"; | ||
| expect(wantsFreshConnection("https://opencode.ai/zen/v1", env)).toBe(true); | ||
| expect(wantsFreshConnection("https://api.opencode.ai/zen/v1", env)).toBe(true); | ||
| expect(wantsFreshConnection("https://api.cloudflare.com/v1", env)).toBe(true); | ||
| expect(wantsFreshConnection("https://other-cloudflare.com/v1", env)).toBe(false); | ||
| expect(wantsFreshConnection("https://notopencode.ai/v1", env)).toBe(false); | ||
| }); | ||
|
|
||
| test("returns false for unparseable URLs without throwing", () => { | ||
| expect(wantsFreshConnection("::not-a-url::", "opencode.ai")).toBe(false); | ||
| }); | ||
| }); | ||
|
|
||
| describe("providerFetch fresh connection dispatch", () => { | ||
| test("injects Connection: close and keepalive: false when host matches env", async () => { | ||
| const previous = process.env.OCX_FRESH_CONNECTION_HOSTS; | ||
| process.env.OCX_FRESH_CONNECTION_HOSTS = "special-relay.test"; | ||
| let observedInit: RequestInit | undefined; | ||
|
|
||
| const dummyProvider: OcxProviderConfig = { | ||
| adapter: "openai-responses", | ||
| baseUrl: "https://special-relay.test/v1", | ||
| fetch: (async (_input, init) => { | ||
| observedInit = init; | ||
| return new Response("ok", { status: 200 }); | ||
| }) as typeof globalThis.fetch, | ||
| }; | ||
|
|
||
| try { | ||
| const fetcher = providerFetch(dummyProvider); | ||
| await fetcher("https://special-relay.test/v1/responses", { method: "POST" }); | ||
|
|
||
| expect(observedInit).toBeDefined(); | ||
| expect((observedInit as any)?.keepalive).toBe(false); | ||
| const headers = new Headers(observedInit?.headers); | ||
| expect(headers.get("Connection")).toBe("close"); | ||
| } finally { | ||
| if (previous === undefined) delete process.env.OCX_FRESH_CONNECTION_HOSTS; | ||
| else process.env.OCX_FRESH_CONNECTION_HOSTS = previous; | ||
| } | ||
| }); | ||
|
|
||
| test("overwrites caller Connection: keep-alive header when fresh connection is enforced", async () => { | ||
| const previous = process.env.OCX_FRESH_CONNECTION_HOSTS; | ||
| process.env.OCX_FRESH_CONNECTION_HOSTS = "special-relay.test"; | ||
| let observedInit: RequestInit | undefined; | ||
|
|
||
| const dummyProvider: OcxProviderConfig = { | ||
| adapter: "openai-responses", | ||
| baseUrl: "https://special-relay.test/v1", | ||
| fetch: (async (_input, init) => { | ||
| observedInit = init; | ||
| return new Response("ok", { status: 200 }); | ||
| }) as typeof globalThis.fetch, | ||
| }; | ||
|
|
||
| try { | ||
| const fetcher = providerFetch(dummyProvider); | ||
| await fetcher("https://special-relay.test/v1/responses", { | ||
| method: "POST", | ||
| headers: { Connection: "keep-alive" }, | ||
| }); | ||
|
|
||
| expect((observedInit as any)?.keepalive).toBe(false); | ||
| const headers = new Headers(observedInit?.headers); | ||
| expect(headers.get("Connection")).toBe("close"); | ||
| } finally { | ||
| if (previous === undefined) delete process.env.OCX_FRESH_CONNECTION_HOSTS; | ||
| else process.env.OCX_FRESH_CONNECTION_HOSTS = previous; | ||
| } | ||
| }); | ||
|
|
||
| test("accepts a Request object as input and applies fresh connection settings", async () => { | ||
| const previous = process.env.OCX_FRESH_CONNECTION_HOSTS; | ||
| process.env.OCX_FRESH_CONNECTION_HOSTS = ".special-relay.test"; | ||
| let observedInit: RequestInit | undefined; | ||
|
|
||
| const dummyProvider: OcxProviderConfig = { | ||
| adapter: "openai-responses", | ||
| baseUrl: "https://sub.special-relay.test/v1", | ||
| fetch: (async (_input, init) => { | ||
| observedInit = init; | ||
| return new Response("ok", { status: 200 }); | ||
| }) as typeof globalThis.fetch, | ||
| }; | ||
|
|
||
| try { | ||
| const fetcher = providerFetch(dummyProvider); | ||
| const req = new Request("https://sub.special-relay.test/v1/responses", { | ||
| method: "POST", | ||
| headers: { "x-custom": "value" }, | ||
| }); | ||
| await fetcher(req); | ||
|
|
||
| expect((observedInit as any)?.keepalive).toBe(false); | ||
| const headers = new Headers(observedInit?.headers); | ||
| expect(headers.get("Connection")).toBe("close"); | ||
| expect(headers.get("x-custom")).toBe("value"); | ||
| } finally { | ||
| if (previous === undefined) delete process.env.OCX_FRESH_CONNECTION_HOSTS; | ||
| else process.env.OCX_FRESH_CONNECTION_HOSTS = previous; | ||
| } | ||
| }); | ||
|
|
||
| test("preserves default keepalive and headers when host is not configured", async () => { | ||
| const previous = process.env.OCX_FRESH_CONNECTION_HOSTS; | ||
| delete process.env.OCX_FRESH_CONNECTION_HOSTS; | ||
| let observedInit: RequestInit | undefined; | ||
|
|
||
| const dummyProvider: OcxProviderConfig = { | ||
| adapter: "openai-responses", | ||
| baseUrl: "https://normal-relay.test/v1", | ||
| fetch: (async (_input, init) => { | ||
| observedInit = init; | ||
| return new Response("ok", { status: 200 }); | ||
| }) as typeof globalThis.fetch, | ||
| }; | ||
|
|
||
| try { | ||
| const fetcher = providerFetch(dummyProvider); | ||
| await fetcher("https://normal-relay.test/v1/responses", { method: "POST" }); | ||
|
|
||
| expect(observedInit).toBeDefined(); | ||
| expect((observedInit as any)?.keepalive).toBeUndefined(); | ||
| const headers = new Headers(observedInit?.headers); | ||
| expect(headers.has("Connection")).toBe(false); | ||
| } finally { | ||
| if (previous !== undefined) process.env.OCX_FRESH_CONNECTION_HOSTS = previous; | ||
| } | ||
| }); | ||
|
|
||
| test("matches the destination introduced by a dispatch override", async () => { | ||
| const previous = process.env.OCX_FRESH_CONNECTION_HOSTS; | ||
| process.env.OCX_FRESH_CONNECTION_HOSTS = "special-relay.test"; | ||
| let observedInput: Parameters<typeof globalThis.fetch>[0] | undefined; | ||
| let observedInit: RequestInit | undefined; | ||
|
|
||
| const dummyProvider: OcxProviderConfig = { | ||
| adapter: "openai-responses", | ||
| baseUrl: "https://normal-relay.test/v1", | ||
| fetch: (async (input, init) => { | ||
| observedInput = input; | ||
| observedInit = init; | ||
| return new Response("ok", { status: 200 }); | ||
| }) as typeof globalThis.fetch, | ||
| }; | ||
|
|
||
| try { | ||
| const fetcher = providerFetch(dummyProvider, undefined, { | ||
| dispatchOverride: (_input, init, execute) => | ||
| execute("https://special-relay.test/v1/responses", init), | ||
| }); | ||
| await fetcher("https://normal-relay.test/v1/responses", { method: "POST" }); | ||
|
|
||
| expect(observedInput).toBe("https://special-relay.test/v1/responses"); | ||
| expect((observedInit as any)?.keepalive).toBe(false); | ||
| const headers = new Headers(observedInit?.headers); | ||
| expect(headers.get("Connection")).toBe("close"); | ||
| } finally { | ||
| if (previous === undefined) delete process.env.OCX_FRESH_CONNECTION_HOSTS; | ||
| else process.env.OCX_FRESH_CONNECTION_HOSTS = previous; | ||
| } | ||
| }); | ||
|
|
||
| test("does not match a destination removed by a dispatch override", async () => { | ||
| const previous = process.env.OCX_FRESH_CONNECTION_HOSTS; | ||
| process.env.OCX_FRESH_CONNECTION_HOSTS = "special-relay.test"; | ||
| let observedInput: Parameters<typeof globalThis.fetch>[0] | undefined; | ||
| let observedInit: RequestInit | undefined; | ||
|
|
||
| const dummyProvider: OcxProviderConfig = { | ||
| adapter: "openai-responses", | ||
| baseUrl: "https://special-relay.test/v1", | ||
| fetch: (async (input, init) => { | ||
| observedInput = input; | ||
| observedInit = init; | ||
| return new Response("ok", { status: 200 }); | ||
| }) as typeof globalThis.fetch, | ||
| }; | ||
|
|
||
| try { | ||
| const fetcher = providerFetch(dummyProvider, undefined, { | ||
| dispatchOverride: (_input, init, execute) => | ||
| execute("https://normal-relay.test/v1/responses", init), | ||
| }); | ||
| await fetcher("https://special-relay.test/v1/responses", { method: "POST" }); | ||
|
|
||
| expect(observedInput).toBe("https://normal-relay.test/v1/responses"); | ||
| expect((observedInit as any)?.keepalive).toBeUndefined(); | ||
| const headers = new Headers(observedInit?.headers); | ||
| expect(headers.has("Connection")).toBe(false); | ||
| } finally { | ||
| if (previous === undefined) delete process.env.OCX_FRESH_CONNECTION_HOSTS; | ||
| else process.env.OCX_FRESH_CONNECTION_HOSTS = previous; | ||
| } | ||
| }); | ||
|
|
||
| test("a beforeDispatch hook cannot defeat the fresh-connection decision", async () => { | ||
| // The hook receives a copy it cannot send. Even if it could, `Connection` is decided | ||
| // inside the executor, which runs after the hook, so the policy wins either way. | ||
| const previous = process.env.OCX_FRESH_CONNECTION_HOSTS; | ||
| process.env.OCX_FRESH_CONNECTION_HOSTS = "special-relay.test"; | ||
| let observedInit: RequestInit | undefined; | ||
| let sawHeaders = false; | ||
|
|
||
| const dummyProvider: OcxProviderConfig = { | ||
| adapter: "openai-responses", | ||
| baseUrl: "https://special-relay.test/v1", | ||
| fetch: (async (_input, init) => { | ||
| observedInit = init; | ||
| return new Response("ok", { status: 200 }); | ||
| }) as typeof globalThis.fetch, | ||
| }; | ||
|
|
||
| try { | ||
| const fetcher = providerFetch(dummyProvider, undefined, { | ||
| beforeDispatch: headers => { | ||
| sawHeaders = headers.get("x-custom") === "value"; | ||
| headers.set("Connection", "keep-alive"); | ||
| }, | ||
| }); | ||
| await fetcher("https://special-relay.test/v1/responses", { | ||
| method: "POST", | ||
| headers: { "x-custom": "value" }, | ||
| }); | ||
|
|
||
| expect(sawHeaders).toBe(true); | ||
| const headers = new Headers(observedInit?.headers); | ||
| expect(headers.get("Connection")).toBe("close"); | ||
| expect(headers.get("x-custom")).toBe("value"); | ||
| } finally { | ||
| if (previous === undefined) delete process.env.OCX_FRESH_CONNECTION_HOSTS; | ||
| else process.env.OCX_FRESH_CONNECTION_HOSTS = previous; | ||
| } | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
Repository: lidge-jun/opencodex
Length of output: 20037
🏁 Script executed:
Repository: lidge-jun/opencodex
Length of output: 18417
Run the required validation checks before merge. This change spans
src/server/responses/fetch-helpers.ts:109-117andscripts/test-layout/layout.json:728. Runbun run test:changed,bun run typecheck, andbun run privacy:scan.The supplied guidance does not require
bun run prepushor a platform-specific test-layout probe.📍 Affects 2 files
src/server/responses/fetch-helpers.ts#L109-L117(this comment)scripts/test-layout/layout.json#L728-L728🤖 Prompt for AI Agents
Source: Coding guidelines