Repository navigation
ci: an agent lane's push runs the fast checks; the heavy suites run when ready and before merge (AST-030 FR20) - #9200
Conversation
…hen ready and before merge (AST-030 FR20, DEC-14)
|
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.
|
|
/approve-spec df685d0 |
PR Analysis Report🔎 Preview ·
|
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Requesting changes: the deferral itself follows the owner-approved FR20 and DEC-14, but a deferred push leaves one misleading failure and two smaller issues to fix.
test-native-swiftstarts on every deferred push and fails with "The Swift runner could not be decided" (ci.yml:788-800); guard it with the deferral and add it to the suite'sHEAVYlist.- FR20, CONTRIBUTING and the description say a deferred push runs "lint and typecheck only", but path-filtered workflows such as the CLI smoke test still run; scope that wording to ci.yml.
- The shared agent login is hard-coded in both
heavy-caps/action.ymlandHEAVY_CANDIDATE; keep one list so the caps and the deferral cannot drift.
[Automated review]
| # every test job; anything undecidable picks macOS. | ||
| native-swift-runner: | ||
| if: ${{ (github.event_name != 'pull_request' || !github.event.pull_request.draft && (github.event.pull_request.head.repo.full_name == github.repository || contains(github.event.pull_request.labels.*.name, 'ci:run'))) && (github.event_name != 'workflow_dispatch') }} | ||
| needs: [heavy-ci] |
There was a problem hiding this comment.
Deferring native-swift-runner leaves test-native-swift (it needs this job under always() && !cancelled()) to start on every deferred push and fail at "Require a decided Swift runner" with "The Swift runner could not be decided; refusing to skip required work." Give test-native-swift needs: [heavy-ci, native-swift-runner] and the same needs.heavy-ci.outputs.deferred != 'true' guard, and add it to HEAVY in ci-heavy-deferral.test.mjs.
| */ | ||
| export const HEAVY_CANDIDATE = | ||
| "(github.event_name == 'pull_request' && github.event.action == 'synchronize' && " + | ||
| "github.run_attempt == '1' && github.event.pull_request.user.login == 'cixzhang' && " + |
There was a problem hiding this comment.
The shared agent login is already listed in .github/actions/heavy-caps/action.yml:35 (['cixzhang'].includes(login)). Keep one list, for example an exported SHARED_AGENT_LOGINS used here through contains(fromJSON(...), github.event.pull_request.user.login), and assert in heavy-caps-action.test.mjs that the action's list matches, so the caps and the deferral cannot drift.
| shared agent login, into `main`, in its run's first attempt, runs lint and | ||
| typecheck only, unless its trusted classification is spec-only. Its heavy jobs |
There was a problem hiding this comment.
"Runs lint and typecheck only" is broader than the change: the deferral gates ci.yml alone, so cli-smoke-test.yml, internal-registry.yml, codemod-verify.yml and crowdin-download.yml still run when their paths change (and test-native-swift starts today). Consider "ci.yml runs only typecheck (lint keeps its own workflow); the path-filtered workflows (FR14) are unaffected", and the same in CONTRIBUTING.
…he FR20 wording to ci.yml
github-actions
Bot
added
the
needs:spec-owner-review
|
/approve-spec b28ef00 |
github-actions
Bot
removed
the
needs:spec-owner-review
There was a problem hiding this comment.
Approval recommended: the deferral follows FR20 and DEC-14, which an engineering owner approved at this head, and the follow-up commit resolves the earlier test-native-swift, scope-wording and login-list points. A deferred agent push now fails only test, build and docsite-test, each with the "Heavy CI deferred" reason, and every other kind of event runs the same jobs as before. Before merging an agent pull request, use Re-run all jobs (or close and reopen it); a failed-jobs-only re-run keeps the deferral and fails again.
[Automated review]
github-actions
Bot
added
the
needs:spec-owner-review
|
/approve-spec a0555a2 |
github-actions
Bot
removed
the
needs:spec-owner-review
There was a problem hiding this comment.
Approval recommended: the deferral follows FR20 and DEC-14, which an engineering owner approved at this head, and merging main in left the deferral and the report message intact. A deferred agent push fails only test, build and docsite-test, each with the "Heavy CI deferred" reason, and every other kind of event runs the same CI jobs as before. Next, mark the pull request ready so its full CI runs at this head; when merging agent pull requests later, use Re-run all jobs (or close and reopen), because a failed-jobs-only re-run keeps the deferral and fails again.
[Automated review]
There was a problem hiding this comment.
Changes requested: the deferral itself follows FR20 and DEC-14, which an engineering owner approved at this head, but the branch no longer merges into main. main has since removed the size-base job that this change guards, so GitHub reports a conflict, and no CI started when the pull request was marked ready. When you merge main, drop the size-base job and its guard and remove 'size-base' from HEAVY in ci-heavy-deferral.test.mjs (otherwise 9 of its 19 cases fail), and mention the spec-only exemption in the CONTRIBUTING bullet; the new head then needs a fresh /approve-spec.
[Automated review]
| needs: [heavy-ci] | ||
| if: ${{ (github.event_name != 'pull_request' || !github.event.pull_request.draft && (github.event.pull_request.head.repo.full_name == github.repository || contains(github.event.pull_request.labels.*.name, 'ci:run'))) && (!cancelled() && needs.heavy-ci.outputs.deferred != 'true' && (github.event_name == 'pull_request')) }} |
There was a problem hiding this comment.
main has since deleted this whole size-base job (#9164), so the branch conflicts here. Keep main's deletion when you merge it, and drop 'size-base' from HEAVY in .github/scripts/ci-heavy-deferral.test.mjs with it.
| 'native-swift-runner', | ||
| 'test-native-swift', | ||
| 'run-labels', | ||
| 'size-base', |
There was a problem hiding this comment.
Once main is merged in, ci.jobs['size-base'] is undefined and 9 of this suite's 19 cases fail with "Cannot read properties of undefined (reading 'needs')". Remove this entry when you resolve the conflict.
| - **Pull requests from the shared agent login run the heavy suites when ready | ||
| and before merge.** A push to a ready agent pull request runs lint and, in | ||
| the CI workflow, only typecheck (path-filtered workflows run as usual); | ||
| `test`, `build` and `docsite-test` fail with "Heavy CI deferred" | ||
| until the pull request is marked ready or reopened, or its latest run is | ||
| re-run with **Re-run all jobs**, which runs everything at that head. |
There was a problem hiding this comment.
FR20 exempts a change whose trusted classification is spec-only; such a push runs everything and its checks do not fail. Worth saying here, for example "... fail with "Heavy CI deferred" (unless the change is spec-only) until ...".
|
/approve-spec 05edf15 |
There was a problem hiding this comment.
Changes requested: the deferral follows FR20 and DEC-14, which an engineering owner approved at this head, and merging main resolved the earlier conflict, but the description is missing the CI impact: line that AST-030 FR19 requires of a pull request that changes CI. Add that line to the description, including time to green (a deferred push stays red until the ready or pre-merge run) and the change in average concurrent jobs; no new commit is needed, so the spec approval stands. While editing, drop size-base from the description's heavy-job list, since main removed that job.
[Automated review]
|
/approve-spec 1c171f7 |
There was a problem hiding this comment.
Changes requested: the deferral follows FR20 and DEC-14, which an engineering owner approved at this head, but making build-storybook and run-labels need heavy-ci puts a job that skips on every release-check and every non-agent run into the dependency chain of pr-a11y-shard, a11y-contracts-shard, pr-a11y-guards, pr-a11y-specs and pr-rtl-shard, whose conditions have no status check, so GitHub skips them and every release-check fails at pr-a11y-audit (AST-030 FR12, FR13). Give those five jobs a status-check condition (for example !cancelled() && needs.build-storybook.result == 'success' && …), and make ci-heavy-deferral.test.mjs read every ancestor rather than only direct needs, with a release-check case and a ci:run-a11y/ci:run-rtl case. A smaller fix for the same pass: a failed-jobs-only re-run of a deferred run reports an unavailable analysis instead of the deferral, because only heavy-ci, which that re-run does not repeat, uploads heavy-ci-deferred-<attempt>.
[Automated review]
| build-storybook: | ||
| needs: [admission] | ||
| if: ${{ (github.event_name != 'pull_request' || !github.event.pull_request.draft && (github.event.pull_request.head.repo.full_name == github.repository || contains(github.event.pull_request.labels.*.name, 'ci:run'))) && ((github.event_name != 'workflow_dispatch' || inputs.operation == 'release-check') && always() && !cancelled()) }} | ||
| needs: [admission, heavy-ci] |
There was a problem hiding this comment.
Needing heavy-ci here (and in run-labels at line 1000) puts a job that skips on every release-check and every non-agent run into the dependency chain of pr-a11y-shard, a11y-contracts-shard, pr-a11y-guards, pr-a11y-specs and pr-rtl-shard. Their conditions have no status-check function, so GitHub's default success() reads the whole chain and skips them; pr-a11y-audit then fails with "Accessibility audit shards did not all succeed: skipped" and release-check fails (AST-030 FR12, FR13). Give those five jobs a status-check condition, for example !cancelled() && needs.build-storybook.result == 'success' && (...), adding needs.run-labels.result == 'success' where they need it.
| const needed = [jobNeeds].flat().map(id => needs[id] ?? {result: 'success'}); | ||
| const succeeded = needed.every(need => need.result === 'success'); |
There was a problem hiding this comment.
This reads only the job's direct needs; GitHub's default success() also fails when any ancestor was skipped. With every ancestor counted, a release-check dispatch and a ci:run-a11y/ci:run-rtl pull request lose five owners on this head. Please model the whole chain and add those two cases.
| if: steps.identity.outputs.valid == 'true' && steps.identity.outputs.source_conclusion == 'failure' | ||
| uses: actions/download-artifact@v8 | ||
| with: | ||
| name: heavy-ci-deferred-${{ steps.identity.outputs.run_attempt }} |
There was a problem hiding this comment.
On Re-run failed jobs, heavy-ci (which succeeded) is not repeated, so no heavy-ci-deferred-2 exists while test, build and docsite-test fail again with "Heavy CI deferred"; the report then says the analysis is unavailable. Uploading the marker from the required contexts' deferral step, which every still-deferred attempt runs, keeps the message.
…; jobs below heavy-ci carry status checks; the deferral record moves to test
… failed-jobs re-run cases
|
/approve-spec e054d22 |
There was a problem hiding this comment.
Changes requested: the deferral follows FR20 and DEC-14, which an engineering owner approved at this head, and the release-check and failed-jobs re-run issues from the last review are fixed. One wiring problem remains: HEAVY_RUNS uses !cancelled(), which also drops the implicit success check on admission, so when CI is full test-ui, test-node, native-swift-runner, run-labels and test-build start instead of stopping (a fork run with ci:run, which cannot cancel itself, runs them to completion), against AST-030 FR16. Requiring needs.admission.result == 'success' in those five jobs and adding a CI-full case to ci-heavy-deferral.test.mjs would fix it; the new commit will need a fresh /approve-spec.
[Automated review]
| export const HEAVY_RUNS = | ||
| "!cancelled() && needs.heavy-ci.outputs.deferred != 'true'"; |
There was a problem hiding this comment.
!cancelled() also drops Actions' implicit success() over admission. When room reports CI full, admission fails at its first step; on the base test-ui, test-node, native-swift-runner, run-labels and test-build then skip, but with this guard they start (a fork run, which cannot cancel itself, runs them to completion), against AST-030 FR16. Adding needs.admission.result == 'success' for the jobs that need admission keeps heavy-ci's skip harmless and restores the stop; a CI-full case in ci-heavy-deferral.test.mjs would hold it.
…hem (AST-030 FR16)
|
/approve-spec a325536 |
There was a problem hiding this comment.
Approval recommended: the deferral follows FR20 and DEC-14, which an engineering owner approved at this head, and requiring a successful admission in test-ui, test-node, native-swift-runner, run-labels and test-build restores the CI-full stop, with a new test case that holds it. Before merging an agent pull request, use Re-run all jobs (or close and reopen it), because a failed-jobs-only re-run keeps the deferral and fails again. The description could also be corrected: the new suite has 24 cases, the deferral step comes after the CI-full stop, and docsite-test already had !cancelled().
[Automated review]
Maintainer impact
test,buildanddocsite-testchecks fail with "Heavy CI deferred: runs when marked ready and before merge", so nothing can merge on that run. The PR report says the same.main, release checks.Authority
The CI owner, 2026-10-31 12:17: "CI 1 yes" (end-state decision 1).
spec:AST-030gains FR20 and DEC-14, plus a Verification row and a CONTRIBUTING line. That needs an owner's/approve-specat this head.Change
ci.yml:heavy-cijob (ubuntu-slim) starts only for an agent-lanesynchronizein attempt 1 intomain. It reads the trusted scope and outputsdeferred, which is true unless the change is spec-only. Every other push skips it, and nothing waits on it.heavy-ci(beside CI'sroom/admission) and run under!cancelled() && needs.heavy-ci.outputs.deferred != 'true'.test,buildanddocsite-testneedheavy-ci. Their second step (after the fork gate) fails with the reason when the push is deferred.buildalso starts when deferred, since its Storybook build skipped.docsite-testadds!cancelled(), so a skippedheavy-cinever skips it.heavy-ciuploadsheavy-ci-deferred-<attempt>.pr-comment.yml: for a failed source run, it downloads that attempt's deferral and reports "Heavy CI deferred: runs when marked ready and before merge. Lint, typecheck and any path-filtered checks ran on this push." in place of an unavailable analysis.ci-pr-gate.mjs:SHARED_AGENT_LOGINS(the same list heavy-caps uses; a test holds them equal),HEAVY_CANDIDATE,HEAVY_DEFERRED,HEAVY_RUNSand the message.ungated()also strips the deferral wrapper, so the existing routing tests still read each job's own condition.Pre-merge step (for the merge tooling)
Re-run all jobs of the latest CI run (
gh run rerun <id>, without--failed) through the CI slot, or close and reopen the pull request. A failed-jobs-only re-run would re-run the joins without their skipped lanes, and the joins then fail (fail-closed), so it cannot merge either.heavy-ciskips on every run but an agent push, and Actions' implicit success check needs every ancestor to have succeeded. So the five jobs below the Storybook build that had no status check of their own (pr-a11y-shard,a11y-contracts-shard,pr-a11y-guards,pr-a11y-specs,pr-rtl-shard) now require their direct needs to have succeeded instead. A contract test checks every job below the decision, and a release-check and an opt-in label run are simulated with ancestors included.heavy-citotest, which a failed-jobs re-run repeats. That attempt then has its own record and still says "Heavy CI deferred"; a full re-run never defers.heavy-cineeds onlyroom, beside admission, so the Storybook build it gates still starts as soon as there is room (FR16). It also stays out when CI is full.heavy-ci, on a standard GitHub-hosted slim runner (about half a job-minute), only on an agent-lane push to a ready pull request. On a deferred push, job-minutes per update drop from about 45 to about 3 (lint, typecheck andheavy-ci). The measured replay is about 124 deferred pushes a day: about 5,300 fewer runner-minutes a day, about 3.7 fewer concurrent jobs on average. Every other update is unchanged. Time to green: a deferred push does not turn green;test,buildanddocsite-testfail with the reason until the ready or pre-merge run, which takes the usual time (about 16 minutes).Evidence
ci-heavy-deferral.test.mjs(new, 18 cases) evaluates every affected job and step condition as Actions does.build's condition, from a heavy job, or removingdocsite-test's deferral step each fails the suite..github/scriptssuite: all pass, except four environment-only failures that also fail onmainhere: Chromium launch, and analyze-pr timeouts.Scope
Writing rubric results