diff --git a/.changeset/yummy-radios-flow.md b/.changeset/yummy-radios-flow.md new file mode 100644 index 0000000..51eb819 --- /dev/null +++ b/.changeset/yummy-radios-flow.md @@ -0,0 +1,5 @@ +--- +"layne": patch +--- + +Checks if the PR has already been merged and doesn't run Layne diff --git a/src/__tests__/github.test.ts b/src/__tests__/github.test.ts index 5d3175b..65b6e2a 100644 --- a/src/__tests__/github.test.ts +++ b/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()', () => { diff --git a/src/__tests__/server.test.ts b/src/__tests__/server.test.ts index 623055d..bfcdcae 100644 --- a/src/__tests__/server.test.ts +++ b/src/__tests__/server.test.ts @@ -148,6 +148,7 @@ beforeEach(() => { (findPullRequestBySha as ReturnType).mockResolvedValue(null); (getLatestCheckRun as ReturnType).mockResolvedValue({ conclusion: 'failure' }); (getPullRequest as ReturnType).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).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).mockResolvedValueOnce(null); + (findPullRequestBySha as ReturnType).mockResolvedValueOnce({ + number: 42, + head: { ref: 'feature/login', sha: 'abc123' }, + base: { ref: 'main', sha: 'def456' }, + labels: [], + }); + (getPullRequest as ReturnType).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).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).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).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(); + }); }); diff --git a/src/github.ts b/src/github.ts index 4d036f4..13e3240 100644 --- a/src/github.ts +++ b/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; } /** diff --git a/src/server.ts b/src/server.ts index 1724113..4d19075 100644 --- a/src/server.ts +++ b/src/server.ts @@ -233,7 +233,12 @@ async function handleIssueComment(payload: Record): 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; } // ---------------------------------------------------------------------------