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 (3)
📜 Recent review details🧰 Additional context used📓 Path-based instructions (1)**/*.go📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🔇 Additional comments (3)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR updates Redis cache fencing, quota persistence and accounting, OAuth error handling, billing settlement, request validation, expression evaluation, configuration loading, and frontend interaction flows. ChangesBackend and frontend behavior updates
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 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 |
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
`@web/default/src/features/system-settings/models/model-ratio-visual-editor.tsx`:
- Line 232: Add an explicit return type annotation to the getModelRowSignature
function, using the actual type of the value it returns and preserving its
current behavior.
- Around line 557-576: Replace the nested ternary used for billingMode in the
setEditData block with an explicitly typed variable using the exact allowed
billing-mode literals, and assign it through straightforward if-else logic:
tiered_expr for that billing mode, per-request when a price is present,
otherwise per-token. Use the variable in the existing setEditData call and leave
the surrounding editableModel mapping unchanged.
In `@web/default/src/i18n/locales/ru.json`:
- Line 67: Update the "{{target}} test failed" translation in the Russian locale
to explicitly indicate test failure, using wording such as "Тест {{target}} не
пройден" or "Тест {{target}} завершился с ошибкой" instead of language meaning
not performed or completed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: cb798633-29c4-418b-aeaf-6d7842260e8e
📒 Files selected for processing (12)
web/default/src/features/channels/components/data-table-row-actions.tsxweb/default/src/features/channels/components/dialogs/channel-test-dialog.tsxweb/default/src/features/channels/lib/channel-actions.test.tsweb/default/src/features/channels/lib/channel-actions.tsweb/default/src/features/channels/types.tsweb/default/src/features/system-settings/models/model-ratio-visual-editor.tsxweb/default/src/i18n/locales/en.jsonweb/default/src/i18n/locales/fr.jsonweb/default/src/i18n/locales/ja.jsonweb/default/src/i18n/locales/ru.jsonweb/default/src/i18n/locales/vi.jsonweb/default/src/i18n/locales/zh.json
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Backend checks
🧰 Additional context used
📓 Path-based instructions (4)
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/channels/types.tsweb/default/src/features/channels/lib/channel-actions.test.tsweb/default/src/features/channels/components/data-table-row-actions.tsxweb/default/src/features/system-settings/models/model-ratio-visual-editor.tsxweb/default/src/features/channels/lib/channel-actions.tsweb/default/src/features/channels/components/dialogs/channel-test-dialog.tsx
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/channels/types.tsweb/default/src/features/channels/lib/channel-actions.test.tsweb/default/src/features/channels/components/data-table-row-actions.tsxweb/default/src/features/system-settings/models/model-ratio-visual-editor.tsxweb/default/src/features/channels/lib/channel-actions.tsweb/default/src/features/channels/components/dialogs/channel-test-dialog.tsx
web/default/src/features/**/lib/**/*.ts
📄 CodeRabbit inference engine (web/default/AGENTS.md)
表单应使用 React Hook Form + Zod：在功能模块的
lib/下定义 schema，并用z.infer导出表单类型；useForm应配合@hookform/resolvers/zod进行校验。
Files:
web/default/src/features/channels/lib/channel-actions.test.tsweb/default/src/features/channels/lib/channel-actions.ts
web/default/src/**/*.test.ts
📄 CodeRabbit inference engine (web/default/AGENTS.md)
工具函数与纯逻辑应优先编写单元测试；测试文件应命名为
*.test.ts。
Files:
web/default/src/features/channels/lib/channel-actions.test.ts
🔇 Additional comments (12)
web/default/src/features/channels/lib/channel-actions.ts (2)
60-183: LGTM!
342-411: LGTM!web/default/src/features/channels/types.ts (1)
191-191: LGTM!web/default/src/features/channels/components/data-table-row-actions.tsx (1)
56-62: LGTM!Also applies to: 94-112
web/default/src/features/channels/components/dialogs/channel-test-dialog.tsx (1)
348-418: LGTM!Also applies to: 438-506
web/default/src/i18n/locales/en.json (1)
67-68: LGTM!Also applies to: 84-86, 801-802, 2701-2701, 3895-3895
web/default/src/i18n/locales/fr.json (1)
67-68: LGTM!Also applies to: 84-86, 801-802, 2701-2701, 3895-3895
web/default/src/features/channels/lib/channel-actions.test.ts (1)
19-25: 📐 Maintainability & Code QualityWire
node:testinto the workspace test command
I couldn’t find atestscript or workflow that runs these suites, so thesenode:testfiles may not execute automatically. Add a test command/CI job for them.web/default/src/i18n/locales/ja.json (1)
14-15: LGTM!Also applies to: 67-68, 84-86, 801-802, 2701-2701, 3895-3895
web/default/src/i18n/locales/ru.json (1)
68-68: LGTM!Also applies to: 84-86, 801-802, 2701-2701, 3895-3895
web/default/src/i18n/locales/vi.json (1)
14-15: LGTM!Also applies to: 67-68, 84-86, 801-802, 2701-2701, 3895-3895
web/default/src/i18n/locales/zh.json (1)
14-15: LGTM!Also applies to: 67-68, 84-86, 801-802, 2701-2701, 3895-3895
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
web/default/src/features/channels/components/dialogs/channel-test-dialog.tsx (1)
361-408: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winIgnore results from tests belonging to a closed or replaced dialog.
Closing the dialog only resets state; active requests continue and can update
testResults, progress, and selection after another channel is opened. Add an operation-generation guard/abort signal, or prevent closing until testing finishes.Also applies to: 421-514
🤖 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 `@web/default/src/features/channels/components/dialogs/channel-test-dialog.tsx` around lines 361 - 408, Update the dialog test lifecycle around testSingleModel and the related test handlers to invalidate or abort in-flight operations when the dialog closes or the tested channel is replaced. Guard all asynchronous callbacks and completion updates, including testResults, progress, selection, and refreshChannelLists, so results from stale generations are ignored while current-dialog tests continue to work.setting/config/config.go (1)
203-269: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winConfiguration updates are not atomic. A later validation error leaves earlier fields modified despite
LoadFromDBreturning failure.
setting/config/config.go#L203-L269: apply changes to a copied struct and publish it only after all fields validate.setting/config/config_test.go#L101-L113: add a valid earlier field plus an invalid later field and assert the whole config remains unchanged.🤖 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 `@setting/config/config.go` around lines 203 - 269, Make LoadFromDB apply all decoded field updates to a copy of the configuration, using the existing reflection switch for validation, and publish the copied struct only after every field succeeds; leave the original unchanged on any error. In setting/config/config_test.go:101-113, add a valid earlier update followed by an invalid later value and assert that LoadFromDB fails and the entire configuration, including the earlier field, remains unchanged.
🤖 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/redis.go`:
- Around line 256-265: Add lifecycle cleanup for persistent version keys used by
RedisInvalidateVersionedHash, deleting the corresponding versionKey whenever the
associated user or token entity is removed. Do not apply a normal TTL to
versionKey, since generation resets could permit stale cache refills, and ensure
the cleanup path targets both the cached hash key and its version key.
In `@model/checkin.go`:
- Around line 116-117: Update the post-transaction cache invalidation call in
the check-in flow to handle failures from invalidateUserQuotaCache(userId)
instead of discarding them. After the database commit, log the invalidation
error and enqueue a retry, without returning the error to the caller or
triggering another check-in attempt.
In `@model/token_cache.go`:
- Around line 31-32: Update RedisInvalidateVersionedHash, used by
invalidateTokenCache, to assign an expiration to each incremented cache-version
key; choose a TTL that exceeds the asynchronous refill/cache-fence window and
apply it consistently in the shared helper so invalidated token version keys can
expire.
In `@model/usedata.go`:
- Around line 117-128: The quota snapshot retry flow around saveQuotaData must
persist a stable idempotency identifier and transactionally deduplicate
snapshots before applying counters, preventing ambiguous commits from
incrementing totals twice; add the corresponding cross-database constraint and
migration compatible with SQLite, MySQL 5.7.8+, and PostgreSQL 9.6+. In
model/usedata.go lines 117-128, update the failed-snapshot requeue and save path
to retain and reuse this identifier. In model/usedata_test.go lines 12-48, add
coverage for an ambiguous commit followed by a repeated snapshot and assert
counters are applied exactly once.
In `@model/user_cache.go`:
- Around line 71-82: Update the direct HSET mutation paths for user group,
status, role, name, and setting caches to participate in the same version fence
as populateUserCacheIfVersion. Reuse the existing versioned cache helper and
appropriate user cache version key, or explicitly advance that generation on
every mutation, ensuring stale DB refills cannot overwrite newer auth-sensitive
updates.
In `@model/user.go`:
- Line 1274: Update the OIDC identity existence check in the surrounding user
lookup to return true when RowsAffected is greater than zero, so any matching
record is treated as already taken rather than requiring exactly one row.
In `@service/billing_session.go`:
- Around line 92-107: Update the compensation handling in the billing settlement
flow so a partial result from funding.Settle records the residual committed
funding amount instead of retaining the original appliedFundingDelta. Before a
later token-only retry, reconcile or reapply that missing funding portion before
settling the token, while preserving compensationFailed state and existing
full-compensation behavior. Add a test covering a nonzero partial compensation
result and verifying the retry uses the corrected funding amount.
In `@service/pre_consume_quota.go`:
- Around line 76-79: Update the rollback handling around IncreaseTokenQuota in
the pre-consume failure path so a failed compensation is not only logged: make
the token deduction and quota restoration transactional, or persist a durable
retryable compensation record containing the token and amount details. Ensure
the failed request cannot leave the user’s token balance permanently reduced
without a recovery path.
---
Outside diff comments:
In `@setting/config/config.go`:
- Around line 203-269: Make LoadFromDB apply all decoded field updates to a copy
of the configuration, using the existing reflection switch for validation, and
publish the copied struct only after every field succeeds; leave the original
unchanged on any error. In setting/config/config_test.go:101-113, add a valid
earlier update followed by an invalid later value and assert that LoadFromDB
fails and the entire configuration, including the earlier field, remains
unchanged.
In
`@web/default/src/features/channels/components/dialogs/channel-test-dialog.tsx`:
- Around line 361-408: Update the dialog test lifecycle around testSingleModel
and the related test handlers to invalidate or abort in-flight operations when
the dialog closes or the tested channel is replaced. Guard all asynchronous
callbacks and completion updates, including testResults, progress, selection,
and refreshChannelLists, so results from stale generations are ignored while
current-dialog tests continue to work.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 9bbc065d-de7f-420b-b5ea-3e15b313d12d
⛔ Files ignored due to path filters (2)
go.sumis excluded by!**/*.sumweb/bun.lockis excluded by!**/*.lock
📒 Files selected for processing (77)
common/redis.gocontroller/discord.gocontroller/github.gocontroller/linuxdo.gocontroller/oauth.gocontroller/oauth_test.gocontroller/oidc.gocontroller/telegram.gocontroller/token.gocontroller/wechat.gogo.modmiddleware/turnstile-check.gomiddleware/turnstile_check_test.gomodel/checkin.gomodel/errors.gomodel/main.gomodel/main_migration_test.gomodel/redemption.gomodel/redemption_search_test.gomodel/subscription.gomodel/token.gomodel/token_cache.gomodel/usedata.gomodel/usedata_test.gomodel/user.gomodel/user_cache.gomodel/user_oauth_binding.gomodel/user_update_test.gomodel/utils.gopkg/billingexpr/compile.gopkg/billingexpr/expr.mdpkg/billingexpr/run.gopkg/billingexpr/security_cache_test.goservice/billing_session.goservice/billing_session_test.goservice/funding_source.goservice/pre_consume_quota.goservice/quota.goservice/task_billing_test.gosetting/config/config.gosetting/config/config_test.gotools/jsonwrapcheck/allowlist.txtweb/default/package.jsonweb/default/src/components/layout/components/chat-presets-item.tsxweb/default/src/components/layout/components/footer.tsxweb/default/src/features/auth/api.tsweb/default/src/features/auth/lib/turnstile-request.test.tsweb/default/src/features/auth/lib/turnstile-request.tsweb/default/src/features/channels/components/channels-columns.tsxweb/default/src/features/channels/components/dialogs/channel-test-dialog.tsxweb/default/src/features/channels/lib/channel-actions.test.tsweb/default/src/features/channels/lib/channel-actions.tsweb/default/src/features/keys/components/data-table-row-actions.tsxweb/default/src/features/keys/components/dialogs/cc-switch-dialog.tsxweb/default/src/features/models/components/dialogs/view-details-dialog.tsxweb/default/src/features/playground/components/message-error.tsxweb/default/src/features/playground/hooks/use-stream-request.test.tsweb/default/src/features/playground/hooks/use-stream-request.tsweb/default/src/features/pricing/lib/tier-expr.test.tsweb/default/src/features/pricing/lib/tier-expr.tsweb/default/src/features/profile/api.tsweb/default/src/features/subscriptions/components/dialogs/subscription-purchase-dialog.tsxweb/default/src/features/system-settings/integrations/ionet-deployment-settings-section.tsxweb/default/src/features/system-settings/models/model-ratio-visual-editor.tsxweb/default/src/features/system-settings/models/tiered-pricing-support.tsxweb/default/src/features/usage-logs/components/dialogs/audio-preview-dialog.tsxweb/default/src/features/wallet/hooks/use-creem-payment.tsweb/default/src/features/wallet/hooks/use-payment.tsweb/default/src/features/wallet/hooks/use-waffo-payment.tsweb/default/src/i18n/locales/ru.jsonweb/default/src/lib/handle-server-error.test.tsweb/default/src/lib/handle-server-error.tsweb/default/src/lib/oauth.tsweb/default/src/lib/safe-redirect.test.tsweb/default/src/lib/safe-redirect.tsweb/default/src/routes/__root.tsxweb/default/src/routes/oauth/$provider.tsx
💤 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 (8)
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/wallet/hooks/use-waffo-payment.tsweb/default/src/components/layout/components/chat-presets-item.tsxweb/default/src/features/keys/components/dialogs/cc-switch-dialog.tsxweb/default/src/features/playground/hooks/use-stream-request.test.tsweb/default/src/features/wallet/hooks/use-creem-payment.tsweb/default/src/lib/safe-redirect.test.tsweb/default/src/features/playground/components/message-error.tsxweb/default/src/features/auth/lib/turnstile-request.test.tsweb/default/src/features/keys/components/data-table-row-actions.tsxweb/default/src/lib/handle-server-error.test.tsweb/default/src/features/system-settings/integrations/ionet-deployment-settings-section.tsxweb/default/src/features/pricing/lib/tier-expr.test.tsweb/default/src/features/auth/lib/turnstile-request.tsweb/default/src/lib/safe-redirect.tsweb/default/src/features/channels/components/channels-columns.tsxweb/default/src/features/models/components/dialogs/view-details-dialog.tsxweb/default/src/features/wallet/hooks/use-payment.tsweb/default/src/features/usage-logs/components/dialogs/audio-preview-dialog.tsxweb/default/src/features/system-settings/models/tiered-pricing-support.tsxweb/default/src/components/layout/components/footer.tsxweb/default/src/features/subscriptions/components/dialogs/subscription-purchase-dialog.tsxweb/default/src/features/channels/lib/channel-actions.test.tsweb/default/src/features/profile/api.tsweb/default/src/lib/handle-server-error.tsweb/default/src/features/auth/api.tsweb/default/src/lib/oauth.tsweb/default/src/routes/oauth/$provider.tsxweb/default/src/routes/__root.tsxweb/default/src/features/playground/hooks/use-stream-request.tsweb/default/src/features/pricing/lib/tier-expr.tsweb/default/src/features/system-settings/models/model-ratio-visual-editor.tsxweb/default/src/features/channels/lib/channel-actions.tsweb/default/src/features/channels/components/dialogs/channel-test-dialog.tsx
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/wallet/hooks/use-waffo-payment.tsweb/default/src/features/keys/components/dialogs/cc-switch-dialog.tsxweb/default/src/features/playground/hooks/use-stream-request.test.tsweb/default/src/features/wallet/hooks/use-creem-payment.tsweb/default/src/features/playground/components/message-error.tsxweb/default/src/features/auth/lib/turnstile-request.test.tsweb/default/src/features/keys/components/data-table-row-actions.tsxweb/default/src/features/system-settings/integrations/ionet-deployment-settings-section.tsxweb/default/src/features/pricing/lib/tier-expr.test.tsweb/default/src/features/auth/lib/turnstile-request.tsweb/default/src/features/channels/components/channels-columns.tsxweb/default/src/features/models/components/dialogs/view-details-dialog.tsxweb/default/src/features/wallet/hooks/use-payment.tsweb/default/src/features/usage-logs/components/dialogs/audio-preview-dialog.tsxweb/default/src/features/system-settings/models/tiered-pricing-support.tsxweb/default/src/features/subscriptions/components/dialogs/subscription-purchase-dialog.tsxweb/default/src/features/channels/lib/channel-actions.test.tsweb/default/src/features/profile/api.tsweb/default/src/features/auth/api.tsweb/default/src/features/playground/hooks/use-stream-request.tsweb/default/src/features/pricing/lib/tier-expr.tsweb/default/src/features/system-settings/models/model-ratio-visual-editor.tsxweb/default/src/features/channels/lib/channel-actions.tsweb/default/src/features/channels/components/dialogs/channel-test-dialog.tsx
web/default/src/**/*.test.ts
📄 CodeRabbit inference engine (web/default/AGENTS.md)
工具函数与纯逻辑应优先编写单元测试；测试文件应命名为
*.test.ts。
Files:
web/default/src/features/playground/hooks/use-stream-request.test.tsweb/default/src/lib/safe-redirect.test.tsweb/default/src/features/auth/lib/turnstile-request.test.tsweb/default/src/lib/handle-server-error.test.tsweb/default/src/features/pricing/lib/tier-expr.test.tsweb/default/src/features/channels/lib/channel-actions.test.ts
**/*.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:
middleware/turnstile_check_test.gomodel/redemption_search_test.gomodel/user_oauth_binding.gomodel/usedata_test.gomodel/main_migration_test.gomodel/checkin.gocontroller/token.gocontroller/telegram.gosetting/config/config_test.gomodel/utils.gocontroller/linuxdo.gomodel/redemption.goservice/quota.gopkg/billingexpr/security_cache_test.gocontroller/oidc.gocontroller/wechat.gomiddleware/turnstile-check.gocontroller/oauth_test.goservice/pre_consume_quota.gocontroller/github.gopkg/billingexpr/run.goservice/task_billing_test.gopkg/billingexpr/compile.gomodel/subscription.gocontroller/oauth.gocontroller/discord.gomodel/main.gomodel/token_cache.gomodel/errors.goservice/billing_session_test.gomodel/user_cache.gomodel/usedata.gocommon/redis.gosetting/config/config.gomodel/token.goservice/funding_source.gomodel/user_update_test.gomodel/user.goservice/billing_session.go
web/default/src/features/**/lib/**/*.ts
📄 CodeRabbit inference engine (web/default/AGENTS.md)
表单应使用 React Hook Form + Zod：在功能模块的
lib/下定义 schema，并用z.infer导出表单类型；useForm应配合@hookform/resolvers/zod进行校验。
Files:
web/default/src/features/auth/lib/turnstile-request.test.tsweb/default/src/features/pricing/lib/tier-expr.test.tsweb/default/src/features/auth/lib/turnstile-request.tsweb/default/src/features/channels/lib/channel-actions.test.tsweb/default/src/features/pricing/lib/tier-expr.tsweb/default/src/features/channels/lib/channel-actions.ts
web/default/**/package.json
📄 CodeRabbit inference engine (web/default/AGENTS.md)
脚本与包管理以
package.json为准；发布前应执行 typecheck、lint、format、生产构建，并检查产物体积与环境变量配置。
Files:
web/default/package.json
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/security_cache_test.gopkg/billingexpr/expr.mdpkg/billingexpr/run.gopkg/billingexpr/compile.go
web/default/src/routes/**/*.{ts,tsx}
📄 CodeRabbit inference engine (web/default/AGENTS.md)
路由应使用 TanStack Router，并通过
createFileRoute定义；搜索参数应使用 Zod schema +validateSearch校验；认证与重定向应放在beforeLoad中；导航应优先使用useNavigate或Link，避免直接操作window.location。
Files:
web/default/src/routes/oauth/$provider.tsxweb/default/src/routes/__root.tsx
🔇 Additional comments (71)
web/default/src/features/playground/hooks/use-stream-request.test.ts (1)
1-32: LGTM!web/default/src/features/pricing/lib/tier-expr.test.ts (1)
1-58: LGTM!web/default/src/features/system-settings/models/tiered-pricing-support.tsx (1)
458-464: LGTM!go.mod (1)
64-69: LGTM!web/default/src/features/channels/components/dialogs/channel-test-dialog.tsx (1)
98-117: LGTM!Also applies to: 245-359, 995-999
web/default/src/features/channels/lib/channel-actions.test.ts (1)
1-93: LGTM!web/default/src/features/channels/lib/channel-actions.ts (1)
40-218: LGTM!Also applies to: 379-441
web/default/src/i18n/locales/ru.json (1)
67-68: LGTM!Also applies to: 84-86, 801-802, 2701-2701, 3895-3895
web/default/src/features/profile/api.ts (1)
20-20: LGTM!Also applies to: 107-116, 187-194
pkg/billingexpr/expr.md (1)
79-105: LGTM!pkg/billingexpr/run.go (1)
72-88: LGTM!Also applies to: 118-158
pkg/billingexpr/security_cache_test.go (1)
1-59: LGTM!web/default/src/features/pricing/lib/tier-expr.ts (1)
19-19: LGTM!Also applies to: 272-321, 357-361
web/default/src/features/system-settings/models/model-ratio-visual-editor.tsx (1)
131-132: LGTM!Also applies to: 234-248, 488-503, 560-566, 577-577
web/default/src/lib/handle-server-error.test.ts (1)
1-41: LGTM!web/default/package.json (1)
44-44: LGTM!middleware/turnstile-check.go (1)
17-25: LGTM!Also applies to: 35-35, 60-60
middleware/turnstile_check_test.go (1)
1-37: LGTM!web/default/src/features/auth/api.ts (1)
20-20: LGTM!Also applies to: 39-47, 72-75, 84-87, 109-114, 123-126
web/default/src/features/auth/lib/turnstile-request.test.ts (1)
1-37: LGTM!web/default/src/features/auth/lib/turnstile-request.ts (1)
1-26: LGTM!web/default/src/lib/oauth.ts (1)
21-67: LGTM!Also applies to: 154-154, 165-165, 179-179, 190-190
web/default/src/routes/oauth/$provider.tsx (1)
31-32: LGTM!Also applies to: 52-61, 63-77, 89-99
web/default/src/components/layout/components/footer.tsx (1)
26-26: LGTM!Also applies to: 236-238
web/default/src/components/layout/components/chat-presets-item.tsx (1)
213-213: LGTM!web/default/src/features/channels/components/channels-columns.tsx (1)
660-667: LGTM!web/default/src/features/keys/components/data-table-row-actions.tsx (1)
150-150: LGTM!web/default/src/features/keys/components/dialogs/cc-switch-dialog.tsx (1)
149-149: LGTM!web/default/src/features/models/components/dialogs/view-details-dialog.tsx (1)
245-251: LGTM!web/default/src/features/playground/components/message-error.tsx (1)
60-64: LGTM!web/default/src/lib/safe-redirect.test.ts (1)
19-42: LGTM!web/default/src/routes/__root.tsx (1)
63-65: LGTM!Also applies to: 76-77, 90-93
web/default/src/features/usage-logs/components/dialogs/audio-preview-dialog.tsx (1)
117-119: LGTM!web/default/src/features/wallet/hooks/use-creem-payment.ts (1)
39-39: LGTM!web/default/src/features/wallet/hooks/use-payment.ts (1)
104-108: LGTM!web/default/src/features/wallet/hooks/use-waffo-payment.ts (1)
64-64: LGTM!web/default/src/features/subscriptions/components/dialogs/subscription-purchase-dialog.tsx (1)
125-125: LGTM!Also applies to: 147-147, 359-362
web/default/src/features/system-settings/integrations/ionet-deployment-settings-section.tsx (1)
224-228: LGTM!web/default/src/lib/safe-redirect.ts (1)
20-38: 🔒 Security & PrivacyBackslash-prefixed targets already fall back to
/dashboard.new URL('/\\evil.example/path', redirectBase)resolves tohttps://evil.example, so theparsed.origin !== redirectBasecheck rejects it.> Likely an incorrect or invalid review comment.common/redis.go (1)
84-254: LGTM!model/user_cache.go (1)
53-62: LGTM!Also applies to: 119-148
model/utils.go (1)
85-85: LGTM!controller/telegram.go (1)
95-97: LGTM!model/errors.go (1)
7-12: LGTM!Also applies to: 17-21, 26-27, 31-31, 34-34
setting/config/config.go (1)
4-4: LGTM!Also applies to: 42-62, 93-161, 165-202
model/usedata.go (1)
4-4: LGTM!Also applies to: 54-54, 103-116, 129-157
model/redemption_search_test.go (1)
1-20: LGTM!pkg/billingexpr/compile.go (1)
36-38: LGTM!Also applies to: 105-115, 182-182
model/token_cache.go (1)
10-29: LGTM!Also applies to: 46-50
model/token.go (1)
23-28: LGTM!Also applies to: 285-299, 310-339, 385-461, 497-497, 535-535
model/subscription.go (1)
838-839: LGTM!model/main.go (1)
285-287: LGTM!Also applies to: 344-346, 680-756
model/main_migration_test.go (1)
5-20: LGTM!controller/token.go (1)
188-188: LGTM!Also applies to: 272-272
service/quota.go (1)
146-146: LGTM!Also applies to: 417-417
service/task_billing_test.go (1)
100-100: LGTM!Also applies to: 168-175
service/billing_session.go (1)
8-8: LGTM!Also applies to: 27-80, 112-117, 335-336, 348-348, 428-430
service/billing_session_test.go (1)
1-166: LGTM!service/funding_source.go (1)
70-102: LGTM!service/pre_consume_quota.go (1)
55-56: LGTM!model/user.go (1)
582-593: LGTM!Also applies to: 1160-1174, 1189-1234, 1358-1382, 1467-1535, 1665-1665
model/user_update_test.go (1)
12-19: LGTM!Also applies to: 28-146, 148-203, 205-326
controller/oauth.go (1)
38-49: LGTM!Also applies to: 245-247, 262-264
controller/oauth_test.go (1)
4-29: LGTM!Also applies to: 54-64
controller/discord.go (1)
135-135: LGTM!controller/github.go (1)
115-120: LGTM!controller/linuxdo.go (1)
212-212: LGTM!controller/oidc.go (1)
137-137: LGTM!controller/wechat.go (1)
78-78: LGTM!model/user_oauth_binding.go (1)
51-60: LGTM!model/redemption.go (1)
80-84: LGTM!
| func invalidateTokenCache(key string) error { | ||
| return common.RedisInvalidateVersionedHash(getTokenCacheKey(key), getTokenCacheVersionKey(key)) |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
Expire version keys after the cache-fence window.
RedisInvalidateVersionedHash increments cache-version:token:* without assigning an expiration. Every invalidated or deleted token therefore leaves a permanent Redis key, causing unbounded growth. Set a TTL long enough to outlive pending asynchronous refills, preferably in the shared helper.
🤖 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_cache.go` around lines 31 - 32, Update
RedisInvalidateVersionedHash, used by invalidateTokenCache, to assign an
expiration to each incremented cache-version key; choose a TTL that exceeds the
asynchronous refill/cache-fence window and apply it consistently in the shared
helper so invalidated token version keys can expire.
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 (1)
model/user.go (1)
1089-1094: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winDo not expose a Redis failure as a failed database deletion.
The database deletion is already committed when
deleteUserCachefails. Returning that error gives callers an ambiguous result while stale authorization data remains cached. Retry cache deletion asynchronously and report the committed database outcome consistently.Also applies to: 1104-1107
🤖 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/user.go` around lines 1089 - 1094, Update the user deletion flow around DB.Delete and deleteUserCache so a cache-deletion failure does not cause the committed database deletion to be returned as failed. Trigger deletion-cache retry asynchronously when deleteUserCache fails, while returning the successful database outcome consistently; apply the same behavior to the additional affected deletion path.
🤖 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/subscription.go`:
- Around line 838-842: Update model/subscription.go at lines 838-842 to use
group-aware invalidateUserCache when upgradeGroup is non-empty, while retaining
quota-only invalidation otherwise, and enqueue failed invalidations for retry
instead of only logging them. Update model/token.go at lines 472-475 to enqueue
invalidateUserQuotaCache failures after the committed deduction, preserving the
existing successful invalidation path.
In `@model/token.go`:
- Around line 332-338: Ensure failed deleteTokenCache operations for revoked
tokens are persisted or enqueued for retry instead of only logged, so cache
invalidation occurs after Redis recovers. Apply this to both Token.Delete and
the batch deletion flow at model/token.go lines 332-338 and 540-545; preserve
the existing database deletion behavior and handle each cache-deletion failure
consistently.
In `@model/usedata.go`:
- Around line 188-201: Replace the First-then-Create flow in the quota-data
transaction with a portable unique aggregate key covering the bucket fields and
a single atomic upsert that increments the existing aggregate. Update the
relevant quota-data model/schema and the increaseQuotaDataTx path to use
conflict-safe SQL compatible with SQLite, MySQL 5.7.8+, and PostgreSQL 9.6+,
preserving creation when no matching aggregate exists.
In `@model/user.go`:
- Around line 1498-1500: Update both quota mutation paths around
invalidateUserQuotaCache so that, when cache invalidation fails after a
successful quota decrement, they call enqueueUserCacheInvalidationRetry with the
affected user identifier. Keep the existing error logging and successful
mutation response unchanged, and ensure both failure branches schedule a retry.
- Around line 1261-1278: The OAuth ID lookup helpers IsWeChatIdAlreadyTaken,
IsGitHubIdAlreadyTaken, IsDiscordIdAlreadyTaken, IsOidcIdAlreadyTaken, and
IsTelegramIdAlreadyTaken must return both the taken status and the database
error from Find(...).Error. Update every call site to handle and propagate the
error so binding or registration does not continue when the uniqueness lookup
fails.
In `@service/billing_session.go`:
- Around line 80-96: The compensationFailed flag remains set after successful
funding reconciliation, causing later token-step failures to skip compensation.
In the compensationFailed branch around funding.Settle, clear
s.compensationFailed once s.appliedFundingDelta reaches delta, before retrying
the token adjustment, while preserving the existing incomplete-reconciliation
errors.
In `@service/pre_consume_quota.go`:
- Around line 80-85: Update the error handling after DecreaseTokenAndUserQuota
in the pre-consumption flow to treat model.ErrUserQuotaInsufficient the same as
model.ErrTokenQuotaInsufficient. Return the existing pre-consume quota failure
response with Forbidden status and its current retry/logging options, while
preserving the generic update error for other failures.
---
Outside diff comments:
In `@model/user.go`:
- Around line 1089-1094: Update the user deletion flow around DB.Delete and
deleteUserCache so a cache-deletion failure does not cause the committed
database deletion to be returned as failed. Trigger deletion-cache retry
asynchronously when deleteUserCache fails, while returning the successful
database outcome consistently; apply the same behavior to the additional
affected deletion path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e0a1867c-c4fc-4a18-94e9-09200791e9fb
📒 Files selected for processing (17)
common/redis.gocommon/redis_version_test.gomodel/checkin.gomodel/log_test.gomodel/main.gomodel/option.gomodel/subscription.gomodel/token.gomodel/token_cache.gomodel/usedata.gomodel/usedata_test.gomodel/user.gomodel/user_cache.gomodel/user_update_test.goservice/billing_session.goservice/billing_session_test.goservice/pre_consume_quota.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:
common/redis_version_test.gomodel/checkin.gomodel/subscription.gomodel/option.gomodel/log_test.goservice/pre_consume_quota.gomodel/usedata_test.gomodel/token_cache.gomodel/main.goservice/billing_session_test.gomodel/usedata.gomodel/token.gomodel/user_cache.goservice/billing_session.gocommon/redis.gomodel/user_update_test.gomodel/user.go
🔇 Additional comments (17)
common/redis.go (1)
84-119: LGTM!Also applies to: 231-237, 262-265, 297-314
common/redis_version_test.go (1)
1-49: LGTM!model/option.go (1)
704-709: LGTM!model/token_cache.go (1)
35-40: LGTM!model/usedata.go (1)
10-117: LGTM!Also applies to: 141-186, 205-213
model/usedata_test.go (1)
37-79: LGTM!model/user.go (1)
140-156: LGTM!Also applies to: 1398-1478, 1551-1555, 1648-1682
model/user_cache.go (1)
65-86: LGTM!Also applies to: 111-117, 224-232, 250-309
service/billing_session.go (1)
26-43: LGTM!Also applies to: 132-138
model/checkin.go (1)
5-5: LGTM!Also applies to: 117-121
model/log_test.go (1)
861-861: LGTM!model/main.go (2)
285-301: LGTM!Also applies to: 345-366
683-698: 🗄️ Data Integrity & IntegrationBigint migration covers MySQL/PostgreSQL and skips SQLite.
model/token.go (1)
23-28: LGTM!Also applies to: 433-471
model/user_update_test.go (1)
98-114: LGTM!Also applies to: 223-229, 329-348
service/billing_session_test.go (1)
97-132: LGTM!service/pre_consume_quota.go (1)
4-4: LGTM!Also applies to: 56-57, 71-79
| if chargedQuota > 0 || upgradeGroup != "" { | ||
| if cacheErr := invalidateUserQuotaCache(userId); cacheErr != nil { | ||
| common.SysLog("failed to invalidate user quota cache after subscription purchase: " + cacheErr.Error()) | ||
| } | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Make committed user-cache invalidation complete and retryable.
model/subscription.go#L838-L842: use the group-awareinvalidateUserCachewhenupgradeGroup != ""; quota-only invalidation cannot evict a cached old group. Queue failed invalidations for retry.model/token.go#L472-L475: enqueueinvalidateUserQuotaCachefailures after the committed deduction instead of only logging them.
Otherwise stale quota can permit excess consumption, while stale groups can delay or incorrectly preserve access.
📍 Affects 2 files
model/subscription.go#L838-L842(this comment)model/token.go#L472-L475
🤖 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/subscription.go` around lines 838 - 842, Update model/subscription.go
at lines 838-842 to use group-aware invalidateUserCache when upgradeGroup is
non-empty, while retaining quota-only invalidation otherwise, and enqueue failed
invalidations for retry instead of only logging them. Update model/token.go at
lines 472-475 to enqueue invalidateUserQuotaCache failures after the committed
deduction, preserving the existing successful invalidation path.
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
controller/oauth.go (1)
38-48: 🔒 Security & Privacy | 🔴 Critical | ⚡ Quick winPrevent leakage of internal database errors.
Directly exposing
err.Error()in the JSON response can leak sensitive internal details (e.g., database connection strings, SQL syntax errors) if the underlying lookup fails at the database layer. This contradicts the security guarantees verified byTestOAuthBindMasksUniquenessLookupDatabaseError.Log the raw error internally and return a generic error message to the client, similar to how
handleOAuthIdentityLookupErrormasks internal errors.🔒️ Proposed fix to mask internal errors
func handleOAuthUserLookupError(c *gin.Context, err error) bool { if err == nil { return false } if errors.Is(err, model.ErrUserDeleted) { c.JSON(http.StatusOK, gin.H{"success": false, "message": "用户已注销"}) } else { + common.SysError(fmt.Sprintf("OAuth user lookup failed: %v", err)) - c.JSON(http.StatusOK, gin.H{"success": false, "message": err.Error()}) + c.JSON(http.StatusOK, gin.H{"success": false, "message": "获取用户信息失败，请稍后重试"}) } return 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/oauth.go` around lines 38 - 48, Update handleOAuthUserLookupError so only the ErrUserDeleted case returns its existing user-facing message; for all other errors, log the raw error internally and return a generic client-safe error message instead of err.Error(), matching the masking behavior of handleOAuthIdentityLookupError.service/billing_session.go (2)
74-82: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winHandle partial initial funding application.
FundingSource.Settlereturns the amount actually applied, but this path marks funding settled even whenapplied != deltaand proceeds with the full token adjustment. Record reconciliation-pending state and stop before the token step.🤖 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/billing_session.go` around lines 74 - 82, Update the !s.fundingSettled branch around FundingSource.Settle to compare applied with delta; when they differ, retain the applied amount, mark fundingReconcilePending, and return before applying the token adjustment. Only set fundingSettled and clear reconciliation state when the full delta is applied, preserving compensationFailed handling.
46-63: 🗄️ Data Integrity & Integration | 🔴 Critical | 🏗️ Heavy liftMake settlement mutations idempotent before retrying or accepting ambiguous outcomes. The current state machine can finalize token usage without matching funding or apply a token delta twice.
service/billing_session.go#L46-L63: guard retries with a durable settlement operation ID.service/billing_session.go#L83-L94: keep ambiguous reconciliation unresolved and block token adjustment.service/billing_session.go#L123-L131: do not assume failed compensation left the original funding delta committed.service/billing_session_test.go#L103-L130: cover compensation-committed and compensation-not-committed outcomes.service/billing_session_test.go#L258-L274: require exactly-once behavior under an ambiguous token commit before asserting retries.🤖 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/billing_session.go` around lines 46 - 63, Make settlement mutations idempotent across retries and ambiguous outcomes: update service/billing_session.go lines 46-63 in BillingSession.Settle to create and reuse a durable settlement operation ID; keep ambiguous reconciliation unresolved and prevent token adjustment at lines 83-94; at lines 123-131, only treat the original funding delta as committed when compensation confirms it, rather than assuming failed compensation succeeded. Extend service/billing_session_test.go lines 103-130 to cover compensation-committed and compensation-not-committed outcomes, and lines 258-274 to verify exactly-once behavior for ambiguous token commits before retry assertions.model/user_cache.go (1)
111-117: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winEnqueue failed auth-cache invalidations.
Both paths return on invalidation failure without using the new retry helper. Since
User.Updateonly logs this error, stale role or status data can remain authoritative until cache expiry.Proposed pattern
-if err := invalidateUserCache(userId); err != nil { - return err +if err := invalidateUserCache(userId); err != nil { + enqueueUserCacheInvalidationRetry(userId, err) + return err }Also applies to: 289-297
🤖 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/user_cache.go` around lines 111 - 117, Update updateUserCache and the corresponding second invalidation path to enqueue failed auth-cache invalidations through the new retry helper instead of returning the invalidation error directly. Preserve the Redis-disabled behavior and ensure failures from invalidateUserCache are handed to the retry mechanism before returning.model/log_test.go (1)
861-865: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winClean up the snapshot marker created by this test.
The synchronous save now inserts a
QuotaDataSnapshot, but cleanup only deletesQuotaData, leaking state into subsequent tests.Proposed fix
require.NoError(t, DB.AutoMigrate(&QuotaData{}, &QuotaDataSnapshot{})) require.NoError(t, DB.Where("1 = 1").Delete(&QuotaData{}).Error) +require.NoError(t, DB.Where("1 = 1").Delete(&QuotaDataSnapshot{}).Error) t.Cleanup(func() { require.NoError(t, DB.Where("1 = 1").Delete(&QuotaData{}).Error) + require.NoError(t, DB.Where("1 = 1").Delete(&QuotaDataSnapshot{}).Error) })🤖 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/log_test.go` around lines 861 - 865, Update the test cleanup around the synchronous save to also delete all QuotaDataSnapshot records, both before the test and in the t.Cleanup callback, alongside the existing QuotaData deletion. Use the existing DB cleanup pattern and preserve the QuotaData cleanup behavior.
🤖 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/option.go`:
- Around line 704-710: Update the group_ratio_setting/group_ratio branch to
apply ratio_setting.UpdateGroupRatioByJSONString(value) without returning early,
then continue into config.UpdateConfigFromMap(cfg, configMap) so the registered
config is updated as well.
In `@model/token_cache.go`:
- Around line 106-112: Update the pending-entry deadline logic in the token
cache around the deleteEntry handling, including the corresponding block near
the second referenced location, so failed deletion attempts receive the same
bounded retry window as invalidations instead of remaining pending indefinitely.
When the deadline expires, stop retrying and emit a terminal metric or log
identifying the expired deletion.
In `@model/usedata.go`:
- Around line 19-20: The quota data migration around Usedata must populate
AggregateKey for legacy rows, merge duplicate buckets, and create the unique
aggregate-key index only after cleanup, with SQL compatible with SQLite, MySQL
5.7.8+, and PostgreSQL 9.6+. Add coverage in model/usedata_test.go:101-108 for a
populated legacy table and verify later aggregate-key upserts increment the
migrated row; update model/usedata.go:19-20 for the migration, while the test
site requires the corresponding migration and upsert assertions.
In `@model/user.go`:
- Line 1094: Update the user deletion flow around deleteUserCache so cache
deletion failures are logged and scheduled through the persistent delete-mode
retry mechanism, without propagating the cache error after the database deletion
commits. Return success once the DB deletion succeeds, and apply the same
behavior to the corresponding path near the second referenced location.
- Around line 1261-1283: Enforce database-level uniqueness for the provider
identity fields checked by IsWeChatIdAlreadyTaken, IsGitHubIdAlreadyTaken,
IsDiscordIdAlreadyTaken, IsOidcIdAlreadyTaken, and IsTelegramIdAlreadyTaken.
Update the migration to convert empty IDs to NULL, then add compatible unique
constraints for SQLite, MySQL 5.7.8+, and PostgreSQL 9.6+ (or reuse the uniquely
constrained binding table), ensuring legacy duplicate rows are handled before
constraints are created.
- Around line 153-155: Update the error branch after invalidateUserCache in the
user-setting mutation flow to enqueue the existing cache-invalidation retry,
matching the retry behavior used by quota mutations. Preserve the current error
logging and pass the affected userId through the established retry mechanism.
---
Outside diff comments:
In `@controller/oauth.go`:
- Around line 38-48: Update handleOAuthUserLookupError so only the
ErrUserDeleted case returns its existing user-facing message; for all other
errors, log the raw error internally and return a generic client-safe error
message instead of err.Error(), matching the masking behavior of
handleOAuthIdentityLookupError.
In `@model/log_test.go`:
- Around line 861-865: Update the test cleanup around the synchronous save to
also delete all QuotaDataSnapshot records, both before the test and in the
t.Cleanup callback, alongside the existing QuotaData deletion. Use the existing
DB cleanup pattern and preserve the QuotaData cleanup behavior.
In `@model/user_cache.go`:
- Around line 111-117: Update updateUserCache and the corresponding second
invalidation path to enqueue failed auth-cache invalidations through the new
retry helper instead of returning the invalidation error directly. Preserve the
Redis-disabled behavior and ensure failures from invalidateUserCache are handed
to the retry mechanism before returning.
In `@service/billing_session.go`:
- Around line 74-82: Update the !s.fundingSettled branch around
FundingSource.Settle to compare applied with delta; when they differ, retain the
applied amount, mark fundingReconcilePending, and return before applying the
token adjustment. Only set fundingSettled and clear reconciliation state when
the full delta is applied, preserving compensationFailed handling.
- Around line 46-63: Make settlement mutations idempotent across retries and
ambiguous outcomes: update service/billing_session.go lines 46-63 in
BillingSession.Settle to create and reuse a durable settlement operation ID;
keep ambiguous reconciliation unresolved and prevent token adjustment at lines
83-94; at lines 123-131, only treat the original funding delta as committed when
compensation confirms it, rather than assuming failed compensation succeeded.
Extend service/billing_session_test.go lines 103-130 to cover
compensation-committed and compensation-not-committed outcomes, and lines
258-274 to verify exactly-once behavior for ambiguous token commits before retry
assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c685da08-03a6-46e6-8326-b0061e7df29c
📒 Files selected for processing (34)
common/init.gocommon/redis.gocommon/redis_version_test.gocontroller/discord.gocontroller/github.gocontroller/linuxdo.gocontroller/oauth.gocontroller/oauth_test.gocontroller/oidc.gocontroller/telegram.gocontroller/wechat.gomodel/checkin.gomodel/log_test.gomodel/main.gomodel/option.gomodel/payment_method_guard_test.gomodel/subscription.gomodel/token.gomodel/token_cache.gomodel/usedata.gomodel/usedata_test.gomodel/user.gomodel/user_cache.gomodel/user_oauth_binding.gomodel/user_update_test.gooauth/discord.gooauth/generic.gooauth/github.gooauth/linuxdo.gooauth/oidc.gooauth/provider.goservice/billing_session.goservice/billing_session_test.goservice/pre_consume_quota.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_test.gooauth/provider.gooauth/github.gooauth/oidc.gomodel/checkin.gocommon/init.gooauth/discord.gocontroller/oidc.gomodel/option.gooauth/generic.gomodel/payment_method_guard_test.gocontroller/telegram.gocontroller/linuxdo.goservice/pre_consume_quota.gocontroller/github.gooauth/linuxdo.gocommon/redis_version_test.gocontroller/oauth_test.gocontroller/discord.gomodel/token_cache.gocontroller/oauth.gomodel/subscription.gomodel/usedata_test.gomodel/main.goservice/billing_session_test.gomodel/usedata.goservice/billing_session.gocontroller/wechat.gomodel/user_oauth_binding.gomodel/user_cache.gomodel/token.gomodel/user_update_test.gocommon/redis.gomodel/user.go
🔇 Additional comments (33)
common/init.go (1)
131-131: LGTM!model/payment_method_guard_test.go (1)
219-238: LGTM!model/subscription.go (1)
41-50: LGTM!Also applies to: 663-663, 748-748, 848-849, 954-954, 995-995, 1211-1211
controller/oauth_test.go (1)
4-34: LGTM!Also applies to: 59-69, 71-93, 95-132
oauth/discord.go (1)
157-159: LGTM!oauth/generic.go (1)
293-295: LGTM!oauth/github.go (1)
163-165: LGTM!oauth/linuxdo.go (1)
170-172: LGTM!oauth/oidc.go (1)
162-164: LGTM!oauth/provider.go (1)
26-26: LGTM!controller/telegram.go (1)
35-39: LGTM!Also applies to: 99-99
model/checkin.go (1)
118-121: 🔒 Security & PrivacyNo issue here.
enqueueUserCacheInvalidationRetryandinvalidateUserQuotaCacheboth clear the same user cache, and the SQLite path reaches the same quota invalidation throughIncreaseUserQuota.> Likely an incorrect or invalid review comment.model/token_cache.go (2)
69-73: Version-key expiration was already raised in the previous review.The shared invalidation helper still needs a bounded TTL for generated version keys.
5-68: LGTM!Also applies to: 74-105, 113-180, 186-264, 278-282
model/main.go (2)
285-287: LGTM!Also applies to: 301-301, 345-347, 366-366
683-698: 🗄️ Data Integrity & IntegrationNo change needed.
model/usedata.go (1)
4-18: LGTM!Also applies to: 21-124, 149-216
model/usedata_test.go (1)
4-100: LGTM!Also applies to: 110-212
model/user_cache.go (1)
65-85: LGTM!Also applies to: 94-105, 151-158, 181-182, 224-232, 250-287, 300-309
common/redis_version_test.go (1)
1-49: LGTM!service/billing_session.go (1)
8-45: LGTM!Also applies to: 64-73, 95-122, 132-155, 373-374, 386-386, 466-468
service/billing_session_test.go (1)
4-102: LGTM!Also applies to: 132-257, 276-309
service/pre_consume_quota.go (1)
4-4: LGTM!Also applies to: 56-57, 71-86
model/token.go (1)
23-23: LGTM!Also applies to: 28-28, 316-316, 328-328, 337-339, 436-481, 511-511, 546-548, 584-588
model/user.go (1)
1404-1410: LGTM!Also applies to: 1424-1430, 1442-1448, 1462-1468, 1503-1506, 1536-1539, 1558-1562, 1656-1662, 1676-1682
model/user_oauth_binding.go (1)
67-70: LGTM!Also applies to: 86-90, 112-114, 131-148
model/user_update_test.go (1)
98-114: LGTM!Also applies to: 223-260, 270-550, 641-660
common/redis.go (1)
84-118: LGTM!Also applies to: 231-237, 256-293, 297-314
controller/discord.go (1)
133-141: LGTM!Also applies to: 198-202
controller/github.go (1)
115-124: LGTM!Also applies to: 185-189
controller/linuxdo.go (1)
50-54: LGTM!Also applies to: 214-222
controller/oidc.go (1)
135-143: LGTM!Also applies to: 201-205
controller/wechat.go (1)
76-84: LGTM!Also applies to: 147-151
| func IsWeChatIdAlreadyTaken(wechatId string) (bool, error) { | ||
| result := DB.Unscoped().Where("wechat_id = ?", wechatId).Limit(1).Find(&User{}) | ||
| return result.RowsAffected > 0, result.Error | ||
| } | ||
|
|
||
| func IsGitHubIdAlreadyTaken(githubId string) bool { | ||
| return DB.Unscoped().Where("github_id = ?", githubId).Find(&User{}).RowsAffected == 1 | ||
| func IsGitHubIdAlreadyTaken(githubId string) (bool, error) { | ||
| result := DB.Unscoped().Where("github_id = ?", githubId).Limit(1).Find(&User{}) | ||
| return result.RowsAffected > 0, result.Error | ||
| } | ||
|
|
||
| func IsDiscordIdAlreadyTaken(discordId string) bool { | ||
| return DB.Unscoped().Where("discord_id = ?", discordId).Find(&User{}).RowsAffected == 1 | ||
| func IsDiscordIdAlreadyTaken(discordId string) (bool, error) { | ||
| result := DB.Unscoped().Where("discord_id = ?", discordId).Limit(1).Find(&User{}) | ||
| return result.RowsAffected > 0, result.Error | ||
| } | ||
|
|
||
| func IsOidcIdAlreadyTaken(oidcId string) bool { | ||
| return DB.Where("oidc_id = ?", oidcId).Find(&User{}).RowsAffected == 1 | ||
| func IsOidcIdAlreadyTaken(oidcId string) (bool, error) { | ||
| result := DB.Unscoped().Where("oidc_id = ?", oidcId).Limit(1).Find(&User{}) | ||
| return result.RowsAffected > 0, result.Error | ||
| } | ||
|
|
||
| func IsTelegramIdAlreadyTaken(telegramId string) bool { | ||
| return DB.Unscoped().Where("telegram_id = ?", telegramId).Find(&User{}).RowsAffected == 1 | ||
| func IsTelegramIdAlreadyTaken(telegramId string) (bool, error) { | ||
| result := DB.Unscoped().Where("telegram_id = ?", telegramId).Limit(1).Find(&User{}) | ||
| return result.RowsAffected > 0, result.Error |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Enforce OAuth identity uniqueness in the database.
These checks are vulnerable to check-then-insert races because the provider-ID columns are only indexed, not unique; TestIsOidcIdAlreadyTakenHandlesDuplicateLegacyRows confirms duplicates are accepted. Concurrent OAuth registrations can therefore bind one provider identity to multiple users.
Migrate empty IDs to NULL and add unique constraints, or move these identities to the already uniquely constrained binding table.
As per coding guidelines, migrations must work across SQLite, MySQL 5.7.8+, and PostgreSQL 9.6+.
Also applies to: 1691-1693
🤖 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/user.go` around lines 1261 - 1283, Enforce database-level uniqueness
for the provider identity fields checked by IsWeChatIdAlreadyTaken,
IsGitHubIdAlreadyTaken, IsDiscordIdAlreadyTaken, IsOidcIdAlreadyTaken, and
IsTelegramIdAlreadyTaken. Update the migration to convert empty IDs to NULL,
then add compatible unique constraints for SQLite, MySQL 5.7.8+, and PostgreSQL
9.6+ (or reuse the uniquely constrained binding table), ensuring legacy
duplicate rows are handled before constraints are created.
Source: Coding guidelines
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
setting/config/config.go (1)
212-239: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReject fractional and overflowing numeric values.
The float fallback silently converts values such as
"2.9"to2, andSetInt/SetUintcan truncate values outside narrower field ranges. Require an integral finite value and checkfield.OverflowInt,field.OverflowUint, orfield.OverflowFloatbefore assignment.🤖 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 `@setting/config/config.go` around lines 212 - 239, Update the numeric parsing branches in the configuration conversion logic to accept float-formatted fallbacks only when the parsed value is finite and integral, rejecting fractional values. Before each field.SetInt, field.SetUint, or field.SetFloat call, validate the parsed value with the corresponding reflection overflow check, and return the existing invalid-value error when validation 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/usedata.go`:
- Around line 128-140: Update the migration flow around
mergeQuotaDataAggregateRows and quotaDataAggregateKeyIndexName so the existing
aggregate uniqueness index is never dropped during backfill. Preserve the
constraint while clearing stale keys to NULL, merging/deleting duplicate rows,
and assigning keys to survivors; ensure the SQL and migration behavior remain
compatible with SQLite, MySQL 5.7.8+, and PostgreSQL 9.6+.
- Around line 159-204: Replace the unbounded read-and-per-group writes in
mergeQuotaDataAggregateRows with a bounded-batch or portable set-based migration
that avoids holding the entire quota_data table in memory or one long
transaction. Preserve aggregate grouping, survivor selection by ascending id,
summed count/quota/tokenUsed values, duplicate deletion, and error propagation
while ensuring the implementation works on SQLite, MySQL 5.7.8+, and PostgreSQL
9.6+.
In `@types/rw_map.go`:
- Around line 98-103: Update the method containing the m.data assignment so the
mutex is unlocked before invoking onSuccess. Avoid deferring the unlock across
the callback; retain the locked section for updating m.data, then call onSuccess
after releasing the lock so callbacks may safely use ReadAll, Set, or other
mutex-protected methods.
---
Outside diff comments:
In `@setting/config/config.go`:
- Around line 212-239: Update the numeric parsing branches in the configuration
conversion logic to accept float-formatted fallbacks only when the parsed value
is finite and integral, rejecting fractional values. Before each field.SetInt,
field.SetUint, or field.SetFloat call, validate the parsed value with the
corresponding reflection overflow check, and return the existing invalid-value
error when validation fails.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3f653cf6-f096-4316-b841-27acff04564a
📒 Files selected for processing (14)
model/log_test.gomodel/main.gomodel/option.gomodel/option_test.gomodel/token_cache.gomodel/token_cache_test.gomodel/usedata.gomodel/usedata_test.gomodel/user.gomodel/user_cache.gomodel/user_update_test.gosetting/config/config.gosetting/config/config_test.gotypes/rw_map.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/token_cache_test.gomodel/option.gomodel/option_test.gosetting/config/config_test.gomodel/log_test.gotypes/rw_map.gomodel/token_cache.gosetting/config/config.gomodel/usedata.gomodel/main.gomodel/usedata_test.gomodel/user_cache.gomodel/user_update_test.gomodel/user.go
🔇 Additional comments (14)
model/token_cache_test.go (1)
1-78: LGTM!model/option.go (1)
704-714: LGTM!model/option_test.go (1)
7-7: LGTM!Also applies to: 87-88, 99-105
model/token_cache.go (1)
83-89: LGTM!Also applies to: 114-114, 123-123, 183-189, 285-289
model/main.go (1)
39-53: LGTM!Also applies to: 104-110, 307-309, 323-323, 344-349, 373-375, 394-394, 435-440, 470-615, 863-878
model/usedata.go (1)
17-127: LGTM!Also applies to: 141-158, 205-206, 216-331
model/usedata_test.go (1)
1-281: LGTM!model/user_cache.go (1)
5-5: LGTM!Also applies to: 17-50, 88-306, 315-326, 336-356, 372-379, 402-403, 453-482, 493-530
model/user.go (1)
153-155: LGTM!Also applies to: 583-594, 1095-1110, 1163-1177, 1192-1237, 1264-1286, 1366-1390, 1407-1433, 1445-1471, 1493-1565, 1659-1685, 1694-1703
model/user_update_test.go (1)
12-231: LGTM!Also applies to: 233-734, 874-931
model/log_test.go (1)
861-862: LGTM!setting/config/config.go (1)
4-4: LGTM!Also applies to: 62-62, 137-148, 168-211, 240-277
types/rw_map.go (1)
14-22: LGTM!Also applies to: 81-97, 104-104
setting/config/config_test.go (1)
5-20: LGTM!Also applies to: 105-132
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/usedata_test.go`:
- Around line 303-315: Extend the assertions in the QuotaData migration test to
validate row B’s repaired Count, Quota, and TokenUsed values, matching the
expected counters established for that row’s fixture. Keep the existing ordering
and AggregateKey assertions unchanged.
In `@model/usedata.go`:
- Around line 114-124: Update quotaDataAggregateMigrationMember and
quotaDataAggregateMigrationSummary to use int64 for Id, SurvivorID, and every
accumulated total field (CountSum, QuotaSum, and TokenUsedSum), and set their
GORM column types to BIGINT where explicit types are defined. Apply the same
64-bit identifier and total-field changes to the related migration definitions
around the referenced aggregate migration code, preserving SQLite, MySQL, and
PostgreSQL compatibility.
- Around line 240-263: The quota aggregate migration spanning
buildQuotaDataAggregateMigrationTables and
insertQuotaDataAggregateMigrationBatch must not overwrite concurrent quota
writes or clear keys that are concurrently recreated. Protect the complete
row-scan and apply operation with the existing cross-database mechanism that
quiesces quota writes, or redesign the apply as an atomic reconciliation
compatible with SQLite, MySQL 5.7.8+, and PostgreSQL 9.6+; do not rely on a
database-specific locking feature.
- Around line 168-190: Update mergeQuotaDataAggregateRows to acquire a
database-backed migration lock before resetQuotaDataAggregateMigrationTables and
hold it through the final dropQuotaDataAggregateMigrationTables cleanup,
releasing it on every return path. Use the project’s cross-database locking
mechanism compatible with SQLite, MySQL 5.7.8+, and PostgreSQL 9.6+, and ensure
lock acquisition or release errors are handled without allowing concurrent
migration runs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 021feef7-36be-44dd-9fcd-b472d6b2bf38
📒 Files selected for processing (4)
model/usedata.gomodel/usedata_test.gotypes/rw_map.gotypes/rw_map_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:
types/rw_map_test.gotypes/rw_map.gomodel/usedata_test.gomodel/usedata.go
🪛 ast-grep (0.44.1)
model/usedata.go
[error] 232-232: SQL query is built by concatenating a string literal with a variable and passed to a database/sql call (Query, Exec, QueryRow, Prepare, or their Context variants). String concatenation lets attacker-controlled input alter the query structure, enabling SQL injection. Use parameterized queries with placeholders ('?' or '') and pass the values as separate arguments instead of concatenating them into the query string.
Context: DB.Exec("DROP TABLE IF EXISTS " + quoteDBIdentifier(table))
Note: [CWE-89] Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection').
(sql-injection-query-string-concat-go)
🪛 OpenGrep (1.25.0)
model/usedata.go
[ERROR] 233-233: SQL query built via fmt.Sprintf or string concatenation passed to a database method. Use parameterized queries with placeholder arguments.
(coderabbit.sql-injection.go-query-format)
🔇 Additional comments (6)
types/rw_map.go (1)
100-100: LGTM!types/rw_map_test.go (1)
1-34: LGTM!model/usedata_test.go (1)
4-41: LGTM!Also applies to: 202-302
model/usedata.go (3)
19-20: LGTM!Also applies to: 34-43
231-238: LGTM!Also applies to: 302-316
46-46: 🚀 Performance & ScalabilityBound retention for
QuotaDataSnapshotmarkers.saveQuotaDatawrites a durable row per snapshot, but no cleanup path is visible forquota_data_snapshots. If these markers are only needed for the retry window, add explicit pruning so the table doesn’t grow without bound.
|
@CodeRabbit 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 (2)
model/usedata.go (2)
875-913: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winReturn individual quota persistence failures.
When
saveQuotaDataLockedfails, the snapshot is requeued but the callback returns nil, leavingflushErrnil. Callers therefore receive false success and may terminate while the retry exists only in memory.Proposed fix
var flushErr error +var saveErr error ... if err := saveQuotaDataLocked(ctx, quotaData); err != nil { common.SysLog(fmt.Sprintf("saveQuotaData error: %s", err)) failed = append(failed, quotaData) + if saveErr == nil { + saveErr = err + } } ... -return flushErr +if flushErr != nil { + return flushErr +} +return saveErr🤖 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/usedata.go` around lines 875 - 913, Update the quota cache flush callback around saveQuotaDataLocked so an individual persistence failure is propagated through flushErr instead of returning nil. Preserve requeueing failed quota data, but record and return the encountered error after processing or immediately according to the existing retry semantics, ensuring callers receive a non-nil failure when any save fails.
577-603: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftUse the database clock for distributed lock expiry.
expires_atis calculated and compared using each application host’s clock. A fast node can steal a lock still held by a slower node, allowing concurrent migrations or quota flushes. Calculate bothnowand the new expiry atomically from the database clock, with dialect-specific expressions where necessary.As per coding guidelines, all database code must be compatible with SQLite, MySQL 5.7.8+, and PostgreSQL 9.6+.
Also applies to: 669-675
🤖 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/usedata.go` around lines 577 - 603, The quota lock acquisition flow around quotaDataOperationLock must use the database server clock rather than common.GetTimestamp() for both expiry comparisons and new expiry values. Replace the application-time now and expiry expressions in the lock creation and expired-lock Updates paths with dialect-compatible database-time expressions supporting SQLite, MySQL 5.7.8+, and PostgreSQL 9.6+, while preserving the existing lock lease duration and acquisition behavior.Source: Coding guidelines
🤖 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 `@model/usedata.go`:
- Around line 875-913: Update the quota cache flush callback around
saveQuotaDataLocked so an individual persistence failure is propagated through
flushErr instead of returning nil. Preserve requeueing failed quota data, but
record and return the encountered error after processing or immediately
according to the existing retry semantics, ensuring callers receive a non-nil
failure when any save fails.
- Around line 577-603: The quota lock acquisition flow around
quotaDataOperationLock must use the database server clock rather than
common.GetTimestamp() for both expiry comparisons and new expiry values. Replace
the application-time now and expiry expressions in the lock creation and
expired-lock Updates paths with dialect-compatible database-time expressions
supporting SQLite, MySQL 5.7.8+, and PostgreSQL 9.6+, while preserving the
existing lock lease duration and acquisition behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 5b20b078-43a6-4c1d-a3e7-e2b3573f7a0d
📒 Files selected for processing (5)
main.gomain_test.gomodel/log.gomodel/usedata.gomodel/usedata_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:
main_test.gomodel/log.gomain.gomodel/usedata.gomodel/usedata_test.go
🔇 Additional comments (5)
main_test.go (1)
33-38: LGTM!model/log.go (1)
206-206: LGTM!main.go (1)
242-250: LGTM!Also applies to: 288-295
model/usedata_test.go (1)
111-119: LGTM!Also applies to: 415-415, 472-472, 517-585, 614-614, 669-669, 726-726
model/usedata.go (1)
4-4: LGTM!Also applies to: 21-22, 79-91, 170-200, 283-318, 557-576, 613-624, 681-681, 729-798, 916-949
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/usedata.go`:
- Around line 573-580: Make MySQL lock acquisition independent of RowsAffected:
update createQuotaDataOperationLockAttempt and its callers to use an explicit
insert-versus-duplicate signal, or ensure the MySQL DSN path forces
clientFoundRows=false. Update the MySQL setup at model/usedata.go:628-636
accordingly, and revise model/usedata_test.go:917-919 and
model/usedata_test.go:943-960 to assert acquisition semantics rather than
generated SQL text.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 11a40516-0346-4e03-9502-1e0e0e296ca6
📒 Files selected for processing (11)
README.en.mdREADME.fr.mdREADME.ja.mdREADME.mdREADME.zh_CN.mdREADME.zh_TW.mdmain.gomain_test.gomodel/log.gomodel/usedata.gomodel/usedata_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.gomain.gomain_test.gomodel/usedata_test.gomodel/usedata.go
🪛 LanguageTool
README.fr.md
[typographical] ~118-~118: Il manque une espace après le point.
Context: ...e n'est requis pour un essai local. > [!WARNING] > SQLite est réservé à l'évaluation lo...
(ESPACE_APRES_POINT)
🔇 Additional comments (12)
model/log.go (1)
206-206: LGTM!README.zh_TW.md (1)
126-128: LGTM!Also applies to: 493-494
README.fr.md (1)
118-120: LGTM!Also applies to: 430-431
README.md (1)
128-130: LGTM!Also applies to: 495-496
README.zh_CN.md (1)
126-128: LGTM!Also applies to: 493-494
README.ja.md (1)
118-120: LGTM!Also applies to: 430-431
main.go (1)
242-250: LGTM!Also applies to: 288-296
main_test.go (1)
33-39: LGTM!README.en.md (1)
126-128: LGTM!Also applies to: 493-494
model/usedata.go (2)
764-800: Lock loss still does not cancel in-flight database work.The heartbeat only records
lostErr; callback queries continue using the original context and may commit after another owner claims the lease. Use a guard-owned cancellable context for all protected DB operations and cancel it fromsetLost.Also applies to: 809-817, 917-934
4-4: LGTM!Also applies to: 83-91, 171-171, 191-197, 287-287, 557-571, 583-600, 602-627, 637-660, 701-716, 892-916, 935-956, 971-979
model/usedata_test.go (1)
111-119: LGTM!Also applies to: 131-159, 445-445, 502-502, 547-615, 644-644, 699-699, 756-756, 888-916, 920-942
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@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 (2)
model/usedata.go (1)
67-68: 🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy liftBound
QuotaDataSnapshotmarker retention.Each unique flush permanently inserts a snapshot marker, but no cleanup path is present. This table and its primary index will grow with usage indefinitely. Delete markers older than a safe retry horizon via a bounded, portable cleanup job.
As per coding guidelines, the cleanup must support SQLite, MySQL 5.7.8+, and PostgreSQL 9.6+.
🤖 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/usedata.go` around lines 67 - 68, Add a bounded, portable cleanup job for QuotaDataSnapshot that deletes markers older than a safe retry horizon, using database-compatible operations for SQLite, MySQL 5.7.8+, and PostgreSQL 9.6+. Integrate it with the existing snapshot flush or maintenance flow, preserve recent markers needed for retries, and ensure cleanup errors are handled through the established error path.Source: Coding guidelines
model/main.go (1)
470-500: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftPrevent OAuth writes between duplicate cleanup and index creation.
Line 490 commits the cleanup before Lines 493-499 install uniqueness. During rolling startup, another instance can reinsert a duplicate in that gap, causing index creation—and application startup—to fail. Quiesce OAuth identity mutations or hold a database-specific lock across cleanup and constraint installation.
As per coding guidelines, the locking and migration strategy must support SQLite, MySQL 5.7.8+, and PostgreSQL 9.6+.
🤖 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/main.go` around lines 470 - 500, Update migrateUserOAuthIdentityConstraints to prevent concurrent OAuth identity writes across both duplicate cleanup and ensureUserOAuthIdentityUniqueIndex: hold an appropriate database lock or quiesce mutations until the index installation completes. Implement the locking strategy compatibly for SQLite, MySQL 5.7.8+, and PostgreSQL 9.6+, while preserving the existing transaction cleanup and migration behavior.Source: Coding guidelines
🤖 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 `@model/main.go`:
- Around line 470-500: Update migrateUserOAuthIdentityConstraints to prevent
concurrent OAuth identity writes across both duplicate cleanup and
ensureUserOAuthIdentityUniqueIndex: hold an appropriate database lock or quiesce
mutations until the index installation completes. Implement the locking strategy
compatibly for SQLite, MySQL 5.7.8+, and PostgreSQL 9.6+, while preserving the
existing transaction cleanup and migration behavior.
In `@model/usedata.go`:
- Around line 67-68: Add a bounded, portable cleanup job for QuotaDataSnapshot
that deletes markers older than a safe retry horizon, using database-compatible
operations for SQLite, MySQL 5.7.8+, and PostgreSQL 9.6+. Integrate it with the
existing snapshot flush or maintenance flow, preserve recent markers needed for
retries, and ensure cleanup errors are handled through the established error
path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 55ae8c30-b77e-412b-ad75-897171a881a0
📒 Files selected for processing (5)
model/main.gomodel/main_migration_test.gomodel/usedata.gomodel/usedata_test.gomodel/user_update_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/main_migration_test.gomodel/main.gomodel/usedata.gomodel/user_update_test.gomodel/usedata_test.go
🔇 Additional comments (5)
model/main.go (1)
39-53: LGTM!Also applies to: 104-110, 297-349, 369-440, 862-953
model/main_migration_test.go (1)
5-23: LGTM!Also applies to: 45-63
model/usedata.go (1)
21-22: LGTM!Also applies to: 37-61, 176-176, 229-273, 873-883, 1032-1108
model/usedata_test.go (1)
48-365: LGTM!model/user_update_test.go (1)
900-930: LGTM!
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/main.go`:
- Around line 622-629: Update the generated-column definition in the migration
flow around identity.mysqlGeneratedColumn so it does not truncate values from
the source identity column. Prefer a column type that preserves the full source
value, or consistently add matching max-length validation to the OAuth identity
fields in model/user.go and the generated column.
In `@model/usedata.go`:
- Around line 44-46: Update the snapshot cleanup and retry flow associated with
quotaDataSnapshotRetentionSeconds so idempotency markers are not deleted while
their snapshots remain retryable. Retain each marker until its snapshot is
durably retired, or enforce and persist a retry deadline shorter than the marker
retention period before cleanup removes it. Ensure the persistence and cleanup
changes use SQL supported by SQLite, MySQL 5.7.8+, and PostgreSQL 9.6+.
In `@model/user_update_test.go`:
- Around line 947-961: The migration test’s fixed delay does not guarantee
worker startup, and its final receive can hang; add an explicit worker-start
handshake and replace unbounded completion receives with timeout-based waits.
Apply this to model/user_update_test.go ranges 947-961, 985-999, and 1027-1041,
covering migrateUserOAuthIdentityConstraints and the corresponding OAuth and
field update workers; preserve the lock-held assertion while ensuring each
worker has started before checking blocked completion.
In `@model/user.go`:
- Around line 270-305: Update the PostgreSQL and MySQL lock-handling branches
around releasePostgreSQLAdvisoryLock and releaseMySQLNamedLock so released is
set to true only after a successful unlock; allow the deferred cleanup to retry
when release fails. Preserve both fn(conn) and release errors when both occur,
returning the combined failure instead of discarding the release error.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 319d2ec9-20c9-4bd1-a1c3-f844635b084c
📒 Files selected for processing (7)
.env.examplecontroller/oauth.gomodel/main.gomodel/usedata.gomodel/usedata_test.gomodel/user.gomodel/user_update_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:
controller/oauth.gomodel/main.gomodel/usedata.gomodel/user_update_test.gomodel/user.gomodel/usedata_test.go
🪛 ast-grep (0.44.1)
model/main.go
[error] 530-530: SQL query is built by concatenating a string literal with a variable and passed to a database/sql call (Query, Exec, QueryRow, Prepare, or their Context variants). String concatenation lets attacker-controlled input alter the query structure, enabling SQL injection. Use parameterized queries with placeholders ('?' or '') and pass the values as separate arguments instead of concatenating them into the query string.
Context: tx.Exec("LOCK TABLE " + quoteDBIdentifier("users") + " IN SHARE ROW EXCLUSIVE MODE")
Note: [CWE-89] Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection').
(sql-injection-query-string-concat-go)
🪛 OpenGrep (1.25.0)
model/main.go
[ERROR] 531-531: SQL query built via fmt.Sprintf or string concatenation passed to a database method. Use parameterized queries with placeholder arguments.
(coderabbit.sql-injection.go-query-format)
🔇 Additional comments (5)
model/usedata_test.go (1)
291-321: LGTM!.env.example (1)
36-38: LGTM!controller/oauth.go (1)
402-402: The write lock does not close the OAuth uniqueness race.The lock starts after the checks at Lines 284 and 305. Two requests can both observe “not taken” before serializing their inserts, so the previously reported need for database-enforced uniqueness remains.
model/main.go (2)
470-545: LGTM!
633-652: LGTM!
|
@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/user.go (1)
264-307: 🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy liftDo not globally serialize every user update behind the OAuth migration lock.
UpdateandUpdateFieldsacquire this single lock even for unrelated fields, serializing writes across all users; MySQL callers can also fail after the 10-second lock timeout. Restrict coordination to OAuth-identity mutations or use migration-specific read/write fencing.🤖 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/user.go` around lines 264 - 307, The user OAuth identity lock must not guard unrelated user updates. Update the callers of withUserOAuthIdentityMutationLock, including Update and UpdateFields, so only mutations of OAuth identity fields use this lock; leave ordinary user-field writes on their existing path. Preserve coordination for OAuth migration-sensitive changes without imposing a global per-user-update lock or MySQL timeout failures.
🤖 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/usedata.go`:
- Around line 1043-1073: The cleanup logic around quotaDataRetryableSnapshotIDs
must use durable retry state instead of the process-local CacheQuotaData before
deleting QuotaDataSnapshot markers. Update model/usedata.go lines 1043-1073 to
persist and query retry eligibility using a schema compatible with SQLite, MySQL
5.7.8+, and PostgreSQL 9.6+, while preserving batch limits and deletion
behavior; update model/usedata_test.go lines 323-359 to simulate an empty local
cache while another instance owns the retry and verify the marker is retained.
---
Outside diff comments:
In `@model/user.go`:
- Around line 264-307: The user OAuth identity lock must not guard unrelated
user updates. Update the callers of withUserOAuthIdentityMutationLock, including
Update and UpdateFields, so only mutations of OAuth identity fields use this
lock; leave ordinary user-field writes on their existing path. Preserve
coordination for OAuth migration-sensitive changes without imposing a global
per-user-update lock or MySQL timeout failures.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 430bd715-4966-4f84-b666-2527a3ff10b4
📒 Files selected for processing (6)
controller/oauth.gomodel/main.gomodel/usedata.gomodel/usedata_test.gomodel/user.gomodel/user_update_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:
controller/oauth.gomodel/main.gomodel/user_update_test.gomodel/usedata.gomodel/usedata_test.gomodel/user.go
🔇 Additional comments (6)
controller/oauth.go (1)
284-315: LGTM!Also applies to: 402-412
model/main.go (1)
509-513: LGTM!Also applies to: 551-563, 642-643, 672-680
model/usedata_test.go (1)
233-259: LGTM!model/user.go (1)
10-23: LGTM!Also applies to: 69-73, 86-96, 310-321, 378-400, 962-988, 1330-1339
model/user_update_test.go (2)
980-993: The startup handshake still permits false positives.
startedcloses beforerun()attempts the lock, so the goroutine can be descheduled there and the 50 ms branch can pass without testing contention. Use an acquisition-attempt test hook or equivalent deterministic synchronization; this affects all three helper callers.
821-857: LGTM!
|
@coderabbitai review |
✅ Action performedReview finished.
|
Important
📝 变更描述 / Description
(简述：做了什么？为什么这样改能生效？请基于你对代码逻辑的理解来写，避免粘贴未经整理的内容)
🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
Bug fix，我已提交或关联对应 Issue，且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。📸 运行证明 / Proof of Work
(请在此粘贴截图、关键日志或测试报告，以证明变更生效)