From 3e667fe3c6a234f7c4e5c2b980ad14a99ac57d77 Mon Sep 17 00:00:00 2001 From: Agent Date: Thu, 10 Sep 2026 11:28:53 -0400 Subject: [PATCH] fix: reschedule cron job when confirmed subscriber changes frequency MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a confirmed subscriber calls PUT /subscription/email with a changed frequency, db.updateSubscription updates the row but the running CronJob captured the original frequency in createJob and was never rescheduled. A subscriber switching daily→weekly kept the daily cadence until restart. Fix: in actions.ts:registerSubscription, call worker.registerSubscription(sub) when the subscription is already confirmed, so addJob stops the old job and creates a new one with the updated frequency. Changes: - src/actions.ts: add else branch calling worker.registerSubscription when sub.confirmed_at is set - test/reschedule-on-frequency-change.test.js: new failing-before/passing-after test verifying the cron expression is updated after frequency change Closes mailship-e04 --- src/actions.ts | 8 +- src/worker/email.ts | 6 ++ test/reschedule-on-frequency-change.test.js | 83 +++++++++++++++++++++ 3 files changed, 94 insertions(+), 3 deletions(-) create mode 100644 test/reschedule-on-frequency-change.test.js diff --git a/src/actions.ts b/src/actions.ts index ceec6fe..c28caee 100644 --- a/src/actions.ts +++ b/src/actions.ts @@ -23,11 +23,13 @@ export const registerSubscription = instrument( const sub = await db.insertSubscription(pubkey, email, frequency) const callback = `${process.env.BASE_URL}/notify/${sub.id}` - // Only send a confirmation when the subscription is new, unconfirmed, or - // its email address changed. An already-confirmed, unchanged subscription - // (or one where only the frequency changed) skips it. if (!sub.confirmed_at) { + // New or email-changed subscription — send a confirmation email. await mailer.sendConfirm(sub) + } else { + // Already confirmed (e.g. frequency-only change) — reschedule the + // cron job so it uses the new cadence immediately. + worker.registerSubscription(sub) } return { key: sub.key, callback } diff --git a/src/worker/email.ts b/src/worker/email.ts index d60e4a3..7a9b0c6 100644 --- a/src/worker/email.ts +++ b/src/worker/email.ts @@ -6,6 +6,12 @@ import * as db from '../database.js' const jobsById = new Map() +// Test-only accessor to inspect stored jobs +export const getJobCronSource = (id: string): string | undefined => { + const source = jobsById.get(id)?.cronTime.source + return typeof source === 'string' ? source : undefined +} + export const runJob = async (sub: Subscription) => { try { if (!sub.confirmed_at || sub.unsubscribed_at) { diff --git a/test/reschedule-on-frequency-change.test.js b/test/reschedule-on-frequency-change.test.js new file mode 100644 index 0000000..f67229f --- /dev/null +++ b/test/reschedule-on-frequency-change.test.js @@ -0,0 +1,83 @@ +#!/usr/bin/env node +// FAILING test: frequency change does not reschedule the running cron job. +// +// The fix: actions.ts:registerSubscription calls worker.registerSubscription() +// after a DB update on an already-confirmed subscription, so addJob creates +// a new CronJob with the updated frequency on the fly. +// +// Step 1-2: Create subscription via raw DB (bypass mailer for simplicity) +// Step 3: Register cron job with daily (simulating confirmSubscriptionAction) +// Step 4: Call registerSubscription with new frequency — the bug path +// BEFORE FIX: cron stays daily; AFTER FIX: cron becomes weekly + +import * as db from '../dist/database.js' +import { registerSubscription } from '../dist/actions.js' +import { getJobCronSource, removeJob } from '../dist/worker/email.js' +import { registerSubscription as regSub } from '../dist/worker/index.js' + +let passed = 0 +let failed = 0 + +function assert(label, ok, detail) { + if (ok) { + console.log(` ? ${label}`) + passed++ + } else { + console.log(` ? ${label} -- ${detail || ''}`) + failed++ + } +} + +async function main() { + await db.migrate() + + const pubkey = 'freq-test-' + Date.now() + const email = 'freq-test-' + Date.now() + '@example.com' + + // Step 1: Insert subscription directly (bypass mailer) and confirm + console.log('1. Create confirmed subscription with daily frequency') + const sub = await db.insertSubscription(pubkey, email, 'daily') + assert('subscription created', !!sub, 'insert returned null') + if (!sub) { process.exit(1) } + + const confirmed = await db.confirmSubscription(sub.key) + assert('subscription confirmed', !!confirmed, 'confirm returned null') + if (!confirmed) { process.exit(1) } + + // Step 2: Register cron job with daily (simulating confirmSubscriptionAction) + console.log('\n2. Register cron job with daily frequency') + regSub(confirmed) + const dailySource = getJobCronSource(confirmed.id) + assert( + 'cron source is daily', + dailySource === '0 0 17 * * *', + `expected 0 0 17 * * *, got ${dailySource}` + ) + + // Step 3: Register subscription again with weekly — the bug path. + // BEFORE FIX: registerSubscription skips worker call because + // sub.confirmed_at is set → cron stays daily + // AFTER FIX: registerSubscription calls worker.registerSubscription + // → addJob reschedules → cron becomes weekly + console.log('\n3. Change frequency to weekly via registerSubscription') + await registerSubscription({ pubkey, email, frequency: 'weekly' }) + const weeklySource = getJobCronSource(confirmed.id) + assert( + 'cron source is weekly after frequency change', + weeklySource === '0 0 17 * * 1', + `expected 0 0 17 * * 1, got ${weeklySource}` + ) + + // Cleanup + const updated = await db.getSubscriptionByPubkey(pubkey) + if (updated) removeJob(updated) + + console.log('') + console.log(`Results: ${passed} passed, ${failed} failed`) + process.exit(failed > 0 ? 1 : 0) +} + +main().catch(err => { + console.error('Unhandled error in test:', err) + process.exit(1) +}) \ No newline at end of file