Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/yummy-radios-flow.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"layne": patch
---

Checks if the PR has already been merged and doesn't run Layne
23 changes: 20 additions & 3 deletions src/__tests__/github.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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' });

Expand All @@ -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()', () => {
Expand Down
63 changes: 63 additions & 0 deletions src/__tests__/server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: [],
Expand Down Expand Up @@ -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();
});
});

// ---------------------------------------------------------------------------
Expand Down Expand Up @@ -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();
});
});

// ---------------------------------------------------------------------------
Expand Down Expand Up @@ -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();
});
});
2 changes: 1 addition & 1 deletion src/github.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand Down
76 changes: 52 additions & 24 deletions src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down Expand Up @@ -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;
}

// ---------------------------------------------------------------------------
Expand Down