From bfbaa62245d5c8d808cbecab8545c966b9986152 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Sun, 31 May 2026 05:16:06 +0000 Subject: [PATCH 1/3] jsweep: clean update_pull_request_branches.cjs - Remove redundant continue after pushing to mergeable list - Fix else-block indentation for clarity - Simplify loop to sleep before each iteration (except first) instead of after - Add 8 new tests covering: early return on empty PRs, fatal vs non-fatal error counting, isNonFatalUpdateBranchError directly, non-integer PR number filtering Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../setup/js/update_pull_request_branches.cjs | 20 +++--- .../js/update_pull_request_branches.test.cjs | 66 +++++++++++++++++++ 2 files changed, 75 insertions(+), 11 deletions(-) diff --git a/actions/setup/js/update_pull_request_branches.cjs b/actions/setup/js/update_pull_request_branches.cjs index 1a66c0f19ca..92e6cd41af0 100644 --- a/actions/setup/js/update_pull_request_branches.cjs +++ b/actions/setup/js/update_pull_request_branches.cjs @@ -59,14 +59,13 @@ async function filterMergeablePullRequests(owner, repo, pullNumbers) { const isMergeable = pull?.state === "open" && pull?.mergeable === true && pull?.draft !== true && isSameRepository; if (isMergeable) { mergeable.push(pullNumber); - continue; - } - - let skipReason = "not_mergeable"; - if (!isSameRepository) { - skipReason = headRepository ? "head_repository_mismatch" : "head_repository_missing"; + } else { + let skipReason = "not_mergeable"; + if (!isSameRepository) { + skipReason = headRepository ? "head_repository_mismatch" : "head_repository_missing"; + } + core.info(`Skipping PR #${pullNumber}: reason=${skipReason}, mergeable=${String(pull?.mergeable)}, state=${pull?.state || "unknown"}, draft=${String(Boolean(pull?.draft))}, head_repo=${headRepository || "unknown"}`); } - core.info(`Skipping PR #${pullNumber}: reason=${skipReason}, mergeable=${String(pull?.mergeable)}, state=${pull?.state || "unknown"}, draft=${String(Boolean(pull?.draft))}, head_repo=${headRepository || "unknown"}`); } return mergeable; @@ -152,6 +151,9 @@ async function main() { let failedCount = 0; for (let i = 0; i < mergeablePullRequests.length; i++) { + if (i > 0) { + await sleep(UPDATE_DELAY_MS); + } const pullNumber = mergeablePullRequests[i]; try { core.info(`Updating branch for PR #${pullNumber}`); @@ -167,10 +169,6 @@ async function main() { core.error(`Failed to update branch for PR #${pullNumber}: ${getErrorMessage(error)}`); } } - - if (i < mergeablePullRequests.length - 1) { - await sleep(UPDATE_DELAY_MS); - } } await fetchAndLogRateLimit(github, "update_pull_request_branches_end"); diff --git a/actions/setup/js/update_pull_request_branches.test.cjs b/actions/setup/js/update_pull_request_branches.test.cjs index 23a314820bf..65aea551dcc 100644 --- a/actions/setup/js/update_pull_request_branches.test.cjs +++ b/actions/setup/js/update_pull_request_branches.test.cjs @@ -140,4 +140,70 @@ describe("update_pull_request_branches", () => { expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining("reason=head_repository_missing")); expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining("head_repo=unknown")); }); + + it("returns early when there are no open pull requests", async () => { + mockGithub.paginate.mockResolvedValue([]); + + await moduleUnderTest.main(); + + expect(mockGithub.rest.pulls.get).not.toHaveBeenCalled(); + expect(mockGithub.rest.pulls.updateBranch).not.toHaveBeenCalled(); + }); + + it("returns early when no pull requests are mergeable", async () => { + mockGithub.paginate.mockResolvedValue([{ number: 1 }]); + mockGithub.rest.pulls.get.mockResolvedValue({ + data: { state: "open", mergeable: false, draft: false, head: { repo: { full_name: "owner/repo" } } }, + }); + + await moduleUnderTest.main(); + + expect(mockGithub.rest.pulls.updateBranch).not.toHaveBeenCalled(); + }); + + it("counts fatal errors separately from non-fatal errors", async () => { + mockGithub.paginate.mockResolvedValue([{ number: 10 }, { number: 11 }]); + mockGithub.rest.pulls.get.mockResolvedValue({ + data: { state: "open", mergeable: true, draft: false, head: { repo: { full_name: "owner/repo" } } }, + }); + const fatalErr = new Error("Something unexpected"); + const nonFatalErr = Object.assign(new Error("update branch failed"), { status: 422 }); + mockGithub.rest.pulls.updateBranch.mockRejectedValueOnce(fatalErr).mockRejectedValueOnce(nonFatalErr); + + await expect(moduleUnderTest.main()).resolves.not.toThrow(); + expect(mockCore.error).toHaveBeenCalledWith(expect.stringContaining("Failed to update branch for PR #10")); + expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("Skipping PR #11")); + expect(mockCore.notice).toHaveBeenCalledWith(expect.stringContaining("updated=0, skipped=1, failed=1")); + }); + + it("identifies non-fatal error by status 422", () => { + const err = Object.assign(new Error("Unprocessable"), { status: 422 }); + expect(moduleUnderTest.isNonFatalUpdateBranchError(err)).toBe(true); + }); + + it("identifies non-fatal error by message 'update branch failed'", () => { + expect(moduleUnderTest.isNonFatalUpdateBranchError(new Error("Update branch failed"))).toBe(true); + }); + + it("identifies non-fatal error by message 'head branch is not behind'", () => { + expect(moduleUnderTest.isNonFatalUpdateBranchError(new Error("Head branch is not behind base branch"))).toBe(true); + }); + + it("does not treat other errors as non-fatal", () => { + expect(moduleUnderTest.isNonFatalUpdateBranchError(new Error("Something else went wrong"))).toBe(false); + }); + + it("filters out non-integer pull request numbers", async () => { + mockGithub.paginate.mockResolvedValue([{ number: 1 }, { number: "bad" }, { number: null }, { number: 2 }]); + mockGithub.rest.pulls.get.mockResolvedValue({ + data: { state: "open", mergeable: true, draft: false, head: { repo: { full_name: "owner/repo" } } }, + }); + mockGithub.rest.pulls.updateBranch.mockResolvedValue({ data: {} }); + + await moduleUnderTest.main(); + + expect(mockGithub.rest.pulls.updateBranch).toHaveBeenCalledTimes(2); + expect(mockGithub.rest.pulls.updateBranch).toHaveBeenCalledWith(expect.objectContaining({ pull_number: 1 })); + expect(mockGithub.rest.pulls.updateBranch).toHaveBeenCalledWith(expect.objectContaining({ pull_number: 2 })); + }); }); From e2fc8de883ebae2577b761d6f42025d6ee35149d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 31 May 2026 11:35:18 +0000 Subject: [PATCH 2/3] chore: outline plan for PR feedback Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .github/agents/agentic-workflows.md | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/agents/agentic-workflows.md b/.github/agents/agentic-workflows.md index 1cd86e32d91..9a2e0130e84 100644 --- a/.github/agents/agentic-workflows.md +++ b/.github/agents/agentic-workflows.md @@ -216,7 +216,6 @@ gh aw compile --validate - Always reference the instructions file at `.github/aw/github-agentic-workflows.md` for complete documentation - Use the MCP tool `agentic-workflows` when running in GitHub Copilot Cloud - Workflows must be compiled to `.lock.yml` files before running in GitHub Actions -- **Workflow metadata**: Recommend setting an `emoji:` frontmatter value when creating workflows so they are easier to recognize at a glance - **Bash tools are enabled by default** - Don't restrict bash commands unnecessarily since workflows are sandboxed by the AWF - Follow security best practices: minimal permissions, explicit network access, no template injection - **Network configuration**: Use ecosystem identifiers (`node`, `python`, `go`, etc.) or explicit FQDNs in `network.allowed`. Bare shorthands like `npm` or `pypi` are **not** valid. See `.github/aw/network.md` for the full list of valid ecosystem identifiers and domain patterns. From 8f333b1304fb509a6d7abf538c030454136207d3 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 31 May 2026 11:37:25 +0000 Subject: [PATCH 3/3] test: use fake timers for update_pull_request_branches delays Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .../js/update_pull_request_branches.test.cjs | 27 ++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/actions/setup/js/update_pull_request_branches.test.cjs b/actions/setup/js/update_pull_request_branches.test.cjs index 65aea551dcc..3d994aab03c 100644 --- a/actions/setup/js/update_pull_request_branches.test.cjs +++ b/actions/setup/js/update_pull_request_branches.test.cjs @@ -62,7 +62,14 @@ describe("update_pull_request_branches", () => { }); mockGithub.rest.pulls.updateBranch.mockResolvedValue({ data: {} }); - await moduleUnderTest.main(); + vi.useFakeTimers(); + try { + const mainPromise = moduleUnderTest.main(); + await vi.runAllTimersAsync(); + await mainPromise; + } finally { + vi.useRealTimers(); + } expect(mockGithub.rest.pulls.updateBranch).toHaveBeenCalledTimes(2); expect(mockGithub.rest.pulls.updateBranch).toHaveBeenNthCalledWith(1, { @@ -170,7 +177,14 @@ describe("update_pull_request_branches", () => { const nonFatalErr = Object.assign(new Error("update branch failed"), { status: 422 }); mockGithub.rest.pulls.updateBranch.mockRejectedValueOnce(fatalErr).mockRejectedValueOnce(nonFatalErr); - await expect(moduleUnderTest.main()).resolves.not.toThrow(); + vi.useFakeTimers(); + try { + const mainPromise = moduleUnderTest.main(); + await vi.runAllTimersAsync(); + await expect(mainPromise).resolves.not.toThrow(); + } finally { + vi.useRealTimers(); + } expect(mockCore.error).toHaveBeenCalledWith(expect.stringContaining("Failed to update branch for PR #10")); expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("Skipping PR #11")); expect(mockCore.notice).toHaveBeenCalledWith(expect.stringContaining("updated=0, skipped=1, failed=1")); @@ -200,7 +214,14 @@ describe("update_pull_request_branches", () => { }); mockGithub.rest.pulls.updateBranch.mockResolvedValue({ data: {} }); - await moduleUnderTest.main(); + vi.useFakeTimers(); + try { + const mainPromise = moduleUnderTest.main(); + await vi.runAllTimersAsync(); + await mainPromise; + } finally { + vi.useRealTimers(); + } expect(mockGithub.rest.pulls.updateBranch).toHaveBeenCalledTimes(2); expect(mockGithub.rest.pulls.updateBranch).toHaveBeenCalledWith(expect.objectContaining({ pull_number: 1 }));