Skip to content

Commit 37b38f0

Browse files
Copilotpelikhan
andauthored
fix: retry and log updateBranch failures for update-pull-request
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/64e4d53e-e239-416e-9bf1-20a33e83b86b Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
1 parent b09c3eb commit 37b38f0

4 files changed

Lines changed: 61 additions & 6 deletions

File tree

actions/setup/js/update_pull_request.cjs

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ const { sanitizeTitle } = require("./sanitize_title.cjs");
1515
const { parseBoolTemplatable } = require("./templatable.cjs");
1616
const { buildWorkflowRunUrl } = require("./workflow_metadata_helpers.cjs");
1717
const { generateHistoryUrl } = require("./generate_history_link.cjs");
18+
const { getErrorMessage } = require("./error_helpers.cjs");
19+
const { withRetry, isTransientError } = require("./error_recovery.cjs");
1820

1921
/**
2022
* Execute the pull request update API call
@@ -37,11 +39,26 @@ async function executePRUpdate(github, context, prNumber, updateData) {
3739

3840
if (mergeBase) {
3941
core.info(`Updating pull request #${prNumber} branch with base branch changes`);
40-
await github.rest.pulls.updateBranch({
41-
owner: context.repo.owner,
42-
repo: context.repo.repo,
43-
pull_number: prNumber,
44-
});
42+
try {
43+
await withRetry(
44+
() =>
45+
github.rest.pulls.updateBranch({
46+
owner: context.repo.owner,
47+
repo: context.repo.repo,
48+
pull_number: prNumber,
49+
}),
50+
{
51+
maxRetries: 1,
52+
initialDelayMs: 0,
53+
jitterMs: 0,
54+
shouldRetry: isTransientError,
55+
},
56+
`update pull request #${prNumber} branch from base`
57+
);
58+
} catch (error) {
59+
core.warning(`Failed to update pull request #${prNumber} branch from base: ${getErrorMessage(error)}`);
60+
throw error;
61+
}
4562
}
4663

4764
// If we have a body, process it with the appropriate operation

actions/setup/js/update_pull_request.test.cjs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -823,4 +823,28 @@ describe("update_pull_request.cjs - merge_base behavior", () => {
823823
title: "Updated PR",
824824
});
825825
});
826+
827+
it("should retry updateBranch on transient failures", async () => {
828+
mockGithub.rest.pulls.updateBranch.mockRejectedValueOnce(new Error("timeout contacting github")).mockResolvedValueOnce({
829+
data: { message: "Branch updated after retry" },
830+
});
831+
832+
const handler = await updatePRModule.main({ merge_base: true });
833+
const result = await handler({ pull_request_number: 100 });
834+
835+
expect(result.success).toBe(true);
836+
expect(mockGithub.rest.pulls.updateBranch).toHaveBeenCalledTimes(2);
837+
});
838+
839+
it("should log merge-base operation failure when updateBranch fails", async () => {
840+
mockGithub.rest.pulls.updateBranch.mockRejectedValueOnce(new Error("branch update forbidden"));
841+
842+
const handler = await updatePRModule.main({ merge_base: true });
843+
const result = await handler({ pull_request_number: 100 });
844+
845+
expect(result.success).toBe(false);
846+
expect(result.error).toContain("update pull request #100 branch from base failed");
847+
expect(mockGithub.rest.pulls.updateBranch).toHaveBeenCalledTimes(1);
848+
expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("Failed to update pull request #100 branch from base"));
849+
});
826850
});

pkg/workflow/safe_outputs_permissions.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ func ComputePermissionsForSafeOutputs(safeOutputs *SafeOutputsConfig) *Permissio
147147
}
148148
if safeOutputs.UpdatePullRequests != nil && !isHandlerStaged(safeOutputs.Staged, safeOutputs.UpdatePullRequests.Staged) {
149149
safeOutputsPermissionsLog.Print("Adding permissions for update-pull-request")
150-
permissions.Merge(NewPermissionsContentsReadPRWrite())
150+
permissions.Merge(NewPermissionsContentsWritePRWrite())
151151
}
152152
if safeOutputs.ClosePullRequests != nil && !isHandlerStaged(safeOutputs.Staged, safeOutputs.ClosePullRequests.Staged) {
153153
safeOutputsPermissionsLog.Print("Adding permissions for close-pull-request")

pkg/workflow/safe_outputs_permissions_test.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,20 @@ func TestComputePermissionsForSafeOutputs(t *testing.T) {
218218
PermissionPullRequests: PermissionWrite,
219219
},
220220
},
221+
{
222+
name: "update-pull-request requires contents write for updateBranch API",
223+
safeOutputs: &SafeOutputsConfig{
224+
UpdatePullRequests: &UpdatePullRequestsConfig{
225+
UpdateEntityConfig: UpdateEntityConfig{
226+
BaseSafeOutputConfig: BaseSafeOutputConfig{Max: strPtr("1")},
227+
},
228+
},
229+
},
230+
expected: map[PermissionScope]PermissionLevel{
231+
PermissionContents: PermissionWrite,
232+
PermissionPullRequests: PermissionWrite,
233+
},
234+
},
221235
{
222236
name: "create-pull-request with fallback-as-issue (default) - includes issues permission",
223237
safeOutputs: &SafeOutputsConfig{

0 commit comments

Comments
 (0)