diff --git a/actions/setup/js/pr_review_buffer.cjs b/actions/setup/js/pr_review_buffer.cjs index 46271e0ea4..ac92078240 100644 --- a/actions/setup/js/pr_review_buffer.cjs +++ b/actions/setup/js/pr_review_buffer.cjs @@ -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") { diff --git a/actions/setup/js/pr_review_buffer.test.cjs b/actions/setup/js/pr_review_buffer.test.cjs index 2fa3ae9fd1..eb3b3685c1 100644 --- a/actions/setup/js/pr_review_buffer.test.cjs +++ b/actions/setup/js/pr_review_buffer.test.cjs @@ -14,6 +14,9 @@ const mockGithub = { pulls: { createReview: vi.fn(), }, + users: { + getAuthenticated: vi.fn(), + }, }, }; @@ -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({