Skip to content

infra[notask]: consolidate per-package CI workflow families into nx-affected workflows - #3903

Draft
sidj-thr wants to merge 12 commits into
pnpm-monorepo-foundationfrom
feature-nx-workflow-consolidation
Draft

infra[notask]: consolidate per-package CI workflow families into nx-affected workflows#3903
sidj-thr wants to merge 12 commits into
pnpm-monorepo-foundationfrom
feature-nx-workflow-consolidation

Conversation

@sidj-thr

Copy link
Copy Markdown
Contributor

🎯 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>.yml or cpp-test-coverage-<pkg>.yml, benchmark-performance-<pkg>.yml, on-pr-<pkg>.yml, and on-pr-close-<pkg>.yml. That is roughly 75 near-identical wrappers whose only real difference is the data passed in with:.

  • 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.ci block on each package's project.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-shas on a real pull_request, otherwise the base-ref/head-ref inputs), nx show projects --affected --json, intersect with the caller's own config table, emit a matrix JSON plus an any flag. Removes 30 to 40 lines of boilerplate from each consumer.

  • .github/actions/nx-project-matrix (new composite): reads options.ci from each project's project.json and folds it into a matrix plus per-feature boolean flags, so the orchestrator does not need per-package branches. Also emits a carveouts output, 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 onto reusable-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 dynamic secrets[...] lookup pattern.

  • cpp-tests-nx.yml: 7 packages, and both former file families, into one job with one flat matrix gated by a per-row mode: standard | coverage discriminator. standard rows run multi-platform with the shared composites and no coverage; coverage rows 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. The pin-model-manifest job gates per step rather than per job, since matrix is not available in a job-level if:.

  • benchmark-performance-nx.yml: 5 packages. Because it sits on top of the consolidated leaves, its prebuild, desktop-benchmarks, and mobile-benchmarks jobs each call prebuilds-nx.yml, integration-test-nx.yml, and integration-mobile-test-nx.yml exactly 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 with jq, avoiding bash associative arrays.

  • on-pr-nx.yml: 14 per-package PR orchestrators into one. Gate chain (fork-approval, ci-router, authorize), nx-project-matrix on the on-pr target, 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 shared combine-perf-reports composite, extended with an optional extra-args input 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 reusable public-delete-npm-versions.yml already loops a space-separated packages list, auto-scopes to pr-<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 a closed event would cost a resolve pass for no functional gain. Legacy behaviour is preserved exactly, including dry-run: true forced on pull_request events so real deletes only happen through manual dispatch, and paths: 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-llamacpp across most families, audiogen-ggml for C++ tests (no C++ tests yet), asr-ggml for desktop integration, tts-ggml for benchmarks, and three retired dispatch-only close stubs. The benchmark-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_request rather than pull_request_target, and the fork-security hardening (reading every project.json and the nx config from the base branch only, never from PR-authored content) lands with that flip. Both are blocked until nx.json, the project.json files, and these workflows exist on the default branch, since base-branch sparse-checkout and git show base: reads resolve to nothing today.

🧪 How was it tested?

  • actionlint clean 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: matrix context referenced in a job-level if:, moved to step-level if:.

  • act -n structural dry-run on each workflow: the matrix job resolves and succeeds; downstream jobs gate on its runtime output and report skipped under dry-run, which is expected.

  • cpp-tests-nx.yml config resolved through yq and jq: 20 rows expand correctly from the anchors, and anchor-only entries with no package key are dropped by the nx-affected select(.package ...) filter, so they never become matrix rows.

  • nx-project-matrix on-pr target 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 --json returns [] for this branch, since only workflow files changed. Confirms the affected filter is wired correctly rather than matching everything.

  • All edited project.json files JSON-validated.

Not yet done, and the reason this stays a draft:

  • Live dispatch and live PR runs of each consolidated workflow, to confirm real-CI parity against the legacy per-package lanes before any legacy file is deleted.
  • Fork-PR verification of the pull_request_target fork-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-affected and nx-project-matrix composites:

  • nrwl/nx-set-shas: new → afb73a62d26e41464e9254689e1fd6122ee683c1 # v5 (SHA resolved from the v5 tag)

Everything else is carried over verbatim from the legacy per-package files being consolidated. No bumps, no repins:

  • actions/checkout: de0fac2e4500dabe0009e67214ff5f5447ce83dd # 6.0.2
  • actions/setup-node: 53b83947a5a98c8d113130e565377fae1a50d02f # 6.3.0
  • actions/cache: 668228422ae6a00e4ad889ee87cd7109ec5666a7 # 5.0.4
  • pnpm/action-setup: a7487c7e89a18df4991f7f222e4898a00d66ddda # v4.1.0
  • aws-actions/configure-aws-credentials: 8df5847569e6427dd6c4fb1cf565c83acfa8afa7 # 6.0.0
  • actions/upload-artifact: bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # 7.0.0
  • actions/download-artifact: 3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # 8.0.1

Flagging one thing for reviewers: integration-mobile-test-nx.yml carries two different actions/upload-artifact pins, ea165f8d65b6e75b540449e92b4886f43607fa02 # 4.6.2 on one step and bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # 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: read
  • benchmark-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.

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)
@github-actions

Copy link
Copy Markdown
Contributor

Workflow security (shadow mode)

zizmor found 1130 finding(s) in .github/ (highest severity: high). This check is warn-only and does not block the merge.

Findings are annotated inline on the changed files and listed in the job summary.

Reproduce locally:

pipx run zizmor==1.27.0 --offline .github/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant