Uh oh!
There was an error while loading. Please reload this page.
ci: subscribe the four gate workflows to merge_group, and move ci/lint path filtering into the jobs (#3523 steps 1-2) - #3722
Merged
Conversation
…#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
The latest updates on your projects. Learn more about Vercel for GitHub. |
This was referenced Aug 8, 2026
os-project-manager
marked this pull request as ready for review
August 8, 2026 08:38
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Aug 8, 2026
akarma-synetal pushed a commit
to akarma-synetal/objectui
that referenced
this pull request
Aug 10, 2026
…diff (objectstack-ai#3723) (objectstack-ai#3744) `ci.yml`'s `docs` job decided whether to build the site from CHANGED=$(git diff --name-only BASE...HEAD -- 'apps/site/' 'content/' 2>/dev/null || echo "") which collapses two different facts into one empty string: "the diff succeeded and nothing docs-related changed" (correctly a skip) and "the diff could not be computed at all" — a checkout that did not fetch deep enough, a transient git failure, a malformed sha. The second case skipped the entire site build and the job still reported success: no red step, no warning, nothing in the summary. objectstack#4928 named this the filter contract after the same shape produced a fully green, zero-job pull request: when the filter cannot tell, RUN. The capture is now the fail-open form the four gates PR objectstack-ai#3722 added to this workflow already use — `if ! CHANGED=$(git diff …); then should_run=true; exit 0; fi`, with the revision range quoted so a malformed sha reaches git as one argument and is rejected rather than word-split. Measured by executing this step's own `run:` script (pulled out of the parsed YAML, `${{ … }}` substituted) against a fixture repository: unreachable base sha `should_run=false` -> `true`; docs changed `true` -> `true`; only code changed `false` -> `false`. Which pull requests pay for a site build is unchanged. Two things found while measuring, both now written into the step: - Failing open does NOT cover an empty revision range. With no `github.event.pull_request` payload the range interpolates to a bare `...`, which git reads as `HEAD...HEAD` and exits 0 with no output — so the `!= 'pull_request'` early return objectstack-ai#3722 added is what covers the queue and push lanes, not the diff. - `2>/dev/null` is dropped as well as `|| echo ""`. It hid git's own explanation of the failure from the run log, which is the only diagnostic a reader of a skipped build gets. `merge-queue-reporting.test.ts`'s fail-open assertion was a single whole-file `toMatch` per workflow, so ci.yml's three fail-open captures satisfied it on the `docs` gate's behalf — the file was green with the defect in it (measured: 10/10 on the pre-fix tree). It is now per CAPTURE: every `CHANGED=$(git diff …)` in `ci.yml` and `lint.yml` must open as `if ! …`, with a per-file floor so deleting a gate's capture cannot make the check vacuously green, plus a second test rejecting `|| echo ""` and `2>/dev/null` anywhere in the gates — `if !` wrapped around a swallow is fail-closed with the safe spelling around it. No changeset: CI configuration only, no published package changes, in line with objectstack-ai#3722 and the three `ci.yml` commits before it. Claude-Session: https://claude.ai/code/session_01GTRjn8xBqp75dk7kFupVRt Co-authored-by: Claude <noreply@anthropic.com>
akarma-synetal pushed a commit
to akarma-synetal/objectui
that referenced
this pull request
Aug 10, 2026
…objectstack-ai#3387) (objectstack-ai#3769) objectstack#4731 / #4843 把「哪些前端改动发版」的判据统一成**读本仓声明的 `.changeset/*.md`**,而这个判据赖以成立的前提——改了发版包源码就必须带一个 changeset——此前由任何门禁保证。实测的后果:`19716b5bf` fix(charts)、 `5e7ef1141` fix(i18n)、`0e50440`(objectstack-ai#3518,26 个文件跨五个包加十个语言包) 都改了已发布包的源码、都是用户可见修复、都没有 changeset,于是搭着下一次发版 出去,在 CHANGELOG、版本号、平台发布记录里一处都查不到。 新增正向触发门禁 `.github/workflows/changeset-presence.yml` (`scripts/check-changeset-presence.mjs`):改动落在发版包的 `src/` 上时, 本次改动必须**新增**一个 `.changeset/*.md`。 ⛔ 没有加宽 `changeset-guard.yml` 的 paths。它的 `paths: ['.changeset/**']` 是刻意的反向触发,并写在自己的文件头里:`ci.yml`/`lint.yml` 都把 `.changeset/**` 列进 `paths-ignore`,只加 changeset 的 PR 不会启动任何别的 workflow,那个 guard 就是为看见这种 PR 而存在的。而**忘了写 changeset 的 PR 按定义不碰 `.changeset/**`**——唯一能发现它的检查恰好是唯一不会跑的检查。 加宽会毁掉它原本要服务的场景,所以两个门禁并存、方向相反:一个管已有声明的 级别,一个管声明是否存在。 几处判断,连同得出它的测量: - **空 frontmatter 是一等通过写法**,不是变通。要的是「声明一次」,不是强制 发版;纯内部改动/只动测试写 `---` 紧跟 `---` 加一句理由即可,理由就留在仓 库里。因此也**没有**为 `src/` 下的测试文件开豁免口子——教门禁认哪些文件 「不算」正是漏洞的藏身处;顺带一个实测反例:`f1310e40f` 是 `test(...)` 前 缀却同时改了非测试源码,提交信息的前缀并不可信,文件清单才可信。 - **守护面是推导出来的,不是写死的 glob。** issue 提的字面 glob 只覆盖 `packages/` 下一层,而 `@object-ui/console` 在 `apps/console`——本仓最常改 的已发布包,也正是平台 `bump-objectui.sh` 替它写 changeset 的那个包——会被 整整漏掉。改为读 `.changeset/config.json` 的 `fixed` 组:发版覆盖谁,门禁 就守谁,`ignore` 的(`@object-ui/site`、examples)不守。今天推导出 40 个包 目录。既不在 `fixed` 也不在 `ignore` 的包,其源码改动**响亮失败**而不是被 当成「不发版」,分类本身由 `check-changeset-fixed.mjs` 负责。 - **触发器上不加任何 path 过滤。** trigger 上的 `paths` 会跳过整个 workflow (GitHub 没有 per-job path filter),于是不匹配的 PR 根本不会**创建**这个 check;而一个从不上报的必需 check 不会让 PR 失败,只会让它永远 pending, 在合并队列里则要等 ruleset 的 60 分钟超时——这正是 objectstack-ai#3523 的后半段。所以本 门禁在每个 PR 上都上报、由脚本读 diff 决定,并因此**可以**被设为必需,同时 订阅 `merge_group`(`merge-queue-reporting.test.ts` 的名单加了这一条)。 过滤器还会成为脚本守护面的第二份副本,和它自由漂移。 - **push 到 main 不订阅**:改动已经落地,没有还能写的声明,失败只会把 main 染红在下一位提交者头上。`workflow_dispatch` 也不订阅:手动跑没有可判的 revision range,而本门禁宁可响亮失败也不肯自己编一个。 - **每一项缺失输入都响亮失败**(#4690 / objectstack#4928):base 解析不出、 `git diff` 报错、`.changeset/` 目录不存在、包未分类,全部红。方向和 `ci.yml` 里的过滤门禁**相反**:那些决定要不要跑活,「判断不了」就跑;这里 判断本身就是活,「判断不了」就失败。两者都拒绝在什么都没看的情况下报绿。 自身写测过程中被自己的测试抓出一个真实缺陷并修掉:`resolveBaseRef` 原先把 显式 `--base` 只当作候选链的第一环,于是一个在本地 clone 里不存在的 sha 会 静默跌落到 `merge-base with main`,拿**另一个**提交做比较并打印自信的绿灯 (实测 exit 0;修好后 exit 1)。「你指的 base 不存在」和「你没指 base」是 两件不同的事,只有后者可以靠猜回答。(同族的 `check-i18n-en-drift.mjs` 仍是跌落写法,已另开单,本 PR 不动。) 反向验证(先预判方向再跑):去掉 `--diff-filter=A` → 「编辑他人待发 changeset」用例转红(1 failed / 30 passed);恢复上述 base 跌落 → 显式 base 用例转红且 exit 0→1;把 `git diff` 失败吞成空列表 → diff 失败用例转红。第一 项的预判**错了一次并已改正**:原先声称覆盖该 filter 的 "pending" 用例在去掉 filter 后依然全绿——早提交的 changeset 本就落在 diff range 之外,那条用例钉 的是 range 而非 filter。补了真正触达 filter 的两个 fixture,其中「删除待发 changeset」经测量由两道独立防线各自挡住,注释按实测改写。 三处文档会因本门禁变成假话,一并修正:`ci-cd-pipeline.md` 里 「Nothing in CI requires a pull request to add a changeset」(该页被 `ci-cd-pipeline-doc.test.ts` 双向钉住,新 workflow 本就必须在此建节)、 `CONTRIBUTING.md` 的「DON'T create a changeset for ... apps / 测试改动」、 以及 AGENTS.md 那句「纯 bug 修复不需要」——正是这条旧判据放走了上面三条修复。 这页自己的教训就是:一个把 CI 实际强制内容说错的文档比没有文档更糟。 无 changeset:CI 配置 + 仓库级脚本 + 文档,不改任何已发布包源码,与 objectstack-ai#3722 / objectstack-ai#3744 同例。本 PR 也是自指的冒烟测试——门禁在自己的改动上判为「不欠 changeset」 并通过(实测 7 个文件,0 个落在守护面内)。 Fixesobjectstack-ai#3387 Claude-Session: https://claude.ai/code/session_01GTRjn8xBqp75dk7kFupVRt Co-authored-by: Claude <noreply@anthropic.com>
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
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
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/mainat 0cf8f0f:merge_groupsubscriptions repo-widegit grep merge_group origin/main→ no match anywhere in the tree,.github/included. Actions API,event=merge_group, repo-wide:{"total_count": 0}.paths-ignorehides these contexts from a docs-only PRROADMAP.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 ofLint/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_groupruns; objectui's had 0.Commit 1 — step 1: subscribe the trigger
merge_group: types: [checks_requested]onci.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.ymljobs additionally had to be told a third event exists. Both are no-ops forpull_requestandpush:testmoved fromif: github.event_name == 'pull_request'to!= 'push'. Left alone, the queue build — the last validation beforemain— would have been the only one skipping every shard.docs'sshould_runtreated "not push" as "is a pull request" and diffedgithub.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-ignoreskips 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) andlint(lint.yml) each open with aDecide whether this change needs a full runstep, and every step after it carriesif: 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 shapeci.yml'sdocsjob has used since #3450 — no new mechanism — and the exclusion lists are byte-for-byte thepaths-ignorethey 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_requestedis the only activity type GitHub defines formerge_group, and objectstack's baremerge_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 inci.ymlrecords that objectstack spells it the other way and why the two are the same today.2. Does
Build Docs'sshould_runpattern transfer? Yes, with one correction that matters. It needsfetch-depth: 0(added totest,e2e,lint;type-checkalready had it), and itspush/else split does not survive contact withmerge_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 yieldsshould_run=true; rewritten with thedocsjob's spelling, the same input yieldsshould_run=false— a silent full skip that reports green. The pre-existingdocsgate is reported separately rather than changed here.3. Can
concurrencycollide between a queue build and a PR build? No, and this was measured rather than reasoned. Onmerge_groupthegithub.event.pull_requesthalf of the group expression is null, so the group falls back togithub.ref, which on a queue build is the queue's own generation — objectstack's queue runs reporthead_branchasgh-readonly-queue/main/pr-6594-251e888ac9ace8226f3a8450951e5b40a0a84c2c. That can equal neither a PR group (a bare number, orrefs/pull/N/mergeinlint.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 onpull_request, never onmerge_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: ifLint/Type Checkare 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 declaremerge_group; and the entire path-decision logic, executed rather than eyeballed. Therun: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:content/**,*.md)apps/site/**onlypackages/**)scripts/__tests__(this PR)merge_grouppushReverse verification
scripts/__tests__/merge-queue-reporting.test.tswas 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 — nomerge_groupsubscription,testrestricted topull_request,paths-ignoreon bothpull_requesttriggers, no in-job exclusion list, noid: relevantgate, 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
paths-ignorestays on thepushtrigger. Branch protection and the merge queue judge pull requests and queue builds, never pushes tomain, 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.merge_group, everything runs. Queue builds do not short-circuit on paths. objectstack'sci.ymlstates the reason for the same decision — the queue build is the last validation beforemain, "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-checkis left ungated on purpose: it is a checkout and onenodecall, 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, bothTriggers:paragraphs, theWhencolumn fortype-check/test/e2e, and the three sections whose stated rationale quoted the oldpaths-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 theon:block. The list now also lives inside the job, so it reads both — otherwise it would have gone on passing while a**/*.tsexclusion 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 noskip-changesetlabel (get_labelreturns not-found), so there is nothing to skip and no label to apply..github/WORKFLOWS.mdclaims 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