Conversation
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (16)
📜 Recent review details🧰 Additional context used📓 Path-based instructions (2)**/*.go📄 CodeRabbit inference engine (CLAUDE.md)
Files:
relay/channel/**/*.go📄 CodeRabbit inference engine (AGENTS.md)
Files:
🔇 Additional comments (19)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe change adds payment validation, transactional Midjourney billing, secure randomness, scoped updates, lifecycle shutdown controls, SSRF and trusted-proxy validation, secret redaction, replayable request bodies, and defensive service and frontend handling. ChangesPayment integrity and order completion
Transactional Midjourney billing
Scoped state updates
Runtime and input safeguards
Estimated code review effort: 5 (Critical) | ~100 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (1 warning, 2 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
controller/channel.go (1)
1078-1085: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
UpdateChannelwriteschannel_infowithout the channel polling lock.Lines 1079-1085 read
originChannel.ChannelInfo, mutate it, and later persist the wholechannel_infocolumn throughUpdateFields.ManageMultiKeysperforms the same read-modify-write but first takesmodel.GetChannelPollingLock(request.ChannelId)(line 1534) and reloads the channel under that lock.UpdateChanneltakes no lock.If an operator disables a key through
ManageMultiKeyswhile anUpdateChannelrequest is in flight, theUpdateChannelwrite restores the staleMultiKeyStatusListand silently re-enables the disabled key.Take
model.GetChannelPollingLock(patch.Id)before loadingoriginChannel, and release it afterUpdateFieldsreturns.🔒 Proposed fix to serialize the two writers
+ lock := model.GetChannelPollingLock(patch.Id) + lock.Lock() + defer lock.Unlock() + // Preserve existing ChannelInfo to ensure multi-key channels keep correct state even if the client does not send ChannelInfo in the request. originChannel, err := model.GetChannelById(patch.Id, true)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@controller/channel.go` around lines 1078 - 1085, Update UpdateChannel to acquire model.GetChannelPollingLock(patch.Id) before loading originChannel, hold it through the ChannelInfo read-modify-write and UpdateFields persistence, and release it only after UpdateFields returns. Preserve the existing ChannelInfo copy and MultiKeyMode update behavior while serializing it with ManageMultiKeys.model/channel.go (1)
581-618: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winThe multi-key recalculation result is discarded when
channel_infois not selected.Lines 582-618 recalculate
MultiKeySizeand pruneMultiKeyStatusList.editableUpdateMapwriteschannel_infoonly when the caller passes"channel_info"infields. If a caller passes"key"without"channel_info", the new key list is persisted and the storedMultiKeySizestays stale. Line 626 then reloads the oldChannelInfoover the recalculated value.Add
"channel_info"to the update map whenever the recalculation runs and"key"is selected.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@model/channel.go` around lines 581 - 618, The multi-key recalculation in the channel update flow is discarded when only “key” is selected because “channel_info” is absent from editableUpdateMap. In the recalculation block around channel.ChannelInfo.MultiKeySize, ensure the update map includes “channel_info” whenever recalculation runs for a selected key update, so the recalculated size and pruned MultiKeyStatusList are persisted before the later reload.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@controller/channel_update_test.go`:
- Around line 117-131: Strengthen TestUpdateChannelRejectsUnknownStatus by
asserting the response error message specifically identifies the invalid status
validation, not merely response.Success. Extend the channel update tests with
key_mode cases for “append” and “replace”, exercising the corresponding
UpdateChannel paths and asserting both the persisted key list and resulting
MultiKeySize.
- Around line 15-16: Update the test setup around openTokenControllerTestDB to
use the returned db handle consistently for subsequent read assertions, rather
than relying on model.DB; keep the existing migration setup unchanged and
replace any model.DB.First calls in this test with the local db handle.
In `@controller/channel.go`:
- Line 1710: Update the channel field-persistence flow around UpdateFields and
UpdateAbilities so ability rows are rebuilt only when the requested update set
contains an ability-relevant field (models, group, status, priority, weight, or
tag). Keep channel_info-only updates from invoking UpdateAbilities(nil), while
preserving the existing rebuild behavior for updates that affect those fields.
In `@model/channel.go`:
- Around line 619-630: Update model/channel.go lines 619-630 in UpdateFields to
execute the field update, channel reload, and conditional UpdateAbilities(tx)
within one DB.Transaction; invoke the rebuild only when updates include models,
group, status, priority, weight, or tag, and pass the transaction to
UpdateAbilities. No direct change is needed at controller/channel.go line 1710
or its sibling call sites, as the model-level gating handles channel_info-only
updates.
---
Outside diff comments:
In `@controller/channel.go`:
- Around line 1078-1085: Update UpdateChannel to acquire
model.GetChannelPollingLock(patch.Id) before loading originChannel, hold it
through the ChannelInfo read-modify-write and UpdateFields persistence, and
release it only after UpdateFields returns. Preserve the existing ChannelInfo
copy and MultiKeyMode update behavior while serializing it with ManageMultiKeys.
In `@model/channel.go`:
- Around line 581-618: The multi-key recalculation in the channel update flow is
discarded when only “key” is selected because “channel_info” is absent from
editableUpdateMap. In the recalculation block around
channel.ChannelInfo.MultiKeySize, ensure the update map includes “channel_info”
whenever recalculation runs for a selected key update, so the recalculated size
and pruned MultiKeyStatusList are persisted before the later reload.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 59c0d7c2-e7d1-4d09-bdd9-0df929930d5c
📒 Files selected for processing (10)
common/str.gocommon/utils.gocontroller/channel.gocontroller/channel_update_test.gocontroller/model_sync.gocontroller/oauth.gomodel/channel.gomodel/channel_cache.gomodel/checkin.gotools/jsonwrapcheck/allowlist.txt
💤 Files with no reviewable changes (1)
- tools/jsonwrapcheck/allowlist.txt
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Backend checks
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: In Go business code, all JSON marshal/unmarshal operations must use the wrapper functions incommon/json.go(common.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson,common.GetJsonType) and must not directly callencoding/jsonfor actual encode/decode work.
All database code must be compatible with SQLite, MySQL 5.7.8+, and PostgreSQL 9.6+; prefer GORM abstractions over raw SQL, avoid directAUTO_INCREMENT/SERIAL, usecommonGroupCol/commonKeyColandcommonTrueVal/commonFalseValfor DB-specific SQL, branch withcommon.UsingPostgreSQL/common.UsingSQLite/common.UsingMySQL, avoid unsupported DB-specific functions/operators without fallback, and make migrations work across all three databases.
When implementing a new relay channel, confirm whether the provider supportsStreamOptions; if it does, add that channel tostreamSupportedChannels.
For request structs parsed from client JSON and re-marshaled to upstream providers, optional scalar fields must use pointer types withomitemptyso explicit zero/false values are preserved instead of dropped.
When working on tiered/dynamic billing expression code, readpkg/billingexpr/expr.mdfirst and follow its documented expression language, architecture, token normalization, quota conversion, and versioning patterns.
**/*.go: All JSON marshal/unmarshal operations in Go business code must use the wrapper functions incommon/json.go(common.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson,common.GetJsonType) instead of directly importing or callingencoding/jsonfor actual marshal/unmarshal work.
All database code in Go must remain compatible with SQLite, MySQL >= 5.7.8, and PostgreSQL >= 9.6; prefer GORM abstractions, avoid raw SQL unless necessary, use the shared DB helper variables for reserved words and boolean literals, branch with thecommon.UsingPostgreSQL/common.UsingSQLite/common.UsingMySQLflags when need...
Files:
model/channel_cache.gocontroller/oauth.gocontroller/channel_update_test.gocommon/str.gomodel/checkin.gocontroller/model_sync.gocommon/utils.gomodel/channel.gocontroller/channel.go
🔇 Additional comments (18)
common/utils.go (1)
10-11: LGTM!Also applies to: 255-270, 296-300
common/str.go (1)
10-11: LGTM!Also applies to: 42-46
model/channel_cache.go (1)
5-6: LGTM!Also applies to: 225-228
model/checkin.go (1)
4-5: LGTM!Also applies to: 72-76
controller/model_sync.go (1)
8-9: LGTM!Also applies to: 28-35, 166-166, 238-238
controller/oauth.go (1)
82-92: LGTM!model/channel.go (4)
260-264: LGTM!
341-341: LGTM!
704-705: 🗄️ Data Integrity & IntegrationNo change needed for
channel_infoserialization.
ChannelInfoimplementsdriver.Valuerandsql.Scanner, so placing it in the update map uses the same JSON-driver path as the model field.
548-575: 🗄️ Data Integrity & IntegrationNo change needed.
Update()maps only non-pointer fields unconditionally, but there are no directChannel.Update()callers outsidemodel/channel.go, so the current path does not introduce a new partial-update regression.controller/channel.go (6)
922-947: LGTM!
949-1049: LGTM!
1051-1069: LGTM!
1096-1098: LGTM!Also applies to: 1123-1139, 1171-1178
1210-1210: LGTM!
1753-1753: LGTM!Also applies to: 1778-1778, 1826-1826, 1907-1907, 1976-1976
controller/channel_update_test.go (2)
49-93: LGTM!
95-115: LGTM!
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@controller/custom_oauth_test.go`:
- Around line 36-37: Update the test setup around fetchSetting.AllowedPorts to
parse the port from the httptest.NewServer URL and append it to the allowed
ports before making the request. Preserve the existing fixed ports and ensure
the ephemeral test-server port is permitted so validation reaches the private-IP
check.
In `@controller/topup_waffo_pancake_test.go`:
- Around line 194-200: Update the response assertion in the
RequestWaffoPancakePay test to require the specific checkout-session failure
message rather than the generic `"message":"error"` text. Keep the existing
status and database assertions unchanged.
In `@controller/topup_waffo_pancake.go`:
- Around line 501-502: Update the PaymentValidationFromMajorString call in the
Pancake subscription callback to pass an empty expected-currency value instead
of "USD", allowing validatePaymentAgainstSubscriptionOrder to use the
subscription plan’s currency. Keep the remaining amount and currency arguments
unchanged.
In `@controller/topup.go`:
- Around line 371-375: Update the ignore-event log in the webhook handler around
the TradeStatus check to remove the repeated verifyInfo payload. Retain the
trade number and trade status fields, while omitting the
common.GetJsonString(verifyInfo) argument and any other unnecessary duplicated
request data.
- Around line 355-358: Remove the immediately invoked writeErr closure and
directly assign the result of c.Writer.Write to writeErr, preserving the
existing "fail" response bytes and error handling.
In `@controller/user_setting_test.go`:
- Around line 182-227: Add tests alongside
TestUpdateUserSettingPreservesStoredSecretsWhenRedactedFieldsAreOmitted covering
Gotify token retention: preserve an existing token when the request omits it
while the stored notification type is Gotify, and verify the token is not
retained when the stored notification type is non-Gotify. Reuse the existing
setup, request, and persistence assertions while targeting the Gotify-specific
branches.
In `@model/log_cleanup_test.go`:
- Around line 44-61: Extend TestDeleteOldLogBatchPreservesManagementAuditLogs to
call CountOldLog with the same context and cutoff used by DeleteOldLogBatch,
asserting it returns 1 before deletion and 0 afterward; retain the existing
deletion and management-log assertions.
In `@model/payment_method_guard_test.go`:
- Around line 415-417: The currency-mismatch tests around
CompleteSubscriptionOrder should also cover plan-currency fallback. Add a case
using PaymentValidationFromMinorUnits with an empty expected-currency argument,
while retaining the same subscription and plan setup, and assert
ErrPaymentCurrencyMismatch so validatePaymentAgainstSubscriptionOrder exercises
its plan-derived currency branch.
- Around line 385-406: Update
TestRechargeEpayRejectsPaidAmountMismatchBeforeCompletingOrder to pass a payment
method different from the stored TopUp.PaymentMethod, then assert the persisted
payment method remains unchanged after ErrPaymentAmountMismatch. Keep the
existing pending-status and quota assertions.
In `@model/topup.go`:
- Around line 302-304: Update the RecordTopupLog message in the quotaToAdd
branch to use the same Chinese wording as Recharge, and format topUp.Money with
%.2f to match the other providers.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 75395e34-bd54-456b-87cc-cc9a95ab24cd
📒 Files selected for processing (21)
controller/custom_oauth.gocontroller/custom_oauth_test.gocontroller/subscription_payment_epay.gocontroller/subscription_payment_waffo_pancake.gocontroller/topup.gocontroller/topup_creem.gocontroller/topup_stripe.gocontroller/topup_waffo.gocontroller/topup_waffo_pancake.gocontroller/topup_waffo_pancake_test.gocontroller/user.gocontroller/user_setting_test.gomodel/log.gomodel/log_cleanup_test.gomodel/payment_method_guard_test.gomodel/payment_validation.gomodel/subscription.gomodel/topup.goservice/task_polling.goservice/task_polling_test.gotools/jsonwrapcheck/allowlist.txt
💤 Files with no reviewable changes (2)
- tools/jsonwrapcheck/allowlist.txt
- controller/subscription_payment_waffo_pancake.go
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Backend checks
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: In Go business code, all JSON marshal/unmarshal operations must use the wrapper functions incommon/json.go(common.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson,common.GetJsonType) and must not directly callencoding/jsonfor actual encode/decode work.
All database code must be compatible with SQLite, MySQL 5.7.8+, and PostgreSQL 9.6+; prefer GORM abstractions over raw SQL, avoid directAUTO_INCREMENT/SERIAL, usecommonGroupCol/commonKeyColandcommonTrueVal/commonFalseValfor DB-specific SQL, branch withcommon.UsingPostgreSQL/common.UsingSQLite/common.UsingMySQL, avoid unsupported DB-specific functions/operators without fallback, and make migrations work across all three databases.
When implementing a new relay channel, confirm whether the provider supportsStreamOptions; if it does, add that channel tostreamSupportedChannels.
For request structs parsed from client JSON and re-marshaled to upstream providers, optional scalar fields must use pointer types withomitemptyso explicit zero/false values are preserved instead of dropped.
When working on tiered/dynamic billing expression code, readpkg/billingexpr/expr.mdfirst and follow its documented expression language, architecture, token normalization, quota conversion, and versioning patterns.
**/*.go: All JSON marshal/unmarshal operations in Go business code must use the wrapper functions incommon/json.go(common.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson,common.GetJsonType) instead of directly importing or callingencoding/jsonfor actual marshal/unmarshal work.
All database code in Go must remain compatible with SQLite, MySQL >= 5.7.8, and PostgreSQL >= 9.6; prefer GORM abstractions, avoid raw SQL unless necessary, use the shared DB helper variables for reserved words and boolean literals, branch with thecommon.UsingPostgreSQL/common.UsingSQLite/common.UsingMySQLflags when need...
Files:
model/log_cleanup_test.goservice/task_polling_test.gomodel/log.gocontroller/custom_oauth_test.gocontroller/custom_oauth.gomodel/subscription.gocontroller/user_setting_test.gocontroller/topup_stripe.gocontroller/topup_waffo.goservice/task_polling.gocontroller/topup.gocontroller/subscription_payment_epay.gomodel/payment_validation.gocontroller/user.gomodel/payment_method_guard_test.gocontroller/topup_waffo_pancake.gocontroller/topup_waffo_pancake_test.gocontroller/topup_creem.gomodel/topup.go
🔇 Additional comments (34)
controller/custom_oauth.go (3)
15-15: LGTM!
170-184: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftSSRF (CWE-918): Server-Side Request Forgery (SSRF)
Reachability: External
Reachability path
● Entry controller/custom_oauth_test.go:15 TestFetchCustomOAuthDiscoveryRejectsPrivateURLWhenSSRFProtectionEnabled │ ▼ ● Sink controller/custom_oauth.goPrevent DNS rebinding after URL validation.
Line 171 makes the destination decision before Line 197 dials the host. The default transport resolves the hostname during the later connection. An attacker-controlled domain can resolve to an allowed address during validation and to a private address during the outbound dial.
Enforce the resolved-address policy in a custom
DialContext, or pin the validated address for the request.
170-184: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winSSRF (CWE-918): Server-Side Request Forgery (SSRF)
Reachability: External
Reachability path
● Entry controller/custom_oauth_test.go:15 TestFetchCustomOAuthDiscoveryRejectsPrivateURLWhenSSRFProtectionEnabled │ ▼ ● Sink controller/custom_oauth.goValidate each redirect target.
Line 171 validates only the initial
targetURL. Line 197 uses the defaulthttp.Clientredirect behavior. If an attacker supplies an allowed public URL that redirects to a private address, the redirected request bypassesValidateURLWithFetchSetting.Validate
req.URLinCheckRedirect, or disable automatic redirects and validate each resolvedLocationbefore sending the next request.service/task_polling.go (1)
175-175: LGTM!service/task_polling_test.go (1)
13-31: LGTM!controller/user.go (1)
471-485: LGTM!Also applies to: 526-526, 1445-1455, 1501-1502, 1519-1523
controller/user_setting_test.go (1)
132-180: LGTM!model/log.go (2)
1023-1023: 🗄️ Data Integrity & IntegrationCheck logs.type null handling before changing the cleanup predicates.
The count and delete selection both use
type <> ?, which excludes SQLNULLrows.Log.Typehas nonot nullGORM tag in the model, and the available migration SQL does not show the logs table schema. If oldNULLlog rows exist, cleanup can miss bothCountOldLogrows andDeleteOldLogBatchrows. Check production schema/data; addOR type IS NULLif appropriate, or backfill and enforceNOT NULL.
1040-1040: 🔒 Security & PrivacyOther (CWE-367): Time-of-check Time-of-use (TOCTOU) Race Condition
Reachability path
● Entry model/log_cleanup_test.go:44 TestDeleteOldLogBatchPreservesManagementAuditLogs │ ▼ ● Sink model/log.goRecheck the retention predicate during deletion.
DeleteOldLogBatchselects IDs with the predicate at Line 1040, but Line 1060 deletes by ID only. If another transaction changes a selected row toLogTypeManagebefore the delete, the cleanup still removes that management log.Reapply the timestamp and type predicates to the
DELETE, or verify and enforce thatLog.Typecannot change after insertion.#!/usr/bin/env bash set -euo pipefail rg -n -C 6 \ 'LogTypeManage|Type\s*[:=]|UpdateColumn[s]?\(|Updates\(|Save\(' \ --glob '*.go' .model/payment_validation.go (4)
33-45: LGTM!
77-87: LGTM!
89-106: LGTM!
65-70: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winOther (CWE-840)
Reachability: External
Reachability path
● Entry controller/subscription_payment_epay.go:118 SubscriptionEpayNotify: POST 请求：从 POST body 解析参数 │ ▼ ● Hop model/subscription.go:612 CompleteSubscriptionOrder: still allow completion for already purchased orders │ ▼ ● Sink model/payment_validation.goConfirm the intended lower bound for discounted payments.
When
AllowDiscountis true, the check only enforces an upper bound. Any paid amount from 1 minor unit up toexpectedMinorpasses, and the order is credited with the full quota.controller/topup_stripe.gosetsAllowDiscountfromsetting.StripePromotionCodesEnabledfor the top-up path, so a promotion code of any size grants the full quota. Confirm that no minimum paid fraction is required.#!/bin/bash # Description: Inspect how AllowDiscount is set and whether a discount floor exists anywhere. set -euo pipefail rg -n -C4 'AllowDiscount|StripePromotionCodesEnabled' --type=gomodel/subscription.go (2)
545-549: LGTM!
612-612: LGTM!Also applies to: 649-651
model/topup.go (5)
50-55: LGTM!
162-162: LGTM!Also applies to: 189-192, 211-213
230-301: LGTM!
529-529: LGTM!Also applies to: 557-560, 589-591
625-625: LGTM!Also applies to: 656-659, 681-683, 694-694, 725-728, 748-750
controller/subscription_payment_epay.go (1)
163-164: LGTM!Also applies to: 213-214
controller/topup.go (1)
377-386: LGTM!controller/topup_creem.go (3)
62-77: LGTM!Also applies to: 95-95, 427-427, 468-468
321-322: LGTM!
367-371: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftSecurity Misconfiguration (CWE-807)
Reachability: External
Reachability path
● Entry controller/topup_stripe.go:64 RequestPay │ ▼ ● Sink controller/topup_creem.goDo not derive the expected currency from the webhook payload.
When
expectedCreemCurrencyForProductreturns an empty value, Line 370 setsExpectedCurrencyfromevent.Object.Product.Currency.Currencyon Line 321 already comes fromevent.Object.Order.Currency. Both values then come from the same payload, so the currency check compares the payload against itself and always passes. The local expectation is lost exactly when the product is missing fromsetting.CreemProducts.Leave
ExpectedCurrencyempty when the configured product is unknown, or reject the callback. An empty expected currency skips the check, which is no weaker than the self-comparison and does not create a false guarantee.🛡️ Proposed fix
topUpValidation := paymentValidation topUpValidation.ExpectedCurrency = expectedCreemCurrencyForProduct(event.Object.Product.Id) - if topUpValidation.ExpectedCurrency == "" { - topUpValidation.ExpectedCurrency = event.Object.Product.Currency - } + if topUpValidation.ExpectedCurrency == "" { + logger.LogWarn(c.Request.Context(), fmt.Sprintf("Creem 未找到产品货币配置，跳过货币校验 trade_no=%s product_id=%s", referenceId, event.Object.Product.Id)) + }#!/bin/bash # Description: Check whether CreemProducts is expected to list every product id, and find other expected-currency sources. set -euo pipefail rg -n -C4 'CreemProducts' --type=go rg -n -C3 'ExpectedCurrency' --type=gocontroller/topup_stripe.go (3)
226-236: LGTM!
238-248: LGTM!Also applies to: 298-305
312-314: 🎯 Functional CorrectnessNo change needed. Stripe top-ups use
CustomCurrencyExchangeRatefor local display while Stripe prices stay USD; subscription checkout does not enable Stripe promotion codes, so these lines do not break the described flows.controller/topup_waffo.go (2)
63-71: LGTM!Also applies to: 9-9
404-405: LGTM!controller/topup_waffo_pancake.go (1)
528-529: LGTM!controller/topup_waffo_pancake_test.go (1)
4-67: LGTM!model/payment_method_guard_test.go (2)
336-357: LGTM!
359-383: LGTM!
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@controller/user_setting_test.go`:
- Around line 251-275: Complete the persistence assertions in
controller/user_setting_test.go:251-275 by asserting
setting.QuotaWarningThreshold equals 0.8 after the successful update. In
controller/user_setting_test.go:318-323, replace the partial rejected-update
checks with a comparison of the full setting against initialSettings, ensuring
every field remains unchanged.
In `@model/channel.go`:
- Around line 585-586: Preserve explicit key selection in the field-update logic
around `keySelected` and its corresponding handling near the multi-key state
updates: determine selection solely from the presence of `"key"` in `fieldSet`,
without requiring `channel.Key` to be non-empty. Ensure clearing the final key
recalculates `MultiKeySize` and `MultiKeyStatusList` from the empty key list,
and add a regression test covering `UpdateFields("key")` with an empty key and
asserting size zero.
In `@model/log.go`:
- Line 1062: Update DeleteOldLog so a short delete batch caused by the
oldLogCleanupScope re-check does not immediately terminate cleanup: re-count
eligible logs with CountOldLog and continue processing whenever the count is
nonzero, returning only when none remain. Add a regression test covering a
promoted first candidate followed by a later eligible consume log.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3ee44f34-4e11-4ee5-a716-0a5d2fc88dd8
📒 Files selected for processing (16)
controller/channel.gocontroller/channel_update_test.gocontroller/custom_oauth.gocontroller/custom_oauth_test.gocontroller/topup.gocontroller/topup_creem.gocontroller/topup_creem_test.gocontroller/topup_waffo_pancake.gocontroller/topup_waffo_pancake_test.gocontroller/user_setting_test.gomodel/channel.gomodel/channel_update_fields_test.gomodel/log.gomodel/log_cleanup_test.gomodel/payment_method_guard_test.gomodel/topup.go
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Backend checks
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: In Go business code, all JSON marshal/unmarshal operations must use the wrapper functions incommon/json.go(common.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson,common.GetJsonType) and must not directly callencoding/jsonfor actual encode/decode work.
All database code must be compatible with SQLite, MySQL 5.7.8+, and PostgreSQL 9.6+; prefer GORM abstractions over raw SQL, avoid directAUTO_INCREMENT/SERIAL, usecommonGroupCol/commonKeyColandcommonTrueVal/commonFalseValfor DB-specific SQL, branch withcommon.UsingPostgreSQL/common.UsingSQLite/common.UsingMySQL, avoid unsupported DB-specific functions/operators without fallback, and make migrations work across all three databases.
When implementing a new relay channel, confirm whether the provider supportsStreamOptions; if it does, add that channel tostreamSupportedChannels.
For request structs parsed from client JSON and re-marshaled to upstream providers, optional scalar fields must use pointer types withomitemptyso explicit zero/false values are preserved instead of dropped.
When working on tiered/dynamic billing expression code, readpkg/billingexpr/expr.mdfirst and follow its documented expression language, architecture, token normalization, quota conversion, and versioning patterns.
**/*.go: All JSON marshal/unmarshal operations in Go business code must use the wrapper functions incommon/json.go(common.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson,common.GetJsonType) instead of directly importing or callingencoding/jsonfor actual marshal/unmarshal work.
All database code in Go must remain compatible with SQLite, MySQL >= 5.7.8, and PostgreSQL >= 9.6; prefer GORM abstractions, avoid raw SQL unless necessary, use the shared DB helper variables for reserved words and boolean literals, branch with thecommon.UsingPostgreSQL/common.UsingSQLite/common.UsingMySQLflags when need...
Files:
controller/topup_waffo_pancake.gomodel/channel_update_fields_test.gocontroller/topup_creem_test.gocontroller/topup_waffo_pancake_test.gocontroller/topup.gomodel/log.gomodel/channel.gocontroller/user_setting_test.gomodel/log_cleanup_test.gomodel/payment_method_guard_test.gocontroller/topup_creem.gocontroller/channel_update_test.gocontroller/custom_oauth.gomodel/topup.gocontroller/custom_oauth_test.gocontroller/channel.go
🔇 Additional comments (11)
model/topup.go (1)
303-303: LGTM!controller/topup_waffo_pancake.go (1)
502-502: LGTM!controller/topup.go (1)
355-355: LGTM!Also applies to: 369-369
controller/topup_creem.go (1)
79-93: LGTM!Also applies to: 383-389
model/payment_method_guard_test.go (1)
401-407: LGTM!Also applies to: 421-431
controller/topup_waffo_pancake_test.go (1)
195-195: LGTM!controller/topup_creem_test.go (1)
10-25: LGTM!Also applies to: 27-41
model/log.go (1)
1023-1045: LGTM!model/log_cleanup_test.go (1)
54-108: LGTM!controller/custom_oauth.go (1)
15-15: LGTM!Also applies to: 170-174, 185-185
controller/custom_oauth_test.go (1)
37-39: LGTM!Also applies to: 53-53
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
model/channel_update_fields_test.go (1)
14-46: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winRegister
t.Cleanupbefore fallible setup.The helper changes global state before Line 38 registers cleanup. If
gorm.Open,AutoMigrate, ordb.DB()fails, later tests inherit the modifiedDB,LOG_DB, and common flags. The SQLite handle can also remain open.Register cleanup immediately after saving the original state. Hoist
sqlDBand close it only when it is non-nil.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@model/channel_update_fields_test.go` around lines 14 - 46, Move the t.Cleanup registration in setupChannelUpdateFieldsTestDB immediately after capturing the original DB and common-state values, before changing globals or performing SQLite setup. Hoist sqlDB outside the cleanup closure and close it only when non-nil, while restoring all saved state regardless of whether gorm.Open, AutoMigrate, or db.DB fails.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@model/channel.go`:
- Around line 627-640: The channel cleanup currently mutates the caller’s
ChannelInfo before transaction success, leaving the in-memory receiver
inconsistent on rollback. Deep-copy the channel for all cleanup and tentative
reload/update work in the transaction, use that copy as the transaction
receiver, and assign the completed copy to *channel only after DB.Transaction
succeeds; extend the rollback test to assert the original in-memory channel
remains unchanged.
---
Outside diff comments:
In `@model/channel_update_fields_test.go`:
- Around line 14-46: Move the t.Cleanup registration in
setupChannelUpdateFieldsTestDB immediately after capturing the original DB and
common-state values, before changing globals or performing SQLite setup. Hoist
sqlDB outside the cleanup closure and close it only when non-nil, while
restoring all saved state regardless of whether gorm.Open, AutoMigrate, or db.DB
fails.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: fa979d67-049d-43ab-aa54-27d9c8df9f29
📒 Files selected for processing (5)
controller/user_setting_test.gomodel/channel.gomodel/channel_update_fields_test.gomodel/log.gomodel/log_cleanup_test.go
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: In Go business code, all JSON marshal/unmarshal operations must use the wrapper functions incommon/json.go(common.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson,common.GetJsonType) and must not directly callencoding/jsonfor actual encode/decode work.
All database code must be compatible with SQLite, MySQL 5.7.8+, and PostgreSQL 9.6+; prefer GORM abstractions over raw SQL, avoid directAUTO_INCREMENT/SERIAL, usecommonGroupCol/commonKeyColandcommonTrueVal/commonFalseValfor DB-specific SQL, branch withcommon.UsingPostgreSQL/common.UsingSQLite/common.UsingMySQL, avoid unsupported DB-specific functions/operators without fallback, and make migrations work across all three databases.
When implementing a new relay channel, confirm whether the provider supportsStreamOptions; if it does, add that channel tostreamSupportedChannels.
For request structs parsed from client JSON and re-marshaled to upstream providers, optional scalar fields must use pointer types withomitemptyso explicit zero/false values are preserved instead of dropped.
When working on tiered/dynamic billing expression code, readpkg/billingexpr/expr.mdfirst and follow its documented expression language, architecture, token normalization, quota conversion, and versioning patterns.
**/*.go: All JSON marshal/unmarshal operations in Go business code must use the wrapper functions incommon/json.go(common.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson,common.GetJsonType) instead of directly importing or callingencoding/jsonfor actual marshal/unmarshal work.
All database code in Go must remain compatible with SQLite, MySQL >= 5.7.8, and PostgreSQL >= 9.6; prefer GORM abstractions, avoid raw SQL unless necessary, use the shared DB helper variables for reserved words and boolean literals, branch with thecommon.UsingPostgreSQL/common.UsingSQLite/common.UsingMySQLflags when need...
Files:
model/log_cleanup_test.gomodel/channel_update_fields_test.gomodel/log.gocontroller/user_setting_test.gomodel/channel.go
🔇 Additional comments (4)
controller/user_setting_test.go (1)
276-276: LGTM!Also applies to: 321-321
model/log.go (1)
1029-1030: LGTM!Also applies to: 1042-1045, 1062-1062, 1093-1099
model/log_cleanup_test.go (1)
110-140: LGTM!model/channel.go (1)
627-640: 🗄️ Data Integrity & IntegrationNo change needed for
MultiKeyPollingIndex.
NextMultiKeynormalizes the saved polling index before lookup withstart < 0 || start >= len(keys), andMultiKeyPollingIndexis only updated in the polling code path when a key is selected.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
model/channel.go (1)
622-650: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPreserve an empty JSON key array.
When
Keyis[],common.Unmarshalsucceeds with zero keys. Thelen(keys) == 0fallback then splits[]as one newline key.GetKeystreats the same value as zero keys. This persistsMultiKeySizeas1and can retain metadata for index0when no key exists.Track JSON parsing success separately. Use newline splitting only when JSON parsing fails. Add a regression test for
UpdateFields("key")withKey == "[]".Proposed fix
keys := []string{} + parsedJSONKeys := false if keyStr != "" { trimmed := strings.TrimSpace(keyStr) if strings.HasPrefix(trimmed, "[") { var arr []json.RawMessage if err := common.Unmarshal([]byte(trimmed), &arr); err == nil { + parsedJSONKeys = true keys = make([]string, len(arr)) for i, v := range arr { keys[i] = string(v) } } } - if len(keys) == 0 { // fallback to newline split + if !parsedJSONKeys { keys = strings.Split(strings.Trim(keyStr, "\n"), "\n") } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@model/channel.go` around lines 622 - 650, Update the key parsing block in the channel update flow to track whether JSON unmarshalling succeeded separately from the parsed key count, so a valid empty array preserves zero keys and does not fall back to newline splitting. Keep newline splitting only for non-JSON or failed JSON input, and add a regression test covering UpdateFields("key") with Key set to "[]" and verifying MultiKeySize remains zero.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@model/channel.go`:
- Around line 694-705: In the channel update flow around cloneChannelForUpdate
and the final *channel assignment, clear workingChannel.Keys when keySelected is
true after the update succeeds and before copying it to the caller, so GetKeys
reloads the updated Key instead of returning the stale cache. Add coverage that
prepopulates Keys, updates Key, and verifies GetKeys returns the new key list.
---
Outside diff comments:
In `@model/channel.go`:
- Around line 622-650: Update the key parsing block in the channel update flow
to track whether JSON unmarshalling succeeded separately from the parsed key
count, so a valid empty array preserves zero keys and does not fall back to
newline splitting. Keep newline splitting only for non-JSON or failed JSON
input, and add a regression test covering UpdateFields("key") with Key set to
"[]" and verifying MultiKeySize remains zero.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 7b56a354-5dda-4c44-a865-6be8876cc738
📒 Files selected for processing (2)
model/channel.gomodel/channel_update_fields_test.go
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: In Go business code, all JSON marshal/unmarshal operations must use the wrapper functions incommon/json.go(common.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson,common.GetJsonType) and must not directly callencoding/jsonfor actual encode/decode work.
All database code must be compatible with SQLite, MySQL 5.7.8+, and PostgreSQL 9.6+; prefer GORM abstractions over raw SQL, avoid directAUTO_INCREMENT/SERIAL, usecommonGroupCol/commonKeyColandcommonTrueVal/commonFalseValfor DB-specific SQL, branch withcommon.UsingPostgreSQL/common.UsingSQLite/common.UsingMySQL, avoid unsupported DB-specific functions/operators without fallback, and make migrations work across all three databases.
When implementing a new relay channel, confirm whether the provider supportsStreamOptions; if it does, add that channel tostreamSupportedChannels.
For request structs parsed from client JSON and re-marshaled to upstream providers, optional scalar fields must use pointer types withomitemptyso explicit zero/false values are preserved instead of dropped.
When working on tiered/dynamic billing expression code, readpkg/billingexpr/expr.mdfirst and follow its documented expression language, architecture, token normalization, quota conversion, and versioning patterns.
**/*.go: All JSON marshal/unmarshal operations in Go business code must use the wrapper functions incommon/json.go(common.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson,common.GetJsonType) instead of directly importing or callingencoding/jsonfor actual marshal/unmarshal work.
All database code in Go must remain compatible with SQLite, MySQL >= 5.7.8, and PostgreSQL >= 9.6; prefer GORM abstractions, avoid raw SQL unless necessary, use the shared DB helper variables for reserved words and boolean literals, branch with thecommon.UsingPostgreSQL/common.UsingSQLite/common.UsingMySQLflags when need...
Files:
model/channel_update_fields_test.gomodel/channel.go
🔇 Additional comments (2)
model/channel_update_fields_test.go (1)
95-163: LGTM!model/channel.go (1)
8-8: 📐 Maintainability & Code QualityNo compatibility issue with
maps.The module declares Go 1.25.1, and the referenced CI/docker toolchains also use Go 1.25/1.26+, so
maps.Cloneis supported at all listed sites.
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
model/channel.go (1)
622-676: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftBuild multi-key metadata in the update transaction.
At Line 629,
GetChannelByIdruns beforeDB.Transaction, and its error is ignored. If that read fails, this code treats the key list as empty and persists zeroed metadata.A concurrent key update can also commit after this read and before Line 691. This update can then overwrite the new key metadata with metadata calculated from an old key list. Load the persisted key with
tx, return lookup errors, and constructupdates["channel_info"]inside the same transaction.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@model/channel.go` around lines 622 - 676, Move multi-key key loading and metadata construction from the pre-transaction path into the DB transaction, using the transaction handle for the persisted lookup and propagating lookup errors instead of treating failures as an empty key list. Build and assign updates["channel_info"] inside that transaction after calculating MultiKeySize and cleaning status maps, so key metadata is derived from the same transactional state and cannot overwrite concurrent updates with stale data.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@model/channel_update_fields_test.go`:
- Around line 99-121: Update the failed UpdateFields call in the channel
rollback test to include "priority" alongside the existing fields. After
reloading the stored Channel, assert that its priority remains equal to the
original value, covering rollback of the pending channel.Priority change.
---
Outside diff comments:
In `@model/channel.go`:
- Around line 622-676: Move multi-key key loading and metadata construction from
the pre-transaction path into the DB transaction, using the transaction handle
for the persisted lookup and propagating lookup errors instead of treating
failures as an empty key list. Build and assign updates["channel_info"] inside
that transaction after calculating MultiKeySize and cleaning status maps, so key
metadata is derived from the same transactional state and cannot overwrite
concurrent updates with stale data.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a774acf8-201d-4ad3-9db7-9fb7b8feba45
📒 Files selected for processing (2)
model/channel.gomodel/channel_update_fields_test.go
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: In Go business code, all JSON marshal/unmarshal operations must use the wrapper functions incommon/json.go(common.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson,common.GetJsonType) and must not directly callencoding/jsonfor actual encode/decode work.
All database code must be compatible with SQLite, MySQL 5.7.8+, and PostgreSQL 9.6+; prefer GORM abstractions over raw SQL, avoid directAUTO_INCREMENT/SERIAL, usecommonGroupCol/commonKeyColandcommonTrueVal/commonFalseValfor DB-specific SQL, branch withcommon.UsingPostgreSQL/common.UsingSQLite/common.UsingMySQL, avoid unsupported DB-specific functions/operators without fallback, and make migrations work across all three databases.
When implementing a new relay channel, confirm whether the provider supportsStreamOptions; if it does, add that channel tostreamSupportedChannels.
For request structs parsed from client JSON and re-marshaled to upstream providers, optional scalar fields must use pointer types withomitemptyso explicit zero/false values are preserved instead of dropped.
When working on tiered/dynamic billing expression code, readpkg/billingexpr/expr.mdfirst and follow its documented expression language, architecture, token normalization, quota conversion, and versioning patterns.
**/*.go: All JSON marshal/unmarshal operations in Go business code must use the wrapper functions incommon/json.go(common.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson,common.GetJsonType) instead of directly importing or callingencoding/jsonfor actual marshal/unmarshal work.
All database code in Go must remain compatible with SQLite, MySQL >= 5.7.8, and PostgreSQL >= 9.6; prefer GORM abstractions, avoid raw SQL unless necessary, use the shared DB helper variables for reserved words and boolean literals, branch with thecommon.UsingPostgreSQL/common.UsingSQLite/common.UsingMySQLflags when need...
Files:
model/channel_update_fields_test.gomodel/channel.go
🔇 Additional comments (4)
model/channel.go (3)
8-8: LGTM!
578-606: LGTM!
690-709: LGTM!model/channel_update_fields_test.go (1)
124-147: LGTM!Also applies to: 154-157
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@common/rate-limit.go`:
- Around line 19-30: Protect the limiter lifecycle with a mutex-protected
stopped state: update Init to check that state before creating channels or
launching clearExpiredItems, and update Stop to mark the limiter stopped while
holding the mutex before releasing it. Preserve one-time initialization while
ensuring concurrent or later Init calls cannot start cleanup after Stop returns,
and add a concurrent Init/Stop test covering this race.
In `@relay/channel/zhipu/relay-zhipu.go`:
- Line 6: Update the panic-recovery logic in the relay stream handler around the
deferred shutdown sender and recovery block so a recovered panic is propagated
to the consumer as a relay error, rather than signaling normal completion
through stopChan. Suppress data: [DONE] for the recovered-failure path, and
return the relay error when response headers are still writable.
In `@service/channel_affinity_template_test.go`:
- Around line 343-351: Update the cache sequence in the test around
loadChannelAffinityRegex so ^first$ is accessed after the initial inserts, then
insert ^third$ and assert ^second$ is absent. Preserve the existing no-error
checks while making the eviction assertion distinguish LRU behavior from FIFO.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: aaf585ca-e8bf-4e1d-b34c-505062211e3b
📒 Files selected for processing (24)
common/email.gocommon/email_test.gocommon/go-channel.gocommon/go_channel_test.gocommon/rate-limit.gocommon/rate_limit_test.gocommon/system_monitor.gocommon/system_monitor_test.gomain.gomiddleware/audit.gomiddleware/email-verification-rate-limit.gomiddleware/model-rate-limit.gomiddleware/rate-limit.gomiddleware/turnstile-check.gomiddleware/turnstile_check_test.gopkg/billingexpr/billingexpr_test.gopkg/billingexpr/run.gorelay/channel/gemini/relay-gemini.gorelay/channel/gemini/relay_gemini_usage_test.gorelay/channel/zhipu/relay-zhipu.goservice/channel_affinity.goservice/channel_affinity_template_test.goservice/tiered_settle.goservice/tiered_settle_test.go
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: In Go business code, all JSON marshal/unmarshal operations must use the wrapper functions incommon/json.go(common.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson,common.GetJsonType) and must not directly callencoding/jsonfor actual encode/decode work.
All database code must be compatible with SQLite, MySQL 5.7.8+, and PostgreSQL 9.6+; prefer GORM abstractions over raw SQL, avoid directAUTO_INCREMENT/SERIAL, usecommonGroupCol/commonKeyColandcommonTrueVal/commonFalseValfor DB-specific SQL, branch withcommon.UsingPostgreSQL/common.UsingSQLite/common.UsingMySQL, avoid unsupported DB-specific functions/operators without fallback, and make migrations work across all three databases.
When implementing a new relay channel, confirm whether the provider supportsStreamOptions; if it does, add that channel tostreamSupportedChannels.
For request structs parsed from client JSON and re-marshaled to upstream providers, optional scalar fields must use pointer types withomitemptyso explicit zero/false values are preserved instead of dropped.
When working on tiered/dynamic billing expression code, readpkg/billingexpr/expr.mdfirst and follow its documented expression language, architecture, token normalization, quota conversion, and versioning patterns.
**/*.go: All JSON marshal/unmarshal operations in Go business code must use the wrapper functions incommon/json.go(common.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson,common.GetJsonType) instead of directly importing or callingencoding/jsonfor actual marshal/unmarshal work.
All database code in Go must remain compatible with SQLite, MySQL >= 5.7.8, and PostgreSQL >= 9.6; prefer GORM abstractions, avoid raw SQL unless necessary, use the shared DB helper variables for reserved words and boolean literals, branch with thecommon.UsingPostgreSQL/common.UsingSQLite/common.UsingMySQLflags when need...
Files:
pkg/billingexpr/run.gorelay/channel/gemini/relay_gemini_usage_test.gocommon/email_test.gomain.gocommon/go_channel_test.gorelay/channel/gemini/relay-gemini.gocommon/system_monitor_test.goservice/channel_affinity.goservice/tiered_settle_test.gomiddleware/model-rate-limit.gomiddleware/audit.gocommon/rate_limit_test.gorelay/channel/zhipu/relay-zhipu.goservice/channel_affinity_template_test.gomiddleware/rate-limit.gomiddleware/email-verification-rate-limit.gomiddleware/turnstile-check.goservice/tiered_settle.gocommon/email.gocommon/system_monitor.gocommon/go-channel.gomiddleware/turnstile_check_test.gopkg/billingexpr/billingexpr_test.gocommon/rate-limit.go
pkg/billingexpr/**/*.{go,md}
📄 CodeRabbit inference engine (AGENTS.md)
When working on tiered/dynamic billing expression pricing, first read
pkg/billingexpr/expr.mdand ensure all code changes in the billing expression system follow the documented design, architecture, token normalization, quota conversion, and versioning patterns.
Files:
pkg/billingexpr/run.gopkg/billingexpr/billingexpr_test.go
relay/channel/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
When implementing a new channel, confirm whether the provider supports
StreamOptions; if it does, add the channel tostreamSupportedChannels.
Files:
relay/channel/gemini/relay_gemini_usage_test.gorelay/channel/gemini/relay-gemini.gorelay/channel/zhipu/relay-zhipu.go
🔇 Additional comments (22)
common/rate-limit.go (1)
4-4: LGTM!Also applies to: 33-54
common/rate_limit_test.go (1)
13-37: LGTM!middleware/email-verification-rate-limit.go (1)
58-58: LGTM!Also applies to: 78-78
middleware/model-rate-limit.go (1)
133-133: LGTM!common/email.go (1)
8-16: LGTM!Also applies to: 64-93
common/email_test.go (1)
223-268: LGTM!middleware/turnstile-check.go (1)
4-9: LGTM!Also applies to: 22-61, 81-81, 91-91
middleware/turnstile_check_test.go (1)
4-20: LGTM!Also applies to: 48-67
common/system_monitor.go (1)
4-5: LGTM!Also applies to: 33-37, 39-58, 60-82, 84-105
common/system_monitor_test.go (1)
1-18: LGTM!main.go (1)
246-251: LGTM!middleware/rate-limit.go (1)
20-23: LGTM!common/go-channel.go (1)
37-43: LGTM!common/go_channel_test.go (1)
1-29: LGTM!middleware/audit.go (1)
150-150: LGTM!pkg/billingexpr/billingexpr_test.go (1)
9-19: LGTM!Also applies to: 33-33, 47-47, 327-327, 358-358, 432-435, 450-450, 469-469, 518-518, 550-550, 582-582, 648-655, 688-704
pkg/billingexpr/run.go (1)
115-117: LGTM!relay/channel/gemini/relay-gemini.go (1)
1782-1788: LGTM!relay/channel/gemini/relay_gemini_usage_test.go (1)
70-90: LGTM!service/channel_affinity.go (1)
40-44: LGTM!Also applies to: 258-290
service/tiered_settle.go (1)
7-7: LGTM!Also applies to: 192-196
service/tiered_settle_test.go (1)
117-133: LGTM!
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@middleware/auth_session_test.go`:
- Around line 180-204: Update TestUserAuthRejectsNonEnabledStatus to assert the
specific HTTP status returned by UserAuth for a rejected request, replacing the
broad NotEqual check. Preserve the existing stale-session setup and request flow
so the test verifies rejection due to the non-enabled user status.
In `@model/log_test.go`:
- Around line 108-122: Update TestRecordConsumeLogSanitizesPersistedContent to
save the original common.LogConsumeEnabled value, enable it for the test before
calling RecordConsumeLog, and restore the original value with t.Cleanup. Keep
the existing database cleanup and assertions unchanged.
In `@model/token.go`:
- Around line 292-296: Update GetTokenByKey so its database predicate always
includes the Key field, including when key is empty; replace the struct-based
DB.Where condition with an explicit-condition form while preserving the existing
First and success assignment behavior.
In `@service/task_polling.go`:
- Around line 205-206: Update the unsuccessful-response branch after
common.Unmarshal in the task polling flow to return a newly constructed non-nil
error after logging, instead of returning the nil err variable. Preserve the
existing success handling and include sufficient context about the Suno response
failure for the polling caller.
In `@setting/ratio_setting/pricing_validation.go`:
- Around line 96-100: Update loadPricingMapWithOptions so it loads a rewritten
JSON map whose keys are trimmed and normalized, rather than the original
jsonStr; reuse the same normalization rules used by validatePricingMapJSONString
and preserve collision validation. Add a test verifying that one normalized
alias resolves to its configured ratio after an update.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 624b0d1e-b982-446b-a2f1-489dc24c8073
📒 Files selected for processing (32)
common/str.gocommon/str_test.gocontroller/task_video.gocontroller/user_setting_test.gocontroller/video_proxy.gocontroller/video_proxy_test.gomiddleware/auth.gomiddleware/auth_session_test.gomodel/billing_settlement.gomodel/log.gomodel/log_test.gomodel/midjourney.gomodel/option.gomodel/option_test.gomodel/task.gomodel/token.gorelay/channel/minimax/tts.gorelay/channel/openai/audio.gorelay/helper/price.gorelay/helper/price_test.gorelay/relay_task.goservice/error.goservice/error_test.goservice/http.goservice/http_client_test.goservice/midjourney_task.goservice/task_billing.goservice/task_polling.gosetting/ratio_setting/model_ratio.gosetting/ratio_setting/pricing_validation.gosetting/ratio_setting/pricing_validation_test.gosetting/task_billing_setting/rate_card.go
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: In Go business code, all JSON marshal/unmarshal operations must use the wrapper functions incommon/json.go(common.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson,common.GetJsonType) and must not directly callencoding/jsonfor actual encode/decode work.
All database code must be compatible with SQLite, MySQL 5.7.8+, and PostgreSQL 9.6+; prefer GORM abstractions over raw SQL, avoid directAUTO_INCREMENT/SERIAL, usecommonGroupCol/commonKeyColandcommonTrueVal/commonFalseValfor DB-specific SQL, branch withcommon.UsingPostgreSQL/common.UsingSQLite/common.UsingMySQL, avoid unsupported DB-specific functions/operators without fallback, and make migrations work across all three databases.
When implementing a new relay channel, confirm whether the provider supportsStreamOptions; if it does, add that channel tostreamSupportedChannels.
For request structs parsed from client JSON and re-marshaled to upstream providers, optional scalar fields must use pointer types withomitemptyso explicit zero/false values are preserved instead of dropped.
When working on tiered/dynamic billing expression code, readpkg/billingexpr/expr.mdfirst and follow its documented expression language, architecture, token normalization, quota conversion, and versioning patterns.
**/*.go: All JSON marshal/unmarshal operations in Go business code must use the wrapper functions incommon/json.go(common.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson,common.GetJsonType) instead of directly importing or callingencoding/jsonfor actual marshal/unmarshal work.
All database code in Go must remain compatible with SQLite, MySQL >= 5.7.8, and PostgreSQL >= 9.6; prefer GORM abstractions, avoid raw SQL unless necessary, use the shared DB helper variables for reserved words and boolean literals, branch with thecommon.UsingPostgreSQL/common.UsingSQLite/common.UsingMySQLflags when need...
Files:
setting/ratio_setting/pricing_validation_test.gosetting/ratio_setting/model_ratio.gosetting/task_billing_setting/rate_card.gomodel/billing_settlement.gomodel/option_test.gorelay/channel/openai/audio.gocontroller/task_video.gomiddleware/auth.gorelay/helper/price_test.gomodel/log_test.gomodel/option.gocommon/str_test.goservice/http_client_test.gomodel/token.gorelay/relay_task.gocontroller/video_proxy_test.gocontroller/video_proxy.gorelay/channel/minimax/tts.goservice/error.goservice/midjourney_task.gomodel/task.goservice/error_test.gocommon/str.goservice/task_billing.goservice/http.gorelay/helper/price.gosetting/ratio_setting/pricing_validation.gocontroller/user_setting_test.gomodel/log.goservice/task_polling.gomiddleware/auth_session_test.gomodel/midjourney.go
relay/channel/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
When implementing a new channel, confirm whether the provider supports
StreamOptions; if it does, add the channel tostreamSupportedChannels.
Files:
relay/channel/openai/audio.gorelay/channel/minimax/tts.go
🔇 Additional comments (43)
service/error.go (1)
138-165: LGTM!Also applies to: 167-203, 205-208
service/error_test.go (1)
200-239: LGTM!service/midjourney_task.go (1)
47-47: LGTM!service/task_billing.go (1)
152-152: LGTM!Also applies to: 186-186
service/task_polling.go (1)
202-204: LGTM!Also applies to: 228-228, 376-376, 421-421, 465-465
setting/task_billing_setting/rate_card.go (1)
45-47: LGTM!Also applies to: 143-145
common/str.go (1)
22-23: LGTM!Also applies to: 33-63
common/str_test.go (1)
1-31: LGTM!relay/channel/openai/audio.go (1)
49-49: LGTM!relay/helper/price.go (1)
22-22: LGTM!Also applies to: 36-40, 182-194, 204-205
relay/relay_task.go (1)
327-327: LGTM!service/http.go (4)
16-27: LGTM!
39-61: LGTM!
63-84: LGTM!
86-122: LGTM!Also applies to: 138-138
service/http_client_test.go (1)
5-10: LGTM!Also applies to: 33-86
controller/task_video.go (1)
8-15: LGTM!controller/user_setting_test.go (1)
182-224: LGTM!controller/video_proxy.go (3)
23-30: LGTM!
173-179: 🎯 Functional Correctness | ⚡ Quick winConfirm the loss of upstream
Content-Lengthfor streamed video responses.
CopyUpstreamResponseHeadersblockscontent-length. This handler streams the upstream body withio.Copyand never setsContent-Lengthitself, so responses now use chunked transfer encoding. Video players lose the total-size hint that the previous manual copy forwarded. If clients depend on that hint, setContent-Lengthexplicitly fromresp.ContentLengthwhen it is non-negative.Proposed change
service.CopyUpstreamResponseHeaders(c, resp.Header) c.Writer.Header().Set("Cache-Control", "public, max-age=86400") + if resp.ContentLength >= 0 { + c.Writer.Header().Set("Content-Length", strconv.FormatInt(resp.ContentLength, 10)) + } c.Writer.WriteHeader(resp.StatusCode)
190-241: LGTM!middleware/auth.go (3)
337-344: LGTM!
462-465: LGTM!
195-202: 🔒 Security & PrivacyNo change needed. User status has only
UserStatusEnabledandUserStatusDisabled, so this check does not hide pending or unverified states.model/log_test.go (1)
8-11: LGTM!Also applies to: 123-173
model/task.go (4)
491-491: LGTM!
627-627: LGTM!Also applies to: 644-681, 690-690, 710-710
728-728: 🗄️ Data Integrity & IntegrationNo change needed for
sanitizeFailReasonUpdateParamkey matching.The helper sanitizes the same
fail_reasonkey used by the calling bulk update paths.
75-80: 🗄️ Data Integrity & IntegrationNo change needed.
SanitizePersistedLogContentis idempotent for truncated text, so reloading and saving the sameFailReasondoes not append another... [truncated]suffix.> Likely an incorrect or invalid review comment.relay/channel/minimax/tts.go (1)
193-193: LGTM!relay/helper/price_test.go (2)
9-14: LGTM!Also applies to: 66-93
95-116: LGTM!setting/ratio_setting/model_ratio.go (1)
398-401: LGTM!Also applies to: 449-452
setting/ratio_setting/pricing_validation.go (1)
6-42: LGTM!Also applies to: 44-87
setting/ratio_setting/pricing_validation_test.go (1)
10-29: LGTM!controller/video_proxy_test.go (1)
14-57: LGTM!middleware/auth_session_test.go (2)
32-32: LGTM!Also applies to: 57-73
42-54: 🎯 Functional CorrectnessNo change needed for user-cache isolation in these tests.
The middleware test setup disables Redis, so
GetUserCachefalls back to fresh database reads for both disabled and deleted-token-owner cases.model/billing_settlement.go (1)
512-514: LGTM!model/log.go (1)
76-76: LGTM!Also applies to: 398-398, 418-418, 449-449, 470-470, 496-496, 526-526, 590-590, 683-683
model/midjourney.go (1)
39-44: LGTM!Also applies to: 214-214, 448-468
model/option.go (1)
357-362: LGTM!model/option_test.go (1)
4-5: LGTM!Also applies to: 211-230
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (1)
middleware/auth_session_test.go (1)
196-204: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the rejection reason, not only status 200.
freshAuthHelperreturns HTTP 200 with"success": falsefor every rejection branch (banned, insufficient privilege, invalid user info). The handler returns 204. Sorequire.Equal(t, http.StatusOK, ...)proves that the middleware aborted, but it does not prove that the non-enabled status caused the abort. Assert the response body as well.💚 Proposed assertion
require.Equal(t, http.StatusOK, recorder.Code) + require.Contains(t, recorder.Body.String(), `"success":false`)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@middleware/auth_session_test.go` around lines 196 - 204, Update the test around the cookie loop and router.ServeHTTP call to assert the response body’s failure payload in addition to the existing status assertion. Verify the body indicates the expected rejection reason, such as `"success": false`, so the test distinguishes middleware rejection from an unrelated successful response.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@common/str.go`:
- Around line 22-23: The truncation logic using PersistedLogContentLimit and
persistedLogContentTruncatedSuffix must reserve the suffix length when retaining
content, so the complete sanitized result stays within the limit. Update
common/str.go lines 22-23 accordingly, and adjust the expectation in
common/str_test.go lines 29-30 to assert the complete truncated result contains
at most PersistedLogContentLimit runes.
In `@model/token.go`:
- Around line 293-297: Update the token lookup around dbToken to use the
existing commonKeyCol helper in the DB.Where condition, replacing the manually
constructed clause.Eq reference to the reserved key column while preserving the
current query and assignment behavior.
In `@relay/helper/price_test.go`:
- Around line 95-116: Stabilize TestModelPriceHelperPerCallUsesDefaultTaskPrice
by controlling the group ratio and other pricing inputs used by
ModelPriceHelperPerCall. Prefer setting the default group ratio explicitly and
restoring it in t.Cleanup, and assert GroupRatioInfo.GroupRatio and ModelPrice
alongside Quota; also pin the free-model pre-consume setting if it is mutable so
the expected quota remains deterministic.
In `@service/http.go`:
- Around line 43-61: Remove the Gin-context mutation from
ShouldCopyUpstreamHeader, keeping it as a side-effect-free predicate. In
CopyUpstreamResponseHeaders, detect the request ID header and set
common.UpstreamRequestIdKey on the provided context before or alongside the
existing predicate-based filtering, preserving the current request-ID capture
behavior.
In `@service/task_polling.go`:
- Line 233: Update taskNeedsUpdate to sanitize newTask.FailReason with
common.SanitizePersistedLogContent before comparing it to oldTask.FailReason,
matching the normalization used by the task.FailReason assignment. Preserve the
existing fallback behavior for empty failure reasons and avoid repeated updates
when the persisted values are equivalent.
- Around line 207-212: Update the error construction in UpdateSunoTasks to apply
common.MaskSensitiveInfo to responseItems.Message before passing it to
common.SanitizePersistedLogContent, matching the transformation used by
TaskErrorFromUpstreamResponse while preserving the existing error format.
---
Duplicate comments:
In `@middleware/auth_session_test.go`:
- Around line 196-204: Update the test around the cookie loop and
router.ServeHTTP call to assert the response body’s failure payload in addition
to the existing status assertion. Verify the body indicates the expected
rejection reason, such as `"success": false`, so the test distinguishes
middleware rejection from an unrelated successful response.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 18de7509-14c7-4732-8f3e-4755470da483
📒 Files selected for processing (34)
common/str.gocommon/str_test.gocontroller/task_video.gocontroller/user_setting_test.gocontroller/video_proxy.gocontroller/video_proxy_test.gomiddleware/auth.gomiddleware/auth_session_test.gomodel/billing_settlement.gomodel/log.gomodel/log_test.gomodel/midjourney.gomodel/option.gomodel/option_test.gomodel/task.gomodel/token.gomodel/token_readonly_test.gorelay/channel/minimax/tts.gorelay/channel/openai/audio.gorelay/helper/price.gorelay/helper/price_test.gorelay/relay_task.goservice/error.goservice/error_test.goservice/http.goservice/http_client_test.goservice/midjourney_task.goservice/task_billing.goservice/task_polling.goservice/task_polling_test.gosetting/ratio_setting/model_ratio.gosetting/ratio_setting/pricing_validation.gosetting/ratio_setting/pricing_validation_test.gosetting/task_billing_setting/rate_card.go
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: In Go business code, all JSON marshal/unmarshal operations must use the wrapper functions incommon/json.go(common.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson,common.GetJsonType) and must not directly callencoding/jsonfor actual encode/decode work.
All database code must be compatible with SQLite, MySQL 5.7.8+, and PostgreSQL 9.6+; prefer GORM abstractions over raw SQL, avoid directAUTO_INCREMENT/SERIAL, usecommonGroupCol/commonKeyColandcommonTrueVal/commonFalseValfor DB-specific SQL, branch withcommon.UsingPostgreSQL/common.UsingSQLite/common.UsingMySQL, avoid unsupported DB-specific functions/operators without fallback, and make migrations work across all three databases.
When implementing a new relay channel, confirm whether the provider supportsStreamOptions; if it does, add that channel tostreamSupportedChannels.
For request structs parsed from client JSON and re-marshaled to upstream providers, optional scalar fields must use pointer types withomitemptyso explicit zero/false values are preserved instead of dropped.
When working on tiered/dynamic billing expression code, readpkg/billingexpr/expr.mdfirst and follow its documented expression language, architecture, token normalization, quota conversion, and versioning patterns.
**/*.go: All JSON marshal/unmarshal operations in Go business code must use the wrapper functions incommon/json.go(common.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson,common.GetJsonType) instead of directly importing or callingencoding/jsonfor actual marshal/unmarshal work.
All database code in Go must remain compatible with SQLite, MySQL >= 5.7.8, and PostgreSQL >= 9.6; prefer GORM abstractions, avoid raw SQL unless necessary, use the shared DB helper variables for reserved words and boolean literals, branch with thecommon.UsingPostgreSQL/common.UsingSQLite/common.UsingMySQLflags when need...
Files:
model/token_readonly_test.gocontroller/task_video.gorelay/channel/openai/audio.gosetting/task_billing_setting/rate_card.gomodel/billing_settlement.gosetting/ratio_setting/pricing_validation_test.gocommon/str_test.gosetting/ratio_setting/model_ratio.goservice/task_billing.gomodel/option.gomodel/log_test.gomodel/option_test.gorelay/helper/price.gorelay/channel/minimax/tts.goservice/error.goservice/midjourney_task.gomiddleware/auth_session_test.gocontroller/user_setting_test.gocontroller/video_proxy.gomiddleware/auth.goservice/http.goservice/task_polling.gocommon/str.gomodel/task.gosetting/ratio_setting/pricing_validation.gomodel/token.gocontroller/video_proxy_test.gorelay/helper/price_test.goservice/task_polling_test.gomodel/log.goservice/error_test.gorelay/relay_task.goservice/http_client_test.gomodel/midjourney.go
relay/channel/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
When implementing a new channel, confirm whether the provider supports
StreamOptions; if it does, add the channel tostreamSupportedChannels.
Files:
relay/channel/openai/audio.gorelay/channel/minimax/tts.go
🔇 Additional comments (30)
relay/helper/price.go (1)
22-22: LGTM!Also applies to: 36-39, 182-205
relay/relay_task.go (1)
327-327: LGTM!service/error.go (1)
138-208: LGTM!service/midjourney_task.go (1)
47-47: LGTM!service/task_billing.go (1)
152-152: LGTM!Also applies to: 186-186
setting/task_billing_setting/rate_card.go (1)
45-47: LGTM!Also applies to: 143-145
service/error_test.go (1)
200-239: LGTM!controller/task_video.go (1)
8-15: LGTM!controller/video_proxy.go (1)
24-32: LGTM!Also applies to: 174-174, 183-192, 201-207, 217-226, 228-252
middleware/auth.go (1)
195-195: LGTM!Also applies to: 337-344, 462-465
middleware/auth_session_test.go (1)
32-32: LGTM!Also applies to: 42-42, 54-74, 233-267
model/log_test.go (1)
8-11: LGTM!Also applies to: 108-137, 139-153, 155-179
model/task.go (2)
491-491: LGTM!Also applies to: 627-627, 644-681, 690-690, 710-710, 728-728, 743-743
75-81: 📐 Maintainability & Code QualityOnly
model/task.godeclares(*Task).BeforeSave; the matchingBeforeSavedefinitions use different receiver types.model/token_readonly_test.go (1)
8-8: LGTM!Also applies to: 53-62
relay/helper/price_test.go (1)
9-14: LGTM!Also applies to: 65-93
service/http.go (1)
16-28: LGTM!Also applies to: 63-122, 138-138
service/http_client_test.go (1)
5-10: LGTM!Also applies to: 33-50, 52-86
controller/user_setting_test.go (1)
182-223: LGTM!controller/video_proxy_test.go (1)
14-25: LGTM!Also applies to: 27-38, 40-57, 59-77
model/billing_settlement.go (1)
512-514: LGTM!model/log.go (1)
76-76: LGTM!Also applies to: 398-398, 418-418, 449-449, 470-470, 496-496, 526-526, 590-590, 683-683
model/midjourney.go (1)
9-9: LGTM!Also applies to: 39-44, 214-214, 448-468
model/option.go (1)
357-362: LGTM!model/option_test.go (1)
4-5: LGTM!Also applies to: 211-230
relay/channel/minimax/tts.go (1)
193-193: LGTM!relay/channel/openai/audio.go (1)
49-49: LGTM!setting/ratio_setting/model_ratio.go (1)
398-401: LGTM!Also applies to: 449-452
setting/ratio_setting/pricing_validation.go (1)
6-119: LGTM!setting/ratio_setting/pricing_validation_test.go (1)
1-55: LGTM!
| var dbToken Token | ||
| err = DB.Where(clause.Eq{Column: clause.Column{Name: "key"}, Value: key}).First(&dbToken).Error | ||
| if err == nil { | ||
| token = &dbToken | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect commonKeyCol definition and existing key-column predicates in the model package.
rg -nP --type=go -C3 '\bcommonKeyCol\b'
echo "---- other key predicates ----"
rg -nP --type=go -C2 'Where\(.*\bkey\b' modelRepository: MAX-API-Next/MAX-API
Length of output: 158
🏁 Script executed:
#!/bin/bash
set -u
echo "---- repo files matching token.go/common-related ----"
git ls-files | rg -n '(^|/)(token\.go|common|.*_test\.go)$' | head -200
echo "---- search commonKeyCol/text key predicates broad ----"
rg -n --hidden --glob '!vendor/**' --glob '!node_modules/**' --glob '!dist/**' '\bcommonKeyCol\b|Where\(.*\bkey\b|clause\.Column\{\s*Name:\s*"key"' . || true
echo "---- model/token.go relevant section ----"
if [ -f model/token.go ]; then
wc -l model/token.go
sed -n '260,320p' model/token.go | cat -n
fiRepository: MAX-API-Next/MAX-API
Length of output: 12259
Use commonKeyCol for the token key lookup.
commonKeyCol is the repository’s DB-specific helper for the reserved key column, and it is already used around model/token.go. Use DB.Where(commonKeyCol+" = ?", key) instead of clause.Eq{Column: clause.Column{Name: "key"}, Value: key} here.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@model/token.go` around lines 293 - 297, Update the token lookup around
dbToken to use the existing commonKeyCol helper in the DB.Where condition,
replacing the manually constructed clause.Eq reference to the reserved key
column while preserving the current query and assignment behavior.
Source: Coding guidelines
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
service/http.go (1)
68-72: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRemove headers nominated by
Connection.The code rejects
Connectionitself, but it does not reject headers named by its values. An upstreamConnection: X-Request-IDwithX-Request-ID: valuecaptures a hop-by-hop value before filtering. Other nominated headers can also be forwarded.Build a case-insensitive set of header names from all
Connectionvalues before this loop. Skip those names before request-ID capture and response-header copying. Add a test for a nominated safe-looking header.Proposed fix
func CopyUpstreamResponseHeaders(c *gin.Context, header http.Header) { if c == nil || c.Writer == nil { return } + connectionScoped := make(map[string]struct{}) + for key, values := range header { + if !strings.EqualFold(key, "Connection") { + continue + } + for _, value := range values { + for _, name := range strings.Split(value, ",") { + name = strings.TrimSpace(name) + if isSafeResponseHeaderName(name) { + connectionScoped[strings.ToLower(name)] = struct{}{} + } + } + } + } for key, values := range header { + if _, ok := connectionScoped[strings.ToLower(key)]; ok { + continue + } if len(values) > 0 && strings.EqualFold(key, common.RequestIdKey) && isSafeResponseHeaderName(key) && isSafeResponseHeaderValue(values[0]) { c.Set(common.UpstreamRequestIdKey, values[0]) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@service/http.go` around lines 68 - 72, Build a case-insensitive set of hop-by-hop header names nominated by all Connection header values before the header iteration. In the loop over header, skip any nominated name before the RequestId capture condition and before shouldCopyUpstreamHeader, while continuing to reject Connection itself. Add coverage for a nominated safe-looking header to verify it is neither captured nor forwarded.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@service/http.go`:
- Around line 68-72: Build a case-insensitive set of hop-by-hop header names
nominated by all Connection header values before the header iteration. In the
loop over header, skip any nominated name before the RequestId capture condition
and before shouldCopyUpstreamHeader, while continuing to reject Connection
itself. Add coverage for a nominated safe-looking header to verify it is neither
captured nor forwarded.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 6dd16236-7160-490e-a14c-a19938430f3e
📒 Files selected for processing (5)
service/http.goservice/http_client_test.goservice/task_billing_test.goservice/task_polling.goservice/task_polling_test.go
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: In Go business code, all JSON marshal/unmarshal operations must use the wrapper functions incommon/json.go(common.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson,common.GetJsonType) and must not directly callencoding/jsonfor actual encode/decode work.
All database code must be compatible with SQLite, MySQL 5.7.8+, and PostgreSQL 9.6+; prefer GORM abstractions over raw SQL, avoid directAUTO_INCREMENT/SERIAL, usecommonGroupCol/commonKeyColandcommonTrueVal/commonFalseValfor DB-specific SQL, branch withcommon.UsingPostgreSQL/common.UsingSQLite/common.UsingMySQL, avoid unsupported DB-specific functions/operators without fallback, and make migrations work across all three databases.
When implementing a new relay channel, confirm whether the provider supportsStreamOptions; if it does, add that channel tostreamSupportedChannels.
For request structs parsed from client JSON and re-marshaled to upstream providers, optional scalar fields must use pointer types withomitemptyso explicit zero/false values are preserved instead of dropped.
When working on tiered/dynamic billing expression code, readpkg/billingexpr/expr.mdfirst and follow its documented expression language, architecture, token normalization, quota conversion, and versioning patterns.
**/*.go: All JSON marshal/unmarshal operations in Go business code must use the wrapper functions incommon/json.go(common.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson,common.GetJsonType) instead of directly importing or callingencoding/jsonfor actual marshal/unmarshal work.
All database code in Go must remain compatible with SQLite, MySQL >= 5.7.8, and PostgreSQL >= 9.6; prefer GORM abstractions, avoid raw SQL unless necessary, use the shared DB helper variables for reserved words and boolean literals, branch with thecommon.UsingPostgreSQL/common.UsingSQLite/common.UsingMySQLflags when need...
Files:
service/task_billing_test.goservice/http_client_test.goservice/task_polling_test.goservice/http.goservice/task_polling.go
🔇 Additional comments (4)
service/task_billing_test.go (1)
566-596: LGTM!service/http_client_test.go (1)
35-48: LGTM!service/task_polling_test.go (1)
93-100: LGTM!service/task_polling.go (1)
282-284: LGTM!
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
main.go (1)
240-255: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winUse the original shutdown context for background runners.
Line 240 starts a second full
shutdownTimeoutaftershutdownHTTPServer(ctx, srv)returns. If HTTP shutdown consumes its deadline, the process can wait for another full timeout while stopping background runners.Pass
ctxto every background shutdown call. This enforces the configured shutdown deadline for the complete shutdown sequence.Proposed fix
- stopBackgroundRunnerCtx, stopBackgroundRunnerCancel := context.WithTimeout(context.Background(), shutdownTimeout) - if err := common.StopSystemMonitor(stopBackgroundRunnerCtx); err != nil { + if err := common.StopSystemMonitor(ctx); err != nil { common.SysError(fmt.Sprintf("timed out stopping system monitor: %v", err)) } - if err := middleware.StopInMemoryRateLimiter(stopBackgroundRunnerCtx); err != nil { + if err := middleware.StopInMemoryRateLimiter(ctx); err != nil { common.SysError(fmt.Sprintf("timed out stopping in-memory rate limiter: %v", err)) } - if err := model.StopBillingSettlementTaskRunner(stopBackgroundRunnerCtx); err != nil { + if err := model.StopBillingSettlementTaskRunner(ctx); err != nil { common.SysError(fmt.Sprintf("timed out stopping billing settlement runner: %v", err)) } if common.RedisEnabled { - if err := model.StopCacheInvalidationTaskRunner(stopBackgroundRunnerCtx); err != nil { + if err := model.StopCacheInvalidationTaskRunner(ctx); err != nil { common.SysError(fmt.Sprintf("timed out stopping cache invalidation runner: %v", err)) } } - stopBackgroundRunnerCancel()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@main.go` around lines 240 - 255, Use the original shutdown context ctx for all background shutdown calls after shutdownHTTPServer, removing the separate stopBackgroundRunnerCtx/stopBackgroundRunnerCancel timeout context. Pass ctx to StopSystemMonitor, StopInMemoryRateLimiter, StopBillingSettlementTaskRunner, and StopCacheInvalidationTaskRunner so the complete shutdown sequence shares one deadline.relay/channel/ali/image.go (1)
30-69: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCarry the OpenAI
nvalue intoParameters.Nwhenrequest.Extrais nil.When
request.Extrais nil,imageRequest.Parameters.Nbecomes nil, soNValue()returns 0,dto.ValidateImageN("parameters.n", 0)passes, nonpricing ratio is added, and the upstream request omitsn. Use the same unconditional default fromrequest.NasoaiFormEdit2AliImageEditfor this standard field.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@relay/channel/ali/image.go` around lines 30 - 69, Ensure the image request conversion initializes imageRequest.Parameters.N from request.N even when request.Extra is nil, matching the unconditional default behavior used by oaiFormEdit2AliImageEdit. Preserve any parameters parsed from request.Extra, while ensuring NValue(), validation, pricing, and the upstream request all use the OpenAI n value.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@controller/oauth.go`:
- Around line 133-136: Update the unauthenticated branch in GenerateOAuthCode to
replace the hardcoded Chinese bind-login message with the existing i18n key for
“login required,” using the same common.ApiErrorI18n pattern as the other error
paths. Preserve the 401 Unauthorized status and response behavior required by
TestGenerateOAuthCodeRejectsAnonymousBind.
In `@model/auth_flow_test.go`:
- Around line 20-33: Update setupAuthFlowTestDB to add a SQLite busy timeout
parameter to the shared in-memory DSN used by gorm.Open. Keep the shared
in-memory configuration intact so concurrent ConsumeAuthFlow calls wait for the
writer lock and preserve the expected ErrAuthFlowConsumed result.
In `@model/auth_flow.go`:
- Around line 62-64: Update authFlowTokenHash to use a stable persisted signing
key rather than common.SessionSecret, which may be regenerated when
SESSION_SECRET is unset. Source the key from the configured environment variable
or database-backed setting, and ensure the same key is used by session storage
so persisted AuthFlow tokens remain valid across restarts and instances.
In `@oauth/oidc.go`:
- Around line 143-146: Update the required identity-field check in the OIDC
user-info flow to reject only when oidcUser.OpenID is empty; allow an empty
oidcUser.Email so findOrCreateOAuthUser can handle subject-only authentication.
In `@relay/channel/coze/adaptor.go`:
- Line 82: Check the error returned by common2.Unmarshal in the response
handling flow before accessing cozeResponse or starting polling; return or
propagate the decode error immediately when unmarshalling fails, while
preserving the existing behavior for successfully decoded responses.
In `@relay/common/pass_through_body.go`:
- Around line 418-432: Optimize passThroughJSONScanner byte scanning by avoiding
separate bufio.Reader calls from peekByte and readByte for the same byte. Update
these methods and their scanner state to support one-byte pushback or equivalent
buffered-slice scanning, while preserving readByte offset increments only when a
byte is consumed and maintaining existing EOF/error behavior.
In `@service/auth_flow_cleanup.go`:
- Around line 12-23: The background loops in StartAuthFlowCleanup in
service/auth_flow_cleanup.go (lines 12-23) and Monitor in common/pprof.go (lines
15-19) need shutdown control. Pass the existing shutdown context or stop channel
into both loops, select on cancellation alongside ticker events, and stop the
auth cleanup ticker and CPU monitor during graceful shutdown.
---
Outside diff comments:
In `@main.go`:
- Around line 240-255: Use the original shutdown context ctx for all background
shutdown calls after shutdownHTTPServer, removing the separate
stopBackgroundRunnerCtx/stopBackgroundRunnerCancel timeout context. Pass ctx to
StopSystemMonitor, StopInMemoryRateLimiter, StopBillingSettlementTaskRunner, and
StopCacheInvalidationTaskRunner so the complete shutdown sequence shares one
deadline.
In `@relay/channel/ali/image.go`:
- Around line 30-69: Ensure the image request conversion initializes
imageRequest.Parameters.N from request.N even when request.Extra is nil,
matching the unconditional default behavior used by oaiFormEdit2AliImageEdit.
Preserve any parameters parsed from request.Extra, while ensuring NValue(),
validation, pricing, and the upstream request all use the OpenAI n value.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 502414f8-8b00-4b9c-99b4-7a70d592f0c1
📒 Files selected for processing (71)
common/body_storage.gocommon/body_storage_replay_test.gocommon/email.gocommon/pprof.gocommon/pprof_test.gocommon/random_failure_test.gocommon/str.gocommon/utils.gocontroller/codex_oauth.gocontroller/oauth.gocontroller/oauth_test.gocontroller/subscription_payment_epay.gocontroller/topup.gocontroller/user.godocs/openapi/api.jsondto/openai_request.godto/values.gomain.gomain_test.gomodel/ability.gomodel/auth_flow.gomodel/auth_flow_test.gomodel/main.gomodel/subscription.gomodel/twofa.gomodel/user.gooauth/generic.gooauth/http_client.gooauth/http_client_test.gooauth/oidc.gorelay/channel/ali/dto.gorelay/channel/ali/image.gorelay/channel/ali/image_wan.gorelay/channel/ali/rerank.gorelay/channel/ali/text.gorelay/channel/ali/text_test.gorelay/channel/api_request.gorelay/channel/api_request_test.gorelay/channel/baidu/dto.gorelay/channel/baidu/relay-baidu.gorelay/channel/baidu/relay_baidu_test.gorelay/channel/coze/adaptor.gorelay/channel/coze/dto.gorelay/channel/coze/relay-coze.gorelay/channel/coze/relay_coze_test.gorelay/channel/siliconflow/adaptor.gorelay/channel/siliconflow/adaptor_test.gorelay/channel/siliconflow/dto.gorelay/channel/siliconflow/relay-siliconflow.gorelay/channel/vertex/adaptor.gorelay/channel/vertex/service_account.gorelay/channel/vertex/service_account_test.gorelay/channel/zhipu/adaptor.gorelay/channel/zhipu/relay-zhipu.gorelay/channel/zhipu/relay_zhipu_test.gorelay/claude_handler.gorelay/common/outbound_body.gorelay/common/outbound_body_test.gorelay/common/override_test.gorelay/common/pass_through_body.gorelay/common/relay_info.gorelay/compatible_handler.gorelay/responses_handler.gorouter/api-router.gorouter/api_router_test.goservice/auth_flow_cleanup.goservice/system_task.gotools/jsonwrapcheck/allowlist.txtweb/default/src/features/auth/api.tsweb/default/src/features/auth/hooks/use-oauth-login.tsweb/default/src/lib/oauth.ts
💤 Files with no reviewable changes (1)
- tools/jsonwrapcheck/allowlist.txt
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: In Go business code, all JSON marshal/unmarshal operations must use the wrapper functions incommon/json.go(common.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson,common.GetJsonType) and must not directly callencoding/jsonfor actual encode/decode work.
All database code must be compatible with SQLite, MySQL 5.7.8+, and PostgreSQL 9.6+; prefer GORM abstractions over raw SQL, avoid directAUTO_INCREMENT/SERIAL, usecommonGroupCol/commonKeyColandcommonTrueVal/commonFalseValfor DB-specific SQL, branch withcommon.UsingPostgreSQL/common.UsingSQLite/common.UsingMySQL, avoid unsupported DB-specific functions/operators without fallback, and make migrations work across all three databases.
When implementing a new relay channel, confirm whether the provider supportsStreamOptions; if it does, add that channel tostreamSupportedChannels.
For request structs parsed from client JSON and re-marshaled to upstream providers, optional scalar fields must use pointer types withomitemptyso explicit zero/false values are preserved instead of dropped.
When working on tiered/dynamic billing expression code, readpkg/billingexpr/expr.mdfirst and follow its documented expression language, architecture, token normalization, quota conversion, and versioning patterns.
**/*.go: All JSON marshal/unmarshal operations in Go business code must use the wrapper functions incommon/json.go(common.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson,common.GetJsonType) instead of directly importing or callingencoding/jsonfor actual marshal/unmarshal work.
All database code in Go must remain compatible with SQLite, MySQL >= 5.7.8, and PostgreSQL >= 9.6; prefer GORM abstractions, avoid raw SQL unless necessary, use the shared DB helper variables for reserved words and boolean literals, branch with thecommon.UsingPostgreSQL/common.UsingSQLite/common.UsingMySQLflags when need...
Files:
relay/channel/zhipu/relay_zhipu_test.gocommon/pprof_test.gooauth/http_client.gomodel/main.gorelay/channel/coze/dto.gocommon/body_storage_replay_test.gorelay/channel/baidu/dto.gorelay/claude_handler.gorelay/channel/vertex/service_account_test.gocontroller/codex_oauth.gorelay/channel/ali/image_wan.gorelay/compatible_handler.gorelay/responses_handler.gorelay/common/outbound_body_test.gomain_test.gocommon/pprof.gorelay/channel/coze/relay_coze_test.gorelay/channel/siliconflow/relay-siliconflow.godto/values.gorelay/channel/baidu/relay_baidu_test.gorelay/channel/ali/text.gorelay/channel/ali/rerank.gorelay/channel/zhipu/adaptor.gorelay/channel/vertex/adaptor.gomodel/twofa.gocommon/random_failure_test.gorelay/channel/siliconflow/adaptor.gorelay/channel/coze/adaptor.gocommon/utils.goservice/system_task.gorelay/channel/siliconflow/adaptor_test.gorouter/api-router.gorelay/channel/ali/image.goservice/auth_flow_cleanup.gorelay/channel/siliconflow/dto.gocommon/str.gocontroller/subscription_payment_epay.gorelay/channel/vertex/service_account.gomodel/auth_flow_test.gorouter/api_router_test.gorelay/common/outbound_body.gorelay/channel/ali/text_test.gorelay/common/override_test.gomodel/ability.gorelay/common/relay_info.gocontroller/user.gomodel/subscription.gooauth/oidc.gorelay/channel/baidu/relay-baidu.gorelay/channel/coze/relay-coze.gorelay/channel/ali/dto.gorelay/channel/api_request.godto/openai_request.gocommon/email.gomain.gooauth/http_client_test.gorelay/common/pass_through_body.gorelay/channel/api_request_test.gocommon/body_storage.gomodel/user.gocontroller/oauth_test.gorelay/channel/zhipu/relay-zhipu.gomodel/auth_flow.gocontroller/topup.gocontroller/oauth.gooauth/generic.go
relay/channel/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
When implementing a new channel, confirm whether the provider supports
StreamOptions; if it does, add the channel tostreamSupportedChannels.
Files:
relay/channel/zhipu/relay_zhipu_test.gorelay/channel/coze/dto.gorelay/channel/baidu/dto.gorelay/channel/vertex/service_account_test.gorelay/channel/ali/image_wan.gorelay/channel/coze/relay_coze_test.gorelay/channel/siliconflow/relay-siliconflow.gorelay/channel/baidu/relay_baidu_test.gorelay/channel/ali/text.gorelay/channel/ali/rerank.gorelay/channel/zhipu/adaptor.gorelay/channel/vertex/adaptor.gorelay/channel/siliconflow/adaptor.gorelay/channel/coze/adaptor.gorelay/channel/siliconflow/adaptor_test.gorelay/channel/ali/image.gorelay/channel/siliconflow/dto.gorelay/channel/vertex/service_account.gorelay/channel/ali/text_test.gorelay/channel/baidu/relay-baidu.gorelay/channel/coze/relay-coze.gorelay/channel/ali/dto.gorelay/channel/api_request.gorelay/channel/api_request_test.gorelay/channel/zhipu/relay-zhipu.go
dto/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
For request structs parsed from client JSON and then re-marshaled to upstream providers, especially in relay/convert paths, optional scalar fields must use pointer types with
omitemptyso that absent fields stay omitted while explicit zero/false values are preserved.
Files:
dto/values.godto/openai_request.go
web/default/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (web/default/AGENTS.md)
web/default/src/**/*.{ts,tsx}: 前端页面文本与组件内文案必须支持 i18n：React 组件中应使用useTranslation()取得t，并通过t()渲染用户可见文本；子组件也应自行使用useTranslation()保持独立性。
禁止使用两层及以上嵌套三元表达式；复杂逻辑应改用if-else、提前返回或抽取函数。
控制函数圈复杂度，复杂逻辑应拆成更小的函数；变量与函数命名应有意义并遵循驼峰等常规命名约定。
TypeScript 代码应避免使用any，优先使用具体类型或unknown；参数与返回值应显式标注类型；仅类型用途的导入应使用import type。
修改 TypeScript 或 TSX 代码后必须执行类型检查，并修复所有类型错误，不得遗留。
对象非必要不要解构，尤其是组件 props；优先直接使用props.xxx以保持代码清晰。
组件应使用函数式组件与 Hooks，遵循单一职责；组件 props 必须有明确类型（接口或类型别名）。
单文件超过约 200 行时应考虑拆分子组件或抽取自定义 Hooks；类型定义可与组件同文件或放在同模块的types中。
在 React 中应合理使用useMemo、useCallback、React.memo，避免在渲染路径中创建新对象或数组；必要时进行代码分割与动态import。
React Query 的数据获取应使用useQuery、变更应使用useMutation；每个查询需配置唯一queryKey，并在成功后对相关查询执行invalidateQueries；服务端错误应统一交给handleServerError。
Axios 请求应使用项目统一的api实例；GET 请求默认去重，特殊请求可显式关闭；认证与通用错误应在拦截器中统一处理。
服务端错误应统一使用handleServerError，展示层应使用toast.error等统一方式；文案需走 i18n；路由级错误应由errorComponent承接；表单错误应通过form.setError等方式映射到字段。
样式应以 Tailwind 工具类为主，动态类名使用cn()合并；非动态场景避免内联样式；响应式采用移动优先与 Tailwind 断点，主题与暗色模式通过 CSS 变量与dark:处理。
应使用语义化 HTML、正确关联label与输入、保证键盘可操作与合理焦点顺序；必要时添加 ARIA 属性，装饰性图标应使用aria-hidden="true"。
认证与权限应在路由与接口层校验；前后端都应做数据校验（如 Zod）；敏感信息不得落前端存储；避免使用dangerouslySetInnerHTML；跨域与 Cookie 需配合withCredentials并按后端要求处理 CSRF。
组件测试应使用 React Testing Library，关注交互与行为，避免测试实现细节；关键流程可补充集成与 E2E 测试。
环境变量应通过.env读取，并使用VITE_前缀；代码中不得硬编码密钥。
Files:
web/default/src/features/auth/api.tsweb/default/src/features/auth/hooks/use-oauth-login.tsweb/default/src/lib/oauth.ts
web/default/src/features/**
📄 CodeRabbit inference engine (web/default/AGENTS.md)
功能模块应放在
src/features/<feature>/，并按需包含components/、lib/、hooks/、api.ts、types.ts、constants.ts等；通用组件应放在src/components/，通用工具与类型应放在src/lib/。
Files:
web/default/src/features/auth/api.tsweb/default/src/features/auth/hooks/use-oauth-login.ts
🪛 ast-grep (0.45.0)
relay/channel/zhipu/relay_zhipu_test.go
[warning] 9-9: A credential is hard-coded as a string literal. Secrets stored in source code, such as passwords, API keys, and tokens, can be leaked through version control or binaries and used by internal or external malicious actors. Rotate the exposed secret and load it at runtime from a secure secret vault, a Hardware Security Module (HSM), or an environment variable if permitted by your company policy (e.g. password := os.Getenv("APP_PASSWORD")).
Context: apiKey = "test-id.test-secret"
Note: [CWE-798] Use of Hard-coded Credentials.
(hardcoded-credentials-string-literal-go)
🔇 Additional comments (70)
model/user.go (2)
867-920: LGTM!
928-952: 🗄️ Data Integrity & IntegrationDispatch
InsertWithTxcache invalidation after the commit.
InsertWithTxstores aCacheInvalidationTask, butcontroller/oauth.gonever callsdispatchStagedCacheInvalidationafter the outer transaction commits. The registered cache retry worker can eventually run this task, so cache invalidation is not lost.common/utils.go (1)
243-243: LGTM!Also applies to: 255-258
model/ability.go (1)
97-100: LGTM!Also applies to: 172-225
common/email.go (1)
24-28: LGTM!common/pprof.go (1)
12-12: LGTM!Also applies to: 22-50
common/pprof_test.go (1)
11-24: LGTM!common/random_failure_test.go (1)
12-46: LGTM!common/body_storage.go (1)
23-31: LGTM!Also applies to: 92-100, 250-262, 336-360
relay/channel/zhipu/relay_zhipu_test.go (1)
1-26: LGTM!relay/channel/baidu/dto.go (1)
15-26: LGTM!docs/openapi/api.json (1)
869-922: LGTM!relay/channel/ali/text.go (1)
11-22: LGTM!service/system_task.go (1)
7-7: LGTM!Also applies to: 101-106
relay/channel/vertex/service_account.go (1)
12-12: LGTM!Also applies to: 47-66, 134-143, 177-185
relay/channel/baidu/relay-baidu.go (1)
26-51: LGTM!Also applies to: 136-161, 164-189
main.go (1)
131-131: LGTM!Also applies to: 197-197, 275-284
controller/topup.go (1)
12-12: LGTM!Also applies to: 222-228
oauth/generic.go (1)
129-133: LGTM!Also applies to: 200-214, 260-261
oauth/http_client.go (1)
11-18: LGTM!relay/channel/coze/dto.go (1)
22-24: LGTM!relay/channel/vertex/service_account_test.go (1)
10-28: LGTM!relay/channel/ali/rerank.go (1)
7-7: LGTM!Also applies to: 43-43, 67-67
relay/channel/zhipu/adaptor.go (1)
53-56: LGTM!relay/channel/vertex/adaptor.go (1)
269-269: LGTM!relay/channel/siliconflow/adaptor.go (1)
52-56: LGTM!relay/channel/siliconflow/adaptor_test.go (1)
15-38: LGTM!relay/channel/siliconflow/dto.go (1)
20-31: LGTM!relay/channel/coze/relay-coze.go (4)
43-43: LGTM!
58-88: LGTM!
156-205: LGTM!
244-244: LGTM!model/main.go (1)
345-345: LGTM!model/twofa.go (1)
182-182: LGTM!Also applies to: 208-208, 307-307
controller/subscription_payment_epay.go (1)
79-85: LGTM!oauth/oidc.go (1)
72-75: LGTM!Also applies to: 118-121, 86-86, 137-137, 148-148
oauth/http_client_test.go (1)
19-41: LGTM!Also applies to: 43-98
controller/oauth_test.go (1)
31-44: LGTM!Also applies to: 46-71, 73-117, 119-127, 129-162, 201-201, 226-226, 250-250, 277-277
relay/channel/zhipu/relay-zhipu.go (2)
69-77: LGTM!Also applies to: 165-182, 223-232, 241-247, 258-269, 280-280, 291-291
31-45: 🗄️ Data Integrity & IntegrationNo Zhipu
getZhipuTokencall-site updates needed.Each production and test call receives the error value and either returns or asserts on it.
model/auth_flow.go (1)
80-86: LGTM!Also applies to: 88-109, 111-129, 131-148, 150-154
controller/oauth.go (1)
36-50: LGTM!Also applies to: 115-132, 137-169, 191-225, 236-254, 270-283, 314-314, 351-361, 397-397, 484-486
common/body_storage_replay_test.go (1)
10-53: LGTM!relay/responses_handler.go (1)
79-87: LGTM!relay/common/outbound_body_test.go (1)
15-38: LGTM!Also applies to: 40-189
router/api-router.go (1)
46-54: LGTM!Also applies to: 89-89, 109-109
router/api_router_test.go (1)
52-88: LGTM!Also applies to: 210-295
relay/common/outbound_body.go (1)
7-7: LGTM!Also applies to: 22-43
controller/user.go (1)
391-396: LGTM!Also applies to: 459-465
relay/common/pass_through_body.go (2)
100-221: LGTM!Also applies to: 291-337, 434-456, 458-510, 512-555
45-62: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winConsider failing open when the pass-through scan rejects the body.
Pass-through mode forwards the client body unchanged.
newPassThroughFilteredBodynow returns a scan error for any body this scanner cannot parse. Callers inrelay/responses_handler.go,relay/claude_handler.go, andrelay/compatible_handler.goconvert that error intoErrorCodeConvertRequestFailedwith skip-retry. A body that the upstream provider would accept, but this scanner rejects, therefore fails at the gateway instead of upstream.If the scan fails and no controlled field can be present, forwarding the raw body preserves the previous behavior. If you intend to reject unparsable bodies, keep the current behavior and confirm the error code and message are correct for clients.
🛡️ Optional fail-open variant
ranges, scannedSize, scanErr := scanPassThroughJSON(reader, settings) closeErr := reader.Close() if scanErr != nil { - return nil, scanErr + // Pass-through promises raw forwarding; an unparsable body cannot + // contain a controlled field we are able to locate. + basecommon.SysError("pass-through JSON scan failed, forwarding raw body: " + scanErr.Error()) + return nil, nil }> Likely an incorrect or invalid review comment.relay/channel/api_request_test.go (1)
132-144: LGTM!Also applies to: 146-175
relay/channel/ali/dto.go (1)
29-33: 🩺 Stability & AvailabilityNo nil-dereference issue here.
The relevant Ali pointer fields are either guarded before dereference or not dereferenced in the ali channel handlers shown.
relay/claude_handler.go (1)
158-166: LGTM!controller/codex_oauth.go (1)
167-173: LGTM!relay/channel/ali/image_wan.go (1)
19-21: LGTM!Also applies to: 36-40
relay/compatible_handler.go (1)
107-115: LGTM!main_test.go (1)
131-137: LGTM!web/default/src/features/auth/api.ts (1)
90-97: 📐 Maintainability & Code QualityRun the frontend type check.
The PR does not provide a TypeScript type-check result for the changed OAuth request contracts and callers.
web/default/src/features/auth/api.ts#L90-L97: validate the login state request contract.web/default/src/features/auth/hooks/use-oauth-login.ts#L96-L96: validate the GitHub caller.web/default/src/features/auth/hooks/use-oauth-login.ts#L127-L127: validate the Discord caller.web/default/src/features/auth/hooks/use-oauth-login.ts#L148-L148: validate the OIDC caller.web/default/src/features/auth/hooks/use-oauth-login.ts#L173-L173: validate the LinuxDO caller.web/default/src/features/auth/hooks/use-oauth-login.ts#L198-L198: validate the custom-provider caller.web/default/src/lib/oauth.ts#L151-L156: validate the binding state request contract.Source: Coding guidelines
relay/channel/api_request.go (1)
30-45: LGTM!Also applies to: 326-326, 356-356, 563-563
relay/channel/coze/relay_coze_test.go (1)
14-22: LGTM!relay/channel/siliconflow/relay-siliconflow.go (1)
7-7: LGTM!Also applies to: 23-23, 37-37
dto/values.go (1)
7-8: LGTM!Also applies to: 12-29, 34-41, 52-78
relay/channel/baidu/relay_baidu_test.go (1)
43-48: LGTM!common/str.go (1)
84-85: LGTM!relay/channel/ali/text_test.go (1)
14-83: LGTM!relay/common/override_test.go (1)
2001-2017: LGTM!Also applies to: 2035-2049
relay/common/relay_info.go (1)
868-868: LGTM!model/subscription.go (1)
787-796: LGTM!Also applies to: 839-839
dto/openai_request.go (1)
493-500: LGTM!Also applies to: 690-690, 715-722, 733-733, 749-749, 760-760
| func authFlowTokenHash(token string) string { | ||
| return common.GenerateHMACWithKey([]byte("auth-flow-v1:"+common.SessionSecret), token) | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Determine how SessionSecret is initialized and whether it persists.
rg -nP -C5 '\bSessionSecret\b' --type=go
# Description: Confirm the output encoding and length of GenerateHMACWithKey to validate the char(64) column.
ast-grep run --pattern 'func GenerateHMACWithKey($$$) $$$ { $$$ }' --lang goRepository: MAX-API-Next/MAX-API
Length of output: 158
🏁 Script executed:
#!/bin/bash
set -u
echo "== files =="
git ls-files | sed -n '1,120p'
echo "== SessionSecret occurrences case-insensitive (all files) =="
rg -n -i -C 4 'SessionSecret|sessionsecret|\.Secret|generatehmacwithkey|TokenHash|auth-flow-v1|auth_flow' . || true
echo "== Go files common auth_flow nearby =="
fd -t f -e go . | sort | sed -n '1,200p'Repository: MAX-API-Next/MAX-API
Length of output: 22425
Use a persisted key for auth_flow token hashing.
common.SessionSecret falls back to uuid.New().String() when SESSION_SECRET is unset, so process restarts or multi-instance deployments can compute different TokenHash values. That makes persisted AuthFlow rows unreadable and causes ErrAuthFlowInvalid on callbacks. Store the signing key in an environment variable or database and use it for authFlowTokenHash and session storage.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@model/auth_flow.go` around lines 62 - 64, Update authFlowTokenHash to use a
stable persisted signing key rather than common.SessionSecret, which may be
regenerated when SESSION_SECRET is unset. Source the key from the configured
environment variable or database-backed setting, and ensure the same key is used
by session storage so persisted AuthFlow tokens remain valid across restarts and
instances.
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
1 participant
Important
📝 变更描述 / Description
(简述：做了什么？为什么这样改能生效？请基于你对代码逻辑的理解来写，避免粘贴未经整理的内容)
🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
Bug fix，我已提交或关联对应 Issue，且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。📸 运行证明 / Proof of Work
(请在此粘贴截图、关键日志或测试报告，以证明变更生效)