From 7a3d4227e773701f4ed04455cc096732a3c613d2 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Tue, 17 Mar 2026 03:23:01 +0000 Subject: [PATCH 1/3] jsweep: clean add_reviewer.cjs - Remove unused HANDLER_TYPE constant (dead code) - Add 3 edge case tests: undefined reviewers, copilot-only reviewer, default config Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- actions/setup/js/add_reviewer.cjs | 3 -- actions/setup/js/add_reviewer.test.cjs | 56 ++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 3 deletions(-) diff --git a/actions/setup/js/add_reviewer.cjs b/actions/setup/js/add_reviewer.cjs index 043a3f7f5d6..4ad267b79db 100644 --- a/actions/setup/js/add_reviewer.cjs +++ b/actions/setup/js/add_reviewer.cjs @@ -14,9 +14,6 @@ const { createAuthenticatedGitHubClient } = require("./handler_auth.cjs"); // GitHub Copilot reviewer bot username const COPILOT_REVIEWER_BOT = "copilot-pull-request-reviewer[bot]"; -/** @type {string} Safe output type handled by this module */ -const HANDLER_TYPE = "add_reviewer"; - /** * Main handler factory for add_reviewer * Returns a message handler function that processes individual add_reviewer messages diff --git a/actions/setup/js/add_reviewer.test.cjs b/actions/setup/js/add_reviewer.test.cjs index 06b59e8c191..f580404b2f7 100644 --- a/actions/setup/js/add_reviewer.test.cjs +++ b/actions/setup/js/add_reviewer.test.cjs @@ -286,4 +286,60 @@ describe("add_reviewer (Handler Factory Architecture)", () => { expect(result.reviewersAdded).toEqual(["user1", "copilot"]); expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("Failed to add copilot as reviewer")); }); + + it("should handle undefined reviewers as empty array", async () => { + const message = { + type: "add_reviewer", + // reviewers intentionally omitted + }; + + const result = await handler(message, {}); + + expect(result.success).toBe(true); + expect(result.reviewersAdded).toEqual([]); + expect(result.message).toContain("No valid reviewers found"); + expect(mockGithub.rest.pulls.requestReviewers).not.toHaveBeenCalled(); + }); + + it("should add only copilot when copilot is the only reviewer", async () => { + const message = { + type: "add_reviewer", + reviewers: ["copilot"], + }; + + const result = await handler(message, {}); + + expect(result.success).toBe(true); + expect(result.reviewersAdded).toEqual(["copilot"]); + // Only called once for copilot, not for other reviewers + expect(mockGithub.rest.pulls.requestReviewers).toHaveBeenCalledTimes(1); + expect(mockGithub.rest.pulls.requestReviewers).toHaveBeenCalledWith({ + owner: "testowner", + repo: "testrepo", + pull_number: 123, + reviewers: ["copilot-pull-request-reviewer[bot]"], + }); + }); + + it("should use default config values when no options provided", async () => { + const { main } = require("./add_reviewer.cjs"); + const defaultHandler = await main({}); + + const message = { + type: "add_reviewer", + reviewers: ["anyuser"], + }; + + // With no allowed list, all reviewers are allowed + const result = await defaultHandler(message, {}); + + expect(result.success).toBe(true); + expect(result.reviewersAdded).toEqual(["anyuser"]); + expect(mockGithub.rest.pulls.requestReviewers).toHaveBeenCalledWith({ + owner: "testowner", + repo: "testrepo", + pull_number: 123, + reviewers: ["anyuser"], + }); + }); }); From 078568675917b6779526d53e8f2048b0fa9cf7fb Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 17 Mar 2026 03:42:07 +0000 Subject: [PATCH 2/3] test: add 5 more edge case tests for add_reviewer (20 total) Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/add_reviewer.test.cjs | 110 +++++++++++++++++++++++++ 1 file changed, 110 insertions(+) diff --git a/actions/setup/js/add_reviewer.test.cjs b/actions/setup/js/add_reviewer.test.cjs index f580404b2f7..4f4ad6a535d 100644 --- a/actions/setup/js/add_reviewer.test.cjs +++ b/actions/setup/js/add_reviewer.test.cjs @@ -342,4 +342,114 @@ describe("add_reviewer (Handler Factory Architecture)", () => { reviewers: ["anyuser"], }); }); + + it("should return error for invalid pull_request_number", async () => { + const message = { + type: "add_reviewer", + reviewers: ["user1"], + pull_request_number: "not-a-number", + }; + + const result = await handler(message, {}); + + expect(result.success).toBe(false); + expect(result.error).toContain("Invalid pull_request_number"); + expect(mockGithub.rest.pulls.requestReviewers).not.toHaveBeenCalled(); + }); + + it("should filter out copilot when not in allowed list", async () => { + const { main } = require("./add_reviewer.cjs"); + // allowed list does NOT include "copilot" + const restrictedHandler = await main({ max: 10, allowed: ["user1", "user2"] }); + + const message = { + type: "add_reviewer", + reviewers: ["user1", "copilot"], + }; + + const result = await restrictedHandler(message, {}); + + expect(result.success).toBe(true); + expect(result.reviewersAdded).toEqual(["user1"]); + // Only called once — for user1 only; copilot was filtered out + expect(mockGithub.rest.pulls.requestReviewers).toHaveBeenCalledTimes(1); + expect(mockGithub.rest.pulls.requestReviewers).toHaveBeenCalledWith({ + owner: "testowner", + repo: "testrepo", + pull_number: 123, + reviewers: ["user1"], + }); + }); + + it("should sanitize null and falsy values in reviewers array", async () => { + const { main } = require("./add_reviewer.cjs"); + const permissiveHandler = await main({ max: 10, allowed: [] }); + + const message = { + type: "add_reviewer", + // null, undefined, false, and empty string are all falsy and should be stripped + reviewers: [null, undefined, false, "", "user1", " user2 "], + }; + + const result = await permissiveHandler(message, {}); + + expect(result.success).toBe(true); + // Only real usernames survive; whitespace is trimmed + expect(result.reviewersAdded).toEqual(["user1", "user2"]); + expect(mockGithub.rest.pulls.requestReviewers).toHaveBeenCalledWith({ + owner: "testowner", + repo: "testrepo", + pull_number: 123, + reviewers: ["user1", "user2"], + }); + }); + + it("should trim reviewer list to maxCount within a single message", async () => { + const { main } = require("./add_reviewer.cjs"); + const tightHandler = await main({ max: 2, allowed: [] }); + + const message = { + type: "add_reviewer", + reviewers: ["user1", "user2", "user3", "user4"], + }; + + const result = await tightHandler(message, {}); + + expect(result.success).toBe(true); + // Only first 2 reviewers should be added + expect(result.reviewersAdded).toEqual(["user1", "user2"]); + expect(mockGithub.rest.pulls.requestReviewers).toHaveBeenCalledWith({ + owner: "testowner", + repo: "testrepo", + pull_number: 123, + reviewers: ["user1", "user2"], + }); + }); + + it("should count staged calls toward max processedCount", async () => { + const originalEnv = process.env.GH_AW_SAFE_OUTPUTS_STAGED; + process.env.GH_AW_SAFE_OUTPUTS_STAGED = "true"; + + try { + const { main } = require("./add_reviewer.cjs"); + const stagedHandler = await main({ max: 1, allowed: ["user1"] }); + + const message = { + type: "add_reviewer", + reviewers: ["user1"], + }; + + // First call in staged mode — should succeed as a preview + const result1 = await stagedHandler(message, {}); + expect(result1.success).toBe(true); + expect(result1.staged).toBe(true); + + // Second call should be blocked by max count (processedCount was incremented in staged mode) + const result2 = await stagedHandler(message, {}); + expect(result2.success).toBe(false); + expect(result2.error).toContain("Max count"); + } finally { + process.env.GH_AW_SAFE_OUTPUTS_STAGED = originalEnv; + } + }); }); From 075844b313697ff0da7703d29482d19e28f3dbbf Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 17 Mar 2026 03:43:56 +0000 Subject: [PATCH 3/3] test: improve edge case coverage for invalid PR numbers and sanitization Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/add_reviewer.test.cjs | 29 +++++++++++++++----------- 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/actions/setup/js/add_reviewer.test.cjs b/actions/setup/js/add_reviewer.test.cjs index 4f4ad6a535d..b4a06580bd8 100644 --- a/actions/setup/js/add_reviewer.test.cjs +++ b/actions/setup/js/add_reviewer.test.cjs @@ -344,17 +344,22 @@ describe("add_reviewer (Handler Factory Architecture)", () => { }); it("should return error for invalid pull_request_number", async () => { - const message = { - type: "add_reviewer", - reviewers: ["user1"], - pull_request_number: "not-a-number", - }; + const invalidValues = ["not-a-number", null, "abc123"]; - const result = await handler(message, {}); + for (const invalidValue of invalidValues) { + vi.clearAllMocks(); + const message = { + type: "add_reviewer", + reviewers: ["user1"], + pull_request_number: invalidValue, + }; - expect(result.success).toBe(false); - expect(result.error).toContain("Invalid pull_request_number"); - expect(mockGithub.rest.pulls.requestReviewers).not.toHaveBeenCalled(); + const result = await handler(message, {}); + + expect(result.success).toBe(false); + expect(result.error).toContain("Invalid pull_request_number"); + expect(mockGithub.rest.pulls.requestReviewers).not.toHaveBeenCalled(); + } }); it("should filter out copilot when not in allowed list", async () => { @@ -387,14 +392,14 @@ describe("add_reviewer (Handler Factory Architecture)", () => { const message = { type: "add_reviewer", - // null, undefined, false, and empty string are all falsy and should be stripped - reviewers: [null, undefined, false, "", "user1", " user2 "], + // null, undefined, false, empty string, and whitespace-only strings are all stripped + reviewers: [null, undefined, false, "", " ", "user1", " user2 "], }; const result = await permissiveHandler(message, {}); expect(result.success).toBe(true); - // Only real usernames survive; whitespace is trimmed + // Only real usernames survive; whitespace is trimmed; whitespace-only strings are dropped expect(result.reviewersAdded).toEqual(["user1", "user2"]); expect(mockGithub.rest.pulls.requestReviewers).toHaveBeenCalledWith({ owner: "testowner",