infra[notask]: consolidate per-package CI workflow families into nx-affected workflows - #3903
Draft
sidj-thr wants to merge 12 commits into
Draft
infra[notask]: consolidate per-package CI workflow families into nx-affected workflows#3903sidj-thr wants to merge 12 commits into
sidj-thr wants to merge 12 commits into
Conversation
…-mobile-test-nx.yml
Adds the nx-project-matrix composite (reads options.ci from each project.json, folds it into a matrix plus per-feature flags, and emits a carveouts output so bespoke packages route to their own reusables from inside each leaf). Extends combine-perf-reports with an optional extra-args input for the vision addon type, and wires carve-out routing through integration-test-llm-llamacpp.yml.
…w fixes (trailing newline, mobile echo stub)
…onsolidation-combined
…olidation-combined
Contributor
Workflow security (shadow mode)zizmor found 1130 finding(s) in Findings are annotated inline on the changed files and listed in the job summary. Reproduce locally: pipx run zizmor==1.27.0 --offline .github/ |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🎯 What problem does this PR solve?
The CI surface is built out of per-package copies. Every native addon carries its own
prebuilds-<pkg>.yml,integration-test-<pkg>.yml,integration-mobile-test-<pkg>.yml,cpp-tests-<pkg>.ymlorcpp-test-coverage-<pkg>.yml,benchmark-performance-<pkg>.yml,on-pr-<pkg>.yml, andon-pr-close-<pkg>.yml. That is roughly 75 near-identical wrappers whose only real difference is the data passed inwith:.Adding a package, or changing one behaviour, means hand-editing another copy in every family and keeping all of them in sync by hand. Drift between copies is invisible until a lane silently stops testing something.
Nothing in the current setup knows which packages a PR actually touched, so a change to one addon fans out work across lanes that could not possibly be affected.
📝 How does it solve it?
Consolidates each workflow family into a single config-driven, nx-affected workflow. Per-package data moves out of duplicated files and into one config table per family, plus the
options.ciblock on each package'sproject.json..github/actions/nx-affected(new composite): the shared front end for every consolidated workflow. Checkout, setup-node, pnpm install, resolve base/head (nrwl/nx-set-shason a realpull_request, otherwise thebase-ref/head-refinputs),nx show projects --affected --json, intersect with the caller's own config table, emit amatrixJSON plus ananyflag. Removes 30 to 40 lines of boilerplate from each consumer..github/actions/nx-project-matrix(new composite): readsoptions.cifrom each project'sproject.jsonand folds it into a matrix plus per-feature boolean flags, so the orchestrator does not need per-package branches. Also emits acarveoutsoutput, which lets a package with a bespoke shape be routed to its own reusable from inside the relevant leaf rather than special-cased in the parent.prebuilds-nx.yml: 13 packages into one nx-affected matrix. Artifact prefix, linux/mac extra packages, cmake defines, and vulkan/rocm toggles live in one table and map ontoreusable-prebuilds.yml's inputs.integration-mobile-test-nx.yml: 11 packages into a two-dimensional matrix, affected packages by static platform list (Android, iOS). Workdir, npm name, prebuild artifact prefix, Device Farm project and device-pool ARNs, and perf toggles come from the table, with Device Farm ARNs selected through the dynamicsecrets[...]lookup pattern.cpp-tests-nx.yml: 7 packages, and both former file families, into one job with one flat matrix gated by a per-rowmode: standard | coveragediscriminator.standardrows run multi-platform with the shared composites and no coverage;coveragerows run linux-only on the fixed GPU runner with system vcpkg and always-on coverage, including per-package model provisioning (HF plus S3 GGUF, python-venv GGUF conversion) behind config flags. YAML anchors and merge keys keep each package's fixed fields from repeating across its platform rows.integration-test-nx.yml: 10 packages into 65 flattened(package, platform)rows.testCommandSuffix,useTeeLogging,hasGenerateStep,hasPinModelManifest, and env/secret names come from the table. Thepin-model-manifestjob gates per step rather than per job, sincematrixis not available in a job-levelif:.benchmark-performance-nx.yml: 5 packages. Because it sits on top of the consolidated leaves, itsprebuild,desktop-benchmarks, andmobile-benchmarksjobs each callprebuilds-nx.yml,integration-test-nx.yml, andintegration-mobile-test-nx.ymlexactly once and let each self-filter through its own nx-affected pass, instead of calling per-package files in a loop. Summarize metadata is read back withjq, avoiding bash associative arrays.on-pr-nx.yml: 14 per-package PR orchestrators into one. Gate chain (fork-approval, ci-router, authorize),nx-project-matrixon theon-prtarget, the four uniform leaf calls, the flag-gated per-package jobs (sanity-checks, cpp-lint including ROCm, ts-checks, fabric-lockstep, coload-smoke, perf-report), and merge-guard. Perf-report is folded for all four packages that have one: three through the sharedcombine-perf-reportscomposite, extended with an optionalextra-argsinput for the vision addon type, and one through its own RTF aggregator.on-pr-close.yml: 13 per-package close-cleanup files into one, deliberately without nx or a matrix. The reusablepublic-delete-npm-versions.ymlalready loops a space-separatedpackageslist, auto-scopes topr-<n>from the event, and is a no-op for packages with no such version. Deletion is idempotent per package, so adding affected-detection to aclosedevent would cost a resolve pass for no functional gain. Legacy behaviour is preserved exactly, includingdry-run: trueforced onpull_requestevents so real deletes only happen through manual dispatch, andpaths: packages/**scoping.Carve-outs stay on their existing per-package files for now, because each is a genuinely different shape rather than the same shape with different data:
llm-llamacppacross most families,audiogen-ggmlfor C++ tests (no C++ tests yet),asr-ggmlfor desktop integration,tts-ggmlfor benchmarks, and three retired dispatch-only close stubs. Thebenchmark-perf-*files are a separate manual parameter-sweep family and are intentionally untouched.This pass is additive. No legacy per-package workflow is deleted. Removal is a follow-up once real-CI parity is observed on live runs, so that a regression means falling back to a file that still exists rather than a revert.
Deferred follow-up, tracked and intentionally not in this PR: the consolidated workflows currently trigger on
pull_requestrather thanpull_request_target, and the fork-security hardening (reading everyproject.jsonand the nx config from the base branch only, never from PR-authored content) lands with that flip. Both are blocked untilnx.json, theproject.jsonfiles, and these workflows exist on the default branch, since base-branch sparse-checkout andgit show base:reads resolve to nothing today.🧪 How was it tested?
actionlintclean on all 8 added or changed workflow files. No syntax or expression findings. The remaining output is[shellcheck]info, style, and word-splitting findings carried over verbatim from the legacy files being consolidated.One real
[expression]finding was caught and fixed during development:matrixcontext referenced in a job-levelif:, moved to step-levelif:.act -nstructural dry-run on each workflow: thematrixjob resolves and succeeds; downstream jobs gate on its runtime output and reportskippedunder dry-run, which is expected.cpp-tests-nx.ymlconfig resolved throughyqandjq: 20 rows expand correctly from the anchors, and anchor-only entries with nopackagekey are dropped by the nx-affectedselect(.package ...)filter, so they never become matrix rows.nx-project-matrixon-prtarget parity-simmed against the real 14-package fold map. Flags match per package.Carve-out partition sim: the generic matrices are unchanged by carve-out routing. cpp-tests 7 packages and 17 rows, integration-test 10 packages and 65 rows, mobile 11 packages, with carve-outs cleanly separated out.
nx-affected sanity:
nx show projects --affected --base=<base> --head=HEAD --jsonreturns[]for this branch, since only workflow files changed. Confirms the affected filter is wired correctly rather than matching everything.All edited
project.jsonfiles JSON-validated.Not yet done, and the reason this stays a draft:
pull_request_targetfork-security path, which belongs with the deferred follow-up described above.🔐 Action pinning
One third-party action is new to the repo, introduced by the shared
nx-affectedandnx-project-matrixcomposites:nrwl/nx-set-shas: new →afb73a62d26e41464e9254689e1fd6122ee683c1# v5 (SHA resolved from thev5tag)Everything else is carried over verbatim from the legacy per-package files being consolidated. No bumps, no repins:
actions/checkout:de0fac2e4500dabe0009e67214ff5f5447ce83dd# 6.0.2actions/setup-node:53b83947a5a98c8d113130e565377fae1a50d02f# 6.3.0actions/cache:668228422ae6a00e4ad889ee87cd7109ec5666a7# 5.0.4pnpm/action-setup:a7487c7e89a18df4991f7f222e4898a00d66ddda# v4.1.0aws-actions/configure-aws-credentials:8df5847569e6427dd6c4fb1cf565c83acfa8afa7# 6.0.0actions/upload-artifact:bbbca2ddaa5d8feaa63e36b76fdaad77386f024f# 7.0.0actions/download-artifact:3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c# 8.0.1Flagging one thing for reviewers:
integration-mobile-test-nx.ymlcarries two differentactions/upload-artifactpins,ea165f8d65b6e75b540449e92b4886f43607fa02# 4.6.2 on one step andbbbca2ddaa5d8feaa63e36b76fdaad77386f024f# 7.0.0 on another. Both are inherited as-is from the legacy per-package files, so this PR does not change behaviour, but it does now make the inconsistency visible in a single file. Happy to unify to 7.0.0 in this PR if preferred, or leave it for the legacy-removal pass.🛡️ Permissions changes
New workflow files, so each declares its own top-level
permissions:block. Every one is copied from the legacy per-package files it replaces, and each is least-privilege for what that lane does.prebuilds-nx.yml,cpp-tests-nx.yml,integration-test-nx.yml,integration-mobile-test-nx.yml,on-pr-close.yml:contents: readbenchmark-performance-nx.yml:contents: read,packages: read,id-token: write(OIDC for the AWS role assumption the benchmark lane already used)on-pr-nx.yml:contents: read,pull-requests: read,packages: read,checks: read(gate chain reads PR state and merge-guard reads check results)No existing workflow's permissions were widened.