feat(codex): 在渠道列表展示 ChatGPT plan type - #3247
Closed
BruceCheung666 wants to merge 5407 commits into
Closed
Conversation
将散落在多个文件中的预扣费/结算/退款逻辑抽象为统一的 BillingSession 生命周期管理： - 新增 BillingSettler 接口 (relay/common/billing.go) 避免循环引用 - 新增 FundingSource 接口 + WalletFunding / SubscriptionFunding 实现 (service/funding_source.go) - 新增 BillingSession 封装预扣/结算/退款原子操作 (service/billing_session.go) - 新增 SettleBilling 统一结算辅助函数，替换各 handler 中的 quotaDelta 模式 - 重写 PreConsumeBilling 为 BillingSession 工厂入口 - controller/relay.go 退款守卫改用 BillingSession.Refund() 修复的 Bug： - 令牌额度泄漏：PreConsumeTokenQuota 成功但 DecreaseUserQuota 失败时未回滚 - 订阅退款遗漏：FinalPreConsumedQuota=0 但 SubscriptionPreConsumed>0 时跳过退款 - 订阅多扣费：subConsume 强制为 1 但 FinalPreConsumedQuota 不同步 - 退款路径不统一：钱包/订阅退款逻辑现统一由 FundingSource.Refund 分派
- Settle 部分失败保护：新增 fundingSettled 标记，资金来源提交后 令牌调整失败不再导致 Refund 误退已结算的资金 - 订阅多扣费修复：trySubscription 传 subConsume 而非 preConsumedQuota 给 preConsume，保证三者（amount/preConsume/FinalPreConsumedQuota）一致 - 令牌回滚错误记录：preConsume 中 funding 失败时令牌回滚错误不再丢弃 - 移除钱包路径死代码：用户额度不足的 strings.Contains 匹配不可能命中 - WalletFunding.Refund 不重试：IncreaseUserQuota 非幂等，重试会多退
…e recharge card tabs - Defaulting to subscriptions when available and avoiding initial flash when no plans exist. - Adjust the wide-screen layout to place wallet and invite sections side by side, simplify the subscription header and controls, and add padding to prevent card borders from clipping. - Update related i18n strings by adding the new tab label and removing the obsolete subscription blurb.
…-when-no-plans ✨ refactor(wallet): Top-up layout to embed subscription plans into the recharge card tabs
refactor: 抽象统一计费会话 BillingSession
Add a lightweight active-subscription check to skip subscription pre-consume when none exist, reducing unnecessary transactions and locks. In the subscription UI, disable subscription-first options when no active plan is available, show the effective fallback to wallet with a clear notice, and distinguish “invalidated” from “expired” states. Update i18n strings across supported locales to reflect the new messages and status labels.
Aligns the error variable types in the subscription-first path so that quota fallback checks use the correct NewAPIError. This prevents build failures and preserves the intended wallet fallback when subscription pre-consume returns an insufficient quota error.
Routes quota alerts through a subscription-specific check when billing from subscriptions, preventing wallet-based thresholds from triggering false warnings. Updates the notification settings description and localization keys to clarify that both wallet and subscription balances are monitored.
🔔 feat: Add subscription-aware quota notifications and update UI copy
…-fallback ✨ chore: Improve subscription billing fallback and UI states
当上游为 AWS Bedrock 时，message_delta 的 usage 可能缺少 input_tokens、 cache_creation_input_tokens、cache_read_input_tokens 等字段，导致与原生 Anthropic 格式不一致。从 message_start 积累的 claudeInfo 中补全这些字段后 重新序列化，确保客户端收到一致的 usage 格式。
Modified the formatUserLogs function to include a startIdx parameter, allowing for more flexible log ID assignment. Updated calls to this function in GetLogByTokenId and GetUserLogs to pass the appropriate starting index.
feat: add Codex channel disclaimer (i18n, OpenAI terms)
feat: Force beta=true parameter for Anthropic channel
feat(oauth): implement custom OAuth provider
fix: Claude stream block index/type transitions
fix: add paragraph breaks between reasoning summary chunks
# Conflicts: # service/openaicompat/chat_to_responses.go
feat: channel test with stream=true
fix: 使用openai兼容接口调用部分渠道在最终端点为claude原生端点下还是走了openai扣减input_token的逻辑
fix: 补全 streaming message_delta 事件缺失的 input_tokens 和 cache 相关字段
feat: /v1/messages -> /v1/responses
…rable feat: make 5m cache-creation ratio configurable
fix: 如果模型管理有自定义配置则不合并默认配置
…eader-append feat：support $keep_only_declared and deduped $append for header override
chore: update model lists for frequently used channels
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds extraction, propagation, persistence, and UI display of Codex plan_type and ID token across OAuth/token exchange, credential refresh, channel create/update/display paths, and channel listing/search endpoints. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller as Channel Controller
participant OAuthSvc as Codex OAuth Service
participant Service as Credential Service
participant DB as Database
participant Codex as Codex Provider
Client->>Controller: AddChannel / UpdateChannel (Codex)
Controller->>OAuthSvc: Exchange code or refresh token
OAuthSvc->>Codex: Exchange/refresh request
Codex-->>OAuthSvc: AccessToken, IDToken, RefreshToken (may include plan_type)
OAuthSvc->>OAuthSvc: extractCodexPlanTypeFromTokens(IDToken, AccessToken)
OAuthSvc-->>Controller: CodexOAuthTokenResult (AccessToken, IDToken, RefreshToken, PlanType)
Controller->>Service: MergeCodexPlanTypeIntoOtherInfo & persist key
Service->>DB: Update `key` and `other_info` (atomic)
DB-->>Service: Update OK
Service-->>Controller: Updated Channel
Controller->>Controller: bestEffortSyncCodexPlanType / attachCodexPlanTypeForDisplay
Controller-->>Client: Channel response with plan_type included
sequenceDiagram
participant Client
participant Controller
participant DB
Client->>Controller: GetAllChannels / SearchChannels / GetChannel
Controller->>DB: Query channels (includes other_info)
DB-->>Controller: Channels list
Controller->>Controller: attachCodexPlanTypeForDisplay (batch parse/lookup)
Controller-->>Client: Channels with plan_type available for display
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
controller/codex_usage.go (1)
100-107: Consider logging the ignored DB error.Line 107 silently discards the error from
Updates(). While this is a secondary refresh path and maintains existing behavior, logging the error would help diagnose issues without disrupting the primary usage-fetch flow.💡 Optional: Log error instead of ignoring
- _ = model.DB.Model(&model.Channel{}).Where("id = ?", ch.Id).Updates(updates).Error + if dbErr := model.DB.Model(&model.Channel{}).Where("id = ?", ch.Id).Updates(updates).Error; dbErr != nil { + common.SysError("failed to update codex channel after refresh: " + dbErr.Error()) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/codex_usage.go` around lines 100 - 107, Capture the error returned by model.DB.Model(&model.Channel{}).Where("id = ?", ch.Id).Updates(updates).Error instead of discarding it and log it with contextual fields (channel id, the encoded key string and whether other_info was set) using the project's logger; e.g., assign to err, and if err != nil call the existing logger (or log package) to emit a descriptive message including ch.Id, updates["key"], and updates["other_info"] to aid debugging while preserving current behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@controller/codex_usage.go`:
- Around line 100-107: Capture the error returned by
model.DB.Model(&model.Channel{}).Where("id = ?", ch.Id).Updates(updates).Error
instead of discarding it and log it with contextual fields (channel id, the
encoded key string and whether other_info was set) using the project's logger;
e.g., assign to err, and if err != nil call the existing logger (or log package)
to emit a descriptive message including ch.Id, updates["key"], and
updates["other_info"] to aid debugging while preserving current behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b03bc80d-5729-4263-b903-50b2e6b16e48
📒 Files selected for processing (7)
controller/channel.gocontroller/codex_oauth.gocontroller/codex_usage.gorelay/channel/codex/oauth_key.goservice/codex_credential_refresh.goservice/codex_oauth.goweb/src/components/table/channels/ChannelsColumnDefs.jsx
Contributor
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)
relay/channel/codex/oauth_key.go (1)
24-27:⚠️ Potential issue | 🟡 MinorInconsistent empty-input check compared to
parseCodexOAuthKey.
ParseOAuthKeychecksraw == ""while the equivalentparseCodexOAuthKeyinservice/codex_credential_refresh.go(lines 35-37) usesstrings.TrimSpace(raw) == "". This means whitespace-only input like" "will pass the check here but fail in the other function, leading to inconsistent behavior.Proposed fix
func ParseOAuthKey(raw string) (*OAuthKey, error) { - if raw == "" { + if strings.TrimSpace(raw) == "" { return nil, errors.New("codex channel: empty oauth key") }You'll also need to add
"strings"to the imports.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@relay/channel/codex/oauth_key.go` around lines 24 - 27, The empty-input check in ParseOAuthKey is inconsistent with parseCodexOAuthKey: replace the raw == "" check in ParseOAuthKey with strings.TrimSpace(raw) == "" so whitespace-only strings are treated as empty, and add "strings" to the imports; update the error path in ParseOAuthKey accordingly to preserve the same error behavior as parseCodexOAuthKey.
🧹 Nitpick comments (6)
service/codex_oauth.go (2)
200-205: Same status-before-decode issue in code exchange.Same pattern as
refreshCodexOAuthToken: the status code check happens afterDecodeJson. Apply the same fix for consistency and robustness.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@service/codex_oauth.go` around lines 200 - 205, Move the HTTP status check to occur before calling common.DecodeJson: in the code exchange handler where resp and payload are used (same pattern as refreshCodexOAuthToken), first verify resp.StatusCode is 2xx and if not return an error that includes the status and response body content; only call common.DecodeJson(resp.Body, &payload) after confirming a success status. This keeps the check consistent with refreshCodexOAuthToken and avoids attempting to decode error responses.
134-139: Check status code before decoding response body.The status code check (lines 137-139) occurs after
common.DecodeJson(line 134). If the server returns an error status with a non-JSON body or different JSON structure, the decode may fail before you can provide a proper error message about the status. Consider checkingresp.StatusCodebefore attempting to decode.Proposed fix
defer resp.Body.Close() + if resp.StatusCode < 200 || resp.StatusCode >= 300 { + return nil, fmt.Errorf("codex oauth refresh failed: status=%d", resp.StatusCode) + } + var payload struct { AccessToken string `json:"access_token"` RefreshToken string `json:"refresh_token"` IDToken string `json:"id_token"` ExpiresIn int `json:"expires_in"` } if err := common.DecodeJson(resp.Body, &payload); err != nil { return nil, err } - if resp.StatusCode < 200 || resp.StatusCode >= 300 { - return nil, fmt.Errorf("codex oauth refresh failed: status=%d", resp.StatusCode) - }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@service/codex_oauth.go` around lines 134 - 139, Move the HTTP status check to occur immediately after receiving the response and before calling common.DecodeJson: check resp.StatusCode and return a formatted error (including status and optionally the raw body or an extracted error message) if it's not 2xx; only call common.DecodeJson to unmarshal into payload when the status is OK. Update the logic around the existing variables and calls (resp.StatusCode, common.DecodeJson, payload) inside the function handling the token refresh so that non-2xx responses are handled early and decoding is skipped for error responses.service/codex_credential_refresh.go (2)
110-112: Consider logging merge errors for debugging.The merge error is silently ignored. While this is acceptable for best-effort sync, logging the error would help diagnose issues when plan types aren't appearing in the UI.
Proposed enhancement
- if otherInfo, err := MergeCodexPlanTypeIntoOtherInfo(ch.OtherInfo, oauthKey.PlanType); err == nil { + if otherInfo, err := MergeCodexPlanTypeIntoOtherInfo(ch.OtherInfo, oauthKey.PlanType); err != nil { + common.SysError(fmt.Sprintf("failed to merge codex plan type for channel %d: %v", ch.Id, err)) + } else { ch.OtherInfo = otherInfo }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@service/codex_credential_refresh.go` around lines 110 - 112, The MergeCodexPlanTypeIntoOtherInfo call currently swallows errors; update the conditional to log the error when MergeCodexPlanTypeIntoOtherInfo(ch.OtherInfo, oauthKey.PlanType) returns a non-nil err so failures are visible for debugging. Specifically, after calling MergeCodexPlanTypeIntoOtherInfo (the call that returns otherInfo and err), set ch.OtherInfo = otherInfo on success as now, and on error call the appropriate logger (e.g., processLogger or the surrounding function's logger) to record a clear message including the error and context (ch.OtherInfo and oauthKey.PlanType) so plan-merge failures are traceable.
99-103: Fallback extraction appears redundant.At line 59,
oauthKeyis already parsed fromch.Key. IfoauthKey.PlanTypeis empty after that parse (line 99), callingExtractCodexPlanTypeFromOAuthKey(ch.Key)will parse the same JSON again and extract from the same (empty)plan_typefield or attempt JWT extraction from the same tokens already inoauthKey.The JWT extraction fallback in
ExtractCodexPlanTypeFromOAuthKeywould useoauthKey.IDTokenandoauthKey.AccessToken- which are the old tokens before refresh. After refresh (lines 75-84), the new tokens are already inoauthKey, andres.PlanType(derived from those new tokens) was already applied at line 83.This fallback only adds value if the refresh result didn't include a plan type but the old key had one persisted. Consider clarifying the intent or removing if unnecessary.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@service/codex_credential_refresh.go` around lines 99 - 103, The fallback call to ExtractCodexPlanTypeFromOAuthKey(ch.Key) is redundant because oauthKey was already parsed from ch.Key and res.PlanType (from the refreshed tokens) was applied to oauthKey earlier; remove this block or replace it to read the plan type from the original parsed object captured before refresh (e.g., keep an origOAuthKey variable parsed from ch.Key prior to token refresh and use origOAuthKey.PlanType) so you don't re-parse ch.Key or re-run JWT extraction against stale tokens; ensure any remaining logic prefers res.PlanType (already applied to oauthKey) and only uses the persisted original plan type when the refresh result lacks it.controller/codex_usage.go (1)
101-111: Consider handling the database update error.The DB update error at line 108 is silently ignored. While this is a best-effort refresh during a usage fetch, a failed update means the refreshed token won't be persisted, leading to repeated refresh attempts on subsequent requests.
At minimum, consider logging the error for debugging.
Proposed enhancement
- _ = model.DB.Model(&model.Channel{}).Where("id = ?", ch.Id).Updates(updates).Error + if dbErr := model.DB.Model(&model.Channel{}).Where("id = ?", ch.Id).Updates(updates).Error; dbErr != nil { + common.SysError(fmt.Sprintf("failed to persist refreshed codex token for channel %d: %v", ch.Id, dbErr)) + } model.InitChannelCache()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/codex_usage.go` around lines 101 - 111, The DB update call using model.DB.Model(&model.Channel{}).Where("id = ?", ch.Id).Updates(updates).Error currently ignores errors; change it to capture the returned error (e.g. err := ... .Error), and if err != nil log the failure (including ch.Id and the error) so failed refreshes are visible; still proceed with model.InitChannelCache() and service.ResetProxyClientCache() after logging to preserve best-effort behavior. Ensure you reference the same symbols (updates map, ch.OtherInfo, model.InitChannelCache, service.ResetProxyClientCache) when adding the error handling and logging.controller/channel.go (1)
88-123: Display-time backfill doesn't persist plan type.This function updates
channel.OtherInfoin memory for display purposes but doesn't persist the changes to the database. This means every channel list request will re-execute the backfill logic for channels with missing plan types.If this is intentional (to avoid write operations on read paths), it's fine. However, consider whether persisting the backfilled plan type would be beneficial to avoid repeated key parsing and JWT decoding on each request.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/channel.go` around lines 88 - 123, attachCodexPlanTypeForDisplay currently updates channel.OtherInfo only in-memory for display; persist the backfilled plan type so you don't re-run parsing/decoding on every list call: after calling bestEffortSyncCodexPlanType(targets[row.Id], row.Key) in attachCodexPlanTypeForDisplay, check whether targets[row.Id].OtherInfo was changed and then persist it using model.DB (e.g., update the model.Channel row by id via model.DB.Model(&model.Channel{}).Where("id = ?", targets[row.Id].Id).Update("other_info", targets[row.Id].OtherInfo)), and log/handle any DB error; keep references to attachCodexPlanTypeForDisplay, bestEffortSyncCodexPlanType, model.Channel, channel.OtherInfo, and model.DB to locate the code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@relay/channel/codex/oauth_key.go`:
- Around line 24-27: The empty-input check in ParseOAuthKey is inconsistent with
parseCodexOAuthKey: replace the raw == "" check in ParseOAuthKey with
strings.TrimSpace(raw) == "" so whitespace-only strings are treated as empty,
and add "strings" to the imports; update the error path in ParseOAuthKey
accordingly to preserve the same error behavior as parseCodexOAuthKey.
---
Nitpick comments:
In `@controller/channel.go`:
- Around line 88-123: attachCodexPlanTypeForDisplay currently updates
channel.OtherInfo only in-memory for display; persist the backfilled plan type
so you don't re-run parsing/decoding on every list call: after calling
bestEffortSyncCodexPlanType(targets[row.Id], row.Key) in
attachCodexPlanTypeForDisplay, check whether targets[row.Id].OtherInfo was
changed and then persist it using model.DB (e.g., update the model.Channel row
by id via model.DB.Model(&model.Channel{}).Where("id = ?",
targets[row.Id].Id).Update("other_info", targets[row.Id].OtherInfo)), and
log/handle any DB error; keep references to attachCodexPlanTypeForDisplay,
bestEffortSyncCodexPlanType, model.Channel, channel.OtherInfo, and model.DB to
locate the code.
In `@controller/codex_usage.go`:
- Around line 101-111: The DB update call using
model.DB.Model(&model.Channel{}).Where("id = ?", ch.Id).Updates(updates).Error
currently ignores errors; change it to capture the returned error (e.g. err :=
... .Error), and if err != nil log the failure (including ch.Id and the error)
so failed refreshes are visible; still proceed with model.InitChannelCache() and
service.ResetProxyClientCache() after logging to preserve best-effort behavior.
Ensure you reference the same symbols (updates map, ch.OtherInfo,
model.InitChannelCache, service.ResetProxyClientCache) when adding the error
handling and logging.
In `@service/codex_credential_refresh.go`:
- Around line 110-112: The MergeCodexPlanTypeIntoOtherInfo call currently
swallows errors; update the conditional to log the error when
MergeCodexPlanTypeIntoOtherInfo(ch.OtherInfo, oauthKey.PlanType) returns a
non-nil err so failures are visible for debugging. Specifically, after calling
MergeCodexPlanTypeIntoOtherInfo (the call that returns otherInfo and err), set
ch.OtherInfo = otherInfo on success as now, and on error call the appropriate
logger (e.g., processLogger or the surrounding function's logger) to record a
clear message including the error and context (ch.OtherInfo and
oauthKey.PlanType) so plan-merge failures are traceable.
- Around line 99-103: The fallback call to
ExtractCodexPlanTypeFromOAuthKey(ch.Key) is redundant because oauthKey was
already parsed from ch.Key and res.PlanType (from the refreshed tokens) was
applied to oauthKey earlier; remove this block or replace it to read the plan
type from the original parsed object captured before refresh (e.g., keep an
origOAuthKey variable parsed from ch.Key prior to token refresh and use
origOAuthKey.PlanType) so you don't re-parse ch.Key or re-run JWT extraction
against stale tokens; ensure any remaining logic prefers res.PlanType (already
applied to oauthKey) and only uses the persisted original plan type when the
refresh result lacks it.
In `@service/codex_oauth.go`:
- Around line 200-205: Move the HTTP status check to occur before calling
common.DecodeJson: in the code exchange handler where resp and payload are used
(same pattern as refreshCodexOAuthToken), first verify resp.StatusCode is 2xx
and if not return an error that includes the status and response body content;
only call common.DecodeJson(resp.Body, &payload) after confirming a success
status. This keeps the check consistent with refreshCodexOAuthToken and avoids
attempting to decode error responses.
- Around line 134-139: Move the HTTP status check to occur immediately after
receiving the response and before calling common.DecodeJson: check
resp.StatusCode and return a formatted error (including status and optionally
the raw body or an extracted error message) if it's not 2xx; only call
common.DecodeJson to unmarshal into payload when the status is OK. Update the
logic around the existing variables and calls (resp.StatusCode,
common.DecodeJson, payload) inside the function handling the token refresh so
that non-2xx responses are handled early and decoding is skipped for error
responses.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7146ba9e-471c-4011-b903-2f4557278b8f
📒 Files selected for processing (7)
controller/channel.gocontroller/codex_oauth.gocontroller/codex_usage.gorelay/channel/codex/oauth_key.goservice/codex_credential_refresh.goservice/codex_oauth.goweb/src/components/table/channels/ChannelsColumnDefs.jsx
🚧 Files skipped from review as they are similar to previous changes (1)
- controller/codex_oauth.go
…tion links and submission checks
Round remaining balance
enhance channel key viewing
Extract Codex OAuth plan_type from OpenAI JWTs during code exchange and token refresh, then persist it to both the OAuth key JSON and channel other_info for safe list rendering without exposing channel keys. Also backfill the plan type for existing Codex channels when loading channel records, and render a Plus/Team/Pro/Free badge after the type tag in the channel management table.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
controller/channel.go (1)
88-123: Persist the backfill instead of recomputing it on every read.
attachCodexPlanTypeForDisplayonly annotates the response structs. Any Codex row still missingcodex_plan_typewill keep triggering the extraSELECT id, keyplus JSON/JWT parsing on every list/search/get request becauseother_infoin the database never gets repaired. Consider writing the mergedother_infoback once, or doing a one-off backfill, so the read path can stay key-free after the first hit.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@service/codex_oauth.go`:
- Around line 356-363: MergeCodexPlanTypeIntoOtherInfo currently returns an
error on malformed or non-object otherInfo which prevents persisting/backfilling
codex_plan_type; change it so that if common.Unmarshal fails or the decoded
value is not a map (payload), treat otherInfo as empty by initializing payload =
map[string]any{} and continue without returning an error, then set
payload["codex_plan_type"] = normalizedPlanType only when normalizedPlanType is
non-empty and the key is not already present, and finally marshal the payload
(using common.Marshal or existing serializer) and return the resulting string;
reference MergeCodexPlanTypeIntoOtherInfo, normalizeCodexPlanType, payload,
common.Unmarshal, and common.Marshal in your changes.
In `@web/src/components/table/channels/ChannelsColumnDefs.jsx`:
- Around line 68-85: getCodexPlanTypeLabel currently returns hard-coded English
labels ("Plus","Team","Pro","Free"); update it to return localized strings by
routing labels through the i18n translator: accept a translation function (e.g.,
parameter t) or return keys and document consumer to call t; modify the function
signature getCodexPlanTypeLabel(planType, t) (or getCodexPlanTypeLabel(planType)
to return keys like 'plan.plus') and replace the literal returns with
t('plan.plus')/t('plan.team')/t('plan.pro')/t('plan.free') (or the corresponding
keys), and update callers in ChannelsColumnDefs.jsx to pass useTranslation().t
so the UI uses locale JSON-managed strings.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0a7b51ff-a389-4cc8-aaa9-bd3626582cf3
📒 Files selected for processing (7)
controller/channel.gocontroller/codex_oauth.gocontroller/codex_usage.gorelay/channel/codex/oauth_key.goservice/codex_credential_refresh.goservice/codex_oauth.goweb/src/components/table/channels/ChannelsColumnDefs.jsx
🚧 Files skipped from review as they are similar to previous changes (2)
- controller/codex_usage.go
- relay/channel/codex/oauth_key.go
Comment on lines
+356
to
+363
| func MergeCodexPlanTypeIntoOtherInfo(otherInfo string, planType string) (string, error) { | ||
| normalizedPlanType := normalizeCodexPlanType(planType) | ||
| trimmed := strings.TrimSpace(otherInfo) | ||
| payload := make(map[string]any) | ||
| if trimmed != "" { | ||
| if err := common.Unmarshal([]byte(trimmed), &payload); err != nil { | ||
| return "", err | ||
| } |
Contributor
There was a problem hiding this comment.
Don't let malformed other_info block plan-type sync.
A non-object/invalid other_info currently makes this helper return an error, and the new callers only log or ignore that failure. In that case the channel keeps working, but codex_plan_type is never persisted/backfilled even when the key already contains it.
💡 Possible fix
if trimmed != "" {
- if err := common.Unmarshal([]byte(trimmed), &payload); err != nil {
- return "", err
- }
+ if err := common.Unmarshal([]byte(trimmed), &payload); err != nil {
+ payload = make(map[string]any)
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| func MergeCodexPlanTypeIntoOtherInfo(otherInfo string, planType string) (string, error) { | |
| normalizedPlanType := normalizeCodexPlanType(planType) | |
| trimmed := strings.TrimSpace(otherInfo) | |
| payload := make(map[string]any) | |
| if trimmed != "" { | |
| if err := common.Unmarshal([]byte(trimmed), &payload); err != nil { | |
| return "", err | |
| } | |
| func MergeCodexPlanTypeIntoOtherInfo(otherInfo string, planType string) (string, error) { | |
| normalizedPlanType := normalizeCodexPlanType(planType) | |
| trimmed := strings.TrimSpace(otherInfo) | |
| payload := make(map[string]any) | |
| if trimmed != "" { | |
| if err := common.Unmarshal([]byte(trimmed), &payload); err != nil { | |
| payload = make(map[string]any) | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@service/codex_oauth.go` around lines 356 - 363,
MergeCodexPlanTypeIntoOtherInfo currently returns an error on malformed or
non-object otherInfo which prevents persisting/backfilling codex_plan_type;
change it so that if common.Unmarshal fails or the decoded value is not a map
(payload), treat otherInfo as empty by initializing payload = map[string]any{}
and continue without returning an error, then set payload["codex_plan_type"] =
normalizedPlanType only when normalizedPlanType is non-empty and the key is not
already present, and finally marshal the payload (using common.Marshal or
existing serializer) and return the resulting string; reference
MergeCodexPlanTypeIntoOtherInfo, normalizeCodexPlanType, payload,
common.Unmarshal, and common.Marshal in your changes.
Comment on lines
+68
to
+85
| // getCodexPlanTypeLabel normalizes Codex plan types into stable UI labels. | ||
| const getCodexPlanTypeLabel = (planType) => { | ||
| const normalized = String(planType || '') | ||
| .trim() | ||
| .toLowerCase(); | ||
| switch (normalized) { | ||
| case 'plus': | ||
| return 'Plus'; | ||
| case 'team': | ||
| return 'Team'; | ||
| case 'chatgptpro': | ||
| case 'pro': | ||
| return 'Pro'; | ||
| case 'free': | ||
| return 'Free'; | ||
| default: | ||
| return String(planType || '').trim(); | ||
| } |
Contributor
There was a problem hiding this comment.
Localize the new plan badges.
Plus/Team/Pro/Free are hard-coded here, so non-English UIs will always show English and the labels cannot be managed from locale JSON. Please route these labels through t(...) instead (pass t into this helper if needed).
As per coding guidelines web/src/**/*.{ts,tsx,js,jsx}: "Use useTranslation() hook and call t('中文key') in components."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/components/table/channels/ChannelsColumnDefs.jsx` around lines 68 -
85, getCodexPlanTypeLabel currently returns hard-coded English labels
("Plus","Team","Pro","Free"); update it to return localized strings by routing
labels through the i18n translator: accept a translation function (e.g.,
parameter t) or return keys and document consumer to call t; modify the function
signature getCodexPlanTypeLabel(planType, t) (or getCodexPlanTypeLabel(planType)
to return keys like 'plan.plus') and replace the literal returns with
t('plan.plus')/t('plan.team')/t('plan.pro')/t('plan.free') (or the corresponding
keys), and update callers in ChannelsColumnDefs.jsx to pass useTranslation().t
so the UI uses locale JSON-managed strings.
BruceCheung666
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
18 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Extract Codex OAuth plan_type from OpenAI JWTs during code exchange and token refresh, then persist it to both the OAuth key JSON and channel other_info for safe list rendering without exposing channel keys.
Also backfill the plan type for existing Codex channels when loading channel records, and render a Plus/Team/Pro/Free badge after the type tag in the channel management table.
Summary by CodeRabbit