Repository navigation
ci: batch trunk-PR CI on the merge train's ci:run label #546
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -2,13 +2,22 @@ name: CI | |||||
|
|
||||||
| on: | ||||||
| pull_request: | ||||||
| # Trunk -> main promotion PR only, batched by the merge train (AgentWorkforce/cloud | ||||||
| # packages/web/lib/merge-train): opened, reopened, or the sweeper's `ci:run` | ||||||
| # label once per new trunk head. Never on synchronize; other label events | ||||||
| # skip every job. | ||||||
| types: [opened, reopened, labeled] | ||||||
| push: | ||||||
| branches: | ||||||
| - main | ||||||
|
|
||||||
| concurrency: | ||||||
| # PR runs collapse per pull request; push runs key off the commit sha so two | ||||||
| # merges landing together cannot cancel each other. A `labeled` event for any | ||||||
| # label other than `ci:run` runs nothing, so it gets its own throwaway group: | ||||||
| # it must never cancel the PR's in-progress run. | ||||||
| group: ci-${{ github.event_name != 'pull_request' && format('push-{0}', github.sha) || ((github.event.action == 'labeled' && github.event.label.name != 'ci:run') && format('ignored-{0}', github.run_id) || format('pr-{0}', github.event.pull_request.number)) }} | ||||||
| cancel-in-progress: ${{ github.event_name == 'pull_request' }} | ||||||
|
|
||||||
| permissions: | ||||||
| contents: read | ||||||
|
|
@@ -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' && (github.event.action != 'labeled' || github.event.label.name == 'ci:run')) | ||||||
|
There was a problem hiding this comment. P1: This skips the required Prompt for AI agents
Suggested change
Contributor
Author
There was a problem hiding this comment. Not changing this. The merge-train sweeper already handles it, the same way cloud does since #4351: There was a problem hiding this comment. Skipped labels mask promotion checksMedium Severity Unrelated Additional Locations (2)  Reviewed by Cursor Bugbot for commit fd4c6c4. Configure here.
Contributor
Author
There was a problem hiding this comment. Not changing this. The merge-train sweeper already handles it, the same way cloud does since #4351: |
||||||
| name: Go Test | ||||||
| runs-on: ubuntu-latest | ||||||
| timeout-minutes: 15 | ||||||
|
|
@@ -37,7 +46,7 @@ jobs: | |||||
| run: go test ./... | ||||||
|
|
||||||
| go-build: | ||||||
| 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 Build | ||||||
| runs-on: ubuntu-latest | ||||||
| needs: go-test | ||||||
|
|
@@ -68,7 +77,7 @@ jobs: | |||||
| if-no-files-found: error | ||||||
|
|
||||||
| sdk-typecheck: | ||||||
| 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: SDK Typecheck | ||||||
| runs-on: ubuntu-latest | ||||||
| timeout-minutes: 15 | ||||||
|
|
@@ -111,7 +120,7 @@ jobs: | |||||
| run: npm run test:bundle:bun --workspace=packages/sdk/typescript | ||||||
|
|
||||||
| client-typecheck: | ||||||
| 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: Client Typecheck | ||||||
| runs-on: ubuntu-latest | ||||||
| timeout-minutes: 15 | ||||||
|
|
@@ -144,7 +153,7 @@ jobs: | |||||
| run: npm run test --workspace=@relayfile/client | ||||||
|
|
||||||
| e2e: | ||||||
| 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: E2E | ||||||
| runs-on: ubuntu-latest | ||||||
| needs: go-build | ||||||
|
|
@@ -184,7 +193,7 @@ jobs: | |||||
| npx tsx scripts/e2e.ts --ci | ||||||
|
|
||||||
| backend-conformance: | ||||||
| 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: Backend Conformance (Go oracle) | ||||||
| runs-on: ubuntu-latest | ||||||
| timeout-minutes: 25 | ||||||
|
|
@@ -225,7 +234,7 @@ jobs: | |||||
| retention-days: 14 | ||||||
|
|
||||||
| release-tooling: | ||||||
| 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: Release Tooling | ||||||
| runs-on: ubuntu-latest | ||||||
| timeout-minutes: 10 | ||||||
|
|
@@ -249,7 +258,7 @@ jobs: | |||||
| run: npm run test:release | ||||||
|
|
||||||
| workers-typecheck: | ||||||
| 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: Workers Typecheck | ||||||
| runs-on: ubuntu-latest | ||||||
| timeout-minutes: 15 | ||||||
|
|
||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| name: Merge-train ready check | ||
|
|
||
| # Lightweight gate for feature PRs into `trunk` (full CI runs only on the | ||
| # trunk -> main promotion PR). It runs once the author adds the `mergeable` | ||
| # label, and again on every push while the label is on. The merge-train sweeper | ||
| # (AgentWorkforce/cloud packages/web/lib/merge-train) merges a feature PR into | ||
| # trunk only when this check is green on the PR's CURRENT head. | ||
| # | ||
| # Fork PRs run it too. `pull_request` gives a fork no secrets and a read-only | ||
| # token, and nothing here uses a secret or persists credentials. Never switch | ||
| # this to `pull_request_target`. Trust is the sweeper's job: it merges an | ||
| # external PR only after a maintainer's APPROVED review on its exact head. | ||
| # | ||
| # A labelled PR with no run for its head (labelled before this workflow | ||
| # existed, or retargeted onto trunk, which emits `edited`) is kicked by the | ||
| # sweeper re-adding the `ready:check` label: any label event runs this job while | ||
| # `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: Prompt for AI agents
Contributor
Author
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 |
||
| branches: [trunk] | ||
| types: [labeled, synchronize, reopened] | ||
|
|
||
| concurrency: | ||
| # One run per PR: a newer push or label event supersedes the in-flight run. | ||
| # An event the job will skip (`mergeable` absent) gets a throwaway group, so | ||
| # it can never cancel a real run and leave `cancelled` as the head's verdict. | ||
| group: merge-train-ready-${{ contains(github.event.pull_request.labels.*.name, 'mergeable') && format('pr-{0}', github.event.pull_request.number) || format('ignored-{0}', github.run_id) }} | ||
| cancel-in-progress: true | ||
|
|
||
| permissions: {} | ||
|
|
||
| jobs: | ||
| ready: | ||
| # The sweeper reads this exact check name (cloud lib/merge-train/config.ts). | ||
| name: Merge-train ready check | ||
| # Any label event re-runs it while `mergeable` is present (not just the | ||
| # `mergeable` event): a skipped run would otherwise become the latest check | ||
| # for the head and mask the real verdict. | ||
| if: >- | ||
| github.base_ref == 'trunk' && | ||
| contains(github.event.pull_request.labels.*.name, 'mergeable') | ||
| runs-on: ubuntu-latest | ||
| timeout-minutes: 25 | ||
| permissions: | ||
| contents: read | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| with: | ||
| persist-credentials: false | ||
| - uses: actions/setup-go@v5 | ||
| with: | ||
| go-version-file: go.mod | ||
| cache: true | ||
| - name: Go build | ||
| run: go build ./... | ||
| - uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: "22" | ||
| cache: npm | ||
| - run: npm ci | ||
| - name: Build | ||
| run: npm run build | ||
| - name: Typecheck (TS workspaces + go vet) | ||
| run: npm run typecheck | ||
| - name: Merge-train workflow contract | ||
| run: node --test scripts/merge-train-ci-workflows.test.mjs | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,84 @@ | ||
| import assert from "node:assert/strict"; | ||
| import { readFileSync } from "node:fs"; | ||
| import path from "node:path"; | ||
| import { fileURLToPath } from "node:url"; | ||
| import { test } from "node:test"; | ||
| import { parse } from "yaml"; | ||
|
|
||
| /** | ||
| * The merge-train sweeper (AgentWorkforce/cloud packages/web/lib/merge-train) | ||
| * and these workflows share three names: the label that runs promotion CI | ||
| * (`ci:run`), the check every promotion CI run creates (the sweeper's "CI ran | ||
| * on this head" marker), and the feature ready check (`Merge-train ready | ||
| * check`, kicked by `ready:check`). A rename on either side silently breaks | ||
| * the gate, so they are pinned here. | ||
| */ | ||
| const ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); | ||
| const load = (file) => parse(readFileSync(path.join(ROOT, ".github/workflows", file), "utf8")); | ||
| /** The complete trunk-PR + ci:run gate. Asserted exactly: a substring check would | ||
| * still pass if a job appended an `|| ...` bypass. */ | ||
| const TRUNK_CI_GATE = | ||
| "(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'))"; | ||
| const IGNORED_GROUP = | ||
| "(github.event.action == 'labeled' && github.event.label.name != 'ci:run') && format('ignored-{0}', github.run_id)"; | ||
|
|
||
| /** Workflows that run on the trunk -> main promotion PR. */ | ||
| const PROMOTION_WORKFLOWS = ["ci.yml", "contract.yml", "relayfile-evals.yml"]; | ||
| /** Workflows with a concurrency group a stray label event could otherwise cancel. */ | ||
| const CONCURRENCY_WORKFLOWS = ["ci.yml", "contract.yml", "relayfile-evals.yml"]; | ||
| const MARKER = { file: "ci.yml", job: "go-test", name: "Go Test" }; | ||
|
|
||
| for (const file of PROMOTION_WORKFLOWS) { | ||
| test(`${file} runs the trunk PR only on opened/reopened/ci:run, never on synchronize`, () => { | ||
| const workflow = load(file); | ||
| assert.deepEqual(workflow.on.pull_request?.types, ["opened", "reopened", "labeled"]); | ||
| // Trunk -> main PRs only (where a branches filter exists, it must name the default branch). | ||
| if (workflow.on.pull_request?.branches) assert.deepEqual(workflow.on.pull_request.branches, ["main"]); | ||
| const jobs = Object.entries(workflow.jobs); | ||
| assert.ok(jobs.length > 0); | ||
| for (const [id, job] of jobs) { | ||
| // A job with `needs` and no `if` is skipped whenever its gated parent skips. | ||
| if (!job.if && job.needs) continue; | ||
| assert.equal(job.if, TRUNK_CI_GATE, `${file} ${id}: exact trunk PR + ci:run gate`); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| for (const file of CONCURRENCY_WORKFLOWS) { | ||
| test(`${file}: an unrelated label event can never cancel a real promotion CI run`, () => { | ||
| const { concurrency } = load(file); | ||
| assert.ok(String(concurrency.group).includes(IGNORED_GROUP), `${file}: throwaway group for ignored labels`); | ||
| assert.equal(concurrency["cancel-in-progress"], "${{ github.event_name == 'pull_request' }}"); | ||
| }); | ||
| } | ||
|
|
||
| test("the sweeper's CI marker job has no needs, so every real promotion run creates it", () => { | ||
| const job = load(MARKER.file).jobs[MARKER.job]; | ||
| assert.ok(job, "marker job present"); | ||
| assert.equal(job.name ?? MARKER.job, MARKER.name); | ||
| assert.equal(job.needs, undefined); | ||
| }); | ||
|
|
||
| test("feature PRs into trunk get the ready check the sweeper requires", () => { | ||
| const ready = load("merge-train-ready.yml"); | ||
| assert.deepEqual(ready.on.pull_request?.branches, ["trunk"]); | ||
| assert.deepEqual(ready.on.pull_request?.types, ["labeled", "synchronize", "reopened"]); | ||
| const jobs = Object.values(ready.jobs); | ||
| assert.equal(jobs.length, 1); | ||
| const [job] = jobs; | ||
| assert.equal(job.name, "Merge-train ready check"); | ||
| // A skip-bound event (no `mergeable`) must not cancel a real ready-check run. | ||
| assert.equal( | ||
| ready.concurrency.group, | ||
| "merge-train-ready-${{ contains(github.event.pull_request.labels.*.name, 'mergeable') && format('pr-{0}', github.event.pull_request.number) || format('ignored-{0}', github.run_id) }}", | ||
| ); | ||
| // Fork PRs get the check too (no secrets under `pull_request`); trust is the sweeper's gate. | ||
| assert.doesNotMatch(job.if, /head\.repo\.full_name/); | ||
| assert.deepEqual(Object.keys(ready.on), ["pull_request"]); | ||
| const raw = readFileSync(path.join(ROOT, ".github/workflows/merge-train-ready.yml"), "utf8"); | ||
| assert.doesNotMatch(raw, /secrets\./); | ||
| assert.equal(job.steps[0].with?.["persist-credentials"], false); | ||
| assert.match(job.if, /contains\(github\.event\.pull_request\.labels\.\*\.name, 'mergeable'\)/); | ||
| // The sweeper kicks missing runs with `ready:check`: the job runs on ANY label event. | ||
| assert.doesNotMatch(job.if, /event\.label\.name/); | ||
| }); |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔴 Unrelated labels mask failed promotion checks
When any label other than
ci:runis added,go-testskips 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_requestlabeled event creates a new workflow run for the existing PR head. This gate skips Go Test when the added label is notci: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
documentationlabel. 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not changing this. The merge-train sweeper already handles it, the same way cloud does since #4351:
effectiveCheckRuns(cloudpackages/web/lib/merge-train/plan.ts) keeps a newer skipped run of a check name only when no non-skipped run exists. So a skippedci:runmarker 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.