Skip to content

Commit b2d754f

Browse files
authored
fix: add rate-limit retry to PR creation and fallback issue paths (#31244)
1 parent dc23159 commit b2d754f

3 files changed

Lines changed: 258 additions & 32 deletions

File tree

actions/setup/js/create_pull_request.cjs

Lines changed: 42 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ const { checkFileProtection } = require("./manifest_file_helpers.cjs");
3030
const { renderTemplateFromFile, buildProtectedFileList, encodePathSegments, getPromptPath } = require("./messages_core.cjs");
3131
const { COPILOT_REVIEWER_BOT, FAQ_CREATE_PR_PERMISSIONS_URL, MAX_ASSIGNEES } = require("./constants.cjs");
3232
const { isStagedMode } = require("./safe_output_helpers.cjs");
33-
const { withRetry, isTransientError } = require("./error_recovery.cjs");
33+
const { withRetry, isTransientError, RATE_LIMIT_RETRY_CONFIG } = require("./error_recovery.cjs");
3434
const { tryEnforceArrayLimit } = require("./limit_enforcement_helpers.cjs");
3535
const { findAgent, getIssueDetails, assignAgentToIssue } = require("./assign_agent_helpers.cjs");
3636
const { globPatternToRegex } = require("./glob_pattern_helpers.cjs");
@@ -195,9 +195,11 @@ function sanitizeFallbackAssignees(assignees) {
195195
}
196196

197197
/**
198-
* Creates a fallback GitHub issue, retrying without assignees if the API rejects them.
198+
* Creates a fallback GitHub issue, retrying on rate-limit and other transient errors
199+
* (with exponential back-off) and retrying without assignees if the API rejects them.
199200
* This ensures fallback issue creation remains reliable even if an assignee username
200-
* is invalid or the repository does not have that collaborator.
201+
* is invalid, the repository does not have that collaborator, or the installation token
202+
* quota is temporarily exhausted.
201203
* @param {object} githubClient - Authenticated GitHub client
202204
* @param {{owner: string, repo: string}} repoParts - Repository owner and name
203205
* @param {string} title - Issue title
@@ -216,19 +218,29 @@ async function createFallbackIssue(githubClient, repoParts, title, body, labels,
216218
...(assignees && assignees.length > 0 && { assignees }),
217219
};
218220

219-
try {
220-
return await githubClient.rest.issues.create(payload);
221-
} catch (error) {
222-
const status = typeof error === "object" && error !== null && "status" in error ? error.status : undefined;
223-
const message = getErrorMessage(error).toLowerCase();
224-
const isAssigneeError = status === 422 && (message.includes("assignee") || message.includes("assignees") || message.includes("unprocessable"));
225-
if (isAssigneeError && assignees && assignees.length > 0) {
226-
core.warning(`Fallback issue creation failed due to assignee error, retrying without assignees: ${getErrorMessage(error)}`);
227-
const { assignees: _removed, ...payloadWithoutAssignees } = payload;
228-
return await githubClient.rest.issues.create(payloadWithoutAssignees);
229-
}
230-
throw error;
231-
}
221+
return withRetry(
222+
async () => {
223+
try {
224+
return await githubClient.rest.issues.create(payload);
225+
} catch (error) {
226+
const status = typeof error === "object" && error !== null && "status" in error ? error.status : undefined;
227+
const message = getErrorMessage(error).toLowerCase();
228+
const isAssigneeError = status === 422 && (message.includes("assignee") || message.includes("assignees") || message.includes("unprocessable"));
229+
if (isAssigneeError && payload.assignees && payload.assignees.length > 0) {
230+
const removedAssignees = payload.assignees.join(", ");
231+
core.warning(`Fallback issue creation failed due to assignee error, retrying without assignees: ${getErrorMessage(error)}`);
232+
// Mutate payload in-place so that any subsequent withRetry attempts also
233+
// omit assignees and do not re-trigger the same 422 path.
234+
delete payload.assignees;
235+
payload.body = `${payload.body}\n\n> [!NOTE]\n> Assignees (${removedAssignees}) could not be set on this issue due to an API error.`;
236+
return await githubClient.rest.issues.create(payload);
237+
}
238+
throw error;
239+
}
240+
},
241+
RATE_LIMIT_RETRY_CONFIG,
242+
`create fallback issue in ${repoParts.owner}/${repoParts.repo}`
243+
);
232244
}
233245

234246
/**
@@ -1743,15 +1755,20 @@ ${patchPreview}`;
17431755

17441756
// Try to create the pull request, with fallback to issue creation
17451757
try {
1746-
const { data: pullRequest } = await githubClient.rest.pulls.create({
1747-
owner: repoParts.owner,
1748-
repo: repoParts.repo,
1749-
title: title,
1750-
body: body,
1751-
head: branchName,
1752-
base: baseBranch,
1753-
draft: draft,
1754-
});
1758+
const { data: pullRequest } = await withRetry(
1759+
() =>
1760+
githubClient.rest.pulls.create({
1761+
owner: repoParts.owner,
1762+
repo: repoParts.repo,
1763+
title: title,
1764+
body: body,
1765+
head: branchName,
1766+
base: baseBranch,
1767+
draft: draft,
1768+
}),
1769+
RATE_LIMIT_RETRY_CONFIG,
1770+
`create pull request in ${repoParts.owner}/${repoParts.repo}`
1771+
);
17551772

17561773
core.info(`Created pull request #${pullRequest.number}: ${pullRequest.html_url}`);
17571774

actions/setup/js/create_pull_request.test.cjs

Lines changed: 203 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2114,3 +2114,206 @@ describe("create_pull_request - threat detection caution", () => {
21142114
expect((between.match(/\n/g) || []).length).toBeGreaterThanOrEqual(2);
21152115
});
21162116
});
2117+
2118+
describe("create_pull_request - rate-limit retry", () => {
2119+
let originalEnv;
2120+
let tempDir;
2121+
2122+
/**
2123+
* Creates a mock GitHub API rate-limit error object (HTTP 403 with x-ratelimit-remaining: 0)
2124+
* that matches what octokit returns when the installation token quota is exhausted.
2125+
* @param {string} [message]
2126+
* @returns {Error}
2127+
*/
2128+
function createRateLimitError(message = "API rate limit exceeded") {
2129+
return Object.assign(new Error(message), {
2130+
status: 403,
2131+
response: { headers: { "x-ratelimit-remaining": "0" }, status: 403 },
2132+
});
2133+
}
2134+
2135+
beforeEach(() => {
2136+
originalEnv = { ...process.env };
2137+
process.env.GH_AW_WORKFLOW_ID = "test-workflow";
2138+
process.env.GITHUB_REPOSITORY = "test-owner/test-repo";
2139+
process.env.GITHUB_BASE_REF = "main";
2140+
tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "create-pr-rate-limit-test-"));
2141+
2142+
global.core = {
2143+
info: vi.fn(),
2144+
warning: vi.fn(),
2145+
error: vi.fn(),
2146+
debug: vi.fn(),
2147+
setFailed: vi.fn(),
2148+
setOutput: vi.fn(),
2149+
startGroup: vi.fn(),
2150+
endGroup: vi.fn(),
2151+
summary: {
2152+
addRaw: vi.fn().mockReturnThis(),
2153+
write: vi.fn().mockResolvedValue(undefined),
2154+
},
2155+
};
2156+
2157+
global.github = {
2158+
rest: {
2159+
pulls: {
2160+
create: vi.fn().mockResolvedValue({ data: { number: 42, html_url: "https://github.com/test/pull/42" } }),
2161+
requestReviewers: vi.fn().mockResolvedValue({}),
2162+
},
2163+
repos: {
2164+
get: vi.fn().mockResolvedValue({ data: { default_branch: "main" } }),
2165+
},
2166+
issues: {
2167+
create: vi.fn().mockResolvedValue({ data: { number: 99, html_url: "https://github.com/test/issues/99" } }),
2168+
addLabels: vi.fn().mockResolvedValue({}),
2169+
},
2170+
},
2171+
graphql: vi.fn(),
2172+
};
2173+
2174+
global.context = {
2175+
eventName: "issues",
2176+
repo: { owner: "test-owner", repo: "test-repo" },
2177+
payload: {},
2178+
runId: "12345",
2179+
};
2180+
2181+
global.exec = {
2182+
exec: vi.fn().mockResolvedValue(0),
2183+
getExecOutput: vi.fn().mockImplementation(async (program, args) => {
2184+
if (program === "git" && args[0] === "rev-list") {
2185+
return { exitCode: 0, stdout: "1", stderr: "" };
2186+
}
2187+
return { exitCode: 0, stdout: "main", stderr: "" };
2188+
}),
2189+
};
2190+
2191+
delete require.cache[require.resolve("./create_pull_request.cjs")];
2192+
});
2193+
2194+
afterEach(() => {
2195+
for (const key of Object.keys(process.env)) {
2196+
if (!(key in originalEnv)) {
2197+
delete process.env[key];
2198+
}
2199+
}
2200+
Object.assign(process.env, originalEnv);
2201+
2202+
if (tempDir && fs.existsSync(tempDir)) {
2203+
fs.rmSync(tempDir, { recursive: true, force: true });
2204+
}
2205+
2206+
delete global.core;
2207+
delete global.github;
2208+
delete global.context;
2209+
delete global.exec;
2210+
vi.clearAllMocks();
2211+
});
2212+
2213+
it("should retry PR creation on rate limit error and succeed", async () => {
2214+
vi.useFakeTimers();
2215+
try {
2216+
global.github.rest.pulls.create.mockRejectedValueOnce(createRateLimitError()).mockResolvedValue({ data: { number: 42, html_url: "https://github.com/test/pull/42" } });
2217+
2218+
const { main } = require("./create_pull_request.cjs");
2219+
const handler = await main({ allow_empty: true });
2220+
2221+
const resultPromise = handler({ title: "Test PR", body: "Test body" }, {});
2222+
2223+
await vi.runAllTimersAsync();
2224+
2225+
const result = await resultPromise;
2226+
2227+
expect(result.success).toBe(true);
2228+
expect(result.pull_request_number).toBe(42);
2229+
// 1 initial (rate-limited) + 1 retry (succeeds) = 2 calls total
2230+
expect(global.github.rest.pulls.create).toHaveBeenCalledTimes(2);
2231+
expect(global.core.warning).toHaveBeenCalledWith(expect.stringContaining("create pull request"));
2232+
} finally {
2233+
vi.useRealTimers();
2234+
}
2235+
});
2236+
2237+
it("should fall back to issue when PR creation fails after all rate-limit retries", async () => {
2238+
vi.useFakeTimers();
2239+
try {
2240+
global.github.rest.pulls.create.mockRejectedValue(createRateLimitError());
2241+
global.github.rest.issues.create.mockResolvedValue({ data: { number: 99, html_url: "https://github.com/test/issues/99" } });
2242+
2243+
const { main } = require("./create_pull_request.cjs");
2244+
const handler = await main({ allow_empty: true });
2245+
2246+
const resultPromise = handler({ title: "Test PR", body: "Test body" }, {});
2247+
2248+
await vi.runAllTimersAsync();
2249+
2250+
const result = await resultPromise;
2251+
2252+
// Should fall back to issue creation after PR retries are exhausted
2253+
expect(result.success).toBe(true);
2254+
expect(result.fallback_used).toBe(true);
2255+
expect(result.issue_number).toBe(99);
2256+
// 1 initial + 5 retries = 6 total PR creation attempts (RATE_LIMIT_RETRY_CONFIG.maxRetries = 5)
2257+
expect(global.github.rest.pulls.create).toHaveBeenCalledTimes(6);
2258+
expect(global.github.rest.issues.create).toHaveBeenCalled();
2259+
} finally {
2260+
vi.useRealTimers();
2261+
}
2262+
});
2263+
2264+
it("should retry fallback issue creation on rate limit error and succeed", async () => {
2265+
vi.useFakeTimers();
2266+
try {
2267+
// PR creation fails with a non-rate-limit error to trigger fallback immediately
2268+
global.github.rest.pulls.create.mockRejectedValue(new Error("Some PR creation error"));
2269+
// Fallback issue creation first fails with rate limit, then succeeds
2270+
global.github.rest.issues.create.mockRejectedValueOnce(createRateLimitError()).mockResolvedValue({ data: { number: 99, html_url: "https://github.com/test/issues/99" } });
2271+
2272+
const { main } = require("./create_pull_request.cjs");
2273+
const handler = await main({ allow_empty: true });
2274+
2275+
const resultPromise = handler({ title: "Test PR", body: "Test body" }, {});
2276+
2277+
await vi.runAllTimersAsync();
2278+
2279+
const result = await resultPromise;
2280+
2281+
expect(result.success).toBe(true);
2282+
expect(result.fallback_used).toBe(true);
2283+
expect(result.issue_number).toBe(99);
2284+
// Fallback issue: 1 rate-limited attempt + 1 successful retry = 2 calls
2285+
expect(global.github.rest.issues.create).toHaveBeenCalledTimes(2);
2286+
expect(global.core.warning).toHaveBeenCalledWith(expect.stringContaining("create fallback issue"));
2287+
} finally {
2288+
vi.useRealTimers();
2289+
}
2290+
});
2291+
2292+
it("should append a note to the fallback issue body when assignees are removed due to 422 error", async () => {
2293+
// PR creation fails with a non-rate-limit error to trigger fallback immediately
2294+
global.github.rest.pulls.create.mockRejectedValue(new Error("Some PR creation error"));
2295+
2296+
const assigneeError = Object.assign(new Error("Validation Failed: assignees are invalid"), {
2297+
status: 422,
2298+
response: { status: 422 },
2299+
});
2300+
// First call fails with assignee 422, second succeeds
2301+
global.github.rest.issues.create.mockRejectedValueOnce(assigneeError).mockResolvedValue({ data: { number: 77, html_url: "https://github.com/test/issues/77" } });
2302+
2303+
const { main } = require("./create_pull_request.cjs");
2304+
const handler = await main({ allow_empty: true, assignees: ["user1", "user2"] });
2305+
2306+
const result = await handler({ title: "Test PR", body: "Test body" }, {});
2307+
2308+
expect(result.success).toBe(true);
2309+
expect(result.fallback_used).toBe(true);
2310+
expect(result.issue_number).toBe(77);
2311+
expect(global.github.rest.issues.create).toHaveBeenCalledTimes(2);
2312+
// Second call (without assignees) should have a note in the body
2313+
const secondCall = global.github.rest.issues.create.mock.calls[1][0];
2314+
expect(secondCall.assignees).toBeUndefined();
2315+
expect(secondCall.body).toContain("user1");
2316+
expect(secondCall.body).toContain("user2");
2317+
expect(secondCall.body).toContain("could not be set");
2318+
});
2319+
});

actions/setup/js/push_to_pull_request_branch.cjs

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ const { isStagedMode } = require("./safe_output_helpers.cjs");
88
const { pushSignedCommits } = require("./push_signed_commits.cjs");
99
const { updateActivationCommentWithCommit, updateActivationComment } = require("./update_activation_comment.cjs");
1010
const { getErrorMessage } = require("./error_helpers.cjs");
11+
const { withRetry, RATE_LIMIT_RETRY_CONFIG } = require("./error_recovery.cjs");
1112
const { normalizeBranchName } = require("./normalize_branch_name.cjs");
1213
const { pushExtraEmptyCommit } = require("./extra_empty_commit.cjs");
1314
const { detectForkPR, checkBranchPushable } = require("./pr_helpers.cjs");
@@ -471,13 +472,18 @@ async function main(config = {}) {
471472
});
472473

473474
try {
474-
const { data: issue } = await githubClient.rest.issues.create({
475-
owner: repoParts.owner,
476-
repo: repoParts.repo,
477-
title: issueTitle,
478-
body: issueBody,
479-
labels: ["agentic-workflows"],
480-
});
475+
const { data: issue } = await withRetry(
476+
() =>
477+
githubClient.rest.issues.create({
478+
owner: repoParts.owner,
479+
repo: repoParts.repo,
480+
title: issueTitle,
481+
body: issueBody,
482+
labels: ["agentic-workflows"],
483+
}),
484+
RATE_LIMIT_RETRY_CONFIG,
485+
`create manifest-protection review issue in ${repoParts.owner}/${repoParts.repo}`
486+
);
481487
core.info(`Created manifest-protection review issue #${issue.number}: ${issue.html_url}`);
482488
await updateActivationComment(github, context, core, issue.html_url, issue.number, "issue");
483489
return {

0 commit comments

Comments
 (0)