Conversation
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>
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>
supersede-older-reviews for PR reviews and shift guidance to COMMENT-first defaults
There was a problem hiding this comment.
Pull request overview
Adds an opt-in supersede-older-reviews capability for submit-pull-request-review so newer workflow runs can dismiss stale same-workflow CHANGES_REQUESTED bot reviews, and updates docs/tooling text to recommend COMMENT-first defaults for automated review workflows.
Changes:
- Introduces
supersede-older-reviewsin the safe-outputs schema and Go compiler config parsing, and emits it into handler config assupersede_older_reviews. - Implements runtime superseding in the PR review buffer by listing PR reviews and dismissing same-workflow bot
CHANGES_REQUESTEDreviews after posting the replacement review. - Updates safe-output tool descriptions and documentation to steer users toward
allowed-events: [COMMENT]by default, with tests added for the new behavior.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/submit_pr_review_footer_test.go | Adds Go tests for parsing/emitting supersede-older-reviews into handler config. |
| pkg/workflow/submit_pr_review.go | Adds SupersedeOlderReviews to config and parses supersede-older-reviews from YAML. |
| pkg/workflow/js/safe_outputs_tools.json | Updates tool description copy to recommend COMMENT-first usage. |
| pkg/workflow/compiler_safe_outputs_handlers.go | Emits supersede_older_reviews in handler config when enabled. |
| pkg/parser/schemas/main_workflow_schema.json | Extends workflow schema with supersede-older-reviews boolean. |
| docs/src/content/docs/reference/safe-outputs.md | Documents the new field and updates guidance/examples around COMMENT-first defaults. |
| docs/src/content/docs/reference/safe-outputs-specification.md | Updates the specification snippet to include allowed-events guidance and supersede-older-reviews. |
| docs/src/content/docs/reference/safe-outputs-pull-requests.md | Adds/expands submit-pull-request-review reference section including new option. |
| docs/src/content/docs/reference/glossary.md | Updates glossary entry for allowed-events to reflect preferred COMMENT default. |
| docs/src/content/docs/reference/frontmatter-full.md | Adds frontmatter documentation for supersede-older-reviews. |
| actions/setup/js/submit_pr_review.test.cjs | Adds JS test ensuring supersede mode is propagated to the review buffer with warning text. |
| actions/setup/js/submit_pr_review.cjs | Parses supersede_older_reviews and enables supersede behavior on the shared review buffer. |
| actions/setup/js/safe_outputs_tools.json | Updates tool description copy to recommend COMMENT-first usage (runtime copy). |
| actions/setup/js/pr_review_buffer.test.cjs | Adds JS tests verifying stale same-workflow blocking review dismissal and failure-is-warning behavior. |
| actions/setup/js/pr_review_buffer.cjs | Implements the runtime review superseding (list + dismiss matching stale reviews). |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 15/15 changed files
- Comments generated: 4
| /** @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; | ||
| } | ||
| reviews.push(...data); | ||
| if (data.length < perPage) { | ||
| break; | ||
| } | ||
| 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; | ||
| 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)}`); | ||
| } |
There was a problem hiding this comment.
maybeSupersedeOlderReviews() is documented as best-effort, but the listReviews pagination loop isn't wrapped in error handling. If github.rest.pulls.listReviews fails (permissions, transient API error, rate limiting), the exception will bubble up and fail the overall review submission path, contradicting the best-effort contract. Wrap the listing/pagination portion (and ideally the whole supersede routine) in a try/catch that logs a warning and returns without failing the review submission.
| /** @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; | |
| } | |
| reviews.push(...data); | |
| if (data.length < perPage) { | |
| break; | |
| } | |
| 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; | |
| 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)}`); | |
| } | |
| 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, | |
| }); | |
| if (!Array.isArray(data) || data.length === 0) { | |
| break; | |
| } | |
| reviews.push(...data); | |
| if (data.length < perPage) { | |
| break; | |
| } | |
| 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; | |
| 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)}`); | |
| } | |
| } | |
| } catch (supersedeError) { | |
| core.warning(`Failed to supersede older reviews: ${getErrorMessage(supersedeError)}`); | |
| return; |
| 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 |
There was a problem hiding this comment.
In this example, allowed-events: [COMMENT] is presented alongside supersede-older-reviews: true. A few lines later the doc says to set supersede-older-reviews only when intentionally using REQUEST_CHANGES, which reads as contradictory. Consider either (a) updating the example to include REQUEST_CHANGES when supersede-older-reviews is enabled, or (b) explicitly noting that superseding can still be used with COMMENT-only workflows to clean up stale blocking reviews from earlier runs/configurations.
| supersede-older-reviews: true # dismiss older same-workflow REQUEST_CHANGES reviews after posting a replacement review |
| 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" |
There was a problem hiding this comment.
This configuration snippet sets allowed-events: [COMMENT] but also enables supersede-older-reviews: true. Since the section text below frames superseding as something you enable when you intentionally allow REQUEST_CHANGES, this example may confuse readers. Consider adjusting the snippet to include REQUEST_CHANGES when superseding is enabled, or add a short note that superseding can also be used in COMMENT-only mode to dismiss stale blocking reviews from prior workflow runs.
| 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" | |
| 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 | |
| allowed-repos: ["org/repo1"] # additional allowed repositories | |
| footer: "always" # "always", "none", or "if-body" |
| 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 |
There was a problem hiding this comment.
In the specification example, allowed-events: [COMMENT] is shown together with supersede-older-reviews: true. Given superseding is primarily relevant when REQUEST_CHANGES reviews are being created, consider either changing the example to include REQUEST_CHANGES when superseding is enabled or clarifying in the inline comment that this option can also be used to dismiss legacy blocking reviews even if the current workflow only posts COMMENT 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 or legacy REQUEST_CHANGES reviews |
🧪 Test Quality Sentinel ReportTest Quality Score: 60/100
Test Classification DetailsView All Test Classifications (5 tests)
Flagged Tests — Requires Review
|
|
@copilot review all comments |
|
Hey
|
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>
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>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
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>
Done — I reviewed all comments and addressed the remaining actionable ones. I made supersede cleanup fully best-effort (listReviews failures now warn and continue) and aligned the docs examples/comments for supersede + allowed-events. Commit: Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
✅ Smoke CI completed successfully! |
|
🤖 Smoke CI check for this PR. Workflow run: https://github.com/github/gh-aw/actions/runs/24748453102 |
Add new glossary entries for terms introduced in recent changes: - GitHub Access Mode (tools.github.mode / gh-proxy) from PR #27707 - Comment Memory (tools.comment-memory) from PR #27479 - Supersede Older Reviews (supersede-older-reviews) from PR #27662 - Update AWF entry to mention sandbox.agent.version from PR #27626 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Stale bot
REQUEST_CHANGESreviews could continue blocking PRs after re-review because there was no lifecycle control forsubmit-pull-request-review. This change adds a scoped supersede path for older blocking reviews and updates docs to recommendCOMMENTas the default review mode.submit-pull-request-reviewnow supports review supersedingsupersede-older-reviews: booleanto safe-outputs schema/config.Runtime behavior for stale blocking review cleanup
CHANGES_REQUESTEDreviews that are:GH_AW_CALLER_WORKFLOW_IDviagh-aw-workflow-call-idmarker matching (for reusable workflow call scoping),GH_AW_WORKFLOW_IDmarker matching when call-id is unavailable.Safety/operational constraints in implementation
COMMENTremains the preferred default unless merge-blocking behavior is intentionally needed.Documentation and tool contract updates
supersede-older-reviews,allowed-events: [COMMENT]as preferred default.> [!WARNING]
>
>