-
Notifications
You must be signed in to change notification settings - Fork 425
Add update_pull_request_branches maintenance operation with dedicated workflow job #28108
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
81ac174
e6542a5
a003433
649024c
b064a93
69eacaa
8f30fef
1507b7f
6546f14
d71669e
c74ae15
a40c8ae
d73f1e1
2911334
aca2295
78679e2
3c51429
bec130a
f592da1
9819257
c3a5a66
3f789eb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,211 @@ | ||
| // @ts-check | ||
| /// <reference types="@actions/github-script" /> | ||
|
|
||
| const { getErrorMessage } = require("./error_helpers.cjs"); | ||
| const { withRetry, isTransientError, sleep } = require("./error_recovery.cjs"); | ||
| const { fetchAndLogRateLimit } = require("./github_rate_limit_logger.cjs"); | ||
|
|
||
| const ACTIVE_SESSION_STATES = new Set(["open", "active", "in_progress", "queued"]); | ||
| const LIST_PULL_REQUESTS_PER_PAGE = 100; | ||
| const SESSION_LIST_LIMIT = 1000; | ||
| const UPDATE_DELAY_MS = 1000; | ||
|
|
||
| /** | ||
| * @param {unknown} value | ||
| * @returns {number | null} | ||
| */ | ||
| function parsePullRequestNumber(value) { | ||
| if (typeof value === "number" && Number.isInteger(value) && value > 0) return value; | ||
| if (typeof value !== "string") return null; | ||
| const trimmed = value.trim(); | ||
| if (!trimmed) return null; | ||
| const parsed = Number.parseInt(trimmed, 10); | ||
| return Number.isInteger(parsed) && parsed > 0 ? parsed : null; | ||
| } | ||
|
|
||
| /** | ||
| * @param {unknown} value | ||
| * @returns {boolean} | ||
| */ | ||
| function isActiveSessionState(value) { | ||
| return typeof value === "string" && ACTIVE_SESSION_STATES.has(value.trim().toLowerCase()); | ||
| } | ||
|
|
||
| /** | ||
| * @returns {Promise<Set<number>>} | ||
| */ | ||
| async function listPullRequestsWithActiveSessions() { | ||
| core.info("Listing agent sessions to identify PRs with active sessions"); | ||
| const { stdout } = await exec.getExecOutput("gh", ["agent-task", "list", "--limit", String(SESSION_LIST_LIMIT), "--json", "pullRequestNumber,state"], { | ||
| silent: true, | ||
| }); | ||
|
|
||
| if (!stdout.trim()) return new Set(); | ||
|
|
||
| /** @type {Array<{pullRequestNumber?: number | string, state?: string}>} */ | ||
| const sessions = JSON.parse(stdout); | ||
| const prNumbers = new Set(); | ||
|
|
||
| for (const session of sessions) { | ||
| if (!isActiveSessionState(session?.state)) continue; | ||
| const prNumber = parsePullRequestNumber(session?.pullRequestNumber); | ||
| if (prNumber !== null) prNumbers.add(prNumber); | ||
| } | ||
|
|
||
| core.info(`Found ${prNumbers.size} pull request(s) with active agent sessions`); | ||
| return prNumbers; | ||
| } | ||
|
|
||
| /** | ||
| * @param {string} owner | ||
| * @param {string} repo | ||
| * @returns {Promise<number[]>} | ||
| */ | ||
| async function listOpenPullRequests(owner, repo) { | ||
| const pulls = await github.paginate(github.rest.pulls.list, { | ||
| owner, | ||
| repo, | ||
| state: "open", | ||
| per_page: LIST_PULL_REQUESTS_PER_PAGE, | ||
| }); | ||
|
|
||
| return pulls.map(pr => pr.number).filter(number => Number.isInteger(number)); | ||
| } | ||
|
|
||
| /** | ||
| * @param {string} owner | ||
| * @param {string} repo | ||
| * @param {number[]} pullNumbers | ||
| * @returns {Promise<number[]>} | ||
| */ | ||
| async function filterMergeablePullRequests(owner, repo, pullNumbers) { | ||
| const mergeable = []; | ||
|
|
||
| for (const pullNumber of pullNumbers) { | ||
| const { data: pull } = await withRetry( | ||
| () => | ||
| github.rest.pulls.get({ | ||
| owner, | ||
| repo, | ||
| pull_number: pullNumber, | ||
| }), | ||
| { | ||
| maxRetries: 2, | ||
| initialDelayMs: 500, | ||
| maxDelayMs: 2000, | ||
| jitterMs: 0, | ||
| shouldRetry: isTransientError, | ||
| }, | ||
| `fetch pull request #${pullNumber}` | ||
| ); | ||
|
|
||
| const isMergeable = pull?.state === "open" && pull?.mergeable === true && pull?.draft !== true; | ||
| if (isMergeable) { | ||
| mergeable.push(pullNumber); | ||
| continue; | ||
| } | ||
|
|
||
| core.info(`Skipping PR #${pullNumber}: mergeable=${String(pull?.mergeable)}, state=${pull?.state || "unknown"}, draft=${String(Boolean(pull?.draft))}`); | ||
| } | ||
|
Comment on lines
+54
to
+70
|
||
|
|
||
| return mergeable; | ||
| } | ||
|
|
||
| /** | ||
| * @param {unknown} error | ||
| * @returns {boolean} | ||
| */ | ||
| function isNonFatalUpdateBranchError(error) { | ||
| if (typeof error === "object" && error !== null && "status" in error && error.status === 422) { | ||
| return true; | ||
| } | ||
|
|
||
| const message = getErrorMessage(error).toLowerCase(); | ||
| return message.includes("update branch failed") || message.includes("head branch is not behind"); | ||
| } | ||
|
|
||
| /** | ||
| * @param {string} owner | ||
| * @param {string} repo | ||
| * @param {number} pullNumber | ||
| * @returns {Promise<void>} | ||
| */ | ||
| async function updatePullRequestBranch(owner, repo, pullNumber) { | ||
| await withRetry( | ||
| () => | ||
| github.rest.pulls.updateBranch({ | ||
| owner, | ||
| repo, | ||
| pull_number: pullNumber, | ||
| }), | ||
| { | ||
| maxRetries: 2, | ||
| initialDelayMs: 1000, | ||
| maxDelayMs: 10000, | ||
| shouldRetry: isTransientError, | ||
| }, | ||
| `update branch for pull request #${pullNumber}` | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
| * Update all mergeable PR branches that do not have active agent sessions. | ||
| * @returns {Promise<void>} | ||
| */ | ||
| async function main() { | ||
| const owner = context.repo.owner; | ||
| const repo = context.repo.repo; | ||
|
|
||
| core.info(`Updating pull request branches in ${owner}/${repo}`); | ||
| await fetchAndLogRateLimit(github, "update_pull_request_branches_start"); | ||
|
|
||
| const openPullRequests = await listOpenPullRequests(owner, repo); | ||
| core.info(`Found ${openPullRequests.length} open pull request(s)`); | ||
| if (openPullRequests.length === 0) return; | ||
|
|
||
| const mergeablePullRequests = await filterMergeablePullRequests(owner, repo, openPullRequests); | ||
| core.info(`Found ${mergeablePullRequests.length} mergeable pull request(s)`); | ||
| if (mergeablePullRequests.length === 0) return; | ||
|
|
||
| const pullRequestsWithSessions = await listPullRequestsWithActiveSessions(); | ||
| const eligiblePullRequests = mergeablePullRequests.filter(number => !pullRequestsWithSessions.has(number)); | ||
| core.info(`Found ${eligiblePullRequests.length} eligible pull request(s) without active sessions`); | ||
| if (eligiblePullRequests.length === 0) return; | ||
|
|
||
| let updatedCount = 0; | ||
| let skippedCount = 0; | ||
| let failedCount = 0; | ||
|
|
||
| for (let i = 0; i < eligiblePullRequests.length; i++) { | ||
| const pullNumber = eligiblePullRequests[i]; | ||
| try { | ||
| core.info(`Updating branch for PR #${pullNumber}`); | ||
| await updatePullRequestBranch(owner, repo, pullNumber); | ||
| updatedCount++; | ||
| } catch (error) { | ||
| if (isNonFatalUpdateBranchError(error)) { | ||
| skippedCount++; | ||
| core.warning(`Skipping PR #${pullNumber}: ${getErrorMessage(error)}`); | ||
| } else { | ||
| failedCount++; | ||
| core.error(`Failed to update branch for PR #${pullNumber}: ${getErrorMessage(error)}`); | ||
| } | ||
| } | ||
|
|
||
| if (i < eligiblePullRequests.length - 1) { | ||
| await sleep(UPDATE_DELAY_MS); | ||
| } | ||
| } | ||
|
|
||
| await fetchAndLogRateLimit(github, "update_pull_request_branches_end"); | ||
| core.notice(`update_pull_request_branches completed: updated=${updatedCount}, skipped=${skippedCount}, failed=${failedCount}`); | ||
| } | ||
|
|
||
| module.exports = { | ||
| main, | ||
| parsePullRequestNumber, | ||
| isActiveSessionState, | ||
| listPullRequestsWithActiveSessions, | ||
| filterMergeablePullRequests, | ||
| isNonFatalUpdateBranchError, | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,110 @@ | ||
| // @ts-check | ||
| import { describe, it, expect, beforeEach, vi } from "vitest"; | ||
|
|
||
| vi.mock("./github_rate_limit_logger.cjs", () => ({ | ||
| fetchAndLogRateLimit: vi.fn().mockResolvedValue(undefined), | ||
| })); | ||
|
|
||
| const moduleUnderTest = await import("./update_pull_request_branches.cjs"); | ||
|
|
||
| describe("update_pull_request_branches", () => { | ||
| /** @type {any} */ | ||
| let mockCore; | ||
| /** @type {any} */ | ||
| let mockGithub; | ||
| /** @type {any} */ | ||
| let mockExec; | ||
| /** @type {any} */ | ||
| let mockContext; | ||
|
|
||
| beforeEach(() => { | ||
| vi.clearAllMocks(); | ||
|
|
||
| mockCore = { | ||
| info: vi.fn(), | ||
| warning: vi.fn(), | ||
| error: vi.fn(), | ||
| debug: vi.fn(), | ||
| notice: vi.fn(), | ||
| }; | ||
| mockGithub = { | ||
| paginate: vi.fn(), | ||
| rest: { | ||
| pulls: { | ||
| list: vi.fn(), | ||
| get: vi.fn(), | ||
| updateBranch: vi.fn(), | ||
| }, | ||
| }, | ||
| }; | ||
| mockExec = { | ||
| getExecOutput: vi.fn(), | ||
| }; | ||
| mockContext = { | ||
| repo: { | ||
| owner: "owner", | ||
| repo: "repo", | ||
| }, | ||
| }; | ||
|
|
||
| global.core = mockCore; | ||
| global.github = mockGithub; | ||
| global.exec = mockExec; | ||
| global.context = mockContext; | ||
| }); | ||
|
|
||
| it("updates only mergeable pull requests without active sessions", async () => { | ||
| mockGithub.paginate.mockResolvedValue([{ number: 1 }, { number: 2 }, { number: 3 }]); | ||
| mockGithub.rest.pulls.get.mockImplementation(async ({ pull_number }) => { | ||
| if (pull_number === 1) return { data: { state: "open", mergeable: true, draft: false } }; | ||
| if (pull_number === 2) return { data: { state: "open", mergeable: false, draft: false } }; | ||
| return { data: { state: "open", mergeable: true, draft: false } }; | ||
| }); | ||
| mockExec.getExecOutput.mockResolvedValue({ | ||
| stdout: JSON.stringify([ | ||
| { pullRequestNumber: 3, state: "open" }, | ||
| { pullRequestNumber: 10, state: "closed" }, | ||
| ]), | ||
| stderr: "", | ||
| exitCode: 0, | ||
| }); | ||
| mockGithub.rest.pulls.updateBranch.mockResolvedValue({ data: {} }); | ||
|
|
||
| await moduleUnderTest.main(); | ||
|
|
||
| expect(mockGithub.rest.pulls.updateBranch).toHaveBeenCalledTimes(1); | ||
| expect(mockGithub.rest.pulls.updateBranch).toHaveBeenCalledWith({ | ||
| owner: "owner", | ||
| repo: "repo", | ||
| pull_number: 1, | ||
| }); | ||
| }); | ||
|
|
||
| it("continues on non-fatal updateBranch failures", async () => { | ||
| mockGithub.paginate.mockResolvedValue([{ number: 7 }]); | ||
| mockGithub.rest.pulls.get.mockResolvedValue({ data: { state: "open", mergeable: true, draft: false } }); | ||
| mockExec.getExecOutput.mockResolvedValue({ | ||
| stdout: JSON.stringify([]), | ||
| stderr: "", | ||
| exitCode: 0, | ||
| }); | ||
| const err = new Error("Update branch failed"); | ||
| // @ts-ignore | ||
| err.status = 422; | ||
| mockGithub.rest.pulls.updateBranch.mockRejectedValue(err); | ||
|
|
||
| await expect(moduleUnderTest.main()).resolves.not.toThrow(); | ||
| expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("Skipping PR #7")); | ||
| }); | ||
|
|
||
| it("parses pull request numbers and active states correctly", () => { | ||
| expect(moduleUnderTest.parsePullRequestNumber(12)).toBe(12); | ||
| expect(moduleUnderTest.parsePullRequestNumber("34")).toBe(34); | ||
| expect(moduleUnderTest.parsePullRequestNumber("0")).toBeNull(); | ||
| expect(moduleUnderTest.parsePullRequestNumber("not-a-number")).toBeNull(); | ||
|
|
||
| expect(moduleUnderTest.isActiveSessionState("OPEN")).toBe(true); | ||
| expect(moduleUnderTest.isActiveSessionState("in_progress")).toBe(true); | ||
| expect(moduleUnderTest.isActiveSessionState("closed")).toBe(false); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot you want PR WITHOUT active sessions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated to explicitly filter to PRs without active sessions before branch updates (new
filterPullRequestsWithoutActiveSessionspath + test coverage). Addressed in commita003433.