Skip to content

Force COMMENT for PR review when reviewer is the PR author #15838

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 1 commit into from
Feb 15, 2026
Merged

Force COMMENT for PR review when reviewer is the PR author #15838

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
18 changes: 17 additions & 1 deletion actions/setup/js/pr_review_buffer.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,9 +203,25 @@ function createReviewBuffer() {
}

// Determine review event and body
const event = reviewMetadata ? reviewMetadata.event : "COMMENT";
let event = reviewMetadata ? reviewMetadata.event : "COMMENT";
let body = reviewMetadata ? reviewMetadata.body : "";

// Force COMMENT when the reviewer is also the PR author.
// GitHub API rejects APPROVE and REQUEST_CHANGES on your own PRs.
if (event !== "COMMENT" && pullRequest?.user?.login) {
try {
const { data: authenticatedUser } = await github.rest.users.getAuthenticated();
if (authenticatedUser?.login === pullRequest.user.login) {
core.warning(`Cannot submit ${event} review on own PR (author: ${pullRequest.user.login}). Forcing event to COMMENT.`);
event = "COMMENT";
}
} catch (authError) {
// If we can't determine the authenticated user, proceed with the original event.
// The GitHub API will reject it if it's invalid, and the error will be caught below.
core.warning(`Could not determine authenticated user: ${getErrorMessage(authError)}. Proceeding with event=${event}.`);
}
}

// Determine if we should add footer based on footer mode
let shouldAddFooter = false;
if (footerMode === "always") {
Expand Down
163 changes: 163 additions & 0 deletions actions/setup/js/pr_review_buffer.test.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ const mockGithub = {
pulls: {
createReview: vi.fn(),
},
users: {
getAuthenticated: vi.fn(),
},
},
};

Expand Down Expand Up @@ -457,6 +460,166 @@ describe("pr_review_buffer (factory pattern)", () => {
expect(callArgs.body).toContain("test-workflow");
});

it("should force COMMENT when reviewer is the PR author and event is APPROVE", async () => {
buffer.addComment({ path: "test.js", line: 1, body: "comment" });
buffer.setReviewMetadata("LGTM", "APPROVE");
buffer.setReviewContext({
repo: "owner/repo",
repoParts: { owner: "owner", repo: "repo" },
pullRequestNumber: 42,
pullRequest: { head: { sha: "abc123" }, user: { login: "bot-user" } },
});

mockGithub.rest.users.getAuthenticated.mockResolvedValue({
data: { login: "bot-user" },
});
mockGithub.rest.pulls.createReview.mockResolvedValue({
data: {
id: 700,
html_url: "https://github.com/owner/repo/pull/42#pullrequestreview-700",
},
});

const result = await buffer.submitReview();

expect(result.success).toBe(true);
expect(result.event).toBe("COMMENT");
const callArgs = mockGithub.rest.pulls.createReview.mock.calls[0][0];
expect(callArgs.event).toBe("COMMENT");
expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("Cannot submit APPROVE review on own PR"));
});

it("should force COMMENT when reviewer is the PR author and event is REQUEST_CHANGES", async () => {
buffer.addComment({ path: "test.js", line: 1, body: "comment" });
buffer.setReviewMetadata("Fix this", "REQUEST_CHANGES");
buffer.setReviewContext({
repo: "owner/repo",
repoParts: { owner: "owner", repo: "repo" },
pullRequestNumber: 42,
pullRequest: { head: { sha: "abc123" }, user: { login: "bot-user" } },
});

mockGithub.rest.users.getAuthenticated.mockResolvedValue({
data: { login: "bot-user" },
});
mockGithub.rest.pulls.createReview.mockResolvedValue({
data: {
id: 701,
html_url: "https://github.com/owner/repo/pull/42#pullrequestreview-701",
},
});

const result = await buffer.submitReview();

expect(result.success).toBe(true);
expect(result.event).toBe("COMMENT");
const callArgs = mockGithub.rest.pulls.createReview.mock.calls[0][0];
expect(callArgs.event).toBe("COMMENT");
expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("Cannot submit REQUEST_CHANGES review on own PR"));
});

it("should not force COMMENT when reviewer is different from PR author", async () => {
buffer.addComment({ path: "test.js", line: 1, body: "comment" });
buffer.setReviewMetadata("LGTM", "APPROVE");
buffer.setReviewContext({
repo: "owner/repo",
repoParts: { owner: "owner", repo: "repo" },
pullRequestNumber: 42,
pullRequest: { head: { sha: "abc123" }, user: { login: "pr-author" } },
});

mockGithub.rest.users.getAuthenticated.mockResolvedValue({
data: { login: "reviewer-bot" },
});
mockGithub.rest.pulls.createReview.mockResolvedValue({
data: {
id: 702,
html_url: "https://github.com/owner/repo/pull/42#pullrequestreview-702",
},
});

const result = await buffer.submitReview();

expect(result.success).toBe(true);
expect(result.event).toBe("APPROVE");
const callArgs = mockGithub.rest.pulls.createReview.mock.calls[0][0];
expect(callArgs.event).toBe("APPROVE");
});

it("should skip author check when event is already COMMENT", async () => {
buffer.addComment({ path: "test.js", line: 1, body: "comment" });
buffer.setReviewMetadata("Some feedback", "COMMENT");
buffer.setReviewContext({
repo: "owner/repo",
repoParts: { owner: "owner", repo: "repo" },
pullRequestNumber: 42,
pullRequest: { head: { sha: "abc123" }, user: { login: "bot-user" } },
});

mockGithub.rest.pulls.createReview.mockResolvedValue({
data: {
id: 703,
html_url: "https://github.com/owner/repo/pull/42#pullrequestreview-703",
},
});

const result = await buffer.submitReview();

expect(result.success).toBe(true);
// Should not call getAuthenticated since event is already COMMENT
expect(mockGithub.rest.users.getAuthenticated).not.toHaveBeenCalled();
});

it("should skip author check when pullRequest has no user info", async () => {
buffer.addComment({ path: "test.js", line: 1, body: "comment" });
buffer.setReviewMetadata("LGTM", "APPROVE");
buffer.setReviewContext({
repo: "owner/repo",
repoParts: { owner: "owner", repo: "repo" },
pullRequestNumber: 42,
pullRequest: { head: { sha: "abc123" } }, // No user field
});

mockGithub.rest.pulls.createReview.mockResolvedValue({
data: {
id: 705,
html_url: "https://github.com/owner/repo/pull/42#pullrequestreview-705",
},
});

const result = await buffer.submitReview();

expect(result.success).toBe(true);
expect(result.event).toBe("APPROVE");
// Should not call getAuthenticated since there's no user info to compare against
expect(mockGithub.rest.users.getAuthenticated).not.toHaveBeenCalled();
});

it("should proceed with original event when getAuthenticated fails", async () => {
buffer.addComment({ path: "test.js", line: 1, body: "comment" });
buffer.setReviewMetadata("LGTM", "APPROVE");
buffer.setReviewContext({
repo: "owner/repo",
repoParts: { owner: "owner", repo: "repo" },
pullRequestNumber: 42,
pullRequest: { head: { sha: "abc123" }, user: { login: "pr-author" } },
});

mockGithub.rest.users.getAuthenticated.mockRejectedValue(new Error("Bad credentials"));
mockGithub.rest.pulls.createReview.mockResolvedValue({
data: {
id: 704,
html_url: "https://github.com/owner/repo/pull/42#pullrequestreview-704",
},
});

const result = await buffer.submitReview();

expect(result.success).toBe(true);
expect(result.event).toBe("APPROVE");
expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("Could not determine authenticated user"));
});

it("should handle API errors gracefully", async () => {
buffer.addComment({ path: "test.js", line: 1, body: "comment" });
buffer.setReviewContext({
Expand Down
Loading