fix: prevent layne from running on merged PRs (#54)

* fix: prevent layne from running on merged prs

* Add changeset

Author: julio-rocketchat

.changeset/yummy-radios-flow.md

@@ -0,0 +1,5 @@
+---
+"layne": patch
+---
+
+Checks if the PR has already been merged and doesn't run Layne

src/__tests__/github.test.ts

@@ -210,9 +210,9 @@ describe('findPullRequestBySha()', () => {
     }));
   });
 
-  it('returns the first PR when results are found', async () => {
-    const pr = { number: 7, head: { ref: 'feat/x', sha: 'sha123' }, base: { ref: 'main', sha: 'base1' }, labels: [] };
-    mockListPRsForCommit.mockResolvedValueOnce({ data: [pr, { number: 8 }] });
+  it('returns the first open PR when results are found', async () => {
+    const pr = { number: 7, state: 'open', head: { ref: 'feat/x', sha: 'sha123' }, base: { ref: 'main', sha: 'base1' }, labels: [] };
+    mockListPRsForCommit.mockResolvedValueOnce({ data: [pr, { number: 8, state: 'closed' }] });
 
     const result = await findPullRequestBySha({ ...BASE, headSha: 'sha123' });
 
@@ -226,6 +226,23 @@ describe('findPullRequestBySha()', () => {
 
     expect(result).toBeNull();
   });
+
+  it('returns null when all associated PRs are closed or merged', async () => {
+    mockListPRsForCommit.mockResolvedValueOnce({ data: [{ number: 5, state: 'closed' }, { number: 6, state: 'merged' }] });
+
+    const result = await findPullRequestBySha({ ...BASE, headSha: 'sha123' });
+
+    expect(result).toBeNull();
+  });
+
+  it('returns the open PR when the list has mixed states', async () => {
+    const openPr = { number: 9, state: 'open', head: { ref: 'feat/y', sha: 'sha456' }, base: { ref: 'main', sha: 'base2' }, labels: [] };
+    mockListPRsForCommit.mockResolvedValueOnce({ data: [{ number: 8, state: 'closed' }, openPr] });
+
+    const result = await findPullRequestBySha({ ...BASE, headSha: 'sha456' });
+
+    expect(result).toBe(openPr);
+  });
 });
 
 describe('ensureLabelsExist()', () => {

src/__tests__/server.test.ts

@@ -148,6 +148,7 @@ beforeEach(() => {
   (findPullRequestBySha as ReturnType<typeof vi.fn>).mockResolvedValue(null);
   (getLatestCheckRun as ReturnType<typeof vi.fn>).mockResolvedValue({ conclusion: 'failure' });
   (getPullRequest as ReturnType<typeof vi.fn>).mockResolvedValue({
+    state:  'open',
     head:   { sha: 'abc123', ref: 'feature/login' },
     base:   { sha: 'def456', ref: 'main' },
     labels: [],
@@ -610,6 +611,41 @@ describe('workflow_run trigger — workflow_run event', () => {
     expect(res).toEqual({ status: 200, body: 'Accepted' });
     expect(scanQueue.add).toHaveBeenCalledOnce();
   });
+
+  it('does not enqueue a scan when cache hits but PR is already merged', async () => {
+    (getPullRequest as ReturnType<typeof vi.fn>).mockResolvedValueOnce({ state: 'closed' });
+
+    const res = await processWebhookRequest(webhookRequest(workflowRunPayload(), { event: 'workflow_run' }));
+
+    expect(res).toEqual({ status: 200, body: 'PR not found' });
+    expect(findPullRequestBySha).not.toHaveBeenCalled();
+    expect(scanQueue.add).not.toHaveBeenCalled();
+  });
+
+  it('does not enqueue when PR is merged by the time getPullRequest state check runs', async () => {
+    (redis.get as ReturnType<typeof vi.fn>).mockResolvedValueOnce(null);
+    (findPullRequestBySha as ReturnType<typeof vi.fn>).mockResolvedValueOnce({
+      number: 42,
+      head:   { ref: 'feature/login', sha: 'abc123' },
+      base:   { ref: 'main',          sha: 'def456' },
+      labels: [],
+    });
+    (getPullRequest as ReturnType<typeof vi.fn>).mockResolvedValueOnce({ state: 'closed' });
+
+    const res = await processWebhookRequest(webhookRequest(workflowRunPayload(), { event: 'workflow_run' }));
+
+    expect(res).toEqual({ status: 200, body: 'PR not found' });
+    expect(scanQueue.add).not.toHaveBeenCalled();
+  });
+
+  it('returns PR not found when getPullRequest throws during state validation', async () => {
+    (getPullRequest as ReturnType<typeof vi.fn>).mockRejectedValueOnce(new Error('GitHub API error'));
+
+    const res = await processWebhookRequest(webhookRequest(workflowRunPayload(), { event: 'workflow_run' }));
+
+    expect(res).toEqual({ status: 200, body: 'PR not found' });
+    expect(scanQueue.add).not.toHaveBeenCalled();
+  });
 });
 
 // ---------------------------------------------------------------------------
@@ -837,6 +873,16 @@ describe('workflow_job trigger — workflow_job event', () => {
     expect(res).toEqual({ status: 200, body: 'Accepted' });
     expect(scanQueue.add).toHaveBeenCalledOnce();
   });
+
+  it('does not enqueue a scan when cache hits but PR is already merged', async () => {
+    (getPullRequest as ReturnType<typeof vi.fn>).mockResolvedValueOnce({ state: 'closed' });
+
+    const res = await processWebhookRequest(webhookRequest(workflowJobPayload(), { event: 'workflow_job' }));
+
+    expect(res).toEqual({ status: 200, body: 'PR not found' });
+    expect(findPullRequestBySha).not.toHaveBeenCalled();
+    expect(scanQueue.add).not.toHaveBeenCalled();
+  });
 });
 
 // ---------------------------------------------------------------------------
@@ -1095,4 +1141,21 @@ describe('issue_comment handler', () => {
     expect(createPrComment).toHaveBeenCalled();
     expect(scanQueue.add).not.toHaveBeenCalled();
   });
+
+  it('does not store exceptions or enqueue scan when PR is already merged', async () => {
+    (getPullRequest as ReturnType<typeof vi.fn>).mockResolvedValueOnce({
+      state:  'closed',
+      head:   { sha: 'abc123', ref: 'feature/login' },
+      base:   { sha: 'def456', ref: 'main' },
+      labels: [],
+    });
+
+    const res = await processWebhookRequest(webhookRequest(
+      commentPayload(), { event: 'issue_comment' }
+    ));
+
+    expect(res).toEqual({ status: 200, body: 'PR not open' });
+    expect(storeExceptions).not.toHaveBeenCalled();
+    expect(scanQueue.add).not.toHaveBeenCalled();
+  });
 });

src/github.ts

@@ -175,7 +175,7 @@ export async function findPullRequestBySha({ installationId, owner, repo, headSh
     commit_sha: headSha,
   });
 
-  return data[0] ?? null;
+  return data.find(pr => pr.state === 'open') ?? null;
 }
 
 /**

src/server.ts

@@ -233,7 +233,12 @@ async function handleIssueComment(payload: Record<string, unknown>): Promise<{ s
     return { status: 200, body: 'PR not found' };
   }
 
-  const prData = pr as { head: { sha: string; ref: string }; base: { sha: string; ref: string }; labels?: Array<{ name: string }> };
+  const prData = pr as { state?: string; head: { sha: string; ref: string }; base: { sha: string; ref: string }; labels?: Array<{ name: string }> };
+
+  if (prData.state !== 'open') {
+    debug('server', `issue_comment on non-open PR #${issueData.number} (state=${prData.state ?? 'unknown'}), ignoring`);
+    return { status: 200, body: 'PR not open' };
+  }
   const headSha = prData.head.sha;
 
   await storeExceptions({
@@ -464,40 +469,63 @@ async function resolvePrData({ installation, repository, headSha }: {
   const cacheKey = prCacheKey(repository.full_name, headSha);
   const cached   = await redis.get(cacheKey);
 
+  let prData: PRCacheData | null = null;
+
   if (cached) {
-    // JSON.parse result typed as PRCacheData shape
-    return JSON.parse(cached) as PRCacheData;
+    prData = JSON.parse(cached) as PRCacheData;
+  } else {
+    debug('server', `PR cache miss for ${repository.full_name}@${headSha} — querying GitHub API`);
+
+    try {
+      const pr = await findPullRequestBySha({
+        installationId: (installation as { id: number }).id,
+        owner:          repository.owner.login,
+        repo:           repository.name,
+        headSha,
+      });
+
+      if (!pr) return null;
+
+      const apiPr = pr as { number: number; head: { ref: string }; base: { sha: string; ref: string }; labels?: Array<{ name: string }> };
+
+      prData = {
+        prNumber:       apiPr.number,
+        headSha,
+        headRef:        apiPr.head.ref,
+        baseSha:        apiPr.base.sha,
+        baseRef:        apiPr.base.ref,
+        labels:         apiPr.labels?.map(l => l.name) ?? [],
+        installationId: (installation as { id: number }).id,
+        cloneUrl:       repository.clone_url ?? '',
+        repoFullName:   repository.full_name,
+      };
+    } catch (err) {
+      console.error(`[server] Failed to recover PR from GitHub API: ${(err as Error).message}`);
+      return null;
+    }
   }
 
-  debug('server', `PR cache miss for ${repository.full_name}@${headSha} — querying GitHub API`);
+  if (!prData) return null;
 
+  // Confirm the PR is still open — the cache may be stale (written when the PR
+  // was open) and the workflow may have fired after merge.
   try {
-    const pr = await findPullRequestBySha({
-      installationId: (installation as { id: number }).id,
+    const livePr = await getPullRequest({
+      installationId: prData.installationId,
       owner:          repository.owner.login,
       repo:           repository.name,
-      headSha,
+      prNumber:       prData.prNumber,
     });
-
-    if (!pr) return null;
-
-    const prData = pr as { number: number; head: { ref: string }; base: { sha: string; ref: string }; labels?: Array<{ name: string }> };
-
-    return {
-      prNumber:       prData.number,
-      headSha,
-      headRef:        prData.head.ref,
-      baseSha:        prData.base.sha,
-      baseRef:        prData.base.ref,
-      labels:         prData.labels?.map(l => l.name) ?? [],
-      installationId: (installation as { id: number }).id,
-      cloneUrl:       repository.clone_url ?? '',
-      repoFullName:   repository.full_name,
-    };
+    if ((livePr as { state?: string }).state !== 'open') {
+      debug('server', `PR #${prData.prNumber} is no longer open, skipping scan`);
+      return null;
+    }
   } catch (err) {
-    console.error(`[server] Failed to recover PR from GitHub API: ${(err as Error).message}`);
+    console.error(`[server] Failed to verify PR state: ${(err as Error).message}`);
     return null;
   }
+
+  return prData;
 }
 
 // ---------------------------------------------------------------------------