diff --git a/packages/api/src/schedules/fire.spec.ts b/packages/api/src/schedules/fire.spec.ts index 8221c83db6..df9c71daae 100644 --- a/packages/api/src/schedules/fire.spec.ts +++ b/packages/api/src/schedules/fire.spec.ts @@ -570,6 +570,44 @@ describe('fireSchedule', () => { expect(global.fetch).not.toHaveBeenCalled(); }); + /** + * A balance skip is not a no-op: it stamps the card and walks the balance-skip streak + * toward auto-disable. The preflight above it (user, config, permission and balance + * lookups) can outlast the 5-minute lease, so writing it under a dead claim is a write + * on behalf of a fire that no longer owns the occurrence. + */ + it('does not record a balance skip under a superseded claim', async () => { + const { methods, calls } = makeMethods(); + (methods.revalidateClaim as jest.Mock).mockResolvedValue(false); + mockFetch(async () => okResponse()); + + const result = await fireSchedule( + makeDeps(methods, { isOutOfBalance: async () => true }), + makeSchedule(), + LIMITS, + dueAt(), + ); + + expect(result.skipped).toBe('superseded'); + expect(methods.recordSkippedRun).not.toHaveBeenCalled(); + expect(calls.skipped).toEqual([]); + }); + + it('records the balance skip normally while the claim is still valid', async () => { + const { methods, calls } = makeMethods(); + mockFetch(async () => okResponse()); + + const result = await fireSchedule( + makeDeps(methods, { isOutOfBalance: async () => true }), + makeSchedule(), + LIMITS, + dueAt(), + ); + + expect(result.skipped).toBe('balance'); + expect(calls.skipped).toEqual(['skipped_balance']); + }); + /** * A `duplicate` means ANOTHER worker holds this occurrence's row — not that the * occurrence is done. Advancing past it hands the occurrence away: if that other diff --git a/packages/api/src/schedules/fire.ts b/packages/api/src/schedules/fire.ts index 23d49e4d10..ad87580285 100644 --- a/packages/api/src/schedules/fire.ts +++ b/packages/api/src/schedules/fire.ts @@ -343,6 +343,21 @@ export async function fireSchedule( } if (await deps.isOutOfBalance(user)) { + // Revalidate BEFORE writing the skip. Everything above (user, config, permission + // and balance lookups) can outlast the 5-minute lease, and a skip is not a no-op: + // it stamps the card and walks the balance-skip streak toward auto-disable. Under + // a dead claim — lease taken over, schedule deleted, account deletion begun — that + // is a write on behalf of a fire that no longer owns this occurrence. The reserve + // path below has its own revalidation for the same reason; this one guards the + // branch that returns before ever reaching it. + if ( + claimToken != null && + !(await methods.revalidateClaim(schedule.id, claimToken, !options?.manual)) + ) { + await releaseSupersededLease(); + await advance(); + return { fired: false, skipped: 'superseded' as const }; + } // Skip rows carry no `bookkept:false` marker, so the reconciler has no path to // repair a half-applied skip. If the schedule-side bookkeeping throws, do NOT // advance: leave the occurrence due so the next claim retries it (the insert is