Skip to content

fix(ci): prevent no-op review signal cancellations - #5812

Draft
cixzhang wants to merge 1 commit into
mainfrom
fix/noop-workflow-cancellation
Draft

fix(ci): prevent no-op review signal cancellations#5812
cixzhang wants to merge 1 commit into
mainfrom
fix/noop-workflow-cancellation

Conversation

@cixzhang

@cixzhang cixzhang commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Why

A head update followed by a draft/ready lifecycle transition can enqueue two Review signal runs for the same commit. Workflow-level cancellation makes the useful run appear cancelled/red even though the later duplicate succeeds.

What

  • Stop starting Review signal for draft/ready transitions, which do not change its classification.
  • Serialize only effect-bearing flag jobs without cancelling the running job.
  • Load a dependency-free policy from trusted base code, reject superseded event heads, and recheck the live PR head before every write.
  • Add mutation-sensitive coverage for the duplicate event, superseded head, current effect-bearing run, and stale-write suppression.

Risk

Opened, synchronized, reopened, review-approval, and manual recovery paths remain covered. The workflow still never checks out or executes PR-controlled code.

Testing

  • 72 focused workflow/helper tests
  • actionlint
  • Prettier
  • pnpm check:repo
  • pre-commit repository checks

No Changeset: workflow-only infrastructure change.

@vercel

vercel Bot commented Sep 1, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
astryx Ready Ready Preview Sep 1, 2026 4:52am UTC

Request Review

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Sep 1, 2026
github-actions Bot added a commit that referenced this pull request Sep 1, 2026
@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

PR Analysis Report

Preview availability: Storybook and Sandbox were not published for this CI run.

No new or modified components detected.

Bundle Size Summary

No component packages changed.

Accessibility Audit

Status: No accessibility violations detected.


Generated by PR Enrichment workflow | View full report

@cixzhang cixzhang left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Semantic verdict: request changes

An approved PR can still end with review-required: pending: flagging may snapshot reviews before an approval, Review clear then writes success, and the older flag run can overwrite it with pending on the same head. Because this status is required, maintainers are left with an approved but unmergeable PR and no later event guaranteed to repair it.

Please make approval the final projected state for this same-head interleaving, and add a focused test that pauses flagging after its review snapshot, lets Review clear succeed, then proves the final status remains success.

[Reviewed by Robohands]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Meta Open Source bot.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant