Repository navigation
ci: batch trunk-PR CI on the merge train's ci:run label - #546
Conversation
Port AgentWorkforce/cloud #4350/#4351 so the merge-train sweeper can manage this repo: - ci.yml, contract.yml and relayfile-evals.yml run the trunk -> main PR on opened/reopened/labeled only (never synchronize); every job skips label events other than `ci:run`. - Their concurrency gives unrelated label events a throwaway group so they never cancel a real promotion run; push runs key off the sha. - New `Merge-train ready check` workflow on `mergeable` feature PRs into trunk (go build + TS build + typecheck/go vet). - scripts/merge-train-ci-workflows.test.mjs pins the shared names (ci:run, `Go Test` marker, ready check). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 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 |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
Devin Review found 1 potential issue.
1 flag not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)
| jobs: | ||
| go-test: | ||
| if: (github.event_name != 'pull_request' && github.event_name != 'pull_request_target') || (github.head_ref == 'trunk' && github.event.pull_request.head.repo.full_name == github.repository && github.base_ref == 'main') | ||
| if: (github.event_name != 'pull_request' && github.event_name != 'pull_request_target') || (github.head_ref == 'trunk' && github.event.pull_request.head.repo.full_name == github.repository && github.base_ref == 'main' && (github.event.action != 'labeled' || github.event.label.name == 'ci:run')) |
There was a problem hiding this comment.
🔴 Unrelated labels mask failed promotion checks
When any label other than ci:run is added, go-test skips and publishes a newer check on the same head. GitHub accepts skipped required checks, so an earlier failing Go Test can stop blocking promotion.
Learn more
A pull_request labeled event creates a new workflow run for the existing PR head. This gate skips Go Test when the added label is not ci:run, but the skipped job still has a check result on that head. GitHub accepts skipped jobs as successful required checks, and the newer result can mask the failed Go Test result from the actual CI run. The same gate appears on the other CI jobs and in contract and evals. The ready check already recognizes the analogous latest-check masking problem for feature PRs.
Example: Go Test fails on a trunk-to-main PR. Someone then adds a documentation label. The resulting Go Test job is skipped on the same head; the newest required Go Test result no longer reflects the failed tests.
Recommended fix: Avoid creating skipped promotion check runs for unrelated labels, or keep the check jobs running when any label event creates a run. Apply the same policy consistently to CI, Contract, and Evals, and test that adding an unrelated label after a failed run cannot turn the promotion checks green.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Not changing this. The merge-train sweeper already handles it, the same way cloud does since #4351: effectiveCheckRuns (cloud packages/web/lib/merge-train/plan.ts) keeps a newer skipped run of a check name only when no non-skipped run exists. So a skipped ci:run marker or job left by an unrelated label never masks the real verdict, whether green or red. The sweeper also treats a skipped marker as no verdict. On branch protection: this repo has no required status checks, and the sweeper, not GitHub's mergeability, is the merge gate for the trunk -> main PR.
A fork PR's ready job was skipped (same-repo condition), which the sweeper reads as "missing" and re-kicks with `ready:check` every tick. Under `pull_request` a fork gets no secrets and a read-only token, and the job uses neither, so it is safe to run. Trust stays the sweeper's gate: an external PR merges only with a maintainer's APPROVED review on its exact head. The contract test now pins: no same-repo condition, `pull_request` as the only trigger, no secrets, checkout without persisted credentials. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A substring check would still pass if a job appended an `|| ...` bypass that runs promotion CI on unrelated label events. Pin the complete expression. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found and verified against the latest diff
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".github/workflows/merge-train-ready.yml">
<violation number="1" location=".github/workflows/merge-train-ready.yml:20">
P1: `pull_request` runs the PR's workflow revision, so a feature PR can remove these steps and still produce the check the sweeper trusts. Load this gate from the base branch and safely test the PR revision, or reject workflow changes in the sweeper.</violation>
</file>
<file name=".github/workflows/ci.yml">
<violation number="1" location=".github/workflows/ci.yml:31">
P1: This skips the required `Go Test` check for unrelated labels, and GitHub treats skipped jobs as successful; a same-head label run can therefore mask failed promotion tests. Keep this check running on label events or prevent ignored-label runs from publishing the required check context.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Re-trigger cubic
| # `mergeable` is on. `edited` is deliberately not a trigger, since body edits | ||
| # would re-run it constantly. | ||
| on: | ||
| pull_request: |
There was a problem hiding this comment.
P1: pull_request runs the PR's workflow revision, so a feature PR can remove these steps and still produce the check the sweeper trusts. Load this gate from the base branch and safely test the PR revision, or reject workflow changes in the sweeper.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At .github/workflows/merge-train-ready.yml, line 16:
<comment>`pull_request` runs the PR's workflow revision, so a feature PR can remove these steps and still produce the check the sweeper trusts. Load this gate from the base branch and safely test the PR revision, or reject workflow changes in the sweeper.</comment>
<file context>
@@ -0,0 +1,62 @@
+# `mergeable` is on. `edited` is deliberately not a trigger, since body edits
+# would re-run it constantly.
+on:
+ pull_request:
+ branches: [trunk]
+ types: [labeled, synchronize, reopened]
</file context>
There was a problem hiding this comment.
Valid concern, and it's the same as cloud's existing ready check. Mitigation is the sweeper's trust gate: external PRs merge only with a maintainer's APPROVED review on the exact head, and that reviewer sees any workflow edit. Internal authors have write access anyway. I'm raising a follow-up for the sweeper to hold any PR that edits .github/workflows/merge-train-ready.yml unless a maintainer approved it. pull_request_target with a checkout of the PR head would be worse: it gives untrusted code a privileged token.
| jobs: | ||
| go-test: | ||
| if: (github.event_name != 'pull_request' && github.event_name != 'pull_request_target') || (github.head_ref == 'trunk' && github.event.pull_request.head.repo.full_name == github.repository && github.base_ref == 'main') | ||
| if: (github.event_name != 'pull_request' && github.event_name != 'pull_request_target') || (github.head_ref == 'trunk' && github.event.pull_request.head.repo.full_name == github.repository && github.base_ref == 'main' && (github.event.action != 'labeled' || github.event.label.name == 'ci:run')) |
There was a problem hiding this comment.
P1: This skips the required Go Test check for unrelated labels, and GitHub treats skipped jobs as successful; a same-head label run can therefore mask failed promotion tests. Keep this check running on label events or prevent ignored-label runs from publishing the required check context.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At .github/workflows/ci.yml, line 31:
<comment>This skips the required `Go Test` check for unrelated labels, and GitHub treats skipped jobs as successful; a same-head label run can therefore mask failed promotion tests. Keep this check running on label events or prevent ignored-label runs from publishing the required check context.</comment>
<file context>
@@ -19,7 +28,7 @@ env:
jobs:
go-test:
- if: (github.event_name != 'pull_request' && github.event_name != 'pull_request_target') || (github.head_ref == 'trunk' && github.event.pull_request.head.repo.full_name == github.repository && github.base_ref == 'main')
+ if: (github.event_name != 'pull_request' && github.event_name != 'pull_request_target') || (github.head_ref == 'trunk' && github.event.pull_request.head.repo.full_name == github.repository && github.base_ref == 'main' && (github.event.action != 'labeled' || github.event.label.name == 'ci:run'))
name: Go Test
runs-on: ubuntu-latest
</file context>
| if: (github.event_name != 'pull_request' && github.event_name != 'pull_request_target') || (github.head_ref == 'trunk' && github.event.pull_request.head.repo.full_name == github.repository && github.base_ref == 'main' && (github.event.action != 'labeled' || github.event.label.name == 'ci:run')) | |
| if: (github.event_name != 'pull_request' && github.event_name != 'pull_request_target') || (github.head_ref == 'trunk' && github.event.pull_request.head.repo.full_name == github.repository && github.base_ref == 'main') |
There was a problem hiding this comment.
Not changing this. The merge-train sweeper already handles it, the same way cloud does since #4351: effectiveCheckRuns (cloud packages/web/lib/merge-train/plan.ts) keeps a newer skipped run of a check name only when no non-skipped run exists. So a skipped run left by an unrelated label never masks the real verdict, whether green or red. On branch protection: this repo has no required status checks, and the sweeper, not GitHub's mergeability, is the merge gate for the trunk -> main PR. GitHub has no label filter at the on: level, so these skip-only runs can't be avoided.
There was a problem hiding this comment.
All reported issues were addressed across 5 files
Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.
View guided diff | Re-trigger cubic
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit fd4c6c4. Configure here.
| jobs: | ||
| go-test: | ||
| if: (github.event_name != 'pull_request' && github.event_name != 'pull_request_target') || (github.head_ref == 'trunk' && github.event.pull_request.head.repo.full_name == github.repository && github.base_ref == 'main') | ||
| if: (github.event_name != 'pull_request' && github.event_name != 'pull_request_target') || (github.head_ref == 'trunk' && github.event.pull_request.head.repo.full_name == github.repository && github.base_ref == 'main' && (github.event.action != 'labeled' || github.event.label.name == 'ci:run')) |
There was a problem hiding this comment.
Skipped labels mask promotion checks
Medium Severity
Unrelated labeled events still start the promotion workflows and skip every job, which publishes a new skipped Go Test (and the other job names) on the same head. The sweeper uses the latest check as the CI-ran marker, so that skip can hide a real success and leave the trunk PR looking like CI never ran.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit fd4c6c4. Configure here.
There was a problem hiding this comment.
Not changing this. The merge-train sweeper already handles it, the same way cloud does since #4351: effectiveCheckRuns (cloud packages/web/lib/merge-train/plan.ts) keeps a newer skipped run of a check name only when no non-skipped run exists. So a skipped run left by an unrelated label never masks the real verdict, whether green or red. On branch protection: this repo has no required status checks, and the sweeper, not GitHub's mergeability, is the merge gate for the trunk -> main PR. GitHub has no label filter at the on: level, so these skip-only runs can't be avoided.
The ready check's per-PR concurrency group also caught label events the job skips (no `mergeable`, e.g. the sweeper adding `external`), cancelling an in-flight real run and leaving `cancelled` as the head's verdict. Those events now get a throwaway group. Pinned in the contract test; ready-check timeout raised where the build is heavy; promotion branches filter pinned where present. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
kjgbot
added
the
mergeable
1 participant
Prepares relayfile for the merge-train sweeper (AgentWorkforce/cloud
packages/web/lib/merge-train), porting cloud #4350/#4351. Same shape as relaycast#497 and relayauth#101. relayfile stays in dry-run.Changes
ci.yml,contract.yml,relayfile-evals.yml: the trunk -> main PR now runs onopened/reopened/labeledonly, neversynchronize. Every job skips label events other thanci:run.labeledevents get a throwawayignored-<run_id>group (#4351). Push runs key off the sha, so main runs no longer cancel each other.merge-train-ready.yml(Merge-train ready check): runs on feature PRs intotrunkwhilemergeableis on. It runsgo build,npm run build,npm run typecheck(TS workspaces +go vet) and the workflow contract test.scripts/merge-train-ci-workflows.test.mjs: pinsci:run, theGo Testmarker check, and the ready check.Effect on manual promotion
A human trunk -> main PR gets CI when it's opened, or when
ci:runis added by hand.Verification
node --test scripts/merge-train-ci-workflows.test.mjs: 8/8 pass. Red-first: 6 of 8 fail against the old workflows.actionlintis clean.🤖 Generated with Claude Code
Note
Medium Risk
Changes when and how CI runs for trunk promotion and merge-train gates; misconfiguration could block merges or skip required checks, but changes are workflow-only with contract tests.
Overview
Aligns GitHub Actions with the merge-train sweeper so full CI on trunk → main promotion PRs is batched instead of re-running on every push.
Promotion workflows (
ci.yml,contract.yml,relayfile-evals.yml) now listen foropened/reopened/labeledonly (notsynchronize), and jobs run on label events only when the label isci:run. Concurrency is split: PR runs group per PR (cancellable),mainpushes group per commit SHA (no cross-cancel), and unrelated label events use anignored-<run_id>group so they cannot cancel an in-flight promotion CI run.Adds
merge-train-ready.yml— a lighter Merge-train ready check on PRs intotrunkwhile themergeablelabel is present (build + typecheck + contract test). Addsscripts/merge-train-ci-workflows.test.mjsto pin shared names (ci:run,Go Testmarker job, ready check name) and gate/concurrency expressions against silent drift from the cloud merge-train config.Reviewed by Cursor Bugbot for commit 189f99d. Bugbot is set up for automated code reviews on this repo. Configure here.