fix: prevent duplicate suno task refunds via cas status update - #6074
Conversation
WalkthroughTask polling now reconciles refundable failed tasks with CAS-protected quota markers. Timeout and platform finalization paths gate refunds on winning state transitions, while billing preserves pending markers after funding failures. Scheduler work detection includes pending refunds. ChangesTask refund reconciliation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant PlatformResult
participant updateSunoTasks
participant TaskPersistence
participant RefundTaskQuota
PlatformResult->>updateSunoTasks: provide status and fail reason
updateSunoTasks->>TaskPersistence: UpdateWithStatus(prevStatus)
TaskPersistence-->>updateSunoTasks: CAS result
updateSunoTasks->>RefundTaskQuota: refund after winning failure transition
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
移植自上游 PR QuantumNous#6074。Suno 轮询原先「先退款、后无条件 Update」， 重叠轮询/sweep/多实例场景下同一失败任务会退多次款。改为与本 fork 其余任务路径同构的 UpdateWithStatus(prevStatus) CAS：只有 状态迁移的赢者才执行 RefundTaskQuota，且仅在旧状态非 FAILURE 且 Quota 非零时退款。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
service/task_billing.go (1)
165-203: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftRefund→marker-clear ordering leaves a residual duplicate-refund window.
RefundTaskQuotaperforms the real funding/token refund first, then clears the persistedquotamarker with a plain, non-CAStask.UpdateQuota()at the end. That write failing (logged, but still returnstrue) leaves a nonzeroquotamarker on an already-refunded task.sweepUnrefundedFailedTasksinservice/task_polling.go(Lines 101-133) has no way to distinguish "never refunded" from "refunded but marker-write failed" — it will re-claim the marker and callRefundTaskQuotaagain, issuing a second real refund.This gap only matters for callers that invoke
RefundTaskQuotadirectly without pre-claiming viamodel.ClaimQuotaForRefund— i.e.sweepTimedOutTasks(service/task_polling.go Lines 88-90),updateSunoTasks(service/task_polling.go Lines 347-349), andupdateVideoSingleTask(service/task_polling.go Lines 638-639). The sweep path itself is safe because it already zeroes the marker viaClaimQuotaForRefundbefore callingRefundTaskQuota, making the trailingUpdateQuota()a no-op there.Consider having
RefundTaskQuotaitself claim (zero) the quota marker atomically before performing the external refund, and restore it viaRestoreQuotaAfterFailedRefundonly if the refund fails — mirroring the pattern already implemented insweepUnrefundedFailedTasks. That closes the gap for every caller uniformly and would let the sweep function drop its now-redundant claim/restore logic. Adding a regression test simulating a successful refund with a failing final quota write would also be valuable given this is exactly the failure mode the PR targets.🛡️ Illustrative direction (not a full patch)
func RefundTaskQuota(ctx context.Context, task *model.Task, reason string) bool { quota := task.Quota if quota == 0 { return true } + claimed, err := model.ClaimQuotaForRefund(task.ID, quota) + if err != nil { + logger.LogError(ctx, fmt.Sprintf("claim quota marker failed task %s: %s", task.TaskID, err.Error())) + return false + } + if !claimed { + return true // already claimed/refunded elsewhere + } + // 1. 退还资金来源（钱包或订阅） if err := taskAdjustFunding(task, -quota); err != nil { logger.LogWarn(ctx, fmt.Sprintf("退还资金来源失败 task %s: %s", task.TaskID, err.Error())) + if _, restoreErr := model.RestoreQuotaAfterFailedRefund(task.ID, quota); restoreErr != nil { + logger.LogError(ctx, fmt.Sprintf("restore quota marker failed task %s: %s", task.TaskID, restoreErr.Error())) + } return false } ... - task.Quota = 0 - if err := task.UpdateQuota(); err != nil { - logger.LogError(ctx, fmt.Sprintf("退款成功但清除 task quota 失败 task %s: %s", task.TaskID, err.Error())) - } + task.Quota = 0 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 `@service/task_billing.go` around lines 165 - 203, Update RefundTaskQuota to atomically claim and clear the task quota marker via the existing ClaimQuotaForRefund before performing any funding or token refund; restore it with RestoreQuotaAfterFailedRefund when the refund fails, and return without refunding if the claim is unsuccessful. Remove the redundant claim/restore flow from sweepUnrefundedFailedTasks so it delegates marker handling to RefundTaskQuota, while preserving successful refund logging and final quota persistence behavior.
🧹 Nitpick comments (1)
model/task.go (1)
311-332: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winDedup the shared "non-legacy unrefunded failure" filter.
The
status = FAILURE AND quota != 0 AND (submit_time <= 0 OR submit_time >= TaskRefundLegacyCutoff)predicate is duplicated verbatim betweenGetUnrefundedFailedTasksandHasTaskPollingWork. Extracting a shared scope avoids the two definitions silently drifting apart in a future edit. These queries also run on every poll cycle and schedulerEnabled()check (viacontroller/system_task_handlers.go) — worth double-checking there's a supporting index (e.g. onstatus+quota, orstatus+updated_at) so this doesn't become a full-table scan hotspot as thetaskstable grows.♻️ Proposed dedup
+func unrefundedFailureScope(db *gorm.DB) *gorm.DB { + return db.Where("status = ?", TaskStatusFailure). + Where("quota != ?", 0). + Where("(submit_time <= ? OR submit_time >= ?)", 0, TaskRefundLegacyCutoff) +} + func GetUnrefundedFailedTasks(updatedBefore int64, limit int) []*Task { if limit <= 0 { return nil } var tasks []*Task - err := DB.Where("status = ?", TaskStatusFailure). - Where("quota != ?", 0). - Where("updated_at <= ?", updatedBefore). - Where("(submit_time <= ? OR submit_time >= ?)", 0, TaskRefundLegacyCutoff). + err := unrefundedFailureScope(DB). + Where("updated_at <= ?", updatedBefore). Order("id"). Limit(limit). Find(&tasks).Error if err != nil { return nil } return tasks }func HasTaskPollingWork() bool { if HasUnfinishedSyncTasks() { return true } var id int64 - err := DB.Model(&Task{}). - Where("status = ?", TaskStatusFailure). - Where("quota != ?", 0). - Where("(submit_time <= ? OR submit_time >= ?)", 0, TaskRefundLegacyCutoff). + err := unrefundedFailureScope(DB.Model(&Task{})). Limit(1). Pluck("id", &id).Error return err == nil && id != 0 }Also applies to: 359-376
🤖 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/task.go` around lines 311 - 332, Extract the shared non-legacy unrefunded-failure predicate into a reusable query scope and apply it in both GetUnrefundedFailedTasks and HasTaskPollingWork, preserving their additional conditions. Check the task schema/index definitions and add or reuse a supporting index for these polling queries, following the project’s migration conventions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@service/task_billing.go`:
- Around line 165-203: Update RefundTaskQuota to atomically claim and clear the
task quota marker via the existing ClaimQuotaForRefund before performing any
funding or token refund; restore it with RestoreQuotaAfterFailedRefund when the
refund fails, and return without refunding if the claim is unsuccessful. Remove
the redundant claim/restore flow from sweepUnrefundedFailedTasks so it delegates
marker handling to RefundTaskQuota, while preserving successful refund logging
and final quota persistence behavior.
---
Nitpick comments:
In `@model/task.go`:
- Around line 311-332: Extract the shared non-legacy unrefunded-failure
predicate into a reusable query scope and apply it in both
GetUnrefundedFailedTasks and HasTaskPollingWork, preserving their additional
conditions. Check the task schema/index definitions and add or reuse a
supporting index for these polling queries, following the project’s migration
conventions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 06be9f75-51dd-4794-a154-05a0b17a583c
📒 Files selected for processing (7)
controller/system_task_handlers.gomodel/task.gomodel/task_cas_test.goservice/task_billing.goservice/task_billing_test.goservice/task_polling.goservice/task_polling_test.go
…umNous#6074) * fix: prevent duplicate suno task refunds via cas status update * fix: reconcile failed task refunds --------- Co-authored-by: CaIon <i@caion.me>
同步上游 10 个提交(至 1721144),重点整合: - QuantumNous#6329 鉴权重构:dashboard 会话全面改为无状态 token(access/refresh + 版本栅栏 + 会话管理),gin session 全部移除。fork 侧适配: - turnstile 一次性消费改为按 token 键控内存缓存(兼容发码+注册两步流) - TRUSTED_PROXY_CIDRS 作为 TRUSTED_PROXIES 的兼容别名保留 - UserBase/ToBaseUser 保留 ParentId(子号计费),补 AuthVersion/CacheSchema - OAuth 绑定改 flow_token 流,保留 GitHub 账号年龄门禁(消费 flow 后校验) - RecordUserIP 反欺诈埋点移入 setupLoginAtAuthVersion - 子号/代理鉴权门(SubPermission/RejectSubAccount/AgentAuth)原样保留 - profile 嫁接上游 LoginSessionsCard(会话管理 UI),绑定卡接入 popup+postMessage 新绑定机制 - 2FA/OAuth/微信登录后 redirect 目标经 handleLoginSuccess 传递恢复 - web/default → web/ 扁平化 + 删除 classic 主题:fork 全部前端定制 (agent/supplier/detector/sub-account 等 180+ 文件)迁移至新路径, 保留 fork 的 i18n 按需加载、每表分页记忆、主题调校与设计系统 - QuantumNous#6157 渠道代理客户端重构(别名缓存+失效清理),保留 fork 全局代理 与 RELAY_DISABLE_HTTP2;QuantumNous#6074 suno CAS 防重复退款;QuantumNous#6163 playground 自动分组;QuantumNous#6224 无限额度密钥显示已用量;QuantumNous#6032 realtime GA 去 beta 头 - 语言文件三方合并:fork ~6280 键 + 上游新增 55 键鉴权文案 - fork 刻意删除的组件与 workflows 维持删除(上次合并曾误恢复) 验证:go build/test 全绿,前端 tsgo 类型检查与 rsbuild 构建通过。 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sync 14 upstream commits. The dominant change is structural: upstream promoted the frontend from web/default to the web/ root and deleted the classic theme entirely, so most of the diff is renames and deletions. Notable upstream work: - refactor(auth): stateless dashboard tokens replacing sessions (QuantumNous#6329) - feat(channel): upstream model discovery for Codex and advanced custom channels (QuantumNous#6184, QuantumNous#5971) - fix: CAS status update prevents duplicate suno task refunds (QuantumNous#6074) - fix: no duplicate tool calls in Responses-to-Chat streaming (QuantumNous#6225) Fork-side resolutions: - Drop the classic theme, following upstream. electron/ and the release/electron-build workflows stay deleted as this fork already removed them; GHCR publishing continues via docker-build.yml. - UserBase keeps the fork's per-user Ratio alongside upstream's new Role, AuthVersion and CacheSchema fields. GetUserCache adopts upstream's cache-population path, which returns ToBaseUser() and so still carries Ratio. - web-router keeps the fork's dynamic index injector (SystemName/Logo templating) on top of upstream's renamed frontendFS. serveIndex collapses to the single-frontend WebAssets now that classic is gone. - Restore the fork's invoices feature (5 files), which git's rename detection dropped during the web/default -> web/ move, and re-register its route in routeTree.gen.ts. Backend builds and the full Go test suite passes. The frontend is not yet type-checked or built locally. Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude <noreply@anthropic.com> Co-Authored-By: Happy <yesreply@happy.engineering>
…umNous#6074) * fix: prevent duplicate suno task refunds via cas status update * fix: reconcile failed task refunds --------- Co-authored-by: CaIon <i@caion.me>
…umNous#6074) * fix: prevent duplicate suno task refunds via cas status update * fix: reconcile failed task refunds --------- Co-authored-by: CaIon <i@caion.me>
…umNous#6074) * fix: prevent duplicate suno task refunds via cas status update * fix: reconcile failed task refunds --------- Co-authored-by: CaIon <i@caion.me>
2 participants
📝 变更描述 / Description
Suno 轮询里失败退款原来是直接调
RefundTaskQuota，没有并发保护。重叠轮询、超时 sweep、多实例同时命中有机率触发重复退款。改法：同步视频任务的 CAS保护
🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
Bug fix，我已提交或关联对应 Issue，且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。📸 运行证明 / Proof of Work
触发一次suno 生成失败任务, 只退款一次
Summary by CodeRabbit