From 0e904dff2f1917373e41ef6125cc4190ddade32f Mon Sep 17 00:00:00 2001 From: PoterPan Date: Sat, 29 Aug 2026 18:08:54 +0800 Subject: [PATCH 1/2] =?UTF-8?q?feat(overdue):=20=E5=82=AC=E7=B9=B3?= =?UTF-8?q?=E6=94=B9=E7=82=BA=E6=AF=8F=E6=97=A5=E6=8F=90=E9=86=92=EF=BC=8C?= =?UTF-8?q?=E7=9B=B4=E5=88=B0=E5=85=A8=E9=83=A8=E7=B9=B3=E5=AE=8C=20(#48)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 把逾期催繳的去重鍵從「每期一次」改為「每期每日一次」： - notification_logs 新增 event 欄（'' = 整期 slot；逾期列帶台北營業日） - sendOverdueForPeriod 的 cron 路徑以當日日期為 slot，隔天會再催 - 送失敗仍釋放當日 slot、誠實回報，admin 手動催繳維持整期 slot 不變 - 收回本期開繳仍清空該期所有逾期列 測試 349 → 353，新增 overdue-daily.test.ts --- README.md | 14 +-- README.zh-TW.md | 11 +-- .../worker/migrations/0006_overdue_daily.sql | 32 +++++++ packages/worker/src/core/billing.ts | 9 +- packages/worker/src/core/notify.ts | 12 +-- packages/worker/src/core/scheduled.ts | 31 ++++--- .../worker/test/core/overdue-daily.test.ts | 88 +++++++++++++++++++ .../worker/test/core/overdue-preview.test.ts | 15 ++-- 8 files changed, 175 insertions(+), 37 deletions(-) create mode 100644 packages/worker/migrations/0006_overdue_daily.sql create mode 100644 packages/worker/test/core/overdue-daily.test.ts diff --git a/README.md b/README.md index 0c02e0c..1f2a76c 100644 --- a/README.md +++ b/README.md @@ -9,7 +9,7 @@ ![Cloudflare Workers](https://img.shields.io/badge/Cloudflare-Workers-F38020?logo=cloudflare&logoColor=white) ![TypeScript](https://img.shields.io/badge/TypeScript-5-3178C6?logo=typescript&logoColor=white) ![React](https://img.shields.io/badge/React-18-61DAFB?logo=react&logoColor=black) -![Vitest](https://img.shields.io/badge/tests-349%20passing-0f6e63?logo=vitest&logoColor=white) +![Vitest](https://img.shields.io/badge/tests-353%20passing-0f6e63?logo=vitest&logoColor=white) ![Serverless](https://img.shields.io/badge/100%25-serverless-074340)
@@ -66,11 +66,12 @@ pre-wired) and a multi-workspace-ready data model, so it generalizes well beyond notice (Discord / Google Chat / Slack — body shape auto-detected by host) with a deep link straight to that member's period review — one submit settles every subscription, so the link opens all of them together with a 一鍵全部核准 button. -- ⏰ **Daily cron, idempotent** — opens billing, sends one batched overdue reminder per period, and - enforces screenshot retention — all deduped through `notification_logs`. +- ⏰ **Daily cron, idempotent** — opens billing, sends an overdue reminder **every day** until a + period has no unpaid bills left, and enforces screenshot retention — all deduped through + `notification_logs`. - 🛡️ **Access-gated admin** — the whole admin host sits behind Cloudflare Access (email OTP); the SPA and its API are same-origin so the Access JWT reaches the Worker. -- 🧪 **Real-runtime tests** — 349 Vitest cases run against actual Miniflare D1 + R2 (FK constraints +- 🧪 **Real-runtime tests** — 353 Vitest cases run against actual Miniflare D1 + R2 (FK constraints enforced), not mocks. ## How a payment flows @@ -240,8 +241,9 @@ payment screenshots) and fill in `wrangler.toml` accordingly — `database_id`, period for review (shared screenshot once, every row listed, 一鍵全部核准), phone-friendly. Both are optional and best-effort (a slow or failing endpoint never blocks the payment). - **Daily cron** (01:00 UTC = 09:00 Asia/Taipei) — idempotently opens each period's bills, posts the - billing-opened notice (tagging plan roles), sends **one batched overdue reminder per period** - listing all unpaid members, and runs screenshot retention. Everything dedups via `notification_logs`. + billing-opened notice (tagging plan roles), sends **an overdue reminder every day** listing the + still-unpaid members (stopping once everyone has paid), and runs screenshot retention. Everything + dedups via `notification_logs`. ## Roadmap diff --git a/README.zh-TW.md b/README.zh-TW.md index 8c9b771..dd0cb1b 100644 --- a/README.zh-TW.md +++ b/README.zh-TW.md @@ -9,7 +9,7 @@ ![Cloudflare Workers](https://img.shields.io/badge/Cloudflare-Workers-F38020?logo=cloudflare&logoColor=white) ![TypeScript](https://img.shields.io/badge/TypeScript-5-3178C6?logo=typescript&logoColor=white) ![React](https://img.shields.io/badge/React-18-61DAFB?logo=react&logoColor=black) -![Vitest](https://img.shields.io/badge/tests-349%20passing-0f6e63?logo=vitest&logoColor=white) +![Vitest](https://img.shields.io/badge/tests-353%20passing-0f6e63?logo=vitest&logoColor=white) ![Serverless](https://img.shields.io/badge/100%25-serverless-074340)
@@ -60,11 +60,11 @@ ChipPot 解決的是一個很具體的痛點：社團大量採購 OpenAI / Anthr - 📲 **繳費推播** — 成員送出繳費時，可推一則 Bark 和／或 webhook（Discord／Google Chat／Slack，body 格式依主機自動判斷）給擁有者，並帶上直接跳到該成員該期審核的深連結——一次送出會結清所有訂閱， 所以連結會把它們一起打開，可一鍵全部核准。 -- ⏰ **每日 cron、冪等** — 自動開帳、每期發**一則**整批催繳、執行截圖保存期清理，全部經 - `notification_logs` 去重。 +- ⏰ **每日 cron、冪等** — 自動開帳、進入催繳後**每天發一則**整批催繳（直到全部繳完）、執行 + 截圖保存期清理，全部經 `notification_logs` 去重。 - 🛡️ **Access 保護的後台** — 整個後台主機在 Cloudflare Access 後（email OTP）；SPA 與其 API 同源， Access JWT 因此能到達 Worker。 -- 🧪 **真環境測試** — 349 個 Vitest 案例跑在真正的 Miniflare D1 + R2（強制 FK 約束），不是 mock。 +- 🧪 **真環境測試** — 353 個 Vitest 案例跑在真正的 Miniflare D1 + R2（強制 FK 約束），不是 mock。 ## 一筆繳費怎麼跑 @@ -224,7 +224,8 @@ pnpm --filter @chippot/worker register 手機上也好操作。存檔前可先按**送出測試**確認打得通。兩者都選填、best-effort（端點慢或掛掉都不會 卡住成員繳費）。 - **每日 cron**（01:00 UTC = 台北 09:00）— 冪等地開出各期帳單、發開繳通知（tag 方案身分組）、 - **每期發一則整批逾期催繳**（列出所有未繳者），並執行截圖保存期清理。全部經 `notification_logs` 去重。 + **每天對仍有未繳者的期別發一則整批逾期催繳**（直到全部繳完），並執行截圖保存期清理。全部經 + `notification_logs` 去重。 ## 後續規劃 diff --git a/packages/worker/migrations/0006_overdue_daily.sql b/packages/worker/migrations/0006_overdue_daily.sql new file mode 100644 index 0000000..702b6ce --- /dev/null +++ b/packages/worker/migrations/0006_overdue_daily.sql @@ -0,0 +1,32 @@ +-- #48 每日催繳: the overdue reminder's dedup slot is keyed on (workspace, period, DAY) so the +-- daily cron keeps reminding every day until the period has no unpaid bills left, instead of +-- exactly once per period. The 'event' column carries the Taipei business date for overdue rows; +-- it is '' for every other notification type, which is exactly the old whole-entity slot. +-- SQLite cannot ALTER a table-level UNIQUE, so the table is rebuilt. notification_logs has no +-- foreign keys in either direction, so a plain rebuild is safe. +CREATE TABLE notification_logs_new ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + workspace_id INTEGER NOT NULL, + type TEXT NOT NULL CHECK (type IN ('billing_opened','overdue','receipt')), + period TEXT NOT NULL, + plan_id INTEGER NOT NULL DEFAULT 0, + user_id INTEGER NOT NULL DEFAULT 0, + subscription_id INTEGER NOT NULL DEFAULT 0, + -- '' = the whole-entity slot (billing_opened / receipt). Overdue rows carry the YYYY-MM-DD + -- business date the reminder was sent, so one message is allowed per day, per period. + event TEXT NOT NULL DEFAULT '', + external_channel_type TEXT, + external_message_id TEXT, + sent_at TEXT NOT NULL, + UNIQUE(workspace_id, type, period, plan_id, user_id, subscription_id, event) +); + +INSERT INTO notification_logs_new + (id, workspace_id, type, period, plan_id, user_id, subscription_id, event, + external_channel_type, external_message_id, sent_at) +SELECT id, workspace_id, type, period, plan_id, user_id, subscription_id, '', + external_channel_type, external_message_id, sent_at +FROM notification_logs; + +DROP TABLE notification_logs; +ALTER TABLE notification_logs_new RENAME TO notification_logs; diff --git a/packages/worker/src/core/billing.ts b/packages/worker/src/core/billing.ts index 25b5ca4..4b4cc46 100644 --- a/packages/worker/src/core/billing.ts +++ b/packages/worker/src/core/billing.ts @@ -607,10 +607,11 @@ export async function retractPeriodBilling( // call is the one that closed the period (see marker_cleared). env.DB.prepare("DELETE FROM notification_logs WHERE workspace_id = ? AND type = 'billing_opened' AND period = ?") .bind(workspaceId, period), - // The overdue slot is claimed once per (workspace, period) and never expires, so leaving it - // behind would permanently mute overdue reminders if this period is ever re-opened — - // claimNotification would lose and sendOverdueForPeriod would just report already_sent, with - // no error anywhere. Kept as its own statement so it cannot inflate the marker's changes count. + // The cron's overdue slot is keyed per day (#48), so a re-opened period's stale slots would + // otherwise fire "already_sent" reminders that describe a period that no longer exists — and + // sendOverdueForPeriod reports already_sent with no error anywhere. Delete every overdue row + // (all days) so reminders restart from clean. Kept as its own statement so it cannot inflate + // the marker's changes count. // ('receipt' is declared in the type union but never claimed, so there is no slot to release.) env.DB.prepare("DELETE FROM notification_logs WHERE workspace_id = ? AND type = 'overdue' AND period = ?") .bind(workspaceId, period), diff --git a/packages/worker/src/core/notify.ts b/packages/worker/src/core/notify.ts index c2eb109..dfa9c1a 100644 --- a/packages/worker/src/core/notify.ts +++ b/packages/worker/src/core/notify.ts @@ -38,22 +38,24 @@ export interface NotificationKey { planId?: number; userId?: number; subscriptionId?: number; + /** Distinguishes two messages that share one entity. '' = the whole-entity slot. */ + event?: string; } /** * Claim a notification slot to guarantee at-most-once sending. Inserts a notification_logs * row; returns true if this caller won the slot (should send), false if already sent. - * Uses NOT NULL DEFAULT 0 sentinels so the UNIQUE actually dedupes (roadmap §4.1). + * Uses NOT NULL DEFAULT 0 / '' sentinels so the UNIQUE actually dedupes (roadmap §4.1). */ export async function claimNotification(db: D1Database, k: NotificationKey): Promise { const res = await db .prepare( `INSERT INTO notification_logs - (workspace_id, type, period, plan_id, user_id, subscription_id, external_channel_type, sent_at) - VALUES (?, ?, ?, ?, ?, ?, 'discord', ?) - ON CONFLICT(workspace_id, type, period, plan_id, user_id, subscription_id) DO NOTHING` + (workspace_id, type, period, plan_id, user_id, subscription_id, event, external_channel_type, sent_at) + VALUES (?, ?, ?, ?, ?, ?, ?, 'discord', ?) + ON CONFLICT(workspace_id, type, period, plan_id, user_id, subscription_id, event) DO NOTHING` ) - .bind(k.workspaceId, k.type, k.period, k.planId ?? 0, k.userId ?? 0, k.subscriptionId ?? 0, nowUtcIso()) + .bind(k.workspaceId, k.type, k.period, k.planId ?? 0, k.userId ?? 0, k.subscriptionId ?? 0, k.event ?? "", nowUtcIso()) .run(); return (res.meta.changes ?? 0) > 0; } diff --git a/packages/worker/src/core/scheduled.ts b/packages/worker/src/core/scheduled.ts index 21ea91f..d381860 100644 --- a/packages/worker/src/core/scheduled.ts +++ b/packages/worker/src/core/scheduled.ts @@ -109,9 +109,11 @@ export interface OverdueResult { /** * Send the overdue reminder for ONE period as a single batched public message listing every * unpaid member — pending OR rejected (a rejected submission still owes) — tagged once with - * their plans + total, deduped per (ws, period). + * their plans + total, deduped per (ws, period, DAY): the daily cron keeps reminding every day + * until the period has no unpaid bills left (#48). * - * force=false (cron): only fires when ≥1 member is past overdue_days; claim-then-send. + * force=false (cron): only fires when ≥1 member is past overdue_days; claim-then-send. The slot + * carries the Taipei business date, so a re-run the same day is deduped but the next day fires. * force=true (admin "催繳未繳成員"): lists ALL unpaid members regardless of overdue_days and clears * the dedup slot first so it always re-sends. The two lists genuinely differ, which is why the UI * must not call both "立即重發" — see the copy in views/PushStatus.tsx. @@ -124,7 +126,7 @@ export async function sendOverdueForPeriod( workspaceId: number, period: string, notifier: Notifier, - opts: { force: boolean; dryRun?: boolean; now?: Date } + opts: { force: boolean; dryRun?: boolean; now?: Date; dayKey?: string } ): Promise { const bare = (outcome: OverdueOutcome, overdueDays = 0): OverdueResult => ({ notified: 0, outcome, overdue_days: overdueDays, people: [] }); @@ -136,6 +138,10 @@ export async function sendOverdueForPeriod( if (!channelId) return bare("no_channel", settings.overdue_days); if (!env.DISCORD_BOT_TOKEN) return bare("no_bot_token", settings.overdue_days); const today = taipeiDate(opts.now ?? new Date()); + // The cron dedupes per business day; the admin force-resend keeps the entity-wide slot so it can + // always fire (it deletes that whole-entity slot below). A caller-provided dayKey overrides the + // date (tests / replay). + const dayKey = opts.dayKey ?? today; const rows = await env.DB .prepare( @@ -167,15 +173,18 @@ export async function sendOverdueForPeriod( if (opts.dryRun) return { notified: 0, outcome: "preview", overdue_days: settings.overdue_days, people }; if (opts.force) { - // force = admin resend: clear the slot so the claim below always wins. This delete-then-claim - // isn't atomic, but force is an occasional single-admin dashboard action whose button is - // disabled while in flight; the only risk is a duplicate message from two truly-concurrent - // resends, which we accept (no DO/lock — YAGNI). Unlike the billing_opened slot, the overdue - // row carries no "period is open" meaning, so a momentary gap is harmless. + // force = admin resend: clear the entity-wide slot so the claim below always wins. This + // delete-then-claim isn't atomic, but force is an occasional single-admin dashboard action whose + // button is disabled while in flight; the only risk is a duplicate message from two + // truly-concurrent resends, which we accept (no DO/lock — YAGNI). Unlike the billing_opened + // slot, the overdue row carries no "period is open" meaning, so a momentary gap is harmless. await env.DB.prepare("DELETE FROM notification_logs WHERE workspace_id = ? AND type = 'overdue' AND period = ?") .bind(workspaceId, period).run(); } - if (!(await claimNotification(env.DB, { workspaceId, type: "overdue", period }))) { + // The cron's slot is per day (dayKey); the admin's force resend reuses the '' entity-wide slot, + // which is what the DELETE above just cleared. + const event = opts.force ? "" : dayKey; + if (!(await claimNotification(env.DB, { workspaceId, type: "overdue", period, event }))) { return { notified: 0, outcome: "already_sent", overdue_days: settings.overdue_days, people }; } if (!(await notifier.sendOverdue(env, channelId, period, people, settings.overdue_template))) { @@ -185,8 +194,8 @@ export async function sendOverdueForPeriod( // would just lose and report already_sent, with no error surfacing anywhere. That is the exact // trap 收回本期開繳 avoids by deleting this row, so a failed send releases it for the same reason. // We can delete unconditionally: this call won the claim above, so the row is the one we wrote. - await env.DB.prepare("DELETE FROM notification_logs WHERE workspace_id = ? AND type = 'overdue' AND period = ?") - .bind(workspaceId, period).run(); + await env.DB.prepare("DELETE FROM notification_logs WHERE workspace_id = ? AND type = 'overdue' AND period = ? AND event = ?") + .bind(workspaceId, period, event).run(); return { notified: 0, outcome: "send_failed", overdue_days: settings.overdue_days, people }; } return { notified: people.length, outcome: "sent", overdue_days: settings.overdue_days, people }; diff --git a/packages/worker/test/core/overdue-daily.test.ts b/packages/worker/test/core/overdue-daily.test.ts new file mode 100644 index 0000000..75d288e --- /dev/null +++ b/packages/worker/test/core/overdue-daily.test.ts @@ -0,0 +1,88 @@ +import { env } from "cloudflare:test"; +import { beforeAll, describe, expect, it } from "vitest"; +import { runDailyTasks, sendOverdueForPeriod } from "../../src/core/scheduled"; +import type { Notifier, OverduePerson } from "../../src/core/notify"; + +/** + * #48 每日催繳: the overdue reminder must fire once per DAY (not once per period) until + * everyone has paid. The dedup slot for 'overdue' is keyed on (workspace, period, day), so + * consecutive cron runs keep reminding while any bill is still pending/rejected, and stop + * the moment the period has no unpaid bills left. + */ + +const TS = "2026-05-01T00:00:00.000Z"; +const CHAN = "chan-9890"; + +const sent = { overdue: [] as { period: string; people: OverduePerson[] }[] }; +const notifier: Notifier = { + async sendBillingOpened() { return true; }, + async sendOverdue(_e, _ch, period, people, _t) { sent.overdue.push({ period, people }); return true; }, + async sendPaymentNudge() { return true; }, +}; + +/** Seed a workspace + one member + one unpaid bill in `period`, due on day 5 of that month. */ +async function seedUnpaid(wsId: number, period: string) { + await env.DB.batch([ + env.DB.prepare(`INSERT INTO workspaces (id,name,owner_id,channel_type,billing_day,settings,created_at,updated_at) VALUES (?,?,?,?,?,?,?,?)`) + .bind(wsId, "W", "o", "discord", 5, JSON.stringify({ discord_billing_channel_id: CHAN, overdue_days: 3, proof_retention_months: 24 }), TS, TS), + env.DB.prepare(`INSERT INTO users (id,workspace_id,discord_id,display_name,created_at,updated_at) VALUES (?,?,?,?,?,?)`) + .bind(wsId, wsId, `d-${wsId}`, "M", TS, TS), + env.DB.prepare(`INSERT INTO plans (id,workspace_id,name,provider,monthly_amount,created_at,updated_at) VALUES (?,?,?,?,?,?,?)`) + .bind(wsId, wsId, "ChatGPT", "openai", 315, TS, TS), + env.DB.prepare(`INSERT INTO subscriptions (id,workspace_id,user_id,plan_id,start_date,billing_day,created_at,updated_at) VALUES (?,?,?,?,?,?,?,?)`) + .bind(wsId, wsId, wsId, wsId, `${period}-01`, 5, TS, TS), + env.DB.prepare(`INSERT INTO payments (workspace_id,subscription_id,period,period_start,period_end,due_date,amount,status,source,created_at,updated_at) VALUES (?,?,?,?,?,?,?,?,?,?,?)`) + .bind(wsId, wsId, period, `${period}-01`, `${period}-31`, `${period}-05`, 315, "pending", "cron", TS, TS), + ]); +} + +beforeAll(async () => { + (env as any).DISCORD_BOT_TOKEN = "test-bot-token"; + // Three independent periods so each test can mutate its own without touching the others. + await seedUnpaid(9890, "2028-05"); + await seedUnpaid(9891, "2028-06"); + await seedUnpaid(9892, "2028-07"); + await seedUnpaid(9893, "2028-11"); // cron integration workspace +}); + +describe("#48 每日催繳", () => { + it("逾期成員未繳時，隔天會再催一次（每日一則，而非每期一則）", async () => { + // due 2028-05-05, overdue_days 3 → overdue from 05-08. + const r1 = await sendOverdueForPeriod(env, 9890, "2028-05", notifier, { force: false, now: new Date("2028-05-10T00:00:00Z") }); + expect(r1).toMatchObject({ outcome: "sent", notified: 1 }); + + const r2 = await sendOverdueForPeriod(env, 9890, "2028-05", notifier, { force: false, now: new Date("2028-05-11T00:00:00Z") }); + expect(r2).toMatchObject({ outcome: "sent", notified: 1 }); + }); + + it("同一天只發一則（同日重複觸發去重）", async () => { + const day = new Date("2028-06-12T00:00:00Z"); + const first = await sendOverdueForPeriod(env, 9891, "2028-06", notifier, { force: false, now: day }); + expect(first).toMatchObject({ outcome: "sent", notified: 1 }); + + const second = await sendOverdueForPeriod(env, 9891, "2028-06", notifier, { force: false, now: day }); + expect(second).toMatchObject({ outcome: "already_sent", notified: 0 }); + expect(second.people.length).toBe(1); // 名單仍帶回，UI 才講得出「已經催過」 + }); + + it("全部繳完後不再催（無 pending/rejected）", async () => { + await env.DB.prepare("UPDATE payments SET status = 'paid' WHERE workspace_id = ? AND period = ?").bind(9892, "2028-07").run(); + const r = await sendOverdueForPeriod(env, 9892, "2028-07", notifier, { force: false, now: new Date("2028-07-13T00:00:00Z") }); + expect(r).toMatchObject({ outcome: "none_due", notified: 0 }); + }); + + it("每日 cron 對同一期別連續兩天都計入 overdueSent", async () => { + // period 2028-11, due 2028-11-05. Both runs land past overdue_days and on distinct days. + // runDailyTasks sweeps every workspace in the DB, so count only the 2028-11 messages. + const forNov = () => sent.overdue.filter((m) => m.period === "2028-11").length; + + const before = forNov(); + const s1 = await runDailyTasks(env, new Date("2028-11-09T16:30:00.000Z"), notifier); // Taipei 11-10 + expect(forNov()).toBe(before + 1); + expect(s1.overdueSent).toBeGreaterThanOrEqual(1); + + const s2 = await runDailyTasks(env, new Date("2028-11-10T16:30:00.000Z"), notifier); // Taipei 11-11 + expect(forNov()).toBe(before + 2); + expect(s2.overdueSent).toBeGreaterThanOrEqual(1); + }); +}); diff --git a/packages/worker/test/core/overdue-preview.test.ts b/packages/worker/test/core/overdue-preview.test.ts index 34a62f4..b9d20a4 100644 --- a/packages/worker/test/core/overdue-preview.test.ts +++ b/packages/worker/test/core/overdue-preview.test.ts @@ -76,15 +76,18 @@ describe("sendOverdueForPeriod 結果物件", () => { expect(sent.length).toBe(before + 1); }); - it("非 force 的第二次送出回 already_sent 而不是假的 sent", async () => { - // 上一個 it 已經佔用 (ws, overdue, period) 的 slot；cron 再跑一次不能重送。 + it("非 force 的第二次送出回 already_sent 而不是假的 sent（同日去重）", async () => { + // #48: the cron slot is per DAY. This second force=false send uses the same `now` as the + // previous it block's send above would have — but that send was force=true (the '' slot), so + // pin the day here to exercise the cron's per-day slot directly: two sends on one day. const before = sent.length; - const r = await sendOverdueForPeriod(env, WS, PERIOD, notifier, { - force: false, dryRun: false, now: new Date("2098-06-30T00:00:00Z"), - }); + const day = new Date("2098-06-30T00:00:00Z"); + const first = await sendOverdueForPeriod(env, WS, PERIOD, notifier, { force: false, dryRun: false, now: day }); + expect(first).toMatchObject({ outcome: "sent", notified: 2 }); + const r = await sendOverdueForPeriod(env, WS, PERIOD, notifier, { force: false, dryRun: false, now: day }); expect(r).toMatchObject({ outcome: "already_sent", notified: 0 }); expect(r.people.length).toBe(2); // 名單還是要帶回來，UI 才講得出「這 2 位已經催過了」 - expect(sent.length).toBe(before); + expect(sent.length).toBe(before + 1); }); it("沒有 bot token 時回 no_bot_token 而不是假成功", async () => { From 6f853497cdb0ffc2e5d61f581964714722dfba6d Mon Sep 17 00:00:00 2001 From: PoterPan Date: Sat, 29 Aug 2026 18:25:04 +0800 Subject: [PATCH 2/2] =?UTF-8?q?fix(overdue):=20=E4=BE=9D=20Codex=20?= =?UTF-8?q?=E5=AF=A9=E6=9F=A5=E4=BF=AE=E6=AD=A3=20force=20=E6=B8=85=20slot?= =?UTF-8?q?=20=E7=AF=84=E5=9C=8D=E8=88=87=E9=87=8B=E6=94=BE=E5=AE=89?= =?UTF-8?q?=E5=85=A8=20(#48)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - force 催繳只清 '' 整期 slot，不再誤刪 cron 的每日 slot（同日重試仍去重） - 送失敗改用「以 claim 回傳的 row id 釋放」，避免併發 force 互相刪到 slot - 補測試：cron→force→同日 cron 不重送、失敗釋放、retract 清空多日 slot - README 測試數 353→355、migrations 範圍 0005→0006 --- README.md | 6 ++-- README.zh-TW.md | 6 ++-- packages/worker/src/core/notify.ts | 26 ++++++++++++++++- packages/worker/src/core/scheduled.ts | 19 ++++++------ .../worker/test/core/billing-retract.test.ts | 15 ++++++---- .../worker/test/core/overdue-daily.test.ts | 29 ++++++++++++++++++- 6 files changed, 78 insertions(+), 23 deletions(-) diff --git a/README.md b/README.md index 1f2a76c..0eaeb53 100644 --- a/README.md +++ b/README.md @@ -9,7 +9,7 @@ ![Cloudflare Workers](https://img.shields.io/badge/Cloudflare-Workers-F38020?logo=cloudflare&logoColor=white) ![TypeScript](https://img.shields.io/badge/TypeScript-5-3178C6?logo=typescript&logoColor=white) ![React](https://img.shields.io/badge/React-18-61DAFB?logo=react&logoColor=black) -![Vitest](https://img.shields.io/badge/tests-353%20passing-0f6e63?logo=vitest&logoColor=white) +![Vitest](https://img.shields.io/badge/tests-355%20passing-0f6e63?logo=vitest&logoColor=white) ![Serverless](https://img.shields.io/badge/100%25-serverless-074340)
@@ -71,7 +71,7 @@ pre-wired) and a multi-workspace-ready data model, so it generalizes well beyond `notification_logs`. - 🛡️ **Access-gated admin** — the whole admin host sits behind Cloudflare Access (email OTP); the SPA and its API are same-origin so the Access JWT reaches the Worker. -- 🧪 **Real-runtime tests** — 353 Vitest cases run against actual Miniflare D1 + R2 (FK constraints +- 🧪 **Real-runtime tests** — 355 Vitest cases run against actual Miniflare D1 + R2 (FK constraints enforced), not mocks. ## How a payment flows @@ -142,7 +142,7 @@ packages/ src/core/ channel-agnostic domain logic src/adapters/discord/ Ed25519 verify · commands · handler · notify src/routes/ interactions · upload · admin · images - migrations/ D1 schema (0001…0005) + migrations/ D1 schema (0001…0006) scripts/ register-commands.mjs test/ Vitest (real Miniflare D1/R2) web/ public token-gated upload page (Vite/React) diff --git a/README.zh-TW.md b/README.zh-TW.md index dd0cb1b..a7a3a92 100644 --- a/README.zh-TW.md +++ b/README.zh-TW.md @@ -9,7 +9,7 @@ ![Cloudflare Workers](https://img.shields.io/badge/Cloudflare-Workers-F38020?logo=cloudflare&logoColor=white) ![TypeScript](https://img.shields.io/badge/TypeScript-5-3178C6?logo=typescript&logoColor=white) ![React](https://img.shields.io/badge/React-18-61DAFB?logo=react&logoColor=black) -![Vitest](https://img.shields.io/badge/tests-353%20passing-0f6e63?logo=vitest&logoColor=white) +![Vitest](https://img.shields.io/badge/tests-355%20passing-0f6e63?logo=vitest&logoColor=white) ![Serverless](https://img.shields.io/badge/100%25-serverless-074340)
@@ -64,7 +64,7 @@ ChipPot 解決的是一個很具體的痛點：社團大量採購 OpenAI / Anthr 截圖保存期清理，全部經 `notification_logs` 去重。 - 🛡️ **Access 保護的後台** — 整個後台主機在 Cloudflare Access 後（email OTP）；SPA 與其 API 同源， Access JWT 因此能到達 Worker。 -- 🧪 **真環境測試** — 353 個 Vitest 案例跑在真正的 Miniflare D1 + R2（強制 FK 約束），不是 mock。 +- 🧪 **真環境測試** — 355 個 Vitest 案例跑在真正的 Miniflare D1 + R2（強制 FK 約束），不是 mock。 ## 一筆繳費怎麼跑 @@ -132,7 +132,7 @@ packages/ src/core/ 與管道無關的核心邏輯 src/adapters/discord/ Ed25519 驗章 · 指令 · handler · 通知 src/routes/ interactions · upload · admin · images - migrations/ D1 schema（0001…0005） + migrations/ D1 schema（0001…0006） scripts/ register-commands.mjs test/ Vitest（真 Miniflare D1/R2） web/ 公開的 token-gated 上傳頁（Vite/React） diff --git a/packages/worker/src/core/notify.ts b/packages/worker/src/core/notify.ts index dfa9c1a..87ec5ed 100644 --- a/packages/worker/src/core/notify.ts +++ b/packages/worker/src/core/notify.ts @@ -48,6 +48,22 @@ export interface NotificationKey { * Uses NOT NULL DEFAULT 0 / '' sentinels so the UNIQUE actually dedupes (roadmap §4.1). */ export async function claimNotification(db: D1Database, k: NotificationKey): Promise { + return (await claimNotificationSlot(db, k)).won; +} + +export interface NotificationSlot { + /** True if this caller won the slot (should send); false if already claimed. */ + won: boolean; + /** The claimed row's id (D1 last_row_id); null when `won` is false. */ + id: number | null; +} + +/** + * Claim with ownership: on a win the caller owns `id`, and a later release must go through + * releaseSlot(id) — never a keyed DELETE — so a concurrent claim that replaced the row cannot have + * its slot deleted by someone else's failed send (see sendOverdueForPeriod, #48 / Codex finding 2). + */ +export async function claimNotificationSlot(db: D1Database, k: NotificationKey): Promise { const res = await db .prepare( `INSERT INTO notification_logs @@ -57,7 +73,15 @@ export async function claimNotification(db: D1Database, k: NotificationKey): Pro ) .bind(k.workspaceId, k.type, k.period, k.planId ?? 0, k.userId ?? 0, k.subscriptionId ?? 0, k.event ?? "", nowUtcIso()) .run(); - return (res.meta.changes ?? 0) > 0; + const won = (res.meta.changes ?? 0) > 0; + return { won, id: won ? Number(res.meta.last_row_id) : null }; +} + +/** Release a slot by the row id the claim returned. Deleting by id (not by key) is what makes the + * release ownership-safe: a stale id simply deletes nothing instead of a concurrent claim's row. */ +export async function releaseSlot(db: D1Database, id: number): Promise { + const res = await db.prepare("DELETE FROM notification_logs WHERE id = ?").bind(id).run(); + return res.meta.changes ?? 0; } /** diff --git a/packages/worker/src/core/scheduled.ts b/packages/worker/src/core/scheduled.ts index d381860..2c793b8 100644 --- a/packages/worker/src/core/scheduled.ts +++ b/packages/worker/src/core/scheduled.ts @@ -3,7 +3,7 @@ import { parseSettings } from "../env"; import { taipeiPeriod, taipeiDate, taipeiDayOfMonth, daysBetween } from "./time"; import { ensurePeriodPayment } from "./billing"; import { runRetention } from "./retention"; -import { claimNotification, type Notifier, type PlanOpenLine, type OverduePerson } from "./notify"; +import { claimNotification, claimNotificationSlot, releaseSlot, type Notifier, type PlanOpenLine, type OverduePerson } from "./notify"; export interface DailySummary { paymentsEnsured: number; @@ -173,18 +173,20 @@ export async function sendOverdueForPeriod( if (opts.dryRun) return { notified: 0, outcome: "preview", overdue_days: settings.overdue_days, people }; if (opts.force) { - // force = admin resend: clear the entity-wide slot so the claim below always wins. This - // delete-then-claim isn't atomic, but force is an occasional single-admin dashboard action whose - // button is disabled while in flight; the only risk is a duplicate message from two + // force = admin resend: clear the entity-wide ('' slot) only, then claim it afresh below. The + // cron's per-day slots are left untouched, so a same-day cron retry still reads as already sent. + // The delete-then-claim isn't atomic, but force is an occasional single-admin dashboard action + // whose button is disabled while in flight; the only risk is a duplicate message from two // truly-concurrent resends, which we accept (no DO/lock — YAGNI). Unlike the billing_opened // slot, the overdue row carries no "period is open" meaning, so a momentary gap is harmless. - await env.DB.prepare("DELETE FROM notification_logs WHERE workspace_id = ? AND type = 'overdue' AND period = ?") + await env.DB.prepare("DELETE FROM notification_logs WHERE workspace_id = ? AND type = 'overdue' AND period = ? AND event = ''") .bind(workspaceId, period).run(); } // The cron's slot is per day (dayKey); the admin's force resend reuses the '' entity-wide slot, // which is what the DELETE above just cleared. const event = opts.force ? "" : dayKey; - if (!(await claimNotification(env.DB, { workspaceId, type: "overdue", period, event }))) { + const slot = await claimNotificationSlot(env.DB, { workspaceId, type: "overdue", period, event }); + if (!slot.won) { return { notified: 0, outcome: "already_sent", overdue_days: settings.overdue_days, people }; } if (!(await notifier.sendOverdue(env, channelId, period, people, settings.overdue_template))) { @@ -193,9 +195,8 @@ export async function sendOverdueForPeriod( // Keeping it would mute this period's reminders permanently: every later claim (cron included) // would just lose and report already_sent, with no error surfacing anywhere. That is the exact // trap 收回本期開繳 avoids by deleting this row, so a failed send releases it for the same reason. - // We can delete unconditionally: this call won the claim above, so the row is the one we wrote. - await env.DB.prepare("DELETE FROM notification_logs WHERE workspace_id = ? AND type = 'overdue' AND period = ? AND event = ?") - .bind(workspaceId, period, event).run(); + // The release goes by the row id we won, so it can never delete a concurrent claim's row. + await releaseSlot(env.DB, slot.id!); return { notified: 0, outcome: "send_failed", overdue_days: settings.overdue_days, people }; } return { notified: people.length, outcome: "sent", overdue_days: settings.overdue_days, people }; diff --git a/packages/worker/test/core/billing-retract.test.ts b/packages/worker/test/core/billing-retract.test.ts index e266498..e0a71ed 100644 --- a/packages/worker/test/core/billing-retract.test.ts +++ b/packages/worker/test/core/billing-retract.test.ts @@ -45,6 +45,9 @@ beforeAll(async () => { // an overdue reminder already fired for the mis-opened period; its dedup slot must be released // on retract, or a later re-open could never remind anyone again env.DB.prepare(`INSERT INTO notification_logs (workspace_id,type,period,plan_id,user_id,subscription_id,sent_at) VALUES (?,?,?,?,?,?,?)`).bind(WS,"overdue",P,0,0,0,TS), + // #48: the cron also leaves per-day slots (event = business date); retract must clear those too, + // not just the '' slot — a re-open must restart reminders from clean. + env.DB.prepare(`INSERT INTO notification_logs (workspace_id,type,period,plan_id,user_id,subscription_id,event,sent_at) VALUES (?,?,?,?,?,?,?,?)`).bind(WS,"overdue",P,0,0,0,"2027-11-05",TS), // scoping pins: another period of the same workspace, and the same period of another workspace env.DB.prepare(`INSERT INTO notification_logs (workspace_id,type,period,plan_id,user_id,subscription_id,sent_at) VALUES (?,?,?,?,?,?,?)`).bind(WS,"overdue",P_NEXT,0,0,0,TS), env.DB.prepare(`INSERT INTO notification_logs (workspace_id,type,period,plan_id,user_id,subscription_id,sent_at) VALUES (?,?,?,?,?,?,?)`).bind(WS_OTHER,"overdue",P,0,0,0,TS), @@ -80,7 +83,7 @@ describe("retractPeriodBilling", () => { const tokens = (await env.DB.prepare("SELECT COUNT(*) c FROM upload_tokens WHERE workspace_id=? AND period=?").bind(WS, P).first<{ c: number }>())!.c; expect(tokens).toBe(3); expect(await env.BUCKET.get("proof-9700-orphan")).not.toBeNull(); - expect(await countOverdueLogs(WS, P)).toBe(1); + expect(await countOverdueLogs(WS, P)).toBe(2); // '' slot + one dated (per-day) slot, both untouched on dry run }); it("apply deletes pending/rejected + the orphaned proof, keeps paid/verified whole, clears the marker", async () => { @@ -118,11 +121,11 @@ describe("retractPeriodBilling", () => { expect(await countOpenedLogs(P_NEXT)).toBe(1); }); - // The overdue slot is claimed once per (workspace, period) and never expires. If a mis-opened - // period had already sent a reminder, leaving that row behind would silently mute overdue - // reminders forever after a re-open — sendOverdueForPeriod just reports already_sent, no error. - it("releases the overdue dedup slot, scoped to this workspace+period", async () => { - expect(await countOverdueLogs(WS, P)).toBe(0); + // The overdue slot is claimed once per (workspace, period, day) after #48. If a mis-opened period + // had already sent a reminder (one or more days), leaving those rows behind would silently mute + // overdue reminders forever after a re-open — sendOverdueForPeriod just reports already_sent, no error. + it("releases the overdue dedup slots (all days), scoped to this workspace+period", async () => { + expect(await countOverdueLogs(WS, P)).toBe(0); // both the '' slot and the dated slot are gone expect(await countOverdueLogs(WS, P_NEXT)).toBe(1); // another period of the same workspace expect(await countOverdueLogs(WS_OTHER, P)).toBe(1); // the same period of another workspace diff --git a/packages/worker/test/core/overdue-daily.test.ts b/packages/worker/test/core/overdue-daily.test.ts index 75d288e..aab1c07 100644 --- a/packages/worker/test/core/overdue-daily.test.ts +++ b/packages/worker/test/core/overdue-daily.test.ts @@ -1,7 +1,7 @@ import { env } from "cloudflare:test"; import { beforeAll, describe, expect, it } from "vitest"; import { runDailyTasks, sendOverdueForPeriod } from "../../src/core/scheduled"; -import type { Notifier, OverduePerson } from "../../src/core/notify"; +import { claimNotification, type Notifier, type OverduePerson } from "../../src/core/notify"; /** * #48 每日催繳: the overdue reminder must fire once per DAY (not once per period) until @@ -71,6 +71,33 @@ describe("#48 每日催繳", () => { expect(r).toMatchObject({ outcome: "none_due", notified: 0 }); }); + it("手動催繳不會抹掉當天 cron 已送的 slot：同日 cron 重試仍回 already_sent", async () => { + const day = new Date("2028-05-15T00:00:00Z"); + // cron 先送一次（event = 2028-05-15） + await sendOverdueForPeriod(env, 9890, "2028-05", notifier, { force: false, now: day }); + + // admin 手動催繳：只清 '' slot，不動每日 slot + await sendOverdueForPeriod(env, 9890, "2028-05", notifier, { force: true, now: day }); + + // 同日 cron 重試不得再送 + const r = await sendOverdueForPeriod(env, 9890, "2028-05", notifier, { force: false, now: day }); + expect(r).toMatchObject({ outcome: "already_sent", notified: 0 }); + }); + + it("送失敗只釋放自己贏得的 slot，不刪掉他人同日的 slot", async () => { + // 用會失敗的 notifier 走一次 force（'' slot）：送失敗必須釋放名額，讓下一次能再送。 + const failing: Notifier = { + async sendBillingOpened() { return true; }, + async sendOverdue() { return false; }, + async sendPaymentNudge() { return true; }, + }; + const r = await sendOverdueForPeriod(env, 9891, "2028-06", failing, { force: true, now: new Date("2028-06-12T00:00:00Z") }); + expect(r).toMatchObject({ outcome: "send_failed", notified: 0 }); + + // '' slot 真的還回去了：再 claim 一次要成功。 + expect(await claimNotification(env.DB, { workspaceId: 9891, type: "overdue", period: "2028-06", event: "" })).toBe(true); + }); + it("每日 cron 對同一期別連續兩天都計入 overdueSent", async () => { // period 2028-11, due 2028-11-05. Both runs land past overdue_days and on distinct days. // runDailyTasks sweeps every workspace in the DB, so count only the 2028-11 messages.