diff --git a/packages/api/src/schedules/fire.spec.ts b/packages/api/src/schedules/fire.spec.ts index 82eaf4e4b4..83f1d68ae8 100644 --- a/packages/api/src/schedules/fire.spec.ts +++ b/packages/api/src/schedules/fire.spec.ts @@ -415,7 +415,10 @@ describe('fireSchedule', () => { }); it('releases the lease on a manual run-now when file resolution fails', async () => { - const { methods, runs, calls } = makeMethods(); + const { methods, runs } = makeMethods(); + // An owner edit can rotate the token while the file lookup is in flight while + // deliberately preserving this fire's unique lease holder. + methods.releaseLease.mockResolvedValueOnce(false); mockFetch(async () => okResponse()); const deps = makeDeps(methods, { resolveFiles: async () => { @@ -428,7 +431,8 @@ describe('fireSchedule', () => { expect(result.fired).toBe(false); expect(runs.size).toBe(0); // Run-now releases so the user can retry immediately (no misleading lease-held 409). - expect(calls.releaseLease).toBe(1); + expect(methods.releaseLease).toHaveBeenCalledWith('sched-1', 'ct-1'); + expect(methods.releaseLeaseByHolder).toHaveBeenCalledWith('sched-1', 'inst-1'); expect(global.fetch).not.toHaveBeenCalled(); }); diff --git a/packages/api/src/schedules/fire.ts b/packages/api/src/schedules/fire.ts index c6b8c9d1ca..cf05d10b9c 100644 --- a/packages/api/src/schedules/fire.ts +++ b/packages/api/src/schedules/fire.ts @@ -156,15 +156,35 @@ export async function fireSchedule( } }; + /** + * Releases a manual run's lease without leaving its old holder behind when an owner + * edit rotates the claim token during an asynchronous preflight. The token fence is + * authoritative when it still matches; the unique holder fallback can only clear this + * fire's lease and safely no-ops if another worker has taken it over. + */ + const releaseManualLease = async () => { + const released = await methods.releaseLease(schedule.id, claimToken); + if (!released) { + await releaseSupersededLease(); + } + return released; + }; + // A manual run-now must never reschedule the next automatic occurrence; it only // releases its serialization lease. Both writes are token-fenced, so an owner edit // racing after durable enqueue can make them miss while deliberately preserving the // old holder fields. Release by the unique OLD holder on a miss: an edit is unwedged // immediately, while a takeover changed leaseBy and remains untouched. const advance = async () => { - const advanced = options?.manual - ? await methods.releaseLease(schedule.id, claimToken) - : await methods.advanceSchedule(schedule.id, nextRunAt, scheduledFor, claimToken); + if (options?.manual) { + return releaseManualLease(); + } + const advanced = await methods.advanceSchedule( + schedule.id, + nextRunAt, + scheduledFor, + claimToken, + ); if (!advanced) { await releaseSupersededLease(); } @@ -183,9 +203,10 @@ export async function fireSchedule( * so a repeat click isn't met with a stale "already in progress". */ const stepAsideSuperseded = async () => { - await releaseSupersededLease(); if (options?.manual) { - await methods.releaseLease(schedule.id, claimToken); + await releaseManualLease(); + } else { + await releaseSupersededLease(); } return { fired: false, skipped: 'superseded' as const }; }; @@ -311,7 +332,7 @@ export async function fireSchedule( // re-pick this failing row every tick and starve others / hammer the file // lookup); manual run-now releases so the user can retry immediately. if (options?.manual) { - await methods.releaseLease(schedule.id, claimToken); + await releaseManualLease(); } return { fired: false, error: 'File resolution failed' }; } @@ -390,7 +411,7 @@ export async function fireSchedule( // MUST release its lease, or repeated Run-now clicks hit a misleading "already // in progress" 409 for the full manual-lease TTL even after capacity frees. if (options?.manual) { - await methods.releaseLease(schedule.id, claimToken); + await releaseManualLease(); } return { fired: false, skipped: 'capacity' as const }; } @@ -443,7 +464,7 @@ export async function fireSchedule( // Manual run-now still releases its lease so repeated clicks aren't met with a // stale "already in progress". if (options?.manual) { - await methods.releaseLease(schedule.id, claimToken); + await releaseManualLease(); } return { fired: false, skipped: 'duplicate' as const }; }