Skip to content

[skill] Correct babysit-pr-to-pass-ci on the pr-gate cooldown - #35633

Closed
michaelzhang-ai wants to merge 1 commit into
mainfrom
cursor/babysit-ci-skill-cooldown-lessons-d1f1
Closed

michaelzhang-ai wants to merge 1 commit into
mainfrom
cursor/babysit-ci-skill-cooldown-lessons-d1f1

Conversation

@michaelzhang-ai

@michaelzhang-ai michaelzhang-ai commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator

Motivation

Babysitting #34481 and #34485 through pr-gate for about a week surfaced one piece of guidance in this skill that is actively counterproductive, plus a few gaps that cost real time. Everything below is checked against the current pr-gate.yml / pr-test.yml rather than recalled.

The cooldown advice was wrong. The skill said to wait out the cooldown rather than burn retries. Waiting does not work, for two compounding reasons:

  • pr-gate rate-limits on the run's triggering_actor and exempts anyone with write access, so the outcome turns on identity rather than on the PR. Agent pushes share one bot identity, so unrelated agents pushing to the same repository consume the same one-run-per-120-minutes budget. A window verified clear can be taken seconds later — that happened twice.
  • A gate-blocked run still counts against the window. pr-gate tries to discount such runs (didRunPassGate), but its exclusion list is ['check-changes', 'pr-gate', 'call-gate', 'pr-test-finish'] and pr-test.yml's notify-pr-states job is not in it. That job is if: always() and succeeds, so a run that never got past the gate reads as having consumed CI. Each blocked push therefore renews its own window: retry-by-push cannot converge. Observed directly — a blocked push at 05:48 moved the next opportunity to 07:48.

The escape hatch is a rerun, not a push: a rerun's triggering_actor is github-actions[bot], which pr-gate substitutes with the PR author, so a write-access author clears the gate immediately. That is what finally unblocked both PRs after six days.

Modifications

Six surgical edits, +15/-5 lines in SKILL.md:

  • Known SGLang gates — replace "wait for cooldown" with the actual mechanics above and the rerun escape hatch, plus: do not manufacture a junk commit to force a fresh run; rebase onto the base branch if a new head SHA is genuinely wanted.
  • Handle an unrelated or infrastructure failure — carve an exception into the blanket ban on full-run reruns. A gate-blocked run leaves its downstream jobs skipped, not failed, so --failed has nothing to resurrect and the whole run must be rerun. Without this the skill forbids its own escape hatch.
  • Preflight — confirm the credentials can actually rerun a workflow before relying on it, since a read-only default token makes every gh run rerun fail; check the run's injected secrets for one with repo + workflow scope, verify the account via gh api user and the x-oauth-scopes header, and never print the value.
  • Monitor persistently — re-read the PR's state/mergedAt each cycle, not just its checks. [AMD] Let the diffusion AITer backend take grouped-query K/V (fix Cosmos3-Nano startup) #34485 was reported as "one CI cycle away" when it had been merged eleven hours earlier, because gh pr checks says nothing about merge state. Also: treat cancelled as "never reported" rather than a retryable failure, since a merge or superseding push cancels in-flight runs.
  • Track only current-head runs — read a workflow's trigger before concluding a run is merely late. A workflow can carry a PR-shaped name and no pull_request trigger at all (pr-test-amd.yml is now a daily ROCm 7.0 shadow; the PR gate is pr-test-amd-rocm720.yml), which makes selecting it unsatisfiable. Phrased so it stays true as that ownership drifts. Also note nightly-registered tests are unreachable from any PR run.
  • Diagnose — make "pre-existing on main" checkable: diff the set of failing cases against a recent run of the same workflow file on the base branch, comparing cases rather than partition indices, since sharding is by estimated time and cases migrate between partitions. This is what separated [AMD] Keep the PTX-inline-asm diffusion norm fusions off on ROCm (fix FLUX warmup crash) #34481's effect from five pre-existing AMD failures, and it also surfaced a case the PR fixed incidentally.

Accuracy Tests

Documentation only — no runtime code touched, so there is nothing to test at runtime. The factual claims were verified against the workflows on main rather than from memory: gateJobs at pr-gate.yml:202 omits notify-pr-states; that job exists at pr-test.yml:561 with always(); the github-actions[bot] -> PR-author substitution is at pr-gate.yml:93-99; and hasHighPermission short-circuits the limit at pr-gate.yml:112. Also checked the file still parses as a valid skill (frontmatter intact, ordered-list numbering consistent, no broken relative links).

Speed Tests and Profiling

Not applicable.

Checklist

Review and Merge Process

  1. Ping Merge Oncalls to start the process. See the PR Merge Process.
  2. Get approvals from CODEOWNERS and other reviewers.
  3. Trigger CI tests with comments or contact authorized users to do so.
    • Common commands include /tag-and-rerun-ci, /tag-run-ci-label, /rerun-failed-ci
  4. After green CI and required approvals, ask Merge Oncalls or people with Write permission to merge the PR.

CI States

Latest PR Test (Base): ✅ Run #32331298661
Latest PR Test (Extra): ❌ Run #32331298457

Babysitting two AMD PRs through pr-gate turned up guidance in this skill
that is actively counterproductive, plus a few gaps worth closing.

The cooldown advice said to wait it out. Waiting does not work: pr-gate
rate-limits on triggering_actor, agent pushes share one bot identity so
unrelated agents consume the same budget, and a gate-blocked run still
counts against the window because pr-test.yml's notify-pr-states job runs
always(), succeeds, and is absent from pr-gate's gateJobs exclusion list.
Each blocked push therefore renews the window and retry-by-push cannot
converge. A rerun is the escape hatch, since its triggering_actor is
github-actions[bot], which pr-gate substitutes with the PR author.

That also needs an exception to the blanket ban on full-run reruns: a
gate-blocked run leaves its downstream jobs skipped rather than failed,
so --failed has nothing to resurrect and the whole run must be rerun.

Preflight now checks that the credentials can rerun at all, since a
read-only default token makes the skill's main lever fail silently.

Also: re-read PR state and mergedAt each cycle, because polling checks
alone let a merged PR look like it is still one cycle from green;
treat cancelled as 'never reported' rather than as a retryable failure;
verify a workflow actually has a pull_request trigger before waiting on
a run that can never appear, and note that nightly-registered tests are
unreachable from a PR; and establish 'pre-existing on main' by diffing
failing cases against the same workflow on the base branch, comparing
cases rather than partition indices.

Co-authored-by: quitenode <quitenode@users.noreply.github.com>
@github-actions github-actions Bot added the documentation Improvements or additions to documentation label Aug 20, 2026
@Jiminator
Jiminator deleted the cursor/babysit-ci-skill-cooldown-lessons-d1f1 branch September 14, 2026 04:41
@alexnails
alexnails restored the cursor/babysit-ci-skill-cooldown-lessons-d1f1 branch September 14, 2026 05:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant