Add dismiss-review safe output with actor-bound PR review dismissal guards

actions/setup/js/dismiss_pull_request_review.cjs

@@ -0,0 +1,163 @@
+// @ts-check
+/// <reference types="@actions/github-script" />
+
+/**
+ * @typedef {import('./types/handler-factory').HandlerFactoryFunction} HandlerFactoryFunction
+ */
+
+const { getErrorMessage } = require("./error_helpers.cjs");
+const { resolveTarget, isStagedMode, logStagedPreviewInfo, checkRequiredFilter } = require("./safe_output_helpers.cjs");
+const { resolveTargetRepoConfig, resolveAndValidateRepo } = require("./repo_helpers.cjs");
+const { createAuthenticatedGitHubClient } = require("./handler_auth.cjs");
+
+/** @type {string} Safe output type handled by this module */
+const HANDLER_TYPE = "dismiss_pull_request_review";
+
+/**
+ * Resolve the effective actor used as both the dismisser and default expected author.
+ * @returns {string}
+ */
+function getEffectiveActor() {
+  const actor = (process.env.GITHUB_ACTOR || context?.actor || "github-actions[bot]").trim();
+  return actor || "github-actions[bot]";
+}
+
+/**
+ * Main handler factory for dismiss_pull_request_review.
+ * @type {HandlerFactoryFunction}
+ */
+async function main(config = {}) {
+  const maxCount = config.max || 10;
+  const targetConfig = config.target || "triggering";
+  const isStaged = isStagedMode(config);
+  const { defaultTargetRepo, allowedRepos } = resolveTargetRepoConfig(config);
+  const githubClient = await createAuthenticatedGitHubClient(config);
+  const requiredLabels = Array.isArray(config.required_labels) ? config.required_labels : [];
+  const requiredTitlePrefix = config.required_title_prefix || "";
+  const dismisser = getEffectiveActor();
+
+  let processedCount = 0;
+
+  return async function handleDismissPullRequestReview(message) {
+    if (processedCount >= maxCount) {
+      return {
+        success: false,
+        error: `Max count of ${maxCount} reached`,
+      };
+    }
+
+    const reviewId = Number.parseInt(String(message.review_id || ""), 10);
+    if (!Number.isInteger(reviewId) || reviewId <= 0) {
+      return {
+        success: false,
+        error: "review_id must be a positive integer",
+      };
+    }
+
+    const justification = typeof message.justification === "string" ? message.justification.trim() : "";
+    if (justification.length < 20) {
+      return {
+        success: false,
+        error: "justification must be at least 20 characters",
+      };
+    }
+
+    const expectedAuthor = typeof message.author === "string" && message.author.trim().length > 0 ? message.author.trim() : dismisser;
+    if (expectedAuthor !== dismisser) {
+      return {
+        success: false,
+        error: `author must match the current workflow actor (${dismisser})`,
+      };
+    }
+
+    const targetResult = resolveTarget({
+      targetConfig,
+      item: message,
+      context,
+      itemType: "pull request review dismissal",
+      // In resolveTarget conventions, supportsPR=false means PR-only handlers.
+      supportsPR: false,
+    });
+    if (!targetResult.success) {
+      return {
+        success: false,
+        error: targetResult.error,
+      };
+    }
+    const pullRequestNumber = targetResult.number;
+
+    const repoResult = resolveAndValidateRepo(message, defaultTargetRepo, allowedRepos, "pull request review dismissal");
+    if (!repoResult.success) {
+      return {
+        success: false,
+        error: repoResult.error,
+      };
+    }
+    const { owner, repo } = repoResult.repoParts;
+
+    const filterResult = await checkRequiredFilter(githubClient, repoResult.repoParts, pullRequestNumber, requiredLabels, requiredTitlePrefix, HANDLER_TYPE);
+    if (filterResult) return filterResult;
+
+    if (isStaged) {
+      logStagedPreviewInfo(`Would dismiss review #${reviewId} on PR #${pullRequestNumber} (${owner}/${repo}) as ${dismisser}`);
+      processedCount++;
+      return {
+        success: true,
+        staged: true,
+        review_id: reviewId,
+        pull_request_number: pullRequestNumber,
+        repo: `${owner}/${repo}`,
+        author: expectedAuthor,
+      };
+    }
+
+    try {
+      const { data: review } = await githubClient.rest.pulls.getReview({
+        owner,
+        repo,
+        pull_number: pullRequestNumber,
+        review_id: reviewId,
+      });
+
+      const reviewAuthorLogin = review?.user?.login;
+      if (typeof reviewAuthorLogin !== "string" || reviewAuthorLogin.trim() === "") {
+        return {
+          success: false,
+          error: "review author is unavailable for dismissal validation",
+        };
+      }
+      const reviewAuthor = reviewAuthorLogin.trim();
+      if (reviewAuthor !== expectedAuthor) {
+        return {
+          success: false,
+          error: `review author (${reviewAuthor || "unknown"}) must match dismisser (${dismisser})`,
+        };
+      }
+
+      const { data: dismissed } = await githubClient.rest.pulls.dismissReview({
+        owner,
+        repo,
+        pull_number: pullRequestNumber,
+        review_id: reviewId,
+        message: justification,
+      });
+
+      processedCount++;
+      return {
+        success: true,
+        review_id: reviewId,
+        pull_request_number: pullRequestNumber,
+        repo: `${owner}/${repo}`,
+        author: reviewAuthor,
+        review_url: dismissed?.html_url || review?.html_url,
+      };
+    } catch (error) {
+      return {
+        success: false,
+        error: getErrorMessage(error),
+      };
+    }
+  };
+}
+
+module.exports = { main, HANDLER_TYPE };

actions/setup/js/dismiss_pull_request_review.test.cjs

@@ -0,0 +1,114 @@
+import { describe, it, expect, beforeEach, vi } from "vitest";
+
+const mockCore = {
+  debug: vi.fn(),
+  info: vi.fn(),
+  warning: vi.fn(),
+  error: vi.fn(),
+  setFailed: vi.fn(),
+  setOutput: vi.fn(),
+  summary: {
+    addRaw: vi.fn().mockReturnThis(),
+    write: vi.fn().mockResolvedValue(undefined),
+  },
+};
+
+global.core = mockCore;
+
+const mockGetReview = vi.fn();
+const mockDismissReview = vi.fn();
+
+const mockGithub = {
+  rest: {
+    pulls: {
+      getReview: mockGetReview,
+      dismissReview: mockDismissReview,
+    },
+  },
+};
+
+global.github = mockGithub;
+global.context = {
+  actor: "github-actions[bot]",
+  eventName: "pull_request",
+  repo: { owner: "test-owner", repo: "test-repo" },
+  payload: { pull_request: { number: 42 } },
+};
+
+describe("dismiss_pull_request_review", () => {
+  let handler;
+
+  beforeEach(async () => {
+    vi.resetModules();
+    vi.clearAllMocks();
+    process.env.GITHUB_ACTOR = "github-actions[bot]";
+
+    mockGetReview.mockResolvedValue({
+      data: {
+        html_url: "https://github.com/test-owner/test-repo/pull/42#pullrequestreview-123",
+        user: { login: "github-actions[bot]" },
+      },
+    });
+    mockDismissReview.mockResolvedValue({
+      data: {
+        html_url: "https://github.com/test-owner/test-repo/pull/42#pullrequestreview-123",
+      },
+    });
+
+    const { main } = require("./dismiss_pull_request_review.cjs");
+    handler = await main({ max: 10 });
+  });
+
+  it("dismisses a review when author matches current actor", async () => {
+    const result = await handler({
+      type: "dismiss_pull_request_review",
+      review_id: 123,
+      justification: "This stale review no longer reflects the updated implementation.",
+    });
+
+    expect(result.success).toBe(true);
+    expect(mockGetReview).toHaveBeenCalledWith(
+      expect.objectContaining({
+        pull_number: 42,
+        review_id: 123,
+      })
+    );
+    expect(mockDismissReview).toHaveBeenCalledWith(
+      expect.objectContaining({
+        pull_number: 42,
+        review_id: 123,
+      })
+    );
+  });
+
+  it("rejects when provided author differs from current actor", async () => {
+    const result = await handler({
+      type: "dismiss_pull_request_review",
+      review_id: 123,
+      author: "octocat",
+      justification: "This stale review no longer reflects the updated implementation.",
+    });
+
+    expect(result.success).toBe(false);
+    expect(result.error).toContain("author must match the current workflow actor");
+    expect(mockDismissReview).not.toHaveBeenCalled();
+  });
+
+  it("rejects when fetched review author differs from current actor", async () => {
+    mockGetReview.mockResolvedValueOnce({
+      data: {
+        user: { login: "octocat" },
+      },
+    });
+
+    const result = await handler({
+      type: "dismiss_pull_request_review",
+      review_id: 123,
+      justification: "This stale review no longer reflects the updated implementation.",
+    });
+
+    expect(result.success).toBe(false);
+    expect(result.error).toContain("review author");
+    expect(mockDismissReview).not.toHaveBeenCalled();
+  });
+});

actions/setup/js/safe_output_handler_manager.cjs

@@ -49,6 +49,7 @@ const HANDLER_MAP = {
   update_release: "./update_release.cjs",
   create_pull_request_review_comment: "./create_pr_review_comment.cjs",
   submit_pull_request_review: "./submit_pr_review.cjs",
+  dismiss_pull_request_review: "./dismiss_pull_request_review.cjs",
   reply_to_pull_request_review_comment: "./reply_to_pr_review_comment.cjs",
   resolve_pull_request_review_thread: "./resolve_pr_review_thread.cjs",
   create_pull_request: "./create_pull_request.cjs",
@@ -164,6 +165,7 @@ const THREAT_WARNING_ABORT_TYPES = new Set([
   "merge_pull_request",
   "mark_pull_request_as_ready_for_review",
   "resolve_pull_request_review_thread",
+  "dismiss_pull_request_review",
   "add_labels",
   "remove_labels",
   "add_reviewer",

actions/setup/js/safe_outputs_handlers.cjs

@@ -1978,6 +1978,31 @@ function createHandlers(server, appendSafeOutput, config = {}) {
     return defaultHandler("submit_pull_request_review")(args);
   };
 
+  /**
+   * Handler for dismiss_pull_request_review tool (MCP server phase).
+   * Enforces justification minimum length and actor-author consistency before recording.
+   */
+  const dismissPullRequestReviewHandler = args => {
+    const justification = (args && typeof args.justification === "string" ? args.justification : "").trim();
+    if (justification.length < 20) {
+      throw {
+        code: -32602,
+        message: `${ERR_VALIDATION}: dismiss_pull_request_review: 'justification' must be at least 20 characters`,
+      };
+    }
+
+    const actor = (process.env.GITHUB_ACTOR || context?.actor || "github-actions[bot]").trim() || "github-actions[bot]";
+    const author = args && typeof args.author === "string" ? args.author.trim() : "";
+    if (author && author !== actor) {
+      throw {
+        code: -32602,
+        message: `${ERR_VALIDATION}: dismiss_pull_request_review: 'author' must match current workflow actor (${actor})`,
+      };
+    }
+
+    return defaultHandler("dismiss_pull_request_review")(args);
+  };
+
   /**
    * Recursively copy all regular files from srcDir into destDir, preserving the relative
    * path structure under srcDir. Non-regular entries (sockets, devices, pipes, symlinks)
@@ -2187,6 +2212,7 @@ function createHandlers(server, appendSafeOutput, config = {}) {
     addCommentHandler,
     createPullRequestReviewCommentHandler,
     submitPullRequestReviewHandler,
+    dismissPullRequestReviewHandler,
     updateIssueHandler,
     updatePullRequestHandler,
   };

actions/setup/js/safe_outputs_handlers.test.cjs

@@ -87,6 +87,7 @@ describe("safe_outputs_handlers", () => {
     delete process.env.GH_AW_ASSETS_BRANCH;
     delete process.env.GH_AW_ASSETS_MAX_SIZE_KB;
     delete process.env.GH_AW_ASSETS_ALLOWED_EXTS;
+    delete process.env.GITHUB_ACTOR;
   });
 
   describe("probe intent helpers", () => {
@@ -2707,6 +2708,55 @@ describe("safe_outputs_handlers", () => {
     });
   });
 
+  describe("dismissPullRequestReviewHandler", () => {
+    it("should write entry and return success with valid justification", () => {
+      const result = handlers.dismissPullRequestReviewHandler({
+        review_id: 123,
+        justification: "This stale review no longer matches the latest patch.",
+      });
+
+      const data = JSON.parse(result.content[0].text);
+      expect(data.result).toBe("success");
+      expect(mockAppendSafeOutput).toHaveBeenCalledWith(
+        expect.objectContaining({
+          type: "dismiss_pull_request_review",
+          review_id: 123,
+        })
+      );
+    });
+
+    it("should throw MCP error when justification is shorter than 20 characters", () => {
+      expect(() =>
+        handlers.dismissPullRequestReviewHandler({
+          review_id: 123,
+          justification: "too short",
+        })
+      ).toThrow(
+        expect.objectContaining({
+          code: -32602,
+          message: expect.stringContaining("at least 20 characters"),
+        })
+      );
+    });
+
+    it("should throw MCP error when author does not match current workflow actor", () => {
+      process.env.GITHUB_ACTOR = "github-actions[bot]";
+
+      expect(() =>
+        handlers.dismissPullRequestReviewHandler({
+          review_id: 123,
+          justification: "This stale review no longer matches the latest patch.",
+          author: "octocat",
+        })
+      ).toThrow(
+        expect.objectContaining({
+          code: -32602,
+          message: expect.stringContaining("must match current workflow actor"),
+        })
+      );
+    });
+  });
+
   describe("createPullRequestReviewCommentHandler", () => {
     it("should write entry and return success", () => {
       const result = handlers.createPullRequestReviewCommentHandler({ path: "src/foo.js", line: 5, body: "Consider renaming." });