Skip to content

ci: subscribe the four gate workflows to merge_group, and move ci/lint path filtering into the jobs (#3523 steps 1-2) - #3722

Merged
os-project-manager merged 2 commits into
mainfrom
claude/issue-3523-merge-group-triggers
Aug 8, 2026
Merged

ci: subscribe the four gate workflows to merge_group, and move ci/lint path filtering into the jobs (#3523 steps 1-2)#3722
os-project-manager merged 2 commits into
mainfrom
claude/issue-3523-merge-group-triggers

Conversation

@os-project-manager

Copy link
Copy Markdown
Collaborator

Fixes #3523

Steps 1 and 2 of the card. Step 3 — writing context names into the branch-protection and merge-queue required sets — is a repository-settings change and is deliberately not in this PR.

The premise, re-measured before implementing

Both load-bearing claims still held on origin/main at 0cf8f0f:

Claim Re-measurement
Zero merge_group subscriptions repo-wide git grep merge_group origin/main → no match anywhere in the tree, .github/ included. Actions API, event=merge_group, repo-wide: {"total_count": 0}.
paths-ignore hides these contexts from a docs-only PR PR #3714 (docs-only, ROADMAP.md) carries exactly 6 check runs: Close issues referenced in other repositories, Vercel Preview Comments, dependabot, label, Control Byte Scan, Internal Docs Link Check. Not one of Lint / Type Check / Test (shard N/4) / Build & E2E / Changeset Fixed Group Check.

The cross-repo asymmetry the card describes also still holds: objectstack's queue has 3552 merge_group runs; objectui's had 0.

Commit 1 — step 1: subscribe the trigger

merge_group: types: [checks_requested] on ci.yml, lint.yml, control-bytes.yml, docs-links.yml — the four the card names, and no others. Pure addition: nothing about pull-request or push behaviour changes.

Two ci.yml jobs additionally had to be told a third event exists. Both are no-ops for pull_request and push:

  • test moved from if: github.event_name == 'pull_request' to != 'push'. Left alone, the queue build — the last validation before main — would have been the only one skipping every shard.
  • docs's should_run treated "not push" as "is a pull request" and diffed github.event.pull_request.*, which is null on a queue build: an empty revision range, i.e. the site build silently skipped.

Commit 2 — step 2: make the contexts report on every PR

on.pull_request.paths-ignore skips the whole workflow when every changed file matches, and GitHub has no per-job path filter. A check that is never created does not fail a required-status-check rule — it leaves the PR pending, and inside the queue it fails on the ruleset's 60-minute status-check timeout. That is why the queue's required set was empty in the first place, and it is the half of the P0 that a trigger alone does not fix.

The filter is not deleted, it moves. type-check, test, e2e (ci.yml) and lint (lint.yml) each open with a Decide whether this change needs a full run step, and every step after it carries if: steps.relevant.outputs.should_run == 'true'. The job always runs and always reports; the paths decide only whether it does any work. That is the shape ci.yml's docs job has used since #3450 — no new mechanism — and the exclusion lists are byte-for-byte the paths-ignore they replace, so which PRs pay for a full run is exactly as before.

The three PM assumptions, checked

1. Is types: [checks_requested] the right spelling? Both spellings are legal and equivalent today: checks_requested is the only activity type GitHub defines for merge_group, and objectstack's bare merge_group: demonstrably produces queue builds (3552 of them). I chose the explicit form so that a second activity type added later cannot silently start queue builds these workflows were never written for. The comment in ci.yml records that objectstack spells it the other way and why the two are the same today.

2. Does Build Docs's should_run pattern transfer? Yes, with one correction that matters. It needs fetch-depth: 0 (added to test, e2e, lint; type-check already had it), and its push/else split does not survive contact with merge_group — see commit 1. It also fails closed: 2>/dev/null || echo "" turns an unresolvable diff into "nothing relevant changed". The new gates fail open instead (objectstack#4928's filter contract). Measured both ways against a fixture repository: as shipped, an unreachable base sha yields should_run=true; rewritten with the docs job's spelling, the same input yields should_run=false — a silent full skip that reports green. The pre-existing docs gate is reported separately rather than changed here.

3. Can concurrency collide between a queue build and a PR build? No, and this was measured rather than reasoned. On merge_group the github.event.pull_request half of the group expression is null, so the group falls back to github.ref, which on a queue build is the queue's own generation — objectstack's queue runs report head_branch as gh-readonly-queue/main/pr-6594-251e888ac9ace8226f3a8450951e5b40a0a84c2c. That can equal neither a PR group (a bare number, or refs/pull/N/merge in lint.yml's case) nor a push group (refs/heads/main). No change was needed.

What I could verify, and what I could not

Could not: that a check run actually appears on a gh-readonly-queue/** generation in this repository. That requires enqueueing a PR, which means merging it, and I may not merge. This PR's own CI runs on pull_request, never on merge_group, so the trigger's first real exercise is the first PR that enters the queue after this lands. Please treat that as the acceptance test for step 1 — and note the card's warning still applies: if Lint / Type Check are currently in the required set, any PR entering the queue will hang for 60 minutes and fail, until this lands.

Could: the equivalent construct producing queue check runs on objectstack, with the gh-readonly-queue/** head branch quoted above; that the YAML parses and that exactly these four workflows declare merge_group; and the entire path-decision logic, executed rather than eyeballed. The run: script was extracted from the shipped YAML, its ${{ }} expressions substituted the way GitHub substitutes them, and run against a fixture repository — 20 cases, all passing:

Case ci.yml lint.yml
docs-only (content/**, *.md) skip skip
root markdown only skip skip
changeset-only skip skip
apps/site/** only skip run (not on lint's list — the pre-existing asymmetry, preserved)
code (packages/**) run run
docs + code run run
workflows + scripts/__tests__ (this PR) run run
unresolvable base sha run (fail-open) run (fail-open)
merge_group run run
push run run

Reverse verification

scripts/__tests__/merge-queue-reporting.test.ts was run against the pre-change workflows (git checkout origin/main -- .github/workflows/). Predicted direction: red, one failure per regression. Observed: 9 of 10 assertions fail, each naming its own — no merge_group subscription, test restricted to pull_request, paths-ignore on both pull_request triggers, no in-job exclusion list, no id: relevant gate, no fail-open guard. The one that stays green is the honesty check on the workflow-name list, which is independent of the fix by design.

Two deliberate narrowings — arguable, so stated plainly

  1. paths-ignore stays on the push trigger. Branch protection and the merge queue judge pull requests and queue builds, never pushes to main, so the push lane gains nothing from losing it and would cost a full CI run on every docs merge. It also leaves the ignore list one authored home, which the new pin test then holds the in-job copies against. If you want the push lane unfiltered too, say so and it is a two-line follow-up.
  2. On merge_group, everything runs. Queue builds do not short-circuit on paths. objectstack's ci.yml states the reason for the same decision — the queue build is the last validation before main, "the one place a skipped job can never be the right answer" — and a docs-only PR whose queue build validated nothing would rebuild this card's failure mode one level down. The cost is a full CI run per docs PR at merge time only.

changeset-check is left ungated on purpose: it is a checkout and one node call, so short-circuiting it would cost more complexity than it saves. It now reports on every PR, which it previously did not.

Docs and pins

  • content/docs/guide/ci-cd-pipeline.md: inventory rows, both Triggers: paragraphs, the When column for type-check / test / e2e, and the three sections whose stated rationale quoted the old paths-ignore (control-bytes.yml, docs-links.yml, changeset-guard.yml) are corrected rather than left describing the previous regime. A new Merge Queue section writes down the enforced queue, the three-step ordering and the deadlock, and the four contexts that can never be required structurally.
  • scripts/__tests__/merge-queue-reporting.test.ts (new) pins both steps.
  • scripts/__tests__/lint-workflow.test.ts: its TypeScript-exclusion tripwire read only the on: block. The list now also lives inside the job, so it reads both — otherwise it would have gone on passing while a **/*.ts exclusion was added to the gate.

No changeset

Nothing under packages/** changes, so no package version moves. This repository has no changeset-required gate in .github/workflows/ and no skip-changeset label (get_label returns not-found), so there is nothing to skip and no label to apply. .github/WORKFLOWS.md claims otherwise; that document is stale in five separate ways and is filed separately rather than fixed here.

Verification run

Gates enumerated from the workflow files themselves, run one by one, all green:

check-control-bytes.mjs · check-doc-links.mjs · check-changeset-fixed.mjs · check-changeset-no-major.mjs · check-lint-coverage.mjs (45/45) · check-type-check-coverage.mjs · check:spec-symbols · check:i18n-keys · check:i18n-drift · type-check:scripts · turbo run type-check (78/78) · type-check:vitest-setup · turbo run lint (45/45, 0 errors) · vitest run (1023 files, 12044 passed, 1 skipped).

Not run locally: Build & E2E (Playwright browsers) and the site build — neither can be reached by this diff, and both run on this PR.


Generated by Claude Code

claude added 2 commits August 8, 2026 07:36
…#3523)

Step 1 of objectui#3523, on its own commit as the issue's ruling requires:
this is a pure addition and changes nothing about which pull requests or
pushes run CI.

The merge queue is enforced on this repository by a ruleset (#3243 measured a
direct push to `main` returning 405 `Changes must be made through the merge
queue`), but not one of the 22 workflows subscribed `merge_group` — repo-wide
`event=merge_group` runs stood at total_count = 0. A queue nothing subscribes
to can only carry an empty required-check set, so it rebuilt each PR on the
current `main` and let it through without validating anything. On 2026-08-07
that was cashed in: #3503 / #3510 / #3516 merged between 02:11Z and 02:15Z
with `Type Check` at conclusion=failure, on a `main` poisoned by #3498, and
#3505 had to hot-fix it.

The four workflows named by the issue now carry the trigger. It is spelled
`merge_group: types: [checks_requested]`; `checks_requested` is the only
activity type GitHub defines for this event today, so this is equivalent to
objectstack's bare `merge_group:` and merely refuses to inherit a second type
added later.

Two `ci.yml` jobs additionally needed to be told the third event exists —
both changes are no-ops for `pull_request` and `push`:

- `test` moves from `if: github.event_name == 'pull_request'` to
  `!= 'push'`, so a queue build actually runs the suite instead of skipping
  every shard.
- `docs`'s `should_run` treated anything that is not `push` as a pull request
  and diffed `github.event.pull_request.*`, which is null on a queue build —
  an empty revision range, i.e. the site build silently skipped on the last
  check before `main`.

Verified rather than assumed: `concurrency` needs no merge-queue case. On
`merge_group` the group expression falls back to `github.ref`, which is the
queue's own generation — measured on objectstack, whose 3552 queue runs report
head_branch `gh-readonly-queue/main/pr-6594-251e888a…`. That can collide with
neither a PR group (a bare number) nor a push group (`refs/heads/main`).

Refs #3523
…e jobs (#3523)

Step 2 of objectui#3523, deliberately a separate commit from step 1: step 1
adds runs that did not exist, this one changes which pull requests start CI,
and mixing them would make that impossible to review apart.

`on.pull_request.paths-ignore` skips the WHOLE workflow when every changed
file matches, and GitHub has no per-job path filter. A docs-only or
changeset-only PR therefore started neither `ci.yml` nor `lint.yml` — #3509
measured zero check runs from them. A check that is never *created* does not
fail a required-status-check rule, it leaves the pull request pending; inside
the merge queue it fails on the ruleset's 60-minute status-check timeout. So
none of `Lint`, `Type Check`, `Test (shard N/4)`, `Build & E2E` or
`Changeset Fixed Group Check` could be made required while the filter lived
on the trigger — which is why the queue's required set was empty to begin
with.

The filter is not deleted, it moves. `type-check`, `test`, `e2e` (ci.yml) and
`lint` (lint.yml) each open with a `Decide whether this change needs a full
run` step, and every step after it carries
`if: steps.relevant.outputs.should_run == 'true'`. The job always runs and
always reports; the paths decide only whether it does any work. That is the
shape `ci.yml`'s `docs` job has used since #3450 — not a new mechanism — and
the exclusion lists are byte-for-byte the `paths-ignore` they replace, so
which PRs pay for a full run is exactly as before.

Two deliberate narrowings, both stated so they can be argued with:

- `paths-ignore` stays on the `push` trigger. Branch protection and the merge
  queue judge pull requests and queue builds, never pushes to `main`, so the
  push lane gains nothing from losing it and would cost a full CI run on
  every docs merge. It also gives the ignore list one authored home, which
  `merge-queue-reporting.test.ts` then pins the in-job copies against.
- `changeset-check` is not gated. It is a checkout and one `node` call, so
  short-circuiting it would cost more in complexity than it saves.

The gate fails OPEN: an unresolvable diff runs everything rather than
reporting green having built nothing (objectstack#4928's filter contract).
Measured both ways against a fixture repository — as shipped an unreachable
base sha yields should_run=true; rewritten with the `|| echo ""` spelling
`ci.yml`'s older `docs` gate still uses, the same input yields
should_run=false, i.e. a silent full skip. That pre-existing `docs` gate is
reported separately rather than changed here.

`scripts/__tests__/merge-queue-reporting.test.ts` pins both steps; against
the pre-change workflows 9 of its 10 assertions go red, each naming its own
regression. `lint-workflow.test.ts`'s TypeScript-exclusion tripwire now reads
the in-job list as well as the trigger, which is where that list now lives.

Refs #3523
@vercel

vercel Bot commented Aug 8, 2026

Copy link
Copy Markdown

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

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
objectui Ignored Ignored Aug 8, 2026 8:21am

Request Review

@os-project-manager
os-project-manager marked this pull request as ready for review August 8, 2026 08:38
@os-project-manager
os-project-manager added this pull request to the merge queue Aug 8, 2026
Merged via the queue into main with commit f710fc4 Aug 8, 2026
17 checks passed
@os-project-manager
os-project-manager deleted the claude/issue-3523-merge-group-triggers branch August 8, 2026 08:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

2 participants