ai-customer-service/docs/CODE_REVIEW_REPORT_SYSTEMATIC_2026-05-11.md

# AI-Customer-Service 系统性代码审查报告

> 审查日期：2026-05-11
> 审查范围：全仓库代码、构建、测试、安全、架构
> 审查人：Hermes Agent (go-project-review + two-stage-review)
> 基线 commit：`67922c5` (HEAD)

---

## 1. 项目规模总览

| 维度 | 数值 | 备注 |
|------|------|------|
| Go 源文件 | 109 | 含测试文件 |
| 生产代码行 | ~4,347 | 不含测试、注释、空行 |
| 测试代码行 | ~11,072 | 含 integration + e2e |
| SQL migration 行 | 130 | 3 个 migration 文件 |
| 模块依赖 | 2 | `github.com/google/uuid`, `github.com/lib/pq` |
| Go 版本 | 1.22 | 与 Dockerfile 一致 |
| 单服务架构 | 是 | `cmd/ai-customer-service` 单一入口 |

### 包级覆盖率（internal/）

| 包 | 覆盖率 | 状态 |
|----|--------|------|
| `internal/platform/health` | 100.0% | ✅ |
| `internal/platform/logging` | 100.0% | ✅ |
| `internal/service/handoff` | 100.0% | ✅ |
| `internal/service/intent` | 100.0% | ✅ |
| `internal/service/reply` | 100.0% | ✅ |
| `internal/domain/error/cserrors` | 97.2% | ✅ |
| `internal/http/middleware` | 92.0% | ✅ |
| `internal/service/dialog` | 88.5% | ✅ |
| `internal/store/memory` | 85.6% | ✅ |
| `internal/config` | 86.1% | ✅ |
| `internal/platform/httpx` | 83.3% | ✅ |
| `internal/http/handlers` | 79.9% | ⚠️ |
| `internal/http` | 80.2% | ⚠️ |
| `internal/service/platformevents` | 84.0% | ⚠️ |
| `internal/service/platformdelivery` | 65.2% | ⚠️ |
| `internal/platformadapter` | 66.1% | ⚠️ |
| `internal/app` | 64.6% | ⚠️ |
| `internal/domain/platformevent` | 58.8% | ⚠️ |
| `internal/store/postgres` | ~10.0% | ❌ 测试因缺少 DB 失败 |
| **internal/ 总覆盖率** | **63.4%** | ⚠️ |

---

## 2. 构建与测试验证

### 2.1 静态验证

| 检查项 | 结果 | 详情 |
|--------|------|------|
| `go build ./...` | ✅ 通过 | 零错误 |
| `go vet ./...` | ✅ 通过 | 零警告 |
| `go test -race ./internal/...` | ✅ 通过 | 零 DATA RACE |
| `go test ./internal/... -count=1 -p 1` | ✅ 24/24 通过 | 全部通过 |
| `go test ./... -count=1 -p 1` | ⚠️ 部分失败 | `postgres`, `e2e`, `integration` 因缺少本地 PostgreSQL（端口 5434）失败 |

### 2.2 测试环境缺口

- **postgres 包**：21 个测试全部因 `dial tcp 127.0.0.1:5434: connection refused` 失败
- **e2e 包**：2 个测试同样因缺少 PostgreSQL 失败
- **integration 包**：1 个测试同样因缺少 PostgreSQL 失败

**判断**：这属于**环境前置条件缺失**，不是代码缺陷。但 CI 若未配置共享测试 DB，则本地无法完整验证全链路。

---

## 3. 安全审查

### 3.1 SQL 注入风险

| 检查项 | 结果 |
|--------|------|
| `fmt.Sprintf.*SELECT/INSERT/UPDATE/DELETE` 危险模式 | ✅ 未发现 |
| `$1, $2...` 参数化查询占位符 | ✅ 29 处 |
| 字符串拼接 SQL | ✅ 未发现 |

**结论**：PostgreSQL 层全面使用参数化查询，SQL 注入风险为零。

### 3.2 硬编码凭证

| 检查项 | 结果 |
|--------|------|
| 代码中硬编码 password/secret/api_key | ✅ 未发现 |
| 配置读取 secret 的代码 | ✅ 仅有正常的 `getEnv("AI_CS_WEBHOOK_SECRET", "")` |
| 环境变量示例文件 | ⚠️ `.env.platform-adapters.example` 存在，但无真实值 |

### 3.3 敏感日志泄露

| 检查项 | 结果 |
|--------|------|
| log.*password/secret/token/key | ✅ 未发现 |
| audit 日志中是否记录敏感字段 | ✅ audit payload 仅记录 intent/reply/error_code 等业务字段 |

### 3.4 Webhook 安全机制

- **HMAC-SHA256** 签名验证 ✅
- **时间戳防重放**（默认 300 秒偏移窗口）✅
- **常量时间比较** `hmac.Equal` ✅
- **请求体回置** `r.Body = io.NopCloser(bytes.NewReader(body))` ✅
- **平台级秘钥隔离** Sub2API / NewAPI 各自独立 secret ✅

### 3.5 goroutine 安全

- `app.go:184` 启动 callback worker：`go worker.Start(workerCtx)`
- worker 通过 `context.WithCancel` 管理生命周期
- `Shutdown` 方法中调用 `cancel()` 并通过 `workerClosers` 链式关闭
- **判断**：有明确的取消路径，无泄漏风险 ✅

### 3.6 Token / 密钥文件泄露

| 检查项 | 结果 |
|--------|------|
| `*.pem`, `*.key`, `token.txt` | ✅ 未发现 |
| `.env*` 文件 | ⚠️ 仅 `.env.platform-adapters.example`（示例值，无真实凭证）|

---

## 4. 代码质量与架构审查

### 4.1 架构分层（A 级）

```
cmd/                → 启动入口（main.go 干净，仅装配 + signal 处理）
internal/app/       → 依赖注入装配中心（New 函数显式创建所有组件）
internal/domain/    → 纯领域模型（无外部依赖）
internal/service/   → 业务逻辑（dialog/intent/reply/handoff/delivery/events）
internal/http/      → HTTP 传输层（handlers + middleware + router）
internal/store/     → 持久化抽象（memory + postgres 双实现）
internal/platform/  → 基础设施（health/httpx/logging）
```

**优点**：
- `dialog.Service` 仅通过接口依赖（`SessionRepository`, `AuditRepository`, `TicketRepository`, `DedupRepository`, `IntentRecognizer`, `HandoffDecider`），可测试性高
- `app.go` 作为装配中心，清晰表达组件依赖图
- memory store 与 postgres store 可互换，支持 test/dev/prod 多环境

### 4.2 配置管理（A- 级）

- 全量环境变量驱动，无配置文件硬编码
- `config.Load()` 包含 10+ 项校验规则：
  - production 强制要求 Postgres + Webhook Secret
  - 平台适配器启用时强制要求 Ingress Secret
  - 正整数校验（timeout、batch size、retry schedule）
- **建议**：`getEnvInt` / `getEnvBool` 在解析失败时静默回退到 fallback 值，这在生产环境可能导致预期外行为。建议对关键配置（如 DSN、secret）增加 "解析失败即报错" 的严格模式。

### 4.3 数据库设计（A 级）

- Migration 文件使用 Flyway 风格命名（`0001_init.up.sql`）
- 约束完整：CHECK 约束覆盖 channel、status、priority 枚举值
- 索引合理：`(channel, open_id)`、`(status, priority, created_at)`、`(session_id, created_at DESC)`
- 外键正确：ON DELETE CASCADE / SET NULL
- Outbox 模式：平台事件通过 `cs_platform_event_outbox` + `cs_platform_event_dead_letters` 实现可靠投递

### 4.4 限流与防护（B+ 级）

- `httpx.RateLimiter`：滑动窗口限流，基于 IP / X-Forwarded-For
- `http.MaxBytesReader`：请求体大小限制
- **P1 风险**：RateLimiter 的 `Allow` 方法每次请求都重新分配 `valid` 切片（`var valid []time.Time`），在高并发下产生 GC 压力。建议改为原地过滤或环形缓冲区。
- **P2 风险**：`clientIP` 和 `rateLimitKey` 中对 IPv6 地址的处理可能不正确（IPv6 地址包含多个 `:`，`strings.LastIndex(addr, ":")` 会在第一个冒号处截断）。

### 4.5 认证授权（B 级）

- Webhook 层：HMAC 签名验证 ✅
- 内部 API 层：基于 Header 的 RBAC（`X-CS-Actor-ID`, `X-CS-Actor-Role`）
- **P1 风险**：`RequireRoles` 中间件**不验证 Actor 身份真实性**，仅检查 header 存在性和角色是否在白名单。这在生产环境中需要配合反向代理（如 API Gateway）的 JWT/OAuth 验证，否则存在 header 伪造风险。文档中需明确标注此信任边界。

### 4.6 错误处理（A- 级）

- 统一的错误码体系（`cserrors` 包，100%+ 覆盖率）
- 错误码分级：`CS_AUTH_403x`、`CS_REQ_400x`、`CS_SYS_500x`、`CS_SES_400x`
- audit 写入失败不阻断主流程（` _ = s.Audit.Add(...)`），符合 P0 质量标准
- **建议**：部分 `fmt.Errorf("db is nil")` 错误信息可以更丰富（如包含调用上下文）。

### 4.7 Graceful Shutdown（A 级）

- 10 秒超时 `context.WithTimeout(context.Background(), 10*time.Second)`
- 关闭顺序：SetReady(false) → SetLive(false) → Server.Shutdown → closers 链式调用
- worker cancel 包含在 closers 中

---

## 5. 已知问题清单

### P0 — 阻塞级

| # | 问题 | 位置 | 影响 | 建议 |
|---|------|------|------|------|
| P0-1 | **未提交改动规模过大** | 全仓库 | 评审边界模糊、回滚困难、CI 基线不稳定 | `git status` 显示 15+ 个 modified 文件 + 3 个 untracked 文件。建议立即收口，分批次提交并打 tag |
| P0-2 | **Makefile test 目标与 README/CI 不一致** | `Makefile:2` | 本地执行 `make test` 时并发跑测试，可能污染共享 PostgreSQL 测试库 | 将 `Makefile` 中 `go test ./...` 改为 `go test ./... -count=1 -p 1`，与 README 和 CI 保持一致 |

### P1 — 必须修复

| # | 问题 | 位置 | 影响 | 建议 |
|---|------|------|------|------|
| P1-1 | **ticket_handler.List 覆盖率 0%** | `internal/http/handlers/ticket_handler.go:33` | 列表接口无回归保护 | 补充单元测试或 integration 测试 |
| P1-2 | **newapi_adapter.BuildIngressAck 覆盖率 0%** | `internal/platformadapter/newapi_adapter.go:24` | NewAPI 占位逻辑无验证 | 补充 501 响应测试 |
| P1-3 | **Authz header 伪造风险未文档化** | `internal/http/middleware/authz.go` | 内部 API 若直接暴露可被绕过 | 在 `docs/SECURITY_BOUNDARY.md` 中明确标注：RequireRoles 依赖上游网关做真实身份验证 |
| P1-4 | **RateLimiter GC 压力** | `internal/platform/httpx/limits.go:67` | 高并发下频繁分配切片 | 改为原地过滤或预分配环形缓冲区 |
| P1-5 | **IPv6 地址截断风险** | `internal/platform/httpx/limits.go:110-114` | IPv6 地址在 rate limit key 中被错误截断 | 使用 `net.SplitHostPort` 替代手动字符串操作 |

### P2 — 建议修复

| # | 问题 | 位置 | 影响 | 建议 |
|---|------|------|------|------|
| P2-1 | **配置解析失败静默回退** | `internal/config/config.go:201-255` | 拼写错误的环境变量值被静默忽略 | 对生产模式的关键配置增加解析失败报错 |
| P2-2 | **callback worker 无连接池限制** | `internal/app/app.go:172` | `&http.Client{Timeout: ...}` 使用默认连接池 | 显式配置 `Transport.MaxIdleConns` 和 `MaxIdleConnsPerHost` |
| P2-3 | **缺少 SQLite/内存测试回退** | `test/integration`, `test/e2e` | 无 PostgreSQL 时无法跑 integration/e2e | 引入 testcontainers-go 或 SQLite 内存模式作为测试回退 |
| P2-4 | **平台事件 worker 缺少优雅关闭等待** | `internal/app/app.go:164-188` | `cancel()` 后立即返回，不等待 worker goroutine 真正退出 | 使用 `sync.WaitGroup` 等待 worker 完成当前轮次 |

---

## 6. 与 spec 的合规性检查（Stage 1）

基于 `IMPLEMENTATION_PLAN.md` 和 `README.md` 的 spec 对照：

| Spec 项 | 实现状态 | 位置 |
|---------|----------|------|
| HTTP 服务启动 | ✅ 完成 | `cmd/ai-customer-service/main.go` |
| Health/Live/Ready | ✅ 完成 | `internal/platform/health`, `internal/http/handlers/health_handler.go` |
| Webhook 接收最小 JSON | ✅ 完成 | `internal/http/handlers/webhook_handler.go` |
| Session 管理（内存 + Postgres） | ✅ 完成 | `internal/store/memory/session_store.go`, `internal/store/postgres/session_store.go` |
| Intent 识别（规则版） | ✅ 完成 | `internal/service/intent/service.go` |
| Reply 生成（规则版） | ✅ 完成 | `internal/service/reply/service.go` |
| Handoff 转人工 | ✅ 完成 | `internal/service/handoff/service.go` |
| Audit 日志 | ✅ 完成 | `internal/store/memory/audit_store.go`, `internal/store/postgres/audit_store.go` |
| PostgreSQL 持久化 | ✅ 完成 | `internal/store/postgres/` |
| Migration | ✅ 完成 | `db/migration/` |
| HMAC Webhook 安全校验 | ✅ 完成 | `internal/http/handlers/webhook_security.go` |
| Sub2API 平台适配 | ✅ 完成 | `internal/platformadapter/sub2api_adapter.go` |
| NewAPI 平台适配 | ⚠️ 501 占位 | `internal/platformadapter/newapi_adapter.go` |
| OpenAPI 占位文档 | ⚠️ 未找到 | `internal/openapi/` 目录为空？ |
| Rate Limiting | ✅ 完成 | `internal/platform/httpx/limits.go` |
| Ticket 工作流（assign/resolve/close） | ✅ 完成 | `internal/store/postgres/ticket_workflow.go` |
| Platform Event Outbox + Dead Letter | ✅ 完成 | `internal/store/postgres/platform_event_store.go` |
| Callback Worker + Retry | ✅ 完成 | `internal/service/platformdelivery/worker.go` |

**OpenAPI 文档缺口**：spec 要求 "生成最小 OpenAPI 占位文档"，但 `internal/openapi/` 目录为空，需确认是否已迁移到 `docs/` 或其他位置。

---

## 7. 依赖健康度

| 依赖 | 版本 | 状态 | 备注 |
|------|------|------|------|
| `github.com/google/uuid` | v1.6.0 | ✅ 最新稳定 | — |
| `github.com/lib/pq` | v1.10.9 | ✅ 最新稳定 | 注意：pgx v5 是更现代的替代，但 pq 仍被维护 |

**建议**：`lib/pq` 已进入维护模式，官方推荐新项目使用 `pgx`。若未来需要连接池高级功能（如 statement cache、batch insert），建议迁移到 `pgx/v5`。

---

## 8. 交付风险

| 风险 | 等级 | 说明 |
|------|------|------|
| Dirty worktree | 🔴 高 | 15+ modified + 3 untracked，未收口前不应视为可发布基线 |
| 测试环境依赖 | 🟡 中 | postgres/e2e/integration 测试需要本地 PostgreSQL 5434 端口 |
| Authz 信任边界 | 🟡 中 | RequireRoles 不验证身份真实性，需上游网关配合 |
| 覆盖率缺口 | 🟡 中 | ticket_handler.List、newapi_adapter、postgres 包覆盖不足 |
| 依赖维护 | 🟢 低 | lib/pq 长期维护模式，但当前无功能缺口 |

---

## 9. 结论与评级

### 综合评级：B+（良好，有条件可进入下一阶段）

| 维度 | 评级 | 说明 |
|------|------|------|
| 架构设计 | A | 分层清晰，依赖注入完整，接口隔离到位 |
| 安全基线 | A- | SQL 注入零风险，Webhook HMAC 完善，Authz 信任边界需文档化 |
| 测试覆盖 | B | 核心业务逻辑覆盖良好，但 postgres 层和 e2e 受环境限制无法验证 |
| 代码质量 | B+ | 零 vet 警告，零 race，命名规范，错误码体系完整 |
| 交付就绪 | C+ | **Dirty worktree 严重**，未收口前不可视为可发布基线 |

### 下一步行动

1. **立即收口**：将当前 dirty worktree 分批次提交，核心代码与文档变更分开 commit
2. **修复 P0-2**：同步 Makefile test 目标与 README/CI 的 `-p 1` 约束
3. **补充 P1 项测试**：ticket_handler.List、newapi_adapter 的 501 响应
4. **文档化 Authz 信任边界**：在 `docs/SECURITY_BOUNDARY.md` 中标注 RequireRoles 的前置条件
5. **评估 PostgreSQL 测试策略**：引入 testcontainers-go 或 CI 中预置测试 DB，使 e2e/integration 可在本地完整运行
6. **OpenAPI 占位文档**：确认 `internal/openapi/` 是否仍需要补充，或已迁移至其他位置

---

## 附录：审查工具链输出

```bash
# 构建
cd /home/long/project/ai-customer-service && go build ./...
# → exit 0, 零输出

# 静态分析
cd /home/long/project/ai-customer-service && go vet ./...
# → exit 0, 零输出

# Race 检测
cd /home/long/project/ai-customer-service && go test -race ./internal/... -count=1 -p 1
# → 24/24 ok, 零 DATA RACE

# 覆盖率（internal/）
cd /home/long/project/ai-customer-service && go test ./internal/... -coverprofile=/tmp/ai_cs_cov.out -count=1 -p 1
go tool cover -func=/tmp/ai_cs_cov.out | grep "^total"
# → total: (statements) 63.4%

# SQL 注入扫描
grep -rn "fmt\.Sprintf.*SELECT\|fmt\.Sprintf.*INSERT\|fmt\.Sprintf.*UPDATE\|fmt\.Sprintf.*DELETE" internal/ --include="*.go" | grep -v _test.go
# → 零匹配

# 参数化查询验证
grep -rn "\$[0-9]" internal/store/postgres --include="*.go" | grep -v _test.go | wc -l
# → 29

# 硬编码凭证扫描
grep -rn "password.*=\|secret.*=\|api_key.*=\|token.*=.*\"" internal/ --include="*.go" | grep -v "_test.go\|testutil\|factory\|Config\|struct {"
# → 仅 webhook security 的配置读取代码，无硬编码

# Git 状态
git status --short
# → 15+ modified, 3 untracked
```