From 149ec36c77cd4a36196a3a405a50c4322ef4f173 Mon Sep 17 00:00:00 2001 From: syntaxbullet Date: Tue, 8 Sep 2026 16:21:37 +0200 Subject: [PATCH] Prepare worker and Discord snapshots through bounded catch-up --- src/discord/progress.test.ts | 14 ++++++++ src/discord/progress.ts | 8 +++-- src/discord/routes.ts | 16 ++++++---- src/reminders/worker.test.ts | 14 ++++++++ src/reminders/worker.ts | 58 +++++++++++++++++++--------------- src/sharing/live-card-image.ts | 13 +++++--- src/sharing/live-discord.ts | 6 ++-- src/sharing/routes.ts | 24 +++++++------- 8 files changed, 99 insertions(+), 54 deletions(-) diff --git a/src/discord/progress.test.ts b/src/discord/progress.test.ts index 1e7c711..c18497e 100644 --- a/src/discord/progress.test.ts +++ b/src/discord/progress.test.ts @@ -112,3 +112,17 @@ test('snapshot quotas are shared with website operations and new controls cannot f.setDiscordTime('2026-09-04T12:16:00Z'); expect((await click(preview, 'Choose public destination')).data.content).toContain('expired'); }); + +test('a website edit during a deferred public share invalidates the prepared snapshot', async () => { + const env = setup(); + const habit = await f.json('/habits', 'POST', { name: 'Reading', method: 'manual' }, 201); + const panel = await f.run({ data: { name: 'progress' } }); + await click(panel, 'Preview a habit', false, [habit.id]); const preview = await env.drain(); + await click(preview, 'Choose public destination'); const confirmation = await env.drain(); + await click(confirmation, 'Share snapshot publicly'); + const delivery = env.drain(); + await f.json(`/habits/${habit.id}/days/2026-09-04/progress`, 'PUT', { done: true }); + const result = await delivery; + expect(result.data.content).toContain('changed'); + expect(env.sends).toHaveLength(0); +}); diff --git a/src/discord/progress.ts b/src/discord/progress.ts index 56eac98..236453c 100644 --- a/src/discord/progress.ts +++ b/src/discord/progress.ts @@ -1,6 +1,6 @@ import { createHash } from 'node:crypto'; import { renderProgressImage } from './progress-image'; -import { ApiError } from '../habits/service'; +import { ApiError, HabitService } from '../habits/service'; import { liveCardData } from '../sharing/live-card-image'; import type { createSharingRoutes } from '../sharing/routes'; import type { createLiveDiscord } from '../sharing/live-discord'; @@ -14,9 +14,11 @@ export type ProgressOperations = { export type DeferredMessage = { message: Message; image?: Uint8Array; imageDescription?: string }; const fingerprint = (value: unknown) => createHash('sha256').update(JSON.stringify(value)).digest('hex'); function imageData(c: Context, data: ControlData) { - const result = liveCardData(c.db, c.service.user.id, c.now); + // Preparation happened before dispatch. Fresh read caches detect edits across deferred awaits. + const snapshot = new HabitService(c.db, c.service.user, c.now); + const result = liveCardData(c.db, c.service.user.id, c.now, snapshot); if (data.habitId) { - const habits = c.service.list(); + const habits = snapshot.list(); const index = habits.findIndex(h => h.id === data.habitId); if (index < 0) throw new ApiError(409, 'This habit changed. Run /progress again.'); result.habits = [result.habits[index]!]; diff --git a/src/discord/routes.ts b/src/discord/routes.ts index 1905933..c213d33 100644 --- a/src/discord/routes.ts +++ b/src/discord/routes.ts @@ -54,15 +54,17 @@ export function createDiscordRoutes(db: AppDatabase, config: CommandConfig, now // Help needs no account creation, habit synchronization, or saved UI state. if (i.type === 2 && i.data?.name === 'help') return c.json(help(!!i.guild_id, config.origin)); let pending: (() => Promise) | undefined; - const reply = db.transaction((): Reply => { - const existing = db.select().from(users).where(eq(users.discordId, profile.id)).get(); - const userId = existing?.id ?? crypto.randomUUID(); - if (!existing) db.insert(users).values({ id: userId, discordId: profile.id, username: profile.username, globalName: profile.global_name, avatarHash: profile.avatar, createdAt: now(), updatedAt: now() }).run(); + // Resolve the account before queueing catch-up; no transaction is held across yields. + const existing = db.select().from(users).where(eq(users.discordId, profile.id)).get(); + const userId = existing?.id ?? crypto.randomUUID(); + if (!existing) db.insert(users).values({ id: userId, discordId: profile.id, username: profile.username, globalName: profile.global_name, avatarHash: profile.avatar, createdAt: now(), updatedAt: now() }).run(); + const receipt = db.select().from(discordReceipts).where(eq(discordReceipts.id, i.id)).get(); + if (receipt && receipt.expiresAt > now()) return c.json(receipt.response); + const owner = publicUser(db.select().from(users).where(eq(users.id, userId)).get()!); + const reply = await HabitService.withPrepared(db, owner, now, service => db.transaction((): Reply => { const receipt = db.select().from(discordReceipts).where(eq(discordReceipts.id, i.id)).get(); if (receipt && receipt.expiresAt > now()) return receipt.response; - const user = publicUser(db.select().from(users).where(eq(users.id, userId)).get()!); const controls = new ControlStore(db, userId, now()); controls.prune(); - const service = new HabitService(db, user, now()); service.sync(); const ctx: Context = { db, service, controls, interaction: i, now: now(), origin: config.origin, remindersAvailable: config.remindersAvailable ?? false, changed: () => changed(userId), progress: dispatch.progress, defer: i.token ? job => { pending = job; } : undefined }; let result: Reply; try { @@ -83,7 +85,7 @@ export function createDiscordRoutes(db: AppDatabase, config: CommandConfig, now } db.insert(discordReceipts).values({ id: i.id, userId, response: result, expiresAt: now() + 86400000 }).run(); return result; - }); + })); if (pending) { const work = pending; const job = async () => { diff --git a/src/reminders/worker.test.ts b/src/reminders/worker.test.ts index d72e2e2..279175f 100644 --- a/src/reminders/worker.test.ts +++ b/src/reminders/worker.test.ts @@ -231,3 +231,17 @@ test('delivered slots skip habit evaluation while later slots and retries remain await tick('10:00'); expect(sync).toHaveBeenCalled(); expect(sent).toHaveLength(2); } finally { sync?.mockRestore(); f.close(); } }); + +test('reminder catch-up yields and rechecks opt-out before claiming or sending', async () => { + const f = fixture(); let requests = 0; + try { + await f.json('/habits', 'POST', { name: 'Backlog', method: 'tasks', tasks: Array.from({ length: 50 }, (_, i) => ({ name: `Task ${i}` })) }, 201); + f.db.insert(reminderSettings).values({ userId: 'alice', ...defaultReminder, enabled: true, updatedAt: 0 }).run(); + const work = sendDueReminders(f.db, config, async () => { requests++; throw new Error('No sends expected'); }, () => Date.parse('2027-09-04T18:00:00Z')); + await new Promise(resolve => setTimeout(resolve, 0)); + f.db.update(reminderSettings).set({ enabled: false }).run(); + await work; + expect(requests).toBe(0); + expect(f.db.select().from(reminderDeliveries).all()).toHaveLength(0); + } finally { f.close(); } +}); diff --git a/src/reminders/worker.ts b/src/reminders/worker.ts index 86ac3d3..db55b83 100644 --- a/src/reminders/worker.ts +++ b/src/reminders/worker.ts @@ -22,34 +22,40 @@ export async function sendDueReminders(db: AppDatabase, config: ReminderConfig, // Retain recent delivery history and deduplication records without unbounded growth. db.delete(reminderDeliveries).where(lt(reminderDeliveries.updatedAt, now() - 90 * 86400000)).run(); for (const { user } of rows) { - function due(checkDeliveries = false) { - 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(); - if (!currentUser || !prefs?.enabled) return null; - const timestamp = now(); const clock = localClock(timestamp, currentUser.timezone); - if (inQuietHours(clock, prefs.quietStart, prefs.quietEnd)) return null; - const date = localDate(timestamp, currentUser.timezone); - const times = timesForDate(prefs, date).filter(time => time <= clock); - if (!times.length) return null; - if (checkDeliveries) { - 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. - // Let stale pending records reach its recovery step before skipping the user. - const stalePending = previous.some(row => row.status === 'pending' && row.updatedAt < timestamp - 60000); - if (!stalePending) { - const legacy = previous.find(row => row.slot === '*'); - 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; + async function due(checkDeliveries = false) { + function eligible() { + 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(); + if (!currentUser || !prefs?.enabled) return null; + const timestamp = now(); const clock = localClock(timestamp, currentUser.timezone); + if (inQuietHours(clock, prefs.quietStart, prefs.quietEnd)) return null; + const date = localDate(timestamp, currentUser.timezone); + const times = timesForDate(prefs, date).filter(time => time <= clock); + if (!times.length) return null; + if (checkDeliveries) { + 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. + // Let stale pending records reach its recovery step before skipping the user. + const stalePending = previous.some(row => row.status === 'pending' && row.updatedAt < timestamp - 60000); + if (!stalePending) { + const legacy = previous.find(row => row.slot === '*'); + 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; + } } + return { currentUser, prefs, date, times }; } - const service = new HabitService(db, { id: currentUser.id, discordId: currentUser.discordId, username: currentUser.username, displayName: currentUser.globalName ?? currentUser.username, avatarUrl: null, timezone: currentUser.timezone }, timestamp); - service.sync(); - // Avoid sending an artificial "today" after a backwards timezone change. - if (service.today !== localDate(timestamp, 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: currentUser.timezone, times, entries: entriesForDate(prefs, service.today) } : null; + const initial = eligible(); if (!initial) return null; + 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 => { + // 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 = due(true); if (!initial) continue; + const initial = await due(true); if (!initial) continue; const claims = db.transaction(tx => { // A crashed/aborted send must not block later times indefinitely. tx.update(reminderDeliveries).set({ status: "uncertain" }).where(and(eq(reminderDeliveries.userId, user.id), eq(reminderDeliveries.status, "pending"), lt(reminderDeliveries.updatedAt, now() - 60000))).run(); @@ -87,7 +93,7 @@ export async function sendDueReminders(db: AppDatabase, config: ReminderConfig, 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; } // A user may opt out or finish a habit while Discord opens the DM channel. - const latest = due(); + const latest = await due(); 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)))]; // Bound the label prefix so the count and link always fit Discord's message limit. diff --git a/src/sharing/live-card-image.ts b/src/sharing/live-card-image.ts index 1dab9c4..992a541 100644 --- a/src/sharing/live-card-image.ts +++ b/src/sharing/live-card-image.ts @@ -3,7 +3,7 @@ import { cachedAvatar } from './avatar-cache'; import { createCanvas, GlobalFonts } from "@napi-rs/canvas"; import { existsSync } from "node:fs"; import { eq } from "drizzle-orm"; -import type { AppDatabase } from "../auth"; +import { publicUser, type AppDatabase } from "../auth"; import { users } from "../db/schema"; import { HabitService, ApiError } from "../habits/service"; import { addDays } from "../habits/calendar"; @@ -25,11 +25,16 @@ for (const [file, family] of [ } } -export function liveCardData(db: AppDatabase, userId: string, now: number) { +export async function loadLiveCardData(db: AppDatabase, userId: string, now: () => number) { + const user = db.select().from(users).where(eq(users.id, userId)).get(); + if (!user) throw new ApiError(404, 'Account unavailable.'); + return HabitService.withPrepared(db, publicUser(user), now, service => liveCardData(db, userId, service.now, service)); +} +export function liveCardData(db: AppDatabase, userId: string, now: number, prepared?: HabitService) { const user = db.select().from(users).where(eq(users.id, userId)).get(); if (!user) throw new ApiError(404, "Account unavailable."); - const service = new HabitService(db, { ...user, displayName: user.globalName ?? user.username, avatarUrl: null }, now); - service.sync(); + const service = prepared ?? new HabitService(db, { ...user, displayName: user.globalName ?? user.username, avatarUrl: null }, now); + if (!prepared) service.sync(); const from = addDays(service.today, -29); const avatarUrl = /^\d{17,20}$/.test(user.discordId) && /^(?:a_)?[a-f0-9]{32}$/i.test(user.avatarHash ?? '') ? `https://cdn.discordapp.com/avatars/${user.discordId}/${user.avatarHash}.png?size=128` : null; diff --git a/src/sharing/live-discord.ts b/src/sharing/live-discord.ts index 7a0d738..c54de1c 100644 --- a/src/sharing/live-discord.ts +++ b/src/sharing/live-discord.ts @@ -8,7 +8,7 @@ import type { AppDatabase, AuthEnv, createAuth } from "../auth"; import { liveDiscordCards, users } from "../db/schema"; import { ApiError } from "../habits/service"; import { endOfDay, localDate } from "../habits/calendar"; -import { liveCardData, renderLiveCard } from "./live-card-image"; +import { loadLiveCardData, renderLiveCard } from "./live-card-image"; import type { PublicUser } from "../shared/user"; import type { DiscordSharingConfig } from "./config"; import type { DiscordFetch } from "./routes"; @@ -65,7 +65,7 @@ export function createLiveDiscord(db: AppDatabase, auth: ReturnType 0) { schedule(id, delay); return; } update(id, { dirty: false, retryAt: null }); - const data = liveCardData(db, id, now()); + const data = await loadLiveCardData(db, id, now); // Hash the rendered content, excluding timestamps and private task details. const hash = createHash('sha256').update(JSON.stringify({ data, theme: current.theme, renderer: 3, content: liveMessageText })).digest('hex'); if (current.messageId && current.imageHash === hash) { @@ -157,7 +157,7 @@ export function createLiveDiscord(db: AppDatabase, auth: ReturnType c.json(state(c.get('user').id))); app.get('/preview', async c => { const theme = c.req.query('theme') === 'dark' ? 'dark' : 'light'; - const image = await renderLiveCard(liveCardData(db, c.get('user').id, now()), theme, c.get('user').id); + const image = await renderLiveCard(await loadLiveCardData(db, c.get('user').id, now), theme, c.get('user').id); c.header('Content-Type', 'image/png'); c.header('Content-Disposition', 'inline; filename="minabot-live-progress.png"'); return c.body(new Uint8Array(image)); }); diff --git a/src/sharing/routes.ts b/src/sharing/routes.ts index 0bf81b8..2425405 100644 --- a/src/sharing/routes.ts +++ b/src/sharing/routes.ts @@ -78,17 +78,19 @@ export function createSharingRoutes(db: AppDatabase, auth: ReturnType { const input = await json(c.req.raw, shareInput); - const service = new HabitService(db, c.get("user"), now()); service.sync(); - if (input.to > service.today) throw new ApiError(422, "The end date cannot be after today."); - const result: ShareData = { from: input.from, to: input.to, timezone: service.user.timezone, habits: input.habitIds.map(id => { - const habit = service.current(id); - const chart = service.calendar([id], input.from, input.to, service.settings(id), false); - return { name: habit.name, color: chart.settings.mainColor, - legend: { empty: chart.settings.emptyColor, complete: chart.settings.mainColor, - partial: chart.days.some(d => d.shadeCount > 1) ? [0.25, 0.5, 0.75].map(ratio => shade(ratio, 4, chart.settings, false).color) : [] }, - completed: chart.days.filter(d => d.completed > 0).length, scheduled: chart.days.filter(d => d.due > 0).length, - days: chart.days.map(d => ({ date: d.date, color: d.color, due: d.due > 0, complete: d.completed > 0, ratio: d.ratio })) }; - }) }; + const result = await HabitService.withPrepared(db, c.get("user"), now, service => { + if (input.to > service.today) throw new ApiError(422, "The end date cannot be after today."); + const result: ShareData = { from: input.from, to: input.to, timezone: service.user.timezone, habits: input.habitIds.map(id => { + const habit = service.current(id); + const chart = service.calendar([id], input.from, input.to, service.settings(id), false); + return { name: habit.name, color: chart.settings.mainColor, + legend: { empty: chart.settings.emptyColor, complete: chart.settings.mainColor, + partial: chart.days.some(d => d.shadeCount > 1) ? [0.25, 0.5, 0.75].map(ratio => shade(ratio, 4, chart.settings, false).color) : [] }, + completed: chart.days.filter(d => d.completed > 0).length, scheduled: chart.days.filter(d => d.due > 0).length, + days: chart.days.map(d => ({ date: d.date, color: d.color, due: d.due > 0, complete: d.completed > 0, ratio: d.ratio })) }; + }) }; + return result; + }); result.botAvatarUrl = await botAvatarUrl(); return c.json(result); });