From e0c63b04aa292b1dc0e9ed2c2d36e220fc93fa33 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Apr 2026 20:26:57 +0000 Subject: [PATCH 1/6] Initial plan From 1f8dcc7d746e0bb93ee38e7a02dd92700dda7e8b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Apr 2026 20:48:12 +0000 Subject: [PATCH 2/6] feat: add supersede-older-reviews support for submit PR reviews Agent-Logs-Url: https://github.com/github/gh-aw/sessions/a045c112-653a-4cd4-ac03-99595109b555 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/pr_review_buffer.cjs | 83 +++++++++++++++++ actions/setup/js/pr_review_buffer.test.cjs | 88 +++++++++++++++++++ actions/setup/js/safe_outputs_tools.json | 2 +- actions/setup/js/submit_pr_review.cjs | 8 ++ actions/setup/js/submit_pr_review.test.cjs | 15 ++++ .../docs/reference/frontmatter-full.md | 7 ++ docs/src/content/docs/reference/glossary.md | 2 +- .../reference/safe-outputs-pull-requests.md | 21 +++++ .../reference/safe-outputs-specification.md | 2 + .../content/docs/reference/safe-outputs.md | 7 +- pkg/parser/schemas/main_workflow_schema.json | 4 + .../compiler_safe_outputs_handlers.go | 1 + pkg/workflow/js/safe_outputs_tools.json | 2 +- pkg/workflow/submit_pr_review.go | 15 +++- pkg/workflow/submit_pr_review_footer_test.go | 54 ++++++++++++ 15 files changed, 304 insertions(+), 7 deletions(-) diff --git a/actions/setup/js/pr_review_buffer.cjs b/actions/setup/js/pr_review_buffer.cjs index a848407fc2d..4c156736137 100644 --- a/actions/setup/js/pr_review_buffer.cjs +++ b/actions/setup/js/pr_review_buffer.cjs @@ -22,6 +22,9 @@ const { generateFooterWithMessages } = require("./messages_footer.cjs"); const { getErrorMessage } = require("./error_helpers.cjs"); const { isStagedMode } = require("./safe_output_helpers.cjs"); +const { matchesWorkflowId } = require("./generate_footer.cjs"); + +const SUPERSCEDE_REVIEW_MESSAGE = "Superseded by updated review from same workflow."; /** * @typedef {Object} BufferedComment @@ -71,6 +74,9 @@ function createReviewBuffer() { /** @type {boolean} Staged mode: when true, preview review without submitting (set via setStaged(), reset on buffer clear) */ let stagedMode = false; + + /** @type {boolean} When true, dismiss older same-workflow REQUEST_CHANGES reviews after posting a replacement review. */ + let supersedeOlderReviews = false; /** * Add a validated comment to the buffer. * Rejects comments targeting a different repo/PR than the first comment. @@ -172,6 +178,17 @@ function createReviewBuffer() { } } + /** + * Enable/disable superseding older same-workflow REQUEST_CHANGES reviews. + * @param {boolean} value - Whether supersede behavior is enabled + */ + function setSupersedeOlderReviews(value) { + supersedeOlderReviews = value === true; + if (supersedeOlderReviews) { + core.info("PR review supersede mode enabled"); + } + } + /** * Check if there are buffered comments to submit. * @returns {boolean} @@ -322,8 +339,71 @@ function createReviewBuffer() { requestParams.body = body; } + /** + * Dismiss older REQUEST_CHANGES reviews from the same workflow after posting a replacement review. + * This is best-effort: failures are logged as warnings and do not fail the current review submission. + * @param {number} currentReviewID + */ + async function maybeSupersedeOlderReviews(currentReviewID) { + if (!supersedeOlderReviews) { + return; + } + + const workflowID = process.env.GH_AW_WORKFLOW_ID || ""; + if (!workflowID) { + core.warning("supersede-older-reviews is enabled but GH_AW_WORKFLOW_ID is not set. Skipping stale review dismissal."); + return; + } + + /** @type {any[]} */ + const reviews = []; + let page = 1; + const perPage = 100; + while (true) { + const { data } = await github.rest.pulls.listReviews({ + owner: repoParts.owner, + repo: repoParts.repo, + pull_number: pullRequestNumber, + per_page: perPage, + page, + }); + + if (!Array.isArray(data) || data.length === 0) { + break; + } + reviews.push(...data); + if (data.length < perPage) { + break; + } + page++; + } + + const staleReviews = reviews.filter(review => { + if (!review || review.id === currentReviewID) return false; + if (review.state !== "CHANGES_REQUESTED") return false; + if (review.user?.login !== "github-actions[bot]") return false; + return matchesWorkflowId(review.body, workflowID); + }); + + for (const staleReview of staleReviews) { + try { + await github.rest.pulls.dismissReview({ + owner: repoParts.owner, + repo: repoParts.repo, + pull_number: pullRequestNumber, + review_id: staleReview.id, + message: SUPERSCEDE_REVIEW_MESSAGE, + }); + core.info(`Dismissed superseded review #${staleReview.id}`); + } catch (dismissError) { + core.warning(`Failed to dismiss stale review #${staleReview.id}: ${getErrorMessage(dismissError)}`); + } + } + } + try { const { data: review } = await github.rest.pulls.createReview(requestParams); + await maybeSupersedeOlderReviews(review.id); core.info(`Created PR review #${review.id}: ${review.html_url}`); @@ -348,6 +428,7 @@ function createReviewBuffer() { try { requestParams.event = "COMMENT"; const { data: review } = await github.rest.pulls.createReview(requestParams); + await maybeSupersedeOlderReviews(review.id); core.info(`Created PR review #${review.id}: ${review.html_url}`); return { success: true, @@ -375,6 +456,7 @@ function createReviewBuffer() { const bodyOnlyParams = { ...requestParams }; delete bodyOnlyParams.comments; const { data: review } = await github.rest.pulls.createReview(bodyOnlyParams); + await maybeSupersedeOlderReviews(review.id); core.info(`Created PR review #${review.id} (body-only fallback): ${review.html_url}`); return { success: true, @@ -423,6 +505,7 @@ function createReviewBuffer() { setFooterMode, setIncludeFooter: setFooterMode, // Backward compatibility alias setStaged, + setSupersedeOlderReviews, hasBufferedComments, hasReviewMetadata, getBufferedCount, diff --git a/actions/setup/js/pr_review_buffer.test.cjs b/actions/setup/js/pr_review_buffer.test.cjs index 35ac1877bf7..bb6bf74fdbe 100644 --- a/actions/setup/js/pr_review_buffer.test.cjs +++ b/actions/setup/js/pr_review_buffer.test.cjs @@ -13,6 +13,8 @@ const mockGithub = { rest: { pulls: { createReview: vi.fn(), + listReviews: vi.fn(), + dismissReview: vi.fn(), }, }, }; @@ -574,6 +576,92 @@ describe("pr_review_buffer (factory pattern)", () => { expect(mockGithub.rest.pulls.createReview).toHaveBeenCalledTimes(2); }); + it("should dismiss older same-workflow REQUEST_CHANGES reviews when supersede mode is enabled", async () => { + const previousWorkflowId = process.env.GH_AW_WORKFLOW_ID; + process.env.GH_AW_WORKFLOW_ID = "test-workflow"; + try { + buffer.setSupersedeOlderReviews(true); + buffer.setReviewMetadata("Updated review", "COMMENT"); + buffer.setReviewContext({ + repo: "owner/repo", + repoParts: { owner: "owner", repo: "repo" }, + pullRequestNumber: 42, + pullRequest: { head: { sha: "abc123" } }, + }); + + mockGithub.rest.pulls.createReview.mockResolvedValue({ + data: { + id: 900, + html_url: "https://github.com/owner/repo/pull/42#pullrequestreview-900", + }, + }); + mockGithub.rest.pulls.listReviews.mockResolvedValue({ + data: [ + { id: 100, state: "CHANGES_REQUESTED", user: { login: "github-actions[bot]" }, body: "\nOld blocking review" }, + { id: 101, state: "CHANGES_REQUESTED", user: { login: "some-other-bot" }, body: "" }, + { id: 102, state: "APPROVED", user: { login: "github-actions[bot]" }, body: "" }, + ], + }); + mockGithub.rest.pulls.dismissReview.mockResolvedValue({ data: {} }); + + const result = await buffer.submitReview(); + + expect(result.success).toBe(true); + expect(mockGithub.rest.pulls.listReviews).toHaveBeenCalledTimes(1); + expect(mockGithub.rest.pulls.dismissReview).toHaveBeenCalledTimes(1); + expect(mockGithub.rest.pulls.dismissReview).toHaveBeenCalledWith({ + owner: "owner", + repo: "repo", + pull_number: 42, + review_id: 100, + message: "Superseded by updated review from same workflow.", + }); + } finally { + if (previousWorkflowId === undefined) { + delete process.env.GH_AW_WORKFLOW_ID; + } else { + process.env.GH_AW_WORKFLOW_ID = previousWorkflowId; + } + } + }); + + it("should warn and continue when stale review dismissal fails", async () => { + const previousWorkflowId = process.env.GH_AW_WORKFLOW_ID; + process.env.GH_AW_WORKFLOW_ID = "test-workflow"; + try { + buffer.setSupersedeOlderReviews(true); + buffer.setReviewMetadata("Updated review", "COMMENT"); + buffer.setReviewContext({ + repo: "owner/repo", + repoParts: { owner: "owner", repo: "repo" }, + pullRequestNumber: 42, + pullRequest: { head: { sha: "abc123" } }, + }); + + mockGithub.rest.pulls.createReview.mockResolvedValue({ + data: { + id: 901, + html_url: "https://github.com/owner/repo/pull/42#pullrequestreview-901", + }, + }); + mockGithub.rest.pulls.listReviews.mockResolvedValue({ + data: [{ id: 200, state: "CHANGES_REQUESTED", user: { login: "github-actions[bot]" }, body: "" }], + }); + mockGithub.rest.pulls.dismissReview.mockRejectedValue(new Error("permission denied")); + + const result = await buffer.submitReview(); + + expect(result.success).toBe(true); + expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("Failed to dismiss stale review #200")); + } finally { + if (previousWorkflowId === undefined) { + delete process.env.GH_AW_WORKFLOW_ID; + } else { + process.env.GH_AW_WORKFLOW_ID = previousWorkflowId; + } + } + }); + it("should handle API errors gracefully", async () => { buffer.addComment({ path: "test.js", line: 1, body: "comment" }); buffer.setReviewContext({ diff --git a/actions/setup/js/safe_outputs_tools.json b/actions/setup/js/safe_outputs_tools.json index f674643bb08..d4a2f9c7ae0 100644 --- a/actions/setup/js/safe_outputs_tools.json +++ b/actions/setup/js/safe_outputs_tools.json @@ -360,7 +360,7 @@ }, { "name": "submit_pull_request_review", - "description": "Submit a pull request review with a status decision. All create_pull_request_review_comment outputs are automatically collected and included as inline comments in this review. Use APPROVE to approve the PR, REQUEST_CHANGES to request changes, or COMMENT for general feedback without a decision. If you don't call this tool, review comments are still submitted as a COMMENT review.", + "description": "Submit a pull request review with a status decision. All create_pull_request_review_comment outputs are automatically collected and included as inline comments in this review. Preferred default: use COMMENT for informative, non-blocking bot feedback. Use REQUEST_CHANGES only when merge-blocking is intentionally required. If you don't call this tool, review comments are still submitted as a COMMENT review.", "inputSchema": { "type": "object", "properties": { diff --git a/actions/setup/js/submit_pr_review.cjs b/actions/setup/js/submit_pr_review.cjs index 6769fa4f4ae..f45ce974a81 100644 --- a/actions/setup/js/submit_pr_review.cjs +++ b/actions/setup/js/submit_pr_review.cjs @@ -9,6 +9,7 @@ const { resolveTarget, isStagedMode, logStagedPreviewInfo } = require("./safe_ou const { resolveTargetRepoConfig, resolveAndValidateRepo } = require("./repo_helpers.cjs"); const { createAuthenticatedGitHubClient } = require("./handler_auth.cjs"); const { getErrorMessage } = require("./error_helpers.cjs"); +const { parseBoolTemplatable } = require("./templatable.cjs"); /** @type {string} Safe output type handled by this module */ const HANDLER_TYPE = "submit_pull_request_review"; @@ -30,6 +31,7 @@ async function main(config = {}) { const maxCount = config.max || 1; const targetConfig = config.target || "triggering"; const buffer = config._prReviewBuffer; + const supersedeOlderReviews = parseBoolTemplatable(config.supersede_older_reviews, false); const { defaultTargetRepo, allowedRepos } = resolveTargetRepoConfig(config); const githubClient = await createAuthenticatedGitHubClient(config); @@ -59,6 +61,12 @@ async function main(config = {}) { if (isStagedMode(config)) { logStagedPreviewInfo("PR review will be previewed without being submitted"); } + if (supersedeOlderReviews) { + core.warning("submit_pull_request_review: supersede-older-reviews is best-effort. Prefer allowed-events: [COMMENT] by default and use REQUEST_CHANGES only when merge-blocking is required."); + if (typeof buffer.setSupersedeOlderReviews === "function") { + buffer.setSupersedeOlderReviews(true); + } + } let processedCount = 0; diff --git a/actions/setup/js/submit_pr_review.test.cjs b/actions/setup/js/submit_pr_review.test.cjs index e39f925bef1..572a7c91378 100644 --- a/actions/setup/js/submit_pr_review.test.cjs +++ b/actions/setup/js/submit_pr_review.test.cjs @@ -94,6 +94,21 @@ describe("submit_pr_review (Handler Factory Architecture)", () => { expect(result.error).toContain("No PR review buffer available"); }); + it("should enable supersede mode on review buffer when configured", async () => { + const { main } = require("./submit_pr_review.cjs"); + const localBuffer = createReviewBuffer(); + const supersedeSpy = vi.spyOn(localBuffer, "setSupersedeOlderReviews"); + + await main({ + max: 1, + _prReviewBuffer: localBuffer, + supersede_older_reviews: true, + }); + + expect(supersedeSpy).toHaveBeenCalledWith(true); + expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("supersede-older-reviews is best-effort")); + }); + it("should set review metadata for APPROVE event", async () => { const message = { type: "submit_pull_request_review", diff --git a/docs/src/content/docs/reference/frontmatter-full.md b/docs/src/content/docs/reference/frontmatter-full.md index 37e2e549598..6931997c17e 100644 --- a/docs/src/content/docs/reference/frontmatter-full.md +++ b/docs/src/content/docs/reference/frontmatter-full.md @@ -3775,6 +3775,13 @@ safe-outputs: allowed-events: [] # Array of strings + # When true, after posting a replacement review this workflow dismisses older + # REQUEST_CHANGES reviews previously posted by the same workflow on the same pull + # request. This is best-effort and requires workflow markers in prior review + # bodies. + # (optional) + supersede-older-reviews: true + # GitHub token to use for this specific output type. Overrides global github-token # if specified. # (optional) diff --git a/docs/src/content/docs/reference/glossary.md b/docs/src/content/docs/reference/glossary.md index d6ec2dda28a..9c522b02da1 100644 --- a/docs/src/content/docs/reference/glossary.md +++ b/docs/src/content/docs/reference/glossary.md @@ -215,7 +215,7 @@ A field on `create-pull-request` and `push-to-pull-request-branch` safe outputs ### Allowed Events (`allowed-events:`) -A field on `submit-pull-request-review:` safe outputs that restricts which PR review event types the agent may submit. Accepts an array of `APPROVE`, `COMMENT`, and `REQUEST_CHANGES`. When set, the safe-outputs handler rejects any review event not in the list, providing infrastructure-level enforcement regardless of what the agent attempts to output. If omitted, all three event types are allowed. Example: `allowed-events: [COMMENT, REQUEST_CHANGES]` prevents the agent from approving PRs. See [Safe Outputs Reference](/gh-aw/reference/safe-outputs/#submit-pr-review-submit-pull-request-review). +A field on `submit-pull-request-review:` safe outputs that restricts which PR review event types the agent may submit. Accepts an array of `APPROVE`, `COMMENT`, and `REQUEST_CHANGES`. When set, the safe-outputs handler rejects any review event not in the list, providing infrastructure-level enforcement regardless of what the agent attempts to output. If omitted, all three event types are allowed. Preferred default for bot reviews: `allowed-events: [COMMENT]`. Example: `allowed-events: [COMMENT, REQUEST_CHANGES]` prevents the agent from approving PRs. See [Safe Outputs Reference](/gh-aw/reference/safe-outputs/#submit-pr-review-submit-pull-request-review). ### Allowed Files diff --git a/docs/src/content/docs/reference/safe-outputs-pull-requests.md b/docs/src/content/docs/reference/safe-outputs-pull-requests.md index af777a6be7c..ea6fe3d4e86 100644 --- a/docs/src/content/docs/reference/safe-outputs-pull-requests.md +++ b/docs/src/content/docs/reference/safe-outputs-pull-requests.md @@ -11,6 +11,7 @@ This page is the primary reference for pull-request-focused safe outputs: - [`update-pull-request`](#pull-request-updates-update-pull-request) - [`close-pull-request`](#close-pull-request-close-pull-request) - [`create-pull-request-review-comment`](#pr-review-comments-create-pull-request-review-comment) +- [`submit-pull-request-review`](#submit-pr-review-submit-pull-request-review) - [`reply-to-pull-request-review-comment`](#reply-to-pr-review-comment-reply-to-pull-request-review-comment) - [`resolve-pull-request-review-thread`](#resolve-pr-review-thread-resolve-pull-request-review-thread) - [`push-to-pull-request-branch`](#push-to-pr-branch-push-to-pull-request-branch) @@ -172,6 +173,26 @@ safe-outputs: When `target: "*"` is configured, the agent must supply `pull_request_number` in each `create_pull_request_review_comment` tool call to identify which PR to comment on — omitting it will cause the comment to fail. For cross-repository scenarios, the agent can also supply `repo` (in `owner/repo` format) to route the comment to a PR in a different repository; the value must match `target-repo` or appear in `allowed-repos`. +## Submit PR Review (`submit-pull-request-review:`) + +Submits a consolidated pull request review. Inline comments buffered by `create-pull-request-review-comment` are included automatically. + +```yaml wrap +safe-outputs: + submit-pull-request-review: + max: 1 + allowed-events: [COMMENT] # preferred default + supersede-older-reviews: true # dismiss older same-workflow REQUEST_CHANGES reviews after replacement + target: "triggering" # or "*", or explicit PR number + target-repo: "owner/repo" # cross-repository + allowed-repos: ["org/repo1"] # additional allowed repositories + footer: "always" # "always", "none", or "if-body" +``` + +Use `allowed-events` to control review decisions (`APPROVE`, `COMMENT`, `REQUEST_CHANGES`). Prefer `allowed-events: [COMMENT]` by default so bot reviews remain informative and non-blocking. + +When you intentionally allow `REQUEST_CHANGES`, set `supersede-older-reviews: true` to dismiss older blocking reviews from the same workflow after posting a replacement review. This behavior is best-effort. + ## Reply to PR Review Comment (`reply-to-pull-request-review-comment:`) Replies to existing review comments on pull requests. Use this to respond to reviewer feedback, answer questions, or acknowledge comments. The `comment_id` must be the numeric ID of an existing review comment. diff --git a/docs/src/content/docs/reference/safe-outputs-specification.md b/docs/src/content/docs/reference/safe-outputs-specification.md index 1622e140bce..08365151a99 100644 --- a/docs/src/content/docs/reference/safe-outputs-specification.md +++ b/docs/src/content/docs/reference/safe-outputs-specification.md @@ -1565,6 +1565,8 @@ submit-pull-request-review: target: "triggering" | "*" | # Required when not in pull_request trigger target-repo: owner/repo # Cross-repository target allowed-repos: [...] # Additional allowed repositories + allowed-events: [COMMENT] # Preferred default for non-blocking bot reviews + supersede-older-reviews: true # Best-effort dismissal of older same-workflow REQUEST_CHANGES reviews footer: "always" | "none" | "if-body" # Footer on review body ``` diff --git a/docs/src/content/docs/reference/safe-outputs.md b/docs/src/content/docs/reference/safe-outputs.md index 71d14b20326..179a3446915 100644 --- a/docs/src/content/docs/reference/safe-outputs.md +++ b/docs/src/content/docs/reference/safe-outputs.md @@ -668,12 +668,17 @@ safe-outputs: target: "triggering" # or "*", or e.g. ${{ github.event.inputs.pr_number }} when not in pull_request trigger target-repo: "owner/repo" # cross-repository: submit review on PR in another repo allowed-repos: ["org/repo1", "org/repo2"] # additional allowed repositories - allowed-events: [COMMENT, REQUEST_CHANGES] # restrict allowed review event types (default: all allowed) + allowed-events: [COMMENT] # preferred default: informative reviews without merge-blocking + supersede-older-reviews: true # dismiss older same-workflow REQUEST_CHANGES reviews after posting a replacement review footer: false # omit AI-generated footer from review body (default: true) ``` Use `allowed-events` to restrict which review event types the agent can submit. This provides infrastructure-level enforcement — for example, `allowed-events: [COMMENT, REQUEST_CHANGES]` prevents the agent from submitting APPROVE reviews regardless of what the agent attempts to output. If omitted, all event types (APPROVE, COMMENT, REQUEST_CHANGES) are allowed. +**Recommendation:** prefer `allowed-events: [COMMENT]` as the default for automated review workflows. This keeps AI feedback visible without creating a persistent merge-blocking state. + +Set `supersede-older-reviews: true` only when your workflow intentionally uses `REQUEST_CHANGES` and you want newer runs to dismiss older blocking reviews from the same workflow. Superseding is best-effort and happens after the replacement review is posted. + ### Resolve PR Review Thread (`resolve-pull-request-review-thread:`) Resolves review threads on pull requests. diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index 61f1e0e8d8a..58fcea152f0 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -6052,6 +6052,10 @@ "description": "Optional list of allowed review event types. If omitted, all event types (APPROVE, COMMENT, REQUEST_CHANGES) are allowed. Use this to restrict the agent to specific event types, e.g. [COMMENT, REQUEST_CHANGES] to prevent approvals.", "minItems": 1 }, + "supersede-older-reviews": { + "type": "boolean", + "description": "When true, after posting a replacement review this workflow dismisses older REQUEST_CHANGES reviews previously posted by the same workflow on the same pull request. This is best-effort and requires workflow markers in prior review bodies." + }, "github-token": { "$ref": "#/$defs/github_token", "description": "GitHub token to use for this specific output type. Overrides global github-token if specified." diff --git a/pkg/workflow/compiler_safe_outputs_handlers.go b/pkg/workflow/compiler_safe_outputs_handlers.go index ea53107c906..323799e839c 100644 --- a/pkg/workflow/compiler_safe_outputs_handlers.go +++ b/pkg/workflow/compiler_safe_outputs_handlers.go @@ -337,6 +337,7 @@ var handlerRegistry = map[string]handlerBuilder{ AddIfNotEmpty("target-repo", c.TargetRepoSlug). AddStringSlice("allowed_repos", c.AllowedRepos). AddStringSlice("allowed_events", c.AllowedEvents). + AddIfTrue("supersede_older_reviews", c.SupersedeOlderReviews). AddIfNotEmpty("github-token", c.GitHubToken). AddStringPtr("footer", getEffectiveFooterString(c.Footer, cfg.Footer)). AddIfTrue("staged", c.Staged). diff --git a/pkg/workflow/js/safe_outputs_tools.json b/pkg/workflow/js/safe_outputs_tools.json index cd6f246f6e8..633ab635113 100644 --- a/pkg/workflow/js/safe_outputs_tools.json +++ b/pkg/workflow/js/safe_outputs_tools.json @@ -413,7 +413,7 @@ }, { "name": "submit_pull_request_review", - "description": "Submit a pull request review with a status decision. All create_pull_request_review_comment outputs are automatically collected and included as inline comments in this review. Use APPROVE to approve the PR, REQUEST_CHANGES to request changes, or COMMENT for general feedback without a decision. If you don't call this tool, review comments are still submitted as a COMMENT review.", + "description": "Submit a pull request review with a status decision. All create_pull_request_review_comment outputs are automatically collected and included as inline comments in this review. Preferred default: use COMMENT for informative, non-blocking bot feedback. Use REQUEST_CHANGES only when merge-blocking is intentionally required. If you don't call this tool, review comments are still submitted as a COMMENT review.", "inputSchema": { "type": "object", "properties": { diff --git a/pkg/workflow/submit_pr_review.go b/pkg/workflow/submit_pr_review.go index fc75fc40749..09de47fe843 100644 --- a/pkg/workflow/submit_pr_review.go +++ b/pkg/workflow/submit_pr_review.go @@ -15,8 +15,9 @@ var submitPRReviewLog = logger.New("workflow:submit_pr_review") type SubmitPullRequestReviewConfig struct { BaseSafeOutputConfig `yaml:",inline"` SafeOutputTargetConfig `yaml:",inline"` - Footer *string `yaml:"footer,omitempty"` // Controls when to show footer in PR review body: "always" (default), "none", or "if-body" (only when review has body text) - AllowedEvents []string `yaml:"allowed-events,omitempty"` // Optional list of allowed review event types: APPROVE, COMMENT, REQUEST_CHANGES. If omitted, all event types are allowed. + Footer *string `yaml:"footer,omitempty"` // Controls when to show footer in PR review body: "always" (default), "none", or "if-body" (only when review has body text) + AllowedEvents []string `yaml:"allowed-events,omitempty"` // Optional list of allowed review event types: APPROVE, COMMENT, REQUEST_CHANGES. If omitted, all event types are allowed. + SupersedeOlderReviews bool `yaml:"supersede-older-reviews,omitempty"` // When true, dismisses older same-workflow REQUEST_CHANGES reviews after a replacement review is posted. } // parseSubmitPullRequestReviewConfig handles submit-pull-request-review configuration @@ -100,7 +101,15 @@ func (c *Compiler) parseSubmitPullRequestReviewConfig(outputMap map[string]any) } } - submitPRReviewLog.Printf("Parsed submit-pull-request-review config: max=%d, target=%s, target_repo=%s, allowed_events=%v", templatableIntValue(config.Max), config.Target, config.TargetRepoSlug, config.AllowedEvents) + if supersedeOlderReviews, exists := configMap["supersede-older-reviews"]; exists { + if supersedeEnabled, ok := supersedeOlderReviews.(bool); ok { + config.SupersedeOlderReviews = supersedeEnabled + } else { + submitPRReviewLog.Printf("Invalid supersede-older-reviews value: must be a boolean") + } + } + + submitPRReviewLog.Printf("Parsed submit-pull-request-review config: max=%d, target=%s, target_repo=%s, allowed_events=%v, supersede_older_reviews=%t", templatableIntValue(config.Max), config.Target, config.TargetRepoSlug, config.AllowedEvents, config.SupersedeOlderReviews) } else { // If configData is nil or not a map, set the default max config.Max = defaultIntStr(1) diff --git a/pkg/workflow/submit_pr_review_footer_test.go b/pkg/workflow/submit_pr_review_footer_test.go index 32422c1a455..8b6db8d56fd 100644 --- a/pkg/workflow/submit_pr_review_footer_test.go +++ b/pkg/workflow/submit_pr_review_footer_test.go @@ -279,6 +279,20 @@ func TestSubmitPRReviewFooterConfig(t *testing.T) { assert.Empty(t, config.AllowedEvents, "AllowedEvents should be empty when not configured") }) + t.Run("parses supersede-older-reviews field", func(t *testing.T) { + compiler := NewCompiler() + outputMap := map[string]any{ + "submit-pull-request-review": map[string]any{ + "max": 1, + "supersede-older-reviews": true, + }, + } + + config := compiler.parseSubmitPullRequestReviewConfig(outputMap) + require.NotNil(t, config, "Config should be parsed") + assert.True(t, config.SupersedeOlderReviews, "SupersedeOlderReviews should be parsed") + }) + t.Run("parses all three valid event types in allowed-events", func(t *testing.T) { compiler := NewCompiler() outputMap := map[string]any{ @@ -567,4 +581,44 @@ func TestSubmitPRReviewFooterInHandlerConfig(t *testing.T) { } } }) + + t.Run("supersede_older_reviews included in submit_pull_request_review handler config when true", func(t *testing.T) { + compiler := NewCompiler() + workflowData := &WorkflowData{ + Name: "Test", + SafeOutputs: &SafeOutputsConfig{ + SubmitPullRequestReview: &SubmitPullRequestReviewConfig{ + BaseSafeOutputConfig: BaseSafeOutputConfig{Max: strPtr("1")}, + SupersedeOlderReviews: true, + }, + }, + } + + var steps []string + compiler.addHandlerManagerConfigEnvVar(&steps, workflowData) + require.NotEmpty(t, steps, "Steps should not be empty") + + stepsContent := strings.Join(steps, "") + require.Contains(t, stepsContent, "GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG") + + for _, step := range steps { + if strings.Contains(step, "GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG") { + parts := strings.Split(step, "GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG: ") + if len(parts) == 2 { + jsonStr := strings.TrimSpace(parts[1]) + jsonStr = strings.Trim(jsonStr, "\"") + jsonStr = strings.ReplaceAll(jsonStr, "\\\"", "\"") + var handlerConfig map[string]any + err := json.Unmarshal([]byte(jsonStr), &handlerConfig) + require.NoError(t, err, "Should unmarshal handler config") + + submitConfig, ok := handlerConfig["submit_pull_request_review"].(map[string]any) + require.True(t, ok, "submit_pull_request_review config should exist") + supersedeOlderReviews, hasSupersedeOlderReviews := submitConfig["supersede_older_reviews"].(bool) + require.True(t, hasSupersedeOlderReviews, "supersede_older_reviews should be in handler config when true") + assert.True(t, supersedeOlderReviews, "supersede_older_reviews should be true") + } + } + } + }) } From 0ccc97238ef978e6afcadf1aabc429e79efc79ae Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Apr 2026 20:56:15 +0000 Subject: [PATCH 3/6] fix: tighten supersede review filtering and pagination safety Agent-Logs-Url: https://github.com/github/gh-aw/sessions/a045c112-653a-4cd4-ac03-99595109b555 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/pr_review_buffer.cjs | 20 ++++++++++++-------- actions/setup/js/pr_review_buffer.test.cjs | 9 +++++---- 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/actions/setup/js/pr_review_buffer.cjs b/actions/setup/js/pr_review_buffer.cjs index 4c156736137..c9c42d06a37 100644 --- a/actions/setup/js/pr_review_buffer.cjs +++ b/actions/setup/js/pr_review_buffer.cjs @@ -24,7 +24,8 @@ const { getErrorMessage } = require("./error_helpers.cjs"); const { isStagedMode } = require("./safe_output_helpers.cjs"); const { matchesWorkflowId } = require("./generate_footer.cjs"); -const SUPERSCEDE_REVIEW_MESSAGE = "Superseded by updated review from same workflow."; +const SUPERSEDE_REVIEW_MESSAGE = "Superseded by updated review from same workflow."; +const MAX_SUPERSEDE_REVIEW_PAGES = 10; /** * @typedef {Object} BufferedComment @@ -342,9 +343,9 @@ function createReviewBuffer() { /** * Dismiss older REQUEST_CHANGES reviews from the same workflow after posting a replacement review. * This is best-effort: failures are logged as warnings and do not fail the current review submission. - * @param {number} currentReviewID + * @param {number} currentReviewId */ - async function maybeSupersedeOlderReviews(currentReviewID) { + async function maybeSupersedeOlderReviews(currentReviewId) { if (!supersedeOlderReviews) { return; } @@ -355,11 +356,11 @@ function createReviewBuffer() { return; } - /** @type {any[]} */ + /** @type {Array<{id: number, state?: string, user?: {login?: string, type?: string}, body?: string}>} */ const reviews = []; let page = 1; const perPage = 100; - while (true) { + while (page <= MAX_SUPERSEDE_REVIEW_PAGES) { const { data } = await github.rest.pulls.listReviews({ owner: repoParts.owner, repo: repoParts.repo, @@ -377,11 +378,14 @@ function createReviewBuffer() { } page++; } + if (page > MAX_SUPERSEDE_REVIEW_PAGES) { + core.warning(`supersede-older-reviews reached pagination safety limit (${MAX_SUPERSEDE_REVIEW_PAGES} pages).`); + } const staleReviews = reviews.filter(review => { - if (!review || review.id === currentReviewID) return false; + if (!review || review.id === currentReviewId) return false; if (review.state !== "CHANGES_REQUESTED") return false; - if (review.user?.login !== "github-actions[bot]") return false; + if (review.user?.type !== "Bot") return false; return matchesWorkflowId(review.body, workflowID); }); @@ -392,7 +396,7 @@ function createReviewBuffer() { repo: repoParts.repo, pull_number: pullRequestNumber, review_id: staleReview.id, - message: SUPERSCEDE_REVIEW_MESSAGE, + message: SUPERSEDE_REVIEW_MESSAGE, }); core.info(`Dismissed superseded review #${staleReview.id}`); } catch (dismissError) { diff --git a/actions/setup/js/pr_review_buffer.test.cjs b/actions/setup/js/pr_review_buffer.test.cjs index bb6bf74fdbe..e7cf39834d9 100644 --- a/actions/setup/js/pr_review_buffer.test.cjs +++ b/actions/setup/js/pr_review_buffer.test.cjs @@ -597,9 +597,10 @@ describe("pr_review_buffer (factory pattern)", () => { }); mockGithub.rest.pulls.listReviews.mockResolvedValue({ data: [ - { id: 100, state: "CHANGES_REQUESTED", user: { login: "github-actions[bot]" }, body: "\nOld blocking review" }, - { id: 101, state: "CHANGES_REQUESTED", user: { login: "some-other-bot" }, body: "" }, - { id: 102, state: "APPROVED", user: { login: "github-actions[bot]" }, body: "" }, + { id: 100, state: "CHANGES_REQUESTED", user: { login: "github-actions[bot]", type: "Bot" }, body: "\nOld blocking review" }, + { id: 101, state: "CHANGES_REQUESTED", user: { login: "human-user", type: "User" }, body: "" }, + { id: 102, state: "APPROVED", user: { login: "github-actions[bot]", type: "Bot" }, body: "" }, + { id: 103, state: "CHANGES_REQUESTED", user: { login: "github-actions[bot]", type: "Bot" }, body: "" }, ], }); mockGithub.rest.pulls.dismissReview.mockResolvedValue({ data: {} }); @@ -645,7 +646,7 @@ describe("pr_review_buffer (factory pattern)", () => { }, }); mockGithub.rest.pulls.listReviews.mockResolvedValue({ - data: [{ id: 200, state: "CHANGES_REQUESTED", user: { login: "github-actions[bot]" }, body: "" }], + data: [{ id: 200, state: "CHANGES_REQUESTED", user: { login: "github-actions[bot]", type: "Bot" }, body: "" }], }); mockGithub.rest.pulls.dismissReview.mockRejectedValue(new Error("permission denied")); From 222d904b46f7d34c42fa8b65fbeaec7f9a13c7a1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Apr 2026 21:17:05 +0000 Subject: [PATCH 4/6] fix: scope supersede review matching to workflow call id Agent-Logs-Url: https://github.com/github/gh-aw/sessions/074978ea-6a05-4bb4-b3b5-61863df6ef6b Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/pr_review_buffer.cjs | 20 ++++++-- actions/setup/js/pr_review_buffer.test.cjs | 58 ++++++++++++++++++++-- 2 files changed, 68 insertions(+), 10 deletions(-) diff --git a/actions/setup/js/pr_review_buffer.cjs b/actions/setup/js/pr_review_buffer.cjs index c9c42d06a37..77898b70949 100644 --- a/actions/setup/js/pr_review_buffer.cjs +++ b/actions/setup/js/pr_review_buffer.cjs @@ -22,7 +22,7 @@ const { generateFooterWithMessages } = require("./messages_footer.cjs"); const { getErrorMessage } = require("./error_helpers.cjs"); const { isStagedMode } = require("./safe_output_helpers.cjs"); -const { matchesWorkflowId } = require("./generate_footer.cjs"); +const { generateWorkflowCallIdMarker, matchesWorkflowId } = require("./generate_footer.cjs"); const SUPERSEDE_REVIEW_MESSAGE = "Superseded by updated review from same workflow."; const MAX_SUPERSEDE_REVIEW_PAGES = 10; @@ -262,6 +262,11 @@ function createReviewBuffer() { footerContext.triggeringPRNumber, footerContext.triggeringDiscussionNumber ); + + const callerWorkflowId = process.env.GH_AW_CALLER_WORKFLOW_ID || ""; + if (callerWorkflowId) { + body += "\n" + generateWorkflowCallIdMarker(callerWorkflowId); + } } // Build comments array for the API @@ -350,11 +355,13 @@ function createReviewBuffer() { return; } - const workflowID = process.env.GH_AW_WORKFLOW_ID || ""; - if (!workflowID) { - core.warning("supersede-older-reviews is enabled but GH_AW_WORKFLOW_ID is not set. Skipping stale review dismissal."); + const workflowId = process.env.GH_AW_WORKFLOW_ID || ""; + const callerWorkflowId = process.env.GH_AW_CALLER_WORKFLOW_ID || ""; + if (!workflowId && !callerWorkflowId) { + core.warning("supersede-older-reviews is enabled but neither GH_AW_WORKFLOW_ID nor GH_AW_CALLER_WORKFLOW_ID is set. Skipping stale review dismissal."); return; } + const workflowCallMarker = callerWorkflowId ? generateWorkflowCallIdMarker(callerWorkflowId) : ""; /** @type {Array<{id: number, state?: string, user?: {login?: string, type?: string}, body?: string}>} */ const reviews = []; @@ -386,7 +393,10 @@ function createReviewBuffer() { if (!review || review.id === currentReviewId) return false; if (review.state !== "CHANGES_REQUESTED") return false; if (review.user?.type !== "Bot") return false; - return matchesWorkflowId(review.body, workflowID); + if (workflowCallMarker) { + return review.body?.includes(workflowCallMarker) || false; + } + return matchesWorkflowId(review.body, workflowId); }); for (const staleReview of staleReviews) { diff --git a/actions/setup/js/pr_review_buffer.test.cjs b/actions/setup/js/pr_review_buffer.test.cjs index e7cf39834d9..ed3e65a0cdc 100644 --- a/actions/setup/js/pr_review_buffer.test.cjs +++ b/actions/setup/js/pr_review_buffer.test.cjs @@ -396,6 +396,46 @@ describe("pr_review_buffer (factory pattern)", () => { expect(callArgs.body).toContain("test-workflow"); }); + it("should append workflow-call-id marker to review body when available", async () => { + const previousCallerWorkflowId = process.env.GH_AW_CALLER_WORKFLOW_ID; + process.env.GH_AW_CALLER_WORKFLOW_ID = "owner/repo/CallerA"; + try { + buffer.addComment({ path: "test.js", line: 1, body: "comment" }); + buffer.setReviewMetadata("Review body", "COMMENT"); + buffer.setReviewContext({ + repo: "owner/repo", + repoParts: { owner: "owner", repo: "repo" }, + pullRequestNumber: 42, + pullRequest: { head: { sha: "abc123" } }, + }); + buffer.setFooterContext({ + workflowName: "test-workflow", + runUrl: "https://github.com/owner/repo/actions/runs/123", + workflowSource: "owner/repo/workflows/test.md@v1", + workflowSourceURL: "https://github.com/owner/repo/blob/main/test.md", + }); + + mockGithub.rest.pulls.createReview.mockResolvedValue({ + data: { + id: 405, + html_url: "https://github.com/owner/repo/pull/42#pullrequestreview-405", + }, + }); + + const result = await buffer.submitReview(); + expect(result.success).toBe(true); + + const callArgs = mockGithub.rest.pulls.createReview.mock.calls[0][0]; + expect(callArgs.body).toContain(""); + } finally { + if (previousCallerWorkflowId === undefined) { + delete process.env.GH_AW_CALLER_WORKFLOW_ID; + } else { + process.env.GH_AW_CALLER_WORKFLOW_ID = previousCallerWorkflowId; + } + } + }); + it("should skip footer when setIncludeFooter('none') is called", async () => { buffer.addComment({ path: "test.js", line: 1, body: "comment" }); buffer.setReviewMetadata("Review body", "COMMENT"); @@ -576,9 +616,11 @@ describe("pr_review_buffer (factory pattern)", () => { expect(mockGithub.rest.pulls.createReview).toHaveBeenCalledTimes(2); }); - it("should dismiss older same-workflow REQUEST_CHANGES reviews when supersede mode is enabled", async () => { + it("should dismiss older same-workflow-call REQUEST_CHANGES reviews when supersede mode is enabled", async () => { const previousWorkflowId = process.env.GH_AW_WORKFLOW_ID; + const previousCallerWorkflowId = process.env.GH_AW_CALLER_WORKFLOW_ID; process.env.GH_AW_WORKFLOW_ID = "test-workflow"; + process.env.GH_AW_CALLER_WORKFLOW_ID = "owner/repo/CallerA"; try { buffer.setSupersedeOlderReviews(true); buffer.setReviewMetadata("Updated review", "COMMENT"); @@ -597,10 +639,11 @@ describe("pr_review_buffer (factory pattern)", () => { }); mockGithub.rest.pulls.listReviews.mockResolvedValue({ data: [ - { id: 100, state: "CHANGES_REQUESTED", user: { login: "github-actions[bot]", type: "Bot" }, body: "\nOld blocking review" }, - { id: 101, state: "CHANGES_REQUESTED", user: { login: "human-user", type: "User" }, body: "" }, - { id: 102, state: "APPROVED", user: { login: "github-actions[bot]", type: "Bot" }, body: "" }, - { id: 103, state: "CHANGES_REQUESTED", user: { login: "github-actions[bot]", type: "Bot" }, body: "" }, + { id: 100, state: "CHANGES_REQUESTED", user: { login: "github-actions[bot]", type: "Bot" }, body: "\nOld blocking review" }, + { id: 101, state: "CHANGES_REQUESTED", user: { login: "human-user", type: "User" }, body: "" }, + { id: 102, state: "APPROVED", user: { login: "github-actions[bot]", type: "Bot" }, body: "" }, + { id: 103, state: "CHANGES_REQUESTED", user: { login: "github-actions[bot]", type: "Bot" }, body: "" }, + { id: 104, state: "CHANGES_REQUESTED", user: { login: "github-actions[bot]", type: "Bot" }, body: "" }, ], }); mockGithub.rest.pulls.dismissReview.mockResolvedValue({ data: {} }); @@ -623,6 +666,11 @@ describe("pr_review_buffer (factory pattern)", () => { } else { process.env.GH_AW_WORKFLOW_ID = previousWorkflowId; } + if (previousCallerWorkflowId === undefined) { + delete process.env.GH_AW_CALLER_WORKFLOW_ID; + } else { + process.env.GH_AW_CALLER_WORKFLOW_ID = previousCallerWorkflowId; + } } }); From 7cc59ceb97000c1d35f7765d285e8da8d97a7ff6 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Apr 2026 21:18:24 +0000 Subject: [PATCH 5/6] chore: align workflow call id naming in supersede tests Agent-Logs-Url: https://github.com/github/gh-aw/sessions/074978ea-6a05-4bb4-b3b5-61863df6ef6b Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/pr_review_buffer.cjs | 6 +++--- actions/setup/js/pr_review_buffer.test.cjs | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/actions/setup/js/pr_review_buffer.cjs b/actions/setup/js/pr_review_buffer.cjs index 77898b70949..6604f0d8590 100644 --- a/actions/setup/js/pr_review_buffer.cjs +++ b/actions/setup/js/pr_review_buffer.cjs @@ -356,12 +356,12 @@ function createReviewBuffer() { } const workflowId = process.env.GH_AW_WORKFLOW_ID || ""; - const callerWorkflowId = process.env.GH_AW_CALLER_WORKFLOW_ID || ""; - if (!workflowId && !callerWorkflowId) { + const workflowCallId = process.env.GH_AW_CALLER_WORKFLOW_ID || ""; + if (!workflowId && !workflowCallId) { core.warning("supersede-older-reviews is enabled but neither GH_AW_WORKFLOW_ID nor GH_AW_CALLER_WORKFLOW_ID is set. Skipping stale review dismissal."); return; } - const workflowCallMarker = callerWorkflowId ? generateWorkflowCallIdMarker(callerWorkflowId) : ""; + const workflowCallMarker = workflowCallId ? generateWorkflowCallIdMarker(workflowCallId) : ""; /** @type {Array<{id: number, state?: string, user?: {login?: string, type?: string}, body?: string}>} */ const reviews = []; diff --git a/actions/setup/js/pr_review_buffer.test.cjs b/actions/setup/js/pr_review_buffer.test.cjs index ed3e65a0cdc..ef476b0203d 100644 --- a/actions/setup/js/pr_review_buffer.test.cjs +++ b/actions/setup/js/pr_review_buffer.test.cjs @@ -616,7 +616,7 @@ describe("pr_review_buffer (factory pattern)", () => { expect(mockGithub.rest.pulls.createReview).toHaveBeenCalledTimes(2); }); - it("should dismiss older same-workflow-call REQUEST_CHANGES reviews when supersede mode is enabled", async () => { + it("should dismiss older reviews matching workflow-call-id when supersede mode is enabled", async () => { const previousWorkflowId = process.env.GH_AW_WORKFLOW_ID; const previousCallerWorkflowId = process.env.GH_AW_CALLER_WORKFLOW_ID; process.env.GH_AW_WORKFLOW_ID = "test-workflow"; From 6d5baa692a66aa4089adbd7933708dda519dc46e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Apr 2026 21:30:46 +0000 Subject: [PATCH 6/6] fix: make supersede listing best-effort and align docs examples Agent-Logs-Url: https://github.com/github/gh-aw/sessions/0cc78ee4-76d7-404a-8949-d4625e88d412 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/pr_review_buffer.cjs | 91 ++++++++++--------- actions/setup/js/pr_review_buffer.test.cjs | 35 +++++++ .../reference/safe-outputs-pull-requests.md | 2 +- .../reference/safe-outputs-specification.md | 2 +- .../content/docs/reference/safe-outputs.md | 2 +- 5 files changed, 85 insertions(+), 47 deletions(-) diff --git a/actions/setup/js/pr_review_buffer.cjs b/actions/setup/js/pr_review_buffer.cjs index 6604f0d8590..a53515cf87c 100644 --- a/actions/setup/js/pr_review_buffer.cjs +++ b/actions/setup/js/pr_review_buffer.cjs @@ -362,56 +362,59 @@ function createReviewBuffer() { return; } const workflowCallMarker = workflowCallId ? generateWorkflowCallIdMarker(workflowCallId) : ""; + try { + /** @type {Array<{id: number, state?: string, user?: {login?: string, type?: string}, body?: string}>} */ + const reviews = []; + let page = 1; + const perPage = 100; + while (page <= MAX_SUPERSEDE_REVIEW_PAGES) { + const { data } = await github.rest.pulls.listReviews({ + owner: repoParts.owner, + repo: repoParts.repo, + pull_number: pullRequestNumber, + per_page: perPage, + page, + }); - /** @type {Array<{id: number, state?: string, user?: {login?: string, type?: string}, body?: string}>} */ - const reviews = []; - let page = 1; - const perPage = 100; - while (page <= MAX_SUPERSEDE_REVIEW_PAGES) { - const { data } = await github.rest.pulls.listReviews({ - owner: repoParts.owner, - repo: repoParts.repo, - pull_number: pullRequestNumber, - per_page: perPage, - page, - }); - - if (!Array.isArray(data) || data.length === 0) { - break; + if (!Array.isArray(data) || data.length === 0) { + break; + } + reviews.push(...data); + if (data.length < perPage) { + break; + } + page++; } - reviews.push(...data); - if (data.length < perPage) { - break; + if (page > MAX_SUPERSEDE_REVIEW_PAGES) { + core.warning(`supersede-older-reviews reached pagination safety limit (${MAX_SUPERSEDE_REVIEW_PAGES} pages).`); } - page++; - } - if (page > MAX_SUPERSEDE_REVIEW_PAGES) { - core.warning(`supersede-older-reviews reached pagination safety limit (${MAX_SUPERSEDE_REVIEW_PAGES} pages).`); - } - const staleReviews = reviews.filter(review => { - if (!review || review.id === currentReviewId) return false; - if (review.state !== "CHANGES_REQUESTED") return false; - if (review.user?.type !== "Bot") return false; - if (workflowCallMarker) { - return review.body?.includes(workflowCallMarker) || false; - } - return matchesWorkflowId(review.body, workflowId); - }); + const staleReviews = reviews.filter(review => { + if (!review || review.id === currentReviewId) return false; + if (review.state !== "CHANGES_REQUESTED") return false; + if (review.user?.type !== "Bot") return false; + if (workflowCallMarker) { + return review.body?.includes(workflowCallMarker) || false; + } + return matchesWorkflowId(review.body, workflowId); + }); - for (const staleReview of staleReviews) { - try { - await github.rest.pulls.dismissReview({ - owner: repoParts.owner, - repo: repoParts.repo, - pull_number: pullRequestNumber, - review_id: staleReview.id, - message: SUPERSEDE_REVIEW_MESSAGE, - }); - core.info(`Dismissed superseded review #${staleReview.id}`); - } catch (dismissError) { - core.warning(`Failed to dismiss stale review #${staleReview.id}: ${getErrorMessage(dismissError)}`); + for (const staleReview of staleReviews) { + try { + await github.rest.pulls.dismissReview({ + owner: repoParts.owner, + repo: repoParts.repo, + pull_number: pullRequestNumber, + review_id: staleReview.id, + message: SUPERSEDE_REVIEW_MESSAGE, + }); + core.info(`Dismissed superseded review #${staleReview.id}`); + } catch (dismissError) { + core.warning(`Failed to dismiss stale review #${staleReview.id}: ${getErrorMessage(dismissError)}`); + } } + } catch (listOrSupersedeError) { + core.warning(`Failed to supersede older reviews: ${getErrorMessage(listOrSupersedeError)}`); } } diff --git a/actions/setup/js/pr_review_buffer.test.cjs b/actions/setup/js/pr_review_buffer.test.cjs index ef476b0203d..b4b36e27e46 100644 --- a/actions/setup/js/pr_review_buffer.test.cjs +++ b/actions/setup/js/pr_review_buffer.test.cjs @@ -711,6 +711,41 @@ describe("pr_review_buffer (factory pattern)", () => { } }); + it("should warn and continue when stale review listing fails", async () => { + const previousWorkflowId = process.env.GH_AW_WORKFLOW_ID; + process.env.GH_AW_WORKFLOW_ID = "test-workflow"; + try { + buffer.setSupersedeOlderReviews(true); + buffer.setReviewMetadata("Updated review", "COMMENT"); + buffer.setReviewContext({ + repo: "owner/repo", + repoParts: { owner: "owner", repo: "repo" }, + pullRequestNumber: 42, + pullRequest: { head: { sha: "abc123" } }, + }); + + mockGithub.rest.pulls.createReview.mockResolvedValue({ + data: { + id: 902, + html_url: "https://github.com/owner/repo/pull/42#pullrequestreview-902", + }, + }); + mockGithub.rest.pulls.listReviews.mockRejectedValue(new Error("rate limited")); + + const result = await buffer.submitReview(); + + expect(result.success).toBe(true); + expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("Failed to supersede older reviews")); + expect(mockGithub.rest.pulls.dismissReview).not.toHaveBeenCalled(); + } finally { + if (previousWorkflowId === undefined) { + delete process.env.GH_AW_WORKFLOW_ID; + } else { + process.env.GH_AW_WORKFLOW_ID = previousWorkflowId; + } + } + }); + it("should handle API errors gracefully", async () => { buffer.addComment({ path: "test.js", line: 1, body: "comment" }); buffer.setReviewContext({ diff --git a/docs/src/content/docs/reference/safe-outputs-pull-requests.md b/docs/src/content/docs/reference/safe-outputs-pull-requests.md index ea6fe3d4e86..68a28e72a58 100644 --- a/docs/src/content/docs/reference/safe-outputs-pull-requests.md +++ b/docs/src/content/docs/reference/safe-outputs-pull-requests.md @@ -181,7 +181,7 @@ Submits a consolidated pull request review. Inline comments buffered by `create- safe-outputs: submit-pull-request-review: max: 1 - allowed-events: [COMMENT] # preferred default + allowed-events: [COMMENT, REQUEST_CHANGES] # include REQUEST_CHANGES when superseding older blocking reviews supersede-older-reviews: true # dismiss older same-workflow REQUEST_CHANGES reviews after replacement target: "triggering" # or "*", or explicit PR number target-repo: "owner/repo" # cross-repository diff --git a/docs/src/content/docs/reference/safe-outputs-specification.md b/docs/src/content/docs/reference/safe-outputs-specification.md index 08365151a99..d5c66d63dae 100644 --- a/docs/src/content/docs/reference/safe-outputs-specification.md +++ b/docs/src/content/docs/reference/safe-outputs-specification.md @@ -1566,7 +1566,7 @@ submit-pull-request-review: target-repo: owner/repo # Cross-repository target allowed-repos: [...] # Additional allowed repositories allowed-events: [COMMENT] # Preferred default for non-blocking bot reviews - supersede-older-reviews: true # Best-effort dismissal of older same-workflow REQUEST_CHANGES reviews + supersede-older-reviews: true # Best-effort dismissal of older same-workflow REQUEST_CHANGES reviews (including legacy blockers) footer: "always" | "none" | "if-body" # Footer on review body ``` diff --git a/docs/src/content/docs/reference/safe-outputs.md b/docs/src/content/docs/reference/safe-outputs.md index 179a3446915..690e638b961 100644 --- a/docs/src/content/docs/reference/safe-outputs.md +++ b/docs/src/content/docs/reference/safe-outputs.md @@ -668,7 +668,7 @@ safe-outputs: target: "triggering" # or "*", or e.g. ${{ github.event.inputs.pr_number }} when not in pull_request trigger target-repo: "owner/repo" # cross-repository: submit review on PR in another repo allowed-repos: ["org/repo1", "org/repo2"] # additional allowed repositories - allowed-events: [COMMENT] # preferred default: informative reviews without merge-blocking + allowed-events: [COMMENT, REQUEST_CHANGES] # include REQUEST_CHANGES when using supersede mode for blocking reviews supersede-older-reviews: true # dismiss older same-workflow REQUEST_CHANGES reviews after posting a replacement review footer: false # omit AI-generated footer from review body (default: true) ```