Repository navigation
Conversation
Removes the scope classifier jobs (check-scope, interaction-scope, native-swift-scope, check-components). Each job that owns tests now runs .github/actions/test-scope right after a sparse checkout: it reads the pull request's file list and the base commit's trusted classifier, and skips the job's remaining steps with success when nothing in its scope changed. Anything it cannot decide puts everything in scope (spec:AST-030 FR3, FR4). native-swift-runner is the one pre-job left, because only a job's runner has to be chosen before it starts. Also: - fixture-contrast's steps run in test-build (same Playwright runner); - registry-contract becomes test-registry-e2e and runs for release-check only; its network-free production gate runs with the unit tests; - ci-duration and the weekly a11y and RTL workflows are removed (the full audits run on main); - the Internal Registry job becomes test-deps, scoped by its trigger paths; - the test join requires release-bump on a release-branch pull request.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
|
AI review status for this pull request.
|
The main-tier release-check dispatcher runs on every landed commit, like the Main workflow, so it is a landed-commit workflow.
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Approval needs an engineering-owner decision first: moving the full-catalog ShadCN registry check off pull requests (draft spec:AST-026 FR10 says required pull-request CI runs it) and deleting the ci-duration job are not settled by any current spec, so an engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do so if they are an engineering owner. Whatever that decision, these need changes:
- A
ci:heavy-lanespull request now runs the whole visual plan and the full RTL roster (ci.yml:2079,:2366), but AST-030 FR13 and rtl-audit-evidence INV7 limit pull-request visual and RTL audits to the affected components. - Deleting
rtl-weekly.ymlremoves the scheduled full-roster audit that INV7 still requires, so keep it or amend INV7 and its verification row with owner approval. - Add
.github/actions/test-scope/to thepathsofpr-interactionandnative-swift-runner, so a change to the scope action runs the jobs it gates. - When
docsite-test's own scope step falls back on a spec-only pull request, it skipsValidate spec recordswhile the docsite owners skipped as spec-only, so make that job validate the records, or fail, when its scope is undecided. - Please split the registry,
ci-duration, weekly-sweep and Internal Registry changes out of this 3,967-line pull request.
[Automated review]
| if: github.event_name == 'pull_request' | ||
| env: | ||
| BROAD_STABLE_VISUAL: ${{ needs.check-components.outputs.broad_stable_visual }} | ||
| BROAD_STABLE_VISUAL: 'true' |
There was a problem hiding this comment.
Hard-coding BROAD_STABLE_VISUAL: 'true' sends every opted-in pull request to the whole stable plan (COMMAND=release), so the focused tiers below can no longer run. AST-030 FR13 has a pull request's visual audit cover the changed components and their reverse dependents; derive this flag from the analysis artifact or visual-scope.mjs, as the deleted check-components output did.
| --force-full "${{ needs.check-components.outputs.force_full_component_audits }}" \ | ||
| --has-components "${{ needs.check-components.outputs.has_rtl_components }}" \ | ||
| --has-harness "${{ needs.check-components.outputs.has_rtl_harness }}" \ | ||
| --force-full true \ |
There was a problem hiding this comment.
--force-full true makes every opted-in pull request audit the full RTL roster. AST-030 FR13 and rtl-audit-evidence INV7 scope a pull request's RTL audit to the affected components; pass the analysis artifact's forceFullComponentAudits here instead, and the matching value in the pr-rtl join (line 2574).
| id: scope | ||
| uses: ./.test-scope/.github/actions/test-scope | ||
| with: | ||
| paths: '^(internal/interaction-harness/|internal/a11y-spec/|playwright\.interaction\.config\.ts$|packages/[^/]+/src/|packages/themes/[^/]+/src/|apps/storybook/|\.github/workflows/ci\.yml$|package\.json$|pnpm-lock\.yaml$)' |
There was a problem hiding this comment.
The code that decides applies moved from ci.yml, which this pattern still lists, to .github/actions/test-scope/, which it does not. A pull request that changes only the action gets applies=false here, so the proofs never run on it. Add |\.github/actions/test-scope/ (and the same to native-swift-runner's paths, line 760), with a touched case in ci-interaction.test.mjs.
| if: github.event_name == 'workflow_dispatch' && inputs.operation == 'release-check' | ||
| # The preview and hidden production registries are generated and verified | ||
| # independently, so each gets its own leg (~9 minutes in one job). `test` | ||
| # joins them: the job result is success only when both passed. |
There was a problem hiding this comment.
test no longer joins test-registry-e2e; release-check does. This comment now points at the wrong join.
| report completeness and exclusion visibility remain requirements, not authority | ||
| to suppress findings. | ||
| - `.github/workflows/rtl-weekly.yml` — full-roster scheduled audit. | ||
| - `.github/workflows/ci.yml` release-check — the full-roster audit on every push |
There was a problem hiding this comment.
INV7 still says "The scheduled unfiltered audit checks the full live roster and shared regressions. Neither silently replaces the other.", and its verification row still names the weekly-summary tests this pull request deletes. Keep rtl-weekly.yml, or amend INV7 and that row with owner approval; per-push release-check is the stacked DEC-7, which main's AST-030 FR12 does not allow ("Release checks MUST NOT run on every push").
…per job Review follow-ups: - The registry contract, ci-duration, the weekly sweeps and the Internal Registry rename move to their own pull request, so this one only removes the scope classifier jobs and folds fixture-contrast into test-build. registry-contract stays a pull-request job here, deciding its own scope. - A ci:heavy-lanes pull request keeps its visual and RTL audits on the affected components (spec:AST-030 FR13): the scope action can also run the base's component-audit-scope.cjs and list the changed files, and the RTL and visual jobs read those instead of a classification job. - The interaction and native Swift scopes include the scope action, so a change to it runs the jobs it gates.
|
Posted by Cindy's agent on her behalf. /approve-spec 910c16e |
There was a problem hiding this comment.
Changes are needed before this can merge, although the per-job scope step follows AST-030's trusted-base and fail-closed rules and the earlier registry, ci-duration and weekly-sweep concerns are resolved. Please fix the two routing defects below with routing tests for each (AST-030 FR9), and split the change along its job groups or show why it cannot land in parts.
- On a
ci:heavy-lanespull request outside RTL scope, such as aninternal/a11y-spec/or CLI-only change,pr-rtl-shardnow always runs whilertl-join.mjsstill accepts an out-of-scope result only from skipped shards, sopr-rtlfails (and on a spec-, docsite- or tooling-only pull request with the label, the heavy jobs fail downloading apr-analysisartifact thatbuild-storybooknever uploads). - If
docsite-test's own scope lookup fails on a spec-only pull request, it falls back to everything in scope, sees the docsite owners' skipped success and skipsValidate spec records, so the required check passes with no record validated. - At 3,080 changed lines the migration still decomposes by job group, and the
fixture-contrastfold and themain-heavy-lanes.ymlregistry entry are separable.
[Automated review]
| (needs.check-components.outputs.has_rtl_components == 'true' || | ||
| needs.check-components.outputs.has_rtl_harness == 'true' || | ||
| needs.check-components.outputs.force_full_component_audits == 'true') | ||
| (github.event_name == 'workflow_dispatch' && inputs.operation == 'release-check')) |
There was a problem hiding this comment.
Without its scope gate this job now runs on every ci:heavy-lanes pull request and reports success, but rtl-join.mjs still accepts --should-run false only when the shards were skipped (rtl-join.mjs:161). An opted-in change outside RTL scope, such as internal/a11y-spec/ or packages/cli/, therefore turns pr-rtl red, after the full-suite case has audited every package's full roster. Gate this job's work on the scope outputs, as pr-rtl-merge does, and let the join accept a successful no-op shard result, with a routing test for that case.
|
|
||
| - name: Validate spec records | ||
| if: needs.check-scope.outputs.spec_only == 'true' | ||
| if: steps.scope.outputs.spec_only == 'true' |
There was a problem hiding this comment.
If this job's own scope lookup falls back (a 403 is not retried) while docsite-browser and docsite-data decided spec-only, spec_only is false here, both result checks above see success, and this step skips: the required docsite-test passes with no spec record validated, and test-ui's knowledge check skipped too. Report whether the scope was decided and validate, or fail, when it was not (AST-030 FR4).
| needs: [build-storybook, check-components] | ||
| if: ((github.event_name == 'pull_request' && contains(github.event.pull_request.labels.*.name, 'ci:heavy-lanes')) || (github.event_name == 'workflow_dispatch' && inputs.operation == 'release-check')) && needs.check-components.outputs.has_components == 'true' | ||
| needs: [build-storybook] | ||
| if: ((github.event_name == 'pull_request' && contains(github.event.pull_request.labels.*.name, 'ci:heavy-lanes')) || (github.event_name == 'workflow_dispatch' && inputs.operation == 'release-check')) |
There was a problem hiding this comment.
With check-components gone, nothing gates this job on component scope: on a spec-, docsite- or tooling-only pull request with ci:heavy-lanes, build-storybook uploads no pr-analysis, so the download below fails and pr-a11y turns red where the base skipped it. The same holds for pr-a11y-specs, pr-visual and the RTL shards. Add the scope step and gate the work, downloads included, on its audit outputs.
|
/approve-spec 910c16e Posted by Cindy's agent on her behalf. |
…ocsite-test validates Review follow-ups: - On a ci:heavy-lanes pull request, the accessibility, visual and RTL jobs decide their own component scope and skip their work, downloads included, when it does not apply, so a spec-, docsite- or tooling-only change no longer fails on a missing analysis artifact. The RTL join accepts shards that decided their scope and did nothing. - The scope action reports whether it decided. docsite-test validates spec records when its own scope is spec-only or undecided, so a failed lookup can never pass the required check unvalidated (spec:AST-030 FR4).
|
/approve-spec 215480c Posted by Cindy's agent on her behalf. |
|
/approve-spec 215480c |
There was a problem hiding this comment.
Changes are needed before this can merge: the per-job scope step follows AST-030's trusted-base and fail-closed rules, and the earlier docsite-test fallback and out-of-scope heavy-lane failures are fixed, but three points remain.
- Split the 3,235-line change by job group, or show which current AST-030 claim an intermediate state would break; the
fixture-contrastfold can land first on its own. - Add tests that fail when the
decidedclause is dropped fromValidate spec recordsor when the heavy-lanepr-analysisdownload gates are removed (AST-030 FR9); today only the setup step's condition is pinned. - On a release-branch pull request the scope action reports every audit in full, so a release bump labelled
ci:heavy-lanesstill failspr-a11y, Stable visual regression andpr-rtl; report no audits there.
[Automated review]
|
|
||
| - name: Validate spec records | ||
| if: needs.check-scope.outputs.spec_only == 'true' | ||
| if: (steps.scope.outputs.spec_only == 'true' || steps.scope.outputs.decided != 'true') |
There was a problem hiding this comment.
No test pins this decided clause: spec-owner-workflow.test.mjs checks the condition only on the setup step above, so dropping it here leaves every routing test green while an undecided spec-only pull request passes docsite-test unvalidated (AST-030 FR9). Assert this step's own if directly, as ci-docsite-browser.test.mjs does for the docsite owners' steps.
| const pr = context.payload.pull_request; | ||
| if (pr.base.ref.startsWith('release/v')) { | ||
| core.info('Release-branch pull request: the generated version bump.'); | ||
| report({...everything, spec_only: 'true', tooling_only: 'true', release_bump: 'true', touches: 'false', applies: 'false'}); |
There was a problem hiding this comment.
For component-audit jobs, everything already holds every audit in full (line 143) and this return sets no changed_files, so a release bump labelled ci:heavy-lanes sends pr-a11y-guards, pr-a11y-specs, pr-rtl-shard and pr-visual after a pr-analysis artifact that build-storybook never uploads on this lane. Spread auditNone here when audit is set, as the comment at line 129 intends, and have the visual scope step report no stable visual work on a specialized lane.
Review follow-ups: - On a release-branch pull request the scope action reports no component audit, and the visual jobs report no stable visual work on any specialized lane, so a labelled release bump never waits for an analysis artifact that was never built. - Routing tests pin docsite-test's own validation condition and every heavy lane's analysis-download gate (spec:AST-030 FR9).
|
/approve-spec 50f50e0 |
There was a problem hiding this comment.
Changes are needed before this can merge: the per-job scope step follows AST-030's trusted-base and fail-closed rules and the release-bump heavy-lane fix works, but two points remain.
- Split the 3,311-line change by job group, or show which current AST-030 claim an intermediate state would break; the
fixture-contrastfold can land first on its own, since at the base it andtest-buildshare the same scope condition and runner. - The test "reports no stable visual work on a specialized lane" (
ci-test-routing.test.mjs:884) still passes when the short circuit is deleted, becauseindexOfreturns -1; assert that the index exists, or run the step withSPECIALIZED=trueand expecthas_stable_visual=false(AST-030 FR9).
[Automated review]
| expect(decide.run.indexOf('"$SPECIALIZED" = true'), name).toBeLessThan( | ||
| decide.run.indexOf('!= pull_request'), | ||
| ); |
There was a problem hiding this comment.
This still passes when the short circuit is deleted: indexOf returns -1, which is less than the index of != pull_request. With the block removed from both Decide stable visual scope steps, every .github/scripts test passes, so a labelled release bump could again go after artifacts that are never built with CI green (AST-030 FR9). Assert toBeGreaterThanOrEqual(0) on the index first, or run the step script with SPECIALIZED=true and no files and expect has_stable_visual=false.
…precede the PR check
|
/approve-spec 1f3c699 |
There was a problem hiding this comment.
Changes are needed before this can merge: the per-job scope step follows AST-030's trusted-base and fail-closed rules, and the specialized-lane visual test now fails when the short circuit it pins is deleted, but at 3,313 changed lines the change still needs to be split or shown to be inseparable. Please split it along its job groups (for example the scope action with the docsite and test owners, then the build owners, then the heavy lanes with check-components, then the interaction, native Swift and helper removal), or name the current AST-030 claim an intermediate state would break; the fixture-contrast fold can land first on its own, since at the base it and test-build share the same scope condition and runner.
[Automated review]
#8888 is superseded by four atomic PRs; this change now sits on the last of them. The relevance-registry edit drops out: that registry is not on main.
#8888 is superseded by four atomic PRs; this change now sits on the last of them. The relevance-registry edits drop out: that registry is not on main.
This pull request now stands alone on main. The tree is main plus its own change, re-applied to main's job layout: - the draft and outside-contributor gate on every pull_request workflow, with the required checks failing for an unlabelled fork (FR17); - the per-author heavy-suite caps as a standalone heavy-caps step that pr-interaction, test-build and fixture-contrast run right after their checkout, each limit one entry in an ordered list (FR16); - CONTRIBUTING, the spec and the tests.
Who this helps: everyone opening a pull request. Each CI run starts 4 fewer jobs, and no test waits about a minute for a scope job before it can start.
Stacked: on #8826, #8881, #8880, #8877 and #8884, which land first. The base moves to
mainand the branch is rebased once they merge. The off-PR and informational checks (registry contract, ci-duration, the weekly sweeps, test-deps) are split into a follow-up PR stacked on this one.What changes
check-scope,interaction-scope,native-swift-scopeandcheck-componentsare removed..github/actions/test-scoperight after a sparse checkout. The action reads the PR's file list and the base commit's classifier (change-scope and its two helpers).native-swift-runnerexists because only it chooses a runner (macOS when Swift changed), and a runner has to be picked before the job starts. It runs on a free slim runner for a few seconds.ci:heavy-lanesPR the accessibility, visual and RTL audits still cover only the affected components (spec:AST-030 FR13).component-audit: true, the same action also runs the base'scomponent-audit-scope.cjsand lists the changed files.rtl-shard-scope.mjsandrtl-join.mjs. The join accepts shards that decided their scope and did nothing.visual-scope.mjson the file list.decided.docsite-testvalidates spec records when its own scope is spec-only or undecided, so a failed lookup can't pass the required check unvalidated (FR4).test,buildanddocsite-teststill report on every PR. Thetestjoin also requiresrelease-bumpto pass on a release-branch PR, whichcheck-scopeused to enforce.test-build, which already installs Playwright on the same runner (one job fewer per run).Line count
+1,724 / −1,587, net +137.
decidedpath and the routing tests that pin them are all review findings.Why the scope-job removal is one PR
check-scopefeeds 19 jobs and the required joins, andcheck-componentsfeeds the audit jobs.test-build's existing Playwright runner. Separating it would first mean givingfixture-contrastits own scope step, only to delete it in the next PR.main-heavy-lanes.ymlregistry entry keeps this stack's tests green until the slimmed ci: every workflow declares when it runs, as an allowlist (AST-030 FR14) #8884 removes that registry entirely. At that rebase it's dropped, not kept.Testing
vitest run --project node .github/scripts internal/shadcn-registry internal/lab-readiness: all pass (1,580).docsite-testvalidates when its scope is undecided (the step's own condition);test-scope-action.test.mjsruns the action's real script against a stubbed GitHub API serving this repo's classifier files. It covers:pnpm lab:readiness:checkpasses.