Revalidate reminders synchronously after readiness yields
This commit is contained in:
@@ -245,3 +245,23 @@ test('reminder catch-up yields and rechecks opt-out before claiming or sending',
|
|||||||
expect(f.db.select().from(reminderDeliveries).all()).toHaveLength(0);
|
expect(f.db.select().from(reminderDeliveries).all()).toHaveLength(0);
|
||||||
} finally { f.close(); }
|
} finally { f.close(); }
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('an opt-out queued at readiness completion prevents the final reminder POST', async () => {
|
||||||
|
const { f } = await setup(); let posts = 0, evaluations = 0;
|
||||||
|
const { HabitService } = await import('../habits/service');
|
||||||
|
const original = HabitService.withPrepared;
|
||||||
|
const prepare = spyOn(HabitService, 'withPrepared').mockImplementation((db, user, now, run) => original(db, user, now, service => {
|
||||||
|
const result = run(service);
|
||||||
|
if (++evaluations === 2) queueMicrotask(() => f.db.update(reminderSettings).set({ enabled: false }).run());
|
||||||
|
return result;
|
||||||
|
}));
|
||||||
|
try {
|
||||||
|
await sendDueReminders(f.db, config, async url => {
|
||||||
|
if (!url.endsWith('/users/@me/channels')) posts++;
|
||||||
|
return Response.json({ id: '223456789012345678' });
|
||||||
|
}, () => Date.parse('2026-09-04T18:00:00Z'));
|
||||||
|
expect(evaluations).toBe(2);
|
||||||
|
expect(posts).toBe(0);
|
||||||
|
expect(f.db.select().from(reminderDeliveries).get()?.status).toBe('skipped');
|
||||||
|
} finally { prepare.mockRestore(); f.close(); }
|
||||||
|
});
|
||||||
|
|||||||
@@ -22,38 +22,40 @@ export async function sendDueReminders(db: AppDatabase, config: ReminderConfig,
|
|||||||
// Retain recent delivery history and deduplication records without unbounded growth.
|
// Retain recent delivery history and deduplication records without unbounded growth.
|
||||||
db.delete(reminderDeliveries).where(lt(reminderDeliveries.updatedAt, now() - 90 * 86400000)).run();
|
db.delete(reminderDeliveries).where(lt(reminderDeliveries.updatedAt, now() - 90 * 86400000)).run();
|
||||||
for (const { user } of rows) {
|
for (const { user } of rows) {
|
||||||
async function due(checkDeliveries = false) {
|
function eligible(checkDeliveries = false) {
|
||||||
function eligible() {
|
const currentUser = db.select().from(users).where(eq(users.id, user.id)).get();
|
||||||
const currentUser = db.select().from(users).where(eq(users.id, user.id)).get();
|
const prefs = db.select().from(reminderSettings).where(eq(reminderSettings.userId, user.id)).get();
|
||||||
const prefs = db.select().from(reminderSettings).where(eq(reminderSettings.userId, user.id)).get();
|
if (!currentUser || !prefs?.enabled) return null;
|
||||||
if (!currentUser || !prefs?.enabled) return null;
|
const timestamp = now(); const clock = localClock(timestamp, currentUser.timezone);
|
||||||
const timestamp = now(); const clock = localClock(timestamp, currentUser.timezone);
|
if (inQuietHours(clock, prefs.quietStart, prefs.quietEnd)) return null;
|
||||||
if (inQuietHours(clock, prefs.quietStart, prefs.quietEnd)) return null;
|
const date = localDate(timestamp, currentUser.timezone);
|
||||||
const date = localDate(timestamp, currentUser.timezone);
|
const times = timesForDate(prefs, date).filter(time => time <= clock);
|
||||||
const times = timesForDate(prefs, date).filter(time => time <= clock);
|
if (!times.length) return null;
|
||||||
if (!times.length) return null;
|
if (checkDeliveries) {
|
||||||
if (checkDeliveries) {
|
const previous = db.select().from(reminderDeliveries).where(and(eq(reminderDeliveries.userId, user.id), eq(reminderDeliveries.date, date))).all();
|
||||||
const previous = db.select().from(reminderDeliveries).where(and(eq(reminderDeliveries.userId, user.id), eq(reminderDeliveries.date, date))).all();
|
// This is only a cheap eligibility hint. The transaction below still owns claims.
|
||||||
// This is only a cheap eligibility hint. The transaction below still owns claims.
|
// Let stale pending records reach its recovery step before skipping the user.
|
||||||
// Let stale pending records reach its recovery step before skipping the user.
|
const stalePending = previous.some(row => row.status === 'pending' && row.updatedAt < timestamp - 60000);
|
||||||
const stalePending = previous.some(row => row.status === 'pending' && row.updatedAt < timestamp - 60000);
|
if (!stalePending) {
|
||||||
if (!stalePending) {
|
const legacy = previous.find(row => row.slot === '*');
|
||||||
const legacy = previous.find(row => row.slot === '*');
|
const claimable = (prior: typeof previous[number] | undefined) => !prior || (prior.status === 'deferred' && (prior.retryAt ?? 0) <= timestamp);
|
||||||
const claimable = (prior: typeof previous[number] | undefined) => !prior || (prior.status === 'deferred' && (prior.retryAt ?? 0) <= timestamp);
|
if (previous.some(row => row.status === 'pending') || (legacy ? !claimable(legacy) : !times.some(slot => claimable(previous.find(row => row.slot === slot))))) return null;
|
||||||
if (previous.some(row => row.status === 'pending') || (legacy ? !claimable(legacy) : !times.some(slot => claimable(previous.find(row => row.slot === slot))))) return null;
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
return { currentUser, prefs, date, times };
|
|
||||||
}
|
}
|
||||||
const initial = eligible(); if (!initial) return null;
|
return { currentUser, prefs, date, times };
|
||||||
|
}
|
||||||
|
function readDue(prepared?: HabitService, checkDeliveries = false) {
|
||||||
|
const latest = eligible(checkDeliveries); if (!latest) return null;
|
||||||
|
const owner = latest.currentUser;
|
||||||
|
const service = prepared ?? new HabitService(db, { id: owner.id, discordId: owner.discordId, username: owner.username, displayName: owner.globalName ?? owner.username, avatarUrl: null, timezone: owner.timezone }, now());
|
||||||
|
if (service.today !== latest.date || service.user.timezone !== owner.timezone) return null;
|
||||||
|
const remaining = service.list().filter(habit => { const day = service.day(habit.id, service.today); return day.due && !day.complete; }).length;
|
||||||
|
return remaining ? { date: service.today, remaining, timezone: service.user.timezone, times: latest.times, entries: entriesForDate(latest.prefs, service.today) } : null;
|
||||||
|
}
|
||||||
|
async function due(checkDeliveries = false) {
|
||||||
|
const initial = eligible(checkDeliveries); if (!initial) return null;
|
||||||
const owner = initial.currentUser;
|
const owner = initial.currentUser;
|
||||||
return HabitService.withPrepared(db, { id: owner.id, discordId: owner.discordId, username: owner.username, displayName: owner.globalName ?? owner.username, avatarUrl: null, timezone: owner.timezone }, now, service => {
|
return HabitService.withPrepared(db, { id: owner.id, discordId: owner.discordId, username: owner.username, displayName: owner.globalName ?? owner.username, avatarUrl: null, timezone: owner.timezone }, now, service => readDue(service, checkDeliveries));
|
||||||
// Revalidate preferences and local date after bounded catch-up may have yielded.
|
|
||||||
const latest = eligible();
|
|
||||||
if (!latest || service.today !== latest.date || service.user.timezone !== latest.currentUser.timezone) return null;
|
|
||||||
const remaining = service.list().filter(habit => { const day = service.day(habit.id, service.today); return day.due && !day.complete; }).length;
|
|
||||||
return remaining ? { date: service.today, remaining, timezone: service.user.timezone, times: latest.times, entries: entriesForDate(latest.prefs, service.today) } : null;
|
|
||||||
});
|
|
||||||
}
|
}
|
||||||
const initial = await due(true); if (!initial) continue;
|
const initial = await due(true); if (!initial) continue;
|
||||||
const claims = db.transaction(tx => {
|
const claims = db.transaction(tx => {
|
||||||
@@ -93,7 +95,10 @@ export async function sendDueReminders(db: AppDatabase, config: ReminderConfig,
|
|||||||
const destination = channel.ok ? await channel.json().catch(() => null) as { id?: string } | null : null;
|
const destination = channel.ok ? await channel.json().catch(() => null) as { id?: string } | null : null;
|
||||||
if (!destination?.id || !/^\d{17,20}$/.test(destination.id)) { update("failed"); continue; }
|
if (!destination?.id || !/^\d{17,20}$/.test(destination.id)) { update("failed"); continue; }
|
||||||
// A user may opt out or finish a habit while Discord opens the DM channel.
|
// A user may opt out or finish a habit while Discord opens the DM channel.
|
||||||
const latest = await due();
|
const prepared = await due();
|
||||||
|
// Releasing readiness can run a queued edit before this continuation. Re-read
|
||||||
|
// synchronously, with fresh caches, and invoke POST without another yield.
|
||||||
|
const latest = prepared ? readDue() : null;
|
||||||
if (!latest || latest.date !== initial.date || !claims.some(claim => claim.slot === "*" || latest.times.includes(claim.slot))) { update("skipped"); continue; }
|
if (!latest || latest.date !== initial.date || !claims.some(claim => claim.slot === "*" || latest.times.includes(claim.slot))) { update("skipped"); continue; }
|
||||||
const names = [...new Set(latest.entries.filter(entry => claims.some(claim => claim.slot === "*" ? latest.times.includes(entry.time) : claim.slot === entry.time)).map(entry => entry.name?.trim()).filter((name): name is string => Boolean(name)))];
|
const names = [...new Set(latest.entries.filter(entry => claims.some(claim => claim.slot === "*" ? latest.times.includes(entry.time) : claim.slot === entry.time)).map(entry => entry.name?.trim()).filter((name): name is string => Boolean(name)))];
|
||||||
// Bound the label prefix so the count and link always fit Discord's message limit.
|
// Bound the label prefix so the count and link always fit Discord's message limit.
|
||||||
|
|||||||
Reference in New Issue
Block a user