feat(openapi): auto-generate OpenAPI spec via Fuego#3292
Closed
0-don wants to merge 1 commit 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:
WalkthroughMigrates most HTTP controllers from Gin to go-fuego typed handlers with DTO request/response types, adds a comprehensive Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Gin as "Gin Router"
participant Fuego as "fuego.Engine"
participant DTO as "dto (Router/ParseParams)"
participant Controller
participant DB as "Database"
participant OpenAPI as "OpenAPI Engine"
Client->>Gin: HTTP request (e.g. GET /api/...)
Gin->>Fuego: routed via fuegogin adapter
Fuego->>DTO: build Fuego context, ParseParams
DTO->>Controller: invoke handler (fuego context)
Controller->>DB: query / update
DB-->>Controller: data
Controller-->>DTO: return (*dto.Response[T], error)
DTO-->>Gin: serialize response JSON
Gin-->>Client: HTTP response
Note over OpenAPI, DTO: if ENABLE_OPENAPI=true
DTO->>OpenAPI: register route metadata (operationId, schemas)
OpenAPI-->>Client: serves /openapi.json and /swagger
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![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: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (17)
setting/console_setting/validation.go (1)
4-4:⚠️ Potential issue | 🟠 MajorCoding guideline violation: Direct use of
encoding/json.The
parseJSONArrayfunction usesjson.Unmarshaldirectly (line 30), which violates the coding guideline requiring all JSON operations to use wrapper functions fromcommon/json.go. Theencoding/jsonimport should be removed after updating this function.🔧 Proposed fix to use common.Unmarshal
import ( - "encoding/json" "fmt" "net/url" "regexp"func parseJSONArray(jsonStr string, typeName string) ([]map[string]interface{}, error) { var list []map[string]interface{} - if err := json.Unmarshal([]byte(jsonStr), &list); err != nil { + if err := common.Unmarshal([]byte(jsonStr), &list); err != nil { return nil, fmt.Errorf("%s格式错误：%s", typeName, err.Error()) } return list, nil }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."Also applies to: 28-34
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@setting/console_setting/validation.go` at line 4, The parseJSONArray function currently calls json.Unmarshal directly (and the file imports "encoding/json"); replace that direct use with the project's JSON wrapper (e.g., call common.Unmarshal or the Unmarshal function exported from common/json.go) inside parseJSONArray, handle and return errors the same way, and then remove the "encoding/json" import; ensure you reference parseJSONArray when making the change so all direct json.Unmarshal usages in that function are replaced with the wrapper.dto/openai_request.go (1)
444-452:⚠️ Potential issue | 🟡 MinorDirect
encoding/jsonusage violates coding guidelines.The
ParseToolCalls()andSetToolCalls()methods usejson.Unmarshalandjson.Marshaldirectly, but the coding guidelines require usingcommon/json.gowrappers (common.Marshal,common.Unmarshal).Proposed fix
func (m *Message) ParseToolCalls() []ToolCallRequest { if m.ToolCalls == nil { return nil } var toolCalls []ToolCallRequest - if err := json.Unmarshal(m.ToolCalls, &toolCalls); err == nil { + if err := common.Unmarshal(m.ToolCalls, &toolCalls); err == nil { return toolCalls } return toolCalls } func (m *Message) SetToolCalls(toolCalls any) { - toolCallsJson, _ := json.Marshal(toolCalls) + toolCallsJson, _ := common.Marshal(toolCalls) m.ToolCalls = toolCallsJson }As per coding guidelines: "All JSON marshal/unmarshal operations MUST use wrapper functions in
common/json.go."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dto/openai_request.go` around lines 444 - 452, Replace direct calls to encoding/json in ParseToolCalls and SetToolCalls with the project's JSON wrapper functions: use common.Unmarshal to decode m.ToolCalls into toolCalls in ParseToolCalls (preserve the same return behavior) and use common.Marshal to encode toolCalls into m.ToolCalls in SetToolCalls; ensure you handle and propagate or log errors consistently with other DTO methods (i.e., check the error from common.Marshal instead of ignoring it) and reference the Message receiver and m.ToolCalls field when making these changes.model/user.go (1)
79-97:⚠️ Potential issue | 🟠 MajorReplace direct
json.Unmarshalandjson.Marshalcalls withcommon.Unmarshalandcommon.Marshalwrapper functions.Lines 82 and 91 directly call
json.Unmarshalandjson.Marshal, violating the repository JSON wrapper policy. All JSON operations must use the wrapper functions fromcommon/json.go.💡 Proposed fix
func (user *User) GetSetting() types.UserSetting { setting := types.UserSetting{} if user.Setting != "" { - err := json.Unmarshal([]byte(user.Setting), &setting) + err := common.Unmarshal([]byte(user.Setting), &setting) if err != nil { common.SysLog("failed to unmarshal setting: " + err.Error()) } } return setting } func (user *User) SetSetting(setting types.UserSetting) { - settingBytes, err := json.Marshal(setting) + settingBytes, err := common.Marshal(setting) if err != nil { common.SysLog("failed to marshal setting: " + err.Error()) return } user.Setting = string(settingBytes) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@model/user.go` around lines 79 - 97, Replace direct json calls in GetSetting and SetSetting with the JSON wrapper helpers: in GetSetting call common.Unmarshal([]byte(user.Setting), &setting) instead of json.Unmarshal and handle the returned error exactly as before; in SetSetting call common.Marshal(setting) instead of json.Marshal, check its error, and set user.Setting = string(marshaledBytes) on success. Keep the existing common.SysLog error messages but use the errors returned from common.Unmarshal/common.Marshal and ensure the types (types.UserSetting and []byte) are passed correctly.controller/wechat.go (1)
43-47:⚠️ Potential issue | 🟡 MinorUse
common.DecodeJsoninstead of directjson.NewDecoder().Decode().This violates the coding guideline requiring all JSON operations to use wrapper functions from
common/json.go.Suggested fix
- err = json.NewDecoder(httpResponse.Body).Decode(&res) + err = common.DecodeJson(httpResponse.Body, &res)You may also need to update the import to remove direct
encoding/jsonusage if no longer needed:import ( - "encoding/json" "errors"As per coding guidelines: "All JSON marshal/unmarshal operations MUST use wrapper functions in
common/json.go(common.DecodeJson). 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 `@controller/wechat.go` around lines 43 - 47, Replace the direct call to json.NewDecoder(...).Decode(&res) in the wechat login response handling with the project's JSON wrapper: call common.DecodeJson on httpResponse.Body into the wechatLoginResponse variable (res) instead of using encoding/json; update the import list to remove "encoding/json" if it becomes unused and ensure the code references common.DecodeJson where the symbol res (type wechatLoginResponse) and the error handling remain the same.controller/model_sync.go (1)
3-6:⚠️ Potential issue | 🟡 MinorDirect
encoding/jsonimport violates coding guidelines.The coding guidelines state: "All JSON marshal/unmarshal operations MUST use wrapper functions in
common/json.go." This file importsencoding/jsondirectly and usesjson.Unmarshalin thefetchJSONfunction (lines 174, 177, 199, 201).♻️ Suggested fix
import ( "context" - "encoding/json" "errors" "fmt"Then replace
json.Unmarshal(buf, out)calls withcommon.Unmarshal(buf, out).As per coding guidelines: "All JSON marshal/unmarshal operations MUST use wrapper functions in
common/json.go(common.Marshal,common.Unmarshal, ...)."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/model_sync.go` around lines 3 - 6, The file imports encoding/json and calls json.Unmarshal inside fetchJSON; remove the direct encoding/json import, add the common package import, and replace all json.Unmarshal(buf, out) calls in the fetchJSON function (and any other uses in this file) with common.Unmarshal(buf, out) so JSON operations use the wrapper in common/json.go.controller/setup.go (1)
57-97:⚠️ Potential issue | 🟠 MajorMake the setup flow atomic.
This path creates the root user, flips in-memory
operation_setting.*, updates options, setsconstant.Setup, and only then writes the final setup record. If any later step fails, the request returns failure after partial state has already leaked into memory or the database. Please wrap the persistence in a transaction and only updateoperation_setting.*/constant.Setupafter the transaction commits.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/setup.go` around lines 57 - 97, The setup flow mutates DB and in-memory state incrementally, leaking partial state on failure; wrap all persistence (creating rootUser via model.DB.Create, writing options via model.UpdateOption or equivalent, and creating the model.Setup record) in a single DB transaction (use model.DB.Begin(), use tx.Create/tx.Exec or add tx-aware versions of UpdateOption, and tx.Commit()/tx.Rollback() on error), return failures and rollback on any error, and only after tx.Commit() set operation_setting.SelfUseModeEnabled, operation_setting.DemoSiteEnabled and constant.Setup so in-memory flags change only when the DB transaction has succeeded.controller/codex_usage.go (1)
72-91:⚠️ Potential issue | 🟡 MinorHandle refreshed-token write failures before resetting caches.
If persisting the new token fails here, this request can still succeed while the expired credential remains stored. The next usage fetch will re-enter the refresh path, and the cache reset becomes misleading.
Suggested fix
encoded, encErr := common.Marshal(oauthKey) if encErr == nil { - _ = model.DB.Model(&model.Channel{}).Where("id = ?", ch.Id).Update("key", string(encoded)).Error - model.InitChannelCache() - service.ResetProxyClientCache() + if updateErr := model.DB.Model(&model.Channel{}).Where("id = ?", ch.Id).Update("key", string(encoded)).Error; updateErr != nil { + common.SysError("failed to persist refreshed codex token: " + updateErr.Error()) + } else { + model.InitChannelCache() + service.ResetProxyClientCache() + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/codex_usage.go` around lines 72 - 91, The refreshed token is being used and caches reset even if persisting the encoded oauthKey fails; change the flow in the RefreshCodexOAuthTokenWithProxy success branch to (1) marshal oauthKey, (2) perform the DB update call (the Model(&model.Channel{}).Where(...).Update("key", string(encoded")) and check its .Error, and only on success call model.InitChannelCache() and service.ResetProxyClientCache() and proceed to call service.FetchCodexWhamUsage with oauthKey.AccessToken; if the DB update or encoding fails, do not reset caches or proceed with the new token—handle the error (return or fall back to using the previous token) so the in-memory/cache state isn't misleading.controller/midjourney.go (1)
6-6:⚠️ Potential issue | 🟠 MajorCoding guideline violation: Use
common.Marshal/common.Unmarshalinstead ofencoding/json.Multiple locations in this file directly use
encoding/json:
- Line 6: Import of
encoding/json- Line 80:
json.Marshal(map[string]any{...})- Line 110:
json.Unmarshal(responseBody, &responseItems)- Lines 143, 147, 155:
json.Marshal(...)for properties, buttons, and videoUrls- Line 243:
json.Marshal(newTask.VideoUrls)As per coding guidelines: "All JSON marshal/unmarshal operations MUST use wrapper functions in
common/json.go(common.Marshal,common.Unmarshal, etc.). Do NOT directly import or callencoding/jsonin business code."♻️ Suggested fix pattern
-import ( - "encoding/json" ... -) +// Remove encoding/json import -body, _ := json.Marshal(map[string]any{ +body, _ := common.Marshal(map[string]any{ -err = json.Unmarshal(responseBody, &responseItems) +err = common.Unmarshal(responseBody, &responseItems) -propertiesStr, _ := json.Marshal(responseItem.Properties) +propertiesStr, _ := common.Marshal(responseItem.Properties)Also applies to: 80-82, 110-110, 143-147, 155-161, 243-244
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/midjourney.go` at line 6, Replace direct uses of encoding/json in controller/midjourney.go with the common wrapper functions: remove the "encoding/json" import and change all json.Marshal(...) calls to common.Marshal(...) (including the map[string]any payload, properties/buttons/videoUrls marshals, and json.Marshal(newTask.VideoUrls)) and change json.Unmarshal(responseBody, &responseItems) to common.Unmarshal(responseBody, &responseItems); ensure error handling remains the same and that you import the package that defines common.Marshal/common.Unmarshal if not already imported so functions like the map payload, responseItems unmarshalling, and newTask.VideoUrls conversions all use the common wrappers.controller/topup_creem.go (3)
158-164:⚠️ Potential issue | 🟠 MajorParse the webhook payload with
common.Unmarshal.
bodyBytesis already buffered. Re-reading it throughc.ShouldBindJSONadds a second parse path and skips the repo JSON wrapper.♻️ Minimal change
- // 重新设置body供后续的ShouldBindJSON使用 - c.Request.Body = io.NopCloser(bytes.NewReader(bodyBytes)) - - // 解析新格式的webhook数据 - var webhookEvent dto.CreemWebhookEvent - if err := c.ShouldBindJSON(&webhookEvent); err != nil { + // 解析新格式的webhook数据 + var webhookEvent dto.CreemWebhookEvent + if err := common.Unmarshal(bodyBytes, &webhookEvent); err != nil {As 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).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/topup_creem.go` around lines 158 - 164, The current code re-parses the request with c.ShouldBindJSON which bypasses the repo JSON wrapper and duplicates parsing; replace that with common.Unmarshal using the already-buffered bodyBytes to decode into dto.CreemWebhookEvent (i.e., call common.Unmarshal(bodyBytes, &webhookEvent) and handle the error similarly), keep or restore c.Request.Body if needed for downstream, and remove the c.ShouldBindJSON path so parsing goes through the required common.Unmarshal wrapper.
95-114:⚠️ Potential issue | 🟠 MajorAvoid leaving a stale local top-up order behind on checkout failures.
The
model.TopUprow is inserted beforecontroller.genCreemLinksucceeds. If checkout creation fails on that path, the request returns an error but the local order remainspending.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/topup_creem.go` around lines 95 - 114, The code inserts a pending model.TopUp before calling genCreemLink, leaving a stale pending order if genCreemLink fails; modify the error branch after genCreemLink to rollback the local order by either deleting the inserted topUp (call a delete method such as topUp.Delete() or a repository delete-by-TradeNo) or updating its Status to a terminal state (e.g., common.TopUpStatusFailed via topUp.Update or UpdateStatus), then log the rollback result before returning the dto.Fail; ensure you reference the TopUp instance (topUp), the Insert call site, and the genCreemLink error branch when making the change.
87-91:⚠️ Potential issue | 🟠 MajorHandle
model.GetUserByIdfailures before usinguser.A missing user or DB error can currently turn into a nil dereference on
user.Id,user.Email, oruser.Usernameinstead of a controlled API failure.🛡️ Minimal fix
- user, _ := model.GetUserById(id, false) + user, err := model.GetUserById(id, false) + if err != nil || user == nil { + return dto.Fail[dto.CreemPayData]("获取用户信息失败") + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/topup_creem.go` around lines 87 - 91, The call to model.GetUserById must be checked for errors and nil before accessing user fields; change the code around dto.UserID(c) and model.GetUserById(id, false) to capture the returned error, verify that err == nil and user != nil, and if not return a controlled API error response (e.g., 404/500) instead of proceeding to generate the reference or use user.Id, user.Email, or user.Username; ensure the unique reference generation (the reference := fmt.Sprintf(...) line) and any subsequent uses of user are only executed after the nil/error check.controller/token.go (1)
229-240:⚠️ Potential issue | 🟠 Major
status_only=falsecurrently skips the full update path.
if p.StatusOnly != ""treats any non-empty value as status-only. A request withstatus_only=falsewill still bypass the other field assignments.UpdateModelMetaalready guards on"true", and this handler should do the same.💡 Suggested fix
- if p.StatusOnly != "" { + if p.StatusOnly == "true" { cleanToken.Status = token.Status } else { cleanToken.Name = token.Name cleanToken.ExpiredTime = token.ExpiredTime🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/token.go` around lines 229 - 240, The handler currently checks if p.StatusOnly != "" which treats any non-empty value (including "false") as status-only and skips full updates; change the condition to explicitly check for the string "true" (i.e., if p.StatusOnly == "true") so only requests that explicitly set status_only=true take the short path that only assigns cleanToken.Status, and otherwise perform the full-field assignments (Name, ExpiredTime, RemainQuota, UnlimitedQuota, ModelLimitsEnabled, ModelLimits, AllowIps, Group, CrossGroupRetry); this matches the existing UpdateModelMeta behavior and ensures status_only=false will execute the full update path.controller/channel.go (5)
798-878:⚠️ Potential issue | 🟠 MajorResource leak: response body not closed on non-200 status.
When
response.StatusCode != http.StatusOK, the function returns without closingresponse.Body. Thedefer response.Body.Close()on Line 860 is not reached.🐛 Suggested fix
response, err := client.Do(request) if err != nil { return dto.Fail[[]string](err.Error()) } + defer response.Body.Close() + // check status code if response.StatusCode != http.StatusOK { return dto.Fail[[]string]("Failed to fetch models") } - defer response.Body.Close()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/channel.go` around lines 798 - 878, In FetchModels, the http.Response body may be leaked when response.StatusCode != http.StatusOK because the defer response.Body.Close() is after the status check; ensure response.Body is always closed by moving defer response.Body.Close() to immediately after receiving the response (right after client.Do(request) succeeds) or explicitly calling response.Body.Close() before each early return (e.g., before returning on non-200 status and on error paths after client.Do); update FetchModels to guarantee response.Body.Close() runs for all execution paths.
475-493:⚠️ Potential issue | 🟡 MinorUse
common.Marshalinstead ofjson.Marshal.Line 479 uses
json.Marshal(v)directly. Per coding guidelines, this should usecommon.Marshal.🔧 Suggested fix
case string: keyStr = strings.TrimSpace(v) default: - bytes, err := json.Marshal(v) + bytes, err := common.Marshal(v) if err != nil { return nil, fmt.Errorf("Vertex AI key JSON 编码失败: %w", err) }As per coding guidelines: "All JSON marshal/unmarshal operations MUST use wrapper functions in
common/json.go".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/channel.go` around lines 475 - 493, Replace the direct call to json.Marshal in the switch default branch with common.Marshal to follow project JSON wrapper usage: change json.Marshal(v) to common.Marshal(v) in the block that produces keyStr (the switch handling key.(type) -> default), keep the existing error handling and error message (fmt.Errorf("Vertex AI key JSON 编码失败: %w", err)) and ensure the common package is imported if not already.
681-796:⚠️ Potential issue | 🟡 MinorUse
common.Unmarshalinstead ofjson.Unmarshal.Lines 724-725 use
json.Unmarshaldirectly. Per coding guidelines, this should usecommon.Unmarshal.🔧 Suggested fix
if strings.HasPrefix(strings.TrimSpace(originChannel.Key), "[") { // JSON数组格式 var arr []json.RawMessage - if err := json.Unmarshal([]byte(strings.TrimSpace(originChannel.Key)), &arr); err == nil { + if err := common.Unmarshal([]byte(strings.TrimSpace(originChannel.Key)), &arr); err == nil { existingKeys = make([]string, len(arr)) for i, v := range arr { existingKeys[i] = string(v)As per coding guidelines: "All JSON marshal/unmarshal operations MUST use wrapper functions in
common/json.go".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/channel.go` around lines 681 - 796, In UpdateChannel (type PatchChannel) replace the direct call to json.Unmarshal that parses originChannel.Key into arr (the block that declares var arr []json.RawMessage and calls json.Unmarshal([]byte(strings.TrimSpace(originChannel.Key)), &arr)) with common.Unmarshal to comply with the project wrapper; keep the same target variable and error handling (i.e., call common.Unmarshal([]byte(strings.TrimSpace(originChannel.Key)), &arr), check err == nil or return the same dto.Fail error), and ensure any imports/reference to json.RawMessage remain valid after the swap.
1399-1468:⚠️ Potential issue | 🟡 MinorUse
common.Marshalinstead ofjson.Marshalin SSE handler.Lines 1445-1446, 1454, and 1459 use
json.Marshaldirectly. Per coding guidelines, these should usecommon.Marshal.🔧 Suggested fix
// 创建进度回调函数 progressCallback := func(progress ollama.OllamaPullResponse) { - data, _ := json.Marshal(progress) + data, _ := common.Marshal(progress) fmt.Fprintf(c.Writer, "data: %s\n\n", string(data)) c.Writer.Flush() } // 执行拉取 err = ollama.PullOllamaModelStream(baseURL, key, req.ModelName, progressCallback) if err != nil { - errorData, _ := json.Marshal(gin.H{ + errorData, _ := common.Marshal(gin.H{ "error": err.Error(), }) fmt.Fprintf(c.Writer, "data: %s\n\n", string(errorData)) } else { - successData, _ := json.Marshal(gin.H{ + successData, _ := common.Marshal(gin.H{ "message": fmt.Sprintf("Model %s pulled successfully", req.ModelName), })As per coding guidelines: "All JSON marshal/unmarshal operations MUST use wrapper functions in
common/json.go".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/channel.go` around lines 1399 - 1468, The SSE handler OllamaPullModelStream is calling json.Marshal directly (see uses inside progressCallback and when building errorData and successData); replace those json.Marshal calls with the project's wrapper common.Marshal to comply with the JSON marshalling guideline. Update calls in the progressCallback (where progress is marshalled) and the two places constructing errorData and successData to use common.Marshal, handle the returned bytes and errors the same way currently handled from json.Marshal, and keep the rest of the SSE framing (fmt.Fprintf and c.Writer.Flush) unchanged.
3-23:⚠️ Potential issue | 🟡 MinorDirect
encoding/jsonimport violates coding guidelines.The coding guidelines require all JSON marshal/unmarshal operations to use wrapper functions from
common/json.go. This file importsencoding/jsondirectly and uses it in several places.As per coding guidelines: "All JSON marshal/unmarshal operations MUST use wrapper functions in
common/json.go".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/channel.go` around lines 3 - 23, This file imports "encoding/json" directly in controller/channel.go which violates the guideline; remove the direct import and replace any uses of json.Marshal/json.Unmarshal (or other encoding/json functions) with the project's JSON wrapper functions from common/json.go (e.g., the common package’s marshal/unmarshal helpers), updating all call sites in this file (handlers and any functions referencing encoding/json) to call common's wrapper functions instead and adjust error handling accordingly.
🧹 Nitpick comments (17)
relay/channel/claude/relay-claude.go (1)
4-4: Pre-existing: Directencoding/jsonusage violates coding guidelines.This file imports and uses
encoding/jsondirectly in several places (lines 88, 376, 510, 823) instead of the requiredcommon/json.gowrappers (common.Marshal,common.Unmarshal). While not introduced by this PR, consider addressing this in a follow-up.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 callencoding/jsonin business code."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@relay/channel/claude/relay-claude.go` at line 4, This file currently imports and uses encoding/json directly; replace those direct calls (the usages around the locations noted) with the project's JSON wrappers: use common.Marshal for marshalling and common.Unmarshal (or common.UnmarshalJsonStr/common.DecodeJson as appropriate) for unmarshalling and decoding, and remove the "encoding/json" import from relay-claude.go; ensure each spot that previously called json.Marshal/json.Unmarshal or related functions is converted to the matching common.* wrapper and update error handling to reflect the wrapper return values.dto/group.go (1)
4-7: Consider using a more specific type forRatio.Using
anyforRatioloses type safety. If the ratio can be multiple types (e.g.,float64orstring), consider documenting this in the comment, or if it's always numeric, usefloat64instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dto/group.go` around lines 4 - 7, The UserGroupInfo struct currently uses a broad any type for the Ratio field which loses type safety; change Ratio to a concrete numeric type (e.g., float64) if it is always numeric, or define a small custom type/union (e.g., string|float64 via a custom UnmarshalJSON) and document the allowed forms in a comment; update the struct's Ratio field (UserGroupInfo.Ratio) and add a short comment above it describing the expected type and behavior for JSON unmarshalling.web/src/components/table/model-deployments/modals/EditDeploymentModal.jsx (1)
86-104: Remove the unusedloadModelsfunction andmodelsstate.The
/api/models/listendpoint exists and is correctly implemented, but theloadModelsfunction is never used—the component only edits the deployment name and never renders any model selection UI. This causes unnecessary API calls every time the modal opens. Remove theloadModelsfunction (lines 86-104), themodelsstate (line 53), theloadingModelsstate (line 54), and the useEffect that triggers loading (lines 137-142).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/table/model-deployments/modals/EditDeploymentModal.jsx` around lines 86 - 104, Remove the unused loadModels function and its associated state and effect: delete the loadModels async function, the models state variable (e.g., const [models, setModels]), the loadingModels state (e.g., const [loadingModels, setLoadingModels]), and the useEffect that calls loadModels on mount; also remove any references to setModels or setLoadingModels in the component (notably inside loadModels) so the modal no longer makes unnecessary /api/models/list calls when opening.model/twofa.go (1)
316-318: Consider using parameterized boolean for cross-database compatibility.The raw
"is_enabled = true"literal may behave inconsistently across databases. SQLite stores booleans as 0/1, while PostgreSQL usestrue/false. Using a parameterized query lets GORM handle the conversion.Suggested change
- if err := DB.Model(&TwoFA{}).Where("is_enabled = true").Count(&enabledUsers).Error; err != nil { + if err := DB.Model(&TwoFA{}).Where("is_enabled = ?", true).Count(&enabledUsers).Error; err != nil {As per coding guidelines: "All database code MUST be fully compatible with SQLite, MySQL >= 5.7.8, and PostgreSQL >= 9.6 simultaneously."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@model/twofa.go` around lines 316 - 318, The WHERE clause uses a raw "is_enabled = true" literal which is not portable across SQLite/MySQL/Postgres; update the query in the DB.Model(&TwoFA{})...Count(&enabledUsers) call to use a parameterized boolean (e.g., use Where("is_enabled = ?", true) or an equivalent GORM predicate such as Where(&TwoFA{IsEnabled: true})) so GORM can handle type conversion across databases (refer to DB, TwoFA, Count, and enabledUsers in the changed line).dto/responses.go (3)
121-123:Faildoes not explicitly setSuccess: false.Unlike
FailMsgandFailPage, theFail[T]helper only setsMessagebut omitsSuccess: false. While Go zero-valuesbooltofalse, this is inconsistent with other helpers that explicitly set the field. For clarity and maintainability, consider being explicit.♻️ Suggested fix
func Fail[T any](msg string) (*Response[T], error) { - return &Response[T]{Message: msg}, nil + return &Response[T]{Success: false, Message: msg}, nil }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dto/responses.go` around lines 121 - 123, The Fail[T] helper currently only sets Message and relies on the bool zero-value; update the Fail[T] function to explicitly set Success: false (matching the behavior of FailMsg and FailPage) so the returned *Response[T] includes Success: false alongside Message and maintains consistency with other helpers that construct Response values.
139-141:FailPagealso omitsSuccess: false.Same issue as
Fail[T]— theSuccessfield relies on zero-value rather than explicit assignment.♻️ Suggested fix
func FailPage[T any](msg string) (*Response[PageData[T]], error) { - return &Response[PageData[T]]{Message: msg}, nil + return &Response[PageData[T]]{Success: false, Message: msg}, nil }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dto/responses.go` around lines 139 - 141, FailPage currently returns a Response[PageData[T]] without explicitly setting Success to false; update the function that constructs the failure response (FailPage[T]) to include Success: false in the returned &Response[PageData[T]] literal (same fix as for Fail[T]) so the failure is explicit rather than relying on the zero value.
49-49: Type assertion could panic if context doesn't wrap gin.Context.The cast
c.Context().(*gin.Context)will panic if the underlying context is not a*gin.Context. Consider using a type assertion with ok-check and returning an error or handling gracefully.🛡️ Proposed defensive check
-func GinCtx(c FuegoCtx) *gin.Context { return c.Context().(*gin.Context) } +func GinCtx(c FuegoCtx) *gin.Context { + gc, ok := c.Context().(*gin.Context) + if !ok { + panic("FuegoCtx.Context() is not *gin.Context") + } + return gc +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dto/responses.go` at line 49, The GinCtx helper currently does an unchecked type assertion c.Context().(*gin.Context) which can panic if the wrapped context isn't a *gin.Context*; update GinCtx to perform a safe type assertion (ctx, ok := c.Context().(*gin.Context)) and return a non-panicking result (e.g., change signature to GinCtx(c FuegoCtx) (*gin.Context, error) or (*gin.Context, bool)) and handle the not-ok case by returning nil with an explanatory error or false so callers can handle it gracefully; update all call sites to handle the new return form.dto/query_params.go (1)
194-205: Inconsistent timestamp types in Midjourney params.
GetAllMidjourneyParamsandGetUserMidjourneyParamsusestringfor timestamp fields, while other param structs likeGetAllTaskParams(lines 209-217) useint64. Consider aligning for consistency unless there's a specific reason for the difference.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dto/query_params.go` around lines 194 - 205, Change the StartTimestamp and EndTimestamp fields in GetAllMidjourneyParams and GetUserMidjourneyParams from string to int64 to match the other parameter structs (e.g., GetAllTaskParams) for consistency; update the field types for StartTimestamp and EndTimestamp in both structs (GetAllMidjourneyParams, GetUserMidjourneyParams) and ensure their `query` tags/descriptions remain the same and any callers/parsers that expect ints are adjusted accordingly.dto/params.go (1)
24-42: Consider handling unexported struct fields.The reflection logic iterates over all struct fields including unexported ones. If a struct
Pcontains an unexported field with aqueryorheadertag,fieldValue.Set()will panic because unexported fields are not settable via reflection.🛡️ Proposed defensive check
for i := range paramsType.NumField() { field := paramsType.Field(i) fieldValue := paramsValue.Field(i) + + // Skip unexported fields (cannot be set via reflection) + if !fieldValue.CanSet() { + continue + } if tag := field.Tag.Get("query"); tag != "" {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dto/params.go` around lines 24 - 42, The loop over struct fields currently calls fieldValue.Set (and builds slices) for any field with a "query" tag, which will panic for unexported fields; modify the loop that iterates paramsType.NumField() to skip fields that are unexported or not settable (e.g., check field.PkgPath != "" or use fieldValue.CanSet()/field.IsExported()) before handling tags and before calling reflect.MakeSlice/fieldValue.Set and setParamValue, so only exported/settable fields with query/header tags are processed.dto/misc.go (1)
4-6: Givehttp_statsa concrete schema.
anyturns/api/status/testinto an opaque blob in the generated OpenAPI. If this payload is even moderately stable, a dedicated DTO or at leastmap[string]anywill keep the new docs usable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dto/misc.go` around lines 4 - 6, The TestStatusData DTO currently uses a totally opaque type for HttpStats (TestStatusData.HttpStats) which makes OpenAPI unusable; replace the any with a concrete schema: either define a new struct (e.g., type HTTPStats or TestHTTPStats with the expected fields like Requests int, LatencyMs float64, Errors int, etc.) and change TestStatusData.HttpStats to that struct, or if fields are truly dynamic use map[string]any instead; update the TestStatusData definition to reference the chosen type so the generated OpenAPI shows a usable schema for /api/status/test.controller/subscription_payment_epay.go (1)
22-26: Same pattern as Stripe handler: consider separating error checks.Similar to the Stripe payment handler, the combined condition conflates body parsing errors with validation errors.
♻️ Suggested improvement
req, err := c.Body() - if err != nil || req.PlanId <= 0 { - return dto.Fail[dto.EpayPayResponse]("参数错误") + if err != nil { + return dto.Fail[dto.EpayPayResponse]("请求体解析失败") + } + if req.PlanId <= 0 { + return dto.Fail[dto.EpayPayResponse]("参数错误") }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/subscription_payment_epay.go` around lines 22 - 26, The combined error check in SubscriptionRequestEpay conflates JSON/body parsing errors with request validation; first check the result of c.Body() and return a parsing/BadRequest error if err != nil, then separately validate req.PlanId (<= 0) and return a validation error via dto.Fail[dto.EpayPayResponse]("参数错误") if invalid. Update SubscriptionRequestEpay to call c.Body(), handle and log/return the parse error immediately, and only after a successful parse perform the PlanId validation so each failure path is distinct and clear.controller/subscription_payment_stripe.go (1)
21-24: Consider separating body parsing errors from validation errors.The combined condition
err != nil || req.PlanId <= 0returns the same generic "参数错误" message for both JSON parsing failures and invalid PlanId. This could make debugging harder when the actual issue is malformed JSON.♻️ Suggested improvement
req, err := c.Body() - if err != nil || req.PlanId <= 0 { - return dto.Fail[dto.StripePayLinkData]("参数错误") + if err != nil { + return dto.Fail[dto.StripePayLinkData]("请求体解析失败") + } + if req.PlanId <= 0 { + return dto.Fail[dto.StripePayLinkData]("参数错误") }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/subscription_payment_stripe.go` around lines 21 - 24, Split the combined check into two steps so parsing errors and validation errors return different messages: first call c.Body() and if err != nil return a parsing/JSON error (e.g. "请求体解析错误") referencing the existing c.Body() call and variable req; then separately validate req.PlanId (req.PlanId <= 0) and return a distinct validation error message (e.g. "PlanId 无效") so malformed JSON and invalid PlanId are distinguishable. Ensure you update the existing return statements near the c.Body() and req.PlanId checks accordingly.controller/checkin.go (1)
33-55: Consider extracting stats conversion to a helper function.The manual type assertions from
statsMaptodto.CheckinStatsare verbose but safe. If this pattern is used elsewhere, extracting it to a helper could improve maintainability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/checkin.go` around lines 33 - 55, Extract the repeated manual type-assertion block into a helper (e.g., mapToCheckinStats or convertStatsMapToDTO) that accepts the statsMap (map[string]interface{}) and returns a dto.CheckinStats; move the logic that reads "total_quota", "total_checkins", "checkin_count", "checked_in_today" and converts "records" ([]model.CheckinRecord -> []dto.CheckinRecord) into that helper, then replace the inline block in controller/checkin.go with a single call to the helper; ensure the helper handles missing keys safely and preserves the same type-assertion behavior for dto.CheckinStats, model.CheckinRecord, and dto.CheckinRecord.dto/topup.go (1)
14-17: Givedto.EpayPayResponse.Paramsa concrete type.
interface{}will generate an effectively untyped schema for this field, which weakens the new OpenAPI surface. If this payload is always map-like, prefer the real returned shape here instead ofinterface{}.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dto/topup.go` around lines 14 - 17, The Params field on struct EpayPayResponse is currently interface{} which yields an untyped OpenAPI schema; replace it with a concrete type that matches the returned payload (e.g., a defined struct representing the params or at minimum a map type like map[string]interface{} or map[string]json.RawMessage) by updating the EpayPayResponse definition and any unmarshalling usages; ensure the chosen type has appropriate json tags and update imports/usages of EpayPayResponse.Params (constructor, unmarshalling logic, tests) to reflect the new type so the generated OpenAPI schema is properly typed.router/openapi.go (1)
33-39: Scope the description sanitiser to Fuego-generated prefixes.Right now this removes everything before the first
---\n\nin every operation description. That will also delete legitimate hand-written markdown once routes start adding richer docs.♻️ Safer approach
- if idx := strings.Index(op.Description, marker); idx != -1 { - op.Description = strings.TrimSpace(op.Description[idx+len(marker):]) - } + if strings.HasPrefix(op.Description, "#### Controller:") || strings.HasPrefix(op.Description, "#### Middlewares:") { + if idx := strings.Index(op.Description, marker); idx != -1 { + op.Description = strings.TrimSpace(op.Description[idx+len(marker):]) + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@router/openapi.go` around lines 33 - 39, The sanitiser currently strips everything before the first marker in every operation description; update stripFuegoDescriptions so it only removes the marker when the description actually begins with the Fuego-generated prefix: compute a trimmed copy (e.g., pre := strings.TrimLeft(op.Description, " \t\r\n")) and only when strings.HasPrefix(pre, marker) (or idx == 0 on the trimmed string) set op.Description = strings.TrimSpace(pre[len(marker):]); reference stripFuegoDescriptions, the marker constant, and op.Description to find and modify the logic so hand-written markdown later in the description is preserved.controller/subscription.go (1)
122-125: The USD fallback currently overrides every supplied currency.
req.Plan.Currencyis set to"USD"unconditionally right after the empty-check in both create and update, so the branch above is dead and any caller-provided currency is ignored. Either make USD-only behavior explicit, or keep the assignment inside the empty-check only.Also applies to: 174-177
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/subscription.go` around lines 122 - 125, The code unconditionally overwrites req.Plan.Currency to "USD" right after the empty check, so any caller-provided currency is ignored; update the logic so the assignment to "USD" only happens when req.Plan.Currency is empty (i.e., remove the unconditional req.Plan.Currency = "USD" and keep it inside the if block), and apply the same fix to the second occurrence mentioned (the other block at 174-177) so both create/update paths only default to USD when no currency was supplied.controller/deployment.go (1)
643-687: Redundant slice allocation.The
itemsslice is allocated twice - first as empty on Line 659, then immediately re-allocated with capacity on Line 661.♻️ Suggested simplification
func ListDeploymentContainers(c fuego.ContextNoBody) (*dto.Response[dto.ContainerListResponse], error) { client, err := getIoEnterpriseClient() if err != nil { return dto.Fail[dto.ContainerListResponse](err.Error()) } deploymentID := strings.TrimSpace(c.PathParam("id")) if deploymentID == "" { return dto.Fail[dto.ContainerListResponse]("deployment ID is required") } containers, err := client.ListContainers(deploymentID) if err != nil { return dto.Fail[dto.ContainerListResponse](err.Error()) } - items := make([]dto.ContainerItem, 0) + var items []dto.ContainerItem if containers != nil { items = make([]dto.ContainerItem, 0, len(containers.Workers))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/deployment.go` around lines 643 - 687, The items slice in ListDeploymentContainers is allocated twice (once as make([]dto.ContainerItem, 0) then again with capacity), so remove the redundant initial allocation and only initialize items when containers != nil—e.g., declare var items []dto.ContainerItem or allocate with make([]dto.ContainerItem, 0, len(containers.Workers)) inside the if block that iterates containers.Workers; ensure subsequent append calls and the final dto.ContainerListResponse use that single items variable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e77bfab0-f1f1-4687-a0cb-156f202d4552
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (120)
AGENTS.mdCLAUDE.mdDockerfileREADME.fr.mdREADME.ja.mdREADME.mdREADME.zh_CN.mdREADME.zh_TW.mdcommon/gin.gocontroller/billing.gocontroller/channel-billing.gocontroller/channel-test.gocontroller/channel.gocontroller/channel_affinity_cache.gocontroller/channel_upstream_update.gocontroller/checkin.gocontroller/codex_oauth.gocontroller/codex_usage.gocontroller/console_migrate.gocontroller/custom_oauth.gocontroller/deployment.gocontroller/group.gocontroller/image.gocontroller/log.gocontroller/midjourney.gocontroller/misc.gocontroller/missing_models.gocontroller/model.gocontroller/model_meta.gocontroller/model_sync.gocontroller/oauth.gocontroller/option.gocontroller/passkey.gocontroller/performance.gocontroller/prefill_group.gocontroller/pricing.gocontroller/ratio_config.gocontroller/ratio_sync.gocontroller/redemption.gocontroller/relay.gocontroller/secure_verification.gocontroller/setup.gocontroller/subscription.gocontroller/subscription_payment_creem.gocontroller/subscription_payment_epay.gocontroller/subscription_payment_stripe.gocontroller/swag_video.gocontroller/task.gocontroller/telegram.gocontroller/token.gocontroller/topup.gocontroller/topup_creem.gocontroller/topup_stripe.gocontroller/twofa.gocontroller/uptime_kuma.gocontroller/usedata.gocontroller/user.gocontroller/vendor_meta.gocontroller/wechat.godocker-compose.ymldocs/openapi/api.jsondocs/openapi/relay.jsondto/api_responses.godto/channel.godto/channel_affinity_cache.godto/channel_billing.godto/channel_testing.godto/checkin.godto/codex_usage.godto/custom_oauth.godto/deployment.godto/group.godto/misc.godto/model_meta.godto/model_sync.godto/openai_request.godto/option.godto/params.godto/performance.godto/pricing.godto/query_params.godto/ratio_sync_result.godto/responses.godto/router.godto/setup.godto/subscription.godto/token.godto/topup.godto/topup_creem.godto/twofa.godto/type_aliases.godto/uptime_kuma.godto/user.gogo.modmodel/channel.gomodel/task.gomodel/twofa.gomodel/user.gomodel/user_cache.gorelay/channel/claude/relay-claude.gorouter/api-router.gorouter/dashboard.gorouter/main.gorouter/openapi.gorouter/relay-router.gorouter/video-router.goservice/openaicompat/chat_to_responses.goservice/openaicompat/responses_to_chat.gosetting/chat.gosetting/console_setting/validation.gosetting/ratio_setting/exposed_cache.gotypes/channel_settings.gotypes/oauth_errors.gotypes/openai_video.gotypes/user_settings.goweb/src/components/table/model-deployments/modals/EditDeploymentModal.jsxweb/src/components/table/usage-logs/UsageLogsColumnDefs.jsxweb/src/components/topup/SubscriptionPlansCard.jsxweb/src/components/topup/index.jsxweb/src/hooks/models/useModelsData.jsx
💤 Files with no reviewable changes (3)
- web/src/components/table/usage-logs/UsageLogsColumnDefs.jsx
- controller/image.go
- controller/swag_video.go
0-don
force-pushed
the
feat/openapi-fuego
branch
from
d72c8c2 to
00a4b43
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (10)
types/channel_settings.go (1)
1-50:⚠️ Potential issue | 🟡 MinorComplete the enum constant forwarding in
dto/type_aliases.go.The type aliases correctly use
=to preserve methods. However, constant forwarding is incomplete: onlyVertexKeyTypeAPIKeyandAwsKeyTypeApiKeyare forwarded, whileVertexKeyTypeJSONandAwsKeyTypeAKSKare missing. Add these to thevarblock indto/type_aliases.go:VertexKeyTypeJSON = types.VertexKeyTypeJSON AwsKeyTypeAKSK = types.AwsKeyTypeAKSKWithout these, any call site expecting
dto.VertexKeyTypeJSONordto.AwsKeyTypeAKSKwill fail with undefined reference errors.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@types/channel_settings.go` around lines 1 - 50, The dto constant forwarding is missing two enum constants; update the var block in dto/type_aliases.go to forward VertexKeyTypeJSON and AwsKeyTypeAKSK by adding entries that assign dto.VertexKeyTypeJSON = types.VertexKeyTypeJSON and dto.AwsKeyTypeAKSK = types.AwsKeyTypeAKSK so callers can reference dto.VertexKeyTypeJSON and dto.AwsKeyTypeAKSK just like the existing VertexKeyTypeAPIKey and AwsKeyTypeApiKey forwards.model/user.go (1)
79-97:⚠️ Potential issue | 🟠 MajorUse
common.Unmarshal/common.Marshalin these setting helpers.Line 82 and Line 91 call
encoding/jsondirectly inside business code. Please switch to thecommon/json.gowrappers.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.”Suggested fix
func (user *User) GetSetting() types.UserSetting { setting := types.UserSetting{} if user.Setting != "" { - err := json.Unmarshal([]byte(user.Setting), &setting) + err := common.Unmarshal([]byte(user.Setting), &setting) if err != nil { common.SysLog("failed to unmarshal setting: " + err.Error()) } } return setting } func (user *User) SetSetting(setting types.UserSetting) { - settingBytes, err := json.Marshal(setting) + settingBytes, err := common.Marshal(setting) if err != nil { common.SysLog("failed to marshal setting: " + err.Error()) return } user.Setting = string(settingBytes) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@model/user.go` around lines 79 - 97, Replace direct encoding/json usage in User.GetSetting and User.SetSetting with the common JSON wrappers: in GetSetting call common.Unmarshal or common.UnmarshalJsonStr to decode user.Setting into types.UserSetting and log errors via common.SysLog if unmarshal returns an error; in SetSetting call common.Marshal to produce bytes (or string) from the types.UserSetting and assign to user.Setting, handling and logging any marshal error similarly. Update only the logic in the GetSetting and SetSetting methods (referenced by those exact function names) to use common.Marshal/common.Unmarshal variants instead of json.Marshal/json.Unmarshal.controller/pricing.go (1)
14-23:⚠️ Potential issue | 🟠 MajorGuard the context
idtype assertion to prevent runtime panic.Line 22 uses
userId.(int)without checking. The"id"context value can come from session data (viasession.Get("id")in middleware) which has no guaranteed type. If middleware stores a non-int value, this direct assertion will panic the request.Use the comma-ok pattern to safely convert:
Suggested fix
- userId, exists := dto.GinCtx(c).Get("id") + rawUserID, exists := dto.GinCtx(c).Get("id") @@ var group string if exists { - user, err := model.GetUserCache(userId.(int)) - if err == nil { - group = user.Group - for g := range groupRatio { - ratio, ok := ratio_setting.GetGroupGroupRatio(group, g) - if ok { - groupRatio[g] = ratio - } - } - } + if userID, ok := rawUserID.(int); ok { + user, err := model.GetUserCache(userID) + if err == nil { + group = user.Group + for g := range groupRatio { + ratio, ok := ratio_setting.GetGroupGroupRatio(group, g) + if ok { + groupRatio[g] = ratio + } + } + } + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/pricing.go` around lines 14 - 23, The code performs a bare type assertion userId.(int) when reading dto.GinCtx(c).Get("id"), which can panic if the stored session/context value isn't an int; update the guard to use the comma-ok pattern to verify the type before calling model.GetUserCache: check exists and do a safe type assertion (e.g., idInt, ok := userId.(int)) and only call model.GetUserCache(idInt) when ok is true (otherwise handle the non-int case by leaving group unset or returning an error); change any code paths that assume user is loaded (the user variable from model.GetUserCache) to respect the new guard.controller/wechat.go (1)
3-18:⚠️ Potential issue | 🟠 MajorReplace direct JSON decoding with
common.DecodeJson.Line 44 uses
json.NewDecoder(...).Decode(...)directly instead of thecommon.DecodeJsonwrapper. This violates the requirement that all JSON operations in business code must use wrapper functions fromcommon/json.go.Suggested fix
import ( - "encoding/json" "errors" "fmt" "net/http" "strconv" @@ - err = json.NewDecoder(httpResponse.Body).Decode(&res) + err = common.DecodeJson(httpResponse.Body, &res) if err != nil { return "", err }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/wechat.go` around lines 3 - 18, The handler currently calls json.NewDecoder(c.Request.Body).Decode(&...) directly; replace that call with the project wrapper common.DecodeJson to comply with the JSON wrapper requirement. Locate the direct use of json.NewDecoder(...).Decode(...) in controller/wechat.go (the handler that decodes the request into the dto/model) and swap it for common.DecodeJson(c.Request.Body, &<targetStruct>) (or common.DecodeJson(c, &<targetStruct>) if the wrapper signature expects gin.Context), propagate and handle the returned error the same way the current decode error is handled, and remove the direct import/usage of encoding/json for decoding so the code consistently uses common.DecodeJson.setting/console_setting/validation.go (2)
28-34:⚠️ Potential issue | 🟡 MinorUse
common.Unmarshalinstead ofjson.Unmarshal.The
parseJSONArrayfunction directly callsjson.Unmarshal, which violates the coding guidelines requiring use ofcommon.Unmarshal.Proposed fix
func parseJSONArray(jsonStr string, typeName string) ([]map[string]interface{}, error) { var list []map[string]interface{} - if err := json.Unmarshal([]byte(jsonStr), &list); err != nil { + if err := common.Unmarshal([]byte(jsonStr), &list); err != nil { return nil, fmt.Errorf("%s格式错误：%s", typeName, err.Error()) } return list, nil }As per coding guidelines: "All JSON marshal/unmarshal operations MUST use wrapper functions in
common/json.go."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@setting/console_setting/validation.go` around lines 28 - 34, The parseJSONArray function currently calls json.Unmarshal directly; replace that call with the project's wrapper common.Unmarshal to comply with the JSON marshaling guideline—invoke common.Unmarshal([]byte(jsonStr), &list) inside parseJSONArray, handle the error the same way (wrap with fmt.Errorf("%s格式错误：%s", typeName, err.Error())), and remove the direct use of json.Unmarshal so only common.Unmarshal is used.
4-4:⚠️ Potential issue | 🟡 MinorDirect
encoding/jsonimport violates coding guidelines.The coding guidelines require using wrapper functions from
common/json.go(common.Marshal,common.Unmarshal, etc.) instead of directly importingencoding/json.Proposed fix
import ( - "encoding/json" "fmt" "net/url"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 `@setting/console_setting/validation.go` at line 4, Remove the direct "encoding/json" import and replace all uses of json.Marshal/json.Unmarshal (and any other encoding/json calls) with the wrapper functions from common/json.go (e.g., common.Marshal, common.Unmarshal). Update the import block to import the common wrapper package (use the package name used in the repo, e.g., common) and refactor any functions in this file that call encoding/json to call common.Marshal/common.Unmarshal instead, preserving the same error handling and types.controller/model_meta.go (2)
4-4:⚠️ Potential issue | 🟠 MajorUse
common.Marshalinstead of directencoding/jsoncalls.
enrichModelsstill marshals withjson.Marshal(Line 177, Line 267), which violates the project JSON-wrapper rule.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.”Suggested fix
-import ( - "encoding/json" +import ( "sort" "strconv" "strings" + "github.com/QuantumNous/new-api/common" "github.com/QuantumNous/new-api/constant" "github.com/QuantumNous/new-api/dto" "github.com/QuantumNous/new-api/model" "github.com/go-fuego/fuego" ) @@ - if b, err := json.Marshal(eps); err == nil { + if b, err := common.Marshal(eps); err == nil { mm.Endpoints = string(b) } @@ - if b, err := json.Marshal(eps); err == nil { + if b, err := common.Marshal(eps); err == nil { mm.Endpoints = string(b) }Also applies to: 177-179, 267-269
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/model_meta.go` at line 4, The code in enrichModels is calling encoding/json directly (json.Marshal at the two marshal sites) which violates the project wrapper rule; replace those json.Marshal calls in the enrichModels function with the wrapper common.Marshal, remove the direct "encoding/json" import, add or ensure the common package is imported, and keep existing error handling (use the returned bytes and error from common.Marshal the same way you did for json.Marshal).
25-45:⚠️ Potential issue | 🟡 MinorDon’t discard list metadata errors.
CountandGetVendorModelCountserrors are ignored (Line 34, Line 37), so this endpoint can return successful responses with incorrecttotal/vendor_counts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/model_meta.go` around lines 25 - 45, In GetAllModelsMeta, don't ignore errors from model.DB.Model(&model.Model{}).Count(&total) and model.GetVendorModelCounts(); capture the error results, and if either returns an error return dto.Fail[ModelsMetaListData](err.Error()) (or wrap both errors into a single failure) instead of proceeding with potentially incorrect totals—update the Count call to check its error and check the returned error from GetVendorModelCounts() before building and returning dto.Ok.controller/deployment.go (1)
237-243:⚠️ Potential issue | 🟠 Major
SearchDeploymentsfilters only the current page, not the full result set.Line 237 applies pagination before keyword filtering, so Line 248-258 only searches that page. Line 266-268 then reports
totalfrom this partial slice, which undercounts matches.Also applies to: 248-268
controller/topup_creem.go (1)
87-92:⚠️ Potential issue | 🔴 CriticalGuard against nil user before dereference.
Line 88 ignores lookup errors, and Line 91 dereferences
user.Id; failed lookup can panic at runtime.💡 Minimal fix
- user, _ := model.GetUserById(id, false) + user, err := model.GetUserById(id, false) + if err != nil || user == nil { + return dto.Fail[dto.CreemPayData]("获取用户信息失败") + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/topup_creem.go` around lines 87 - 92, The code calls model.GetUserById(id, false) and ignores the error then dereferences user.Id when building reference/referenceId, which can panic if lookup failed; update the dto.UserID + model.GetUserById usage to capture and check the returned error and also check for nil user (e.g., if err != nil || user == nil) and return or handle the error before using user.Id so reference/referenceId generation only runs when a valid user is present.
♻️ Duplicate comments (14)
go.mod (1)
3-4:⚠️ Potential issue | 🟠 MajorSync Heroku Go version with the module Go version.
Line 3 (
go1.18) conflicts with Line 4 (go 1.25.7), which can build with the wrong toolchain on Heroku.Suggested fix
-// +heroku goVersion go1.18 +// +heroku goVersion go1.25.7Use this to confirm mismatch before/after:
#!/bin/bash set -euo pipefail sed -n '1,8p' go.mod rg -n '^\s*// \+heroku goVersion|^go ' go.mod🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@go.mod` around lines 3 - 4, The Heroku metadata comment "// +heroku goVersion go1.18" is out of sync with the module Go directive "go 1.25.7"; update the Heroku comment to match the module version (change the commented token to "go1.25.7") so both specify the same toolchain, then re-run the provided verification snippet to confirm the comment and the go directive are aligned. Reference the comment string "// +heroku goVersion go1.18" and the module directive "go 1.25.7" when making the edit.dto/custom_oauth.go (1)
67-84:⚠️ Potential issue | 🟠 MajorMake update semantics explicit for string fields.
Line 72 says “keep existing” behavior, but Lines 67-80 use plain
stringfor most updateable fields, so omitted fields are indistinguishable from empty values in sparse updates.Suggested patch-style DTO adjustment
type UpdateCustomOAuthProviderRequest struct { - Name string `json:"name"` - Slug string `json:"slug"` + Name *string `json:"name,omitempty"` + Slug *string `json:"slug,omitempty"` Icon *string `json:"icon"` // Optional: if nil, keep existing Enabled *bool `json:"enabled"` // Optional: if nil, keep existing - ClientId string `json:"client_id"` - ClientSecret string `json:"client_secret"` // Optional: if empty, keep existing - AuthorizationEndpoint string `json:"authorization_endpoint"` - TokenEndpoint string `json:"token_endpoint"` - UserInfoEndpoint string `json:"user_info_endpoint"` - Scopes string `json:"scopes"` - UserIdField string `json:"user_id_field"` - UsernameField string `json:"username_field"` - DisplayNameField string `json:"display_name_field"` - EmailField string `json:"email_field"` + ClientId *string `json:"client_id,omitempty"` + ClientSecret *string `json:"client_secret,omitempty"` + AuthorizationEndpoint *string `json:"authorization_endpoint,omitempty"` + TokenEndpoint *string `json:"token_endpoint,omitempty"` + UserInfoEndpoint *string `json:"user_info_endpoint,omitempty"` + Scopes *string `json:"scopes,omitempty"` + UserIdField *string `json:"user_id_field,omitempty"` + UsernameField *string `json:"username_field,omitempty"` + DisplayNameField *string `json:"display_name_field,omitempty"` + EmailField *string `json:"email_field,omitempty"` WellKnown *string `json:"well_known"` // Optional: if nil, keep existing AuthStyle *int `json:"auth_style"` // Optional: if nil, keep existing AccessPolicy *string `json:"access_policy"` // Optional: if nil, keep existing AccessDeniedMessage *string `json:"access_denied_message"` // Optional: if nil, keep existing }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dto/custom_oauth.go` around lines 67 - 84, The DTO currently uses plain string types for most updateable fields so callers cannot distinguish "omit field" from "set empty"; change all fields that should support sparse updates (Name, Slug, ClientId, ClientSecret, AuthorizationEndpoint, TokenEndpoint, UserInfoEndpoint, Scopes, UserIdField, UsernameField, DisplayNameField, EmailField) from string to *string in the Custom OAuth DTO in dto/custom_oauth.go (leave Icon, Enabled, WellKnown, AuthStyle, AccessPolicy, AccessDeniedMessage as pointers already), and then ensure the update handling logic treats nil as "keep existing" and non-nil (including empty string) as "apply update."service/openaicompat/responses_to_chat.go (3)
444-445:⚠️ Potential issue | 🟡 MinorPreserve upstream creation time in reverse conversion.
Using conversion-time
nowrewrites provenance. Prefer sourceresp.Created, with fallback to current time only when missing.Also applies to: 522-523
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@service/openaicompat/responses_to_chat.go` around lines 444 - 445, The code currently sets now := int(time.Now().Unix()) which overwrites the source response creation time; change the assignment so the timestamp used for conversion prefers the upstream resp.Created value and only falls back to the current time when resp.Created is zero/missing (e.g., set now = int(resp.Created) if resp.Created != 0 else int(time.Now().Unix())); update both occurrences around respID/now (the respID creation block using common.GetUUID() and the second occurrence at lines ~522-523) so provenance is preserved.
262-323:⚠️ Potential issue | 🟠 MajorDo not silently drop malformed
tools/tool_choice.Invalid JSON currently falls through and mutates semantics by producing empty/default tool config. Return a parse error instead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@service/openaicompat/responses_to_chat.go` around lines 262 - 323, The current parsing of req.Tools and req.ToolChoice silently ignores JSON unmarshal errors and falls through to leave out.Tools/out.ToolChoice empty or default; update the logic in the code handling req.Tools (the common.Unmarshal -> tools block) and req.ToolChoice (the common.Unmarshal -> tcStr/tcMap blocks) to detect and propagate parsing errors instead of swallowing them: when common.Unmarshal returns an error for req.Tools or req.ToolChoice, return that parsing error (or wrap it with context) from the enclosing function rather than continuing, so malformed JSON inputs produce a parse error; ensure this change covers both the string-path and map-path branches (the tcStr and tcMap flows) and any place where tools are marshaled/unmarshaled before appending to out.Tools.
3-12:⚠️ Potential issue | 🟠 MajorRemove direct
encoding/jsondependency in this business path.This file still imports
encoding/jsonand constructs JSON status directly; keep JSON handling withincommon/json.gowrappers.As per coding guidelines
**/*.go: All JSON marshal/unmarshal operations MUST use wrapper functions fromcommon/json.goand must not directly callencoding/jsonin business code.Also applies to: 519-524
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@service/openaicompat/responses_to_chat.go` around lines 3 - 12, Remove the direct dependency on encoding/json in responses_to_chat.go: delete encoding/json from the import list and replace any direct calls to json.Marshal/json.Unmarshal or manual JSON construction with the JSON wrapper functions provided in the common package (e.g., common.MarshalJSON / common.UnmarshalJSON or the actual wrapper names in common/json.go). Update all locations in this file that create status JSON blobs (currently using encoding/json) to call the common wrappers and propagate/handle their errors accordingly, and remove the unused encoding/json import from the import block.controller/channel-test.go (1)
749-760:⚠️ Potential issue | 🟡 MinorCompute elapsed time before the early failure return.
Line 751 returns before timing is computed, so failed tests always return
Time: 0.0, and theresult.newAPIErrorbranch is practically unreachable.Suggested fix
tik := time.Now() result := testChannel(channel, testModel, endpointType, isStream) - if result.localErr != nil { - return dto.TestChannelResponse{Success: false, Message: result.localErr.Error(), Time: 0.0}, nil - } tok := time.Now() milliseconds := tok.Sub(tik).Milliseconds() - go channel.UpdateResponseTime(milliseconds) consumedTime := float64(milliseconds) / 1000.0 - if result.newAPIError != nil { - return dto.TestChannelResponse{Success: false, Message: result.newAPIError.Error(), Time: consumedTime}, nil + if result.localErr != nil { + msg := result.localErr.Error() + if result.newAPIError != nil { + msg = result.newAPIError.Error() + } + return dto.TestChannelResponse{Success: false, Message: msg, Time: consumedTime}, nil } + go channel.UpdateResponseTime(milliseconds) return dto.TestChannelResponse{Success: true, Message: "", Time: consumedTime}, nil🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/channel-test.go` around lines 749 - 760, The timing is computed after the early return so failures (result.localErr and result.newAPIError) report Time: 0.0 and the newAPIError branch is unreachable; move the elapsed-time calculation and the async channel.UpdateResponseTime call to before any early returns: call testChannel(...) into result, then compute tok := time.Now(), milliseconds := tok.Sub(tik).Milliseconds(), fire go channel.UpdateResponseTime(milliseconds), compute consumedTime := float64(milliseconds)/1000.0, and only then evaluate result.localErr and result.newAPIError to return the appropriate dto.TestChannelResponse with the correct Time value.controller/user.go (2)
690-697:⚠️ Potential issue | 🟠 MajorReturn after delete; don’t call
Update(false)on a deleted user.After successful
user.Delete()(Line 694), execution still reaches Line 715 and callsuser.Update(false).Suggested fix
case "delete": if user.Role == common.RoleRootUser { return dto.Fail[dto.ManageUserData](common.TranslateMessage(ginCtx, i18n.MsgUserCannotDeleteRootUser)) } if err := user.Delete(); err != nil { return dto.Fail[dto.ManageUserData](err.Error()) } + return dto.Ok(dto.ManageUserData{Role: user.Role, Status: user.Status})Also applies to: 715-718
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/user.go` around lines 690 - 697, In the "delete" case inside the switch (case "delete") ensure you return immediately after a successful user.Delete() to avoid falling through to the later user.Update(false) call; specifically, after checking RoleRootUser and calling user.Delete(), return the appropriate dto.Fail or success response so execution does not reach user.Update(false) (and similarly ensure any other branches that delete a user do not continue to call Update(false)).
543-548:⚠️ Potential issue | 🟠 MajorReplace direct JSON round-trip with
common.Marshal/common.Unmarshal.
UpdateSelfstill uses directjson.Marshal/json.Unmarshal(Line 543, Line 547).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 `@controller/user.go` around lines 543 - 548, The UpdateSelf handler is performing a direct JSON round-trip using encoding/json (json.Marshal/json.Unmarshal) on requestData and user; replace those calls with the project's JSON wrappers: call common.Marshal(requestData) instead of json.Marshal and common.Unmarshal(requestDataBytes, &user) or better yet call common.Unmarshal directly on the original payload if available; update the references in UpdateSelf to use common.Marshal/common.Unmarshal so no direct encoding/json usage remains (operate on variables requestData and user and preserve existing error handling/return paths).controller/token.go (1)
40-41:⚠️ Potential issue | 🟡 MinorHandle
CountUserTokenserrors before returning paged data.Line 40 drops the error and can return incorrect
totalmetadata on DB failure.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/token.go` around lines 40 - 41, CountUserTokens error is ignored causing incorrect paging metadata; change the call in the token list handler to capture and check the error from model.CountUserTokens (e.g., total, err := model.CountUserTokens(dto.UserID(c))) and if err != nil return an appropriate error response (or wrap/log and return dto.Error/dto.InternalServerError) instead of proceeding to dto.OkPage with a possibly invalid total; keep using buildMaskedTokenResponses(tokens) for the data payload when no error occurs.controller/codex_oauth.go (2)
93-99:⚠️ Potential issue | 🟠 MajorHandle
session.Save()failures in both start and cleanup flows.At Line 97 and Line 192, save errors are ignored. If save fails, the flow can return success while state/verifier persistence (or cleanup) did not happen.
Also applies to: 189-193
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/codex_oauth.go` around lines 93 - 99, The session.Save() result is currently ignored in both the OAuth start and cleanup flows; update the code around sessions.Default(ginCtx) usage (where session.Set(... codexOAuthSessionKey(...))) to capture and handle the error returned by session.Save() — in the start handler (returning dto.Ok(dto.CodexOAuthStartData{AuthorizeURL: flow.AuthorizeURL})) and in the cleanup handler remove/clear logic, check the error from session.Save() and if non-nil return an appropriate error response (or log and return an internal error) instead of proceeding as success so state/verifier persistence and cleanup failures are surfaced. Ensure you reference the same codexOAuthSessionKey, flow.State/flow.Verifier, and session.Save() calls when implementing the checks.
96-97:⚠️ Potential issue | 🟠 MajorEnforce OAuth flow TTL using stored
created_at.Line 96 stores
created_at, but completion only validatesstate/verifier(Line 150-155). Expired flows are not rejected, so verifier lifetime is effectively session-bound.Also applies to: 150-155
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/codex_oauth.go` around lines 96 - 97, The OAuth completion handler currently only checks state/verifier but ignores the saved timestamp set via session.Set(codexOAuthSessionKey(channelID, "created_at"), ...), allowing expired flows to succeed; update the completion path (the code that checks state/verifier around lines 150-155) to read the stored created_at using session.Get(codexOAuthSessionKey(channelID, "created_at")), compare it to time.Now().Unix() using your configured TTL (e.g., oauthTTL), and reject the flow (clear relevant session keys with session.Delete and session.Save) when created_at+TTL < now, returning the appropriate error/HTTP response; keep using codexOAuthSessionKey to locate the keys so changes remain localized.controller/channel-billing.go (1)
4-4:⚠️ Potential issue | 🟠 MajorReplace direct
encoding/jsonunmarshal calls withcommon.Unmarshal.Direct
json.Unmarshalremains in changed paths (e.g., Line 79, Line 94, Line 118, etc.), which violates repo JSON rules. Please switch these callsites and removeencoding/jsonimport from Line 4.♻️ Minimal pattern to apply at each site
-import "encoding/json" +// remove encoding/json import -err = json.Unmarshal(body, &response) +err = common.Unmarshal(body, &response)As 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).Also applies to: 79-80, 94-95, 118-119, 137-138, 152-153, 174-175, 203-204, 218-219, 248-249, 301-302, 317-318
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/channel-billing.go` at line 4, Replace all direct uses of json.Unmarshal in controller/channel-billing.go with the repo wrapper common.Unmarshal and remove the encoding/json import. Specifically, locate each json.Unmarshal callsite in this file (the multiple occurrences noted in the review) and change the call to common.Unmarshal(target, data) or the appropriate common.UnmarshalJsonStr/common.DecodeJson variant to match the input type, then update error handling to use the returned error from common.Unmarshal. Finally, delete the unused "encoding/json" import from the file header.router/video-router.go (1)
19-19:⚠️ Potential issue | 🟠 MajorDocument media proxy response correctly.
Line 19 documents
/videos/:task_id/contentasdto.TaskResponseDoc, but this endpoint is a content proxy and should be modeled as binary/media response in OpenAPI.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@router/video-router.go` at line 19, The route proxy.GinGet("/videos/:task_id/content", controller.VideoProxy, dto.GinResp[dto.TaskResponseDoc]()) is incorrectly documented as dto.TaskResponseDoc; update the OpenAPI docs to model this endpoint as a binary/media response instead: replace dto.TaskResponseDoc with a media response schema (e.g., dto.MediaResponseDoc or a new MediaBinaryDoc) in the dto.GinResp call for the "/videos/:task_id/content" route and ensure the schema declares binary format and appropriate content-types (video/* or application/octet-stream) so controller.VideoProxy is documented as returning raw media.controller/topup.go (1)
136-137:⚠️ Potential issue | 🟡 MinorAlign minimum payable threshold checks across endpoints.
Line 136 uses
< 0.01, while Line 314 uses<= 0.01. This makes quote and payment validation inconsistent for0.01.💡 Minimal fix
- if payMoney < 0.01 { + if payMoney <= 0.01 { return dto.Fail[dto.EpayPayResponse]("充值金额过低") }Also applies to: 314-315
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/topup.go` around lines 136 - 137, The minimum-payable check is inconsistent: one place uses "if payMoney < 0.01" and another uses "<= 0.01", causing 0.01 to be treated differently across endpoints; change the condition(s) so both use the same comparison (for example, replace "if payMoney < 0.01" with "if payMoney <= 0.01") to make validation consistent for the payMoney threshold, and update the matching check at the other location (the check at ~line 314) if needed so both use the identical operator.
🧹 Nitpick comments (9)
web/src/components/topup/index.jsx (1)
574-580: Extract duplicated amount-response handling into one helper.
getAmountandgetStripeAmountnow contain near-identical success/error parsing. A shared helper will reduce drift and keep toast behavior consistent.Refactor sketch
+ const applyAmountResponse = ({ success, message, data }, toastId) => { + if (success) { + setAmount(parseFloat(data)); + return; + } + setAmount(0); + Toast.error({ content: t('错误：') + (message || data), id: toastId }); + }; @@ - const { success, message, data } = res.data; - if (success) { - setAmount(parseFloat(data)); - } else { - setAmount(0); - Toast.error({ content: t('错误：') + (message || data), id: 'getAmount' }); - } + applyAmountResponse(res.data, 'getAmount'); @@ - const { success, message, data } = res.data; - if (success) { - setAmount(parseFloat(data)); - } else { - setAmount(0); - Toast.error({ content: t('错误：') + (message || data), id: 'getAmount' }); - } + applyAmountResponse(res.data, 'getStripeAmount');Also applies to: 600-606
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/topup/index.jsx` around lines 574 - 580, Extract the duplicated response handling in getAmount and getStripeAmount into a single helper (e.g., handleAmountResponse) that accepts the response object (res.data), calls setAmount(parseFloat(data)) on success and on failure calls setAmount(0) and Toast.error with t('错误：') + (message || data) and a consistent id ('getAmount'); then replace the duplicated blocks in getAmount and getStripeAmount with a call to handleAmountResponse(res.data) so both functions share the same parsing, state update (setAmount), and toast behavior.types/user_settings.go (1)
1-25: Preferconstfor the notify-type tokens.These are immutable enum-like strings; declaring them as
constrather thanvarsignals intent and reflects their actual usage throughout the codebase. The existing re-export indto/type_aliases.goensures backward compatibility for all call sites referencingdto.NotifyType*.♻️ Suggested change
-var ( +const ( NotifyTypeEmail = "email" // Email 邮件 NotifyTypeWebhook = "webhook" // Webhook NotifyTypeBark = "bark" // Bark 推送 NotifyTypeGotify = "gotify" // Gotify 推送 )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@types/user_settings.go` around lines 1 - 25, The constants for notify-type tokens are declared as mutable vars; change them to compile-time constants to reflect immutability and intent. Replace the var block that declares NotifyTypeEmail, NotifyTypeWebhook, NotifyTypeBark, and NotifyTypeGotify with a const block in the types/user_settings.go file so these enum-like strings are true constants while preserving their names for callers (including dto/type_aliases.go re-exports).dto/params.go (2)
52-60: Header parameters don't support slice/array types.Unlike query parameters (lines 30-42), header parameters only handle scalar values. If multi-value headers are needed (e.g., comma-separated), this would need additional handling.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dto/params.go` around lines 52 - 60, Header-tagged struct fields currently only read scalar header values via c.Header(tag) and call setParamValue with field.Type.Kind(), so slice/array types are ignored; update the header handling branch (the code using field.Tag.Get("header"), c.Header, and setParamValue) to detect if field.Type.Kind() is slice or array and, for those cases, split the header string (e.g., by commas), trim elements, and convert each element to the slice element type before setting the field (reusing or extending setParamValue logic to accept slice conversions), while leaving scalar handling unchanged.
28-51: Pointer field support is not implemented but not currently needed.The code passes
field.Type.Kind()directly tosetParamValue, so pointer types like*intwould fall through to the default case and error. However, existing param structs indto/query_params.gouse only non-pointer scalars (int,int64,string), so this limitation has no impact.If pointer fields are used in the future to distinguish absent from zero values, add pointer dereferencing logic to handle them:
Optional enhancement for pointer support
default: paramValue := c.QueryParam(tag) if paramValue == "" { continue } + // Handle pointer types + targetValue := fieldValue + targetKind := field.Type.Kind() + if targetKind == reflect.Pointer { + if fieldValue.IsNil() { + fieldValue.Set(reflect.New(field.Type.Elem())) + } + targetValue = fieldValue.Elem() + targetKind = field.Type.Elem().Kind() + } - if err := setParamValue(fieldValue, paramValue, field.Type.Kind()); err != nil { + if err := setParamValue(targetValue, paramValue, targetKind); err != nil { return *p, err }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dto/params.go` around lines 28 - 51, The current query param binding passes field.Type.Kind() to setParamValue which fails for pointer fields (e.g., *int); update the logic in the block that handles query tags to detect pointer kinds for both scalar and slice elements, dereference types/values before calling setParamValue (use field.Type.Elem() and fieldValue.Elem() when Kind() == reflect.Ptr), and ensure you allocate and set pointer values appropriately so setParamValue receives the underlying non-pointer kind; reference the existing symbols field.Type.Kind(), field.Type.Elem(), fieldValue, setParamValue, c.QueryParam and c.QueryParamArr when making the change.dto/user.go (1)
33-36: Consider using*intforGotifyPriorityto distinguish "not provided" from "explicitly set to 0".
GotifyPriorityis anintwithomitempty, which means if a user explicitly sets priority to0, it will be indistinguishable from "not provided" when the struct is marshaled or when checking if the field was set. If0is a valid priority value that users might intentionally set, consider using*int.Suggested change
- GotifyPriority int `json:"gotify_priority,omitempty"` + GotifyPriority *int `json:"gotify_priority,omitempty"`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dto/user.go` around lines 33 - 36, GotifyPriority should be changed from int to *int so callers can distinguish "not provided" (nil) from an explicit 0; update the struct field GotifyPriority to type *int (keeping the `json:"gotify_priority,omitempty"` tag), then update all code that reads or writes GotifyPriority (e.g., any unmarshalling, validation, defaulting, or comparisons) to handle nil safely (use nil checks or helper accessor that returns a default when nil) and adjust any constructors/tests that set or expect an int to instead use pointer values.router/openapi.go (1)
16-29: Consider handlingw.Writeerror.Line 27 ignores the error returned by
w.Write. While unlikely to fail for small static content, best practice is to handle or explicitly discard with a comment.Proposed fix
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "text/html; charset=utf-8") - w.Write([]byte(page)) + _, _ = w.Write([]byte(page)) // Error handling omitted for static content })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@router/openapi.go` around lines 16 - 29, In scalarUIHandler's returned http.HandlerFunc, handle the error from w.Write instead of ignoring it: capture the (n, err) = w.Write([]byte(page)) result inside the closure and either log the error (using the request context logger or http server logger) or explicitly discard it with a comment; ensure you still set the Content-Type header before writing and return after logging/handling the write error to avoid silent failures in the scalarUIHandler response path.controller/prefill_group.go (1)
44-64: Guard against zero-value overwrites inUpdatePrefillGroup.Line 50 validates only
Id. With struct binding, missing fields become zero-values, soName/Typecan be unintentionally cleared during update.Suggested safeguard
func UpdatePrefillGroup(c fuego.ContextWithBody[model.PrefillGroup]) (*dto.Response[model.PrefillGroup], error) { g, err := c.Body() if err != nil { return dto.Fail[model.PrefillGroup](err.Error()) } if g.Id == 0 { return dto.Fail[model.PrefillGroup]("缺少组 ID") } + if g.Name == "" || g.Type == "" { + return dto.Fail[model.PrefillGroup]("组名称和类型不能为空") + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/prefill_group.go` around lines 44 - 64, UpdatePrefillGroup currently takes the bound PrefillGroup struct (g) and calls g.Update(), which will overwrite stored fields with zero-values from missing JSON fields; instead, load the existing record by ID (e.g. fetch via model.GetPrefillGroupById or similar), merge only non-zero / non-empty fields from the incoming g into the loaded entity (preserve existing Name/Type when incoming values are zero/empty), perform the duplicate-name check against the merged name using model.IsPrefillGroupNameDuplicated, and then call Update() on the merged entity so zero-value binding cannot unintentionally clear persisted fields.controller/custom_oauth.go (1)
320-333: Avoid N+1 provider lookups when building OAuth binding responses.Line 322 performs a provider query per binding. Consider preloading providers in one query and mapping by
ProviderIdbefore the loop.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/custom_oauth.go` around lines 320 - 333, The loop in controller/custom_oauth.go is doing N+1 queries by calling model.GetCustomOAuthProviderById for each binding; instead, fetch all relevant providers in one call (e.g., model.GetCustomOAuthProvidersByIds or extend model to accept a slice of IDs), build a map[int]Provider keyed by ProviderId, then iterate bindings and look up provider := providersMap[binding.ProviderId] to populate dto.UserOAuthBindingResponse (keeping the same fields ProviderId, ProviderName, ProviderSlug, ProviderIcon, ProviderUserId) and continue/skip if the provider is missing; this removes per-binding DB calls and uses a single bulk query plus in-memory mapping.dto/router.go (1)
16-17: ScopeusedOperationIDsto router/engine lifecycle instead of package-global state.
usedOperationIDsat Line 17 is process-global, so repeated router setup in the same process can silently drop operation IDs after the first initialization (Line 50-53 fallback can also be exhausted). Prefer storing this map onRouteror per-engine init scope.Also applies to: 50-53
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dto/router.go` around lines 16 - 17, The package-global var usedOperationIDs should be moved onto the Router lifecycle: add a usedOperationIDs field to the Router struct and initialize it when creating the Router (e.g., in the Router constructor/NewRouter or engine init path), then replace all references to the package var (usedOperationIDs) with the receiver field (r.usedOperationIDs) including the fallback logic currently at lines ~50-53 so each Router/engine instance has its own map rather than a process-global map.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5efecf58-f5cc-4983-9d38-4ef72bc0a3b8
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (120)
AGENTS.mdCLAUDE.mdDockerfileREADME.fr.mdREADME.ja.mdREADME.mdREADME.zh_CN.mdREADME.zh_TW.mdcommon/gin.gocontroller/billing.gocontroller/channel-billing.gocontroller/channel-test.gocontroller/channel.gocontroller/channel_affinity_cache.gocontroller/channel_upstream_update.gocontroller/checkin.gocontroller/codex_oauth.gocontroller/codex_usage.gocontroller/console_migrate.gocontroller/custom_oauth.gocontroller/deployment.gocontroller/group.gocontroller/image.gocontroller/log.gocontroller/midjourney.gocontroller/misc.gocontroller/missing_models.gocontroller/model.gocontroller/model_meta.gocontroller/model_sync.gocontroller/oauth.gocontroller/option.gocontroller/passkey.gocontroller/performance.gocontroller/prefill_group.gocontroller/pricing.gocontroller/ratio_config.gocontroller/ratio_sync.gocontroller/redemption.gocontroller/relay.gocontroller/secure_verification.gocontroller/setup.gocontroller/subscription.gocontroller/subscription_payment_creem.gocontroller/subscription_payment_epay.gocontroller/subscription_payment_stripe.gocontroller/swag_video.gocontroller/task.gocontroller/telegram.gocontroller/token.gocontroller/topup.gocontroller/topup_creem.gocontroller/topup_stripe.gocontroller/twofa.gocontroller/uptime_kuma.gocontroller/usedata.gocontroller/user.gocontroller/vendor_meta.gocontroller/wechat.godocker-compose.ymldocs/openapi/api.jsondocs/openapi/relay.jsondto/api_responses.godto/channel.godto/channel_affinity_cache.godto/channel_billing.godto/channel_testing.godto/checkin.godto/codex_usage.godto/custom_oauth.godto/deployment.godto/group.godto/misc.godto/model_meta.godto/model_sync.godto/openai_request.godto/option.godto/params.godto/performance.godto/pricing.godto/query_params.godto/ratio_sync_result.godto/responses.godto/router.godto/setup.godto/subscription.godto/token.godto/topup.godto/topup_creem.godto/twofa.godto/type_aliases.godto/uptime_kuma.godto/user.gogo.modmodel/channel.gomodel/task.gomodel/twofa.gomodel/user.gomodel/user_cache.gorelay/channel/claude/relay-claude.gorouter/api-router.gorouter/dashboard.gorouter/main.gorouter/openapi.gorouter/relay-router.gorouter/video-router.goservice/openaicompat/chat_to_responses.goservice/openaicompat/responses_to_chat.gosetting/chat.gosetting/console_setting/validation.gosetting/ratio_setting/exposed_cache.gotypes/channel_settings.gotypes/oauth_errors.gotypes/openai_video.gotypes/user_settings.goweb/src/components/table/model-deployments/modals/EditDeploymentModal.jsxweb/src/components/table/usage-logs/UsageLogsColumnDefs.jsxweb/src/components/topup/SubscriptionPlansCard.jsxweb/src/components/topup/index.jsxweb/src/hooks/models/useModelsData.jsx
💤 Files with no reviewable changes (3)
- controller/swag_video.go
- web/src/components/table/usage-logs/UsageLogsColumnDefs.jsx
- controller/image.go
🚧 Files skipped from review as they are similar to previous changes (32)
- dto/group.go
- service/openaicompat/chat_to_responses.go
- controller/billing.go
- dto/setup.go
- README.ja.md
- setting/chat.go
- model/task.go
- model/twofa.go
- types/openai_video.go
- relay/channel/claude/relay-claude.go
- docker-compose.yml
- dto/performance.go
- controller/checkin.go
- dto/uptime_kuma.go
- controller/channel_upstream_update.go
- model/channel.go
- setting/ratio_setting/exposed_cache.go
- controller/relay.go
- web/src/components/topup/SubscriptionPlansCard.jsx
- dto/subscription.go
- README.zh_TW.md
- dto/codex_usage.go
- Dockerfile
- web/src/components/table/model-deployments/modals/EditDeploymentModal.jsx
- controller/setup.go
- controller/console_migrate.go
- dto/openai_request.go
- dto/channel_affinity_cache.go
- controller/usedata.go
- dto/token.go
- dto/deployment.go
- README.zh_CN.md
0-don
force-pushed
the
feat/openapi-fuego
branch
from
00a4b43 to
9be3016
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
setting/console_setting/validation.go (1)
3-4:⚠️ Potential issue | 🟡 MinorDirect
encoding/jsonusage violates coding guidelines.The
parseJSONArrayfunction usesjson.Unmarshaldirectly (line 30), but coding guidelines require all JSON operations to use wrapper functions fromcommon/json.go. The getter functions were correctly updated to usecommon.Unmarshal, but this validation helper was missed.Proposed fix
Remove the
encoding/jsonimport and updateparseJSONArrayto usecommon.Unmarshal:import ( - "encoding/json" "fmt" "net/url" "regexp"func parseJSONArray(jsonStr string, typeName string) ([]map[string]interface{}, error) { var list []map[string]interface{} - if err := json.Unmarshal([]byte(jsonStr), &list); err != nil { + if err := common.Unmarshal([]byte(jsonStr), &list); err != nil { return nil, fmt.Errorf("%s格式错误：%s", typeName, err.Error()) } return list, nil }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."Also applies to: 28-34
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@setting/console_setting/validation.go` around lines 3 - 4, The parseJSONArray helper currently calls json.Unmarshal and imports encoding/json; remove the encoding/json import and replace the direct json.Unmarshal call in parseJSONArray with the project's wrapper common.Unmarshal (importing the common package if not already present), ensuring error handling remains the same; update any related import block to remove encoding/json and add common so all JSON unmarshal behavior uses common.Unmarshal instead of encoding/json.model/user.go (1)
79-97:⚠️ Potential issue | 🟠 MajorUse
commonJSON wrappers instead ofencoding/jsonin these updated methods.
GetSetting/SetSettingstill directly calljson.Unmarshal/json.Marshal, which violates the repository JSON wrapper rule.🔧 Proposed fix
func (user *User) GetSetting() types.UserSetting { setting := types.UserSetting{} if user.Setting != "" { - err := json.Unmarshal([]byte(user.Setting), &setting) + err := common.Unmarshal([]byte(user.Setting), &setting) if err != nil { common.SysLog("failed to unmarshal setting: " + err.Error()) } } return setting } func (user *User) SetSetting(setting types.UserSetting) { - settingBytes, err := json.Marshal(setting) + settingBytes, err := common.Marshal(setting) if err != nil { common.SysLog("failed to marshal setting: " + err.Error()) return } user.Setting = string(settingBytes) }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.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@model/user.go` around lines 79 - 97, Replace direct calls to encoding/json in User.GetSetting and User.SetSetting with the repository's common JSON wrapper functions (e.g., common.UnmarshalJSON and common.MarshalJSON or the actual wrapper names in common/json.go); call the wrapper to unmarshal user.Setting into types.UserSetting and to marshal the setting before assigning user.Setting, preserving the existing error logging via common.SysLog when the wrapper returns an error, and remove any direct use/import of encoding/json in this file.controller/subscription_payment_stripe.go (1)
52-68:⚠️ Potential issue | 🟠 MajorMake the purchase-cap check and order creation atomic.
CountUserSubscriptionsByPlanruns before any write, andgenStripeSubscriptionLinkruns before the pending order exists locally. Two concurrent requests can both pass the cap, and a laterorder.Insert()failure leaves a live Checkout Session with nothing for the webhook to reconcile. Persist or reserve the order first, or make the flow idempotent end-to-end.Also applies to: 71-81
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/subscription_payment_stripe.go` around lines 52 - 68, The purchase-cap check and external Stripe Checkout creation must be atomic: instead of calling CountUserSubscriptionsByPlan then genStripeSubscriptionLink before persisting, create a pending reservation/order record first (use the generated referenceId from referenceId and store status="pending" with userId, planId, and Stripe reference) inside a DB transaction or by enforcing a unique/reservation constraint so concurrent requests cannot exceed MaxPurchasePerUser; then call genStripeSubscriptionLink and update the pending order with the Checkout session info, and ensure order.Insert() (or order.Update pending→active/cancelled) is retried/rolled back on failure to keep state consistent. Apply the same change to the other block that creates purchases (the code referenced in the comment, e.g., the block around genStripeSubscriptionLink/order.Insert()) to make the flow idempotent end-to-end.controller/user.go (1)
395-402:⚠️ Potential issue | 🟡 MinorReplace direct
json.Marshalwithcommon.Marshal.Line 395 uses
json.Marshal(defaultConfig)directly, violating the JSON wrapper requirement.As per coding guidelines: "All JSON marshal/unmarshal operations MUST use wrapper functions in
common/json.go."🛠️ Suggested fix
- configBytes, err := json.Marshal(defaultConfig) + configBytes, err := common.Marshal(defaultConfig)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/user.go` around lines 395 - 402, Replace the direct call to json.Marshal(defaultConfig) with the project's JSON wrapper common.Marshal (from common/json.go) inside the function that builds the default sidebar config; call common.Marshal(defaultConfig), check the returned error the same way, and pass the error message into common.SysLog exactly as before, returning the marshaled string on success. Ensure you remove the json.Marshal import usage if it becomes unused and keep the variable names (configBytes, err, defaultConfig) unchanged to match surrounding code.
♻️ Duplicate comments (11)
service/openaicompat/responses_to_chat.go (3)
444-445:⚠️ Potential issue | 🟡 MinorPreserve the upstream
createdtimestamp here.
CreatedAtis still derived from conversion time, so a round-tripped response looks newly generated. Useresp.Createdwhen it is non-zero and only fall back totime.Now()when the source timestamp is missing.Also applies to: 519-523
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@service/openaicompat/responses_to_chat.go` around lines 444 - 445, The CreatedAt value is being set from the conversion time (now) instead of preserving the upstream timestamp; update the code that sets CreatedAt to use resp.Created when resp.Created != 0 and only fall back to time.Now() (the current int timestamp stored in now) when resp.Created is zero. Locate the respID/now block and any other places noted (e.g., the second occurrence around the 519-523 region) and replace the unconditional use of now with a conditional that prefers resp.Created for the CreatedAt field.
263-293:⚠️ Potential issue | 🟠 MajorReturn an error for malformed
tools/tool_choice.These branches still swallow
common.Unmarshalfailures and keep going with empty/default tool settings. That mutates an invalid compat request into a different valid one instead of surfacing the bad payload to the caller.Also applies to: 297-321
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@service/openaicompat/responses_to_chat.go` around lines 263 - 293, The code currently swallows common.Unmarshal errors when decoding req.Tools (and similarly for the tool_choice branch at 297-321) and silently produces a mutated request; change the logic to return an error immediately when common.Unmarshal fails instead of continuing. Locate the block that calls common.Unmarshal(req.Tools, &tools) and, if err != nil, return a descriptive error (propagate or wrap the unmarshalling error) so the handler does not append empty/default dto.ToolCallRequest entries to out.Tools; apply the same change to the tool_choice unmarshalling path so malformed payloads are surfaced to the caller.
4-4:⚠️ Potential issue | 🟡 MinorRemove the direct
encoding/jsondependency here.This file only needs a constant JSON status payload, but the repo rule is to keep business-code JSON handling behind
common/json.go. Please build the"completed"payload viacommon.Marshaland assign the bytes instead of importingencoding/jsonhere. As per coding guidelines,**/*.go: All JSON marshal/unmarshal operations MUST use wrapper functions fromcommon/json.go(common.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson,common.GetJsonType()). Do NOT directly import or callencoding/jsonin business code.Also applies to: 519-523
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@service/openaicompat/responses_to_chat.go` at line 4, This file currently imports encoding/json to create a constant JSON "completed" payload; remove that direct dependency and instead build the payload using common.Marshal (from common/json.go) and assign the resulting []byte to the constant/variable used for the completed payload (replace any use of encoding/json.Marshal). Update the import list to remove "encoding/json" and call common.Marshal where the "completed" payload is constructed so all JSON operations use the common.Marshal wrapper per repo guidelines.controller/channel-test.go (1)
749-760:⚠️ Potential issue | 🟡 MinorMeasure elapsed time before returning the error response.
result.newAPIErroris always accompanied byresult.localErr, so the first error branch makes the later one dead and failed tests still reportTime: 0. ComputeconsumedTimeimmediately aftertestChannel(...)returns and use it for both success and failure paths.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/channel-test.go` around lines 749 - 760, The elapsed time is computed only after handling result.localErr so the result.newAPIError path never gets a real Time value; after calling testChannel(channel, testModel, endpointType, isStream) (where result, tik are set), immediately compute tok := time.Now(), milliseconds := tok.Sub(tik).Milliseconds(), consumedTime := float64(milliseconds)/1000.0 and then call go channel.UpdateResponseTime(milliseconds); then use consumedTime in both error returns and the success return so all paths (result.localErr, result.newAPIError, and success) report the measured time.dto/responses.go (1)
126-133:⚠️ Potential issue | 🟡 MinorGuard
OkPageagainst nil*common.PageInfo.
OkPageis exported and dereferencespunconditionally. Any caller that passesnilwill panic before the handler can return a response.Suggested fix
func OkPage[T any](p *common.PageInfo, items []T, total int) (*Response[PageData[T]], error) { + page, pageSize := 0, 0 + if p != nil { + page = p.GetPage() + pageSize = p.GetPageSize() + } return &Response[PageData[T]]{ Success: true, Data: PageData[T]{ - Page: p.GetPage(), - PageSize: p.GetPageSize(), + Page: page, + PageSize: pageSize, Total: total, Items: items, }, }, nil }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dto/responses.go` around lines 126 - 133, OkPage currently dereferences p (calling p.GetPage() and p.GetPageSize()) without checking for nil which can panic; add a nil-check at the start of OkPage and handle it gracefully (e.g., if p == nil return nil and a descriptive error using fmt.Errorf or return a Response with Success=false and an error) so callers won't panic; update references in the function to only call p.GetPage() and p.GetPageSize() after the nil-check and ensure the returned error message clearly mentions PageInfo being nil.controller/oauth.go (1)
30-33:⚠️ Potential issue | 🟠 MajorClear stale affiliate codes when the request omits
aff.Only setting
"aff"whenp.Aff != ""lets a previous invite code survive into a later OAuth flow in the same browser, so signups can be attributed to the wrong inviter.Suggested fix
p, _ := dto.ParseParams[dto.GenerateOAuthCodeParams](c) if p.Aff != "" { session.Set("aff", p.Aff) + } else { + session.Delete("aff") } session.Set("oauth_state", state)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/oauth.go` around lines 30 - 33, The code only sets session.Set("aff", p.Aff) when p.Aff != "" so a previous affiliate code can persist; update the GenerateOAuthCodeParams handling (dto.ParseParams / p.Aff) to explicitly clear stale aff values when p.Aff is empty by calling session.Delete("aff") (or session.Set("aff", "") if your session API lacks Delete) in the else branch instead of doing nothing, ensuring each OAuth request either sets the new aff or removes any prior one.controller/channel-billing.go (1)
78-80:⚠️ Potential issue | 🟠 MajorUse
common.Unmarshalin these updated balance parsers.These DTO migrations still deserialize provider payloads with
json.Unmarshal, which bypasses the repo wrappers and violates the JSON handling rule. Please switch each touched site tocommon.Unmarshal.Minimal fix pattern
- err = json.Unmarshal(body, &response) + err = common.Unmarshal(body, &response)As 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 call encoding/json in business code.Also applies to: 93-95, 117-119, 136-138, 151-153, 173-175, 202-204, 217-219, 300-302, 316-318
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/channel-billing.go` around lines 78 - 80, Replace direct calls to json.Unmarshal with the project's wrapper common.Unmarshal in all updated balance parsers (e.g., where response := dto.OpenAICreditGrants{} and err = json.Unmarshal(body, &response) is used) so JSON handling consistently uses common/json.go; update each occurrence noted (around the blocks at ~78-80, 93-95, 117-119, 136-138, 151-153, 173-175, 202-204, 217-219, 300-302, 316-318) to call common.Unmarshal(body, &<responseVar>) and preserve existing error handling and variable names (e.g., response, grantsResp, balanceResp) so behavior and error flow remain identical.controller/vendor_meta.go (2)
16-18:⚠️ Potential issue | 🟠 MajorHandle
Counterrors before returning pagination metadata.If the count query fails here, this still returns a successful page with
total=0, which makes pagination silently drift from the actual dataset.Suggested fix
var total int64 - model.DB.Model(&model.Vendor{}).Count(&total) + if err := model.DB.Model(&model.Vendor{}).Count(&total).Error; err != nil { + return dto.FailPage[*model.Vendor](err.Error()) + } return dto.OkPage(pageInfo, vendors, int(total))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/vendor_meta.go` around lines 16 - 18, The Count call on model.DB.Model(&model.Vendor{}).Count(&total) can fail but its error is ignored, causing dto.OkPage(pageInfo, vendors, int(total)) to return an incorrect total; update the code to capture and handle the error returned by Count (e.g., err := model.DB.Model(&model.Vendor{}).Count(&total)), and if err != nil return an appropriate error response (or log and return dto.Err/HTTP error) instead of continuing to call dto.OkPage; keep the rest of the flow (vendors, pageInfo) intact so only the Count path is validated before returning.
73-80:⚠️ Potential issue | 🟠 MajorReject empty vendor names on update too.
CreateVendorMetaalready blocks blank names, but this path skips the same guard and can run the duplicate check and update against an invalid payload.Suggested fix
if v.Id == 0 { return dto.Fail[model.Vendor]("缺少供应商 ID") } + if v.Name == "" { + return dto.Fail[model.Vendor]("供应商名称不能为空") + } // 名称冲突检查 if dup, err := model.IsVendorNameDuplicated(v.Id, v.Name); err != nil {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/vendor_meta.go` around lines 73 - 80, The update path currently skips rejecting blank vendor names; add the same guard used in CreateVendorMeta to validate v.Name (e.g., check strings.TrimSpace(v.Name) == "" and return dto.Fail[model.Vendor]("缺少供应商名称") or the same error message used by CreateVendorMeta) before calling model.IsVendorNameDuplicated and performing the update so empty names are rejected early; ensure you reference the v.Name field and keep the duplicate check (model.IsVendorNameDuplicated) only after this validation.controller/topup.go (1)
174-176:⚠️ Potential issue | 🟡 MinorUse the same pay-amount floor as
RequestAmount.This branch still accepts
payMoney == 0.01, while Line 367 rejects it. Quote and checkout can therefore disagree for the same request.Suggested fix
- if payMoney < 0.01 { + if payMoney <= 0.01 { return dto.Fail[dto.EpayPayResponse]("充值金额过低") }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/topup.go` around lines 174 - 176, The current check allows payMoney == 0.01 while the RequestAmount validation (referenced as RequestAmount) rejects 0.01, causing inconsistent behavior; update the condition that now reads "if payMoney < 0.01" to use the same floor logic as RequestAmount (e.g., "if payMoney <= RequestAmount" or "if payMoney <= 0.01") so 0.01 is rejected consistently, and keep the same dto.Fail[dto.EpayPayResponse] return path.controller/token.go (1)
38-41:⚠️ Potential issue | 🟡 MinorDon't drop the
CountUserTokenserror.Line 40 ignores the error from
model.CountUserTokens. If the count query fails, this returns a page payload with potentially incorrect pagination metadata.💡 Suggested fix
tokens, err := model.GetAllUserTokens(dto.UserID(c), page.GetStartIdx(), page.GetPageSize()) if err != nil { return dto.FailPage[*model.Token](err.Error()) } - total, _ := model.CountUserTokens(dto.UserID(c)) + total, err := model.CountUserTokens(dto.UserID(c)) + if err != nil { + return dto.FailPage[*model.Token](err.Error()) + } return dto.OkPage(page, buildMaskedTokenResponses(tokens), int(total))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/token.go` around lines 38 - 41, The call to model.CountUserTokens ignores its error, risking incorrect pagination; update the controller handler around CountUserTokens(dto.UserID(c)) to capture (total, err) and if err != nil return dto.FailPage[*model.Token](err.Error()) (or an appropriate dto error response) instead of proceeding to dto.OkPage; keep the rest of the response flow intact using the existing buildMaskedTokenResponses(tokens) and dto.OkPage when no error occurs.
🧹 Nitpick comments (1)
dto/router.go (1)
16-21: Keep route-registration state scoped to a router build.
usedOperationIDsandnoopEnginelive for the lifetime of the process, so a second initialization in the same process reuses claimed IDs and prior no-op registrations. That makes spec generation order-dependent and brittle in tests.Also applies to: 65-72
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dto/router.go` around lines 16 - 21, The package-level globals usedOperationIDs and noopEngine cause cross-build state reuse; instead create them per-router build (e.g., as fields on the router/builder struct or returned by NewRouter/NewBuilder) so each initialization gets its own sync.Map and fuego.NewEngine() instance. Replace references to the globals (usedOperationIDs, noopEngine) with the instance fields on the builder/router (or local variables constructed in the build function) so operation IDs and the no-op engine are scoped to a single build lifecycle.
🤖 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/misc.go`:
- Around line 230-246: The current flow leaks account existence by returning
dto.FailMsg when model.IsEmailAlreadyTaken(p.Email) is false; change it so the
endpoint always returns the same generic success response (e.g., dto.Msg(""))
regardless of whether the email exists, and only perform the actions that
generate codes and send mail (common.GenerateVerificationCode,
common.RegisterVerificationCodeWithKey, common.SendEmail) when
model.IsEmailAlreadyTaken(p.Email) is true; ensure you do not generate or
register codes or call common.SendEmail for unknown emails and preserve existing
error handling/logging when sending fails for known accounts.
- Around line 258-261: The error log in the ResetUserPassword flow currently
logs the raw email (req.Email) via common.SysError; modify the failure handling
in the block after model.ResetUserPasswordByEmail to avoid logging the full
email — either omit req.Email entirely or mask it (e.g., show only the domain or
first/last char) before passing to common.SysError, and keep the existing
err.Error() context; ensure the change is made where
model.ResetUserPasswordByEmail is called and common.SysError is invoked so logs
no longer contain unmasked user identifiers.
In `@controller/model_sync.go`:
- Around line 259-264: The SyncUpstreamModels function currently treats any
error from c.Body() as a bad request, but c.Body() can return io.EOF for an
empty body which should be allowed; update the error handling around the
c.Body() call so that if err == io.EOF you treat req as the zero-value (empty)
request and continue, while all other errors still return dto.Fail; reference
the SyncUpstreamModels function and the c.Body() call when applying this change.
In `@controller/prefill_group.go`:
- Around line 49-58: UpdatePrefillGroup currently allows clearing required
fields; add the same non-empty validation for g.Name and g.Type as in
CreatePrefillGroup inside the UpdatePrefillGroup handler before proceeding
(after checking g.Id and before calling model.IsPrefillGroupNameDuplicated),
returning dto.Fail[model.PrefillGroup] with the same messages used on create
when either Name or Type is empty; ensure the duplicate-name check
(model.IsPrefillGroupNameDuplicated) remains in place and only runs after the
new validation.
In `@controller/redemption.go`:
- Around line 78-81: The partial-failure return constructs a
dto.Response[[]string] but omits the Success field, causing inconsistent
responses; in the error branch where the code returns Message:
common.TranslateMessage(ginCtx, i18n.MsgRedemptionCreateFailed) and Data: keys,
set Success: false on the dto.Response to match the normal response shape
(locate the return that builds dto.Response[[]string] with Message and Data in
redemption.go and add Success: false alongside Message/Data).
In `@controller/telegram.go`:
- Around line 39-40: Replace occurrences where you return err.Error() in the
controller (the c.JSON(200, dto.ApiResponse{Message: err.Error()}) calls at the
three sites) with a generic client-facing message (e.g. "internal server error"
or "failed to process request"); instead log the original error server-side
using the existing logger (e.g. logger.Error/ log.Printf) so the internal
details are recorded but not exposed in ApiResponse.Message. Ensure you update
all three locations (the snippets at lines shown) and use the ApiResponse type
for the generic message.
In `@dto/params.go`:
- Around line 28-42: The code currently treats reflect.Array the same as
reflect.Slice and calls reflect.MakeSlice(field.Type, ...), which panics for
fixed-size arrays; update the branch that handles query tags in the params
parsing (the switch on field.Type.Kind used alongside fieldValue.Set, sliceType
and setParamValue) to either remove reflect.Array from the case so only slices
are handled, or add a separate path for reflect.Array that: checks the param
count equals the array length, creates a new array value via
reflect.New(field.Type).Elem() or similar, fills elements by calling
setParamValue for each index using field.Type.Elem(), and then calls
fieldValue.Set(arrayValue); ensure errors are returned when counts mismatch or
element parsing fails.
In `@router/api-router.go`:
- Around line 40-41: The current change repurposes GET /api/models for
DashboardListModels and moves GetAllModelsMeta to /api/models/list, breaking
existing admin clients; restore compatibility by keeping the legacy admin route
as an alias: register the original handler (GetAllModelsMeta) at "/models" in
addition to its new "/models/list" path (or alternatively move
DashboardListModels off "/models"), updating the router setup where
dto.NewRouter/ publicModels and dto.Get are used to ensure both
DashboardListModels and GetAllModelsMeta remain reachable (apply the same
aliasing/fix to the other affected block referenced in the review).
In `@router/relay-router.go`:
- Around line 166-169: The /v1/realtime WebSocket route is registered directly
via wsRouter and RelayRealtime so it won't appear in OpenAPI; add a parallel
OpenAPI registration using the project's router generator (e.g., call
dto.NewRouter / Fuego registration for the "/v1/realtime" path and associate it
with RelayRealtime and the Distribute() middleware) so the endpoint is included
in /openapi.json, or if websockets are intentionally excluded, add an explicit
comment near wsRouter/RelayRealtime stating they are out of OpenAPI scope.
- Around line 156-157: The POST relay routes (e.g., the registration call
pg.GinPost("/chat/completions", controller.Playground,
dto.GinResp[dto.ChatCompletionResponse]())) only include response annotations,
so the OpenAPI generator has no request-body type; update each raw POST route
registration (including the blocks around lines referenced) to also include the
appropriate GinBody[...]() annotation for the incoming payload (for example
GinBody[dto.ChatCompletionRequest]() for /chat/completions and the corresponding
DTO types for the other endpoints at 177-205, 242-243, 265-275, 278-280) so the
generator can infer and emit the request body schemas. Ensure you add the
GinBody[...]() argument before or alongside dto.GinResp[...]() in the pg.GinPost
calls for each affected handler (e.g., controller.Playground and the other
controller handlers).
---
Outside diff comments:
In `@controller/subscription_payment_stripe.go`:
- Around line 52-68: The purchase-cap check and external Stripe Checkout
creation must be atomic: instead of calling CountUserSubscriptionsByPlan then
genStripeSubscriptionLink before persisting, create a pending reservation/order
record first (use the generated referenceId from referenceId and store
status="pending" with userId, planId, and Stripe reference) inside a DB
transaction or by enforcing a unique/reservation constraint so concurrent
requests cannot exceed MaxPurchasePerUser; then call genStripeSubscriptionLink
and update the pending order with the Checkout session info, and ensure
order.Insert() (or order.Update pending→active/cancelled) is retried/rolled back
on failure to keep state consistent. Apply the same change to the other block
that creates purchases (the code referenced in the comment, e.g., the block
around genStripeSubscriptionLink/order.Insert()) to make the flow idempotent
end-to-end.
In `@controller/user.go`:
- Around line 395-402: Replace the direct call to json.Marshal(defaultConfig)
with the project's JSON wrapper common.Marshal (from common/json.go) inside the
function that builds the default sidebar config; call
common.Marshal(defaultConfig), check the returned error the same way, and pass
the error message into common.SysLog exactly as before, returning the marshaled
string on success. Ensure you remove the json.Marshal import usage if it becomes
unused and keep the variable names (configBytes, err, defaultConfig) unchanged
to match surrounding code.
In `@model/user.go`:
- Around line 79-97: Replace direct calls to encoding/json in User.GetSetting
and User.SetSetting with the repository's common JSON wrapper functions (e.g.,
common.UnmarshalJSON and common.MarshalJSON or the actual wrapper names in
common/json.go); call the wrapper to unmarshal user.Setting into
types.UserSetting and to marshal the setting before assigning user.Setting,
preserving the existing error logging via common.SysLog when the wrapper returns
an error, and remove any direct use/import of encoding/json in this file.
In `@setting/console_setting/validation.go`:
- Around line 3-4: The parseJSONArray helper currently calls json.Unmarshal and
imports encoding/json; remove the encoding/json import and replace the direct
json.Unmarshal call in parseJSONArray with the project's wrapper
common.Unmarshal (importing the common package if not already present), ensuring
error handling remains the same; update any related import block to remove
encoding/json and add common so all JSON unmarshal behavior uses
common.Unmarshal instead of encoding/json.
---
Duplicate comments:
In `@controller/channel-billing.go`:
- Around line 78-80: Replace direct calls to json.Unmarshal with the project's
wrapper common.Unmarshal in all updated balance parsers (e.g., where response :=
dto.OpenAICreditGrants{} and err = json.Unmarshal(body, &response) is used) so
JSON handling consistently uses common/json.go; update each occurrence noted
(around the blocks at ~78-80, 93-95, 117-119, 136-138, 151-153, 173-175,
202-204, 217-219, 300-302, 316-318) to call common.Unmarshal(body,
&<responseVar>) and preserve existing error handling and variable names (e.g.,
response, grantsResp, balanceResp) so behavior and error flow remain identical.
In `@controller/channel-test.go`:
- Around line 749-760: The elapsed time is computed only after handling
result.localErr so the result.newAPIError path never gets a real Time value;
after calling testChannel(channel, testModel, endpointType, isStream) (where
result, tik are set), immediately compute tok := time.Now(), milliseconds :=
tok.Sub(tik).Milliseconds(), consumedTime := float64(milliseconds)/1000.0 and
then call go channel.UpdateResponseTime(milliseconds); then use consumedTime in
both error returns and the success return so all paths (result.localErr,
result.newAPIError, and success) report the measured time.
In `@controller/oauth.go`:
- Around line 30-33: The code only sets session.Set("aff", p.Aff) when p.Aff !=
"" so a previous affiliate code can persist; update the GenerateOAuthCodeParams
handling (dto.ParseParams / p.Aff) to explicitly clear stale aff values when
p.Aff is empty by calling session.Delete("aff") (or session.Set("aff", "") if
your session API lacks Delete) in the else branch instead of doing nothing,
ensuring each OAuth request either sets the new aff or removes any prior one.
In `@controller/token.go`:
- Around line 38-41: The call to model.CountUserTokens ignores its error,
risking incorrect pagination; update the controller handler around
CountUserTokens(dto.UserID(c)) to capture (total, err) and if err != nil return
dto.FailPage[*model.Token](err.Error()) (or an appropriate dto error response)
instead of proceeding to dto.OkPage; keep the rest of the response flow intact
using the existing buildMaskedTokenResponses(tokens) and dto.OkPage when no
error occurs.
In `@controller/topup.go`:
- Around line 174-176: The current check allows payMoney == 0.01 while the
RequestAmount validation (referenced as RequestAmount) rejects 0.01, causing
inconsistent behavior; update the condition that now reads "if payMoney < 0.01"
to use the same floor logic as RequestAmount (e.g., "if payMoney <=
RequestAmount" or "if payMoney <= 0.01") so 0.01 is rejected consistently, and
keep the same dto.Fail[dto.EpayPayResponse] return path.
In `@controller/vendor_meta.go`:
- Around line 16-18: The Count call on
model.DB.Model(&model.Vendor{}).Count(&total) can fail but its error is ignored,
causing dto.OkPage(pageInfo, vendors, int(total)) to return an incorrect total;
update the code to capture and handle the error returned by Count (e.g., err :=
model.DB.Model(&model.Vendor{}).Count(&total)), and if err != nil return an
appropriate error response (or log and return dto.Err/HTTP error) instead of
continuing to call dto.OkPage; keep the rest of the flow (vendors, pageInfo)
intact so only the Count path is validated before returning.
- Around line 73-80: The update path currently skips rejecting blank vendor
names; add the same guard used in CreateVendorMeta to validate v.Name (e.g.,
check strings.TrimSpace(v.Name) == "" and return
dto.Fail[model.Vendor]("缺少供应商名称") or the same error message used by
CreateVendorMeta) before calling model.IsVendorNameDuplicated and performing the
update so empty names are rejected early; ensure you reference the v.Name field
and keep the duplicate check (model.IsVendorNameDuplicated) only after this
validation.
In `@dto/responses.go`:
- Around line 126-133: OkPage currently dereferences p (calling p.GetPage() and
p.GetPageSize()) without checking for nil which can panic; add a nil-check at
the start of OkPage and handle it gracefully (e.g., if p == nil return nil and a
descriptive error using fmt.Errorf or return a Response with Success=false and
an error) so callers won't panic; update references in the function to only call
p.GetPage() and p.GetPageSize() after the nil-check and ensure the returned
error message clearly mentions PageInfo being nil.
In `@service/openaicompat/responses_to_chat.go`:
- Around line 444-445: The CreatedAt value is being set from the conversion time
(now) instead of preserving the upstream timestamp; update the code that sets
CreatedAt to use resp.Created when resp.Created != 0 and only fall back to
time.Now() (the current int timestamp stored in now) when resp.Created is zero.
Locate the respID/now block and any other places noted (e.g., the second
occurrence around the 519-523 region) and replace the unconditional use of now
with a conditional that prefers resp.Created for the CreatedAt field.
- Around line 263-293: The code currently swallows common.Unmarshal errors when
decoding req.Tools (and similarly for the tool_choice branch at 297-321) and
silently produces a mutated request; change the logic to return an error
immediately when common.Unmarshal fails instead of continuing. Locate the block
that calls common.Unmarshal(req.Tools, &tools) and, if err != nil, return a
descriptive error (propagate or wrap the unmarshalling error) so the handler
does not append empty/default dto.ToolCallRequest entries to out.Tools; apply
the same change to the tool_choice unmarshalling path so malformed payloads are
surfaced to the caller.
- Line 4: This file currently imports encoding/json to create a constant JSON
"completed" payload; remove that direct dependency and instead build the payload
using common.Marshal (from common/json.go) and assign the resulting []byte to
the constant/variable used for the completed payload (replace any use of
encoding/json.Marshal). Update the import list to remove "encoding/json" and
call common.Marshal where the "completed" payload is constructed so all JSON
operations use the common.Marshal wrapper per repo guidelines.
---
Nitpick comments:
In `@dto/router.go`:
- Around line 16-21: The package-level globals usedOperationIDs and noopEngine
cause cross-build state reuse; instead create them per-router build (e.g., as
fields on the router/builder struct or returned by NewRouter/NewBuilder) so each
initialization gets its own sync.Map and fuego.NewEngine() instance. Replace
references to the globals (usedOperationIDs, noopEngine) with the instance
fields on the builder/router (or local variables constructed in the build
function) so operation IDs and the no-op engine are scoped to a single build
lifecycle.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f39b5dd4-0f1c-4202-b895-6e01463ce3af
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (120)
AGENTS.mdCLAUDE.mdDockerfileREADME.fr.mdREADME.ja.mdREADME.mdREADME.zh_CN.mdREADME.zh_TW.mdcommon/gin.gocontroller/billing.gocontroller/channel-billing.gocontroller/channel-test.gocontroller/channel.gocontroller/channel_affinity_cache.gocontroller/channel_upstream_update.gocontroller/checkin.gocontroller/codex_oauth.gocontroller/codex_usage.gocontroller/console_migrate.gocontroller/custom_oauth.gocontroller/deployment.gocontroller/group.gocontroller/image.gocontroller/log.gocontroller/midjourney.gocontroller/misc.gocontroller/missing_models.gocontroller/model.gocontroller/model_meta.gocontroller/model_sync.gocontroller/oauth.gocontroller/option.gocontroller/passkey.gocontroller/performance.gocontroller/prefill_group.gocontroller/pricing.gocontroller/ratio_config.gocontroller/ratio_sync.gocontroller/redemption.gocontroller/relay.gocontroller/secure_verification.gocontroller/setup.gocontroller/subscription.gocontroller/subscription_payment_creem.gocontroller/subscription_payment_epay.gocontroller/subscription_payment_stripe.gocontroller/swag_video.gocontroller/task.gocontroller/telegram.gocontroller/token.gocontroller/topup.gocontroller/topup_creem.gocontroller/topup_stripe.gocontroller/twofa.gocontroller/uptime_kuma.gocontroller/usedata.gocontroller/user.gocontroller/vendor_meta.gocontroller/wechat.godocker-compose.ymldocs/openapi/api.jsondocs/openapi/relay.jsondto/api_responses.godto/channel.godto/channel_affinity_cache.godto/channel_billing.godto/channel_testing.godto/checkin.godto/codex_usage.godto/custom_oauth.godto/deployment.godto/group.godto/misc.godto/model_meta.godto/model_sync.godto/openai_request.godto/option.godto/params.godto/performance.godto/pricing.godto/query_params.godto/ratio_sync_result.godto/responses.godto/router.godto/setup.godto/subscription.godto/token.godto/topup.godto/topup_creem.godto/twofa.godto/type_aliases.godto/uptime_kuma.godto/user.gogo.modmodel/channel.gomodel/task.gomodel/twofa.gomodel/user.gomodel/user_cache.gorelay/channel/claude/relay-claude.gorouter/api-router.gorouter/dashboard.gorouter/main.gorouter/openapi.gorouter/relay-router.gorouter/video-router.goservice/openaicompat/chat_to_responses.goservice/openaicompat/responses_to_chat.gosetting/chat.gosetting/console_setting/validation.gosetting/ratio_setting/exposed_cache.gotypes/channel_settings.gotypes/oauth_errors.gotypes/openai_video.gotypes/user_settings.goweb/src/components/table/model-deployments/modals/EditDeploymentModal.jsxweb/src/components/table/usage-logs/UsageLogsColumnDefs.jsxweb/src/components/topup/SubscriptionPlansCard.jsxweb/src/components/topup/index.jsxweb/src/hooks/models/useModelsData.jsx
💤 Files with no reviewable changes (3)
- controller/image.go
- web/src/components/table/usage-logs/UsageLogsColumnDefs.jsx
- controller/swag_video.go
✅ Files skipped from review due to trivial changes (33)
- types/openai_video.go
- web/src/components/table/model-deployments/modals/EditDeploymentModal.jsx
- dto/channel_affinity_cache.go
- README.zh_TW.md
- README.ja.md
- types/channel_settings.go
- dto/model_meta.go
- types/user_settings.go
- dto/codex_usage.go
- dto/option.go
- dto/setup.go
- dto/group.go
- README.zh_CN.md
- docker-compose.yml
- dto/uptime_kuma.go
- controller/relay.go
- dto/ratio_sync_result.go
- types/oauth_errors.go
- README.md
- dto/channel_testing.go
- web/src/hooks/models/useModelsData.jsx
- dto/token.go
- dto/performance.go
- relay/channel/claude/relay-claude.go
- dto/channel_billing.go
- router/main.go
- controller/midjourney.go
- dto/twofa.go
- dto/checkin.go
- dto/topup_creem.go
- dto/model_sync.go
- controller/billing.go
- dto/subscription.go
🚧 Files skipped from review as they are similar to previous changes (31)
- Dockerfile
- setting/chat.go
- dto/openai_request.go
- service/openaicompat/chat_to_responses.go
- AGENTS.md
- web/src/components/topup/SubscriptionPlansCard.jsx
- CLAUDE.md
- common/gin.go
- model/task.go
- model/user_cache.go
- controller/codex_usage.go
- controller/secure_verification.go
- controller/subscription_payment_creem.go
- router/video-router.go
- controller/ratio_config.go
- controller/performance.go
- dto/misc.go
- go.mod
- dto/channel.go
- web/src/components/topup/index.jsx
- dto/custom_oauth.go
- controller/checkin.go
- controller/topup_stripe.go
- controller/subscription.go
- dto/deployment.go
- controller/ratio_sync.go
- dto/user.go
- controller/topup_creem.go
- dto/query_params.go
- dto/topup.go
- dto/api_responses.go
0-don
force-pushed
the
feat/openapi-fuego
branch
from
9be3016 to
4a3f172
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
dto/openai_request.go (1)
3-4:⚠️ Potential issue | 🟡 MinorUse
common.Marshal/common.Unmarshalinstead ofencoding/jsondirectly.This file imports
encoding/jsonand usesjson.Unmarshal(Line 444) andjson.Marshal(Line 451) directly. As per coding guidelines, all JSON marshal/unmarshal operations must use wrapper functions fromcommon/json.go.Proposed fix
At Line 444:
- if err := json.Unmarshal(m.ToolCalls, &toolCalls); err == nil { + if err := common.Unmarshal(m.ToolCalls, &toolCalls); err == nil {At Line 451:
- toolCallsJson, _ := json.Marshal(toolCalls) + toolCallsJson, _ := common.Marshal(toolCalls)Then remove the
encoding/jsonimport if no longer needed.Also applies to: 444-448, 450-453
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dto/openai_request.go` around lines 3 - 4, Replace direct uses of encoding/json: locate calls to json.Unmarshal and json.Marshal in dto/openai_request.go (the functions where the code unmarshals/marshals request bodies) and swap them to use the wrapper functions common.Unmarshal and common.Marshal from common/json.go, update the import list to remove "encoding/json" and add/import the common package if not already present, and ensure error handling stays the same after the replacement.controller/topup_creem.go (1)
87-91:⚠️ Potential issue | 🔴 CriticalHandle a missing user before using it.
model.GetUserByIdis ignored here, but Line 91 and Line 110 dereferenceuser. Any auth gap or lookup failure turns this request into a panic instead of a clean API error.Suggested fix
id := dto.UserID(c) - user, _ := model.GetUserById(id, false) + if id <= 0 { + return dto.Fail[dto.CreemPayData]("用户未登录") + } + user, err := model.GetUserById(id, false) + if err != nil { + log.Printf("获取Creem支付用户失败: %v", err) + return dto.Fail[dto.CreemPayData]("获取用户信息失败") + } + if user == nil { + return dto.Fail[dto.CreemPayData]("用户不存在") + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/topup_creem.go` around lines 87 - 91, The code calls model.GetUserById and ignores its error, then dereferences user.Id when building reference and later at line ~110; update the handler to check both the returned error and whether user is nil (from model.GetUserById and dto.UserID) before using user, and return/abort with a proper API error (e.g., 400/401/404) if lookup fails; ensure you log the lookup error and avoid dereferencing user when nil so subsequent lines that build reference (fmt.Sprintf(..., user.Id,...)) and any uses later are safe.controller/subscription.go (2)
122-146:⚠️ Potential issue | 🟠 MajorValidate
CustomSecondsfor custom-duration plans.These paths validate
QuotaResetCustomSeconds, butDurationUnit == model.SubscriptionDurationCustomcan still be saved withCustomSeconds <= 0. That leaves admins able to create or update plans with no valid subscription length.Also applies to: 174-198
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/subscription.go` around lines 122 - 146, The code allows saving plans with DurationUnit == model.SubscriptionDurationCustom and DurationValue <= 0; add a validation that when req.Plan.DurationUnit == model.SubscriptionDurationCustom then req.Plan.DurationValue must be > 0 and return dto.Fail[model.SubscriptionPlan] with an appropriate message if not. Apply the same check in both places where plan validation happens (the block around req.Plan.DurationUnit handling and the corresponding update block referenced in the comment), checking req.Plan.DurationUnit, model.SubscriptionDurationCustom and req.Plan.DurationValue.
201-229:⚠️ Potential issue | 🟠 MajorCheck
RowsAffectedbefore invalidating cache when updating subscription plans.GORM
Updates()andUpdate()do not return errors whenWHERE id = ?matches zero rows. Both handlers currently return success (and invalidate cache) even when the plan ID doesn't exist. Validate that rows were actually affected before responding.Suggested fix
For the plan update (line 222):
- if err := tx.Model(&model.SubscriptionPlan{}).Where("id = ?", id).Updates(updateMap).Error; err != nil { - return err - } + result := tx.Model(&model.SubscriptionPlan{}).Where("id = ?", id).Updates(updateMap) + if result.Error != nil { + return result.Error + } + if result.RowsAffected == 0 { + return gorm.ErrRecordNotFound + }For the status update (line 243):
- if err := model.DB.Model(&model.SubscriptionPlan{}).Where("id = ?", id).Update("enabled", *req.Enabled).Error; err != nil { - return dto.FailMsg(err.Error()) - } + result := model.DB.Model(&model.SubscriptionPlan{}).Where("id = ?", id).Update("enabled", *req.Enabled) + if result.Error != nil { + return dto.FailMsg(result.Error.Error()) + } + if result.RowsAffected == 0 { + return dto.FailMsg("subscription plan not found") + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/subscription.go` around lines 201 - 229, The transaction currently ignores whether the Updates actually affected any rows; modify the block using model.DB.Transaction so you capture the gorm result from tx.Model(&model.SubscriptionPlan{}).Where("id = ?", id).Updates(updateMap) (e.g., store as res), check res.Error and then check res.RowsAffected; if RowsAffected == 0 return a not-found error (or gorm.ErrRecordNotFound) from the transaction so the outer handler returns failure and you only invalidate cache when rows were actually updated; apply the same pattern for the status update path that uses Update/Updates.
♻️ Duplicate comments (28)
web/src/components/topup/index.jsx (2)
298-302:⚠️ Potential issue | 🟠 MajorValidate
checkout_urlbefore processing Creem callback.Similar to the other payment path,
processCreemCallback(data)is invoked whensuccessis true, butdata.checkout_urlis not validated. If the field is missing,window.open(undefined, '_blank')will open an empty tab.Proposed fix
const { success, message, data } = res.data; if (success) { + if (!data?.checkout_url) { + showError(message || t('支付返回数据异常')); + return; + } processCreemCallback(data); } else {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/topup/index.jsx` around lines 298 - 302, The code calls processCreemCallback(data) when res.data.success is true without ensuring data.checkout_url exists, causing window.open(undefined, '_blank') to open empty tabs; before calling processCreemCallback (or inside processCreemCallback) validate that res.data.data exists and that data.checkout_url is a non-empty, well-formed URL (e.g., non-empty string, optionally check startsWith 'http' or use URL parsing), and if invalid call showError(message || t('支付失败')) instead of proceeding; reference the symbols res.data, success, message, data, data.checkout_url, processCreemCallback, showError and window.open when making the change.
236-237:⚠️ Potential issue | 🟠 MajorValidate required redirect fields before continuing checkout.
Both payment paths trust
successwithout verifying redirect payload completeness. Missingdata.url/data.paramsordata.checkout_urlcan break checkout at runtime.Proposed fix
if (payWay === 'stripe') { // Stripe 支付回调处理 window.open(data.pay_link, '_blank'); } else { // 普通支付表单提交 - let params = data.params; - let url = data.url; + const params = data?.params; + const url = data?.url; + if (!url || !params || typeof params !== 'object') { + showError(message || t('支付返回数据异常')); + return; + } let form = document.createElement('form');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/topup/index.jsx` around lines 236 - 237, The code assigns let params = data.params and let url = data.url without validating the redirect payload; update the checkout flow in the function handling payment responses (where params and url are set) to verify required fields (data.url and data.params for redirect-based flow, and data.checkout_url when applicable) before proceeding, and if missing, log or set an error and return early to abort checkout; ensure you handle both branches that use data.url/data.params and the branch that expects data.checkout_url so runtime failures are prevented.dto/params.go (1)
28-42:⚠️ Potential issue | 🔴 Critical
reflect.MakeSlicewill panic whenfield.Type.Kind()isreflect.Array.The case handles both
reflect.Sliceandreflect.Array, butreflect.MakeSlice(line 36) only accepts slice types. When the field is a fixed-size array, this will cause a runtime panic. Either removereflect.Arrayfrom this case or add separate handling that validates the element count matches the array length.Suggested fix: remove Array support
- case reflect.Slice, reflect.Array: + case reflect.Slice:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dto/params.go` around lines 28 - 42, The code treats reflect.Slice and reflect.Array the same but calls reflect.MakeSlice (which panics for arrays); update the query-tag handling in the loop so that when field.Type.Kind() == reflect.Slice you keep the existing logic using reflect.MakeSlice and sliceType := field.Type.Elem(), reflect.MakeSlice and setParamValue; when field.Type.Kind() == reflect.Array handle it separately by validating len(paramValues) == field.Type.Len(), creating a new array value (reflect.New(field.Type).Elem()) and populating elements with setParamValue on array.Index(i), then call fieldValue.Set(arrayVal); ensure you remove arrays from the slice-path to avoid calling reflect.MakeSlice on array types.controller/prefill_group.go (1)
44-65:⚠️ Potential issue | 🟠 MajorAdd
Name/Typevalidation toUpdatePrefillGroupfor consistency withCreatePrefillGroup.
CreatePrefillGrouprequires non-emptyNameandType(lines 25-27), butUpdatePrefillGrouponly validatesId != 0. This allows updates that clear required fields.Suggested fix
if g.Id == 0 { return dto.Fail[model.PrefillGroup]("缺少组 ID") } + if g.Name == "" || g.Type == "" { + return dto.Fail[model.PrefillGroup]("组名称和类型不能为空") + } // 名称冲突检查 dup, err := model.IsPrefillGroupNameDuplicated(g.Id, g.Name)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/prefill_group.go` around lines 44 - 65, UpdatePrefillGroup currently only checks g.Id and allows empty Name/Type; add the same non-empty validations used in CreatePrefillGroup: ensure g.Name != "" and g.Type != "" (return dto.Fail[model.PrefillGroup] with appropriate messages like "缺少组名称" and "缺少组类型") before calling model.IsPrefillGroupNameDuplicated and g.Update(); keep the duplicate-name check and existing error handling around model.IsPrefillGroupNameDuplicated and g.Update().controller/channel-test.go (1)
749-761:⚠️ Potential issue | 🟡 MinorElapsed time is computed after the early failure return, so failed tests always report
Time: 0.When
result.localErr != nil, the function returns at line 752 withTime: 0.0beforetokandmillisecondsare computed (lines 754-755). This means failed channel tests always show zero elapsed time even after a real upstream round-trip.Suggested fix: compute elapsed time before any return
tik := time.Now() result := testChannel(channel, testModel, endpointType, isStream) + tok := time.Now() + milliseconds := tok.Sub(tik).Milliseconds() + consumedTime := float64(milliseconds) / 1000.0 if result.localErr != nil { - return dto.TestChannelResponse{Success: false, Message: result.localErr.Error(), Time: 0.0}, nil + return dto.TestChannelResponse{Success: false, Message: result.localErr.Error(), Time: consumedTime}, nil } - tok := time.Now() - milliseconds := tok.Sub(tik).Milliseconds() go channel.UpdateResponseTime(milliseconds) - consumedTime := float64(milliseconds) / 1000.0 if result.newAPIError != nil { return dto.TestChannelResponse{Success: false, Message: result.newAPIError.Error(), Time: consumedTime}, nil }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/channel-test.go` around lines 749 - 761, The elapsed-time is calculated after early returns so failures always report Time: 0; move the timing computation and the asynchronous call to channel.UpdateResponseTime to occur immediately after calling testChannel (use tik := time.Now(); after result := testChannel(...), compute tok := time.Now(), milliseconds := tok.Sub(tik).Milliseconds(), go channel.UpdateResponseTime(milliseconds), and consumedTime := float64(milliseconds)/1000.0) before checking result.localErr or result.newAPIError so both success and failure TestChannelResponse (and the UpdateResponseTime call) report the real elapsed time; update the return paths that currently set Time: 0.0 to use consumedTime and keep dto.TestChannelResponse usage unchanged.controller/topup_creem.go (1)
158-163:⚠️ Potential issue | 🟠 MajorDecode the verified body with
common.Unmarshal.
bodyBytesis already the source of truth here, so rebinding throughc.ShouldBindJSONis redundant and keeps the unresolved JSON-wrapper violation in place. ParsebodyBytesdirectly and drop the body reset. As per coding guidelines, "All JSON marshal/unmarshal operations MUST use wrapper functions fromcommon/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 `@controller/topup_creem.go` around lines 158 - 163, The code currently resets c.Request.Body and calls c.ShouldBindJSON to parse JSON into dto.CreemWebhookEvent, which is redundant; instead parse the verified bodyBytes directly using common.Unmarshal into the existing webhookEvent variable and remove the c.Request.Body = io.NopCloser(...) and the c.ShouldBindJSON call; ensure you call common.Unmarshal(bodyBytes, &webhookEvent) (and handle its error) rather than using encoding/json or Gin binding so the project-wide JSON wrapper functions are used.controller/token.go (2)
190-193:⚠️ Potential issue | 🟡 MinorValidate the delete path param with
PathParamIntErr.
PathParamIntsilently turns an invalid id into0, so malformed requests can reachDeleteTokenByIdwith the wrong identifier. Fail early on parse errors here.Minimal fix
- id := c.PathParamInt("id") - err := model.DeleteTokenById(id, dto.UserID(c)) + id, err := c.PathParamIntErr("id") + if err != nil { + return dto.FailMsg(err.Error()) + } + err = model.DeleteTokenById(id, dto.UserID(c))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/token.go` around lines 190 - 193, Replace the silent parse with explicit validation in DeleteToken: use c.PathParamIntErr("id") instead of c.PathParamInt("id"), check the returned error and return it (or a suitable HTTP/biz error) immediately on parse failure so you never call model.DeleteTokenById with an invalid id; keep dto.UserID(c) as before when calling DeleteTokenById once the id is validated.
34-41:⚠️ Potential issue | 🟡 MinorDon't drop the
CountUserTokenserror.If the count query fails, this still returns a page payload with incorrect pagination metadata. Bubble that failure instead of returning a partial success.
💡 Suggested fix
- total, _ := model.CountUserTokens(dto.UserID(c)) + total, err := model.CountUserTokens(dto.UserID(c)) + if err != nil { + return dto.FailPage[*model.Token](err.Error()) + } return dto.OkPage(page, buildMaskedTokenResponses(tokens), int(total))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/token.go` around lines 34 - 41, GetAllTokens currently ignores the error from model.CountUserTokens and may return incorrect pagination; call model.CountUserTokens(dto.UserID(c)) capturing (total, err) and if err != nil return dto.FailPage[*model.Token](err.Error()) instead of proceeding to dto.OkPage; locate symbols GetAllTokens, model.CountUserTokens, dto.FailPage, dto.OkPage, and buildMaskedTokenResponses to implement this error check and early return.controller/model_sync.go (1)
259-264:⚠️ Potential issue | 🟠 MajorTreat
io.EOFas an empty sync request.The comment says empty bodies are allowed, but
c.Body()returns an error for an absent body. Right now a no-body request is rejected before the fast path can run.Suggested fix
req, err := c.Body() if err != nil { - return dto.Fail[dto.SyncUpstreamResult]("请求参数格式错误") + if errors.Is(err, io.EOF) { + req = dto.SyncRequest{} + } else { + return dto.Fail[dto.SyncUpstreamResult]("请求参数格式错误") + } }In go-fuego/fuego v0.19.x, what does ContextWithBody[T].Body() return when the HTTP request body is empty or absent? Does it return io.EOF?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/model_sync.go` around lines 259 - 264, SyncUpstreamModels currently rejects requests when c.Body() returns an error, but an absent/empty HTTP body yields io.EOF and should be treated as an allowed empty sync request; update the handler to check the error from c.Body(), treat io.EOF (and possibly io.ErrUnexpectedEOF) as non-fatal and continue with an empty/default dto.SyncRequest, while other errors still return dto.Fail; reference the SyncUpstreamModels function and the call to c.Body() and dto.Fail[dto.SyncUpstreamResult] to locate where to branch on io.EOF and create/use a zero-value req for the fast path.controller/misc.go (2)
230-241:⚠️ Potential issue | 🟠 MajorMake the reset-email response indistinguishable for unknown accounts.
Returning a specific failure here lets unauthenticated callers enumerate registered emails. Return the same generic success payload either way, and only generate/send a reset link when the account actually exists.
Safer response pattern
- if !model.IsEmailAlreadyTaken(p.Email) { - return dto.FailMsg("该邮箱地址未注册") - } + if !model.IsEmailAlreadyTaken(p.Email) { + return dto.Msg("") + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/misc.go` around lines 230 - 241, The current handler returns dto.FailMsg when model.IsEmailAlreadyTaken(p.Email) is false, enabling email enumeration; change the flow so the endpoint always returns the same generic success response regardless of whether the email exists, but only call common.GenerateVerificationCode, common.RegisterVerificationCodeWithKey and common.SendEmail when model.IsEmailAlreadyTaken(p.Email) is true. Locate the block using model.IsEmailAlreadyTaken, dto.FailMsg, common.GenerateVerificationCode, common.RegisterVerificationCodeWithKey and common.SendEmail and move/guard the code that generates the token and sends the email behind the existence check while ensuring the function returns the generic success payload in both cases.
258-261:⚠️ Potential issue | 🟠 MajorDon't log raw email addresses on reset failures.
This writes a user identifier into error logs. Omit or mask
req.Emailin the failure path.Minimal fix
- common.SysError("failed to reset password for " + req.Email + ": " + err.Error()) + common.SysError("failed to reset password: " + err.Error())🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/misc.go` around lines 258 - 261, The failure log in the ResetUserPasswordByEmail path currently includes the raw user email; update the error logging in the branch that calls model.ResetUserPasswordByEmail so it does not write req.Email directly to logs—either omit the email or mask it (e.g., replace local part with asterisks or only log the domain) before passing to common.SysError, and keep the rest of the error text (err.Error()) for diagnostics; change the call site that constructs the message to use the masked/omitted identifier instead of req.Email.controller/channel-billing.go (1)
78-80:⚠️ Potential issue | 🟠 MajorUse
common.Unmarshalin these updated balance parsers.These changed call sites still bypass the repo JSON wrappers, so any shared decode behavior in
common/json.gois skipped. Please switch eachjson.Unmarshal(body, &response)here tocommon.Unmarshal(body, &response).♻️ Minimal change
- err = json.Unmarshal(body, &response) + err = common.Unmarshal(body, &response)As 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 call encoding/json in business code.Also applies to: 93-95, 117-119, 136-138, 151-153, 173-175, 202-204, 217-219, 300-302, 316-318
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/channel-billing.go` around lines 78 - 80, Replace direct calls to json.Unmarshal(body, &response) with common.Unmarshal(body, &response) in controller/channel-billing.go for each balance parser (e.g., where response is declared as dto.OpenAICreditGrants or other dto types). Keep the existing error handling (err = ...) unchanged, just swap json.Unmarshal -> common.Unmarshal and ensure the common package is imported; update all occurrences noted (lines for the groups matching the pattern json.Unmarshal(body, &response)). This uses the shared wrapper in common/json.go and preserves current variable names and error checks.controller/codex_oauth.go (2)
93-99:⚠️ Potential issue | 🟠 MajorDon't ignore
session.Save()failures in either OAuth phase.If the first save fails, the client gets an authorize URL that can never complete. If the cleanup save fails, stale state/verifier values remain in the session. Please check and propagate both errors.
Minimal fix
- _ = session.Save() + if err := session.Save(); err != nil { + common.SysError("failed to save codex oauth session: " + err.Error()) + return dto.Fail[dto.CodexOAuthStartData]("failed to persist oauth session") + }- _ = session.Save() + if err := session.Save(); err != nil { + common.SysError("failed to clear codex oauth session: " + err.Error()) + return dto.Fail[dto.CodexOAuthCompleteData]("failed to clear oauth session") + }Also applies to: 189-192
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/codex_oauth.go` around lines 93 - 99, The session.Save() calls are currently ignored in both the OAuth start and cleanup paths (around the use of sessions.Default, codexOAuthSessionKey, flow.State/Verifier and when deleting/clearing those keys), so change both sites to capture the returned error from session.Save(), and if non-nil return/propagate an appropriate error response instead of proceeding; specifically, after setting state/verifier/created_at capture err := session.Save() and return an error to the caller if err != nil (do not return dto.Ok on failure), and likewise after deleting/clearing keys in the cleanup flow capture err := session.Save() and return/propagate the error so stale values aren’t left in the session. Ensure the error handling is consistent with other controller responses and uses the same DTO/error-return conventions.
150-155:⚠️ Potential issue | 🟠 MajorExpire stale OAuth flows using
created_at.
created_atis stored on start but never checked here, so the PKCE verifier stays valid until the whole session expires instead of a short OAuth window. Reject missing/expired timestamps, clear the stored OAuth state, and return an expiration error.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/codex_oauth.go` around lines 150 - 155, Check the stored "created_at" timestamp from the session (use codexOAuthSessionKey(channelID, "created_at")) alongside expectedState and verifier; if the timestamp is missing or older than the allowed OAuth window, treat the flow as expired: delete the session keys for state, verifier and created_at (via sessions.Default(ginCtx).Delete(...)) and call session.Save(), then return the expiration error (e.g., dto.Fail[dto.CodexOAuthCompleteData]("oauth flow expired")). Ensure you still return the existing error when state/verifier are blank, and use a clear short window (e.g., a few minutes) when comparing times.controller/redemption.go (1)
88-94:⚠️ Potential issue | 🟡 MinorReject malformed redemption IDs early.
PathParamInt("id")can keep going with0on bad input, so this handler may reachDeleteRedemptionById(0)instead of failing fast. Use the error-returning helper here.Minimal fix
func DeleteRedemption(c fuego.ContextNoBody) (dto.MessageResponse, error) { - id := c.PathParamInt("id") - err := model.DeleteRedemptionById(id) + id, err := c.PathParamIntErr("id") + if err != nil { + return dto.FailMsg(err.Error()) + } + err = model.DeleteRedemptionById(id) if err != nil { return dto.FailMsg(err.Error()) } return dto.Msg("") }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/redemption.go` around lines 88 - 94, DeleteRedemption currently uses c.PathParamInt("id") which returns 0 for malformed IDs; update the handler to parse the id using the error-returning helper (e.g., c.PathParamIntErr or equivalent) and if parsing returns an error immediately return dto.FailMsg(err.Error()); only call model.DeleteRedemptionById(id) when parsing succeeded, and otherwise avoid calling the delete with id==0.controller/user.go (3)
492-496:⚠️ Potential issue | 🟠 Major
UpdateSelfadvertises no body but reads one anyway.
fuego.ContextNoBodywill generate a no-request-body contract for this endpoint, yet the handler decodes JSON from the request and then round-trips it throughencoding/json. That produces the wrong OpenAPI and still violates the repo JSON-wrapper rule. Please switch this tofuego.ContextWithBody[...]with a typed DTO; if any manual JSON round-trip remains, usecommon.Marshal/common.Unmarshal.As per coding guidelines, “All JSON marshal/unmarshal operations MUST use wrapper functions in
common/json.go... Do NOT directly callencoding/jsonin business code.”Also applies to: 543-548
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/user.go` around lines 492 - 496, The UpdateSelf handler currently uses fuego.ContextNoBody but reads and decodes a JSON body (via dto.Decode/encoding/json), causing an incorrect OpenAPI contract and violating the repo JSON-wrapper rule; change the handler signature to use fuego.ContextWithBody[YourTypedDTO] (replace YourTypedDTO with a concrete request DTO), update the body parsing to accept that typed DTO instead of map[string]interface{}, remove direct encoding/json calls and any dto.Decode round-trip, and if you must manually marshal/unmarshal use common.Marshal/common.Unmarshal; apply the same fixes to the other similar handler referenced around lines 543-548.
33-35:⚠️ Potential issue | 🟠 MajorDecode
Loginthrough the repo JSON wrapper.This controller still calls
encoding/jsondirectly. That bypasses the shared JSON wrapper layer and makes request decoding behavior inconsistent with the rest of the codebase.Minimal fix
var loginRequest dto.LoginRequest - err := json.NewDecoder(c.Request.Body).Decode(&loginRequest) + err := common.DecodeJson(c.Request.Body, &loginRequest) if err != nil { common.ApiErrorI18n(c, i18n.MsgInvalidParams) return }As per coding guidelines, “All JSON marshal/unmarshal operations MUST use wrapper functions in
common/json.go... Do NOT directly callencoding/jsonin business code.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/user.go` around lines 33 - 35, The Login handler currently decodes the request using encoding/json (json.NewDecoder(c.Request.Body).Decode(&loginRequest)); replace that direct call with the project's shared JSON wrapper functions from common/json.go (the repo's DecodeRequest/UnmarshalRequest or similarly named wrapper) so all JSON decoding uses the common behavior; update the code in the controller's Login path (where variable loginRequest of type dto.LoginRequest is populated) to call the wrapper, handle and log any wrapper-returned error the same way you currently do, and remove the direct encoding/json import/usage.
693-721:⚠️ Potential issue | 🟠 MajorReturn after
deleteinstead of falling through toUpdate(false).The
deletebranch already persists viauser.Delete(), butuser.Update(false)still runs afterward. That can turn a successful delete into an error path or rewrite the deleted row.Minimal fix
case "delete": if user.Role == common.RoleRootUser { return dto.Fail[dto.ManageUserData](common.TranslateMessage(ginCtx, i18n.MsgUserCannotDeleteRootUser)) } if err := user.Delete(); err != nil { return dto.Fail[dto.ManageUserData](err.Error()) } + return dto.Ok(dto.ManageUserData{Role: user.Role, Status: user.Status}) case "promote": if myRole != common.RoleRootUser { return dto.Fail[dto.ManageUserData](common.TranslateMessage(ginCtx, i18n.MsgUserAdminCannotPromote)) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/user.go` around lines 693 - 721, In the "delete" case inside the handler, stop falling through to the shared post-switch Update by returning immediately after a successful user.Delete() call: after if err := user.Delete(); err != nil { ... } add a direct return (e.g. return dto.Ok(dto.ManageUserData{Role: user.Role, Status: user.Status}) or appropriate success response) so that user.Update(false) is not invoked on a deleted record; alternatively refactor so only non-delete branches call user.Update(false) (refer to the "delete" branch, user.Delete(), and user.Update(false)).controller/topup.go (1)
173-176:⚠️ Potential issue | 🟡 MinorKeep the minimum-payment check identical in quote and checkout.
RequestEpaystill allows0.01(< 0.01) whileRequestAmountrejects it (<= 0.01). The two endpoints can disagree on the same amount again, so please share one helper/predicate here.Also applies to: 367-368
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/topup.go` around lines 173 - 176, The minimum-payment check is inconsistent between RequestEpay and RequestAmount (one uses "< 0.01", the other "<= 0.01"); create a single helper predicate (e.g., isPayableAmount or isValidMinPayment) that calls getPayMoney(req.Amount, group) and enforces the agreed rule (pick <= 0.01 or < 0.01) and use that helper in both RequestEpay and RequestAmount (and the other occurrence referenced) so both endpoints share the identical comparison logic.README.fr.md (1)
228-229:⚠️ Potential issue | 🟡 MinorPolish the French OpenAPI sentence.
activer avec ... accessible à ...still reads awkwardly here.✍️ Suggested wording
- - 📖 Spécification OpenAPI 3.0 auto-générée — activer avec `ENABLE_OPENAPI=true`, accessible à `/openapi.json` et `/swagger` (Scalar UI) + - 📖 Spécification OpenAPI 3.0 générée automatiquement — activez-la avec `ENABLE_OPENAPI=true` ; elle est alors disponible via `/openapi.json` et `/swagger` (Scalar UI)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.fr.md` around lines 228 - 229, Reword the French OpenAPI sentence for clarity: change the line mentioning ENABLE_OPENAPI, /openapi.json and /swagger to something like "Spécification OpenAPI 3.0 générée automatiquement — activez-la avec ENABLE_OPENAPI=true ; la spec est disponible à /openapi.json et l'interface Scalar UI à /swagger." Ensure the variables/symbols ENABLE_OPENAPI, /openapi.json and /swagger are retained exactly as-is.controller/deployment.go (1)
108-122:⚠️ Potential issue | 🟡 MinorKeep
StatusCountson one scope.
"all"comes fromdl.Total(full result set), but the per-status values are computed fromdl.Deployments(current page only). Those numbers are not comparable in the same payload. Either compute all counts globally or make"all"page-scoped too.Also applies to: 216-221
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/deployment.go` around lines 108 - 122, computeStatusCounts currently mixes scopes: it sets "all" using the external total (dl.Total) but computes per-status counts from the page slice (dl.Deployments), making the payload inconsistent; update computeStatusCounts to keep scopes consistent by making "all" reflect the same scope as the per-status counts (i.e., set "all" to int64(len(deployments)) or remove the total parameter and derive total from deployments), and apply the same fix to the other StatusCounts usage noted (the block around the second occurrence) so both "all" and per-status counts come from the same source.router/relay-router.go (3)
166-169:⚠️ Potential issue | 🟠 MajorRegister
/v1/realtimethrough the OpenAPI router as well.This direct Gin registration bypasses
dto.NewRouter, so it won't appear in/openapi.json. If WebSockets are intentionally out of scope, please call that out next to this route.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@router/relay-router.go` around lines 166 - 169, The /v1/realtime WebSocket route is being registered directly on Gin (wsRouter, RelayRealtime) which bypasses dto.NewRouter/OpenAPI registration; either move this registration into the OpenAPI-aware router initialization (use the same dto.NewRouter/OpenAPIRouter flow to add the GET "/realtime" -> RelayRealtime handler so it appears in /openapi.json), or if WebSockets are intentionally excluded, add a clear inline comment next to the wsRouter/RelayRealtime registration stating it is intentionally omitted from dto.NewRouter/OpenAPI and why.
198-198:⚠️ Potential issue | 🟠 Major
/v1/audio/speechis not aMessageResponse.This endpoint relays synthesized audio bytes, but the OpenAPI annotation says JSON. Generated clients will deserialize the response incorrectly unless this is documented as binary/audio content. Please verify the generated spec uses an audio/binary response here.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@router/relay-router.go` at line 198, The route registration wrongly declares RelayAudioSpeech as returning dto.MessageResponse (JSON); change the route's response metadata so the OpenAPI spec marks it as binary/audio (e.g., audio/mpeg or audio/wav) instead of dto.GinResp[dto.MessageResponse] — update the Gin route call that currently uses dto.GinResp[dto.MessageResponse]() to use the binary/file response helper (or a new dto.GinRespBinary/Bytes variant) and ensure RelayAudioSpeech's handler signature and response type return raw []byte (or an io.Reader) and set the proper Content-Type to the correct audio MIME so generated clients treat it as audio/binary.
156-156:⚠️ Potential issue | 🟠 MajorAdd
GinBody[...]()to the raw relay POST routes.These handlers are still registered as raw
gin.HandlerFuncs.GinResp[...]()only documents the response, so the generated spec will miss request-body schemas for the core relay, Suno, Gemini beta, and Midjourney POST endpoints. Please verify these operations emitrequestBodyentries inopenapi.json.Also applies to: 177-197, 201-205, 242-243, 257-257, 265-275, 278-280
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@router/relay-router.go` at line 156, The POST routes registered with pg.GinPost (e.g., the /chat/completions call using controller.Playground) only include dto.GinResp[...]() so the OpenAPI generator never gets request body schemas; update each POST registration in router/relay-router.go (including the ranges 177-197, 201-205, 242-243, 257, 265-275, 278-280) to include dto.GinBody[<RequestDTO>]() before dto.GinResp[<ResponseDTO>](), e.g. pg.GinPost(path, handler, dto.GinBody[YourRequestType](), dto.GinResp[YourResponseType]()) so the openapi.json emits requestBody entries for core relay, Suno, Gemini beta, and Midjourney endpoints.router/api-router.go (2)
40-41:⚠️ Potential issue | 🟠 MajorKeep
GET /api/modelsbackward compatible.This now points at
controller.DashboardListModels, while the admin metadata listing moved to/api/models/list. Existing admin clients that still call/api/modelswill break, which conflicts with the PR objective that current API routes keep working.Also applies to: 377-384
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@router/api-router.go` around lines 40 - 41, The GET /api/models route was changed to use controller.DashboardListModels which breaks backward compatibility; revert the mapping so dto.Get(publicModels, "/models", ...) continues to call the original admin metadata handler (i.e., restore the previous handler that returns the admin models metadata) and add a new route dto.Get(publicModels, "/models/list", controller.DashboardListModels) to host the dashboard-specific listing; locate the route registration using the publicModels variable and dto.Get calls to make this change.
266-267:⚠️ Potential issue | 🟠 MajorDocument
/api/channel/ollama/pull/streamas SSE, notMessageResponse.
controller.OllamaPullModelStreambinds a JSON body and writestext/event-stream, but this registration only advertises a JSONMessageResponse. That makes both the request body and the response content type wrong in the generated spec. Please verify this route emits arequestBodyand a streaming response definition.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@router/api-router.go` around lines 266 - 267, The route registration for controller.OllamaPullModelStream currently advertises a JSON MessageResponse (dto.GinResp[dto.MessageResponse]()) but the handler binds a JSON request body and emits text/event-stream SSE; update the route metadata in the ch.GinPost call to include a requestBody schema for the JSON input (matching the handler's binding type) and replace the response descriptor with an SSE/streaming response descriptor so OpenAPI shows content-type text/event-stream and a streaming response; locate the DTO helper used for responses (dto.GinResp[...]()) and swap it for the appropriate SSE/stream helper or add a custom response descriptor for controller.OllamaPullModelStream so the generated spec includes requestBody and a streaming SSE response.router/video-router.go (2)
19-19:⚠️ Potential issue | 🟠 MajorDon't document the media proxy as
TaskResponseDoc.
/v1/videos/:task_id/contentis a media proxy, not the task-status JSON endpoint. This will generate the wrong schema/content type inopenapi.jsonand mislead generated clients.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@router/video-router.go` at line 19, The route registration uses dto.GinResp[dto.TaskResponseDoc]() which incorrectly documents the media proxy as a JSON task response; update the proxy.GinGet call for "/videos/:task_id/content" (the controller.VideoProxy handler) to not use dto.TaskResponseDoc — either remove the dto.GinResp[...] argument (pass no response schema) or replace it with the correct media response/documentation type for binary/media (e.g., a Video/Media content doc) so OpenAPI treats this endpoint as a media stream rather than a TaskResponse JSON. Ensure the change targets the proxy.GinGet call and controller.VideoProxy reference.
27-27:⚠️ Potential issue | 🟠 MajorAnnotate the raw video POST handlers with
GinBody[...]()too.These are still plain
gin.HandlerFuncregistrations. With onlyGinResp[...](), the generator has no request-body schema for the create/remix/video-task endpoints, so the new spec is incomplete. Please verify these paths emitrequestBodyentries in the generatedopenapi.json.Also applies to: 29-29, 34-34, 43-44, 56-56
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@router/video-router.go` at line 27, Handlers like video.GinPost("/video/generations", controller.RelayTask, dto.GinResp[dto.TaskResponseDoc]()) are missing request-body annotations (only GinResp is present), so the OpenAPI generator won’t emit requestBody entries; fix by adding GinBody[...]() into those route registrations (use video.GinPost and include GinBody[<appropriate request DTO>]() before controller.RelayTask and dto.GinResp[...]()); do this for the create/remix/video-task endpoints (the other similar registrations around the same area) and ensure the GinBody generic matches the request DTO type used by the controller so requestBody appears in openapi.json.
🧹 Nitpick comments (1)
dto/responses.go (1)
48-49: Consider adding a type assertion safety check forGinCtx.The type assertion
c.Context().(*gin.Context)will panic ifContext()returns something other than*gin.Context. While this is safe when exclusively usingfuegogin, a defensive check would make the code more robust:Suggested defensive approach
-func GinCtx(c FuegoCtx) *gin.Context { return c.Context().(*gin.Context) } +// GinCtx extracts the underlying *gin.Context from any fuego context. +// Panics if used with a non-gin fuego adapter. +func GinCtx(c FuegoCtx) *gin.Context { + gc, ok := c.Context().(*gin.Context) + if !ok { + panic("dto.GinCtx: context is not *gin.Context; only fuegogin adapters are supported") + } + return gc +}This provides a clearer error message if the assumption is ever violated.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dto/responses.go` around lines 48 - 49, The GinCtx helper does an unchecked type assertion that can panic; update GinCtx to perform a safe comma-ok assertion on c.Context(), e.g. ctx, ok := c.Context().(*gin.Context), and if !ok produce a clear error (either return nil and an error or keep the same signature but panic with a descriptive message) that includes the actual dynamic type (using reflect.TypeOf(c.Context())) so callers get a helpful message; change the implementation in the GinCtx function to use this safe assertion and include the function name and offending type in the error text.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dfaf7541-b362-4add-b51f-a243ee578ef3
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (120)
AGENTS.mdCLAUDE.mdDockerfileREADME.fr.mdREADME.ja.mdREADME.mdREADME.zh_CN.mdREADME.zh_TW.mdcommon/gin.gocontroller/billing.gocontroller/channel-billing.gocontroller/channel-test.gocontroller/channel.gocontroller/channel_affinity_cache.gocontroller/channel_upstream_update.gocontroller/checkin.gocontroller/codex_oauth.gocontroller/codex_usage.gocontroller/console_migrate.gocontroller/custom_oauth.gocontroller/deployment.gocontroller/group.gocontroller/image.gocontroller/log.gocontroller/midjourney.gocontroller/misc.gocontroller/missing_models.gocontroller/model.gocontroller/model_meta.gocontroller/model_sync.gocontroller/oauth.gocontroller/option.gocontroller/passkey.gocontroller/performance.gocontroller/prefill_group.gocontroller/pricing.gocontroller/ratio_config.gocontroller/ratio_sync.gocontroller/redemption.gocontroller/relay.gocontroller/secure_verification.gocontroller/setup.gocontroller/subscription.gocontroller/subscription_payment_creem.gocontroller/subscription_payment_epay.gocontroller/subscription_payment_stripe.gocontroller/swag_video.gocontroller/task.gocontroller/telegram.gocontroller/token.gocontroller/topup.gocontroller/topup_creem.gocontroller/topup_stripe.gocontroller/twofa.gocontroller/uptime_kuma.gocontroller/usedata.gocontroller/user.gocontroller/vendor_meta.gocontroller/wechat.godocker-compose.ymldocs/openapi/api.jsondocs/openapi/relay.jsondto/api_responses.godto/channel.godto/channel_affinity_cache.godto/channel_billing.godto/channel_testing.godto/checkin.godto/codex_usage.godto/custom_oauth.godto/deployment.godto/group.godto/misc.godto/model_meta.godto/model_sync.godto/openai_request.godto/option.godto/params.godto/performance.godto/pricing.godto/query_params.godto/ratio_sync_result.godto/responses.godto/router.godto/setup.godto/subscription.godto/token.godto/topup.godto/topup_creem.godto/twofa.godto/type_aliases.godto/uptime_kuma.godto/user.gogo.modmodel/channel.gomodel/task.gomodel/twofa.gomodel/user.gomodel/user_cache.gorelay/channel/claude/relay-claude.gorouter/api-router.gorouter/dashboard.gorouter/main.gorouter/openapi.gorouter/relay-router.gorouter/video-router.goservice/openaicompat/chat_to_responses.goservice/openaicompat/responses_to_chat.gosetting/chat.gosetting/console_setting/validation.gosetting/ratio_setting/exposed_cache.gotypes/channel_settings.gotypes/oauth_errors.gotypes/openai_video.gotypes/user_settings.goweb/src/components/table/model-deployments/modals/EditDeploymentModal.jsxweb/src/components/table/usage-logs/UsageLogsColumnDefs.jsxweb/src/components/topup/SubscriptionPlansCard.jsxweb/src/components/topup/index.jsxweb/src/hooks/models/useModelsData.jsx
💤 Files with no reviewable changes (3)
- controller/image.go
- web/src/components/table/usage-logs/UsageLogsColumnDefs.jsx
- controller/swag_video.go
✅ Files skipped from review due to trivial changes (34)
- dto/model_meta.go
- README.ja.md
- docker-compose.yml
- README.zh_CN.md
- dto/group.go
- dto/setup.go
- README.md
- controller/channel_upstream_update.go
- dto/ratio_sync_result.go
- dto/option.go
- controller/billing.go
- controller/telegram.go
- dto/codex_usage.go
- dto/uptime_kuma.go
- dto/channel_testing.go
- web/src/components/table/model-deployments/modals/EditDeploymentModal.jsx
- dto/channel_affinity_cache.go
- types/oauth_errors.go
- dto/token.go
- dto/performance.go
- dto/checkin.go
- dto/twofa.go
- dto/topup_creem.go
- dto/custom_oauth.go
- controller/pricing.go
- dto/subscription.go
- service/openaicompat/responses_to_chat.go
- dto/channel_billing.go
- dto/query_params.go
- controller/option.go
- dto/model_sync.go
- controller/group.go
- dto/topup.go
- dto/api_responses.go
🚧 Files skipped from review as they are similar to previous changes (31)
- types/openai_video.go
- types/user_settings.go
- README.zh_TW.md
- setting/chat.go
- service/openaicompat/chat_to_responses.go
- AGENTS.md
- controller/missing_models.go
- web/src/components/topup/SubscriptionPlansCard.jsx
- model/task.go
- controller/secure_verification.go
- dto/pricing.go
- model/twofa.go
- model/user_cache.go
- common/gin.go
- types/channel_settings.go
- controller/ratio_config.go
- router/main.go
- controller/relay.go
- controller/topup_stripe.go
- model/user.go
- CLAUDE.md
- model/channel.go
- controller/oauth.go
- controller/checkin.go
- dto/type_aliases.go
- controller/vendor_meta.go
- dto/channel.go
- go.mod
- controller/midjourney.go
- controller/subscription_payment_creem.go
- Dockerfile
0-don
force-pushed
the
feat/openapi-fuego
branch
from
4a3f172 to
da9bb7f
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
setting/console_setting/validation.go (1)
4-4:⚠️ Potential issue | 🟡 MinorReplace
encoding/jsonwithcommonwrapper functions.The
parseJSONArrayfunction directly usesjson.Unmarshal, which violates the coding guideline requiring all JSON operations to use wrapper functions fromcommon/json.go.Proposed fix
import ( - "encoding/json" "fmt" "net/url" "regexp"func parseJSONArray(jsonStr string, typeName string) ([]map[string]interface{}, error) { var list []map[string]interface{} - if err := json.Unmarshal([]byte(jsonStr), &list); err != nil { + if err := common.Unmarshal([]byte(jsonStr), &list); err != nil { return nil, fmt.Errorf("%s格式错误：%s", typeName, err.Error()) } return list, nil }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."Also applies to: 28-34
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@setting/console_setting/validation.go` at line 4, The parseJSONArray function (and other JSON uses in this file) currently calls encoding/json directly; remove the encoding/json import, import the common package instead, and replace direct json.Unmarshal calls with the common package's JSON wrapper (e.g., common.UnmarshalJSON or the project’s Unmarshal helper) when parsing into the target type (used in parseJSONArray and the code around lines 28-34). Ensure you propagate/handle the wrapper's returned error exactly as before and update imports accordingly.controller/subscription_payment_creem.go (1)
58-70:⚠️ Potential issue | 🟠 MajorReconcile the pending order when checkout creation fails.
order.Insert()happens beforegenCreemLink(). If Creem link generation returns an error, the handler exits with a permanent pending order, so retries can accumulate dead rows. Add a compensating delete or status update on the error path.Also applies to: 90-94
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/subscription_payment_creem.go` around lines 58 - 70, The handler currently inserts a SubscriptionOrder (via order.Insert()) before calling genCreemLink(), so if genCreemLink() fails the order remains stuck as pending; update the error paths to reconcile that pending order by either deleting the created SubscriptionOrder or updating its Status (and maybe adding an ErrorReason) when genCreemLink() returns an error; locate the order creation code referencing SubscriptionOrder, order.Insert(), and genCreemLink() and add a compensating delete or status update on every early return after genCreemLink() fails (also apply the same fix to the similar block around the second instance mentioned).controller/wechat.go (1)
4-17:⚠️ Potential issue | 🟠 MajorReplace direct
encoding/jsonusage with thecommon.DecodeJsonwrapper.This file imports
encoding/jsondirectly and usesjson.NewDecoder(httpResponse.Body).Decode(&res)at line 43 ingetWeChatIdByCode. The coding guidelines require all JSON operations to use wrapper functions fromcommon/json.go. Replace the decoder call withcommon.DecodeJson(httpResponse.Body, &res)and remove theencoding/jsonimport.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/wechat.go` around lines 4 - 17, In getWeChatIdByCode replace the direct json decoding call json.NewDecoder(httpResponse.Body).Decode(&res) with the project wrapper common.DecodeJson(httpResponse.Body, &res) and remove the direct encoding/json import; update the import block to drop "encoding/json", ensure common is imported (it already is) and handle/return any error from common.DecodeJson the same way the current decoder error is handled so behavior remains consistent.controller/topup.go (1)
27-50:⚠️ Potential issue | 🟠 MajorClone
operation_setting.PayMethodsbefore appending.
payMethods := operation_setting.PayMethodsonly copies the slice header. If eitherappendreuses the backing array, concurrentGetTopUpInforequests end up mutating shared config state.Minimal fix
- payMethods := operation_setting.PayMethods + payMethods := append([]map[string]string(nil), operation_setting.PayMethods...)Also applies to: 53-80
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/topup.go` around lines 27 - 50, GetTopUpInfo mutates the shared operation_setting.PayMethods slice because payMethods := operation_setting.PayMethods copies only the slice header; fix by cloning the slice before any append (e.g., create a new slice and copy elements or use append on an empty slice) so appends don’t mutate the shared backing array; update the initial assignment of payMethods (and the analogous block later around where additional methods are appended) to clone operation_setting.PayMethods before using append (refer to symbols: GetTopUpInfo, operation_setting.PayMethods, payMethods, stripeMethod, append).controller/model_meta.go (1)
4-4:⚠️ Potential issue | 🟠 MajorReplace
encoding/jsonwithcommonJSON wrappers.Direct import of
encoding/jsonviolates the coding guideline requiring all JSON operations to use wrapper functions fromcommon/json.go. Thejson.Marshalcalls inenrichModels(lines 177, 267) should usecommon.Marshalinstead.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."🔧 Proposed fix
Remove the
encoding/jsonimport and replacejson.Marshalwithcommon.Marshal:import ( - "encoding/json" "sort" "strconv" "strings"Then at lines 177-179:
- if b, err := json.Marshal(eps); err == nil { + if b, err := common.Marshal(eps); err == nil { mm.Endpoints = string(b) }And at line 267:
- if b, err := json.Marshal(eps); err == nil { + if b, err := common.Marshal(eps); err == nil { mm.Endpoints = string(b) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/model_meta.go` at line 4, Remove the direct encoding/json import and replace uses of json.Marshal inside enrichModels with the project wrapper common.Marshal; specifically, update the import list to drop "encoding/json" and change both json.Marshal calls in the enrichModels function (the calls around the previous line numbers 177 and 267) to call common.Marshal, ensuring any error handling is preserved and the common package is imported.
♻️ Duplicate comments (17)
controller/vendor_meta.go (1)
16-18:⚠️ Potential issue | 🟠 MajorPropagate the vendor count failure before returning the page.
The list query fails fast, but the count query still ignores
.Error, so a broken count silently turns into a successful page withtotal=0.Suggested fix
var total int64 - model.DB.Model(&model.Vendor{}).Count(&total) + if err := model.DB.Model(&model.Vendor{}).Count(&total).Error; err != nil { + return dto.FailPage[*model.Vendor](err.Error()) + } return dto.OkPage(pageInfo, vendors, int(total))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/vendor_meta.go` around lines 16 - 18, The count query ignores errors: capture and handle the result of model.DB.Model(&model.Vendor{}).Count(&total) (e.g., the returned db := model.DB.Model(&model.Vendor{}).Count(&total)) and if db.Error != nil return or propagate that error instead of always calling dto.OkPage(pageInfo, vendors, int(total)); ensure the function returns the error case before dto.OkPage so a failed count doesn't silently yield total=0.router/video-router.go (1)
19-19:⚠️ Potential issue | 🟠 MajorDocument the content proxy as binary, not
TaskResponseDoc.
/videos/:task_id/contentproxies media bytes, so registering it as the task-status JSON schema will publish the wrong content type inopenapi.jsonand mislead generated clients for this route.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@router/video-router.go` at line 19, The route registration incorrectly documents /videos/:task_id/content as JSON TaskResponseDoc; update the OpenAPI response registration for proxy.GinGet("/videos/:task_id/content", controller.VideoProxy, dto.GinResp[dto.TaskResponseDoc]()) to declare a binary/media response instead (e.g., use the framework's binary response helper such as a dto.GinBinary() or a GinResp variant that emits application/octet-stream or the correct media type) so the generated openapi.json and clients know this endpoint returns raw media bytes rather than JSON; keep the handler reference controller.VideoProxy and only change the response descriptor.README.fr.md (1)
228-229:⚠️ Potential issue | 🟡 MinorPolish this French OpenAPI sentence.
The current wording still reads awkwardly in French and is the same phrasing that was already flagged earlier.
Suggested wording
**Documentation API :** -- 📖 Spécification OpenAPI 3.0 auto-générée — activer avec `ENABLE_OPENAPI=true`, accessible à `/openapi.json` et `/swagger` (Scalar UI) +- 📖 Spécification OpenAPI 3.0 générée automatiquement — activez-la avec `ENABLE_OPENAPI=true`; la spécification est disponible via `/openapi.json` et l'interface Scalar via `/swagger`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.fr.md` around lines 228 - 229, Reformulez la phrase française concernant l'API OpenAPI pour qu'elle soit plus naturelle : remplacez la ligne contenant "Spécification OpenAPI 3.0 auto-générée — activer avec `ENABLE_OPENAPI=true`, accessible à `/openapi.json` et `/swagger` (Scalar UI)" par une version fluide, par exemple en indiquant clairement que l'OpenAPI 3.0 est générée automatiquement, comment l'activer avec la variable d'environnement ENABLE_OPENAPI=true, et où y accéder (/openapi.json et /swagger, interface Scalar UI) en une seule phrase concise et idiomatique.controller/redemption.go (2)
88-90:⚠️ Potential issue | 🟡 MinorReject malformed
idvalues before calling the delete path.
GetRedemptionalready uses the error-returning parser. This branch still lets an invalid path param flow straight intomodel.DeleteRedemptionById.Suggested fix
func DeleteRedemption(c fuego.ContextNoBody) (dto.MessageResponse, error) { - id := c.PathParamInt("id") - err := model.DeleteRedemptionById(id) + id, err := c.PathParamIntErr("id") + if err != nil { + return dto.FailMsg(err.Error()) + } + err = model.DeleteRedemptionById(id) if err != nil { return dto.FailMsg(err.Error()) } return dto.Msg("") }#!/bin/bash # Compare how redemption handlers parse route ids in controller/redemption.go. rg -nP -C2 'PathParamInt(?:Err)?\("id"\)' controller/redemption.goExpected:
controller/redemption.go:GetRedemptionusesPathParamIntErr("id");controller/redemption.go:DeleteRedemptionshould follow the same pattern.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/redemption.go` around lines 88 - 90, DeleteRedemption currently calls model.DeleteRedemptionById with an unvalidated path param; mirror GetRedemption by using c.PathParamIntErr("id") to parse and validate the id, return the parsing error immediately if non-nil, and only then call model.DeleteRedemptionById; update the DeleteRedemption function's control flow to return the parse error (same error type/handling used by GetRedemption) instead of passing malformed ids into the model layer.
78-81:⚠️ Potential issue | 🟡 MinorKeep the partial-insert failure on the standard error shape.
This is still the one failure branch here that hand-builds
dto.Responseinstead of returning the normal failure envelope. At minimum, mark it explicitly as unsuccessful.Suggested fix
return &dto.Response[[]string]{ + Success: false, Message: common.TranslateMessage(ginCtx, i18n.MsgRedemptionCreateFailed), Data: keys, }, nil🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/redemption.go` around lines 78 - 81, The partial-insert failure branch currently constructs a dto.Response[[]string] without marking it as a failure; update the return to use the standard failure shape by setting Success: false on the constructed response (e.g., &dto.Response[[]string]{Success: false, Message: common.TranslateMessage(ginCtx, i18n.MsgRedemptionCreateFailed), Data: keys}) so this branch explicitly signals failure; if your codebase has a canonical failure helper (e.g., NewFailureResponse / common.FailureResponse), prefer using that helper instead of hand-building the envelope.web/src/components/topup/SubscriptionPlansCard.jsx (1)
126-127:⚠️ Potential issue | 🟠 MajorGuard provider payloads before redirecting or submitting.
These success branches still assume the backend returned the expected URL payload. Missing
pay_link/checkout_urlopens a blank tab, and missing Epaydatathrows onres.data.data.url.Suggested fix
if (res.data?.success) { - window.open(res.data.data?.pay_link, '_blank'); + const payLink = res.data?.data?.pay_link; + if (!payLink) { + showError(res.data?.message || t('支付链接缺失')); + return; + } + window.open(payLink, '_blank'); showSuccess(t('已打开支付页面')); closeBuy(); } else { showError(res.data?.message || t('支付失败')); } @@ if (res.data?.success) { - window.open(res.data.data?.checkout_url, '_blank'); + const checkoutUrl = res.data?.data?.checkout_url; + if (!checkoutUrl) { + showError(res.data?.message || t('支付链接缺失')); + return; + } + window.open(checkoutUrl, '_blank'); showSuccess(t('已打开支付页面')); closeBuy(); } else { showError(res.data?.message || t('支付失败')); } @@ if (res.data?.success) { - submitEpayForm({ url: res.data.data.url, params: res.data.data.params }); + const epayUrl = res.data?.data?.url; + const epayParams = res.data?.data?.params; + if (!epayUrl || !epayParams || typeof epayParams !== 'object') { + showError(res.data?.message || t('支付链接缺失')); + return; + } + submitEpayForm({ url: epayUrl, params: epayParams }); showSuccess(t('已发起支付')); closeBuy(); } else { showError(res.data?.message || t('支付失败')); }Also applies to: 150-151, 175-176
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/topup/SubscriptionPlansCard.jsx` around lines 126 - 127, The success branches in SubscriptionPlansCard.jsx assume payload URL fields exist (res.data.data.pay_link, res.data.data.url, res.data.checkout_url) which can open blank tabs or throw; update the success handlers to validate the URL before redirecting/submitting: check that res?.data?.data and a non-empty string URL (pay_link or url) or res?.data?.checkout_url exist and are valid, only then call window.open or submit the form, otherwise handle the error path (show toast/error message and avoid opening a blank tab). Apply this guard to the three occurrences around the shown lines (the pay_link case and the other two checkout_url/url cases).web/src/components/topup/index.jsx (3)
783-785:⚠️ Potential issue | 🟡 MinorGuard the Creem amount display against incomplete product data.
If
priceorcurrencyis missing, this row rendersNaNorundefined. Reject incomplete products before opening the modal or add safe fallbacks here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/topup/index.jsx` around lines 783 - 785, The Creem product display can render "NaN" or "undefined" when selectedCreemProduct.price or selectedCreemProduct.currency is missing; update the render logic around selectedCreemProduct in the component (the JSX that currently accesses selectedCreemProduct.currency and Number(selectedCreemProduct.price).toFixed(2)) to first validate the product and provide safe fallbacks: only render the row if selectedCreemProduct exists and Number.isFinite(Number(selectedCreemProduct.price)), otherwise show a placeholder (e.g., "-" or "N/A"), and resolve currency with a safe map lookup using optional chaining and a default of selectedCreemProduct.currency or an empty string; alternatively, prevent the modal from opening when the product is incomplete by adding the same validation where the modal is triggered.
259-262:⚠️ Potential issue | 🟡 MinorDon't pass
undefinedintoshowError.These fallback branches still call
showError(res)when the response object is missing, so the user gets an empty error toast in the payment and amount flows. Reuse the translated fallback you already use in the!successpaths.Also applies to: 302-305, 624-632, 650-658
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/topup/index.jsx` around lines 259 - 262, Certain branches call showError(res) when res may be undefined, producing empty toasts; update those fallback calls (e.g., in the payment/amount flows where showError is invoked after an unsuccessful API response) to pass the translated fallback string instead of res — reuse the same t('支付失败') (or the local variable message || t('支付失败')) pattern used in the !success paths; locate uses of showError in this component (the branches around success checks and the calls at the referenced spots) and replace showError(res) with showError(message || t('支付失败')) or showError(t('支付失败')) as appropriate.
229-237:⚠️ Potential issue | 🟠 MajorValidate redirect payloads before using them.
The success branches still assume
data.pay_link,data.url,data.params, anddata.checkout_urlare present. A partial success payload will currently open a blank tab or submit a broken form instead of surfacing a recoverable error.Also applies to: 298-300
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/topup/index.jsx` around lines 229 - 237, The success branch assumes payload fields exist; validate payloads before using them: check that data is defined and that data.pay_link is a non-empty string before calling window.open (in the payWay === 'stripe' branch), and verify data.url is a non-empty string and data.params is an object/has expected keys before building/submitting the form (and similarly check data.checkout_url where used around the other block at 298-300). If any required field is missing or invalid, do not open a blank tab or submit a broken form—instead surface a user-facing error/console error and abort the action. Ensure checks reference the local variables payWay, data.pay_link, data.url, data.params, and data.checkout_url so the fix covers both spots mentioned.controller/prefill_group.go (1)
49-58:⚠️ Potential issue | 🟠 MajorKeep
NameandTyperequired on update too.
CreatePrefillGrouprejects emptyName/Type, butUpdatePrefillGroupstill accepts them and can clear required fields.Minimal fix
if g.Id == 0 { return dto.Fail[model.PrefillGroup]("缺少组 ID") } + if g.Name == "" || g.Type == "" { + return dto.Fail[model.PrefillGroup]("组名称和类型不能为空") + } // 名称冲突检查 dup, err := model.IsPrefillGroupNameDuplicated(g.Id, g.Name)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/prefill_group.go` around lines 49 - 58, UpdatePrefillGroup currently allows clearing required fields; add the same non-empty checks used in CreatePrefillGroup to UpdatePrefillGroup by validating g.Name and g.Type at the start of the update path (before duplication and DB operations). Specifically, in the UpdatePrefillGroup handler/function, ensure you return dto.Fail when g.Name == "" or g.Type == "" (use the same error messages as CreatePrefillGroup), then proceed with the existing duplication check (model.IsPrefillGroupNameDuplicated) and update logic so required fields cannot be blank on update.controller/channel-test.go (1)
749-760:⚠️ Potential issue | 🟡 MinorCompute the elapsed time before the early failure return.
result.localErrstill exits before timing is calculated, so failed tests always reportTime: 0, and the laternewAPIErrorbranch never runs.Minimal fix
tik := time.Now() result := testChannel(channel, testModel, endpointType, isStream) + tok := time.Now() + milliseconds := tok.Sub(tik).Milliseconds() + consumedTime := float64(milliseconds) / 1000.0 if result.localErr != nil { - return dto.TestChannelResponse{Success: false, Message: result.localErr.Error(), Time: 0.0}, nil - } - tok := time.Now() - milliseconds := tok.Sub(tik).Milliseconds() - go channel.UpdateResponseTime(milliseconds) - consumedTime := float64(milliseconds) / 1000.0 - if result.newAPIError != nil { - return dto.TestChannelResponse{Success: false, Message: result.newAPIError.Error(), Time: consumedTime}, nil + msg := result.localErr.Error() + if result.newAPIError != nil { + msg = result.newAPIError.Error() + } + return dto.TestChannelResponse{Success: false, Message: msg, Time: consumedTime}, nil } + go channel.UpdateResponseTime(milliseconds) return dto.TestChannelResponse{Success: true, Message: "", Time: consumedTime}, nil🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/channel-test.go` around lines 749 - 760, The elapsed time is computed after checking result.localErr so failures report Time: 0; move the timing logic to happen immediately after calling testChannel and before any early returns: after receiving result from testChannel (in function containing tik/tok), compute tok := time.Now(); milliseconds := tok.Sub(tik).Milliseconds(); call go channel.UpdateResponseTime(milliseconds); set consumedTime := float64(milliseconds)/1000.0 and then use consumedTime in both early return for result.localErr and the later newAPIError branch (and any other returns) so failed and successful TestChannelResponse values carry the correct elapsed Time.controller/misc.go (2)
230-246:⚠️ Potential issue | 🟠 MajorMake reset-email responses indistinguishable.
Returning
该邮箱地址未注册still lets unauthenticated callers enumerate accounts. Return the same generic success payload either way and only send mail when the account exists.Minimal fix
- if !model.IsEmailAlreadyTaken(p.Email) { - return dto.FailMsg("该邮箱地址未注册") - } + if !model.IsEmailAlreadyTaken(p.Email) { + return dto.Msg("") + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/misc.go` around lines 230 - 246, The current flow leaks account existence by returning dto.FailMsg("该邮箱地址未注册") when model.IsEmailAlreadyTaken(p.Email) is false; change it so the handler always returns the same generic success response (e.g., dto.Msg("")) to callers, and only perform GenerateVerificationCode/RegisterVerificationCodeWithKey and SendEmail when model.IsEmailAlreadyTaken(p.Email) is true; keep using common.GenerateVerificationCode, common.RegisterVerificationCodeWithKey and common.SendEmail for the send-path and keep error handling/logging internal, but never return an "email not registered" error to the client.
258-261:⚠️ Potential issue | 🟠 MajorDon’t log raw email addresses on reset failures.
This public recovery path still writes user identifiers into logs. Omit or mask
req.Emailbefore callingSysError.Minimal fix
- common.SysError("failed to reset password for " + req.Email + ": " + err.Error()) + common.SysError("failed to reset password: " + err.Error())🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/misc.go` around lines 258 - 261, The SysError call in the password reset path logs the raw req.Email; replace that with a masked identifier before calling common.SysError (e.g., compute a maskedEmail from req.Email by hiding most of the local-part or only logging the domain), then call common.SysError with the maskedEmail and err.Error(); update the block around model.ResetUserPasswordByEmail, req.Email, and common.SysError to use maskedEmail so no full email is written to logs.controller/topup.go (1)
173-176:⚠️ Potential issue | 🟡 MinorKeep the quote and checkout minimum rule identical.
RequestEpayandRequestAmountstill disagree on whether0.01is valid, so the quote flow can reject an amount the payment flow accepts.Also applies to: 367-368
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/topup.go` around lines 173 - 176, The minimum-amount check is inconsistent between the quote and payment flows; make the rule identical by centralizing the threshold into a single constant or helper and using it in both places (e.g., define MIN_TOPUP_AMOUNT and/or a helper like isValidTopupAmount or reuse getPayMoney logic) so RequestEpay, RequestAmount and any checks that currently compare against 0.01 all reference that single symbol (update the check around getPayMoney and the corresponding validation in RequestAmount/RequestEpay to use the shared constant/helper).router/openapi.go (1)
89-95:⚠️ Potential issue | 🟠 MajorWire
anthropicApiKeyinto a helper that routes actually use.This file registers
anthropicApiKey, but none of the exported security helpers reference it. Any route that only usessecToken()orsecTokenOrDashboard()will still be documented as Bearer-only unless it manually overrides security elsewhere.#!/bin/bash # Expect anthropicApiKey to be referenced by a reusable helper or route registration, # not only by the scheme definition itself. rg -n -C2 'anthropicApiKey|secToken\(|secTokenOrDashboard\(|option\.Security\(' \ router/openapi.go router/relay-router.goAlso applies to: 120-149
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@router/openapi.go` around lines 89 - 95, The OpenAPI registration defines the security scheme "anthropicApiKey" but the exported security helpers (secToken, secTokenOrDashboard) and route registration do not reference it, leaving routes documented as Bearer-only; update the helpers and any reusable security helper usage to include the anthropicApiKey scheme (e.g., add a reference to the "anthropicApiKey" scheme in the same place they add the Bearer scheme or option.Security), and ensure routes that call secToken() or secTokenOrDashboard() now compose both schemes so generated docs include the x-api-key option; update functions named secToken, secTokenOrDashboard and any option.Security composition code to include the "anthropicApiKey" security requirement alongside the existing Bearer scheme.router/api-router.go (1)
40-41:⚠️ Potential issue | 🟠 MajorKeep
GET /api/modelsbackward compatible.
/api/modelsnow servesDashboardListModelswhileGetAllModelsMetamoved to/api/models/list, which breaks existing admin clients instead of preserving the legacy path.Also applies to: 377-384
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@router/api-router.go` around lines 40 - 41, Restore backward compatibility by keeping the legacy GET /api/models route mapped to the original handler and move the dashboard handler to the new path: change the dto.Get call that currently wires controller.DashboardListModels to "/models" so that controller.GetAllModelsMeta handles "/models" and add a new dto.Get for controller.DashboardListModels at "/models/list" (use the same publicModels/router setup created by dto.NewRouter and preserve middleware via apiRouter.Group and secDashboard()).controller/channel-billing.go (1)
78-80:⚠️ Potential issue | 🟠 MajorUse
common.Unmarshalin these updated balance decoders.These changed parsers still call
json.Unmarshal, which bypasses the repo JSON wrappers. Switch each site tocommon.Unmarshal(body, &response).#!/bin/bash # Expect no direct json.Unmarshal calls in the updated balance parsing paths. rg -n -C1 'json\.Unmarshal\(' controller/channel-billing.goAs per coding guidelines,
All JSON marshal/unmarshal operations MUST use wrapper functions in common/json.go.Also applies to: 93-95, 117-119, 136-138, 151-153, 173-175, 202-204, 217-219, 300-302, 316-318
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/channel-billing.go` around lines 78 - 80, Replace direct calls to json.Unmarshal in the updated balance parsing code paths with the repo wrapper common.Unmarshal; e.g., where you create the response struct dto.OpenAICreditGrants and call json.Unmarshal(body, &response) (and the other similar response variables in the same file) change each call to common.Unmarshal(body, &response) and propagate the returned error handling unchanged. Ensure every occurrence the reviewer listed (the balance decoders that create response structs and call json.Unmarshal) is updated so no direct json.Unmarshal calls remain in controller/channel-billing.go.
🧹 Nitpick comments (3)
model/user.go (1)
837-853: Avoid shadowing in cache-hit branch for clarity.Line 850 uses
:=and shadows bothsettinganderr; consider explicitcachedSetting/cacheErrnames to make the control flow less error-prone.Optional cleanup
if !fromDB && common.RedisEnabled { - setting, err := getUserSettingCache(id) - if err == nil { - return setting, nil + cachedSetting, cacheErr := getUserSettingCache(id) + if cacheErr == nil { + return cachedSetting, nil } // Don't return error - fall through to DB }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@model/user.go` around lines 837 - 853, In GetUserSetting, avoid shadowing the outer variables by not using := for the cache branch; call getUserSettingCache into new locals (e.g., cachedSetting, cacheErr) or assign explicitly to the existing outer vars (setting, err) without :=, then handle cacheErr and return the appropriate types; update references to setting/err in the cache-hit branch to use the chosen non-shadowing names and keep the defer logic that calls shouldUpdateRedis/updateUserSettingCache unchanged.dto/responses.go (1)
13-18: Make the gin-only assumption explicit in these helpers.
FuegoCtxreads like a generic Fuego contract, butGinCtx/UserID/PageInfoimmediately type-assertc.Context()to*gin.Context. That will panic under non-gin adapters or lightweight test doubles, so either narrow the abstraction to gin-backed contexts or guard the cast.Also applies to: 48-57
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dto/responses.go` around lines 13 - 18, The helpers assume the underlying fuego.Context is gin-backed but FuegoCtx is generic; make this explicit by narrowing the abstraction or guarding casts: either rename/type-rename FuegoCtx to GinFuegoCtx (or add a clear comment) and change the helper signatures (GinCtx, UserID, PageInfo) to accept/expect *gin.Context, or keep FuegoCtx but modify GinCtx, UserID, and PageInfo to perform a safe type assertion on c.Context() (use v, ok := c.Context().(*gin.Context)) and handle the non-gin case by returning an error/zero value instead of panicking; update callers accordingly. Ensure references to FuegoCtx, GinCtx, UserID, and PageInfo in the file are updated to match the chosen approach.controller/subscription.go (1)
20-22: Avoid usingmodel.SubscriptionPlanas the admin request schema.Now that these handlers feed the OpenAPI spec, binding straight into the GORM model leaks persistence fields into the API contract and lets create accept fields you never validate. A dedicated admin DTO with only writable fields would keep the spec and handler surface safer.
Also applies to: 107-112, 155-163
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/subscription.go` around lines 20 - 22, The handler currently exposes the GORM model by using model.SubscriptionPlan in AdminUpsertSubscriptionPlanRequest (and other request structs that bind model.SubscriptionPlan); instead define a dedicated admin DTO (e.g., AdminSubscriptionPlanDTO) that contains only the writable API fields, replace AdminUpsertSubscriptionPlanRequest.Plan's type with that DTO, update any other request structs that currently reference model.SubscriptionPlan, and in the handler map/validate the DTO -> model.SubscriptionPlan before persisting so persistence-only fields are not part of the OpenAPI contract or accepted input.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@model/user.go`:
- Around line 79-83: Replace direct encoding/json usage in model/user.go by
calling the repository JSON wrapper functions from common/json.go: in
User.GetSetting replace json.Unmarshal([]byte(user.Setting), &setting) with the
common unmarshal wrapper (e.g., common.UnmarshalJSON or the wrapper function
defined in common/json.go) and propagate/handle its error; likewise replace the
json.Marshal call(s) used to persist user.Setting (e.g., in SetSetting or the
method that writes user.Setting) with the common marshal wrapper (e.g.,
common.MarshalJSON) and handle the returned bytes/error the same way as before.
Ensure you import and call the exact wrapper function names provided in
common/json.go and keep existing error handling behavior.
In `@router/api-router.go`:
- Around line 93-95: The OpenAPI annotation is wrong: controller.EpayNotify
returns plain "success"/"fail" text but the route was documented with
dto.GinResp[dto.MessageResponse] via userPaymentPublic.GinPost/GinGet; update
the route documentation for /epay/notify to reflect a plain text response (e.g.,
remove dto.MessageResponse wrapper and use a raw/text response type or
dto.GinResp[string]/a dedicated plain-text response helper) so the generated
schema/content-type is text/plain and matches controller.EpayNotify.
In `@web/src/components/topup/index.jsx`:
- Around line 771-777: The label "充值数量" is incorrect for the value being
rendered; update the UI in the component that renders selectedCreemProduct.name
so the label matches the data or render the correct quantity field: either
change the label text to something like "充值产品" (or "产品名称") where the JSX uses
selectedCreemProduct.name, or replace selectedCreemProduct.name with the actual
quantity/quota property (e.g., selectedCreemProduct.amount,
selectedCreemProduct.quota, or selectedCreemProduct.value) if such a field
exists; locate the pair in the topup component where selectedCreemProduct.name
is displayed and make the label/data consistent.
---
Outside diff comments:
In `@controller/model_meta.go`:
- Line 4: Remove the direct encoding/json import and replace uses of
json.Marshal inside enrichModels with the project wrapper common.Marshal;
specifically, update the import list to drop "encoding/json" and change both
json.Marshal calls in the enrichModels function (the calls around the previous
line numbers 177 and 267) to call common.Marshal, ensuring any error handling is
preserved and the common package is imported.
In `@controller/subscription_payment_creem.go`:
- Around line 58-70: The handler currently inserts a SubscriptionOrder (via
order.Insert()) before calling genCreemLink(), so if genCreemLink() fails the
order remains stuck as pending; update the error paths to reconcile that pending
order by either deleting the created SubscriptionOrder or updating its Status
(and maybe adding an ErrorReason) when genCreemLink() returns an error; locate
the order creation code referencing SubscriptionOrder, order.Insert(), and
genCreemLink() and add a compensating delete or status update on every early
return after genCreemLink() fails (also apply the same fix to the similar block
around the second instance mentioned).
In `@controller/topup.go`:
- Around line 27-50: GetTopUpInfo mutates the shared
operation_setting.PayMethods slice because payMethods :=
operation_setting.PayMethods copies only the slice header; fix by cloning the
slice before any append (e.g., create a new slice and copy elements or use
append on an empty slice) so appends don’t mutate the shared backing array;
update the initial assignment of payMethods (and the analogous block later
around where additional methods are appended) to clone
operation_setting.PayMethods before using append (refer to symbols:
GetTopUpInfo, operation_setting.PayMethods, payMethods, stripeMethod, append).
In `@controller/wechat.go`:
- Around line 4-17: In getWeChatIdByCode replace the direct json decoding call
json.NewDecoder(httpResponse.Body).Decode(&res) with the project wrapper
common.DecodeJson(httpResponse.Body, &res) and remove the direct encoding/json
import; update the import block to drop "encoding/json", ensure common is
imported (it already is) and handle/return any error from common.DecodeJson the
same way the current decoder error is handled so behavior remains consistent.
In `@setting/console_setting/validation.go`:
- Line 4: The parseJSONArray function (and other JSON uses in this file)
currently calls encoding/json directly; remove the encoding/json import, import
the common package instead, and replace direct json.Unmarshal calls with the
common package's JSON wrapper (e.g., common.UnmarshalJSON or the project’s
Unmarshal helper) when parsing into the target type (used in parseJSONArray and
the code around lines 28-34). Ensure you propagate/handle the wrapper's returned
error exactly as before and update imports accordingly.
---
Duplicate comments:
In `@controller/channel-billing.go`:
- Around line 78-80: Replace direct calls to json.Unmarshal in the updated
balance parsing code paths with the repo wrapper common.Unmarshal; e.g., where
you create the response struct dto.OpenAICreditGrants and call
json.Unmarshal(body, &response) (and the other similar response variables in the
same file) change each call to common.Unmarshal(body, &response) and propagate
the returned error handling unchanged. Ensure every occurrence the reviewer
listed (the balance decoders that create response structs and call
json.Unmarshal) is updated so no direct json.Unmarshal calls remain in
controller/channel-billing.go.
In `@controller/channel-test.go`:
- Around line 749-760: The elapsed time is computed after checking
result.localErr so failures report Time: 0; move the timing logic to happen
immediately after calling testChannel and before any early returns: after
receiving result from testChannel (in function containing tik/tok), compute tok
:= time.Now(); milliseconds := tok.Sub(tik).Milliseconds(); call go
channel.UpdateResponseTime(milliseconds); set consumedTime :=
float64(milliseconds)/1000.0 and then use consumedTime in both early return for
result.localErr and the later newAPIError branch (and any other returns) so
failed and successful TestChannelResponse values carry the correct elapsed Time.
In `@controller/misc.go`:
- Around line 230-246: The current flow leaks account existence by returning
dto.FailMsg("该邮箱地址未注册") when model.IsEmailAlreadyTaken(p.Email) is false; change
it so the handler always returns the same generic success response (e.g.,
dto.Msg("")) to callers, and only perform
GenerateVerificationCode/RegisterVerificationCodeWithKey and SendEmail when
model.IsEmailAlreadyTaken(p.Email) is true; keep using
common.GenerateVerificationCode, common.RegisterVerificationCodeWithKey and
common.SendEmail for the send-path and keep error handling/logging internal, but
never return an "email not registered" error to the client.
- Around line 258-261: The SysError call in the password reset path logs the raw
req.Email; replace that with a masked identifier before calling common.SysError
(e.g., compute a maskedEmail from req.Email by hiding most of the local-part or
only logging the domain), then call common.SysError with the maskedEmail and
err.Error(); update the block around model.ResetUserPasswordByEmail, req.Email,
and common.SysError to use maskedEmail so no full email is written to logs.
In `@controller/prefill_group.go`:
- Around line 49-58: UpdatePrefillGroup currently allows clearing required
fields; add the same non-empty checks used in CreatePrefillGroup to
UpdatePrefillGroup by validating g.Name and g.Type at the start of the update
path (before duplication and DB operations). Specifically, in the
UpdatePrefillGroup handler/function, ensure you return dto.Fail when g.Name ==
"" or g.Type == "" (use the same error messages as CreatePrefillGroup), then
proceed with the existing duplication check (model.IsPrefillGroupNameDuplicated)
and update logic so required fields cannot be blank on update.
In `@controller/redemption.go`:
- Around line 88-90: DeleteRedemption currently calls model.DeleteRedemptionById
with an unvalidated path param; mirror GetRedemption by using
c.PathParamIntErr("id") to parse and validate the id, return the parsing error
immediately if non-nil, and only then call model.DeleteRedemptionById; update
the DeleteRedemption function's control flow to return the parse error (same
error type/handling used by GetRedemption) instead of passing malformed ids into
the model layer.
- Around line 78-81: The partial-insert failure branch currently constructs a
dto.Response[[]string] without marking it as a failure; update the return to use
the standard failure shape by setting Success: false on the constructed response
(e.g., &dto.Response[[]string]{Success: false, Message:
common.TranslateMessage(ginCtx, i18n.MsgRedemptionCreateFailed), Data: keys}) so
this branch explicitly signals failure; if your codebase has a canonical failure
helper (e.g., NewFailureResponse / common.FailureResponse), prefer using that
helper instead of hand-building the envelope.
In `@controller/topup.go`:
- Around line 173-176: The minimum-amount check is inconsistent between the
quote and payment flows; make the rule identical by centralizing the threshold
into a single constant or helper and using it in both places (e.g., define
MIN_TOPUP_AMOUNT and/or a helper like isValidTopupAmount or reuse getPayMoney
logic) so RequestEpay, RequestAmount and any checks that currently compare
against 0.01 all reference that single symbol (update the check around
getPayMoney and the corresponding validation in RequestAmount/RequestEpay to use
the shared constant/helper).
In `@controller/vendor_meta.go`:
- Around line 16-18: The count query ignores errors: capture and handle the
result of model.DB.Model(&model.Vendor{}).Count(&total) (e.g., the returned db
:= model.DB.Model(&model.Vendor{}).Count(&total)) and if db.Error != nil return
or propagate that error instead of always calling dto.OkPage(pageInfo, vendors,
int(total)); ensure the function returns the error case before dto.OkPage so a
failed count doesn't silently yield total=0.
In `@README.fr.md`:
- Around line 228-229: Reformulez la phrase française concernant l'API OpenAPI
pour qu'elle soit plus naturelle : remplacez la ligne contenant "Spécification
OpenAPI 3.0 auto-générée — activer avec `ENABLE_OPENAPI=true`, accessible à
`/openapi.json` et `/swagger` (Scalar UI)" par une version fluide, par exemple
en indiquant clairement que l'OpenAPI 3.0 est générée automatiquement, comment
l'activer avec la variable d'environnement ENABLE_OPENAPI=true, et où y accéder
(/openapi.json et /swagger, interface Scalar UI) en une seule phrase concise et
idiomatique.
In `@router/api-router.go`:
- Around line 40-41: Restore backward compatibility by keeping the legacy GET
/api/models route mapped to the original handler and move the dashboard handler
to the new path: change the dto.Get call that currently wires
controller.DashboardListModels to "/models" so that controller.GetAllModelsMeta
handles "/models" and add a new dto.Get for controller.DashboardListModels at
"/models/list" (use the same publicModels/router setup created by dto.NewRouter
and preserve middleware via apiRouter.Group and secDashboard()).
In `@router/openapi.go`:
- Around line 89-95: The OpenAPI registration defines the security scheme
"anthropicApiKey" but the exported security helpers (secToken,
secTokenOrDashboard) and route registration do not reference it, leaving routes
documented as Bearer-only; update the helpers and any reusable security helper
usage to include the anthropicApiKey scheme (e.g., add a reference to the
"anthropicApiKey" scheme in the same place they add the Bearer scheme or
option.Security), and ensure routes that call secToken() or
secTokenOrDashboard() now compose both schemes so generated docs include the
x-api-key option; update functions named secToken, secTokenOrDashboard and any
option.Security composition code to include the "anthropicApiKey" security
requirement alongside the existing Bearer scheme.
In `@router/video-router.go`:
- Line 19: The route registration incorrectly documents /videos/:task_id/content
as JSON TaskResponseDoc; update the OpenAPI response registration for
proxy.GinGet("/videos/:task_id/content", controller.VideoProxy,
dto.GinResp[dto.TaskResponseDoc]()) to declare a binary/media response instead
(e.g., use the framework's binary response helper such as a dto.GinBinary() or a
GinResp variant that emits application/octet-stream or the correct media type)
so the generated openapi.json and clients know this endpoint returns raw media
bytes rather than JSON; keep the handler reference controller.VideoProxy and
only change the response descriptor.
In `@web/src/components/topup/index.jsx`:
- Around line 783-785: The Creem product display can render "NaN" or "undefined"
when selectedCreemProduct.price or selectedCreemProduct.currency is missing;
update the render logic around selectedCreemProduct in the component (the JSX
that currently accesses selectedCreemProduct.currency and
Number(selectedCreemProduct.price).toFixed(2)) to first validate the product and
provide safe fallbacks: only render the row if selectedCreemProduct exists and
Number.isFinite(Number(selectedCreemProduct.price)), otherwise show a
placeholder (e.g., "-" or "N/A"), and resolve currency with a safe map lookup
using optional chaining and a default of selectedCreemProduct.currency or an
empty string; alternatively, prevent the modal from opening when the product is
incomplete by adding the same validation where the modal is triggered.
- Around line 259-262: Certain branches call showError(res) when res may be
undefined, producing empty toasts; update those fallback calls (e.g., in the
payment/amount flows where showError is invoked after an unsuccessful API
response) to pass the translated fallback string instead of res — reuse the same
t('支付失败') (or the local variable message || t('支付失败')) pattern used in the
!success paths; locate uses of showError in this component (the branches around
success checks and the calls at the referenced spots) and replace showError(res)
with showError(message || t('支付失败')) or showError(t('支付失败')) as appropriate.
- Around line 229-237: The success branch assumes payload fields exist; validate
payloads before using them: check that data is defined and that data.pay_link is
a non-empty string before calling window.open (in the payWay === 'stripe'
branch), and verify data.url is a non-empty string and data.params is an
object/has expected keys before building/submitting the form (and similarly
check data.checkout_url where used around the other block at 298-300). If any
required field is missing or invalid, do not open a blank tab or submit a broken
form—instead surface a user-facing error/console error and abort the action.
Ensure checks reference the local variables payWay, data.pay_link, data.url,
data.params, and data.checkout_url so the fix covers both spots mentioned.
In `@web/src/components/topup/SubscriptionPlansCard.jsx`:
- Around line 126-127: The success branches in SubscriptionPlansCard.jsx assume
payload URL fields exist (res.data.data.pay_link, res.data.data.url,
res.data.checkout_url) which can open blank tabs or throw; update the success
handlers to validate the URL before redirecting/submitting: check that
res?.data?.data and a non-empty string URL (pay_link or url) or
res?.data?.checkout_url exist and are valid, only then call window.open or
submit the form, otherwise handle the error path (show toast/error message and
avoid opening a blank tab). Apply this guard to the three occurrences around the
shown lines (the pay_link case and the other two checkout_url/url cases).
---
Nitpick comments:
In `@controller/subscription.go`:
- Around line 20-22: The handler currently exposes the GORM model by using
model.SubscriptionPlan in AdminUpsertSubscriptionPlanRequest (and other request
structs that bind model.SubscriptionPlan); instead define a dedicated admin DTO
(e.g., AdminSubscriptionPlanDTO) that contains only the writable API fields,
replace AdminUpsertSubscriptionPlanRequest.Plan's type with that DTO, update any
other request structs that currently reference model.SubscriptionPlan, and in
the handler map/validate the DTO -> model.SubscriptionPlan before persisting so
persistence-only fields are not part of the OpenAPI contract or accepted input.
In `@dto/responses.go`:
- Around line 13-18: The helpers assume the underlying fuego.Context is
gin-backed but FuegoCtx is generic; make this explicit by narrowing the
abstraction or guarding casts: either rename/type-rename FuegoCtx to GinFuegoCtx
(or add a clear comment) and change the helper signatures (GinCtx, UserID,
PageInfo) to accept/expect *gin.Context, or keep FuegoCtx but modify GinCtx,
UserID, and PageInfo to perform a safe type assertion on c.Context() (use v, ok
:= c.Context().(*gin.Context)) and handle the non-gin case by returning an
error/zero value instead of panicking; update callers accordingly. Ensure
references to FuegoCtx, GinCtx, UserID, and PageInfo in the file are updated to
match the chosen approach.
In `@model/user.go`:
- Around line 837-853: In GetUserSetting, avoid shadowing the outer variables by
not using := for the cache branch; call getUserSettingCache into new locals
(e.g., cachedSetting, cacheErr) or assign explicitly to the existing outer vars
(setting, err) without :=, then handle cacheErr and return the appropriate
types; update references to setting/err in the cache-hit branch to use the
chosen non-shadowing names and keep the defer logic that calls
shouldUpdateRedis/updateUserSettingCache unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 682bd8b2-ec72-439d-9fef-13ecff74aa12
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (120)
AGENTS.mdCLAUDE.mdDockerfileREADME.fr.mdREADME.ja.mdREADME.mdREADME.zh_CN.mdREADME.zh_TW.mdcommon/gin.gocontroller/billing.gocontroller/channel-billing.gocontroller/channel-test.gocontroller/channel.gocontroller/channel_affinity_cache.gocontroller/channel_upstream_update.gocontroller/checkin.gocontroller/codex_oauth.gocontroller/codex_usage.gocontroller/console_migrate.gocontroller/custom_oauth.gocontroller/deployment.gocontroller/group.gocontroller/image.gocontroller/log.gocontroller/midjourney.gocontroller/misc.gocontroller/missing_models.gocontroller/model.gocontroller/model_meta.gocontroller/model_sync.gocontroller/oauth.gocontroller/option.gocontroller/passkey.gocontroller/performance.gocontroller/prefill_group.gocontroller/pricing.gocontroller/ratio_config.gocontroller/ratio_sync.gocontroller/redemption.gocontroller/relay.gocontroller/secure_verification.gocontroller/setup.gocontroller/subscription.gocontroller/subscription_payment_creem.gocontroller/subscription_payment_epay.gocontroller/subscription_payment_stripe.gocontroller/swag_video.gocontroller/task.gocontroller/telegram.gocontroller/token.gocontroller/topup.gocontroller/topup_creem.gocontroller/topup_stripe.gocontroller/twofa.gocontroller/uptime_kuma.gocontroller/usedata.gocontroller/user.gocontroller/vendor_meta.gocontroller/wechat.godocker-compose.ymldocs/openapi/api.jsondocs/openapi/relay.jsondto/api_responses.godto/channel.godto/channel_affinity_cache.godto/channel_billing.godto/channel_testing.godto/checkin.godto/codex_usage.godto/custom_oauth.godto/deployment.godto/group.godto/misc.godto/model_meta.godto/model_sync.godto/openai_request.godto/option.godto/params.godto/performance.godto/pricing.godto/query_params.godto/ratio_sync_result.godto/responses.godto/router.godto/setup.godto/subscription.godto/token.godto/topup.godto/topup_creem.godto/twofa.godto/type_aliases.godto/uptime_kuma.godto/user.gogo.modmodel/channel.gomodel/task.gomodel/twofa.gomodel/user.gomodel/user_cache.gorelay/channel/claude/relay-claude.gorouter/api-router.gorouter/dashboard.gorouter/main.gorouter/openapi.gorouter/relay-router.gorouter/video-router.goservice/openaicompat/chat_to_responses.goservice/openaicompat/responses_to_chat.gosetting/chat.gosetting/console_setting/validation.gosetting/ratio_setting/exposed_cache.gotypes/channel_settings.gotypes/oauth_errors.gotypes/openai_video.gotypes/user_settings.goweb/src/components/table/model-deployments/modals/EditDeploymentModal.jsxweb/src/components/table/usage-logs/UsageLogsColumnDefs.jsxweb/src/components/topup/SubscriptionPlansCard.jsxweb/src/components/topup/index.jsxweb/src/hooks/models/useModelsData.jsx
💤 Files with no reviewable changes (3)
- controller/image.go
- web/src/components/table/usage-logs/UsageLogsColumnDefs.jsx
- controller/swag_video.go
✅ Files skipped from review due to trivial changes (35)
- Dockerfile
- types/openai_video.go
- README.md
- docker-compose.yml
- README.ja.md
- dto/channel_testing.go
- types/user_settings.go
- dto/codex_usage.go
- service/openaicompat/chat_to_responses.go
- dto/topup_creem.go
- dto/ratio_sync_result.go
- dto/setup.go
- types/oauth_errors.go
- dto/option.go
- controller/billing.go
- dto/topup.go
- dto/pricing.go
- controller/relay.go
- dto/uptime_kuma.go
- dto/token.go
- dto/performance.go
- controller/option.go
- dto/channel.go
- dto/custom_oauth.go
- dto/checkin.go
- dto/twofa.go
- dto/model_sync.go
- dto/channel_billing.go
- dto/type_aliases.go
- dto/misc.go
- dto/subscription.go
- dto/query_params.go
- dto/router.go
- dto/api_responses.go
- controller/user.go
🚧 Files skipped from review as they are similar to previous changes (35)
- controller/channel_upstream_update.go
- dto/channel_affinity_cache.go
- README.zh_TW.md
- dto/model_meta.go
- relay/channel/claude/relay-claude.go
- web/src/components/table/model-deployments/modals/EditDeploymentModal.jsx
- types/channel_settings.go
- dto/openai_request.go
- controller/missing_models.go
- common/gin.go
- controller/secure_verification.go
- setting/chat.go
- AGENTS.md
- dto/params.go
- controller/ratio_config.go
- model/twofa.go
- dto/group.go
- model/task.go
- controller/subscription_payment_epay.go
- setting/ratio_setting/exposed_cache.go
- controller/pricing.go
- model/user_cache.go
- model/channel.go
- router/main.go
- controller/ratio_sync.go
- controller/task.go
- controller/log.go
- CLAUDE.md
- controller/checkin.go
- controller/midjourney.go
- go.mod
- controller/codex_oauth.go
- controller/twofa.go
- dto/user.go
- router/relay-router.go
0-don
force-pushed
the
feat/openapi-fuego
branch
9 times, most recently
from
59b6d6d to
925107b
Compare
This comment was marked as spam.
This comment was marked as spam.
0-don
force-pushed
the
feat/openapi-fuego
branch
5 times, most recently
from
52a55ef to
036373f
Compare
0-don
force-pushed
the
feat/openapi-fuego
branch
from
036373f to
c52b6cd
Compare
Replace static docs/openapi/*.json files with auto-generated OpenAPI spec using Fuego's fuegogin adapter. Gin remains the HTTP runtime; Fuego only generates the spec at startup. Key changes: - Add dto/router.go wrapper providing typed route registration helpers (Get, PostB, PutB, Delete, etc.) that register in both Gin and the OpenAPI spec simultaneously - Add dto/responses.go with Response[T], Ok/Fail helpers, PageData[T] for consistent typed API responses - Refactor all controllers from raw gin.Context to Fuego typed handlers with proper request/response types - Add 25+ DTO files (channel, token, user, subscription, deployment, performance, custom_oauth, etc.) - Move shared types to types/ package to break dto>model>relay import cycle (channel_settings, user_settings, openai_video, oauth_errors) - Add router/openapi.go for spec serving (/openapi.json) and Scalar UI (/swagger) - Model sync: empty request body (io.EOF) treated as valid empty request - Gated by ENABLE_OPENAPI=true env var (off by default) - Bump Go to 1.25+, add go-fuego/fuego dependency Stats: 201 routes use native Fuego typed handlers, 92 remain as raw gin.HandlerFunc (relay streaming, OAuth sessions, webhooks) Related: QuantumNous#907, QuantumNous#2348, QuantumNous#2353, QuantumNous#1128, QuantumNous#2502
0-don
force-pushed
the
feat/openapi-fuego
branch
from
c52b6cd to
2aa856a
Compare
Collaborator
|
Thank you for your contribution. However, this PR modifies too many files for us to review. We're sorry, but your PR will be closed. |
Contributor
Author
|
@Calcium-Ion @seefs001 I am running the same instance premerged with all the pull requests I have opened here and its working perfectly fine this will finally make new-api typesafe is there a way we can make this work should I make a smaller pull request first? we can finally update the docs and have an actual view of the correct schema every time a new update comes out |
7 tasks
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
2 participants
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.
📝 变更描述 / Description
Replaces static
docs/openapi/api.jsonanddocs/openapi/relay.jsonwith auto-generated OpenAPI spec via Fuego. Gin remains the HTTP runtime; Fuego only generates the spec at startup. Gated byENABLE_OPENAPI=trueenv var (off by default).201 routes use native Fuego typed handlers with proper request/response types; 92 remain as raw
gin.HandlerFunc(relay streaming, OAuth sessions, webhooks, redirects that need raw*gin.Context) but are still documented in the spec viaGinResp[T]()/GinBody[T]()annotations. The spec is served atGET /openapi.jsonand Scalar UI atGET /swagger.This works because Fuego wraps Gin's router, registering routes in both Gin and an OpenAPI spec builder simultaneously. Controllers are refactored from raw
gin.Contextto Fuego typed handlers with DTOs in a newdto/package. Shared types moved totypes/to break an import cycle (dto > model > relay/common > dto), re-exported viadto/type_aliases.gofor backwards compatibility.🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
📸 运行证明 / Proof of Work
Backend:
go build ./...passes.Frontend:
bun run buildpasses.Functional verification:
ENABLE_OPENAPI=true:/openapi.jsonreturns valid spec,/swaggershows Scalar UI with all 293 routes documentedENABLE_OPENAPI=false(default): routes still work, no spec servedPOST /api/models/sync-upstreamwith empty body succeeds (no parse error)