From b5a16aeef79b168e82989e76b687aedb27c5072f Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 1 Nov 2026 02:17:37 +0000 Subject: [PATCH 1/2] ci(ui): start the Test UI shards when a ui-consumed package changes The shards read the `ui` filter output alone, so a diff confined to one of the seven packages ui consumes (packages/lint/**, spec, ...) never ran a ui test, though the affected-test selector already answers mode=full for it. They now start on `ui || ui_consumers`, the shape Build UI Deps, Type Check UI and Build & E2E UI read; the Test UI aggregate explains a skip only when both outputs said false. A select-affected-tests pin holds the shards' if: to every outer package the selector turns full for. Claude-Session: https://claude.ai/code/session_01QUs3BsPpUdAGmm7wLdoKaK Co-authored-by: Claude --- .github/workflows/ci.yml | 46 ++++++++++++++----- ui/scripts/__tests__/helpers/root-ci.ts | 30 ++++++++++-- .../__tests__/select-affected-tests.test.ts | 43 ++++++++++++++++- 3 files changed, 102 insertions(+), 17 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 522e171e501..38b86ea1ffe 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -173,14 +173,16 @@ jobs: # the objectui skill bundle; see the filter below); # ui_consumers — one of the seven packages objectui installs from this # tree changed, so the Console is compiled (Type Check - # UI) and built (Build & E2E UI) against the new shape in - # the PR that moves it; + # UI), built (Build & E2E UI) and tested (Test UI, since + # objectstack#28220) against the new shape in the PR + # that moves it; # ui_tree — `ui/package.json` exists in this checkout. # On merge_group `ui` and `ui_consumers` come from the queue diff # (objectstack#25867): an entry whose merged diff against # `merge_group.base_sha` touches no ui path and none of the seven # packages skips the five ui jobs, exactly as the same diff does on its - # PR head; an entry that touches one runs them. If the step fails (the + # PR head; an entry that touches one runs them (Lint UI reads `ui` + # alone, so one of the seven by itself does not start it). If the step fails (the # base cannot be fetched, the diff errors), it is `continue-on-error` # there, its outputs are empty and `|| 'true'` runs every ui job. # @@ -557,9 +559,12 @@ jobs: # skipped on the PR and in the queue alike. - 'package.json' # The seven packages objectui installs from this tree (the set its - # `.objectstack-pin` used to pack). A change here compiles and builds - # the Console in the same PR — the check the retired Spec Shape Gate - # ran hourly against a clone of objectui main. + # `.objectstack-pin` used to pack). A change here compiles, builds + # and tests the Console in the same PR — the check the retired Spec + # Shape Gate ran hourly against a clone of objectui main. Test UI + # reads this list since objectstack#28220; its selector turns full + # for any of these packages, and its test pins this list against + # the package graph the selector reads. ui_consumers: - 'packages/spec/**' - 'packages/client/**' @@ -3727,10 +3732,25 @@ jobs: # non-documentation root file, `patches/`, one of the seven packages the # `ui_consumers` filter names). The selector's header states the rules and # what it does not see; the full battery on `main` is the backstop. + # + # ── Test UI: when the shards start (objectstack#28220, ruling A) ───────── + # On `ui` OR `ui_consumers`, the shape Build UI Deps, Type Check UI and + # Build & E2E UI already read. Until this entry the shards read `ui` alone, + # so a diff confined to one of the seven packages compiled and built the + # Console against the new shape and never ran a ui test that pins it: the + # selector above already answered `mode=full` for such a diff, but the job + # it answers in never started. The queue build of objectstack#28192 + # (packages/lint/** + packages/metadata-protocol/**, run 41091107812) + # skipped all sixteen shards; the next queue build that started them + # (objectstack#28197) failed a ui test that imports @objectstack/lint, and a + # bystander was ejected for it. Which files run stays the selector's answer. + # `ui/scripts/__tests__/select-affected-tests.test.ts` holds the join: every + # package outside ui/ the selector turns full for is covered by a list this + # `if:` reads. Lint UI keeps `ui` alone (its eslint reads none of the seven). test-ui: name: Test UI (${{ matrix.shard }}/16) needs: [filter, ui-deps] - if: ${{ !cancelled() && needs.filter.outputs.ui_tree != 'false' && needs.filter.outputs.ui != 'false' }} + if: ${{ !cancelled() && needs.filter.outputs.ui_tree != 'false' && (needs.filter.outputs.ui != 'false' || needs.filter.outputs.ui_consumers != 'false') }} runs-on: ubuntu-latest timeout-minutes: 30 permissions: @@ -3895,7 +3915,8 @@ jobs: # # The verdict reads the matrix rollup `needs.test-ui.result`, and only # `success` passes, with one exception: `skipped`, when `filter` concluded - # `success` AND said the diff reaches no ui path (the shards' own `if:`). + # `success` AND said the diff reaches no ui path and none of the seven + # packages ui consumes (the shards' own `if:`, objectstack#28220). # That is #4928's skipped-only-when-filter-succeeded guard, and it makes a # queue build or PR that touches no ui path read green here the way Lint # UI and Build & E2E UI read `skipped` there. Everything else is red: @@ -3926,17 +3947,18 @@ jobs: OS_FILTER_RESULT: ${{ needs.filter.result }} OS_UI_TREE: ${{ needs.filter.outputs.ui_tree }} OS_UI: ${{ needs.filter.outputs.ui }} + OS_UI_CONSUMERS: ${{ needs.filter.outputs.ui_consumers }} run: | - echo "Test UI (k/16) rollup: '$OS_TEST_UI_RESULT'; filter: '$OS_FILTER_RESULT' (ui_tree='$OS_UI_TREE', ui='$OS_UI')." + 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')." case "$OS_TEST_UI_RESULT" in success) echo "✓ Test UI: every shard concluded success." ;; skipped) - if [ "$OS_FILTER_RESULT" = 'success' ] && { [ "$OS_UI_TREE" = 'false' ] || [ "$OS_UI" = 'false' ]; }; then - echo "✓ Test UI: the shards were skipped because filter found no ui path in this diff." + if [ "$OS_FILTER_RESULT" = 'success' ] && { [ "$OS_UI_TREE" = 'false' ] || { [ "$OS_UI" = 'false' ] && [ "$OS_UI_CONSUMERS" = 'false' ]; }; }; then + echo "✓ Test UI: the shards were skipped because filter found no ui path and no ui-consumed package in this diff." else - 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)." + 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)." exit 1 fi ;; diff --git a/ui/scripts/__tests__/helpers/root-ci.ts b/ui/scripts/__tests__/helpers/root-ci.ts index 2501ec19ecb..a183ce57bcc 100644 --- a/ui/scripts/__tests__/helpers/root-ci.ts +++ b/ui/scripts/__tests__/helpers/root-ci.ts @@ -125,23 +125,45 @@ export function triggerBlock(ci: string): string { * list, the reading that would make every "is not hidden" pin vacuous. */ export function uiFilterGlobs(ci: string): string[] { + return filterGlobs(ci, 'ui'); +} + +/** + * The globs of one named list of the `filter` job's paths filter (`ui`, + * `ui_consumers`, `core`, ...), read the way `uiFilterGlobs` reads `ui:`. + * Throws when the filter declares no such list or it lists no globs. + */ +export function filterGlobs(ci: string, name: string): string[] { const filter = jobBlock(ci, 'filter'); const lines = filter.split('\n'); - const at = lines.findIndex((line) => /^ +ui:[ \t]*$/.test(line)); - if (at === -1) throw new Error(`${rootCiPath}: the \`filter\` job declares no \`ui:\` paths filter`); + const at = lines.findIndex((line) => new RegExp(`^ +${name}:[ \\t]*$`).test(line)); + if (at === -1) throw new Error(`${rootCiPath}: the \`filter\` job declares no \`${name}:\` paths filter`); const indent = lines[at].search(/\S/); const globs: string[] = []; for (const line of lines.slice(at + 1)) { if (/^\s*(#.*)?$/.test(line)) continue; if (line.search(/\S/) <= indent) break; const m = line.match(/^\s+-\s+'([^']+)'\s*$/); - if (!m) throw new Error(`${rootCiPath}: cannot read the \`ui:\` filter entry ${JSON.stringify(line)}`); + if (!m) throw new Error(`${rootCiPath}: cannot read the \`${name}:\` filter entry ${JSON.stringify(line)}`); globs.push(m[1]); } - if (globs.length === 0) throw new Error(`${rootCiPath}: the \`ui:\` paths filter lists no globs`); + if (globs.length === 0) throw new Error(`${rootCiPath}: the \`${name}:\` paths filter lists no globs`); return globs; } +/** + * The `filter` outputs a job's `if:` starts it on: every + * `needs.filter.outputs. != 'false'` it spells, in order. Throws when the + * job carries no `if:` line or the line reads no filter output. + */ +export function jobFilterOutputs(ci: string, key: string): string[] { + const line = jobBlock(ci, key).match(/^ {4}if: (.+)$/m); + if (!line) throw new Error(`${rootCiPath}: the \`${key}\` job carries no one-line \`if:\``); + const outputs = [...line[1].matchAll(/needs\.filter\.outputs\.([A-Za-z_][\w-]*) != 'false'/g)].map((m) => m[1]); + if (outputs.length === 0) throw new Error(`${rootCiPath}: the \`${key}\` job's \`if:\` reads no filter output`); + return outputs; +} + /** A path relative to this tree, as the root workflow's filters see it (`ui/`). */ export function repoPath(uiRelative: string): string { return `${nestedPrefix()}${uiRelative}`; diff --git a/ui/scripts/__tests__/select-affected-tests.test.ts b/ui/scripts/__tests__/select-affected-tests.test.ts index 19c4c803afa..21890c13df9 100644 --- a/ui/scripts/__tests__/select-affected-tests.test.ts +++ b/ui/scripts/__tests__/select-affected-tests.test.ts @@ -20,7 +20,18 @@ import { trackedFiles, } from '../select-affected-tests.mjs'; import { shardTotal } from '../../../scripts/ci-shard-matrix.mjs'; -import { jobBlock, rootCi, triggerBlock, UI_JOBS, uiRun, withoutComments } from './helpers/root-ci'; +import { workspaceRoot } from '../workspace-root.mjs'; +import { + filterGlobs, + globMatches, + jobBlock, + jobFilterOutputs, + rootCi, + triggerBlock, + UI_JOBS, + uiRun, + withoutComments, +} from './helpers/root-ci'; /** * objectui#12214 — the affected-test selector behind the two-speed `Test` lane. @@ -447,6 +458,36 @@ describe("the root ci.yml's Test UI job wires the selector fail-open (objectui#1 expect(job).toMatch(/uses: actions\/checkout@\S+\n\s+with:\n\s+fetch-depth: 0\n/); }); + // objectstack#28220: the selector answered `mode=full` for a diff confined + // to packages/lint/**, but the shards read only the `ui` output, so the job + // that answer lives in never started and a ui test pinning a lint value went + // unrun until a bystander's queue build. The selector and the job's `if:` + // answer one question from two ends, so this holds them together: every + // workspace package outside this tree that the selector turns full for (read + // from the package graph, never listed here) must match a glob of a filter + // list the shards' `if:` reads. + it('starts the shards on every package outside the tree the selector turns full for (objectstack#28220)', () => { + const ci = rootCi(); + const wsRoot = workspaceRoot(repoRoot); + const manifests = trackedFiles(wsRoot).filter( + (rel) => rel.includes('/') && rel.endsWith('/package.json') && !rel.startsWith('ui/') && !rel.startsWith('patches/'), + ); + const full = outsideTreeReasons({ root: repoRoot, outside: manifests }).map((reason) => reason.slice(0, reason.indexOf(': '))); + // The live graph reaches the seven packages the root `ui_consumers` list names. + expect(full.length, 'the selector turned full for no outer package: the graph read nothing').toBeGreaterThanOrEqual(7); + expect(full).toContain('packages/lint/package.json'); + + const outputs = jobFilterOutputs(ci, UI_JOBS.test).filter((o) => o !== 'ui_tree'); + const covered = (globs: string[]) => full.filter((rel) => globs.some((glob) => globMatches(glob, rel))); + const globs = outputs.flatMap((o) => filterGlobs(ci, o)); + expect( + full.filter((rel) => !covered(globs).includes(rel)), + `a package the selector runs every ui test for matches none of the lists \`${UI_JOBS.test}\` starts on (${outputs.join(', ')})`, + ).toEqual([]); + // Able to say no: the shards' old condition, \`ui\` alone, misses packages/lint. + expect(covered(filterGlobs(ci, 'ui'))).not.toContain('packages/lint/package.json'); + }); + it('keeps the matrix width and the full-suite command for the full battery', () => { // The width is the job's own `shard:` list, read by the monorepo's one reader of // shard sizes (it refuses any other spelling); every `/N` the job's run step From 33b9f44f31db4e1f915867a39e13875a555d0258 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 1 Nov 2026 02:21:33 +0000 Subject: [PATCH 2/2] ci(ui): keep the Test UI change to ci.yml alone The select-affected-tests pin reconciled the ui_consumers list against the package graph, a second check the ruling did not take; the Test UI aggregate, which now explains a skip only when ui and ui_consumers both said false, already reds on the first diff an if: regression would skip. Claude-Session: https://claude.ai/code/session_01QUs3BsPpUdAGmm7wLdoKaK Co-authored-by: Claude --- .github/workflows/ci.yml | 18 ++++---- ui/scripts/__tests__/helpers/root-ci.ts | 30 ++----------- .../__tests__/select-affected-tests.test.ts | 43 +------------------ 3 files changed, 14 insertions(+), 77 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 38b86ea1ffe..393aea6025b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -182,9 +182,10 @@ jobs: # `merge_group.base_sha` touches no ui path and none of the seven # packages skips the five ui jobs, exactly as the same diff does on its # PR head; an entry that touches one runs them (Lint UI reads `ui` - # alone, so one of the seven by itself does not start it). If the step fails (the - # base cannot be fetched, the diff errors), it is `continue-on-error` - # there, its outputs are empty and `|| 'true'` runs every ui job. + # alone, so one of the seven by itself does not start it). If the step + # fails (the base cannot be fetched, the diff errors), it is + # `continue-on-error` there, its outputs are empty and `|| 'true'` runs + # every ui job. # # `ui_tree` is what keeps the ui jobs SKIPPED, not failed, on a tree that # does not carry ui/ yet: `ui` and `ui_consumers` fall back to 'true' on @@ -562,9 +563,8 @@ jobs: # `.objectstack-pin` used to pack). A change here compiles, builds # and tests the Console in the same PR — the check the retired Spec # Shape Gate ran hourly against a clone of objectui main. Test UI - # reads this list since objectstack#28220; its selector turns full - # for any of these packages, and its test pins this list against - # the package graph the selector reads. + # reads this list since objectstack#28220: its selector already + # turns full for any of these packages. ui_consumers: - 'packages/spec/**' - 'packages/client/**' @@ -3744,9 +3744,9 @@ jobs: # skipped all sixteen shards; the next queue build that started them # (objectstack#28197) failed a ui test that imports @objectstack/lint, and a # bystander was ejected for it. Which files run stays the selector's answer. - # `ui/scripts/__tests__/select-affected-tests.test.ts` holds the join: every - # package outside ui/ the selector turns full for is covered by a list this - # `if:` reads. Lint UI keeps `ui` alone (its eslint reads none of the seven). + # The Test UI aggregate below explains a skip only when both outputs said + # false, so an `if:` that drops `ui_consumers` again reds there on the first + # such diff. Lint UI keeps `ui` alone: its eslint reads none of the seven. test-ui: name: Test UI (${{ matrix.shard }}/16) needs: [filter, ui-deps] diff --git a/ui/scripts/__tests__/helpers/root-ci.ts b/ui/scripts/__tests__/helpers/root-ci.ts index a183ce57bcc..2501ec19ecb 100644 --- a/ui/scripts/__tests__/helpers/root-ci.ts +++ b/ui/scripts/__tests__/helpers/root-ci.ts @@ -125,45 +125,23 @@ export function triggerBlock(ci: string): string { * list, the reading that would make every "is not hidden" pin vacuous. */ export function uiFilterGlobs(ci: string): string[] { - return filterGlobs(ci, 'ui'); -} - -/** - * The globs of one named list of the `filter` job's paths filter (`ui`, - * `ui_consumers`, `core`, ...), read the way `uiFilterGlobs` reads `ui:`. - * Throws when the filter declares no such list or it lists no globs. - */ -export function filterGlobs(ci: string, name: string): string[] { const filter = jobBlock(ci, 'filter'); const lines = filter.split('\n'); - const at = lines.findIndex((line) => new RegExp(`^ +${name}:[ \\t]*$`).test(line)); - if (at === -1) throw new Error(`${rootCiPath}: the \`filter\` job declares no \`${name}:\` paths filter`); + const at = lines.findIndex((line) => /^ +ui:[ \t]*$/.test(line)); + if (at === -1) throw new Error(`${rootCiPath}: the \`filter\` job declares no \`ui:\` paths filter`); const indent = lines[at].search(/\S/); const globs: string[] = []; for (const line of lines.slice(at + 1)) { if (/^\s*(#.*)?$/.test(line)) continue; if (line.search(/\S/) <= indent) break; const m = line.match(/^\s+-\s+'([^']+)'\s*$/); - if (!m) throw new Error(`${rootCiPath}: cannot read the \`${name}:\` filter entry ${JSON.stringify(line)}`); + if (!m) throw new Error(`${rootCiPath}: cannot read the \`ui:\` filter entry ${JSON.stringify(line)}`); globs.push(m[1]); } - if (globs.length === 0) throw new Error(`${rootCiPath}: the \`${name}:\` paths filter lists no globs`); + if (globs.length === 0) throw new Error(`${rootCiPath}: the \`ui:\` paths filter lists no globs`); return globs; } -/** - * The `filter` outputs a job's `if:` starts it on: every - * `needs.filter.outputs. != 'false'` it spells, in order. Throws when the - * job carries no `if:` line or the line reads no filter output. - */ -export function jobFilterOutputs(ci: string, key: string): string[] { - const line = jobBlock(ci, key).match(/^ {4}if: (.+)$/m); - if (!line) throw new Error(`${rootCiPath}: the \`${key}\` job carries no one-line \`if:\``); - const outputs = [...line[1].matchAll(/needs\.filter\.outputs\.([A-Za-z_][\w-]*) != 'false'/g)].map((m) => m[1]); - if (outputs.length === 0) throw new Error(`${rootCiPath}: the \`${key}\` job's \`if:\` reads no filter output`); - return outputs; -} - /** A path relative to this tree, as the root workflow's filters see it (`ui/`). */ export function repoPath(uiRelative: string): string { return `${nestedPrefix()}${uiRelative}`; diff --git a/ui/scripts/__tests__/select-affected-tests.test.ts b/ui/scripts/__tests__/select-affected-tests.test.ts index 21890c13df9..19c4c803afa 100644 --- a/ui/scripts/__tests__/select-affected-tests.test.ts +++ b/ui/scripts/__tests__/select-affected-tests.test.ts @@ -20,18 +20,7 @@ import { trackedFiles, } from '../select-affected-tests.mjs'; import { shardTotal } from '../../../scripts/ci-shard-matrix.mjs'; -import { workspaceRoot } from '../workspace-root.mjs'; -import { - filterGlobs, - globMatches, - jobBlock, - jobFilterOutputs, - rootCi, - triggerBlock, - UI_JOBS, - uiRun, - withoutComments, -} from './helpers/root-ci'; +import { jobBlock, rootCi, triggerBlock, UI_JOBS, uiRun, withoutComments } from './helpers/root-ci'; /** * objectui#12214 — the affected-test selector behind the two-speed `Test` lane. @@ -458,36 +447,6 @@ describe("the root ci.yml's Test UI job wires the selector fail-open (objectui#1 expect(job).toMatch(/uses: actions\/checkout@\S+\n\s+with:\n\s+fetch-depth: 0\n/); }); - // objectstack#28220: the selector answered `mode=full` for a diff confined - // to packages/lint/**, but the shards read only the `ui` output, so the job - // that answer lives in never started and a ui test pinning a lint value went - // unrun until a bystander's queue build. The selector and the job's `if:` - // answer one question from two ends, so this holds them together: every - // workspace package outside this tree that the selector turns full for (read - // from the package graph, never listed here) must match a glob of a filter - // list the shards' `if:` reads. - it('starts the shards on every package outside the tree the selector turns full for (objectstack#28220)', () => { - const ci = rootCi(); - const wsRoot = workspaceRoot(repoRoot); - const manifests = trackedFiles(wsRoot).filter( - (rel) => rel.includes('/') && rel.endsWith('/package.json') && !rel.startsWith('ui/') && !rel.startsWith('patches/'), - ); - const full = outsideTreeReasons({ root: repoRoot, outside: manifests }).map((reason) => reason.slice(0, reason.indexOf(': '))); - // The live graph reaches the seven packages the root `ui_consumers` list names. - expect(full.length, 'the selector turned full for no outer package: the graph read nothing').toBeGreaterThanOrEqual(7); - expect(full).toContain('packages/lint/package.json'); - - const outputs = jobFilterOutputs(ci, UI_JOBS.test).filter((o) => o !== 'ui_tree'); - const covered = (globs: string[]) => full.filter((rel) => globs.some((glob) => globMatches(glob, rel))); - const globs = outputs.flatMap((o) => filterGlobs(ci, o)); - expect( - full.filter((rel) => !covered(globs).includes(rel)), - `a package the selector runs every ui test for matches none of the lists \`${UI_JOBS.test}\` starts on (${outputs.join(', ')})`, - ).toEqual([]); - // Able to say no: the shards' old condition, \`ui\` alone, misses packages/lint. - expect(covered(filterGlobs(ci, 'ui'))).not.toContain('packages/lint/package.json'); - }); - it('keeps the matrix width and the full-suite command for the full battery', () => { // The width is the job's own `shard:` list, read by the monorepo's one reader of // shard sizes (it refuses any other spelling); every `/N` the job's run step