fix(ai): add explicit Responses relay session affinity - #3918
Conversation
cd9693a to
e9faf88
Compare
yazzang-homelab
left a comment
There was a problem hiding this comment.
Independent architect review.
This changes who receives session-routing headers, so I reviewed it as a data-egress boundary rather than a feature flag. It holds up.
The security predicate is strict and correctly built. isCanonicalOpenAIAffinityOrigin (openai-responses.ts:146-161):
url.origin === "https://api.openai.com" &&
url.username === "" && url.password === "" &&
(url.pathname === "" || url.pathname === "/" || url.pathname === "/v1") &&
url.search === "" && url.hash === ""Pinning origin rather than hostname is the load-bearing choice — it fixes scheme, host and port together, so http://api.openai.com, https://api.openai.com:8443, and https://api.openai.com.evil.test all fail. The explicit username/password checks are redundant against origin (a userinfo URL still yields the same origin) but they are the right kind of redundant: the classic https://api.openai.com@evil.test confusion is what this shape defends against, and stating it makes the intent unmissable. search/hash empty closes the remaining wiggle room. Parse failure returns false — fail-closed.
The gate denies by default in the direction that matters. shouldSendOpenAIResponsesSessionHeaders (169-186):
provider === "openai"+ canonical origin →true. Existing behaviour preserved, including with caching off, exactly as the doc comment says.provider === "openai"+ non-canonical base URL → requiressupportsResponsesSessionAffinity === trueandcacheRetention !== "none". So relabelling a relay as provideropenaidoes not smuggle the headers out.- Any other known provider → hard
falseviaisKnownProvider. This is the check I most wanted to find. Without it, widening affinity would eventually leak session ids to anthropic/google/azure transports as the provider list grows. - Unknown provider → needs a base URL, the explicit opt-in, and a non-canonical origin.
Opt-in rather than inferred is the right default for this; Fixes #3689 describes a routing need, and inferring it from "looks like a Responses endpoint" would have been the tempting shortcut.
One thing to tidy, non-blocking. This file now carries three near-identical URL predicates with three different strictnesses and three different parse-failure behaviours:
| predicate | check | on parse failure |
|---|---|---|
isCanonicalOpenAIAffinityOrigin |
full origin + userinfo + path + query + hash | false |
isDefaultOpenAIBaseUrl |
hostname + path | exact string equality |
isOpenAIHostBaseUrl |
hostname only | startsWith prefix match |
Today that is not a bug: isOpenAIHostBaseUrl gates only supportsDeveloperRole (line 749) and isDefaultOpenAIBaseUrl gates base-URL defaulting — capability decisions, not egress. But isOpenAIHostBaseUrl's startsWith fallback on an unparseable URL is exactly the kind of loose matcher that becomes a hole the day someone reaches for the nearest-looking helper on a security path. A one-line comment on isCanonicalOpenAIAffinityOrigin saying it is the predicate for egress decisions, and that the other two are capability-only, would cost nothing and prevent that.
Second, smaller: an unknown provider pointed at canonical https://api.openai.com with the opt-in set returns false (the !isCanonicalOpenAIAffinityOrigin term). That is defensible — canonical OpenAI should be reached through provider openai — but it is a silent no-op for a plausible misconfiguration. A debug log there would save someone an afternoon.
CHANGELOG entry is under ## [Unreleased] (packages/ai, line 6), and the file is intact at 245,120 bytes — worth stating explicitly today, since eleven open PRs currently have a one-byte changelog (#3942). Merges cleanly into current dev.
gajae.pr-review-verdict.v1 merge-approved sha256:e9faf8880ba90be118215a84ea234b4befd03ff2 reviewer:architect evidence:read of openai-responses.ts:135-195,573,736-756 at this head; merge-tree vs origin/dev clean
|
Closing during the emergency maintenance freeze. This PR is not in the retained critical or maintainer-owned set. Do not open a replacement PR unless a maintainer explicitly directs it. — |
bc21554 to
cb37653
Compare
Custom OpenAI Responses relays need stable session headers without widening implicit affinity to unrelated providers. Add an explicit capability, preserve existing body and transform contracts, and verify provider/runtime propagation. Fixes Yeachan-Heo#3689
The model-level false survival fixture re-registered the unknown "relay" provider with supportsResponsesSessionAffinity:true but no baseUrl. assertResponsesSessionAffinitySupported correctly rejects that shape for unknown providers, so the fixture now keeps the same custom relay base on the transport-only override while still proving model-level false wins over provider-level true.
cb37653 to
44fb042
Compare
Empty commit so the pull_request synchronize event reruns against 44fb042 after the runtime-provider fixture fix.
Summary
Verification
bun test packages/ai/test/openai-responses-cache-affinity.test.tsbun test packages/coding-agent/test/models-config-send-session-headers.test.ts packages/coding-agent/test/model-registry-runtime-provider.test.tsbun test packages/coding-agent/test/model-registry.test.ts --test-name-pattern 'Responses|affinity|provider override|supportsResponses'bun --cwd=packages/ai run check:typesbun --cwd=packages/coding-agent run check:typesbun run check:schemasgit diff --checkFixes #3689