-
Notifications
You must be signed in to change notification settings - Fork 694
fix(codex): scope Responses Lite to explicit official clients #2278
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
e63671b
b8586e5
55975b5
3c94b47
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 |
|---|---|---|
| @@ -1,10 +1,13 @@ | ||
| package codex | ||
|
|
||
| import ( | ||
| "bytes" | ||
| "context" | ||
| "encoding/json" | ||
| "errors" | ||
| "fmt" | ||
| "io" | ||
| "mime" | ||
| "net/http" | ||
| "strings" | ||
| "sync" | ||
|
|
@@ -30,13 +33,20 @@ const ( | |
| ) | ||
|
|
||
| // OutboundTransformer implements transformer.Outbound for Codex proxy. | ||
| // It always talks to the Codex Responses upstream (SSE only) and adapts requests accordingly. | ||
| // It normally talks to the Codex Responses upstream over SSE and adapts requests accordingly. | ||
| // The official backend always streams SSE; compatible relays that return a | ||
| // completed JSON response are also supported for non-stream callers. | ||
| // | ||
| //nolint:containedctx // It is used as a transformer. | ||
| type OutboundTransformer struct { | ||
| tokens oauth.TokenGetter | ||
| transport string | ||
|
|
||
| // official reports whether the configured upstream is the official Codex | ||
| // backend (chatgpt.com). Official endpoints always stream SSE, so they keep | ||
| // the DoStream path; only compatible relays get the JSON passthrough path. | ||
| official bool | ||
|
|
||
| // reuse existing Responses outbound for payload building. | ||
| responsesOutbound *responses.OutboundTransformer | ||
|
|
||
|
|
@@ -62,6 +72,18 @@ type Params struct { | |
| Transport string | ||
| } | ||
|
|
||
| // isOfficialCodexBaseURL reports whether baseURL points at the official Codex | ||
| // backend. Everything else is treated as a compatible relay that may return a | ||
| // completed JSON response instead of SSE. | ||
| func isOfficialCodexBaseURL(baseURL string) bool { | ||
| return strings.Contains(strings.ToLower(baseURL), "chatgpt.com") | ||
| } | ||
|
|
||
| // isOfficialCodex reports whether the transformer targets the official Codex backend. | ||
| func (t *OutboundTransformer) isOfficialCodex() bool { | ||
| return t != nil && t.official | ||
| } | ||
|
|
||
| func NewOutboundTransformer(params Params) (*OutboundTransformer, error) { | ||
| if params.TokenProvider == nil { | ||
| return nil, errors.New("token provider is required") | ||
|
|
@@ -87,6 +109,7 @@ func NewOutboundTransformer(params Params) (*OutboundTransformer, error) { | |
| return &OutboundTransformer{ | ||
| tokens: params.TokenProvider, | ||
| transport: params.Transport, | ||
| official: isOfficialCodexBaseURL(baseURL), | ||
| responsesOutbound: ro, | ||
| }, nil | ||
| } | ||
|
|
@@ -145,13 +168,12 @@ func (t *OutboundTransformer) TransformRequest(ctx context.Context, llmReq *llm. | |
| rawUserAgent = llmReq.RawRequest.Headers.Get("User-Agent") | ||
| rawTurnMetadata = llmReq.RawRequest.Headers.Get(TurnMetadataHeader) | ||
|
|
||
| // Non-Codex inbound clients omit the Responses Lite signal. Fabricate it | ||
| // so the Codex upstream sees the same protocol shape as a real Codex | ||
| // client. This must be set on the raw request before the underlying | ||
| // Responses outbound runs: it reads this header to emit an explicit | ||
| // parallel_tool_calls=false body, matching what real Codex sends. | ||
| if strings.TrimSpace(rawHeaders.Get(ResponsesLiteHeader)) == "" { | ||
| rawHeaders.Set(ResponsesLiteHeader, "true") | ||
| // Responses Lite selects a private Codex protocol mode. It is not | ||
| // identity metadata: never fabricate it for OpenAI-compatible clients. | ||
| // A client-selected Lite request is retained only for the official Codex | ||
| // backend; relays are not assumed to implement the same private protocol. | ||
| if !t.isOfficialCodex() || !strings.EqualFold(strings.TrimSpace(rawHeaders.Get(ResponsesLiteHeader)), "true") { | ||
| rawHeaders.Del(ResponsesLiteHeader) | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -204,21 +226,6 @@ func (t *OutboundTransformer) TransformRequest(ctx context.Context, llmReq *llm. | |
| reqCopy.ReasoningSummary = lo.ToPtr("auto") | ||
| } | ||
|
|
||
| // Responses Lite (signaled on the raw request above) rejects requests | ||
| // whose reasoning context is not "all_turns"; clients that never sent a | ||
| // reasoning block would otherwise fail upstream with HTTP 400. Only fill | ||
| // in a missing context — never override what the client explicitly sent. | ||
| if providerExt := reqCopy.ProviderExtensions; providerExt == nil || providerExt.OpenAIResponses == nil || | ||
| providerExt.OpenAIResponses.Request == nil || providerExt.OpenAIResponses.Request.ReasoningContext == "" { | ||
| oaiExt := llm.EnsureOpenAIResponsesProviderExtensions(&reqCopy) | ||
| if oaiExt != nil { | ||
| if oaiExt.Request == nil { | ||
| oaiExt.Request = &llm.OpenAIResponsesRequestExtensions{ReasoningContext: "all_turns"} | ||
| } else { | ||
| oaiExt.Request.ReasoningContext = "all_turns" | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Codex Responses rejects token limit fields, so strip them out. | ||
|
|
@@ -426,6 +433,22 @@ func (e *codexExecutor) Do(ctx context.Context, request *httpclient.Request) (*h | |
| return e.inner.Do(ctx, request) | ||
| } | ||
|
|
||
| // The official Codex backend always streams SSE, so keep the historical | ||
| // DoStream path for it. Compatible relays may instead return a completed | ||
| // Responses JSON document; for those, execute once and dispatch on the | ||
| // response Content-Type. The request is never reissued, which could | ||
| // duplicate model execution and billing. | ||
| if e.transformer != nil && e.transformer.isOfficialCodex() { | ||
| return e.doStreamAndAggregate(ctx, request) | ||
| } | ||
|
|
||
| return e.doOnceAndDispatch(ctx, request) | ||
| } | ||
|
|
||
| // doStreamAndAggregate preserves the original Codex behavior: consume the | ||
| // upstream SSE stream and aggregate the events into a completed Responses | ||
| // JSON body. | ||
| func (e *codexExecutor) doStreamAndAggregate(ctx context.Context, request *httpclient.Request) (*httpclient.Response, error) { | ||
| stream, err := e.inner.DoStream(ctx, request) | ||
| if err != nil { | ||
| return nil, err | ||
|
|
@@ -436,7 +459,6 @@ func (e *codexExecutor) Do(ctx context.Context, request *httpclient.Request) (*h | |
| }() | ||
|
|
||
| var chunks []*httpclient.StreamEvent | ||
|
|
||
| for stream.Next() { | ||
| ev := stream.Current() | ||
| if ev == nil { | ||
|
|
@@ -472,6 +494,87 @@ func (e *codexExecutor) Do(ctx context.Context, request *httpclient.Request) (*h | |
| }, nil | ||
| } | ||
|
|
||
| // doOnceAndDispatch sends a single upstream request and inspects the completed | ||
| // body. Codex normally responds with SSE even for downstream non-stream | ||
| // requests, but compatible relays can return a completed Responses JSON | ||
| // document instead. JSON responses pass through unchanged; everything else is | ||
| // decoded as SSE and aggregated into a completed JSON response. | ||
| func (e *codexExecutor) doOnceAndDispatch(ctx context.Context, request *httpclient.Request) (*httpclient.Response, error) { | ||
| response, err := e.inner.Do(ctx, request) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| if response == nil { | ||
| return nil, errors.New("empty response") | ||
| } | ||
| if isJSONResponse(response) { | ||
| return response, nil | ||
| } | ||
|
|
||
| chunks, err := decodeSSEChunks(ctx, response.Body) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| if err := responses.TopLevelWebSocketError(chunks); err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| body, _, err := e.transformer.AggregateStreamChunks(ctx, request, chunks) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| return &httpclient.Response{ | ||
| StatusCode: http.StatusOK, | ||
| Headers: http.Header{ | ||
| "Content-Type": []string{"application/json"}, | ||
| }, | ||
| Body: body, | ||
| Request: request, | ||
| }, nil | ||
| } | ||
|
Comment on lines
+497
to
+535
Contributor
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. 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Description: Inspect httpclient Do implementations for Response.Request assignment.
set -euo pipefail
fd -t f -e go . llm/httpclient | while IFS= read -r file; do
rg -n -C 6 'func .*\bDo\(' "$file"
done
rg -n -C 3 'Request:\s' --glob 'llm/httpclient/*.go'Repository: looplj/axonhub Length of output: 152 🏁 Script executed: set -euo pipefail
printf '%s\n' '--- matching httpclient files ---'
fd -t f -e go . | rg '(^|/)httpclient/|outbound\.go$' | head -200
printf '%s\n' '--- Response type and Do implementations ---'
rg -n -C 8 'type Response struct|func .*Do\(' --glob '*.go' .
printf '%s\n' '--- Request field assignments ---'
rg -n -C 4 'Request\s*:' --glob '*.go' .Repository: looplj/axonhub Length of output: 50371 🏁 Script executed: set -euo pipefail
file=llm/httpclient/client.go
printf '%s\n' '--- httpclient response construction ---'
sed -n '240,300p' "$file"
printf '%s\n' '--- codex outbound implementation ---'
sed -n '400,545p' llm/transformer/openai/codex/outbound.go
printf '%s\n' '--- codex executor and transformer initialization ---'
rg -n -C 8 'type codexExecutor|transformer\s*:|codexExecutor\{|doOnceAndDispatch|e\.transformer' llm/transformer/openai/codex --glob '*.go'
printf '%s\n' '--- httpclient Do interface and implementations ---'
rg -n -C 6 'type .*Do|Do\(ctx context\.Context, request \*Request\)|Do\(ctx context\.Context,.*Request' llm --glob '*.go'Repository: looplj/axonhub Length of output: 33579 🏁 Script executed: set -euo pipefail
printf '%s\n' '--- response request consumers ---'
rg -n -C 10 'Response\.Request|httpResp\.Request|TransformResponse\(' llm/transformer/openai/codex llm/transformer/openai --glob '*.go'
printf '%s\n' '--- websocket Do response construction ---'
sed -n '88,145p' llm/transformer/openai/responses/websocket_executor.go
printf '%s\n' '--- codex transport and constructor paths ---'
rg -n -C 10 'Transport|NewOutbound|CustomizeExecutor|OutboundTransformer\{' llm/transformer/openai/codex --glob '*.go'
printf '%s\n' '--- non-test Do implementations returning httpclient.Response ---'
python3 - <<'PY'
from pathlib import Path
import re
for p in Path("llm").rglob("*.go"):
if p.name.endswith("_test.go"):
continue
s=p.read_text(errors="replace")
if re.search(r'func\s+\([^)]*\)\s+Do\s*\([^)]*\*httpclient\.Request', s):
if "httpclient.Response" in s:
print(p)
PYRepository: looplj/axonhub Length of output: 50371 🏁 Script executed: set -euo pipefail
printf '%s\n' '--- websocket response construction ---'
sed -n '88,140p' llm/transformer/openai/responses/websocket_executor.go
printf '%s\n' '--- codex constructor and transport selection ---'
rg -n -C 12 'func New|Transport|CustomizeExecutor|OutboundTransformer\{' llm/transformer/openai/codex --glob '*.go' | head -240
printf '%s\n' '--- relay test response setup ---'
sed -n '350,510p' llm/transformer/openai/codex/outbound_executor_test.go
printf '%s\n' '--- pipeline response-preserving wrappers ---'
rg -n -C 8 'func .*Do\(ctx context\.Context, request \*httpclient\.Request\)' llm/pipeline --glob '*.go' --glob '!*.gen.go' --glob '!*_test.go'Repository: looplj/axonhub Length of output: 34678 🏁 Script executed: set -euo pipefail
python3 - <<'PY'
from pathlib import Path
checks = {
"llm/httpclient/client.go": ("Request: request,",),
"llm/transformer/openai/responses/websocket_executor.go": ("Request: request,",),
"llm/transformer/openai/codex/outbound.go": (
"if e.transformer != nil && e.transformer.isOfficialCodex()",
"return e.doOnceAndDispatch(ctx, request)",
"e.transformer.AggregateStreamChunks(ctx, request, chunks)",
),
}
for name, snippets in checks.items():
text = Path(name).read_text()
print(name)
for snippet in snippets:
print(f" {'present' if snippet in text else 'MISSING'}: {snippet}")
PYRepository: looplj/axonhub Length of output: 526 Guard relay aggregation when 🤖 Prompt for AI Agents |
||
|
|
||
| func isJSONResponse(response *httpclient.Response) bool { | ||
| if response == nil { | ||
| return false | ||
| } | ||
|
|
||
| mediaType, _, err := mime.ParseMediaType(response.Headers.Get("Content-Type")) | ||
| if err != nil { | ||
| return false | ||
| } | ||
|
|
||
| mediaType = strings.ToLower(mediaType) | ||
|
|
||
| return mediaType == "application/json" || strings.HasSuffix(mediaType, "+json") | ||
| } | ||
|
|
||
| func decodeSSEChunks(ctx context.Context, body []byte) ([]*httpclient.StreamEvent, error) { | ||
| stream := httpclient.NewDefaultSSEDecoder(ctx, io.NopCloser(bytes.NewReader(body))) | ||
| defer func() { | ||
| _ = stream.Close() | ||
| }() | ||
|
|
||
| chunks := make([]*httpclient.StreamEvent, 0) | ||
| for stream.Next() { | ||
| ev := stream.Current() | ||
| if ev == nil { | ||
| continue | ||
| } | ||
|
|
||
| chunks = append(chunks, &httpclient.StreamEvent{ | ||
| Type: ev.Type, | ||
| LastEventID: ev.LastEventID, | ||
| Data: append([]byte(nil), ev.Data...), | ||
| }) | ||
| } | ||
| if err := stream.Err(); err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| return chunks, nil | ||
| } | ||
|
|
||
| func (e *codexExecutor) DoStream(ctx context.Context, request *httpclient.Request) (streams.Stream[*httpclient.StreamEvent], error) { | ||
| return e.inner.DoStream(ctx, request) | ||
| } | ||
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.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Match the official Codex host, not a substring.
isOfficialCodexBaseURLusesstrings.Containson the whole base URL. A relay URL such ashttps://chatgpt.com.relay.example/v1orhttps://proxy.example/chatgpt.com/v1is then classified as official. Two consequences follow for that relay: the executor takes the SSE-onlydoStreamAndAggregatepath, andTransformRequestforwards the privateX-OpenAI-Internal-Codex-Responses-Liteheader to a third-party host. Parse the URL and compare the hostname instead.🛠️ Proposed fix using hostname comparison
func isOfficialCodexBaseURL(baseURL string) bool { - return strings.Contains(strings.ToLower(baseURL), "chatgpt.com") + parsed, err := url.Parse(strings.TrimSpace(baseURL)) + if err != nil { + return false + } + + host := strings.ToLower(parsed.Hostname()) + + return host == "chatgpt.com" || strings.HasSuffix(host, ".chatgpt.com") }Add the
net/urlimport for this change.📝 Committable suggestion
🤖 Prompt for AI Agents