Problem
BountiesService.expireOverdue(), run hourly by BountyExpiryScheduler, loads every overdue OPEN, FUNDED or CLAIMED bounty and then saves them one at a time:
const overdue = await this.bountyRepo.createQueryBuilder('bounty')
.where('bounty.deadline IS NOT NULL AND bounty.deadline < :now', { now: new Date() })
.andWhere('bounty.status IN (:...statuses)', { statuses: [OPEN, FUNDED, CLAIMED] })
.getMany();
for (const bounty of overdue) {
bounty.status = BountyStatus.EXPIRED;
await this.bountyRepo.save(bounty);
}The status filter only applies at read time. Between the SELECT and each save(), and across a loop of per-row awaits that can take a while on a large backlog, a webhook can move the same bounty forward: CLAIMED → IN_REVIEW when a PR opens (#168), or on to MERGED and PAID. The save() then issues UPDATE ... SET status='expired' WHERE id=... from the stale in-memory copy and clobbers the newer status.
- The loop also skips
assertTransition(), unlike every other status change in BountiesService, so it doesn't consult the state machine (IN_REVIEW → EXPIRED isn't a valid transition).
- A bounty that has just been merged and paid can be flipped to EXPIRED. Since EXPIRED → REFUNDED is allowed, the sponsor can then also refund a bounty that was already paid out.
Suggested fix
Replace the loop with one conditional update, so the status check and the write are atomic:
const result = await this.bountyRepo.createQueryBuilder()
.update(Bounty)
.set({ status: BountyStatus.EXPIRED })
.where('deadline IS NOT NULL AND deadline < :now', { now })
.andWhere('status IN (:...statuses)', { statuses: [OPEN, FUNDED, CLAIMED] })
.execute();
return result.affected ?? 0;Or, per row: update({ id, status: In([...]) }, { status: EXPIRED }) and count affected. Add a test where the bounty's status changes between the read and the write, and it isn't expired.
Related: #152, #266, #337, #347.
Problem
BountiesService.expireOverdue(), run hourly byBountyExpiryScheduler, loads every overdue OPEN, FUNDED or CLAIMED bounty and then saves them one at a time:The status filter only applies at read time. Between the
SELECTand eachsave(), and across a loop of per-row awaits that can take a while on a large backlog, a webhook can move the same bounty forward: CLAIMED → IN_REVIEW when a PR opens (#168), or on to MERGED and PAID. Thesave()then issuesUPDATE ... SET status='expired' WHERE id=...from the stale in-memory copy and clobbers the newer status.assertTransition(), unlike every other status change inBountiesService, so it doesn't consult the state machine (IN_REVIEW → EXPIREDisn't a valid transition).Suggested fix
Replace the loop with one conditional update, so the status check and the write are atomic:
Or, per row:
update({ id, status: In([...]) }, { status: EXPIRED })and countaffected. Add a test where the bounty's status changes between the read and the write, and it isn't expired.Related: #152, #266, #337, #347.