Skip to content

Treat stacked-PR update_branch API failures as non-fatal in PR Sous Chef safe outputs #53300

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.

Sign up for GitHub

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

Merged
pelikhan merged 11 commits into from
Aug 17, 2026
Merged

Treat stacked-PR update_branch API failures as non-fatal in PR Sous Chef safe outputs #53300

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
1c7a623
Initial plan
Copilot Aug 17, 2026
b259fe7
Handle stacked PR branch-update errors as non-fatal
Copilot Aug 17, 2026
96f593c
Gate stacked PR update-branch non-fatal handling on 422 status
Copilot Aug 17, 2026
cf51145
Merge branch 'main' into copilot/aw-fix-failed-jobs-pr-sous-chef
github-actions[bot] Aug 17, 2026
b494d67
Try stacked PR stack-sync fallback for update_branch failures
Copilot Aug 17, 2026
8f5ba56
Merge remote-tracking branch 'origin/main' into copilot/aw-fix-failed…
Copilot Aug 17, 2026
2ed52ee
Add explicit reason logging for update-branch return paths
Copilot Aug 17, 2026
4a098c4
Merge branch 'main' into copilot/aw-fix-failed-jobs-pr-sous-chef
github-actions[bot] Aug 17, 2026
97e7f8b
Add update-branch.stacks toggle and stacked PR debug logging
Copilot Aug 17, 2026
0384389
Address PR-finisher review feedback for stacked PR fallback
Copilot Aug 17, 2026
8861800
Fix brittle daily regression audit workflow contract test
Copilot Aug 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions actions/setup/js/update_pull_request.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@ function isNonFatalUpdateBranchError(error) {
// GitHub update-branch API also returns 403 with this message when a PR contains workflow
// file changes and the check times out, rather than the usual "refusing to allow" phrase.
const hasWorkflowsScopeRequired = message.includes("`workflows` scope may be required") || message.includes("unable to determine if workflow can be created or updated");
const hasStackedPRUnsupportedError = message.includes("updating a stacked pr's branch via this endpoint is not supported");

if (hasStackedPRUnsupportedError) {
return true;

@pelikhan pelikhan Aug 17, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot add logging before any return statement with reason

Copilot AI Aug 17, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implemented in 2ed52ee. Added explicit core.info(...) logging with reason before each return decision in update-branch classification/fallback paths (including fatal vs non-fatal classification and stacked-PR stack-sync fallback exits).

}
Comment thread
Copilot marked this conversation as resolved.
Outdated

if (status !== undefined) {
if (status === 403 && (hasWorkflowsPermissionError || hasWorkflowsScopeRequired)) {
Expand Down
22 changes: 22 additions & 0 deletions actions/setup/js/update_pull_request.test.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1009,6 +1009,28 @@ describe("update_pull_request.cjs - update_branch behavior", () => {
expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("branch from base (non-fatal)"));
});

it("should continue title/body updates when updateBranch reports stacked-PR unsupported", async () => {
const stackedPRError = new Error("Updating a stacked PR's branch via this endpoint is not supported.");
stackedPRError.status = 422;
mockGithub.rest.pulls.updateBranch.mockRejectedValueOnce(stackedPRError);

const handler = await updatePRModule.main({ update_branch: true });

@github-actions github-actions Bot Aug 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/tdd] Missing assertion: this test relies on the beforeEach mock returning an empty stacks list, causing tryStackedPRUpdateBranch to silently fall back to the warning path — but there is no assertion that request was actually called, so a regression where tryStackedPRUpdateBranch is skipped entirely would still pass.

💡 Suggested addition 
// Verify the sync path was attempted before falling back
expect(mockGithub.request).toHaveBeenCalledWith(
  "GET /repos/{owner}/{repo}/stacks",
  expect.objectContaining({ pull_request: 100 })
);

This pins the intended behaviour: when the stacked-PR error fires, the code must always attempt stack resolution before warning.

@copilot please address this.

Copilot AI Aug 17, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 0384389: added an assertion in update_pull_request.test.cjs to verify GET /repos/{owner}/{repo}/stacks is attempted in the stacked-PR unsupported path before non-fatal fallback.

const result = await handler({
pull_request_number: 100,
title: "Updated PR",
});

expect(result.success).toBe(true);
expect(mockGithub.rest.pulls.updateBranch).toHaveBeenCalledTimes(1);
expect(mockGithub.rest.pulls.update).toHaveBeenCalledWith({
owner: "testowner",
repo: "testrepo",
pull_number: 100,
title: "Updated PR",
});
expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("branch from base (non-fatal)"));
});

it("should continue title/body updates when updateBranch gets workflows-scope-required 403 (scope phrase variant)", async () => {
// Message matches only the "`workflows` scope may be required" branch of hasWorkflowsScopeRequired.
const scopeError = new Error("Validation failed; `workflows` scope may be required due to timeout in check.");
Expand Down
Loading