From 186cf53804a339189c41c87d1c3bc0f826fc0ddd Mon Sep 17 00:00:00 2001 From: JianFeeeee Date: Mon, 28 Sep 2026 09:49:22 +0800 Subject: [PATCH] =?UTF-8?q?fix(push):=20=E9=A2=9D=E5=BA=A6=E9=A2=84?= =?UTF-8?q?=E7=95=99**=E7=9C=9F=E7=9A=84**=E6=8C=AA=E5=88=B0=20accessToken?= =?UTF-8?q?=20=E4=B9=8B=E5=90=8E=EF=BC=88=E4=B8=8A=E4=B8=80=E7=89=88?= =?UTF-8?q?=E5=8F=AA=E5=86=99=E4=BA=86=E6=B3=A8=E9=87=8A=EF=BC=89?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 起因：pi 在邮件驱动的一轮里当场抓出来的 pi 收到那封 `[收尾验证]` 邮件后，自己翻代码核对， 在会话文件里写下（原文）： The code contradicts its own comment (item ②: reserve should be *after* accessToken) The commit only changed the argument (`len(tokens)` → `1`) and the comment — it Fix ① (per-batch) is real and tested. Fix ② is claimed but not implemented. Confirmed — the bug is real. 它甚至自己造了探针（`zz_probe_test.go`，跑完已删）来实证。 **我独立复核确认它是对的**： `reserveDaily(1)` 在第 212 行，`accessToken` 在第 215 行 —— 预留仍在**之前**。2026-09-26 那次我只改了 ①（`len(tokens)` → `1`）， 把 ② 写进了注释，**代码没动**。 ## 为什么当时那批判据没接住 `push_test.go` 原有 3 格只验 ①（按批次计），**造不出「accessToken 失败」这条路** —— `hmsStub` 的 `/token` 永远返回 200 + 令牌。 ⇒ 「注释说修了」与「代码真修了」能分家，而没有任何东西会发现。 ## 改法 ① `hmsStub` 加 `failToken` 开关（`/token` 可返回 400）。 ② `reserveDaily(1)` 挪到 `accessToken` 成功**之后**、真正发请求之前。 仍保持**前置预留**语义（不是"发成功后再扣"）—— 那会超发， 并发下多个 goroutine 都能通过检查。宁可少算也不多发。 ③ 新增 `TestHMSAccessTokenFailureDoesNotBurnQuota`：三次 accessToken 失败后 断言 `dayCount == 0`、零推送发出、且恢复正常后仍能发（额度没被吃掉）。 ## 变异验证（这格判据本该在 2026-09-26 就存在） 把 `reserveDaily` 挪回 `accessToken` 之前（= 还原成 bug）⇒ ★ accessToken 失败不该扣额度，实际已扣 3 条 （一次网络抖动静默烧配额就是这么来的） ## 教训（与本仓 python-probe-shadowing / baseline-residue 同族） **「我写了注释说明怎么修」不等于「我改了代码」。** 审查报告给了两条，我处理了一条，把另一条**誊进了注释**就当做了。 写完注释应当立刻核对行号 —— 那是 5 秒钟的事，而这次是别人替我发现的。 ★ 另一层：**别人（或另一个 Agent）独立复核出来的结论，要自己再验一遍再改**。 我逐条查了行号才动手，没有因为"pi 说的"就直接信。 --- server/internal/push/hms.go | 21 +++++---- server/internal/push/push_test.go | 72 +++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 9 deletions(-) diff --git a/server/internal/push/hms.go b/server/internal/push/hms.go index 3b62843..eb7afbf 100644 --- a/server/internal/push/hms.go +++ b/server/internal/push/hms.go @@ -200,22 +200,25 @@ func (h *HMS) Send(ctx context.Context, tokens []string, n NewMail) error { * 多设备自部署用户会按 1/设备数 的速度提前耗尽 1000 条/天。 * ⇒ 改按批次计：一次 `Send` = 一条消息。 * - * ② **扣在投递之前**：`accessToken` 失败 / HTTP 失败 / 华为回非成功码， - * 这些**一条都没发出去**，额度却已经扣了，而失败只 `log.Printf` + * ② **扣在投递之前**：原顺序是 reserveDaily → accessToken → HTTP 请求。 + * `accessToken` 失败 / HTTP 失败 / 华为回非成功码，这些**一条都没发出去**， + * 额度却已经扣了，而失败只 `log.Printf` * ⇒ 一次网络抖动静默烧掉配额。 - * ⇒ 挪到 `accessToken` **之后**、真正发请求之前。 + * ⇒ 预留挪到 `accessToken` **之后**、真正发请求之前（见下方调用点）。 * - * ★ 为什么不是"发送成功后再扣"：那会超发（并发下多个 goroutine 都能通过检查）。 - * 保留前置预留、但放在"确认能发"之后，是**宁可少算也不多发**的取舍 —— - * 少算的代价是偶尔一次失败没计入，超发的代价是真超额被华为拒。 + * ★ 为什么不是"发送成功后再扣"：那会超发（并发下多个 goroutine + * 都能通过检查）。保留前置预留、但放在"确认能发"之后，是**宁可少算也不多发** + * 的取舍 —— 少算的代价是偶尔一次失败没计入，超发的代价是真超额被华为拒。 */ - if !h.reserveDaily(1) { - return fmt.Errorf("达到每日推送上限 %d 条（HMS_DAILY_LIMIT）", h.DailyLimit) - } tok, err := h.accessToken(ctx) if err != nil { return err } + // 必须在 accessToken **之后**预留（②）：拿不到 token 就一条都发不出去。 + // 挪回上面一行会让 DailyLimit=1 时一次网络抖动烧穿当日配额。 + if !h.reserveDaily(1) { + return fmt.Errorf("达到每日推送上限 %d 条（HMS_DAILY_LIMIT）", h.DailyLimit) + } data, _ := json.Marshal(map[string]string{ // 与客户端约定的形状（见文档与给 dsh 的契约）：点通知按它跳转。 "type": "new_mail", diff --git a/server/internal/push/push_test.go b/server/internal/push/push_test.go index 275d014..57f2678 100644 --- a/server/internal/push/push_test.go +++ b/server/internal/push/push_test.go @@ -415,6 +415,13 @@ type hmsStub struct { pushReqs []map[string]any authHdrs []string code string + // failToken 让 /token 返回 400（模拟 accessToken 失败）。 + // + // 为什么需要它（2026-09-28）：审查报告指出「额度扣在投递之前」—— + // accessToken 失败时**一条都没发出去**、额度却已经扣了。 + // 而当时那批判据**测不出这件事**：造不出 token 失败这条路。 + // 修完代码后补这一格，就是为了让「注释说修了」与「代码真修了」不再能分家。 + failToken bool } func newHMSStub(t *testing.T) *hmsStub { @@ -424,7 +431,14 @@ func newHMSStub(t *testing.T) *hmsStub { if r.URL.Path == "/token" { s.mu.Lock() s.tokenReq++ + fail := s.failToken s.mu.Unlock() + if fail { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusBadRequest) + _, _ = io.WriteString(w, `{"error":"invalid_token"}`) + return + } w.Header().Set("Content-Type", "application/json") _, _ = io.WriteString(w, `{"access_token":"tok-abc","expires_in":3600}`) return @@ -629,3 +643,61 @@ func str(v any) string { s, _ := v.(string) return s } + +// ── 2026-09-28：额度必须在「确认能发」之后才扣 ──────────────────────── +// +// 这格判据是**补的**，而它本该在 2026-09-26 那次修复时就存在。 +// +// 当时发生的事：审查报告 §二.1 写了两条（①按 token 数扣 ②扣在投递之前）， +// 实现只改了 ①，而 ② **被写进了注释、代码没动** —— `reserveDaily` 仍在 +// `accessToken` 之前。注释与代码分家，而当时那批判据测不出来： +// 造不出「accessToken 失败」这条路。 +// +// 抓住它的是 pi：它在一次邮件驱动的回合里自己发现了这处不一致。 +func TestHMSAccessTokenFailureDoesNotBurnQuota(t *testing.T) { + s := newHMSStub(t) + h := s.hms() + h.DailyLimit = 5 + + s.mu.Lock() + s.failToken = true + s.mu.Unlock() + + // 三次都应该在拿 token 这一步就失败 + for i := 0; i < 3; i++ { + if err := h.Send(context.Background(), []string{"tok-1"}, NewMail{MailID: "m1"}); err == nil { + t.Fatalf("第 %d 次：accessToken 失败时 Send 必须返回错误", i+1) + } + } + + s.mu.Lock() + pushes := len(s.pushReqs) + s.failToken = false + s.mu.Unlock() + + if pushes != 0 { + t.Fatalf("accessToken 失败时不该发出任何推送，实际发了 %d 次", pushes) + } + + // ★ 核心断言：额度**一点没扣**。 + // 若预留仍在 accessToken 之前，这里的 used 就是 3， + // DailyLimit=5 也会只剩 2 次机会 —— 而这三次什么都没发出去。 + h.dayMu.Lock() + used := h.dayCount + h.dayMu.Unlock() + if used != 0 { + t.Fatalf("★ accessToken 失败不该扣额度，实际已扣 %d 条（"+ + "一次网络抖动静默烧配额就是这么来的）", used) + } + + // 恢复正常后仍能发送 ⇒ 额度没被那三次失败吃掉 + if err := h.Send(context.Background(), []string{"tok-1"}, NewMail{MailID: "m-ok"}); err != nil { + t.Fatalf("恢复后必须能发送: %v", err) + } + h.dayMu.Lock() + used = h.dayCount + h.dayMu.Unlock() + if used != 1 { + t.Fatalf("成功这一次应恰好计 1 条，实际 %d", used) + } +}