From ecddfa0376a08752ca52b651b35fb08227e67dbb Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Mon, 17 Aug 2026 13:15:02 -0400 Subject: [PATCH] fix: release superseded engine claims --- packages/api/src/schedules/engine.spec.ts | 13 ++++++++++ packages/api/src/schedules/engine.ts | 29 ++++++++++++++++++----- 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/packages/api/src/schedules/engine.spec.ts b/packages/api/src/schedules/engine.spec.ts index 58eec69202..05e6d2ba4d 100644 --- a/packages/api/src/schedules/engine.spec.ts +++ b/packages/api/src/schedules/engine.spec.ts @@ -49,6 +49,7 @@ function makeMethods(schedule: FireableSchedule) { advanceSchedule: jest.fn( async (_id: string, _next: Date | null, _from?: Date, _token?: string) => true, ), + releaseLeaseByHolder: jest.fn(async () => undefined), disableSchedule: jest.fn(async (_id: string, _reason: string, _token?: string) => undefined), releaseLease: jest.fn(async () => true), holdsLease: jest.fn(async () => true), @@ -135,6 +136,18 @@ describe('runTick misfire skip-forward', () => { expect(fetchSpy).not.toHaveBeenCalled(); }); + it('releases its old holder when an owner edit fences the misfire advance', async () => { + const schedule = makeClaimedSchedule({ nextRunAt: staleAt() }); + const methods = makeMethods(schedule); + // The edit rotated claimToken/nextRunAt after this worker claimed but preserved + // its unique lease holder, so the token-fenced advance no longer matches. + methods.advanceSchedule.mockResolvedValueOnce(false); + + await tickOnce(makeDeps(methods)); + + expect(methods.releaseLeaseByHolder).toHaveBeenCalledWith('sched-1', 'inst-1'); + }); + it('disables and clears a stale occurrence whose cadence is uncomputable', async () => { const schedule = makeClaimedSchedule({ nextRunAt: staleAt(), timezone: 'Not/AZone' }); const methods = makeMethods(schedule); diff --git a/packages/api/src/schedules/engine.ts b/packages/api/src/schedules/engine.ts index c56800f5f5..91e4253ca0 100644 --- a/packages/api/src/schedules/engine.ts +++ b/packages/api/src/schedules/engine.ts @@ -402,6 +402,27 @@ export function startScheduleEngine(deps: ScheduleEngineDeps): ScheduleEngine { return true; } const scheduledFor = schedule.nextRunAt ?? new Date(); + /** + * Advances this claimed occurrence without preserving a superseded holder. + * Owner edits rotate the token/next occurrence but intentionally leave the old + * lease in place; a fenced miss therefore releases only this worker's unique + * holder. Infrastructure errors keep the lease as retry backoff, as before. + */ + const advanceClaim = async (nextRunAt: Date | null): Promise => { + try { + const advanced = await deps.methods.advanceSchedule( + schedule.id, + nextRunAt, + scheduledFor, + schedule.claimToken, + ); + if (!advanced && schedule.leaseBy != null) { + await deps.methods.releaseLeaseByHolder(schedule.id, schedule.leaseBy); + } + } catch { + // Leave the claim lease as bounded backoff for transient storage failures. + } + }; // Use the CLAIM's clock for the misfire cutoff: the claim wrote // leaseUntil = now + LEASE_MS from the claiming worker's clock, so // leaseUntil - LEASE_MS is that worker's "now" at claim — usually this very @@ -440,9 +461,7 @@ export function startScheduleEngine(deps: ScheduleEngineDeps): ScheduleEngine { // next FUTURE occurrence. Without it nextRunAt keeps pointing at the stale // one, so every later tick reclaims and skips the same occurrence forever and // a schedule overdue past an outage never fires again. - await deps.methods - .advanceSchedule(schedule.id, next, scheduledFor, schedule.claimToken) - .catch(() => undefined); + await advanceClaim(next); logger.info(`[schedules] skipped stale occurrence for ${schedule.id} (misfire grace)`); return false; } @@ -492,9 +511,7 @@ export function startScheduleEngine(deps: ScheduleEngineDeps): ScheduleEngine { .then(() => true) .catch(() => false); if (disabled) { - await deps.methods - .advanceSchedule(schedule.id, null, scheduledFor, schedule.claimToken) - .catch(() => undefined); + await advanceClaim(null); } } }