fix: Fixes an issue where Layne would rerun if there's a failed scan in the PR (#10)

Author: julio-rocketchat

.changeset/spotty-breads-appear.md

@@ -0,0 +1,5 @@
+---
+"layne": patch
+---
+
+Fixes an issue that wouldn't reschedule a Layne scan if there's an existing failed scan

src/__tests__/server.test.js

@@ -273,8 +273,8 @@ describe('processWebhookRequest()', () => {
     expect(opts.jobId).toBe('org/my-repo#42@abc123');
   });
 
-  it('does not create a second check run when the job already exists', async () => {
-    scanQueue.getJob.mockResolvedValueOnce({ id: 'org/my-repo#42@abc123' });
+  it('does not create a second check run when the job is already active', async () => {
+    scanQueue.getJob.mockResolvedValueOnce({ id: 'org/my-repo#42@abc123', getState: vi.fn().mockResolvedValue('waiting') });
 
     const res = await processWebhookRequest(webhookRequest(prPayload('opened')));
 
@@ -283,6 +283,30 @@ describe('processWebhookRequest()', () => {
     expect(scanQueue.add).not.toHaveBeenCalled();
   });
 
+  it('removes the old job and re-enqueues when the existing job is already completed (re-run scenario)', async () => {
+    const mockRemove = vi.fn().mockResolvedValue(undefined);
+    scanQueue.getJob.mockResolvedValueOnce({ id: 'org/my-repo#42@abc123', getState: vi.fn().mockResolvedValue('completed'), remove: mockRemove });
+
+    const res = await processWebhookRequest(webhookRequest(prPayload('opened')));
+
+    expect(res).toEqual({ status: 200, body: 'Accepted' });
+    expect(mockRemove).toHaveBeenCalled();
+    expect(createCheckRun).toHaveBeenCalled();
+    expect(scanQueue.add).toHaveBeenCalled();
+  });
+
+  it('removes the old job and re-enqueues when the existing job is in a failed state (re-run scenario)', async () => {
+    const mockRemove = vi.fn().mockResolvedValue(undefined);
+    scanQueue.getJob.mockResolvedValueOnce({ id: 'org/my-repo#42@abc123', getState: vi.fn().mockResolvedValue('failed'), remove: mockRemove });
+
+    const res = await processWebhookRequest(webhookRequest(prPayload('opened')));
+
+    expect(res).toEqual({ status: 200, body: 'Accepted' });
+    expect(mockRemove).toHaveBeenCalled();
+    expect(createCheckRun).toHaveBeenCalled();
+    expect(scanQueue.add).toHaveBeenCalled();
+  });
+
   it('does not create a second check run while another request is accepting the same webhook', async () => {
     redis.set.mockResolvedValueOnce(null);
 
@@ -330,7 +354,7 @@ describe('processWebhookRequest()', () => {
 
     it('increments webhooksTotal with deduplicated=true when a duplicate webhook is received', async () => {
       loadScanConfig.mockResolvedValue(PR_TRIGGER_CONFIG);
-      scanQueue.getJob.mockResolvedValueOnce({ id: 'existing-job' });
+      scanQueue.getJob.mockResolvedValueOnce({ id: 'existing-job', getState: vi.fn().mockResolvedValue('waiting') });
       await processWebhookRequest(webhookRequest(prPayload('opened')));
       expect(webhooksTotal.inc).toHaveBeenCalledWith({ action: 'opened', deduplicated: 'true' });
     });
@@ -488,8 +512,8 @@ describe('workflow_run trigger — workflow_run event', () => {
     expect(scanQueue.add).not.toHaveBeenCalled();
   });
 
-  it('deduplicates: does not enqueue when the job already exists', async () => {
-    scanQueue.getJob.mockResolvedValueOnce({ id: 'org/my-repo#42@abc123' });
+  it('deduplicates: does not enqueue when the job is already active', async () => {
+    scanQueue.getJob.mockResolvedValueOnce({ id: 'org/my-repo#42@abc123', getState: vi.fn().mockResolvedValue('active') });
 
     const res = await processWebhookRequest(webhookRequest(workflowRunPayload(), { event: 'workflow_run' }));
 
@@ -498,6 +522,18 @@ describe('workflow_run trigger — workflow_run event', () => {
     expect(scanQueue.add).not.toHaveBeenCalled();
   });
 
+  it('removes the old job and re-enqueues when the existing job has already completed (re-run scenario)', async () => {
+    const mockRemove = vi.fn().mockResolvedValue(undefined);
+    scanQueue.getJob.mockResolvedValueOnce({ id: 'org/my-repo#42@abc123', getState: vi.fn().mockResolvedValue('completed'), remove: mockRemove });
+
+    const res = await processWebhookRequest(webhookRequest(workflowRunPayload(), { event: 'workflow_run' }));
+
+    expect(res).toEqual({ status: 200, body: 'Accepted' });
+    expect(mockRemove).toHaveBeenCalled();
+    expect(createCheckRun).toHaveBeenCalled();
+    expect(scanQueue.add).toHaveBeenCalled();
+  });
+
   it('returns 200 with "PR not found" when cache is cold and GitHub API returns nothing', async () => {
     redis.get.mockResolvedValue(null);
     findPullRequestBySha.mockResolvedValue(null);
@@ -703,8 +739,8 @@ describe('workflow_job trigger — workflow_job event', () => {
     expect(scanQueue.add).not.toHaveBeenCalled();
   });
 
-  it('deduplicates: does not enqueue when the job already exists', async () => {
-    scanQueue.getJob.mockResolvedValueOnce({ id: 'org/my-repo#42@abc123' });
+  it('deduplicates: does not enqueue when the job is already active', async () => {
+    scanQueue.getJob.mockResolvedValueOnce({ id: 'org/my-repo#42@abc123', getState: vi.fn().mockResolvedValue('active') });
 
     const res = await processWebhookRequest(webhookRequest(workflowJobPayload(), { event: 'workflow_job' }));
 
@@ -713,6 +749,18 @@ describe('workflow_job trigger — workflow_job event', () => {
     expect(scanQueue.add).not.toHaveBeenCalled();
   });
 
+  it('removes the old job and re-enqueues when the existing job has already completed (re-run scenario)', async () => {
+    const mockRemove = vi.fn().mockResolvedValue(undefined);
+    scanQueue.getJob.mockResolvedValueOnce({ id: 'org/my-repo#42@abc123', getState: vi.fn().mockResolvedValue('completed'), remove: mockRemove });
+
+    const res = await processWebhookRequest(webhookRequest(workflowJobPayload(), { event: 'workflow_job' }));
+
+    expect(res).toEqual({ status: 200, body: 'Accepted' });
+    expect(mockRemove).toHaveBeenCalled();
+    expect(createCheckRun).toHaveBeenCalled();
+    expect(scanQueue.add).toHaveBeenCalled();
+  });
+
   it('returns 200 with "PR not found" when cache is cold and GitHub API returns nothing', async () => {
     redis.get.mockResolvedValue(null);
     findPullRequestBySha.mockResolvedValue(null);

src/server.js

@@ -327,9 +327,23 @@ async function resolvePrData({ installation, repository, headSha }) {
 // Shared enqueue path (used by both pull_request and workflow_run triggers)
 // ---------------------------------------------------------------------------
 
+// Returns true if the job is waiting/active/delayed (a real duplicate).
+// If the job is in a terminal state (completed/failed), removes it from BullMQ
+// so the next scanQueue.add() call with the same jobId isn't silently dropped
+// by BullMQ's duplicate-key Lua check.
+async function isJobActive(jobId) {
+  const job = await scanQueue.getJob(jobId);
+  if (!job) return false;
+  const state = await job.getState();
+  if (state === 'waiting' || state === 'active' || state === 'delayed') return true;
+  await job.remove();
+  debug('server', `removed ${state} job ${jobId} to allow re-scan`);
+  return false;
+}
+
 async function enqueueScan({ pull_request, repository, installation, jobId, action }) {
-  if (await scanQueue.getJob(jobId)) {
-    debug('server', `duplicate webhook ignored: job already exists for ${jobId}`);
+  if (await isJobActive(jobId)) {
+    debug('server', `duplicate webhook ignored: job already active for ${jobId}`);
     webhooksTotal.inc({ action, deduplicated: 'true' });
     return ACCEPTED_RESPONSE;
   }
@@ -343,8 +357,8 @@ async function enqueueScan({ pull_request, repository, installation, jobId, acti
   let checkRunId = null;
 
   try {
-    if (await scanQueue.getJob(jobId)) {
-      debug('server', `duplicate webhook ignored after lock: job already exists for ${jobId}`);
+    if (await isJobActive(jobId)) {
+      debug('server', `duplicate webhook ignored after lock: job already active for ${jobId}`);
       webhooksTotal.inc({ action, deduplicated: 'true' });
       return ACCEPTED_RESPONSE;
     }