Files
tokens-reef/docs/reviews/2026-04-20-code-review-report.md

120 lines
6.5 KiB
Markdown
Raw Normal View History

# 2026-04-20 Code Review Report
## Scope
本次 review 以代码实现和真实验证结果为主,不以停用业务说明或文档状态作为主判断依据。重点覆盖:
- 后端 Go 代码实现与测试执行结果
- 前端 Vue/TypeScript 实现与质量门禁结果
- 高风险管理操作的真实代码路径
## Validation Summary
已执行的验证:
- `backend`: `go test ./...`
- `backend`: 定向复现 `internal/pkg/logger` 相关测试
- `frontend`: `npm run test:run`
- `frontend`: `npm run typecheck`
- `frontend`: `npm run lint:check`
验证结论:
- 后端除 `internal/pkg/logger` 外,其余项目包测试通过
- 前端 `vitest` 59 个测试文件、354 个测试全部通过
- 前端 `typecheck` 失败
- 前端 `lint:check` 失败
## Findings
### 1. High: Windows 下 logger `Sync()` 在标准输出被 pipe 接管时会卡死
证据:
- [logger.go#L199](/D:/project/sub2api-merge/backend/internal/pkg/logger/logger.go#L199)
- [logger.go#L274](/D:/project/sub2api-merge/backend/internal/pkg/logger/logger.go#L274)
- [logger.go#L275](/D:/project/sub2api-merge/backend/internal/pkg/logger/logger.go#L275)
- [logger.go#L404](/D:/project/sub2api-merge/backend/internal/pkg/logger/logger.go#L404)
- [logger_test.go#L12](/D:/project/sub2api-merge/backend/internal/pkg/logger/logger_test.go#L12)
- [logger_test.go#L60](/D:/project/sub2api-merge/backend/internal/pkg/logger/logger_test.go#L60)
- [logger_test.go#L132](/D:/project/sub2api-merge/backend/internal/pkg/logger/logger_test.go#L132)
- [logger_test.go#L169](/D:/project/sub2api-merge/backend/internal/pkg/logger/logger_test.go#L169)
实际验证中,`go test ./...``internal/pkg/logger` 失败,`TestInit_DualOutput` 超时 10 分钟,阻塞栈停在 `FlushFileBuffers -> zap Sync -> logger.Sync()`。这说明当前实现对 Windows pipe 场景不安全,不只是测试代码问题。
### 2. High: “删除全部未使用兑换码”只删除前 1000 条
证据:
- [RedeemView.vue#L737](/D:/project/sub2api-merge/frontend/src/views/admin/RedeemView.vue#L737)
- [RedeemView.vue#L740](/D:/project/sub2api-merge/frontend/src/views/admin/RedeemView.vue#L740)
- [RedeemView.vue#L749](/D:/project/sub2api-merge/frontend/src/views/admin/RedeemView.vue#L749)
- [RedeemView.vue#L750](/D:/project/sub2api-merge/frontend/src/views/admin/RedeemView.vue#L750)
当前实现固定只拉取第 1 页、每页 1000 条未使用兑换码,然后直接批量删除并提示成功。当未使用兑换码总数大于 1000 时,剩余数据会被静默遗漏。
### 3. Medium-High: 兑换码生成非事务化,叠加幂等重试后存在“部分成功再放大”风险
证据:
- [admin_service.go#L2056](/D:/project/sub2api-merge/backend/internal/service/admin_service.go#L2056)
- [admin_service.go#L2072](/D:/project/sub2api-merge/backend/internal/service/admin_service.go#L2072)
- [admin_service.go#L2092](/D:/project/sub2api-merge/backend/internal/service/admin_service.go#L2092)
- [redeem_handler.go#L103](/D:/project/sub2api-merge/backend/internal/handler/admin/redeem_handler.go#L103)
- [idempotency.go#L406](/D:/project/sub2api-merge/backend/internal/service/idempotency.go#L406)
- [idempotency.go#L410](/D:/project/sub2api-merge/backend/internal/service/idempotency.go#L410)
服务层逐条 `Create`,中间任一点失败都会提前返回;幂等层则把执行错误记为 `failed_retryable`。如果请求已经成功插入部分兑换码,再按同一个 key 重试,存在超量生成的可能。
### 4. Medium: 兑换码批量删除会吞掉单条失败,并整体返回成功
证据:
- [admin_service.go#L2104](/D:/project/sub2api-merge/backend/internal/service/admin_service.go#L2104)
- [admin_service.go#L2107](/D:/project/sub2api-merge/backend/internal/service/admin_service.go#L2107)
- [admin_service.go#L2111](/D:/project/sub2api-merge/backend/internal/service/admin_service.go#L2111)
- [redeem_handler.go#L243](/D:/project/sub2api-merge/backend/internal/handler/admin/redeem_handler.go#L243)
- [redeem_handler.go#L252](/D:/project/sub2api-merge/backend/internal/handler/admin/redeem_handler.go#L252)
- [redeem_handler.go#L253](/D:/project/sub2api-merge/backend/internal/handler/admin/redeem_handler.go#L253)
- [admin_service_delete_test.go#L534](/D:/project/sub2api-merge/backend/internal/service/admin_service_delete_test.go#L534)
- [admin_service_delete_test.go#L543](/D:/project/sub2api-merge/backend/internal/service/admin_service_delete_test.go#L543)
当前实现只累计成功删除数,失败项不会中断,也不会返回失败明细。更关键的是,测试已经把这种语义固化成预期行为,因此这是实现策略问题,不是单纯的漏测。
### 5. Medium: 前端测试全绿,但静态质量门禁仍然损坏
证据:
- [SoraAdminView.vue#L243](/D:/project/sub2api-merge/frontend/src/views/admin/SoraAdminView.vue#L243)
- [SoraAdminView.vue#L259](/D:/project/sub2api-merge/frontend/src/views/admin/SoraAdminView.vue#L259)
- [Icon.vue#L16](/D:/project/sub2api-merge/frontend/src/components/icons/Icon.vue#L16)
- [Icon.vue#L17](/D:/project/sub2api-merge/frontend/src/components/icons/Icon.vue#L17)
- [AppSidebar.spec.ts#L24](/D:/project/sub2api-merge/frontend/src/components/layout/__tests__/AppSidebar.spec.ts#L24)
- [package.json#L8](/D:/project/sub2api-merge/frontend/package.json#L8)
- [package.json#L11](/D:/project/sub2api-merge/frontend/package.json#L11)
- [package.json#L12](/D:/project/sub2api-merge/frontend/package.json#L12)
- [package.json#L14](/D:/project/sub2api-merge/frontend/package.json#L14)
`vitest` 全绿并不代表前端处于可交付状态。当前 `typecheck``lint:check` 都失败,说明测试集没有覆盖编译期约束与静态门禁。
### 6. Low: 仍有部分管理接口是占位实现
证据:
- [admin_service.go#L765](/D:/project/sub2api-merge/backend/internal/service/admin_service.go#L765)
- [redeem_handler.go#L284](/D:/project/sub2api-merge/backend/internal/handler/admin/redeem_handler.go#L284)
- [proxy_handler.go#L262](/D:/project/sub2api-merge/backend/internal/handler/admin/proxy_handler.go#L262)
- [dashboard_handler.go#L180](/D:/project/sub2api-merge/backend/internal/handler/admin/dashboard_handler.go#L180)
这批接口目前主要是 mock/占位返回。它们不是这次最关键的问题,但如果继续对外暴露,建议明确收敛策略。
## Overall Assessment
本次代码库的主要问题集中在三类:
- Windows 环境下的日志稳定性
- 管理端兑换码批量操作的语义正确性
- 前端“测试通过但质量门禁失败”的交付断层
其中,日志 `Sync()` 卡死和兑换码批量操作缺陷建议优先处理。