feat: expand review queue instead of expiring pending reviewers

src/bot/__tests__/acceptPairingSlot.test.ts

@@ -27,8 +27,9 @@ function makeInterview(overrides: Partial<PairingSession> = {}): PairingSession
         interestedTeammates: [],
       },
     ],
-    pendingTeammates: [{ userId: 'u1', expiresAt: 9999999999, messageTimestamp: 'ts-1' }],
+    pendingTeammates: [{ userId: 'u1', messageTimestamp: 'ts-1' }],
     declinedTeammates: [],
+    nextExpandAt: 0,
     ...overrides,
   };
 }

src/bot/__tests__/getReviewInfo.test.ts

@@ -52,6 +52,7 @@ describe('getReviewInfo', () => {
         acceptedReviewers: [],
         declinedReviewers: [],
         pendingReviewers: [],
+        nextExpandAt: 0,
         hackerRankUrl: '',
         yardstickUrl: '',
       };

src/bot/__tests__/requestReview.test.ts

@@ -409,10 +409,10 @@ _Candidate Identifier: some-identifier_
         pendingReviewers: [
           {
             userId: 'user-id',
-            expiresAt: expect.any(Number),
             messageTimestamp: '100',
           },
         ],
+        nextExpandAt: expect.any(Number),
         hackerRankUrl: 'https://www.hackerrank.com/test/example123?authkey=validkey123',
         yardstickUrl:
           'https://script.google.com/a/sourceallies.com/macros/s/abc123/exec?page=hackerrank&candidate=John+Doe&zohoId=12345',

src/bot/requestPairingSession.ts

@@ -33,7 +33,7 @@
   slots: SlotState[];
 }
 
 function readStateFromBody(body: any, slotCount: number): ModalState {
   const v = body.view.state.values;
   return {
     candidateName: v['candidate-name']?.['candidate-name']?.value,
@@ -169,7 +169,7 @@
         trigger_id: shortcut.trigger_id,
         view: this.dialog(languages, 1),
       });
     } catch (err: any) {
       await chatService.sendDirectMessage(
         client,
         shortcut.user.id,
@@ -251,17 +251,14 @@
         slots,
         declinedTeammates: [],
         pendingTeammates: [],
+        nextExpandAt: determineExpirationTime(new Date()),
       };
       await pairingSessionsRepo.create(interview);
 
       const pendingTeammates: PendingPairingTeammate[] = [];
       for (const teammate of teammates) {
         const ts = await pairingRequestService.sendTeammateDM(this.app, teammate.id, interview);
-        pendingTeammates.push({
-          userId: teammate.id,
-          expiresAt: determineExpirationTime(new Date()),
-          messageTimestamp: ts,
-        });
+        pendingTeammates.push({ userId: teammate.id, messageTimestamp: ts });
       }
 
       if (pendingTeammates.length > 0) {

src/bot/requestReview.ts

@@ -8,7 +8,6 @@ import { Block, KnownBlock, PlainTextOption, View } from '@slack/types';
 import { blockUtils } from '@utils/blocks';
 import log from '@utils/log';
 import { bold, codeBlock, compose, italic, mention, ul } from '@utils/text';
-import { PendingReviewer } from '@models/ActiveReview';
 import {
   ActionId,
   CandidateType,
@@ -294,12 +293,7 @@ export const requestReview = {
         deadlineDisplay,
         candidateTypeDisplay,
       );
-      const pendingReviewer: PendingReviewer = {
-        userId: reviewer.id,
-        expiresAt: determineExpirationTime(new Date()),
-        messageTimestamp: messageTimestamp,
-      };
-      pendingReviewers.push(pendingReviewer);
+      pendingReviewers.push({ userId: reviewer.id, messageTimestamp });
     }
 
     await activeReviewRepo.create({
@@ -314,6 +308,7 @@ export const requestReview = {
       acceptedReviewers: [],
       declinedReviewers: [],
       pendingReviewers: pendingReviewers,
+      nextExpandAt: determineExpirationTime(new Date()),
       hackerRankUrl: hackerRankUrlValue,
       yardstickUrl: yardstickUrlValue,
     });

src/cron/__tests__/expirePairingRequests.test.ts

@@ -1,6 +1,6 @@
 import { pairingSessionsRepo } from '@repos/pairingSessionsRepo';
 import { pairingRequestService } from '@/services/PairingRequestService';
-import { PairingSession, PendingPairingTeammate } from '@models/PairingSession';
+import { PairingSession } from '@models/PairingSession';
 import { InterviewFormat } from '@bot/enums';
 import { App } from '@slack/bolt';
 import { expirePairingRequests } from '../expirePairingRequests';
@@ -9,7 +9,7 @@ Date.now = jest.fn();
 const nowMock = jest.mocked(Date.now);
 nowMock.mockReturnValue(1000000);
 
-function makeInterview(pendingTeammates: PendingPairingTeammate[]): PairingSession {
+function makeInterview(nextExpandAt: number): PairingSession {
   return {
     threadId: Math.random().toString(),
     requestorId: 'recruiter-1',
@@ -19,40 +19,22 @@ function makeInterview(pendingTeammates: PendingPairingTeammate[]): PairingSessi
     requestedAt: new Date(),
     teammatesNeededCount: 2,
     slots: [],
-    pendingTeammates,
+    pendingTeammates: [],
     declinedTeammates: [],
-  };
-}
-
-function makePending(dateOffsetMs: number): PendingPairingTeammate {
-  return {
-    userId: Math.random().toString(),
-    expiresAt: Date.now() + dateOffsetMs,
-    messageTimestamp: '123',
+    nextExpandAt,
   };
 }
 
 const mockError = Error('mock error');
 
 describe('expirePairingRequests', () => {
-  let declineTeammate: jest.SpyInstance;
+  let expandTeammates: jest.SpyInstance;
   let app: App;
 
-  const pending11 = makePending(+1);
-  const pending12 = makePending(-10);
-  const interview1 = makeInterview([pending11, pending12]);
-
-  const pending21 = makePending(+50);
-  const pending22 = makePending(+1);
-  const interview2 = makeInterview([pending21, pending22]);
-
-  const pending31 = makePending(-1000);
-  const pending32 = makePending(-1);
-  const pending33 = makePending(-5);
-  const interview3 = makeInterview([pending31, pending32, pending33]);
-
-  const pending41 = makePending(0);
-  const interview4 = makeInterview([pending41]);
+  const interview1 = makeInterview(Date.now() - 10); // expired
+  const interview2 = makeInterview(Date.now() + 50); // not yet
+  const interview3 = makeInterview(Date.now() - 1000); // expired
+  const interview4 = makeInterview(Date.now()); // exact match — not expired (strict >)
 
   beforeEach(async () => {
     jest.resetAllMocks();
@@ -67,20 +49,14 @@ describe('expirePairingRequests', () => {
       },
     } as App;
 
-    declineTeammate = jest
-      .spyOn(pairingRequestService, 'declineTeammate')
-      .mockResolvedValueOnce(interview1)
+    expandTeammates = jest
+      .spyOn(pairingRequestService, 'expandTeammates')
+      .mockResolvedValueOnce(undefined)
       .mockRejectedValueOnce(mockError)
-      .mockResolvedValueOnce(interview3)
-      .mockResolvedValueOnce(interview3);
+      .mockResolvedValueOnce(undefined);
 
     const allInterviews = [interview1, interview2, interview3, interview4];
     pairingSessionsRepo.listAll = jest.fn().mockResolvedValue(allInterviews);
-    pairingSessionsRepo.getByThreadIdOrUndefined = jest
-      .fn()
-      .mockImplementation((threadId: string) =>
-        Promise.resolve(allInterviews.find(i => i.threadId === threadId)),
-      );
 
     await expirePairingRequests(app);
   });
@@ -94,44 +70,18 @@ describe('expirePairingRequests', () => {
     expect(pairingSessionsRepo.listAll).toHaveBeenCalled();
   });
 
-  it('should decline only the requests that have expired', () => {
-    expect(declineTeammate).toHaveBeenCalledWith(
-      expect.anything(),
-      interview1,
-      pending12.userId,
-      expect.any(String),
-    );
-    expect(declineTeammate).toHaveBeenCalledWith(
-      expect.anything(),
-      interview3,
-      pending31.userId,
-      expect.any(String),
-    );
-    expect(declineTeammate).toHaveBeenCalledWith(
-      expect.anything(),
-      interview3,
-      pending32.userId,
-      expect.any(String),
-    );
-    expect(declineTeammate).toHaveBeenCalledWith(
-      expect.anything(),
-      interview3,
-      pending33.userId,
-      expect.any(String),
-    );
+  it('should expand only the sessions whose nextExpandAt has passed', () => {
+    expect(expandTeammates).toHaveBeenCalledWith(expect.anything(), interview1);
+    expect(expandTeammates).toHaveBeenCalledWith(expect.anything(), interview3);
+    expect(expandTeammates).not.toHaveBeenCalledWith(expect.anything(), interview2);
   });
 
-  it('should not expire requests that expire on this exact millisecond', () => {
-    expect(declineTeammate).not.toHaveBeenCalledWith(
-      expect.anything(),
-      expect.anything(),
-      pending41.userId,
-      expect.any(String),
-    );
+  it('should not expand a session whose nextExpandAt is exactly now', () => {
+    expect(expandTeammates).not.toHaveBeenCalledWith(expect.anything(), interview4);
   });
 
   it('should not stop when a single request fails', () => {
-    expect(declineTeammate).toHaveBeenCalledTimes(4);
+    expect(expandTeammates).toHaveBeenCalledTimes(2);
   });
 
   it('should notify the errors channel when there is a failure', () => {
@@ -142,22 +92,4 @@ describe('expirePairingRequests', () => {
       }),
     );
   });
-
-  it('should skip teammates that are no longer pending (concurrent response)', async () => {
-    const alreadyResponded = makePending(-500);
-    const interviewWithResponded = makeInterview([alreadyResponded]);
-    // Fresh fetch returns interview where this teammate has already been moved out of pending
-    const freshWithoutTeammate = makeInterview([]);
-    freshWithoutTeammate.threadId = interviewWithResponded.threadId;
-
-    pairingSessionsRepo.listAll = jest.fn().mockResolvedValue([interviewWithResponded]);
-    pairingSessionsRepo.getByThreadIdOrUndefined = jest
-      .fn()
-      .mockResolvedValue(freshWithoutTeammate);
-    declineTeammate.mockClear();
-
-    await expirePairingRequests(app);
-
-    expect(declineTeammate).not.toHaveBeenCalled();
-  });
 });

src/cron/__tests__/reviewProcessor.test.ts

@@ -1,5 +1,5 @@
 import { CandidateType, Deadline } from '@/bot/enums';
-import { ActiveReview, PendingReviewer } from '@/database/models/ActiveReview';
+import { ActiveReview } from '@/database/models/ActiveReview';
 import { activeReviewRepo } from '@/database/repos/activeReviewsRepo';
 import { RequestService } from '@/services';
 import { App } from '@slack/bolt';
@@ -9,53 +9,35 @@ Date.now = jest.fn();
 const nowMock = jest.mocked(Date.now);
 nowMock.mockReturnValue(1000000);
 
-function mockReview(pendingReviewers: PendingReviewer[]): ActiveReview {
+function mockReview(nextExpandAt: number): ActiveReview {
   return {
     threadId: Math.random().toString(),
     acceptedReviewers: [],
     dueBy: Deadline.MONDAY,
     candidateIdentifier: '',
     candidateType: CandidateType.FULL_TIME,
     languages: [],
-    pendingReviewers,
+    pendingReviewers: [],
     declinedReviewers: [],
     requestedAt: new Date(),
     requestorId: 'some-id',
     reviewersNeededCount: 2,
+    nextExpandAt,
     hackerRankUrl: '',
     yardstickUrl: '',
   };
 }
 
-function mockPendingReviewer(dateOffsetMs: number): PendingReviewer {
-  return {
-    userId: Math.random().toString(),
-    expiresAt: Date.now() + dateOffsetMs,
-    messageTimestamp: '123',
-  };
-}
-
 const mockError = Error('mock error');
 
 describe('Review Processor', () => {
-  let expireRequest: jest.SpyInstance;
+  let expandRequest: jest.SpyInstance;
   let app: App;
 
-  const reviewer11 = mockPendingReviewer(+1);
-  const reviewer12 = mockPendingReviewer(-10);
-  const review1 = mockReview([reviewer11, reviewer12]);
-
-  const reviewer21 = mockPendingReviewer(+50);
-  const reviewer22 = mockPendingReviewer(+1);
-  const review2 = mockReview([reviewer21, reviewer22]);
-
-  const reviewer31 = mockPendingReviewer(-1000);
-  const reviewer32 = mockPendingReviewer(-1);
-  const reviewer33 = mockPendingReviewer(-5);
-  const review3 = mockReview([reviewer31, reviewer32, reviewer33]);
-
-  const reviewer41 = mockPendingReviewer(0);
-  const review4 = mockReview([reviewer41]);
+  const review1 = mockReview(Date.now() - 10); // expired
+  const review2 = mockReview(Date.now() + 50); // not yet
+  const review3 = mockReview(Date.now() - 1000); // expired
+  const review4 = mockReview(Date.now()); // exact match — not expired (strict >)
 
   beforeEach(async () => {
     jest.resetAllMocks();
@@ -68,23 +50,14 @@ describe('Review Processor', () => {
         },
       },
     } as App;
-    expireRequest = jest
-      .spyOn(RequestService, 'expireRequest')
-      .mockImplementation()
+    expandRequest = jest
+      .spyOn(RequestService, 'expandRequest')
       .mockResolvedValueOnce(undefined)
       .mockRejectedValueOnce(mockError)
-      .mockResolvedValueOnce(undefined)
       .mockResolvedValueOnce(undefined);
 
     const activeReviews = [review1, review2, review3, review4];
     activeReviewRepo.listAll = jest.fn().mockResolvedValue(activeReviews);
-    activeReviewRepo.getReviewByThreadIdOrFail = jest
-      .fn()
-      .mockImplementation((threadId: string) => {
-        return activeReviews.find(activeReview => {
-          return activeReview.threadId === threadId;
-        });
-      });
 
     await expireRequests(app);
   });
@@ -98,23 +71,18 @@ describe('Review Processor', () => {
     expect(activeReviewRepo.listAll).toHaveBeenCalled();
   });
 
-  it('should decline only the requests that failed', () => {
-    expect(expireRequest).toHaveBeenCalledWith(expect.anything(), review1, reviewer12.userId);
-    expect(expireRequest).toHaveBeenCalledWith(expect.anything(), review3, reviewer31.userId);
-    expect(expireRequest).toHaveBeenCalledWith(expect.anything(), review3, reviewer32.userId);
-    expect(expireRequest).toHaveBeenCalledWith(expect.anything(), review3, reviewer33.userId);
+  it('should expand only the reviews whose nextExpandAt has passed', () => {
+    expect(expandRequest).toHaveBeenCalledWith(expect.anything(), review1);
+    expect(expandRequest).toHaveBeenCalledWith(expect.anything(), review3);
+    expect(expandRequest).not.toHaveBeenCalledWith(expect.anything(), review2);
   });
 
-  it('should not expire requests that expire on this exact millisecond, give the user a little be more time for being so lucky', () => {
-    expect(expireRequest).not.toHaveBeenCalledWith(
-      expect.anything(),
-      expect.anything(),
-      reviewer41.userId,
-    );
+  it('should not expand a review whose nextExpandAt is exactly now', () => {
+    expect(expandRequest).not.toHaveBeenCalledWith(expect.anything(), review4);
   });
 
   it('should not stop when a single request fails', () => {
-    expect(expireRequest).toHaveBeenCalledTimes(4);
+    expect(expandRequest).toHaveBeenCalledTimes(2);
   });
 
   it('should notify the errors channel when there is a failure', () => {