test(bot): cover the reschedule corner case in scheduleReminderFire
Lock down the pre-send cancel that fixed the dropped 8:20 PM fire:
- cancel UPDATE always runs BEFORE boss.send (regression: stately
dedupe silently rejected the new send when a stale created job
existed; now we tombstone the stale row first)
- cancel scopes to state='created' only (active and completed jobs
must survive — they're in-flight or historical)
- cancel filters by THIS reminder's singletonKey (no cross-reminder
cancellation)
- boss.send still receives singletonKey + startAfter + retryLimit
- first-time schedule (zero stale rows) still calls send
- cancel UPDATE error degrades to "send anyway" — the handler-level
recent-run dedupe will catch any duplicate that lands
- boss.send returning null is surfaced (so the caller's logger
captures jobId: null instead of silently treating it as success)
77 bot tests now (was 70).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
2e6fbfa7a5
commit
6942745085
119
apps/bot/src/scheduler/reminder-jobs.test.ts
Normal file
119
apps/bot/src/scheduler/reminder-jobs.test.ts
Normal file
@ -0,0 +1,119 @@
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
|
||||
// pg-boss + db mocks. Hoisted so the vi.mock factories below can refer
|
||||
// to the spies — see https://vitest.dev/api/vi.html#vi-hoisted.
|
||||
const {
|
||||
bossSendMock,
|
||||
dbExecuteMock,
|
||||
} = vi.hoisted(() => ({
|
||||
bossSendMock: vi.fn(async (..._args: unknown[]) => "new-job-id"),
|
||||
dbExecuteMock: vi.fn(async (..._args: unknown[]) => ({ rows: [] as unknown[] })),
|
||||
}));
|
||||
|
||||
vi.mock("../db.js", () => ({
|
||||
db: { execute: (...a: unknown[]) => dbExecuteMock(...a) },
|
||||
}));
|
||||
vi.mock("../logger.js", () => ({
|
||||
logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn() },
|
||||
}));
|
||||
|
||||
// We don't import pg-boss directly — scheduleReminderFire receives a
|
||||
// PgBoss instance as its first arg. Build a minimal stub that exposes
|
||||
// just the .send method (and createQueue / work for registerReminderJobs
|
||||
// if we ever wire it here).
|
||||
const fakeBoss = {
|
||||
send: bossSendMock,
|
||||
} as unknown as Parameters<typeof scheduleReminderFire>[0];
|
||||
|
||||
import { scheduleReminderFire } from "./reminder-jobs.js";
|
||||
|
||||
const REMINDER_ID = "11111111-1111-1111-1111-111111111111";
|
||||
const SINGLETON_KEY = `reminder:${REMINDER_ID}`;
|
||||
const FIRE_AT = new Date("2026-05-10T12:20:00.000Z");
|
||||
|
||||
beforeEach(() => {
|
||||
bossSendMock.mockReset();
|
||||
bossSendMock.mockResolvedValue("new-job-id");
|
||||
dbExecuteMock.mockReset();
|
||||
dbExecuteMock.mockResolvedValue({ rows: [] });
|
||||
});
|
||||
|
||||
describe("scheduleReminderFire — pre-send cancel of stale created jobs", () => {
|
||||
it("ALWAYS runs the cancel-stale UPDATE before boss.send (regression: stately dedupe dropped reschedules)", async () => {
|
||||
// Repro of the dropped-fire bug: the queue was on policy=stately
|
||||
// and a prior schedule had left a 'created' job in pg-boss with
|
||||
// the same singletonKey. The new send returned null and the
|
||||
// user's 8:20 PM fire was silently lost. Under the fix, we MUST
|
||||
// tombstone any prior created jobs FIRST so the new send wins
|
||||
// even under standard policy.
|
||||
dbExecuteMock.mockResolvedValueOnce({ rows: [{ id: "stale-1" }] });
|
||||
const result = await scheduleReminderFire(fakeBoss, REMINDER_ID, FIRE_AT);
|
||||
// Order matters: cancel happens before send.
|
||||
expect(dbExecuteMock).toHaveBeenCalledTimes(1);
|
||||
expect(bossSendMock).toHaveBeenCalledTimes(1);
|
||||
expect(dbExecuteMock.mock.invocationCallOrder[0]).toBeLessThan(
|
||||
bossSendMock.mock.invocationCallOrder[0]!,
|
||||
);
|
||||
expect(result).toBe("new-job-id");
|
||||
});
|
||||
|
||||
it("scopes the cancel to state='created' only — active/completed jobs MUST survive", async () => {
|
||||
// The cancel must NOT touch in-flight runs (state='active') nor
|
||||
// historical fires (state='completed'). Otherwise we'd nuke the
|
||||
// run that's currently sending and the user gets phantom 'failed'
|
||||
// rows in the activity feed.
|
||||
await scheduleReminderFire(fakeBoss, REMINDER_ID, FIRE_AT);
|
||||
const sqlStmt = dbExecuteMock.mock.calls[0]![0];
|
||||
// Drizzle's sql template returns an SQL object; serialise to inspect.
|
||||
const text = JSON.stringify(sqlStmt);
|
||||
expect(text).toMatch(/state\s*=\s*'?created'?/);
|
||||
expect(text).not.toMatch(/state\s*=\s*'?active'?/);
|
||||
expect(text).not.toMatch(/state\s*=\s*'?completed'?/);
|
||||
});
|
||||
|
||||
it("targets only THIS reminder's singletonKey (doesn't cross-cancel other reminders)", async () => {
|
||||
await scheduleReminderFire(fakeBoss, REMINDER_ID, FIRE_AT);
|
||||
const sqlStmt = dbExecuteMock.mock.calls[0]![0];
|
||||
const text = JSON.stringify(sqlStmt);
|
||||
// The reminderId must appear in the WHERE clause's bound params
|
||||
// (drizzle stores them in the serialised payload).
|
||||
expect(text).toContain(REMINDER_ID);
|
||||
});
|
||||
|
||||
it("passes the singleton key through to boss.send for diagnostics", async () => {
|
||||
await scheduleReminderFire(fakeBoss, REMINDER_ID, FIRE_AT);
|
||||
const [, , opts] = bossSendMock.mock.calls[0]!;
|
||||
expect(opts).toMatchObject({
|
||||
singletonKey: SINGLETON_KEY,
|
||||
startAfter: FIRE_AT,
|
||||
retryLimit: 3,
|
||||
});
|
||||
});
|
||||
|
||||
it("still sends when the cancel UPDATE returns zero rows (first-time schedule)", async () => {
|
||||
// First time scheduling a reminder — no stale rows exist.
|
||||
dbExecuteMock.mockResolvedValueOnce({ rows: [] });
|
||||
const result = await scheduleReminderFire(fakeBoss, REMINDER_ID, FIRE_AT);
|
||||
expect(bossSendMock).toHaveBeenCalledTimes(1);
|
||||
expect(result).toBe("new-job-id");
|
||||
});
|
||||
|
||||
it("DEGRADES SAFELY: if the cancel UPDATE throws, the send still runs", async () => {
|
||||
// pg connection blip during cancel must not strand the schedule.
|
||||
// Worst case we end up with two created jobs and the
|
||||
// handler-level recent-run dedupe drops the duplicate fire.
|
||||
dbExecuteMock.mockRejectedValueOnce(new Error("connection reset"));
|
||||
const result = await scheduleReminderFire(fakeBoss, REMINDER_ID, FIRE_AT);
|
||||
expect(bossSendMock).toHaveBeenCalledTimes(1);
|
||||
expect(result).toBe("new-job-id");
|
||||
});
|
||||
|
||||
it("returns boss.send's null when pg-boss itself rejects the send", async () => {
|
||||
// Defense check: if pg-boss returns null for any reason (queue
|
||||
// missing, future stately-style policy quirks, etc), surface that
|
||||
// up so the caller's logger captures jobId: null.
|
||||
bossSendMock.mockResolvedValueOnce(null);
|
||||
const result = await scheduleReminderFire(fakeBoss, REMINDER_ID, FIRE_AT);
|
||||
expect(result).toBeNull();
|
||||
});
|
||||
});
|
||||
Loading…
x
Reference in New Issue
Block a user