Feat responses api - #4381
Closed
sw629241 wants to merge 7 commits into
Closed
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds WebSocket GET upgrade support for OpenAI Responses (/v1/responses), implements per-adaptor ConvertOpenAIResponsesRequest, introduces a WebSocket responses relay helper that captures provider HTTP output and emits OpenAI-compatible WS events, and updates auth/distributor/request validation to support WS-based Responses requests and model extraction. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller as Controller (relay.go)
participant WsHelper as WssResponsesHelper (relay/wss_responses.go)
participant Adaptor as Channel Adaptor
participant ProviderAPI as Provider API
participant Service as Quota Service
Client->>Controller: GET /v1/responses (Upgrade: websocket)
Controller->>Controller: upgrader.Upgrade() (subprotocol openai-beta.responses-v1)
Controller->>WsHelper: WssResponsesHelper(c, relayInfo)
WsHelper->>WsHelper: ws.ReadMessage() -> OpenAIResponsesRequest
WsHelper->>Adaptor: ConvertOpenAIResponsesRequest()
WsHelper->>WsHelper: sendWsResponseEvent(response.created)
WsHelper->>WsHelper: sendWsResponseEvent(response.in_progress)
WsHelper->>Adaptor: DoRequest()/DoResponse()
Adaptor->>ProviderAPI: HTTP Request
ProviderAPI-->>Adaptor: HTTP Response (body, usage)
WsHelper->>WsHelper: capture response (CaptureResponseWriter)
WsHelper->>WsHelper: parse -> OpenAITextResponse / build usageData
WsHelper->>Client: emit output events (output_item/content_part/done)
WsHelper->>Service: PostTextConsumeQuota(usageData)
WsHelper->>Client: send response.completed / close WS
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
controller/relay.go (2)
88-106:⚠️ Potential issue | 🔴 CriticalError handler doesn't cover the new WebSocket-Responses path.
After
upgrader.Upgradesucceeds forRelayFormatOpenAIResponses+ GET,c.Writeris hijacked. IfnewAPIErroris set thereafter (e.g., fromWssResponsesHelper,PreConsumeBilling, channel selection, or any retry-loop failure), this deferred handler falls through to thedefaultbranch and callsc.JSON(...)on the hijacked writer — which at best is a no-op that corrupts the WS frame stream and at worst panics. TheRelayFormatOpenAIRealtimebranch correctly routes tohelper.WssError(c, ws, ...). Please add a parallel case forRelayFormatOpenAIResponseswhenws != nil, e.g.:switch relayFormat { case types.RelayFormatOpenAIRealtime: helper.WssError(c, ws, newAPIError.ToOpenAIError()) + case types.RelayFormatOpenAIResponses: + if ws != nil { + helper.WssError(c, ws, newAPIError.ToOpenAIError()) + } else { + c.JSON(newAPIError.StatusCode, gin.H{ + "error": newAPIError.ToOpenAIError(), + }) + } case types.RelayFormatClaude:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/relay.go` around lines 88 - 106, The defer error handler must handle the hijacked-response WebSocket path: when relayFormat == types.RelayFormatOpenAIResponses and ws != nil, send the error over the WS instead of calling c.JSON on the hijacked writer. Update the defer block that checks newAPIError (referencing newAPIError, relayFormat, ws) to add a branch similar to the RelayFormatOpenAIRealtime case — e.g., call helper.WssError(c, ws, newAPIError.ToOpenAIError()) (or another WS-specific helper used for Responses) when relayFormat == types.RelayFormatOpenAIResponses and ws is non-nil; otherwise keep the existing Claude and default JSON behavior. Ensure newAPIError.SetMessage(...) and logger.LogError(...) are still executed before routing the response.
78-86:⚠️ Potential issue | 🔴 CriticalAdd subprotocol negotiation for the
/v1/responsesWebSocket endpoint.The package-level
upgraderat lines 250–255 only advertisesSubprotocols: []string{"realtime"}. The/v1/responsesendpoint shares this same upgrader, but it receives WebSocket clients sendingSec-WebSocket-Protocolheaders with responses-specific values likeopenai-beta.responses-v1andopenai-model.<model>(which the middleware at `middleware/distributor.go:352–353 extracts). Since these protocols are not in the upgrader's whitelist, Gorilla will fail to negotiate them, causing strict WebSocket clients to close the connection.The
adaptor.go:SetupRequestHeadermethod (lines 209–220) only handlesSec-WebSocket-Protocolnegotiation forRelayModeRealtime, not forRelayModeResponses. Add equivalent protocol negotiation for responses or use a request-scoped upgrader that includes the expected responses subprotocols.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/relay.go` around lines 78 - 86, The websocket upgrade for types.RelayFormatOpenAIResponses is using the package-level upgrader that only advertises "realtime", causing negotiation failures; fix by negotiating responses subprotocols either by creating a request-scoped upgrader in relay.go before calling upgrader.Upgrade (populate Subprotocols with the expected values like "openai-beta.responses-v1" and the extracted "openai-model.<model>" from middleware/distributor.go) or by enhancing adaptor.go:SetupRequestHeader to set the Sec-WebSocket-Protocol header for RelayModeResponses the same way it does for RelayModeRealtime, then call upgrader.Upgrade using that negotiated header; update the code paths referencing upgrader and the relayFormat check (types.RelayFormatOpenAIResponses) to use the new per-request negotiation so strict clients can connect.
🧹 Nitpick comments (3)
relay/helper/valid_request.go (1)
37-43: Consider usinghttp.MethodGetconstant.Minor stylistic nit:
c.Request.Method == "GET"elsewhere in this repo (e.g.,controller/relay.goLine 78 useshttp.MethodGet). A small consistency tweak:- if c.Request.Method == "GET" && c.GetHeader("Upgrade") == "websocket" { + if c.Request.Method == http.MethodGet && strings.EqualFold(c.GetHeader("Upgrade"), "websocket") {Note the
strings.EqualFold— RFC 7230 header tokens are case-insensitive and theUpgradevalue is commonlywebsocketbut some clients sendWebSocket. Downstreamcontroller.Relay(Line 78) only switches onhttp.MethodGetwithout inspecting the header, so mismatched casing here would skip this branch and then hand the request toGetAndValidateResponsesRequest, which would fail body parsing on a GET — returning 400 instead of upgrading.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@relay/helper/valid_request.go` around lines 37 - 43, Replace the literal "GET" check with the http.MethodGet constant and make the Upgrade header comparison case-insensitive (use strings.EqualFold) so the branch that constructs a dto.OpenAIResponsesRequest for websocket upgrades triggers regardless of header casing; update the condition that uses c.Request.Method and c.GetHeader("Upgrade") and ensure it still constructs the same dto.OpenAIResponsesRequest and falls back to GetAndValidateResponsesRequest(c) otherwise (this aligns behavior with controller.Relay).relay/wss_responses.go (1)
311-321: Consider propagatingWriteJSONerrors.Discarding the return values of
SetWriteDeadlineandWriteJSONmeans a closed/broken WS connection is invisible to the caller; the helper keeps sending subsequent events (and still bills quota) against a dead connection. Returning an error (or at least logging it) would letWssResponsesHelpershort-circuit the event sequence.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@relay/wss_responses.go` around lines 311 - 321, sendWsResponseEvent currently ignores errors from ws.SetWriteDeadline and ws.WriteJSON; change its signature to return an error and propagate any non-nil error from SetWriteDeadline or WriteJSON (wrap with context like "sendWsResponseEvent: ...") instead of discarding them, then update callers (notably WssResponsesHelper) to check that returned error and short-circuit/stop sending further events and handle/close the broken connection accordingly; ensure the call sites of sendWsResponseEvent handle and surface the error so quota/billing logic can stop when the websocket is closed.relay/channel/zhipu_4v/adaptor.go (1)
135-154: Non-text Responses input parts are silently dropped.The loop only keeps
input_textparts; anyinput_image/input_file/ tool-result parts inrequest.Inputdisappear without error. If GLM‑4V (a vision model) is reached via the Responses WS path, images will be stripped without the caller knowing. Either forward image parts asimage_urlMediaContent, or return an explicit error when unsupported parts are present.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@relay/channel/zhipu_4v/adaptor.go` around lines 135 - 154, The current loop in the request input handling only keeps "input_text" and silently drops other types (images/files/tool results), causing non-text media to be lost; update the logic in the block that calls request.ParseInput() and builds contentParts/dto.MediaContent so that: 1) for image-like inputs (e.g., "input_image") create a dto.MediaContent with a suitable Type (e.g., "image_url" or "image") and populate the URL/metadata instead of dropping it; 2) for file or other unsupported types either convert them into an appropriate dto.MediaContent (e.g., "file_url" or similar) or return an explicit error indicating unsupported input types; and 3) ensure oaiReq.Messages is populated using msg.SetMediaContent(contentParts) when multiple media parts exist (same as current branch) so non-text parts are forwarded rather than discarded (refer to request.ParseInput(), dto.MediaContent, msg.SetMediaContent, and oaiReq.Messages to locate where to change behavior).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@middleware/auth.go`:
- Around line 314-319: The middleware currently reads credentials from the query
parameter (`c.Query("api_key")`) for requests to "/v1/responses", which leaks
secrets via logs; change the logic in the middleware handling (the block using
strings.HasPrefix(c.Request.URL.Path, "/v1/responses")) to stop accepting
api_key from the URL by default, instead accept the API key from a header (reuse
the existing Sec-WebSocket-Protocol handling or a dedicated header like
"X-OpenAI-Insecure-Api-Key") and set Authorization from that header, and if you
need to keep query-string support make it conditional on a new config flag
(e.g., AllowInsecureApiKeyInQuery) so the query form is disabled in production;
update any related docs/comments to indicate the query parameter is
insecure/testing-only.
In `@middleware/distributor.go`:
- Around line 343-368: Remove the commented-out debug line and stop silently
forcing a hard-coded model; in the /v1/responses branch that inspects
Sec-WebSocket-Protocol, change the fallback heuristic loop (the loop that sets
modelRequest.Model when part contains "-" and lacks "openai-"/"realtime") so it
only assigns the first viable candidate and immediately break after assignment
(do not let later parts overwrite it). Remove the unconditional fallback that
sets modelRequest.Model = "gpt-5.3-codex"; instead either abort the request with
a 400 (e.g., via c.AbortWithStatusJSON) so the existing empty-model validation
handles it, or gate a default-model fallback behind a config flag and make the
action emit a loud logger.LogError/logger.LogInfo with the chosen default.
In `@relay/channel/claude/adaptor.go`:
- Line 4: Replace direct use of encoding/json and json.Unmarshal in
relay/channel/claude/adaptor.go with the repo wrappers (use common.Unmarshal or
common.UnmarshalJsonStr as appropriate) to centralize JSON handling (replace the
import and the json.Unmarshal call around the code that parses Responses at the
json.Unmarshal site); additionally update the request-parts loop that only
handles input_text (the loop handling input_text at Lines ~144–148) to either
map input_image/input_file to the same image_url field as the Gemini adaptor
does (see relay/channel/gemini/adaptor.go mapping of input_image → image_url) or
return a clear "unsupported input type" error so non-text parts are not silently
dropped. Ensure you reference the parsing function/variable names used in this
file when making the changes so the correct unmarshalling and part-mapping logic
is updated.
In `@relay/channel/gemini/adaptor.go`:
- Line 4: The file imports encoding/json and calls json.Unmarshal (around the
code handling Gemini responses near the json.Unmarshal call at Line ~266);
replace that usage with the repo wrapper common.Unmarshal and remove the
encoding/json import if it becomes unused: locate the json.Unmarshal invocation
in adaptor.go (the Gemini adaptor parsing path), change it to call
common.Unmarshal with the same target variable, handle the returned error the
same way, and drop the encoding/json import from the import block so all JSON
operations use common.Unmarshal per the repo guideline.
In `@relay/channel/zhipu_4v/adaptor.go`:
- Line 4: The code directly imports "encoding/json" and calls json.Unmarshal
(seen in the adaptor.go import and the json.Unmarshal usage around lines
125-133); replace those direct uses with the project's JSON wrappers: remove the
"encoding/json" import and call common.Unmarshal (or common.UnmarshalJsonStr if
you convert bytes to string first) wherever json.Unmarshal is used in this file
(e.g., the adaptor.go function that currently calls json.Unmarshal), and adjust
error handling to match the common package's return types.
- Around line 47-49: GetRequestURL currently unconditionally sets info.IsStream
= false which disables streaming for all relay modes; change the code to only
set info.IsStream = false when the request is using the Responses path (i.e.,
when info.Mode or the equivalent flag indicates Responses/Responses API),
following the same gating pattern as used in relay/channel/openai/adaptor.go;
update the logic inside Adaptor.GetRequestURL to check the relay mode first and
only override info.IsStream for the Responses case so other modes keep their
original streaming setting.
In `@relay/channel/zhipu/adaptor.go`:
- Line 4: Replace direct use of encoding/json with the repo JSON wrapper: remove
the "encoding/json" import and replace calls to json.Unmarshal (and any other
direct json.* calls in adaptor.go, e.g., the json.Unmarshal usage around line
103) with common.Unmarshal; update the import to include the common package and
pass the same byte slice or string and target struct to common.Unmarshal so the
file uses common.Unmarshal (and other common.* wrappers if needed) instead of
encoding/json.
- Around line 46-51: GetRequestURL and DoResponse are unconditionally forcing
info.IsStream = false which breaks streaming for Zhipu channels; change both
places to only set info.IsStream = false when info.RelayMode ==
constant.RelayModeResponses (mirror Gemini adaptor logic), import
relay/constant, and restore the conditional dispatch in DoResponse to call
zhipuStreamHandler when info.IsStream is true and zhipuHandler when false; apply
the same fix/audit to relay/channel/zhipu_4v/adaptor.go to ensure streaming
remains enabled for non-Responses modes.
In `@relay/wss_responses.go`:
- Line 5: Replace direct calls to json.Unmarshal in this file with the
repository wrapper: change both occurrences of json.Unmarshal (the two spots
flagged around lines where json.Unmarshal is used) to common.Unmarshal and
remove the "encoding/json" import from the import block; ensure error handling
stays the same and that you import the package that exposes the wrapper (common)
if not already imported so functions like common.Unmarshal are available.
- Around line 154-173: The context swaps c.Writer to a CaptureResponseWriter and
sets c.Request.Method to "POST" before calling adaptor.DoResponse, but
restoration only occurs after that call and will be skipped on panic; after
assigning capture and originalMethod/originalWriter, immediately register defer
statements to restore c.Writer = originalWriter and c.Request.Method =
originalMethod so they always run (use the existing symbols
CaptureResponseWriter, capture, originalWriter, originalMethod and ensure the
defer is placed before calling adaptor.DoResponse).
- Line 203: Remove the debug printf that prints token counts: delete the call to
fmt.Printf("[DEBUG] usageData: %+v\n", usageData) (or replace it with the
project's structured logger at debug level) so usageData is no longer written to
stdout on every request; if you need the info keep it behind the project logger
(e.g., logger.Debug/Debugf) and avoid printing raw token counts.
- Around line 175-299: The code currently only emits terminal websocket events
inside the successful capture.Body -> json.Unmarshal -> len(Choices)>0 branch,
so paths like newAPIError != nil, capture.Body.Len()==0, unmarshal failures, or
empty choices never send a terminal event; modify the logic around the response
handling (referencing newAPIError, capture.Body, dto.OpenAITextResponse,
chatResp, sendWsResponseEvent, responseID, responsesReq.Model, usageData) so
that every exit path emits an appropriate terminal event (response.completed on
success, and response.failed or response.error with a descriptive error payload
on failures/incomplete cases), i.e., before returning for the newAPIError case
and after any failed parsing/empty-body/empty-choices branch build and send a
terminal event (including error details when available) using
sendWsResponseEvent with the correct sequence and response fields
(status/completed_at/usage/error) to ensure the client is always closed out.
- Around line 143-152: The type assertion resp.(*http.Response) in the
adaptor.DoRequest handling can panic if an adaptor returns a different concrete
type; update the code around adaptor.DoRequest to use the comma-ok form (e.g.,
httpResp, ok := resp.(*http.Response)) and if ok is false return a proper API
error (use types.NewAPIError or the project equivalent) that indicates a
misconfigured adaptor rather than panicking; keep the existing defer
service.CloseResponseBodyGracefully(httpResp) and the subsequent call to
service.RelayErrorHandler(c.Request.Context(), httpResp, false) only in the
successful-ok branch.
---
Outside diff comments:
In `@controller/relay.go`:
- Around line 88-106: The defer error handler must handle the hijacked-response
WebSocket path: when relayFormat == types.RelayFormatOpenAIResponses and ws !=
nil, send the error over the WS instead of calling c.JSON on the hijacked
writer. Update the defer block that checks newAPIError (referencing newAPIError,
relayFormat, ws) to add a branch similar to the RelayFormatOpenAIRealtime case —
e.g., call helper.WssError(c, ws, newAPIError.ToOpenAIError()) (or another
WS-specific helper used for Responses) when relayFormat ==
types.RelayFormatOpenAIResponses and ws is non-nil; otherwise keep the existing
Claude and default JSON behavior. Ensure newAPIError.SetMessage(...) and
logger.LogError(...) are still executed before routing the response.
- Around line 78-86: The websocket upgrade for types.RelayFormatOpenAIResponses
is using the package-level upgrader that only advertises "realtime", causing
negotiation failures; fix by negotiating responses subprotocols either by
creating a request-scoped upgrader in relay.go before calling upgrader.Upgrade
(populate Subprotocols with the expected values like "openai-beta.responses-v1"
and the extracted "openai-model.<model>" from middleware/distributor.go) or by
enhancing adaptor.go:SetupRequestHeader to set the Sec-WebSocket-Protocol header
for RelayModeResponses the same way it does for RelayModeRealtime, then call
upgrader.Upgrade using that negotiated header; update the code paths referencing
upgrader and the relayFormat check (types.RelayFormatOpenAIResponses) to use the
new per-request negotiation so strict clients can connect.
---
Nitpick comments:
In `@relay/channel/zhipu_4v/adaptor.go`:
- Around line 135-154: The current loop in the request input handling only keeps
"input_text" and silently drops other types (images/files/tool results), causing
non-text media to be lost; update the logic in the block that calls
request.ParseInput() and builds contentParts/dto.MediaContent so that: 1) for
image-like inputs (e.g., "input_image") create a dto.MediaContent with a
suitable Type (e.g., "image_url" or "image") and populate the URL/metadata
instead of dropping it; 2) for file or other unsupported types either convert
them into an appropriate dto.MediaContent (e.g., "file_url" or similar) or
return an explicit error indicating unsupported input types; and 3) ensure
oaiReq.Messages is populated using msg.SetMediaContent(contentParts) when
multiple media parts exist (same as current branch) so non-text parts are
forwarded rather than discarded (refer to request.ParseInput(),
dto.MediaContent, msg.SetMediaContent, and oaiReq.Messages to locate where to
change behavior).
In `@relay/helper/valid_request.go`:
- Around line 37-43: Replace the literal "GET" check with the http.MethodGet
constant and make the Upgrade header comparison case-insensitive (use
strings.EqualFold) so the branch that constructs a dto.OpenAIResponsesRequest
for websocket upgrades triggers regardless of header casing; update the
condition that uses c.Request.Method and c.GetHeader("Upgrade") and ensure it
still constructs the same dto.OpenAIResponsesRequest and falls back to
GetAndValidateResponsesRequest(c) otherwise (this aligns behavior with
controller.Relay).
In `@relay/wss_responses.go`:
- Around line 311-321: sendWsResponseEvent currently ignores errors from
ws.SetWriteDeadline and ws.WriteJSON; change its signature to return an error
and propagate any non-nil error from SetWriteDeadline or WriteJSON (wrap with
context like "sendWsResponseEvent: ...") instead of discarding them, then update
callers (notably WssResponsesHelper) to check that returned error and
short-circuit/stop sending further events and handle/close the broken connection
accordingly; ensure the call sites of sendWsResponseEvent handle and surface the
error so quota/billing logic can stop when the websocket is closed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d92e3949-48c7-4456-bcd8-6e59861cb9ff
📒 Files selected for processing (10)
controller/relay.gomiddleware/auth.gomiddleware/distributor.gorelay/channel/claude/adaptor.gorelay/channel/gemini/adaptor.gorelay/channel/zhipu/adaptor.gorelay/channel/zhipu_4v/adaptor.gorelay/helper/valid_request.gorelay/wss_responses.gorouter/relay-router.go
Comment on lines
+314
to
+319
| if strings.HasPrefix(c.Request.URL.Path, "/v1/responses") { | ||
| apiKey := c.Query("api_key") | ||
| if apiKey != "" { | ||
| c.Request.Header.Set("Authorization", "Bearer "+apiKey) | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
API key in query string leaks via logs.
Accepting api_key as a query-string parameter causes the credential to be written to web server access logs, reverse-proxy logs, browser history, and Referer headers on outbound navigation. WebSocket clients already have a header-based path via Sec-WebSocket-Protocol (see Lines 279–292) — prefer extending that (e.g., accept openai-insecure-api-key.sk-xxx for /v1/responses as well) and either gate the query-string form behind a config flag or document that it is for insecure/testing use only.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@middleware/auth.go` around lines 314 - 319, The middleware currently reads
credentials from the query parameter (`c.Query("api_key")`) for requests to
"/v1/responses", which leaks secrets via logs; change the logic in the
middleware handling (the block using strings.HasPrefix(c.Request.URL.Path,
"/v1/responses")) to stop accepting api_key from the URL by default, instead
accept the API key from a header (reuse the existing Sec-WebSocket-Protocol
handling or a dedicated header like "X-OpenAI-Insecure-Api-Key") and set
Authorization from that header, and if you need to keep query-string support
make it conditional on a new config flag (e.g., AllowInsecureApiKeyInQuery) so
the query form is disabled in production; update any related docs/comments to
indicate the query parameter is insecure/testing-only.
Comment on lines
+343
to
+368
| if strings.HasPrefix(c.Request.URL.Path, "/v1/responses") && modelRequest.Model == "" { | ||
| // logger.LogInfo(c, "DEBUG: headers: " + fmt.Sprintf("%v", c.Request.Header)) | ||
| modelRequest.Model = c.Query("model") | ||
| if modelRequest.Model == "" { | ||
| protocol := c.GetHeader("Sec-WebSocket-Protocol") | ||
| if protocol != "" { | ||
| parts := strings.Split(protocol, ",") | ||
| for _, part := range parts { | ||
| part = strings.TrimSpace(part) | ||
| if strings.HasPrefix(part, "openai-model.") { | ||
| modelRequest.Model = strings.TrimPrefix(part, "openai-model.") | ||
| break | ||
| } | ||
| // Fallback: If it's a common model name pattern but not prefixed | ||
| if !strings.HasPrefix(part, "openai-") && !strings.Contains(part, "realtime") && strings.Contains(part, "-") { | ||
| modelRequest.Model = part | ||
| } | ||
| } | ||
| } | ||
| } | ||
| // 終極保底：如果真的什麼都找不到，強制使用這個預設值以避免 400 錯誤 | ||
| if modelRequest.Model == "" { | ||
| modelRequest.Model = "gpt-5.3-codex" | ||
| logger.LogInfo(c, "WebSocket responses: model completely missing, fallback to gpt-5.3-codex") | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Silent hard-coded model fallback and stray debug artifact.
A few concerns in this /v1/responses block:
- Missing
breakin the heuristic branch (Lines 357–359). Theopenai-model.branch breaks on first match, but the fallback heuristic keeps iterating and overwritesmodelRequest.Modelwith the last matching part. If a client advertises multiple subprotocols this will silently land on whichever ordered last. Eitherbreakafter assignment or only take the first viable candidate. - Silent fallback to
gpt-5.3-codex(Lines 364–367). Force-setting an arbitrary model hides the fact that the request was malformed, disrupts billing/channel routing, and will 403 for any user whose group lacks that model. Prefer returning a 400 (letting the distributor’s existing empty-model check at Line 79 do its job) so the client learns to send a model. At minimum, gate this behind a config option and loudly log. - Commented-out debug log (Line 344). Please drop it rather than leaving it as a dead comment.
🔧 Suggested change
if strings.HasPrefix(c.Request.URL.Path, "/v1/responses") && modelRequest.Model == "" {
- // logger.LogInfo(c, "DEBUG: headers: " + fmt.Sprintf("%v", c.Request.Header))
modelRequest.Model = c.Query("model")
if modelRequest.Model == "" {
protocol := c.GetHeader("Sec-WebSocket-Protocol")
if protocol != "" {
parts := strings.Split(protocol, ",")
for _, part := range parts {
part = strings.TrimSpace(part)
if strings.HasPrefix(part, "openai-model.") {
modelRequest.Model = strings.TrimPrefix(part, "openai-model.")
break
}
- // Fallback: If it's a common model name pattern but not prefixed
if !strings.HasPrefix(part, "openai-") && !strings.Contains(part, "realtime") && strings.Contains(part, "-") {
modelRequest.Model = part
+ break
}
}
}
}
- // 終極保底：如果真的什麼都找不到，強制使用這個預設值以避免 400 錯誤
- if modelRequest.Model == "" {
- modelRequest.Model = "gpt-5.3-codex"
- logger.LogInfo(c, "WebSocket responses: model completely missing, fallback to gpt-5.3-codex")
- }
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@middleware/distributor.go` around lines 343 - 368, Remove the commented-out
debug line and stop silently forcing a hard-coded model; in the /v1/responses
branch that inspects Sec-WebSocket-Protocol, change the fallback heuristic loop
(the loop that sets modelRequest.Model when part contains "-" and lacks
"openai-"/"realtime") so it only assigns the first viable candidate and
immediately break after assignment (do not let later parts overwrite it). Remove
the unconditional fallback that sets modelRequest.Model = "gpt-5.3-codex";
instead either abort the request with a 400 (e.g., via c.AbortWithStatusJSON) so
the existing empty-model validation handles it, or gate a default-model fallback
behind a config flag and make the action emit a loud
logger.LogError/logger.LogInfo with the chosen default.
| package claude | ||
|
|
||
| import ( | ||
| "encoding/json" |
Contributor
There was a problem hiding this comment.
Use common.Unmarshal instead of encoding/json, and drop image/file inputs are silently ignored.
Two items on this conversion:
-
Guideline violation — direct
encoding/jsonuse. The newencoding/jsonimport (Line 4) andjson.Unmarshalcall (Line 132) bypass the repo-mandated wrappers. Please swap tocommon.Unmarshal(orcommon.UnmarshalJsonStrover the raw bytes) so JSON handling stays centralized. -
Silent drop of non-text inputs. The loop at Lines 144–148 only handles
input_text, so anyinput_image/input_fileparts in a Responses request are discarded without error. Claude's Messages API does support images, and the sibling Gemini adaptor (relay/channel/gemini/adaptor.goLines 279–287) already mapsinput_image→image_url. Consider either mirroring Gemini's handling or returning an explicit "unsupported input type" error so clients aren't confused by a silently truncated prompt.
🔧 Suggested fix for the JSON wrapper usage
-import (
- "encoding/json"
+import (
"errors"
"fmt"
"io"
"net/http"
"net/url"
"github.com/QuantumNous/new-api/dto"
+ "github.com/QuantumNous/new-api/common"
...
- if err := json.Unmarshal(request.Instructions, &instrStr); err == nil && instrStr != "" {
+ if err := common.Unmarshal(request.Instructions, &instrStr); err == nil && instrStr != "" {As per coding guidelines: "All JSON marshal/unmarshal operations MUST use wrapper functions in common/json.go (common.Marshal, common.Unmarshal, common.UnmarshalJsonStr, common.DecodeJson, common.GetJsonType). Do NOT directly import or call encoding/json in business code."
Also applies to: 112-162
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@relay/channel/claude/adaptor.go` at line 4, Replace direct use of
encoding/json and json.Unmarshal in relay/channel/claude/adaptor.go with the
repo wrappers (use common.Unmarshal or common.UnmarshalJsonStr as appropriate)
to centralize JSON handling (replace the import and the json.Unmarshal call
around the code that parses Responses at the json.Unmarshal site); additionally
update the request-parts loop that only handles input_text (the loop handling
input_text at Lines ~144–148) to either map input_image/input_file to the same
image_url field as the Gemini adaptor does (see relay/channel/gemini/adaptor.go
mapping of input_image → image_url) or return a clear "unsupported input type"
error so non-text parts are not silently dropped. Ensure you reference the
parsing function/variable names used in this file when making the changes so the
correct unmarshalling and part-mapping logic is updated.
| package gemini | ||
|
|
||
| import ( | ||
| "encoding/json" |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use common.Unmarshal instead of encoding/json.
The encoding/json import (Line 4) and json.Unmarshal on Line 266 bypass the repo-mandated JSON wrappers. Please switch to common.Unmarshal and drop the encoding/json import if it becomes unused.
-import (
- "encoding/json"
...
- if err := json.Unmarshal(request.Instructions, &instrStr); err == nil && instrStr != "" {
+ if err := common.Unmarshal(request.Instructions, &instrStr); err == nil && instrStr != "" {As per coding guidelines: "All JSON marshal/unmarshal operations MUST use wrapper functions in common/json.go (common.Marshal, common.Unmarshal, common.UnmarshalJsonStr, common.DecodeJson, common.GetJsonType). Do NOT directly import or call encoding/json in business code."
Also applies to: 264-272
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@relay/channel/gemini/adaptor.go` at line 4, The file imports encoding/json
and calls json.Unmarshal (around the code handling Gemini responses near the
json.Unmarshal call at Line ~266); replace that usage with the repo wrapper
common.Unmarshal and remove the encoding/json import if it becomes unused:
locate the json.Unmarshal invocation in adaptor.go (the Gemini adaptor parsing
path), change it to call common.Unmarshal with the same target variable, handle
the returned error the same way, and drop the encoding/json import from the
import block so all JSON operations use common.Unmarshal per the repo guideline.
| package zhipu_4v | ||
|
|
||
| import ( | ||
| "encoding/json" |
Contributor
There was a problem hiding this comment.
Use the common JSON wrappers instead of encoding/json.
Direct import and use of encoding/json (line 4 import, line 127 json.Unmarshal) is disallowed in business code. Replace with common.Unmarshal (or common.UnmarshalJsonStr if you first coerce to string) and drop the import.
🛠️ Proposed fix
import (
- "encoding/json"
"errors"
"fmt"
"io"
"net/http"
@@
- if len(request.Instructions) > 0 {
- var instrStr string
- if err := json.Unmarshal(request.Instructions, &instrStr); err == nil && instrStr != "" {
+ if len(request.Instructions) > 0 {
+ var instrStr string
+ if err := common.Unmarshal(request.Instructions, &instrStr); err == nil && instrStr != "" {
oaiReq.Messages = append(oaiReq.Messages, dto.Message{
Role: "system",
Content: instrStr,
})
}
}As per coding guidelines: "All JSON marshal/unmarshal operations MUST use wrapper functions in common/json.go... Do NOT directly import or call encoding/json in business code."
Also applies to: 125-133
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@relay/channel/zhipu_4v/adaptor.go` at line 4, The code directly imports
"encoding/json" and calls json.Unmarshal (seen in the adaptor.go import and the
json.Unmarshal usage around lines 125-133); replace those direct uses with the
project's JSON wrappers: remove the "encoding/json" import and call
common.Unmarshal (or common.UnmarshalJsonStr if you convert bytes to string
first) wherever json.Unmarshal is used in this file (e.g., the adaptor.go
function that currently calls json.Unmarshal), and adjust error handling to
match the common package's return types.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (5)
relay/wss_responses.go (5)
207-207:⚠️ Potential issue | 🟡 MinorRemove the debug stdout print.
This writes unstructured usage data on every request. Drop it or move it behind the project logger at debug level.
🧹 Proposed fix
- fmt.Printf("[DEBUG] usageData: %+v\n", usageData) -🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@relay/wss_responses.go` at line 207, Remove the stray debug stdout print in wss_responses.go: delete the fmt.Printf("[DEBUG] usageData: %+v\n", usageData) line (or replace it with the project's structured logger at debug level if you need to keep the message), ensuring you use the existing logger API rather than fmt.Printf and keep the output behind debug logging.
5-5:⚠️ Potential issue | 🟠 MajorReplace direct
encoding/jsonusage with common wrappers.Use
common.Unmarshaland remove the directencoding/jsonimport.🛠️ Proposed fix
import ( "bytes" - "encoding/json" "fmt" @@ - if err := json.Unmarshal(message, &responsesReq); err != nil { + if err := common.Unmarshal(message, &responsesReq); err != nil { @@ - if err := json.Unmarshal(capture.Body.Bytes(), &chatResp); err == nil && len(chatResp.Choices) > 0 { + if err := common.Unmarshal(capture.Body.Bytes(), &chatResp); err == nil && len(chatResp.Choices) > 0 {As per coding guidelines: “All JSON marshal/unmarshal operations MUST use wrapper functions from
common/json.go... Do NOT directly import or callencoding/jsonin business code.”Also applies to: 79-79, 212-212
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@relay/wss_responses.go` at line 5, The file currently imports and uses encoding/json directly; replace all direct encoding/json usage (e.g., json.Unmarshal calls at the noted locations) with the project wrapper functions in common/json.go (use common.Unmarshal for decoding and common.Marshal if encoding is required), and remove the encoding/json import from relay/wss_responses.go; update every occurrence referenced (including the usages around the earlier-mentioned spots) to call common.Unmarshal with the same arguments and handle returned error/value the same way.
147-152:⚠️ Potential issue | 🟡 MinorGuard the response type assertion.
resp.(*http.Response)can panic if an adaptor returns an unexpected concrete response type; convert that into a relay error instead.🐛 Proposed fix
- httpResp := resp.(*http.Response) + httpResp, ok := resp.(*http.Response) + if !ok { + return types.NewError(fmt.Errorf("unexpected response type: %T", resp), types.ErrorCodeDoRequestFailed, types.ErrOptionWithSkipRetry()) + } defer service.CloseResponseBodyGracefully(httpResp)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@relay/wss_responses.go` around lines 147 - 152, The direct type assertion resp.(*http.Response) can panic if adaptor.DoRequest returns a different concrete type; replace it with a safe type check (respHTTP, ok := resp.(*http.Response)) and if !ok return a relay error via types.NewOpenAIError (use an appropriate ErrorCode like types.ErrorCodeDoRequestFailed and http.StatusInternalServerError) instead of panicking, and only call service.CloseResponseBodyGracefully(respHTTP) when the assertion succeeds; keep the existing adaptor.DoRequest error handling and variable names (resp, adaptor.DoRequest, types.NewOpenAIError, service.CloseResponseBodyGracefully).
158-177:⚠️ Potential issue | 🟠 MajorRestore the Gin context with defers.
If
adaptor.DoResponsepanics or future code adds an early return,c.Writerandc.Request.Methodremain mutated for the rest of the request lifecycle.🛠️ Proposed fix
originalWriter := c.Writer c.Writer = capture + defer func() { c.Writer = originalWriter }() @@ originalMethod := c.Request.Method c.Request.Method = "POST" + defer func() { c.Request.Method = originalMethod }() usage, newAPIError := adaptor.DoResponse(c, httpResp, info) - - c.Request.Method = originalMethod - c.Writer = originalWriter🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@relay/wss_responses.go` around lines 158 - 177, Save the original values (originalWriter := c.Writer, originalMethod := c.Request.Method, prevIsStream := info.IsStream) and immediately restore them in a defer before calling adaptor.DoResponse so they are reset even if DoResponse panics or returns early; set info.IsStream = false and c.Request.Method = "POST" for the call, then let the deferred function restore c.Writer = originalWriter, c.Request.Method = originalMethod, and info.IsStream = prevIsStream after adaptor.DoResponse returns.
179-181:⚠️ Potential issue | 🔴 CriticalAlways emit a terminal Responses event.
After
response.created/response.in_progress, failures, empty captured bodies, unmarshal failures, or empty choices return withoutresponse.completed,response.failed, orresponse.incomplete, leaving strict WS clients waiting until timeout.Also applies to: 210-317
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@controller/relay.go`:
- Around line 93-94: The current branch handling types.RelayFormatOpenAIRealtime
and types.RelayFormatOpenAIResponses calls helper.WssError(c, ws,
newAPIError.ToOpenAIError()) but helper.WssError returns early when ws is nil,
so HTTP /v1/responses clients get no JSON body; update the branch so that if ws
!= nil you call helper.WssError(c, ws, newAPIError.ToOpenAIError()), otherwise
write an HTTP JSON error using the Gin context (e.g. c.AbortWithStatusJSON or
c.JSON) with newAPIError.ToOpenAIError() as the payload — change the code around
the RelayFormatOpenAIRealtime / RelayFormatOpenAIResponses case to explicitly
handle ws == nil and send the OpenAI-formatted error as an HTTP JSON response.
In `@relay/wss_responses.go`:
- Around line 78-82: When unmarshalling the first WS message into
dto.OpenAIResponsesRequest (responsesReq) do not immediately replace
info.Request; instead parse into a temporary struct, validate that required
fields (e.g., non-empty input) are present and that the model field matches the
already-authorized/handshake model stored on info (or the prior routed request)
and run the same Responses validation routine you use elsewhere (e.g.,
ValidateResponsesRequest or equivalent). If validation fails, return
types.NewError(...) and do not overwrite info.Request; only assign info.Request
= &responsesReq after all checks pass. This ensures pricing/channel selection
remains consistent and empty inputs or model-swapped payloads are rejected
before routing.
- Around line 53-69: Before entering the ReadMessage loop, call
info.ClientWs.SetReadLimit with the byte limit derived from
constant.MaxRequestBodyMB (convert MB to bytes, e.g., constant.MaxRequestBodyMB
* 1024 * 1024) so the websocket (info.ClientWs) enforces the same max body size
as HTTP requests; keep the SetReadDeadline/SetWriteDeadline and SetPingHandler
as-is and then proceed to the existing ReadMessage loop.
- Around line 337-338: The code currently calls ws.WriteJSON(msg), which
bypasses the project-specific JSON wrapper; instead call common.Marshal(msg) to
produce JSON bytes, handle any marshal error, then send those bytes with
ws.WriteMessage(websocket.TextMessage, jsonBytes) after the existing
ws.SetWriteDeadline(time.Now().Add(10 * time.Second)); replace the ws.WriteJSON
call with this flow and ensure you import/use websocket.TextMessage and
handle/report marshal/write errors appropriately.
---
Duplicate comments:
In `@relay/wss_responses.go`:
- Line 207: Remove the stray debug stdout print in wss_responses.go: delete the
fmt.Printf("[DEBUG] usageData: %+v\n", usageData) line (or replace it with the
project's structured logger at debug level if you need to keep the message),
ensuring you use the existing logger API rather than fmt.Printf and keep the
output behind debug logging.
- Line 5: The file currently imports and uses encoding/json directly; replace
all direct encoding/json usage (e.g., json.Unmarshal calls at the noted
locations) with the project wrapper functions in common/json.go (use
common.Unmarshal for decoding and common.Marshal if encoding is required), and
remove the encoding/json import from relay/wss_responses.go; update every
occurrence referenced (including the usages around the earlier-mentioned spots)
to call common.Unmarshal with the same arguments and handle returned error/value
the same way.
- Around line 147-152: The direct type assertion resp.(*http.Response) can panic
if adaptor.DoRequest returns a different concrete type; replace it with a safe
type check (respHTTP, ok := resp.(*http.Response)) and if !ok return a relay
error via types.NewOpenAIError (use an appropriate ErrorCode like
types.ErrorCodeDoRequestFailed and http.StatusInternalServerError) instead of
panicking, and only call service.CloseResponseBodyGracefully(respHTTP) when the
assertion succeeds; keep the existing adaptor.DoRequest error handling and
variable names (resp, adaptor.DoRequest, types.NewOpenAIError,
service.CloseResponseBodyGracefully).
- Around line 158-177: Save the original values (originalWriter := c.Writer,
originalMethod := c.Request.Method, prevIsStream := info.IsStream) and
immediately restore them in a defer before calling adaptor.DoResponse so they
are reset even if DoResponse panics or returns early; set info.IsStream = false
and c.Request.Method = "POST" for the call, then let the deferred function
restore c.Writer = originalWriter, c.Request.Method = originalMethod, and
info.IsStream = prevIsStream after adaptor.DoResponse returns.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0a3be1ac-114a-4c65-8049-b1ed593e5157
📒 Files selected for processing (4)
controller/relay.gorelay/helper/valid_request.gorelay/wss_responses.gotypes/error.go
✅ Files skipped from review due to trivial changes (1)
- types/error.go
Comment on lines
+53
to
+69
| // Set a 5-minute timeout as requested by user | ||
| timeout := 5 * time.Minute | ||
| info.ClientWs.SetReadDeadline(time.Now().Add(timeout)) | ||
| info.ClientWs.SetWriteDeadline(time.Now().Add(timeout)) | ||
|
|
||
| // Default Ping handler in Gorilla responds with a Pong. | ||
| info.ClientWs.SetPingHandler(func(appData string) error { | ||
| info.ClientWs.SetReadDeadline(time.Now().Add(timeout)) | ||
| info.ClientWs.SetWriteDeadline(time.Now().Add(timeout)) | ||
| return info.ClientWs.WriteMessage(websocket.PongMessage, []byte(appData)) | ||
| }) | ||
|
|
||
| // 1. Read the first message from WebSocket | ||
| var message []byte | ||
| var err error | ||
| for { | ||
| _, message, err = info.ClientWs.ReadMessage() |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C3 '\bSetReadLimit\b|MaxBytesReader|ErrRequestBodyTooLarge|RequestBodyTooLarge|UnmarshalBodyReusable' --glob '*.go'Repository: QuantumNous/new-api
Length of output: 17424
🏁 Script executed:
cat -n relay/wss_responses.go | head -100Repository: QuantumNous/new-api
Length of output: 3472
🏁 Script executed:
rg -n 'maxBytes|MaxBodySize|BodySize|maxBodySize' --glob '*.go' -i | head -30Repository: QuantumNous/new-api
Length of output: 1812
🏁 Script executed:
rg -n 'maxMB|MaxMB|MaxBodySize|REQUEST_BODY_SIZE' --glob '*.go' -i | head -20Repository: QuantumNous/new-api
Length of output: 696
🏁 Script executed:
rg -n 'GetAndValidAudioRequest|GetBodyStorage' relay/wss_responses.goRepository: QuantumNous/new-api
Length of output: 45
🏁 Script executed:
rg -n 'ReadMessage|SetReadLimit' relay/wss_responses.goRepository: QuantumNous/new-api
Length of output: 113
🏁 Script executed:
rg -B5 -A5 'SetReadLimit' --glob '*.go'Repository: QuantumNous/new-api
Length of output: 45
Add WebSocket read size limit before message reading.
WebSocket frame reading at line 69 lacks a size constraint. Call SetReadLimit with the configured body size limit (matching HTTP requests which use constant.MaxRequestBodyMB, defaulting to 32–128 MB) before the ReadMessage loop to prevent unbounded memory consumption.
Current code
// Set a 5-minute timeout as requested by user
timeout := 5 * time.Minute
info.ClientWs.SetReadDeadline(time.Now().Add(timeout))
info.ClientWs.SetWriteDeadline(time.Now().Add(timeout))
// Default Ping handler in Gorilla responds with a Pong.
info.ClientWs.SetPingHandler(func(appData string) error {
info.ClientWs.SetReadDeadline(time.Now().Add(timeout))
info.ClientWs.SetWriteDeadline(time.Now().Add(timeout))
return info.ClientWs.WriteMessage(websocket.PongMessage, []byte(appData))
})
// 1. Read the first message from WebSocket
var message []byte
var err error
for {
_, message, err = info.ClientWs.ReadMessage()
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@relay/wss_responses.go` around lines 53 - 69, Before entering the ReadMessage
loop, call info.ClientWs.SetReadLimit with the byte limit derived from
constant.MaxRequestBodyMB (convert MB to bytes, e.g., constant.MaxRequestBodyMB
* 1024 * 1024) so the websocket (info.ClientWs) enforces the same max body size
as HTTP requests; keep the SetReadDeadline/SetWriteDeadline and SetPingHandler
as-is and then proceed to the existing ReadMessage loop.
Comment on lines
+337
to
+338
| _ = ws.SetWriteDeadline(time.Now().Add(10 * time.Second)) | ||
| return ws.WriteJSON(msg) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C2 '\.WriteJSON\s*\(|encoding/json|json\.(Unmarshal|Marshal)' relay/wss_responses.goRepository: QuantumNous/new-api
Length of output: 711
🏁 Script executed:
# Check common/json.go for available Marshal functions
fd -name "json.go" -path "*/common/*" | head -5 | xargs cat -n
# Also check what websocket functions are available
rg -n "websocket\." relay/wss_responses.go | head -20Repository: QuantumNous/new-api
Length of output: 485
🏁 Script executed:
# Get full context around lines 337-338 to see the complete function
sed -n '320,339p' relay/wss_responses.goRepository: QuantumNous/new-api
Length of output: 503
🏁 Script executed:
# Check if there are any uses of common.Marshal in the file already
rg -n "common\.(Marshal|Unmarshal|UnmarshalJsonStr|DecodeJson|GetJsonType)" relay/wss_responses.goRepository: QuantumNous/new-api
Length of output: 117
🏁 Script executed:
# Check common/json.go to see Marshal signature
find . -path "*/common/json.go" -exec cat {} \; | head -100Repository: QuantumNous/new-api
Length of output: 817
🏁 Script executed:
# Check websocket import and available constants
rg -n "websocket|WriteMessage|WriteJSON" relay/wss_responses.go | head -30Repository: QuantumNous/new-api
Length of output: 437
🏁 Script executed:
# Count all WriteJSON calls in the file
rg -c "WriteJSON" relay/wss_responses.goRepository: QuantumNous/new-api
Length of output: 64
🏁 Script executed:
# Verify websocket.TextMessage exists in gorilla/websocket
rg -n "TextMessage" --type go -g "*.go" | head -5Repository: QuantumNous/new-api
Length of output: 171
Replace WriteJSON with common.Marshal + WriteMessage to use the required JSON wrapper.
WriteJSON marshals via encoding/json internally, bypassing the required common.Marshal wrapper. Build JSON bytes with common.Marshal, then write as a text frame.
Suggested fix
}
_ = ws.SetWriteDeadline(time.Now().Add(10 * time.Second))
- return ws.WriteJSON(msg)
+ jsonData, err := common.Marshal(msg)
+ if err != nil {
+ return err
+ }
+ return ws.WriteMessage(websocket.TextMessage, jsonData)
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@relay/wss_responses.go` around lines 337 - 338, The code currently calls
ws.WriteJSON(msg), which bypasses the project-specific JSON wrapper; instead
call common.Marshal(msg) to produce JSON bytes, handle any marshal error, then
send those bytes with ws.WriteMessage(websocket.TextMessage, jsonBytes) after
the existing ws.SetWriteDeadline(time.Now().Add(10 * time.Second)); replace the
ws.WriteJSON call with this flow and ensure you import/use websocket.TextMessage
and handle/report marshal/write errors appropriately.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
relay/wss_responses.go (1)
127-155: Hardcoded sampling params inresponse.created.
temperature: 1.0,top_p: 1.0, and the zero penalty fields are emitted regardless of what the client sent. Strict clients that compare theresponseenvelope with the original request will see divergence. Echo the values fromresponsesReq(or omit them) instead of hardcoding.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@relay/wss_responses.go` around lines 127 - 155, The response.created event currently hardcodes sampling params (temperature, top_p, presence_penalty, frequency_penalty) which can diverge from the original request; update the sendWsResponseEvent payload construction in the response.created block to echo these values from the incoming responsesReq (or omit them when not present), and ensure response.in_progress only includes fields you intend to mirror (keep model and status as-is); modify the code paths that build the gin.H maps (used in sendWsResponseEvent for "response.created" and "response.in_progress") to read temperature, top_p, presence_penalty, and frequency_penalty from responsesReq rather than using literal constants.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@relay/wss_responses.go`:
- Around line 67-77: The defer currently emits a terminal "response.failed" with
a magic sequence_number 999 which can break monotonic sequence checks and can be
sent before any "response.created"; update the defer in wss_responses.go to use
a tracked local sequence counter (e.g., lastEmittedSeq or seq) instead of 999
and increment it when sending events via sendWsResponseEvent, and change the
logic so that if no "response.created" has been emitted yet (track a boolean
like responseOpened), either synthesize and emit a proper "response.created"
(with the next sequence) before emitting "response.failed" or emit a plain error
event (e.g., an "error" or "response.error" stream event) to avoid out-of-spec
ordering; reference newAPIError, info.ClientWs and sendWsResponseEvent when
making these changes.
- Around line 192-364: The adaptor may return nil or a non-*dto.Usage and
currently PostTextConsumeQuota is only called when usage.(*dto.Usage) succeeds,
leaving pre-consumed quota unreconciled; update the logic after
adaptor.DoResponse to always call service.PostTextConsumeQuota: if usage is nil
or not a *dto.Usage, build a fallback dto.Usage (e.g. use
info.EstimatePromptTokens / info.SetEstimatePromptTokens or captured assistant
text length to set PromptTokens/CompletionTokens and TotalTokens as
best-effort), log that a fallback estimate was used, then call
service.PostTextConsumeQuota(c, info, fallbackUsage, nil) so quota settlement
always runs even when the adaptor returns unexpected types.
---
Nitpick comments:
In `@relay/wss_responses.go`:
- Around line 127-155: The response.created event currently hardcodes sampling
params (temperature, top_p, presence_penalty, frequency_penalty) which can
diverge from the original request; update the sendWsResponseEvent payload
construction in the response.created block to echo these values from the
incoming responsesReq (or omit them when not present), and ensure
response.in_progress only includes fields you intend to mirror (keep model and
status as-is); modify the code paths that build the gin.H maps (used in
sendWsResponseEvent for "response.created" and "response.in_progress") to read
temperature, top_p, presence_penalty, and frequency_penalty from responsesReq
rather than using literal constants.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0beb4ed6-8de4-43ff-8a01-10fa585c757e
📒 Files selected for processing (2)
controller/relay.gorelay/wss_responses.go
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
relay/wss_responses.go (4)
110-113:⚠️ Potential issue | 🔴 CriticalValidate the first WS payload before replacing
info.Request.Line 113 trusts the frame after routing/channel selection already happened. Reject a message model that differs from the authorized handshake model, and run the same required-field validation used by the HTTP Responses path before assigning
info.Request.Proposed guard
if err := common.Unmarshal(message, &responsesReq); err != nil { return types.NewError(err, types.ErrorCodeInvalidRequest) } + if responsesReq.Model == "" { + responsesReq.Model = info.OriginModelName + } else if info.OriginModelName != "" && responsesReq.Model != info.OriginModelName { + return types.NewError( + fmt.Errorf("model mismatch: handshake model %q, message model %q", info.OriginModelName, responsesReq.Model), + types.ErrorCodeInvalidRequest, + types.ErrOptionWithSkipRetry(), + ) + } + input := bytes.TrimSpace(responsesReq.Input) + if len(input) == 0 || bytes.Equal(input, []byte("null")) { + return types.NewError(fmt.Errorf("input is required"), types.ErrorCodeInvalidRequest) + } info.Request = &responsesReqVerify the existing validation/model fields before finalizing the guard:
#!/bin/bash rg -n -C3 '\bOriginModelName\b|\bValidateResponses|\bOpenAIResponsesRequest\b|\bInput\b' --type go
412-413:⚠️ Potential issue | 🟠 MajorReplace
WriteJSONwithcommon.MarshalplusWriteMessage.Line 413 bypasses the repository JSON wrapper because Gorilla’s
WriteJSONmarshals internally. Marshal withcommon.Marshal, then send a text frame.Proposed fix
_ = ws.SetWriteDeadline(time.Now().Add(10 * time.Second)) - return ws.WriteJSON(msg) + jsonData, err := common.Marshal(msg) + if err != nil { + return err + } + return ws.WriteMessage(websocket.TextMessage, jsonData) }Verify there are no remaining direct WS JSON writes:
#!/bin/bash rg -n -C2 '\.WriteJSON\s*\(|encoding/json|json\.(Marshal|Unmarshal)' --type goAs per coding guidelines: “All JSON marshal/unmarshal operations MUST use wrapper functions from
common/json.go(common.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson,common.GetJsonType). Do NOT directly import or callencoding/jsonin business code.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@relay/wss_responses.go` around lines 412 - 413, Replace the direct gorilla websocket call ws.WriteJSON(msg) with common.Marshal to produce the JSON bytes and then send them using ws.WriteMessage(websocket.TextMessage, jsonBytes); keep the existing ws.SetWriteDeadline(time.Now().Add(10 * time.Second)) and propagate/return any marshal or write errors instead of discarding them, and ensure you import/use common.Marshal (not encoding/json). Update the code path where ws.WriteJSON is used (reference: ws.WriteJSON) to call common.Marshal(msg) and ws.WriteMessage(websocket.TextMessage, buf), and run the provided grep check to verify no remaining .WriteJSON or direct encoding/json usage exist.
254-256:⚠️ Potential issue | 🟠 MajorDon’t report parse/empty-capture failures as successful completions.
If
capture.Bodyis unmarshalable, has no choices, or is empty, lines 367-396 currently emitresponse.completedwith empty output and ignore write errors. That hides provider/adapter failures and can bill a blank “success”; emitresponse.failed/an API error for parse failures, and return any terminal-event write error.Also applies to: 367-396
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@relay/wss_responses.go` around lines 254 - 256, The current handling around capture.Body, common.Unmarshal and dto.OpenAITextResponse treats parse failures, empty captures, or zero choices as successful completions; update the logic so that if common.Unmarshal returns an error, capture.Body.Len()==0, or chatResp.Choices is empty you emit response.failed (or an API error event) instead of response.completed and return the write error from emitting the terminal event; likewise when emitting response.completed ensure you check and return any write error instead of ignoring it. Use the existing symbols capture.Body, common.Unmarshal, dto.OpenAITextResponse, response.completed and response.failed to locate and change the branching so parse/empty cases produce response.failed and all terminal-event write errors are propagated.
52-101:⚠️ Potential issue | 🟠 MajorAdd a WebSocket read limit before
ReadMessage.Line 101 reads client frames without a size cap. Apply the same request-size limit used by HTTP bodies before entering the read loop to avoid unbounded memory growth.
Proposed fix
import ( "bytes" "fmt" "io" "net/http" "time" "github.com/QuantumNous/new-api/common" + "github.com/QuantumNous/new-api/constant" "github.com/QuantumNous/new-api/dto" @@ timeout := 5 * time.Minute + info.ClientWs.SetReadLimit(int64(constant.MaxRequestBodyMB) * 1024 * 1024) info.ClientWs.SetReadDeadline(time.Now().Add(timeout)) info.ClientWs.SetWriteDeadline(time.Now().Add(timeout))Verify the project-wide limit symbol and WS usage:
#!/bin/bash rg -n -C3 '\bMaxRequestBodyMB\b|\bSetReadLimit\b|\bReadMessage\b' --type go🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@relay/wss_responses.go` around lines 52 - 101, Add a read size cap on the websocket before entering the ReadMessage loop by calling info.ClientWs.SetReadLimit(...) with the project-wide request size limit converted to bytes; locate the existing constant (e.g., MaxRequestBodyMB or similar) used for HTTP bodies, compute bytes as MaxRequestBodyMB * 1024 * 1024 (or use an existing MaxRequestBodyBytes), and place the SetReadLimit call just before the for { _, message, err = info.ClientWs.ReadMessage() } loop so ReadMessage is bounded; reference info.ClientWs.SetReadLimit, info.ClientWs.ReadMessage, and the project constant (MaxRequestBodyMB / MaxRequestBodyBytes) when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@relay/wss_responses.go`:
- Around line 218-228: The current code fabricates a prompt-only dto.Usage when
upstream usage is missing, which can bypass the nil-usage handling in
service.PostTextConsumeQuota; instead, leave usage as nil when the upstream
value is absent (i.e., do not construct a dto.Usage with CompletionTokens: 0)
and call service.PostTextConsumeQuota(c, info, nil, nil) so the settlement sees
a true "unknown" usage; if you still need a non-nil usage for WebSocket display,
build that separate display fallback only after settlement (e.g., create a local
displayUsage from info.GetEstimatePromptTokens() for WS reporting) rather than
passing it into PostTextConsumeQuota.
- Around line 289-295: The event payload for "response.output_text.delta" is
using the "text" key but consumers and the DTO expect a "delta" field; update
the map passed to sendWsResponseEvent (the call in sendWsResponseEvent(...,
"response.output_text.delta", gin.H{...})) to use "delta": content instead of
"text": content (i.e., replace the "text" key with "delta", keeping the other
keys like "content_index", "item_id", and "output_index" unchanged so clients
receive the payload under the expected delta JSON field).
---
Duplicate comments:
In `@relay/wss_responses.go`:
- Around line 412-413: Replace the direct gorilla websocket call
ws.WriteJSON(msg) with common.Marshal to produce the JSON bytes and then send
them using ws.WriteMessage(websocket.TextMessage, jsonBytes); keep the existing
ws.SetWriteDeadline(time.Now().Add(10 * time.Second)) and propagate/return any
marshal or write errors instead of discarding them, and ensure you import/use
common.Marshal (not encoding/json). Update the code path where ws.WriteJSON is
used (reference: ws.WriteJSON) to call common.Marshal(msg) and
ws.WriteMessage(websocket.TextMessage, buf), and run the provided grep check to
verify no remaining .WriteJSON or direct encoding/json usage exist.
- Around line 254-256: The current handling around capture.Body,
common.Unmarshal and dto.OpenAITextResponse treats parse failures, empty
captures, or zero choices as successful completions; update the logic so that if
common.Unmarshal returns an error, capture.Body.Len()==0, or chatResp.Choices is
empty you emit response.failed (or an API error event) instead of
response.completed and return the write error from emitting the terminal event;
likewise when emitting response.completed ensure you check and return any write
error instead of ignoring it. Use the existing symbols capture.Body,
common.Unmarshal, dto.OpenAITextResponse, response.completed and response.failed
to locate and change the branching so parse/empty cases produce response.failed
and all terminal-event write errors are propagated.
- Around line 52-101: Add a read size cap on the websocket before entering the
ReadMessage loop by calling info.ClientWs.SetReadLimit(...) with the
project-wide request size limit converted to bytes; locate the existing constant
(e.g., MaxRequestBodyMB or similar) used for HTTP bodies, compute bytes as
MaxRequestBodyMB * 1024 * 1024 (or use an existing MaxRequestBodyBytes), and
place the SetReadLimit call just before the for { _, message, err =
info.ClientWs.ReadMessage() } loop so ReadMessage is bounded; reference
info.ClientWs.SetReadLimit, info.ClientWs.ReadMessage, and the project constant
(MaxRequestBodyMB / MaxRequestBodyBytes) when making the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 77f81d23-7528-4e33-9c01-95d9d49904bb
📒 Files selected for processing (1)
relay/wss_responses.go
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
relay/wss_responses.go (4)
303-312:⚠️ Potential issue | 🔴 CriticalWrong field name for
response.output_text.delta: usedelta, nottext.Per the Responses DTO/protocol (
relay/channel/openai/relay_responses.go,chat_via_responses.go), theoutput_text.deltaevent carries the incremental text under thedeltakey. Emitting it astextwill cause strict clients to drop the content.🛠️ Proposed fix
if err := sendWsResponseEvent(info.ClientWs, seqCounter, "response.output_text.delta", gin.H{ "content_index": 0, "item_id": itemID, "output_index": 0, - "text": content, + "delta": content, }); err != nil {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@relay/wss_responses.go` around lines 303 - 312, The event for "response.output_text.delta" is using the wrong payload key ("text"); update the payload sent by sendWsResponseEvent (the call that builds the gin.H with content_index, item_id, output_index, ...) to use "delta" instead of "text" so the incremental text is emitted under the expected key; locate the call around seqCounter increment that uses info.ClientWs and modify the gin.H map to replace the "text" field with "delta".
232-246:⚠️ Potential issue | 🟠 MajorFabricated prompt-only usage bypasses
PostTextConsumeQuota's nil-usage handling.When the adaptor returns no/non-
*dto.Usagevalue, this code builds adto.Usage{PromptTokens=estimate, CompletionTokens=0}and passes it to settlement. That:
- Reports
CompletionTokens=0for successful generations where the upstream simply didn't return usage, under-billing completion tokens.- Prevents
PostTextConsumeQuotafrom seeingniland appending its "上游无计费信息" marker / falling back to its own estimation path (seeservice/text_quota.go:294-304).Pass
nilto settlement when upstream usage is genuinely missing, and build the WS displayusageDatafallback separately.🛠️ Proposed adjustment
u, ok := usage.(*dto.Usage) if !ok || u == nil { - u = &dto.Usage{ - PromptTokens: info.GetEstimatePromptTokens(), - CompletionTokens: 0, - TotalTokens: info.GetEstimatePromptTokens(), - } + u = nil } - // Force quota consumption service.PostTextConsumeQuota(c, info, u, nil) if newAPIError != nil { return newAPIError } + + if u == nil { + u = &dto.Usage{ + PromptTokens: info.GetEstimatePromptTokens(), + CompletionTokens: 0, + TotalTokens: info.GetEstimatePromptTokens(), + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@relay/wss_responses.go` around lines 232 - 246, The code currently constructs a non-nil dto.Usage when the adaptor returns no usage and passes it into service.PostTextConsumeQuota, which prevents PostTextConsumeQuota from performing its nil-handling/estimation; instead, leave the value passed to PostTextConsumeQuota as nil when the upstream returned no *dto.Usage (i.e., only set u = &dto.Usage{...} for WS display fallback, not for settlement), and ensure service.PostTextConsumeQuota(c, info, usageForSettlement, nil) receives nil in that case while the separate display variable (e.g., usageData) holds the estimated PromptTokens/CompletionTokens for the WS response.
52-69:⚠️ Potential issue | 🟠 MajorMissing
SetReadLimit— WS frames are unbounded.The
ReadMessageloop has no frame-size cap. A malicious/misbehaving client can force unbounded buffering and OOM the server. HTTP ingress is bounded byconstant.MaxRequestBodyMB; mirror that here before the read loop.🛠️ Proposed fix
info.ClientWs.SetReadDeadline(time.Now().Add(timeout)) info.ClientWs.SetWriteDeadline(time.Now().Add(timeout)) + info.ClientWs.SetReadLimit(int64(constant.MaxRequestBodyMB) * 1024 * 1024)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@relay/wss_responses.go` around lines 52 - 69, The WebSocket read loop lacks a frame-size cap allowing OOM attacks; before entering the ReadMessage loop (near where you set timeouts and ping handler on info.ClientWs and before using info.ClientWs.ReadMessage / the responses processing), call info.ClientWs.SetReadLimit(...) using the same limit as HTTP ingress (constant.MaxRequestBodyMB converted to bytes) to bound frame size; ensure this is done alongside the existing SetReadDeadline/SetWriteDeadline and SetPingHandler to enforce both size and time limits.
418-427:⚠️ Potential issue | 🟠 Major
ws.WriteJSONbypasses the requiredcommon.Marshalwrapper.
WriteJSONusesencoding/jsoninternally, which violates the repo-wide JSON policy. Build the payload viacommon.Marshaland push the bytes as a text frame.🛠️ Proposed fix
_ = ws.SetWriteDeadline(time.Now().Add(10 * time.Second)) - return ws.WriteJSON(msg) + jsonData, err := common.Marshal(msg) + if err != nil { + return err + } + return ws.WriteMessage(websocket.TextMessage, jsonData) }As per coding guidelines: "All JSON marshal/unmarshal operations MUST use wrapper functions in
common/json.go... Do NOT directly import or callencoding/jsonin business code."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@relay/wss_responses.go` around lines 418 - 427, sendWsResponseEvent currently calls ws.WriteJSON which uses encoding/json and violates the repo JSON policy; instead build the message map (as already done), call common.Marshal(msg) to get []byte, then send those bytes as a text frame using the websocket connection (e.g., ws.WriteMessage(websocket.TextMessage, payload)) after setting the write deadline. Update error handling to return any marshal or write errors from common.Marshal and ws.WriteMessage, and remove the ws.WriteJSON usage. Ensure references: function sendWsResponseEvent, method ws.WriteJSON -> replace with common.Marshal and ws.WriteMessage(websocket.TextMessage,...).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@relay/wss_responses.go`:
- Around line 114-125: After normalizing responsesReq.Model from
info.OriginModelName, add an explicit guard that rejects an empty model name: if
responsesReq.Model == "" return a descriptive invalid-request error (use
types.NewError with types.ErrorCodeInvalidRequest) so requests where both
info.OriginModelName and responsesReq.Model are empty are rejected instead of
forwarded; reference the existing normalization logic (responsesReq.Model and
info.OriginModelName) and the error construction pattern used elsewhere
(types.NewError) to implement this check.
- Around line 230-246: WssResponsesHelper is calling
service.PostTextConsumeQuota unconditionally after adaptor.DoResponse, which
settles actual usage even when DoResponse returned newAPIError and the outer
relay controller then refunds pre-consumed quota; to fix, only settle actual
quota when DoResponse succeeded: check newAPIError immediately after
adaptor.DoResponse (the newAPIError returned by adaptor.DoResponse) and return
the error before calling service.PostTextConsumeQuota, or move the
PostTextConsumeQuota call to after the nil-check for newAPIError so
PostTextConsumeQuota(c, info, u, nil) is invoked only when newAPIError == nil.
---
Duplicate comments:
In `@relay/wss_responses.go`:
- Around line 303-312: The event for "response.output_text.delta" is using the
wrong payload key ("text"); update the payload sent by sendWsResponseEvent (the
call that builds the gin.H with content_index, item_id, output_index, ...) to
use "delta" instead of "text" so the incremental text is emitted under the
expected key; locate the call around seqCounter increment that uses
info.ClientWs and modify the gin.H map to replace the "text" field with "delta".
- Around line 232-246: The code currently constructs a non-nil dto.Usage when
the adaptor returns no usage and passes it into service.PostTextConsumeQuota,
which prevents PostTextConsumeQuota from performing its nil-handling/estimation;
instead, leave the value passed to PostTextConsumeQuota as nil when the upstream
returned no *dto.Usage (i.e., only set u = &dto.Usage{...} for WS display
fallback, not for settlement), and ensure service.PostTextConsumeQuota(c, info,
usageForSettlement, nil) receives nil in that case while the separate display
variable (e.g., usageData) holds the estimated PromptTokens/CompletionTokens for
the WS response.
- Around line 52-69: The WebSocket read loop lacks a frame-size cap allowing OOM
attacks; before entering the ReadMessage loop (near where you set timeouts and
ping handler on info.ClientWs and before using info.ClientWs.ReadMessage / the
responses processing), call info.ClientWs.SetReadLimit(...) using the same limit
as HTTP ingress (constant.MaxRequestBodyMB converted to bytes) to bound frame
size; ensure this is done alongside the existing
SetReadDeadline/SetWriteDeadline and SetPingHandler to enforce both size and
time limits.
- Around line 418-427: sendWsResponseEvent currently calls ws.WriteJSON which
uses encoding/json and violates the repo JSON policy; instead build the message
map (as already done), call common.Marshal(msg) to get []byte, then send those
bytes as a text frame using the websocket connection (e.g.,
ws.WriteMessage(websocket.TextMessage, payload)) after setting the write
deadline. Update error handling to return any marshal or write errors from
common.Marshal and ws.WriteMessage, and remove the ws.WriteJSON usage. Ensure
references: function sendWsResponseEvent, method ws.WriteJSON -> replace with
common.Marshal and ws.WriteMessage(websocket.TextMessage,...).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 40e67365-2e52-412f-94d8-81ff0fadacec
📒 Files selected for processing (1)
relay/wss_responses.go
Comment on lines
+114
to
+125
| // Model consistency check | ||
| if responsesReq.Model == "" { | ||
| responsesReq.Model = info.OriginModelName | ||
| } | ||
| if responsesReq.Model != info.OriginModelName { | ||
| return types.NewError(fmt.Errorf("model mismatch: expected %s, got %s", info.OriginModelName, responsesReq.Model), types.ErrorCodeInvalidRequest, types.ErrOptionWithSkipRetry()) | ||
| } | ||
|
|
||
| // Required field validation | ||
| if len(responsesReq.Input) == 0 || string(responsesReq.Input) == "null" { | ||
| return types.NewError(fmt.Errorf("input is required"), types.ErrorCodeInvalidRequest) | ||
| } |
Contributor
There was a problem hiding this comment.
Edge case: empty OriginModelName + empty request model yields empty model downstream.
If both info.OriginModelName and responsesReq.Model are empty, line 116 assigns "" to itself, line 118's equality check passes, and the request proceeds with an empty model — later the adaptor/quota pipeline may mis-route or bill against an empty model name. Consider explicitly rejecting empty model after the normalization.
🛠️ Suggested guard
if responsesReq.Model == "" {
responsesReq.Model = info.OriginModelName
}
+ if responsesReq.Model == "" {
+ return types.NewError(fmt.Errorf("model is required"), types.ErrorCodeInvalidRequest, types.ErrOptionWithSkipRetry())
+ }
if responsesReq.Model != info.OriginModelName {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@relay/wss_responses.go` around lines 114 - 125, After normalizing
responsesReq.Model from info.OriginModelName, add an explicit guard that rejects
an empty model name: if responsesReq.Model == "" return a descriptive
invalid-request error (use types.NewError with types.ErrorCodeInvalidRequest) so
requests where both info.OriginModelName and responsesReq.Model are empty are
rejected instead of forwarded; reference the existing normalization logic
(responsesReq.Model and info.OriginModelName) and the error construction pattern
used elsewhere (types.NewError) to implement this check.
Comment on lines
+230
to
+246
| usage, newAPIError := adaptor.DoResponse(c, httpResp, info) | ||
|
|
||
| // Usage handling & mandatory quota settlement | ||
| u, ok := usage.(*dto.Usage) | ||
| if !ok || u == nil { | ||
| u = &dto.Usage{ | ||
| PromptTokens: info.GetEstimatePromptTokens(), | ||
| CompletionTokens: 0, | ||
| TotalTokens: info.GetEstimatePromptTokens(), | ||
| } | ||
| } | ||
| // Force quota consumption | ||
| service.PostTextConsumeQuota(c, info, u, nil) | ||
|
|
||
| if newAPIError != nil { | ||
| return newAPIError | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect how the outer controller handles newAPIError for OpenAIResponses relay format
rg -nP -C5 '\bWssResponsesHelper\b|RelayFormatOpenAIResponses' --type=go
# Check quota pre-consumption / refund hooks
rg -nP -C3 '\bPreConsumeQuota\b|\breturnPreConsumedQuota\b|ReturnPreConsumedQuota' --type=goRepository: QuantumNous/new-api
Length of output: 15617
🏁 Script executed:
# Search for quota pre-consumption and refund patterns
rg -nP 'PreConsumeQuota|ConsumeQuota.*pre|returnPreConsumedQuota|ReturnPreConsumedQuota' --type=go
# Also search for quota refund/return on error in the relay controller
rg -nP 'PostTextConsumeQuota|TextConsumeQuota' --type=go -A5 -B5Repository: QuantumNous/new-api
Length of output: 8310
🏁 Script executed:
# Search for quota pre-consumption in the relay controller
rg -nP 'ConsumeQuota|PreConsumed|returnPreConsumed' relay/ service/ --type=go -B3 -A3 | head -100
# Search specifically in relay.go for error handling with quota
rg -nP 'newAPIError|defer.*Quota|returnPreConsumed' controller/relay.go --type=go -B5 -A5Repository: QuantumNous/new-api
Length of output: 12721
🏁 Script executed:
# Check how PostTextConsumeQuota handles empty/error usage
rg -nP 'PostTextConsumeQuota|calculateTextQuotaSummary' service/text_quota.go -A20 | head -80
# Compare other handlers: do they call consume functions only on success?
rg -nP 'if.*newAPIError|if.*openaiErr' relay/compatible_handler.go relay/gemini_handler.go relay/claude_handler.go -B2 -A8Repository: QuantumNous/new-api
Length of output: 5802
🏁 Script executed:
# Look at the full defer refund logic in relay.go
sed -n '175,184p' controller/relay.go
# And confirm the wss_responses.go flow one more time
sed -n '230,246p' relay/wss_responses.go
# Check if there's any special handling in PostTextConsumeQuota for error cases
sed -n '294,350p' service/text_quota.goRepository: QuantumNous/new-api
Length of output: 4526
PostTextConsumeQuota settles quota unconditionally despite DoResponse error, causing downstream refund inconsistency.
WssResponsesHelper calls PostTextConsumeQuota unconditionally at line 242, settling actual usage, then returns newAPIError at line 245 if DoResponse failed. The outer relay controller then refunds pre-consumed quota in its defer (controller/relay.go:180) when newAPIError != nil, resulting in net billing of settled_actual - pre_consumed. This differs from other handlers (gemini_handler, claude_handler, etc.) which return errors before calling quota consumption functions, avoiding the double settlement.
The comment "Force quota consumption" suggests intent, but confirm whether the unconditional settlement followed by outer refund is desired, or adjust to either skip PostTextConsumeQuota on error or prevent outer refund for this path.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@relay/wss_responses.go` around lines 230 - 246, WssResponsesHelper is calling
service.PostTextConsumeQuota unconditionally after adaptor.DoResponse, which
settles actual usage even when DoResponse returned newAPIError and the outer
relay controller then refunds pre-consumed quota; to fix, only settle actual
quota when DoResponse succeeded: check newAPIError immediately after
adaptor.DoResponse (the newAPIError returned by adaptor.DoResponse) and return
the error before calling service.PostTextConsumeQuota, or move the
PostTextConsumeQuota call to after the nil-check for newAPIError so
PostTextConsumeQuota(c, info, u, nil) is invoked only when newAPIError == nil.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Description: OpenAI Responses API Support
This Pull Request implements support for the OpenAI Responses API (both HTTP SSE and WebSocket) in New API, specifically bridging it to Claude (Anthropic), Gemini, and Zhipu GLM models.
Key Changes
1. Adaptor Implementations
ConvertOpenAIResponsesRequestfor:2. WebSocket Protocol Bridging
relay/wss_responses.goto handle WebSocket-specific protocol alignment.response.created->response.in_progress->response.output_item.added-> ... ->response.completed.usage(mappingprompt_tokenstoinput_tokens) to satisfy strict client-side validation (e.g., Codex CLI).3. Middleware & Authentication
?api_key=Query String authentication inmiddleware/auth.gofor WebSocket clients that cannot set custom headers.Sec-WebSocket-Protocolheaders. Added a fallback togpt-5.3-codexfor legacy tool compatibility.4. Stability & Core Routing
/v1/responsesWebSocket entry point inrouter/relay-router.go.controller/relay.goto handle WebSocket upgrades and dispatching.relay/helper/valid_request.goto allow empty-body GET requests for WebSocket handshakes.Verification
go build ./....Summary by CodeRabbit
New Features
Bug Fixes