Skip to content

Commit 23d1303

Browse files
ci(ui): start the Test UI shards when a package ui consumes changes (#28501)
<!-- claude-projects-attribution: {"github_login":"os-zhuang"} --> _Requested by **Sam** · [project thread](https://claude.ai/code/project/chan_01LjZwNRFiG2fkp8SNf6rHNx?thread=cmsg_01LjZwNRFiG2fkp8SNf6rHNxHHSCmqGnGSksQmHYvbECcR)_ domain:devx#1 · os-zhuang · session_01QUs3BvP5Aq9USg5GtmgAAR Fixes #28220 Ruling-ref: 6545651687 (letter A, maintainer 「同意」 2026-10-31T15:50Z) Clause-②: no Metadata-protocol: none ## 维护者速读 - **改了什么:** 只改了 lint、spec 等「ui 依赖的 7 个平台包」的 PR,现在合并前也会跑 ui 的 16 片单元测试(Test UI)。只改了根 `ci.yml`。 - **为什么改:** 以前这类 PR 只编译和打包 ui,不跑 ui 测试;坏了要等下一个无关 PR 进队列时才红,把别人踢出队列(#28219、#28491 等四次)。 - **风险与回滚:** 不影响产品代码。代价是 runner 分钟:每个这类合并约多 376 runner-min(PR 一次 + 队列一次,各约 188)。回滚 revert 这一个 PR 即可。 - **先看:** `.github/workflows/ci.yml` 的 `test-ui` 的 `if:` 一行;其次 `test-ui-gate` 判「跳过是否合理」的那段 shell。 - **两分钟验证:** 合并后第一个只改 `packages/spec/**` 或 `packages/lint/**` 的 PR,检查列表里 `Test UI (k/16)` 显示 success 而不是 skipped。 - **需要你做的:** 这个 PR 改了 `.github/workflows/**`(Tier H),需要你的批准,然后席位落地。 ## What changes `.github/workflows/ci.yml` only. - **`test-ui` (`Test UI (n/16)`)** — the job-level `if:` now reads `needs.filter.outputs.ui != 'false' || needs.filter.outputs.ui_consumers != 'false'` (inside the existing `ui_tree` conjunct), the exact shape `ui-deps`, `type-check-ui` and `build-e2e-ui` already use. Which files run is unchanged: `ui/scripts/select-affected-tests.mjs` already answers `mode=full` for any of the seven packages (`outsideTreeReasons`), so a `ui_consumers`-only diff runs the whole suite across the 16 shards. - **`test-ui-gate` (the required `Test UI` context)** — its #4928 guard explains a `skipped` rollup only when `filter` succeeded and said `ui_tree=false`, or `ui=false` **and** `ui_consumers=false` (it read `ui` alone). This follows from the same `if:`: a skip is "expected" only on the diffs the shards' condition excludes. It also makes the gate the regression check for this card: if the `if:` ever drops `ui_consumers` again, the first `ui_consumers`-only diff reds `Test UI` instead of passing a silent skip. - **`lint-ui` — not changed.** Measured below: ui's eslint reads none of the seven packages. - Comments: the `filter` outputs block, the `ui_consumers:` list and a new "when the shards start" paragraph above `test-ui` say what starts the shards and why. `scripts/pm/check-expected-skips.mjs` and `scripts/check-ci-filter-parity.mjs` — **no edit needed, measured.** `check-expected-skips` derives its roster from each job's `if:` (`deriveGatedSkips`), so the row moves by itself: `--roster` printed `Test UI (${{ matrix.shard }}/16) [ci.yml › test-ui; gate: job-gate filter ui_tree + filter ui; derived]` on `origin/main` `ea74ebf93` and prints `… gate: job-gate filter ui_tree + filter ui|ui_consumers; derived` on this head; its `--self-test` pins no live job name by design and passes unchanged. `check-ci-filter-parity` has no ui subject (it holds `core`/`crosspkg` to the cross-package declarations, Build Core's inputs and Test Core's turbo inputs); its scan and `--self-test` pass on this head. ## Why A diff confined to `packages/lint/**` (or spec, client, core, formula, sdui-parser, types) compiled and built the Console against the new shape (Type Check UI, Build & E2E UI) but never started the ui tests that pin that shape. The red surfaced on the next unrelated PR that did start the shards, and that bystander was ejected from the queue: #28192 → #28197 (#28219), and PR #28467 → p0 #28491, the fourth recurrence. ## Tests <details><summary>Reproduce first: the skip on origin/main ea74ebf</summary> Measured run — the queue build of #28192 (`3efccfd5`, `packages/lint/**` + `packages/metadata-protocol/**`), [run 41091107812](https://github.com/objectstack-ai/objectstack/actions/runs/41091107812), `merge_group`, read from the Actions jobs endpoint: ```text Build UI Deps success Type Check UI success Build & E2E UI success Live E2E UI success Test UI (${{ matrix.shard }}/16) skipped Test UI success (the aggregate read the skip as explained) Lint UI skipped ``` Filter evaluation — the live `filter` job's `dorny/paths-filter` block read out of `ci.yml`, matched with picomatch 4.0.7 (the matcher `dorny/paths-filter` uses), then each ui job's job-level `if:` evaluated with those outputs on a `pull_request` event (a one-off scratch script, not committed): ```text before (origin/main ea74ebf) after (this head) diff: packages/lint/src/index.ts diff: packages/lint/src/index.ts filter outputs: ui=false ui_consumers=true filter outputs: ui=false ui_consumers=true Build UI Deps STARTS Build UI Deps STARTS Type Check UI STARTS Type Check UI STARTS Test UI (n/16) SKIPPED Test UI (n/16) STARTS Lint UI SKIPPED Lint UI SKIPPED Build & E2E UI STARTS Build & E2E UI STARTS Live E2E UI STARTS Live E2E UI STARTS ``` `packages/spec/src/ui/action.zod.ts` reads the same as lint on both sides; `ui/packages/core/src/index.ts` starts every ui job on both; `scripts/foo.mjs` starts none on both. The selector already answers full for such a diff: `outsideTreeReasons({ outside: ['packages/lint/src/index.ts'] })` returns `packages/lint/src/index.ts: @objectstack/lint changed, and this tree's tests read its built output (through @object-ui/console)`, and over every tracked `packages/*/package.json` it turns full for exactly the seven `ui_consumers` packages (client, core, formula, lint, sdui-parser, spec, types). </details> <details><summary>The test-ui-gate verdict, every case</summary> The gate's `run:` block read out of this head's `ci.yml` and executed with each input set (`OS_TEST_UI_RESULT OS_FILTER_RESULT OS_UI_TREE OS_UI OS_UI_CONSUMERS -> exit`): ```text skipped success true false false -> exit 0 no ui path, no consumed package: explained skipped success true false true -> exit 1 a consumed package changed and the shards skipped: red (was exit 0) skipped success true true false -> exit 1 unchanged skipped success false true true -> exit 0 no ui tree: explained, unchanged skipped failure true false false -> exit 1 filter failed: red, unchanged success success true false true -> exit 0 unchanged ``` </details> ## Acceptance notes - **Lint UI: not added, measured.** `ui/eslint.config.js` sets no `parserOptions.project`/`projectService` (no type-aware rules, so no rule resolves a type from an `@objectstack/*` dist), and the two local rules that name `@objectstack/*` (`ui/eslint-rules/no-inline-spec-config.js`, `no-unprefixed-query-params.js`) match AST shapes and read no file. The ruling's condition ("only if ui's eslint reads those packages") is not met. Worth the seat's eye, not acted on here: other **Lint UI** steps do read the seven packages — `pnpm --filter @objectstack/spec check:liveness` runs over spec's sources, and the skill-example / doc-snippet / doc-example / readme-export type checks compile against built `@object-ui/*` closures that include the `@objectstack/*` dists (`check:skill-examples --build-filter` derives 12 filters, `@object-ui/data-objectstack...` among them). So a spec change can still red Lint UI first on a bystander's ui PR. carrier: domain:devx seat (a card if the seat wants Lint UI on `ui_consumers` too, by the same one-line shape). - **The same gap one step wider (noted, not filed).** The selector also turns full for every non-documentation workspace-root file and for `patches/**`; 21 such root files are tracked (`.npmrc`, `eslint.config.mjs`, `vitest.preset.mts`, `paseo.json`, `.gitattributes`, …) and several are not in the `ui:` list, nor is `patches/**`, so a diff confined to one of them still skips the shards. Not the ruling's scope; dedupe words: `ui filter patches npmrc Test UI skipped root file`. carrier: domain:devx seat. - **No permanent pin added.** A select-affected-tests pin reconciling the `ui_consumers` list against the selector's package graph was drafted and dropped: it is the second, reconciling check the ruling declined with option B. The `test-ui-gate` change above is the in-surface regression check instead. - Assignees: none set on this PR (the dispatch contract leaves the PR's assignee to the seat). ## Runner-minute cost (against #25869 and #25881) - **Per full Test UI battery: ~188 runner-min.** Measured on the last four `push` runs of `main` (41258652854, 41261634108, 41261929545, 41262951436; 16/16 shards each): 185.2 to 192.1 runner-min summed, mean shard 11.6 to 12.0 min, slowest shard 13.6 to 14.4 min wall. A `ui_consumers`-only diff is always a full battery (the selector's `outsideTreeReasons`). - **Per such merge: at least ~376 runner-min** — one PR-head run plus one queue entry; each extra push to the PR adds ~188. Build UI Deps already runs on these diffs, so the seven-package build #25869 de-duplicated is replayed from its cache and adds nothing. - **How many: 220 of 1410 merges in the 7 days to 2026-11-01T02:00Z (15.6%)** touched one of the seven packages and no `ui:` path (first-parent `origin/main`, the live filter globs, picomatch); 22 of 138 in the last 24 h. At 22 to 31 a day that is ~8,300 to ~11,700 runner-min a day, lower bound. - **Against #25881:** that card measured ~220 runner-min per queue entry for the four ui lanes and stopped paying it on entries that touch no ui path. This PR pays a part of it back (~188, Test UI alone) on the ~16% of entries that touch a package ui consumes — exactly the entries where it was skipped though its inputs changed. Entries touching neither stay skipped. - **Wall clock:** not on the measured critical path. On run 41091107812 (lint-only queue entry) Build UI Deps finished by 16:37:32Z and Test Core's slowest shard ended at 16:54:40Z of a run that finished at 16:55:38Z; sixteen shards of at most 14.4 min, started when Build UI Deps ended, would have finished by about 16:52Z. ## Verification Gate list derived with `node scripts/pm/dispatch-gates.mjs --commands` on this head (1 path vs merge base `ea74ebf93`): 53 derived, **53 run, 53 exit 0**; `--ran` reconciles `53 derived, 53 run, 0 NOT-MEASURED, 0 UNRUN` (a derived zero: every row records its exit code). Run at head `33b9f44f31`, after a full `pnpm build` under `scripts/pm/os-verify-lock.sh` (exit 0, 13m42s) for the four `dist/`-reading Build Core guards; `check:pm-dispatch-gates` ran its fast tier (1937 cases pass, 7 slow sections deferred). Beyond the derived list, the ui tests that read the root `ci.yml`'s ui jobs: `pnpm --dir ui exec vitest run` on `select-affected-tests.test.ts`, `select-affected-tests-wiring.test.ts`, `turbo-cache-save-bound.test.ts`, `check-action-forward-parity.test.ts`, `lint-workflow.test.ts` — 5 files, 148 tests pass. `node scripts/pm/check-expected-skips.mjs --self-test`: 206 cases pass. Every CI family is the PR's own check-runs. <details><summary>Every derived command and its exit code</summary> ```text node scripts/check-closing-keyword-parity.mjs :: exit 0 node scripts/check-closing-keyword-parity.mjs --self-test :: exit 0 node scripts/check-comment-mask-corpus.mjs :: exit 0 node scripts/check-dts-emitted.mjs --self-test :: exit 0 node scripts/check-self-test-wired.mjs :: exit 0 node scripts/check-self-test-wired.mjs --self-test :: exit 0 node scripts/check-self-test-workflow-commands.mjs :: exit 0 node scripts/check-self-test-workflow-commands.mjs --self-test :: exit 0 node scripts/check-step-collectors.mjs :: exit 0 node scripts/check-step-collectors.mjs --self-test :: exit 0 node scripts/check-whole-set-label-write.mjs :: exit 0 node scripts/check-whole-set-label-write.mjs --self-test :: exit 0 node scripts/ci/assert-turbo-remote-cache.mjs --self-test :: exit 0 node scripts/ci/plan-test-core-shards.mjs --self-test :: exit 0 node scripts/ci/prune-turbo-cache.mjs --self-test :: exit 0 node scripts/ci/scheduled-full-run.mjs --self-test :: exit 0 node scripts/docs-audit/check-drift-comment.mjs :: exit 0 node scripts/pm/ci-failure.mjs --self-test :: exit 0 pnpm check:agent-test-spelling :: exit 0 pnpm check:aggregator-roster :: exit 0 pnpm check:ci-filter-parity :: exit 0 pnpm check:cross-package-test-inputs :: exit 0 pnpm check:declared-population-live :: exit 0 pnpm check:driver-memory-census :: exit 0 pnpm check:gitlink-declared :: exit 0 pnpm check:node-version :: exit 0 pnpm check:nul-bytes :: exit 0 pnpm check:pm-close-cards :: exit 0 pnpm check:pm-expected-skips :: exit 0 pnpm check:pm-fleet-write-dispatch :: exit 0 pnpm check:pm-fleet-write-execute :: exit 0 pnpm check:pm-fleet-write-validate :: exit 0 pnpm check:pm-governed-merges :: exit 0 pnpm check:pm-issue-transfer :: exit 0 pnpm check:pm-post-stamped :: exit 0 pnpm check:pm-record-recognisers :: exit 0 pnpm check:pnpm-acquisition :: exit 0 pnpm check:pnpm-filter-targets :: exit 0 pnpm check:refd-timer-probe :: exit 0 pnpm check:required-contexts :: exit 0 pnpm check:shard-attestation :: exit 0 pnpm check:stall-guard-budget :: exit 0 pnpm check:stall-guard-headroom :: exit 0 pnpm check:type-check-coverage :: exit 0 pnpm check:watch-hint-literal :: exit 0 pnpm check:workflow-status-functions :: exit 0 pnpm check:workflow-step-name-quoting :: exit 0 pnpm check:dts-closure :: exit 0 pnpm check:dual-build-cjs-loads :: exit 0 pnpm check:lean-entry-closure :: exit 0 pnpm check:sourcemap-no-sources-content :: exit 0 pnpm check:type-check-debt :: exit 0 pnpm check:pm-dispatch-gates :: exit 0 ``` </details> ## Human Minimum - [x] 1. Schemas start as Zod; `packages/spec` holds no runtime logic. (not touched) - [x] 2. Every name I added uses the casing for its kind. (`OS_UI_CONSUMERS`, a CI-only step env) - [x] 3. No formatter ran; `pnpm lint` passes. (no lintable file changed; CI) - [x] 4. One issue, one branch off an up-to-date `main`, one pull request. - [x] 5. A change that publishes carries a changeset, and both declaration lines are filled in here and in the changeset. (publishes nothing: `skip-changeset`) - [x] 6. `packages/spec` touched: its generated artifacts are regenerated and committed. (not touched) - [x] 7. No hand edits to `content/docs/references/`, `packages/*/CHANGELOG.md` or `content/docs/releases/`. - [x] 8. Title, body and comments are in English. - [x] The *Before Submitting* list in CONTRIBUTING.md is done. --- _Generated by [Claude Code](https://claude.ai/code/session_01QUs3BsPpUdAGmm7wLdoKaK)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 3b45d4a commit 23d1303

1 file changed

Lines changed: 36 additions & 14 deletions

File tree

‎.github/workflows/ci.yml‎

Lines changed: 36 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -173,16 +173,19 @@ jobs:
173173
# the objectui skill bundle; see the filter below);
174174
# ui_consumers — one of the seven packages objectui installs from this
175175
# tree changed, so the Console is compiled (Type Check
176-
# UI) and built (Build & E2E UI) against the new shape in
177-
# the PR that moves it;
176+
# UI), built (Build & E2E UI) and tested (Test UI, since
177+
# objectstack#28220) against the new shape in the PR
178+
# that moves it;
178179
# ui_tree — `ui/package.json` exists in this checkout.
179180
# On merge_group `ui` and `ui_consumers` come from the queue diff
180181
# (objectstack#25867): an entry whose merged diff against
181182
# `merge_group.base_sha` touches no ui path and none of the seven
182183
# packages skips the five ui jobs, exactly as the same diff does on its
183-
# PR head; an entry that touches one runs them. If the step fails (the
184-
# base cannot be fetched, the diff errors), it is `continue-on-error`
185-
# there, its outputs are empty and `|| 'true'` runs every ui job.
184+
# PR head; an entry that touches one runs them (Lint UI reads `ui`
185+
# alone, so one of the seven by itself does not start it). If the step
186+
# fails (the base cannot be fetched, the diff errors), it is
187+
# `continue-on-error` there, its outputs are empty and `|| 'true'` runs
188+
# every ui job.
186189
#
187190
# `ui_tree` is what keeps the ui jobs SKIPPED, not failed, on a tree that
188191
# does not carry ui/ yet: `ui` and `ui_consumers` fall back to 'true' on
@@ -557,9 +560,11 @@ jobs:
557560
# skipped on the PR and in the queue alike.
558561
- 'package.json'
559562
# The seven packages objectui installs from this tree (the set its
560-
# `.objectstack-pin` used to pack). A change here compiles and builds
561-
# the Console in the same PR — the check the retired Spec Shape Gate
562-
# ran hourly against a clone of objectui main.
563+
# `.objectstack-pin` used to pack). A change here compiles, builds
564+
# and tests the Console in the same PR — the check the retired Spec
565+
# Shape Gate ran hourly against a clone of objectui main. Test UI
566+
# reads this list since objectstack#28220: its selector already
567+
# turns full for any of these packages.
563568
ui_consumers:
564569
- 'packages/spec/**'
565570
- 'packages/client/**'
@@ -3727,10 +3732,25 @@ jobs:
37273732
# non-documentation root file, `patches/`, one of the seven packages the
37283733
# `ui_consumers` filter names). The selector's header states the rules and
37293734
# what it does not see; the full battery on `main` is the backstop.
3735+
#
3736+
# ── Test UI: when the shards start (objectstack#28220, ruling A) ─────────
3737+
# On `ui` OR `ui_consumers`, the shape Build UI Deps, Type Check UI and
3738+
# Build & E2E UI already read. Until this entry the shards read `ui` alone,
3739+
# so a diff confined to one of the seven packages compiled and built the
3740+
# Console against the new shape and never ran a ui test that pins it: the
3741+
# selector above already answered `mode=full` for such a diff, but the job
3742+
# it answers in never started. The queue build of objectstack#28192
3743+
# (packages/lint/** + packages/metadata-protocol/**, run 41091107812)
3744+
# skipped all sixteen shards; the next queue build that started them
3745+
# (objectstack#28197) failed a ui test that imports @objectstack/lint, and a
3746+
# bystander was ejected for it. Which files run stays the selector's answer.
3747+
# The Test UI aggregate below explains a skip only when both outputs said
3748+
# false, so an `if:` that drops `ui_consumers` again reds there on the first
3749+
# such diff. Lint UI keeps `ui` alone: its eslint reads none of the seven.
37303750
test-ui:
37313751
name: Test UI (${{ matrix.shard }}/16)
37323752
needs: [filter, ui-deps]
3733-
if: ${{ !cancelled() && needs.filter.outputs.ui_tree != 'false' && needs.filter.outputs.ui != 'false' }}
3753+
if: ${{ !cancelled() && needs.filter.outputs.ui_tree != 'false' && (needs.filter.outputs.ui != 'false' || needs.filter.outputs.ui_consumers != 'false') }}
37343754
runs-on: ubuntu-latest
37353755
timeout-minutes: 30
37363756
permissions:
@@ -3895,7 +3915,8 @@ jobs:
38953915
#
38963916
# The verdict reads the matrix rollup `needs.test-ui.result`, and only
38973917
# `success` passes, with one exception: `skipped`, when `filter` concluded
3898-
# `success` AND said the diff reaches no ui path (the shards' own `if:`).
3918+
# `success` AND said the diff reaches no ui path and none of the seven
3919+
# packages ui consumes (the shards' own `if:`, objectstack#28220).
38993920
# That is #4928's skipped-only-when-filter-succeeded guard, and it makes a
39003921
# queue build or PR that touches no ui path read green here the way Lint
39013922
# UI and Build & E2E UI read `skipped` there. Everything else is red:
@@ -3926,17 +3947,18 @@ jobs:
39263947
OS_FILTER_RESULT: ${{ needs.filter.result }}
39273948
OS_UI_TREE: ${{ needs.filter.outputs.ui_tree }}
39283949
OS_UI: ${{ needs.filter.outputs.ui }}
3950+
OS_UI_CONSUMERS: ${{ needs.filter.outputs.ui_consumers }}
39293951
run: |
3930-
echo "Test UI (k/16) rollup: '$OS_TEST_UI_RESULT'; filter: '$OS_FILTER_RESULT' (ui_tree='$OS_UI_TREE', ui='$OS_UI')."
3952+
echo "Test UI (k/16) rollup: '$OS_TEST_UI_RESULT'; filter: '$OS_FILTER_RESULT' (ui_tree='$OS_UI_TREE', ui='$OS_UI', ui_consumers='$OS_UI_CONSUMERS')."
39313953
case "$OS_TEST_UI_RESULT" in
39323954
success)
39333955
echo "✓ Test UI: every shard concluded success."
39343956
;;
39353957
skipped)
3936-
if [ "$OS_FILTER_RESULT" = 'success' ] && { [ "$OS_UI_TREE" = 'false' ] || [ "$OS_UI" = 'false' ]; }; then
3937-
echo "✓ Test UI: the shards were skipped because filter found no ui path in this diff."
3958+
if [ "$OS_FILTER_RESULT" = 'success' ] && { [ "$OS_UI_TREE" = 'false' ] || { [ "$OS_UI" = 'false' ] && [ "$OS_UI_CONSUMERS" = 'false' ]; }; }; then
3959+
echo "✓ Test UI: the shards were skipped because filter found no ui path and no ui-consumed package in this diff."
39383960
else
3939-
echo "::error title=Test UI::the Test UI shards were skipped, but filter (result '$OS_FILTER_RESULT', ui_tree='$OS_UI_TREE', ui='$OS_UI') does not explain the skip. A skip passes only when filter succeeded and said the diff reaches no ui path (#4928)."
3961+
echo "::error title=Test UI::the Test UI shards were skipped, but filter (result '$OS_FILTER_RESULT', ui_tree='$OS_UI_TREE', ui='$OS_UI', ui_consumers='$OS_UI_CONSUMERS') does not explain the skip. A skip passes only when filter succeeded and said the diff reaches no ui path and none of the seven packages ui consumes (#4928)."
39403962
exit 1
39413963
fi
39423964
;;

0 commit comments

Comments
 (0)