fix: revalidate the claim before recording a balance skip

The balance branch wrote its skip before any revalidation — the first one guards
the RESERVE path further down, which this branch returns before ever reaching.
Everything above it (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 the occurrence.
This commit is contained in:
Danny Avila 2026-07-27 10:17:19 -04:00
parent b52713fa17
commit 7ab5268fab
2 changed files with 53 additions and 0 deletions

View file

@ -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

View file

@ -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