Repository navigation
ci: run installed npm acceptance nightly instead of on PR updates - #15719
Conversation
|
å·ēčŊåŪĄæäšĪ č°åšĶæđåĻįŽĶåčŪūčŪĄïžPR æīæ°č·ģčŋåŪčĢ
éŠæķåäļæļļååįĐéĩïžåŪæķčŋčĄč·ģčŋåįąŧåĻåäļĪäļŠæšį TUI äŧŧåĄïžæåĻčŋčĄäŋįå
ĻéĻäŧŧåĄãååįĐéĩäŧéčŋ S1ïžééŧåĄïžïžåŧščŪŪčĄĨå
åĪéīåĪąčīĨįčŠåĻæĨéã æ°åĒį schedule å°æŠæĨå
Ĩäŧåšį°æį Main CI Failure Issue æĩįĻïžå
ķ workflow_run įåŽåčĄĻ å schedule å
čŪļåčĄĻ é―äļå
åŦ įŽįŦéŠčŊïž
éŠčŊčūđįïžæŽæŽĄæŠæ§čĄå Ļé build/typecheck æįåŪååđģå°åŪčĢ éŠæķïžæčŋäļæŽĄæĨčŊĒæķïžčŋįŦŊ CI äŧæäŧŧåĄčŋčĄäļãåŪčĢ åå―åŧķčŋå°åĪéīåį°įåčå·ēåĻčŪūčŪĄå PR čŊīæäļæįĄŪčŪ°å―ïžäļä―äļšįžšé·éåĪæåšã |
|
æŽå°éŠčŊïžhead
PR č§ĶåéĒåŪæĩïžå―å head į tui-parity runïž
åģïžåäļ head äļåŪčĢ
éŠæķåå
ķååđģå°ååįĐéĩįĄŪåŪčĒŦ PR äšäŧķč·ģčŋïžæšį TUI äļĪæĄč
ŋäļåŋ
é check äŋæįŧŋãåĪéīč·Ŋåūïž äļéįĻéĄđïžæ 䚧å UI / TUI æđåĻïžæ æ æŠåūïžæŠč°åšĶįåŪååđģå°åĪéīčŋčĄïžčŪūčŪĄææĄĢå·ēčŪ°å―čŊĨåčïžã |
|
head æīæ°åįåĪéŠïž äļäļæĄčŊæŪč·åĻ
PR äšäŧķįč·ģčŋčĄäļšäļäļéĒéĢæĄčŊčŪšäļčīïžæŽč―Ūį |
|
Manual-dispatch experiment on this PR head (
Two points worth recording:
No product or test-budget changes were made; this is observation-only evidence for the scheduling change. |
qwen-code-dev-bot
left a comment
qwen-code-dev-bot
left a comment
There was a problem hiding this comment.
Final gate review at head b3bc3ee (re-read immediately before submitting).
- No prior formal reviews or unresolved threads on this PR.
- Required CI per the repository ruleset (
Lint & Static (ubuntu-latest, Node 22.x)): COMPLETED/SUCCESS at this head; only the non-requiredreview-prlane was still queued at submission time. - Independent check found no Critical:
- Producer gating is airtight:
installed-npmruns only onschedule/workflow_dispatch;installed-distributionhas a bareneeds: installed-npmwith noif:override, so a skipped or failed producer skips the four consumers â no distribution run can start without the canonical artifact. - The added
github.event_name != 'schedule'terms onchanges/parity/noflickerare exactly what keeps the fail-closed classifier semantics intact on PR events (a skipped classifier still runs the lanes vianeeds.changes.result != 'success') while nightly runs execute only the installed suite. - Concurrency keeps
cancel-in-progressscoped topull_request, and schedule runs fall to the run-ID group, so PR churn cannot cancel nightly evidence. - The failure reporter admits only
tui-parityschedulefailures onmain(plus the pre-existing push branch); the new meta-test matrix pins success/cancelled/skipped/feature-branch/PR/manual-dispatch as non-filing, and theit.eachextension covers the schedule file-issue route. - The size baseline bump matches the workflow growth; design docs (bilingual pair) document the tradeoff, including that nightly coverage no longer gates merges and high-risk changes need a manual dispatch.
- Producer gating is airtight:
qqqys
left a comment
qqqys
left a comment
There was a problem hiding this comment.
Approving at b3bc3ee932ba404aa7272caa61ec125a14eafda9.
This moves the installed-npm acceptance producer out of the PR path and onto a nightly, and wires nightly failures into the existing issue reporter. The scheduling change has one configuration that nothing in CI can exercise before merge, so that is where I spent the review.
The schedule path is unverifiable by observation here, and it is the one that could silently do nothing. The author's dispatch run (40623408739) proves workflow_dispatch works â all seven jobs green â but on a dispatch changes still runs, because its new guard is github.event_name != 'schedule'. On a nightly tick changes is skipped, which is a different graph, and a producer that depended on it would be skipped too. So I read the job graph at this head rather than extrapolating from the dispatch:
installed-npm(line 248-251) carriesif: "${{ github.event_name == 'schedule' || github.event_name == 'workflow_dispatch' }}"and noneeds:at all. It is therefore reachable on a schedule tick withchangesskipped. This is the load-bearing property of the whole change.installed-distribution(line 301-304) keepsneeds: 'installed-npm'with no customif, so the defaultsuccess()applies: it runs only after the producer succeeds, and onpull_requestâ where the producer'sifis false and the job is skipped â the matrix is skipped with it. That is the intended removal of per-PR cost, not an accidental one, and the failure path is still correct: a red producer does not distribute.parityandnoflickerboth keepneeds: 'changes'and gainedgithub.event_name != 'schedule'inside their existing!cancelled() && (...)expression, so a nightly tick runs only the acceptance chain and leaves the source-built TUI lanes alone.
The new test pins exactly the invariant that matters: expect(job.needs).toBeUndefined(), alongside the precise trigger set (['pull_request','schedule','workflow_dispatch']), the literal cron and changes.if. A future edit that hangs installed-npm off changes would go red instead of quietly emptying the nightly.
The cron arithmetic checks out. 17 19 * * * is 19:17 UTC, which is 03:17 the next day in Asia/Shanghai â matching the comment above it â and the :17 minute does avoid the top-of-hour hosted burst as claimed.
The failure reporter will actually fire on the workflow name it lists. main-ci-failure-issue.yml adds 'tui-parity' both to on.workflow_run.workflows and to the schedule branch's contains(fromJSON('[...]'), github.event.workflow.name) allow-list. That match is by exact string, so I confirmed the producer workflow declares name: 'tui-parity' at line 1 of tui-parity.yml. A mismatch here would have been silent â the nightly would run and fail with no issue ever filed, which is the precise downside of moving a gate out of the PR path.
The test changes extend coverage rather than relax it. The updated if pins are still exact-string toBe assertions, not loosened matchers. The truth table gains eight rows that are mostly negative controls: tui-parity on schedule/main files only for failure, and is explicitly false for success, cancelled, skipped, for a feature branch, and for pull_request and workflow_dispatch events. The one rewritten case became it.each(['workflow_dispatch', 'schedule']) with its three assertions unchanged, so schedule gained coverage instead of dispatch losing it.
On the size ratchet, since the baseline moved and the numbers do not line up exactly: .size-baseline records 22589 tui-parity.yml while the file at this head is 22694 bytes. That is not a violation. check-workflow-size.sh fails only when size > base + GROWTH_ALLOWANCE (4096), i.e. above 26685, and warns on banked slack only when size + 20000 < base; 22694 trips neither. The 105-byte excess is main-side drift folded in by the merge commit, and the script's own file_matches_base logic exists to keep that from red-walling unrelated PRs. Lint & Static, which runs the shell gate, passed at this head.
On the history: the PR has no Review records and no inline threads, so there is no earlier blocking finding to re-verify. @wenshao's round was filed against the first commit 03257a00 with C=0, S=1; the single Suggestion â that the new nightly was not yet wired into Main CI Failure Issue â is what the second commit 43dcb558 implements, and it was non-blocking in any case. That round's verdict predates the merge from main, which is why I re-read the graph at b3bc3ee9 instead of carrying it forward; the merged content is what Test (ubuntu-latest, Node 22.x) validated when it passed in 39m21s, since those workflow tests compare the pinned strings against the real file.
On CI: zero failing checks at this head. Test (ubuntu-latest, Node 22.x), Lint & Static, Integration Tests (no-AK, No Sandbox), web-shell E2E Smoke, and tui-parity's own three jobs â Check TUI impact, TUI parity snapshots, OpenTUI no-flicker gate â all passed, the last three because a PR event resolves the workflow from the PR's own branch. review-pr is still pending and does not gate this.
What this approval does not carry: no actual scheduled run has executed, since the cron only exists after merge. The nightly path is verified statically against the job graph and by the pinned tests, not by an observed tick, so the first 03:17 run is worth a glance to confirm the acceptance chain starts with changes skipped. The tradeoff itself â an installed-npm regression now surfaces up to a day later instead of on the PR â is a deliberate, documented decision recorded in both design files, not a defect.
chiga0
left a comment
chiga0
left a comment
There was a problem hiding this comment.
Scope: CI workflow YAML only (.github/workflows/tui-parity.yml, main-ci-failure-issue.yml), test updates, design docs, size baseline. No product source changed. Tier: Scan.
No blocking findings.
Approval blockers: none.
Checked:
- Workflow name field: tui-parity.yml declares
name: 'tui-parity'â matches the string added to main-ci-failure-issue.yml's workflow_run.workflows list. â - installed-npm guard:
if: "${{ github.event_name == 'schedule' || github.event_name == 'workflow_dispatch' }}"â PR updates skip it as stated. â - changes job guard:
if: "${{ github.event_name != 'schedule' }}"â skipped on nightly runs, runs on PR and manual dispatch. â - parity/noflicker guards: both add
github.event_name != 'schedule'to the existing condition â skipped on nightly. â - installed-distribution still
needs: 'installed-npm'with no independent trigger â it will be skipped whenever installed-npm is skipped (PR updates). â - Concurrency group: keyed on
github.event_name, so PR runs and nightly runs occupy separate groups and cannot cancel each other. cancel-in-progress only fires for pull_request. â - Failure reporter: tui-parity schedule failures on main branch now enter the issue route; PR and workflow_dispatch paths not included (condition requires schedule event for tui-parity). â
- Size baseline updated from 22223 â 22589 (366-byte increase consistent with the diff). â
- Tests in e2e-workflow.test.js and main-ci-failure-issue-workflow.test.js pin all new scheduling conditions and failure-reporter rows. â
Not covered (deliberately inapplicable): no persisted format change â contract-asymmetry class not applicable; no auth/credential change; no product runtime behavior changed â execution rungs not run.
Cross-check: qwen-code-dev-bot approved at head b3bc3ee; wenshao reviewed an older head (03257a0) with C=0 S=1 and no blockers; no findings from either that contradict or extend this review.
Reviewed with AI assistance.
|
Released in v0.26.3. |
What this PR does
Run installed npm/private Bun acceptance and its four-platform distribution matrix nightly at 03:17 Asia/Shanghai (19:17 UTC), or by manual dispatch, instead of on every PR update. Nightly runs skip the source TUI lanes; existing source TUI PR filtering and manual behavior stay unchanged. All product assertions, native hosts, toolchains and canonical artifact dependencies remain intact.
Scheduled failures on main also enter the existing deduplicated CI issue reporter. Repeated test failures reuse their tracker; infrastructure-only failures retain the existing one-rerun policy. PR and manual runs do not enter this new reporting route.
Why it's needed
The installed jobs bypass the existing impact classifier: a one-line Java test change still queued installed acceptance. Five successful runs sampled today had a median producer queue of 67.3 minutes versus 8.8 minutes executing. The producer and four consumers used 90.7â108.5 total runner minutes per run. Broad PR triggers make that cost recur for unrelated changes.
Reviewer Test Plan
How to verify
On a PR update, installed acceptance and its dependent distribution matrix should be skipped; source TUI jobs should retain their existing impact classification and fail-closed behavior.
On a scheduled run, only the installed producer and its successful-producer-dependent matrix should execute. The classifier and source TUI jobs should skip.
A manual dispatch against a selected ref should still run source TUI and installed acceptance. A failed producer must not start distribution consumers without a canonical artifact.
A failed nightly run on main should create or reuse the existing failure issue after normal infrastructure/stale-attempt handling. Success, cancellation, skipping, PR and manual runs should not file through this new route.
Evidence (Before & After)
N/A for product UI. Scheduling regression tests failed in two places against the original workflow, then all 76 workflow tests passed after the change. All 92 runner-routing tests passed. ESLint, formatting, CI budget, workflow size, diff checks and bilingual links passed. Actionlint passed with only the unchanged
macos-15-intelunknown-label diagnostic excluded because the local validator's runner inventory is older. Independent static correctness review found no issues.The reporting follow-up added a failing regression for the missing workflow watch and schedule route; after wiring both, all 287 tests across the failure reporter, CI platform lanes and TUI/E2E workflow suites passed, including existing issue-reuse tests. Reporter actionlint passed without exclusions. No synthetic GitHub failure or issue was created to test reporting.
Tested on
Risk & Scope
Linked Issues
Maintainer-requested follow-up to observed CI queueing; no linked issue.
äļæčŊīæ
æđåĻå åŪđ
npm åŪčĢ åį§æ Bun éŠæķååįŧååđģå°ååįĐéĩæđäļšæŊåĪĐå䚎æķéī 03:17ïžUTC 19:17ïžææåĻč§ĶåïžäļåéæŊæŽĄ PR æīæ°æ§čĄãåĪéīč·ģčŋæšį TUI äŧŧåĄïžį°ææšį TUI į PR čŋæŧĪåæåĻčĄäļšäŋæäļåãå ĻéĻ䚧åæčĻãåįåđģå°ãå·Ĩå ·éūååŊäļ䚧įĐäūčĩäļåã
main åĪéīåĪąčīĨåæķæĨå Ĩå·ēæįåŧé CI issue æĨéæĩįĻãéåĪæĩčŊåĪąčīĨåĪįĻč·čļŠåïžįšŊåšįĄčŪūæ―åĪąčīĨäŋįå·ēæįéĶæŽĄéč·įįĨãPR åæåĻčŋčĄäļčŋå Ĩæ°åĒæĨéč·Ŋåūã
åå
åŪčĢ äŧŧåĄįŧčŋäšį°æå―ąååįąŧåĻïžäŧ äŋŪæđäļčĄ Java æĩčŊį PR äŧįķæéįåū åŪčĢ éŠæķãäŧåĪĐéæ ·įäšæŽĄæåčŋčĄäļïžäš§įĐįæäŧŧåĄæéäļä―æ°äļš 67.3 åéïžčæ§čĄäļä―æ°äŧ 8.8 åéã䚧įĐįæååäļŠæķčīđč æŊč―ŪįīŊčŪĄä―ŋįĻ 90.7â108.5 runner åéãåŪ―æģį PR č§ĶåčåīčŪĐæ å ģæđåĻäđååĪæŋæ čŋäļææŽã
čŊåŪĄéŠčŊčŪĄå
éŠčŊæđåž
PR æīæ°åšč·ģčŋåŪčĢ éŠæķåå ķäūčĩįååįĐéĩïžæšį TUI äŧŧåĄäŋææĒæå―ąååįąŧååįąŧåĪąčīĨäŧæ§čĄįčĄäļšã
åŪæķčŋčĄåšäŧ æ§čĄåŪčĢ 䚧įĐįæäŧŧåĄåäūčĩå ķæåįįĐéĩïžåįąŧåĻåæšį TUI äŧŧåĄč·ģčŋã
æåĻéæĐ ref čŋčĄäŧåšæ§čĄæšį TUI ååŪčĢ éŠæķã䚧įĐįæåĪąčīĨæķïžäļåšåĻįžšå°åŊäļ䚧įĐįæ åĩäļåŊåĻååæķčīđč ã
main åĪéīåĪąčīĨåšåĻæēŋįĻåšįĄčŪūæ―åæ§ attempt åĪįč§ååååŧšæåĪįĻå·ēæåĪąčīĨ issueãæåãåæķãč·ģčŋãPR åæåĻčŋčĄäļåšéčŋæ°åĒč·Ŋåūåŧšåã
ååčŊæŪ
䚧å UI äļéįĻãč°åšĶåå―æĩčŊåĻæ§å·Ĩä―æĩäļæäļĪéĄđåĪąčīĨïžäŋŪæđå 76 éĄđå·Ĩä―æĩæĩčŊå ĻéĻéčŋã92 éĄđ runner č·ŊįąæĩčŊå ĻéĻéčŋãESLintãæ žåžãCI éĒįŪãå·Ĩä―æĩä―į§Ŋãdiff æĢæĨååčŊéūæĨæĢæĨéčŋãActionlint äŧ æéĪæŠäŋŪæđį
macos-15-intelæŠįĨæ įūčŊæåéčŋïžåå æŊæŽæšæ ĄéŠåĻį runner æļ åčūæ§ãįŽįŦéææĢįĄŪæ§čŊåŪĄæŠåį°éŪéĒãæĨéčĄĨå å éčŋåå―æĩčŊįĄŪčŪĪįžšå°įåŽååŪæķč·ŊįąäžåĪąčīĨïžæĨå ĨåïžåĪąčīĨæĨåãCI åđģå°äŧŧåĄå TUI/E2E å·Ĩä―æĩäļäļŠåĨäŧķå ą 287 éĄđæĩčŊéčŋïžå åŦå·ēæ issue åĪįĻæĩčŊãæĨåå·Ĩä―æĩ actionlint æ éæéĪéĄđåģéčŋãæēĄæäļšéŠčŊæĨéčåķé GitHub åĪąčīĨææĩčŊ issueã
æĩčŊåđģå°
éĢéĐäļčåī
å ģčéŪéĒ
įŧīæĪč éåŊđč§åŊå°į CI æéčĶæąčŋčĄįæđčŋïžæ å ģč issueã