Files
codex-proxy-rs/docs/code-quality-audit.md
T
2026-06-14 16:07:57 +08:00

404 lines
18 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# 代码质量审计与改造计划
日期:2026-06-14
范围:当前仓库 `src/`、`tests/`,不包含 `designs/`。
目标:确认 `/tmp/code_quality_audit.md` 的结论,并基于当前代码给出后续改造顺序。项目处于早期阶段,不需要保留低质量兼容层;改造应以清晰边界、可测试、可维护为目标。
## 审计原则
1. 不为了“拆小文件”而拆文件。文件大小本身不是问题,问题是职责混杂、变更理由混在一起、审查和定位成本过高。
2. 不用“兼容历史”掩盖坏设计。内部 API 可以在测试保护下重构,外部行为必须由测试证明未回退。
3. Rust 代码遵循本项目的 `$rust-best-practices`:明确错误类型、少 clone、少分配、少隐式 panic,但性能优化要有证据。
4. 日志输出使用中文,保留必要英文专业术语,例如 WebSocket、SSE、OAuth、quota、token。
5. 每个改造切片先写或补齐测试,再改生产代码,最后跑目标测试、`cargo fmt --check`、`cargo clippy --all-targets --all-features --locked -- -D warnings`。
## 审计时量化事实
审计开始时 `src/` + `tests/` 约 37,055 行。生产代码里最需要关注的文件:
| 文件 | 行数 | 判断 |
|---|---:|---|
| `src/codex/serving/dispatch/mod.rs` | 1474 | 已有子模块,但主文件仍承担请求编排、上游调用、stream audit、fallback、限流处理等多种职责 |
| `src/codex/gateway/transport/websocket.rs` | 1175 | WebSocket pool、连接建立、请求编码、SSE 转发、错误分类、rate limit 捕获混在同一文件 |
| `src/codex/accounts/repository.rs` | 1103 | 账号 CRUD、token 更新、quota、usage、lease、pool read model 混在同一存储文件 |
| `src/codex/gateway/transport/client.rs` | 783 | 上游 transport 编排较重,但当前还可接受 |
| `src/codex/accounts/pool.rs` | 627 | 调度策略集中,复杂度主要来自业务策略,不是首要拆分对象 |
## Claude 审计确认
### 1. `update_tokens` / `update_from_claims` 重复
结论:成立。
当前 `AccountRepository::update_tokens` 和 `AccountRepository::update_from_claims` 都重复处理:
- `Utc::now().to_rfc3339()`
- access token 加密
- refresh token 可选加密
- expires_at 格式化
- “没有新 refresh_token 时保留旧值”的分支
需要修复,但不建议照搬报告里的 `QueryBuilder` 方案。当前只有几个固定 SQL 形态,动态构建 SQL 会把绑定顺序变成新的维护风险。更合适的第一步是:
- 补测试:`update_from_claims` 省略 refresh_token 时必须保留旧 refresh_token。
- 提取私有 token 写入准备 helper。
- 将 UPDATE SQL 改成命名多行常量。
- 保持 `disabled` / `banned` 不被 `update_tokens` 重新激活的现有语义。
### 2. SQL 字符串地狱
结论:部分成立。
报告里的状态已经有些过期:`LIST_POOL_ACCOUNTS_SQL`、`RECORD_USAGE_SQL`、`GET_USAGE_SQL` 等已是多行常量。但问题仍存在:
- `insert/get/list/list_metadata/update_tokens/update_from_claims/quota/lease/AccountUsageRepository::list/summary` 仍有较多长单行 SQL。
- 一部分 SQL 的业务语义不容易在 review 时看清,例如 token 更新时的状态保护、quota cooldown 的最大值保留、refresh lease 的抢占条件。
建议先使用命名多行常量。`sqlx::query_file!` 不是当前第一选择,因为项目当前使用运行时 `sqlx::query`,切换到宏式编译期校验会引入额外数据库元数据/离线缓存约束,改造面比可读性收益更大。
### 3. `AccountRepository` God Class
结论:部分成立。
`AccountUsageRepository` 已经存在,说明项目已经开始拆 usage 读模型;但 `AccountRepository` 里仍混有:
- 账号 CRUD
- token 更新
- pool read model
- quota JSON 与 cooldown
- usage record/get/window sync
- refresh lease
这确实不是理想边界。后续可以拆,但不应机械拆成很多小文件。推荐按变更理由拆:
- account 基础读写
- token 写入
- quota 状态
- usage 统计写入与窗口同步
- refresh lease
- pool read model
是否暴露多个 repository 类型,需要看调用侧是否真的收益明显。可以先做内部模块拆分,再决定是否调整服务层依赖。
### 4. 超大文件
结论:成立,但“拆文件”不是目标。
三个文件已经大到影响定位和审查:
- `dispatch/mod.rs`
- `websocket.rs`
- `repository.rs`
但文件拆分必须跟职责边界一致:
- `websocket.rs` 适合按 pool、connection、request codec、SSE forwarding、error classification 拆。
- `dispatch/mod.rs` 适合把 stream audit、fallback/rate-limit 应用、上游请求执行拆到更明确的模块。
- `repository.rs` 适合先把 SQL 与行映射收拢,再拆 token/quota/usage/lease。
不建议为了把每个文件压到某个行数而过度碎片化。
### 5. `Account` clone 性能开销
结论:部分成立,不是第一优先级。
`AccountPool` 确实会 clone `Account`,而 `Account` 包含 token 字符串。这个成本存在,但当前设计也有合理性:`AccountPool` 在 `Mutex` 后面,返回 owned `Account` 可以避免把锁生命周期带入 async 上游调用。
直接改成 `Arc<Account>` 不是无成本优化,会牵涉:
- pool 内账号状态更新如何替换 snapshot
- 调度策略如何避免共享可变状态
- token 刷新和 cooldown 更新如何保持一致
建议先完成结构性重构。性能优化应结合 profile 或压测数据,再考虑 immutable snapshot + `Arc<Account>`。
### 6. 字符串分配过度
结论:审计过泛,暂不作为直接改造项。
`.to_string()` / `.clone()` 数量本身不能证明问题。当前很多转换来自:
- JSON/SSE 协议需要 owned `String`
- secret 解密后写入运行时 pool
- header、metadata、测试数据构造
- async 任务跨 await / spawn 的所有权要求
后续可以用 Clippy、profile 和热点路径 review 定点优化,不能按数量批量替换成 `Cow` 或引用。
## 我的补充审计
### P0:token 更新重复和测试缺口
`update_tokens` 已有测试覆盖“没有新 refresh_token 时保留旧值”,但 `update_from_claims` 缺同类测试。由于导入已有账号时会走 `update_from_claims`,这是第一批代码改造前必须补齐的行为测试。
改造目标:
- 增加 `update_from_claims` 保留 refresh_token 测试。
- 提取 token 写入准备逻辑。
- 将对应 UPDATE SQL 改为命名多行常量。
- 保持现有行为,不顺手重构调用层。
### P0:Repository SQL 可读性
`repository.rs` 的 SQL 可读性是当前最明显的维护问题。建议第一阶段只处理账号 repository 内的长 SQL,不扩散到全项目。
改造目标:
- 账号表常用 SELECT 语句提取为常量。
- token/quota/lease/usage summary 的长 SQL 改成多行常量。
- SQL 常量命名表达业务语义,例如 `UPDATE_TOKENS_PRESERVING_DISABLED_STATUS_SQL`。
### P1:Repository 边界
存储层已经有一些 DDD 边界,但 `AccountRepository` 仍承载太多变更理由。推荐在 SQL 可读性改善后再拆,避免一边搬文件一边改 SQL。
推荐顺序:
1. 先把 SQL 常量和 row mapper 收拢。
2. 再拆 token/quota/lease/usage 内部模块。
3. 最后评估是否需要多个 public repository。
### P1:WebSocket transport 边界
`websocket.rs` 当前实现已经覆盖原版核心链路,但职责过密:
- pool lifecycle
- idle probe / GC
- handshake request 构造
- request body 转换
- WS -> SSE 转发
- rate limit 内部事件捕获
- WS 错误分类
建议后续按“协议转换”和“连接池”先拆,不改变行为:
- `websocket/pool.rs`
- `websocket/connection.rs`
- `websocket/codec.rs`
- `websocket/stream.rs`
- `websocket/error.rs`
这个拆分是为了边界清楚,不是为了追求小文件。
### P1:Dispatch 编排边界
`dispatch/mod.rs` 已有 `affinity/fallback/refresh/routing/stream/usage` 子模块,但主文件仍过重。建议把以下逻辑迁出:
- 上游 HTTP SSE / WebSocket stream audit
- quota/rate-limit header 应用
- fallback account retry 决策
- 上游请求执行 wrapper
目标是让 `mod.rs` 更像编排入口,而不是承载所有细节。
### P1:日志一致性
当前日志大部分已经是中文,且不少已使用结构化字段。但仍需继续统一:
- 非中文日志消息需要改成中文,例如 UpdateChecker 相关日志。
- `error = %e`、`account_id = %account_id` 这类结构化字段应保持。
- 不把 token、cookie、refresh_token 放进日志。
- HTTP trace 日志已存在中文入口,应继续检查是否有 request_id/span 传播缺口。
### P2:生产代码里的 `unwrap()`
大多数 `unwrap()` 位于测试模块。生产路径中发现 `usage_snapshots.rs` 的时间桶对齐使用 `and_hms_opt(...).unwrap()`。逻辑上这些时间值可证明有效,但风格上仍建议改成有语义的 helper 或 `expect` 文案,避免以后被复制到不可证明场景。
### P2:测试文件过大
`tests/codex_gateway/websocket.rs` 约 1436 行,已经接近生产大文件的认知成本。测试拆分应按场景拆,而不是一个 endpoint 一个文件:
- WebSocket pool 复用/淘汰
- WebSocket fallback
- WS -> SSE 转换
- rate limit internal event
- error classification
### P2:注释质量
项目里已有中文注释,但后续应保持“解释为什么”,避免解释显而易见的代码行为。适合保留的注释包括:
- token/refresh_token 保留策略
- SSE 响应头已发出后不能回滚 HTTP 状态
- 原版协议兼容的非显然参数
- 安全链路中的指纹/安装 ID/turn state 传递原因
## 推荐改造顺序
### Phase 1:账号 repository P0
目标:先处理最确定、最小风险的问题。
1. 补 `update_from_claims` refresh_token 保留测试。
2. 提取 token 更新准备 helper。
3. UPDATE token/claims SQL 改命名多行常量。
4. 账号 repository 的明显长 SQL 改命名常量。
5. 跑 `cargo test codex_accounts::repository` 或对应过滤测试,再跑 fmt/clippy。
验收条件:
- `update_tokens` 和 `update_from_claims` 不再各自重复 token 加密、expires_at 格式化和 updated_at 生成逻辑。
- `update_from_claims` 省略 refresh_token 时保留旧 refresh_token,有测试覆盖。
- `update_tokens` 仍保持现有保护:`disabled` / `banned` 账号不会因为 token 更新被重新激活。
- token/claims/quota/lease/usage summary 的长 UPDATE/SELECT SQL 不再以内联单行字符串形式散落在方法体里。
- 不引入 `QueryBuilder`,除非实际需要动态字段集合。当前固定 SQL 形态优先使用命名多行常量,降低绑定顺序风险。
- 不调整调用层、不拆 public repository API、不改数据库 schema。
- 验证命令至少包括:
- `cargo test --test codex_accounts repository --locked`
- `cargo fmt --check`
- `cargo clippy --all-targets --all-features --locked -- -D warnings`
Phase 1 不包含:
- `AccountRepository` public API 拆分。
- WebSocket/Dispatch 文件拆分。
- `Account` 改 `Arc<Account>`。
- 全局 `.clone()` / `.to_string()` 批量替换。
- 日志系统重构。
Phase 1 状态:已完成。
已完成内容:
- 增加 `update_from_claims` 省略 `refresh_token` 时保留旧 refresh token 的测试。
- 提取 token 写入准备逻辑,统一 access token 加密、可选 refresh token 加密、expires_at 格式化和 updated_at 生成。
- 将 token/claims 更新 SQL 改为命名多行常量,并保留 `disabled` / `banned` 不被 token 更新重新激活的语义。
- 将账号 repository 内 usage、quota、refresh lease 的长 SQL 和实现迁出到内部模块,public API 保持不变。
验证结果:
- `cargo test --test codex_accounts repository --locked`:13 passed。
- `cargo test prepare_token_write_should_encrypt_tokens_and_format_timestamps --locked`:1 passed。
- `cargo fmt --check`:通过。
- `cargo clippy --all-targets --all-features --locked -- -D warnings`:通过。
### Phase 2:WebSocket transport 边界
目标:在不改变 WebSocket 行为的前提下,按职责拆分 `src/codex/gateway/transport/websocket.rs`。
建议顺序:
1. 先拆错误类型和协议编解码 helper。
2. 再拆连接建立、握手请求构造、消息发送。
3. 最后拆连接池生命周期和 stream 转发。
验收条件:
- WebSocket pool 的 55 分钟 max age、25 秒 ping、串行复用语义保持不变。
- WS -> SSE 输出格式保持不变。
- rate limit internal event、turn state、set-cookie、rate-limit header 捕获保持不变。
- HTTP SSE fallback 行为保持不变。
- 不引入 IP proxy/VPN 相关能力。
Phase 2 状态:已完成。
已完成内容:
- 新增 `websocket/pool.rs`,集中 WebSocket pool key、pool config、55 分钟 max age、keepalive、GC、shutdown、account cap、busy key bypass 等池化生命周期逻辑。
- 新增 `websocket/codec.rs`,集中 upstream request body 转换、WS frame 文本提取、WS event 分类、WS error frame 分类、retry-after/header 提取。
- `websocket.rs` 保留对外入口、连接建立、active connection 归还/丢弃、WS -> SSE stream 编排。
- 没有引入 IP proxy/VPN 能力。
验证结果:
- `cargo test --test codex_gateway websocket --locked`:20 passed。
- `cargo fmt --check`:通过。
- `cargo clippy --all-targets --all-features --locked -- -D warnings`:通过。
### Phase 3:Dispatch 编排边界
目标:让 `src/codex/serving/dispatch/mod.rs` 更像请求编排入口,把可独立变化的细节迁到已有子模块或新的内部模块。
建议顺序:
1. 先确认已有 `affinity/fallback/refresh/routing/stream/usage` 子模块职责。
2. 优先迁出 stream audit 和 upstream response 转换逻辑。
3. 再评估 fallback/retry 决策、quota/rate-limit 应用是否需要继续迁出。
验收条件:
- 外部 API、SSE 输出、fallback 语义、quota cooldown、session affinity 保持不变。
- `dispatch/mod.rs` 不再同时承载所有 stream 细节。
- 每个迁移切片都有目标测试覆盖。
Phase 3 状态:已完成。
已完成内容:
- 新增 `dispatch/audit.rs`,集中 HTTP SSE / WebSocket stream 的审计、usage 记录、上游失败识别和账户 slot 释放。
- 扩展 `dispatch/fallback.rs`,将 fallback retry 的副作用迁入同一模块,包括 quota cooldown、Cloudflare cooldown、ban/quota 状态更新和备用账户获取。
- 新增 `dispatch/limits.rs`,集中 rate-limit header 解析、quota 缓存同步、window 同步和被动 cooldown 应用。
- `dispatch/mod.rs` 保留服务入口、请求编排和上游 client request 构造。上游 request 构造暂不硬拆,避免为了降行数引入复杂 lifetime/owned context 设计。
验证结果:
- `cargo test --test codex_serving --locked`:57 passed。
- `cargo fmt --check`:通过。
- `cargo clippy --all-targets --all-features --locked -- -D warnings`:通过。
### Phase 4:日志、panic 点和测试组织收尾
目标:处理不会改变业务行为但影响长期维护质量的问题。
检查项:
1. 日志消息使用中文,保留 WebSocket/SSE/OAuth/quota/token 等专业术语。
2. 生产代码中避免无语义的 `unwrap()` / `expect()`。
3. 测试文件按场景组织,不为了拆小而过度拆分。
4. 继续避免 token、cookie、refresh token 等敏感信息进入日志。
Phase 4 状态:已完成。
已完成内容:
- 将 fingerprint 更新检查器日志中的 `UpdateChecker` 英文前缀改为中文表达,保留 fingerprint 专业术语。
- 清理 `usage_snapshots.rs` 生产路径中的时间桶对齐 `unwrap()`,改为无 panic 的 helper;测试代码中的 unwrap 保留。
- 将 `tests/codex_gateway/websocket.rs` 中 WebSocket pool 场景迁到 `tests/codex_gateway/websocket/pool.rs`,保留共享 helper 和非 pool 场景测试在父文件。
- 不继续拆更细的测试文件,避免把一个协议场景拆成过多小模块。
验证结果:
- `cargo test usage_snapshots --locked`:4 passed。
- `cargo test --test codex_gateway websocket --locked`:20 passed。
- `cargo fmt --check`:通过。
- `cargo clippy --all-targets --all-features --locked -- -D warnings`:通过。
## 改造后边界
本轮改造没有追求小文件数量,而是按变更理由拆分。当前关键文件规模如下:
| 文件 | 行数 | 当前职责 |
|---|---:|---|
| `src/codex/accounts/repository.rs` | 705 | 账号基础读写、row mapper、pool read model |
| `src/codex/accounts/repository/token.rs` | 203 | token/claims 写入和 refresh_token 保留策略 |
| `src/codex/accounts/repository/quota.rs` | 125 | quota JSON、quota cooldown、Cloudflare cooldown |
| `src/codex/accounts/repository/usage.rs` | 403 | usage 累计、window 同步、usage 列表和汇总 |
| `src/codex/accounts/repository/lease.rs` | 51 | refresh lease 抢占和释放 |
| `src/codex/gateway/transport/websocket.rs` | 506 | WebSocket 对外入口、连接建立、WS -> SSE 编排 |
| `src/codex/gateway/transport/websocket/pool.rs` | 492 | WebSocket pool 生命周期、keepalive、GC、shutdown |
| `src/codex/gateway/transport/websocket/codec.rs` | 206 | upstream body 转换、frame/event/error/header 解析 |
| `src/codex/serving/dispatch/mod.rs` | 1013 | 服务入口、请求编排、上游 request 构造 |
| `src/codex/serving/dispatch/audit.rs` | 329 | HTTP SSE/WebSocket stream 审计和账户 slot 释放 |
| `src/codex/serving/dispatch/fallback.rs` | 255 | fallback retry 分类和副作用应用 |
| `src/codex/serving/dispatch/limits.rs` | 80 | rate-limit header、quota 缓存、window/cooldown 同步 |
| `tests/codex_gateway/websocket.rs` | 880 | WebSocket 非 pool 协议场景和共享测试 helper |
| `tests/codex_gateway/websocket/pool.rs` | 559 | WebSocket pool 复用、淘汰、cap、keepalive、shutdown 场景 |
`dispatch/mod.rs` 仍然偏大,但当前保留的是请求主流程和上游 request 构造。继续硬拆需要引入额外 context 类型或更复杂的生命周期设计,收益暂时低于风险。
## 后续观察项
### 性能优化
目标:只优化有证据的热点。
1. 跑压测或 profile,确认 clone/string 分配是否是瓶颈。
2. 如果 `Account` clone 是热点,再设计 immutable account snapshot 或 `Arc<Account>`。
3. 不做无证据的全局 `Cow`/引用替换。
### 后续日志治理
本轮只处理审计中确认的日志消息一致性问题,没有重构日志系统。后续如果继续做日志最佳实践,应单独落地 HTTP/span/request_id 字段治理,避免和结构重构混在一起。