From a353be9196b9ac5f0514cc563f272d89dead6d20 Mon Sep 17 00:00:00 2001 From: Marko Kubrachenko Date: Tue, 29 Sep 2026 16:05:11 +0200 Subject: [PATCH 1/3] fix: skip closed pull requests so they stay in Closed --- README.md | 2 +- src/main.ts | 6 +++++ src/pull_request_toolkit.ts | 8 +++++++ tests/main.test.ts | 44 ++++++++++++++++++++++++++++++++++++- 4 files changed, 58 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index b296f1a..7851075 100644 --- a/README.md +++ b/README.md @@ -15,7 +15,7 @@ This action automates a couple of processes connected with the management of Git The linkage and estimation checks are retried every 15 seconds for 2 minutes so that the user can set them up after the pull request is created without this action failing. -The action skips pull requests that come from external forks, that do not target the repository's default branch, or whose creator is not a member of any Product Engineering team. Teams listed in `SKIP_LINKING_AND_ESTIMATE_CHECKS_FOR_TEAMS` in [`src/consts.ts`](src/consts.ts) are exempt from the linking and estimate checks. +The action skips pull requests that come from external forks, that are closed, that do not target the repository's default branch, or whose creator is not a member of any Product Engineering team. Teams listed in `SKIP_LINKING_AND_ESTIMATE_CHECKS_FOR_TEAMS` in [`src/consts.ts`](src/consts.ts) are exempt from the linking and estimate checks. ## Action input diff --git a/src/main.ts b/src/main.ts index 14d5164..123a080 100644 --- a/src/main.ts +++ b/src/main.ts @@ -85,6 +85,12 @@ export async function main({ await pullRequestToolkit.closeIssuesMentionedInPullRequestBody(); } + // A closed pull request belongs in "Closed", the status set below would move it back to "Pull Request". + if (await pullRequestToolkit.isClosed()) { + core.info('Pull request is closed. Skipping toolkit action.'); + return; + } + // Skip when the pull request is not into the default branch. We don't want to run this on releases or pull request chains. if (!(await pullRequestToolkit.isToDefaultBranch())) { core.info(`Skipping toolkit action for pull request not into the default branch.`); diff --git a/src/pull_request_toolkit.ts b/src/pull_request_toolkit.ts index 001142b..6572203 100644 --- a/src/pull_request_toolkit.ts +++ b/src/pull_request_toolkit.ts @@ -81,6 +81,14 @@ export class PullRequestToolkit { return !!pullRequest.merged; } + /** + * Checks whether the pull request is closed, merged or not. + */ + public async isClosed(): Promise { + const pullRequest = await this.getPullRequest(); + return pullRequest.state === 'closed'; + } + /** * Finds the human creator of the pull request, falling back to a human assignee if it was created by a bot. */ diff --git a/tests/main.test.ts b/tests/main.test.ts index 3024e42..f2ecdb9 100644 --- a/tests/main.test.ts +++ b/tests/main.test.ts @@ -1,6 +1,8 @@ -import { describe, expect, test, vi } from 'vitest'; +import { afterEach, describe, expect, test, vi } from 'vitest'; +import { GitHubModel } from '../src/github_model.ts'; import { main } from '../src/main.ts'; +import { PullRequestToolkit } from '../src/pull_request_toolkit.ts'; import type { Context, Core, GetOctokitFunction } from '../src/types.ts'; function makeContext(pullRequest: Record) { @@ -17,6 +19,10 @@ function makePullRequest(creatorLogin: string) { } describe('main', () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + test('skips pull requests from Dependabot, which run without access to the Actions secrets', async () => { const core = { info: vi.fn(), error: vi.fn(), setFailed: vi.fn() } as unknown as Core; const getOctokit = vi.fn() as unknown as GetOctokitFunction; @@ -35,4 +41,40 @@ describe('main', () => { expect(core.setFailed).toHaveBeenCalledWith('Missing org-github-token input!'); }); + + test('does not put a closed pull request back on the team board when the check is re-run', async () => { + // Re-runs replay the original event payload, so only the freshly fetched pull request shows it is closed. + vi.spyOn(GitHubModel.prototype, 'getPullRequest').mockResolvedValue({ + state: 'closed', + merged: true, + draft: false, + user: { login: 'VojtaM39' }, + base: { ref: 'master', repo: { default_branch: 'master' } }, + } as never); + vi.spyOn(PullRequestToolkit.prototype, 'isPullRequestToolkitRequiredForRepo').mockResolvedValue(true); + vi.spyOn(PullRequestToolkit.prototype, 'linkIssuesMentionedInPullRequestBody').mockResolvedValue(); + const closeIssues = vi + .spyOn(PullRequestToolkit.prototype, 'closeIssuesMentionedInPullRequestBody') + .mockResolvedValue(); + vi.spyOn(PullRequestToolkit.prototype, 'findUsersProductEngineeringChildTeamName').mockResolvedValue( + 'Infrastructure', + ); + vi.spyOn(PullRequestToolkit.prototype, 'isTested').mockResolvedValue(false); + vi.spyOn(PullRequestToolkit.prototype, 'assignCreator').mockResolvedValue(); + vi.spyOn(PullRequestToolkit.prototype, 'getTeamLabels').mockResolvedValue(['t-infra']); + const findProject = vi.spyOn(PullRequestToolkit.prototype, 'findProjectForTeam').mockResolvedValue(null); + const core = { info: vi.fn(), error: vi.fn(), setFailed: vi.fn() } as unknown as Core; + const getOctokit = vi.fn() as unknown as GetOctokitFunction; + + await main({ + getOctokit, + context: makeContext(makePullRequest('VojtaM39')), + core, + input: { 'org-github-token': 'token', 'apify-api-token': 'token' }, + }); + + expect(closeIssues).toHaveBeenCalled(); + expect(findProject).not.toHaveBeenCalled(); + expect(core.setFailed).not.toHaveBeenCalled(); + }); }); From accaf869d5611625c11a8f1168f3e973c3c2be1c Mon Sep 17 00:00:00 2001 From: Marko Kubrachenko Date: Tue, 29 Sep 2026 16:08:17 +0200 Subject: [PATCH 2/3] test: use the file's cast idiom for the mocked pull request --- tests/main.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/main.test.ts b/tests/main.test.ts index f2ecdb9..692ee94 100644 --- a/tests/main.test.ts +++ b/tests/main.test.ts @@ -50,7 +50,7 @@ describe('main', () => { draft: false, user: { login: 'VojtaM39' }, base: { ref: 'master', repo: { default_branch: 'master' } }, - } as never); + } as unknown as Awaited>); vi.spyOn(PullRequestToolkit.prototype, 'isPullRequestToolkitRequiredForRepo').mockResolvedValue(true); vi.spyOn(PullRequestToolkit.prototype, 'linkIssuesMentionedInPullRequestBody').mockResolvedValue(); const closeIssues = vi From 3fd3c49a4eff8c8415016123f6d184d9ea66acd0 Mon Sep 17 00:00:00 2001 From: Marko Kubrachenko Date: Wed, 30 Sep 2026 14:17:21 +0200 Subject: [PATCH 3/3] docs: note that isClosed covers merged pull requests --- src/pull_request_toolkit.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/pull_request_toolkit.ts b/src/pull_request_toolkit.ts index 6572203..b8be947 100644 --- a/src/pull_request_toolkit.ts +++ b/src/pull_request_toolkit.ts @@ -82,7 +82,8 @@ export class PullRequestToolkit { } /** - * Checks whether the pull request is closed, merged or not. + * Checks whether the pull request is closed, including merged ones: + * the REST API has no merged state, a merged pull request is `state: 'closed'` with `merged: true`. */ public async isClosed(): Promise { const pullRequest = await this.getPullRequest();