diff --git a/apps/bot/src/lib/safe-remove-job.ts b/apps/bot/src/lib/safe-remove-job.ts index b785218..2a65889 100644 --- a/apps/bot/src/lib/safe-remove-job.ts +++ b/apps/bot/src/lib/safe-remove-job.ts @@ -5,6 +5,8 @@ import { logger } from '../logger.js'; * Removes a BullMQ job when present. Locked (active) jobs cannot be removed — * those errors are swallowed so callers can still schedule a replacement jobId * or rely on removeOnComplete after the worker finishes. + * Jobs that belong to a Job Scheduler must be removed via removeJobScheduler; + * direct remove is ignored here. */ export async function safeRemoveBullJob( job: Job | null | undefined, @@ -17,10 +19,14 @@ export async function safeRemoveBullJob( await job.remove(); } catch (error) { const message = error instanceof Error ? error.message : String(error); - if (message.includes('locked') || message.includes('could not be removed')) { + if ( + message.includes('locked') || + message.includes('could not be removed') || + message.includes('belongs to a job scheduler') + ) { logger.warn( { error, jobId: context?.jobId ?? job.id, label: context?.label }, - 'BullMQ job is locked; skipping remove' + 'BullMQ job could not be removed directly; skipping' ); return; } diff --git a/apps/bot/src/modules/scheduler/service.ts b/apps/bot/src/modules/scheduler/service.ts index 52274ee..bebf892 100644 --- a/apps/bot/src/modules/scheduler/service.ts +++ b/apps/bot/src/modules/scheduler/service.ts @@ -78,6 +78,58 @@ export function scheduleJobId(scheduleId: string): string { return `schedule-${scheduleId}`; } +/** BullMQ delayed instance ids look like `repeat:{schedulerId}:{millis}`. */ +const REPEAT_INSTANCE_ID_RE = /^repeat:(.+):(\d+)$/; + +function collectSchedulerIds(scheduleId: string, jobId: string | null | undefined): string[] { + const ids = new Set(); + ids.add(scheduleJobId(scheduleId)); + if (!jobId) { + return [...ids]; + } + + const repeatMatch = jobId.match(REPEAT_INSTANCE_ID_RE); + if (repeatMatch?.[1]) { + ids.add(repeatMatch[1]); + } else if (!jobId.startsWith('repeat:')) { + ids.add(jobId); + } + return [...ids]; +} + +async function removeScheduleJobSchedulers( + queue: typeof scheduleQueue, + scheduleId: string, + jobId: string | null | undefined +): Promise { + for (const schedulerId of collectSchedulerIds(scheduleId, jobId)) { + try { + await queue.removeJobScheduler(schedulerId); + } catch (error) { + logger.warn({ schedulerId, error }, 'Failed to remove schedule job scheduler'); + } + } + + // Legacy / hash-keyed schedulers: match by payload scheduleId. + try { + const schedulers = await queue.getJobSchedulers(); + for (const scheduler of schedulers) { + const data = scheduler.template?.data as { scheduleId?: string } | undefined; + if (data?.scheduleId !== scheduleId) { + continue; + } + const id = scheduler.id ?? scheduler.key; + try { + await queue.removeJobScheduler(id); + } catch (error) { + logger.warn({ schedulerId: id, error }, 'Failed to remove matched schedule job scheduler'); + } + } + } catch (error) { + logger.warn({ scheduleId, error }, 'Failed to scan job schedulers for schedule'); + } +} + function isValidCron(pattern: string): boolean { const parts = pattern.trim().split(/\s+/); return parts.length >= 5 && parts.length <= 6; @@ -230,25 +282,36 @@ export async function removeScheduleJob( jobId: string | null, options?: { scheduleId?: string; cron?: string | null } ): Promise { - const schedulerId = options?.scheduleId ? scheduleJobId(options.scheduleId) : jobId; - if (schedulerId && options?.cron) { - try { - await scheduleQueue.removeJobScheduler(schedulerId); - } catch (error) { - logger.warn({ schedulerId, error }, 'Failed to remove schedule job scheduler'); + if (options?.scheduleId) { + await removeScheduleJobSchedulers(scheduleQueue, options.scheduleId, jobId); + } else if (options?.cron && jobId) { + // Best-effort for callers without scheduleId: extract scheduler from repeat instance id. + for (const schedulerId of collectSchedulerIds('unknown', jobId)) { + if (schedulerId === 'schedule-unknown') { + continue; + } + try { + await scheduleQueue.removeJobScheduler(schedulerId); + } catch (error) { + logger.warn({ schedulerId, error }, 'Failed to remove schedule job scheduler'); + } } } - const candidateIds = new Set(); - if (jobId) { - candidateIds.add(jobId); + // One-shot delayed jobs only — never job.remove() on repeat:* scheduler children. + const oneShotIds = new Set(); + if (jobId && !jobId.startsWith('repeat:')) { + oneShotIds.add(jobId); } - if (schedulerId) { - candidateIds.add(schedulerId); + if (options?.scheduleId) { + oneShotIds.add(scheduleJobId(options.scheduleId)); } - for (const id of candidateIds) { + for (const id of oneShotIds) { const job = await scheduleQueue.getJob(id); + if (job?.id && String(job.id).startsWith('repeat:')) { + continue; + } await safeRemoveBullJob(job, { label: 'schedule', jobId: id }); } } diff --git a/apps/webui/src/lib/module-configs/scheduler.ts b/apps/webui/src/lib/module-configs/scheduler.ts index 0ba9ed9..7db0fe5 100644 --- a/apps/webui/src/lib/module-configs/scheduler.ts +++ b/apps/webui/src/lib/module-configs/scheduler.ts @@ -48,6 +48,81 @@ function scheduleJobId(scheduleId: string): string { return `schedule-${scheduleId}`; } +/** BullMQ delayed instance ids look like `repeat:{schedulerId}:{millis}`. */ +const REPEAT_INSTANCE_ID_RE = /^repeat:(.+):(\d+)$/; + +function collectSchedulerIds(scheduleId: string, jobId: string | null | undefined): string[] { + const ids = new Set(); + ids.add(scheduleJobId(scheduleId)); + if (!jobId) { + return [...ids]; + } + + const repeatMatch = jobId.match(REPEAT_INSTANCE_ID_RE); + if (repeatMatch?.[1]) { + ids.add(repeatMatch[1]); + } else if (!jobId.startsWith('repeat:')) { + ids.add(jobId); + } + return [...ids]; +} + +async function removeQueueSchedulersForSchedule( + scheduleId: string, + jobId: string | null | undefined +): Promise { + const queue = getScheduleQueue(); + + for (const schedulerId of collectSchedulerIds(scheduleId, jobId)) { + try { + await queue.removeJobScheduler(schedulerId); + } catch { + // Already gone or legacy key — keep trying other candidates. + } + } + + try { + const schedulers = await queue.getJobSchedulers(); + for (const scheduler of schedulers) { + const data = scheduler.template?.data as { scheduleId?: string } | undefined; + if (data?.scheduleId !== scheduleId) { + continue; + } + const id = scheduler.id ?? scheduler.key; + try { + await queue.removeJobScheduler(id); + } catch { + // ignore + } + } + } catch { + // ignore scan failures + } + + // Older BullMQ repeatables (pre job-scheduler API). + try { + const repeatables = await queue.getRepeatableJobs(); + for (const repeatable of repeatables) { + const matchesId = + repeatable.id === scheduleJobId(scheduleId) || + (jobId != null && + (repeatable.key === jobId || + (REPEAT_INSTANCE_ID_RE.test(jobId) && + repeatable.key.includes(jobId.match(REPEAT_INSTANCE_ID_RE)?.[1] ?? '')))); + if (!matchesId) { + continue; + } + try { + await queue.removeRepeatableByKey(repeatable.key); + } catch { + // ignore + } + } + } catch { + // ignore + } +} + export async function listScheduledMessagesDashboard(guildId: string): Promise { const schedules = await prisma.scheduledMessage.findMany({ where: { guildId, enabled: true }, @@ -168,34 +243,33 @@ export async function deleteScheduledMessageDashboard(guildId: string, scheduleI } const queue = getScheduleQueue(); - const schedulerId = scheduleJobId(existing.id); + await removeQueueSchedulersForSchedule(existing.id, existing.jobId); - if (existing.cron) { - try { - await queue.removeJobScheduler(schedulerId); - } catch { - // Scheduler may already be gone (legacy repeatables / one-shots). - } + // One-shot delayed jobs only — never job.remove() on repeat:* scheduler children + // (BullMQ throws code -8: belongs to a job scheduler). + const oneShotIds = new Set(); + oneShotIds.add(scheduleJobId(existing.id)); + if (existing.jobId && !existing.jobId.startsWith('repeat:')) { + oneShotIds.add(existing.jobId); } - const candidateIds = new Set(); - candidateIds.add(schedulerId); - if (existing.jobId) { - candidateIds.add(existing.jobId); - } - - for (const id of candidateIds) { + for (const id of oneShotIds) { const job = await queue.getJob(id); - if (!job) { + if (!job || (job.id && String(job.id).startsWith('repeat:'))) { continue; } try { await job.remove(); } catch (error) { const message = error instanceof Error ? error.message : String(error); - if (!message.includes('locked') && !message.includes('could not be removed')) { - throw error; + if ( + message.includes('locked') || + message.includes('could not be removed') || + message.includes('belongs to a job scheduler') + ) { + continue; } + throw error; } } diff --git a/docs/PHASE-TRACKING.md b/docs/PHASE-TRACKING.md index 7f8bd89..c5829d4 100644 --- a/docs/PHASE-TRACKING.md +++ b/docs/PHASE-TRACKING.md @@ -607,11 +607,13 @@ - Dashboard: Timing-Mode (einmalig vs. Cron), naive `datetime-local` an Server, Fehlertexte i18n - Dark Mode: `color-scheme` + Input `text-foreground` für datetime-local Lesbarkeit - Shared Helper `resolveScheduleRunAt` / `guildLocalDateTimeToUtc` +- Delete: Scheduler-Kinder (`repeat:…`) nicht per `job.remove()`; Legacy-Hash-IDs via `removeJobScheduler` / Scan ### Manuell testen - [ ] WebUI einmalig: Datum/Uhrzeit in Server-Zeitzone → Nachricht zur erwarteten lokalen Zeit - [ ] WebUI Cron `0 9 * * *` bei Europe/Berlin → ca. 09:00 Berlin (nicht UTC) - [ ] Cron-Plan löschen → keine weiteren Sends +- [ ] Alten Cron-Eintrag (jobId `repeat:…`) im Dashboard löschen → kein API-Error -8 - [ ] Dark Mode: Cron-Feld und datetime-local lesbar/bedienbar