Repository navigation
Keep the required checks green when a pull request's description is edited - #676
Conversation
…edited The unit and E2E workflows listened for `edited` so a base move would re-run their `changes` job. Any other edit started a run that skipped every job, and GitHub reports the newest run on a commit as its checks, so the six required checks went back to "Expected" on every title or description edit. Drop `edited` from both workflows. A new base-change.yml re-runs the latest run of each for the PR's head when the base moves; a re-run is an attempt of the same run, and `changes` reads the file list live. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… head's run Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@ryanwelcher TL;DR: editing a PR's title or description resets the six required checks, which is very annoying. With this change, a new small workflow re-runs them only when a PR’s base changes. |
|
Amazing! Great addition. |
Why
Since #660, editing a pull request's title or description turns its six required checks (
unit,JourneysandPackaged smokeon macOS and Windows) into "Expected — Waiting for status to be reported", even though they already passed on that commit. The merge box shows them as pending until someone pushes or re-runs. Seen on #675: the runs at 07:59 passed, and the edits at 08:18 and 08:45 started runs 37904200109/37904200065 and 37906918098/37906918097 with every job skipped.What changes
Root cause: #660 made
unit-tests.ymlande2e.ymllisten foredited, so that a base move would re-run theirchangesjob. For any other edit,changesis skipped and every job after it is skipped too. GitHub treats the newest run of a workflow on a commit as that commit's checks. A skipped matrix job is reported with its name unexpanded (unit (${{ matrix.os }})), so the requiredunit (macos-latest)and the rest disappear from the newest run.Fix: both workflows stop listening for
edited, so a title or description edit starts nothing in them and the earlier results stand. A new.github/workflows/base-change.ymlhandles a base move instead. It finds the latest Unit tests and E2E runs for the PR's head and re-runs them withgh run rerun; a run that is still in progress is cancelled first and re-run once it stops. A re-run is a new attempt of the same run rather than a newer run, so it replaces those checks directly. Thechangesjob reads the PR's file list from the API at run time, so the re-run sees the new base: a Markdown-only stacked PR moved onto trunk is evaluated as code again, which is the guarantee #660 added.Deliberately not in it: re-running anything for a fork PR. A fork's token is read-only, so the workflow posts a warning instead (see Risks).
How to test this
Platforms: none. This is CI only, and it runs on GitHub's Linux runners. The workflows only take effect once this is on trunk:
editedruns use the workflow files from the PR's merge commit, so a PR opened after this merges, or one that is pushed to after it merges, picks them up.Starting state: this branch merged to trunk. You need a pull request against trunk whose head was pushed after the merge, with all six required checks passed.
rerunandforkjobs are both skipped. In the merge box, all six required checks are still ✓, not "Expected — Waiting for status to be reported".trunk. Expected: a Re-run on base change run whosererunjob printsre-running run <id>forunit-tests.ymlande2e.yml. Unit tests and E2E each show a second attempt of their earlier run. In it,changesreportscode=true(B now carries A's code), and the real macOS and Windows jobs run in place of the stubs.What must not have happened:
Cannot be tested here: none of this runs before merge (see Platforms), and there is no unit test for workflow files. In particular, it is unverified that
GITHUB_TOKENwithactions: writemay cancel and re-run apull_requestrun in this repository. Steps 3 and 4 are the check for that, and the first stacked retarget after merge should be watched. The workflows passactionlint(with shellcheck), and thegh run listlookup was run against #675's real runs.Risks and limitations
editedruns did check out a fresh merge with the new base.::warning::asks for a manual re-run or a push.pull_request_targetwould have a write token, but re-running a fork's run with it might bypass the approval that a first-time contributor's run waits for, so I did not use it. It is rare: a fork PR based on an upstream branch other than trunk.editedruns (as on Draw the next-action cue so it shows on a warning notice, and use design system tokens for the window's remaining values #675 now), re-running it replays that skip. A push, or re-running the earlier passing run from the Actions tab, clears it once.Related
Follow-up to #660. Seen on #675.
Design decisions and alternatives considered
edited, but give the matrix jobs expanded names when skipped: not possible. GitHub does not expand a matrix for a job that is skipped at the job level, which is why Skip the unit and E2E suites on pull requests that change only Markdown #660 added stub jobs in the first place.editedand run the stubs/real jobs again on every edit: every title edit would cost two Linux jobs at best and the full macOS/Windows suites at worst, and it would cancel or duplicate runs.workflow_dispatchfrom the edit workflow: this would start a fresh run on the head with the new base. But it is a different event, so thepull_request-only logic inchangeswould need a second branch, and the checks would come from a run nobody pushed. A re-run reuses everything that already exists.base-change-<PR>, so a second move cancels the first and re-runs whatever the first left behind. Any other edit gets a group per run (-<run_id>), so it cannot cancel a base move. The concurrency group is evaluated before the job'sif:, so without this a description edit made during a base move would have cancelled it.Review outcome
6 [fix here] · 1 [follow-up]: 5 fixed, 1 covered by How to test, 1 follow-up recorded in Risks.
npm run lintclean; unit suite 2096 pass, 0 fail, 2 skipped (local, macOS).actionlintclean on all workflows.da2dbfa, base7ee3ead. Findings:if:. Fixed.da2dbfa → 667bd95re-reviewed by the same separate context. Migrate CI to Builkdite & Implement Code Signing #1–Credentials to enter the admin dashboard are actually: admin / password #4 resolved. One new 🔵: a push during the cancel-and-wait loop could still be cancelled. Fixed in667bd95 → 8b5ea25, which moves the head check to just before each re-run. I checked that last change myself; it was not reviewed again.Implementation notes
gh run list --workflow <file> --event pull_request --commit <head sha> --branch <head ref> --limit 1.--commitmatches the run's head SHA, not the merge commit's, and--eventleaves out trunk push runs.actions: write(cancel, re-run) andpull-requests: read(the head check). The workflow's default permissions are{}. Every value from the event reaches the script throughenv:, never interpolated into it.if ! gh run rerunkeeps a failure for one workflow from skipping the other underbash -e. The job then exits non-zero.changesjobs lose theirif:, and the concurrency groups lose their-editsuffix, since neither workflow seeseditedany more.Screenshots: nothing on screen changed.
🤖 Generated with Claude Code